RestSharp 107 migration

[feinoujc]

Wondering if there are plans to upgrade this library to use RestSharp 107? It has some pretty important internals changes that make its use more usable in dotnet core but also looks like it's a pretty major update

[CraigHawker]

This isn't something that we've actively considered, but that's largely as this library was initially designed as a training aid and has no official support or proactive maintenance.

That said, it's an important question and deserves some thought.

The breaking changes are important to note, but it's unclear at this time whether those changes would be transparent in the library or whether they would require updates to method signatures used by consumers.

If you, or anyone else, has any further input on this (for or against a potentially breaking update) then please do comment on this issue.

[OzBob]

https://github.com/restsharp/RestSharp/releases

107.0.0 now does not reference Newtonsoft (again). Consistent Newtonsoft package references across projects is gone again.
"Default JSON serializer is now using System.Text.Json, SimpleJson is gone" a
and
"Support for .NET 4.5 is removed, for legacy .NET Framework use the .NET Standard package"

which had some CamelCase issues restsharp/RestSharp#1719, but also hints at being able to configure which Json library to use client.UseNewtonsoftJson()

[CraigHawker]

Thank you @OzBob for your comments.

The serializer change is important to note, as I know we've had some issues with datetimes in JSON before. It would need thorough testing to make sure that it works the same way. The use of Newtonsoft (or the switch to something else, to be more specific) isn't something that I think we are concerned about at a library level as this should have been transparent to users anyway; the only practical change is that if anyone is also using Newtonsoft in their library for something else then they can now easily upgrade.

Explicit support for 4.5 being removed is also something to be aware of, but we also have to be aware that 4.5 is old now (a phrase which I know someone will use against me by highlighting that the VAF defaults to creating 4.5 assemblies!). The only real fallback strategy here would be to keep the current codebase as a "4.5 version", and keep the current 4.5-compatible nuget package. After this work were done we'd effectively mark this as legacy and promote that they upgrade to 4.7.2 to be able to use the .NET Standard 2.0 release.

Again: more feedback welcomed.

[feinoujc]

I tried briefly to upgrade to see what would be involved, a couple of notes in addition to @OzBob 's

• The removal of the interfaces in 107 will be something that needs to be addressed, especially with the way the test runner is written. We may need to add our own interfaces back
• It'd be good to add an constructor to the client that allows providing their own HttpClient. This will allow consumers to integrate with the dotnet core DI / HttpClientFactory if they choose.
• This is more of an opinion: Since we are clearly in a breaking change situation, consider removal of all "Sync over Async" operations, which is generally discouraged

Let me know if there is any parts of this I can assist with, once we have a path forward

[feinoujc]

It'd be good to add an constructor to the client that allows providing their own HttpClient. This will allow consumers to integrate with the dotnet core DI / HttpClientFactory if they choose.

This is also apparently a very important step to being able to use this library in Blazor . I'm not familiar with this myself, but maybe others will find this important

[dmusic]

I think it is worth noting that authentication specifics are moved away from RestRequest to RestClientOptions class which is used during initialization of RestClient (see this). I've tried to port some parts of this code with RestSharp 107.x used in .NET 6 Web API but facing problems with authentication over SSO.

[CraigHawker]

I @dmusic - thank you for the additional feedback. If the authentication issue is this one then my suspicion is elsewhere rather than this library itself. It could be checked by creating a .NET 6 console application and using the library from there.

I am going to look to add this to my list of stuff for 2022Q2. I'm not 100% sure that I'll be able to get this into that timeframe (I already have a list of 10 other things, and some of them are quite large), but let's try and aim for that.

@feinoujc - I'd definitely be glad of some assistance. There are some decisions to be made here (removal of sync methods, for example) so I'd like to reach out to the group at the appropriate time and see what the consensus is. I think my general guidance is that we can/should accept some breaking changes where needed, but we should also consider the workload involved in upgrades; if there are ways to work around these breaking changes from a consumer's perspective then we should try to do so.

[CraigHawker]

This can be tested using version 2.0.1-issue-20 in nuget: https://www.nuget.org/packages/MFaaP.MFWSClient/2.0.1-issue-20

[CraigHawker]

Proposed additional changes:

• Move *Sync methods to separate package (only if we can auto-generate them?)
• Move to CalVer versioning, as with everything that M-Files is currently producing
• Add CICD pipeline for more consistent releases

[CraigHawker]

Version 2.0.2 adds explicit dotnet 7.0 support.

[CraigHawker]

This has been merged in 1041471.

[CraigHawker]

These changes are in https://www.nuget.org/packages/MFaaP.MFWSClient/23.10.0.15-prerelease (currently being validated by nuget).