php-http / message-factory Goto Github PK

PSR-17 is an upcoming proposal that will standardize HTTP Message factories. The work of defining the actual interfaces is happening right now in @http-interop http-factory repository.

Can we work together to make http-interop/http-factory the standard moving forward?

Are there any real life use cases?

There is no upper bound on the PHP version (and too late to add one in 1.x). We should test on PHP 8.0 with the CI.

Proposed by @mekras in this.

I was thinking about creating a virtual package, and I am going to explain here, why I won't introduce one:

The actual dependency is the PSR-7 implementation, not the message factory implementation. In an ideal case, message implementations should have their own factories implementing these interfaces. If it is not possible, we should have them either in discovery or bridge packages:

https://github.com/mekras/httplug-diactoros-bridge

Bridge packages could even be extracted from discovery to make them usable separately.

To sum up: factories belong to implementation. We should rely on implementations and automatically provide factories, when they don't (served by the discovery layer).

Are they really necessary?

php-http/httplug#42 introduced the Body interface to abstract different HTTP message bodies.

imho, the MessageFactory is playing the same role as HttpMethodsClient and thus should have the same arguments for the body. it will also simplify the send method implementation for PSR-7 compatible http clients as they can simply delegate to the factory.

@sagikazarmark said in the discussion on the adapter PR

Actually the MessageFactory has nothing to do with the Client/Adapter thing. The MessageFactory
package has been built for PSR-7 and I would like to make it as small as possible.

However I see the reason behind why it would make sense to handle Body classes in the message
factory: some content might need custom headers. To solve this I am going to add a getContentHeaders > method to the Body interface.

This repo does only contain interfaces but it has a dependency on php-http/message. Shouldn't it be the other way around?

I believe php-http/message should depend on php-http/message-factory, maybe I have missed something.

currently if i want to create a MultiPart Form Request (Form Fields + Files) i cant do this without relying on a concrete Implementation (like https://github.com/guzzle/psr7/blob/master/src/MultipartStream.php)

either the current StreamFactory can handle mixed Stream Inputs, or we need a whole new Factory

It currently says mixed. Shouldn't it be string|UriInterface instead?

• ServerMessageFactory
• UploadedFileFactory

PRs are very welcome

while working on the MessageHttpClient, i noticed that we have this signature:

createRequest(
        $method,
        $uri,
        $protocolVersion = '1.1',
        array $headers = [],
        $body = null
    );

I doubt that many people ever need to change the protocol version (if this should not be a setup instruction anyways). i would suggest that if this parameter is needed at all, it should be the last. method and uri are required, headers and body are more likely to be needed to be specified. as is, i would have to pass '1.1' in every createRequest call which feels strange

We should support discovering PSR-17 factories and return them as PHP-HTTP message factories.

In the next major version we can probably drop support for PHP-HTTP factories?

The PSR-7 includes a StreamInterface but there is nothing about MultipartStreams. This issue discuss possible approaches to create an abstraction over how to create MultipartStreams. The possible solutions and alternatives has been discussed in length at #30 and on Slack and this is a summary.

Background

A HTTP message with a multipart stream looks like this. As you can see it is many bodies separeted with a boundary.

POST / HTTP1/1
User-Agent: curl/7.21.2 (x86_64-apple-darwin)
Host: localhost:8080
Accept: */*
Content-Length: 1143
Expect: 100-continue
Content-Type: multipart/form-data; boundary=----------------------------83ff53821b7c

------------------------------83ff53821b7c
Content-Disposition: form-data; name="img"; filename="a.png"
Content-Type: application/octet-stream

?PNG

IHD?wS??iCCPICC Profilex?T?kA?6n??Zk?x?"IY?hE?6?bk
Y?<ߡ)??????9Nyx?+=?Y"|@5-?�M?S?%?@?H8??qR>?׋??inf???O?????b??N?????~N??>?!?
??V?J?p?8?da?sZHO?Ln?}&???wVQ?y?g????E??0
 ??
   IDAc????????-IEND?B`?
------------------------------83ff53821b7c
Content-Disposition: form-data; name="foo"

bar
------------------------------83ff53821b7c--

The boundary

The boundary is what separates two streams in the request. It is usually just a random list of chars but in some special use case you may want to define your custom boundary. I guess a use case for a custom boundary is when the POST data is similar to the randomized boundary, so you want to be 100% sure that the boundary is never found within the POST body by providing a boundary yourself.

This is how you use the boundary when sending a request:

$multipartStream = getMultipartStream('somehow..');
$boundary = $multipartStream->getBoundary();

$request = MessageFactoryDiscovery::find()->createRequest(
  'POST',
  'http://example.com',
  ['Content-Type' => 'multipart/form-data; boundary='.$boundary],
  $multipartStream
);
$response = HttpClientDiscovery::find()->sendRequest($request);

Implementations

The only implementation of a MultipartStream is by Guzzle. Zend Diactoros has no support but we could create a MultipartStreamBuilder that are using Guzzle or Diactoros. See POC: https://github.com/Nyholm/MultipartStreamBuilder

A builder does the same thing as a factory but with a simpler API. Since we have factories for Streams, Uri, Request, Responses etc we should have e factory for MultipartStreams as well. The builder could, however, be an implementation detail in the DiactorosMultipartStreamFactory.

The problems

We (HTTPlug) want to create a MultipartStreamFactory that developers can rely upon to create a MultipartStream (Just as we do with the StreamFactory, UriFactory, RequestFactory etc..). Since there is no MultipartStreamInterface in PSR7 there is no clear solution and a lot of questions.

1) Parameters

What parameters should be used when calling MultipartStreamFactory::createMultipartStream?
We ​_need_​ "formname", "resource", "headers", "filename" and possibly other “options”. Guzzle is using a multidimensional array. It is quite confusing but it does make sense.

    /**
     * @param array  $elements Array of associative arrays, each containing a
     *                         required "name" key mapping to the form field,
     *                         name, a required "contents" key mapping to a
     *                         StreamInterface/resource/string, an optional
     *                         "headers" associative array of custom headers,
     *                         and an optional "filename" key mapping to a
     *                         string to send as the filename in the part.
     * @param string $boundary You can optionally provide a specific boundary
     *
     * @throws \InvalidArgumentException
     */
    public function __construct(array $elements = [], $boundary = null)

What we also have discussed is:

1.a createMultipartStream(array $data)
1.b createMultipartStream(array $data, &$boundary = null)
2. createMultipartStream(...$part)
3. createMultipartStream(array $names, array $resources, array $headers, array $options)
4. createMultipartStream(AValueObject[] $data)
5. createMultipartStream(array &$options, ...$part)

Using an undefined number of parameters makes it difficult to call..:
Since PHP 5.6 you can use ...$var to unpack variables. Using variadics (option 2 & 5) may look like this:

$files = [StreamInterfaces]
foreach ($files as $file) {
  // build a nice formatted $parts
}

$stream $factory->getMultipartStream($options, ...$parts);

_I believe the best approach is doing something like Guzzle_

2) Return value

The MultipartStream is just a special kind of StreamInterface. To use a MultipartStream we need the boundary. We cannot have a MultipartStreamFactory::getBoundary because factories should be stateless. We could either use a output parameter or return a value object that holds a StreamInterface and the boundary.

class MultipartStream
{
    private $stream;
    private $boundary;

    public function __construct(StreamInterface $stream, $boundary)
    {
        $this->stream = $stream;
        $this->boundary = $boundary;
    }

    public function getStream()
    {
        return $this->stream;
    }

    public function getBoundary()
    {
        return $this->boundary;
    }
}

_I believe the best approach is using a value object_

3) What resources to support?

The StreamFactory converts strings, StreamInterfaces and file resources to StreamInterfaces. Should the MultipartStreamFactory do the same? Should we only allow StreamInterfaces?

We have also discussed allowing paths to files. That would however lead to a lot more complex error handling and more potential exceptions.

_I believe the best approach is using the same resources as the StreamFactory_

4) Should MultipartStreamFactory extend StreamFactory

The Interface Segregation Principle says that no client should be forced to depend on methods it does not use. So the question we should ask ourselves:

Will a client that creates multipart streams also create normal streams?

One could also argue about DX. An implementation of MultipartStreamFactory will most likely be a StreamFactory as well. If a client needs both a MultipartStreamFactory and StreamFactory it would be weird to be forced to declare with two constrictor arguments for the same object.

public function __construct(StreamFactory $stream, MultipartStreamFactory $multipartStream)

_I believe the best approach is to let the MultipartStreamFactory extend the StreamFactory_

What to consider?

Does my/our suggestions make sense? Anything else we should consider? Would it be easy to use a MultipartStreamFactory like this?

Configurations, contrib, conduct, etc.

Is there an interest in setting up Travis on this repo

@dbu You mentioned that in some cases we might want to typehint to RequestFactory, rather than MessageFactory. In that case using RequestFactory as a binding type would make sense as well.