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/12/15 18:01:04 UTC

[jackrabbit-filevault] branch bugfix/prevent-NoSuchElementException-when-jackrabbit-docviewparser-is-disabled created (now eaccc17)

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

kwin pushed a change to branch bugfix/prevent-NoSuchElementException-when-jackrabbit-docviewparser-is-disabled
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git.


      at eaccc17  JCRVLT-??? Prevent NoSuchElementException in ValidationExecutor.validateGenericJcrData

This branch includes the following new commits:

     new eaccc17  JCRVLT-??? Prevent NoSuchElementException in ValidationExecutor.validateGenericJcrData

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] 01/01: JCRVLT-??? Prevent NoSuchElementException in ValidationExecutor.validateGenericJcrData

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

kwin pushed a commit to branch bugfix/prevent-NoSuchElementException-when-jackrabbit-docviewparser-is-disabled
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit eaccc17d451b21f53c2678a8da35bf45fdaa9874
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed Dec 15 19:00:34 2021 +0100

    JCRVLT-??? Prevent NoSuchElementException in
    ValidationExecutor.validateGenericJcrData
    
    This happened in case validator "jackrabbit-docviewparser" was disabled
    and there were no nodePaths being collected
---
 .../vault/validation/ValidationExecutor.java       | 27 ++++++++++-----
 .../impl/DocumentViewParserValidatorFactory.java   |  3 +-
 .../vault/validation/ValidationExecutorTest.java   | 40 ++++++++++++++++++++--
 3 files changed, 58 insertions(+), 12 deletions(-)

diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java
index 2195566..ee96d47 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/ValidationExecutor.java
@@ -21,16 +21,18 @@ import java.io.InputStream;
 import java.nio.file.Path;
 import java.util.Collection;
 import java.util.HashMap;
+import java.util.HashSet;
 import java.util.LinkedHashMap;
 import java.util.LinkedList;
 import java.util.List;
 import java.util.Map;
+import java.util.Set;
 import java.util.stream.Collectors;
 
 import org.apache.commons.io.FilenameUtils;
+import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.vault.util.Constants;
 import org.apache.jackrabbit.vault.util.PlatformNameFormat;
-import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.vault.validation.impl.util.EnhancedBufferedInputStream;
 import org.apache.jackrabbit.vault.validation.impl.util.ResettableInputStream;
 import org.apache.jackrabbit.vault.validation.impl.util.ValidatorException;
@@ -50,6 +52,7 @@ import org.apache.jackrabbit.vault.validation.spi.Validator;
 import org.apache.jackrabbit.vault.validation.spi.impl.AdvancedFilterValidator;
 import org.apache.jackrabbit.vault.validation.spi.impl.AdvancedPropertiesValidator;
 import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidator;
+import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidatorFactory;
 import org.apache.jackrabbit.vault.validation.spi.util.NodeContextImpl;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
@@ -108,6 +111,13 @@ public final class ValidationExecutor {
                 DocumentViewParserValidator.class.cast(validator).setDocumentViewXmlValidators(documentViewXmlValidators);
             }
         }
+        
+        // jcr path validators requires documentviewparservalidator (as that retrieves the JCR paths in the first place)
+        if (!(jcrPathValidators.isEmpty() || nodePathValidators.isEmpty()) && !genericJcrDataValidators.containsKey(DocumentViewParserValidatorFactory.ID)) {
+            Set<String> pathValidatorIds = new HashSet<>(jcrPathValidators.keySet());
+            pathValidatorIds.addAll(nodePathValidators.keySet());
+            throw new IllegalStateException("For the validators with id(s) '" + String.join(", ", pathValidatorIds) + "' it is mandatory to also enable validator '" + DocumentViewParserValidatorFactory.ID + "'");
+        }
     }
 
     /**
@@ -325,15 +335,16 @@ public final class ValidationExecutor {
             nodePathsAndLineNumbers.put(filePathToNodePath(filePath), 0);
         }
 
-        // generate node context
-        NodeContext nodeContext = new NodeContextImpl(nodePathsAndLineNumbers.keySet().iterator().next(), filePath, basePath);
-        for (Map.Entry<String, JcrPathValidator> entry : jcrPathValidators.entrySet()) {
-            Collection<ValidationMessage> messages = entry.getValue().validateJcrPath(nodeContext, input == null, isDocViewXml);
-            if (messages != null && !messages.isEmpty()) {
-                enrichedMessages.addAll(ValidationViolation.wrapMessages(entry.getKey(), messages, filePath, basePath, null, 0, 0));
+        if (!jcrPathValidators.isEmpty() || !nodePathValidators.isEmpty()) {
+            NodeContext nodeContext = new NodeContextImpl(nodePathsAndLineNumbers.keySet().stream().findFirst().orElseThrow(() -> new IllegalStateException("No node paths have been collected")), filePath, basePath);
+            for (Map.Entry<String, JcrPathValidator> entry : jcrPathValidators.entrySet()) {
+                Collection<ValidationMessage> messages = entry.getValue().validateJcrPath(nodeContext, input == null, isDocViewXml);
+                if (messages != null && !messages.isEmpty()) {
+                    enrichedMessages.addAll(ValidationViolation.wrapMessages(entry.getKey(), messages, filePath, basePath, null, 0, 0));
+                }
             }
+            enrichedMessages.addAll(validateNodePaths(filePath, basePath, nodePathsAndLineNumbers));
         }
-        enrichedMessages.addAll(validateNodePaths(filePath, basePath, nodePathsAndLineNumbers));
         return enrichedMessages;
     }
 
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java
index 0cb1f72..509c34f 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidatorFactory.java
@@ -34,6 +34,7 @@ public class DocumentViewParserValidatorFactory implements ValidatorFactory {
 
     private SAXParser saxParser;
 
+    public static final String ID = ValidatorFactory.ID_PREFIX_JACKRABBIT + "docviewparser";
     public DocumentViewParserValidatorFactory() throws ParserConfigurationException, SAXException {
 
         SAXParserFactory spf = SAXParserFactory.newInstance();
@@ -53,7 +54,7 @@ public class DocumentViewParserValidatorFactory implements ValidatorFactory {
 
     @Override
     public @NotNull String getId() {
-        return ValidatorFactory.ID_PREFIX_JACKRABBIT + "docviewparser";
+        return ID;
     }
 
     @Override
diff --git a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java
index 9860812..de9adc7 100644
--- a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java
+++ b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ValidationExecutorTest.java
@@ -44,6 +44,7 @@ import org.apache.jackrabbit.vault.validation.spi.ValidationContext;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessage;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessageSeverity;
 import org.apache.jackrabbit.vault.validation.spi.Validator;
+import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidatorFactory;
 import org.apache.jackrabbit.vault.validation.spi.util.NodeContextImpl;
 import org.hamcrest.MatcherAssert;
 import org.hamcrest.Matchers;
@@ -91,7 +92,7 @@ public class ValidationExecutorTest {
         validators.put("propertiesid", propertiesValidator);
         validators.put("genericmetadataid", genericMetaInfDataValidator);
         validators.put("genericmetadataid2", genericMetaInfDataValidator2);
-        validators.put("genericjcrdataid", genericJcrDataValidator);
+        validators.put(DocumentViewParserValidatorFactory.ID, genericJcrDataValidator);
         validators.put("genericjcrdataid2", genericJcrDataValidator2);
         validators.put("jcrpathid", jcrPathValidator);
         validators.put("metainfpathid", metaInfPathValidator);
@@ -173,7 +174,7 @@ public class ValidationExecutorTest {
         try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/genericfile.xml")) {
             Collection<ValidationViolation> messages = validate(input, executor, Paths.get(""), "apps/genericfile.xml", false);
             assertViolation(messages, 
-                    new ValidationViolation("genericjcrdataid", ValidationMessageSeverity.WARN, "error1", Paths.get("apps","genericfile.xml"), Paths.get(""), null, 0, 0, null),
+                    new ValidationViolation(DocumentViewParserValidatorFactory.ID, ValidationMessageSeverity.WARN, "error1", Paths.get("apps","genericfile.xml"), Paths.get(""), null, 0, 0, null),
                     new ValidationViolation("jcrpathid", ValidationMessageSeverity.ERROR, "patherror", Paths.get("apps","genericfile.xml"), Paths.get(""), null, 0, 0, null));
             Assert.assertEquals("Test", answer.getValue());
             Assert.assertEquals("Test", answer2.getValue());
@@ -198,6 +199,39 @@ public class ValidationExecutorTest {
     }
 
     @Test
+    public void testGenericJcrDataWithNoGenericJcrDataValidator()
+            throws URISyntaxException, IOException, SAXException, ParserConfigurationException, ConfigurationException {
+        Map<String, Validator> validators = new LinkedHashMap<>();
+        validators.put("docviewid", docViewXmlValidator);
+        validators.put("propertiesid", propertiesValidator);
+        validators.put("genericmetadataid", genericMetaInfDataValidator);
+        validators.put("genericmetadataid2", genericMetaInfDataValidator2);
+        validators.put("jcrpathid", jcrPathValidator);
+        validators.put("metainfpathid", metaInfPathValidator);
+        validators.put("nodepathid", nodePathValidator);
+        validators.put("unusedid", unusedValidator);
+        Assert.assertThrows(IllegalStateException.class, () -> new ValidationExecutor(validators));
+    }
+
+    @Test
+    public void testGenericJcrDataWithNoGenericJcrDataAndNoPathValidator()
+            throws URISyntaxException, IOException, SAXException, ParserConfigurationException, ConfigurationException {
+        Map<String, Validator> validators = new LinkedHashMap<>();
+        validators.put("docviewid", docViewXmlValidator);
+        validators.put("propertiesid", propertiesValidator);
+        validators.put("genericmetadataid", genericMetaInfDataValidator);
+        validators.put("genericmetadataid2", genericMetaInfDataValidator2);
+        validators.put("metainfpathid", metaInfPathValidator);
+        validators.put("unusedid", unusedValidator);
+        executor = new ValidationExecutor(validators);
+        try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/genericfile.xml")) {
+            Collection<ValidationViolation> messages = validate(input, executor, Paths.get(""), "apps/genericfile.xml", false);
+            MatcherAssert.assertThat(messages, AnyValidationViolationMatcher.noValidationInCollection());
+            Mockito.verify(genericJcrDataValidator, Mockito.never()).validateJcrData(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any());
+        }
+    }
+    
+    @Test
     public void testJcrRootFolder() throws URISyntaxException, IOException, SAXException {
         Collection<ValidationViolation> messages = validateFolder(executor, Paths.get(""), "apps.dir", false);
         MatcherAssert.assertThat(messages, AnyValidationViolationMatcher.noValidationInCollection());
@@ -211,7 +245,7 @@ public class ValidationExecutorTest {
         Mockito.when(genericJcrDataValidator.done()).thenReturn(Collections.singleton(new ValidationMessage(ValidationMessageSeverity.ERROR, "test1")));
         Mockito.when(genericJcrDataValidator2.done()).thenReturn(Collections.singleton(new ValidationMessage(ValidationMessageSeverity.WARN, "test2")));
         
-        assertViolation(executor.done(), new ValidationViolation("genericjcrdataid", ValidationMessageSeverity.ERROR, "test1"), new ValidationViolation("genericjcrdataid2", ValidationMessageSeverity.WARN, "test2")); 
+        assertViolation(executor.done(), new ValidationViolation(DocumentViewParserValidatorFactory.ID, ValidationMessageSeverity.ERROR, "test1"), new ValidationViolation("genericjcrdataid2", ValidationMessageSeverity.WARN, "test2")); 
     }
 
     private Collection<ValidationViolation> validate(InputStream input, ValidationExecutor executor, Path basePath, String resourcePath,