Merge scopes when adding a dependency that exists but with different scope

components/sbm-core/src/main/java/org/springframework/sbm/build/api/DependencyChangeResolver.java

@@ -0,0 +1,89 @@
+package org.springframework.sbm.build.api;
+
+import lombok.NonNull;
+import org.apache.commons.lang3.tuple.Pair;
+import org.openrewrite.maven.tree.Scope;
+
+import java.util.*;
+import java.util.stream.Collectors;
+
+import static org.openrewrite.maven.tree.Scope.*;
+
+/**
+ * Resolve the dependency change spec. Their is a ascending
+ * order of the scopes where a higher order covers its predecessor
+ * and more. Below is the ascending order of the scopes.
+ * <p>
+ * Test (lowest), Runtime, Provided, Compile (highest)
+ * <p>
+ * Based on the above scope, the following rule decides the fate
+ * of a proposed dependency change spec.
+ * <p>
+ * Rule 1 :- If the proposed dependency already exists
+ * transitively but its scope is lesser than the proposed
+ * scope, the proposed dependency will be added to  the
+ * build file.
+ * <p>
+ * Rule 2 :- If the proposed dependency already declared
+ * directly but its scope is lesser than the the proposed
+ * scope, the existing dependency will be replaced.
+ * <p>
+ * Rule 3 :- If there is no matching dependency already exists
+ * the proposed dependency will beadded.
+ */
+public class DependencyChangeResolver {
+
+    public static final EnumSet<Scope> ALL_SCOPES = EnumSet.range(None, System);
+    @NonNull
+    private BuildFile buildFile;
+    private @NonNull Dependency proposedChangeSpec;
+    private List<Dependency> potentialMatches;
+
+    private static Map<Scope, List<Scope>> UPGRADE_GRAPH = new HashMap<>();
+
+    static {
+        // For a given scope (key), SBM will upgrade ( upsert) if any of the listed scope
+        // exists in the directly included dependencies
+        UPGRADE_GRAPH.put(Compile, List.of(Test, Provided, Runtime));
+        UPGRADE_GRAPH.put(Provided, List.of(Test, Runtime));
+        UPGRADE_GRAPH.put(Runtime, List.of(Test));
+        UPGRADE_GRAPH.put(Test, Collections.emptyList());
+    }
+
+    public DependencyChangeResolver(BuildFile buildFile, @NonNull Dependency proposedChangeSpec) {
+        this.buildFile = buildFile;
+        this.proposedChangeSpec = proposedChangeSpec;
+        this.potentialMatches = buildFile.getEffectiveDependencies()
+                .stream()
+                .filter(d -> d.equals(this.proposedChangeSpec))
+                .collect(Collectors.toList());
+    }
+
+    /**
+     * Return a pair of dependencies to be removed ( left) and added ( right)
+     * @return
+     */
+    public Pair<List<Dependency>, Optional<Dependency>> apply() {
+        if (potentialMatches.isEmpty())
+            return Pair.of(Collections.emptyList(), Optional.of(proposedChangeSpec));
+
+        Scope proposedDependencyScope = Scope.fromName(proposedChangeSpec.getScope());
+        List<Scope> supersededScopes = UPGRADE_GRAPH.get(proposedDependencyScope);
+
+        Optional<Dependency> right = potentialMatches
+                .stream()
+                .filter(d -> supersededScopes.contains(fromName(d.getScope())))
+                .findAny()
+                .map(any -> proposedChangeSpec);
+
+        List<Dependency> left = buildFile
+                .getDeclaredDependencies(ALL_SCOPES.toArray(new Scope[0]))
+                .stream()
+                .filter(proposedChangeSpec::equals)
+                .collect(Collectors.toList());
+
+        return Pair.of(left, right);
+
+    }
+
+}

components/sbm-core/src/main/java/org/springframework/sbm/build/migration/actions/AddDependencies.java

@@ -16,8 +16,10 @@
 package org.springframework.sbm.build.migration.actions;
 
 import lombok.AllArgsConstructor;
+import org.apache.commons.lang3.tuple.Pair;
 import org.springframework.sbm.build.api.BuildFile;
 import org.springframework.sbm.build.api.Dependency;
+import org.springframework.sbm.build.api.DependencyChangeResolver;
 import org.springframework.sbm.engine.recipe.AbstractAction;
 import org.springframework.sbm.engine.context.ProjectContext;
 import lombok.Getter;
@@ -29,6 +31,9 @@
 import javax.validation.Valid;
 import java.util.ArrayList;
 import java.util.List;
+import java.util.Optional;
+import java.util.function.Predicate;
+import java.util.stream.Collectors;
 
 @Setter
 @Getter
@@ -51,6 +56,23 @@ public AddDependencies(List<Dependency> dependencies) {
     @Override
     public void apply(ProjectContext context) {
         BuildFile buildFile = context.getBuildFile();
-        buildFile.addDependencies(dependencies);
+        List<Pair<List<Dependency>, Optional<Dependency>>> pairs = dependencies.stream()
+                .map(d -> new DependencyChangeResolver(buildFile, d))
+                .map(DependencyChangeResolver::apply)
+                .collect(Collectors.toList());
+
+        List<Dependency> removeList = pairs.stream()
+                .map(Pair::getLeft)
+                .flatMap(List::stream)
+                .collect(Collectors.toList());
+
+        List<Dependency> addList = pairs.stream()
+                .map(Pair::getRight)
+                .filter(Optional::isPresent)
+                .map(Optional::get)
+                .collect(Collectors.toList());
+
+        buildFile.removeDependencies(removeList);
+        buildFile.addDependencies(addList);
     }
 }

components/sbm-core/src/test/java/org/springframework/sbm/build/api/DependencyChangeResolverTest.java

@@ -0,0 +1,142 @@
+package org.springframework.sbm.build.api;
+
+import org.apache.commons.lang3.tuple.Pair;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.extension.ExtendWith;
+import org.mockito.Mock;
+import org.mockito.junit.jupiter.MockitoExtension;
+import org.springframework.sbm.build.impl.OpenRewriteMavenBuildFile;
+
+import java.util.Collections;
+import java.util.List;
+import java.util.Optional;
+import java.util.Set;
+
+import static org.assertj.core.api.Assertions.assertThat;
+import static org.mockito.Mockito.any;
+import static org.mockito.Mockito.when;
+
+@ExtendWith(MockitoExtension.class)
+public class DependencyChangeResolverTest {
+
+    @Mock
+    OpenRewriteMavenBuildFile buildFile;
+
+    @Test
+    public void givenBuildFile_addNewDependency_withScopeCompile_expectNewDependencyAdded(){
+        Dependency proposedDependency =
+                new Dependency.DependencyBuilder()
+                .groupId("org.springframework.sbm")
+                .artifactId("directDependency")
+                .version("1.0")
+                .scope("compile")
+                .build();
+
+        when(buildFile.getEffectiveDependencies())
+                .thenReturn(Collections.emptySet());
+
+        Pair<List<Dependency>, Optional<Dependency>> pair =
+                new DependencyChangeResolver(buildFile, proposedDependency).apply();
+
+        assertThat(pair.getLeft().isEmpty());
+        assertThat(pair.getRight().isPresent());
+        assertThat(pair.getRight().get().equals(proposedDependency));
+    }
+
+    @Test
+    public void givenBuildFile_addExistingTransitiveDependency_withLowerScope_expectNoOp(){
+        Dependency proposedDependency =
+                new Dependency.DependencyBuilder()
+                        .groupId("org.springframework.sbm")
+                        .artifactId("directDependency")
+                        .version("1.0")
+                        .scope("test")
+                        .build();
+
+        Dependency existingDependency =
+                new Dependency.DependencyBuilder()
+                        .groupId("org.springframework.sbm")
+                        .artifactId("directDependency")
+                        .version("1.0")
+                        .scope("compile")
+                        .build();
+
+        when(buildFile.getEffectiveDependencies())
+                .thenReturn(Set.of(existingDependency));
+
+        when(buildFile.getDeclaredDependencies(any()))
+                .thenReturn(Collections.emptyList());
+
+        Pair<List<Dependency>, Optional<Dependency>> pair =
+                new DependencyChangeResolver(buildFile, proposedDependency).apply();
+
+        assertThat(pair.getLeft().isEmpty());
+        assertThat(pair.getRight().isEmpty());
+    }
+
+    @Test
+    public void givenBuildFile_addExistingTransitiveDependency_withHigherScope_expectNoOp(){
+        Dependency proposedDependency =
+                new Dependency.DependencyBuilder()
+                        .groupId("org.springframework.sbm")
+                        .artifactId("directDependency")
+                        .version("1.0")
+                        .scope("compile")
+                        .build();
+
+        Dependency existingDependency =
+                new Dependency.DependencyBuilder()
+                        .groupId("org.springframework.sbm")
+                        .artifactId("directDependency")
+                        .version("1.0")
+                        .scope("test")
+                        .build();
+
+        when(buildFile.getEffectiveDependencies())
+                .thenReturn(Set.of(existingDependency));
+
+        when(buildFile.getDeclaredDependencies(any()))
+                .thenReturn(Collections.emptyList());
+
+        Pair<List<Dependency>, Optional<Dependency>> pair =
+                new DependencyChangeResolver(buildFile, proposedDependency).apply();
+
+        assertThat(pair.getLeft().isEmpty());
+        assertThat(pair.getRight().isPresent());
+        assertThat(pair.getRight().get().equals(proposedDependency));
+    }
+
+    @Test
+    public void givenBuildFile_addExistingDirectDependency_withHigherScope_expectNoOp(){
+        Dependency proposedDependency =
+                new Dependency.DependencyBuilder()
+                        .groupId("org.springframework.sbm")
+                        .artifactId("directDependency")
+                        .version("1.0")
+                        .scope("compile")
+                        .build();
+
+        Dependency existingDependency =
+                new Dependency.DependencyBuilder()
+                        .groupId("org.springframework.sbm")
+                        .artifactId("directDependency")
+                        .version("1.0")
+                        .scope("test")
+                        .build();
+
+        when(buildFile.getEffectiveDependencies())
+                .thenReturn(Set.of(existingDependency));
+
+        when(buildFile.getDeclaredDependencies(any()))
+                .thenReturn(List.of(existingDependency));
+
+        Pair<List<Dependency>, Optional<Dependency>> pair =
+                new DependencyChangeResolver(buildFile, proposedDependency).apply();
+
+        assertThat(!pair.getLeft().isEmpty());
+        assertThat(pair.getLeft().get(0).getScope().equals("test"));
+        assertThat(pair.getRight().isPresent());
+        assertThat(pair.getRight().get().getScope().equals("compile"));
+    }
+
+}