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/06/15 14:26:46 UTC

[jackrabbit-filevault] 01/01: JCRVLT-637 only determine path of non-docview xml once

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

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

commit eceb952f9b745517dce7b90a625563c4ad247ab2
Author: Konrad Windszus <kw...@apache.org>
AuthorDate: Wed Jun 15 16:26:40 2022 +0200

    JCRVLT-637 only determine path of non-docview xml once
    
    clarify javadoc of GenericJcrDataValidator
---
 .../vault/validation/ValidationExecutor.java       | 21 ++++++----
 .../validation/spi/GenericJcrDataValidator.java    |  3 +-
 .../spi/impl/DocumentViewParserValidator.java      |  1 -
 .../DocumentViewParserValidatorTest.java           |  8 ++--
 .../validation/ReturnNodeAndLineNumberAnswer.java  | 48 ----------------------
 .../vault/validation/ValidationExecutorTest.java   |  8 ++--
 6 files changed, 22 insertions(+), 67 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 11ce728d..b866f356 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
@@ -309,13 +309,10 @@ public final class ValidationExecutor {
                                 enrichedMessages.addAll(ValidationViolation.wrapMessages(entry.getKey(), messages, filePath, basePath, null, 0, 0));
                             }
                         } 
-
-                        // only do it if we haven't collected node paths from a previous run
-                        String nodePath = filePathToNodePath(filePath);
-                        boolean treatedAsBinaryFile = nodePathsAndLineNumbers.size() == 1 &&
-                            nodePathsAndLineNumbers.getOrDefault(nodePath, Integer.MIN_VALUE) == 0;
-
-                        if (nodePathsAndLineNumbers.isEmpty() || treatedAsBinaryFile) {
+                        // if we haven't collected node paths from a previous run the input is no docview xml
+                        if (nodePathsAndLineNumbers.isEmpty()) {
+                            // convert file name to node path
+                            String nodePath = filePathToNodePath(filePath);
                             log.debug("Found non-docview node '{}'", nodePath);
                             isDocViewXml = false;
                             nodePathsAndLineNumbers.put(nodePath, 0);
@@ -375,7 +372,15 @@ public final class ValidationExecutor {
     static <T> Map<String, T> filterValidatorsByClass(Map<String, Validator> allValidators, Class<T> type) {
         return allValidators.entrySet().stream()
                 .filter(x -> type.isInstance(x.getValue()))
-                .collect(Collectors.toMap(Map.Entry::getKey, x -> type.cast(x.getValue())));
+                // keep map order
+                .collect(Collectors.toMap(
+                    Map.Entry::getKey, 
+                    x -> type.cast(x.getValue()), 
+                    (u, v) -> {
+                        throw new IllegalStateException(String.format("Duplicate key %s", u));
+                    }, 
+                    LinkedHashMap::new
+                ));
     }
 
 }
diff --git a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/GenericJcrDataValidator.java b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/GenericJcrDataValidator.java
index 30e86f04..5d0100ad 100644
--- a/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/GenericJcrDataValidator.java
+++ b/vault-validation/src/main/java/org/apache/jackrabbit/vault/validation/spi/GenericJcrDataValidator.java
@@ -22,6 +22,7 @@ import java.nio.file.Path;
 import java.util.Collection;
 import java.util.Map;
 
+import org.apache.jackrabbit.vault.validation.spi.impl.DocumentViewParserValidator;
 import org.jetbrains.annotations.NotNull;
 import org.jetbrains.annotations.Nullable;
 import org.osgi.annotation.versioning.ProviderType;
@@ -57,7 +58,7 @@ public interface GenericJcrDataValidator extends Validator {
      * @param input the input stream of the file which ends up below jcr_root in the package located at filePath
      * @param filePath file path relative to the jcr_root directory (i.e. does not start with {@code jcr_root})
      * @param basePath the absolute file path of jcr_root (base for {@code filePath)})
-     * @param nodePathsAndLineNumbers a map which should be filled with all node path and their according line numbers if nodes are detected in the given input
+     * @param nodePathsAndLineNumbers a map with all found node paths and their according line numbers (for the current input stream). Must only be modified in case the given file is a docview xml and the implementation is {@link DocumentViewParserValidator})
      * @return a collection of validation messages or {@code null}
      * @throws IOException in case the input stream could not be accessed
      */
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 b788bb52..f20b7e42 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
@@ -102,7 +102,6 @@ public class DocumentViewParserValidator implements GenericJcrDataValidator {
             
         } else {
             messages.add(new ValidationMessage(ValidationMessageSeverity.INFO, "This file is not detected as docview xml file and therefore treated as binary"));
-            nodePathsAndLineNumbers.put(ValidationExecutor.filePathToNodePath(filePath), 0);
         }
         
        return messages;
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 c2f8604f..1e4a00c4 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
@@ -268,17 +268,15 @@ public class DocumentViewParserValidatorTest {
 
     @Test
     public void testDocViewWithRegularFileNameWithRootElementDifferentThanJcrRoot() throws IOException {
-        // https://issues.apache.org/jira/browse/JCRVLT-358"
+        // https://issues.apache.org/jira/browse/JCRVLT-358 and https://issues.apache.org/jira/browse/JCRVLT-637
         try (InputStream input = this.getClass().getResourceAsStream("/simple-package/jcr_root/apps/child2/child1.xml")) {
             Collection<ValidationMessage> messages = validator.validateJcrData(input, Paths.get("apps", "child2", "child1.xml"), Paths.get(""), nodePathsAndLineNumbers);
             MatcherAssert.assertThat(messages, AnyValidationViolationMessageMatcher.noValidationViolationMessageInCollection());
 
             Mockito.verifyNoMoreInteractions(docViewXmlValidator);
 
-            // verify node names
-            Map<String, Integer> expectedNodePathsAndLineNumber = new HashMap<>();
-            expectedNodePathsAndLineNumber.put("/apps/child2/child1.xml", 0);
-            Assert.assertEquals(expectedNodePathsAndLineNumber, nodePathsAndLineNumbers);
+            // verify node names in case this is no docview xml
+            Assert.assertEquals(new HashMap<>(), nodePathsAndLineNumbers);
         }
     }
 
diff --git a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ReturnNodeAndLineNumberAnswer.java b/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ReturnNodeAndLineNumberAnswer.java
deleted file mode 100644
index ef34765b..00000000
--- a/vault-validation/src/test/java/org/apache/jackrabbit/vault/validation/ReturnNodeAndLineNumberAnswer.java
+++ /dev/null
@@ -1,48 +0,0 @@
-/*
- * 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.vault.validation;
-
-import java.util.Collection;
-import java.util.Collections;
-import java.util.Map;
-
-import org.mockito.invocation.InvocationOnMock;
-import org.mockito.stubbing.Answer;
-
-class ReturnNodeAndLineNumberAnswer<T extends Collection<?>> implements Answer<T> {
-
-    private final String nodePath;
-    private final int lineNumber;
-    private final T returnValue;
-
-    public ReturnNodeAndLineNumberAnswer(String nodePath, int lineNumber) {
-        this(nodePath, lineNumber, (T) Collections.emptyList());
-    }
-
-    public ReturnNodeAndLineNumberAnswer(String nodePath, int lineNumber, T returnValue) {
-        this.nodePath = nodePath;
-        this.lineNumber = lineNumber;
-        this.returnValue = returnValue;
-    }
-
-    @Override
-    public T answer(InvocationOnMock invocationOnMock) {
-        Map nodePathsAndLineNumbers = invocationOnMock.getArgument(3, Map.class);
-        nodePathsAndLineNumbers.put(nodePath, lineNumber);
-        return returnValue;
-    }
-}
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 6f97e5df..bfb585f5 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
@@ -199,15 +199,15 @@ public class ValidationExecutorTest {
     }
 
     /*
-     * JCRVLT-637: verify that when the DocumentViewXmlValidator returns the binary file as nodePath with lineNumber 0 to the
-     * ValidationExecutor, {@code isDocViewXml} is set to false
+     * JCRVLT-637: verify that when the DocumentViewXmlValidator returns the binary file to the
+     * ValidationExecutor (i.e. without modifying {@code nodePathsAndLineNumbers}), {@code isDocViewXml} is set to {@code false}
      */
     @Test
     public void testGenericJcrDataWithBinaryFileDetected()
         throws URISyntaxException, IOException, SAXException, ParserConfigurationException, ConfigurationException {
         Mockito.when(genericJcrDataValidator.shouldValidateJcrData(Mockito.any(), Mockito.any())).thenReturn(true);
-        ReturnNodeAndLineNumberAnswer<Collection<ValidationMessage>> answer = new ReturnNodeAndLineNumberAnswer<>("/apps/genericfile.xml", 0);
-        Mockito.when(genericJcrDataValidator.validateJcrData(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenAnswer(answer);
+        // must not modify the 4th parameter
+        Mockito.when(genericJcrDataValidator.validateJcrData(Mockito.any(), Mockito.any(), Mockito.any(), Mockito.any())).thenReturn(Collections.emptyList());
         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, AnyValidationViolationMessageMatcher.noValidationViolationMessageInCollection());