Issue #3478621 by catch, nicxvan, longwave, alexpott: Add filecache to OOP hook attribute parsing

core/lib/Drupal/Core/Hook/HookCollectorPass.php

@@ -6,6 +6,7 @@
 
 use Drupal\Component\Annotation\Doctrine\StaticReflectionParser;
 use Drupal\Component\Annotation\Reflection\MockFileFinder;
+use Drupal\Component\FileCache\FileCacheFactory;
 use Drupal\Core\Extension\ProceduralCall;
 use Drupal\Core\Hook\Attribute\Hook;
 use Drupal\Core\Hook\Attribute\LegacyHook;
@@ -156,6 +157,9 @@ public static function collectAllHookImplementations(array $module_filenames): s
    * @return void
    */
   protected function collectModuleHookImplementations($dir, $module, $module_preg): void {
+    $hook_file_cache = FileCacheFactory::get('hook_implementations');
+    $procedural_hook_file_cache = FileCacheFactory::get('procedural_hook_implementations:' . $module_preg);
+
     $iterator = new \RecursiveDirectoryIterator($dir, \FilesystemIterator::SKIP_DOTS | \FilesystemIterator::UNIX_PATHS | \FilesystemIterator::FOLLOW_SYMLINKS);
     $iterator = new \RecursiveCallbackFilterIterator($iterator, static::filterIterator(...));
     $iterator = new \RecursiveIteratorIterator($iterator);
@@ -163,32 +167,56 @@ protected function collectModuleHookImplementations($dir, $module, $module_preg)
     foreach ($iterator as $fileinfo) {
       assert($fileinfo instanceof \SplFileInfo);
       $extension = $fileinfo->getExtension();
+      $filename = $fileinfo->getPathname();
+
       if ($extension === 'module' && !$iterator->getDepth()) {
         // There is an expectation for all modules to be loaded. However,
         // .module files are not supposed to be in subdirectories.
-        include_once $fileinfo->getPathname();
+        include_once $filename;
       }
       if ($extension === 'php') {
-        $namespace = preg_replace('#^src/#', "Drupal/$module/", $iterator->getSubPath());
-        $class = $namespace . '/' . $fileinfo->getBasename('.php');
-        $class = str_replace('/', '\\', $class);
-        foreach (static::getHookAttributesInClass($class) as $attribute) {
+        $cached = $hook_file_cache->get($filename);
+        if ($cached) {
+          $class = $cached['class'];
+          $attributes = $cached['attributes'];
+        }
+        else {
+          $namespace = preg_replace('#^src/#', "Drupal/$module/", $iterator->getSubPath());
+          $class = $namespace . '/' . $fileinfo->getBasename('.php');
+          $class = str_replace('/', '\\', $class);
+          if (class_exists($class)) {
+            $attributes = static::getHookAttributesInClass($class);
+            $hook_file_cache->set($filename, ['class' => $class, 'attributes' => $attributes]);
+          }
+          else {
+            $attributes = [];
+          }
+        }
+        foreach ($attributes as $attribute) {
           $this->addFromAttribute($attribute, $class, $module);
         }
       }
       else {
-        $finder = MockFileFinder::create($fileinfo->getPathName());
-        $parser = new StaticReflectionParser('', $finder);
-        foreach ($parser->getMethodAttributes() as $function => $attributes) {
-          if (!StaticReflectionParser::hasAttribute($attributes, LegacyHook::class) && preg_match($module_preg, $function, $matches)) {
-            $this->addProceduralImplementation($fileinfo, $matches['hook'], $matches['module'], $matches['function']);
+        $implementations = $procedural_hook_file_cache->get($filename);
+        if ($implementations === NULL) {
+          $finder = MockFileFinder::create($filename);
+          $parser = new StaticReflectionParser('', $finder);
+          $implementations = [];
+          foreach ($parser->getMethodAttributes() as $function => $attributes) {
+            if (!StaticReflectionParser::hasAttribute($attributes, LegacyHook::class) && preg_match($module_preg, $function, $matches)) {
+              $implementations[] = ['function' => $function, 'module' => $matches['module'], 'hook' => $matches['hook']];
+            }
           }
+          $procedural_hook_file_cache->set($filename, $implementations);
+        }
+        foreach ($implementations as $implementation) {
+          $this->addProceduralImplementation($fileinfo, $implementation['hook'], $implementation['module'], $implementation['function']);
         }
       }
       if ($extension === 'inc') {
         $parts = explode('.', $fileinfo->getFilename());
         if (count($parts) === 3 && $parts[0] === $module) {
-          $this->groupIncludes[$parts[1]][] = $fileinfo->getPathname();
+          $this->groupIncludes[$parts[1]][] = $filename;
         }
       }
     }
@@ -223,9 +251,6 @@ protected static function filterIterator(\SplFileInfo $fileInfo, $key, \Recursiv
    *   An array of Hook attributes on this class. The $method property is guaranteed to be set.
    */
   protected static function getHookAttributesInClass(string $class): array {
-    if (!class_exists($class)) {
-      return [];
-    }
     $reflection_class = new \ReflectionClass($class);
     $class_implementations = [];
     // Check for #[Hook] on the class itself.

core/modules/system/tests/modules/hook_collector_on_behalf/hook_collector_on_behalf.info.yml

+name: 'Test Hooks on behalf of other modules'
+type: module
+description: 'Test hooks invoked on behalf of other modules when installed later.'
+package: Testing
+version: VERSION

core/modules/system/tests/modules/hook_collector_on_behalf/src/Hook/OnBehalfOfOtherModuleHook.php

+<?php
+
+declare(strict_types=1);
+
+namespace Drupal\hook_collector_on_behalf\Hook;
+
+use Drupal\Core\Hook\Attribute\Hook;
+
+/**
+ * Hook implementation on behalf of another module.
+ */
+class OnBehalfOfOtherModuleHook {
+
+  /**
+   * Implements hook_module_preinstall().
+   */
+  #[Hook('cache_flush', module: 'respond_install_uninstall_hook_test')]
+  public function flush(): void {
+    // Set a global value we can check in test code.
+    $GLOBALS['on_behalf_oop'] = 'on_behalf_oop';
+  }
+
+}

core/modules/system/tests/modules/hook_collector_on_behalf_procedural/hook_collector_on_behalf_procedural.info.yml

+name: 'Test Hooks on behalf of other modules'
+type: module
+description: 'Test hooks invoked on behalf of other modules when installed later.'
+package: Testing
+version: VERSION

core/modules/system/tests/modules/hook_collector_on_behalf_procedural/hook_collector_on_behalf_procedural.module

+<?php
+
+/**
+ * @file
+ * Implement hooks.
+ */
+
+declare(strict_types=1);
+
+/**
+ * This implements a hook on behalf of another module.
+ *
+ * We do not have implements so this does not get converted.
+ */
+function respond_install_uninstall_hook_test_cache_flush(): void {
+  // Set a global value we can check in test code.
+  $GLOBALS['on_behalf_procedural'] = 'on_behalf_procedural';
+}

core/tests/Drupal/KernelTests/Core/Hook/HookCollectorPassTest.php

@@ -82,4 +82,23 @@ public function testOrdering(): void {
     $this->assertLessThan($priorities['drupal_hook.order2']['order'], $priorities['drupal_hook.order2']['module_handler_test_all2_order2']);
   }
 
+  /**
+   * Test hooks implemented on behalf of an uninstalled module.
+   *
+   * They should be picked up but only executed when the other
+   * module is installed.
+   */
+  public function testHooksImplementedOnBehalfFileCache(): void {
+    $module_installer = $this->container->get('module_installer');
+    $this->assertTrue($module_installer->install(['hook_collector_on_behalf']));
+    $this->assertTrue($module_installer->install(['hook_collector_on_behalf_procedural']));
+    drupal_flush_all_caches();
+    $this->assertFalse(isset($GLOBALS['on_behalf_oop']));
+    $this->assertFalse(isset($GLOBALS['on_behalf_procedural']));
+    $this->assertTrue($module_installer->install(['respond_install_uninstall_hook_test']));
+    drupal_flush_all_caches();
+    $this->assertTrue(isset($GLOBALS['on_behalf_oop']));
+    $this->assertTrue(isset($GLOBALS['on_behalf_procedural']));
+  }
+
 }