spring-projects / spring-framework

Spring Framework
https://spring.io/projects/spring-framework
Apache License 2.0
56.63k stars 38.14k forks source link

Provide Dispatcher using RequestMatchers for MockWebServer okhttp #23468

Closed membersound closed 4 years ago

membersound commented 5 years ago

For integration testing WebClient, it is advised to use okhttp MockWebServer (see for example #19852). Which is great in general, but it lacks an important ability that MockRestServiceServer offered: writing conditions for the responses to return:

mockServer.expect(ExpectedCount.once(), requestTo(path))
                .andExpect(method(HttpMethod.POST))
                .andExpect(jsonPath("$", hasSize(1)))
                .andExpect(jsonPath("$[0].someField").value("some value"));

In MockWebServer that's simply not possible. Your only chance is to mockWebServer.enqueue(), and if you have a service that sends multiple webservice requests, there is no chance to match responses to explicit requests.

okhttp offers a "solution" by using a custom Dispatcher with custom logic: https://github.com/square/okhttp/tree/master/mockwebserver

Idea: it would be great if spring could offer a Dispatcher that can take RequestMatcher like above. Something like:

dispatcher.addExpected(new MockResponse().body(body), method(HttpMethod.POST), requestTo(path), jsonPath("$[0].someField").value("some value"));

The Dispatcher should then only return the mocked response if all the RequestMatcher have been true.

rstoyanchev commented 5 years ago

There seems to be no built-in support for matching requests to responses indeed. One would have to enqueue responses and then verify the recorded requests, but the server is responding blindly which could lead to strange failures, and requests might not follow a predictable order.

This is a little surprising but I don't think the Spring Framework is the right place for it. If anything such a custom Dispatcher would be best provided by OkHttp. @adriancole any insight on why something like this is missing or might be supported or accepted would be appreciated. Taking a quick look around the web there seems to be a proliferation of custom dispatchers for MockWebServer with boilerplate request matching.

codefromthecrypt commented 5 years ago

usually the answer to this sort of thing in okhttp is either the code involved to do what's asked isn't a lot or it there isn't a single pattern that's solves everyone elegantly. In zipkin, we have a custom dispatcher inside zipkin-junit, but technically it is more a server than an assertion tool as it fully implements routing.

What I'm sensing here is a desire to have a routing api over MWS. Not sure this has been asked specifically for or not, but I wouldn't be surprised if it became popular cc @swankjesse

swankjesse commented 5 years ago

As OkHttp maintainer I'm reluctant to build real dispatching into MockWebServer. My target use case is embedding in JUnit tests; any real dispatching brings new responsibilities.

That said, it's not absurd to do a dispatcher library that plugs in!

rstoyanchev commented 5 years ago

@adriancole and @swankjesse, thanks for your feedback!

We have some experience with creating and maintaining similar type of functionality for Spring's RestTemplate which we didn't want to re-create for the more recent WebClient given the existence of OkHttp MockWebServer and others.

From our experience, a basic form of matching requests to responses couldn't be more straight forward. You can see ours here, but for MWS it could be enqueueResponse(MockRequest, MockResponse) + check the next MockRequest's properties match the current request, or perhaps more chained style, something like:

enqueueResponse(response).forUrl(uri1).httpMethod("GET");
enqueueResponse(response).forUrl(uri2).httpMethod("POST");
// etc.

Over time we allowed specifying how many requests to expect and if they should match in order of declaration or any order. This added a a little extra complexity but not bad at all and for that it is a very useful feature.

membersound commented 4 years ago

As spring nowadays totally relies on the usage of MockWebServer for WebClient junit tests, it would be great if this feature could be implemented. It's kind of useless especially in concurrent or async applications (which is often the case using WebClient) to test without a matching dispatcher.