Repository navigation
ExecuteRequestAsync as public #2163
Description
Continue discussion #1952
Api was changed, it's nice. But it still difficult to implement generic logic for all requests.
If we check implementation for ExecuteAsync and DownloadStreamAsync methods, they have common part: ExecuteRequestAsync.
Why don't you want to make this method public? This will allow library developers to write more versatile and more enjoyable code.
Activity
RomanSoloweow commentedon Mar 3, 2024
@alexeyzimarev any news?
alexeyzimarev commentedon Mar 6, 2024
I have no issue with that but it would mean the function will become a part of the interface, which will introduce breaking changes for everyone who use the interface.
RomanSoloweow commentedon Mar 7, 2024
Which interface you mean?
RomanSoloweow commentedon Mar 7, 2024
I think it's can be not included in interface. Only as part of RestClient object
rassilon commentedon Mar 14, 2024
There is RestRequest.OnAfterRequest for the brave..
alexeyzimarev commentedon Mar 14, 2024
@rassilon I think the idea here to enable creating extensions that would use
ExecuteRequestAsync. However, it will require exposing the internalHttpResponsetype as public. I don't really see a problem doing that.One option is to make
ExecuteRequestAsyncthe only function in the interface, and convert bothExecuteAsyncandDownloadStreamAsyncto extensions. It would also allow resolving issues with extending the basic functionality, like returning a tuple for downloads (there's an open issue for that, and a PR).rassilon commentedon Mar 14, 2024
That definitely sounds like a good idea.
RomanSoloweow commentedon Mar 15, 2024
Sounds perfect
RomanSoloweow commentedon Apr 9, 2024
@alexeyzimarev any news?
alexeyzimarev commentedon Jun 6, 2024
I looked it now and tried to be "smart" by using default interface implementation for that function, but it didn't work because of .NET Framework target.
Essentially, both
ExecuteAsyncandDownloadStreamAsyncdepend onExecuteRequestAsync. If that function becomes the only one in the interface, people who, for example, use the interface for mocking the client, will struggle to returnRestResponseback as they'd need to properly populateHttpResponseMessageand it is not trivial.The default interface implementation could have worked, but apart from issues with .NET Framework (I tried to use partials and pragmas to make it class/interface), the function uses private properties and functions, so it quickly became a mess.
So, right now, unfortunately, I don't have a good solution there.
niwrA commentedon Jun 27, 2024
Our team chose to use this, but it has so far taken me hours to find a way to properly test this. The solutions that I found were against older versions and no longer work. It wasn't trivial to mock with IHttpClient either, mind you. I will try and see if I can spare some time to help, because otherwise I fear this otherwise excellent and long standing library will lose out to regular IHttpClient. Intuitively I'm thinking that a solution could start with wrapping all code in a new object with a good interface. This could leave everything else working as it is now, but provide a new interface and code block that could eventually take over without breaking changes for existing users.
alexeyzimarev commentedon Jun 27, 2024
I mentioned it before that mocking the HTTP client isn't the best idea as you can only ensure that your test fake receives the right parameters, but it won't guarantee that the request will be sent over the wire in the shape and form expected by the remote endpoint. Neither can you test how the response is deserialised.
There are better way to test API calls, and RestSharp tests uses three methods:
WireMockwhen you actually want to send requests over the wire (locally) and mock the expected response, so the client can handle thoseRichardSzalay.MockHttpfor asserting thatHttpRequestMessageis correctly formed, and also for validating response handling by providing the mocked response back, but without actually making an HTTP callThose three methods allow us to ensure that RestSharp works correctly, and they can also be applied easily to test the code that uses RestSharp.
MockHttp might not be the best choice as it requires configuring a delegating handler, which often requires creating a special branch in the application code that'd be only used for tests. This I understand. But using WireMock doesn't pose this issue as it's just a server running locally.
I don't think this issue was ever meant to be about testing. If you still need to mock the client, there's a simple interface to implement.