Repository: vackosar/gitflow-incremental-builder
Pull request #67 (2018-11-04 14:52:14)

Fix issue #66: Regex-support for forceBuildModules
The value is still first split by "," but then each part
is compiled to a java.util.Pattern.

Additional changes:
- cleaned up exception handling in Configuration, also to
  provide more context for the user
- introduced dedicated test for Configuration
  (which does not yet cover all cases but it is a start)
- removed toRealPath() in Configuration.parseKey()
  (to align with PR #63)


Patch:
diff --git a/README.md b/README.md
index 012f9aa..2906ec2 100644
--- a/README.md
+++ b/README.md
@@ -293,6 +293,10 @@ Defines artifact ids of modules to build forcibly, even if these modules have no
 ```
 mvn clean install -Dgib.forceBuildModules=unchanged-module-1,unchanged-module-2
 ```
+Regular expressions are also supported in each comma separated part, e.g.:
+```
+mvn clean install -Dgib.forceBuildModules=unchanged-module-.*,another-module
+```
 Each of these modules is subject to `argsForNotImpactedModules` and `skipTestsForNotImpactedModules`.
 
 This property has no effect in case `buildAll` is enabled.
diff --git a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/Configuration.java b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/Configuration.java
index bfe88f4..5083c89 100644
--- a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/Configuration.java
+++ b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/Configuration.java
@@ -2,13 +2,15 @@ package com.vackosar.gitflowincrementalbuild.boundary;
 
 import com.vackosar.gitflowincrementalbuild.control.Property;
 
-import org.apache.maven.MavenExecutionException;
 import org.apache.maven.execution.MavenExecutionRequest;
 import org.apache.maven.execution.MavenSession;
 
 import javax.inject.Inject;
 import javax.inject.Singleton;
-import java.io.IOException;
+
+import static java.util.stream.Collectors.collectingAndThen;
+import static java.util.stream.Collectors.toList;
+
 import java.nio.file.Path;
 import java.util.AbstractMap;
 import java.util.Arrays;
@@ -16,10 +18,15 @@ import java.util.Collections;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
+import java.util.Map.Entry;
 import java.util.Optional;
+import java.util.Set;
 import java.util.function.Predicate;
 import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+import java.util.stream.Collector;
 import java.util.stream.Collectors;
+import java.util.stream.Stream;
 
 @Singleton
 public class Configuration {
@@ -38,7 +45,7 @@ public class Configuration {
     public final boolean skipTestsForNotImpactedModules;
     public final Map<String, String> argsForNotImpactedModules;
     public final boolean buildAll;
-    public final List<String> forceBuildModules;
+    public final List<Pattern> forceBuildModules;
     public final List<String> excludeTransitiveModulesPackagedAs;
     public final boolean compareToMergeBase;
     public final boolean fetchBaseBranch;
@@ -47,58 +54,69 @@ public class Configuration {
     public final boolean failOnMissingGitDir;
 
     @Inject
-    public Configuration(MavenSession session) throws IOException {
-        try {
-            checkProperties();
-            enabled = Boolean.valueOf(Property.enabled.getValue());
-            key = parseKey(session);
-            disableBranchComparison = Boolean.valueOf(Property.disableBranchComparison.getValue());
-            referenceBranch = Property.referenceBranch.getValue();
-            baseBranch = Property.baseBranch.getValue();
-            uncommited = Boolean.valueOf(Property.uncommited.getValue());
-            untracked = Boolean.valueOf(Property.untracked.getValue());
-            makeUpstream = alsoMakeBehaviours.contains(session.getRequest().getMakeBehavior());
-            skipTestsForNotImpactedModules = Boolean.valueOf(Property.skipTestsForNotImpactedModules.getValue());
-            argsForNotImpactedModules = Collections.unmodifiableMap(
-                    parseSpaceDelimitedArgs(Property.argsForNotImpactedModules.getValue()));
-            buildAll = Boolean.valueOf(Property.buildAll.getValue());
-            forceBuildModules = Collections.unmodifiableList(
-                    parseDelimited(Property.forceBuildModules.getValue(), ","));
-            excludeTransitiveModulesPackagedAs = Collections.unmodifiableList(
-                    parseDelimited(Property.excludeTransitiveModulesPackagedAs.getValue(), ","));
-            compareToMergeBase = Boolean.valueOf(Property.compareToMergeBase.getValue());
-            fetchReferenceBranch = Boolean.valueOf(Property.fetchReferenceBranch.getValue());
-            fetchBaseBranch = Boolean.valueOf(Property.fetchBaseBranch.getValue());
-            excludePathRegex = Pattern.compile(Property.excludePathRegex.getValue()).asPredicate();
-            failOnMissingGitDir = Boolean.valueOf(Property.failOnMissingGitDir.getValue());
-        } catch (MavenExecutionException | IOException e) {
-            throw new RuntimeException(e);
+    public Configuration(MavenSession session) {
+        checkProperties();
+
+        enabled = Boolean.valueOf(Property.enabled.getValue());
+        key = parseKey(session);
+        disableBranchComparison = Boolean.valueOf(Property.disableBranchComparison.getValue());
+        referenceBranch = Property.referenceBranch.getValue();
+        baseBranch = Property.baseBranch.getValue();
+        uncommited = Boolean.valueOf(Property.uncommited.getValue());
+        untracked = Boolean.valueOf(Property.untracked.getValue());
+        makeUpstream = alsoMakeBehaviours.contains(session.getRequest().getMakeBehavior());
+        skipTestsForNotImpactedModules = Boolean.valueOf(Property.skipTestsForNotImpactedModules.getValue());
+
+        argsForNotImpactedModules = parseDelimited(Property.argsForNotImpactedModules.getValue(), " ")
+                .map(Configuration::keyValueStringToEntry)
+                .collect(collectingAndThen(toLinkedMap(), Collections::unmodifiableMap));
+
+        buildAll = Boolean.valueOf(Property.buildAll.getValue());
+
+        forceBuildModules = parseDelimited(Property.forceBuildModules.getValue(), ",")
+                .map(str -> compilePattern(str, Property.forceBuildModules))
+                .collect(collectingAndThen(toList(), Collections::unmodifiableList));
+
+        excludeTransitiveModulesPackagedAs = parseDelimited(Property.excludeTransitiveModulesPackagedAs.getValue(), ",")
+                .collect(collectingAndThen(toList(), Collections::unmodifiableList));
+
+        compareToMergeBase = Boolean.valueOf(Property.compareToMergeBase.getValue());
+        fetchReferenceBranch = Boolean.valueOf(Property.fetchReferenceBranch.getValue());
+        fetchBaseBranch = Boolean.valueOf(Property.fetchBaseBranch.getValue());
+        excludePathRegex = compilePattern(Property.excludePathRegex).asPredicate();
+        failOnMissingGitDir = Boolean.valueOf(Property.failOnMissingGitDir.getValue());
+    }
+
+    private static void checkProperties() {
+        Set<String> availablePropertyNames = Arrays.stream(Property.values())
+                .map(Property::fullName)
+                .collect(Collectors.toSet());
+        String invalidPropertyNames = System.getProperties().keySet().stream()
+                .map(k -> (String) k)
+                .filter(k -> k.startsWith(Property.PREFIX) && !availablePropertyNames.contains(k))
+                .collect(Collectors.joining("\n\t"));
+        if (!invalidPropertyNames.isEmpty()) {
+            throw new IllegalArgumentException(
+                    String.format("Invalid GIB properties found:\n\t%s\nAllowed properties:\n%s", invalidPropertyNames, Property.exemplifyAll()));
         }
     }
 
-    private static Optional<Path> parseKey(MavenSession session) throws IOException {
-        Path pomDir = session.getCurrentProject().getBasedir().toPath();
+    private static Optional<Path> parseKey(MavenSession session) {
         String keyOptionValue = Property.repositorySshKey.getValue();
         if (keyOptionValue != null && ! keyOptionValue.isEmpty()) {
-            return Optional.of(pomDir.resolve(keyOptionValue).toAbsolutePath().toRealPath().normalize());
+            Path pomDir = session.getCurrentProject().getBasedir().toPath();
+            return Optional.of(pomDir.resolve(keyOptionValue).toAbsolutePath().normalize());
         } else {
             return Optional.empty();
         }
     }
 
-    private static List<String> parseDelimited(String value, String delimiter) {
+    private static Stream<String> parseDelimited(String value, String delimiter) {
         return value.isEmpty()
-                ? Collections.emptyList()
+                ? Stream.empty()
                 : Arrays.stream(value.split(delimiter))
                     .map(String::trim)
-                    .filter(s -> !s.isEmpty())
-                    .collect(Collectors.toList());
-    }
-
-    private static Map<String, String> parseSpaceDelimitedArgs(String value) {
-        return parseDelimited(value, " ").stream()
-                .map(Configuration::keyValueStringToEntry)
-                .collect(Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> a, LinkedHashMap::new));
+                    .filter(s -> !s.isEmpty());
     }
 
     private static Map.Entry<String, String> keyValueStringToEntry(String pair) {
@@ -108,15 +126,19 @@ public class Configuration {
                 : new AbstractMap.SimpleEntry<>(pair, "");
     }
 
-    private static void checkProperties() throws MavenExecutionException {
+    private static Collector<Entry<String, String>, ?, LinkedHashMap<String, String>> toLinkedMap() {
+        return Collectors.toMap(Map.Entry::getKey, Map.Entry::getValue, (a, b) -> a, LinkedHashMap::new);
+    }
+
+    private static Pattern compilePattern(String patternString, Property property) {
         try {
-            System.getProperties().entrySet().stream().map(Map.Entry::getKey)
-                    .filter(o -> o instanceof String).map(o -> (String) o)
-                    .filter(s -> s.startsWith(Property.PREFIX))
-                    .map(s -> s.replaceFirst(Property.PREFIX, ""))
-                    .forEach(Property::valueOf);
-        } catch (IllegalArgumentException e) {
-            throw new MavenExecutionException("Invalid invalid GIB property found. Allowed properties: \n" + Property.exemplifyAll(), e);
+            return Pattern.compile(patternString);
+        } catch (PatternSyntaxException e) {
+            throw new IllegalArgumentException("GIB property " + property.fullName() + " defines an invalid pattern string", e);
         }
     }
+
+    private static Pattern compilePattern(Property property) {
+        return compilePattern(property.getValue(), property);
+    }
 }
diff --git a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
index 1a1edd6..e89b179 100644
--- a/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
+++ b/src/main/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemover.java
@@ -10,6 +10,7 @@ import org.eclipse.jgit.api.errors.GitAPIException;
 import javax.inject.Inject;
 import java.io.IOException;
 import java.util.*;
+import java.util.regex.Pattern;
 import java.util.stream.Collectors;
 import java.util.stream.Stream;
 
@@ -51,7 +52,7 @@ class UnchangedProjectsRemover {
                 mavenSession.getGoals().add("validate");
             } else if (!configuration.forceBuildModules.isEmpty()) {
                 Stream<MavenProject> forceBuildModules = mavenSession.getProjects().stream()
-                        .filter(p -> configuration.forceBuildModules.contains(p.getArtifactId()))
+                        .filter(p -> matchesAny(p.getArtifactId(), configuration.forceBuildModules))
                         .filter(p -> !rebuild.contains(p))
                         .map(this::applyNotImpactedModuleArgs);
                 mavenSession.setProjects(
@@ -126,5 +127,9 @@ class UnchangedProjectsRemover {
             Stream.of(project),
             mavenSession.getProjectDependencyGraph().getUpstreamProjects(project, true).stream());
     }
+
+    private boolean matchesAny(final String str, Collection<Pattern> patterns) {
+        return patterns.stream().anyMatch(pattern -> pattern.matcher(str).matches());
+    }
 }
 


Test patch:
diff --git a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/ConfigurationTest.java b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/ConfigurationTest.java
new file mode 100644
index 0000000..0f841db
--- /dev/null
+++ b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/ConfigurationTest.java
@@ -0,0 +1,128 @@
+package com.vackosar.gitflowincrementalbuild.boundary;
+
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
+
+import java.util.Arrays;
+import java.util.Properties;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+import org.apache.maven.execution.MavenExecutionRequest;
+import org.apache.maven.execution.MavenSession;
+import org.hamcrest.core.IsInstanceOf;
+import org.junit.Before;
+import org.junit.Rule;
+import org.junit.Test;
+import org.junit.rules.ExpectedException;
+import org.junit.rules.RuleChain;
+import org.junit.runner.RunWith;
+import org.junit.runners.model.Statement;
+import org.mockito.Mock;
+import org.mockito.Mockito;
+import org.mockito.runners.MockitoJUnitRunner;
+
+import com.google.common.collect.ImmutableMap;
+import com.vackosar.gitflowincrementalbuild.control.Property;
+
+/**
+ * Tests the system properties parsing logic in {@link Configuration}.
+ *
+ * @author famod
+ *
+ */
+@RunWith(MockitoJUnitRunner.class)
+public class ConfigurationTest {
+
+    private final ExpectedException thrown = ExpectedException.none();
+
+    @Rule
+    public final RuleChain ruleChain = RuleChain
+            // properly reset system properties after each test
+            .outerRule((base, description) -> new Statement() {
+                @Override
+                public void evaluate() throws Throwable {
+                    final Properties backup = (Properties) System.getProperties().clone();
+                    try {
+                        base.evaluate();
+                    } finally {
+                        System.setProperties(backup);
+                    }
+                }
+            })
+            .around(thrown);
+
+    @Mock
+    private MavenExecutionRequest mavenExecutionRequestMock;
+
+    @Mock
+    private MavenSession mavenSessioMock;
+
+    @Before
+    public void setup() {
+        Mockito.when(mavenSessioMock.getRequest()).thenReturn(mavenExecutionRequestMock);
+    }
+
+    @Test
+    public void invalidProperty() {
+        String invalidProperty = Property.PREFIX + "invalid";
+        System.setProperty(invalidProperty, "invalid");
+        thrown.expect(IllegalArgumentException.class);
+        thrown.expectMessage(invalidProperty);
+        thrown.expectMessage(Property.disableBranchComparison.fullName());  // just one of those valid ones
+
+        new Configuration(mavenSessioMock);
+    }
+
+    @Test
+    public void enabled() {
+        System.setProperty(Property.enabled.fullName(), "false");
+
+        Configuration configuration = new Configuration(mavenSessioMock);
+
+        assertFalse(configuration.enabled);
+    }
+
+    @Test
+    public void argsForNotImpactedModules() {
+        System.setProperty(Property.argsForNotImpactedModules.fullName(), "x=true a=false");
+
+        Configuration configuration = new Configuration(mavenSessioMock);
+
+        assertEquals(ImmutableMap.of("x", "true", "a", "false"), configuration.argsForNotImpactedModules);
+    }
+
+    @Test
+    public void excludeTransitiveModulesPackagedAs() {
+        System.setProperty(Property.excludeTransitiveModulesPackagedAs.fullName(), "ear,war");
+
+        Configuration configuration = new Configuration(mavenSessioMock);
+
+        assertEquals(Arrays.asList("ear", "war"), configuration.excludeTransitiveModulesPackagedAs);
+    }
+
+    @Test
+    public void forceBuildModules_pattern() {
+        String expectedPatternString = ".*-some-artifact";
+        System.setProperty(Property.forceBuildModules.fullName(), expectedPatternString);
+
+        Configuration configuration = new Configuration(mavenSessioMock);
+
+        assertNotNull("Field forceBuildModules is null", configuration.forceBuildModules);
+        assertEquals("Unexpected number of Patterns in forceBuildModules", 1, configuration.forceBuildModules.size());
+        Pattern pattern = configuration.forceBuildModules.get(0);
+        assertNotNull("Pattern form forceBuildModules is null", pattern);
+        assertEquals("Unexpected pattern string of Pattern from forceBuildModules", expectedPatternString, pattern.pattern());
+    }
+
+    @Test
+    public void forceBuildModules_patternInvalid() {
+        System.setProperty(Property.forceBuildModules.fullName(), "*-some-artifact");   // pattern is missing the dot
+        thrown.expect(IllegalArgumentException.class);
+        thrown.expectMessage(Property.forceBuildModules.fullName());
+        thrown.expectCause(IsInstanceOf.instanceOf(PatternSyntaxException.class));
+
+        new Configuration(mavenSessioMock);
+    }
+}
diff --git a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java
index de2128d..3b9c93d 100644
--- a/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java
+++ b/src/test/java/com/vackosar/gitflowincrementalbuild/boundary/UnchangedProjectsRemoverTest.java
@@ -44,7 +44,7 @@ import com.vackosar.gitflowincrementalbuild.control.Property;
 @RunWith(MockitoJUnitRunner.class)
 public class UnchangedProjectsRemoverTest {
 
-    private static final String ARTIFACT_ID_1 = "first-module";
+    private static final String ARTIFACT_ID_1 = "first-unchanged-module";
     private static final String ARTIFACT_ID_2 = "changed-module";
     private static final String ARTIFACT_ID_2_DEP_WAR = ARTIFACT_ID_2 + "-dependent-war";
 
@@ -159,6 +159,36 @@ public class UnchangedProjectsRemoverTest {
                 Arrays.asList(mavenProjectMock, unchangedModuleMock, changedModuleMock));
     }
 
+    @Test
+    public void singleChanged_forceBuildModules_oneWildcard() throws GitAPIException, IOException {
+        MavenProject changedModuleMock = addModuleMock(ARTIFACT_ID_2, true);
+        MavenProject unchangedModuleMock = addModuleMock("unchanged-module", false);
+
+        ImmutableMap<Property, String> propertyMap = ImmutableMap.of(
+                Property.forceBuildModules,
+                ".*unchanged-module");
+
+        buildUnderTest(propertyMap).act();
+
+        Mockito.verify(mavenSessionMock).setProjects(
+                Arrays.asList(mavenProjectMock, unchangedModuleMock, changedModuleMock));
+    }
+
+    @Test
+    public void singleChanged_forceBuildModules_twoWildcards() throws GitAPIException, IOException {
+        MavenProject changedModuleMock = addModuleMock(ARTIFACT_ID_2, true);
+        MavenProject unchangedModuleMock = addModuleMock("unchanged-module", false);
+
+        ImmutableMap<Property, String> propertyMap = ImmutableMap.of(
+                Property.forceBuildModules,
+                "first-.*-module,unchanged-.*");
+
+        buildUnderTest(propertyMap).act();
+
+        Mockito.verify(mavenSessionMock).setProjects(
+                Arrays.asList(mavenProjectMock, unchangedModuleMock, changedModuleMock));
+    }
+
     @Test
     public void singleChanged_excludeTransitiveModulesPackagedAs_oneTransitive_oneExcluded() throws GitAPIException, IOException {
         MavenProject changedProjectMock = addModuleMock(ARTIFACT_ID_2, true);
@@ -311,7 +341,7 @@ public class UnchangedProjectsRemoverTest {
         return buildUnderTest(ImmutableMap.of());
     }
 
-    private UnchangedProjectsRemover buildUnderTest(ImmutableMap<Property, String> propertyMap) throws IOException {
+    private UnchangedProjectsRemover buildUnderTest(ImmutableMap<Property, String> propertyMap) {
         Configuration configuration;
         Properties origProps = (Properties) System.getProperties().clone();
         try {