From 931133353d593ddf7a75f50c3d7b3aa6ca902153 Mon Sep 17 00:00:00 2001 From: Rene Pfeuffer Date: Fri, 31 Jan 2020 12:27:02 +0100 Subject: [PATCH] Add detection of circular dependencies --- .../java/sonia/scm/plugin/ExplodedSmp.java | 56 +----- .../java/sonia/scm/plugin/SmpNodeBuilder.java | 16 ++ .../sonia/scm/plugin/ExplodedSmpTest.java | 173 ------------------ .../sonia/scm/plugin/SmpNodeBuilderTest.java | 11 ++ .../sonia/scm/plugin/scm-c-plugin.smp | Bin 5727 -> 5487 bytes 5 files changed, 28 insertions(+), 228 deletions(-) delete mode 100644 scm-webapp/src/test/java/sonia/scm/plugin/ExplodedSmpTest.java diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/ExplodedSmp.java b/scm-webapp/src/main/java/sonia/scm/plugin/ExplodedSmp.java index 60bcc09dcb..0dc986ee4e 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/ExplodedSmp.java +++ b/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 +public final class ExplodedSmp { private static final Logger logger = LoggerFactory.getLogger(ExplodedSmp.class); @@ -90,60 +90,6 @@ public final class ExplodedSmp implements Comparable return new ExplodedSmp(directory, Plugins.parsePluginDescriptor(desc)); } - /** - * {@inheritDoc} - */ - @Override - public int compareTo(ExplodedSmp o) - { - int result; - - Set depends = plugin.getDependenciesInclusiveOptionals(); - Set 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 ---------------------------------------------------------- /** diff --git a/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java b/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java index ed8eb6d840..115fd9a0b0 100644 --- a/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java +++ b/scm-webapp/src/main/java/sonia/scm/plugin/SmpNodeBuilder.java @@ -29,6 +29,8 @@ class SmpNodeBuilder { otherNode.addChild(node); } }); + + nodes.forEach(this::checkForCircle); }); return nodes; @@ -47,4 +49,18 @@ class SmpNodeBuilder { } }); } + + 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)); + } } diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/ExplodedSmpTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/ExplodedSmpTest.java deleted file mode 100644 index c4a4c0a0d3..0000000000 --- a/scm-webapp/src/test/java/sonia/scm/plugin/ExplodedSmpTest.java +++ /dev/null @@ -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 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 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 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 list(ExplodedSmp... e) - { - List es = Lists.newArrayList(e); - - Collections.sort(es); - - return es; - } - - //~--- get methods ---------------------------------------------------------- - - /** - * Method description - * - * - * @param es - * @param p - * @param a - */ - private void is(List es, int p, String a) - { - assertEquals(a, es.get(p).getPlugin().getInformation().getName()); - } -} diff --git a/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java b/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java index d40c0ff66b..4a94cb64c5 100644 --- a/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java +++ b/scm-webapp/src/test/java/sonia/scm/plugin/SmpNodeBuilderTest.java @@ -60,6 +60,17 @@ public class SmpNodeBuilderTest { 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 nodes, String id) { return nodes.stream().filter(node -> node.getId().equals(id)).findAny().orElseThrow(() -> new AssumptionViolatedException("node not found")); } diff --git a/scm-webapp/src/test/resources/sonia/scm/plugin/scm-c-plugin.smp b/scm-webapp/src/test/resources/sonia/scm/plugin/scm-c-plugin.smp index b80169b9b594a9560803fbdde39124e020dd4834..9e4c00ba713ae547b593acf85b7411b2183819f9 100644 GIT binary patch delta 1522 zcmZ{k&1(~35XL9jY`2?c&AR!rNsOixq@of0C{csjVk%M$r9@CcG1lJlAq1)*6zrjT zQG^$YSHVBPQt;4|y$j+^LF`Eoig*@W>dbDkna%D7^11Ibzh`Ej$?l0i$K{22DH0W? zp2_54soCgum%FEK)>V-5=32dS;OsrI8B3I-K#!xH^{Vxu_i_*hnj6*n%*pGuIj^{4 zi+|oRYh0Z26L*(db^&wy(%l|ClNtpQzqsGoxE%p7RDdIp7Z;5Q6@Ai4b6adEg{{R< zEpi&$u`t7CRpfb1ChZQ1aTvS%@b%l<_bAsx07+!*8<{a=WN{W$RFp?{rh@<)2RMz; z7#U>m!Q}R3Utd-ulKE`_OschA0y^Twp? zs49H}-b8Z)uVmo)W*swJs>>o0V!vL@NpOZ;~2yDmw|PU2K<9ibd^zKK#M^Jn00?;$)$W~DXk?LQrkD% wyC%HxPmWsCouxq&v^1{y(cO3N3oo-4C;MbVqwR5wTuA4ld-$i>)-hV}AE(|lhX4Qo delta 2871 zcmb7`3s4i+8pk&YBmset2NHtXsNww-LO?+@UcmsChz|$|DhdLY02&NXq%i_QFUk<3 zfQY5iC{(YA2wb5+d9=!afKo-dRbJ&~O9+nvCWFY`jRf~z$&CFnIkR*2od5SfJNtdV zm6`Wis_*easgcAoqTX68Kn0FP2J?8KJevdiwE@hUg>(C}YFvY$zo<=LJVhiJ<6(=w zblcCg^6++FakU-*3>}0S9wJjflafABoQPR|5cO`SQs;H9*s<&8p15Hu}7q(!MSN z07zhdHbEcCO#vSbbPMt?)F88Q6AkCnYAi2FFAaB>ksBtzjvqK+9NBya=Qx^PaIa4oDvK@bIPT-MZ5WPvJj+y zHVN+pkee)=Oy%b0fw?)2eYjoUFC3~1&e_v)i%`k>sW9)_yzfitsAnF!YHP%f4BNS4 zZT>OM=O>sM7X@c3ZitO8(hCPsY3IL~-asG2KOG3{WW2ay9AY(>T3kh`8u+PWZ`M?! zSWJp6-%?n!=%F2Z(YMT{t7cRC7?HaA>v&@Vk@P&Z-;

G~2T_?@nE~L@MjR{}T9c z$F-*YF%)KWf zoXV~HX}-mH48D>ab3Pxac%A3ytn(&C=sMAvso$mP9N(hj`&(viR|($lnw2Rk`$^dd z)#&VB2l8B=nO$uRxyVEYmjx8X)-@jy?c8_C4m)5r{>b0cBq6)cecKUEcXYI&7J8D> zJa@p8oJE>p$&|uP2qpO*^+uTo3*|hev zR_9DlSrvt38hX#CxtgfCp~dOtmXp<}-5$B!+3kWmx@&hP;av~<(jMOUCXTTWY>oQ!LZVqA*l3h$bX zQw8XW8tTJ>fWy~$!iwY@$-(@q=kvqE`7dmK30db;?n+~9ywB>mEV>uZzuc{sQt`Y4 zF*RJ0lo(iC64cq%@EaZXx&e8eX`577nOXvj2%kmq^Hci^ozIQmir+LGBTFz0W;~w4 zIBxur{~ce3X6MY5M#&}*-E&-%xzP0fAyLpqwwV0u_xQPQD82HYp_}S+ZpR7)<4ig~ zWcR<7)Acxu7sD8evAzOOcWnH*KR~i0Stj{j1yR)_XibwQY(z!mAYiTt@rESc5 z-uEK~gMF>MSo@HZrAISp)3pV5N%PIsJ7NxQT`Zpcqc`^PU1w3xF58UGX^RpsBb|2c zw+y8I!MZpDnY;({$G?n5;%qEK_E+hazdp=Tz3hVNTVE=acpeAm|=|Zg*a$@9D=nnq|6d^;P0UO!?~f42MFG9B!lo)ssa+>jQC4}0b~jea@D2;5XO}u zWnwTeQ=bR*|1}0LDG=*QWzg8(|Cf%5JRKGx#t|_}w@U7-cFTflQyEewNekvyMWw11 z!-K`E)B<0aU@ZVDWs^@zS*3zL4!*x$v44}-Qe}O{8wOQ7ymG;6U-lyrT53anY;0B4 j{>ruhthkS)2QwxS&f*k8E0(>do}uPG@I{=}g);van8Ctk