laminas / laminas-diactoros

PSR HTTP Message implementations
https://docs.laminas.dev/laminas-diactoros/
BSD 3-Clause "New" or "Revised" License
484 stars 64 forks source link

Allow using serializers with other PSR implementations #43

Open matt-allan opened 4 years ago

matt-allan commented 4 years ago

Feature Request

Q A
New Feature yes
RFC no
BC Break no

Summary

Hey there 👋

I'm working on a project that could use the Laminas\Diactoros\Request\ArraySerializer and Laminas\Diactoros\Response\ArraySerializer. However I don't want to force the users of my package to use the laminas-diactoros PSR-7 implementation.

Any chance the serializers could be altered to use the PSR-17 factories and extracted to a separate package? I.e. something like this:

<?php

declare(strict_types=1);

namespace Laminas\Diactoros\Serializer;

use Psr\Http\Message\RequestFactoryInterface;
use Psr\Http\Message\StreamFactoryInterface;
use Psr\Http\Message\RequestInterface;
use Throwable;

use function sprintf;

final class ArraySerializer
{
    private RequestFactoryInterface $requestFactory;

    private StreamFactoryInterface $streamFactory;

    public function __construct(
        \Psr\Http\Message\RequestFactoryInterface $requestFactory,
        \Psr\Http\Message\StreamFactoryInterface $streamFactory
    ) {
        $this->requestFactory = $requestFactory;
        $this->streamFactory = $streamFactory;
    }

    public static function toArray(RequestInterface $request) : array
    {
        // ...
    }

    public function fromArray(array $serializedRequest) : RequestInterface
    {
        $uri = self::getValueFromKey($serializedRequest, 'uri');
        $method = self::getValueFromKey($serializedRequest, 'method');
        $request = $this->requestFactory->createRequest($method, $uri);
        $body = $this->streamFactory->createStream(self::getValueFromKey($serializedRequest,
                'body'));
        // ...
    }
}

Thanks!

weierophinney commented 4 years ago

I've actually been thinking about this for some time; thanks for the nudge! I'll try and schedule this for development soon.

weierophinney commented 4 years ago

I have this extracted here: https://github.com/weierophinney/laminas-diactoros-serializer

I'll be asking the @laminas/technical-steering-committee if we can bring this under the Laminas umbrella during the next TSC meeting (first Monday of July).

matt-allan commented 4 years ago

Awesome, thanks so much!

This is a huge help; I wrote my own serializers using identical serialization so this will just drop right in!

weierophinney commented 4 years ago

@matt-allan During the TSC meeting this week, some suggestions for improving the architecture were provided. I'm going to see which suggestions I can incorporate, and then will bring this back to the TSC. You can, of course, continue to use the package I've created in the meantime; just pin it to a sha1 when you do.

Xerkus commented 1 year ago

@weierophinney I think we will want to handle this for 3.0.0

weierophinney commented 1 year ago

actually... I'm removing from the 3.0 milestone, as it's a bigger issue.

I think the plan should be:

The problem I have with doing this for 3.0 is because there's likely a lot of work for this, and I'd rather not delay pushing out our PSR-7 v2 support for something that's (a) optional, (b) has already lingered years, and (c) is not clearly defined. Because of the history limits with free versions of Slack, I don't have the full discussion notes from when we discussed this in July, 2020. All I have are what are in the minutes from the TSC meeting that month, and... frankly it's vague (e.g. "like to see interfaces created for serialization", "might be better suited as hydrators/extractors") or clearly a larger effort (e.g. "can they create HTTP/2 messages as well as HTTP/1.1").

I'll still start working on it, but I'm going to push to 3.1.

weierophinney commented 1 year ago

I've prepared version 0.2.0 of laminas-psr7-serializer: https://github.com/weierophinney/laminas-psr7-serializer/releases/tag/0.2.0

I'll ask at the next TSC meeting if we can bring that into Laminas proper, and then we can work on updating the classes in Diactoros to proxy to that instead. When we do, we'll emit a deprecation notice, so folks know to update their code to the new library.

One thing to note: laminas-psr7-serializer does not define interfaces. It would be trivial to extract these, but essentially it becomes an interface per serialization type. The only place I see that being worthwhile is with array serialization, as that's the one place you could end up with different implementations. However, even there, it becomes problematic, because you wouldn't be able to rely on deserializing back to an object if the array structures vary, which makes me lean towards not providing interfaces.