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/05 10:26:09 UTC

[jackrabbit-filevault] branch master updated: JCRVLT-623 check for correct docview property values (according to the (#217)

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

kwin pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/jackrabbit-filevault.git


The following commit(s) were added to refs/heads/master by this push:
     new 51a4cce8 JCRVLT-623 check for correct docview property values (according to the (#217)
51a4cce8 is described below

commit 51a4cce8fb58038244eb09b986fef4bed5913726
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Tue Apr 5 12:26:05 2022 +0200

    JCRVLT-623 check for correct docview property values (according to the (#217)
    
    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   | 44 +++++++++++++++++++++-
 .../spi/impl/DocumentViewParserValidator.java      |  2 +-
 .../DocumentViewParserValidatorTest.java           | 20 +++++++++-
 .../jcr_root/apps/invalid/inconvertibletypes.xml   | 29 ++++++++++++++
 9 files changed, 113 insertions(+), 7 deletions(-)

diff --git a/src/site/markdown/validation.md b/src/site/markdown/validation.md
index b0cf197c..8b4a649f 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 07f29c3b..b6fd8cbb 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 8aea047d..3b098f63 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 49899486..62e3e80b 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 30963929..ec35aeaf 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 741196fc..70bfb3fe 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,26 @@ public class ValidatorDocViewParserHandler implements DocViewParserHandler {
         return violations;
     }
 
+    private void validatePropertyValues(Collection<DocViewProperty2> properties, String nodePath, int lineNumber, int columnNumber) {
+        for (DocViewProperty2 property : properties) {
+            if (property.getType() != PropertyType.UNDEFINED) {
+                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 c915041f..b788bb52 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 5fa268a3..c2f8604f 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", 28, 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", 28, 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 00000000..dbfd6f0b
--- /dev/null
+++ b/vault-validation/src/test/resources/simple-package/jcr_root/apps/invalid/inconvertibletypes.xml
@@ -0,0 +1,29 @@
+<?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"
+        attribute5="value"
+        attribute6="{String}value"
+    ></somepath>
+</jcr:root>