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


##########
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);
+            }
+        }

Review Comment:
   I'm not insist, but Visitor will be easier to understand.
   Can we add a visitor that call the Consumer for every ConfigNode instance?
   The same code could be reused for validating removed values.
   



-- 
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