zstan commented on code in PR #6276:
URL: https://github.com/apache/ignite-3/pull/6276#discussion_r2218307795
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNode.java:
##########
@@ -185,11 +193,17 @@ void addChildNodes(ConfigNode... childNodes) {
}
/**
- * Returns {@code true} if this node is a root node, {@code false}
otherwise.
+ * Add a polymorhic child node to this node.
Review Comment:
```suggestion
* Add a polymorphic child nodes to this node.
```
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparatorSelfTest.java:
##########
@@ -410,7 +448,7 @@ void testDeleteAfterRename() {
EnumSet.of(Flags.IS_ROOT));
ConfigNode node1Ver1 = new ConfigNode(root, Map.of(Attributes.NAME,
"oldTestCount"), List.of(),
- EnumSet.of(Flags.IS_VALUE));
+ EnumSet.of(Flags.IS_VALUE, Flags.HAS_DEFAULT));
Review Comment:
is it really need for ("oldTestCount" and "newTestCount") to have
Flags.HAS_DEFAULT ?
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparator.java:
##########
@@ -45,153 +55,141 @@ public static void ensureCompatible(
List<ConfigNode> actualTrees,
ComparisonContext compContext
) {
- LeafNodesVisitor shuttle = new LeafNodesVisitor(new
Validator(actualTrees), compContext);
-
- for (ConfigNode tree : snapshotTrees) {
- tree.accept(shuttle);
+ // Ensure that both collections include the same kinds
+ Map<String, List<ConfigNode>> snapshotByKind = new HashMap<>();
+ for (ConfigNode node : snapshotTrees) {
+ snapshotByKind.computeIfAbsent(node.kind(), (k) -> new
ArrayList<>()).add(node);
}
- }
- /**
- * Compares the configuration trees are equals by dumping their state to
string.
- */
- public static void compare(List<ConfigNode> tree1, List<ConfigNode> tree2)
{
- String dump1 = dumpTree(tree1);
- String dump2 = dumpTree(tree2);
- assertEquals(dump1, dump2, "Configuration metadata mismatch");
- }
-
- /**
- * Traverses the tree and triggers validation for leaf nodes.
- */
- private static class LeafNodesVisitor implements ConfigShuttle {
- private final Consumer<ConfigNode> validator;
- private final ComparisonContext compContext;
-
- private LeafNodesVisitor(Consumer<ConfigNode> validator,
ComparisonContext compContext) {
- this.validator = validator;
- this.compContext = compContext;
+ Map<String, List<ConfigNode>> actualByKind = new HashMap<>();
+ for (ConfigNode node : actualTrees) {
+ actualByKind.computeIfAbsent(node.kind(), (k) -> new
ArrayList<>()).add(node);
}
- @Override
- public void visit(ConfigNode node) {
- assert node.isRoot() || node.isInnerNode() || node.isNamedNode()
|| node.isValue();
-
- if (node.isValue() && !compContext.shouldIgnore(node.path())) {
- validator.accept(node);
- }
+ if (!snapshotByKind.keySet().equals(actualByKind.keySet())) {
+ String error = format(
+ "Configuration kind does not match. Expected {} but got
{}",
+ snapshotByKind.keySet(),
+ actualByKind.keySet()
+ );
+ throw new IllegalStateException(error);
Review Comment:
as i can see - this case is not covered by tests, if so - it need to be
covered.
##########
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,
Review Comment:
let`s use the same namings, as for example in "ensureCompatible" ? i.e.
snapshot and actual ? otherwise it confusing
##########
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) {
+ // Get a subclass
+ Map.Entry<String, ConfigNode> subclass =
polymorphicNode.entrySet().stream()
+ .filter(e -> !e.getKey().isEmpty())
+ .findFirst()
+ .orElseThrow();
+
+ // Validate whether a subclass is compatible with a single node
+ validateConfigNode(subclass.getKey(), subclass.getValue(),
updatedChild.node(), comparisonContext, errors);
+ } else {
+ // Converting multiple subclasses to single node is not compatible
+ errors.addChildError(original, childName, "Node was changed from
polymorphic-node to single-node");
+ }
+ }
+
+ private static void validatePolymorphicToPolymorphic(
+ ConfigNode original,
+ ComparisonContext context,
+ Errors errors,
+ String childName,
+ Node originalChild,
+ Node updatedChild
+ ) {
+ Map<String, ConfigNode> originalPolyNodes = originalChild.nodes();
+ Map<String, ConfigNode> updatedPolyNodes = updatedChild.nodes();
+
+ // All sub-fields per each sub-class : Original
+ Map<String, Set<String>> originalNodesChildren = new LinkedHashMap<>();
+ for (Entry<String, ConfigNode> e : originalPolyNodes.entrySet()) {
+ originalNodesChildren.computeIfAbsent(e.getKey(), k -> new
HashSet<>()).addAll(e.getValue().children().keySet());
+ }
+
+ // All sub-fields per each sub-class : Updated
+ Map<String, Set<String>> updatedNodesChildren = new LinkedHashMap<>();
+ for (Entry<String, ConfigNode> e : updatedPolyNodes.entrySet()) {
+ updatedNodesChildren.computeIfAbsent(e.getKey(), k -> new
HashSet<>()).addAll(e.getValue().children().keySet());
+ }
+
+ // No child properties has neither been added nor removed - check
compatibility of every node
+ if (originalNodesChildren.equals(updatedNodesChildren)) {
+ for (String key : originalNodesChildren.keySet()) {
+ ConfigNode originalVariant = originalPolyNodes.get(key);
+ ConfigNode updatedVariant = updatedPolyNodes.get(key);
+
+ validateConfigNode(key, originalVariant, updatedVariant,
context, errors);
+ }
+ } else {
+ // Build a map of changes per subclass
+ Map<String, List<ChildChange>> changes =
findChangesBetweenNodesChildren(updatedNodesChildren, originalNodesChildren);
+
+ Set<String> skipErrorKeys = new HashSet<>();
+
+ // Valid polymorphic-node -> polymorphic-node transformations:
+ // 1. Moving a child node from a base class to all subclasses iff
the child node has a default value
+ // 2. Moving a child node from a subclass to a base class iff the
child node has a default value.
Review Comment:
```suggestion
// 2. Moving a child node from a subclass to a base class if the
child node has a default value.
```
##########
modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparatorSelfTest.java:
##########
@@ -394,13 +408,37 @@ void testInCompatibleRename() {
root2.addChildNodes(newNode2);
assertIncompatible(oldNode1, newNode1);
+
+ // Incorrectly renaming leaf nodes.
+
+ root1 = createRoot("root1");
+ oldNode2 = createChild("oldTestCount");
+ root1.addChildNodes(oldNode2);
+
+ root2 = createRoot("root1");
+ newNode2 = createChild("newTestCount", Set.of(),
Set.of("oldTestCount_misspelled"), List.of());
+ root2.addChildNodes(newNode2);
+
+ assertIncompatible(root1, root2);
+
+ // Incorrectly renaming intermediate nodes.
+
+ root1 = createRoot("root1");
+ oldNode2 = createNode("node", "X");
+ oldNode2.addChildNodes(createChild("value"));
+ root1.addChildNodes(oldNode2);
+
+ root2 = createRoot("root1");
+ newNode2 = createNode("node_new", "X", Set.of("node_misspelled"),
List.of());
+ newNode2.addChildNodes(createChild("value"));
+ root2.addChildNodes(newNode2);
+
+ assertIncompatible(root1, root2);
}
/**
- * Test scenario. <br>
- * config ver1 has property : prop1 <br>
- * config ver2 has renamed property : prop1 -> prop2 <br>
- * config ver3 has deleted property : prop1, prop2 <br>
+ * Test scenario. <br> config ver1 has property : prop1 <br> config ver2
has renamed property : prop1 -> prop2 <br> config ver3 has
Review Comment:
such formatting is hard to read ( revert it plz
##########
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) {
+ // Get a subclass
+ Map.Entry<String, ConfigNode> subclass =
polymorphicNode.entrySet().stream()
+ .filter(e -> !e.getKey().isEmpty())
+ .findFirst()
+ .orElseThrow();
+
+ // Validate whether a subclass is compatible with a single node
+ validateConfigNode(subclass.getKey(), subclass.getValue(),
updatedChild.node(), comparisonContext, errors);
+ } else {
+ // Converting multiple subclasses to single node is not compatible
+ errors.addChildError(original, childName, "Node was changed from
polymorphic-node to single-node");
+ }
+ }
+
+ private static void validatePolymorphicToPolymorphic(
+ ConfigNode original,
+ ComparisonContext context,
+ Errors errors,
+ String childName,
+ Node originalChild,
+ Node updatedChild
+ ) {
+ Map<String, ConfigNode> originalPolyNodes = originalChild.nodes();
+ Map<String, ConfigNode> updatedPolyNodes = updatedChild.nodes();
+
+ // All sub-fields per each sub-class : Original
+ Map<String, Set<String>> originalNodesChildren = new LinkedHashMap<>();
+ for (Entry<String, ConfigNode> e : originalPolyNodes.entrySet()) {
+ originalNodesChildren.computeIfAbsent(e.getKey(), k -> new
HashSet<>()).addAll(e.getValue().children().keySet());
+ }
+
+ // All sub-fields per each sub-class : Updated
+ Map<String, Set<String>> updatedNodesChildren = new LinkedHashMap<>();
+ for (Entry<String, ConfigNode> e : updatedPolyNodes.entrySet()) {
+ updatedNodesChildren.computeIfAbsent(e.getKey(), k -> new
HashSet<>()).addAll(e.getValue().children().keySet());
+ }
+
+ // No child properties has neither been added nor removed - check
compatibility of every node
+ if (originalNodesChildren.equals(updatedNodesChildren)) {
+ for (String key : originalNodesChildren.keySet()) {
+ ConfigNode originalVariant = originalPolyNodes.get(key);
+ ConfigNode updatedVariant = updatedPolyNodes.get(key);
+
+ validateConfigNode(key, originalVariant, updatedVariant,
context, errors);
+ }
+ } else {
+ // Build a map of changes per subclass
+ Map<String, List<ChildChange>> changes =
findChangesBetweenNodesChildren(updatedNodesChildren, originalNodesChildren);
+
+ Set<String> skipErrorKeys = new HashSet<>();
+
+ // Valid polymorphic-node -> polymorphic-node transformations:
+ // 1. Moving a child node from a base class to all subclasses iff
the child node has a default value
Review Comment:
```suggestion
// 1. Moving a child node from a base class to all subclasses if
the child node has a default value
```
--
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
