From 2d755aae9a5671c832560039206c8a5e9d0aef88 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Thu, 16 Jan 2020 08:56:57 +0100 Subject: [PATCH] Fix plugin load order The old algorithm failed, because the tree below lead to the issue, that the scm-branchwp-plugin was loaded before the scm-review-plugin was ready. This commit changes the order in the way, that leafs are loaded last. +- scm-editor-plugin d +- scm-branchwp-plugin a +- scm-mail-plugin c +- scm-review-plugin b +- scm-branchwp-plugin a +- scm-branchwp-plugin a --- .../sonia/scm/plugin/PluginProcessor.java | 40 +++---------- .../java/sonia/scm/plugin/PluginTree.java | 47 +++++++++++++-- .../java/sonia/scm/plugin/PluginTreeTest.java | 60 ++++++++++--------- 3 files changed, 83 insertions(+), 64 deletions(-) diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java index 431cb44e1b..031f31694e 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginProcessor.java @@ -191,11 +191,11 @@ public final class PluginProcessor logger.info("install plugin tree:\n{}", pluginTree); - List rootNodes = pluginTree.getRootNodes(); + List leafLastNodes = pluginTree.getLeafLastNodes(); logger.trace("create plugin wrappers and build classloaders"); - Set wrappers = createPluginWrappers(classLoader, rootNodes); + Set wrappers = createPluginWrappers(classLoader, leafLastNodes); logger.debug("collected {} plugins", wrappers.size()); @@ -259,33 +259,6 @@ public final class PluginProcessor } } - /** - * Method description - * - * - * @param plugins - * @param classLoader - * @param nodes - * - * @throws IOException - */ - private void appendPluginWrappers(Set plugins, - ClassLoader classLoader, List nodes) - throws IOException - { - - // TODO fix plugin loading order - for (PluginNode node : nodes) - { - appendPluginWrapper(plugins, classLoader, node); - } - - for (PluginNode node : nodes) - { - appendPluginWrappers(plugins, classLoader, node.getChildren()); - } - } - /** * Method description * @@ -484,19 +457,22 @@ public final class PluginProcessor * * * @param classLoader - * @param rootNodes + * @param nodes * * @return * * @throws IOException */ private Set createPluginWrappers(ClassLoader classLoader, - List rootNodes) + List nodes) throws IOException { Set plugins = Sets.newHashSet(); - appendPluginWrappers(plugins, classLoader, rootNodes); + for (PluginNode node : nodes) + { + appendPluginWrapper(plugins, classLoader, node); + } return plugins; } 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 2ebe06db02..1eb2704c90 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/PluginTree.java @@ -39,6 +39,8 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.util.Arrays; +import java.util.LinkedHashSet; +import java.util.LinkedList; import java.util.List; //~--- JDK imports ------------------------------------------------------------ @@ -134,13 +136,26 @@ public final class PluginTree * * @return */ - public List getRootNodes() + public List getLeafLastNodes() { - return rootNodes; + LinkedHashSet leafFirst = new LinkedHashSet<>(); + + rootNodes.forEach(node -> appendLeafFirst(leafFirst, node)); + + LinkedList leafLast = new LinkedList<>(); + + leafFirst.stream().map(PluginNodeHashWrapper::getPluginNode).forEach(leafLast::addFirst); + + return leafLast; } - //~--- methods -------------------------------------------------------------- + private void appendLeafFirst(LinkedHashSet leafFirst, PluginNode node) { + node.getChildren().forEach(child -> appendLeafFirst(leafFirst, child)); + leafFirst.add(new PluginNodeHashWrapper(node)); + } + + //~--- methods -------------------------------------------------------------- /** * Method description * @@ -235,8 +250,32 @@ public final class PluginTree append(buffer, indent + " ", child); } } -//~--- fields --------------------------------------------------------------- + + //~--- fields --------------------------------------------------------------- /** Field description */ private final List rootNodes = Lists.newArrayList(); + + private static class PluginNodeHashWrapper { + private final PluginNode pluginNode; + + private PluginNodeHashWrapper(PluginNode pluginNode) { + this.pluginNode = pluginNode; + } + + public PluginNode getPluginNode() { + return pluginNode; + } + + @Override + public int hashCode() { + return pluginNode.getId().hashCode(); + } + + @Override + public boolean equals(Object obj) { + return obj instanceof PluginNodeHashWrapper + && ((PluginNodeHashWrapper) obj).pluginNode.getId().equals(this.pluginNode.getId()); + } + } } 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 33a139aa5a..6e4e001885 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/PluginTreeTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/PluginTreeTest.java @@ -76,7 +76,7 @@ public class PluginTreeTest false, null, null); ExplodedSmp smp = createSmp(plugin); - new PluginTree(smp).getRootNodes(); + new PluginTree(smp).getLeafLastNodes(); } /** @@ -88,7 +88,7 @@ public class PluginTreeTest @Test(expected = PluginNotInstalledException.class) public void testPluginNotInstalled() throws IOException { - new PluginTree(createSmpWithDependency("b", "a")).getRootNodes(); + new PluginTree(createSmpWithDependency("b", "a")).getLeafLastNodes(); } /** @@ -98,10 +98,10 @@ public class PluginTreeTest * @throws IOException */ @Test - public void testRootNotes() throws IOException + public void testNodes() throws IOException { List smps = createSmps("a", "b", "c"); - List nodes = unwrapIds(new PluginTree(smps).getRootNodes()); + List nodes = unwrapIds(new PluginTree(smps).getLeafLastNodes()); assertThat(nodes, containsInAnyOrder("a", "b", "c")); } @@ -119,7 +119,7 @@ public class PluginTreeTest null, null); ExplodedSmp smp = createSmp(plugin); - new PluginTree(smp).getRootNodes(); + new PluginTree(smp).getLeafLastNodes(); } /** @@ -140,17 +140,31 @@ public class PluginTreeTest //J+ PluginTree tree = new PluginTree(smps); - List rootNodes = tree.getRootNodes(); + List nodes = tree.getLeafLastNodes(); - assertThat(unwrapIds(rootNodes), containsInAnyOrder("a")); + System.out.println(tree); - PluginNode a = rootNodes.get(0); + assertThat(unwrapIds(nodes), contains("a", "b", "c")); + } - assertThat(unwrapIds(a.getChildren()), containsInAnyOrder("b", "c")); + @Test + public void testComplexDependencies() throws IOException + { + //J- + ExplodedSmp[] smps = new ExplodedSmp[]{ + createSmpWithDependency("a", "b", "c", "d"), + createSmpWithDependency("b", "c"), + createSmpWithDependency("c"), + createSmpWithDependency("d") + }; + //J+ - PluginNode b = a.getChild("b"); + PluginTree tree = new PluginTree(smps); + List nodes = tree.getLeafLastNodes(); - assertThat(unwrapIds(b.getChildren()), containsInAnyOrder("c")); + System.out.println(tree); + + assertThat(unwrapIds(nodes), contains("d", "c", "b", "a")); } @Test @@ -162,15 +176,11 @@ public class PluginTreeTest }; PluginTree tree = new PluginTree(smps); - List rootNodes = tree.getRootNodes(); + List nodes = tree.getLeafLastNodes(); - assertThat(unwrapIds(rootNodes), containsInAnyOrder("a")); + System.out.println(tree); - PluginNode a = rootNodes.get(0); - assertThat(unwrapIds(a.getChildren()), containsInAnyOrder("b", "c")); - - PluginNode b = a.getChild("b"); - assertThat(unwrapIds(b.getChildren()), containsInAnyOrder("c")); + assertThat(unwrapIds(nodes), contains("a", "b", "c")); } @Test @@ -185,15 +195,9 @@ public class PluginTreeTest System.out.println(tree); - List rootNodes = tree.getRootNodes(); + List nodes = tree.getLeafLastNodes(); - assertThat(unwrapIds(rootNodes), containsInAnyOrder("a")); - - PluginNode a = rootNodes.get(0); - assertThat(unwrapIds(a.getChildren()), containsInAnyOrder("b")); - - PluginNode b = a.getChild("b"); - assertThat(unwrapIds(b.getChildren()), containsInAnyOrder("c")); + assertThat(unwrapIds(nodes), contains("a", "b", "c")); } @Test @@ -203,9 +207,9 @@ public class PluginTreeTest }; PluginTree tree = new PluginTree(smps); - List rootNodes = tree.getRootNodes(); + List nodes = tree.getLeafLastNodes(); - assertThat(unwrapIds(rootNodes), containsInAnyOrder("a")); + assertThat(unwrapIds(nodes), containsInAnyOrder("a")); } /**