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/11/25 12:22:29 UTC
[ignite-3] branch main updated: IGNITE-15971 Fix problems with dynamic configuration (#467)
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 8ab0d1d IGNITE-15971 Fix problems with dynamic configuration (#467)
8ab0d1d is described below
commit 8ab0d1d770962594669598cb8488bbf772184458
Author: Roman Puchkovskiy <ro...@gmail.com>
AuthorDate: Thu Nov 25 16:22:17 2021 +0400
IGNITE-15971 Fix problems with dynamic configuration (#467)
---
.../configuration/processor/ItProcessorTest.java | 12 ++
.../ConfigurationSchemaWithWrongPostfix.java | 27 +++
.../configuration/processor/Processor.java | 184 +++++++++++++--------
.../configuration/ConfigurationChanger.java | 30 ++--
.../configuration/ConfigurationManager.java | 4 -
.../configuration/ConfigurationRegistry.java | 44 +++--
.../asm/ConfigurationAsmGenerator.java | 20 ++-
.../internal/configuration/tree/InnerNode.java | 35 ++--
.../configuration/util/ConfigurationFlattener.java | 10 +-
.../configuration/util/ConfigurationUtil.java | 11 +-
.../configuration/ConfigurationRegistryTest.java | 19 +++
.../ConfigurationAnyListenerTest.java | 4 +-
.../notifications/ConfigurationListenerTest.java | 60 ++++++-
13 files changed, 320 insertions(+), 140 deletions(-)
diff --git a/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/configuration/processor/ItProcessorTest.java b/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/configuration/processor/ItProcessorTest.java
index 38b0669..638ea35 100644
--- a/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/configuration/processor/ItProcessorTest.java
+++ b/modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/configuration/processor/ItProcessorTest.java
@@ -292,4 +292,16 @@ public class ItProcessorTest extends AbstractProcessorTest {
return batchCompile(classes);
}
+
+ @Test
+ void wrongSchemaPostfix() {
+ String packageName = "org.apache.ignite.internal.configuration.processor";
+
+ ClassName schema = ClassName.get(packageName, "ConfigurationSchemaWithWrongPostfix");
+
+ Compilation compilation = compile(schema);
+
+ assertThat(compilation).failed();
+ assertThat(compilation).hadErrorContaining(schema + " must end with 'ConfigurationSchema'");
+ }
}
diff --git a/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/internal/configuration/processor/ConfigurationSchemaWithWrongPostfix.java b/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/internal/configuration/processor/ConfigurationSchemaWithWrongPostfix.java
new file mode 100644
index 0000000..f7382ee
--- /dev/null
+++ b/modules/configuration-annotation-processor/src/integrationTest/resources/org/apache/ignite/internal/configuration/processor/ConfigurationSchemaWithWrongPostfix.java
@@ -0,0 +1,27 @@
+/*
+ * 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.processor;
+
+import org.apache.ignite.configuration.annotation.ConfigurationRoot;
+
+/**
+ * All configuration schemas must end with 'ConfigurationSchema'.
+ */
+@ConfigurationRoot(rootName = "wrongPostfix")
+public class ConfigurationSchemaWithWrongPostfix {
+}
diff --git a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
index 3ce0bd9..8c3f356 100644
--- a/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
+++ b/modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
@@ -95,6 +95,9 @@ public class Processor extends AbstractProcessor {
/** Error format for an empty field. */
private static final String EMPTY_FIELD_ERROR_FORMAT = "Field %s cannot be empty: %s";
+
+ /** Postfix with which any configuration schema class name must end. */
+ private static final String CONFIGURATION_SCHEMA_POSTFIX = "ConfigurationSchema";
/** {@inheritDoc} */
@Override
@@ -395,7 +398,7 @@ public class Processor extends AbstractProcessor {
TypeName schemaFieldTypeName = TypeName.get(schemaFieldType);
boolean leafField = isPrimitiveOrArray(schemaFieldType)
- || !((ClassName) schemaFieldTypeName).simpleName().contains("ConfigurationSchema");
+ || !((ClassName) schemaFieldTypeName).simpleName().contains(CONFIGURATION_SCHEMA_POSTFIX);
boolean namedListField = field.getAnnotation(NamedConfigValue.class) != null;
@@ -550,86 +553,123 @@ public class Processor extends AbstractProcessor {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ String simpleName = clazz.getSimpleName().toString();
+
+ if (!simpleName.endsWith(CONFIGURATION_SCHEMA_POSTFIX)) {
+ throw new ProcessorException(
+ String.format("%s must end with '%s'", clazz.getQualifiedName(), CONFIGURATION_SCHEMA_POSTFIX));
+ }
+
if (clazz.getAnnotation(InternalConfiguration.class) != null) {
- checkIncompatibleClassAnnotations(
- clazz,
- InternalConfiguration.class,
- Config.class, PolymorphicConfig.class, PolymorphicConfigInstance.class
- );
-
- checkNotContainsPolymorphicIdField(clazz, InternalConfiguration.class, fields);
-
- if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
- checkNotExistSuperClass(clazz, InternalConfiguration.class);
- } else {
- checkExistSuperClass(clazz, InternalConfiguration.class);
-
- TypeElement superClazz = superClass(clazz);
-
- if (superClazz.getAnnotation(InternalConfiguration.class) != null) {
- throw new ProcessorException(String.format(
- "Superclass must not have %s: %s",
- simpleName(InternalConfiguration.class),
- clazz.getQualifiedName()
- ));
- }
-
- checkSuperclassContainAnyAnnotation(clazz, superClazz, ConfigurationRoot.class, Config.class);
-
- checkNoConflictFieldNames(clazz, superClazz, fields, fields(superClazz));
- }
+ validateInternalConfigurationSchemaClass(clazz, fields);
} else if (clazz.getAnnotation(PolymorphicConfig.class) != null) {
- checkIncompatibleClassAnnotations(
- clazz,
- PolymorphicConfig.class,
- ConfigurationRoot.class, Config.class, PolymorphicConfigInstance.class
- );
-
- checkNotExistSuperClass(clazz, PolymorphicConfig.class);
-
- List<VariableElement> typeIdFields = collectAnnotatedFields(fields, PolymorphicId.class);
-
- if (typeIdFields.size() != 1 || fields.indexOf(typeIdFields.get(0)) != 0) {
- throw new ProcessorException(String.format(
- "Class with %s must contain one field with %s and it should be the first in the schema: %s",
- simpleName(PolymorphicConfig.class),
- simpleName(PolymorphicId.class),
- clazz.getQualifiedName()
- ));
- }
+ validatePolymorphicConfigSchemaClass(clazz, fields);
} else if (clazz.getAnnotation(PolymorphicConfigInstance.class) != null) {
- checkIncompatibleClassAnnotations(
- clazz,
- PolymorphicConfigInstance.class,
- ConfigurationRoot.class, Config.class
- );
-
- checkNotContainsPolymorphicIdField(clazz, PolymorphicConfigInstance.class, fields);
-
- String id = clazz.getAnnotation(PolymorphicConfigInstance.class).value();
-
- if (id == null || id.isBlank()) {
+ validatePolymorphicConfigInstanceSchemaClass(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ checkNotContainsPolymorphicIdField(clazz, ConfigurationRoot.class, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ checkNotContainsPolymorphicIdField(clazz, Config.class, fields);
+ }
+ }
+
+ /**
+ * Checks configuration schema with {@link InternalConfiguration}.
+ *
+ * @param clazz type element under validation
+ * @param fields non-static fields of the class under validation
+ */
+ private void validateInternalConfigurationSchemaClass(TypeElement clazz, List<VariableElement> fields) {
+ checkIncompatibleClassAnnotations(
+ clazz,
+ InternalConfiguration.class,
+ Config.class, PolymorphicConfig.class, PolymorphicConfigInstance.class
+ );
+
+ checkNotContainsPolymorphicIdField(clazz, InternalConfiguration.class, fields);
+
+ if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ checkNotExistSuperClass(clazz, InternalConfiguration.class);
+ } else {
+ checkExistSuperClass(clazz, InternalConfiguration.class);
+
+ TypeElement superClazz = superClass(clazz);
+
+ if (superClazz.getAnnotation(InternalConfiguration.class) != null) {
throw new ProcessorException(String.format(
- EMPTY_FIELD_ERROR_FORMAT,
- simpleName(PolymorphicConfigInstance.class) + ".id()",
+ "Superclass must not have %s: %s",
+ simpleName(InternalConfiguration.class),
clazz.getQualifiedName()
));
}
-
- checkExistSuperClass(clazz, PolymorphicConfigInstance.class);
-
- TypeElement superClazz = superClass(clazz);
-
- checkSuperclassContainAnyAnnotation(clazz, superClazz, PolymorphicConfig.class);
-
+
+ checkSuperclassContainAnyAnnotation(clazz, superClazz, ConfigurationRoot.class, Config.class);
+
checkNoConflictFieldNames(clazz, superClazz, fields, fields(superClazz));
- } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
- checkNotContainsPolymorphicIdField(clazz, ConfigurationRoot.class, fields);
- } else if (clazz.getAnnotation(Config.class) != null) {
- checkNotContainsPolymorphicIdField(clazz, Config.class, fields);
}
}
-
+
+ /**
+ * Checks configuration schema with {@link PolymorphicConfig}.
+ *
+ * @param clazz type element under validation
+ * @param fields non-static fields of the class under validation
+ */
+ private void validatePolymorphicConfigSchemaClass(TypeElement clazz, List<VariableElement> fields) {
+ checkIncompatibleClassAnnotations(
+ clazz,
+ PolymorphicConfig.class,
+ ConfigurationRoot.class, Config.class, PolymorphicConfigInstance.class
+ );
+
+ checkNotExistSuperClass(clazz, PolymorphicConfig.class);
+
+ List<VariableElement> typeIdFields = collectAnnotatedFields(fields, PolymorphicId.class);
+
+ if (typeIdFields.size() != 1 || fields.indexOf(typeIdFields.get(0)) != 0) {
+ throw new ProcessorException(String.format(
+ "Class with %s must contain one field with %s and it should be the first in the schema: %s",
+ simpleName(PolymorphicConfig.class),
+ simpleName(PolymorphicId.class),
+ clazz.getQualifiedName()
+ ));
+ }
+ }
+
+ /**
+ * Checks configuration schema with {@link PolymorphicConfigInstance}.
+ *
+ * @param clazz type element under validation
+ * @param fields non-static fields of the class under validation
+ */
+ private void validatePolymorphicConfigInstanceSchemaClass(TypeElement clazz, List<VariableElement> fields) {
+ checkIncompatibleClassAnnotations(
+ clazz,
+ PolymorphicConfigInstance.class,
+ ConfigurationRoot.class, Config.class
+ );
+
+ checkNotContainsPolymorphicIdField(clazz, PolymorphicConfigInstance.class, fields);
+
+ String id = clazz.getAnnotation(PolymorphicConfigInstance.class).value();
+
+ if (id == null || id.isBlank()) {
+ throw new ProcessorException(String.format(
+ EMPTY_FIELD_ERROR_FORMAT,
+ simpleName(PolymorphicConfigInstance.class) + ".id()",
+ clazz.getQualifiedName()
+ ));
+ }
+
+ checkExistSuperClass(clazz, PolymorphicConfigInstance.class);
+
+ TypeElement superClazz = superClass(clazz);
+
+ checkSuperclassContainAnyAnnotation(clazz, superClazz, PolymorphicConfig.class);
+
+ checkNoConflictFieldNames(clazz, superClazz, fields, fields(superClazz));
+ }
+
/** {@inheritDoc} */
@Override
public Set<String> getSupportedAnnotationTypes() {
@@ -782,6 +822,7 @@ public class Processor extends AbstractProcessor {
* @param incompatibleAnnotations Incompatible class annotations with {@code clazzAnnotation}.
* @throws ProcessorException If there is an incompatible class annotation with {@code clazzAnnotation}.
*/
+ @SafeVarargs
private void checkIncompatibleClassAnnotations(
TypeElement clazz,
Class<? extends Annotation> clazzAnnotation,
@@ -900,6 +941,7 @@ public class Processor extends AbstractProcessor {
* @param superClazzAnnotations Superclass annotations.
* @throws ProcessorException If the superclass has none of the annotations from {@code superClazzAnnotations}.
*/
+ @SafeVarargs
private void checkSuperclassContainAnyAnnotation(
TypeElement clazz,
TypeElement superClazz,
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
index c84266d..7a2d430 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
@@ -385,24 +385,24 @@ public abstract class ConfigurationChanger implements DynamicConfigurationChange
return CompletableFuture
.supplyAsync(() -> {
SuperRoot curRoots = localRoots.roots;
-
+
SuperRoot changes = curRoots.copy();
-
+
src.reset();
-
+
src.descend(changes);
-
+
addDefaults(changes);
-
+
Map<String, Serializable> allChanges = createFlattenedUpdatesMap(curRoots, changes);
-
+
// Unlikely but still possible.
if (allChanges.isEmpty()) {
return null;
}
-
+
dropNulls(changes);
-
+
List<ValidationIssue> validationIssues = ValidationUtil.validate(
curRoots,
changes,
@@ -410,23 +410,25 @@ public abstract class ConfigurationChanger implements DynamicConfigurationChange
cachedAnnotations,
validators
);
-
+
if (!validationIssues.isEmpty()) {
throw new ConfigurationValidationException(validationIssues);
}
-
+
return allChanges;
}, pool)
.thenCompose(allChanges -> {
if (allChanges == null) {
return completedFuture(null);
}
-
+
return storage.write(allChanges, localRoots.version)
- .thenCompose(casResult -> {
- if (casResult) {
+ .thenCompose(casWroteSuccessfully -> {
+ if (casWroteSuccessfully) {
return localRoots.changeFuture;
} else {
+ // Here we go to next iteration of an implicit spin loop; we have to do it via recursion
+ // because we work with async code (futures).
return localRoots.changeFuture.thenCompose(v -> changeInternally(src));
}
})
@@ -435,7 +437,7 @@ public abstract class ConfigurationChanger implements DynamicConfigurationChange
});
});
}
-
+
/**
* Updates configuration from storage listener.
*
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationManager.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationManager.java
index 1854ee9..4956a94 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationManager.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationManager.java
@@ -17,8 +17,6 @@
package org.apache.ignite.internal.configuration;
-import static org.apache.ignite.internal.configuration.util.ConfigurationUtil.checkConfigurationType;
-
import com.typesafe.config.ConfigFactory;
import com.typesafe.config.ConfigObject;
import java.lang.annotation.Annotation;
@@ -63,8 +61,6 @@ public class ConfigurationManager implements IgniteComponent {
Collection<Class<?>> internalSchemaExtensions,
Collection<Class<?>> polymorphicSchemaExtensions
) {
- checkConfigurationType(rootKeys, storage);
-
registry = new ConfigurationRegistry(
rootKeys,
validators,
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
index 4b7f5e4..01e5474 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
@@ -112,9 +112,9 @@ public class ConfigurationRegistry implements IgniteComponent {
Collection<Class<?>> polymorphicSchemaExtensions
) {
checkConfigurationType(rootKeys, storage);
-
- Set<Class<?>> allSchemas = collectSchemas(viewReadOnly(rootKeys, RootKey::schemaClass));
-
+
+ Set<Class<?>> allSchemas = collectAllSchemas(rootKeys, internalSchemaExtensions, polymorphicSchemaExtensions);
+
final Map<Class<?>, Set<Class<?>>> internalExtensions = internalExtensionsWithCheck(allSchemas, internalSchemaExtensions);
final Map<Class<?>, Set<Class<?>>> polymorphicExtensions = polymorphicExtensionsWithCheck(allSchemas, polymorphicSchemaExtensions);
@@ -145,7 +145,27 @@ public class ConfigurationRegistry implements IgniteComponent {
configs.put(rootKey.key(), cfg);
});
}
-
+
+ /**
+ * Collects all schemas and subschemas (recursively) from root keys, internal and polymorphic schema extensions.
+ *
+ * @param rootKeys root keys
+ * @param internalSchemaExtensions internal schema extensions
+ * @param polymorphicSchemaExtensions polymorphic schema extensions
+ * @return set of all schema classes
+ */
+ private Set<Class<?>> collectAllSchemas(Collection<RootKey<?, ?>> rootKeys,
+ Collection<Class<?>> internalSchemaExtensions,
+ Collection<Class<?>> polymorphicSchemaExtensions) {
+ Set<Class<?>> allSchemas = new HashSet<>();
+
+ allSchemas.addAll(collectSchemas(viewReadOnly(rootKeys, RootKey::schemaClass)));
+ allSchemas.addAll(collectSchemas(internalSchemaExtensions));
+ allSchemas.addAll(collectSchemas(polymorphicSchemaExtensions));
+
+ return allSchemas;
+ }
+
/**
* Registers default validator implementation to the validators map.
*
@@ -321,13 +341,9 @@ public class ConfigurationRegistry implements IgniteComponent {
Set<Class<?>> allSchemas,
Collection<Class<?>> polymorphicSchemaExtensions
) {
- if (polymorphicSchemaExtensions.isEmpty()) {
- return Map.of();
- }
+ Map<Class<?>, Set<Class<?>>> polymorphicExtensionsByParent = polymorphicSchemaExtensions(polymorphicSchemaExtensions);
- Map<Class<?>, Set<Class<?>>> polymorphicExtensions = polymorphicSchemaExtensions(polymorphicSchemaExtensions);
-
- Set<Class<?>> notInAllSchemas = difference(polymorphicExtensions.keySet(), allSchemas);
+ Set<Class<?>> notInAllSchemas = difference(polymorphicExtensionsByParent.keySet(), allSchemas);
if (!notInAllSchemas.isEmpty()) {
throw new IllegalArgumentException(
@@ -337,7 +353,7 @@ public class ConfigurationRegistry implements IgniteComponent {
Collection<Class<?>> noPolymorphicExtensionsSchemas = allSchemas.stream()
.filter(ConfigurationUtil::isPolymorphicConfig)
- .filter(not(polymorphicExtensions::containsKey))
+ .filter(not(polymorphicExtensionsByParent::containsKey))
.collect(toList());
if (!noPolymorphicExtensionsSchemas.isEmpty()) {
@@ -346,9 +362,9 @@ public class ConfigurationRegistry implements IgniteComponent {
);
}
- checkPolymorphicConfigIds(polymorphicExtensions);
+ checkPolymorphicConfigIds(polymorphicExtensionsByParent);
- for (Map.Entry<Class<?>, Set<Class<?>>> e : polymorphicExtensions.entrySet()) {
+ for (Map.Entry<Class<?>, Set<Class<?>>> e : polymorphicExtensionsByParent.entrySet()) {
Class<?> schemaClass = e.getKey();
Field typeIdField = schemaFields(schemaClass).get(0);
@@ -362,7 +378,7 @@ public class ConfigurationRegistry implements IgniteComponent {
}
}
- return polymorphicExtensions;
+ return polymorphicExtensionsByParent;
}
/**
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
index e892fa9..623fa9b 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
@@ -392,7 +392,11 @@ public class ConfigurationAsmGenerator {
assert internalExtensions.isEmpty() || polymorphicExtensions.isEmpty() :
"Internal and polymorphic extensions are not allowed at the same time: " + schemaClass;
-
+ if (isPolymorphicConfig(schemaClass) && polymorphicExtensions.isEmpty()) {
+ throw new IllegalArgumentException(schemaClass
+ + " is polymorphic but polymorphic extensions are absent");
+ }
+
List<Field> schemaFields = schemaFields(schemaClass);
Collection<Field> internalExtensionsFields = extensionsFields(internalExtensions, true);
Collection<Field> polymorphicExtensionsFields = extensionsFields(polymorphicExtensions, false);
@@ -1324,7 +1328,7 @@ public class ConfigurationAsmGenerator {
.ret();
}
}
-
+
/**
* Implements {@link InnerNode#constructDefault(String)} method.
*
@@ -2560,8 +2564,8 @@ public class ConfigurationAsmGenerator {
.condition(isNull(thisField.getField(polymorphicIdField.getName(), String.class)))
.ifTrue(throwException(
IllegalStateException.class,
- constantString("Polymorphic configuration type is not defined: "
- + polymorphicIdField.getDeclaringClass().getName())
+ constantString(polymorphicTypeNotDefinedErrorMessage(
+ polymorphicIdField))
))
)
)
@@ -2611,7 +2615,13 @@ public class ConfigurationAsmGenerator {
return codeBlock;
}
-
+
+ private String polymorphicTypeNotDefinedErrorMessage(Field polymorphicIdField) {
+ return "Polymorphic configuration type is not defined: "
+ + polymorphicIdField.getDeclaringClass().getName()
+ + ". See @" + PolymorphicConfig.class.getSimpleName() + " documentation.";
+ }
+
/**
* Creates a bytecode block of code that sets the default value for a field from the schema for {@link
* InnerNode#constructDefault(String)}.
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java
index e987ca8..3ae360b 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java
@@ -28,11 +28,11 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
public final <T> T accept(String key, ConfigurationVisitor<T> visitor) {
return visitor.visitInnerNode(key, this);
}
-
+
/**
* Method with auto-generated implementation. Must look like this:
* <pre><code>
- * {@literal @}Override public void traverseChildren(ConfigurationVisitor visitor, boolean includeInternal) {
+ * {@literal @}Override public <T> void traverseChildren(ConfigurationVisitor visitor, boolean includeInternal) {
* visitor.visitInnerNode("pojoField1", this.pojoField1);
*
* visitor.visitNamedListNode("pojoField2", this.pojoField2);
@@ -52,29 +52,25 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
* @param <T> Parameter type of the passed visitor.
*/
public abstract <T> void traverseChildren(ConfigurationVisitor<T> visitor, boolean includeInternal);
-
+
/**
* Method with auto-generated implementation. Must look like this:
* <pre><code>
- * {@literal @}Override public void traverseChild(String key, ConfigurationVisitor visitor, boolean includeInternal) throws
+ * {@literal @}Override public <T> T traverseChild(String key, ConfigurationVisitor visitor, boolean includeInternal) throws
* NoSuchElementException {
- * if (boolean includeInternal) {
+ * if (includeInternal) {
* switch (key) {
* case "pojoField1":
- * visitor.visitInnerNode("pojoField1", this.pojoField1);
- * break;
+ * return visitor.visitInnerNode("pojoField1", this.pojoField1);
*
* case "pojoField2":
- * visitor.visitNamedListNode("pojoField2", this.pojoField2);
- * break;
+ * return visitor.visitNamedListNode("pojoField2", this.pojoField2);
*
* case "primitiveField1":
- * visitor.visitLeafNode("primitiveField1", this.primitiveField1);
- * break;
+ * return visitor.visitLeafNode("primitiveField1", this.primitiveField1);
*
* case "primitiveField2":
- * visitor.visitLeafNode("primitiveField2", this.primitiveField2);
- * break;
+ * return visitor.visitLeafNode("primitiveField2", this.primitiveField2);
*
* default:
* throw new NoSuchElementException(key);
@@ -83,8 +79,7 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
* else {
* switch (key) {
* case "primitiveField2":
- * visitor.visitLeafNode("primitiveField2", this.primitiveField2);
- * break;
+ * return visitor.visitLeafNode("primitiveField2", this.primitiveField2);
* default:
* throw new NoSuchElementException(key);
* }
@@ -104,7 +99,7 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
ConfigurationVisitor<T> visitor,
boolean includeInternal
) throws NoSuchElementException;
-
+
/**
* Method with auto-generated implementation. Must look like this:
* <pre><code>
@@ -156,7 +151,7 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
ConfigurationSource src,
boolean includeInternal
) throws NoSuchElementException;
-
+
/**
* Assigns default value to the corresponding leaf. Defaults are gathered from configuration schema class.
*
@@ -164,14 +159,14 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
* @throws NoSuchElementException If there's no such field or it is not a leaf value.
*/
public abstract void constructDefault(String fieldName) throws NoSuchElementException;
-
+
/**
* Returns class of corresponding configuration schema.
*
* @return Class of corresponding configuration schema.
*/
public abstract Class<?> schemaType();
-
+
/** {@inheritDoc} */
@Override
public InnerNode copy() {
@@ -181,7 +176,7 @@ public abstract class InnerNode implements TraversableTreeNode, ConstructableTre
throw new IllegalStateException(e);
}
}
-
+
/**
* Returns specific {@code Node} of the value. Overridden for polymorphic configuration to get a specific polymorphic configuration
* instance.
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
index e8f4279..5e9ce80 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
@@ -110,7 +110,7 @@ public class ConfigurationFlattener {
@Override
public Void doVisitLeafNode(String key, Serializable newVal) {
// Read same value from old tree.
- Serializable oldVal = oldInnerNodesStack.peek().traverseChild(key, ConfigurationUtil.leafNodeVisitor(), true);
+ Serializable oldVal = oldInnerNodesStack.element().traverseChild(key, ConfigurationUtil.leafNodeVisitor(), true);
// Do not put duplicates into the resulting map.
if (singleTreeTraversal || !Objects.deepEquals(oldVal, newVal)) {
@@ -119,12 +119,12 @@ public class ConfigurationFlattener {
return null;
}
-
+
/** {@inheritDoc} */
@Override
public Void doVisitInnerNode(String key, InnerNode newNode) {
// Read same node from old tree.
- InnerNode oldNode = oldInnerNodesStack.peek().traverseChild(key, ConfigurationUtil.innerNodeVisitor(), true);
+ InnerNode oldNode = oldInnerNodesStack.element().traverseChild(key, ConfigurationUtil.innerNodeVisitor(), true);
// Skip subtree that has not changed.
if (oldNode == newNode && !singleTreeTraversal) {
@@ -155,7 +155,7 @@ public class ConfigurationFlattener {
@Override
public Void doVisitNamedListNode(String key, NamedListNode<?> newNode) {
// Read same named list node from old tree.
- NamedListNode<?> oldNode = oldInnerNodesStack.peek().traverseChild(key, ConfigurationUtil.namedListNodeVisitor(), true);
+ NamedListNode<?> oldNode = oldInnerNodesStack.element().traverseChild(key, ConfigurationUtil.namedListNodeVisitor(), true);
// Skip subtree that has not changed.
if (oldNode == newNode && !singleTreeTraversal) {
@@ -211,7 +211,7 @@ public class ConfigurationFlattener {
Integer oldIdx = oldKeysToOrderIdxMap == null ? null : oldKeysToOrderIdxMap.get(newNodeKey);
// We should "persist" changed indexes only.
- if (newIdx != oldIdx || singleTreeTraversal || newNamedElement == null) {
+ if (!Objects.equals(newIdx, oldIdx) || singleTreeTraversal || newNamedElement == null) {
String orderKey = currentKey() + NamedListNode.ORDER_IDX;
resMap.put(orderKey, deletion || newNamedElement == null ? null : newIdx);
diff --git a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
index 6345404..9c3d4e6 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java
@@ -496,13 +496,18 @@ public class ConfigurationUtil {
while (!queue.isEmpty()) {
Class<?> cls = queue.poll();
- if (!cls.isAnnotationPresent(ConfigurationRoot.class) && !cls.isAnnotationPresent(Config.class)
- && !cls.isAnnotationPresent(PolymorphicConfig.class)) {
+ if (!cls.isAnnotationPresent(ConfigurationRoot.class)
+ && !cls.isAnnotationPresent(Config.class)
+ && !cls.isAnnotationPresent(InternalConfiguration.class)
+ && !cls.isAnnotationPresent(PolymorphicConfig.class)
+ && !cls.isAnnotationPresent(PolymorphicConfigInstance.class)) {
throw new IllegalArgumentException(String.format(
- "Configuration schema must contain @%s or @%s or @%s: %s",
+ "Configuration schema must contain one of @%s, @%s, @%s, @%s, @%s: %s",
ConfigurationRoot.class.getSimpleName(),
Config.class.getSimpleName(),
+ InternalConfiguration.class.getSimpleName(),
PolymorphicConfig.class.getSimpleName(),
+ PolymorphicConfigInstance.class.getSimpleName(),
cls.getName()
));
} else {
diff --git a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
index 2228915..e9dcd45 100644
--- a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
+++ b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
@@ -18,6 +18,8 @@
package org.apache.ignite.internal.configuration;
import static org.apache.ignite.configuration.annotation.ConfigurationType.LOCAL;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertThrows;
import java.util.List;
@@ -104,6 +106,23 @@ public class ConfigurationRegistryTest {
configRegistry.stop();
}
+
+ @Test
+ void missingPolymorphicExtension() {
+ IllegalArgumentException ex = assertThrows(
+ IllegalArgumentException.class,
+ () -> new ConfigurationRegistry(
+ List.of(ThirdRootConfiguration.KEY),
+ Map.of(),
+ new TestConfigurationStorage(LOCAL),
+ List.of(),
+ List.of()
+ )
+ );
+ assertThat(ex.getMessage(), is("Polymorphic configuration schemas for which no extensions were found: "
+ + "[class org.apache.ignite.internal.configuration.ConfigurationRegistryTest$"
+ + "FirstPolymorphicConfigurationSchema]"));
+ }
/**
* First root configuration.
diff --git a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationAnyListenerTest.java b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationAnyListenerTest.java
index fbdfcdb..fabd273 100644
--- a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationAnyListenerTest.java
+++ b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationAnyListenerTest.java
@@ -32,10 +32,10 @@ import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
import static org.junit.jupiter.api.Assertions.assertThrows;
-import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.UUID;
+import java.util.concurrent.CopyOnWriteArrayList;
import org.apache.ignite.configuration.ConfigurationListenOnlyException;
import org.apache.ignite.configuration.NamedConfigurationTree;
import org.apache.ignite.configuration.annotation.Config;
@@ -104,7 +104,7 @@ public class ConfigurationAnyListenerTest {
private RootConfiguration rootConfig;
/** Notification events. */
- private final List<String> events = new ArrayList<>();
+ private final List<String> events = new CopyOnWriteArrayList<>();
/**
* Before each.
diff --git a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java
index ceab2cb..458aa47 100644
--- a/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java
+++ b/modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java
@@ -27,6 +27,8 @@ import static org.apache.ignite.internal.configuration.notifications.Configurati
import static org.apache.ignite.internal.configuration.notifications.ConfigurationListenerTestUtils.configNamedListenerOnRename;
import static org.apache.ignite.internal.configuration.notifications.ConfigurationListenerTestUtils.configNamedListenerOnUpdate;
import static org.apache.ignite.internal.configuration.notifications.ConfigurationListenerTestUtils.doNothingConsumer;
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.Matchers.is;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertNull;
@@ -40,10 +42,14 @@ import java.util.UUID;
import java.util.concurrent.CompletableFuture;
import java.util.concurrent.CountDownLatch;
import java.util.concurrent.Future;
+import java.util.concurrent.atomic.AtomicInteger;
import org.apache.ignite.configuration.annotation.Config;
import org.apache.ignite.configuration.annotation.ConfigValue;
import org.apache.ignite.configuration.annotation.ConfigurationRoot;
import org.apache.ignite.configuration.annotation.NamedConfigValue;
+import org.apache.ignite.configuration.annotation.PolymorphicConfig;
+import org.apache.ignite.configuration.annotation.PolymorphicConfigInstance;
+import org.apache.ignite.configuration.annotation.PolymorphicId;
import org.apache.ignite.configuration.annotation.Value;
import org.apache.ignite.configuration.notifications.ConfigurationListener;
import org.apache.ignite.configuration.notifications.ConfigurationNamedListListener;
@@ -68,6 +74,9 @@ public class ConfigurationListenerTest {
@NamedConfigValue
public ChildConfigurationSchema elements;
+
+ @ConfigValue
+ public PolyConfigurationSchema polymorph;
}
/**
@@ -78,7 +87,40 @@ public class ConfigurationListenerTest {
@Value(hasDefault = true)
public String str = "default";
}
-
+
+ /**
+ * Base class for polymorhphic configs.
+ */
+ @PolymorphicConfig
+ public static class PolyConfigurationSchema {
+ public static final String STRING = "string";
+ public static final String LONG = "long";
+
+ @PolymorphicId(hasDefault = true)
+ public String type = STRING;
+
+ @Value(hasDefault = true)
+ public int commonIntVal = 11;
+ }
+
+ /**
+ * String-based variant of a polymorphic config.
+ */
+ @PolymorphicConfigInstance(PolyConfigurationSchema.STRING)
+ public static class StringPolyConfigurationSchema extends PolyConfigurationSchema {
+ @Value(hasDefault = true)
+ public String specificVal = "original";
+ }
+
+ /**
+ * Long-based variant of a polymorphic config.
+ */
+ @PolymorphicConfigInstance(PolyConfigurationSchema.LONG)
+ public static class LongPolyConfigurationSchema extends PolyConfigurationSchema {
+ @Value(hasDefault = true)
+ public long specificVal = 12;
+ }
+
private ConfigurationRegistry registry;
private ParentConfiguration configuration;
@@ -95,7 +137,7 @@ public class ConfigurationListenerTest {
Map.of(),
testConfigurationStorage,
List.of(),
- List.of()
+ List.of(StringPolyConfigurationSchema.class, LongPolyConfigurationSchema.class)
);
registry.start();
@@ -915,4 +957,18 @@ public class ConfigurationListenerTest {
configuration.elements().get(key).str().update(newVal).get(1, SECONDS);
}
+
+ @Test
+ void polymorphicParentFieldChangeNotificationHappens() throws Exception {
+ AtomicInteger intHolder = new AtomicInteger();
+
+ configuration.polymorph().commonIntVal().listen(event -> {
+ intHolder.set(event.newValue());
+ return CompletableFuture.completedFuture(null);
+ });
+
+ configuration.polymorph().commonIntVal().update(42).get(1, SECONDS);
+
+ assertThat(intHolder.get(), is(42));
+ }
}