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
- Remove
ComplexDataDefinitionBase::__sleep()entirely. This allows the parentDataDefinition::__sleep()to handle serialization, which already excludestypedDataManagerbut preserves all other properties includingpropertyDefinitions. - Move the
propertyDefinitionsexclusion intoEntityDataDefinition::__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
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
Comment #2
michaellander commentedAlso worth noting, if you look at
\Drupal\Core\TypedData\ListDataDefinition, we do not unsetitemDefinitionduring serialization. Which means there's a discrepancy between how they are handling their child definitions.Comment #3
michaellander commentedComment #5
michaellander commentedComment #6
michaellander commentedComment #7
andypostBoth
__sleep()and_wakeup()are soft-deprecated in PHP 8.5Please do not add more
Comment #8
michaellander commentedGood to know! I'm proposing removing the
_sleep()method override in this particular case, so win-win.Comment #9
oily commentedLooks like unit or kernel test coverage can be added given the steps to reproduce in the IS.
Comment #10
michaellander commentedI added test coverage.
Comment #11
oily commentedRe: #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
Comment #12
alexpottRemoving 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.
Comment #13
smustgrave commentedAppears to have test failures if those can be looked at.
Comment #15
michaellander commentedComment #16
michaellander commentedI was able to look into this a bit more, but it seems part of the issue is
MapDataDefinitiontreatspropertyDefinitionsas a real authored state, where asEntityDataDefinitiontreats it as a lazy derived state fromentity_field.manager. So I believe the fix is correct, aspropertyDefinitionsshouldn'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:
to
EntityDataDefinition.Comment #17
michaellander commentedAdded the approach that is probably most appropriate, but looking for feedback.
Comment #18
mradcliffeHmm, not sure why it wouldn't be getting it back via this pattern, which was always kind of weird anyway.
Comment #19
michaellander commented