Factory aware interfaces & traits

[codemasher]

A long overdue addition. I wanted to submit a PR a couple years ago already but forgot about it, and now each time I come back to a project that uses HTTP factories, I curse about the non-existence of these interfaces that should have been included in the first draft of http-factory - similar to LoggerAwareInterface and LoggerAwareTrait.

[shadowhand]

I disagree strongly with this because dependencies should be provided via constructor not via setter.

[codemasher]

That does not only bloat the constructor arguments (I've been there before) unless you use a DI container (which i personally despise) but also reduces flexibility. Also following this argumentation, LoggerAwareInterface should not exist either.

[shadowhand]

I was not part of PSR-3 working group, so I have no comments on why LoggerAwareInterface came to be.

you use a DI container

As everyone should be. Dependencies of classes should be declared in the constructor. I have never heard a valid argument against it, and now that we have named arguments there is even less of a reason to avoid the constructor.

This gets an emphatic 👎🏼 from me.

[shadowhand]

No thanks.

[codemasher]

Dependencies of classes should be declared in the constructor.

A DI container is not a dependency - it may or may not contain the required dependency. To reiterate: we came up with the concept of dependency injection to avoid hidden dependencies that static classes and functions would create, then when dependencies became a lot, we came up with DI containers... that create hidden dependencies again. smh,

Either way, setting a HTTP factory - in my current case setting a response factory on a PSR-18 http client - can come in very handy. Being able to only set these in the constructor of the client and not on the fly on an existing instance is in fact a nuisance.

[codemasher]

The more I think about the points made by @shadowhand previously, the less they make sense.
By saying that the factory should be passed in the constructor of the http client, you're saying that we have to rely on a concrete implementation and therefore making e.g. the ClientInterface superfluous.

We cannot know how a given ClientInterface was instanced or whether it uses a response factory or anything like that because none of it is dictated by PSR-18; we have to live with the provided implementation and whatever ResponseInterface it provides.
So the basic PSR-18 implementation looks as follows - the only thing we know is the sendRequest() method:

use Psr\Http\Client\ClientInterface;

class PSR18Client implements ClientInterface{

	public function __construct(/* ??? */){
		// ???
	}

	public function sendRequest(RequestInterface $request):ResponseInterface{
		// ???

		return $response;
	}
}

What I do know from previous research is that some PSR-7 implementations use additional convenience methods, which is fine as long as these are being used in their own little cosmos. However, they may break in case they encounter a PSR-7 interface from a different implementation when one of the convenience methods suddenly does not exist, which is where the PSR-17 http factories come into play.

The following example wraps a given (unknown) PSR-18 client and also acts as one by itself (similar to my OAuth client):

class MyHttpClient implements ClientInterface{

	protected ClientInterface $http;

	public function __construct(ClientInterface $http){
		$this->http = $http;
	}

	public function sendRequest(RequestInterface $request):ResponseInterface{
	
		// maybe examine and modify the request...
		
		$response = $this->http->sendRequest($request);
		
		// perhaps do something with the response...

		return $response;
	}
}

Now I want the given http client to return a modified ResponseInterface in certain cases, which it won't do without either re-implementing and re-instancing the client or having it use a response factory.
Here's the catch: as mentioned before, I have no clue if the given client uses a factory or if I can provide one on my own.
The ability to provide an optional ResponseInterface in sendRequest() could solve half of the problem, the client could then use the given response to populate instead:

class PSR18Client implements ClientInterface{

	// ...

	public function sendRequest(RequestInterface $request, ResponseInterface $response = null):ResponseInterface{
		$response ?? = new Response;
		// or:
		$response ??= $this->responseFactory->createResponse();
		
		// do stuff...

		return $response;
	}
}

While this would be cool, it would still require the logic outside the given http client's sendRequest().
Now cue the ResponseFactoryAwareInterface; the vendor http client would look like this:

class PSR18Client implements ClientInterface, ResponseFactoryAwareInterface{
	use ResponseFactoryAwareTrait;

	// ...

	public function sendRequest(RequestInterface $request):ResponseInterface{
		$response = $this->responseFactory->createResponse();
		
		// do stuff...

		return $response;
	}
}

This way we know about the capabilities of the given client, and we can act accordingly.
We know that it uses a http factory internally and we also know that we can change this factory if needed.

And in my own client I could do this:

class MyHttpClient implements ClientInterface{

	protected ClientInterface $http;

	public function __construct(ClientInterface $http){
		$this->http = $http;
		
		if(!$this->http instanceof ResponseFactoryAwareInterface){
			throw new Exception('ResponseFactoryAwareInterface unsupported');
		}
		
		$this->http->setResponseFactory(new MyDefaultResponseFactory);
	}

	public function sendRequest(RequestInterface $request):ResponseInterface{
	
		// examine request...
			
		if(/* whatever the case */){
			$this->http->setResponseFactory(new SomeOtherResponseFactory);
		}
		
		$response = $this->http->sendRequest($request);
		
		// ...

		return $response;
	}
}

In conclusion: the PSR-17 factories are nice, but are barely flexible to use as there's no reliable way to determine if a given client instance supports them. The use cases for the several proposed interfaces may vary, but their current absence helps nobody.

Bonus:

In my specific case I want the vendor client to use a different StreamInterface for file downloads, which is not guaranteed with only a custom supplied response or changing the response factory.
The issue that arises with the PSR-18 client in case one relies on the underlying instances is that there's no way to prevent a client from switching/replacing instances; e.g. when it lazy fills the body, which would defeat the purpose of the given response via the factory:

class EvilPSR18Client implements ClientInterface, ResponseFactoryAwareInterface{
	use ResponseFactoryAwareTrait;

	public function sendRequest(RequestInterface $request):ResponseInterface{

		// do stuff...
		
		$body = new UnknownStreamInterface($content);

		$response = $this->responseFactory
			->createResponse()
			->withBody($body); // <- evil

		// ...

		return $response;
	}
}

So in this case it would be useful to also supply a StreamFactoryInterface to the http client (which, again, we only can know if a StreamFactoryAwareInterface would exist).
In addition, providing a test suite (similar to http-interop/http-factory-tests) that ensures that the proper given instances are returned would be helpful.

[shadowhand]

We cannot know how a given ClientInterface was instanced or whether it uses a response factory or anything like that because none of it is dictated by PSR-18; we have to live with the provided implementation and whatever ResponseInterface it provides

That is correct. There is no definition in PSR-18 that allows a choice of PSR-7 implementation.

What I do know from previous research is that some PSR-7 implementations use additional convenience methods, which is fine as long as these are being used in their own little cosmos. However, they may break in case they encounter a PSR-7 interface from a different implementation when one of the convenience methods suddenly does not exist, which is where the PSR-17 http factories come into play.

If that is the case, then a bug should be reported to the PSR-18 package author. The point of all PSR interfaces is to promote interoperability, and if a PSR-18 package is not interoperable, then it is breaking the contract.

Now I want the given http client to return a modified ResponseInterface in certain cases, which it won't do without either re-implementing and re-instancing the client or having it use a response factory.

Per my last point, depending on a specific implementation of ResponseInterface is breaking the PSR-18 contract. To modify a response so that it is specific to an application, one should:

• Recreate the response after it is returned from the client, or
• Wrap the client, as you described, and recreate the response

But neither of these solutions make any sense because then we are depending on a concrete class, rather than interface, aka we are back to depending on (eg) Guzzle specifically.

The ability to provide an optional ResponseInterface in sendRequest() could solve half of the problem

This makes no sense. A response MUST contain, at minimum, a status code. It is not possible to know what the response status will be before sending the request.

In conclusion: the PSR-17 factories are nice, but are barely flexible to use as there's no reliable way to determine if a given client instance supports them.

PSR-17 was not created to support PSR-18 in any way. PSR-17 was created in parallel with PSR-15, to ensure that middleware and response handlers had a way to create PSR-7 objects, so that a project could install disparate middleware without every middleware depending on a different PSR-7 implementation. Basically, we wanted to avoid this situation:

FooMiddleware (PSR-17) depends on RedHttp (PSR-7)
BarMiddleware (PSR-17) depends on GreenHttp (PSR-7)
BazMiddleware (PSR-17) depends on RedHttp (PSR-7)
BuzMiddleware (PSR-17) depends on BlueHttp (PSR-7)

Could PSR-18 clients also make use of PSR-17? Absolutely. But the need for it is less obvious, because it is unlikely that a project will install multiple PSR-18 clients.

The use cases for the several proposed interfaces may vary, but their current absence helps nobody.

The absence is a reflection of that fact that swapping the PSR-7 implementation inside of a PSR-18 client has no intrinsic value. PSRs defines the contract that all implementations use, and therefore the input and output types are consistent.

In my specific case I want the vendor client to use a different StreamInterface for file downloads, which is not guaranteed with only a custom supplied response or changing the response factory.

Except that by doing so, it would no longer be possible to rely on PSR-7 interfaces as parameter or return types, because that added functionality (assuming it was added methods) would be indeterminate. And changing the return types is not possible because of the interface.

I don't have a good recommendation for how to resolve your specific case, because you didn't say why you want to swap out the stream interface, but I can say with certainty that HttpFactoryAwareInterface is not the correct solution.