Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:104472
MIME-Version: 1.0
References: <CAF+90c-GfwWSJ+LUUYrb6gpZRrwP=XX=hWFD9DL-3xQhBOgp+A@mail.gmail.com>
 <CAOWwgp=Wh1JUUYYx7WhDyk_1w5tHF3AX4K46mkpoYS=yWz7ihg@mail.gmail.com>
In-Reply-To: <CAOWwgp=Wh1JUUYYx7WhDyk_1w5tHF3AX4K46mkpoYS=yWz7ihg@mail.gmail.com>
Date: Tue, 19 Feb 2019 11:31:10 +0100
Message-ID: <CAF+90c-jiNwT6e3o30B6_x2_waZ79XAYvHN3_8dOc31mU+KvhA@mail.gmail.com>
To: Nicolas Grekas <nicolas.grekas+php@gmail.com>
Cc: PHP internals <internals@lists.php.net>
Content-Type: multipart/alternative; boundary="0000000000007fefaf05823cbe49"
Subject: Re: [RFC] New custom object serialization mechanism
From: nikita.ppv@gmail.com (Nikita Popov)

--0000000000007fefaf05823cbe49
Content-Type: text/plain; charset="UTF-8"

On Mon, Feb 18, 2019 at 4:12 PM Nicolas Grekas <nicolas.grekas+php@gmail.com>
wrote:

> Hi Nikita,
>
> I'd like to propose a new custom object serialization mechanism intended
>> to replace the broken Serializable interface:
>>
>> https://wiki.php.net/rfc/custom_object_serialization
>>
>> This was already previously discussed in
>> https://externals.io/message/98834, this just brings it into RFC form.
>> The latest motivation for this is https://bugs.php.net/bug.php?id=77302,
>> a compatibility issue in 7.3 affecting Symfony, caused by Serializable. We
>> can't fix Serializable, but we can at least make sure that a working
>> alternative exists.
>>
>
> Is there anything we can to do to help the RFC move forward? I'm spending
> a great deal of time removing Serializable from the Symfony code base. It's
> not pretty nor fun... but we have no choice since as ppl move to PHP 7.3,
> they can now spot when the current mechanism is breaking their serialized
> payloads (typically from user objects put in the session storage).
>
> About interface vs magic methods, technicaly, we can work with an
> interface provided by PHP core, so that if that's a blocker for voters to
> approve the RFC, let's do it - the current situation is really aweful :).
> FYI, I found myself implementing some getState()/setState() methods of my
> own, decoupled from the underlying mechanisms used by PHP. That allows me
> to still use Serializable for BC reasons when needed, or  move to __sleep
> when possible, then move to the new 7.4 style when ready, without requiring
> any changes from the consumer POV. That's a good illustration of what I
> meant in my previous email: domain-specific interfaces in everyone's code
> is a better design as it allows better decoupling. It's also a better
> design to express that "instances of this interface of mine MUST be
> serializable". IMHO.
>
> Nicolas
>

I think for me the main open question here is whether we want to just
handle the serialization issue or try to come up with something more
general. If we limit this to just serialization, then the design should
stay as-is -- for all the reasons you have already outlined, using an
interface for this is inappropriate.

For a more general mechanism, I think we would need something along the
lines of (ignoring naming):

interface GetState {
    public function getState(string $purpose): array;
}
interface SetState {
    public function setState(array $data, string $purpose): void;
}

We need separate interfaces for get/set to properly cater to cases like
JSON, where only get is meaningful (right now). We need the $purpose
argument to allow different state representations for different purposes,
e.g. JSON will often need more preparation than PHP serialization. The
$purpose argument is a string, to allow extension for custom purposes.

Seeing that, this is really not something I would feel comfortable with
introducing in core PHP. Our track record for introducing reusable
general-purpose interfaces is not exactly stellar (say, did you hear about
SplObserver and SplSubject?) and I wouldn't want to do this without seeing
the concept fleshed out extensively in userland first.

Nikita

--0000000000007fefaf05823cbe49--