tkalkirill commented on a change in pull request #290:
URL: https://github.com/apache/ignite-3/pull/290#discussion_r698570089
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java
##########
@@ -99,25 +124,24 @@ public ConfigurationRegistry(
return cgen.instantiateNode(rootKey.schemaClass());
}
};
- }
- /** {@inheritDoc} */
- @Override public void start() {
rootKeys.forEach(rootKey -> {
- cgen.compileRootSchema(rootKey.schemaClass());
+ cgen.compileRootSchema(rootKey.schemaClass(), extensions);
DynamicConfiguration<?, ?> cfg = cgen.instantiateCfg(rootKey,
changer);
configs.put(rootKey.key(), cfg);
});
+ }
+ /** {@inheritDoc} */
+ @Override public void start() {
changer.start();
}
/** {@inheritDoc} */
@Override public void stop() {
- if (changer != null)
- changer.stop();
+ changer.stop();
Review comment:
Since the **changer** has become `final`.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/SuperRoot.java
##########
@@ -28,109 +26,120 @@
import org.apache.ignite.internal.configuration.tree.ConfigurationSource;
import org.apache.ignite.internal.configuration.tree.ConfigurationVisitor;
import org.apache.ignite.internal.configuration.tree.InnerNode;
+import org.apache.ignite.lang.IgniteBiTuple;
+import org.jetbrains.annotations.Nullable;
-/** */
+/**
+ * Holder of root configurations.
+ */
public final class SuperRoot extends InnerNode {
- /** */
- private final SortedMap<String, InnerNode> roots = new TreeMap<>();
+ /** Root configurations. Mapping: {@link RootKey#key} -> key :
configuration. */
+ private final SortedMap<String, IgniteBiTuple<RootKey<?, ?>, InnerNode>>
roots = new TreeMap<>();
Review comment:
I need a function that returns two objects, but your proposal will only
have one.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/ConfigurationVisitorImpl.java
##########
@@ -29,18 +29,26 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
-import com.typesafe.config.ConfigValue;
-import org.apache.ignite.internal.configuration.tree.ConfigurationVisitor;
-import org.apache.ignite.internal.configuration.tree.InnerNode;
-import org.apache.ignite.internal.configuration.tree.NamedListNode;
/**
* {@link ConfigurationVisitor} implementation that converts a configuration
tree to a combination of {@link Map} and
- * {@link List} objects to convert the result into HOCON {@link ConfigValue}
instance.
+ * {@link List} objects.
*/
-class HoconConfigurationVisitor implements ConfigurationVisitor<Object> {
+public class ConfigurationVisitorImpl implements ConfigurationVisitor<Object> {
Review comment:
What about `ConverterConfigToMapObject ` ?
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -291,26 +318,44 @@ public synchronized void compileRootSchema(Class<?>
rootSchemaClass) {
}
}
- private ClassDefinition createNodeClass(Class<?> schemaClass) {
+ /**
+ * Construct a {@link InnerNode} definition for a configuration schema.
+ *
+ * @param schemaClass Configuration schema class.
+ * @param schemaFields Fields of the schema and its extensions.
+ * @param schemaExtensions Internal extensions of the configuration schema.
+ * @return Constructed {@link InnerNode} definition for the configuration
schema.
+ */
+ private ClassDefinition createNodeClass(
+ Class<?> schemaClass,
+ Set<Field> schemaFields,
+ Set<Class<?>> schemaExtensions
+ ) {
SchemaClassesInfo schemaClassInfo = schemasInfo.get(schemaClass);
+ ParameterizedType[] interfaces = union(schemaExtensions,
schemaClass).stream()
+ .flatMap(cls -> Stream.of(prefix(cls) + VIEW_CLASS_POSTFIX,
prefix(cls) + CHANGE_CLASS_POSTFIX))
+ .map(ParameterizedType::typeFromJavaClassName)
+ .toArray(ParameterizedType[]::new);
+
// Node class definition.
ClassDefinition classDef = new ClassDefinition(
of(PUBLIC, FINAL),
internalName(schemaClassInfo.nodeClassName),
type(InnerNode.class),
- typeFromJavaClassName(schemaClassInfo.viewClassName),
- typeFromJavaClassName(schemaClassInfo.changeClassName)
+ interfaces
);
- // Spec field.
- FieldDefinition specField = classDef.declareField(of(PRIVATE, FINAL),
"_spec", schemaClass);
+ // Spec fields.
+ Map<Class<?>, FieldDefinition> specFields = new HashMap<>();
- // org.apache.ignite.internal.configuration.tree.InnerNode#schemaType
- addNodeSchemaTypeMethod(classDef, specField);
+ int i = 0;
- // Cached array of reflected fields. Helps to avoid unnecessary
clonings.
- Field[] schemaFields = schemaClass.getDeclaredFields();
+ for (Class<?> schemaCls : union(schemaExtensions, schemaClass))
+ specFields.put(schemaCls, classDef.declareField(of(PRIVATE,
FINAL), "_spec" + i++, schemaCls));
Review comment:
I suggest leaving it as it is.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/ConfigurationAsmGenerator.java
##########
@@ -431,26 +460,27 @@ else if (isConfigValue(schemaField))
/**
* Implements default constructor for the node class. It initializes
{@code _spec} field and every other field
* that represents named list configuration.
+ *
* @param classDef Node class definition.
- * @param schemaClass Configuration Schema class.
- * @param specField Field definition for the {@code _spec} field of the
node class.
- * @param schemaFields Configuration Schema class fields.
+ * @param specFields Definition of fields for the {@code _spec#} fields of
the node class.
+ * Mapping: configuration schema class -> {@code _spec#} field.
+ * @param schemaFields Fields of the schema and its extensions.
* @param fieldDefs Field definitions for all fields of node class
excluding {@code _spec}.
*/
private void addNodeConstructor(
ClassDefinition classDef,
- Class<?> schemaClass,
- FieldDefinition specField,
- Field[] schemaFields,
+ Map<Class<?>, FieldDefinition> specFields,
+ Set<Field> schemaFields,
Map<String, FieldDefinition> fieldDefs
) {
MethodDefinition ctor = classDef.declareConstructor(of(PUBLIC));
// super();
ctor.getBody().append(ctor.getThis()).invokeConstructor(InnerNode.class);
- // this._spec = new MyConfigurationSchema();
- ctor.getBody().append(ctor.getThis().setField(specField,
newInstance(schemaClass)));
+ // this._spec# = new MyConfigurationSchema();
+ for (Map.Entry<Class<?>, FieldDefinition> e : specFields.entrySet())
+ ctor.getBody().append(ctor.getThis().setField(e.getValue(),
newInstance(e.getKey())));
Review comment:
It looks like optimization, I propose to do it in a separate ticket.
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/tree/ConfigurationVisitorImpl.java
##########
@@ -29,18 +29,26 @@
import java.util.stream.Collectors;
import java.util.stream.IntStream;
import java.util.stream.Stream;
-import com.typesafe.config.ConfigValue;
-import org.apache.ignite.internal.configuration.tree.ConfigurationVisitor;
-import org.apache.ignite.internal.configuration.tree.InnerNode;
-import org.apache.ignite.internal.configuration.tree.NamedListNode;
/**
* {@link ConfigurationVisitor} implementation that converts a configuration
tree to a combination of {@link Map} and
- * {@link List} objects to convert the result into HOCON {@link ConfigValue}
instance.
+ * {@link List} objects.
*/
-class HoconConfigurationVisitor implements ConfigurationVisitor<Object> {
+public class ConfigurationVisitorImpl implements ConfigurationVisitor<Object> {
Review comment:
I think it's ok.
--
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]
