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.

Support from Acquia helps fund testing for Drupal Acquia logo

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
FileSize
592 bytes

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

andypost’s picture

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
FileSize
519 bytes
1.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

FileSize
91.98 KB

Please review the error I have added in the screenshot.

andypost’s picture

Status: Needs work » Needs review
FileSize
728 bytes
1.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.