Merged in bugfix/classloader_dependencies (pull request #399)

Bugfix: Classloader dependencies

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

@@ -51,7 +51,7 @@ import java.util.Set;
  *
  * @author Sebastian Sdorra
  */
-public final class ExplodedSmp implements Comparable<ExplodedSmp>
+public final class ExplodedSmp
 {
 
   private static final Logger logger = LoggerFactory.getLogger(ExplodedSmp.class);
@@ -90,60 +90,6 @@ public final class ExplodedSmp implements Comparable<ExplodedSmp>
     return new ExplodedSmp(directory, Plugins.parsePluginDescriptor(desc));
   }
 
-  /**
-   * {@inheritDoc}
-   */
-  @Override
-  public int compareTo(ExplodedSmp o)
-  {
-    int result;
-
-    Set<String> depends = plugin.getDependenciesInclusiveOptionals();
-    Set<String> odepends = o.plugin.getDependenciesInclusiveOptionals();
-
-    if (depends.isEmpty() && odepends.isEmpty())
-    {
-      result = 0;
-    }
-    else if (depends.isEmpty() &&!odepends.isEmpty())
-    {
-      result = -1;
-    }
-    else if (!depends.isEmpty() && odepends.isEmpty())
-    {
-      result = 1;
-    }
-    else
-    {
-      String id = plugin.getInformation().getName(false);
-      String oid = o.plugin.getInformation().getName(false);
-
-      if (depends.contains(oid) && odepends.contains(id))
-      {
-        StringBuilder b = new StringBuilder("circular dependency detected: ");
-
-        b.append(id).append(" depends on ").append(oid).append(" and ");
-        b.append(oid).append(" depends on ").append(id);
-
-        throw new PluginCircularDependencyException(b.toString());
-      }
-      else if (depends.contains(oid))
-      {
-        result = 999;
-      }
-      else if (odepends.contains(id))
-      {
-        result = -999;
-      }
-      else
-      {
-        result = 0;
-      }
-    }
-
-    return result;
-  }
-
   //~--- get methods ----------------------------------------------------------
 
   /**

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

@@ -33,8 +33,6 @@ package sonia.scm.plugin;
 
 //~--- non-JDK imports --------------------------------------------------------
 
-import com.google.common.collect.Lists;
-import com.google.common.collect.Ordering;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
 
@@ -82,9 +80,17 @@ public final class PluginTree
    */
   public PluginTree(List<ExplodedSmp> smps)
   {
-    Iterable<ExplodedSmp> smpOrdered = Ordering.natural().sortedCopy(smps);
 
-    for (ExplodedSmp smp : smpOrdered)
+    smps.forEach(s -> {
+      InstalledPluginDescriptor plugin = s.getPlugin();
+      logger.trace("plugin: {}", plugin.getInformation().getName());
+      logger.trace("dependencies: {}", plugin.getDependencies());
+      logger.trace("optional dependencies: {}", plugin.getOptionalDependencies());
+    });
+
+    rootNodes = new SmpNodeBuilder().buildNodeTree(smps);
+
+    for (ExplodedSmp smp : smps)
     {
       InstalledPluginDescriptor plugin = smp.getPlugin();
 
@@ -102,18 +108,7 @@ public final class PluginTree
 
       PluginCondition condition = plugin.getCondition();
 
-      if ((condition == null) || condition.isSupported())
-      {
-        if (plugin.getDependencies().isEmpty() && plugin.getOptionalDependencies().isEmpty())
-        {
-          rootNodes.add(new PluginNode(smp));
-        }
-        else
-        {
-          appendNode(smp);
-        }
-      }
-      else
+      if (!condition.isSupported())
       {
         //J-
         throw new PluginConditionFailedException(
@@ -156,84 +151,6 @@ public final class PluginTree
 
 
   //~--- methods --------------------------------------------------------------
-  /**
-   * Method description
-   *
-   *
-   * @param smp
-   */
-  private void appendNode(ExplodedSmp smp)
-  {
-    PluginNode child = new PluginNode(smp);
-
-    for (String dependency : smp.getPlugin().getDependencies())
-    {
-      if (!appendNode(rootNodes, child, dependency))
-      {
-        //J-
-        throw new PluginNotInstalledException(
-          String.format(
-            "dependency %s of %s is not installed",
-            dependency,
-            child.getId()
-          )
-        );
-        //J+
-      }
-    }
-
-    boolean rootNode = smp.getPlugin().getDependencies().isEmpty();
-    for (String dependency : smp.getPlugin().getOptionalDependencies()) {
-      if (appendNode(rootNodes, child, dependency)) {
-        rootNode = false;
-      } else {
-        logger.info("optional dependency {} of {} is not installed", dependency, child.getId());
-      }
-    }
-
-    if (rootNode) {
-      logger.info("could not find optional dependencies of {}, append it as root node", child.getId());
-      rootNodes.add(new PluginNode(smp));
-    }
-  }
-
-  /**
-   * Method description
-   *
-   *
-   * @param nodes
-   * @param child
-   * @param dependency
-   *
-   * @return
-   */
-  private boolean appendNode(List<PluginNode> nodes, PluginNode child,
-    String dependency)
-  {
-    logger.debug("check for {} as dependency of {}", dependency, child.getId());
-
-    boolean found = false;
-
-    for (PluginNode node : nodes)
-    {
-      if (node.getId().equals(dependency))
-      {
-        logger.debug("add plugin {} as child of {}", child.getId(), node.getId());
-        node.addChild(child);
-        found = true;
-
-        break;
-      }
-      else if (appendNode(node.getChildren(), child, dependency))
-      {
-        found = true;
-
-        break;
-      }
-    }
-
-    return found;
-  }
 
   @Override
   public String toString() {
@@ -254,5 +171,5 @@ public final class PluginTree
   //~--- fields ---------------------------------------------------------------
 
   /** Field description */
-  private final List<PluginNode> rootNodes = Lists.newArrayList();
+  private final List<PluginNode> rootNodes;
 }

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

@@ -0,0 +1,76 @@
+package sonia.scm.plugin;
+
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.List;
+import java.util.Set;
+import java.util.stream.Collectors;
+
+class SmpNodeBuilder {
+
+  List<PluginNode> buildNodeTree(ExplodedSmp[] smps) {
+    return buildNodeTree(Arrays.asList(smps));
+  }
+
+  List<PluginNode> buildNodeTree(Collection<ExplodedSmp> smps) {
+    Set<String> availablePlugins = getAvailablePluginNames(smps);
+    smps.forEach(smp -> this.assertDependenciesFulfilled(availablePlugins, smp));
+
+    List<PluginNode> nodes = createNodes(smps);
+
+    nodes.forEach(node ->
+      nodes.forEach(otherNode -> appendIfDependsOnOtherNode(node, otherNode))
+    );
+
+    nodes.forEach(this::checkForCircle);
+
+    return nodes;
+  }
+
+  private Set<String> getAvailablePluginNames(Collection<ExplodedSmp> smps) {
+    return smps.stream()
+      .map(ExplodedSmp::getPlugin)
+      .map(InstalledPluginDescriptor::getInformation)
+      .map(PluginInformation::getName)
+      .collect(Collectors.toSet());
+  }
+
+  private void appendIfDependsOnOtherNode(PluginNode node, PluginNode otherNode) {
+    if (node.getPlugin().getPlugin().getDependenciesInclusiveOptionals().contains(otherNode.getId())
+      && !otherNode.getChildren().contains(node)) {
+      otherNode.addChild(node);
+    }
+  }
+
+  private List<PluginNode> createNodes(Collection<ExplodedSmp> smps) {
+    return smps.stream().map(PluginNode::new).collect(Collectors.toList());
+  }
+
+  private void assertDependenciesFulfilled(Set<String> availablePlugins, ExplodedSmp smp) {
+    smp.getPlugin().getDependencies().forEach(dependency -> {
+      if (!availablePlugins.contains(dependency)) {
+        throw new PluginNotInstalledException(
+          String.format(
+            "dependency %s of %s is not installed",
+            dependency,
+            smp.getPlugin().getInformation().getName()
+          )
+        );
+      }
+    });
+  }
+
+  private void checkForCircle(PluginNode pluginNode) {
+    pluginNode.getChildren().forEach(child -> assertDoesNotContainsDependency(pluginNode, child));
+  }
+
+  private void assertDoesNotContainsDependency(PluginNode pluginNode, PluginNode childNode) {
+    if (childNode == pluginNode) {
+      throw new PluginCircularDependencyException("circular dependency detected: " +
+        childNode.getId() + " depends on " + pluginNode.getId() + " and " +
+        pluginNode.getId() + " depends on " + childNode.getId()
+      );
+    }
+    childNode.getChildren().forEach(child -> assertDoesNotContainsDependency(pluginNode, child));
+  }
+}

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

@@ -1,173 +0,0 @@
-/**
- * Copyright (c) 2010, Sebastian Sdorra All rights reserved.
- *
- * Redistribution and use in source and binary forms, with or without
- * modification, are permitted provided that the following conditions are met:
- *
- * 1. Redistributions of source code must retain the above copyright notice,
- * this list of conditions and the following disclaimer. 2. Redistributions in
- * binary form must reproduce the above copyright notice, this list of
- * conditions and the following disclaimer in the documentation and/or other
- * materials provided with the distribution. 3. Neither the name of SCM-Manager;
- * nor the names of its contributors may be used to endorse or promote products
- * derived from this software without specific prior written permission.
- *
- * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS"
- * AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE
- * IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE
- * ARE DISCLAIMED. IN NO EVENT SHALL THE REGENTS OR CONTRIBUTORS BE LIABLE FOR
- * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL
- * DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR
- * SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER
- * CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY,
- * OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE
- * OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
- *
- * http://bitbucket.org/sdorra/scm-manager
- *
- */
-
-
-
-package sonia.scm.plugin;
-
-//~--- non-JDK imports --------------------------------------------------------
-
-import com.google.common.collect.Lists;
-
-import org.junit.Test;
-
-import org.mockito.internal.util.collections.Sets;
-
-import static org.junit.Assert.*;
-
-//~--- JDK imports ------------------------------------------------------------
-
-import java.util.Collections;
-import java.util.List;
-
-/**
- *
- * @author Sebastian Sdorra
- */
-public class ExplodedSmpTest
-{
-
-  /**
-   * Method description
-   *
-   */
-  @Test
-  public void testCompareTo()
-  {
-    ExplodedSmp e1 = create("a", "c", "1", "a");
-    ExplodedSmp e3 = create("a", "a", "1");
-    ExplodedSmp e2 = create("a", "b", "1");
-    List<ExplodedSmp> es = list(e1, e2, e3);
-
-    is(es, 2, "a");
-  }
-
-  /**
-   * Method description
-   *
-   */
-  @Test(expected = PluginCircularDependencyException.class)
-  public void testCompareToCyclicDependency()
-  {
-    ExplodedSmp e1 = create("a", "1", "c");
-    ExplodedSmp e2 = create("b", "1");
-    ExplodedSmp e3 = create("c", "1", "a");
-
-    list(e1, e2, e3);
-  }
-
-  /**
-   * Method description
-   *
-   */
-  @Test
-  public void testCompareToTransitiveDependencies()
-  {
-    ExplodedSmp e1 = create("a", "1", "b");
-    ExplodedSmp e2 = create("b", "1");
-    ExplodedSmp e3 = create("c", "1", "a");
-
-    List<ExplodedSmp> es = list(e1, e2, e3);
-
-    is(es, 0, "b");
-    is(es, 1, "a");
-    is(es, 2, "c");
-  }
-
-  /**
-   * Method description
-   *
-   */
-  @Test
-  public void testMultipleDependencies()
-  {
-    ExplodedSmp e1 = create("a", "1", "b", "c");
-    ExplodedSmp e2 = create("b", "1", "c");
-    ExplodedSmp e3 = create("c", "1");
-    List<ExplodedSmp> es = list(e1, e2, e3);
-
-    is(es, 2, "a");
-  }
-
-  /**
-   * Method description
-   *
-   *
-   * @param name
-   * @param version
-   * @param dependencies
-   *
-   * @return
-   */
-  private ExplodedSmp create(String name, String version,
-    String... dependencies)
-  {
-    PluginInformation info = new PluginInformation();
-
-    info.setName(name);
-    info.setVersion(version);
-
-    InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(2, info, null, null, false,
-                      Sets.newSet(dependencies), null);
-
-    return new ExplodedSmp(null, plugin);
-  }
-
-  /**
-   * Method description
-   *
-   *
-   * @param e
-   *
-   * @return
-   */
-  private List<ExplodedSmp> list(ExplodedSmp... e)
-  {
-    List<ExplodedSmp> es = Lists.newArrayList(e);
-
-    Collections.sort(es);
-
-    return es;
-  }
-
-  //~--- get methods ----------------------------------------------------------
-
-  /**
-   * Method description
-   *
-   *
-   * @param es
-   * @param p
-   * @param a
-   */
-  private void is(List<ExplodedSmp> es, int p, String a)
-  {
-    assertEquals(a, es.get(p).getPlugin().getInformation().getName());
-  }
-}

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

@@ -34,13 +34,13 @@ package sonia.scm.plugin;
 //~--- non-JDK imports --------------------------------------------------------
 
 import com.google.common.base.Function;
-import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.Lists;
 
 import org.junit.Rule;
 import org.junit.Test;
 import org.junit.rules.TemporaryFolder;
 
+import static com.google.common.collect.ImmutableSet.of;
 import static org.hamcrest.Matchers.*;
 
 import static org.junit.Assert.*;
@@ -50,9 +50,11 @@ import static org.junit.Assert.*;
 import java.io.IOException;
 
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 /**
  *
@@ -171,8 +173,8 @@ public class PluginTreeTest
   public void testWithOptionalDependency() throws IOException {
     ExplodedSmp[] smps = new ExplodedSmp[] {
       createSmpWithDependency("a"),
-      createSmpWithDependency("b", null, ImmutableSet.of("a")),
-      createSmpWithDependency("c", null, ImmutableSet.of("a", "b"))
+      createSmpWithDependency("b", null, of("a")),
+      createSmpWithDependency("c", null, of("a", "b"))
     };
 
     PluginTree tree = new PluginTree(smps);
@@ -183,12 +185,74 @@ public class PluginTreeTest
     assertThat(unwrapIds(nodes), contains("a", "b", "c"));
   }
 
+  @Test
+  public void testRealWorldDependencies() throws IOException {
+    //J-
+    ExplodedSmp[] smps = new ExplodedSmp[]{
+      createSmpWithDependency("scm-editor-plugin"),
+      createSmpWithDependency("scm-ci-plugin"),
+      createSmpWithDependency("scm-jenkins-plugin"),
+      createSmpWithDependency("scm-issuetracker-plugin"),
+      createSmpWithDependency("scm-hg-plugin"),
+      createSmpWithDependency("scm-git-plugin"),
+      createSmpWithDependency("scm-directfilelink-plugin"),
+      createSmpWithDependency("scm-ssh-plugin"),
+      createSmpWithDependency("scm-pushlog-plugin"),
+      createSmpWithDependency("scm-cas-plugin"),
+      createSmpWithDependency("scm-tagprotection-plugin"),
+      createSmpWithDependency("scm-groupmanager-plugin"),
+      createSmpWithDependency("scm-webhook-plugin"),
+      createSmpWithDependency("scm-svn-plugin"),
+      createSmpWithDependency("scm-support-plugin"),
+      createSmpWithDependency("scm-statistic-plugin"),
+      createSmpWithDependency("scm-rest-legacy-plugin"),
+      createSmpWithDependency("scm-gravatar-plugin"),
+      createSmpWithDependency("scm-legacy-plugin"),
+      createSmpWithDependency("scm-authormapping-plugin"),
+      createSmpWithDependency("scm-readme-plugin"),
+      createSmpWithDependency("scm-script-plugin"),
+      createSmpWithDependency("scm-activity-plugin"),
+      createSmpWithDependency("scm-mail-plugin"),
+      createSmpWithDependency("scm-branchwp-plugin", of(), of("scm-editor-plugin", "scm-review-plugin", "scm-mail-plugin" )),
+      createSmpWithDependency("scm-notify-plugin", "scm-mail-plugin"),
+      createSmpWithDependency("scm-redmine-plugin", "scm-issuetracker-plugin"),
+      createSmpWithDependency("scm-jira-plugin", "scm-mail-plugin", "scm-issuetracker-plugin"),
+      createSmpWithDependency("scm-review-plugin", of("scm-mail-plugin"), of("scm-editor-plugin")),
+      createSmpWithDependency("scm-pathwp-plugin", of(), of("scm-editor-plugin")),
+      createSmpWithDependency("scm-cockpit-legacy-plugin", "scm-statistic-plugin", "scm-rest-legacy-plugin", "scm-activity-plugin")
+    };
+    //J+
+
+    Arrays.stream(smps)
+      .forEach(smp -> System.out.println(smp.getPlugin()));
+
+
+    PluginTree tree = new PluginTree(smps);
+    List<PluginNode> nodes = tree.getLeafLastNodes();
+
+    System.out.println(tree);
+
+    assertEachParentHasChild(nodes, "scm-review-plugin", "scm-branchwp-plugin");
+  }
+
+  private void assertEachParentHasChild(List<PluginNode> nodes, String parentName, String childName) {
+    nodes.forEach(node -> assertEachParentHasChild(node, parentName, childName));
+  }
+
+  private void assertEachParentHasChild(PluginNode pluginNode, String parentName, String childName) {
+    if (pluginNode.getId().equals(parentName)) {
+      assertThat(pluginNode.getChildren().stream().map(PluginNode::getId).collect(Collectors.toList()), contains(childName));
+    }
+    assertEachParentHasChild(pluginNode.getChildren(), parentName, childName);
+  }
+
+
   @Test
   public void testWithDeepOptionalDependency() throws IOException {
     ExplodedSmp[] smps = new ExplodedSmp[] {
       createSmpWithDependency("a"),
       createSmpWithDependency("b", "a"),
-      createSmpWithDependency("c", null, ImmutableSet.of("b"))
+      createSmpWithDependency("c", null, of("b"))
     };
 
     PluginTree tree = new PluginTree(smps);
@@ -203,7 +267,7 @@ public class PluginTreeTest
   @Test
   public void testWithNonExistentOptionalDependency() throws IOException {
     ExplodedSmp[] smps = new ExplodedSmp[] {
-      createSmpWithDependency("a", null, ImmutableSet.of("b"))
+      createSmpWithDependency("a", null, of("b"))
     };
 
     PluginTree tree = new PluginTree(smps);

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

@@ -0,0 +1,117 @@
+package sonia.scm.plugin;
+
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.internal.AssumptionViolatedException;
+import org.junit.rules.TemporaryFolder;
+
+import java.io.IOException;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Set;
+
+import static com.google.common.collect.ImmutableSet.of;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class SmpNodeBuilderTest {
+
+  @Rule
+  public TemporaryFolder tempFolder = new TemporaryFolder();
+
+  @Test
+  public void onePluginOnly() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[] {
+      createSmpWithDependency("scm-ssh-plugin")
+    };
+
+    List<PluginNode> nodes = new SmpNodeBuilder().buildNodeTree(smps);
+
+    assertThat(nodes).extracting("id").contains("scm-ssh-plugin");
+  }
+
+  @Test
+  public void simpleDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[]{
+      createSmpWithDependency("scm-editor-plugin"),
+      createSmpWithDependency("scm-pathwp-plugin", "scm-editor-plugin")
+    };
+
+    List<PluginNode> nodes = new SmpNodeBuilder().buildNodeTree(smps);
+
+    assertThat(nodes).extracting("id").containsExactlyInAnyOrder("scm-editor-plugin", "scm-pathwp-plugin");
+    assertThat(findNode(nodes, "scm-editor-plugin").getChildren()).extracting("id").contains("scm-pathwp-plugin");
+  }
+
+  @Test(expected = PluginNotInstalledException.class)
+  public void shouldFailForUnfulfilledDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[]{
+      createSmpWithDependency("scm-pathwp-plugin", "scm-editor-plugin")
+    };
+
+    new SmpNodeBuilder().buildNodeTree(smps);
+  }
+
+  @Test
+  public void shouldNotFailForUnfulfilledOptionalDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[]{
+      createSmpWithDependency("scm-pathwp-plugin", of(), of("scm-editor-plugin"))
+    };
+
+    new SmpNodeBuilder().buildNodeTree(smps);
+  }
+
+  @Test(expected = PluginCircularDependencyException.class)
+  public void shouldFailForCircularDependency() throws IOException {
+    ExplodedSmp[] smps = new ExplodedSmp[]{
+      createSmpWithDependency("scm-pathwp-plugin", "scm-editor-plugin"),
+      createSmpWithDependency("scm-editor-plugin", "scm-review-plugin"),
+      createSmpWithDependency("scm-review-plugin", "scm-pathwp-plugin")
+    };
+
+    new SmpNodeBuilder().buildNodeTree(smps);
+  }
+
+  private PluginNode findNode(List<PluginNode> nodes, String id) {
+    return nodes.stream().filter(node -> node.getId().equals(id)).findAny().orElseThrow(() -> new AssumptionViolatedException("node not found"));
+  }
+
+  private ExplodedSmp createSmp(InstalledPluginDescriptor plugin) throws IOException
+  {
+    return new ExplodedSmp(tempFolder.newFile().toPath(), plugin);
+  }
+
+  private PluginInformation createInfo(String name, String version) {
+    PluginInformation info = new PluginInformation();
+
+    info.setName(name);
+    info.setVersion(version);
+
+    return info;
+  }
+
+  private ExplodedSmp createSmpWithDependency(String name,
+                                              String... dependencies)
+    throws IOException
+  {
+    Set<String> dependencySet = new HashSet<>();
+
+    for (String d : dependencies)
+    {
+      dependencySet.add(d);
+    }
+    return createSmpWithDependency(name, dependencySet, null);
+  }
+
+  private ExplodedSmp createSmpWithDependency(String name, Set<String> dependencies, Set<String> optionalDependencies) throws IOException {
+    InstalledPluginDescriptor plugin = new InstalledPluginDescriptor(
+      2,
+      createInfo(name, "1"),
+      null,
+      null,
+      false,
+      dependencies,
+      optionalDependencies
+    );
+    return createSmp(plugin);
+  }
+}