sashapolo commented on a change in pull request #72:
URL: https://github.com/apache/ignite-3/pull/72#discussion_r598520481
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationRegistry.java
##########
@@ -71,6 +80,32 @@ public void registerStorage(ConfigurationStorage
configurationStorage) {
return (T)configs.get(rootKey.key());
}
+ /** */
Review comment:
I think a proper javadoc is needed here
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/tree/NamedListNode.java
##########
@@ -95,6 +95,11 @@ private NamedListNode(NamedListNode<N> node) {
return this;
}
+ /** Very dirty. */
Review comment:
Same as above, such comments are not really helpful
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/internal/util/ConfigurationUtil.java
##########
@@ -255,8 +255,12 @@ private InnerConfigurationSource(Map<String, ?> map) {
assert val == null || val instanceof Map || val instanceof
Serializable;
- if (val == null)
- node.construct(key, null);
+ if (val == null) {
+ if (node instanceof NamedListNode)
+ ((NamedListNode<?>)node).forceDelete(key); // Bad,
but necessary.
Review comment:
can you elaborate this comment a little bit? For example, why is this
bad or why is this necessary
##########
File path:
modules/runner/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java
##########
@@ -23,7 +23,7 @@
* Module is responsible for preparing configuration when module is started.
*
* Preparing configuration includes reading it from configuration file,
parsing it and initializing
- * {@link Configurator} object.
+ * {@code Configurator} object.
Review comment:
I can't find this class anywhere, are you sure this javadoc is up to
date?
##########
File path:
modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
##########
@@ -35,6 +52,213 @@
return gson.toJson(obj);
}
+ /** */
+ public static ConfigurationVisitor<JsonElement> jsonVisitor() {
+ return new ConfigurationVisitor<JsonElement>() {
+ /** */
+ private final Deque<JsonObject> jsonObjectsStack = new
ArrayDeque<>();
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitLeafNode(String key,
Serializable val) {
+ JsonElement jsonLeaf = toJsonLeaf(val);
+
+ if (!jsonObjectsStack.isEmpty())
Review comment:
I would suggest extracting this `if` into a function, this pattern is
repeated quite often
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationRegistry.java
##########
@@ -71,6 +80,32 @@ public void registerStorage(ConfigurationStorage
configurationStorage) {
return (T)configs.get(rootKey.key());
}
+ /** */
+ public <T> T represent(List<String> path, ConfigurationVisitor<T>
representationVisitor) {
+ SuperRoot puperRoot = changer.superPuperRoot();
+
+ Object node;
+ try {
+ node = ConfigurationUtil.find(path, puperRoot);
+ }
+ catch (KeyNotFoundException e) {
+ throw new IllegalArgumentException(e.getMessage());
+ }
+
+ if (node instanceof TraversableTreeNode)
+ return ((TraversableTreeNode)node).accept(null,
representationVisitor);
+
+ assert node == null || node instanceof Serializable;
+
+ return representationVisitor.visitLeafNode(null, (Serializable)node);
+ }
+
+ /** */
+ public CompletableFuture<?> change(List<String> path, ConfigurationSource
changesSource) {
+ //TODO IGNITE-14372 Not implemented yet.
+ return CompletableFuture.completedFuture(null);
Review comment:
Can we throw a `NotImplemented` exception instead?
##########
File path:
modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
##########
@@ -35,6 +52,213 @@
return gson.toJson(obj);
}
+ /** */
+ public static ConfigurationVisitor<JsonElement> jsonVisitor() {
+ return new ConfigurationVisitor<JsonElement>() {
+ /** */
+ private final Deque<JsonObject> jsonObjectsStack = new
ArrayDeque<>();
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitLeafNode(String key,
Serializable val) {
+ JsonElement jsonLeaf = toJsonLeaf(val);
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, jsonLeaf);
+
+ return jsonLeaf;
+ }
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitInnerNode(String key, InnerNode
node) {
+ JsonObject innerJsonNode = new JsonObject();
+
+ jsonObjectsStack.push(innerJsonNode);
+
+ node.traverseChildren(this);
+
+ jsonObjectsStack.pop();
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, innerJsonNode);
+
+ return innerJsonNode;
+ }
+
+ /** {@inheritDoc} */
+ @Override public <N extends InnerNode> JsonElement
visitNamedListNode(String key, NamedListNode<N> node) {
+ JsonObject namedListJsonNode = new JsonObject();
+
+ jsonObjectsStack.push(namedListJsonNode);
+
+ for (String subkey : node.namedListKeys())
+ node.get(subkey).accept(subkey, this);
+
+ jsonObjectsStack.pop();
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, namedListJsonNode);
+
+ return namedListJsonNode;
+ }
+
+ /** */
+ @NotNull private JsonElement toJsonLeaf(Serializable val) {
+ if (val == null)
+ return JsonNull.INSTANCE;
+
+ Class<? extends Serializable> valClass = val.getClass();
+
+ if (!valClass.isArray())
+ return toJsonPrimitive(val);
+
+ JsonArray jsonArray = new JsonArray();
+
+ for (int i = 0; i < Array.getLength(val); i++)
+ jsonArray.add(toJsonPrimitive(Array.get(val, i)));
+
+ return jsonArray;
+ }
+
+ /** */
+ @NotNull private JsonElement toJsonPrimitive(Object val) {
+ if (val == null)
+ return JsonNull.INSTANCE;
+
+ if (val instanceof Boolean)
+ return new JsonPrimitive((Boolean)val);
+
+ if (val instanceof String)
+ return new JsonPrimitive((String)val);
+
+ if (val instanceof Number)
+ return new JsonPrimitive((Number)val);
+
+ assert false : val;
+
+ throw new
IllegalArgumentException(val.getClass().getCanonicalName());
+ }
+ };
+ }
+
+ /** */
+ public static ConfigurationSource jsonSource(JsonElement jsonElement) {
+ //TODO IGNITE-14372 Finish this implementation.
+ return null;
+ }
+
+ private static class JsonObjectConfigurationSource implements
ConfigurationSource {
+ /** Shared. */
+ private final List<String> path;
+
+ /** */
+ private final JsonObject jsonObject;
+
+ private JsonObjectConfigurationSource(List<String> path, JsonObject
jsonObject) {
+ this.path = path;
+ this.jsonObject = jsonObject;
+ }
+
+ @Override public <T> T unwrap(Class<T> clazz) {
+ throw new IllegalArgumentException(""); //TODO IGNITE-14372
Implement.
+ }
+
+ @Override public void descend(ConstructableTreeNode node) {
+ for (Map.Entry<String, JsonElement> entry : jsonObject.entrySet())
{
+ String key = entry.getKey();
+
+ path.add(key);
+
+ JsonElement jsonElement = entry.getValue();
+
+ try {
+ if (jsonElement.isJsonArray() ||
jsonElement.isJsonPrimitive())
+ node.construct(key, new
JsonPrimitiveConfigurationSource(path, jsonElement));
+ else if (jsonElement.isJsonNull()) {
+ node.construct(key, null);
+ }
+ else {
+ assert jsonElement.isJsonObject();
+
+ List<String> path = new ArrayList<>(this.path.size() +
1);
+ path.addAll(this.path);
+ path.add(key);
+
+ node.construct(key, new
JsonObjectConfigurationSource(path, jsonElement.getAsJsonObject()));
+ }
+ }
+ catch (NoSuchElementException e) {
+ throw new IllegalArgumentException(""); //TODO
IGNITE-14372 Update comment.
+ }
+
+ path.remove(path.size() - 1);
+ }
+ }
+ }
+
+ private static class JsonPrimitiveConfigurationSource implements
ConfigurationSource {
+ private final List<String> path;
+
+ private final JsonElement jsonLeaf;
+
+ private JsonPrimitiveConfigurationSource(List<String> path,
JsonElement jsonLeaf) {
+ this.path = path;
+ this.jsonLeaf = jsonLeaf;
+ }
+
+ @Override public <T> T unwrap(Class<T> clazz) {
+ if (clazz.isArray() != jsonLeaf.isJsonArray())
+ throw new IllegalArgumentException(""); //TODO IGNITE-14372
Update comment.
+
+ return null;
+ }
+
+ @Override public void descend(ConstructableTreeNode node) {
+ throw new IllegalArgumentException(""); //TODO IGNITE-14372 Update
comment.
+ }
+
+ private <T> T unwrap(JsonPrimitive jsonPrimitive, Class<T> clazz) {
+ assert !clazz.isArray();
+
+ if (clazz == String.class) {
+ if (!jsonPrimitive.isString())
+ throw new IllegalArgumentException("");
+
+ return clazz.cast(jsonPrimitive.getAsString());
+ }
+
+ if (Number.class.isAssignableFrom(clazz)) {
+ if (!jsonPrimitive.isNumber())
+ throw new IllegalArgumentException("");
+
+ if (clazz == Double.class)
+ return clazz.cast(jsonPrimitive.getAsDouble());
+
+ if (clazz == Long.class)
+ return clazz.cast(jsonPrimitive.getAsLong());
+
+ if (clazz == Integer.class) {
+ long longValue = jsonPrimitive.getAsLong();
+
+ if (longValue < Integer.MIN_VALUE || longValue >
Integer.MAX_VALUE)
Review comment:
you can use `Math.toIntExact` instead
##########
File path:
modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
##########
@@ -35,6 +52,213 @@
return gson.toJson(obj);
}
+ /** */
+ public static ConfigurationVisitor<JsonElement> jsonVisitor() {
+ return new ConfigurationVisitor<JsonElement>() {
+ /** */
+ private final Deque<JsonObject> jsonObjectsStack = new
ArrayDeque<>();
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitLeafNode(String key,
Serializable val) {
+ JsonElement jsonLeaf = toJsonLeaf(val);
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, jsonLeaf);
+
+ return jsonLeaf;
+ }
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitInnerNode(String key, InnerNode
node) {
+ JsonObject innerJsonNode = new JsonObject();
+
+ jsonObjectsStack.push(innerJsonNode);
+
+ node.traverseChildren(this);
+
+ jsonObjectsStack.pop();
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, innerJsonNode);
+
+ return innerJsonNode;
+ }
+
+ /** {@inheritDoc} */
+ @Override public <N extends InnerNode> JsonElement
visitNamedListNode(String key, NamedListNode<N> node) {
+ JsonObject namedListJsonNode = new JsonObject();
+
+ jsonObjectsStack.push(namedListJsonNode);
+
+ for (String subkey : node.namedListKeys())
+ node.get(subkey).accept(subkey, this);
+
+ jsonObjectsStack.pop();
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, namedListJsonNode);
+
+ return namedListJsonNode;
+ }
+
+ /** */
+ @NotNull private JsonElement toJsonLeaf(Serializable val) {
+ if (val == null)
+ return JsonNull.INSTANCE;
+
+ Class<? extends Serializable> valClass = val.getClass();
+
+ if (!valClass.isArray())
+ return toJsonPrimitive(val);
+
+ JsonArray jsonArray = new JsonArray();
+
+ for (int i = 0; i < Array.getLength(val); i++)
+ jsonArray.add(toJsonPrimitive(Array.get(val, i)));
+
+ return jsonArray;
+ }
+
+ /** */
+ @NotNull private JsonElement toJsonPrimitive(Object val) {
+ if (val == null)
+ return JsonNull.INSTANCE;
+
+ if (val instanceof Boolean)
+ return new JsonPrimitive((Boolean)val);
+
+ if (val instanceof String)
+ return new JsonPrimitive((String)val);
+
+ if (val instanceof Number)
+ return new JsonPrimitive((Number)val);
+
+ assert false : val;
Review comment:
what's the point of this assertion?
##########
File path:
modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
##########
@@ -35,6 +52,213 @@
return gson.toJson(obj);
}
+ /** */
+ public static ConfigurationVisitor<JsonElement> jsonVisitor() {
+ return new ConfigurationVisitor<JsonElement>() {
+ /** */
+ private final Deque<JsonObject> jsonObjectsStack = new
ArrayDeque<>();
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitLeafNode(String key,
Serializable val) {
+ JsonElement jsonLeaf = toJsonLeaf(val);
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, jsonLeaf);
+
+ return jsonLeaf;
+ }
+
+ /** {@inheritDoc} */
+ @Override public JsonElement visitInnerNode(String key, InnerNode
node) {
+ JsonObject innerJsonNode = new JsonObject();
+
+ jsonObjectsStack.push(innerJsonNode);
+
+ node.traverseChildren(this);
+
+ jsonObjectsStack.pop();
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, innerJsonNode);
+
+ return innerJsonNode;
+ }
+
+ /** {@inheritDoc} */
+ @Override public <N extends InnerNode> JsonElement
visitNamedListNode(String key, NamedListNode<N> node) {
+ JsonObject namedListJsonNode = new JsonObject();
+
+ jsonObjectsStack.push(namedListJsonNode);
+
+ for (String subkey : node.namedListKeys())
+ node.get(subkey).accept(subkey, this);
+
+ jsonObjectsStack.pop();
+
+ if (!jsonObjectsStack.isEmpty())
+ jsonObjectsStack.peek().add(key, namedListJsonNode);
+
+ return namedListJsonNode;
+ }
+
+ /** */
+ @NotNull private JsonElement toJsonLeaf(Serializable val) {
+ if (val == null)
+ return JsonNull.INSTANCE;
+
+ Class<? extends Serializable> valClass = val.getClass();
Review comment:
Abbreviation plugin is complaining here and in a couple of other places
##########
File path:
modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
##########
@@ -216,6 +216,16 @@ public InnerNode getRootNode(RootKey<?, ?> rootKey) {
return change(new SuperRoot(rootKeys, changes),
storagesTypes.iterator().next());
}
+ /** */
+ public SuperRoot superPuperRoot() {
Review comment:
I understand that superPuper is kind of funny, but I don't think it's
really needed here =)
--
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.
For queries about this service, please contact Infrastructure at:
[email protected]
