Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow using serializers with other PSR implementations #43

Open
matt-allan opened this issue Jun 16, 2020 · 7 comments
Open

Allow using serializers with other PSR implementations #43

matt-allan opened this issue Jun 16, 2020 · 7 comments

Comments

@matt-allan
Copy link

matt-allan commented Jun 16, 2020

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
Copy link
Member

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

@weierophinney
Copy link
Member

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
Copy link
Author

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
Copy link
Member

@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
Copy link
Member

Xerkus commented May 1, 2023

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

@weierophinney weierophinney added this to the 3.0.0 milestone May 1, 2023
@weierophinney
Copy link
Member

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

I think the plan should be:

  • Finalize laminas-diactoros-serializer (and instead name it laminas-psr7-serializer)
  • Once that's released, in a minor version, we update the implementations here to proxy to those classes
  • Next major, we remove the implementations

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 weierophinney modified the milestones: 3.0.0, 3.1.0 May 3, 2023
@weierophinney
Copy link
Member

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.

@Ocramius Ocramius removed this from the 3.1.0 milestone Aug 1, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants