Verify pending plugins on startup

scm-webapp/src/main/java/sonia/scm/plugin/DefaultPluginManager.java

@@ -80,7 +80,7 @@ public class DefaultPluginManager implements PluginManager {
     this.eventBus = eventBus;
 
     this.computeInstallationDependencies();
-    this.contextFactory = (availablePlugins -> PluginInstallationContext.of(getInstalled(), availablePlugins));
+    this.contextFactory = (availablePlugins -> PluginInstallationContext.from(getInstalled(), availablePlugins));
   }
 
   @VisibleForTesting

scm-webapp/src/main/java/sonia/scm/plugin/ExplodedSmp.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.plugin;
 
 //~--- non-JDK imports --------------------------------------------------------
@@ -30,11 +30,9 @@ import com.google.common.base.Function;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
-//~--- JDK imports ------------------------------------------------------------
-
 import java.nio.file.Path;
 
-import java.util.Set;
+//~--- JDK imports ------------------------------------------------------------
 
 /**
  * The ExplodedSmp object represents an extracted SCM-Manager plugin. The object

scm-webapp/src/main/java/sonia/scm/plugin/PluginInstallationContext.java

@@ -41,18 +41,30 @@ public final class PluginInstallationContext {
     return new PluginInstallationContext(Collections.emptyMap());
   }
 
-  public static PluginInstallationContext of(Iterable<InstalledPlugin> installed, Iterable<AvailablePlugin> pending) {
+  public static PluginInstallationContext fromDescriptors(Iterable<? extends PluginDescriptor> installed, Iterable<? extends PluginDescriptor> pending) {
     Map<String, NameAndVersion> dependencies = new HashMap<>();
-    append(dependencies, installed);
-    append(dependencies, pending);
+    appendDescriptors(dependencies, installed);
+    appendDescriptors(dependencies, pending);
     return new PluginInstallationContext(dependencies);
   }
 
-  private static <P extends Plugin> void append(Map<String, NameAndVersion> dependencies, Iterable<P> plugins) {
-    for (Plugin plugin : plugins) {
-      PluginInformation information = plugin.getDescriptor().getInformation();
-      dependencies.put(information.getName(), new NameAndVersion(information.getName(), information.getVersion()));
-    }
+  public static PluginInstallationContext from(Iterable<? extends Plugin> installed, Iterable<? extends Plugin> pending) {
+    Map<String, NameAndVersion> dependencies = new HashMap<>();
+    appendPlugins(dependencies, installed);
+    appendPlugins(dependencies, pending);
+    return new PluginInstallationContext(dependencies);
+  }
+
+  private static <P extends PluginDescriptor> void appendDescriptors(Map<String, NameAndVersion> dependencies, Iterable<P> descriptors) {
+    descriptors.forEach(desc -> appendPlugins(dependencies, desc.getInformation()));
+  }
+
+  private static <P extends Plugin> void appendPlugins(Map<String, NameAndVersion> dependencies, Iterable<P> plugins) {
+    plugins.forEach(plugin -> appendPlugins(dependencies, plugin.getDescriptor().getInformation()));
+  }
+
+  private static void appendPlugins(Map<String, NameAndVersion> dependencies, PluginInformation information) {
+    dependencies.put(information.getName(), new NameAndVersion(information.getName(), information.getVersion()));
   }
 
   public Optional<NameAndVersion> find(String name) {

scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.plugin;
 
 //~--- non-JDK imports --------------------------------------------------------
@@ -35,7 +35,6 @@ import com.google.common.hash.Hashing;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 import sonia.scm.lifecycle.classloading.ClassLoaderLifeCycle;
-import sonia.scm.plugin.ExplodedSmp.PathTransformer;
 
 import javax.xml.bind.JAXBContext;
 import javax.xml.bind.JAXBException;
@@ -50,11 +49,14 @@ import java.nio.file.Path;
 import java.text.SimpleDateFormat;
 import java.util.ArrayList;
 import java.util.Date;
+import java.util.HashMap;
 import java.util.List;
+import java.util.Map;
 import java.util.Set;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
-import static java.util.stream.Collectors.toList;
+import static java.util.stream.Collectors.toSet;
 
 //~--- JDK imports ------------------------------------------------------------
 
@@ -162,25 +164,16 @@ public final class PluginProcessor
   {
     logger.info("collect plugins");
 
-    Set<Path> archives = collect(pluginDirectory, new PluginArchiveFilter());
+    Set<ExplodedSmp> installedPlugins = findInstalledPlugins();
+    logger.debug("found {} installed plugins", installedPlugins.size());
 
-    logger.debug("extract {} archives", archives.size());
+    Set<ExplodedSmp> newlyInstalledPlugins = installPending(installedPlugins);
+    logger.debug("finished installation of {} plugins", newlyInstalledPlugins.size());
 
-    extract(archives);
-
-    List<Path> dirs =
-      collectPluginDirectories(pluginDirectory)
-      .stream()
-      .filter(isPluginDirectory())
-      .collect(toList());
-
-    logger.debug("process {} directories: {}", dirs.size(), dirs);
-
-    List<ExplodedSmp> smps = Lists.transform(dirs, new PathTransformer());
+    Set<ExplodedSmp> plugins = concat(installedPlugins, newlyInstalledPlugins);
 
     logger.trace("start building plugin tree");
-
-    PluginTree pluginTree = new PluginTree(smps);
+    PluginTree pluginTree = new PluginTree(plugins);
 
     logger.info("install plugin tree:\n{}", pluginTree);
 
@@ -195,6 +188,46 @@ public final class PluginProcessor
     return ImmutableSet.copyOf(wrappers);
   }
 
+  private ImmutableSet<ExplodedSmp> concat(Set<ExplodedSmp> installedPlugins, Set<ExplodedSmp> newlyInstalledPlugins) {
+    return ImmutableSet.<ExplodedSmp>builder().addAll(installedPlugins).addAll(newlyInstalledPlugins).build();
+  }
+
+  private Set<ExplodedSmp> installPending(Set<ExplodedSmp> installedPlugins) throws IOException {
+    Set<Path> archives = collect(pluginDirectory, new PluginArchiveFilter());
+    logger.debug("start installation of {} pending archives", archives.size());
+
+    Map<Path, InstalledPluginDescriptor> pending = new HashMap<>();
+    for (Path archive : archives) {
+      pending.put(archive, SmpDescriptorExtractor.extractPluginDescriptor(archive));
+    }
+
+    PluginInstallationContext installationContext = PluginInstallationContext.fromDescriptors(
+      installedPlugins.stream().map(ExplodedSmp::getPlugin).collect(toSet()),
+      pending.values()
+    );
+
+    for (Map.Entry<Path, InstalledPluginDescriptor> entry : pending.entrySet()) {
+      try {
+        PluginInstallationVerifier.verify(installationContext, entry.getValue());
+      } catch (PluginException ex) {
+        Path path = entry.getKey();
+        logger.error("failed to install smp {}, because it could not be verified", path);
+        logger.error("to restore scm-manager functionality remove the smp file {} from the plugin directory", path);
+        throw ex;
+      }
+    }
+
+    return extract(archives);
+  }
+
+  private Set<ExplodedSmp> findInstalledPlugins() throws IOException {
+    return collectPluginDirectories(pluginDirectory)
+      .stream()
+      .filter(isPluginDirectory())
+      .map(ExplodedSmp::create)
+      .collect(Collectors.toSet());
+  }
+
   private Predicate<Path> isPluginDirectory() {
     return dir -> Files.exists(dir.resolve(DIRECTORY_METAINF).resolve("scm").resolve("plugin.xml"));
   }
@@ -505,10 +538,12 @@ public final class PluginProcessor
    *
    * @throws IOException
    */
-  private void extract(Iterable<Path> archives) throws IOException
+  private Set<ExplodedSmp> extract(Iterable<Path> archives) throws IOException
   {
     logger.debug("extract archives");
 
+    ImmutableSet.Builder<ExplodedSmp> extracted = ImmutableSet.builder();
+
     for (Path archive : archives)
     {
       File archiveFile = archive.toFile();
@@ -519,17 +554,18 @@ public final class PluginProcessor
 
       logger.debug("extract plugin {}", smp.getPlugin());
 
-      File directory =
-        PluginsInternal.createPluginDirectory(pluginDirectory.toFile(),
-          smp.getPlugin());
+      File directory = PluginsInternal.createPluginDirectory(pluginDirectory.toFile(), smp.getPlugin());
 
-      String checksum = com.google.common.io.Files.hash(archiveFile,
-                          Hashing.sha256()).toString();
+      String checksum = com.google.common.io.Files.hash(archiveFile, Hashing.sha256()).toString();
       File checksumFile = PluginsInternal.getChecksumFile(directory);
 
       PluginsInternal.extract(smp, checksum, directory, checksumFile, false);
       moveArchive(archive);
+
+      extracted.add(ExplodedSmp.create(directory.toPath()));
     }
+
+    return extracted.build();
   }
 
   /**

scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.plugin;
 
 //~--- non-JDK imports --------------------------------------------------------
@@ -30,6 +30,7 @@ import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
 import java.util.Arrays;
+import java.util.Collection;
 import java.util.LinkedHashSet;
 import java.util.LinkedList;
 import java.util.List;
@@ -71,7 +72,7 @@ public final class PluginTree
    *
    * @param smps
    */
-  public PluginTree(List<ExplodedSmp> smps)
+  public PluginTree(Collection<ExplodedSmp> smps)
   {
 
     smps.forEach(s -> {

scm-webapp/src/main/java/sonia/scm/plugin/SmpDescriptorExtractor.java

@@ -21,7 +21,7 @@
  * OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE
  * SOFTWARE.
  */
-    
+
 package sonia.scm.plugin;
 
 import javax.xml.bind.JAXBContext;
@@ -33,9 +33,12 @@ import java.nio.file.Path;
 import java.util.zip.ZipEntry;
 import java.util.zip.ZipInputStream;
 
-class SmpDescriptorExtractor {
+final class SmpDescriptorExtractor {
 
-  InstalledPluginDescriptor extractPluginDescriptor(Path file) throws IOException {
+  private SmpDescriptorExtractor() {
+  }
+
+  static InstalledPluginDescriptor extractPluginDescriptor(Path file) throws IOException {
     try (ZipInputStream zipInputStream = new ZipInputStream(Files.newInputStream(file), StandardCharsets.UTF_8))    {
       ZipEntry nextEntry;
       while ((nextEntry = zipInputStream.getNextEntry()) != null) {

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

@@ -45,7 +45,7 @@ class PluginInstallationContextTest {
     Set<InstalledPlugin> installed = installed("scm-git-plugin", "1.0.0");
     Set<AvailablePlugin> pending = Collections.emptySet();
 
-    PluginInstallationContext context = PluginInstallationContext.of(installed, pending);
+    PluginInstallationContext context = PluginInstallationContext.from(installed, pending);
     Optional<NameAndVersion> plugin = context.find("scm-git-plugin");
     assertThat(plugin).contains(new NameAndVersion("scm-git-plugin", "1.0.0"));
   }
@@ -55,7 +55,7 @@ class PluginInstallationContextTest {
     Set<InstalledPlugin> installed = Collections.emptySet();
     Set<AvailablePlugin> pending = pending("scm-hg-plugin", "1.0.0");
 
-    PluginInstallationContext context = PluginInstallationContext.of(installed, pending);
+    PluginInstallationContext context = PluginInstallationContext.from(installed, pending);
     Optional<NameAndVersion> plugin = context.find("scm-hg-plugin");
     assertThat(plugin).contains(new NameAndVersion("scm-hg-plugin", "1.0.0"));
   }
@@ -65,7 +65,7 @@ class PluginInstallationContextTest {
     Set<InstalledPlugin> installed = installed("scm-svn-plugin", "1.1.0");
     Set<AvailablePlugin> pending = pending("scm-svn-plugin", "1.2.0");
 
-    PluginInstallationContext context = PluginInstallationContext.of(installed, pending);
+    PluginInstallationContext context = PluginInstallationContext.from(installed, pending);
     Optional<NameAndVersion> plugin = context.find("scm-svn-plugin");
     assertThat(plugin).contains(new NameAndVersion("scm-svn-plugin", "1.2.0"));
   }
@@ -75,24 +75,41 @@ class PluginInstallationContextTest {
     Set<InstalledPlugin> installed = Collections.emptySet();
     Set<AvailablePlugin> pending = Collections.emptySet();
 
-    PluginInstallationContext context = PluginInstallationContext.of(installed, pending);
+    PluginInstallationContext context = PluginInstallationContext.from(installed, pending);
     Optional<NameAndVersion> plugin = context.find("scm-legacy-plugin");
     assertThat(plugin).isEmpty();
   }
 
+  @Test
+  void shouldCreateContextFromDescriptor() {
+    Set<InstalledPluginDescriptor> installed = mockDescriptor(InstalledPluginDescriptor.class, "scm-svn-plugin", "1.1.0");
+    Set<AvailablePluginDescriptor> pending = mockDescriptor(AvailablePluginDescriptor.class, "scm-svn-plugin", "1.2.0");
+
+    PluginInstallationContext context = PluginInstallationContext.fromDescriptors(installed, pending);
+    Optional<NameAndVersion> plugin = context.find("scm-svn-plugin");
+    assertThat(plugin).contains(new NameAndVersion("scm-svn-plugin", "1.2.0"));
+  }
+
   private Set<InstalledPlugin> installed(String name, String version) {
-    return mockSingleton(InstalledPlugin.class, name, version);
+    return mockPlugin(InstalledPlugin.class, name, version);
   }
 
   private Set<AvailablePlugin> pending(String name, String version) {
-    return mockSingleton(AvailablePlugin.class, name, version);
+    return mockPlugin(AvailablePlugin.class, name, version);
   }
 
-  private <P extends Plugin> Set<P> mockSingleton(Class<P> pluginClass, String name, String version) {
+  private <P extends Plugin> Set<P> mockPlugin(Class<P> pluginClass, String name, String version) {
     P plugin = mock(pluginClass, Answers.RETURNS_DEEP_STUBS);
     when(plugin.getDescriptor().getInformation().getName()).thenReturn(name);
     when(plugin.getDescriptor().getInformation().getVersion()).thenReturn(version);
     return Collections.singleton(plugin);
   }
 
+  private <D extends PluginDescriptor> Set<D> mockDescriptor(Class<D> descriptorClass, String name, String version) {
+    D desc = mock(descriptorClass, Answers.RETURNS_DEEP_STUBS);
+    when(desc.getInformation().getName()).thenReturn(name);
+    when(desc.getInformation().getVersion()).thenReturn(version);
+    return Collections.singleton(desc);
+  }
+
 }

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

@@ -67,7 +67,7 @@ class SmpDescriptorExtractorTest {
   void shouldExtractPluginXml(@TempDir Path tempDir) throws IOException {
     Path pluginFile = createZipFile(tempDir, "META-INF/scm/plugin.xml", PLUGIN_XML);
 
-    InstalledPluginDescriptor installedPluginDescriptor = new SmpDescriptorExtractor().extractPluginDescriptor(pluginFile);
+    InstalledPluginDescriptor installedPluginDescriptor = SmpDescriptorExtractor.extractPluginDescriptor(pluginFile);
 
     Assertions.assertThat(installedPluginDescriptor.getInformation().getName()).isEqualTo("scm-test-plugin");
   }
@@ -76,14 +76,14 @@ class SmpDescriptorExtractorTest {
   void shouldFailWithoutPluginXml(@TempDir Path tempDir) throws IOException {
     Path pluginFile = createZipFile(tempDir, "META-INF/wrong/plugin.xml", PLUGIN_XML);
 
-    assertThrows(IOException.class, () -> new SmpDescriptorExtractor().extractPluginDescriptor(pluginFile));
+    assertThrows(IOException.class, () -> SmpDescriptorExtractor.extractPluginDescriptor(pluginFile));
   }
 
   @Test
   void shouldFailWithIllegalPluginXml(@TempDir Path tempDir) throws IOException {
     Path pluginFile = createZipFile(tempDir, "META-INF/scm/plugin.xml", "<not><parsable>content</parsable></not>");
 
-    assertThrows(IOException.class, () -> new SmpDescriptorExtractor().extractPluginDescriptor(pluginFile));
+    assertThrows(IOException.class, () -> SmpDescriptorExtractor.extractPluginDescriptor(pluginFile));
   }
 
   Path createZipFile(Path tempDir, String internalFileName, String content) throws IOException {