You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@jackrabbit.apache.org by kw...@apache.org on 2021/06/09 18:52:37 UTC

[jackrabbit-filevault-package-maven-plugin] branch bugfix/JCRVLT-528-validate-files-order created (now f608200)

This is an automated email from the ASF dual-hosted git repository.

kwin pushed a change to branch bugfix/JCRVLT-528-validate-files-order
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault-package-maven-plugin.git.


      at f608200  JCRVLT-528 order files correctly for validation

This branch includes the following new commits:

     new f608200  JCRVLT-528 order files correctly for validation

The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails.  The revisions
listed as "add" were already present in the repository and have only
been added to this reference.


[jackrabbit-filevault-package-maven-plugin] 01/01: JCRVLT-528 order files correctly for validation

Posted by kw...@apache.org.
This is an automated email from the ASF dual-hosted git repository.

kwin pushed a commit to branch bugfix/JCRVLT-528-validate-files-order
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault-package-maven-plugin.git

commit f608200b257699c80bfd5e6829e41d9f6936957b
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed Jun 9 20:52:21 2021 +0200

    JCRVLT-528 order files correctly for validation
    
    (parent folder and .content.xml must come first)
    Switch to NIO Files in some more places
---
 .../maven/packaging/AbstractValidateMojo.java      |  10 +-
 .../maven/packaging/ValidateFilesMojo.java         | 110 +++++++++++++++------
 .../maven/packaging/ValidatePackageMojo.java       |  14 +--
 .../maven/packaging/ValidateFilesMojoTest.java     |  52 ++++++++++
 .../maven/packaging/it/ValidateFilesIT.java        |   2 +-
 5 files changed, 148 insertions(+), 40 deletions(-)

diff --git a/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/AbstractValidateMojo.java b/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/AbstractValidateMojo.java
index a6cb1bd..0131061 100644
--- a/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/AbstractValidateMojo.java
+++ b/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/AbstractValidateMojo.java
@@ -19,6 +19,7 @@ package org.apache.jackrabbit.filevault.maven.packaging;
 import java.io.File;
 import java.io.IOException;
 import java.nio.charset.StandardCharsets;
+import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Comparator;
 import java.util.HashMap;
@@ -182,8 +183,8 @@ public abstract class AbstractValidateMojo extends AbstractMojo {
      */
     public static final Artifact IGNORE_ARTIFACT = new DefaultArtifact("ignore", "ignore", "1.0", "", "", "", null);
 
-    protected String getProjectRelativeFilePath(File file) {
-        return "'" + project.getBasedir().toPath().relativize(file.toPath()).toString() + "'";
+    protected String getProjectRelativeFilePath(Path file) {
+        return "'" + project.getBasedir().toPath().relativize(file).toString() + "'";
     }
 
     public AbstractValidateMojo() {
@@ -377,8 +378,9 @@ public abstract class AbstractValidateMojo extends AbstractMojo {
         return validatorSettingsById;
     }
 
+    
     /** 
-     * Comparator on file names which makes sure that the {@code .content.xml} files come first. 
+     * Comparator on file names (excluding paths) which makes sure that the files named {@code .content.xml} files come first. 
      */
     static final class DotContentXmlFirstComparator implements Comparator<String> {
         @Override
@@ -388,7 +390,7 @@ public abstract class AbstractValidateMojo extends AbstractMojo {
             } else if (Constants.DOT_CONTENT_XML.equals(s2)) {
                 return 1;
             }
-            return 0;
+            return s1.compareTo(s2);
         }
     }
 }
diff --git a/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojo.java b/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojo.java
index 3b12903..b2adef9 100644
--- a/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojo.java
+++ b/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojo.java
@@ -17,15 +17,18 @@
 package org.apache.jackrabbit.filevault.maven.packaging;
 
 import java.io.File;
-import java.io.FileInputStream;
 import java.io.FileNotFoundException;
 import java.io.IOException;
 import java.io.InputStream;
+import java.nio.file.Files;
+import java.nio.file.Path;
 import java.nio.file.Paths;
-import java.util.Arrays;
 import java.util.Collection;
 import java.util.Collections;
+import java.util.Comparator;
 import java.util.List;
+import java.util.SortedSet;
+import java.util.TreeSet;
 
 import org.apache.commons.lang3.StringUtils;
 import org.apache.jackrabbit.filevault.maven.packaging.validator.impl.context.DirectoryValidationContext;
@@ -177,7 +180,7 @@ public class ValidateFilesMojo extends AbstractValidateMojo {
                 metaInfRootDirectory = metaInfoVaultSourceDirectory.getParentFile();
             }
             File generatedMetaInfRootDirectory = new File(workDirectory, Constants.META_INF);
-            getLog().info("Validate files in generatedMetaInfRootDirectory " + getProjectRelativeFilePath(generatedMetaInfRootDirectory) + " and metaInfRootDir " + getProjectRelativeFilePath(generatedMetaInfRootDirectory));
+            getLog().info("Validate files in generatedMetaInfRootDirectory " + getProjectRelativeFilePath(generatedMetaInfRootDirectory.toPath()) + " and metaInfRootDir " + getProjectRelativeFilePath(generatedMetaInfRootDirectory.toPath()));
             ValidationContext context = new DirectoryValidationContext(generatedMetaInfRootDirectory, metaInfRootDirectory, resolver, getLog());
             ValidationExecutor executor = validationExecutorFactory.createValidationExecutor(context, false, false, getValidatorSettingsForPackage(context.getProperties().getId(), false));
             if (executor == null) {
@@ -185,12 +188,12 @@ public class ValidateFilesMojo extends AbstractValidateMojo {
             }
             validationHelper.printUsedValidators(getLog(), executor, context, true);
             if (metaInfRootDirectory != null) {
-                validateDirectory(validationHelper, executor, metaInfRootDirectory, true);
+                validateDirectoryRecursively(validationHelper, executor, metaInfRootDirectory.toPath(), true);
             }
-            validateDirectory(validationHelper, executor, generatedMetaInfRootDirectory, true);
+            validateDirectoryRecursively(validationHelper, executor, generatedMetaInfRootDirectory.toPath(), true);
             File jcrSourceDirectory = AbstractSourceAndMetadataPackageMojo.getJcrSourceDirectory(jcrRootSourceDirectory, builtContentDirectory, getLog());
             if (jcrSourceDirectory != null) {
-                validateDirectory(validationHelper, executor, jcrSourceDirectory, false);
+                validateDirectoryRecursively(validationHelper, executor, jcrSourceDirectory.toPath(), false);
             }
             validationHelper.printMessages(executor.done(), getLog(), buildContext, project.getBasedir().toPath());
         } catch (IOException | ConfigurationException e) {
@@ -199,29 +202,51 @@ public class ValidateFilesMojo extends AbstractValidateMojo {
         validationHelper.failBuildInCaseOfViolations(failOnValidationWarnings);
     }
 
-    private void validateDirectory(ValidationHelper validationHelper, ValidationExecutor executor, File baseDir, boolean isMetaInf) {
-        Scanner scanner = buildContext.newScanner(baseDir);
+    private void validateDirectoryRecursively(ValidationHelper validationHelper, ValidationExecutor executor, Path baseDir, boolean isMetaInf) {
+        Scanner scanner = buildContext.newScanner(baseDir.toFile());
         // make sure filtering does work equally as within the package goal
         scanner.setExcludes(excludes);
         scanner.addDefaultExcludes();
         scanner.scan();
         getLog().info("Scanning baseDir " + getProjectRelativeFilePath(baseDir) + "...");
-        List<String> sortedFileNames = Arrays.asList(scanner.getIncludedFiles());
-        sortedFileNames.sort(new DotContentXmlFirstComparator());
-        for (String fileName : sortedFileNames) {
-            validateFile(validationHelper, executor, baseDir, isMetaInf, fileName);
-        }
-        for (String relativeFile : scanner.getIncludedDirectories()) {
-            validateFolder(validationHelper, executor, baseDir, isMetaInf, relativeFile);
+        SortedSet<Path> sortedFileAndFolderNames = sortAndEnrichFilesAndFolders(baseDir, scanner.getIncludedFiles(), scanner.getIncludedDirectories());
+        
+        for (Path fileOrFolder : sortedFileAndFolderNames) {
+            getLog().info("Scanning path " + fileOrFolder + "...");
+            if (Files.isDirectory(baseDir.resolve(fileOrFolder))) {
+                validateFolder(validationHelper, executor, baseDir, isMetaInf, fileOrFolder);
+            } else {
+                validateFile(validationHelper, executor, baseDir, isMetaInf, fileOrFolder);
+            }
         }
     }
 
+    static SortedSet<Path> sortAndEnrichFilesAndFolders(Path baseDir, String[] files, String[] directories) {
+        // first sort by segments
+        SortedSet<Path> paths = new TreeSet<>(new ParentAndDotContentXmlFirstComparator());
+        for (String file : files) {
+            paths.add(Paths.get(file));
+        }
+        for (String directory : directories) {
+            paths.add(Paths.get(directory));
+        }
+        
+        for (Path path : paths) {
+            Path parent = path.getParent();
+            if (parent != null && !paths.contains(parent)) {
+                if (Files.isDirectory(baseDir.resolve(parent))) {
+                    paths.add(parent);
+                }
+            }
+        }
+        return paths;
+    }
 
-    private void validateFile(ValidationHelper validationHelper, ValidationExecutor executor, File baseDir, boolean isMetaInf, String relativeFile) {
-        File absoluteFile = new File(baseDir, relativeFile);
-        validationHelper.clearPreviousValidationMessages(buildContext, absoluteFile);
+    private void validateFile(ValidationHelper validationHelper, ValidationExecutor executor, Path baseDir, boolean isMetaInf, Path relativeFile) {
+        Path absoluteFile = baseDir.resolve(relativeFile);
+        validationHelper.clearPreviousValidationMessages(buildContext, absoluteFile.toFile());
         getLog().debug("Validating file " + getProjectRelativeFilePath(absoluteFile) + "...");
-        try (InputStream input = new FileInputStream(absoluteFile)) {
+        try (InputStream input = Files.newInputStream(absoluteFile)) {
             validateInputStream(validationHelper, executor, input, baseDir, isMetaInf, relativeFile);
         } catch (FileNotFoundException e) {
             getLog().error("Could not find file " + getProjectRelativeFilePath(absoluteFile), e);
@@ -230,23 +255,23 @@ public class ValidateFilesMojo extends AbstractValidateMojo {
         }
     }
     
-    private void validateFolder(ValidationHelper validationHelper, ValidationExecutor executor, File baseDir, boolean isMetaInf, String relativeFile) {
-        File absoluteFile = new File(baseDir, relativeFile);
-        validationHelper.clearPreviousValidationMessages(buildContext, absoluteFile);
-        getLog().debug("Validating folder " + getProjectRelativeFilePath(absoluteFile) + "...");
+    private void validateFolder(ValidationHelper validationHelper, ValidationExecutor executor, Path baseDir, boolean isMetaInf, Path relativeFolder) {
+        Path absoluteFolder = baseDir.resolve(relativeFolder);
+        validationHelper.clearPreviousValidationMessages(buildContext, absoluteFolder.toFile());
+        getLog().debug("Validating folder " + getProjectRelativeFilePath(absoluteFolder) + "...");
         try {
-            validateInputStream(validationHelper, executor, null, baseDir, isMetaInf, relativeFile);
+            validateInputStream(validationHelper, executor, null, baseDir, isMetaInf, relativeFolder);
         } catch (IOException e) {
-            getLog().error("Could not validate folder " + getProjectRelativeFilePath(absoluteFile), e);
+            getLog().error("Could not validate folder " + getProjectRelativeFilePath(absoluteFolder), e);
         }
     }
     
-    private void validateInputStream(ValidationHelper validationHelper, ValidationExecutor executor, InputStream input, File baseDir, boolean isMetaInf, String relativeFile) throws IOException {
+    private void validateInputStream(ValidationHelper validationHelper, ValidationExecutor executor, InputStream input, Path baseDir, boolean isMetaInf, Path relativeFile) throws IOException {
         final Collection<ValidationViolation> messages;
         if (isMetaInf) {
-            messages = executor.validateMetaInf(input, Paths.get(relativeFile), baseDir.toPath());
+            messages = executor.validateMetaInf(input, relativeFile, baseDir);
         } else {
-            messages = executor.validateJcrRoot(input, Paths.get(relativeFile), baseDir.toPath());
+            messages = executor.validateJcrRoot(input, relativeFile, baseDir);
         }
         validationHelper.printMessages(messages, getLog(), buildContext, project.getBasedir().toPath());
     }
@@ -300,4 +325,33 @@ public class ValidateFilesMojo extends AbstractValidateMojo {
         }
         return false;
     }
+    
+
+    /** 
+     * Comparator on paths which makes sure that the parent folders come first, then a file in the parent folder called {@code .content.xml} and then all other subfolders/files ordered lexicographically. 
+     */
+    static final class ParentAndDotContentXmlFirstComparator implements Comparator<Path> {
+        private final DotContentXmlFirstComparator dotXmlFirstComparator;
+        
+        
+        public ParentAndDotContentXmlFirstComparator() {
+            super();
+            this.dotXmlFirstComparator = new DotContentXmlFirstComparator();
+        }
+
+        @Override
+        public int compare(Path s1, Path s2) {
+            if (s1.getNameCount() < s2.getNameCount()) {
+                return -1;
+            } else if (s1.getNameCount() > s2.getNameCount()) {
+                return 1;
+            } else {
+                if (s1.getParent() != null && s1.getParent().equals(s2.getParent())) {
+                    return dotXmlFirstComparator.compare(s1.getFileName().toString(), s2.getFileName().toString());
+                } else {
+                    return s1.compareTo(s2);
+                }
+            }
+        }
+    }
 }
diff --git a/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidatePackageMojo.java b/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidatePackageMojo.java
index 811dadb..ebe93ba 100644
--- a/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidatePackageMojo.java
+++ b/src/main/java/org/apache/jackrabbit/filevault/maven/packaging/ValidatePackageMojo.java
@@ -85,14 +85,14 @@ public class ValidatePackageMojo extends AbstractValidateMojo {
         try {
             boolean foundPackage = false;
             if (packageFile != null && !packageFile.toString().isEmpty() && !packageFile.isDirectory()) {
-                validatePackage(validationHelper, packageFile);
+                validatePackage(validationHelper, packageFile.toPath());
                 foundPackage = true;
             } 
             if (!attachedArtifacts.isEmpty()) {
                 for (Artifact attached : attachedArtifacts) {
                     // validate attached artifacts with given classifiers
                     if (classifiers.contains(attached.getClassifier())) {
-                        validatePackage(validationHelper, attached.getFile());
+                        validatePackage(validationHelper, attached.getFile().toPath());
                         foundPackage = true;
                     }
                 }
@@ -106,19 +106,19 @@ public class ValidatePackageMojo extends AbstractValidateMojo {
         }
     }
 
-    private void validatePackage(ValidationHelper validationHelper, File file) throws IOException, ParserConfigurationException, SAXException, MojoExecutionException {
+    private void validatePackage(ValidationHelper validationHelper, Path file) throws IOException, ParserConfigurationException, SAXException, MojoExecutionException {
         getLog().info("Start validating package " + getProjectRelativeFilePath(file) + "...");
 
         // open file to extract the meta data for the validation context
         ArchiveValidationContextImpl context;
         ValidationExecutor executor;
-        try (Archive archive = new ZipArchive(file)) {
+        try (Archive archive = new ZipArchive(file.toFile())) {
             archive.open(true);
-            context = new ArchiveValidationContextImpl(archive, file.toPath(), resolver, getLog());
+            context = new ArchiveValidationContextImpl(archive, file, resolver, getLog());
             executor = validationExecutorFactory.createValidationExecutor(context, false, enforceRecursiveSubpackageValidation, getValidatorSettingsForPackage(context.getProperties().getId(), false));
             if (executor != null) {
                 validationHelper.printUsedValidators(getLog(), executor, context, true);
-                validateArchive(validationHelper, archive, file.toPath(), context, executor);
+                validateArchive(validationHelper, archive, file, context, executor);
             } else {
                 throw new MojoExecutionException("No registered validators found!");
             }
@@ -135,7 +135,7 @@ public class ValidatePackageMojo extends AbstractValidateMojo {
     private void validateEntry(ValidationHelper validationHelper, Archive archive, Archive.Entry entry, Path entryPath, Path packagePath, ArchiveValidationContextImpl context,
             ValidationExecutor executor) throws IOException, SAXException, ParserConfigurationException {
         // sort children to make sure that .content.xml comes first!
-        List<Archive.Entry> sortedEntryList = new ArrayList<Archive.Entry>(entry.getChildren());
+        List<Archive.Entry> sortedEntryList = new ArrayList<>(entry.getChildren());
         sortedEntryList.sort(Comparator.comparing(Archive.Entry::getName, new DotContentXmlFirstComparator()));
         
         for (Archive.Entry childEntry : sortedEntryList) {
diff --git a/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojoTest.java b/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojoTest.java
new file mode 100644
index 0000000..95f7c45
--- /dev/null
+++ b/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/ValidateFilesMojoTest.java
@@ -0,0 +1,52 @@
+/*
+ * 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.jackrabbit.filevault.maven.packaging;
+
+import java.io.File;
+import java.nio.file.Path;
+import java.nio.file.Paths;
+import java.util.SortedSet;
+import java.util.TreeSet;
+
+import org.junit.Assert;
+import org.junit.Test;
+
+public class ValidateFilesMojoTest {
+
+    @Test
+    public void testSortAndEnrichFilesAndFolders() {
+        SortedSet<Path> expectedPaths = new TreeSet<>();
+        expectedPaths.add(Paths.get("apps"));
+        expectedPaths.add(Paths.get("apps", ".content.xml"));
+        expectedPaths.add(Paths.get("apps", "file"));
+        expectedPaths.add(Paths.get("apps", "huhu"));
+        expectedPaths.add(Paths.get("apps", "test"));
+        expectedPaths.add(Paths.get("apps", "test", ".content.xml"));
+        expectedPaths.add(Paths.get("apps", "test", "huhu"));
+        Assert.assertEquals(expectedPaths, ValidateFilesMojo.sortAndEnrichFilesAndFolders(Paths.get("base"), new String[] { 
+                "apps" + File.separatorChar + "huhu",
+                "apps" + File.separatorChar + "file",
+                "apps" + File.separatorChar + "test" + File.separatorChar + "huhu", 
+                "apps" + File.separatorChar + "test" + File.separatorChar + ".content.xml",
+                "apps" + File.separatorChar + ".content.xml",
+            }, 
+            new String[]{ 
+                    "apps",
+                    "apps" + File.separatorChar + "test"
+            }));
+    }
+}
diff --git a/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/it/ValidateFilesIT.java b/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/it/ValidateFilesIT.java
index 0dbe2f0..c5d4c6c 100644
--- a/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/it/ValidateFilesIT.java
+++ b/src/test/java/org/apache/jackrabbit/filevault/maven/packaging/it/ValidateFilesIT.java
@@ -32,7 +32,7 @@ public class ValidateFilesIT {
                 .setTestGoals("clean", "process-classes") // make sure the validate-files mojo is not skipped
                 .setBuildExpectedToFail(true)
                 .build()
-                .verifyExpectedLogLines(Paths.get("target","vault-work","META-INF","vault","filter.xml").toString());
+                .verifyExpectedLogLines(Paths.get("src","main","content", "META-INF","vault","filter.xml").toString());
     }
 
     @Test