feat: allow some relation nodes to not define key/unique properties

This commit relaxes validation `NKRN-001`, which, until now,
required that every node target, referenced by a relationship,
defines at least one key or unique constraint.

The validation now allows such node targets to not define any key
or unique constraints themselves, if and only if another node covers
some of its labels and properties.

If the node reference overrides the mappings for key properties,
all these properties must be matched by key or unique constraints
defined elsewhere for any of the node labels.

Signed-off-by: Florent Biville <[email protected]>

Author: nord-erik

core/src/main/java/org/neo4j/importer/v1/pipeline/ImportExecutionPlan.java

@@ -25,7 +25,7 @@
 import java.util.Objects;
 import java.util.Set;
 import java.util.stream.Collectors;
-import org.neo4j.importer.v1.graph.Graphs;
+import org.neo4j.importer.v1.util.graph.Graphs;
 
 /**
  * Represents the entire parallelizable execution plan for an import step graph.

core/src/main/java/org/neo4j/importer/v1/pipeline/ImportPipeline.java

@@ -36,7 +36,6 @@
 import org.neo4j.importer.v1.ImportSpecification;
 import org.neo4j.importer.v1.actions.Action;
 import org.neo4j.importer.v1.actions.ActionStage;
-import org.neo4j.importer.v1.graph.Graphs;
 import org.neo4j.importer.v1.sources.Source;
 import org.neo4j.importer.v1.targets.CustomQueryTarget;
 import org.neo4j.importer.v1.targets.KeyMapping;
@@ -45,6 +44,7 @@
 import org.neo4j.importer.v1.targets.RelationshipTarget;
 import org.neo4j.importer.v1.targets.Target;
 import org.neo4j.importer.v1.targets.Targets;
+import org.neo4j.importer.v1.util.graph.Graphs;
 
 /**
  * {@link ImportPipeline} exposes a topologically-ordered set of {@link ImportStep},

core/src/main/java/org/neo4j/importer/v1/util/collections/Sets.java

@@ -0,0 +1,103 @@
+/*
+ * Copyright (c) "Neo4j"
+ * Neo4j Sweden AB [https://neo4j.com]
+ *
+ * Licensed 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.neo4j.importer.v1.util.collections;
+
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.Set;
+import java.util.stream.Collectors;
+import java.util.stream.IntStream;
+import java.util.stream.Stream;
+
+public class Sets {
+
+    /**
+     * Returns all elements of the first set not contained in set2
+     * @param set1 first set
+     * @param set2 second set
+     * @return elements of set1 not in set2
+     * @param <T> type of elements
+     */
+    public static <T> Set<T> difference(Set<T> set1, Set<T> set2) {
+        return set1.stream().filter(element -> !set2.contains(element)).collect(Collectors.toSet());
+    }
+
+    /**
+     * @see Sets#generateNonEmptySubsets(Set, Comparator)
+     */
+    public static <T> Stream<Set<T>> generateNonEmptySubsets(Set<T> inputSet) {
+        return generateNonEmptySubsets(inputSet, null);
+    }
+
+    /**
+     * This generates a {@code Stream} of all non-empty subsets of the provided input set.
+     * <p>
+     * The method optionally accepts a {@code Comparator<Set<T>>} to sort the resulting
+     * stream of subsets. If the comparator is {@code null}, the subsets are returned
+     * in the order they are generated.
+     * <p>
+     * This method has an exponential time complexity of O(n * 2^n)**, where n is the size
+     * of the input set. Please call this sparingly.
+     *
+     * @param <T> type of inputs.
+     * @param inputSet inputs
+     * @param order optional {@code Comparator<Set<T>>} order to sort subsets with. If {@code null}, no ordering is applied.
+     * @return {@code Stream<Set<T>>} containing all 2^n - 1 non-empty subsets, optionally ordered.
+     */
+    public static <T> Stream<Set<T>> generateNonEmptySubsets(Set<T> inputSet, Comparator<Set<T>> order) {
+        var elements = new ArrayList<>(inputSet);
+        var n = elements.size();
+        if (n == 0) {
+            return Stream.empty();
+        }
+
+        Stream<Set<T>> result = IntStream.range(1, 1 << n /* 2^n unordered subsets */)
+                .mapToObj(subsetIndex -> IntStream.range(0, n)
+                        /*
+                         * The bitwise subset generation method maps each element in the input list
+                         * to a specific bit position (inputIndex) and uses an integer 'subsetIndex' to
+                         * determine which elements to include.
+                         *
+                         * Given inputSet = ["Apple" (inputIndex=0),
+                         *                   "Banana" (inputIndex=1),
+                         *                   "Cherry" (inputIndex=2)],
+                         * the generation loop iterates through 'subsetIndex' from 1 to 7 (2^3 - 1).
+                         *
+                         * The filter condition is: {@code (subsetIndex & (1 << inputIndex)) != 0}
+                         *
+                         * ----------------------------------------------------------------------------------------------------------------------------
+                         * | subsetIndex | Binary | Bits Set            | inputIndex=0   | inputIndex=1 | inputIndex=2 | Subset
+                         * | --------------------------------------------------------------------------------------------------------------------------
+                         * | 1          | 001    | inputIndex=0         | YES            | NO           | NO           | {"Apple"}
+                         * | 2          | 010    | inputIndex=1         | NO             | YES          | NO           | {"Banana"}
+                         * | 3          | 011    | inputIndex=0, 1      | YES            | YES          | NO           | {"Apple", "Banana"}
+                         * | 4          | 100    | inputIndex=2         | NO             | NO           | YES          | {"Cherry"}
+                         * | 5          | 101    | inputIndex=0, 2      | YES            | NO           | YES          | {"Apple", "Cherry"}
+                         * | 6          | 110    | inputIndex=1, 2      | NO             | YES          | YES          | {"Banana", "Cherry"}
+                         * | 7          | 111    | inputIndex=0, 1, 2   | YES            | YES          | YES          | {"Apple", "Banana", "Cherry"}
+                         * ----------------------------------------------------------------------------------------------------------------------------
+                         */
+                        .filter(inputIndex -> (subsetIndex & (1 << inputIndex)) != 0)
+                        .mapToObj(elements::get)
+                        .collect(Collectors.toSet()));
+
+        if (order != null) {
+            return result.sorted(order);
+        }
+        return result;
+    }
+}

core/src/main/java/org/neo4j/importer/v1/graph/Graphs.java renamed to core/src/main/java/org/neo4j/importer/v1/util/graph/Graphs.java

@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.neo4j.importer.v1.graph;
+package org.neo4j.importer.v1.util.graph;
 
 import java.util.ArrayDeque;
 import java.util.ArrayList;

core/src/main/java/org/neo4j/importer/v1/graph/Pair.java renamed to core/src/main/java/org/neo4j/importer/v1/util/graph/Pair.java

@@ -14,7 +14,7 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-package org.neo4j.importer.v1.graph;
+package org.neo4j.importer.v1.util.graph;
 
 import java.util.Objects;
 

core/src/main/java/org/neo4j/importer/v1/validation/SpecificationValidators.java

@@ -27,7 +27,7 @@
 import java.util.function.Function;
 import java.util.stream.Collectors;
 import org.neo4j.importer.v1.ImportSpecification;
-import org.neo4j.importer.v1.graph.Graphs;
+import org.neo4j.importer.v1.util.graph.Graphs;
 
 public class SpecificationValidators {
 

core/src/main/java/org/neo4j/importer/v1/validation/plugin/NoDanglingDependsOnValidator.java

@@ -22,11 +22,11 @@
 import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
-import org.neo4j.importer.v1.graph.Pair;
 import org.neo4j.importer.v1.targets.CustomQueryTarget;
 import org.neo4j.importer.v1.targets.NodeTarget;
 import org.neo4j.importer.v1.targets.RelationshipTarget;
 import org.neo4j.importer.v1.targets.Target;
+import org.neo4j.importer.v1.util.graph.Pair;
 import org.neo4j.importer.v1.validation.SpecificationValidationResult.Builder;
 import org.neo4j.importer.v1.validation.SpecificationValidator;
 

core/src/main/java/org/neo4j/importer/v1/validation/plugin/NoDependencyCycleValidator.java

@@ -26,13 +26,13 @@
 import java.util.Set;
 import java.util.concurrent.atomic.AtomicBoolean;
 import java.util.stream.Collectors;
-import org.neo4j.importer.v1.graph.Graphs;
-import org.neo4j.importer.v1.graph.Pair;
 import org.neo4j.importer.v1.targets.CustomQueryTarget;
 import org.neo4j.importer.v1.targets.NodeReference;
 import org.neo4j.importer.v1.targets.NodeTarget;
 import org.neo4j.importer.v1.targets.RelationshipTarget;
 import org.neo4j.importer.v1.targets.Target;
+import org.neo4j.importer.v1.util.graph.Graphs;
+import org.neo4j.importer.v1.util.graph.Pair;
 import org.neo4j.importer.v1.validation.SpecificationValidationResult.Builder;
 import org.neo4j.importer.v1.validation.SpecificationValidator;
 

core/src/main/java/org/neo4j/importer/v1/validation/plugin/NoKeylessRelationshipNodeValidator.java

@@ -16,19 +16,31 @@
  */
 package org.neo4j.importer.v1.validation.plugin;
 
+import java.util.HashMap;
 import java.util.HashSet;
 import java.util.LinkedHashMap;
+import java.util.List;
 import java.util.Map;
+import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
+import org.neo4j.importer.v1.targets.KeyMapping;
+import org.neo4j.importer.v1.targets.NodeReference;
 import org.neo4j.importer.v1.targets.NodeTarget;
+import org.neo4j.importer.v1.targets.PropertyMapping;
 import org.neo4j.importer.v1.targets.RelationshipTarget;
+import org.neo4j.importer.v1.util.collections.Sets;
 import org.neo4j.importer.v1.validation.SpecificationValidationResult.Builder;
 import org.neo4j.importer.v1.validation.SpecificationValidator;
 
 public class NoKeylessRelationshipNodeValidator implements SpecificationValidator {
     private static final String ERROR_CODE = "NKRN-001";
 
-    private final Set<String> keylessNodes = new HashSet<>();
+    private final Map<String, NodeShape> visitedNodeShapes = new HashMap<>();
+
+    private final Set<String> targetsWithoutConstraint = new HashSet<>();
+
+    private final Set<NodePattern> nodePatternsWithConstraints = new HashSet<>();
 
     private final Map<String, String> errorMessages = new LinkedHashMap<>();
 
@@ -44,28 +56,25 @@ public Set<Class<? extends SpecificationValidator>> requires() {
 
     @Override
     public void visitNodeTarget(int index, NodeTarget target) {
+        visitedNodeShapes.put(target.getName(), new NodeShape(target));
         var schema = target.getSchema();
-        if (schema.getKeyConstraints().isEmpty()
-                && schema.getUniqueConstraints().isEmpty()) {
-            keylessNodes.add(target.getName());
+        var keyConstraints = schema.getKeyConstraints();
+        var uniqueConstraints = schema.getUniqueConstraints();
+        if (keyConstraints.isEmpty() && uniqueConstraints.isEmpty()) {
+            targetsWithoutConstraint.add(target.getName());
+        } else {
+            keyConstraints.forEach(constraint -> nodePatternsWithConstraints.add(
+                    new NodePattern(constraint.getLabel(), new HashSet<>(constraint.getProperties()))));
+            uniqueConstraints.forEach(constraint -> nodePatternsWithConstraints.add(
+                    new NodePattern(constraint.getLabel(), new HashSet<>(constraint.getProperties()))));
         }
     }
 
     public void visitRelationshipTarget(int index, RelationshipTarget target) {
-        var startNode = target.getStartNodeReference().getName();
-        if (keylessNodes.contains(startNode)) {
-            errorMessages.put(
-                    String.format("$.targets.relationships[%d].start_node_reference", index),
-                    String.format(
-                            "Node %s must define a key or unique constraint for property id, none found", startNode));
-        }
-        var endNode = target.getEndNodeReference().getName();
-        if (keylessNodes.contains(endNode)) {
-            errorMessages.put(
-                    String.format("$.targets.relationships[%d].end_node_reference", index),
-                    String.format(
-                            "Node %s must define a key or unique constraint for property id, none found", endNode));
-        }
+        checkNode(
+                String.format("$.targets.relationships[%d].start_node_reference", index),
+                target.getStartNodeReference());
+        checkNode(String.format("$.targets.relationships[%d].end_node_reference", index), target.getEndNodeReference());
     }
 
     @Override
@@ -75,4 +84,118 @@ public boolean report(Builder builder) {
         });
         return !errorMessages.isEmpty();
     }
+
+    private void checkNode(String path, NodeReference nodeReference) {
+        var nodeName = nodeReference.getName();
+        if (!targetsWithoutConstraint.contains(nodeName)) {
+            return;
+        }
+        // match call can be costly, only run it for node targets without any key/unique constraints
+        var keyMappings = nodeReference.getKeyMappings();
+        var matchResult = visitedNodeShapes.get(nodeName).match(nodePatternsWithConstraints, keyMappings);
+        if (!matchResult.matches()) {
+            var unmatchedList = matchResult.unmatchedProperties().stream()
+                    .map(Object::toString)
+                    .collect(Collectors.joining("','", "'", "'"));
+            errorMessages.put(
+                    path,
+                    String.format(
+                            "Node '%s' must define key or unique constraints for %s the properties (%s)",
+                            nodeName, keyMappings.isEmpty() ? "any of" : "all of", unmatchedList));
+        }
+    }
+
+    private static final class NodeShape {
+        private final List<String> labels;
+        private final List<String> properties;
+
+        public NodeShape(NodeTarget target) {
+            this.labels = target.getLabels();
+            this.properties = target.getProperties().stream()
+                    .map(PropertyMapping::getTargetProperty)
+                    .collect(Collectors.toList());
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (!(o instanceof NodeShape)) return false;
+            NodeShape that = (NodeShape) o;
+            return Objects.equals(labels, that.labels) && Objects.equals(properties, that.properties);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(labels, properties);
+        }
+
+        @Override
+        public String toString() {
+            return "NodeTargetShape{" + "labels=" + labels + ", properties=" + properties + '}';
+        }
+
+        public PropertyMatchResult match(Set<NodePattern> patterns, List<KeyMapping> keyMappings) {
+            var allProperties = new HashSet<>(this.properties);
+            var keys = keyMappings.stream().map(KeyMapping::getNodeProperty).collect(Collectors.toSet());
+            // if no known key properties, we need to check all the non-empty subsets from the node properties
+            var coveredProperties = Sets.generateNonEmptySubsets(keys.isEmpty() ? allProperties : keys)
+                    .flatMap(combination -> this.labels.stream().map(label -> new NodePattern(label, combination)))
+                    .filter(patterns::contains)
+                    // all key properties must match a key/unique constraint pattern
+                    // (individually or as part of a larger subset)
+                    .flatMap(pattern -> pattern.properties.stream())
+                    .collect(Collectors.toSet());
+            if (keys.isEmpty()) {
+                var success = !coveredProperties.isEmpty();
+                return new PropertyMatchResult(success, success ? Set.of() : allProperties);
+            }
+            var uncoveredKeys = Sets.difference(keys, coveredProperties);
+            return new PropertyMatchResult(uncoveredKeys.isEmpty(), uncoveredKeys);
+        }
+    }
+
+    private static final class PropertyMatchResult {
+        private final boolean matches;
+        private final Set<String> unmatchedProperties;
+
+        public PropertyMatchResult(boolean matches, Set<String> unmatchedProperties) {
+            this.matches = matches;
+            this.unmatchedProperties = unmatchedProperties;
+        }
+
+        public boolean matches() {
+            return matches;
+        }
+
+        public List<String> unmatchedProperties() {
+            return unmatchedProperties.stream().sorted().collect(Collectors.toList());
+        }
+    }
+
+    private static final class NodePattern {
+
+        private final String label;
+        private final Set<String> properties;
+
+        public NodePattern(String label, Set<String> properties) {
+            this.label = label;
+            this.properties = properties;
+        }
+
+        @Override
+        public boolean equals(Object o) {
+            if (!(o instanceof NodePattern)) return false;
+            NodePattern that = (NodePattern) o;
+            return Objects.equals(label, that.label) && Objects.equals(properties, that.properties);
+        }
+
+        @Override
+        public int hashCode() {
+            return Objects.hash(label, properties);
+        }
+
+        @Override
+        public String toString() {
+            return "NodePattern{" + "label='" + label + '\'' + ", properties=" + properties + '}';
+        }
+    }
 }