tkalkirill commented on code in PR #2386:
URL: https://github.com/apache/ignite-3/pull/2386#discussion_r1286639201
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java:
##########
@@ -78,12 +78,12 @@ public interface ConfigurationModule {
}
/**
- * Returns classes of internal schema extensions (annotated with {@link
InternalConfiguration})
- * provided by this module.
+ * Returns classes of schema extensions (annotated with {@link
ConfigurationExtension})
+ * provided by this module, including internal extensions.
*
- * @return internal schema extensions' classes
+ * @return all schema extensions' classes including internal
Review Comment:
```suggestion
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationTreeGenerator.java:
##########
@@ -74,22 +74,22 @@ public ConfigurationTreeGenerator(RootKey<?, ?>...
rootKeys) {
* Constructor that takes a collection of root keys and a collection of
internal schema extensions.
*
* @param rootKeys Root keys.
- * @param internalSchemaExtensions Internal schema extensions.
+ * @param allSchemaExtensions Schema extensions.
* @param polymorphicSchemaExtensions Polymorphic schema extensions.
*/
public ConfigurationTreeGenerator(
Collection<RootKey<?, ?>> rootKeys,
- Collection<Class<?>> internalSchemaExtensions,
+ Collection<Class<?>> allSchemaExtensions,
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationTreeGenerator.java:
##########
@@ -74,22 +74,22 @@ public ConfigurationTreeGenerator(RootKey<?, ?>...
rootKeys) {
* Constructor that takes a collection of root keys and a collection of
internal schema extensions.
*
* @param rootKeys Root keys.
- * @param internalSchemaExtensions Internal schema extensions.
+ * @param allSchemaExtensions Schema extensions.
* @param polymorphicSchemaExtensions Polymorphic schema extensions.
*/
public ConfigurationTreeGenerator(
Collection<RootKey<?, ?>> rootKeys,
- Collection<Class<?>> internalSchemaExtensions,
+ Collection<Class<?>> allSchemaExtensions,
Collection<Class<?>> polymorphicSchemaExtensions) {
this.rootKeys = rootKeys.stream().collect(toMap(RootKey::key,
identity()));
- Set<Class<?>> allSchemas = collectAllSchemas(rootKeys,
internalSchemaExtensions, polymorphicSchemaExtensions);
+ Set<Class<?>> allSchemas = collectAllSchemas(rootKeys,
allSchemaExtensions, polymorphicSchemaExtensions);
Review Comment:
same
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java:
##########
@@ -78,12 +78,12 @@ public interface ConfigurationModule {
}
/**
- * Returns classes of internal schema extensions (annotated with {@link
InternalConfiguration})
- * provided by this module.
+ * Returns classes of schema extensions (annotated with {@link
ConfigurationExtension})
+ * provided by this module, including internal extensions.
*
- * @return internal schema extensions' classes
+ * @return all schema extensions' classes including internal
*/
- default Collection<Class<?>> internalSchemaExtensions() {
+ default Collection<Class<?>> allSchemaExtensions() {
Review Comment:
Polymorphic configuration is also an extension of schemas, the name is not
quite suitable, I think we need to remove the `all` prefix.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/AbstractAsmGenerator.java:
##########
@@ -64,17 +64,20 @@ abstract class AbstractAsmGenerator {
/** Configuration schema class. */
final Class<?> schemaClass;
- /** Internal extensions of the configuration schema. */
- final Set<Class<?>> internalExtensions;
+ /** Extensions of the configuration schema (including internal ones). */
+ final Set<Class<?>> allConfigurationExtensions;
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/AbstractAsmGenerator.java:
##########
@@ -64,17 +64,20 @@ abstract class AbstractAsmGenerator {
/** Configuration schema class. */
final Class<?> schemaClass;
- /** Internal extensions of the configuration schema. */
- final Set<Class<?>> internalExtensions;
+ /** Extensions of the configuration schema (including internal ones). */
+ final Set<Class<?>> allConfigurationExtensions;
/** Polymorphic extensions of the configuration schema. */
final Set<Class<?>> polymorphicExtensions;
/** Fields of the schema class. */
final List<Field> schemaFields;
+ /** Fields of extensions of the configuration schema. */
+ final Collection<Field> extensionFields;
Review Comment:
It would be necessary to indicate that public extensions and do not forget
about the javaDoc.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationTreeGenerator.java:
##########
@@ -177,30 +177,30 @@ private static Set<Class<?>>
collectAllSchemas(Collection<RootKey<?, ?>> rootKey
* Get configuration schemas and their validated internal extensions with
checks.
*
* @param allSchemas All configuration schemas.
- * @param internalSchemaExtensions Internal extensions ({@link
InternalConfiguration}) of configuration schemas
+ * @param schemaExtensions Extensions ({@link ConfigurationExtension}) of
configuration schemas
* ({@link ConfigurationRoot} and {@link Config}).
* @return Mapping: original of the schema -> internal schema extensions.
* @throws IllegalArgumentException If the schema extension is invalid.
*/
- private Map<Class<?>, Set<Class<?>>> internalExtensionsWithCheck(
+ private Map<Class<?>, Set<Class<?>>> allConfigurationExtensionsWithCheck(
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationImplAsmGenerator.java:
##########
@@ -150,20 +150,22 @@ class ConfigurationImplAsmGenerator extends
AbstractAsmGenerator {
ConfigurationImplAsmGenerator(
ConfigurationAsmGenerator cgen,
Class<?> schemaClass,
- Set<Class<?>> internalExtensions,
+ Set<Class<?>> allConfigurationExtensions,
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java:
##########
@@ -232,20 +232,22 @@ class InnerNodeAsmGenerator extends AbstractAsmGenerator {
InnerNodeAsmGenerator(
ConfigurationAsmGenerator cgen,
Class<?> schemaClass,
- Set<Class<?>> internalExtensions,
+ Set<Class<?>> allConfigurationExtensions,
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java:
##########
@@ -192,11 +193,11 @@ public synchronized void compileRootSchema(
assert schemasInfo.containsKey(schemaClass) : schemaClass;
- Set<Class<?>> internalExtensions =
internalSchemaExtensions.getOrDefault(schemaClass, Set.of());
+ Set<Class<?>> allExtensions =
allSchemaExtensions.getOrDefault(schemaClass, Set.of());
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java:
##########
@@ -677,6 +677,16 @@ public static Collection<Field>
extensionsFields(Collection<Class<?>> extensions
}
}
+ public static boolean isInternalExtension(Class<?> schemaClass) {
+ ConfigurationExtension ext =
schemaClass.getAnnotation(ConfigurationExtension.class);
+ return ext != null && ext.internal();
+ }
+
+ public static boolean isExtension(Class<?> schemaClass) {
Review Comment:
maybe **isPublicExtension**
##########
modules/runner/src/main/java/org/apache/ignite/internal/configuration/CompoundModule.java:
##########
@@ -69,8 +69,8 @@ private <T> List<T> unionFromModulesExtractedWith(Function<?
super Configuration
/** {@inheritDoc} */
@Override
- public Collection<Class<?>> internalSchemaExtensions() {
- return
unionFromModulesExtractedWith(ConfigurationModule::internalSchemaExtensions);
+ public Collection<Class<?>> allSchemaExtensions() {
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/util/ConfigurationUtil.java:
##########
@@ -570,15 +570,15 @@ public static List<String> classNames(Field... fields) {
}
/**
- * Get configuration schemas and their validated internal extensions.
+ * Get configuration schemas and their validated extensions.
*
- * @param extensions Schema extensions with {@link InternalConfiguration}.
- * @return Mapping: original of the scheme -> internal schema extensions.
+ * @param extensions Schema extensions with {@link ConfigurationExtension}.
+ * @return Mapping: original of the scheme -> schema extensions.
* @throws IllegalArgumentException If the schema extension is invalid.
- * @see InternalConfiguration
+ * @see ConfigurationExtension
*/
- public static Map<Class<?>, Set<Class<?>>>
internalSchemaExtensions(Collection<Class<?>> extensions) {
- return schemaExtensions(extensions, InternalConfiguration.class);
+ public static Map<Class<?>, Set<Class<?>>>
allSchemaExtensions(Collection<Class<?>> extensions) {
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java:
##########
@@ -264,10 +264,10 @@ public void setInjectedNameFieldValue(String value) {
}
/**
- * Returns schemas for {@link InternalConfiguration internal configuration
extensions}.
+ * Returns schemas for {@link ConfigurationExtension configuration
extensions}.
*/
@Nullable
Review Comment:
why ignore?
##########
modules/configuration/src/testFixtures/java/org/apache/ignite/internal/configuration/testframework/ConfigurationExtension.java:
##########
@@ -98,8 +97,8 @@ public class ConfigurationExtension implements
BeforeEachCallback, AfterEachCall
/** Key to store {@link StorageRevisionListenerHolderImpl} in {@link
ExtensionContext.Store} for each test. */
private static final Object REVISION_LISTENER_PER_TEST_HOLDER_KEY = new
Object();
- /** All {@link InternalConfiguration} classes in classpath. */
- private static final List<Class<?>> INTERNAL_EXTENSIONS;
+ /** All {@link ConfigurationExtension} classes in classpath. */
+ private static final List<Class<?>> ALL_EXTENSIONS;
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/InnerNode.java:
##########
@@ -264,10 +264,10 @@ public void setInjectedNameFieldValue(String value) {
}
/**
- * Returns schemas for {@link InternalConfiguration internal configuration
extensions}.
+ * Returns schemas for {@link ConfigurationExtension configuration
extensions}.
*/
@Nullable
- public Class<?>[] internalSchemaTypes() {
+ public Class<?>[] extensionSchemaTypes() {
Review Comment:
why ignore?
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/DirectProxyAsmGenerator.java:
##########
@@ -100,18 +100,20 @@ class DirectProxyAsmGenerator extends
AbstractAsmGenerator {
DirectProxyAsmGenerator(
ConfigurationAsmGenerator cgen,
Class<?> schemaClass,
- Set<Class<?>> internalExtensions,
+ Set<Class<?>> allConfigurationExtensions,
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/AbstractAsmGenerator.java:
##########
@@ -89,19 +92,21 @@ abstract class AbstractAsmGenerator {
AbstractAsmGenerator(
ConfigurationAsmGenerator cgen,
Class<?> schemaClass,
- Set<Class<?>> internalExtensions,
+ Set<Class<?>> allConfigurationExtensions,
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java:
##########
@@ -160,14 +161,14 @@ public synchronized InnerNode instantiateNode(Class<?>
schemaClass) {
* Generates, defines, loads and initializes all dynamic classes required
for the given configuration schema.
*
* @param rootSchemaClass Class of the root configuration
schema.
- * @param internalSchemaExtensions Internal extensions ({@link
InternalConfiguration}) of configuration schemas ({@link
- * ConfigurationRoot} and {@link
Config}). Mapping: original schema -> extensions.
+ * @param allSchemaExtensions All extensions (including internal)
({@link ConfigurationExtension}) of configuration schemas
+ * {@link ConfigurationRoot} and {@link
Config}). Mapping: original schema -> extensions.
* @param polymorphicSchemaExtensions Polymorphic extensions ({@link
PolymorphicConfigInstance}) of configuration schemas ({@link
* PolymorphicConfig}). Mapping:
original schema -> extensions.
*/
public synchronized void compileRootSchema(
Class<?> rootSchemaClass,
- Map<Class<?>, Set<Class<?>>> internalSchemaExtensions,
+ Map<Class<?>, Set<Class<?>>> allSchemaExtensions,
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationTreeGenerator.java:
##########
@@ -74,22 +74,22 @@ public ConfigurationTreeGenerator(RootKey<?, ?>...
rootKeys) {
* Constructor that takes a collection of root keys and a collection of
internal schema extensions.
*
* @param rootKeys Root keys.
- * @param internalSchemaExtensions Internal schema extensions.
+ * @param allSchemaExtensions Schema extensions.
* @param polymorphicSchemaExtensions Polymorphic schema extensions.
*/
public ConfigurationTreeGenerator(
Collection<RootKey<?, ?>> rootKeys,
- Collection<Class<?>> internalSchemaExtensions,
+ Collection<Class<?>> allSchemaExtensions,
Collection<Class<?>> polymorphicSchemaExtensions) {
this.rootKeys = rootKeys.stream().collect(toMap(RootKey::key,
identity()));
- Set<Class<?>> allSchemas = collectAllSchemas(rootKeys,
internalSchemaExtensions, polymorphicSchemaExtensions);
+ Set<Class<?>> allSchemas = collectAllSchemas(rootKeys,
allSchemaExtensions, polymorphicSchemaExtensions);
- Map<Class<?>, Set<Class<?>>> internalExtensions =
internalExtensionsWithCheck(allSchemas, internalSchemaExtensions);
+ Map<Class<?>, Set<Class<?>>> allConfigurationExtensions =
allConfigurationExtensionsWithCheck(allSchemas, allSchemaExtensions);
Review Comment:
same
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -549,21 +549,21 @@ void testCheckConfigurationTypeNoError() {
@Test
void testInternalSchemaExtensions() {
Review Comment:
why ignore?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscr...@ignite.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
