tkalkirill commented on code in PR #1540:
URL: https://github.com/apache/ignite-3/pull/1540#discussion_r1073333953
##########
modules/configuration-annotation-processor/src/main/java/org/apache/ignite/internal/configuration/processor/ConfigurationProcessor.java:
##########
@@ -518,6 +518,14 @@ private void createPojoBindings(
}
changeClsBuilder.addMethod(changeMtdBuilder.build());
+
+ if (valAnnotation == null) {
Review Comment:
Please add an example of what it will be and describe what it is for.
I also don't see a test for this logic.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java:
##########
@@ -345,15 +346,15 @@ private ClassDefinition createNodeClass() {
}
// Add change methods.
- MethodDefinition changeMtd0 = addNodeChangeMethod(
+ List<MethodDefinition> changeMethods = addNodeChangeMethod(
innerNodeClassDef,
schemaField,
changeMtd -> getThisFieldCode(changeMtd, fieldDef),
(changeMtd, newValue) -> setThisFieldCode(changeMtd,
newValue, fieldDef),
null
);
- addNodeChangeBridgeMethod(innerNodeClassDef,
changeClassName(schemaField.getDeclaringClass()), changeMtd0);
+ addNodeChangeBridgeMethod(innerNodeClassDef,
changeClassName(schemaField.getDeclaringClass()), changeMethods.get(0));
Review Comment:
Please add comment why `get(0)`.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java:
##########
@@ -1610,15 +1646,15 @@ private ClassDefinition
createPolymorphicExtensionNodeClass(
viewMtd -> getThisFieldCode(viewMtd,
parentInnerNodeFieldDef, polymorphicTypeIdFieldDef)
);
- MethodDefinition changeMtd0 = addNodeChangeMethod(
+ List<MethodDefinition> changeMethods = addNodeChangeMethod(
classDef,
polymorphicField,
changeMtd -> getThisFieldCode(changeMtd,
parentInnerNodeFieldDef, polymorphicFieldDef),
(changeMtd, newValue) -> setThisFieldCode(changeMtd,
newValue, parentInnerNodeFieldDef, polymorphicFieldDef),
changeMtd -> getThisFieldCode(changeMtd,
parentInnerNodeFieldDef, polymorphicTypeIdFieldDef)
);
- addNodeChangeBridgeMethod(classDef,
polymorphicExtensionClassInfo.changeClassName, changeMtd0);
+ addNodeChangeBridgeMethod(classDef,
polymorphicExtensionClassInfo.changeClassName, changeMethods.get(0));
Review Comment:
Same.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java:
##########
@@ -1586,15 +1622,15 @@ private ClassDefinition
createPolymorphicExtensionNodeClass(
continue;
}
- MethodDefinition changeMtd0 = addNodeChangeMethod(
+ List<MethodDefinition> changeMethods = addNodeChangeMethod(
classDef,
schemaField,
changeMtd -> getThisFieldCode(changeMtd,
parentInnerNodeFieldDef, schemaFieldDef),
(changeMtd, newValue) -> setThisFieldCode(changeMtd,
newValue, parentInnerNodeFieldDef, schemaFieldDef),
null
);
- addNodeChangeBridgeMethod(classDef,
schemaClassInfo.changeClassName, changeMtd0);
+ addNodeChangeBridgeMethod(classDef,
schemaClassInfo.changeClassName, changeMethods.get(0));
Review Comment:
Same about `get(0)`.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/asm/InnerNodeAsmGenerator.java:
##########
@@ -731,56 +718,105 @@ private MethodDefinition addNodeChangeMethod(
// this.field = newValue;
bytecodeBlock.append(setFieldCodeFun.apply(changeMtd, newValue));
} else {
- BytecodeExpression newValue;
+ shortChangeMtd = createShortChangeMethod(classDef, schemaField,
getFieldCodeFun, setFieldCodeFun, getPolymorphicTypeIdFieldFun);
- if (isConfigValue(schemaField)) {
- // newValue = (this.field == null) ? new ValueNode() :
(ValueNode)this.field.copy();
- newValue = cgen.newOrCopyNodeField(schemaField,
getFieldCodeFun.apply(changeMtd));
- } else {
- assert isNamedConfigValue(schemaField) : schemaField;
+ // change.accept(this.field); OR
change.accept(this.field.specificNode());
+ bytecodeBlock.append(changeVar.invoke(ACCEPT,
changeMtd.getThis().invoke(shortChangeMtd, List.of())));
+ }
- // newValue = (ValueNode)this.field.copy();
- newValue = cgen.copyNodeField(schemaField,
getFieldCodeFun.apply(changeMtd));
- }
+ // return this;
+ bytecodeBlock.append(changeMtd.getThis()).retObject();
- // this.field = newValue;
- bytecodeBlock.append(setFieldCodeFun.apply(changeMtd, newValue));
+ enrichWithPolymorphicTypeCheck(schemaField,
getPolymorphicTypeIdFieldFun, changeMtd, bytecodeBlock);
- // this.field;
- BytecodeExpression getFieldCode = getFieldCodeFun.apply(changeMtd);
+ if (shortChangeMtd == null) {
+ return List.of(changeMtd);
+ }
- if (isPolymorphicConfig(schemaFieldType) &&
isConfigValue(schemaField)) {
- // this.field.specificNode();
- getFieldCode = getFieldCode.invoke(SPECIFIC_NODE_MTD);
- }
+ return List.of(changeMtd, shortChangeMtd);
+ }
- // change.accept(this.field); OR
change.accept(this.field.specificNode());
- bytecodeBlock.append(changeVar.invoke(ACCEPT, getFieldCode));
+ /**
+ * Creates a "short" method to return a changed field instance. Name is
the same as for {@code changeFoo(Consumer<Foo> change)"}.
Review Comment:
Please add an example of what it would look like.
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/ConfigurationRegistry.java:
##########
@@ -272,6 +275,39 @@ public CompletableFuture<Void> change(ConfigurationSource
changesSrc) {
return changer.change(changesSrc);
}
+ /**
+ * Change configuration. Gives the possibility to atomically update
several root trees.
+ *
+ * @param change Closure that would consume a mutable super root instance.
+ * @return Future that is completed on change completion.
+ */
+ public CompletableFuture<Void> change(Consumer<SuperRootChange> change) {
+ return change(new ConfigurationSource() {
+ @Override
+ public void descend(ConstructableTreeNode node) {
+ assert node instanceof SuperRoot : "Descending always starts
with super root.";
Review Comment:
```suggestion
assert node instanceof SuperRoot : "Descending always starts
with super root: " + node ;
```
--
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]
