This is an automated email from the ASF dual-hosted git repository.
amashenkov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git
The following commit(s) were added to refs/heads/main by this push:
new 3c7abf3a914 IGNITE-26006 Configuration compatibility. Correctly handle
adding new fields in compatibility test (#6305)
3c7abf3a914 is described below
commit 3c7abf3a914eb83fe75a4fe9086d6045c99950e5
Author: Max Zhuravkov <[email protected]>
AuthorDate: Thu Jul 24 17:27:38 2025 +0300
IGNITE-26006 Configuration compatibility. Correctly handle adding new
fields in compatibility test (#6305)
---
.../ConfigurationCompatibilityTest.java | 27 +-
.../GenerateConfigurationSnapshot.java | 54 ++++
.../compatibility/framework/ConfigNode.java | 109 ++++++-
.../framework/ConfigNodeSerializer.java | 7 +-
.../framework/ConfigurationTreeComparator.java | 335 +++++++++++++--------
.../ConfigurationTreeComparatorSelfTest.java | 167 ++++++++--
.../compatibility/configuration/snapshot.bin | Bin 4039 -> 4067 bytes
7 files changed, 508 insertions(+), 191 deletions(-)
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/ConfigurationCompatibilityTest.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/ConfigurationCompatibilityTest.java
index 61c343a442a..bfe018dd630 100644
---
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/ConfigurationCompatibilityTest.java
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/ConfigurationCompatibilityTest.java
@@ -37,14 +37,11 @@ import
org.apache.ignite.internal.configuration.ConfigurationModules;
import org.apache.ignite.internal.configuration.ServiceLoaderModulesProvider;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNode;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNodeSerializer;
-import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigShuttle;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationSnapshotManager;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationTreeComparator;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationTreeComparator.ComparisonContext;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationTreeScanner;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationTreeScanner.ScanContext;
-import org.apache.ignite.internal.logger.IgniteLogger;
-import org.apache.ignite.internal.logger.Loggers;
import org.apache.ignite.internal.testframework.IgniteAbstractTest;
import org.apache.ignite.internal.util.io.IgniteUnsafeDataInput;
import org.apache.ignite.internal.util.io.IgniteUnsafeDataOutput;
@@ -58,11 +55,8 @@ import org.junit.jupiter.params.provider.MethodSource;
* Tests for configuration compatibility.
*/
public class ConfigurationCompatibilityTest extends IgniteAbstractTest {
- private static final String DEFAULT_FILE_NAME = "snapshot.bin";
+ static final String DEFAULT_FILE_NAME = "snapshot.bin";
private static final String SNAPSHOTS_RESOURCE_LOCATION =
"compatibility/configuration/";
- private static final Path DEFAULT_SNAPSHOT_FILE = Path.of("modules",
"runner", "build", "work", DEFAULT_FILE_NAME);
-
- private static final IgniteLogger LOG =
Loggers.forClass(ConfigurationCompatibilityTest.class);
/**
* This test ensures that the current configuration can be serialized and
deserialized correctly.
@@ -93,7 +87,7 @@ public class ConfigurationCompatibilityTest extends
IgniteAbstractTest {
* This test ensures that the current configuration metadata wasn't
changed. If the test fails, it means that the current configuration
* metadata has changed, then current snapshot should be renamed to the
latest release version, and a new snapshot should be created.
*
- * @see #main(String[]) method for generating a new snapshot.
+ * @see GenerateConfigurationSnapshot#main(String[]) method for generating
a new snapshot.
*/
@Test
void testConfigurationChanged() throws IOException {
@@ -126,7 +120,7 @@ public class ConfigurationCompatibilityTest extends
IgniteAbstractTest {
return new HashSet<>(modules);
}
- private static List<ConfigNode> loadCurrentConfiguration() {
+ static List<ConfigNode> loadCurrentConfiguration() {
ConfigurationModules modules =
loadConfigurationModules(ConfigurationCompatibilityTest.class.getClassLoader());
ConfigurationModule local = modules.local();
@@ -163,21 +157,6 @@ public class ConfigurationCompatibilityTest extends
IgniteAbstractTest {
return new ConfigurationModules(modules);
}
- /**
- * Generates a snapshot of the current configuration metadata and saves it
to a file.
- */
- public static void main(String[] args) throws IOException {
- List<ConfigNode> configNodes = loadCurrentConfiguration();
-
- ConfigShuttle shuttle = node -> LOG.info(node.toString());
- LOG.info("DUMP TREE:");
- configNodes.forEach(c -> c.accept(shuttle));
-
- ConfigurationSnapshotManager.saveSnapshotToFile(configNodes,
DEFAULT_SNAPSHOT_FILE);
-
- LOG.info("Snapshot saved to: " +
DEFAULT_SNAPSHOT_FILE.toAbsolutePath());
- }
-
/**
* List directory contents for a resource folder. Not recursive. Works for
regular files and also JARs.
*/
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/GenerateConfigurationSnapshot.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/GenerateConfigurationSnapshot.java
new file mode 100644
index 00000000000..b31fcecc17c
--- /dev/null
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/GenerateConfigurationSnapshot.java
@@ -0,0 +1,54 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You 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.apache.ignite.internal.configuration.compatibility;
+
+import static
org.apache.ignite.internal.configuration.compatibility.ConfigurationCompatibilityTest.DEFAULT_FILE_NAME;
+import static
org.apache.ignite.internal.configuration.compatibility.ConfigurationCompatibilityTest.loadCurrentConfiguration;
+
+import java.io.IOException;
+import java.nio.file.Path;
+import java.util.List;
+import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNode;
+import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigShuttle;
+import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationSnapshotManager;
+import org.apache.ignite.internal.logger.IgniteLogger;
+import org.apache.ignite.internal.logger.Loggers;
+
+/**
+ * Generates a snapshot for the current configuration.
+ */
+public class GenerateConfigurationSnapshot {
+ private static final Path DEFAULT_SNAPSHOT_FILE = Path.of("modules",
"runner", "build", "work", DEFAULT_FILE_NAME);
+
+ private static final IgniteLogger LOG =
Loggers.forClass(ConfigurationCompatibilityTest.class);
+
+ /**
+ * Generates a snapshot of the current configuration metadata and saves it
to a file.
+ */
+ public static void main(String[] args) throws IOException {
+ List<ConfigNode> configNodes = loadCurrentConfiguration();
+
+ ConfigShuttle shuttle = node -> LOG.info(node.toString());
+ LOG.info("DUMP TREE:");
+ configNodes.forEach(c -> c.accept(shuttle));
+
+ ConfigurationSnapshotManager.saveSnapshotToFile(configNodes,
DEFAULT_SNAPSHOT_FILE);
+
+ LOG.info("Snapshot saved to: " +
DEFAULT_SNAPSHOT_FILE.toAbsolutePath());
+ }
+}
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNode.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNode.java
index 0293a69fc81..bed44c810e9 100644
---
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNode.java
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNode.java
@@ -42,7 +42,7 @@ public class ConfigNode {
@JsonProperty
private List<ConfigAnnotation> annotations = new ArrayList<>();
@JsonProperty
- private final Map<String, ConfigNode> childNodeMap = new LinkedHashMap<>();
+ private Map<String, Node> childNodeMap = new LinkedHashMap<>();
@JsonProperty
private String flagsHexString;
@JsonProperty
@@ -52,7 +52,7 @@ public class ConfigNode {
// Non-serializable fields.
@JsonIgnore
- @Nullable
+ @Nullable
private ConfigNode parent;
@JsonIgnore
private EnumSet<Flags> flags;
@@ -153,11 +153,18 @@ public class ConfigNode {
return attributes.get(Attributes.CLASS);
}
+ /**
+ * Returns flags for this node.
+ */
+ public Set<Flags> flags() {
+ return flags;
+ }
+
/**
* Returns the child nodes of this node.
*/
- public Collection<ConfigNode> childNodes() {
- return childNodeMap.values();
+ public Map<String, Node> children() {
+ return childNodeMap;
}
/**
@@ -173,7 +180,10 @@ public class ConfigNode {
void addChildNodes(Collection<ConfigNode> childNodes) {
assert !flags.contains(Flags.IS_VALUE) : "Value node can't have
children.";
- childNodes.forEach(e -> childNodeMap.put(e.name(), e));
+ childNodes.forEach(e -> {
+ e.parent = this;
+ addChildNode(e.name(), new Node(e));
+ });
}
/**
@@ -184,12 +194,11 @@ public class ConfigNode {
addChildNodes(List.of(childNodes));
}
- /**
- * Returns {@code true} if this node is a root node, {@code false}
otherwise.
- */
- @JsonIgnore
- public boolean isRoot() {
- return parent == null;
+ private void addChildNode(String name, Node node) {
+ Node existing = childNodeMap.put(name, node);
+ if (existing != null) {
+ throw new IllegalArgumentException("Child node already exists: " +
name);
+ }
}
/**
@@ -216,6 +225,14 @@ public class ConfigNode {
return flags.contains(Flags.IS_INNER_NODE);
}
+ /**
+ * Returns {@code true} if a configuration value that this node presents
has a default value, otherwise {@code false}.
+ */
+ @JsonIgnore
+ public boolean hasDefault() {
+ return flags.contains(Flags.HAS_DEFAULT);
+ }
+
/**
* Returns {@code true} if this node represents internal part of
configuration, {@code false} otherwise.
*/
@@ -269,8 +286,8 @@ public class ConfigNode {
public void accept(ConfigShuttle visitor) {
visitor.visit(this);
- for (ConfigNode child : childNodes()) {
- child.accept(visitor);
+ for (Node child : children().values()) {
+ child.node().accept(visitor);
}
}
@@ -312,7 +329,8 @@ public class ConfigNode {
IS_DEPRECATED(1 << 2),
IS_INTERNAL(1 << 3),
IS_NAMED_NODE(1 << 4),
- IS_INNER_NODE(1 << 5);
+ IS_INNER_NODE(1 << 5),
+ HAS_DEFAULT(1 << 6);
private final int mask;
@@ -351,4 +369,67 @@ public class ConfigNode {
static String KIND = "kind";
static String CLASS = "class";
}
+
+ /**
+ * Node holds a references to a child config node.
+ */
+ public static class Node {
+ /**
+ * Non-polymorphic node.
+ */
+ @JsonProperty
+ private ConfigNode single;
+
+ @SuppressWarnings("unused")
+ Node() {
+ // Default constructor for Jackson deserialization.
+ }
+
+ Node(ConfigNode single) {
+ this.single = single;
+ }
+
+ /**
+ * Returns a single node if this node is a simple node or throws.
+ */
+ ConfigNode node() {
+ assert single != null : "Use the nodes() method instead.";
+ return single;
+ }
+
+ /**
+ * Returns {@code true} if this node represents a value node.
+ *
+ * @see ConfigNode#isValue()
+ */
+ @JsonIgnore
+ boolean isValue() {
+ return single.isValue();
+ }
+
+ /**
+ * Returns {@code true} if this node has a default value.
+ *
+ * @see ConfigNode#hasDefault()
+ */
+ boolean hasDefault() {
+ return single.hasDefault();
+ }
+
+ /**
+ * Returns legacy names.
+ *
+ * @see ConfigNode#legacyPropertyNames()
+ */
+ Set<String> legacyPropertyNames() {
+ return single.legacyPropertyNames();
+ }
+
+ /**
+ * Returns the full path of this node in the configuration tree.
+ */
+ String path() {
+ return single.path();
+ }
+ }
}
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNodeSerializer.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNodeSerializer.java
index 16033713ecb..23f3b822ad8 100644
---
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNodeSerializer.java
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigNodeSerializer.java
@@ -22,6 +22,7 @@ import com.fasterxml.jackson.databind.MappingIterator;
import com.fasterxml.jackson.databind.ObjectMapper;
import java.io.IOException;
import java.util.List;
+import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNode.Node;
import org.apache.ignite.internal.util.io.IgniteDataInput;
import org.apache.ignite.internal.util.io.IgniteDataOutput;
@@ -61,9 +62,9 @@ public class ConfigNodeSerializer {
* Recursively set parent links for all children
*/
private static void restoreParentLinks(ConfigNode node) {
- for (ConfigNode child : node.childNodes()) {
- child.init(node);
- restoreParentLinks(child);
+ for (Node child : node.children().values()) {
+ child.node().init(node);
+ restoreParentLinks(child.node());
}
}
}
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparator.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparator.java
index 1c4bdb76d5a..ddc68a67560 100644
---
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparator.java
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparator.java
@@ -17,18 +17,21 @@
package org.apache.ignite.internal.configuration.compatibility.framework;
+import static org.apache.ignite.internal.lang.IgniteStringFormatter.format;
import static org.junit.jupiter.api.Assertions.assertEquals;
import java.util.ArrayList;
import java.util.Collection;
-import java.util.Collections;
+import java.util.HashMap;
import java.util.List;
+import java.util.Map;
+import java.util.Map.Entry;
import java.util.Objects;
import java.util.Set;
-import java.util.function.Consumer;
import java.util.stream.Collectors;
import org.apache.ignite.configuration.ConfigurationModule;
import org.apache.ignite.configuration.KeyIgnorer;
+import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNode.Node;
/**
* Compares two configuration trees (snapshot and current).
@@ -45,11 +48,11 @@ public class ConfigurationTreeComparator {
List<ConfigNode> actualTrees,
ComparisonContext compContext
) {
- LeafNodesVisitor shuttle = new LeafNodesVisitor(new
Validator(actualTrees), compContext);
+ compContext.reset();
- for (ConfigNode tree : snapshotTrees) {
- tree.accept(shuttle);
- }
+ compareRoots(snapshotTrees, actualTrees, compContext);
+
+ compContext.throwIfNotEmpty();
}
/**
@@ -61,138 +64,52 @@ public class ConfigurationTreeComparator {
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;
- }
-
- @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);
- }
- }
- }
-
- /**
- * Validates value nodes.
- */
- private static class Validator implements Consumer<ConfigNode> {
- private final List<ConfigNode> roots;
-
- private Validator(List<ConfigNode> roots) {
- this.roots = roots;
- }
-
- @Override
- public void accept(ConfigNode leafNode) {
- List<ConfigNode> path = getPath(leafNode);
-
- // Validate path starting from the root.
- Collection<ConfigNode> candidates = roots;
-
- for (ConfigNode node : path) {
- ConfigNode found = find(node, candidates);
- candidates = found.childNodes();
- }
- }
-
- /**
- * Return first node from candidates collection that matches the given
node.
- *
- * @throws IllegalStateException If no match found.
- */
- private ConfigNode find(ConfigNode node, Collection<ConfigNode>
candidates) {
- for (ConfigNode candidate : candidates) {
- if (!match(node, candidate)) {
- continue;
- }
-
- // node is a snapshot node
- // candidate is a current configuration node.
- validateAnnotations(node, candidate);
-
- return candidate;
- }
-
- throw new IllegalStateException("No match found for node: " + node
+ " in candidates: \n\t"
- +
candidates.stream().map(ConfigNode::toString).collect(Collectors.joining("\n\t")));
- }
- }
-
- /**
- * Builds value node path.
- */
- private static List<ConfigNode> getPath(ConfigNode node) {
- List<ConfigNode> path = new ArrayList<>();
- while (node != null) {
- path.add(node);
- node = node.getParent();
- }
-
- Collections.reverse(path);
-
- return path;
- }
-
/**
* Returns {@code true} if given node is compatible with candidate node,
{@code false} otherwise.
*/
- private static boolean match(ConfigNode node, ConfigNode candidate) {
- return Objects.equals(candidate.kind(), node.kind())
- && validateFlags(candidate, node)
- && matchNames(candidate, node)
- &&
candidate.deletedPrefixes().containsAll(node.deletedPrefixes())
- && (!node.isValue() || Objects.equals(candidate.type(),
node.type())); // Value node types can be changed.
+ private static boolean match(ConfigNode candidate, ConfigNode current) {
+ // To make debugging easier.
+ boolean kindMatches = Objects.equals(current.kind(), candidate.kind());
+ boolean nameMatches = matchNames(current, candidate);
+ boolean flagMatch = validateFlags(current, candidate);
+ boolean deletedPrefixesMatch =
current.deletedPrefixes().containsAll(candidate.deletedPrefixes());
+ // Value node types can be changed.
+ boolean nodeTypeMatches = !candidate.isValue() ||
Objects.equals(current.type(), candidate.type());
+ return kindMatches
+ && nameMatches
+ && flagMatch
+ && deletedPrefixesMatch
+ && nodeTypeMatches;
}
- private static void validateAnnotations(ConfigNode candidate, ConfigNode
node) {
- List<String> errors = new ArrayList<>();
-
- ANNOTATION_VALIDATOR.validate(candidate, node, errors);
-
- if (errors.isEmpty()) {
- return;
- }
+ private static void validateAnnotations(ConfigNode candidate, ConfigNode
current, ComparisonContext context) {
+ List<String> annotationErrors = new ArrayList<>();
- StringBuilder sb = new StringBuilder();
- sb.append("Configuration compatibility issues for ")
- .append(node.path())
- .append(':')
- .append(System.lineSeparator());
+ ANNOTATION_VALIDATOR.validate(candidate, current, annotationErrors);
- for (var error : errors) {
- sb.append("\t\t").append(error).append(System.lineSeparator());
+ for (String error : annotationErrors) {
+ context.addError(candidate, error);
}
-
- throw new IllegalStateException(sb.toString());
}
- private static boolean matchNames(ConfigNode candidate, ConfigNode node) {
- return Objects.equals(candidate.name(), node.name())
- || compareUsingLegacyNames(candidate, node);
+ private static boolean matchNames(ConfigNode candidate, ConfigNode
current) {
+ return Objects.equals(candidate.name(), current.name()) ||
compareUsingLegacyNames(candidate, current);
}
- private static boolean compareUsingLegacyNames(ConfigNode candidate,
ConfigNode node) {
- return candidate.legacyPropertyNames().contains(node.name());
+ private static boolean compareUsingLegacyNames(ConfigNode candidate,
ConfigNode current) {
+ return candidate.legacyPropertyNames().contains(current.name());
}
- private static boolean validateFlags(ConfigNode candidate, ConfigNode
node) {
- return node.isRoot() == candidate.isRoot()
- && node.isValue() == candidate.isValue()
- && node.isNamedNode() == candidate.isNamedNode()
- && node.isInnerNode() == candidate.isInnerNode()
- && (!candidate.isInternal() || node.isInternal()) // Public
property\tree can't be hidden.
- && (!node.isDeprecated() || candidate.isDeprecated()); //
Deprecation shouldn't be removed.
+ private static boolean validateFlags(ConfigNode candidate, ConfigNode
current) {
+ // If flags are empty then they should always be compatible.
+ if (candidate.flags().equals(current.flags())) {
+ return true;
+ }
+ return current.isValue() == candidate.isValue()
+ && (!candidate.isInternal() || current.isInternal()) // Public
property\tree can't be hidden.
+ && current.isNamedNode() == candidate.isNamedNode()
+ && current.isInnerNode() == candidate.isInnerNode()
+ && (!current.isDeprecated() || candidate.isDeprecated()); //
Deprecation shouldn't be removed.
}
private static String dumpTree(List<ConfigNode> nodes) {
@@ -232,6 +149,8 @@ public class ConfigurationTreeComparator {
private final KeyIgnorer deletedItems;
+ private final List<String> errors = new ArrayList<>();
+
ComparisonContext(Collection<String> deletedPrefixes) {
this.deletedItems =
KeyIgnorer.fromDeletedPrefixes(deletedPrefixes);
}
@@ -239,5 +158,173 @@ public class ConfigurationTreeComparator {
boolean shouldIgnore(String path) {
return deletedItems.shouldIgnore(path);
}
+
+ void reset() {
+ errors.clear();
+ }
+
+ void addError(Node node, String error) {
+ reportError(node.path(), error);
+ }
+
+ void addError(ConfigNode node, String error) {
+ reportError(node.path(), error);
+ }
+
+ private void reportError(String path, String error) {
+ String message = format("Node: {}: {}", path, error);
+ errors.add(message);
+ }
+
+ private void throwIfNotEmpty() {
+ if (errors.isEmpty()) {
+ return;
+ }
+
+ StringBuilder message = new StringBuilder("There are incompatible
changes:").append(System.lineSeparator());
+ for (String error : errors) {
+
message.append('\t').append(error).append(System.lineSeparator());
+ }
+
+ throw new IllegalStateException(message.toString());
+ }
+ }
+
+
+ private static void compareRoots(List<ConfigNode> candidateRoots,
List<ConfigNode> currentRoots, ComparisonContext context) {
+ List<ConfigNode> removed = new ArrayList<>();
+ List<ConfigNode> added = new ArrayList<>();
+ List<ConfigNode> currentCopy = new ArrayList<>(currentRoots);
+
+ for (ConfigNode root1 : candidateRoots) {
+ boolean matchFound = false;
+
+ for (ConfigNode root2 : new ArrayList<>(currentCopy)) {
+ if (rootsMatch(root1, root2)) {
+ currentCopy.remove(root2);
+ validate(root1, root2, context);
+ matchFound = true;
+ break;
+ }
+ }
+
+ if (!matchFound) {
+ removed.add(root1);
+ }
+ }
+
+ added.addAll(currentCopy);
+
+ // Validate new roots
+ validateNew(added, context);
+ // Reject removed roots.
+ validateRemoved(removed, context);
+ }
+
+ private static boolean rootsMatch(ConfigNode candidate, ConfigNode
current) {
+ boolean nameMatches = Objects.equals(candidate.name(), current.name());
+ boolean kindMatches = Objects.equals(candidate.kind(), current.kind());
+ return nameMatches && kindMatches;
+ }
+
+ private static void validate(Node candidate, Node current,
ComparisonContext context) {
+ compareNodes(candidate.node(), current.node(), context);
+ }
+
+ private static void validate(ConfigNode candidate, ConfigNode current,
ComparisonContext context) {
+ if (!context.errors.isEmpty()) {
+ return;
+ }
+ compareNodes(candidate, current, context);
+ }
+
+ private static void compareNodes(ConfigNode candidate, ConfigNode current,
ComparisonContext context) {
+ if (!match(candidate, current)) {
+ context.addError(candidate, "Node does not match. Previous: " +
candidate + ". Current: " + current);
+ return;
+ }
+
+ validateAnnotations(candidate, current, context);
+
+ compareChildren(candidate.children(), current.children(), context);
+ }
+
+ private static void compareChildren(Map<String, Node> candidate,
Map<String, Node> current, ComparisonContext context) {
+ // Validates matching children.
+ // then validates removed and added ones.
+ List<Node> removed = new ArrayList<>();
+ List<Node> added = new ArrayList<>();
+ Map<String, Node> currentCopy = new HashMap<>(current);
+
+ for (Entry<String, Node> candididateEntry : candidate.entrySet()) {
+ Node candidateNode = candididateEntry.getValue();
+ boolean matchFound = false;
+
+ for (Entry<String, Node> currentEntry : currentCopy.entrySet()) {
+ Node nodeB = currentEntry.getValue();
+ if (equalNames(candididateEntry, currentEntry)) {
+ matchFound = true;
+ currentCopy.remove(currentEntry.getKey());
+ validate(candidateNode, nodeB, context);
+ break;
+ }
+ }
+
+ if (!matchFound) {
+ removed.add(candidateNode);
+ }
+ }
+
+ added.addAll(currentCopy.values());
+
+ validateRemovedChildren(removed, context);
+ validateNewChildren(added, context);
+ }
+
+ private static boolean equalNames(Entry<String, Node> currentEntry,
Entry<String, Node> candidateEntry) {
+ String candidateName = currentEntry.getKey();
+ String currentName = candidateEntry.getKey();
+ if (candidateName.equals(currentName)) {
+ return true;
+ }
+ Node nodeB = candidateEntry.getValue();
+ return nodeB.legacyPropertyNames().stream().anyMatch(n ->
n.equals(candidateName));
+ }
+
+ private static void validateNew(Collection<ConfigNode> nodes,
ComparisonContext context) {
+ // Validate nodes recursively, adding a new node is valid if all its
value nodes have default.
+ for (ConfigNode node : nodes) {
+ if (node.isValue() && !node.hasDefault()) {
+ context.addError(node, "Added a node with no default value");
+ }
+ validateNewChildren(node.children().values(), context);
+ }
+ }
+
+ private static void validateRemoved(Collection<ConfigNode> nodes,
ComparisonContext context) {
+ for (ConfigNode node : nodes) {
+ if (!context.shouldIgnore(node.path())) {
+ context.addError(node, "Node was removed");
+ }
+ }
+ }
+
+ private static void validateNewChildren(Collection<Node> nodes,
ComparisonContext context) {
+ // Validate nodes recursively, adding a new node is valid if all its
value nodes have default.
+ for (Node node : nodes) {
+ if (node.isValue() && !node.hasDefault()) {
+ context.addError(node, "Added a value with no default");
+ } else {
+ validateNew(List.of(node.node()), context);
+ }
+ }
+ }
+
+ private static void validateRemovedChildren(Collection<Node> nodes,
ComparisonContext context) {
+ for (Node node : nodes) {
+ if (!context.shouldIgnore(node.path())) {
+ context.addError(node, "Node was removed");
+ }
+ }
}
}
diff --git
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparatorSelfTest.java
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparatorSelfTest.java
index b9b9f43ac8b..4a725dd7c90 100644
---
a/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparatorSelfTest.java
+++
b/modules/runner/src/test/java/org/apache/ignite/internal/configuration/compatibility/framework/ConfigurationTreeComparatorSelfTest.java
@@ -30,6 +30,8 @@ import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNo
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigNode.Flags;
import
org.apache.ignite.internal.configuration.compatibility.framework.ConfigurationTreeComparator.ComparisonContext;
import org.junit.jupiter.api.Test;
+import org.junit.jupiter.params.ParameterizedTest;
+import org.junit.jupiter.params.provider.ValueSource;
/**
* Self-test for {@link ConfigurationTreeComparator} checks various
compatibility scenarios.
@@ -40,20 +42,20 @@ public class ConfigurationTreeComparatorSelfTest {
void equalConfigurationCanBeFound() {
ConfigNode root1 = createRoot("root1");
root1.addChildNodes(List.of(
- createChild(root1, "child1"),
- createChild(root1, "child2")
+ createChild("child1"),
+ createChild("child2")
));
ConfigNode root2 = createRoot("root1");
root2.addChildNodes(List.of(
- createChild(root2, "child1"),
- createChild(root2, "child2")
+ createChild("child1"),
+ createChild("child2")
));
ConfigNode anotherRoot = createRoot("root2");
anotherRoot.addChildNodes(List.of(
- createChild(anotherRoot, "child1"),
- createChild(anotherRoot, "child2")
+ createChild("child1"),
+ createChild("child2")
));
assertCompatible(List.of(root1), List.of(anotherRoot, root2));
@@ -63,10 +65,10 @@ public class ConfigurationTreeComparatorSelfTest {
void rootCompatibility() {
{
ConfigNode root1 = ConfigNode.createRoot("root", Object.class,
ConfigurationType.LOCAL, true);
- root1.addChildNodes(createChild(root1, "child"));
+ root1.addChildNodes(createChild("child"));
ConfigNode root2 = ConfigNode.createRoot("root", Object.class,
ConfigurationType.LOCAL, false);
- root2.addChildNodes(createChild(root2, "child"));
+ root2.addChildNodes(createChild("child"));
// Internal root can become public.
assertCompatible(root1, root2);
@@ -75,10 +77,10 @@ public class ConfigurationTreeComparatorSelfTest {
}
{
ConfigNode root1 = ConfigNode.createRoot("root", Object.class,
ConfigurationType.LOCAL, true);
- root1.addChildNodes(createChild(root1, "child"));
+ root1.addChildNodes(createChild("child"));
ConfigNode root2 = ConfigNode.createRoot("root", Object.class,
ConfigurationType.DISTRIBUTED, true);
- root2.addChildNodes(createChild(root2, "child"));
+ root2.addChildNodes(createChild("child"));
// Root types can't be changed both ways.
assertIncompatible(root1, root2);
@@ -129,13 +131,13 @@ public class ConfigurationTreeComparatorSelfTest {
void propertyCantBeRemoved() {
ConfigNode root1 = createRoot("root1");
root1.addChildNodes(List.of(
- createChild(root1, "child1")
+ createChild("child1")
));
ConfigNode root2 = createRoot("root1");
root2.addChildNodes(List.of(
- createChild(root2, "child1"),
- createChild(root1, "child2")
+ createChild("child1"),
+ createChild("child2")
));
// Adding a property is compatible change.
@@ -176,7 +178,7 @@ public class ConfigurationTreeComparatorSelfTest {
root1,
Map.of(ConfigNode.Attributes.NAME, "child"),
List.of(),
- EnumSet.of(Flags.IS_VALUE))
+ EnumSet.of(Flags.IS_VALUE, Flags.HAS_DEFAULT))
));
ConfigNode root2 = createRoot("root1");
@@ -185,7 +187,7 @@ public class ConfigurationTreeComparatorSelfTest {
root2,
Map.of(ConfigNode.Attributes.NAME, "child"),
List.of(),
- EnumSet.of(Flags.IS_VALUE, Flags.IS_DEPRECATED))
+ EnumSet.of(Flags.IS_VALUE, Flags.IS_DEPRECATED,
Flags.HAS_DEFAULT))
));
// Adding deprecation is compatible change.
@@ -194,6 +196,67 @@ public class ConfigurationTreeComparatorSelfTest {
assertIncompatible(root2, root1);
}
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void addFieldToConfiguration(boolean hasDefault) {
+ Set<Flags> newFieldFlags = createFlags(hasDefault);
+
+ ConfigNode root1 = createRoot("root1");
+ root1.addChildNodes(List.of(
+ createChild("child1"),
+ createChild("child2")
+ ));
+
+ ConfigNode root2 = createRoot("root1");
+ root2.addChildNodes(List.of(
+ createChild("child1"),
+ createChild("child2"),
+ createChild("child3", newFieldFlags)
+ ));
+
+ if (hasDefault) {
+ assertCompatible(List.of(root1), List.of(root2));
+ assertIncompatible(List.of(root2), List.of(root1));
+ } else {
+ assertIncompatible(List.of(root1), List.of(root2));
+ assertIncompatible(List.of(root2), List.of(root1));
+ }
+ }
+
+ @ParameterizedTest
+ @ValueSource(booleans = {true, false})
+ void addNewRoot(boolean hasDefault) {
+ Set<Flags> fieldFlags = createFlags(hasDefault);
+
+ ConfigNode root1 = createRoot("root1");
+ {
+ root1.addChildNodes(List.of(
+ createChild("child1"),
+ createChild("child2")
+ ));
+ }
+
+ ConfigNode root2 = createRoot("root2");
+ {
+ ConfigNode node = createChild("node1", Set.of());
+ node.addChildNodes(List.of(createChild("f", fieldFlags)));
+
+ root2.addChildNodes(List.of(
+ createChild("child1"),
+ createChild("child2"),
+ node
+ ));
+ }
+
+ if (hasDefault) {
+ assertCompatible(List.of(root1), List.of(root1, root2));
+ assertIncompatible(List.of(root2, root1), List.of(root1));
+ } else {
+ assertIncompatible(List.of(root1), List.of(root1, root2));
+ assertIncompatible(List.of(root2, root1), List.of(root1));
+ }
+ }
+
/**
* Check {@link ConfigurationModule#deletedPrefixes()} functionality.
*/
@@ -340,25 +403,36 @@ public class ConfigurationTreeComparatorSelfTest {
@Test
void testCompatibleRename() {
+ ConfigNode root1 = createRoot("root");
+
ConfigNode oldNode = new ConfigNode(null, Map.of(Attributes.NAME,
"oldTestCount"), List.of(),
EnumSet.of(Flags.IS_VALUE));
+ root1.addChildNodes(oldNode);
+
+ ConfigNode root2 = createRoot("root");
ConfigNode newNode = new ConfigNode(null, Map.of(Attributes.NAME,
"newTestCount"),
List.of(),
EnumSet.of(Flags.IS_VALUE), Set.of("oldTestCount"), List.of());
+ root2.addChildNodes(newNode);
- assertCompatible(oldNode, newNode);
+ assertCompatible(root1, root2);
+ root1 = createRoot("root");
// Equal configs, still compatible
oldNode = new ConfigNode(null, Map.of(Attributes.NAME, "newTestCount"),
List.of(),
EnumSet.of(Flags.IS_VALUE), Set.of("oldTestCount"), List.of());
+ root1.addChildNodes(oldNode);
+
+ root2 = createRoot("root");
newNode = new ConfigNode(null, Map.of(Attributes.NAME, "newTestCount"),
List.of(),
EnumSet.of(Flags.IS_VALUE), Set.of("oldTestCount"), List.of());
+ root2.addChildNodes(newNode);
- assertCompatible(oldNode, newNode);
+ assertCompatible(root1, root2);
// TODO: need to have a possibility to check compatibility between
outdated (officially no more supported) version and current one.
// I.e. previous tree with filled deletedPrefixes and current without
- need to be compatible too, decided to make it later.
@@ -394,6 +468,32 @@ public class ConfigurationTreeComparatorSelfTest {
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(Flags.IS_VALUE,
Flags.HAS_DEFAULT), Set.of("oldTestCount_misspelled"), List.of());
+ root2.addChildNodes(newNode2);
+
+ assertIncompatible(root1, root2);
+
+ // Incorrectly renaming intermediate nodes.
+
+ root1 = createRoot("root1");
+ oldNode2 = createChild("node1", Set.of());
+ oldNode2.addChildNodes(createChild("value"));
+ root1.addChildNodes(oldNode2);
+
+ root2 = createRoot("root1");
+ newNode2 = createChild("node_new", Set.of(),
Set.of("node_misspelled"), List.of());
+ newNode2.addChildNodes(createChild("value"));
+ root2.addChildNodes(newNode2);
+
+ assertIncompatible(root1, root2);
}
/**
@@ -410,7 +510,7 @@ public class ConfigurationTreeComparatorSelfTest {
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));
root.addChildNodes(node1Ver1);
@@ -421,7 +521,7 @@ public class ConfigurationTreeComparatorSelfTest {
ConfigNode node1Ver2 = new ConfigNode(root, Map.of(Attributes.NAME,
"newTestCount"),
List.of(),
- EnumSet.of(Flags.IS_VALUE), Set.of("oldTestCount"), List.of());
+ EnumSet.of(Flags.IS_VALUE, Flags.HAS_DEFAULT),
Set.of("oldTestCount"), List.of());
root.addChildNodes(node1Ver2);
@@ -431,7 +531,7 @@ public class ConfigurationTreeComparatorSelfTest {
EnumSet.of(Flags.IS_ROOT));
ConfigNode node = new ConfigNode(root, Map.of(Attributes.NAME,
"fixed"), List.of(),
- EnumSet.of(Flags.IS_VALUE));
+ EnumSet.of(Flags.IS_VALUE, Flags.HAS_DEFAULT));
root.addChildNodes(node);
@@ -460,12 +560,22 @@ public class ConfigurationTreeComparatorSelfTest {
return ConfigNode.createRoot(name, Object.class,
ConfigurationType.LOCAL, true);
}
- private static ConfigNode createChild(ConfigNode root1, String name) {
+ private static ConfigNode createChild(String name) {
+ return createChild(name, Set.of(), Set.of(), List.of());
+ }
+
+ private static ConfigNode createChild(String name, Set<Flags> flags) {
+ return createChild(name, flags, Set.of(), List.of());
+ }
+
+ private static ConfigNode createChild(String name, Set<Flags> flags,
Set<String> legacyNames, List<String> deletedPrefixes) {
return new ConfigNode(
- root1,
- Map.of(ConfigNode.Attributes.NAME, name, Attributes.CLASS,
int.class.getCanonicalName()),
+ null,
+ Map.of(ConfigNode.Attributes.NAME, name, Attributes.CLASS,
"Object"),
List.of(),
- EnumSet.of(Flags.IS_VALUE)
+ flags.isEmpty() ? EnumSet.noneOf(Flags.class) :
EnumSet.copyOf(flags),
+ legacyNames,
+ deletedPrefixes
);
}
@@ -491,12 +601,17 @@ public class ConfigurationTreeComparatorSelfTest {
private static void assertIncompatible(List<ConfigNode> oldConfig,
List<ConfigNode> newConfig, ComparisonContext compContext) {
try {
- assertCompatible(oldConfig, newConfig, compContext);
- } catch (IllegalStateException ignore) {
+ ConfigurationTreeComparator.ensureCompatible(oldConfig, newConfig,
compContext);
+ } catch (IllegalStateException e) {
// Expected exception
+ System.err.println("Error: " + e.getMessage());
return;
}
fail("Compatibility check passed unexpectedly.");
}
+
+ private static Set<Flags> createFlags(boolean hasDefault) {
+ return hasDefault ? Set.of(Flags.HAS_DEFAULT, Flags.IS_VALUE) :
Set.of(Flags.IS_VALUE);
+ }
}
diff --git
a/modules/runner/src/test/resources/compatibility/configuration/snapshot.bin
b/modules/runner/src/test/resources/compatibility/configuration/snapshot.bin
index 6e100c42880..2d1d8676a82 100644
Binary files
a/modules/runner/src/test/resources/compatibility/configuration/snapshot.bin
and
b/modules/runner/src/test/resources/compatibility/configuration/snapshot.bin
differ
