Allow to easily extend the Mock client.

[mxr576]

Q	A
Bug fix?	no
New feature?	yes
BC breaks?	no
Deprecations?	no
Related tickets	-
Documentation	-
License	MIT

What's in this PR?

This PR allows the extension of the Mock client.

Why?

I have tried to extend the Mock client with some custom logic but I was unable to do that because all properties of the class are private. Also, I think introducing an interface that describes the public methods of the mock client could be useful for the future.

Example Usage

Checklist

• Updated CHANGELOG.md to describe new feature

To Do

[sagikazarmark]

Thanks a lot for the contribution.

Can you please provide a use case for extending the Mock client. Maybe there is some feature we didn't think about?

Also, can you please explain why you think an interface would be useful? Since there aren't really any alternative implementations, I don't think that's necessary.

[mxr576]

I have created a custom FileSystemMockClient client class that can read responses from the file system as an offline solution for our tests. I use the interface to check whether a mock or a real HTTP client is in use when a test is running and based on that I can take further actions, for example skip a test that could be executed only with a real HTTP client that actually communicates with a real back.end.

[fbourigault]

I think there is a far better design for your use case.

To fit the Single Responsibility Principe (the S in SOLID) you should decouple the loading of responses from the client itself. You could imagine a ResponseLoader which take the mock client as argument, load responses from the filesystem and call Http\Mock\Client::addResponse() for each loaded response.

In such case, no need for any interface or protected method. You can achieve your use case with a better design which does not infringe any SOLID principle.

Small note about getLastRequest. There is a pull request for adding it!

[mxr576]

Thanks for the tip, I always try to keep in mind the SOLID principals, but on the other hands these are just tests and they should not be overcomplicated on the implementation level.

What I initially thought is introducing a custom ReponseFactory that could return responses from the file system if needed and pass it as an argument of the Mock client, but that could not work, because the factory would have needed to know what was the request and take actions based on that.

ResponseLoader could be a good solution, but it would require further adjustments because it should be able to work with a HttpClient instance as well that does not have addResponse() method. So in that case, I'd still need to make a decision based on the type of the passed client object and I'd rather do that based on an implemented interface rather a concrete class.

Maybe an implementation that leverages the observer pattern could be better one which would let the first subscriber to react to a request, but as I said, this should not be overcomplicated.

I know about the getLastRequest() PR but I was not sure based on the conversation there when it will be merged and it wasn't the main reason why I created this PR.

[dbu]

sorry, i am -1 on merging this. i think we should not add this much complexity to the testing helpers. i think most of the time people should use phpunit or mockery to mock the client interface. this client is for when several requests are being done.

when it gets more complicated than that, i would point to things like php-vcr, or then write your own class tailored to your needs.

[mxr576]

Okay, let's not change the visibility of the properties, but what about exposing an interface that can be implemented by other, custom Mock clients?

[dbu]

i think an interface would not be a problem to do. but can you please explain the use case for such an interface? interface to me means reusability and interchangeability. in what scenario do we need that for tests?

[mxr576]

In my tests I want to distinguishes those \Http\Client\HttpClient implementations that actually sends request to the API from those that are not. Because \Http\Mock\Client currently implements the same interfaces as all the other \Http\Client\HttpClient implementations this is not possible based on an interface. Moreover, if I would like to create my own Mock http client or clients I can only extend the same interface \Http\Client\HttpClient.
I am open for other suggestions about how this problem can be solved without implementing a logic that identifies Mock HTTP clients based on class names.

[dbu]

what would be the problem of checking for the mock class name?

[soullivaneuh]

I'm also against this change. 👎

For me, the Client should even be final and instead of making this extensible, add needed functionalities.

[dbu]

thanks for the proposition, but we won't merge this as discussed above. but don't let this discourage you from further contributions.