Update index definition naming validation (#34)

Adobe updated the policy in
https://experienceleague.adobe.com/en/docs/experience-manager-cloud-service/content/operations/indexing#preparing-the-new-index-definition

Add unit tests

This closes #33

README.md

@@ -73,13 +73,13 @@ Currently only Oak index definitions of type `lucene` are supported in AEMaaCS.
 
 ## Follow naming policy for Oak index definition node names
 
-There is a mandatory naming policy for Oak index definition node names which enforces them to end with `-custom-<version-as-integer>`. The format is used in [`IndexName`](https://github.com/apache/jackrabbit-oak/blob/08c7b20e0676739d9c445b5249c3f71004b6b894/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/IndexName.java#L36) and allows for upgrades of existing index definitions in blue/green deployments.
+There is a mandatory naming policy for Oak index definition node names which enforces them to either comply with pattern `<indexName>-<productVersion>-custom-<customVersion>` (customized OOTB index) or `<prefix>.<indexName>-<productVersion>-custom-<customVersion>` (fully customized index). The format is used in [`IndexName`](https://github.com/apache/jackrabbit-oak/blob/08c7b20e0676739d9c445b5249c3f71004b6b894/oak-search/src/main/java/org/apache/jackrabbit/oak/plugins/index/search/spi/query/IndexName.java#L36) and allows for upgrades of existing index definitions in blue/green deployments.
 
-Further details in <https://experienceleague.adobe.com/docs/experience-manager-cloud-service/operations/indexing.html?lang=en#changes-in-aem-as-a-cloud-service>.
+Further details in <https://experienceleague.adobe.com/en/docs/experience-manager-cloud-service/content/operations/indexing#preparing-the-new-index-definition>.
 
 # Usage with Maven
 
-You can use this validator with the [FileVault Package Maven Plugin][3] in version 1.1.0 or higher like this
+You can use this validator with the [FileVault Package Maven Plugin][3] in version 1.4.0 or higher like this
 
 ```
 <plugin>

pom.xml

@@ -51,6 +51,7 @@
 
     <properties>
         <maven.version>3.5.4</maven.version>
+        <jackrabbit.version>2.20.8</jackrabbit.version><!-- minimum Jackrabbit API version supported by the referenced filevault artifact -->
         <java.target.version>8</java.target.version> <!-- used for compiler plugin, javadoc plugin and animal-sniffer, for compatibility reasons with Java 9 only the values 6,7,8 and 9 is allowed -->
         <maven.compiler.release>${java.target.version}</maven.compiler.release>
         <project.build.sourceEncoding>UTF-8</project.build.sourceEncoding>
@@ -61,7 +62,7 @@
         <dependency>
             <groupId>org.apache.jackrabbit.vault</groupId>
             <artifactId>vault-validation</artifactId>
-            <version>3.4.2</version>
+            <version>3.8.2</version>
         </dependency>
         <dependency>
             <groupId>org.slf4j</groupId>
@@ -101,18 +102,22 @@
             <scope>test</scope>
         </dependency>
         <!-- only transitive dependencies of 'vault-validation' but must be declared due to https://issues.apache.org/jira/browse/JCRVLT-394 -->
+        <dependency>
+          <groupId>org.apache.jackrabbit</groupId>
+          <artifactId>jackrabbit-spi-commons</artifactId>
+          <version>${jackrabbit.version}</version>
+        </dependency>
+        <dependency>
+          <groupId>org.apache.jackrabbit</groupId>
+          <artifactId>jackrabbit-jcr-commons</artifactId>
+          <version>${jackrabbit.version}</version>
+        </dependency>
         <dependency>
             <groupId>javax.jcr</groupId>
             <artifactId>jcr</artifactId>
             <version>2.0</version>
             <scope>test</scope>
         </dependency>
-        <dependency>
-            <groupId>org.apache.jackrabbit</groupId>
-            <artifactId>jackrabbit-jcr-commons</artifactId>
-            <version>2.20.0</version>
-            <scope>test</scope>
-        </dependency>
         <dependency>
             <groupId>org.apache.jackrabbit</groupId>
             <artifactId>oak-jackrabbit-api</artifactId>

src/main/java/biz/netcentric/filevault/validator/aem/cloud/AemCloudValidator.java

@@ -22,12 +22,16 @@
 import java.util.Collections;
 import java.util.regex.Pattern;
 
+import org.apache.jackrabbit.spi.Name;
+import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
+import org.apache.jackrabbit.util.Text;
 import org.apache.jackrabbit.vault.packaging.PackageProperties;
 import org.apache.jackrabbit.vault.packaging.PackageType;
 import org.apache.jackrabbit.vault.util.Constants;
-import org.apache.jackrabbit.vault.util.DocViewNode;
+import org.apache.jackrabbit.vault.util.DocViewNode2;
 import org.apache.jackrabbit.vault.validation.spi.DocumentViewXmlValidator;
 import org.apache.jackrabbit.vault.validation.spi.MetaInfPathValidator;
+import org.apache.jackrabbit.vault.validation.spi.NodeContext;
 import org.apache.jackrabbit.vault.validation.spi.NodePathValidator;
 import org.apache.jackrabbit.vault.validation.spi.PropertiesValidator;
 import org.apache.jackrabbit.vault.validation.spi.ValidationContext;
@@ -40,15 +44,19 @@ public class AemCloudValidator implements NodePathValidator, MetaInfPathValidato
 
     static final String VIOLATION_MESSAGE_READONLY_MUTABLE_PATH = "Using mutable nodes in this repository location is only allowed in author-specific packages as it is not writable by the underlying service user on a publish instance. Consider to use repoinit scripts instead or move that content to another location. Further details at https://experienceleague.adobe.com/docs/experience-manager-learn/cloud-service/debugging/debugging-aem-as-a-cloud-service/build-and-deployment.html?lang=en#including-%2Fvar-in-content-package";
     static final String VIOLATION_MESSAGE_INSTALL_HOOK_IN_MUTABLE_PACKAGE = "Using install hooks in mutable content packages leads to deployment failures as the underlying service user on the publish does not have the right to execute those.";
-    static final String VIOLATION_MESSAGE_INVALID_INDEX_DEFINITION_NODE_NAME = "All Oak index definition node names must end with '-custom-<integer>' but found name '%s'. Further details at https://experienceleague.adobe.com/docs/experience-manager-cloud-service/operations/indexing.html?lang=en#how-to-use";
+    static final String VIOLATION_MESSAGE_INVALID_INDEX_DEFINITION_NODE_NAME = "All Oak index definition node names must follow scheme '<indexName>-<productVersion>-custom-<customVersion>' (for a customized OOTB index) or '<prefix>.<indexName>-<productVersion>-custom-<customVersion>' (for a fully customized index) but found name '%s'. Further details at https://experienceleague.adobe.com/en/docs/experience-manager-cloud-service/content/operations/indexing#preparing-the-new-index-definition";
     static final String VIOLATION_MESSAGE_LIBS_NODES = "Nodes below '/libs' may be overwritten by future product upgrades. Rather use '/apps'. Further details at https://experienceleague.adobe.com/docs/experience-manager-cloud-service/implementing/developing/full-stack/overlays.html?lang=en#developing";
     static final String VIOLATION_MESSAGE_MUTABLE_NODES_IN_MIXED_PACKAGE = "Mutable nodes in mixed package types are not installed!";
     static final String VIOLATION_MESSAGE_MUTABLE_NODES_AND_IMMUTABLE_NODES_IN_SAME_PACKAGE = "Mutable and immutable nodes must not be mixed in the same package. You must separate those into two packages and give them both a dedicated package type!";
     static final String VIOLATION_MESSAGE_NON_LUCENE_TYPE_INDEX_DEFINITION = "Only oak:QueryIndexDefinitions of type='lucene' are supported in AEMaaCS but found type='%s'. Compare with https://experienceleague.adobe.com/docs/experience-manager-cloud-service/operations/indexing.html?lang=en#changes-in-aem-as-a-cloud-service";
 
     // this path is relative to META-INF
     private static final Path INSTALL_HOOK_PATH = Paths.get(Constants.VAULT_DIR, Constants.HOOKS_DIR);
-    private static final Pattern INDEX_DEFINITION_NAME_PATTERN = Pattern.compile(".*-custom-\\d++");
+    /**
+     * The allowed patterns are defined in https://experienceleague.adobe.com/en/docs/experience-manager-cloud-service/content/operations/indexing#preparing-the-new-index-definition
+     */
+    private static final Pattern INDEX_DEFINITION_NAME_PATTERN = Pattern.compile(".*-\\d++-custom-\\d++");
+
     private static final Collection<String> IMMUTABLE_PATH_PREFIXES = Arrays.asList("/apps", "/libs", "/oak:index");
     private static final Collection<String> WRITABLE_PATHS_BY_DISTRIBUTION_IMPORTER = Arrays.asList(
             "/content",     // access provided by system user content-writer-service and sling-distribution-importer
@@ -69,6 +77,7 @@ public class AemCloudValidator implements NodePathValidator, MetaInfPathValidato
     private boolean hasImmutableNodes;
 
     private static final int MAX_NUM_VIOLATIONS_PER_TYPE = 5;
+    private static final Name PN_TYPE = NameFactoryImpl.getInstance().create(Name.NS_DEFAULT_URI, "type");
     private int numVarNodeViolations = 0;
     private int numLibNodeViolations = 0;
     private int numMutableNodeViolations = 0;
@@ -196,19 +205,19 @@ static boolean isPackagePathInstalledConditionally(String runMode, Path packageR
     }
 
     @Override
-    public @Nullable Collection<ValidationMessage> validate(@NotNull DocViewNode node, @NotNull String nodePath,
-            @NotNull Path filePath, boolean isRoot) {
-        if ("oak:QueryIndexDefinition".equals(node.primary)) {
+    public @Nullable Collection<ValidationMessage> validate(@NotNull DocViewNode2 node, @NotNull NodeContext nodeContext, boolean isRoot) {
+        if ("oak:QueryIndexDefinition".equals(node.getPrimaryType().orElse(""))) {
             Collection<ValidationMessage> messages = new ArrayList<>();
-            String indexType = node.getValue("{}type");
+            String indexType = node.getPropertyValue(PN_TYPE).orElse("");
             if (!"lucene".equals(indexType)) {
                 messages.add(new ValidationMessage(defaultSeverity,
                         String.format(VIOLATION_MESSAGE_NON_LUCENE_TYPE_INDEX_DEFINITION, indexType)));
             }
-            // check node name
-            if (!INDEX_DEFINITION_NAME_PATTERN.matcher(node.name).matches()) {
+            // check node name (jcr qualified name as contained in the path)
+            String qualifiedName = Text.getName(nodeContext.getNodePath());
+            if (!INDEX_DEFINITION_NAME_PATTERN.matcher(qualifiedName).matches()) {
                 messages.add(new ValidationMessage(defaultSeverity,
-                        String.format(VIOLATION_MESSAGE_INVALID_INDEX_DEFINITION_NODE_NAME, node.name)));
+                        String.format(VIOLATION_MESSAGE_INVALID_INDEX_DEFINITION_NODE_NAME, qualifiedName)));
             }
             return messages;
         }

src/test/java/biz/netcentric/filevault/validator/aem/cloud/AemCloudValidatorTest.java

@@ -15,16 +15,25 @@
 
 import java.nio.file.Paths;
 import java.util.ArrayList;
+import java.util.Arrays;
 import java.util.Collection;
+import java.util.List;
 import java.util.Optional;
 
+import org.apache.jackrabbit.spi.Name;
+import org.apache.jackrabbit.spi.commons.name.NameConstants;
+import org.apache.jackrabbit.spi.commons.name.NameFactoryImpl;
 import org.apache.jackrabbit.vault.packaging.PackageType;
+import org.apache.jackrabbit.vault.util.DocViewNode2;
+import org.apache.jackrabbit.vault.util.DocViewProperty2;
+import org.apache.jackrabbit.vault.validation.spi.NodeContext;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessage;
 import org.apache.jackrabbit.vault.validation.spi.ValidationMessageSeverity;
+import org.apache.jackrabbit.vault.validation.spi.util.NodeContextImpl;
 import org.junit.jupiter.api.Assertions;
 import org.junit.jupiter.api.Test;
 
-import static biz.netcentric.filevault.validator.aem.cloud.AemCloudValidator.VIOLATION_MESSAGE_INSTALL_HOOK_IN_MUTABLE_PACKAGE;
+import static org.junit.jupiter.api.Assertions.assertEquals;
 
 class AemCloudValidatorTest {
 
@@ -75,7 +84,7 @@ void testAllowHooksInMutableContent() {
         Collection<ValidationMessage> messages = new ArrayList<>();
         Optional.ofNullable(validator.validateMetaInfPath(Paths.get("vault/hooks/install-hook.jar"))).ifPresent(messages::addAll);
         Optional.ofNullable(validator.done()).ifPresent(messages::addAll);
-        Assertions.assertTrue(messages.stream().anyMatch(message -> message.getMessage().equals(VIOLATION_MESSAGE_INSTALL_HOOK_IN_MUTABLE_PACKAGE)));
+        Assertions.assertTrue(messages.stream().anyMatch(message -> message.getMessage().equals(AemCloudValidator.VIOLATION_MESSAGE_INSTALL_HOOK_IN_MUTABLE_PACKAGE)));
 
         validator = new AemCloudValidator(true, false, true, PackageType.CONTENT, null, ValidationMessageSeverity.ERROR);
         messages = new ArrayList<>();
@@ -84,4 +93,49 @@ void testAllowHooksInMutableContent() {
         Assertions.assertTrue(messages.isEmpty());
     }
 
+    @Test
+    void testValidIndexDefinitions() {
+        AemCloudValidator validator = new AemCloudValidator(true, false, false, PackageType.CONTENT, null, ValidationMessageSeverity.ERROR);
+        Collection<ValidationMessage> messages = new ArrayList<>();
+        List<DocViewProperty2> properties = Arrays.asList(
+                new DocViewProperty2(NameConstants.JCR_PRIMARYTYPE, "oak:QueryIndexDefinition"),
+                new DocViewProperty2(NameFactoryImpl.getInstance().create(Name.NS_DEFAULT_URI, "type"), "lucene"));
+        // valid lucene index definition
+        NodeContext context = new NodeContextImpl("/oak:index/prefix.myindex-1-custom-1", Paths.get("_oak_index/test"),Paths.get("./jcr_root"));
+        DocViewNode2 node = new DocViewNode2(NameConstants.JCR_ROOT, properties);
+        Optional.ofNullable(validator.validate(node, context, true)).ifPresent(messages::addAll);
+        Assertions.assertTrue(messages.isEmpty());
+        context = new NodeContextImpl("/oak:index/productindex-1-custom-1", Paths.get("_oak_index/test"),Paths.get("./jcr_root"));
+        Optional.ofNullable(validator.validate(node, context, true)).ifPresent(messages::addAll);
+        Assertions.assertTrue(messages.isEmpty());
+    }
+
+    @Test
+    void testInvalidLuceneIndexDefinitions() {
+        AemCloudValidator validator = new AemCloudValidator(true, false, false, PackageType.CONTENT, null, ValidationMessageSeverity.ERROR);
+        Collection<ValidationMessage> messages = new ArrayList<>();
+        List<DocViewProperty2> properties = Arrays.asList(
+                new DocViewProperty2(NameConstants.JCR_PRIMARYTYPE, "oak:QueryIndexDefinition"),
+                new DocViewProperty2(NameFactoryImpl.getInstance().create(Name.NS_DEFAULT_URI, "type"), "lucene"));
+        NodeContext context = new NodeContextImpl("/oak:index/myindex", Paths.get("_oak_index/test"),Paths.get("./jcr_root"));
+        DocViewNode2 node = new DocViewNode2(NameConstants.JCR_ROOT, properties);
+        Optional.ofNullable(validator.validate(node, context, true)).ifPresent(messages::addAll);
+        assertEquals(1, messages.size());
+        assertEquals(String.format(AemCloudValidator.VIOLATION_MESSAGE_INVALID_INDEX_DEFINITION_NODE_NAME, "myindex"), messages.iterator().next().getMessage());
+    }
+
+    @Test
+    void testInvalidPropertyIndexDefinition() {
+        AemCloudValidator validator = new AemCloudValidator(true, false, false, PackageType.CONTENT, null, ValidationMessageSeverity.ERROR);
+        Collection<ValidationMessage> messages = new ArrayList<>();
+        List<DocViewProperty2> properties =  Arrays.asList(
+                new DocViewProperty2(NameConstants.JCR_PRIMARYTYPE, "oak:QueryIndexDefinition"),
+                new DocViewProperty2(NameFactoryImpl.getInstance().create(Name.NS_DEFAULT_URI, "type"), "property"));
+        // invalid property index definition
+        DocViewNode2 node = new DocViewNode2(NameConstants.JCR_ROOT, properties);
+        NodeContext context = new NodeContextImpl("/oak:index/prefix.myindex-1-custom-1", Paths.get("_oak_index/test"),Paths.get("./jcr_root"));
+        Optional.ofNullable(validator.validate(node, context, true)).ifPresent(messages::addAll);
+        assertEquals(1, messages.size());
+        assertEquals(String.format(AemCloudValidator.VIOLATION_MESSAGE_NON_LUCENE_TYPE_INDEX_DEFINITION, "property"), messages.iterator().next().getMessage());
+    }
 }