tkalkirill commented on a change in pull request #467:
URL: https://github.com/apache/ignite-3/pull/467#discussion_r755141600
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
Review comment:
I think you can not put it into a separate method
##########
File path:
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationAnyListenerTest.java
##########
@@ -104,7 +104,7 @@
private RootConfiguration rootConfig;
/** Notification events. */
- private final List<String> events = new ArrayList<>();
+ private final List<String> events = new CopyOnWriteArrayList<>();
Review comment:
Better to use `ConcurrentLinkedQueue`
##########
File path:
modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/configuration/processor/ItProcessorTest.java
##########
@@ -292,4 +292,18 @@ private static BatchCompilation batchCompile(String
packageName, String... class
return batchCompile(classes);
}
+
+ @Test
+ void
confiurationSchemaClassShouldBeCheckedToEndWithConfigurationSchemaPostfix() {
+ String packageName =
"org.apache.ignite.internal.configuration.processor";
+
+ ClassName schema = ClassName.get(packageName,
"ConfigurationSchemaWithWrongPostfix");
+
+ Compilation compilation = compile(schema);
+
+ assertThat(compilation).failed();
+ assertThat(compilation).hadErrorContaining("Name of a class annotated
with one of [");
+ assertThat(compilation).hadErrorContaining(
+ "] must end with 'ConfigurationSchema', but for
'ConfigurationSchemaWithWrongPostfix' it does not");
Review comment:
Let's use the link to the class name
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -900,6 +932,7 @@ private void checkNoConflictFieldNames(
* @param superClazzAnnotations Superclass annotations.
* @throws ProcessorException If the superclass has none of the
annotations from {@code superClazzAnnotations}.
*/
+ @SafeVarargs // those are annotations, and we don't care about the actual
types
Review comment:
I think no comment is needed.
##########
File path:
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;
+import org.apache.ignite.configuration.annotation.Value;
+
+@ConfigurationRoot(rootName = "wrongPostfix")
+public class ConfigurationSchemaWithWrongPostfix {
Review comment:
Let's add what is wrong to the description: the schema class must end
with `ConfigurationSchema`.
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
Review comment:
Please separate this and line above by one blank line.
##########
File path:
modules/configuration-annotation-processor/src/integrationTest/java/org/apache/ignite/internal/configuration/processor/ItProcessorTest.java
##########
@@ -292,4 +292,18 @@ private static BatchCompilation batchCompile(String
packageName, String... class
return batchCompile(classes);
}
+
+ @Test
+ void
confiurationSchemaClassShouldBeCheckedToEndWithConfigurationSchemaPostfix() {
Review comment:
Let's rename to `wrongSchemaPostfix`
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
+ throw new ProcessorException(String.format(
+ "Name of a class annotated with one of %s must end with
'ConfigurationSchema',"
+ + " but for '%s' it does not",
supportedAnnotationTypes(), simpleName));
+ }
+ }
+
+ private void validateInternalConfiguration(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);
}
}
-
+
+ private void validatePolymorphicConfig(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()
+ ));
+ }
+ }
+
+ private void validatePolymorphicConfigInstance(TypeElement clazz,
List<VariableElement> fields) {
Review comment:
Add description
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
+ throw new ProcessorException(String.format(
+ "Name of a class annotated with one of %s must end with
'ConfigurationSchema',"
+ + " but for '%s' it does not",
supportedAnnotationTypes(), simpleName));
+ }
+ }
+
+ private void validateInternalConfiguration(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);
}
}
-
+
+ private void validatePolymorphicConfig(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()
+ ));
+ }
+ }
+
+ private void validatePolymorphicConfigInstance(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));
+ }
+
+ private void validateConfigurationRoot(TypeElement clazz,
List<VariableElement> fields) {
+ checkNotContainsPolymorphicIdField(clazz, ConfigurationRoot.class,
fields);
Review comment:
just one line of code, let's not put it into a separate method.
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
+ throw new ProcessorException(String.format(
+ "Name of a class annotated with one of %s must end with
'ConfigurationSchema',"
+ + " but for '%s' it does not",
supportedAnnotationTypes(), simpleName));
+ }
+ }
+
+ private void validateInternalConfiguration(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);
}
}
-
+
+ private void validatePolymorphicConfig(TypeElement clazz,
List<VariableElement> fields) {
Review comment:
Add description
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
+ throw new ProcessorException(String.format(
+ "Name of a class annotated with one of %s must end with
'ConfigurationSchema',"
+ + " but for '%s' it does not",
supportedAnnotationTypes(), simpleName));
+ }
+ }
+
+ private void validateInternalConfiguration(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);
}
}
-
+
+ private void validatePolymorphicConfig(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()
+ ));
+ }
+ }
+
+ private void validatePolymorphicConfigInstance(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));
+ }
+
+ private void validateConfigurationRoot(TypeElement clazz,
List<VariableElement> fields) {
+ checkNotContainsPolymorphicIdField(clazz, ConfigurationRoot.class,
fields);
+ }
+
+ private void validateConfig(TypeElement clazz, List<VariableElement>
fields) {
+ checkNotContainsPolymorphicIdField(clazz, Config.class, fields);
Review comment:
just one line of code, let's not put it into a separate method.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
##########
@@ -145,7 +148,26 @@ public InnerNode createRootNode(RootKey<?, ?> rootKey) {
configs.put(rootKey.key(), cfg);
});
}
-
+
+ private void checkPolymorphicSchemasValidity(Collection<RootKey<?, ?>>
rootKeys,
Review comment:
After a little research, I found that you only need to slightly change
the code in `ConfigurationRegistry#polymorphicExtensionsWithCheck`, and more
specifically remove the `if (polymorphicSchemaExtensions.isEmpty()) return;`
and add all the schemas from `polymorphicSchemaExtensions` and
`internalSchemaExtensions` to the `allSchemas` and everything will work.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -2611,7 +2622,13 @@ private BytecodeBlock treatSourceForConstruct(
return codeBlock;
}
-
+
+ private String polyConfTypeIsNotDefinedMessage(Field polymorphicIdField) {
+ return "Polymorphic configuration type is not defined: "
+ + polymorphicIdField.getDeclaringClass().getName()
+ + ". Maybe default value is missing on a @PolymorphicId field
(with hasDefault=true)?";
Review comment:
Maybe the default is not needed, I think you can refer to the
documentation for the annotation, and then describe how it should be,
It is possible not to create a separate method for the message in this case.
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -782,6 +813,7 @@ private boolean isStringClass(TypeMirror type) {
* @param incompatibleAnnotations Incompatible class annotations with
{@code clazzAnnotation}.
* @throws ProcessorException If there is an incompatible class annotation
with {@code clazzAnnotation}.
*/
+ @SafeVarargs // those are annotations, and we don't care about the actual
types
Review comment:
I think no comment is needed.
##########
File path:
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;
+import org.apache.ignite.configuration.annotation.Value;
+
+@ConfigurationRoot(rootName = "wrongPostfix")
+public class ConfigurationSchemaWithWrongPostfix {
+ @Value
+ public String value1;
Review comment:
No field needed
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
+ throw new ProcessorException(String.format(
Review comment:
A message like this is hard to read: `Name of a class annotated with one
of [interface org.apache.ignite.configuration.annotation.InternalConfiguration,
interface org.apache.ignite.configuration.annotation.PolymorphicConfig,
interface org.apache.ignite.configuration.annotation.Config, interface
org.apache.ignite.configuration.annotation.ConfigurationRoot, interface
org.apache.ignite.configuration.annotation.PolymorphicConfigInstance] must end
with 'ConfigurationSchema', but for 'ConfigurationSchemaWithWrongSuffix' it
does not.`
Let's shorten to:
org.apache.ignite.internal.configuration.processor.ConfigurationSchemaWithWrongSuffix
must end with 'ConfigurationSchema'.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationChanger.java
##########
@@ -383,59 +383,68 @@ public SuperRoot superRoot() {
StorageRoots localRoots = storageRoots;
return CompletableFuture
- .supplyAsync(() -> {
Review comment:
Let's not change the code here, it does not apply to your changes
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -1324,7 +1326,16 @@ private void addNodeConstructMethod(
.ret();
}
}
-
+
+ private void makeSureChangePolymorphicTypeIdMethodIsDefined(@Nullable
MethodDefinition changePolymorphicTypeIdMtd,
+ Field
schemaField) {
+ if (changePolymorphicTypeIdMtd == null) {
Review comment:
It is correct to check for the schema itself, not the field.
##########
File path:
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/Processor.java
##########
@@ -550,86 +550,117 @@ private boolean isObjectClass(TypeMirror type) {
* @throws ProcessorException If the class validation fails.
*/
private void validate(TypeElement clazz, List<VariableElement> fields) {
+ validateClassName(clazz);
+
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));
- }
+ validateInternalConfiguration(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()
- ));
- }
+ validatePolymorphicConfig(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()) {
+ validatePolymorphicConfigInstance(clazz, fields);
+ } else if (clazz.getAnnotation(ConfigurationRoot.class) != null) {
+ validateConfigurationRoot(clazz, fields);
+ } else if (clazz.getAnnotation(Config.class) != null) {
+ validateConfig(clazz, fields);
+ }
+ }
+
+ private void validateClassName(TypeElement clazz) {
+ String simpleName = clazz.getSimpleName().toString();
+ if (!simpleName.endsWith("ConfigurationSchema")) {
+ throw new ProcessorException(String.format(
+ "Name of a class annotated with one of %s must end with
'ConfigurationSchema',"
Review comment:
Let's move the `ConfigurationSchema` into a static variable, I see that
it is used a couple of times
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -1324,7 +1326,16 @@ private void addNodeConstructMethod(
.ret();
}
}
-
+
+ private void makeSureChangePolymorphicTypeIdMethodIsDefined(@Nullable
MethodDefinition changePolymorphicTypeIdMtd,
+ Field
schemaField) {
+ if (changePolymorphicTypeIdMtd == null) {
+ throw new IllegalStateException("Field " +
schemaField.getDeclaringClass().getName() + "."
+ + schemaField.getName() + " is annotated with
@PolymorphicId, but schema extensions are "
Review comment:
It would be more correct to make it `"@" +
PolymorphicId.class.getSimpleName()` so that if the name of the annotation
changes, it would change automatically.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -1238,6 +1238,8 @@ private void addNodeConstructMethod(
FieldDefinition fieldDef = fieldDefs.get(fieldName);
if (isPolymorphicId(schemaField)) {
+
makeSureChangePolymorphicTypeIdMethodIsDefined(changePolymorphicTypeIdMtd,
schemaField);
Review comment:
This is not the right place to check, it needs to be added on line 395.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -2611,7 +2622,13 @@ private BytecodeBlock treatSourceForConstruct(
return codeBlock;
}
-
+
+ private String polyConfTypeIsNotDefinedMessage(Field polymorphicIdField) {
Review comment:
Empty description, let's rename to
`polymorphicTypeNotDefinedErrorMessage`
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java
##########
@@ -52,29 +52,25 @@
* @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
Review comment:
What about `InnerNode#traverseChildren`?
##########
File path:
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java
##########
@@ -915,4 +957,17 @@ void testGetConfigFromNotificationEventOnUpdate() throws
Exception {
configuration.elements().get(key).str().update(newVal).get(1, SECONDS);
}
+
+ @Test
+ void
listenerGetsNotifiedOnCommonPropertyChangeInPolymorphicBaseConfigUnderNoNamedConfig()
throws Exception {
Review comment:
The name is very complicated
##########
File path:
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/ConfigurationRegistryTest.java
##########
@@ -104,6 +106,23 @@ void testValidationPolymorphicConfigurationExtensions() {
configRegistry.stop();
}
+
+ @Test
+ void polymorphicSchemasShouldBeRequiredIfPolymorphicIdIsUsed() {
Review comment:
Let's rename to `missingPolymorphicExtension`
##########
File path:
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/notifications/ConfigurationListenerTest.java
##########
@@ -915,4 +957,17 @@ void testGetConfigFromNotificationEventOnUpdate() throws
Exception {
configuration.elements().get(key).str().update(newVal).get(1, SECONDS);
}
+
+ @Test
+ void
listenerGetsNotifiedOnCommonPropertyChangeInPolymorphicBaseConfigUnderNoNamedConfig()
throws Exception {
+ AtomicInteger intHolder = new AtomicInteger();
Review comment:
Add new line below
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationFlattener.java
##########
@@ -119,12 +120,21 @@ public Void doVisitLeafNode(String key, Serializable
newVal) {
return null;
}
-
+
+ @NotNull
+ private InnerNode currentOldInnerNode() {
Review comment:
Why this method?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
