You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2021/06/04 07:17:02 UTC
[ignite-3] branch main updated: IGNITE-14800 Configuration change
method applies passed lambda before every validation and provides the
ability to read changing configuration in the process. (#158)
This is an automated email from the ASF dual-hosted git repository.
ibessonov 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 b99e362 IGNITE-14800 Configuration change method applies passed lambda before every validation and provides the ability to read changing configuration in the process. (#158)
b99e362 is described below
commit b99e36242e7cf1a8cd9e901e1039c46ebf7fb933
Author: ibessonov <be...@gmail.com>
AuthorDate: Fri Jun 4 10:16:46 2021 +0300
IGNITE-14800 Configuration change method applies passed lambda before every validation and provides the ability to read changing configuration in the process. (#158)
---
.../processor/internal/Processor.java | 1 +
.../configuration/ConfigurationChangerTest.java | 17 ++---
.../internal/util/ConfigurationUtilTest.java | 25 ++++---
.../sample/TraversableTreeNodeTest.java | 22 ++++---
.../ignite/configuration/ConfigurationValue.java | 2 +-
.../ignite/configuration/ConfigurationChanger.java | 76 ++++++++++------------
.../internal/DynamicConfiguration.java | 34 ++++------
.../configuration/internal/DynamicProperty.java | 26 ++++----
.../ignite/configuration/internal/SuperRoot.java | 13 ++--
.../internal/asm/ConfigurationAsmGenerator.java | 30 +++++----
.../internal/util/ConfigurationUtil.java | 73 +++++++++++++++++----
.../configuration/tree/ConfigurationSource.java | 6 ++
.../ignite/configuration/tree/NamedListNode.java | 5 +-
13 files changed, 194 insertions(+), 136 deletions(-)
diff --git a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java
index f6764c2..d9b7611 100644
--- a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java
+++ b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/configuration/processor/internal/Processor.java
@@ -307,6 +307,7 @@ public class Processor extends AbstractProcessor {
.addModifiers(PUBLIC);
TypeSpec.Builder changeClsBuilder = TypeSpec.interfaceBuilder(changeClsName)
+ .addSuperinterface(viewClsName)
.addModifiers(PUBLIC);
ClassName consumerClsName = ClassName.get(Consumer.class);
diff --git a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/ConfigurationChangerTest.java b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/ConfigurationChangerTest.java
index 7e22e2e..60b839b 100644
--- a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/ConfigurationChangerTest.java
+++ b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/ConfigurationChangerTest.java
@@ -33,6 +33,7 @@ import org.apache.ignite.configuration.storage.ConfigurationType;
import org.apache.ignite.configuration.storage.Data;
import org.apache.ignite.configuration.storage.TestConfigurationStorage;
import org.apache.ignite.configuration.tree.InnerNode;
+import org.apache.ignite.configuration.tree.TraversableTreeNode;
import org.apache.ignite.configuration.validation.Immutable;
import org.apache.ignite.configuration.validation.ValidationContext;
import org.apache.ignite.configuration.validation.ValidationIssue;
@@ -42,10 +43,10 @@ import org.junit.jupiter.api.Test;
import static java.lang.annotation.ElementType.FIELD;
import static java.lang.annotation.RetentionPolicy.RUNTIME;
-import static java.util.Collections.singletonMap;
import static java.util.concurrent.CompletableFuture.completedFuture;
import static java.util.concurrent.TimeUnit.SECONDS;
import static org.apache.ignite.configuration.AConfiguration.KEY;
+import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.superRootPatcher;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
@@ -116,7 +117,7 @@ public class ConfigurationChangerTest {
.changeChild(change -> change.changeIntCfg(1).changeStrCfg("1"))
.changeElements(change -> change.create("a", element -> element.changeStrCfg("1")));
- changer.change(singletonMap(KEY, (InnerNode)data)).get(1, SECONDS);
+ changer.change(superRootPatcher(KEY, (TraversableTreeNode)data), null).get(1, SECONDS);
AView newRoot = (AView)changer.getRootNode(KEY);
@@ -144,7 +145,7 @@ public class ConfigurationChangerTest {
.changeChild(change -> change.changeIntCfg(1).changeStrCfg("1"))
.changeElements(change -> change.create("a", element -> element.changeStrCfg("1")));
- changer1.change(singletonMap(KEY, (InnerNode)data1)).get(1, SECONDS);
+ changer1.change(superRootPatcher(KEY, (TraversableTreeNode)data1), null).get(1, SECONDS);
AChange data2 = ((AChange)changer2.createRootNode(KEY))
.changeChild(change -> change.changeIntCfg(2).changeStrCfg("2"))
@@ -153,7 +154,7 @@ public class ConfigurationChangerTest {
.create("b", element -> element.changeStrCfg("2"))
);
- changer2.change(singletonMap(KEY, (InnerNode)data2)).get(1, SECONDS);
+ changer2.change(superRootPatcher(KEY, (TraversableTreeNode)data2), null).get(1, SECONDS);
AView newRoot1 = (AView)changer1.getRootNode(KEY);
@@ -189,7 +190,7 @@ public class ConfigurationChangerTest {
.changeChild(change -> change.changeIntCfg(1).changeStrCfg("1"))
.changeElements(change -> change.create("a", element -> element.changeStrCfg("1")));
- changer1.change(singletonMap(KEY, (InnerNode)data1)).get(1, SECONDS);
+ changer1.change(superRootPatcher(KEY, (TraversableTreeNode)data1), null).get(1, SECONDS);
changer2.addValidator(MaybeInvalid.class, new Validator<MaybeInvalid, Object>() {
@Override public void validate(MaybeInvalid annotation, ValidationContext<Object> ctx) {
@@ -204,7 +205,7 @@ public class ConfigurationChangerTest {
.create("b", element -> element.changeStrCfg("2"))
);
- assertThrows(ExecutionException.class, () -> changer2.change(singletonMap(KEY, (InnerNode)data2)).get(1, SECONDS));
+ assertThrows(ExecutionException.class, () -> changer2.change(superRootPatcher(KEY, (TraversableTreeNode)data2), null).get(1, SECONDS));
AView newRoot = (AView)changer2.getRootNode(KEY);
@@ -235,7 +236,7 @@ public class ConfigurationChangerTest {
AChange data = ((AChange)changer.createRootNode(KEY)).changeChild(child -> child.changeIntCfg(1));
- assertThrows(ExecutionException.class, () -> changer.change(singletonMap(KEY, (InnerNode)data)).get(1, SECONDS));
+ assertThrows(ExecutionException.class, () -> changer.change(superRootPatcher(KEY, (TraversableTreeNode)data), null).get(1, SECONDS));
storage.fail(false);
@@ -298,7 +299,7 @@ public class ConfigurationChangerTest {
childs.create("name", child -> {})
);
- changer.change(Map.of(DefaultsConfiguration.KEY, (InnerNode)change)).get(1, SECONDS);
+ changer.change(superRootPatcher(DefaultsConfiguration.KEY, (TraversableTreeNode)change), null).get(1, SECONDS);
root = (DefaultsView)changer.getRootNode(DefaultsConfiguration.KEY);
diff --git a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/internal/util/ConfigurationUtilTest.java b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/internal/util/ConfigurationUtilTest.java
index 48051c3..c8669b1 100644
--- a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/internal/util/ConfigurationUtilTest.java
+++ b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/internal/util/ConfigurationUtilTest.java
@@ -269,14 +269,12 @@ public class ConfigurationUtilTest {
public void nodeToFlatMap() {
var parentNode = newParentInstance();
- var parentSuperRoot = new SuperRoot(key -> null, Map.of(
- ParentConfiguration.KEY,
- parentNode
- ));
-
assertEquals(
emptyMap(),
- ConfigurationUtil.nodeToFlatMap(null, parentSuperRoot)
+ ConfigurationUtil.nodeToFlatMap(null, new SuperRoot(key -> null, Map.of(
+ ParentConfiguration.KEY,
+ parentNode
+ )))
);
// No defaults in this test so everything must be initialized explicitly.
@@ -290,12 +288,18 @@ public class ConfigurationUtilTest {
assertEquals(
singletonMap("root.elements.name.child.str", "foo"),
- ConfigurationUtil.nodeToFlatMap(null, parentSuperRoot)
+ ConfigurationUtil.nodeToFlatMap(null, new SuperRoot(key -> null, Map.of(
+ ParentConfiguration.KEY,
+ parentNode
+ )))
);
assertEquals(
emptyMap(),
- ConfigurationUtil.nodeToFlatMap(parentSuperRoot, new SuperRoot(key -> null, singletonMap(
+ ConfigurationUtil.nodeToFlatMap(new SuperRoot(key -> null, Map.of(
+ ParentConfiguration.KEY,
+ parentNode
+ )), new SuperRoot(key -> null, singletonMap(
ParentConfiguration.KEY,
(InnerNode)newParentInstance().changeElements(elements ->
elements.delete("void")
@@ -305,7 +309,10 @@ public class ConfigurationUtilTest {
assertEquals(
singletonMap("root.elements.name.child.str", null),
- ConfigurationUtil.nodeToFlatMap(parentSuperRoot, new SuperRoot(key -> null, singletonMap(
+ ConfigurationUtil.nodeToFlatMap(new SuperRoot(key -> null, Map.of(
+ ParentConfiguration.KEY,
+ parentNode
+ )), new SuperRoot(key -> null, singletonMap(
ParentConfiguration.KEY,
(InnerNode)newParentInstance().changeElements(elements ->
elements.delete("name")
diff --git a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/TraversableTreeNodeTest.java b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/TraversableTreeNodeTest.java
index fafcaf8..148fcbb 100644
--- a/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/TraversableTreeNodeTest.java
+++ b/modules/configuration-annotation-processor/src/test/java/org/apache/ignite/configuration/sample/TraversableTreeNodeTest.java
@@ -33,6 +33,7 @@ import org.apache.ignite.configuration.tree.NamedListNode;
import org.apache.ignite.configuration.tree.NamedListView;
import org.apache.ignite.configuration.tree.TraversableTreeNode;
import org.apache.ignite.configuration.validation.Immutable;
+import org.hamcrest.CoreMatchers;
import org.junit.jupiter.api.AfterAll;
import org.junit.jupiter.api.BeforeAll;
import org.junit.jupiter.api.Test;
@@ -43,8 +44,8 @@ import static org.hamcrest.CoreMatchers.instanceOf;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
+import static org.junit.jupiter.api.Assertions.assertNotSame;
import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertSame;
import static org.junit.jupiter.api.Assertions.assertThrows;
/** */
@@ -162,8 +163,7 @@ public class TraversableTreeNodeTest {
parentNode.changeChild(child -> child.changeStrCfg("value"));
- // Assert that change method applied its closure to the same object instead of creating a new one.
- assertSame(childNode, parentNode.child());
+ assertNotSame(childNode, parentNode.child());
}
/**
@@ -180,8 +180,7 @@ public class TraversableTreeNodeTest {
parentNode.changeElements(elements -> elements.update("key", element -> {}));
- // Assert that change method applied its closure to the same object instead of creating a new one.
- assertSame(elementsNode, parentNode.elements());
+ assertNotSame(elementsNode, parentNode.elements());
}
/**
@@ -205,13 +204,18 @@ public class TraversableTreeNodeTest {
((NamedListChange<NamedElementChange>)elementsNode).update("keyPut", element -> element.changeStrCfg("val"));
- // Assert that consecutive put methods don't create new object every time.
- assertSame(elementNode, elementsNode.get("keyPut"));
+ // Assert that consecutive put methods create new object every time.
+ assertNotSame(elementNode, elementsNode.get("keyPut"));
+
+ elementNode = elementsNode.get("keyPut");
assertEquals("val", elementNode.strCfg());
- // Assert that once you put something into list, removing it makes no sense and hence prohibited.
- assertThrows(IllegalStateException.class, () -> ((NamedListChange<?>)elementsNode).delete("keyPut"));
+ ((NamedListChange<?>)elementsNode).delete("keyPut");
+
+ assertThat(elementsNode.namedListKeys(), CoreMatchers.hasItem("keyPut"));
+
+ assertNull(elementsNode.get("keyPut"));
((NamedListChange<?>)elementsNode).delete("keyRemove");
diff --git a/modules/configuration-api/src/main/java/org/apache/ignite/configuration/ConfigurationValue.java b/modules/configuration-api/src/main/java/org/apache/ignite/configuration/ConfigurationValue.java
index c44795c..1e19d4f 100644
--- a/modules/configuration-api/src/main/java/org/apache/ignite/configuration/ConfigurationValue.java
+++ b/modules/configuration-api/src/main/java/org/apache/ignite/configuration/ConfigurationValue.java
@@ -33,5 +33,5 @@ public interface ConfigurationValue<VIEW> extends ConfigurationProperty<VIEW, VI
* @return Future that signifies end of the update operation. Can also be completed with
* {@link ConfigurationValidationException} and {@link ConfigurationChangeException}.
*/
- Future<Void> update(VIEW change) throws ConfigurationValidationException;
+ Future<Void> update(VIEW change);
}
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
index 344a9ab..64d9568 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
@@ -22,6 +22,7 @@ import java.util.HashMap;
import java.util.HashSet;
import java.util.List;
import java.util.Map;
+import java.util.NoSuchElementException;
import java.util.Set;
import java.util.TreeMap;
import java.util.concurrent.CompletableFuture;
@@ -38,7 +39,7 @@ import org.apache.ignite.configuration.storage.ConfigurationType;
import org.apache.ignite.configuration.storage.Data;
import org.apache.ignite.configuration.storage.StorageException;
import org.apache.ignite.configuration.tree.ConfigurationSource;
-import org.apache.ignite.configuration.tree.ConfigurationVisitor;
+import org.apache.ignite.configuration.tree.ConstructableTreeNode;
import org.apache.ignite.configuration.tree.InnerNode;
import org.apache.ignite.configuration.validation.ConfigurationValidationException;
import org.apache.ignite.configuration.validation.ValidationIssue;
@@ -51,6 +52,7 @@ import static java.util.stream.Collectors.toList;
import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.addDefaults;
import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.cleanupMatchingValues;
import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.fillFromPrefixMap;
+import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.nodePatcher;
import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.nodeToFlatMap;
import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.patch;
import static org.apache.ignite.configuration.internal.util.ConfigurationUtil.toPrefixMap;
@@ -222,8 +224,11 @@ public abstract class ConfigurationChanger {
/**
* Initializes the configuration storage - reads data and sets default values for missing configuration properties.
* @param storageType Storage type.
+ * @throws ConfigurationValidationException If configuration validation failed.
+ * @throws ConfigurationChangeException If configuration framework failed to add default values and save them to
+ * storage.
*/
- public void initialize(ConfigurationType storageType) {
+ public void initialize(ConfigurationType storageType) throws ConfigurationValidationException, ConfigurationChangeException {
ConfigurationStorage configurationStorage = storageInstances.get(storageType);
assert configurationStorage != null : storageType;
@@ -247,7 +252,7 @@ public abstract class ConfigurationChanger {
throw new ConfigurationValidationException(validationIssues);
try {
- changeInternally(defaultsNode, storageInstances.get(storageType)).get();
+ changeInternally(nodePatcher(defaultsNode), storageInstances.get(storageType)).get();
}
catch (InterruptedException | ExecutionException e) {
throw new ConfigurationChangeException(
@@ -267,19 +272,24 @@ public abstract class ConfigurationChanger {
ConfigurationSource source,
@Nullable ConfigurationStorage storage
) {
- SuperRoot superRoot = new SuperRoot(rootCreator());
-
- source.descend(superRoot);
-
Set<ConfigurationType> storagesTypes = new HashSet<>();
- superRoot.traverseChildren(new ConfigurationVisitor<Object>() {
- @Override public Object visitInnerNode(String key, InnerNode node) {
+ ConstructableTreeNode collector = new ConstructableTreeNode() {
+ @Override public void construct(String key, ConfigurationSource src) throws NoSuchElementException {
RootKey<?, ?> rootKey = rootKeys.get(key);
- return storagesTypes.add(rootKey.type());
+ if (rootKey == null)
+ throw new NoSuchElementException(key);
+
+ storagesTypes.add(rootKey.type());
}
- });
+
+ @Override public ConstructableTreeNode copy() {
+ throw new UnsupportedOperationException("copy");
+ }
+ };
+
+ source.descend(collector);
assert !storagesTypes.isEmpty();
@@ -299,7 +309,7 @@ public abstract class ConfigurationChanger {
);
}
- return changeInternally(superRoot, actualStorage);
+ return changeInternally(source, actualStorage);
}
/** Stop component. */
@@ -318,30 +328,6 @@ public abstract class ConfigurationChanger {
}
/**
- * Change configuration.
- * @param changes Map of changes by root key.
- * @return Future that is completed on change completion.
- */
- public CompletableFuture<Void> change(Map<RootKey<?, ?>, InnerNode> changes) {
- if (changes.isEmpty())
- return completedFuture(null);
-
- Set<ConfigurationType> storagesTypes = changes.keySet().stream()
- .map(RootKey::type)
- .collect(Collectors.toSet());
-
- assert !storagesTypes.isEmpty();
-
- if (storagesTypes.size() != 1) {
- return CompletableFuture.failedFuture(
- new ConfigurationChangeException("Cannot change configurations belonging to different storages.")
- );
- }
-
- return changeInternally(new SuperRoot(rootCreator(), changes), storageInstances.get(storagesTypes.iterator().next()));
- }
-
- /**
* @return Super root chat contains roots belonging to all storages.
*/
public SuperRoot mergedSuperRoot() {
@@ -350,12 +336,12 @@ public abstract class ConfigurationChanger {
/**
* Internal configuration change method that completes provided future.
- * @param changes Map of changes by root key.
+ * @param src Configuration source.
* @param storage Storage instance.
* @return fut Future that will be completed after changes are written to the storage.
*/
private CompletableFuture<Void> changeInternally(
- SuperRoot changes,
+ ConfigurationSource src,
ConfigurationStorage storage
) {
StorageRoots storageRoots = storagesRootsMap.get(storage.type());
@@ -364,6 +350,12 @@ public abstract class ConfigurationChanger {
.supplyAsync(() -> {
SuperRoot curRoots = storageRoots.roots;
+ SuperRoot changes = curRoots.copy();
+
+ src.reset();
+
+ src.descend(changes);
+
// It is necessary to reinitialize default values every time.
// Possible use case that explicitly requires it: creation of the same named list entry with slightly
// different set of values and different dynamic defaults at the same time.
@@ -375,7 +367,7 @@ public abstract class ConfigurationChanger {
SuperRoot patchedChanges = patch(changes, defaultsNode);
- cleanupMatchingValues(curRoots, changes);
+ cleanupMatchingValues(curRoots, patchedChanges);
Map<String, Serializable> allChanges = nodeToFlatMap(curRoots, patchedChanges);
@@ -384,7 +376,7 @@ public abstract class ConfigurationChanger {
return null;
List<ValidationIssue> validationIssues = ValidationUtil.validate(
- storageRoots.roots,
+ curRoots,
patch(patchedSuperRoot, defaultsNode),
this::getRootNode,
cachedAnnotations,
@@ -398,7 +390,7 @@ public abstract class ConfigurationChanger {
}, pool)
.thenCompose(allChanges -> {
if (allChanges == null)
- return CompletableFuture.completedFuture(true);
+ return completedFuture(true);
return storage.write(allChanges, storageRoots.version)
.exceptionally(throwable -> {
throw new ConfigurationChangeException("Failed to change configuration", throwable);
@@ -417,7 +409,7 @@ public abstract class ConfigurationChanger {
return CompletableFuture.failedFuture(e);
}
- return changeInternally(changes, storage);
+ return changeInternally(src, storage);
}
});
}
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicConfiguration.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicConfiguration.java
index 03073ae..79734f6 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicConfiguration.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicConfiguration.java
@@ -31,7 +31,6 @@ import org.apache.ignite.configuration.ConfigurationTree;
import org.apache.ignite.configuration.RootKey;
import org.apache.ignite.configuration.tree.ConfigurationSource;
import org.apache.ignite.configuration.tree.ConstructableTreeNode;
-import org.apache.ignite.configuration.tree.InnerNode;
/**
* This class represents configuration root or node.
@@ -71,30 +70,25 @@ public abstract class DynamicConfiguration<VIEW, CHANGE> extends ConfigurationNo
@Override public final Future<Void> change(Consumer<CHANGE> change) {
Objects.requireNonNull(change, "Configuration consumer cannot be null.");
- InnerNode rootNodeChange = changer.createRootNode(rootKey);
+ assert keys instanceof RandomAccess;
- if (keys.size() == 1) {
- // Current node is a root.
- change.accept((CHANGE)rootNodeChange);
- }
- else {
- assert keys instanceof RandomAccess;
+ ConfigurationSource src = new ConfigurationSource() {
+ private int level = 0;
- // Transform inner node closure into update tree.
- rootNodeChange.construct(keys.get(1), new ConfigurationSource() {
- private int level = 1;
+ @Override public void descend(ConstructableTreeNode node) {
+ if (level == keys.size())
+ change.accept((CHANGE)node);
+ else
+ node.construct(keys.get(level++), this);
+ }
- @Override public void descend(ConstructableTreeNode node) {
- if (++level == keys.size())
- change.accept((CHANGE)node);
- else
- node.construct(keys.get(level), this);
- }
- });
- }
+ @Override public void reset() {
+ level = 0;
+ }
+ };
// Use resulting tree as update request for the storage.
- return changer.change(Map.of(rootKey, rootNodeChange));
+ return changer.change(src, null);
}
/** {@inheritDoc} */
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicProperty.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicProperty.java
index 1b5bcb7..cba59aa 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicProperty.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/DynamicProperty.java
@@ -19,7 +19,6 @@ package org.apache.ignite.configuration.internal;
import java.io.Serializable;
import java.util.List;
-import java.util.Map;
import java.util.Objects;
import java.util.RandomAccess;
import java.util.concurrent.Future;
@@ -28,8 +27,6 @@ import org.apache.ignite.configuration.ConfigurationValue;
import org.apache.ignite.configuration.RootKey;
import org.apache.ignite.configuration.tree.ConfigurationSource;
import org.apache.ignite.configuration.tree.ConstructableTreeNode;
-import org.apache.ignite.configuration.tree.InnerNode;
-import org.apache.ignite.configuration.validation.ConfigurationValidationException;
/**
* Holder for property value. Expected to be used with numbers, strings and other immutable objects, e.g. IP addresses.
@@ -57,35 +54,36 @@ public class DynamicProperty<T extends Serializable> extends ConfigurationNode<T
}
/** {@inheritDoc} */
- @Override public Future<Void> update(T newValue) throws ConfigurationValidationException {
+ @Override public Future<Void> update(T newValue) {
Objects.requireNonNull(newValue, "Configuration value cannot be null.");
- InnerNode rootNodeChange = changer.createRootNode(rootKey);
-
assert keys instanceof RandomAccess;
assert !keys.isEmpty();
- // Transform leaf value into update tree.
- rootNodeChange.construct(keys.get(1), new ConfigurationSource() {
- private int level = 1;
+ ConfigurationSource src = new ConfigurationSource() {
+ private int level = 0;
@Override public void descend(ConstructableTreeNode node) {
- assert level < keys.size() - 1;
+ assert level < keys.size();
- node.construct(keys.get(++level), this);
+ node.construct(keys.get(level++), this);
}
@Override public <T> T unwrap(Class<T> clazz) {
- assert level == keys.size() - 1;
+ assert level == keys.size();
assert clazz.isInstance(newValue);
return clazz.cast(newValue);
}
- });
+
+ @Override public void reset() {
+ level = 0;
+ }
+ };
// Use resulting tree as update request for the storage.
- return changer.change(Map.of(rootKey, rootNodeChange));
+ return changer.change(src, null);
}
/** {@inheritDoc} */
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/SuperRoot.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/SuperRoot.java
index dbea55b..9596b6d 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/SuperRoot.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/SuperRoot.java
@@ -17,6 +17,7 @@
package org.apache.ignite.configuration.internal;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
@@ -58,7 +59,7 @@ public final class SuperRoot extends InnerNode {
this.nodeCreator = nodeCreator;
for (Map.Entry<RootKey<?, ?>, InnerNode> entry : roots.entrySet())
- this.roots.put(entry.getKey().key(), entry.getValue());
+ this.roots.put(entry.getKey().key(), entry.getValue().copy());
}
/**
@@ -94,8 +95,8 @@ public final class SuperRoot extends InnerNode {
/** {@inheritDoc} */
@Override public <T> void traverseChildren(ConfigurationVisitor<T> visitor) {
- for (Map.Entry<String, InnerNode> entry : roots.entrySet())
- visitor.visitInnerNode(entry.getKey(), entry.getValue());
+ for (String key : new LinkedHashSet<>(roots.keySet()))
+ visitor.visitInnerNode(key, roots.get(key));
}
/** {@inheritDoc} */
@@ -110,7 +111,11 @@ public final class SuperRoot extends InnerNode {
/** {@inheritDoc} */
@Override public void construct(String key, ConfigurationSource src) throws NoSuchElementException {
- assert src != null;
+ if (src == null) {
+ roots.remove(key);
+
+ return;
+ }
InnerNode root = roots.get(key);
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/asm/ConfigurationAsmGenerator.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/asm/ConfigurationAsmGenerator.java
index 8854545..2390639 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/asm/ConfigurationAsmGenerator.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/asm/ConfigurationAsmGenerator.java
@@ -555,14 +555,8 @@ public class ConfigurationAsmGenerator {
changeBody.append(changeMtd.getThis().setField(fieldDef, newValue));
}
else {
- // @ConfigValue fields might be null and have to be initialized before passing into closure.
- if (isConfigValue(schemaField)) {
- // if (this.field == null) this.field = new ValueNode();
- changeBody.append(new IfStatement()
- .condition(isNull(changeMtd.getThis().getField(fieldDef)))
- .ifTrue(changeMtd.getThis().setField(fieldDef, newInstance(fieldDef.getType())))
- );
- }
+ // this.field = (this.field == null) ? new ValueNode() : (ValueNode)this.field.copy();
+ changeBody.append(copyNodeField(changeMtd, fieldDef));
// change.accept(this.field);
changeBody.append(changeMtd.getScope().getVariable("change").invoke(
@@ -720,11 +714,7 @@ public class ConfigurationAsmGenerator {
.condition(isNull(srcVar))
.ifTrue(constructMtd.getThis().setField(fieldDef, constantNull(fieldDef.getType())))
.ifFalse(new BytecodeBlock()
- .append(constructMtd.getThis().setField(fieldDef, inlineIf(
- isNull(constructMtd.getThis().getField(fieldDef)),
- newInstance(fieldDef.getType()),
- constructMtd.getThis().getField(fieldDef).invoke(COPY).cast(fieldDef.getType())
- )))
+ .append(copyNodeField(constructMtd, fieldDef))
.append(srcVar.invoke(DESCEND, constructMtd.getThis().getField(fieldDef)))
)
);
@@ -832,6 +822,20 @@ public class ConfigurationAsmGenerator {
}
/**
+ * Copies field into itself or instantiates it if the field is null.
+ * @param mtd Method definition.
+ * @param fieldDef Field definition.
+ * @return Bytecode expression.
+ */
+ @NotNull private BytecodeExpression copyNodeField(MethodDefinition mtd, FieldDefinition fieldDef) {
+ return mtd.getThis().setField(fieldDef, inlineIf(
+ isNull(mtd.getThis().getField(fieldDef)),
+ newInstance(fieldDef.getType()),
+ mtd.getThis().getField(fieldDef).invoke(COPY).cast(fieldDef.getType())
+ ));
+ }
+
+ /**
* Creates {@code *Node::new} lambda expression with {@link Supplier} type.
* @param nodeClassName Name of the {@code *Node} class.
* @return InvokeDynamic bytecode expression.
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
index b199b43..1d9b01e 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
@@ -21,12 +21,14 @@ import java.io.Serializable;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.HashMap;
+import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.NoSuchElementException;
import java.util.Objects;
import java.util.RandomAccess;
import java.util.stream.Collectors;
+import org.apache.ignite.configuration.RootKey;
import org.apache.ignite.configuration.internal.SuperRoot;
import org.apache.ignite.configuration.tree.ConfigurationSource;
import org.apache.ignite.configuration.tree.ConfigurationVisitor;
@@ -391,17 +393,12 @@ public class ConfigurationUtil {
}
/**
- * Apply changes on top of existing node. Creates completely new object while reusing parts of the original tree
- * that weren't modified.
+ * Convert tree node into patching configuration source.
*
- * @param root Immutable configuration node.
- * @param changes Change or Init object to be applied.
- * @param <C> Type of the root.
- * @return Patched root.
+ * @param changes Tree node that contains prepared changes.
+ * @return Configuration source.
*/
- public static <C extends ConstructableTreeNode> C patch(C root, TraversableTreeNode changes) {
- assert root.getClass() == changes.getClass(); // Yes.
-
+ public static ConfigurationSource nodePatcher(TraversableTreeNode changes) {
var scrVisitor = new ConfigurationVisitor<ConfigurationSource>() {
@Override public ConfigurationSource visitInnerNode(String key, InnerNode node) {
return new PatchInnerConfigurationSource(node);
@@ -412,7 +409,39 @@ public class ConfigurationUtil {
}
};
- ConfigurationSource src = changes.accept(null, scrVisitor);
+ return changes.accept(null, scrVisitor);
+ }
+
+ /**
+ * Convert root node into patching configuration source for the super root.
+ *
+ * @param rootKey Root key.
+ * @param changes Root node.
+ * @return Configuration source.
+ */
+ public static ConfigurationSource superRootPatcher(RootKey rootKey, TraversableTreeNode changes) {
+ ConfigurationSource rootSrc = nodePatcher(changes);
+
+ return new ConfigurationSource() {
+ @Override public void descend(ConstructableTreeNode node) {
+ node.construct(rootKey.key(), rootSrc);
+ }
+ };
+ }
+
+ /**
+ * Apply changes on top of existing node. Creates completely new object while reusing parts of the original tree
+ * that weren't modified.
+ *
+ * @param root Immutable configuration node.
+ * @param changes Change or Init object to be applied.
+ * @param <C> Type of the root.
+ * @return Patched root.
+ */
+ public static <C extends ConstructableTreeNode> C patch(C root, TraversableTreeNode changes) {
+ assert root.getClass() == changes.getClass(); // Yes.
+
+ ConfigurationSource src = nodePatcher(changes);
assert src != null;
@@ -534,7 +563,12 @@ public class ConfigurationUtil {
/** {@inheritDoc} */
@Override public Void visitInnerNode(String key, InnerNode newInnerNode) {
- cleanupMatchingValues(curNode.traverseChild(key, innerNodeVisitor()), newInnerNode);
+ InnerNode oldInnerNode = curNode.traverseChild(key, innerNodeVisitor());
+
+ if (oldInnerNode == newInnerNode)
+ newNode.construct(key, null);
+ else
+ cleanupMatchingValues(oldInnerNode, newInnerNode);
return null;
}
@@ -543,8 +577,21 @@ public class ConfigurationUtil {
@Override public <N extends InnerNode> Void visitNamedListNode(String key, NamedListNode<N> newNamedList) {
NamedListNode<?> curNamedList = curNode.traverseChild(key, namedListNodeVisitor());
- for (String name : newNamedList.namedListKeys())
- cleanupMatchingValues(curNamedList.get(name), newNamedList.get(name));
+ if (curNamedList == newNamedList) {
+ newNode.construct(key, null);
+
+ return null;
+ }
+
+ for (String name : new LinkedHashSet<>(newNamedList.namedListKeys())) {
+ InnerNode oldElement = curNamedList.get(name);
+ N newElement = newNamedList.get(name);
+
+ if (oldElement == newElement)
+ newNamedList.forceDelete(name);
+ else
+ cleanupMatchingValues(oldElement, newElement);
+ }
return null;
}
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/ConfigurationSource.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/ConfigurationSource.java
index 14507a8..8483f2b 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/ConfigurationSource.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/ConfigurationSource.java
@@ -39,4 +39,10 @@ public interface ConfigurationSource {
*/
default void descend(ConstructableTreeNode node) {
}
+
+ /**
+ * Reset internal state, preparing this configuration source for reuse.
+ */
+ default void reset() {
+ }
}
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/NamedListNode.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/NamedListNode.java
index e1c1bfa..8eacd36 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/NamedListNode.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/tree/NamedListNode.java
@@ -84,6 +84,8 @@ public final class NamedListNode<N extends InnerNode> implements NamedListView<N
if (val == null)
map.put(key, val = valSupplier.get());
+ else
+ map.put(key, val = (N)val.copy());
valConsumer.accept(val);
@@ -92,9 +94,6 @@ public final class NamedListNode<N extends InnerNode> implements NamedListView<N
/** {@inheritDoc} */
@Override public NamedListChange<N> delete(String key) {
- if (map.containsKey(key) && map.get(key) != null)
- throw new IllegalStateException("You can't add entity that has just been modified [key=" + key + ']');
-
map.put(key, null);
return this;