Contrib.social

Layout Builder attempts to builds section to determine if it is disabled

Open on Drupal.org →
Created on 11 June 2019, almost 6 years ago
Updated 30 January 2023, over 2 years ago

Problem/Motivation

As long as the module is installed, \Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay::buildMultiple() will run for every entity displayed on the site.
The concept of Layout Builder "being enabled" for a given view mode is actually determined by any number of plugins, only 2 of which are provided by core.

This presents a performance hit for cases where the entity is rendered multiple times, or when it is possible to know that no SectionStorage plugin is active.

Proposed resolution

Allow SectionStorage plugins to indicate that they are not relevant for a given entity display (entity type, bundle, and view mode).

Remaining tasks

User interface changes

N/A

API changes

TBD

Data model changes

N/A

Release notes snippet

Original report

We ran into notices on a client site, which result in a combination of content moderation, layout builder, tokens module and pathauto. The notice appeared after saving a new node, and was triggered by token generation of path auto. Problem is that, layout builders builds its sections in \Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay::buildMultiple() - even if it's not enabled.

Another issue / probably a bug is that this triggers entity validation, i.e. when layout builder tries to validate whether a context is satisfied, it validates the entity. Potentially another bug - maybe there is already some issue for it?

This in turn lets the content moderation constraint try to load revision without ID (NULL), what results in the notice. Potentially another bug. 

Warning: array_flip(): Can only flip STRING and INTEGER values! in Drupal\Core\Entity\ContentEntityStorageBase->loadMultipleRevisions() (line 584 of core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php).

Drupal\Core\Entity\ContentEntityStorageBase->loadMultipleRevisions(Array) (Line: 551)
Drupal\Core\Entity\ContentEntityStorageBase->loadRevision(NULL) (Line: 156)
Drupal\content_moderation\Plugin\Validation\Constraint\ModerationStateConstraintValidator->getOriginalOrInitialState(Object) (Line: 113)
Drupal\content_moderation\Plugin\Validation\Constraint\ModerationStateConstraintValidator->validate(Object, Object) (Line: 191)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateConstraints(Object, '00000000438d59cb0000000052d7805e', Array) (Line: 146)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object) (Line: 153)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validateNode(Object, Array, 1) (Line: 99)
Drupal\Core\TypedData\Validation\RecursiveContextualValidator->validate(Object, Array, NULL) (Line: 90)
Drupal\Core\TypedData\Validation\RecursiveValidator->validate(Object, Array) (Line: 368)
Drupal\Core\Plugin\Context\ContextDefinition->isSatisfiedBy(Object) (Line: 77)
Drupal\Core\Plugin\Context\ContextHandler->Drupal\Core\Plugin\Context\{closure}(Object)
array_filter(Array, Object) (Line: 78)
Drupal\Core\Plugin\Context\ContextHandler->getMatchingContexts(Array, Object) (Line: 65)
Drupal\Core\Plugin\Context\ContextHandler->checkRequirements(Array, Array) (Line: 31)
Drupal\Core\Plugin\Context\ContextHandler->Drupal\Core\Plugin\Context\{closure}(Object)
array_filter(Array, Object) (Line: 37)
Drupal\Core\Plugin\Context\ContextHandler->filterPluginDefinitionsByContexts(Array, Array) (Line: 92)
Drupal\layout_builder\SectionStorage\SectionStorageManager->findByContext(Array, Object) (Line: 297)
Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay->buildSections(Object) (Line: 261)
Drupal\layout_builder\Entity\LayoutBuilderEntityViewDisplay->buildMultiple(Array) (Line: 28)
Drupal\custom_elements\CustomElementsLayoutBuilderEntityViewDisplay->buildMultiple(Array) (Line: 221)
Drupal\Core\Entity\Entity\EntityViewDisplay->build(Object) (Line: 456)
Drupal\Core\Entity\EntityViewBuilder->viewField(Object, Array) (Line: 243)
Drupal\Core\Field\FieldItemList->view(Array) (Line: 1626)
field_tokens('entity', Array, Array, Array, Object)
call_user_func_array('field_tokens', Array) (Line: 403)
Drupal\Core\Extension\ModuleHandler->invokeAll('tokens', Array) (Line: 304)
Drupal\Core\Utility\Token->generate('entity', Array, Array, Array, Object) (Line: 941)
token_tokens('node', Array, Array, Array, Object)
call_user_func_array('token_tokens', Array) (Line: 403)
Drupal\Core\Extension\ModuleHandler->invokeAll('tokens', Array) (Line: 304)
Drupal\Core\Utility\Token->generate('node', Array, Array, Array, Object) (Line: 196)
Drupal\Core\Utility\Token->replace('[node:field_seo_title]', Array, Array, Object) (Line: 212)
Drupal\pathauto\PathautoGenerator->createEntityAlias(Object, 'insert') (Line: 364)
Drupal\pathauto\PathautoGenerator->updateEntityAlias(Object, 'insert') (Line: 86)
pathauto_entity_insert(Object)
call_user_func_array('pathauto_entity_insert', Array) (Line: 403)
Drupal\Core\Extension\ModuleHandler->invokeAll('entity_insert', Array) (Line: 206)
Drupal\Core\Entity\EntityStorageBase->invokeHook('insert', Object) (Line: 835)
Drupal\Core\Entity\ContentEntityStorageBase->invokeHook('insert', Object) (Line: 526)
Drupal\Core\Entity\EntityStorageBase->doPostSave(Object, ) (Line: 720)
Drupal\Core\Entity\ContentEntityStorageBase->doPostSave(Object, ) (Line: 303)
Drupal\multiversion\Entity\Storage\Sql\NodeStorage->doPostSave(Object, ) (Line: 451)
Drupal\Core\Entity\EntityStorageBase->save(Object) (Line: 838)
Drupal\Core\Entity\Sql\SqlContentEntityStorage->save(Object) (Line: 240)
Drupal\multiversion\Entity\Storage\Sql\NodeStorage->save(Object) (Line: 394)
Drupal\Core\Entity\EntityBase->save() (Line: 293)
Drupal\node\NodeForm->save(Array, Object)
call_user_func_array(Array, Array) (Line: 111)
Drupal\Core\Form\FormSubmitter->executeSubmitHandlers(Array, Object) (Line: 51)

Anyway, the root of the problem is that layout builder does stuff when it should not, so let's try to fix this here.

🐛 Bug report
Status

Needs work

Version

10.1 ✨

Component
Layout builder  →

Last updated 2 days ago

Created by

🇦🇹Austria fago Vienna

Live updates comments and jobs are added and updated live.
  • Blocks-Layouts

    Blocks and Layouts Initiative. See the #2811175 Add layouts to Drupal issue.

  • Performance

    It affects performance. It is often combined with the Needs profiling tag.

Sign in to follow issues

Merge Requests

Comments & Activities

Not all content is available!

It's likely this issue predates Contrib.social: some issue and comment data are missing.

  • Comment over 2 years ago →
    needs-review-queue-bot

    The Needs Review Queue Bot → tested this issue. It either no longer applies to Drupal core, or fails the Drupal core commit checks. Therefore, this issue status is now "Needs work".

    Apart from a re-roll or rebase, this issue may need more work to address feedback in the issue or MR comments. To progress an issue, incorporate this feedback as part of the process of updating the issue. This helps other contributors to know what is outstanding.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • Comment over 2 years ago →
    🇦🇺Australia acbramley

    Reroll #29 against 9.5.x and added some defense for OverridesSectionStorage::isSupported.

  • Comment over 2 years ago →
    🇦🇺Australia kim.pepper 🏄‍♂️🇦🇺Sydney, Australia
    +++ b/core/modules/layout_builder/src/Plugin/SectionStorage/DefaultsSectionStorage.php
@@ -407,4 +409,31 @@ public function setContext($name, ComponentContextInterface $context) {
+    if ($bundle) {
...
+    }

    I feel like this could be its own function, e.g. getSectionId()?

  • Comment over 2 years ago →
    🇮🇳India Akram Khan Cuttack, Odisha

    Added patch fixed CCf #33

  • Status changed to Needs review over 2 years ago
  • Comment over 2 years ago →
    🇮🇳India Akram Khan Cuttack, Odisha
  • Status changed to Needs work over 2 years ago
  • Comment over 2 years ago →
    🇺🇸United States smustgrave

    This issue is being reviewed by the kind folks in Slack, #needs-review-queue-initiative. We are working to keep the size of Needs Review queue [2700+ issues] to around 400 (1 month or less), following Review a patch or merge request → as a guide.

    As a bug this will need a test case showing the issue.

  • Comment about 1 year ago →
    🇦🇺Australia acbramley

    Rerolled onto an MR and fixed up a few typing/code style things.

  • Merge request !7340Issue #3060985 → (Open) created by acbramley
  • Pipeline finished with Success
    about 1 year ago
    Total: 616s
    #138211
  • Status changed to Needs review about 2 months ago
  • Comment about 2 months ago →
    🇦🇺Australia acbramley

    Rebased, fixed some minor issues, and added test coverage.

  • Pipeline finished with Canceled
    about 2 months ago
    Total: 157s
    #474515
  • Pipeline finished with Failed
    about 2 months ago
    Total: 951s
    #474517
  • Pipeline finished with Success
    about 2 months ago
    Total: 1002s
    #474533
  • Pipeline finished with Canceled
    about 2 months ago
    Total: 261s
    #475532
  • Pipeline finished with Success
    about 2 months ago
    #475535
  • Pipeline finished with Canceled
    about 2 months ago
    Total: 122s
    #475552
  • Pipeline finished with Success
    about 2 months ago
    Total: 681s
    #475553
  • Comment 27 days ago →
    🇺🇸United States smustgrave

    Seems already been reviewed and feedback has been addressed. Didn't see anything additional.

  • Comment 26 days ago →
    🇬🇧United Kingdom catch

    Agreed with @kimpepper's feedback on the MR. Didn't do an in-depth review of everything.

  • Pipeline finished with Failed
    26 days ago
    Total: 138s
    #490733
  • Pipeline finished with Failed
    26 days ago
    Total: 161s
    #490736
  • Pipeline finished with Failed
    26 days ago
    #490747
  • Comment 26 days ago →
    🇦🇺Australia acbramley

    Much nicer solution, thanks for the links. I didn't see any decisions on 🌱 [policy] Standardize how we implement in-memory caches Needs work with how these memory cache services should be setup wrt. service id names or how specific/generic they should be but I've loosely tried to follow what other things are doing in core already with cache.asset_memory and system.module_admin_links_memory_cache

  • Comment 26 days ago →
    🇦🇺Australia acbramley

    I've tried passing the display's cache tags into the set() call so, in theory, it should be invalidated automatically when the display is saved but that doesn't seem to be the case. Must be missing something?

  • Pipeline finished with Failed
    26 days ago
    #490776
  • Pipeline finished with Success
    26 days ago
    Total: 549s
    #490781
  • Comment 20 days ago →
    needs-review-queue-bot

    The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

    This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

    Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

  • First commit to issue fork.
  • Pipeline finished with Failed
    20 days ago
    Total: 499s
    #495096
  • Pipeline finished with Success
    20 days ago
    Total: 637s
    #495110
  • Comment 20 days ago →
    🇺🇸United States dcam

    Rebased MR 7340

  • Pipeline finished with Failed
    20 days ago
    Total: 258s
    #495353
  • Comment 20 days ago →
    🇳🇿New Zealand danielveza Brisbane, AU

    This has gone through multiple rounds of review, all threads have been resolved. I think it's ready to be RTBC.

  • Pipeline finished with Success
    20 days ago
    Total: 1107s
    #495369
contrib.social Blog FAQ Discussions
Production build 0.71.5 2024