gnodet commented on code in PR #290:
URL: https://github.com/apache/maven-filtering/pull/290#discussion_r4093378541


##########
src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java:
##########
@@ -476,26 +478,38 @@ private String 
getRelativeOutputDirectory(MavenResourcesExecution execution) {
      */
     private String filterFileName(String name, List<FilterWrapper> wrappers) 
throws MavenFilteringException {
 
-        Reader reader = new StringReader(name);
-        for (FilterWrapper wrapper : wrappers) {
-            reader = wrapper.getReader(reader);
-        }
-
-        try (StringWriter writer = new StringWriter()) {
-            char[] buffer = new char[BUFFER_LENGTH];
-            int nRead;
-            while ((nRead = reader.read(buffer, 0, buffer.length)) >= 0) {
-                writer.write(buffer, 0, nRead);
+        char[] buffer = new char[BUFFER_LENGTH];

Review Comment:
   Fixed in b4a240f: moved `buffer` inside the `try` block.



##########
src/main/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFiltering.java:
##########
@@ -476,26 +478,38 @@ private String 
getRelativeOutputDirectory(MavenResourcesExecution execution) {
      */
     private String filterFileName(String name, List<FilterWrapper> wrappers) 
throws MavenFilteringException {
 
-        Reader reader = new StringReader(name);
-        for (FilterWrapper wrapper : wrappers) {
-            reader = wrapper.getReader(reader);
-        }
-
-        try (StringWriter writer = new StringWriter()) {
-            char[] buffer = new char[BUFFER_LENGTH];
-            int nRead;
-            while ((nRead = reader.read(buffer, 0, buffer.length)) >= 0) {
-                writer.write(buffer, 0, nRead);
+        char[] buffer = new char[BUFFER_LENGTH];
+        StringBuilder sb = new StringBuilder();
+        Path path = Path.of(name);
+        Iterator<Path> iterator = path.iterator();
+        while (iterator.hasNext()) {
+            String component = iterator.next().toString();
+            Reader reader = new StringReader(component);
+            for (FilterWrapper wrapper : wrappers) {
+                reader = wrapper.getReader(reader);
             }
 
-            String filteredFilename = writer.toString();
+            try (StringWriter writer = new StringWriter()) {
+                int nRead;
+                while ((nRead = reader.read(buffer, 0, buffer.length)) >= 0) {
+                    writer.write(buffer, 0, nRead);
+                }
+                String filteredComponent = writer.toString();
+                sb.append(filteredComponent);
+                if (iterator.hasNext()) {
+                    sb.append(FileSystems.getDefault().getSeparator());

Review Comment:
   Fixed in b4a240f: replaced `FileSystems.getDefault().getSeparator()` with 
`File.separator` (and updated import accordingly).



##########
src/test/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFilteringTest.java:
##########
@@ -943,6 +944,49 @@ public void testFilterFileName() throws Exception {
 
         List<Path> files = list(targetPathFile);
         assertEquals(1, files.size());
+        assertEquals("subfolder", filename(files.get(0)));
+        assertTrue(Files.isDirectory(files.get(0)));
+
+        files = list(files.get(0));
+        assertEquals(1, files.size());
+        assertEquals("1.0.txt", filename(files.get(0)));
+    }
+
+    @Test
+    public void testFilterFileNameWithFileSeparatorAsEscape() throws Exception 
{
+
+        String unitFilesDir = getBasedir() + 
"/src/test/units-files/maven-filename-filtering";
+
+        Resource resource = new Resource();
+        resource.setDirectory(unitFilesDir);
+        resource.setFiltering(true);
+        resource.addInclude("**/${pom.version}*");
+        resource.setTargetPath("testTargetPath");
+
+        MavenResourcesExecution mavenResourcesExecution = new 
MavenResourcesExecution(
+                Collections.singletonList(resource),
+                outputDirectory,
+                mavenProject,
+                "UTF-8",
+                Collections.<String>emptyList(),
+                Collections.<String>emptyList(),
+                new StubSession());
+        mavenResourcesExecution.setFilterFilenames(true);
+
+        // more likely to occur on windows, where the file
+        // separator is the same as the common escape string "\"
+        
mavenResourcesExecution.setEscapeString(FileSystems.getDefault().getSeparator());
+        mavenResourcesFiltering.filterResources(mavenResourcesExecution);
+
+        Path targetPathFile = outputDirectory.resolve("testTargetPath");
+
+        List<Path> files = list(targetPathFile);
+        assertEquals(1, files.size());
+        assertEquals("subfolder", filename(files.get(0)));
+        assertTrue(Files.isDirectory(files.get(0)));
+
+        files = list(files.get(0));

Review Comment:
   Fixed in b4a240f: declared `subfolderFiles` as a new variable in both 
`testFilterFileName` and `testFilterFileNameWithFileSeparatorAsEscape` (same 
pattern flagged by @elharo).



##########
src/test/java/org/apache/maven/shared/filtering/DefaultMavenResourcesFilteringTest.java:
##########
@@ -943,6 +944,49 @@ public void testFilterFileName() throws Exception {
 
         List<Path> files = list(targetPathFile);
         assertEquals(1, files.size());
+        assertEquals("subfolder", filename(files.get(0)));
+        assertTrue(Files.isDirectory(files.get(0)));
+
+        files = list(files.get(0));

Review Comment:
   Fixed in b4a240f: declared a separate `subfolderFiles` variable instead of 
reusing `files`.



-- 
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]

Reply via email to