Problem/Motivation
*I encountered this issue while generating content with devel_generate. I used claude to diagnose, patch, and update the summary.
ContentEntityStorageBase::createWithSampleValues() generates a random sample value for every field
except the ID, revision and bundle keys, including fields the entity
system is supposed to manage itself. This causes several problems:
default_langcodegets a random boolean. About half the time a sample entity is
created withdefault_langcode = FALSE. Once saved, it has no
default translation row, so it loads with every field empty:label()returns NULL, and so dogetChangedTime()
and the rest. Tests that save sample entities fail
at random because of this.- Other managed fields are randomized too.
langcodeis picked from every language,
includingundandzxx, ignoring the bundle's
configured default. The revision metadata fields andrevision_translation_affectedget random values.
changedgets a random past timestamp that can fall before
created. - The bundle is required even when the entity type supplies one. For example,
MenuLinkContent::preCreate()
adds a default bundle, but
createWithSampleValues()throws when none is passed. generateSampleValue()can return NULL, and that overwrites a field's default value.
Many field types don't implement it and return NULL from the base
class, andFieldItemList::generateSampleItems()sets the field to that NULL, replacing its default.
This affects real sites through EntityReferenceItem::generateSampleValue(). When a reference field has
no entity it can point to, it creates one with
createWithSampleValues(). Devel generate triggers this whenever an auto-create entity reference field
targets a bundle that has no content yet. About half the time, the
auto-created entity is saved without a default translation, and then:
- viewing a page that links to it throws
TypeError: Html::escape(): Argument #1 ($text) must be of type string, null givenfrom
LinkGenerator; - its edit form throws
InvalidArgumentException: The timestamp must be numeric.from
NodeForm.
Steps to reproduce
- On the Page content type, add an entity reference field that targets Article, with "Create referenced entities if
they don't already exist" enabled. - Make sure no Article nodes exist.
- Run
drush devel-generate:content 1 --bundles=page. - Check the auto-created article:
SELECT nid, langcode, default_langcode FROM node_field_data WHERE type = 'article'. If
default_langcodeis 0, view the
generated page while logged in, or open the article's edit form. If it is 1, delete the article and repeat; this
fails about half the time.
Proposed resolution
In createWithSampleValues():
- Set
default_langcodeto TRUE unless the caller passes a value. - Don't generate sample values for
langcode,revision_translation_affected, the revision
metadata keys, or fields of typechanged, so they keep
the values the entity system gives them. - Make the bundle optional. An unknown bundle still throws.
In FieldItemList::generateSampleItems(), keep the existing value when generateSampleValue()
returns NULL.
Implemented in MR17278. CreateSampleEntityTest now saves and reloads every sample entity before validating
it, and new tests cover the default translation and the managed
fields.
Remaining tasks
- Review.
- Decide whether the managed-field changes (
langcode, revision metadata,changed) belong
here or in a follow-up issue.
User interface changes
None.
API changes
createWithSampleValues() no longer throws when no bundle is passed for an entity type that has a bundle
key. It still throws for an unknown bundle.
Data model changes
None.
| Comment | File | Size | Author |
|---|---|---|---|
| #7 | 2915034-create_sample-7.patch | 7.4 KB | tim.plunkett |
| #7 | 2915034-create_sample-7-interdiff.txt | 713 bytes | tim.plunkett |
| #2 | 2915034-create_sample-2-PASS.patch | 7.33 KB | tim.plunkett |
| #2 | 2915034-create_sample-2-FAIL.patch | 4.17 KB | tim.plunkett |
Issue fork drupal-2915034
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
tim.plunkettComment #5
tim.plunkettWeird testbot fluke on that one.
Comment #6
amateescu commentedThe patch looks pretty good, I just have one question:
Doesn't this mean that we're potentially not generating any sample values, and the calling code has no way of knowing that?
Comment #7
tim.plunkettThe base implementation of generateSampleValue() returns NULL.
Many classes don't override that.
IMO, NULL is not a valid sample value.
However, FALSE is valid in the case of a Boolean, so I updated the patch.
Comment #8
amateescu commentedAwesome, no other concerns from me :)
Comment #10
eclipsegc commentedWhat's this bit doing for us?
Comment #11
tim.plunkettThat ensures any default values are persisted if there is no sample value for them.
Also, there is a value being returned that satisfies the conditional
($value !== NULL && !$value)that is causing a SAVEPOINT failure.I'm guessing it's a FALSE value for a BooleanItem on something that should only be TRUE in conjunction with the rest of everything...
Comment #12
eclipsegc commentedOk, after reading, and rereading that chunk of code quite a few times, I get it and am on board.
Eclipse
Comment #17
ghost of drupal pastComment #21
bradjones1I'm going to be bold and re-mark this as RTBC... just got bit by this, and I think it's worth hacking away at the bigger question of test coverage for the broader feature on the referenced issue.
Comment #23
bradjones1Missed a hunk that needed rebasing... marking this needs review, but I agree with the general sentiment above about fixing this, and then tackling the broader test coverage.
Comment #24
danflanagan8Tests are failing, so putting this back to NW.
Comment #25
bradjones1Failure is predictably in
Drupal\KernelTests\Core\Entity\CreateSampleEntityTestbut doesn't jump out at me with anything obvious, will probably need local replication.Comment #30
patryk padus commentedDrupal 9.x give issue:
Drupal\Core\Entity\EntityStorageException: No entity bundle was specified in Drupal\Core\Entity\ContentEntityStorageBase->createWithSampleValues() (line 230 w .../public_html/web/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).
If bundle isn't exist then get error. Bundle removed in Commerce when attribute removed from list but not from LayoutBuilder
Comment #31
patryk padus commentedPath not apply on branch 9.x (latest I test is 9.5.1)
Comment #35
mortona2k commentedI rerolled and fixed tests by saving and reloading each sample entity before validation.
The second commit also stops
createWithSampleValues()from randomizinglangcode, the revision metadata andchangedfields, so sample entities get the bundle's default language, correct revision flags, and a changed time that can't be earlier thancreated.New tests cover both fixes.
The bug shows up in practice with devel generate: an auto-created entity reference target saved with
default_langcode = 0has no default translation and loads with a NULL label, which throws aTypeErroron pages linking to it and "The timestamp must be numeric" on its edit form.I used claude to reroll and make improvements.
Comment #36
smustgrave commentedCan the IS be completed please.
Have not reviewed.
Comment #37
mortona2k commented