From 1d85c5d58aab205a2e808c817d304c2cf98c7bca Mon Sep 17 00:00:00 2001 From: cowwoc Date: Sun, 5 Oct 2025 18:50:11 -0400 Subject: [PATCH 1/2] Add ignorePattern attribute to TrackedProperty for array filtering Fixes #375 - Maven 4 auto-injects --module-version to compilerArgs during cache storage but not during validation, causing parameter mismatches. Changes: - Add ignorePattern field to TrackedProperty in MDO model - Implement regex-based filtering in BuildCacheMojosExecutionStrategy - Filter arrays before comparison (both runtime and cached values) The ignorePattern attribute allows filtering specific array elements before comparison, solving the Maven 4 module-version problem while still detecting legitimate compilerArgs changes. Users can configure this in maven-build-cache-config.xml: Tested with multi-module JPMS project using Maven 4.0.0-rc-4. --- .../BuildCacheMojosExecutionStrategy.java | 59 ++++++++++++++++++- src/main/mdo/build-cache-config.mdo | 5 ++ 2 files changed, 62 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java b/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java index 0a2d4d73..b3d00be7 100644 --- a/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java +++ b/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java @@ -379,7 +379,7 @@ boolean isParamsMatched( Path baseDirPath = project.getBasedir().toPath(); currentValue = normalizedPath(((Path) value), baseDirPath); } else if (value != null && value.getClass().isArray()) { - currentValue = ArrayUtils.toString(value); + currentValue = filterAndStringifyArray(value, trackedProperty.getIgnorePattern()); } else { currentValue = String.valueOf(value); } @@ -388,7 +388,13 @@ boolean isParamsMatched( return false; } - if (!Strings.CS.equals(currentValue, expectedValue)) { + // Apply ignorePattern filtering to expected value if it's an array string representation + String filteredExpectedValue = expectedValue; + if (trackedProperty.getIgnorePattern() != null && expectedValue.startsWith("[") && expectedValue.endsWith("]")) { + filteredExpectedValue = filterArrayString(expectedValue, trackedProperty.getIgnorePattern()); + } + + if (!Strings.CS.equals(currentValue, filteredExpectedValue)) { if (!Strings.CS.equals(currentValue, trackedProperty.getSkipValue())) { LOGGER.info( "Plugin parameter mismatch found. Parameter: {}, expected: {}, actual: {}", @@ -434,6 +440,55 @@ private static String normalizedPath(Path path, Path baseDirPath) { return normalizedPath; } + /** + * Filters array values based on ignore pattern and converts to string representation. + */ + private static String filterAndStringifyArray(Object array, String ignorePattern) { + if (ignorePattern == null) { + return ArrayUtils.toString(array); + } + + java.util.regex.Pattern pattern = java.util.regex.Pattern.compile(ignorePattern); + java.util.List filtered = new java.util.ArrayList<>(); + + int length = java.lang.reflect.Array.getLength(array); + for (int i = 0; i < length; i++) { + Object element = java.lang.reflect.Array.get(array, i); + String elementStr = String.valueOf(element); + if (!pattern.matcher(elementStr).find()) { + filtered.add(element); + } + } + + return filtered.toString(); + } + + /** + * Filters an array string representation (e.g., "[a, b, c]") based on ignore pattern. + */ + private static String filterArrayString(String arrayStr, String ignorePattern) { + if (ignorePattern == null || !arrayStr.startsWith("[") || !arrayStr.endsWith("]")) { + return arrayStr; + } + + java.util.regex.Pattern pattern = java.util.regex.Pattern.compile(ignorePattern); + String content = arrayStr.substring(1, arrayStr.length() - 1); + if (content.trim().isEmpty()) { + return "[]"; + } + + String[] elements = content.split(",\\s*"); + java.util.List filtered = new java.util.ArrayList<>(); + + for (String element : elements) { + if (!pattern.matcher(element.trim()).find()) { + filtered.add(element.trim()); + } + } + + return filtered.toString(); + } + private enum CacheRestorationStatus { SUCCESS, FAILURE, diff --git a/src/main/mdo/build-cache-config.mdo b/src/main/mdo/build-cache-config.mdo index 52ae0da0..25034119 100644 --- a/src/main/mdo/build-cache-config.mdo +++ b/src/main/mdo/build-cache-config.mdo @@ -1459,6 +1459,11 @@ under the License. defaultValue String + + ignorePattern + String + Regular expression pattern to filter out matching values from array/list properties before comparison. Useful for filtering auto-injected values like Maven 4's --module-version + From bb010f6e7ad47186c1232c58cb14a9b1e2d73396 Mon Sep 17 00:00:00 2001 From: cowwoc Date: Mon, 6 Oct 2025 17:45:39 -0400 Subject: [PATCH 2/2] Address reviewer feedback on PR #391 Implemented all suggestions from @AlexanderAshitkin: 1. **Extracted filtering logic to utility class** - Created ArrayFilterUtils with pattern caching - Removed inline filtering methods from BuildCacheMojosExecutionStrategy 2. **Changed pattern matching from find() to matches()** - Ensures exact pattern matching (e.g., "\d+" matches "123" but not "Maven4") - Addresses reviewer's concern about unexpected substring matches 3. **Added pattern caching** - Uses ConcurrentHashMap to cache compiled patterns - Improves performance for repeated pattern usage 4. **Renamed variable to excludePattern** - More descriptive than generic "pattern" 5. **Removed filterArrayString() method** - Simplified approach: only filter current values, not cached values - When ignore pattern changes, cache rebuilds naturally 6. **Added proper imports** - Eliminated fully qualified names (FQNs) - All imports are at the top of the file 7. **Comprehensive unit tests** - 13 test cases covering all scenarios - Tests verify matches() vs find() behavior - Tests validate pattern caching - All 87 tests pass (existing + new) --- .../maven/buildcache/ArrayFilterUtils.java | 83 ++++++++++ .../BuildCacheMojosExecutionStrategy.java | 60 +------ .../buildcache/ArrayFilterUtilsTest.java | 151 ++++++++++++++++++ 3 files changed, 236 insertions(+), 58 deletions(-) create mode 100644 src/main/java/org/apache/maven/buildcache/ArrayFilterUtils.java create mode 100644 src/test/java/org/apache/maven/buildcache/ArrayFilterUtilsTest.java diff --git a/src/main/java/org/apache/maven/buildcache/ArrayFilterUtils.java b/src/main/java/org/apache/maven/buildcache/ArrayFilterUtils.java new file mode 100644 index 00000000..0a9e08e4 --- /dev/null +++ b/src/main/java/org/apache/maven/buildcache/ArrayFilterUtils.java @@ -0,0 +1,83 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.buildcache; + +import java.lang.reflect.Array; +import java.util.ArrayList; +import java.util.List; +import java.util.concurrent.ConcurrentHashMap; +import java.util.regex.Pattern; + +import org.apache.commons.lang3.ArrayUtils; + +/** + * Utility class for filtering array elements based on regex patterns. + */ +public final class ArrayFilterUtils { + + private static final ConcurrentHashMap PATTERN_CACHE = new ConcurrentHashMap<>(); + + private ArrayFilterUtils() { + // Utility class + } + + /** + * Filters array values based on exclude pattern and converts to string representation. + * + * @param array the array to filter + * @param excludePattern the regex pattern to match elements to exclude (null to disable filtering) + * @return string representation of the filtered array + */ + public static String filterAndStringifyArray(Object array, String excludePattern) { + if (excludePattern == null) { + return ArrayUtils.toString(array); + } + + Pattern pattern = getOrCompilePattern(excludePattern); + List filtered = new ArrayList<>(); + + int length = Array.getLength(array); + for (int i = 0; i < length; i++) { + Object element = Array.get(array, i); + String elementStr = String.valueOf(element); + if (!pattern.matcher(elementStr).matches()) { + filtered.add(element); + } + } + + return filtered.toString(); + } + + /** + * Gets a compiled pattern from cache or compiles and caches it. + * + * @param regex the regex pattern string + * @return compiled Pattern object + */ + private static Pattern getOrCompilePattern(String regex) { + return PATTERN_CACHE.computeIfAbsent(regex, Pattern::compile); + } + + /** + * Clears the pattern cache. Primarily for testing purposes. + */ + static void clearPatternCache() { + PATTERN_CACHE.clear(); + } +} diff --git a/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java b/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java index b3d00be7..8314dbf8 100644 --- a/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java +++ b/src/main/java/org/apache/maven/buildcache/BuildCacheMojosExecutionStrategy.java @@ -29,7 +29,6 @@ import java.util.Map; import java.util.Set; -import org.apache.commons.lang3.ArrayUtils; import org.apache.commons.lang3.Strings; import org.apache.maven.SessionScoped; import org.apache.maven.buildcache.artifact.ArtifactRestorationReport; @@ -379,7 +378,7 @@ boolean isParamsMatched( Path baseDirPath = project.getBasedir().toPath(); currentValue = normalizedPath(((Path) value), baseDirPath); } else if (value != null && value.getClass().isArray()) { - currentValue = filterAndStringifyArray(value, trackedProperty.getIgnorePattern()); + currentValue = ArrayFilterUtils.filterAndStringifyArray(value, trackedProperty.getIgnorePattern()); } else { currentValue = String.valueOf(value); } @@ -388,13 +387,7 @@ boolean isParamsMatched( return false; } - // Apply ignorePattern filtering to expected value if it's an array string representation - String filteredExpectedValue = expectedValue; - if (trackedProperty.getIgnorePattern() != null && expectedValue.startsWith("[") && expectedValue.endsWith("]")) { - filteredExpectedValue = filterArrayString(expectedValue, trackedProperty.getIgnorePattern()); - } - - if (!Strings.CS.equals(currentValue, filteredExpectedValue)) { + if (!Strings.CS.equals(currentValue, expectedValue)) { if (!Strings.CS.equals(currentValue, trackedProperty.getSkipValue())) { LOGGER.info( "Plugin parameter mismatch found. Parameter: {}, expected: {}, actual: {}", @@ -440,55 +433,6 @@ private static String normalizedPath(Path path, Path baseDirPath) { return normalizedPath; } - /** - * Filters array values based on ignore pattern and converts to string representation. - */ - private static String filterAndStringifyArray(Object array, String ignorePattern) { - if (ignorePattern == null) { - return ArrayUtils.toString(array); - } - - java.util.regex.Pattern pattern = java.util.regex.Pattern.compile(ignorePattern); - java.util.List filtered = new java.util.ArrayList<>(); - - int length = java.lang.reflect.Array.getLength(array); - for (int i = 0; i < length; i++) { - Object element = java.lang.reflect.Array.get(array, i); - String elementStr = String.valueOf(element); - if (!pattern.matcher(elementStr).find()) { - filtered.add(element); - } - } - - return filtered.toString(); - } - - /** - * Filters an array string representation (e.g., "[a, b, c]") based on ignore pattern. - */ - private static String filterArrayString(String arrayStr, String ignorePattern) { - if (ignorePattern == null || !arrayStr.startsWith("[") || !arrayStr.endsWith("]")) { - return arrayStr; - } - - java.util.regex.Pattern pattern = java.util.regex.Pattern.compile(ignorePattern); - String content = arrayStr.substring(1, arrayStr.length() - 1); - if (content.trim().isEmpty()) { - return "[]"; - } - - String[] elements = content.split(",\\s*"); - java.util.List filtered = new java.util.ArrayList<>(); - - for (String element : elements) { - if (!pattern.matcher(element.trim()).find()) { - filtered.add(element.trim()); - } - } - - return filtered.toString(); - } - private enum CacheRestorationStatus { SUCCESS, FAILURE, diff --git a/src/test/java/org/apache/maven/buildcache/ArrayFilterUtilsTest.java b/src/test/java/org/apache/maven/buildcache/ArrayFilterUtilsTest.java new file mode 100644 index 00000000..8e835636 --- /dev/null +++ b/src/test/java/org/apache/maven/buildcache/ArrayFilterUtilsTest.java @@ -0,0 +1,151 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ +package org.apache.maven.buildcache; + +import org.junit.jupiter.api.AfterEach; +import org.junit.jupiter.api.Test; + +import static org.junit.jupiter.api.Assertions.assertEquals; + +class ArrayFilterUtilsTest { + + @AfterEach + void clearCache() { + ArrayFilterUtils.clearPatternCache(); + } + + @Test + void testFilterAndStringifyArrayWithNullPattern() { + String[] array = new String[] {"--module-version", "1.0.0", "-g"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, null); + assertEquals("{--module-version,1.0.0,-g}", result); + } + + @Test + void testFilterAndStringifyArrayWithMatchingPattern() { + String[] array = new String[] {"--module-version", "1.0.0", "-g", "--module-version"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "--module-version"); + assertEquals("[1.0.0, -g]", result); + } + + @Test + void testFilterAndStringifyArrayWithPartialMatchPattern() { + // Test that matches() is used instead of find() + // Pattern "\\d+" should only match pure numbers like "123", not "Maven4" + String[] array = new String[] {"123", "Maven4", "456", "abc"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "\\d+"); + // Should keep "Maven4" and "abc" because they don't fully match \d+ + assertEquals("[Maven4, abc]", result); + } + + @Test + void testFilterAndStringifyArrayWithRegexPattern() { + String[] array = new String[] {"--module-version", "1.0.0", "--release", "21", "-g"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "--module-.*"); + assertEquals("[1.0.0, --release, 21, -g]", result); + } + + @Test + void testFilterAndStringifyArrayRemovesAllElements() { + String[] array = new String[] {"test", "test", "test"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "test"); + assertEquals("[]", result); + } + + @Test + void testFilterAndStringifyArrayWithNoMatches() { + String[] array = new String[] {"-g", "-verbose", "-parameters"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "--module-version"); + assertEquals("[-g, -verbose, -parameters]", result); + } + + @Test + void testFilterAndStringifyArrayWithEmptyArray() { + String[] array = new String[] {}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "--module-version"); + assertEquals("[]", result); + } + + @Test + void testFilterAndStringifyArrayWithIntArray() { + int[] array = new int[] {1, 2, 3, 4, 5}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "3"); + assertEquals("[1, 2, 4, 5]", result); + } + + @Test + void testFilterAndStringifyArrayWithComplexPattern() { + String[] array = new String[] {"--module-version", "1.0.0", "--add-exports", "module/package"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "--module-version|--add-exports"); + assertEquals("[1.0.0, module/package]", result); + } + + @Test + void testPatternCaching() { + // First call should cache the pattern + String[] array1 = new String[] {"--module-version", "1.0.0"}; + String result1 = ArrayFilterUtils.filterAndStringifyArray(array1, "--module-version"); + + // Second call with same pattern should use cached version + String[] array2 = new String[] {"--module-version", "2.0.0", "-g"}; + String result2 = ArrayFilterUtils.filterAndStringifyArray(array2, "--module-version"); + + assertEquals("[1.0.0]", result1); + assertEquals("[2.0.0, -g]", result2); + } + + @Test + void testMatchesVsFindBehavior() { + // Test that pattern must match the entire string, not just find a substring + String[] array = new String[] {"--module-version-extended", "--module-version", "abc"}; + + // Pattern should only match exact "--module-version", not "--module-version-extended" + String result = ArrayFilterUtils.filterAndStringifyArray(array, "--module-version"); + + // Using matches(), only exact match is excluded + assertEquals("[--module-version-extended, abc]", result); + } + + @Test + void testNumericPatternExactMatch() { + // Regression test for reviewer's concern about \\d+ matching "Maven4" + String[] array = new String[] {"123", "Maven4", "7", "test123"}; + String result = ArrayFilterUtils.filterAndStringifyArray(array, "\\d+"); + + // \\d+ with matches() should only exclude pure numbers "123" and "7" + // Should keep "Maven4" and "test123" since they don't fully match + assertEquals("[Maven4, test123]", result); + } + + @Test + void testMaven4ModuleVersionUseCase() { + // Real-world test case from the PR description + String[] compilerArgs = new String[] { + "-parameters", + "--module-version", + "1.0.0-SNAPSHOT", + "-g" + }; + + String result = ArrayFilterUtils.filterAndStringifyArray(compilerArgs, "--module-version|.*SNAPSHOT.*"); + + // Should filter out both "--module-version" and "1.0.0-SNAPSHOT" + assertEquals("[-parameters, -g]", result); + } +}