Add nonFilteredFiles glob exclusions and opt-in gracefulBinaryHandling - #395

Review

Two issues, one correctness gap and one defensive-copy miss.

1. is silently dropped on two legacy API paths

calls the 5-arg overload which hardwires :

// DefaultMavenFileFilter.java ~line 73
public void copyFile(MavenFileFilterRequest mavenFileFilterRequest) throws MavenFilteringException {
    List<FilterWrapper> filterWrappers = getDefaultFilterWrappers(mavenFileFilterRequest);
    copyFile(                                   // ← 5-arg: gracefulBinaryHandling = false
            mavenFileFilterRequest.getFrom(),
            mavenFileFilterRequest.getTo(),
            mavenFileFilterRequest.isFiltering(),
            filterWrappers,
            mavenFileFilterRequest.getEncoding());
}

Same problem in the 8-arg overload — it also terminates at the 5-arg variant. Any caller that goes through either of those entry points cannot opt into graceful binary handling, regardless of what they pass. The feature silently does nothing for them.

needs a field (getter + setter), and both overloads must route through the 6-arg variant. Without this, the API surface the PR introduces is incomplete — the 6-arg is useful only if called directly; any higher-level entry point ignores it.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review after 75627b9

Both prior findings are addressed.

1. gracefulBinaryHandling silently dropped on legacy paths ✅ Fixed
MavenFileFilterRequest now has gracefulBinaryHandling (getter + setter). copyFile(MavenFileFilterRequest) routes through the 6-arg variant, passing mavenFileFilterRequest.isGracefulBinaryHandling() instead of hardwiring false.

2. Mutable collection in Resource.getNonFilteredFiles() ✅ Fixed
Now returns Collections.unmodifiableList(nonFilteredFiles) when non-null.

Pre-fix test validation: All 6 new tests fail to compile without the production changes (missing matchesNonFilteredGlob, setNonFilteredFiles, setGracefulBinaryHandling symbols) — confirming they are tightly bound to the fix.

SelectorUtils.matchPath(..., false) — passing isCaseSensitive=false is correct: case-insensitive glob matching works across case-sensitive (Linux) and case-insensitive (Windows) file systems without surprises.

Solid. Ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.

Re-review after 1219140

All four Copilot findings addressed. Still solid.

1. MavenFileFilter.copyFile backward-compat ✅ Fixed
The 6-arg overload is now a default method delegating to the 5-arg overload. Existing MavenFileFilter implementations outside this module compile as-is and get gracefulBinaryHandling=false by default. DefaultMavenFileFilter retains its full override.

2. propertiesFiles tracking correctness ✅ Fixed
The guard now reads resource.isFiltering() && filteredExt && filteredGlob && isPropertiesFile(source), so a .properties file excluded by a nonFilteredFiles glob no longer appears in the end-of-run encoding notice.

3. Case-sensitivity choice documented and intentional ✅ Confirmed
SelectorUtils.matchPath(..., false) (case-insensitive) is consistent with the existing filteredFileExtension path (getExtension uses .toLowerCase(Locale.ROOT)). Making glob matching case-sensitive while extension matching is case-insensitive would create a surprising asymmetry. The decision is sound.

4. Getter Javadoc accuracy ✅ Fixed
Getter now says "copied as-is with a WARN log entry" — no longer "silently".

One minor gap remaining: copyOf() propagation test

MavenResourcesExecutionTest covers flatten, propertiesEncoding, delimiters, and interpolatorCustomizer propagation through copyOf(), but not gracefulBinaryHandling. A regression there would go unnoticed. Low risk since the field is one line in copyOf(), but easy to add:

@Test
void copyOfShouldPreserveGracefulBinaryHandling() {
    MavenResourcesExecution original = new MavenResourcesExecution();
    original.setGracefulBinaryHandling(true);
    MavenResourcesExecution copy = original.copyOf();
    assertTrue(copy.isGracefulBinaryHandling());
}

Not a blocker — the implementation is correct, CI is green across all platforms (JDK 17/21/25, Linux/macOS/Windows). The feature is well-designed: the glob mechanism is surgical and per-resource; the graceful fallback is opt-in and always warns. Ready to merge.

This review was generated by an AI agent, Hermès on behalf of @gnodet.