spring-framework icon indicating copy to clipboard operation
spring-framework copied to clipboard

Allow MockRestServiceServer to validate requests but actually obtain response via HTTP call

Open nightswimmings opened this issue 4 years ago • 12 comments

For the cases when using MockRestServiceServer on an integration test, I think we could benefit having some options to make it bypass certain requests that are available in the local environment (acting like a Spy rather than a Stub).

For instance, imagine that we are executing an Integration Test on an app with an endpoint that internally performs 3 requests:

  • 1 request to another endpoint of the same app (architecture considerations aside)
  • 1 request to an external endpoint that is being already mocked with a docker container according to the testing @Profile
  • 1 request to a third-party endpoint that we want to mock

If we autoconfigure MockRestServiceServer and RestTemplate with the simplest setup, we will end up with the requirement of mocking all requests. Thus, it would be nice to allow certain requests bypassing the expectation or at least the need for a mocked response and let them pass through as real calls.

By doing

mockRestServiceServer.expect(requestTo(call));

with no response definition, one would expect mockRestServiceServer to account the expectation but let the request pass through, but it fails instead complaining that no mocked response has been defined.

At least, IMO this should be the default behaviour when the application (not the test) performs a request against itself (localhost/127.0.0.0), or even more accurately, against a Controller of the same app

My need is described here: https://stackoverflow.com/questions/65693954/is-it-possible-to-make-some-of-my-springboottest-app-requests-to-bypass-spy-in

nightswimmings avatar Jan 13 '21 11:01 nightswimmings

MockRestServiceServer is a custom ClientHttpRequestFactory for the RestTemplate and when a call is made to the RestTemplate it needs to provide a response. So I'm not sure I understand the scenario.

BTW if you could please edit your description and provide a complete description. As per our guidelines, it's okay to refer to a SO thread, but an issue created here needs to have a self-sufficient description.

rstoyanchev avatar Jan 13 '21 12:01 rstoyanchev

My apologies, I tried extending the description @rstoyanchev. I can understand effort, design and practical considerations, but to be honest, I see the feature absolutely legit and beneficial for the ecosystem.

nightswimmings avatar Jan 13 '21 13:01 nightswimmings

Thanks for updating the description!

When MockRestServiceServer instruments the RestTemplate with a custom ClientHttpRequestFactory, the resulting ClientHttpRequest needs to return some kind of response. By "let the request pass through" you mean that it should execute an actual HTTP call by using whatever request factory was there before?

To do this, a ResponseCreator would need to re-create the ClientHttpRequest with the original request factory, re-populate it, and use it instead. This should be possible in an application, for example:

RestTemplate restTemplate = new RestTemplate();
ClientHttpRequestFactory origFactory = restTemplate.getRequestFactory();

MockRestServiceServer server = MockRestServiceServer.createServer(restTemplate);

server.expect(requestTo("/foo")).andRespond(request -> {
	MockClientHttpRequest mockRequest = (MockClientHttpRequest) request;

	ClientHttpRequest newRequest = origFactory.createRequest(mockRequest.getURI(), mockRequest.getMethod());
	newRequest.getHeaders().putAll(request.getHeaders());
	StreamUtils.copy(mockRequest.getBodyAsBytes(), newRequest.getBody());

	return newRequest.execute();
});

That ResponseCreator implementation can be extracted out to a re-usable class. So this looks quite achievable with the existing API unless I'm missing something.

rstoyanchev avatar Jan 13 '21 17:01 rstoyanchev

Thanks! Yes, I meant doing the actual HTTP call without mocking. I am not the one to say, I respect you guys for you alone are basically responsible for making Java a competitive language nowadays, and it is amazing what you are building day by day given the complexity behind, but I personally think that the nice way to provide that flexibility is by refactoring this part:

          return new MockAsyncClientHttpRequest(httpMethod, uri) {
               protected ClientHttpResponse executeInternal() throws IOException {
                   ClientHttpResponse response = MockRestServiceServer.this.expectationManager.validateRequest(this);
                   this.setResponse(response);
                   return response;
               }
           };

Some way, if the returned response is signaled as null/undefined etc.. (after keeping conveniently the expectation validation), the code should create a ClientHttpResponse in the same logic that regular resttemplate would do (establishing an actual live http connection). I am not sure how customizable is resttemplate to worry about the capability of replicate behavior without side-effects, but IMO, on the trade-off,that would be much more convenient than throwing an exception when no response has been defined, something the fluent appi does not prevent, and besides, something that an uneducated user as me would expect as a default behavior

nightswimmings avatar Jan 13 '21 20:01 nightswimmings

@nightswimmings I agree this could be a useful feature. It would probably need to be an explicit new option, something like:

server.expect(requestTo("/foo")).andPerformRequest()

In the mean time, I've provided a sample solution that you can use today that would put you in an almost identical position with a very small amount of custom code. If you extract the anonymous ResponseCreator from the sample in my last comment and turn it into a static factory method it would look like this:

RestTemplate restTemplate = new RestTemplate();
ClientHttpRequestFactory originalFactory = restTemplate.getRequestFactory();
MockRestServiceServer server = MockRestServiceServer.createServer(restTemplate);
server.expect(requestTo("/bar")).andRespond(withHttpCall(originalFactory));

As far as I can see nothing stops you from doing this today.

rstoyanchev avatar Jan 14 '21 10:01 rstoyanchev

Your suggestion would be amazing, Rossen! And thanks for the workaround! I already solved it by using 2 different resttemplates on the production code, (I just opened the ticket because I really thought it would be a nice enhancement), but your workaround is much much elegant! Thanks!!

nightswimmings avatar Jan 14 '21 11:01 nightswimmings

hi, what's the status on this? Can it be worked on?

parviz-93 avatar Feb 04 '21 08:02 parviz-93

@parviz-93 yes it can be worked on.

rstoyanchev avatar Feb 04 '21 18:02 rstoyanchev

@rstoyanchev, thank you very much, I will start working on this problem. Will you have some other ideas or preferences besides those that you have already described?

parviz-93 avatar Feb 07 '21 20:02 parviz-93

I apologize for the delay, I will try to create soon a pull request

parviz-93 avatar Mar 23 '21 21:03 parviz-93

@rstoyanchev Could you tell me how to implement this correctly? is it possible to keep an orninal ClientHttpRequestFactory inside the MockClientHttpRequestFactory ? I started writing and found that I was a little puzzled which way to take.

parviz-93 avatar May 04 '21 13:05 parviz-93

Hello, what is the status of this, is this still open?

valituguran avatar Sep 17 '22 15:09 valituguran

Superseded by #29721.

rstoyanchev avatar Jan 09 '23 16:01 rstoyanchev