Repository: gocd/gocd
Issue: Plugin admin page can display stale plugin information after plugin upgrades

##### Issue Type

- Bug Report

##### Summary

<!--- Provide a brief summary of the issue -->
When plugins are updated to new versions without changing which plugins are installed, `/go/admin/plugins` will display stale data in the browser until browser cache expiry.


###### Basic environment details

<!--- We recommend providing the for us to reproduce the issue quicker -->

* Go Version: `22.1.0`

##### Steps to Reproduce

<!--- Provide a link to a live example, or an unambiguous set of steps to -->
<!--- reproduce this bug include code to reproduce, if relevant -->
1. Load the plugins admin page
2. Update a plugin to a new version while changing nothing else
3. Bounce the server
4. Re-load the plugins admin page, not the old version.
5. Clear cache and/or force refresh and notice it changing.

##### Expected Results
<!--- Tell us what should happen -->
The page should be updated reliably when plugins are updated.

##### Actual Results
<!--- Tell us what happens instead -->
Stale data displayed for some indeterminate amount of time.

##### Possible Fix
<!--- Not obligatory, but suggest a fix or reason for the bug -->
It looks like the below defines a custom hash algorithm for `PluginInfo` which excludes `version`, `name` and other aspects of the plugin descriptor so if they are updated it won't cause a refresh. At the very least the `version` should be included so when a new version is released (which may have new values for other fields) the hash changes, and thus the entity `ETag`.

https://github.com/gocd/gocd/blob/5c7c6c226dc2caa1fe729f5142536634912e795a/server/src/main/java/com/thoughtworks/go/server/service/EntityHashes.java#L107-L113

Changed files:
server/src/main/java/com/thoughtworks/go/server/service/EntityHashes.java
api/api-plugin-infos-v7/src/main/java/com/thoughtworks/go/apiv7/plugininfos/PluginInfosControllerV7.java
server/src/test-fast/java/com/thoughtworks/go/server/service/EntityHashingServiceTest.java

diff --git a/api/api-plugin-infos-v7/src/main/java/com/thoughtworks/go/apiv7/plugininfos/PluginInfosControllerV7.java b/api/api-plugin-infos-v7/src/main/java/com/thoughtworks/go/apiv7/plugininfos/PluginInfosControllerV7.java
index a75120a5a9..1568613acb 100644
--- a/api/api-plugin-infos-v7/src/main/java/com/thoughtworks/go/apiv7/plugininfos/PluginInfosControllerV7.java
+++ b/api/api-plugin-infos-v7/src/main/java/com/thoughtworks/go/apiv7/plugininfos/PluginInfosControllerV7.java
@@ -18,7 +18,6 @@ package com.thoughtworks.go.apiv7.plugininfos;
 import com.thoughtworks.go.api.ApiController;
 import com.thoughtworks.go.api.ApiVersion;
 import com.thoughtworks.go.api.spring.ApiAuthenticationHelper;
-import com.thoughtworks.go.api.spring.ToggleRegisterLatest;
 import com.thoughtworks.go.apiv7.plugininfos.representers.PluginInfoRepresenter;
 import com.thoughtworks.go.apiv7.plugininfos.representers.PluginInfosRepresenter;
 import com.thoughtworks.go.config.exceptions.RecordNotFoundException;
diff --git a/server/src/main/java/com/thoughtworks/go/server/service/EntityHashes.java b/server/src/main/java/com/thoughtworks/go/server/service/EntityHashes.java
index ebdffb96bd..870f071742 100644
--- a/server/src/main/java/com/thoughtworks/go/server/service/EntityHashes.java
+++ b/server/src/main/java/com/thoughtworks/go/server/service/EntityHashes.java
@@ -107,6 +107,7 @@ public class EntityHashes implements DigestMixin {
         JsonSerializer<PluginInfo> PLUGIN_INFO = (src, typeOfSrc, context) -> {
             final JsonObject result = new JsonObject();
             result.addProperty("id", src.getDescriptor().id());
+            result.addProperty("version", src.getDescriptor().version());
             result.addProperty("extension", src.getExtensionName());
             result.add("settings", context.serialize(src.getPluginSettings()));
             return result;
diff --git a/server/src/test-fast/java/com/thoughtworks/go/server/service/EntityHashingServiceTest.java b/server/src/test-fast/java/com/thoughtworks/go/server/service/EntityHashingServiceTest.java
index 3459ec986a..ff9b142ec9 100644
--- a/server/src/test-fast/java/com/thoughtworks/go/server/service/EntityHashingServiceTest.java
+++ b/server/src/test-fast/java/com/thoughtworks/go/server/service/EntityHashingServiceTest.java
@@ -41,6 +41,7 @@ import java.util.List;
 
 import static com.thoughtworks.go.server.service.EntityHashingService.ETAG_CACHE_KEY;
 import static org.apache.commons.lang3.StringUtils.isNotBlank;
+import static org.assertj.core.api.Assertions.assertThat;
 import static org.junit.jupiter.api.Assertions.*;
 import static org.mockito.Mockito.*;
 
@@ -72,29 +73,35 @@ public class EntityHashingServiceTest {
 
     @Test
     void digestsCombinedPluginInfoAndCollection() {
-        final CombinedPluginInfo info1 = new CombinedPluginInfo(new NotificationPluginInfo(
-                GoPluginDescriptor.builder().id("foo").build(),
-                new PluggableInstanceSettings(List.of(new PluginConfiguration("user", null)))
-        ));
-        final CombinedPluginInfo info2 = new CombinedPluginInfo(new NotificationPluginInfo(
-                GoPluginDescriptor.builder().id("bar").build(),
-                new PluggableInstanceSettings(List.of(new PluginConfiguration("user", null)))
-        ));
+        PluggableInstanceSettings testSettings = new PluggableInstanceSettings(List.of(new PluginConfiguration("user", null)));
+        final CombinedPluginInfo info1 = new CombinedPluginInfo(
+            new NotificationPluginInfo(GoPluginDescriptor.builder().id("foo").build(), testSettings)
+        );
+        final CombinedPluginInfo info2 = new CombinedPluginInfo(
+            new NotificationPluginInfo(GoPluginDescriptor.builder().id("bar").build(), testSettings)
+        );
         final Collection<CombinedPluginInfo> many = List.of(info1, info2);
 
         final String actual = service.hashForEntity(many);
         assertTrue(actual.matches("[a-f0-9]{64}"));
 
         assertEquals(digests.digest(
-                service.hashForEntity(info1),
-                service.hashForEntity(info2)
+            service.hashForEntity(info1),
+            service.hashForEntity(info2)
         ), actual);
+
+        final CombinedPluginInfo info2v2 = new CombinedPluginInfo(
+            new NotificationPluginInfo(GoPluginDescriptor.builder().id("bar").version("2").build(), testSettings)
+        );
+
+        assertThat(service.hashForEntity(info2v2)).isNotEqualTo(service.hashForEntity(info2));
+
     }
 
     @Test
     @DisplayName("when plugin settings contain secret properties, the digest used for" +
-            "ETags should not change as long as the decrypted values remain the same, " +
-            "even if the crypto salt changes between requests")
+        "ETags should not change as long as the decrypted values remain the same, " +
+        "even if the crypto salt changes between requests")
     void digestIsConsistentForPluginSettingsWithSecretPropertiesEvenWhenCryptoSaltChanges() {
         TestIVProvider.with(new ProductionIVProvider(), () -> {
             final String id = "com.foo.plugin";
@@ -106,9 +113,9 @@ public class EntityHashingServiceTest {
             final PluginSettings p2 = pluginSettings(id, key, secret);
 
             assertNotEquals(
-                    p1.getPluginSettingsProperties().get(0).getEncryptedValue(),
-                    p2.getPluginSettingsProperties().get(0).getEncryptedValue(),
-                    "both entities should have different cipherTexts even though the input values are equal"
+                p1.getPluginSettingsProperties().get(0).getEncryptedValue(),
+                p2.getPluginSettingsProperties().get(0).getEncryptedValue(),
+                "both entities should have different cipherTexts even though the input values are equal"
             );
 
             final String expected = service.hashForEntity(p1);
@@ -162,7 +169,7 @@ public class EntityHashingServiceTest {
 
     @Test
     @DisplayName("hashForEntity() can determine the proper overloaded method for implementations of " +
-            "EnvironmentConfig and List<EnvironmentConfig> without ambiguity")
+        "EnvironmentConfig and List<EnvironmentConfig> without ambiguity")
     void hashesEnvironmentConfigsWithoutClassAmbiguityIssues() {
         // important to test these when typed as the non-specific parent interface
         final EnvironmentConfig basic = new BasicEnvironmentConfig(new CaseInsensitiveString("hello"));
@@ -175,11 +182,11 @@ public class EntityHashingServiceTest {
         // resolving the wrong overload might result in an exception indicating that the object "does not
         // have a ConfigTag"
         assertDoesNotThrow(() -> {
-                    assertTrue(isNotBlank(service.hashForEntity(basic)));
-                    assertTrue(isNotBlank(service.hashForEntity(merged)));
-                    assertTrue(isNotBlank(service.hashForEntity(mult)));
-                    assertTrue(isNotBlank(service.hashForEntity(nested)));
-                }
+                assertTrue(isNotBlank(service.hashForEntity(basic)));
+                assertTrue(isNotBlank(service.hashForEntity(merged)));
+                assertTrue(isNotBlank(service.hashForEntity(mult)));
+                assertTrue(isNotBlank(service.hashForEntity(nested)));
+            }
         );
     }
 
@@ -190,8 +197,8 @@ public class EntityHashingServiceTest {
         MergeEnvironmentConfig merged = new MergeEnvironmentConfig(env1, env2);
 
         when(goCache.get(ETAG_CACHE_KEY, "com.thoughtworks.go.config.BasicEnvironmentConfig.env")).
-                thenReturn("foo").
-                thenReturn("bar");
+            thenReturn("foo").
+            thenReturn("bar");
 
         final String type = MergeEnvironmentConfig.class.getSimpleName();
         assertEquals(digests.digest(type, "foo", "bar"), service.hashForEntity(merged));