Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.ContentEntityBase::updateOriginalValues() is about internal housekeeping (syncing the FieldItemList values back into the "raw" values). It is currently only called by SqlContentEntityStorage::save().
Not sure why we need to do that housekeeping specifically at save time, but:
- why is this done in SqlContentEntityStorage specifically instead of ContentEntityStorageBase ?
- it doesn't feel right that the Storage is in charge of triggering that internal housekeeping ? The fact that ContentEntityBase has internal, protected $fields and $values properties that need to be synced is of no concern to the storage ?
- for that matter, updateOriginalValues() is not part of ContentEntityInterface, and so is not somehing that the storages should be able to call ?
| Comment | File | Size | Author |
|---|---|---|---|
| #4 | 2354199-CEB-updateOriginalValues-3.patch | 1.67 KB | yched |
| #1 | 2354199-CEB-updateOriginalValues-1.patch | 2.81 KB | yched |
Comments
Comment #1
yched CreditAttribution: yched commentedPatch inlines the content of CEB::updateOriginalValues() into CEB::preSave()
Comment #3
yched CreditAttribution: yched commentedInteresting. The only "fails" are notices in KeyValueContentEntityStorageTest.
On inspection, within KeyValueContentEntityStorageTest, the $entity being manupilated and saved has an internal $entity->values that is an array keyed by field name, and not by field name + langcode. Thus ContentEntityBase::preSave() doing $this->values[$name][$langcode] = ... throws notices, because $this->values[$name] is directly the value (a string).
No clue why that is - the same test entity type ('entity_test_label') is used in other tests which also create and save entities fine (but with the SQL storage). The only difference here is KeyValueContentEntityStorageTest alters the entity type to switch to KeyValueEntityStorage.
How can changing the storage class affect the internal structure of ContentEntityBase::$values ? (the phpdoc for that property says it's supposed to be keyed by name + langcode, for that matter)
Comment #4
yched CreditAttribution: yched commentedJust curious - let's see what breaks if we remove that code completely.
Comment #6
yched CreditAttribution: yched commentedJust one fail, and it's in editor ??
The plot thickens...