Inject cache_tags.invalidator instead of using Cache::invalidateTags()

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

working on it.

Patch created from new D8 instance.

Re-roll.

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.

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?

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.

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

Yep all places should get service via proper DI

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

Let's see what testbot says :)

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.

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

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

Still a valid task?

The discussion in #31-34 still stands, the entity handlers can't use dependency injection still.

However, there is no real reason for Cache::invalidateTags() to exist as a wrapper - the static method predates the \Drupal class iirc. We could deprecate that and use \Drupal::service('cache_tags.invalidator')->invalidateTags($tags); in those places, and use injection everywhere else.