Client csharp: Use System.Net.Http.HttpClient instead of RestSharp #1889
Comments
|
@carlosduclos that's a good idea. The Java API client already supports different HTTP libraries, e.g. Jersey1.x, Jersey2.x, okhttp, Retrofit, Retrofit2, etc, via the Let me know if you need help adding HttpClient to the existing CSharp client. |
|
I'm thinking of something similar by using reflection and loading the available library. Having a public static method that creates the ApiClient object. I'll prepare a short sample and I'll post it here. |
|
Here goes a half backed implementation using reflection. The functionality is divided into two classes, one frontend class that presents the methods the user will need and a backend class that does the heavy lifting. Api.cs (to be transformed to a mustache template of course) using System;
using System.Collections;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Reflection;
using System.Text;
using System.Threading.Tasks;
using System.Web;
namespace SwaggerCodegenDemo
{
/// <summary>
/// API client is mainly responible for making the HTTP call to the API backend.
/// </summary>
public class ApiClient
{
/// <summary>
/// Different types of providers. Required for reflection usage only.
/// </summary>
public enum Providers
{
HttpClient,
RestSharp
};
static Dictionary<Providers, string> providers;
static ApiClient()
{
providers = new Dictionary<Providers, string>();
providers.Add(ApiClient.Providers.HttpClient, "httpClient");
providers.Add(ApiClient.Providers.RestSharp, "restSharp");
}
protected Configuration _configuration;
protected ApiClientAbstractProvider _provider;
/// <summary>
/// Initializes a new instance of the <see cref="ApiClient" /> class
/// using the provided configuration and base path ({{basePath}}).
/// If no configuration is provided, then the default configuration is used.
/// </summary>
/// <param name="provider">Communication provider</param>
/// <param name="config">Configuration</param>
public ApiClient(ApiClientAbstractProvider provider, Configuration config = Configuration.Default)
{
_configuration = Configuration.Default;
_provider = provider;
}
/// <summary>
/// Creates a new ApiClient based on the specified provider.
/// </summary>
/// <param name="providerPath">Path to the dll implementing the abstract provider.</param>
/// <param name="providerName">Name of the provider class.</param>
/// <param name="config">Configuration to be used</param>
public static ApiClient Create(string providerPath, Providers providerType, object[] arguments, Configuration config = Configuration.Default)
{
ApiClientAbstractProvider provider = null;
// Not handling exceptions so it crashes in case something goes wrong with reflection
Assembly assembly = Assembly.Load(providerPath);
string providerName = String.Empty;
if (providers.TryGetValue(providerType, out providerName))
{
Type type = assembly.GetType(providerName);
if (providerType == Providers.HttpClient)
{
ConstructorInfo cinfo = type.GetConstructor(new[] { /* constructor parameters */ });
provider = (ApiClientAbstractProvider)cinfo.Invoke(arguments);
}
else if (providerType == Providers.RestSharp)
{
ConstructorInfo cinfo = type.GetConstructor(new[] { /* constructor parameters */ });
provider = (ApiClientAbstractProvider)cinfo.Invoke(arguments);
}
else
{
throw new NotImplementedException("Provider is not implemented");
}
}
else
{
throw new NotSupportedException("Provider is not supported");
}
return new ApiClient(provider, config);
}
/// <summary>
/// Gets the Configuration.
/// </summary>
/// <value>An instance of the Configuration.</value>
public Configuration Configuration { get; }
public async Task<Object> GetAsync(/* some parameters */)
{
return await _provider.GetAsync(/* some parameters */);
}
public async Task<Object> PostAsync(/* some parameters */)
{
return await _provider.GetAsync(/* some parameters */);
}
public async Task<Object> PutAsync(/* some parameters */)
{
return await _provider.GetAsync(/* some parameters */);
}
public async Task<Object> DeleteAsync(/* some parameters */)
{
return await _provider.GetAsync(/* some parameters */);
}
public async Task<Object> HeadAsync(/* some parameters */)
{
return await _provider.GetAsync(/* some parameters */);
}
}
}ApiAbstractProvider.cs (to be transformed to a mustache template too) using System;
using System.Collections.Generic;
using System.IO;
using System.Linq;
using System.Net;
using System.Net.Http;
using System.Text;
using System.Threading.Tasks;
using RestSharp;
namespace SwaggerCodegenDemo
{
/// <summary>
/// API client provider is mainly responsible for implementing the communication between
/// the client and the server.
/// All the logic lives here
/// </summary>
public abstract class ApiClientAbstractProvider
{
protected string _basePath;
public ApiClientAbstractProvider(string basePath = "")
{
_basePath = basePath;
}
public async Task<Object> GetAsync(/* some parameters */)
{
return await Task.FromResult<Object>(null);
}
public async Task<Object> PostAsync(/* some parameters */)
{
return await Task.FromResult<Object>(null);
}
public async Task<Object> PutAsync(/* some parameters */)
{
return await Task.FromResult<Object>(null);
}
public async Task<Object> DeleteAsync(/* some parameters */)
{
return await Task.FromResult<Object>(null);
}
public async Task<Object> HeadAsync(/* some parameters */)
{
return await Task.FromResult<Object>(null);
}
}
} |
|
I have a couple of questions, where do I get the "library" parameter that I specify in the command line (I would assume that it ends up in the Configuration class)? |
|
Hi @carlosduclos sorry for not getting back to you on your suggestion 5 days ago :( I'll take a look tonight and reply back. |
|
The 'library' option can be obtained via CliOption: and later you can use |
|
From first glance, I don't see any issue with your approach using an abstract base class. Java supports multiple HTTP libraries (Retrofit, Jersey, etc) but does not use any abstract base class. Its approach is to generate the ApiClient with hard-coded HTTP library and I think it worths your time to take a look. About "writing a couple of small nugets", can you give an example to illustrate its purpose? |
|
@carlosduclos Your |
|
@wing328 For example, we can have two nugets:
That way the user can compile the code without any extra libraries and specify at runtime which provider will be used. The nugets are created just to pack the dlls in this case, and changing the dll should suffice to change the provider. @jimschubert Sure, the methods listed there were just an example. Sorry for the delay in replying! |
|
@carlosduclos do you need any help with the work? do you mind sharing the progress so far? Thanks in advance. |
|
@carlosduclos Any progress with this? |
|
@andy1547 Sorry, I had to abandon this because of change of priorities at work. I won't have time to work on this so please close this. |
|
For all of those who need a real architectural design with DI, OWIN (for testing with in-memory servers) and a fast HttpClient compared to RestSharp you can use NSwagg which is really good for C# code generation of Swagger API clients. https://github.com/NSwag/NSwag Until someone eventually gets a real template up for swagger-codegen .Net from the Java world... ;) Best Regards |
|
Has anyone made progress on this? I consider the C# codegen to pretty much be useless until this is resolved. |
|
Please checkout 2.3.0 branch, which has the C# API client refactored and it now supports another HTTP library: RESTSharp.portable (https://www.nuget.org/packages/FubarCoder.RestSharp.Portable/) And what issues did you run into with C# API client using RestSharp? |
|
First one, RestSharp is outdated, hasn't been updating since 2015. Also I'm fail to see its purpose in the codegen when HttpClient exists and we can reduce the number of dependencies. Also, RestSharp isn't configured as a .NET Standard so can't be used in .NET Core apps. RestSharp.portable also isn't setup for .NET Standard. |
|
@gehnster according to https://github.com/FubarDevelopment/restsharp.portable#supported-platforms, restsharp.portable supports .NET core. What error did you get when using the auto-generated C# SDK with .NET core? |
|
When I take the auto-generated C# code from swagger-codegen and place it in a .NET Standard project, when I add RestSharp.Portable via NuGet it gives warning If I brought the project down from 2.0 to 1.3, which the NuGet page for FubarCoder.RestSharp.Portable.Core says it supports, it instead gives me an error saying it doesn't support .NET Standard 1.3. Also, there is this open task in RestSharp.Portable from the owner of the project. FubarDevelopment/restsharp.portable#99 |
|
cc @Gronsak @jimschubert to see if they've better clue on why it's not working for .NET standard 1.3 |
|
See: https://github.com/dotnet/standard/blob/master/docs/versions.md . NET Core 2.0 is .NET Standard 2.0. The C# client only supports .NET Standard 1.3. I've compiled it successfully against supported .NET Framework and Mono versions per the linked doc. I haven't compiled it against .NET Core 1.0. I'm 100% in support of moving to HttpClient only. I've discussed this before with other maintainers, and it will be a breaking change because we've exposed the RestSharp API Client and other implementation details outside of the generated client. My work in 2.3.0 to refactor the client code was done to reduce that exposed API, and my end goal was to later implement HttpClient. I've been pretty busy for the last month or so, with some work and family things going on. When I can get some free time, I can prioritize this work. I think it would make the client code a lot easier to be maintain. |
|
I've only had a look at the specs for .NET Core 2.0 since I've been quite busy with school and work the last few months so I can't really tell what's going on past what Jim already said. And just to add my voice to the mix, I also fully support moving to HttpClient aswell, I agree that it would probably make everything easier to maintain. |
|
Furthermore if doing integration test with a aspNetCore2.0 project, utilizing the swagger generated client code, you would do e.g. HttpClient client = new TestServer(new WebHostBuilder().UseStartup<Startup>()).CreateClient();and then this instance would need to be injected when creating a new swagger-generated API client instance. |
|
According to this issue: FubarDevelopment/restsharp.portable#99, RestSharp Portable is no longer being maintained. I've recently attempted to upgrade a project from using 4.5 templates to .NET Standard templates and found RestSharp Portable to be full of issues. |
|
@alexl0gan I anticipated restsharp.portable losing efforts, but it solved a need when it was introduced and provides support for those who need it in older versions of Swagger Codegen. I've been putting off the refactoring effort to support different http libs, because there's been so much churn on the C# generator with respect to target framework, versions, and mixing in the whole concept of .NET Standard versioning. With version 3.0 coming up, I think it's time to fully refactor and support other libraries optionally. I am thinking to support standard http client and Polly in .NET 4.x+ (and .NET Standard), and maybe RestSharp and standard HTTP client in earlier targeting frameworks and C# versions. Any thoughts on this, or recommendations for other libs to support? |
|
The latest version of Restsharp supports .NET Standard 2.0: https://github.com/restsharp/RestSharp#features |
|
My solution was to go back to using the 4.5 templates and use RestSharp 106. So far this is working fine. |
|
@wing328 Are there benefits to RestSharp that would warrant supporting it in newer client versions? I know we don't do telemetry or anything, but is there a feel for how many devs require RestSharp via the exposed The main reason for my suggestion is that RestSharp is an additional library, meaning it may cause conflicts when pulled into applications that already rely on RestSharp elsewhere. My plan is to create an interface similar to the one Carlos has above, but which exposes the per-request and global configuration stuff from my last refactor. Then, I'd create library templates which implement that base type for each of the supported libraries. This obviously wouldn't be difficult to support RestSharp as well, but my concern is that the differences between 3.x and 4.x/.NET Standard would increase the maintenance effort. So is the suggestion to do: .NET 3.5
.NET 4.x+
There are other conditional concerns that would need to be addressed as well, like… if you're targeting a platform without the full FCL, do we change from RestSharp.Portable back to the built-in HttpClient and warn the user that support has been dropped and that RestSharp doesn't support the platform? If we do this, we may want to create a wiki page specific to C# client templating and migrations from 2.x generators. |
|
From my point of view the real problem of C# generator is your ignoring SOLID principles. That's what leads to your problem of breaking backward compatibility due to exposed too much (forgotten encapsulation). Global objects, default configuration, implicite dependencies, side effects... Why? Do you have example of using the code generated in a service with proper straight forward inversion of control through dependency injection? I failed to implement it spending a day. |
|
@Gladskih Not to be pedantic, but SOLID principles don't prevent the issues you've listed. You can have a system that follows SOLID and still exposes static types and internals that should be encapsulated . ;) You're right, though, that there is opportunity for improvement. I'll explain some concerns and considerations here. I've noted the question about the example (there's an open issue for this as well). If you have any particular feature requests, let me know and I'll try to work it into this refactor. The main problem with the generator is that as requests are made and implemented for one consumer, it's fairly difficult to remove that feature once released. We don't do telemetry, so we don't know the impact of huge reactors and tend to avoid many breaking changes in minor versions. In my previous refactor, I removed race conditions in the use of the non-thread-safe In my current in-progress refactor, I'm not going to be as concerned about maintaining a consistent API for the internal ApiClient type. My plan is to have a synchronous client interface and an asynchronous interface to facilitate IoC. I then plan to implement these interfaces using the libs listed above With this design, it will allow consumers to implement this interface and inject their own client accessors. This is related to interface segregation and dependency inversion. Also, I plan to document in the README how to create a custom template for the C# client. This will allow consumers to fully modify and own the changes to the generator template. An issue with this is that changing template properties or adding required supporting files may increase maintenance of custom templates. This is interesting as it technically goes against the open-closed principle. We want to enable our users to modify generated code and make clients their own. We do this in some ways by providing partial classes which our users can implement, providing compile time modifications to generated client code. We also enable our users to generate the client and make direct code changes to files, then exclude the changes from regeneration via Another concern not mentioned earlier is that we have to make some concessions in language features because we support .NET 3.5 and higher. Swagger Codegen supports library-specific overrides, but framework specific switches are done in the templates. This somewhat complicates the templates, which may make users less likely to modify. I'm planning to look at conditional template compilation, for example if you choose .NET 3.5 and we are compiling If you're interested in evaluating changes before I open a pull request, let me know. I may be about a week out before I've refactor the RestSharp client into the above approach. |
|
@jimschubert I just faced the problem: it's easier to develop own api client from scratch than integrate generated one. Custom template may be good... for enthusiasts, technology maniacs. But I've asked several my colleagues who used Swagger (from the both sides Api and Client) about their experience with Swagger-Codegen. And four of them said they didn't even try to autogenerate client as it too complex, and two said they cannot make it work correctly. Then I implement IApiClientConfig and in my composition root just set IMyApi to be resolved into MyApiA, IApiClient - ApiClient, IApiClientConfig - <my _implementation>. Possibly I don't like your RestClient, then I throw away ApiClient with IApiClientConfig and implement my own IApiClient. |
|
@Gladskih I actually didn't implement the original C# client, so I can't speak to the original design. I can say, however, that your expectations in your snippet align with my expectations about my refactor. My goal is to have client methods which accept a path, a request object (similar to fetch API or other requestor patterns), and a per-request config instance (where we set generated options such as expected content type, security pattern to is, etc.). The difference between sync/async client interfaces will be the return type, and maybe an additional cancellation token parameter. This will open consumers to extend the client with their own logging, serializers, transport frameworks, etc. Ideally, these interfaces won't be modified after original creation, and may even be published to NuGet so as to allow pulling in the interfaces separately. This would allow consumers who have in-house framework requirements to expose those frameworks adapted to our client interface. As part of this refactor, I'll also be removing The current client is a little more intuitive, but I'd be interested in your experiences after this refactor. |
|
@jimschubert Oh, I'm glad to see the adequate reaction which I have in deficit in a daily life. What I also would like to see is maximum possible immutability everiwhere. But having such constructions we need some factories to allow on-the-fly configuration... and must to think about objects lifetime with accuracy to prevent redundant objects re-instantiations. Interfaces in a separate package is perfect! P.S. I use 2.3.1 and |
|
If this is being developed now, the |
|
@damienpontifex @pgrm I have a branch in which I've worked on fully refactoring the C# client to allow for user customizations. It creates abstractions around the underlying client library, and provides only a default of RestSharp client, allowing Swagger Codegen users to implement their own instances in place of the RestSharp instance. If you want to check it out, I'd like some feedback: https://github.com/jimschubert/swagger-codegen/tree/csharp-full-client-refactor |
|
I took a look at it @jimschubert - it looks pretty good at first glance. Would be a nice change! |
|
@jimschubert, Which branch is this on and do we have an eta for release? |
|
@SeanFarrow The branch is on my own fork, linked in my last commit. I no longer contribute to swagger-codegen. I have a PR open against openapi-generator (OpenAPITools/openapi-generator#737) to integrate these changes, and I'm waiting for feedback. The change will probably land in our 4.0 release. If you want to try it out, you can check out that PR. If it works for you, you're more than welcome to fork the code in my branch of the swagger-codegen fork and open a PR against swagger-codegen with that same code (changes on my branch are still Apache 2.0 licensed). There was some re-architecture of Swagger Codegen around the time I stopped contributing, so you'll probably have to convert from Mustache to Handebars and fit additional changes into the new architecture. |
|
Hi there I've just stuck with RestSharp socket TIME_WAIT on .Net Core application. Please see (restsharp/RestSharp#1406 and restsharp/RestSharp#1322) It looks that RestSharp author is not going to fix it "right now". Please take a look at the my attempt to use I've refactored The {
"library": "httpClient"
}The problems are that HttpClient package has no support for
Regards |
Hi,
HttpClient comes with the system and it is very feature rich. It would be nice for the generator to be able to generate code that uses HttpClient instead of RestSharp. That way there will be one less external dependency.
I'm willing to write the code if nobody else is doing it.
The text was updated successfully, but these errors were encountered: