Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

enable conditional answers in MkContainer #19

Closed
yegor256 opened this issue Apr 20, 2014 · 27 comments
Closed

enable conditional answers in MkContainer #19

yegor256 opened this issue Apr 20, 2014 · 27 comments

Comments

@yegor256
Copy link
Member

Let's introduce a new utility class MkQueryMatchers, in order to enable conditional answers in MkContainer, for example:

MkContainer container = new MkGrizzlyContainer();
  .next(
    new MkAnswer.Simple("hello, world!"),
    Matchers.allOf(
      MkQueryMatchers.hasHeader(
        "Content-Type",
        Matchers.equalTo("text/plain")
      ),
      MkQueryMatchers.hasBody(
        Matchers.containsString("say hello")
      )
    )
  )
  .start();

This means that the answer will be returned only if the request contains header Content-Type, which is equals to "text/plain" and request body contains "say hello".

By default, when method next(MkAnswer) is used (with one argument only), an everything-matching instance of Matcher<MkQuery> is used.

Besides that, let's add methods take(Matcher<MkAnswer>) and takeAll(Matcher<MkAnswer>), and an utility class MkAnswerMatchers:

MatcherAssert.assertThat(
  container.takeAll(
    MkAnswerMatchers.hasHeader(
      "Content-Type",
      Matchers.equalTo("application/json")
    )
  ),
  Matchers.allOf(
    Matchers.hasSize(5),
    Matchers.hasItem(
      Matchers.allOf(
        MkQueryMatchers.hasHeader("User-Agent"),
        MkQueryMatchers.hasBody(
          Matchers.containsString("say hello")
        )
      )
    )
  )
);

This means that we're taking all requests received by the container, which were answered with responses that had a header "content-type" with a value "application/json". We're asserting that there were 5 those answers. And all of them had body "say hello" and a header "User-Agent".

@vjkoskela
Copy link

I believe this accomplishes part of what we added to the rexsl fork. One question, what is the behavior of take() with respect to the order of requests? For example, if I have two conditional next() statements, the requests for these can arrive in any order. How do I take the request that matched a specific response in order to apply request specific validation to it?

@yegor256
Copy link
Member Author

You should call takeAll(m) and get a collection of all queries/requests which received the answers that match m. Then, you can do whatever you want with this collection. Of course, queries will be positioned inside the collection in some order. And it will be the order of requests arrival to the container.

If you want to make assertions on all of that requests, you should use Matchers.everyItem(m). If you want to assert that at least one of them has some properties, you use Matchers.hasItem(m). Where m is a matcher.

Let me know if I managed to explain it clear enough :)

@dmarkov
Copy link

dmarkov commented Apr 21, 2014

I'm aware of the task, give me some time to find a developer...

@dmarkov
Copy link

dmarkov commented Apr 21, 2014

@carlosmiranda it's yours now, please proceed keeping in mind our principles. Feel free to ask any technical questions right here in the ticket

@dmarkov
Copy link

dmarkov commented Apr 21, 2014

@carlosmiranda The cost of this task is 30 mins (this is exactly how much will be paid, not less not more), when the task is done

@yegor256
Copy link
Member Author

@carlosmiranda please hold on with implementation... let's clarify the requirements first

@carlosmiranda
Copy link
Contributor

@yegor256 , no problem.

@vjkoskela
Copy link

Maybe I've missed something but we specifically want to validate each request against a unique set of requirements. If the matching of request and response is separated from the validation of requests we cannot know which validation requirements to apply to each request.

@carlosmiranda
Copy link
Contributor

@vjkoskela , @yegor256 , any word on what needs to be done?

@yegor256
Copy link
Member Author

@vjkoskela do you know in advance how many requests are you expecting?

@vjkoskela
Copy link

Not exactly. Most test cases will know the exact number, type and requirements for all requests it is mocking. However, there are test cases that target specific functionality and are more permissive to certain common calls that do not directly affect the test case. For example, a test targeting a specific business case may stub a standard positive response to an authorization request and permit any number of request-responses on that endpoint.

@yegor256
Copy link
Member Author

OK, how about this. MkContainer will have three different methods:

  • next(MkAnswer)
  • next(MkAnswer, Matcher<MkQuery>)
  • next(MkAnswer, Matcher<MkQuery>, int).

The first one is just calling the second one with a "matching all" matcher. The second one is calling the third one with 1 as its third argument.

When requests arrive, the container will check which matchers they match. The first successful match will cause the answer to be returned. Then, the container will decrement the number, until it reaches zero.

For example, we want to response to authorization requests always, but only one time to a specific data request:

container.next(authorization_answer, Matchers.is_autorization_request(), Integer.MAX_VALUE);
container.next(data_answer, Matchers.is_data_request());

Makes sense?

@vjkoskela
Copy link

Yes, that makes sense. Is the first form next(MkAnswer) then the lowest priority? e.g. it is only used if no matcher matches the request?

@yegor256
Copy link
Member Author

The priority is set by the order of calls to next(). For example:

container
  .next(alpha, when_request_contains_foo_header)
  .next(beta)
  .next(gamma, when_post_request, 5);

This means that when request arrives the container:

  1. check whether it it contains foo header.
  2. if yes, returns alpha (and never returns it again)
  3. if no, returns beta (and never returns it again)

gamma will be returned only after beta and only when request is POST, and only 5 times.

In your scenario, when you want authentication request be always satisfied and data request only on certain conditions, configuration will look like:

container
  .next(auth_response, if_it_is_auth_request, Integer.MAX_VALUE)
  .next(data_response)

Any number of auth requests will be satisfied, but only one data request will get its answer. All other data requests will get 500.

@vjkoskela
Copy link

I still think there is a conflict between specifying order and specifying matchers. In our case we don't know the order but we would like to setup rules with matchers and execute the one that matches regardless of order.

So if I have the following:

container
.next(alpha, when_request_contains_foo_header)
.next(beta, when_request_contains_far_header)
.next(gamma, when_post_request, 5);

Will the correct response be given if I get the following in any order:

  • request_contains_foo_header
  • request_contains_foo_header
  • post_request (up to five times)

The statement that concerns me, and granted I am not clear on how you intend to implement this, is "gamma will be returned only after beta". Are you implying there is a strict ordering to the expected requests?

@yegor256
Copy link
Member Author

In this example everything will work as you expect. Because you're using matchers in all next() methods. In my example, I'm using Matchers.anything() matcher in the second call to next(). This matcher will catch everything, and my gamma will be returned only after beta is satisfied.

So, your example will work exactly as you expect.

@vjkoskela
Copy link

My concern in your example is that if after the alpha matching request you receive a request that matches gamma then beta is still returned. You are correct that users can make it work; however, the semantics may be a little surprising and can make it easier to write brittle tests.

In our case the semantics were that only one matcher could match a request or else and exception is thrown. I understand that you are doing something more general so that's fine - just my two cents.

@yegor256
Copy link
Member Author

Yeah, as a library we have to be generic..

@carlosmiranda does it all sound clear? can you implement it?

@carlosmiranda
Copy link
Contributor

@yegor256 , @vjkoskela , yeah it seems pretty clear now. If I understand it correctly, we implement MkQueryMatchers, and next(MkAnswer, Matcher<MkQuery>, int), with convenience overloads for next with predetermined Matcher and priority values?

I know it's been assigned to me since last week but please do give me a bit more time to implement, it's only been clarified recently.

@yegor256
Copy link
Member Author

yegor256 commented May 1, 2014

@carlosmiranda yeah you got it right

@yegor256
Copy link
Member Author

yegor256 commented May 1, 2014

@dmarkov please note that we need more time here than usual, thanks

@dmarkov
Copy link

dmarkov commented May 1, 2014

@dmarkov please note that we need more time here than usual, thanks

@yegor256 yes, take your time, thanks for letting me know

carlosmiranda added a commit to carlosmiranda/jcabi-http that referenced this issue May 5, 2014
carlosmiranda added a commit to carlosmiranda/jcabi-http that referenced this issue May 5, 2014
carlosmiranda added a commit to carlosmiranda/jcabi-http that referenced this issue May 5, 2014
@carlosmiranda
Copy link
Contributor

I submitted a pull request for this at #23, please take a look.

ghost pushed a commit that referenced this issue May 5, 2014
#23: pull request #19 Conditional answers with MkContainer.next()
@carlosmiranda
Copy link
Contributor

@yegor256 , #23 has been merged, are we good here?

@yegor256
Copy link
Member Author

yegor256 commented May 5, 2014

I think we're good with this ticket. I'll add a documentation page soon. @vjkoskela I'll keep you updated by email when the feature is fully ready and documented

@yegor256 yegor256 closed this as completed May 5, 2014
@dmarkov
Copy link

dmarkov commented May 5, 2014

@carlosmiranda 30 mins was added to your account, many thanks for your contribution!

@yegor256
Copy link
Member Author

the feature released in version 1.5

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants