Issue #3262359 by phenaproxima: Don't catalog event subscribers as a way of...

Issue #3262359 by phenaproxima: Don't catalog event subscribers as a way of determining the validity of stored results

src/Validation/ReadinessValidationManager.php

@@ -120,47 +120,19 @@ class ReadinessValidationManager implements EventSubscriberInterface {
     $results = $event->getResults();
     $this->keyValueExpirable->setWithExpire(
       'readiness_validation_last_run',
-      [
-        'results' => $results,
-        'listeners' => $this->getListenersAsString(ReadinessCheckEvent::class),
-      ],
+      $results,
       $this->resultsTimeToLive * 60 * 60
     );
     $this->keyValueExpirable->set('readiness_check_timestamp', $this->time->getRequestTime());
     return $this;
   }
 
-  /**
-   * Gets all the listeners for a specific event as single string.
-   *
-   * @return string
-   *   The listeners as a string.
-   */
-  protected function getListenersAsString(string $event_name): string {
-    $listeners = $this->eventDispatcher->getListeners($event_name);
-    $string = '';
-    foreach ($listeners as $listener) {
-      if (is_array($listener)) {
-        $string .= is_object($listener[0]) ? get_class($listener[0]) : $listener[0];
-        $string .= $listener[1];
-      }
-      elseif (is_object($listener)) {
-        $string .= "-" . get_class($listener);
-      }
-      elseif (is_string($listener)) {
-        $string .= "-$listener";
-      }
-    }
-    return $string;
-  }
-
   /**
    * Dispatches the readiness check event if there no stored valid results.
    *
    * @return $this
    *
    * @see self::getResults()
-   * @see self::getStoredValidResults()
    */
   public function runIfNoStoredResults(): self {
     if ($this->getResults() === NULL) {
@@ -179,11 +151,9 @@ class ReadinessValidationManager implements EventSubscriberInterface {
    * @return \Drupal\package_manager\ValidationResult[]|
    *   The validation result objects or NULL if no results are
    *   available or if the stored results are no longer valid.
-   *
-   * @see self::getStoredValidResults()
    */
   public function getResults(?int $severity = NULL): ?array {
-    $results = $this->getStoredValidResults();
+    $results = $this->keyValueExpirable->get('readiness_validation_last_run');
     if ($results !== NULL) {
       if ($severity !== NULL) {
         $results = array_filter($results, function ($result) use ($severity) {
@@ -195,26 +165,6 @@ class ReadinessValidationManager implements EventSubscriberInterface {
     return NULL;
   }
 
-  /**
-   * Gets stored valid results, if any.
-   *
-   * The stored results are considered valid if the current listeners for the
-   * readiness check event are the same as the last time the event was
-   * dispatched.
-   *
-   * @return \Drupal\package_manager\ValidationResult[]|null
-   *   The stored results if available and still valid, otherwise null.
-   */
-  protected function getStoredValidResults(): ?array {
-    $last_run = $this->keyValueExpirable->get('readiness_validation_last_run');
-
-    // If the listeners have not changed return the results.
-    if ($last_run && $last_run['listeners'] === $this->getListenersAsString(ReadinessCheckEvent::class)) {
-      return $last_run['results'];
-    }
-    return NULL;
-  }
-
   /**
    * Deletes any stored readiness validation results.
    */

tests/src/Kernel/ReadinessValidation/ReadinessValidationManagerTest.php

@@ -59,9 +59,9 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase {
     // Rebuild the container to trigger the service to be altered.
     $kernel = $this->container->get('kernel');
     $this->container = $kernel->rebuildContainer();
-    // Confirm that results will be NULL if the run() is not called again
-    // because the readiness checker services order has been altered.
-    $this->assertNull($this->getResultsFromManager());
+    // The stored results should be returned, even though the validators' order
+    // has been changed and the container has been rebuilt.
+    $this->assertValidationResultsEqual($expected_results_all, $this->getResultsFromManager());
     // Confirm that after calling run() the expected results order has changed.
     $expected_results_all_reversed = array_reverse($expected_results_all);
     $this->assertCheckerResultsFromManager($expected_results_all_reversed, TRUE);
@@ -163,6 +163,7 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase {
 
   /**
    * @covers ::runIfNoStoredResults
+   * @covers ::clearStoredResults
    */
   public function testRunIfNeeded(): void {
     $expected_results = array_pop($this->testResults['checker_1']);
@@ -180,9 +181,7 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase {
 
     // Confirm that the new results will be returned because the checkers will
     // be run if the stored results are deleted.
-    /** @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface $key_value */
-    $key_value = $this->container->get('keyvalue.expirable')->get('automatic_updates');
-    $key_value->delete('readiness_validation_last_run');
+    $manager->clearStoredResults();
     $expected_results = $unexpected_results;
     $manager->runIfNoStoredResults();
     $this->assertCheckerResultsFromManager($expected_results);
@@ -194,24 +193,6 @@ class ReadinessValidationManagerTest extends AutomaticUpdatesKernelTestBase {
     $kernel = $this->container->get('kernel');
     $this->container = $kernel->rebuildContainer();
     $this->assertCheckerResultsFromManager($expected_results);
-
-    // Define a constant flag that will cause the readiness checker
-    // service priority to be altered. This will cause the priority of
-    // 'automatic_updates_test.checker' to change from 2 to 4 which will be now
-    // higher than 'automatic_updates_test2.checker' which has a priority of 3.
-    // Because the list of checker IDs is not identical to the previous checker
-    // run runIfNoStoredValidResults() will run the checkers again.
-    define('PACKAGE_MANAGER_TEST_VALIDATOR_PRIORITY', 1);
-
-    // Rebuild the container to trigger the readiness checker services to be
-    // reordered.
-    $kernel = $this->container->get('kernel');
-    $this->container = $kernel->rebuildContainer();
-    $expected_results = $unexpected_results;
-    /** @var \Drupal\automatic_updates\Validation\ReadinessValidationManager $manager */
-    $manager = $this->container->get('automatic_updates.readiness_validation_manager');
-    $manager->runIfNoStoredResults();
-    $this->assertCheckerResultsFromManager($expected_results);
   }
 
   /**