Problem/Motivation

I'm getting ahead of myself with this, but I came across one likely incompatibility with Drupal 9.2.x branch on PHP 8.1:

In PHP 8.1, using the Serializable interface will result in a PHP deprecation notice. Drupal has one such hit.

Steps to reproduce

Run Drupal 9 on PHP 8.1/nightly builds.

Proposed resolution

Implement the PHP 7.4 `__unserialize` and `__serialize` methods in addition to the `serialize` and `unserialize` from the interface.

Remaining tasks

Implement the PHP 7.4 `__unserialize` and `__serialize` methods in addition to the `serialize` and `unserialize` from the interface.

User interface changes

None.

API changes

None.

Data model changes

The serialized format/string might be different, although I suspect this is an unlikely scenario.

Release notes snippet

Todo.

Comments

Ayesh created an issue. See original summary.

andypost’s picture

Title: [PHP 8.1] Unserializable interface deprecation » [PHP 8.1] Serializable interface is deprecated
andypost’s picture

andypost’s picture

There's more usages

core9$ git grep Serializable
core/lib/Drupal/Component/Render/MarkupInterface.php:32:interface MarkupInterface extends \JsonSerializable {
core/lib/Drupal/Core/Extension/Extension.php:8: * This class does not implement the Serializable interface since problems
core/lib/Drupal/Core/Menu/MenuTreeParameters.php:15:class MenuTreeParameters implements \Serializable {
core/modules/locale/tests/src/Kernel/LocaleTranslationTest.php:24:  public function testSerializable() {
core/modules/views/src/ViewExecutable.php:21: * This class does not implement the Serializable interface since problems
core/tests/Drupal/Tests/Core/Entity/EntityTypeTest.php:515:  public function testIsSerializable() {
core/tests/Drupal/Tests/Core/Entity/EntityTypeTest.php:519:    $translation->willImplement(\Serializable::class);
core/tests/Drupal/Tests/Core/TempStore/SharedTempStoreTest.php:361:    $request->willImplement('\Serializable');
andypost’s picture

Version: 9.2.x-dev » 9.3.x-dev
catch’s picture

Priority: Normal » Critical
andypost’s picture

Status: Active » Needs review
StatusFileSize
new592 bytes

Guess kinda it, but ?string (from serialize) vs array from interface

andypost’s picture

StatusFileSize
new2.1 KB
new1.59 KB

Status: Needs review » Needs work

The last submitted patch, 8: 3212021-8.patch, failed testing. View results

andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new519 bytes
new1.6 KB

New method does not needs unserialize()

daffie’s picture

Status: Needs review » Needs work
  1. +++ b/core/lib/Drupal/Core/Menu/MenuTreeParameters.php
    @@ -212,6 +212,24 @@ public function excludeRoot() {
    +    return serialize(self::__serialize());
    

    Maybe a stupid question, but did you not do return serialize($this->__serialize());?

  2. +++ b/core/lib/Drupal/Core/Menu/MenuTreeParameters.php
    @@ -212,6 +212,24 @@ public function excludeRoot() {
    +  public function unserialize($serialized) {
    +    foreach (unserialize($serialized) as $key => $value) {
    +      $this->{$key} = $value;
    +    }
    

    Why do we do here the same as the method __unserialize()? Can we call that method instead to do the work?

  3. +++ b/core/lib/Drupal/Core/Menu/MenuTreeParameters.php
    @@ -212,6 +212,24 @@ public function excludeRoot() {
    +    return $this;
    

    Why does the method unserialize() returns $this? I get that we cannot remove this, because it will be a BC break. Just strange to me.

heni_deepak’s picture

StatusFileSize
new91.98 KB

Please review the error I have added in the screenshot.

andypost’s picture

Status: Needs work » Needs review
StatusFileSize
new728 bytes
new1.55 KB

Thank you, fixed

daffie’s picture

Status: Needs review » Reviewed & tested by the community

All code changes look good to me.
It fixes the problem from the IS.
I could not find any other classes that implement \Serializable in core.
For me it is RTBC.

catch’s picture

Status: Reviewed & tested by the community » Fixed

Committed 4d2631d and pushed to 9.3.x. Thanks!

  • catch committed 4d2631d on 9.3.x
    Issue #3212021 by andypost, heni_deepak, Ayesh, daffie: [PHP 8.1]...
catch’s picture

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.