From 8552693532ab05abbe748d3a27ba2543fec4bc66 Mon Sep 17 00:00:00 2001
From: Adam G-H <32250-phenaproxima@users.noreply.drupalcode.org>
Date: Wed, 22 Jan 2025 18:08:16 +0000
Subject: [PATCH] Issue #3498564 by phenaproxima, fjgarlin, chrisfromredfin: Do
 not store error results in KeyValue storage

---
 src/EnabledSourceHandler.php                  | 20 +++++++++-------
 .../ProjectBrowserTestMock.php                |  9 ++++++-
 .../Functional/EnabledSourceHandlerTest.php   | 24 +++++++++++++++++++
 3 files changed, 43 insertions(+), 10 deletions(-)

diff --git a/src/EnabledSourceHandler.php b/src/EnabledSourceHandler.php
index d1372f36a..9ee083e76 100644
--- a/src/EnabledSourceHandler.php
+++ b/src/EnabledSourceHandler.php
@@ -134,15 +134,17 @@ class EnabledSourceHandler implements LoggerAwareInterface, EventSubscriberInter
         // never store it.
         $this->getActivationData($project);
       }
-      // Store the results for this query as a set of arguments to
-      // ProjectsResultsPage.
-      $storage->set($cache_key, [
-        $results->totalResults,
-        array_column($results->list, 'id'),
-        $results->pluginLabel,
-        $source_id,
-        $results->error,
-      ]);
+      // If there were no query errors, store the results as a set of arguments
+      // to ProjectsResultsPage.
+      if (empty($results->error)) {
+        $storage->set($cache_key, [
+          $results->totalResults,
+          array_column($results->list, 'id'),
+          $results->pluginLabel,
+          $source_id,
+          $results->error,
+        ]);
+      }
     }
     return [$source_id => $results];
   }
diff --git a/tests/modules/project_browser_test/src/Plugin/ProjectBrowserSource/ProjectBrowserTestMock.php b/tests/modules/project_browser_test/src/Plugin/ProjectBrowserSource/ProjectBrowserTestMock.php
index bfd5fea19..c74a5a299 100644
--- a/tests/modules/project_browser_test/src/Plugin/ProjectBrowserSource/ProjectBrowserTestMock.php
+++ b/tests/modules/project_browser_test/src/Plugin/ProjectBrowserSource/ProjectBrowserTestMock.php
@@ -54,6 +54,13 @@ class ProjectBrowserTestMock extends ProjectBrowserSourceBase {
    */
   const MAINTAINED_VALUES = [13028, 19370, 9990];
 
+  /**
+   * An error message to flag when querying.
+   *
+   * @var string|null
+   */
+  public static ?string $resultsError = NULL;
+
   /**
    * Constructor for mock API.
    *
@@ -393,7 +400,7 @@ class ProjectBrowserTestMock extends ProjectBrowserSourceBase {
       }
     }
 
-    return $this->createResultsPage($returned_list, $api_response['total_results'] ?? 0);
+    return $this->createResultsPage($returned_list, $api_response['total_results'] ?? 0, static::$resultsError);
   }
 
   /**
diff --git a/tests/src/Functional/EnabledSourceHandlerTest.php b/tests/src/Functional/EnabledSourceHandlerTest.php
index 1b5877170..dd18731db 100644
--- a/tests/src/Functional/EnabledSourceHandlerTest.php
+++ b/tests/src/Functional/EnabledSourceHandlerTest.php
@@ -6,6 +6,7 @@ namespace Drupal\Tests\project_browser\Functional;
 
 use Drupal\project_browser\EnabledSourceHandler;
 use Drupal\project_browser\ProjectBrowser\Project;
+use Drupal\project_browser_test\Plugin\ProjectBrowserSource\ProjectBrowserTestMock;
 use Drupal\Tests\BrowserTestBase;
 
 /**
@@ -87,6 +88,29 @@ class EnabledSourceHandlerTest extends BrowserTestBase {
     $this->assertFalse(self::hasActivationData($project));
   }
 
+  /**
+   * Tests that query results are not stored if there was an error.
+   */
+  public function testErrorsAreNotStored(): void {
+    /** @var \Drupal\project_browser\EnabledSourceHandler $handler */
+    $handler = $this->container->get(EnabledSourceHandler::class);
+
+    $handler->getProjects('project_browser_test_mock');
+    /** @var \Drupal\Core\KeyValueStore\KeyValueStoreInterface $storage */
+    $storage = $this->container->get('keyvalue')
+      ->get('project_browser:project_browser_test_mock');
+    // Query results should have been stored.
+    $query_cache_key = 'query:' . md5('[]');
+    $this->assertTrue($storage->has($query_cache_key));
+
+    $handler->clearStorage();
+    ProjectBrowserTestMock::$resultsError = 'Nope!';
+
+    $handler->getProjects('project_browser_test_mock');
+    // No query results should have been stored.
+    $this->assertFalse($storage->has($query_cache_key));
+  }
+
   /**
    * Checks if a project object is carrying activation data.
    *
-- 
GitLab