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:

  1. default_langcode gets a random boolean. About half the time a sample entity is
    created with default_langcode = FALSE. Once saved, it has no
    default translation row, so it loads with every field empty: label() returns NULL, and so do getChangedTime()
    and the rest. Tests that save sample entities fail
    at random because of this.
  2. Other managed fields are randomized too. langcode is picked from every language,
    including und and zxx, ignoring the bundle's
    configured default. The revision metadata fields and revision_translation_affected get random values.
    changed gets a random past timestamp that can fall before
    created.
  3. 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.
  4. 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, and FieldItemList::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 given

    from LinkGenerator;

  • its edit form throws InvalidArgumentException: The timestamp must be numeric. from
    NodeForm.

Steps to reproduce

  1. On the Page content type, add an entity reference field that targets Article, with "Create referenced entities if
    they don't already exist" enabled.
  2. Make sure no Article nodes exist.
  3. Run drush devel-generate:content 1 --bundles=page.
  4. Check the auto-created article:
    SELECT nid, langcode, default_langcode FROM node_field_data WHERE type =
        'article'

    . If default_langcode is 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_langcode to TRUE unless the caller passes a value.
  • Don't generate sample values for langcode, revision_translation_affected, the revision
    metadata keys, or fields of type changed, 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.

Issue fork drupal-2915034

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

tim.plunkett created an issue. See original summary.

tim.plunkett’s picture

Status: Active » Needs review
StatusFileSize
new4.17 KB
new7.33 KB

The last submitted patch, 2: 2915034-create_sample-2-FAIL.patch, failed testing. View results

Status: Needs review » Needs work

The last submitted patch, 2: 2915034-create_sample-2-PASS.patch, failed testing. View results

tim.plunkett’s picture

Status: Needs work » Needs review

Weird testbot fluke on that one.

amateescu’s picture

The patch looks pretty good, I just have one question:

+++ b/core/lib/Drupal/Core/Field/FieldItemList.php
@@ -260,8 +260,11 @@ public function view($display_options = []) {
+      if ($value = $field_type_class::generateSampleValue($field_definition)) {

Doesn't this mean that we're potentially not generating any sample values, and the calling code has no way of knowing that?

tim.plunkett’s picture

The 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.

amateescu’s picture

Status: Needs review » Reviewed & tested by the community

Awesome, no other concerns from me :)

Status: Reviewed & tested by the community » Needs work

The last submitted patch, 7: 2915034-create_sample-7.patch, failed testing. View results

eclipsegc’s picture

+++ b/core/lib/Drupal/Core/Field/FieldItemList.php
@@ -260,8 +260,12 @@ public function view($display_options = []) {
+    $values = $this->getValue();

What's this bit doing for us?

tim.plunkett’s picture

That 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...

eclipsegc’s picture

Ok, after reading, and rereading that chunk of code quite a few times, I get it and am on board.

Eclipse

Version: 8.5.x-dev » 8.6.x-dev

Drupal 8.5.0-alpha1 will be released the week of January 17, 2018, which means new developments and disruptive changes should now be targeted against the 8.6.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.6.x-dev » 8.7.x-dev

Drupal 8.6.0-alpha1 will be released the week of July 16, 2018, which means new developments and disruptive changes should now be targeted against the 8.7.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.7.x-dev » 8.8.x-dev

Drupal 8.7.0-alpha1 will be released the week of March 11, 2019, which means new developments and disruptive changes should now be targeted against the 8.8.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Version: 8.8.x-dev » 8.9.x-dev

Drupal 8.8.0-alpha1 will be released the week of October 14th, 2019, which means new developments and disruptive changes should now be targeted against the 8.9.x-dev branch. (Any changes to 8.9.x will also be committed to 9.0.x in preparation for Drupal 9’s release, but some changes like significant feature additions will be deferred to 9.1.x.). For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

ghost of drupal past’s picture

Version: 8.9.x-dev » 9.1.x-dev

Drupal 8.9.0-beta1 was released on March 20, 2020. 8.9.x is the final, long-term support (LTS) minor release of Drupal 8, which means new developments and disruptive changes should now be targeted against the 9.1.x-dev branch. For more information see the Drupal 8 and 9 minor version schedule and the Allowed changes during the Drupal 8 and 9 release cycles.

Version: 9.1.x-dev » 9.2.x-dev

Drupal 9.1.0-alpha1 will be released the week of October 19, 2020, which means new developments and disruptive changes should now be targeted for the 9.2.x-dev branch. For more information see the Drupal 9 minor version schedule and the Allowed changes during the Drupal 9 release cycle.

Version: 9.2.x-dev » 9.3.x-dev

Drupal 9.2.0-alpha1 will be released the week of May 3, 2021, which means new developments and disruptive changes should now be targeted for the 9.3.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

bradjones1’s picture

Status: Needs work » Reviewed & tested by the community

I'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.

bradjones1’s picture

Status: Reviewed & tested by the community » Needs review

Missed 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.

danflanagan8’s picture

Status: Needs review » Needs work

Tests are failing, so putting this back to NW.

bradjones1’s picture

Failure is predictably in Drupal\KernelTests\Core\Entity\CreateSampleEntityTest but doesn't jump out at me with anything obvious, will probably need local replication.

Version: 9.3.x-dev » 9.4.x-dev

Drupal 9.3.0-rc1 was released on November 26, 2021, which means new developments and disruptive changes should now be targeted for the 9.4.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.4.x-dev » 9.5.x-dev

Drupal 9.4.0-alpha1 was released on May 6, 2022, which means new developments and disruptive changes should now be targeted for the 9.5.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

Version: 9.5.x-dev » 10.1.x-dev

Drupal 9.5.0-beta2 and Drupal 10.0.0-beta2 were released on September 29, 2022, which means new developments and disruptive changes should now be targeted for the 10.1.x-dev branch. For more information see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

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

Drupal core is moving towards using a “main” branch. As an interim step, a new 11.x branch has been opened, as Drupal.org infrastructure cannot currently fully support a branch named main. New developments and disruptive changes should now be targeted for the 11.x branch, which currently accepts only minor-version allowed changes. For more information, see the Drupal core minor version schedule and the Allowed changes during the Drupal core release cycle.

patryk padus’s picture

Drupal 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

patryk padus’s picture

Path not apply on branch 9.x (latest I test is 9.5.1)

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.

mortona2k made their first commit to this issue’s fork.

mortona2k’s picture

Status: Needs work » Needs review

I rerolled and fixed tests by saving and reloading each sample entity before validation.

The second commit also stops createWithSampleValues() from randomizing langcode, the revision metadata and changed fields, so sample entities get the bundle's default language, correct revision flags, and a changed time that can't be earlier than created.

New tests cover both fixes.

The bug shows up in practice with devel generate: an auto-created entity reference target saved with default_langcode = 0 has no default translation and loads with a NULL label, which throws a TypeError on pages linking to it and "The timestamp must be numeric" on its edit form.

I used claude to reroll and make improvements.

smustgrave’s picture

Status: Needs review » Needs work
Issue tags: +Needs issue summary update

Can the IS be completed please.

Have not reviewed.

mortona2k’s picture

Issue summary: View changes
Status: Needs work » Needs review
Issue tags: -Needs issue summary update