From f92ea41ca346f2e53f215a887ece67b01c18004e Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 31 Jan 2020 11:47:32 +0100 Subject: [PATCH] Fix order of plugin nodes --- .../java/sonia/scm/plugin/PluginTree.java | 107 ++---------------- .../java/sonia/scm/plugin/SmpNodeBuilder.java | 50 ++++++++ .../java/sonia/scm/plugin/PluginTreeTest.java | 74 +++++++++++- .../sonia/scm/plugin/SmpNodeBuilderTest.java | 106 +++++++++++++++++ 4 files changed, 237 insertions(+), 100 deletions(-) create mode 100644 scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java create mode 100644 scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java index d7dfafbe02..c37ee4a5b6 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java +++ b/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 smps) { - Iterable 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 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 rootNodes = Lists.newArrayList(); + private final List rootNodes; } diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java b/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java new file mode 100644 index 0000000000..ed8eb6d840 --- /dev/null +++ b/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java @@ -0,0 +1,50 @@ +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 buildNodeTree(ExplodedSmp[] smps) { + return buildNodeTree(Arrays.asList(smps)); + } + + List buildNodeTree(Collection smps) { + + Set availablePlugins = smps.stream().map(smp -> smp.getPlugin().getInformation().getName()).collect(Collectors.toSet()); + + smps.forEach(smp -> this.assertDependenciesFulfilled(availablePlugins, smp)); + + List nodes = smps.stream().map(PluginNode::new).collect(Collectors.toList()); + + nodes.forEach(node -> { + ExplodedSmp smp = node.getPlugin(); + nodes.forEach(otherNode -> { + + if (smp.getPlugin().getDependenciesInclusiveOptionals().contains(otherNode.getId()) + && !otherNode.getChildren().contains(node)) { + otherNode.addChild(node); + } + }); + }); + + return nodes; + } + + private void assertDependenciesFulfilled(Set 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() + ) + ); + } + }); + } +} diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/PluginTreeTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/PluginTreeTest.java index 6e4e001885..1c6a0482e6 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginTreeTest.java +++ b/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 nodes = tree.getLeafLastNodes(); + + System.out.println(tree); + + assertEachParentHasChild(nodes, "scm-review-plugin", "scm-branchwp-plugin"); + } + + private void assertEachParentHasChild(List 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); diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java new file mode 100644 index 0000000000..d40c0ff66b --- /dev/null +++ b/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java @@ -0,0 +1,106 @@ +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 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 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); + } + + private PluginNode findNode(List 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 dependencySet = new HashSet<>(); + + for (String d : dependencies) + { + dependencySet.add(d); + } + return createSmpWithDependency(name, dependencySet, null); + } + + private ExplodedSmp createSmpWithDependency(String name, Set dependencies, Set optionalDependencies) throws IOException { + InstalledPluginDescriptor plugin = new InstalledPluginDescriptor( + 2, + createInfo(name, "1"), + null, + null, + false, + dependencies, + optionalDependencies + ); + return createSmp(plugin); + } +}