AMashenkov commented on code in PR #6276:
URL: https://github.com/apache/ignite-3/pull/6276#discussion_r2218914380


##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparator.java:
##########
@@ -232,12 +230,444 @@ public static ComparisonContext 
create(Set<ConfigurationModule> configurationMod
 
         private final KeyIgnorer deletedItems;
 
+        private final Set<String> skipAddRemoveKeys = new HashSet<>();
+
         ComparisonContext(Collection<String> deletedPrefixes) {
             this.deletedItems = 
KeyIgnorer.fromDeletedPrefixes(deletedPrefixes);
         }
 
         boolean shouldIgnore(String path) {
             return deletedItems.shouldIgnore(path);
         }
+
+        boolean shouldIgnore(ConfigNode node, String childName) {
+            return shouldIgnore(node.path() + "." + childName);
+        }
+
+        boolean ignoreAddOrRemove(String path) {
+            return skipAddRemoveKeys.contains(path);
+        }
+
+        boolean ignoreAddOrRemove(ConfigNode node, String childName) {
+            return ignoreAddOrRemove(node.path() + "." + childName);
+        }
+    }
+
+    private static void validateConfigNode(
+            @Nullable String instanceType,
+            ConfigNode original,
+            ConfigNode updated,
+            ComparisonContext context,
+            Errors errors
+    ) {
+        errors.push(instanceType);
+        doValidateConfigNode(original, updated, context, errors);
+        errors.pop();
+    }
+
+    private static void validateNewConfigNode(
+            @Nullable String instanceType,
+            ConfigNode updated,
+            ComparisonContext compContext,
+            Errors errors
+    ) {
+        if (compContext.shouldIgnore(updated.path())) {
+            return;
+        }
+        errors.push(instanceType);
+
+        for (Entry<String, Node> e : updated.children().entrySet()) {
+            Node childNode = e.getValue();
+            if (childNode.isValue() && !childNode.hasDefault()) {
+                errors.addChildError(updated, e.getKey(), "Added a node with 
no default value");
+            }
+
+            if (childNode.isPolymorphic()) {
+                for (Map.Entry<String, ConfigNode> p : 
childNode.nodes().entrySet()) {
+                    ConfigNode node = p.getValue();
+                    validateNewConfigNode(p.getKey(), node, compContext, 
errors);
+                }
+            } else {
+                ConfigNode node = childNode.node();
+                validateNewConfigNode(null, node, compContext, errors);
+            }
+        }
+
+        errors.pop();
+    }
+
+    private static void doValidateConfigNode(ConfigNode original, ConfigNode 
updated, ComparisonContext context, Errors errors) {
+        if (context.shouldIgnore(original.path())) {
+            return;
+        }
+
+        if (!match(original, updated)) {
+            errors.addError(original, "Node does not match. Previous: " + 
original + ". Current: " + updated);
+            return;
+        }
+
+        validateAnnotations(original, updated, errors);
+
+        Set<String> originalChildrenNames = original.children().keySet();
+        Set<String> updatedChildrenNames = updated.children().keySet();
+
+        // Check for removed nodes
+        for (String childName : originalChildrenNames) {
+            if (updatedChildrenNames.contains(childName)) {
+                continue;
+            }
+
+            String path = original.path() + "." + childName;
+            if (context.shouldIgnore(path)) {
+                continue;
+            }
+
+            if (updatedChildrenNames.stream().noneMatch(name -> {
+                Node node = updated.children().get(name);
+                return node != null && 
node.legacyPropertyNames().contains(childName);
+            })) {
+                if (context.ignoreAddOrRemove(original, childName)) {
+                    continue;
+                }
+                errors.addChildError(original, childName, "Node was removed");
+            }
+        }
+
+        validateChildren(original, updated, context, errors);
+    }
+
+    private static void validateChildren(
+            ConfigNode original,
+            ConfigNode updated,
+            ComparisonContext context,
+            Errors errors
+    ) {
+        for (Entry<String, Node> e : original.children().entrySet()) {
+            String childName = e.getKey();
+            // Removed noded have already been handled
+            if (!updated.children().containsKey(childName)) {
+                continue;
+            }
+
+            Node originalChild = e.getValue();
+            Node updatedChild = updated.children().get(childName);
+
+            validateChildNode(original, childName, originalChild, 
updatedChild, context, errors);
+        }
+
+        Set<String> originalChildrenNames = original.children().keySet();
+
+        // Validate new nodes
+        for (Entry<String, Node> e : updated.children().entrySet()) {
+            String childName = e.getKey();
+            Node childNode = updated.children().get(childName);
+
+            if (context.shouldIgnore(original, childName)) {
+                continue;
+            }
+
+            // This child exists in original node under this name or under one 
of its legacy names.
+            boolean existingChildNode = 
originalChildrenNames.contains(childName)
+                    || 
childNode.legacyPropertyNames().stream().anyMatch(originalChildrenNames::contains);
+
+            if (existingChildNode) {
+                continue;
+            }
+
+            if (!childNode.hasDefault() && childNode.isValue() && 
!context.ignoreAddOrRemove(original, childName)) {
+                errors.addChildError(original, childName, "Added a node with 
no default value");
+            }
+        }
+    }
+
+    private static void validateChildNode(
+            ConfigNode original,
+            String childName,
+            Node originalChild,
+            Node updatedChild,
+            ComparisonContext context,
+            Errors errors
+    ) {
+        if (!originalChild.isPolymorphic() && !updatedChild.isPolymorphic()) {
+            // Both are single nodes, recursively check
+            validateConfigNode(null, originalChild.node(), 
updatedChild.node(), context, errors);
+        } else if (originalChild.isPolymorphic() != 
updatedChild.isPolymorphic()) {
+            // Check if node type changed (single vs polymorphic)
+
+            // If changing from single to polymorphic, check if it's a legal 
conversion
+            if (!originalChild.isPolymorphic() && 
updatedChild.isPolymorphic()) {
+                validateSingleToPolymorphic(originalChild, updatedChild, 
context, errors);
+            } else {
+                validatePolymorphicToSingle(original, errors, context, 
childName, originalChild, updatedChild);
+            }
+        } else {
+            assert originalChild.isPolymorphic() && 
updatedChild.isPolymorphic() : "Expected poly vs poly";
+
+            validatePolymorphicToPolymorphic(original, context, errors, 
childName, originalChild, updatedChild);
+        }
+    }
+
+    private static void validateSingleToPolymorphic(
+            Node originalChild,
+            Node updatedChild,
+            ComparisonContext context,
+            Errors errors
+    ) {
+        ConfigNode singleNode = originalChild.node();
+        Map<String, ConfigNode> polymorphicNodes = updatedChild.nodes();
+
+        // Transaction from a single node to polymorphic node should be 
compatible
+        // as long as new nodes are added in compatible fashion. 
+
+        for (Entry<String, ConfigNode> e : polymorphicNodes.entrySet()) {
+            errors.push(e.getKey());
+            validateChildren(singleNode, e.getValue(), context, errors);
+            errors.pop();
+        }
+    }
+
+    private static void validatePolymorphicToSingle(
+            ConfigNode original,
+            Errors errors,
+            ComparisonContext comparisonContext,
+            String childName,
+            Node originalChild,
+            Node updatedChild
+    ) {
+        Map<String, ConfigNode> polymorphicNode = originalChild.nodes();
+
+        // Converting a polymorphic node with 1 subclass can be compatible
+        if (polymorphicNode.size() == 2) {

Review Comment:
   ```suggestion
           if (polymorphicNode.size() == 1) {
   ```



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to