Problem/Motivation
When a module provides base fields for the User entity type, and uses an entity query in hook_entity_base_field_info() to determine them, the site will get into an infinite loop when clearing caches:
- the entity field manager tries to build the list of field definitions for the User entity type
- the custom hook_entity_base_field_info() implementation will start an entity query
- \Drupal\workspaces\EntityQuery\QueryTrait::prepare() checks whether there's an active workspace
- \Drupal\workspaces\Negotiator\SessionWorkspaceNegotiator::applies() tries to check whether the current user is authenticated
- \Drupal\Core\Session\AccountProxy::getAccount() will try to load the user entity, and eventually ends up trying to build the list of field definitions again
Proposed resolution
Swap the order of the conditions in \Drupal\workspaces\EntityQuery\QueryTrait::prepare().
This fixes the problem because the User entity type can not be supported by workspaces (not revisionable nor publishable), so the isEntityTypeSupported() check will return FALSE and it won't advance to the following hasActiveWorkspace() check.
Remaining tasks
Review.
User interface changes
Nope.
API changes
Nope.
Data model changes
Nope.
Release notes snippet
Nope.
| Comment | File | Size | Author |
|---|
Issue fork drupal-3395626
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
amateescu commentedThis should do it.
Adding a test for this one-line fix would be quite hard, because we'd have to simulate the conditions described in the issue summary and somehow add a safeguard for the infinite loop. Per #2972776: [policy, no patch] Better scoping for bug fix test coverage, I think we shouldn't require a test here.
Comment #3
amateescu commentedComment #4
smustgrave commentedAs submaintainer of this component going to rely on you that this won't need a test case.
Change seems simple enough and didn't break anything.
Comment #5
needs-review-queue-bot commentedThe Needs Review Queue Bot tested this issue.
While you are making the above changes, we recommend that you convert this patch to a merge request. Merge requests are preferred over patches. Be sure to hide the old patch files as well. (Converting an issue to a merge request without other contributions to the issue will not receive credit.)
Comment #7
amateescu commentedConverted the patch to a MR.
Comment #8
catchThis sounds like a very bad idea, but also something that doesn't need explicit test coverage for. The condition is the same, it's just returning early from the other half.
Committed/pushed to 11.x and cherry-picked to 10.2.x, thanks!
Comment #11
catch