Problem/Motivation

#918538: Decouple cache tags from cache bins makes Cache::invalidateTags() a simple wrapper for \Drupal::service('cache_tags.invalidator')->invalidateTags($tags);

As a follow-up, we could look into injecting this into services instead of calling out to the static method. That should allow us to simplify unit tests that currently need to set up a container.

Proposed resolution

Inject into new services and existing standalone services that are not commonly subclassed. (E.g., changing default plugin manager would be a bad idea at this point).

Remaining tasks

User interface changes

API changes

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

Berdir’s picture

Status: Postponed » Active

That got in...

Berdir’s picture

Issue tags: +Novice

This might work as a novice task, if you know a about about services and injection.

Start with just one example, I would suggest Token. inject the service, then call that instead of the static method. You probably need to update unit tests for that class.

Berdir’s picture

Another thing to consider is performance. The invalidator uses a tagged services approach, so instantiating it will also create all invalidators that are tagged (not the cache bins, though).

sidharthap’s picture

Status: Active » Needs review
FileSize
30.83 KB

Here is the initial patch. @Berdir please suggest...

Status: Needs review » Needs work

The last submitted patch, 4: 2398085-cache-invalidtags-4.patch, failed testing.

sidharthap’s picture

Assigned: Unassigned » sidharthap

working on it.

sidharthap’s picture

Status: Needs work » Needs review
FileSize
30.82 KB

Patch created from new D8 instance.

Status: Needs review » Needs work

The last submitted patch, 7: 2398085-cache-invalidtags-7.patch, failed testing.

sidharthap’s picture

Status: Needs work » Needs review
FileSize
30.82 KB

Status: Needs review » Needs work

The last submitted patch, 9: 2398085-cache-invalidtags-8.patch, failed testing.

sidharthap’s picture

Assigned: sidharthap » Unassigned
vlad.n’s picture

Status: Needs work » Needs review
FileSize
31.25 KB

Re-roll.

Status: Needs review » Needs work

The last submitted patch, 12: 2398085-cache-invalidtags-12.patch, failed testing.

Status: Needs work » Needs review
Wim Leers’s picture

Status: Needs review » Needs work
Issue tags: +php-novice
  1. +++ b/core/includes/module.inc
    @@ -60,6 +60,16 @@ function system_list_reset() {
    +  // Clear the library info cache.
    +  // Libraries may be provided by all extension types, and may be altered by any
    +  // other extensions (types) due to the nature of
    +  // \Drupal\Core\Extension\ModuleHandler::alter() and the fact that profiles
    +  // are recorded and handled as modules.
    +  // @todo Trigger an event upon module install/uninstall and theme
    +  //   enable/disable, and move this into an event subscriber.
    +  // @see https://drupal.org/node/2206347
    +  \Drupal::service('cache_tags.invalidator')->invalidateTags(['config:core.extension']);
    

    This is not a simple replacement. This is an addition. Therefore out of scope IMO.

  2. +++ b/core/lib/Drupal/Core/Config/Config.php
    @@ -224,7 +224,7 @@ public function save() {
    +      \Drupal::service('cache_tags.invalidator')->invalidateTags($this->getCacheTags());
    
    @@ -241,7 +241,7 @@ public function save() {
    +    \Drupal::service('cache_tags.invalidator')->invalidateTags($this->getCacheTags());
    

    This, which is what the patch is doing everywhere, is not injecting. Please inject the service. Otherwise there's no point.

dawehner’s picture

The thing with the injection here is that the cache tags invalidator should not be needed on a normal page request
but just in a really rare circumstances so I'm curious whether its right to initialize them basically on every request, I guess there is no way around that?

+++ b/core/lib/Drupal/Core/Config/CachedStorage.php
@@ -131,6 +131,7 @@ public function write($name, array $data) {
       $this->cache->set($this->getCacheKey($name), $data);
+      \Drupal::service('cache_tags.invalidator')->invalidateTags(array($this::FIND_BY_PREFIX_CACHE_TAG));
       $this->findByPrefixCache = array();
       return TRUE;
     }
@@ -145,6 +146,7 @@ public function delete($name) {

@@ -145,6 +146,7 @@ public function delete($name) {
     // rebuilding the cache before the storage is gone.
     if ($this->storage->delete($name)) {
       $this->cache->delete($this->getCacheKey($name));
+      \Drupal::service('cache_tags.invalidator')->invalidateTags(array($this::FIND_BY_PREFIX_CACHE_TAG));
       $this->findByPrefixCache = array();
       return TRUE;
     }
@@ -160,6 +162,7 @@ public function rename($name, $new_name) {

@@ -160,6 +162,7 @@ public function rename($name, $new_name) {
     if ($this->storage->rename($name, $new_name)) {
       $this->cache->delete($this->getCacheKey($name));
       $this->cache->delete($this->getCacheKey($new_name));
+      \Drupal::service('cache_tags.invalidator')->invalidateTags(array($this::FIND_BY_PREFIX_CACHE_TAG));
       $this->findByPrefixCache = array();
       return TRUE;
     }

Isn't this also an addition?

Wim Leers’s picture

so I'm curious whether its right to initialize them basically on every request, I guess there is no way around that

Make it a proxy service? Seems very logical.

pwolanin’s picture

This seems to be just switching to \Drupal::service('cache_tags.invalidator') instead of injecting the dependency?

andypost’s picture

Yep all places should get service via proper DI

andypost’s picture

Version: 8.0.x-dev » 8.1.x-dev
radubutco’s picture

Assigned: Unassigned » radubutco
Issue tags: +SprintWeekend2016, +SprintWeekendTM
bogdan.racz’s picture

Assigned: radubutco » Unassigned
radubutco’s picture

Assigned: Unassigned » radubutco
Status: Needs work » Active
radubutco’s picture

This is still work in progress, uploading the patch for a possible review.

Wim Leers’s picture

Status: Active » Needs review

Let's see what testbot says :)

Status: Needs review » Needs work

The last submitted patch, 24: inject-2398085-24.patch, failed testing.

Wim Leers’s picture

Status: Needs work » Needs review

Version: 8.1.x-dev » 8.2.x-dev

Drupal 8.1.0-beta1 was released on March 2, 2016, which means new developments and disruptive changes should now be targeted against the 8.2.x-dev branch. For more information see the Drupal 8 minor version schedule and the Allowed changes during the Drupal 8 release cycle.

Mile23’s picture

Status: Needs review » Needs work
Issue tags: -Novice, -php-novice +\Drupal is an anti-pattern

There's basically no way this is a novice task.

Especially when you consider that ConfigEntityBase is one of the classes we'll be changing here. It inherits from Entity which you must then figure out how to inject services into.

Mile23’s picture

andypost’s picture

Last patch is totally wrong we can inject services into entity classes cos Node::create()
Basically only static calls should be replaced like #12 started

+++ b/core/lib/Drupal/Core/Config/Entity/ConfigEntityBase.php
@@ -122,10 +122,19 @@
+  public function __construct(CacheTagsInvalidatorInterface $cache_tags_invalidator, array $values, $entity_type) {
+    parent::__construct($cache_tags_invalidator, $values, $entity_type);

+++ b/core/lib/Drupal/Core/Entity/ContentEntityStorageBase.php
@@ -49,13 +58,23 @@
-  public function __construct(EntityTypeInterface $entity_type, EntityManagerInterface $entity_manager, CacheBackendInterface $cache) {
+  public function __construct(CacheTagsInvalidatorInterface $cache_tags_invalidator, EntityTypeInterface $entity_type, EntityManagerInterface $entity_manager, CacheBackendInterface $cache) {

this is no-go for entities

bogdan.racz’s picture

Assigned: radubutco » Unassigned
Fabianx’s picture

I think the trick here is to use setter injection instead of changing the arguments.

Berdir’s picture

No, there is no trick. entities do not support dependency injection.

Version: 8.2.x-dev » 8.3.x-dev

Drupal 8.2.0-beta1 was released on August 3, 2016, which means new developments and disruptive changes should now be targeted against the 8.3.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.3.x-dev » 8.4.x-dev

Drupal 8.3.0-alpha1 will be released the week of January 30, 2017, which means new developments and disruptive changes should now be targeted against the 8.4.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.4.x-dev » 8.5.x-dev

Drupal 8.4.0-alpha1 will be released the week of July 31, 2017, which means new developments and disruptive changes should now be targeted against the 8.5.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.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.

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.