Spring MockRestServiceServer should allow for an expectation to occur multiple times. [SPR-11365]

John Mark opened SPR-11365 and commented

Currently, the MockRestServiceServer requires that each expectation only be executed once. It would be nice to be able to say expect this and respond like this any number of times. Similar to the way the Mockito works. When setting up expectations using when().thenReturn() it can be executed any number of times.

---

Affects: 4.0 GA

Issue Links: - #17016 MockRestServiceServer should allow more verification options - #19734 MockRestServiceServer should allow for an expectation to never occur.

3 votes, 5 watchers

Comment From: spring-projects-issues

Eugene Beschastnov commented

I'm currently working on this issue and I would to discuss desired new methods. Currently I implemented methods ResponseActions.times(int times) and ResponseActions.anyNumberOfTimes(), so expectations would look like these:

mockRestServiceServer.expect(requestTo("/number")).andExpect(method(HttpMethod.GET))
    .times(2)
    .andRespond(withSuccess("4", MediaType.TEXT_PLAIN));

and

mockRestServiceServer.expect(requestTo("/number")).andExpect(method(HttpMethod.GET))
    .anyNumberOfTimes()
    .andRespond(withSuccess("1", MediaType.TEXT_PLAIN));

Method names are self-explainable, as it seems to me: times means that expected request should be made the exact number of times, and anyNumberOfTimes means that request may be done any number of times, even zero.

How do you think, should I add any other methods, like in Mockito - atLeastOnce, never, atLeast, atMost etc.? Or is it better to just reuse Mockito's VerificationMode (I don't think it's good solution, but if anyone thinks differently - we can discuss).

Also I'm thinking about adding method allowRequestsInAnyOrder() to class MockRestServiceServer to, well, allow requests be made in any order, but not in strict one, as now. The instantiation of mock server would look like this:

MockRestServiceServer mockRestServiceServer = MockRestServiceServer.createServer(restTemplate).allowRequestsInAnyOrder();

What do you think of it?

Comment From: spring-projects-issues

John Mark commented

Eugene,

These changes look great. I think it makes perfect sense.

The extra methods of atLeastOnce, never, atLeast, atMost, etc. would also be a nice addition to have. I agree with you that reusing a class from Mockito would not be a good idea.

I think allowRequestsInAnyOrder() would also be useful.

Looking forward to these features!

Thanks!

Comment From: spring-projects-issues

Rossen Stoyanchev commented

What John Mark said. I've also scheduled this for inclusion in 4.3.

Comment From: spring-projects-issues

Eugene Beschastnov commented

Ok, I'm done with allowRequestsInAnyOrder() (and I've also found and fixed a couple of bugs), but I need a little more time to work with atLeastOnce, never, atLeast, atMost, etc. - and I need your opinions.

I'm thinking of generalizing these methods in a way similar to Mockito - so there would be general method andExpectIt, which receives instances of new interface OccurrenceExpectation. Those instances can be generated by factory methods like OccurrenceExpectations.atLeastOnce etc. So code would look like this:

import static org.springframework.test.web.client.OccurrenceExpectations.atLeastOnce;
import static org.springframework.test.web.client.OccurrenceExpectations.atMost;

...

mockRestServiceServer.expect(requestTo("/number")).andExpect(method(HttpMethod.GET))
    .andExpectIt(atLeastOnce())
    .andExpectIt(atMost(4))
    .andRespond(withSuccess("1", MediaType.TEXT_PLAIN));

There could also be shortcut methods, like these:

mockRestServiceServer.expect(requestTo("/number")).andExpect(method(HttpMethod.GET))
    .atLeastOnce()
    .atMost(4)
    .andRespond(withSuccess("1", MediaType.TEXT_PLAIN));

Opinions, suggestions?

PS. I'm not sure if OccurrenceExpectation is a good name. Is CountExpectation better? Or could you suggest something even better?

Comment From: spring-projects-issues

Rossen Stoyanchev commented

I haven't seen any actual code so I may be overlooking some detail but my expectation would be that ResponseActions with its two methods andExpect and {andRespond}} shouldn't have to change. We should only need extra RequestMatcher's exposed via static methods. The MockRestRequestMatchers is a central entry point for all such built-in static methods currently which are further organized into several classes in the same package (content, jsonpath, xpath).

Comment From: spring-projects-issues

Eugene Beschastnov commented

But CountExpectation 's (so far I chose this name) are utilized in very different manner than common RequestMatcher. Well, actually I'm preparing to commit my code now, so you may take a look to it in a couple of minutes - I'll bring you a link.

Comment From: spring-projects-issues

Eugene Beschastnov commented

Here it is - https://github.com/spring-projects/spring-framework/compare/master...eugenius:SPR-11365 Or is it better to make the pull request?

Comment From: spring-projects-issues

Rossen Stoyanchev commented

Sorry for the slow response. Having had a quick look I certainly see your point. That said I'll have to spend a little more time before I can provide better feedback.

Comment From: spring-projects-issues

Eugene Beschastnov commented

Ok, I'm looking forward for it. In the meantime, I'll prepare pull request.

Comment From: spring-projects-issues

Rossen Stoyanchev commented

hi Eugene Beschastnov, sorry for taking so long. I'd like to suggest a slightly different approach. In terms of the actual API I'd like to restrict it a little so that it guides you a little more. To see what I mean consider that with the current POC you can do expect(...).times(...).andExpect(...).times(...). Arguably that doesn't makes sense and ideally the API shouldn't allow it. On the implementation level I'd like to keep the current arrangement with RequestMatcherClientHttpRequest implementing ResponseActions. Instead I'd like to add the concept of an actual vs expected request and some abstraction for matching from one to the other (i.e. the strict vs any order). These are just rough ideas for now. I'm going to experiment a little to see how it works out and will share when I have something more concrete.

Comment From: spring-projects-issues

Rossen Stoyanchev commented

Ended up going with something that I think will work well but take a look and let me know if it does or not.

The expected count is specified as an argument to expect:

server.expect(once(), requestTo("/foo")).andExpect(...).andRespond(...);
server.expect(manyTimes(), requestTo("/foo")).andExpect(...).andRespond(...);
server.expect(times(5), requestTo("/foo")).andExpect(...).andRespond(...);
server.expect(min(2), requestTo("/foo")).andExpect(...).andRespond(...);
server.expect(max(4), requestTo("/foo")).andExpect(...).andRespond(...);
server.expect(between(2, 4), requestTo("/foo")).andExpect(...).andRespond(...);

There is also a builder for MockRestServiceServer with an option to go in any order of requests:

MockRestServiceServer.bindTo(this.restTemplate).ignoreExpectOrder().build();

Comment From: spring-projects-issues

Eugene Beschastnov commented

I agree, it looks better.

Comment From: spring-projects-issues

Sam Brannen commented

Nice work, Rossen! :)

Comment From: makaxel

Hello. Are there any plans to add manyTimes() to MockWebServiceServer ?

Comment From: sbrannen

Are there any plans to add manyTimes() to MockWebServiceServer ?

@makaxel, this is the issue tracker for the core Spring Framework.

For issues related to the Spring Web Services project, please use the dedicated issue tracker on JIRA.