NCLSUP-631 Improve processing of duplicate G:A in lockfiles and add o…

…ptional LENIENT lockmode feature

analyzer/src/main/java/org/jboss/gm/analyzer/alignment/io/LockFileIO.java

@@ -10,19 +10,18 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.Set;
 import java.util.stream.Collectors;
 
 import lombok.experimental.UtilityClass;
 
 import org.apache.commons.io.FileUtils;
 import org.commonjava.maven.atlas.ident.ref.InvalidRefException;
-import org.commonjava.maven.atlas.ident.ref.ProjectRef;
 import org.commonjava.maven.atlas.ident.ref.ProjectVersionRef;
 import org.commonjava.maven.atlas.ident.ref.SimpleProjectVersionRef;
 import org.commonjava.maven.ext.common.ManipulationUncheckedException;
 import org.gradle.api.logging.Logger;
+import org.jboss.gm.common.versioning.DynamicVersionParser;
 
 /**
  * Utility class for lock file I/O.
@@ -89,33 +88,31 @@ public void updateLockfiles(Logger logger, File directory,
         }
 
         for (File lockFile : locksFiles) {
-            Map<ProjectRef, ProjectVersionRef> lockedVersionsMap = readProjectVersionRefLocksOfFile(lockFile)
-                    .stream()
-                    .collect(Collectors.toMap(ProjectRef::asProjectRef,
-                            ProjectVersionRef::asProjectVersionRef));
-            Set<ProjectRef> lockedVersions = lockedVersionsMap.keySet();
+            logger.debug("Examining lockfile {}", lockFile);
 
             try {
                 List<String> lockFileLines = FileUtils.readLines(lockFile, Charset.defaultCharset());
-                alignedDependencies.values().forEach(aligned -> {
-                    ProjectRef versionlessAligned = aligned.asProjectRef();
+                alignedDependencies.forEach((key, value) -> {
+
+                    ProjectVersionRef keyProjectVersionRef = SimpleProjectVersionRef.parse(key);
+                    boolean dynamic = DynamicVersionParser.isDynamic(keyProjectVersionRef.getVersionString());
+                    String matcher = key;
                     // Its possible alignedDependencies has a DynamicValue recorded as key but the lockfile has an
-                    // explicit value. Therefore, we have to filter to find a partial match on group:artifact. This
-                    // assumes the lockfile does not have two GA with different V.
-                    Optional<String> result = alignedDependencies.keySet().stream()
-                            .filter(f -> f.contains(versionlessAligned.toString())).findFirst();
-                    if (!result.isPresent()) {
-                        logger.warn("Unable to find a match for {} against {} ", versionlessAligned, alignedDependencies);
-                    } else if (lockedVersions.contains(versionlessAligned)) {
-                        String replacement = alignedDependencies.get(result.get()).toString();
-                        for (int i = 0; i < lockFileLines.size(); i++) {
-                            String line = lockFileLines.get(i);
-                            if (line.contains(versionlessAligned.toString())) {
-                                logger.debug("Found lock file element '{}' to be replaced by {}", line, replacement);
-                                line = line.replaceFirst(versionlessAligned + ":([a-zA-Z0-9.]+)(=.*)*",
-                                        replacement + "$2");
-                                lockFileLines.set(i, line);
-                            }
+                    // explicit value. Therefore, we have to filter to find a partial match on group:artifact.
+                    // One potential issue is that multiple same group:artifact with dynamic versions will
+                    // all end up being replaced.
+                    if (dynamic) {
+                        matcher = keyProjectVersionRef.asProjectRef().toString();
+                    }
+
+                    for (int i = 0; i < lockFileLines.size(); i++) {
+                        String line = lockFileLines.get(i);
+                        if (line.contains(matcher)) {
+                            logger.debug("Found lock file element '{}' to be replaced by {}", line, value);
+                            line = line.replaceFirst(matcher +
+                                    (dynamic ? ":([a-zA-Z0-9.]+)(=.*)*" : "(=.*)*"),
+                                    value + (dynamic ? "$2" : "$1"));
+                            lockFileLines.set(i, line);
                         }
                     }
                 });

analyzer/src/test/java/org/jboss/gm/analyzer/alignment/io/LockFileIOTest.java

@@ -19,6 +19,7 @@
 import org.jboss.gm.common.rules.LoggingRule;
 import org.junit.Rule;
 import org.junit.Test;
+import org.junit.contrib.java.lang.system.SystemOutRule;
 import org.junit.rules.TemporaryFolder;
 
 import static org.assertj.core.api.Assertions.assertThat;
@@ -34,6 +35,9 @@ public class LockFileIOTest {
     @Rule
     public TemporaryFolder tempDir = new TemporaryFolder();
 
+    @Rule
+    public final SystemOutRule systemOutRule = new SystemOutRule().enableLog().muteForSuccessfulTests();
+
     @Test
     public void readNonExistingFileShouldReturnEmptySet()
             throws IOException {
@@ -51,6 +55,7 @@ public void readValidFileShouldReturnExpectedResults() throws URISyntaxException
                 .containsOnly(
                         tuple("undertow-core", "2.0.21.Final"),
                         tuple("commons-lang3", "3.8"),
+                        tuple("commons-lang3", "3.9"),
                         tuple("HdrHistogram", "2.1.10"),
                         tuple("guava", "25.1-android"),
                         tuple("xnio-nio", "3.3.8.Final"));
@@ -102,5 +107,8 @@ public void testUpdateLockFile() throws URISyntaxException, IOException {
         assertThat(FileUtils.readLines(locks.get(0), Charset.defaultCharset())).anyMatch(f -> f.contains(
                 "2.1.10.redhat-00001=compileClasspath,runtimeClasspath"));
 
+        assertThat(systemOutRule.getLog()).contains("Found lock file element 'org.apache.commons:commons-lang3:3"
+                + ".8' to be replaced by org.apache.commons:commons-lang3:3.8.redhat-00001");
+
     }
 }

analyzer/src/test/resources/compileClasspath.lockfile

@@ -1,6 +1,7 @@
 # Dummy similar to what gradle writes when --write-locks is enabled
 io.undertow:undertow-core:2.0.21.Final
 org.apache.commons:commons-lang3:3.8
+org.apache.commons:commons-lang3:3.9
 org.hdrhistogram:HdrHistogram:2.1.10=compileClasspath,runtimeClasspath
 # there are more transitives added by gradle, we only add one in order to not distract
 com.google.guava:guava:25.1-android

cli/src/main/java/org/jboss/gm/cli/Main.java

@@ -329,6 +329,10 @@ public Void call() throws ManipulationException {
             PluginUtils.pluginRemoval(logger, target, Collections.singleton(SEMANTIC_BUILD_VERSIONING));
         }
 
+        if (configuration.addLenientLockMode()) {
+            PluginUtils.addLenientLockMode(logger, target);
+        }
+
         GroovyUtils.runCustomGroovyScript(logger, InvocationStage.FIRST, target, configuration, null, null);
 
         PluginUtils.pluginRemoval(logger, target, configuration.pluginRemoval());

common/src/main/java/org/jboss/gm/common/Configuration.java

@@ -158,6 +158,10 @@ default Properties getProperties() {
     @Key("groovyScripts")
     String[] groovyScripts();
 
+    @Key("lenientLockMode")
+    @DefaultValue("false")
+    boolean addLenientLockMode();
+
     @Key("pluginRemoval")
     Set<String> pluginRemoval();
 

common/src/main/java/org/jboss/gm/common/utils/PluginUtils.java

@@ -40,6 +40,7 @@ public class PluginUtils {
                 Pair.of("nexusStaging", Stream.of("closeRepository", "releaseRepository", "closeAndReleaseRepository").collect(
                         Collectors.toSet())));
         SUPPORTED_PLUGINS.put("io.github.gradle-nexus.publish-plugin", Pair.of("nexusPublishing", Collections.emptySet()));
+        SUPPORTED_PLUGINS.put("nebula.publish-verification", Pair.of("nebulaPublishVerification", Collections.emptySet()));
         SUPPORTED_PLUGINS.put("signing", Pair.of("signing", Collections.emptySet()));
         SUPPORTED_PLUGINS.put(SEMANTIC_BUILD_VERSIONING, Pair.of("preRelease", Collections.emptySet()));
     }
@@ -97,13 +98,7 @@ public static void pluginRemoval(Logger logger, File target, Set<String> plugins
             for (File buildFile : files) {
                 try {
                     List<String> lines = org.apache.commons.io.FileUtils.readLines(buildFile, Charset.defaultCharset());
-                    String eol;
-                    try {
-                        eol = FileIO.determineEOL(buildFile).value();
-                    } catch (ManipulationException e) {
-                        logger.warn("Unable to determine EOL for {}", buildFile);
-                        eol = "\n";
-                    }
+                    String eol = getEOL(logger, buildFile);
 
                     // Remove the plugin
                     boolean removed = lines.removeIf(i -> i.contains(plugin));
@@ -195,4 +190,55 @@ public static boolean checkForSemanticBuildVersioning(Logger logger, File target
         }
         return false;
     }
+
+    public static void addLenientLockMode(Logger logger, File target)
+            throws ManipulationException {
+        final String depLock = "dependencyLocking {";
+
+        Collection<File> files = new HashSet<>();
+        final Collection<File> gradleFiles = FileUtils.listFiles(target, new WildcardFileFilter("*.gradle"),
+                TrueFileFilter.INSTANCE);
+        final Collection<File> kotlinFiles = FileUtils.listFiles(target, new WildcardFileFilter("*.gradle.kts"),
+                TrueFileFilter.INSTANCE);
+        files.addAll(gradleFiles);
+        files.addAll(kotlinFiles);
+
+        for (File buildFile : files) {
+            boolean removed = false;
+            try {
+                List<String> lines = org.apache.commons.io.FileUtils.readLines(buildFile, Charset.defaultCharset());
+
+                for (int i = 0; i < lines.size(); i++) {
+                    String line = lines.get(i);
+                    if (line.contains(depLock)) {
+                        removed = true;
+                        if (buildFile.toString().endsWith("kts")) {
+                            line = line.replace(depLock, depLock + "\n lockMode.set(LockMode.LENIENT) ");
+                        } else {
+                            line = line.replace(depLock, depLock + "\n lockMode = LockMode.LENIENT");
+                        }
+                        lines.set(i, line);
+                    }
+                }
+                if (removed) {
+                    String content = String.join(getEOL(logger, buildFile), lines);
+                    logger.debug("Added LENIENT lockMode to {}", buildFile);
+                    FileUtils.writeStringToFile(buildFile, content, Charset.defaultCharset());
+                }
+            } catch (IOException e) {
+                throw new ManipulationException("Unable to read build file {}", buildFile, e);
+            }
+        }
+    }
+
+    private static String getEOL(Logger logger, File buildFile) {
+        String eol;
+        try {
+            eol = FileIO.determineEOL(buildFile).value();
+        } catch (ManipulationException e) {
+            logger.warn("Unable to determine EOL for {}", buildFile);
+            eol = "\n";
+        }
+        return eol;
+    }
 }

common/src/test/java/org/jboss/gm/common/utils/PluginUtilsTest.java

@@ -533,4 +533,47 @@ public void testCheckForSemanticPlugin2()
         assertTrue(result);
         assertTrue(systemOutRule.getLog().contains("Found Semantic Build Versioning Plugin"));
     }
+
+    @Test
+    public void testLenientLockMode1()
+            throws IOException, ManipulationException {
+
+        File target = folder.newFile("build.gradle");
+        org.apache.commons.io.FileUtils.writeStringToFile(target,
+                "\n" + "buildscript {\n" + "    dependencyLocking {\n"
+                        + "        lockAllConfigurations()\n" + "    }\n" + "\n"
+                        + "    repositories {\n" + "        mavenCentral()\n"
+                        + "        gradlePluginPortal()\n"
+                        + "    }\n",
+                Charset.defaultCharset());
+
+        PluginUtils.addLenientLockMode(logger, target.getParentFile());
+        assertTrue(systemOutRule.getLog().contains("Added LENIENT lockMode"));
+        assertTrue(FileUtils.readFileToString(target, Charset.defaultCharset()).contains("buildscript {\n"
+                + "    dependencyLocking {\n"
+                + " lockMode = LockMode.LENIENT\n"
+                + "        lockAllConfigurations()\n"
+                + "    }\n"));
+    }
+
+    @Test
+    public void testLenientLockMode2()
+            throws IOException, ManipulationException {
+        File target = folder.newFile("build.gradle.kts");
+        org.apache.commons.io.FileUtils.writeStringToFile(target,
+                "\n" + "buildscript {\n" + "    dependencyLocking {\n"
+                        + "        lockAllConfigurations()\n" + "    }\n" + "\n"
+                        + "    repositories {\n" + "        mavenCentral()\n"
+                        + "        gradlePluginPortal()\n"
+                        + "    }\n",
+                Charset.defaultCharset());
+
+        PluginUtils.addLenientLockMode(logger, target.getParentFile());
+        assertTrue(systemOutRule.getLog().contains("Added LENIENT lockMode"));
+        assertTrue(FileUtils.readFileToString(target, Charset.defaultCharset()).contains("buildscript {\n"
+                + "    dependencyLocking {\n"
+                + " lockMode.set(LockMode.LENIENT) \n"
+                + "        lockAllConfigurations()\n"
+                + "    }\n"));
+    }
 }