Match requests with responses/exceptions

[sagikazarmark]

This way the response/exception given cannot be tied to the request. Using a similar approach to the BatchResult we could.

[sagikazarmark]

Well, client common is a dependency, so you could even use BatchResult.

[ddeboer]

Will add this in a separate PR.

[ddeboer]

I agree that linking the request with the expected exception or response is useful. However, this doesn’t work in tests where you don’t yet have a request object because it will be created later or in another layer of the application code.

Should we make it optional to link to a request? That may become confusing.

[sagikazarmark]

Tough question. Would be nice to have both behavior. Let's stick with the current one for now. We can add it if we find a use case (like addExceptionFor with a second, request argument). Generally it should be predictible what goes into the mock client, so no linking between requests and results should not be a problem.

[dbu]

for unit tests, the mock client should be enough. the biggest issue i currently see is that exceptions are always preferred over responses. if i want to test a flow that first returns a response but then runs into an exception, that will be impossible currently.

if we want to get more fancy, we could have something like request matchers (i think that is only a symfony concept) or callable associated with the reaction (response/exception). the mock client would go through that list and return the reaction of the first match. that would however be quite a change to the current behaviour.

[sagikazarmark]

Thinking about the following:

$mockClient->on($request[, $requestMatcher])->return($response/$exception);

When there is no request matcher, an ExactMatcher should be used which compares the requests based on attributes.

[dbu]

that sounds nice. i would allow to pass null für $request to say the request is not specified. otherwise you would need a AlwaysMatch and a request that is not relevant which feels weird. i fear the request matcher could become quite a thing, how to define, how to configure. must have header, must not have header, header value must be ... and so on. or we just use callback functions and document a few simple examples.

[sagikazarmark]

otherwise you would need a AlwaysMatch and a request that is not relevant which feels weird.

Hm, I would probably implement AlwaysMatch anyway to be able to use the same logic (request matcher).

I would also preserve the current addResponse addException methods which would be equivalent to $request =null.

i fear the request matcher could become quite a thing

You mean the exact matcher or request matchers in general?

I think the exact matcher would not be that hard: check method, url, other single values. Compare arrays. Optionally compare bodies?

[dbu]

I would also preserve the current addResponse addException methods which would be equivalent to > $request =null.

that actually makes sense. so do addResponse / addException leads to returning/throwing them in the order added in the future? i think we should start preserve the order, even if that is a BC break strictly speaking. we did not release yet so its ok :-) and it only affects tests and only on the edge case that you wanted an exception, then a response and set up the mock client in the opposite order...

my fear is request matchers in general, not exact match in particular. how would you do the matchers? classes? in this repo? its actually something relevant for other use cases too.

[sagikazarmark]

i think we should start preserve the order

Agreed. But if we use request matchers, I am not completely sure how we can do that. Using request matchers the obvious logic is a first-match logic

my fear is request matchers in general, not exact match in particular. how would you do the matchers? classes? in this repo? its actually something relevant for other use cases too.

I think most of the general matchers can go into the message package. We can also have a callable matcher. Alternatively we can have named request matching:

$mockClient->onMatchingHeader('Authorization', 'blah')->return()

or even

$mockClient->onSomething()->onSomethingElse()->return()

to chain matching rules.

[dbu]

this is what symfony does (for its requests, not psr 7) https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/RequestMatcherInterface.php
https://github.com/symfony/symfony/blob/master/src/Symfony/Component/HttpFoundation/RequestMatcher.php

we could port those to psr 7 (and give the implementation a fluent interface). i did a quick google search, it seems to not yet exist. but maybe its just not found by google. before we do it, we should ask around if we missed it...

[sagikazarmark]

we could port those to psr 7

?? We already have some request matchers in the message package. You ported the one from symfony.