tkalkirill commented on code in PR #2386:
URL: https://github.com/apache/ignite-3/pull/2386#discussion_r1287961892
##########
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 schemaExtensions All extensions (public and internal)
({@link ConfigurationExtension}) of configuration schemas
Review Comment:
```suggestion
* @param schemaExtensions Extensions (public and internal) ({@link
ConfigurationExtension}) of configuration schemas
```
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/annotation/ConfigurationExtension.java:
##########
@@ -39,5 +40,11 @@
@Target(TYPE)
@Retention(RUNTIME)
@Documented
-public @interface InternalConfiguration {
+public @interface ConfigurationExtension {
+ /**
+ * Controls whether this configuration is part of the public configuration
or is hidden from the end user.
+ * An extension is public by default.
+ */
+ boolean internal() default false;
+
Review Comment:
```suggestion
```
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -548,42 +549,64 @@ void testCheckConfigurationTypeNoError() {
}
@Test
- void testInternalSchemaExtensions() {
- assertTrue(internalSchemaExtensions(List.of()).isEmpty());
+ void testExtensionsType() {
+ // public extensions
+
assertTrue(ConfigurationUtil.isPublicExtension(PublicConfigurationSchema.class));
Review Comment:
```suggestion
// Public extensions.
assertTrue(ConfigurationUtil.isPublicExtension(PublicConfigurationSchema.class));
```
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -548,42 +549,64 @@ void testCheckConfigurationTypeNoError() {
}
@Test
- void testInternalSchemaExtensions() {
- assertTrue(internalSchemaExtensions(List.of()).isEmpty());
+ void testExtensionsType() {
Review Comment:
It is better to divide the test into several: public/internal
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -728,6 +774,84 @@ void testGetInternalConfigs() {
assertEquals("foo", subConfig.get("str02"));
}
+ @Test
+ void testGetPublicWithInternalConfigs() {
+ Map<Class<?>, Set<Class<?>>> extensions = schemaExtensions(List.of(
+ InternalFirstRootConfigurationSchema.class,
+ InternalSecondRootConfigurationSchema.class,
+ PublicRootConfigurationSchema.class,
+ InternalFirstConfigurationSchema.class,
+ InternalSecondConfigurationSchema.class,
+ PublicConfigurationSchema.class
+ ));
+
+ ConfigurationAsmGenerator generator = new ConfigurationAsmGenerator();
+ generator.compileRootSchema(InternalRootConfigurationSchema.class,
extensions, Map.of());
+
+ InnerNode innerNode =
generator.instantiateNode(InternalRootConfigurationSchema.class);
+
+ addDefaults(innerNode);
+
+ Map<String, Object> config = (Map<String, Object>) innerNode.accept(
+ null,
+ null,
+ ConverterToMapVisitor.builder()
+ .includeInternal(false)
+ .build()
+ );
+
+ // Check that public configuration will still be received.
+
Review Comment:
```suggestion
```
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -548,42 +549,64 @@ void testCheckConfigurationTypeNoError() {
}
@Test
- void testInternalSchemaExtensions() {
- assertTrue(internalSchemaExtensions(List.of()).isEmpty());
+ void testExtensionsType() {
+ // public extensions
+
assertTrue(ConfigurationUtil.isPublicExtension(PublicConfigurationSchema.class));
- assertThrows(IllegalArgumentException.class, () ->
internalSchemaExtensions(List.of(Object.class)));
+
assertTrue(ConfigurationUtil.isPublicExtension(PublicRootConfigurationSchema.class));
+
+ //internal extensions
Review Comment:
same
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -548,42 +549,64 @@ void testCheckConfigurationTypeNoError() {
}
@Test
- void testInternalSchemaExtensions() {
- assertTrue(internalSchemaExtensions(List.of()).isEmpty());
+ void testExtensionsType() {
+ // public extensions
+
assertTrue(ConfigurationUtil.isPublicExtension(PublicConfigurationSchema.class));
- assertThrows(IllegalArgumentException.class, () ->
internalSchemaExtensions(List.of(Object.class)));
+
assertTrue(ConfigurationUtil.isPublicExtension(PublicRootConfigurationSchema.class));
+
+ //internal extensions
+
assertTrue(ConfigurationUtil.isInternalExtension(InternalSecondConfigurationSchema.class));
+
+
assertTrue(ConfigurationUtil.isInternalExtension(InternalSecondRootConfigurationSchema.class));
+
+ //this one is not an extension at all
Review Comment:
same
##########
modules/configuration/src/test/java/org/apache/ignite/internal/configuration/util/ConfigurationUtilTest.java:
##########
@@ -728,6 +774,84 @@ void testGetInternalConfigs() {
assertEquals("foo", subConfig.get("str02"));
}
+ @Test
+ void testGetPublicWithInternalConfigs() {
+ Map<Class<?>, Set<Class<?>>> extensions = schemaExtensions(List.of(
+ InternalFirstRootConfigurationSchema.class,
+ InternalSecondRootConfigurationSchema.class,
+ PublicRootConfigurationSchema.class,
+ InternalFirstConfigurationSchema.class,
+ InternalSecondConfigurationSchema.class,
+ PublicConfigurationSchema.class
+ ));
+
+ ConfigurationAsmGenerator generator = new ConfigurationAsmGenerator();
+ generator.compileRootSchema(InternalRootConfigurationSchema.class,
extensions, Map.of());
+
+ InnerNode innerNode =
generator.instantiateNode(InternalRootConfigurationSchema.class);
+
+ addDefaults(innerNode);
+
+ Map<String, Object> config = (Map<String, Object>) innerNode.accept(
+ null,
+ null,
+ ConverterToMapVisitor.builder()
+ .includeInternal(false)
+ .build()
+ );
+
+ // Check that public configuration will still be received.
+
+ assertEquals(5, config.size());
+ assertNull(config.get("str0"));
+ assertEquals("foo", config.get("str1"));
+ assertNotNull(config.get("subCfg"));
+ assertNotNull(config.get("namedCfg"));
+ assertNotNull(config.get("publicSubCfg1"));
+
+ Map<String, Object> subConfig = (Map<String, Object>)
config.get("subCfg");
+
+ assertEquals(2, subConfig.size());
+ assertEquals("foo", subConfig.get("str00"));
+ assertEquals("bar", subConfig.get("publicStr03"));
+
+ // Check that public configuration will be received along with the
internal one.
+
Review Comment:
```suggestion
```
--
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
