Problem/Motivation

When data of type Map is serialized and then unserialized, it loses its propertyDefinitions because ComplexDataDefinitionBase::__sleep() unconditionally unsets them during serialization.

The method treats propertyDefinitions as a disposable cache in all cases:

public function __sleep(): array {
  $vars = get_object_vars($this);
  unset($vars['propertyDefinitions'], $vars['typedDataManager']);
  return array_keys($vars);
}

However, this assumption is only valid for EntityDataDefinition, where propertyDefinitions is a lazily-derived state computed from entity_field.manager. For MapDataDefinition, propertyDefinitions is authored state — explicitly set via setPropertyDefinition() — and cannot be reconstructed after unserialization.

This surfaces in any scenario where a MapDataDefinition gets serialized and unserialized during a request. For example, a form using #ajax and #process callbacks that references a Map definition will lose its property definitions after form rebuild.

Additionally, ListDataDefinition does not unset its itemDefinition during serialization, creating an inconsistency in how child definitions are handled across complex and list data types.

Steps to reproduce

$map_definition = MapDataDefinition::create()
  ->setPropertyDefinition('status', DataDefinition::create('string'));
$typed_data = \Drupal::service('typed_data_manager')->create($map_definition);

// Before serialization: property definitions exist.
$before = $typed_data->getDataDefinition()->getPropertyDefinitions();
// ['status' => DataDefinition(string)]

$typed_data = unserialize(serialize($typed_data));

// After serialization: property definitions are gone.
$after = $typed_data->getDataDefinition()->getPropertyDefinitions();
// [] — empty!

Proposed resolution

  1. Remove ComplexDataDefinitionBase::__sleep() entirely. This allows the parent DataDefinition::__sleep() to handle serialization, which already excludes typedDataManager but preserves all other properties including propertyDefinitions.
  2. Move the propertyDefinitions exclusion into EntityDataDefinition::__sleep(), where it is appropriate. Entity data definitions lazily compute their property definitions from the entity field manager, so they should continue to exclude them during serialization (they will be re-derived on access after unserialization).

This is the minimal change: MapDataDefinition (and any other ComplexDataDefinitionBase subclass with authored property definitions) now correctly preserves its state across serialization, while EntityDataDefinition retains its existing lazy-load optimization.

Remaining tasks

Review and commit.

User interface changes

None.

Introduced terminology

None.

API changes

None. ComplexDataDefinitionBase::__sleep() was an internal implementation detail. Subclasses that relied on this behavior (only EntityDataDefinition in core) now have their own implementation.

Data model changes

None.

Release notes snippet

Fixed a bug where MapDataDefinition lost its propertyDefinitions when serialized. The ComplexDataDefinitionBase::__sleep() method incorrectly treated authored property definitions as a disposable cache. The exclusion logic was moved to EntityDataDefinition, where property definitions are genuinely lazily derived and should not be serialized.

Issue fork drupal-3557455

Command icon Show commands

Start within a Git clone of the project using the version control instructions.

Or, if you do not have SSH keys set up on git.drupalcode.org:

Comments

michaellander created an issue. See original summary.

michaellander’s picture

Also worth noting, if you look at \Drupal\Core\TypedData\ListDataDefinition, we do not unset itemDefinition during serialization. Which means there's a discrepancy between how they are handling their child definitions.

michaellander’s picture

Version: 11.3.x-dev » 11.x-dev

michaellander’s picture

Issue summary: View changes
michaellander’s picture

Status: Active » Needs review
andypost’s picture

Issue tags: +PHP 8.5

Both __sleep() and _wakeup() are soft-deprecated in PHP 8.5

Please do not add more

michaellander’s picture

Good to know! I'm proposing removing the _sleep() method override in this particular case, so win-win.

oily’s picture

Looks like unit or kernel test coverage can be added given the steps to reproduce in the IS.

michaellander’s picture

I added test coverage.

oily’s picture

Re: #10 started a code review.

There is a kernel test probably needs fixed. It looks like just one failure of an assert to deal with.

The functionaljavascript failure not sure how it is related?

However the test-only output is promising since it isolates the new test coverage and it fails:
https://git.drupalcode.org/issue/drupal-3557455/-/jobs/7278183

alexpott’s picture

Issue tags: -PHP 8.5

Removing the PHP 8.5 tag - this issue is not about making core PHP 8.5 compatibility - yes it should not make core incompatible but thats true of all issues.

smustgrave’s picture

Status: Needs review » Needs work

Appears to have test failures if those can be looked at.

Version: 11.x-dev » main

Drupal core is now using the main branch as the primary development branch. New developments and disruptive changes should now be targeted to the main branch.

Read more in the announcement.

michaellander’s picture

Status: Needs work » Needs review
michaellander’s picture

I was able to look into this a bit more, but it seems part of the issue is MapDataDefinition treats propertyDefinitions as a real authored state, where as EntityDataDefinition treats it as a lazy derived state from entity_field.manager. So I believe the fix is correct, as propertyDefinitions shouldn't be treated as disposable cache, but this could still mean entity definitions become heavier to serialize.

To preserve the existing lazy derived state(and to pass tests as they exist today) on entity definitions, we'd need to move:

public function __sleep(): array {
  // Do not serialize the lazily-computed entity field definitions.
  $vars = get_object_vars($this);
  unset($vars['propertyDefinitions'], $vars['typedDataManager']);
  return array_keys($vars);
}

to EntityDataDefinition.

michaellander’s picture

Added the approach that is probably most appropriate, but looking for feedback.

mradcliffe’s picture

Hmm, not sure why it wouldn't be getting it back via this pattern, which was always kind of weird anyway.

  public function getPropertyDefinitions() {
    if (!isset($this->propertyDefinitions)) {
      $this->propertyDefinitions = [];
    }
    return $this->propertyDefinitions;
  }
michaellander’s picture

Issue summary: View changes