Improve plugin center error feedback and cache invalidation (#2147)

The plugin center cache was not invalidated when the proxy configuration was changed in the global settings. This caused stale and inconsistent state to be displayed to the user while there was no feedback that something was wrong.

scm-webapp/src/test/java/sonia/scm/api/v2/resources/AvailablePluginResourceTest.java

@@ -24,7 +24,6 @@
 
 package sonia.scm.api.v2.resources;
 
-import de.otto.edison.hal.HalRepresentation;
 import org.apache.shiro.authz.UnauthorizedException;
 import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.ThreadContext;
@@ -42,6 +41,7 @@ import sonia.scm.plugin.AvailablePlugin;
 import sonia.scm.plugin.AvailablePluginDescriptor;
 import sonia.scm.plugin.InstalledPlugin;
 import sonia.scm.plugin.InstalledPluginDescriptor;
+import sonia.scm.plugin.PluginCenterStatus;
 import sonia.scm.plugin.PluginCondition;
 import sonia.scm.plugin.PluginInformation;
 import sonia.scm.plugin.PluginManager;
@@ -116,9 +116,9 @@ class AvailablePluginResourceTest {
     void getAvailablePlugins() throws URISyntaxException, UnsupportedEncodingException {
       AvailablePlugin plugin = createAvailablePlugin();
 
-      when(pluginManager.getAvailable()).thenReturn(Collections.singletonList(plugin));
-      when(pluginManager.getInstalled()).thenReturn(Collections.emptyList());
-      when(collectionMapper.mapAvailable(Collections.singletonList(plugin))).thenReturn(new MockedResultDto());
+      when(pluginManager.getPlugins()).thenReturn(new PluginManager.PluginResult(Collections.emptyList(), Collections.singletonList(plugin)));
+
+      when(collectionMapper.mapAvailable(Collections.singletonList(plugin), PluginCenterStatus.OK)).thenReturn(new MockedResultDto());
 
       MockHttpRequest request = MockHttpRequest.get("/v2/plugins/available");
       request.accept(VndMediaType.PLUGIN_COLLECTION);
@@ -135,9 +135,8 @@ class AvailablePluginResourceTest {
       AvailablePlugin availablePlugin = createAvailablePlugin();
       InstalledPlugin installedPlugin = createInstalledPlugin();
 
-      when(pluginManager.getAvailable()).thenReturn(Collections.singletonList(availablePlugin));
-      when(pluginManager.getInstalled()).thenReturn(Collections.singletonList(installedPlugin));
-      lenient().when(collectionMapper.mapAvailable(Collections.singletonList(availablePlugin))).thenReturn(new MockedResultDto());
+      when(pluginManager.getPlugins()).thenReturn(new PluginManager.PluginResult(Collections.singletonList(installedPlugin), Collections.singletonList(availablePlugin)));
+      lenient().when(collectionMapper.mapAvailable(Collections.singletonList(availablePlugin), PluginCenterStatus.OK)).thenReturn(new MockedResultDto());
 
       MockHttpRequest request = MockHttpRequest.get("/v2/plugins/available");
       request.accept(VndMediaType.PLUGIN_COLLECTION);
@@ -261,7 +260,7 @@ class AvailablePluginResourceTest {
     }
   }
 
-  public class MockedResultDto extends HalRepresentation {
+  public class MockedResultDto extends PluginCollectionDto {
     public String getMarker() {
       return "x";
     }

scm-webapp/src/test/java/sonia/scm/api/v2/resources/InstalledPluginResourceTest.java

@@ -24,7 +24,6 @@
     
 package sonia.scm.api.v2.resources;
 
-import de.otto.edison.hal.HalRepresentation;
 import org.apache.shiro.authz.UnauthorizedException;
 import org.apache.shiro.subject.Subject;
 import org.apache.shiro.util.ThreadContext;
@@ -110,8 +109,9 @@ class InstalledPluginResourceTest {
     @Test
     void getInstalledPlugins() throws URISyntaxException, UnsupportedEncodingException {
       InstalledPlugin installedPlugin = createInstalled("");
-      when(pluginManager.getInstalled()).thenReturn(Collections.singletonList(installedPlugin));
-      when(collectionMapper.mapInstalled(Collections.singletonList(installedPlugin), Collections.emptyList())).thenReturn(new MockedResultDto());
+      PluginManager.PluginResult pluginResult = new PluginManager.PluginResult(Collections.singletonList(installedPlugin), emptyList());
+      when(pluginManager.getPlugins()).thenReturn(pluginResult);
+      when(collectionMapper.mapInstalled(pluginResult)).thenReturn(new MockedResultDto());
 
       MockHttpRequest request = MockHttpRequest.get("/v2/plugins/installed");
       request.accept(VndMediaType.PLUGIN_COLLECTION);
@@ -184,7 +184,8 @@ class InstalledPluginResourceTest {
     }
   }
 
-  public class MockedResultDto extends HalRepresentation {
+  public class MockedResultDto extends PluginCollectionDto {
+
     public String getMarker() {
       return "x";
     }

scm-webapp/src/test/java/sonia/scm/api/v2/resources/PluginDtoCollectionMapperTest.java

@@ -41,14 +41,15 @@ import sonia.scm.plugin.AvailablePlugin;
 import sonia.scm.plugin.AvailablePluginDescriptor;
 import sonia.scm.plugin.InstalledPlugin;
 import sonia.scm.plugin.InstalledPluginDescriptor;
+import sonia.scm.plugin.PluginCenterStatus;
 import sonia.scm.plugin.PluginInformation;
 import sonia.scm.plugin.PluginManager;
 
 import java.net.URI;
 import java.util.ArrayList;
 import java.util.List;
-import java.util.stream.Collectors;
 
+import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
 import static org.mockito.Mockito.lenient;
@@ -88,14 +89,23 @@ class PluginDtoCollectionMapperTest {
     ThreadContext.unbindSubject();
   }
 
+  @Test
+  void shouldMapErrorStatus() {
+    PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper, manager);
+
+    assertThat(mapper.mapInstalled(emptyPluginResult(PluginCenterStatus.ERROR)).getPluginCenterStatus()).isEqualTo(PluginCenterStatus.ERROR);
+    assertThat(mapper.mapInstalled(emptyPluginResult(PluginCenterStatus.OK)).getPluginCenterStatus()).isEqualTo(PluginCenterStatus.OK);
+    assertThat(mapper.mapAvailable(emptyList(), PluginCenterStatus.ERROR).getPluginCenterStatus()).isEqualTo(PluginCenterStatus.ERROR);
+    assertThat(mapper.mapAvailable(emptyList(), PluginCenterStatus.OK).getPluginCenterStatus()).isEqualTo(PluginCenterStatus.OK);
+  }
 
   @Test
   void shouldMapInstalledPluginsWithoutUpdateWhenNoNewerVersionIsAvailable() {
     PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper, manager);
 
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(createAvailablePlugin("scm-other-plugin", "2")));
+      singletonList(createAvailablePlugin("scm-other-plugin", "2"))));
 
     List<HalRepresentation> plugins = result.getEmbedded().getItemsBy("plugins");
     assertThat(plugins).hasSize(1);
@@ -106,11 +116,11 @@ class PluginDtoCollectionMapperTest {
 
   @Test
   void shouldSetNewVersionInInstalledPluginWhenAvailableVersionIsNewer() {
-    PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper,manager);
+    PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper, manager);
 
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(createAvailablePlugin("scm-some-plugin", "2")));
+      singletonList(createAvailablePlugin("scm-some-plugin", "2"))));
 
     PluginDto plugin = getPluginDtoFromResult(result);
     assertThat(plugin.getVersion()).isEqualTo("1");
@@ -122,9 +132,9 @@ class PluginDtoCollectionMapperTest {
     when(subject.isPermitted("plugin:write")).thenReturn(false);
     PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper, manager);
 
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(createAvailablePlugin("scm-some-plugin", "2")));
+      singletonList(createAvailablePlugin("scm-some-plugin", "2"))));
 
     PluginDto plugin = getPluginDtoFromResult(result);
     assertThat(plugin.getLinks().getLinkBy("update")).isEmpty();
@@ -137,9 +147,9 @@ class PluginDtoCollectionMapperTest {
 
     AvailablePlugin availablePlugin = createAvailablePlugin("scm-some-plugin", "2");
     when(availablePlugin.isPending()).thenReturn(true);
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(availablePlugin));
+      singletonList(availablePlugin)));
 
     PluginDto plugin = getPluginDtoFromResult(result);
     assertThat(plugin.getLinks().getLinkBy("update")).isEmpty();
@@ -150,9 +160,9 @@ class PluginDtoCollectionMapperTest {
     when(subject.isPermitted("plugin:write")).thenReturn(true);
     PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper, manager);
 
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(createAvailablePlugin("scm-some-plugin", "2")));
+      singletonList(createAvailablePlugin("scm-some-plugin", "2"))));
 
     PluginDto plugin = getPluginDtoFromResult(result);
     assertThat(plugin.getLinks().getLinkBy("update")).isNotEmpty();
@@ -164,9 +174,9 @@ class PluginDtoCollectionMapperTest {
     when(subject.isPermitted("plugin:write")).thenReturn(true);
     PluginDtoCollectionMapper mapper = new PluginDtoCollectionMapper(resourceLinks, pluginDtoMapper, manager);
 
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(createAvailablePlugin("scm-some-plugin", "2")));
+      singletonList(createAvailablePlugin("scm-some-plugin", "2"))));
 
     PluginDto plugin = getPluginDtoFromResult(result);
     assertThat(plugin.getLinks().getLinkBy("update")).isNotEmpty();
@@ -180,9 +190,9 @@ class PluginDtoCollectionMapperTest {
 
     AvailablePlugin availablePlugin = createAvailablePlugin("scm-some-plugin", "2");
     when(availablePlugin.isPending()).thenReturn(true);
-    HalRepresentation result = mapper.mapInstalled(
+    HalRepresentation result = mapper.mapInstalled(new PluginManager.PluginResult(
       singletonList(createInstalledPlugin("scm-some-plugin", "1")),
-      singletonList(availablePlugin));
+      singletonList(availablePlugin)));
 
     PluginDto plugin = getPluginDtoFromResult(result);
     assertThat(plugin.isPending()).isTrue();
@@ -223,4 +233,12 @@ class PluginDtoCollectionMapperTest {
     lenient().when(plugin.getDescriptor()).thenReturn(descriptor);
     return plugin;
   }
+
+  private static PluginManager.PluginResult emptyPluginResult(PluginCenterStatus status) {
+    return new PluginManager.PluginResult(
+      emptyList(),
+      emptyList(),
+      status
+    );
+  }
 }

scm-webapp/src/test/java/sonia/scm/plugin/DefaultPluginManagerTest.java

@@ -54,6 +54,7 @@ import java.util.Optional;
 import java.util.Set;
 
 import static java.util.Arrays.asList;
+import static java.util.Collections.emptySet;
 import static java.util.Collections.singleton;
 import static java.util.Collections.singletonList;
 import static org.assertj.core.api.Assertions.assertThat;
@@ -133,6 +134,27 @@ class DefaultPluginManagerTest {
       ThreadContext.unbindSubject();
     }
 
+    @Test
+    void shouldReturnSuccessfulPluginResult() {
+      AvailablePlugin editor = createAvailable("scm-editor-plugin");
+      AvailablePlugin jenkins = createAvailable("scm-jenkins-plugin");
+      InstalledPlugin review = createInstalled("scm-review-plugin");
+      InstalledPlugin git = createInstalled("scm-git-plugin");
+
+      when(center.getPluginResult()).thenReturn(new PluginCenterResult(
+        ImmutableSet.of(editor, jenkins),
+        emptySet())
+      );
+
+      when(loader.getInstalledPlugins()).thenReturn(ImmutableList.of(review, git));
+
+      PluginManager.PluginResult plugins = manager.getPlugins();
+
+      assertThat(plugins.getAvailablePlugins()).containsOnly(editor, jenkins);
+      assertThat(plugins.getInstalledPlugins()).containsOnly(review, git);
+      assertThat(plugins.getPluginCenterStatus()).isEqualTo(PluginCenterStatus.OK);
+    }
+
     @Test
     void shouldReturnInstalledPlugins() {
       InstalledPlugin review = createInstalled("scm-review-plugin");
@@ -746,6 +768,7 @@ class DefaultPluginManagerTest {
       assertThrows(AuthorizationException.class, () -> manager.getAvailable());
       assertThrows(AuthorizationException.class, () -> manager.getAvailable("test"));
       assertThrows(AuthorizationException.class, () -> manager.getPluginSets());
+      assertThrows(AuthorizationException.class, () -> manager.getPlugins());
     }
 
   }

scm-webapp/src/test/java/sonia/scm/plugin/PluginCenterLoaderTest.java

@@ -79,6 +79,7 @@ class PluginCenterLoaderTest {
     PluginCenterResult fetched = loader.load(PLUGIN_URL);
     assertThat(fetched.getPlugins()).isSameAs(plugins);
     assertThat(fetched.getPluginSets()).isSameAs(pluginSets);
+    assertThat(fetched.getStatus()).isEqualTo(PluginCenterStatus.OK);
   }
 
   private AdvancedHttpResponse request() throws IOException {
@@ -88,6 +89,14 @@ class PluginCenterLoaderTest {
     return response;
   }
 
+  @Test
+  void shouldReturnEmptySetIfPluginCenterIsDeactivated() {
+    PluginCenterResult fetch = loader.load("");
+    assertThat(fetch.getPlugins()).isEmpty();
+    assertThat(fetch.getPluginSets()).isEmpty();
+    assertThat(fetch.getStatus()).isSameAs(PluginCenterStatus.DEACTIVATED);
+  }
+
   @Test
   void shouldReturnEmptySetIfPluginCenterNotBeReached() throws IOException {
     when(client.get(PLUGIN_URL)).thenReturn(request);
@@ -96,6 +105,7 @@ class PluginCenterLoaderTest {
     PluginCenterResult fetch = loader.load(PLUGIN_URL);
     assertThat(fetch.getPlugins()).isEmpty();
     assertThat(fetch.getPluginSets()).isEmpty();
+    assertThat(fetch.getStatus()).isSameAs(PluginCenterStatus.ERROR);
   }
 
   @Test

scm-webapp/src/test/java/sonia/scm/plugin/PluginCenterTest.java

@@ -33,8 +33,8 @@ import sonia.scm.SCMContextProvider;
 import sonia.scm.cache.CacheManager;
 import sonia.scm.cache.MapCacheManager;
 import sonia.scm.config.ScmConfiguration;
+import sonia.scm.config.ScmConfigurationChangedEvent;
 
-import java.util.Collections;
 import java.util.HashSet;
 import java.util.Set;
 
@@ -92,7 +92,7 @@ class PluginCenterTest {
     Set<PluginSet> pluginSets = new HashSet<>();
 
     PluginCenterResult first = new PluginCenterResult(plugins, pluginSets);
-    when(loader.load(anyString())).thenReturn(first, new PluginCenterResult(Collections.emptySet(), Collections.emptySet()));
+    when(loader.load(anyString())).thenReturn(first, new PluginCenterResult());
 
     assertThat(pluginCenter.getAvailablePlugins()).isSameAs(plugins);
     assertThat(pluginCenter.getAvailablePlugins()).isSameAs(plugins);
@@ -101,12 +101,12 @@ class PluginCenterTest {
 
   @Test
   @SuppressWarnings("unchecked")
-  void shouldClearCache() {
+  void shouldClearCacheOnPluginCenterLogin() {
     Set<AvailablePlugin> plugins = new HashSet<>();
     Set<PluginSet> pluginSets = new HashSet<>();
 
     PluginCenterResult first = new PluginCenterResult(plugins, pluginSets);
-    when(loader.load(anyString())).thenReturn(first, new PluginCenterResult(Collections.emptySet(), Collections.emptySet()));
+    when(loader.load(anyString())).thenReturn(first, new PluginCenterResult());
 
     assertThat(pluginCenter.getAvailablePlugins()).isSameAs(plugins);
     assertThat(pluginCenter.getAvailablePluginSets()).isSameAs(pluginSets);
@@ -115,6 +115,22 @@ class PluginCenterTest {
     assertThat(pluginCenter.getAvailablePluginSets()).isNotSameAs(pluginSets);
   }
 
+  @Test
+  @SuppressWarnings("unchecked")
+  void shouldClearCacheOnConfigChange() {
+    Set<AvailablePlugin> plugins = new HashSet<>();
+    Set<PluginSet> pluginSets = new HashSet<>();
+
+    PluginCenterResult first = new PluginCenterResult(plugins, pluginSets);
+    when(loader.load(anyString())).thenReturn(first, new PluginCenterResult());
+
+    assertThat(pluginCenter.getAvailablePlugins()).isSameAs(plugins);
+    assertThat(pluginCenter.getAvailablePluginSets()).isSameAs(pluginSets);
+    pluginCenter.handle(new ScmConfigurationChangedEvent(null));
+    assertThat(pluginCenter.getAvailablePlugins()).isNotSameAs(plugins);
+    assertThat(pluginCenter.getAvailablePluginSets()).isNotSameAs(pluginSets);
+  }
+
   @Test
   void shouldLoadOnRefresh() {
     Set<AvailablePlugin> plugins = new HashSet<>();