Problem/Motivation

In #3038453-4: When RevisionableEntityBundleInterface::shouldCreateNewRevision is TRUE, create a new revision on PATCH requests and add an entry in the revision log. @Wim Leers reported:

✅ Ah, \Drupal\Core\Entity\ContentEntityStorageBase has a $this->latestRevisionIds static cache, which without this ::resetCache() call would result in the test assertions being unable to correctly assert that a revision is indeed the latest revision. Static caches strike again!

We should not need a global reset cache to be able to fetch an unchanged entity.

Proposed resolution

Fix the cache reset logic for individual entities.

Remaining tasks

  • Write a patch
  • Reviews

User interface changes

None

API changes

None

Data model changes

None

Release notes snippet

None

Issue fork drupal-3038706

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:

About issue forks

Comments

plach created an issue. See original summary.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
Related issues: +#3038453: When RevisionableEntityBundleInterface::shouldCreateNewRevision is TRUE, create a new revision on PATCH requests and add an entry in the revision log.

plach credited Wim Leers.

plach’s picture

plach
he/him
Primary language Italian
Location Venezia
commented
amateescu’s picture

amateescu commented

I bumped into the same problem in #2725523: Add a revision_parent field to revisionable entities and I fixed it like this:

diff --git a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
index 0040ae076a..f5e154abaf 100644
--- a/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
+++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
@@ -723,6 +723,7 @@ protected function doPostSave(EntityInterface $entity, $update) {
     if ($this->entityType->isRevisionable()) {
       $entity->updateLoadedRevisionId();
       $entity->setNewRevision(FALSE);
+      unset($this->latestRevisionIds[$entity->id()]);
     }
   }
wim leers’s picture

wim leers
Location Ghent 🇧🇪🇪🇺
commented
Issue tags: +Workflow Initiative, +API-First Initiative
Related issues: +#2725523: Add a revision_parent field to revisionable entities

#5: 👍

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.

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.

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.

mohit_aghera’s picture

mohit_aghera commented
Status: Active » Postponed (maintainer needs more info)

I came across this issue while doing bug-smash triage.
I see that we've already removed static cache in favour of memory cache in issue #3532741: Switching workspaces shouldn't clear the persistent entity cache

I believe this might not happen anymore since static cache is not in picture ?

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.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Title: ContentEntityStorageBase:: latestRevisionIds breaks ContentEntityStorageBase::loadUnchanged() » Clean up ::entityLoadUnchanged in ResourceTestBase
Component: entity system » jsonapi.module
Category: Bug report » Task
Priority: Major » Normal
Status: Postponed (maintainer needs more info) » Active
Related issues: +#3532741: Switching workspaces shouldn't clear the persistent entity cache

Yep this was removed in #3532741: Switching workspaces shouldn't clear the persistent entity cache but there is still a reference in the JSON:API tests that we need to clean up:

   * @todo Remove this after https://www.drupal.org/project/drupal/issues/3038706 lands.

Repurposing this issue to do that.

longwave’s picture

longwave
he/him
Primary language English
Location UK
commented
Status: Active » Needs review

longwave opened merge request !15028

berdir’s picture

berdir
Primary language German
Location Switzerland
commented
Status: Active » Needs work

Looks like this is causing several test fails.

This is a a different scenario I think, this is about resetting the in-memory cache to account for changes to the entity during a web request. In many cases in functional tests this now happens in web/core/lib/Drupal/Core/Test/RefreshVariablesTrait.php:25, but jsonapi tests don't trigger that.

We could optimize it a bit and only reset the specific entity that we're about to load or reset only the entity.memory service like RefreshVariablesTrait.