postman-code-generators icon indicating copy to clipboard operation
postman-code-generators copied to clipboard

Update the csharp-restsharp generator for RS 107

Open alexeyzimarev opened this issue 3 years ago • 9 comments

I did a major refactoring of RestSharp, and since v107 the API looks different. There are quite a few breaking changes.

Now I got an issue https://github.com/restsharp/RestSharp/issues/1734 about Postman's generated code being incompatible with RS 107. I am not sure if I can lift the necessary refactoring in this repository, but I am more than happy to give the necessary guidelines and reviews should anyone be able to do it.

alexeyzimarev avatar Feb 05 '22 09:02 alexeyzimarev

@alexeyzimarev Thanks for reaching out!

You can definitely contribute to this repository or share the guidelines. We will be happy to incorporate them in the code-generators. Feel free to share your thoughts or create a PR. cc @nrmartell

umeshp7 avatar Feb 05 '22 10:02 umeshp7

I am making some code changes, the PR will follow, it's a matter of time.

For the sake of information, I'd like to record the necessary changes here.

Client options

The following properties of RestClient are to be moved to RestClientOptions:

  • FollowRedirects
  • UserAgent
  • Timeout

The UserAgent is a bit problematic as it's currently produced by parseHeader, but the options object must be populated before this function is called.

Async API

  • ExecuteAsGet and ExecuteAsPost are gone, as we don't care much. There's no difference there except one adds the request body and the other one doesn't. RestSharp is agnostic to the request type
  • Execute is deprecated, it should be var response = await ExecuteAsync

AddParameter and Content-Type header

  • The content type should be specified for the body added, there's no need to explicitly set the header
  • In most cases AddParameter(..., RequestBody) should be replaced by AddStringBody or AddBody
  • Content-Type header should not be set explicitly for well-known content types such as form URL encoded, multipart form data, or JSON.

alexeyzimarev avatar Feb 05 '22 11:02 alexeyzimarev

Ideally, for JSON calls the code generator should produce request and response models as classes or records, but it might be a bit too much. The current generation won't utilise the serialization features of RestSharp.

alexeyzimarev avatar Feb 05 '22 11:02 alexeyzimarev

Thank you for the information @alexeyzimarev Looking forward to the PR

umeshp7 avatar Feb 07 '22 05:02 umeshp7

Hi all. I just logged this with Postman support and they sent me here. I have subscribed for updates. Sounds like Alexey has it covered - thanks! One suggestion: Perhaps keep the old version available, as many users may still be using it. (Also, I'm totally up for proper classes etc :)

garkpit avatar Mar 04 '22 10:03 garkpit

ps. Also happy to beta test. Feel free to tag me @alexeyzimarev once you are ready

garkpit avatar Mar 04 '22 11:03 garkpit

@alexeyzimarev We have worked on a PR for doing this update. Would love to get your review for the same and incorporate if you have any inputs. cc @LuisTejedaS

umeshp7 avatar Nov 04 '22 04:11 umeshp7

Please see https://github.com/postmanlabs/postman-app-support/issues/10591 which demonstrates the current issue using this image: image

GarryLowther avatar Nov 24 '22 12:11 GarryLowther

@umeshp7 I wrote some comments there, and I think it should be merged as it is today and refined later on.

I'd also be happy to contribute when v109 will be ready because there will be some breaking changes concerning options.

I only have two concerns:

  • There seems to be no way to create contract classes, and JSON payloads get produced as a string. I am not sure how to resolve it.
  • The client instance is created in the sample, and when people copy-paste it, they end up with lots of client instances. I hope to get it resolved one way or another in v109 when I can try updating the codegen.

alexeyzimarev avatar Nov 24 '22 14:11 alexeyzimarev