Issue #3412556 by catch, smustgrave, andypost, longwave: Allow...

Issue #3412556 by catch, smustgrave, andypost, longwave: Allow needs_destruction services to run on page cache hits

Issue #3412556 by catch, smustgrave, andypost, longwave: Allow...
e5fc18c5 · Dave Long · 80d83c68 · e5fc18c5 · e5fc18c5 · e5fc18c5
Verified Commit e5fc18c5 authored 1 year ago by Dave Long
--- a/core/core.services.yml
+++ b/core/core.services.yml
@@ -1492,10 +1492,6 @@ services:
    class: Drupal\Core\StreamWrapper\TemporaryStream
    tags:
      - { name: stream_wrapper, scheme: temporary }
-  kernel_destruct_subscriber:
-    class: Drupal\Core\EventSubscriber\KernelDestructionSubscriber
-    calls:
-      - [setContainer, ['@service_container']]
  image.toolkit.manager:
    class: Drupal\Core\ImageToolkit\ImageToolkitManager
    arguments: ['@config.factory']

--- a/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterServicesForDestructionPass.php
+++ b/core/lib/Drupal/Core/DependencyInjection/Compiler/RegisterServicesForDestructionPass.php
@@ -6,7 +6,7 @@
 use Symfony\Component\DependencyInjection\Compiler\CompilerPassInterface;

 /**
- * Adds services with specific tags to "kernel_destruct_subscriber" service.
+ * Adds services to the "kernel.destructable_services" container parameter.
 *
 * Only services tagged with "needs_destruction" are added.
 *
@@ -18,15 +18,8 @@ class RegisterServicesForDestructionPass implements CompilerPassInterface {
   * {@inheritdoc}
   */
  public function process(ContainerBuilder $container) {
-    if (!$container->hasDefinition('kernel_destruct_subscriber')) {
-      return;
-    }
-
-    $definition = $container->getDefinition('kernel_destruct_subscriber');
    $services = $container->findTaggedServiceIds('needs_destruction');
-    foreach ($services as $id => $attributes) {
-      $definition->addMethodCall('registerService', [$id]);
-    }
+    $container->setParameter('kernel.destructable_services', array_keys($services));
  }

 }
--- a/core/lib/Drupal/Core/DrupalKernel.php
+++ b/core/lib/Drupal/Core/DrupalKernel.php
@@ -685,14 +685,21 @@ public function getServiceProviders($origin) {
   * @return void
   */
  public function terminate(Request $request, Response $response) {
-    // Only run terminate() when essential services have been set up properly
-    // by preHandle() before.
-    if (FALSE === $this->prepared) {
-      return;
-    }
-
    if ($this->getHttpKernel() instanceof TerminableInterface) {
-      $this->getHttpKernel()->terminate($request, $response);
+      // Only run terminate() when essential services have been set up properly
+      // by preHandle() before.
+      if ($this->prepared === TRUE) {
+        $this->getHttpKernel()->terminate($request, $response);
+      }
+      // For destructable services, always call the destruct method if they were
+      // initialized during the request. Destruction is not necessary if the
+      // service was not used.
+      foreach ($this->container->getParameter('kernel.destructable_services') as $id) {
+        if ($this->container->initialized($id)) {
+          $service = $this->container->get($id);
+          $service->destruct();
+        }
+      }
    }
  }


--- a/core/lib/Drupal/Core/EventSubscriber/KernelDestructionSubscriber.php
+++ b/core/lib/Drupal/Core/EventSubscriber/KernelDestructionSubscriber.php
@@ -11,6 +11,9 @@
 /**
 * Destructs services that are initiated and tagged with "needs_destruction".
 *
+ * @deprecated in drupal:10.3.0 and is removed from drupal:11.0.0. There is no
+ * replacement.
+ * @see https://www.drupal.org/node/3416021
 * @see \Drupal\Core\DestructableInterface
 */
 class KernelDestructionSubscriber implements EventSubscriberInterface, ContainerAwareInterface {

--- a/core/modules/user/src/EventSubscriber/UserRequestSubscriber.php
+++ b/core/modules/user/src/EventSubscriber/UserRequestSubscriber.php
@@ -60,9 +60,7 @@ public function onKernelTerminate(TerminateEvent $event) {
   * {@inheritdoc}
   */
  public static function getSubscribedEvents(): array {
-    // Should go before other subscribers start to write their caches. Notably
-    // before \Drupal\Core\EventSubscriber\KernelDestructionSubscriber to
-    // prevent instantiation of destructed services.
+    // Should go before other subscribers start to write their caches.
    $events[KernelEvents::TERMINATE][] = ['onKernelTerminate', 300];
    return $events;
  }

--- a/core/phpstan-baseline.neon
+++ b/core/phpstan-baseline.neon
@@ -674,22 +674,6 @@ parameters:
 			count: 2
 			path: lib/Drupal/Core/EventSubscriber/FinishResponseSubscriber.php

-		-
-			message: """
-				#^Class Drupal\\\\Core\\\\EventSubscriber\\\\KernelDestructionSubscriber implements deprecated interface Symfony\\\\Component\\\\DependencyInjection\\\\ContainerAwareInterface\\:
-				since Symfony 6\\.4, use dependency injection instead$#
-			"""
-			count: 1
-			path: lib/Drupal/Core/EventSubscriber/KernelDestructionSubscriber.php
-
-		-
-			message: """
-				#^Usage of deprecated trait Symfony\\\\Component\\\\DependencyInjection\\\\ContainerAwareTrait in class Drupal\\\\Core\\\\EventSubscriber\\\\KernelDestructionSubscriber\\:
-				since Symfony 6\\.4, use dependency injection instead$#
-			"""
-			count: 1
-			path: lib/Drupal/Core/EventSubscriber/KernelDestructionSubscriber.php
-
 		-
 			message: """
 				#^Call to deprecated method getFromDriverName\\(\\) of class Drupal\\\\Core\\\\Extension\\\\DatabaseDriverList\\:

--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryAuthenticatedPerformanceTest.php
@@ -37,7 +37,7 @@ public function testFrontPageAuthenticatedWarmCache(): void {
    }, 'authenticatedFrontPage');
    $this->assertGreaterThanOrEqual(15, $performance_data->getQueryCount());
    $this->assertLessThanOrEqual(16, $performance_data->getQueryCount());
-    $this->assertSame(43, $performance_data->getCacheGetCount());
+    $this->assertSame(45, $performance_data->getCacheGetCount());
    $this->assertSame(0, $performance_data->getCacheSetCount());
    $this->assertSame(0, $performance_data->getCacheDeleteCount());
  }

--- a/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryFrontPagePerformanceTest.php
+++ b/core/profiles/demo_umami/tests/src/FunctionalJavascript/OpenTelemetryFrontPagePerformanceTest.php
@@ -48,9 +48,14 @@ public function testFrontPageHotCache() {
    // in the browser cache.
    $this->drupalGet('<front>');
    $this->drupalGet('<front>');
-    $this->collectPerformanceData(function () {
+    $performance_data = $this->collectPerformanceData(function () {
      $this->drupalGet('<front>');
-    }, 'umamiFrontPageWarmCache');
+    }, 'umamiFrontPageHotCache');
+    $this->assertSession()->pageTextContains('Umami');
+    $this->assertSame(1, $performance_data->getQueryCount());
+    $this->assertSame(1, $performance_data->getCacheGetCount());
+    $this->assertSame(0, $performance_data->getCacheSetCount());
+    $this->assertSame(0, $performance_data->getCacheDeleteCount());
  }

  /**

--- a/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
+++ b/core/profiles/standard/tests/src/FunctionalJavascript/StandardPerformanceTest.php
@@ -55,8 +55,8 @@ public function testAnonymous() {
      $this->drupalGet('');
    }, 'standardFrontPage');
    $this->assertNoJavaScript($performance_data);
-    $this->assertSame(66, $performance_data->getQueryCount());
-    $this->assertSame(135, $performance_data->getCacheGetCount());
+    $this->assertSame(68, $performance_data->getQueryCount());
+    $this->assertSame(137, $performance_data->getCacheGetCount());
    $this->assertSame(47, $performance_data->getCacheSetCount());
    $this->assertSame(0, $performance_data->getCacheDeleteCount());

@@ -66,8 +66,8 @@ public function testAnonymous() {
    });
    $this->assertNoJavaScript($performance_data);

-    $this->assertSame(38, $performance_data->getQueryCount());
-    $this->assertSame(94, $performance_data->getCacheGetCount());
+    $this->assertSame(39, $performance_data->getQueryCount());
+    $this->assertSame(95, $performance_data->getCacheGetCount());
    $this->assertSame(16, $performance_data->getCacheSetCount());
    $this->assertSame(0, $performance_data->getCacheDeleteCount());

@@ -77,8 +77,8 @@ public function testAnonymous() {
      $this->drupalGet('user/' . $user->id());
    });
    $this->assertNoJavaScript($performance_data);
-    $this->assertSame(40, $performance_data->getQueryCount());
-    $this->assertSame(80, $performance_data->getCacheGetCount());
+    $this->assertSame(41, $performance_data->getQueryCount());
+    $this->assertSame(81, $performance_data->getCacheGetCount());
    $this->assertSame(16, $performance_data->getCacheSetCount());
    $this->assertSame(0, $performance_data->getCacheDeleteCount());
  }
@@ -106,8 +106,8 @@ public function testLogin(): void {
    });

    $this->assertGreaterThanOrEqual(38, $performance_data->getQueryCount());
-    $this->assertLessThanOrEqual(39, $performance_data->getQueryCount());
-    $this->assertSame(62, $performance_data->getCacheGetCount());
+    $this->assertLessThanOrEqual(40, $performance_data->getQueryCount());
+    $this->assertSame(64, $performance_data->getCacheGetCount());
    $this->assertSame(1, $performance_data->getCacheSetCount());
    $this->assertSame(1, $performance_data->getCacheDeleteCount());
  }
@@ -136,8 +136,8 @@ public function testLoginBlock(): void {
    $performance_data = $this->collectPerformanceData(function () use ($account) {
      $this->submitLoginForm($account);
    });
-    $this->assertSame(47, $performance_data->getQueryCount());
-    $this->assertSame(83, $performance_data->getCacheGetCount());
+    $this->assertSame(49, $performance_data->getQueryCount());
+    $this->assertSame(85, $performance_data->getCacheGetCount());
    $this->assertSame(1, $performance_data->getCacheSetCount());
    $this->assertSame(1, $performance_data->getCacheDeleteCount());
  }