tkalkirill commented on code in PR #1883:
URL: https://github.com/apache/ignite-3/pull/1883#discussion_r1155849912
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/notifications/ConfigurationNotificationEvent.java:
##########
@@ -35,11 +35,31 @@
*/
@Nullable VIEWT oldValue();
+ /**
+ * Returns old value of the parent (any from the root) or current
configuration.
+ *
+ * <p>For example, if we changed the child configuration, then we can get
both the parent and the current child configuration.
+ *
+ * @param viewClass Configuration interface, for example {@code
RootView.class}.
+ * @param <T> Configuration type.
+ */
+ @Nullable <T> T oldValue(Class<T> viewClass);
Review Comment:
In fact, `oldValue()` is a special case of this method, do you think it's
worth getting rid of `oldValue()` ?
##########
modules/configuration-api/src/main/java/org/apache/ignite/configuration/notifications/ConfigurationNotificationEvent.java:
##########
@@ -35,11 +35,31 @@
*/
@Nullable VIEWT oldValue();
+ /**
+ * Returns old value of the parent (any from the root) or current
configuration.
+ *
+ * <p>For example, if we changed the child configuration, then we can get
both the parent and the current child configuration.
+ *
+ * @param viewClass Configuration interface, for example {@code
RootView.class}.
+ * @param <T> Configuration type.
+ */
+ @Nullable <T> T oldValue(Class<T> viewClass);
+
/**
* Returns updated value of the configuration.
*/
@Nullable VIEWT newValue();
+ /**
+ * Returns new value of the parent (any from the root) or current
configuration.
+ *
+ * <p>For example, if we changed the child configuration, then we can get
both the parent and the current child configuration.
+ *
+ * @param viewClass Configuration interface, for example {@code
RootView.class}.
+ * @param <T> Configuration type.
+ */
+ @Nullable <T> T newValue(Class<T> viewClass);
Review Comment:
Same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/notifications/ConfigurationContainer.java:
##########
@@ -17,60 +17,93 @@
package org.apache.ignite.internal.configuration.notifications;
-import org.apache.ignite.configuration.ConfigurationProperty;
import
org.apache.ignite.configuration.notifications.ConfigurationNotificationEvent;
-import org.apache.ignite.internal.configuration.DynamicConfiguration;
import org.apache.ignite.internal.configuration.tree.InnerNode;
import org.jetbrains.annotations.Nullable;
/**
- * Configuration container for {@link ConfigurationNotificationEvent}.
+ * Configuration container for {@link ConfigurationNotificationEvent}. Stores
old and new nodes for the event, as well as their names.
+ * Represents a stack, where current pair of nodes is at the top, and roots
pair is at the bottom.
*/
class ConfigurationContainer {
- /**
- * Configuration.
- *
- * <p>For {@link ConfigurationNotificationEvent#config} use {@link
this#specificConfig()}.
- */
- final DynamicConfiguration<InnerNode, ?> config;
+ /** Parent container. */
+ @Nullable
+ public final ConfigurationContainer prev;
- /** Key in named list, for {@link ConfigurationNotificationEvent#name}. */
+ /** {@link InnerNode#specificNode()} for the old value. */
@Nullable
- final String name;
+ private final Object oldNode;
- /** Previous container. */
+ /** {@link InnerNode#specificNode()} for the new value. */
@Nullable
- final ConfigurationContainer prev;
+ private final Object newNode;
+
+ /** Old name, if the node is a part of named list. */
+ private final String oldName;
+
+ /** New name, if the node is a part of named list. */
+ private final String newName;
/**
* Constructor.
*
- * @param config Configuration.
- * @param name Key in named list.
+ * @param prev Parent container.
+ * @param oldNode Old inner node value.
+ * @param newNode New inner node value.
+ * @param oldName Name of the old value, if part of the named list.
+ * @param newName Name of the new value, if part of the named list.
*/
ConfigurationContainer(
- DynamicConfiguration<InnerNode, ?> config,
- @Nullable String name,
- @Nullable ConfigurationContainer prev
+ @Nullable ConfigurationContainer prev,
+ @Nullable InnerNode oldNode,
+ @Nullable InnerNode newNode,
+ @Nullable String oldName,
+ @Nullable String newName
) {
- this.config = config;
- this.name = name;
this.prev = prev;
+ this.oldNode = oldNode == null ? null : oldNode.specificNode();
+ this.newNode = newNode == null ? null : newNode.specificNode();
+ this.oldName = oldName;
+ this.newName = newName;
}
/**
- * Returns the configuration for {@link
ConfigurationNotificationEvent#config}.
+ * Finds a node value that corresponds to a given {@code View} class.
+ *
+ * @param clazz Class instance of the {@code *View} type.
+ * @param old Whether an old or a new value is asked.
*/
- @Nullable ConfigurationProperty<InnerNode> specificConfig() {
- return config.isRemovedFromNamedList() ? null :
config.specificConfigTree();
+ public @Nullable <T> T find(Class<T> clazz, boolean old) {
+ Object specificNode = node(old);
+
+ if (clazz.isInstance(specificNode)) {
+ return (T) specificNode;
+ }
+
+ return prev == null ? null : prev.find(clazz, old);
}
/**
- * Returns the configuration class.
+ * Finds a name of the value that corresponds to a given {@code View}
class.
+ *
+ * @param clazz Class instance of the {@code *View} type.
+ * @param old Whether an old or a new value is asked.
*/
- Class<?> configClass() {
- Class<?> polymorphicInstanceConfigType =
config.polymorphicInstanceConfigType();
+ public @Nullable String name(Class<?> clazz, boolean old) {
Review Comment:
same
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/notifications/ConfigurationContainer.java:
##########
@@ -17,60 +17,93 @@
package org.apache.ignite.internal.configuration.notifications;
-import org.apache.ignite.configuration.ConfigurationProperty;
import
org.apache.ignite.configuration.notifications.ConfigurationNotificationEvent;
-import org.apache.ignite.internal.configuration.DynamicConfiguration;
import org.apache.ignite.internal.configuration.tree.InnerNode;
import org.jetbrains.annotations.Nullable;
/**
- * Configuration container for {@link ConfigurationNotificationEvent}.
+ * Configuration container for {@link ConfigurationNotificationEvent}. Stores
old and new nodes for the event, as well as their names.
+ * Represents a stack, where current pair of nodes is at the top, and roots
pair is at the bottom.
*/
class ConfigurationContainer {
- /**
- * Configuration.
- *
- * <p>For {@link ConfigurationNotificationEvent#config} use {@link
this#specificConfig()}.
- */
- final DynamicConfiguration<InnerNode, ?> config;
+ /** Parent container. */
+ @Nullable
+ public final ConfigurationContainer prev;
Review Comment:
```suggestion
public final @Nullable ConfigurationContainer prev;
```
##########
modules/configuration/src/main/java/org/apache/ignite/internal/configuration/notifications/ConfigurationContainer.java:
##########
@@ -17,60 +17,93 @@
package org.apache.ignite.internal.configuration.notifications;
-import org.apache.ignite.configuration.ConfigurationProperty;
import
org.apache.ignite.configuration.notifications.ConfigurationNotificationEvent;
-import org.apache.ignite.internal.configuration.DynamicConfiguration;
import org.apache.ignite.internal.configuration.tree.InnerNode;
import org.jetbrains.annotations.Nullable;
/**
- * Configuration container for {@link ConfigurationNotificationEvent}.
+ * Configuration container for {@link ConfigurationNotificationEvent}. Stores
old and new nodes for the event, as well as their names.
+ * Represents a stack, where current pair of nodes is at the top, and roots
pair is at the bottom.
*/
class ConfigurationContainer {
- /**
- * Configuration.
- *
- * <p>For {@link ConfigurationNotificationEvent#config} use {@link
this#specificConfig()}.
- */
- final DynamicConfiguration<InnerNode, ?> config;
+ /** Parent container. */
+ @Nullable
+ public final ConfigurationContainer prev;
- /** Key in named list, for {@link ConfigurationNotificationEvent#name}. */
+ /** {@link InnerNode#specificNode()} for the old value. */
@Nullable
- final String name;
+ private final Object oldNode;
- /** Previous container. */
+ /** {@link InnerNode#specificNode()} for the new value. */
@Nullable
- final ConfigurationContainer prev;
+ private final Object newNode;
+
+ /** Old name, if the node is a part of named list. */
+ private final String oldName;
+
+ /** New name, if the node is a part of named list. */
+ private final String newName;
/**
* Constructor.
*
- * @param config Configuration.
- * @param name Key in named list.
+ * @param prev Parent container.
+ * @param oldNode Old inner node value.
+ * @param newNode New inner node value.
+ * @param oldName Name of the old value, if part of the named list.
+ * @param newName Name of the new value, if part of the named list.
*/
ConfigurationContainer(
- DynamicConfiguration<InnerNode, ?> config,
- @Nullable String name,
- @Nullable ConfigurationContainer prev
+ @Nullable ConfigurationContainer prev,
+ @Nullable InnerNode oldNode,
+ @Nullable InnerNode newNode,
+ @Nullable String oldName,
+ @Nullable String newName
) {
- this.config = config;
- this.name = name;
this.prev = prev;
+ this.oldNode = oldNode == null ? null : oldNode.specificNode();
+ this.newNode = newNode == null ? null : newNode.specificNode();
+ this.oldName = oldName;
+ this.newName = newName;
}
/**
- * Returns the configuration for {@link
ConfigurationNotificationEvent#config}.
+ * Finds a node value that corresponds to a given {@code View} class.
+ *
+ * @param clazz Class instance of the {@code *View} type.
+ * @param old Whether an old or a new value is asked.
*/
- @Nullable ConfigurationProperty<InnerNode> specificConfig() {
- return config.isRemovedFromNamedList() ? null :
config.specificConfigTree();
+ public @Nullable <T> T find(Class<T> clazz, boolean old) {
Review Comment:
Maybe different methods? findOld and findNew ?
--
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]