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 2022/04/02 17:27:10 UTC

[jackrabbit-filevault] branch feature/JCRVLT-623-validate-docview-prop-values created (now f790f2f)

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

kwin pushed a change to branch feature/JCRVLT-623-validate-docview-prop-values
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git.


      at f790f2f  JCRVLT-623 check for correct docview property values (according to the given type)

This branch includes the following new commits:

     new f790f2f  JCRVLT-623 check for correct docview property values (according to the given type)

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-623 check for correct docview property values (according to the given type)

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

kwin pushed a commit to branch feature/JCRVLT-623-validate-docview-prop-values
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git

commit f790f2febbc777dd51bb6423dd60630786aa627b
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Sat Apr 2 19:26:57 2022 +0200

    JCRVLT-623 check for correct docview property values (according to the
    given type)
    
    Keep original property order from the docview xmlt
---
 src/site/markdown/validation.md                    |  2 +-
 .../vault/fs/impl/io/DocViewSAXHandler.java        |  2 +-
 .../vault/fs/io/DocViewParserHandler.java          | 11 ++++++
 .../jackrabbit/vault/fs/io/package-info.java       |  2 +-
 .../apache/jackrabbit/vault/util/DocViewNode2.java |  8 ++++-
 .../impl/util/ValidatorDocViewParserHandler.java   | 42 +++++++++++++++++++++-
 .../spi/impl/DocumentViewParserValidator.java      |  2 +-
 .../DocumentViewParserValidatorTest.java           | 20 ++++++++++-
 .../jcr_root/apps/invalid/inconvertibletypes.xml   | 27 ++++++++++++++
 9 files changed, 109 insertions(+), 7 deletions(-)

diff --git a/src/site/markdown/validation.md b/src/site/markdown/validation.md
index b0cf197..8b4a649 100644
--- a/src/site/markdown/validation.md
+++ b/src/site/markdown/validation.md
@@ -54,7 +54,7 @@ ID  |  Description | Options | Incremental Execution Limitations
 `jackrabbit-filter` |  Checks for validity of the [filter.xml](./filter.html) (according to a predefined  XML schema). In addition checks that every [docview xml node](./docview.html) is contained in the filter. It also makes sure that all filter root's ancestors are either known/valid roots or are contained in the package dependencies. For ancestor nodes which are not covered by a filter at least a `warn` is emitted. Also it makes sure that `pattern` values for includes/excludes as well [...]
 `jackrabbit-properties ` | Checks for validity of the  [properties.xml](./properties.html) | none | none
 `jackrabbit-dependencies` | Checks for overlapping filter roots of the referenced package dependencies as well as for valid package dependency references (i.e. references which can be resolved). | *severityForUnresolvedDependencies*: severity of validation messages for unresolved dependencies (default = `warn`) | none
-`jackrabbit-docviewparser` | Checks if all docview files in the package are compliant with the [(extended) Document View Format](docview.html). This involves checking for XML validity as well as checking for correct property types. | *allowUndeclaredPrefixInFileName*: if set to `true` then prefixes only used in the encoded docview xml filename don't need to be declared in the XML itself, otherwise a missing declaration leads to a validation error. | none
+`jackrabbit-docviewparser` | Checks if all docview files in the package are compliant with the [(extended) Document View Format](docview.html). This involves checking for XML validity as well as checking for known property types and their valid string serializations. | *allowUndeclaredPrefixInFileName*: if set to `true` then prefixes only used in the encoded docview xml filename don't need to be declared in the XML itself, otherwise a missing declaration leads to a validation error. | none
 `jackrabbit-emptyelements` | Check for empty elements within DocView files (used for ordering purposes, compare with  [(extended) Document View Format](docview.html)) which are included in the filter with import=replace as those are actually not replaced! | none | none
 `jackrabbit-mergelimitations` | Checks for the limitation of import mode=merge outlined at [JCRVLT-255][jcrvlt-255]. | none | none
 `jackrabbit-oakindex` |  Checks if the package (potentially) modifies/creates an OakIndexDefinition. This is done by evaluating both the filter.xml for potential matches as well as the actual content for nodes with jcr:primaryType  `oak:indexDefinition`. | none | none
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXHandler.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXHandler.java
index 07f29c3..b6fd8cb 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXHandler.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/impl/io/DocViewSAXHandler.java
@@ -178,7 +178,7 @@ public class DocViewSAXHandler extends RejectingEntityDefaultHandler implements
      */
     @Override
     public void startDocument() throws SAXException {
-        
+        this.handler.setNameResolver(nameResolver);
     }
 
     @Override
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/DocViewParserHandler.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/DocViewParserHandler.java
index 8aea047..3b098f6 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/DocViewParserHandler.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/DocViewParserHandler.java
@@ -21,6 +21,7 @@ import java.util.Optional;
 
 import javax.jcr.RepositoryException;
 
+import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
 import org.apache.jackrabbit.vault.util.DocViewNode2;
 import org.jetbrains.annotations.NotNull;
 
@@ -62,6 +63,7 @@ public interface DocViewParserHandler {
 
     /**
      * Called when a namespace mapping is defined in the docview xml.
+     * Rather use {@link #setNameResolver(NameResolver)} instead if you just need to resolve JCR names.
      * @param prefix the namespace prefix
      * @param uri the namespace uri
      */
@@ -69,7 +71,16 @@ public interface DocViewParserHandler {
 
     /**
      * Called when a namespace mapping end in the docview xml.
+     * Rather use {@link #setNameResolver(NameResolver)} instead if you just need to resolve JCR names.
      * @param prefix the namespace prefix
      */
     default void endPrefixMapping(String prefix) {};
+
+    /**
+     * Called before the first {@link DocViewParserHandler#startDocViewNode(String, DocViewNode2, Optional, int, int)} is called.
+     * Provides a NameResolver which can be used to resolve JCR names to their qualified form.
+     * Can be used instead of overwriting {@link #startPrefixMapping(String, String)} and {@link #endPrefixMapping(String)}.
+     * @param nameResolver the resolver aware of all namespaces and their prefixes defined in the underlying XML document.
+     */
+    default void setNameResolver(NameResolver nameResolver) {};
 }
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/package-info.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/package-info.java
index 4989948..62e3e80 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/package-info.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/fs/io/package-info.java
@@ -15,7 +15,7 @@
  * limitations under the License.
  */
 
-@Version("2.13.0")
+@Version("2.14.0")
 package org.apache.jackrabbit.vault.fs.io;
 
 import org.osgi.annotation.versioning.Version;
diff --git a/vault-core/src/main/java/org/apache/jackrabbit/vault/util/DocViewNode2.java b/vault-core/src/main/java/org/apache/jackrabbit/vault/util/DocViewNode2.java
index 3096392..ec35aea 100644
--- a/vault-core/src/main/java/org/apache/jackrabbit/vault/util/DocViewNode2.java
+++ b/vault-core/src/main/java/org/apache/jackrabbit/vault/util/DocViewNode2.java
@@ -18,6 +18,7 @@ package org.apache.jackrabbit.vault.util;
 
 import java.util.Collection;
 import java.util.Collections;
+import java.util.LinkedHashMap;
 import java.util.Map;
 import java.util.Objects;
 import java.util.Optional;
@@ -55,7 +56,12 @@ public class DocViewNode2 {
         }
         this.index = index;
         Objects.requireNonNull(properties, "properties must not be null");
-        this.properties = properties.stream().collect(Collectors.toMap(DocViewProperty2::getName, Function.identity()));
+        this.properties = properties.stream().collect(Collectors.toMap(
+                DocViewProperty2::getName, 
+                Function.identity(),
+                (existing, replacement) -> existing,
+                LinkedHashMap::new // keep order of properties in the map
+                ));
     }
 
     public @NotNull DocViewNode2 cloneWithDifferentProperties(@NotNull Collection<DocViewProperty2> properties) {
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/impl/util/ValidatorDocViewParserHandler.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/impl/util/ValidatorDocViewParserHandler.java
index 741196f..db1627a 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/impl/util/ValidatorDocViewParserHandler.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/impl/util/ValidatorDocViewParserHandler.java
@@ -25,14 +25,22 @@ import java.util.List;
 import java.util.Map;
 import java.util.Optional;
 
+import javax.jcr.NamespaceException;
+import javax.jcr.PropertyType;
 import javax.jcr.RepositoryException;
+import javax.jcr.ValueFactory;
+import javax.jcr.ValueFormatException;
 
+import org.apache.jackrabbit.commons.SimpleValueFactory;
+import org.apache.jackrabbit.spi.commons.conversion.NameResolver;
 import org.apache.jackrabbit.vault.fs.io.DocViewParserHandler;
 import org.apache.jackrabbit.vault.util.DocViewNode2;
+import org.apache.jackrabbit.vault.util.DocViewProperty2;
 import org.apache.jackrabbit.vault.validation.ValidationViolation;
 import org.apache.jackrabbit.vault.validation.spi.DocumentViewXmlValidator;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessage;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessageSeverity;
+import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidatorFactory;
 import org.apache.jackrabbit.vault.validation.spi.util.NodeContextImpl;
 import org.jetbrains.annotations.NotNull;
 
@@ -43,18 +51,32 @@ public class ValidatorDocViewParserHandler implements DocViewParserHandler {
     private final @NotNull Path basePath;
     private final Map<String, DocumentViewXmlValidator> validators;
     private final @NotNull List<ValidationViolation> violations;
+    private final @NotNull ValidationMessageSeverity severity;
+    private final ValueFactory valueFactory;
+    private NameResolver nameResolver;
 
-    public ValidatorDocViewParserHandler(Map<String, DocumentViewXmlValidator> docViewValidators, @NotNull Path filePath, @NotNull Path basePath) {
+    public static final String MESSAGE_INVALID_STRING_SERIALIZATION = "Invalid string serialization for type '%s' given in property '%s' : '%s'. This string cannot be converted to the specified type!";
+
+    public ValidatorDocViewParserHandler(@NotNull ValidationMessageSeverity severity, @NotNull Map<String, DocumentViewXmlValidator> docViewValidators, @NotNull Path filePath, @NotNull Path basePath) {
         this.nodePathsAndLineNumbers = new HashMap<>();
         this.filePath = filePath;
         this.basePath = basePath;
         this.validators = docViewValidators;
         violations = new LinkedList<>();
+        this.valueFactory = new SimpleValueFactory();
+        this.severity = severity;
     }
 
     @Override
+    public void setNameResolver(NameResolver nameResolver) {
+        this.nameResolver = nameResolver;
+    }
+
+
+    @Override
     public void startDocViewNode(@NotNull String nodePath, @NotNull DocViewNode2 docViewNode,
             @NotNull Optional<DocViewNode2> parentDocViewNode, int lineNumber, int columnNumber) throws IOException, RepositoryException {
+        validatePropertyValues(docViewNode.getProperties(), nodePath, lineNumber, columnNumber);
         callValidators(true, nodePath, docViewNode, parentDocViewNode, lineNumber, columnNumber);
         if (!docViewNode.getProperties().isEmpty()) {
             nodePathsAndLineNumbers.put(nodePath, lineNumber);
@@ -76,6 +98,24 @@ public class ValidatorDocViewParserHandler implements DocViewParserHandler {
         return violations;
     }
 
+    private void validatePropertyValues(Collection<DocViewProperty2> properties, String nodePath, int lineNumber, int columnNumber) {
+        for (DocViewProperty2 property : properties) {
+            for (String value : property.getStringValues()) {
+                try {
+                    valueFactory.createValue(value, property.getType());
+                } catch (ValueFormatException e) {
+                    String message;
+                    try {
+                        message = String.format(MESSAGE_INVALID_STRING_SERIALIZATION, PropertyType.nameFromValue(property.getType()), nameResolver.getJCRName(property.getName()), value);
+                    } catch (NamespaceException e1) {
+                        message = String.format(MESSAGE_INVALID_STRING_SERIALIZATION, PropertyType.nameFromValue(property.getType()), property.getName(), value);
+                    }
+                    violations.add(new ValidationViolation(DocumentViewParserValidatorFactory.ID, severity, message, filePath, basePath, nodePath, lineNumber, columnNumber, null));
+                }
+            }
+        }
+    }
+
     private void callValidators(boolean isStart, String nodePath, DocViewNode2 docViewNode, Optional<DocViewNode2> parentDocViewNode, int lineNumber,
             int columnNumber) {
         violations.add(new ValidationViolation(ValidationMessageSeverity.DEBUG, "Validate node '" + docViewNode + "' " + (isStart ? "start" : "end")));
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidator.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidator.java
index c915041..b788bb5 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidator.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/impl/DocumentViewParserValidator.java
@@ -112,7 +112,7 @@ public class DocumentViewParserValidator implements GenericJcrDataValidator {
             Map<String, Integer> nodePathsAndLineNumbers) throws IOException {
         List<ValidationMessage> enrichedMessages = new LinkedList<>();
         enrichedMessages.add(new ValidationMessage(ValidationMessageSeverity.DEBUG, "Detected DocView..."));
-        ValidatorDocViewParserHandler handler = new ValidatorDocViewParserHandler(docViewValidators, filePath, basePath);
+        ValidatorDocViewParserHandler handler = new ValidatorDocViewParserHandler(severity, docViewValidators, filePath, basePath);
         try {
             docViewParser.parse(rootNodePath, new InputSource(new CloseShieldInputStream(input)), handler);
             enrichedMessages.addAll(ValidationViolation.wrapMessages(null, handler.getViolations(), filePath, basePath, rootNodePath, 0, 0));
diff --git a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/DocumentViewParserValidatorTest.java b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/DocumentViewParserValidatorTest.java
index 5fa268a..b0e3c23 100644
--- a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/DocumentViewParserValidatorTest.java
+++ b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/DocumentViewParserValidatorTest.java
@@ -40,6 +40,7 @@ import org.apache.jackrabbit.vault.fs.io.DocViewParser.XmlParseException;
 import org.apache.jackrabbit.vault.util.DocViewNode2;
 import org.apache.jackrabbit.vault.util.DocViewProperty2;
 import org.apache.jackrabbit.vault.util.JcrConstants;
+import org.apache.jackrabbit.vault.validation.impl.util.ValidatorDocViewParserHandler;
 import org.apache.jackrabbit.vault.validation.spi.DocumentViewXmlValidator;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessage;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessageSeverity;
@@ -282,7 +283,7 @@ public class DocumentViewParserValidatorTest {
     }
 
     @Test
-    public void testDocViewWithInvalidType() throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
+    public void testDocViewWithUnknownType() throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
         try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/invalid/wrongtype.xml")) {
             Collection<ValidationMessage> messages = validator.validateJcrData(input, Paths.get("apps", "invalid","wrongtype.xml"), Paths.get(""), nodePathsAndLineNumbers);
 
@@ -294,4 +295,21 @@ public class DocumentViewParserValidatorTest {
         }
     }
 
+    @Test
+    public void testDocViewWithInvalidStringSerializationForType() throws ParserConfigurationException, SAXException, URISyntaxException, IOException {
+        try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/invalid/inconvertibletypes.xml")) {
+            Collection<ValidationMessage> messages = validator.validateJcrData(input, Paths.get("apps", "invalid","inconvertibletypes.xml"), Paths.get(""), nodePathsAndLineNumbers);
+
+           ValidationExecutorTest.assertViolation(messages,
+                    new ValidationViolation(DocumentViewParserValidatorFactory.ID,
+                            ValidationMessageSeverity.ERROR,
+                            String.format(ValidatorDocViewParserHandler.MESSAGE_INVALID_STRING_SERIALIZATION, "Long", "attribute2", "1.0"),
+                            Paths.get("apps/invalid/inconvertibletypes.xml"), Paths.get(""), "/apps/invalid/inconvertibletypes/somepath", 26, 6, null),
+                    new ValidationViolation(DocumentViewParserValidatorFactory.ID,
+                            ValidationMessageSeverity.ERROR,
+                            String.format(ValidatorDocViewParserHandler.MESSAGE_INVALID_STRING_SERIALIZATION, "Date", "attribute1", "somedate"),
+                            Paths.get("apps/invalid/inconvertibletypes.xml"), Paths.get(""), "/apps/invalid/inconvertibletypes/somepath", 26, 6, null) 
+                   );
+        }
+    }
 }
diff --git a/vault-validation/src/test/resources/simple-package/jcr_root/apps/invalid/inconvertibletypes.xml b/vault-validation/src/test/resources/simple-package/jcr_root/apps/invalid/inconvertibletypes.xml
new file mode 100644
index 0000000..0b4ab27
--- /dev/null
+++ b/vault-validation/src/test/resources/simple-package/jcr_root/apps/invalid/inconvertibletypes.xml
@@ -0,0 +1,27 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<!--
+  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.
+  -->
+
+<jcr:root xmlns:sling="http://sling.apache.org/jcr/sling/1.0" xmlns:jcr="http://www.jcp.org/jcr/1.0"
+    jcr:primaryType="sling:Folder">
+    <somepath jcr:primaryType="nt:unstructured"
+        attribute2="{Long}1.0"
+        attribute1="{Date}somedate"
+        attribute3="{Boolean}true"
+        attribute4="{Boolean}TRUE"
+    ></somepath>
+</jcr:root>