gnodet-bot commented on code in PR #400:
URL: https://github.com/apache/maven-filtering/pull/400#discussion_r4103373535
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -173,6 +177,71 @@ public List<FilterWrapper> getDefaultFilterWrappers(final
AbstractMavenFiltering
return defaultFilterWrappers;
}
+ /**
+ * Returns {@code true} if the given filter path contains glob pattern
characters
+ * ({@code *}, {@code ?}, {@code {}, or {@code [}).
Review Comment:
💡 **Javadoc syntax:** `{@code {}` is ambiguous — the first `}` closes the
`{@code` tag, so the rendered output loses the `{` character and leaves a stray
`}` in the text.
```suggestion
* ({@code *}, {@code ?}, <code>{</code>, or {@code [}).
```
##########
src/main/java/org/apache/maven/shared/filtering/BaseFilter.java:
##########
@@ -173,6 +177,71 @@ public List<FilterWrapper> getDefaultFilterWrappers(final
AbstractMavenFiltering
return defaultFilterWrappers;
}
+ /**
+ * Returns {@code true} if the given filter path contains glob pattern
characters
+ * ({@code *}, {@code ?}, {@code {}, or {@code [}).
+ */
+ private static boolean isGlobPattern(String path) {
+ return path.indexOf('*') >= 0 || path.indexOf('?') >= 0 ||
path.indexOf('{') >= 0 || path.indexOf('[') >= 0;
+ }
+
+ /**
+ * Expands a glob pattern relative to {@code basedir} and returns the
matched paths in sorted order.
+ * The pattern must use forward slashes as path separators (as is
conventional in Maven filter paths).
+ */
+ private List<Path> expandGlob(Path basedir, String globPattern) throws
IOException {
+ // Normalize to forward slashes; resolveFile accepts them on all
platforms
+ String normalized = globPattern.replace('\\', '/');
+
+ // Find the deepest non-glob path prefix to use as the walk root
+ String[] segments = normalized.split("/");
+ StringBuilder prefix = new StringBuilder();
+ for (String segment : segments) {
+ if (segment.indexOf('*') >= 0
+ || segment.indexOf('?') >= 0
+ || segment.indexOf('{') >= 0
+ || segment.indexOf('[') >= 0) {
+ break;
+ }
+ if (prefix.length() > 0) {
+ prefix.append('/');
+ }
+ prefix.append(segment);
+ }
+
+ Path normalizedBase = basedir.toAbsolutePath().normalize();
+ Path searchRoot = prefix.length() > 0 ?
FilteringUtils.resolveFile(basedir, prefix.toString()) : normalizedBase;
+
+ if (!searchRoot.startsWith(normalizedBase)) {
+ throw new IOException("Filter glob pattern '" + globPattern + "'
resolves outside project basedir");
+ }
+
+ if (!Files.isDirectory(searchRoot)) {
+ return List.of();
+ }
+
+ // Build a relative glob pattern for the suffix beyond the prefix.
+ // We match against the path relative to searchRoot to avoid OS
separator issues:
+ // on Windows, Path.toString() uses '\' which glob treats as an escape
character.
+ // Relative matching with forward-slash patterns works on all
platforms because
+ // Path.relativize() always returns paths comparable with '/' in glob
patterns.
+ String patternSuffix = prefix.length() > 0 ?
normalized.substring(prefix.length() + 1) : normalized;
+ PathMatcher matcher = FileSystems.getDefault().getPathMatcher("glob:"
+ patternSuffix);
+
+ List<Path> matched = new ArrayList<>();
+ try (Stream<Path> stream = Files.walk(searchRoot)) {
Review Comment:
💡 **Performance:** `Files.walk()` without `maxDepth` traverses the entire
subtree under `searchRoot`. For a single-`*` pattern like
`env/dev/*.properties`, the `PathMatcher` correctly rejects files in
subdirectories, but the walker still visits them all. On large directory trees
this is wasteful I/O.
Consider computing `maxDepth` from the pattern — if the suffix contains no
path separator or `**`, pass `maxDepth=1`:
```suggestion
try (Stream<Path> stream = Files.walk(searchRoot,
patternSuffix.contains("/") || patternSuffix.contains("**") ? Integer.MAX_VALUE
: 1)) {
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
