From 2b9487f2a9a1a6da4d067d14179770a15e8271b5 Mon Sep 17 00:00:00 2001 From: JeremyStarTM Date: Wed, 4 Sep 2024 19:55:32 +0200 Subject: [PATCH] Add dependency cycle detection, fix tests --- .../base/utility/DependencyResolver.java | 74 +++++---- .../utility/DependencyResolverTest.java | 152 ++++++++++++++++-- 2 files changed, 184 insertions(+), 42 deletions(-) diff --git a/base/src/main/java/de/staropensource/sosengine/base/utility/DependencyResolver.java b/base/src/main/java/de/staropensource/sosengine/base/utility/DependencyResolver.java index 168a23d..0dbe670 100644 --- a/base/src/main/java/de/staropensource/sosengine/base/utility/DependencyResolver.java +++ b/base/src/main/java/de/staropensource/sosengine/base/utility/DependencyResolver.java @@ -19,6 +19,7 @@ package de.staropensource.sosengine.base.utility; +import de.staropensource.sosengine.base.exception.dependency.DependencyCycleException; import de.staropensource.sosengine.base.exception.dependency.UnmetDependenciesException; import de.staropensource.sosengine.base.implementable.VersioningSystem; import de.staropensource.sosengine.base.logging.LoggerInstance; @@ -51,14 +52,7 @@ public final class DependencyResolver { * * @since v1-alpha1 */ - Set vectors = new HashSet<>(); - - /** - * List of identifiers of already resolved vectors. - * - * @since v1-alpha4 - */ - Set<@NotNull String> vectorsResolved = new HashSet<>(); + @NotNull Set vectors = new HashSet<>(); /** * Contains whether the current {@link DependencyVector} list has been resolved successfully. @@ -87,7 +81,7 @@ public final class DependencyResolver { * @return itself * @since v1-alpha1 */ - public synchronized DependencyResolver addVector(@NotNull DependencyVector vector) { + public synchronized @NotNull DependencyResolver addVector(@NotNull DependencyVector vector) { try { vectors.add(vector); } catch (IllegalArgumentException ignored) {} @@ -102,7 +96,7 @@ public final class DependencyResolver { * @return itself * @since v1-alpha1 */ - public DependencyResolver addVectors(@NotNull DependencyVector[] vectors) { + public @NotNull DependencyResolver addVectors(@NotNull DependencyVector[] vectors) { return addVectors(Arrays.stream(vectors).toList()); } @@ -113,7 +107,7 @@ public final class DependencyResolver { * @return itself * @since v1-alpha1 */ - public DependencyResolver addVectors(@NotNull Collection vectors) { + public @NotNull DependencyResolver addVectors(@NotNull Collection vectors) { for (DependencyVector vector : vectors) // thread-safety addVector(vector); return this; @@ -128,18 +122,14 @@ public final class DependencyResolver { * @throws UnmetDependenciesException when dependencies are unmet * @since v1-alpha1 */ - public synchronized DependencyResolver resolve() throws IllegalStateException, UnmetDependenciesException { + public synchronized @NotNull DependencyResolver resolve() throws IllegalStateException, UnmetDependenciesException { Map<@NotNull DependencyVector, @NotNull String> unmetDependencies = new HashMap<>(); List<@NotNull String> output; for (DependencyVector vector : vectors) { - if (!vectorsResolved.contains(vector.getIdentifier())) { - output = resolveVector(vector); - for (String item : output) - unmetDependencies.put(vector, item); - - vectorsResolved.add(vector.getIdentifier()); - } + output = resolveVector(vector, new LinkedHashSet<>()); + for (String item : output) + unmetDependencies.put(vector, item); } if (!unmetDependencies.isEmpty()) @@ -158,10 +148,10 @@ public final class DependencyResolver { * @throws Exception when some unknown error occurs * @since v1-alpha4 */ - private @NotNull List<@NotNull String> resolveVector(@NotNull DependencyVector vector) throws IllegalStateException { + private @NotNull List<@NotNull String> resolveVector(@NotNull DependencyVector vector, @NotNull LinkedHashSet<@NotNull String> vectorsDependencyStack) throws IllegalStateException { List<@NotNull String> unmetDependencies = new ArrayList<>(); - // provides + vectorsDependencyStack.add(vector.getIdentifier()); // 0 = identifier // 1 = version equal @@ -250,20 +240,39 @@ public final class DependencyResolver { } } - // Resolve vector - DependencyVector dependencyResolved = getMatchingVector(identifier.toString()); - if (dependencyResolved == null) { + // Get vector with same identifier + DependencyVector vectorDependency = getMatchingVector(identifier.toString()); + if (vectorDependency == null) { unmetDependencies.add("Dependency \"" + dependency + "\" is not met: Not found"); continue; } + // Resolve vector + if (vectorsDependencyStack.contains(vectorDependency.getIdentifier())) { + StringBuilder cycle = new StringBuilder(); + + for (String component : vectorsDependencyStack) { + if (!cycle.isEmpty()) + cycle.append(" -> "); + + cycle.append(component); + } + + cycle + .append(" -> ") + .append(identifier); + + throw new DependencyCycleException("Dependency cycle detected: " + cycle); + } else + resolveVector(vectorDependency, vectorsDependencyStack); + VersioningSystem versioningSystemResolved; // Get resolved versioning system try { - versioningSystemResolved = dependencyResolved.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(dependencyResolved.getVersion()); + versioningSystemResolved = vectorDependency.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(vectorDependency.getVersion()); } catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException exception) { - logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + dependencyResolved.getVersioningSystem().getName(), exception); + logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + vectorDependency.getVersioningSystem().getName(), exception); break; } @@ -273,9 +282,9 @@ public final class DependencyResolver { // Get expected VersioningSystem try { - versioningSystemEquals = dependencyResolved.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(versionEqual.toString()); + versioningSystemEquals = vectorDependency.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(versionEqual.toString()); } catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException exception) { - logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + dependencyResolved.getVersioningSystem().getName(), exception); + logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + vectorDependency.getVersioningSystem().getName(), exception); break; } @@ -289,17 +298,17 @@ public final class DependencyResolver { if (!versionSmaller.isEmpty()) // Get expected VersioningSystem try { - versioningSystemSmaller = dependencyResolved.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(versionSmaller.toString()); + versioningSystemSmaller = vectorDependency.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(versionSmaller.toString()); } catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException exception) { - logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + dependencyResolved.getVersioningSystem().getName(), exception); + logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + vectorDependency.getVersioningSystem().getName(), exception); break; } if (!versionBigger.isEmpty()) // Get expected VersioningSystem try { - versioningSystemBigger = dependencyResolved.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(versionBigger.toString()); + versioningSystemBigger = vectorDependency.getVersioningSystem().getDeclaredConstructor(String.class).newInstance(versionBigger.toString()); } catch (NoSuchMethodException | InstantiationException | IllegalAccessException | InvocationTargetException exception) { - logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + dependencyResolved.getVersioningSystem().getName(), exception); + logger.crash("Unable to check version of dependency \"" + dependency + "\": Unable to initialize versioning system " + vectorDependency.getVersioningSystem().getName(), exception); break; } @@ -318,6 +327,7 @@ public final class DependencyResolver { } } + vectorsDependencyStack.remove(vector.getIdentifier()); return unmetDependencies; } diff --git a/base/src/test/java/de/staropensource/sosengine/base/srctests/utility/DependencyResolverTest.java b/base/src/test/java/de/staropensource/sosengine/base/srctests/utility/DependencyResolverTest.java index 235ce80..0344644 100644 --- a/base/src/test/java/de/staropensource/sosengine/base/srctests/utility/DependencyResolverTest.java +++ b/base/src/test/java/de/staropensource/sosengine/base/srctests/utility/DependencyResolverTest.java @@ -19,6 +19,7 @@ package de.staropensource.sosengine.base.srctests.utility; +import de.staropensource.sosengine.base.exception.dependency.DependencyCycleException; import de.staropensource.sosengine.base.exception.dependency.UnmetDependenciesException; import de.staropensource.sosengine.base.implementation.versioning.OneNumberVersioningSystem; import de.staropensource.sosengine.base.srctests.TestBase; @@ -26,6 +27,7 @@ import de.staropensource.sosengine.base.type.DependencyVector; import de.staropensource.sosengine.base.utility.DependencyResolver; import org.jetbrains.annotations.NotNull; import org.junit.jupiter.api.DisplayName; +import org.junit.jupiter.api.Test; import org.junit.jupiter.params.ParameterizedTest; import org.junit.jupiter.params.provider.ValueSource; @@ -165,6 +167,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 5 + dependencies = new HashSet<>(dependencies); dependencies.add("test4>998<1000"); vectors.add( new DependencyVector.Builder() @@ -237,6 +240,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 6p + dependencies = new HashSet<>(dependencies); dependencies.add("test5>998<1000"); vectors.add( new DependencyVector.Builder() @@ -247,6 +251,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 7 + dependencies = new HashSet<>(dependencies); dependencies.add("test6<7900"); vectors.add( new DependencyVector.Builder() @@ -258,8 +263,8 @@ class DependencyResolverTest extends TestBase { ); } default -> { - getLogger().error("layers=" + layers + " is unimplemented"); - throw new IllegalStateException("layers=" + layers + " is unimplemented"); + getLogger().error("layers=" + layers + " is unimplemented in testResolve()"); + throw new IllegalStateException("layers=" + layers + " is unimplemented in testResolve()"); } } @@ -267,11 +272,14 @@ class DependencyResolverTest extends TestBase { try { resolver.resolve(); } catch (UnmetDependenciesException exception) { - getLogger().error("Dependency resolution failed:"); + getLogger().error("Dependency resolution failed in testResolve(layers=" + layers + "):"); for (DependencyVector vector : exception.getUnmetDependencies().keySet()) getLogger().error("-> " + vector.getIdentifier() + "=" + vector.getVersion() + ": " + exception.getUnmetDependencies().get(vector)); - assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution failed (layers=" + layers + "): See logs"); + assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution failed in testResolve(layers=" + layers + "): See logs"); + } catch (DependencyCycleException exception) { + getLogger().error("Dependency resolution failed because of circular dependencies in testResolve(layers=" + layers + "): " + exception.getMessage()); + assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution failed in testResolve(layers=" + layers + "): See logs"); } } @@ -321,6 +329,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 3 + dependencies = new HashSet<>(dependencies); dependencies.add("test2>600<650"); vectors.add( new DependencyVector.Builder() @@ -351,8 +360,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 2 - dependencies = new HashSet<>(); - dependencies.add("testNULL"); + dependencies = new HashSet<>(dependencies); dependencies.add("test1>999"); vectors.add( new DependencyVector.Builder() @@ -383,6 +391,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 5 + dependencies = new HashSet<>(dependencies); dependencies.add("test4<998>990"); vectors.add( new DependencyVector.Builder() @@ -455,6 +464,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 6 + dependencies = new HashSet<>(dependencies); dependencies.add("test5>998<100"); vectors.add( new DependencyVector.Builder() @@ -465,6 +475,7 @@ class DependencyResolverTest extends TestBase { .build() ); // Vector 7 + dependencies = new HashSet<>(dependencies); dependencies.add("test6>7900"); vectors.add( new DependencyVector.Builder() @@ -476,8 +487,8 @@ class DependencyResolverTest extends TestBase { ); } default -> { - getLogger().error("layers=" + layers + " is unimplemented"); - throw new IllegalStateException("layers=" + layers + " is unimplemented"); + getLogger().error("layers=" + layers + " is unimplemented in testResolveWithFailure()"); + throw new IllegalStateException("layers=" + layers + " is unimplemented in testResolveWithFailure()"); } } @@ -485,13 +496,134 @@ class DependencyResolverTest extends TestBase { try { resolver.resolve(); } catch (UnmetDependenciesException exception) { - getLogger().error("Dependency resolution failed (great!):"); + getLogger().error("Dependency resolution failed in testResolve(layers=" + layers + ") (great!):"); for (DependencyVector vector : exception.getUnmetDependencies().keySet()) getLogger().error("-> " + vector.getIdentifier() + "=" + vector.getVersion() + ": " + exception.getUnmetDependencies().get(vector)); return; + } catch (DependencyCycleException exception) { + getLogger().error("Dependency resolution failed because of circular dependencies in testResolveWithFailure(layers=" + layers + "): " + exception.getMessage()); + assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution failed testResolveWithFailure(layers=" + layers + "): See logs"); } - assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution succeeded (layers=" + layers + ")"); + assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution succeeded testResolveWithFailure(layers=" + layers + ")"); + } + + @Test + @DisplayName("resolve (dependency cycle)") + void testResolveDependencyCycle() { + if (isRestricted()) return; + + getLogger().testCall("testResolveDependencyCycle"); + DependencyResolver resolver = new DependencyResolver(); + Set<@NotNull DependencyVector> vectors = new HashSet<>(); + Set<@NotNull String> dependencies = new HashSet<>(); + + // Vector 0 + dependencies.add("test7=110"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test0") + .setVersion("49") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + // Vector 1 + dependencies = new HashSet<>(); + dependencies.add("test0"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test1") + .setVersion("999") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + // Vector 2 + dependencies = new HashSet<>(); + dependencies.add("test1"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test2") + .setVersion("666") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + // Vector 3 + dependencies = new HashSet<>(); + dependencies.add("test2<700"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test3") + .setVersion("666") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + + // Vector 4 + dependencies = new HashSet<>(); + dependencies.add("test3=666"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test4") + .setVersion("49") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + // Vector 5 + dependencies = new HashSet<>(); + dependencies.add("test4<50"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test5") + .setVersion("999") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + // Vector 6 + dependencies = new HashSet<>(); + dependencies.add("test5>990<1000"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test6") + .setVersion("6978") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + // Vector 7 + dependencies = new HashSet<>(); + dependencies.add("test6<7900"); + vectors.add( + new DependencyVector.Builder() + .setIdentifier("test7") + .setVersion("110") + .setVersioningSystem(OneNumberVersioningSystem.class) + .setDependencies(dependencies) + .build() + ); + + resolver.addVectors(vectors); + try { + resolver.resolve(); + } catch (UnmetDependenciesException exception) { + getLogger().error("Dependency resolution failed in testResolveDependencyCycle():"); + for (DependencyVector vector : exception.getUnmetDependencies().keySet()) + getLogger().error("-> " + vector.getIdentifier() + "=" + vector.getVersion() + ": " + exception.getUnmetDependencies().get(vector)); + + assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution failed in testResolveDependencyCycle(): See logs"); + } catch (DependencyCycleException exception) { + getLogger().error("Dependency resolution failed because of circular dependencies in testResolveDependencyCycle() (great!):"); + getLogger().error(exception.getMessage()); + + return; + } + + assertEquals("Please ignore this, this just exists to trigger an error", "", "Dependency resolution succeeded in testResolveDependencyCycle()"); } }