You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@ignite.apache.org by ib...@apache.org on 2021/03/30 08:02:11 UTC

[ignite-3] branch main updated: IGNITE-14442 Fixed NPE in IgniteRunner caused by broken REST module. (#77)

This is an automated email from the ASF dual-hosted git repository.

ibessonov pushed a commit to branch main
in repository https://gitbox.apache.org/repos/asf/ignite-3.git


The following commit(s) were added to refs/heads/main by this push:
     new 8cf0bb2  IGNITE-14442 Fixed NPE in IgniteRunner caused by broken REST module. (#77)
8cf0bb2 is described below

commit 8cf0bb2102fa5d9eb55df8448cc9d6739333a749
Author: ibessonov <be...@gmail.com>
AuthorDate: Tue Mar 30 11:02:01 2021 +0300

    IGNITE-14442 Fixed NPE in IgniteRunner caused by broken REST module. (#77)
---
 .../ignite/configuration/ConfigurationChanger.java | 16 +++++
 .../configuration/ConfigurationRegistry.java       |  4 +-
 .../java/org/apache/ignite/rest/RestModule.java    | 11 +--
 .../InMemoryConfigurationStorage.java              | 84 ++++++++++++++++++++++
 .../configuration/RestConfigurationSchema.java     |  2 +-
 .../rest/presentation/json/JsonConverter.java      |  9 ++-
 .../java/org/apache/ignite/app/IgniteRunner.java   |  5 +-
 .../ignite/configuration/ConfigurationModule.java  | 41 ++++++++---
 .../extended/LocalConfigurationSchema.java         |  3 +-
 9 files changed, 143 insertions(+), 32 deletions(-)

diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
index 7990151..0773fe1 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationChanger.java
@@ -35,6 +35,7 @@ import org.apache.ignite.configuration.internal.validation.ValidationUtil;
 import org.apache.ignite.configuration.storage.ConfigurationStorage;
 import org.apache.ignite.configuration.storage.Data;
 import org.apache.ignite.configuration.storage.StorageException;
+import org.apache.ignite.configuration.tree.ConfigurationSource;
 import org.apache.ignite.configuration.tree.InnerNode;
 import org.apache.ignite.configuration.validation.ConfigurationValidationException;
 import org.apache.ignite.configuration.validation.ValidationIssue;
@@ -184,6 +185,21 @@ public class ConfigurationChanger {
         }
     }
 
+    /** Temporary until the IGNITE-14372 */
+    public CompletableFuture<?> changeX(
+        List<String> path,
+        ConfigurationSource source,
+        ConfigurationStorage storage
+    ) {
+        assert path.isEmpty() : "Path support is not yet implemented.";
+
+        SuperRoot superRoot = new SuperRoot(rootKeys);
+
+        source.descend(superRoot);
+
+        return change(superRoot, storage.getClass());
+    }
+
     /**
      * Get root node by root key. Subject to revisiting.
      *
diff --git a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationRegistry.java b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationRegistry.java
index 5770249..b7931de 100644
--- a/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationRegistry.java
+++ b/modules/configuration/src/main/java/org/apache/ignite/configuration/ConfigurationRegistry.java
@@ -109,8 +109,8 @@ public class ConfigurationRegistry {
     }
 
     /** */
-    public CompletableFuture<?> change(List<String> path, ConfigurationSource changesSource) {
-        throw new UnsupportedOperationException("IGNITE-14372 Not implemented yet.");
+    public CompletableFuture<?> change(List<String> path, ConfigurationSource changesSource, ConfigurationStorage storage) {
+        return changer.changeX(path, changesSource, storage);
     }
 
     /**
diff --git a/modules/rest/src/main/java/org/apache/ignite/rest/RestModule.java b/modules/rest/src/main/java/org/apache/ignite/rest/RestModule.java
index a434eaa..37580cb 100644
--- a/modules/rest/src/main/java/org/apache/ignite/rest/RestModule.java
+++ b/modules/rest/src/main/java/org/apache/ignite/rest/RestModule.java
@@ -28,7 +28,6 @@ import io.netty.channel.socket.nio.NioServerSocketChannel;
 import io.netty.handler.codec.http.HttpHeaderValues;
 import io.netty.handler.logging.LogLevel;
 import io.netty.handler.logging.LoggingHandler;
-import java.io.Reader;
 import java.nio.charset.StandardCharsets;
 import java.util.List;
 import java.util.Map;
@@ -77,17 +76,11 @@ public class RestModule {
     }
 
     /** */
-    public void prepareStart(ConfigurationRegistry sysCfg, Reader moduleConfReader) {
+    public void prepareStart(ConfigurationRegistry sysCfg) {
         sysConf = sysCfg;
+        sysCfg.registerRootKey(RestConfiguration.KEY);
 
         presentation = new JsonPresentation();
-
-//        FormatConverter converter = new JsonConverter();
-//
-//        Configurator<RestConfigurationImpl> restConf = Configurator.create(RestConfigurationImpl::new,
-//            converter.convertFrom(moduleConfReader, "rest", InitRest.class));
-//
-//        sysConfig.registerConfigurator(restConf);
     }
 
     /**
diff --git a/modules/rest/src/main/java/org/apache/ignite/rest/configuration/InMemoryConfigurationStorage.java b/modules/rest/src/main/java/org/apache/ignite/rest/configuration/InMemoryConfigurationStorage.java
new file mode 100644
index 0000000..a74ff59
--- /dev/null
+++ b/modules/rest/src/main/java/org/apache/ignite/rest/configuration/InMemoryConfigurationStorage.java
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+package org.apache.ignite.rest.configuration;
+
+import java.io.Serializable;
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+import java.util.Set;
+import java.util.concurrent.CompletableFuture;
+import java.util.concurrent.ConcurrentHashMap;
+import java.util.concurrent.atomic.AtomicLong;
+import org.apache.ignite.configuration.storage.ConfigurationStorage;
+import org.apache.ignite.configuration.storage.ConfigurationStorageListener;
+import org.apache.ignite.configuration.storage.Data;
+import org.apache.ignite.configuration.storage.StorageException;
+
+/**
+ * Temporary configuration storage.
+ */
+public class InMemoryConfigurationStorage implements ConfigurationStorage {
+    /** Map to store values. */
+    private Map<String, Serializable> map = new ConcurrentHashMap<>();
+
+    /** Change listeners. */
+    private List<ConfigurationStorageListener> listeners = new ArrayList<>();
+
+    /** Storage version. */
+    private AtomicLong version = new AtomicLong(0);
+
+    /** {@inheritDoc} */
+    @Override public synchronized Data readAll() throws StorageException {
+        return new Data(new HashMap<>(map), version.get());
+    }
+
+    /** {@inheritDoc} */
+    @Override public synchronized CompletableFuture<Boolean> write(Map<String, Serializable> newValues, long sentVersion) throws StorageException {
+        if (sentVersion != version.get())
+            return CompletableFuture.completedFuture(false);
+
+        for (Map.Entry<String, Serializable> entry : newValues.entrySet()) {
+            if (entry.getValue() != null)
+                map.put(entry.getKey(), entry.getValue());
+            else
+                map.remove(entry.getKey());
+        }
+
+        version.incrementAndGet();
+
+        listeners.forEach(listener -> listener.onEntriesChanged(new Data(newValues, version.get())));
+
+        return CompletableFuture.completedFuture(true);
+    }
+
+    /** {@inheritDoc} */
+    @Override public synchronized Set<String> keys() throws StorageException {
+        return map.keySet();
+    }
+
+    /** {@inheritDoc} */
+    @Override public void addListener(ConfigurationStorageListener listener) {
+        listeners.add(listener);
+    }
+
+    /** {@inheritDoc} */
+    @Override public void removeListener(ConfigurationStorageListener listener) {
+        listeners.remove(listener);
+    }
+}
diff --git a/modules/rest/src/main/java/org/apache/ignite/rest/configuration/RestConfigurationSchema.java b/modules/rest/src/main/java/org/apache/ignite/rest/configuration/RestConfigurationSchema.java
index 0003eae..31e7160 100644
--- a/modules/rest/src/main/java/org/apache/ignite/rest/configuration/RestConfigurationSchema.java
+++ b/modules/rest/src/main/java/org/apache/ignite/rest/configuration/RestConfigurationSchema.java
@@ -27,7 +27,7 @@ import static org.apache.ignite.rest.RestModule.DFLT_PORT;
 /**
  * Configuration schema for REST endpoint subtree.
  */
-@ConfigurationRoot(rootName = "rest")
+@ConfigurationRoot(rootName = "rest", storage = InMemoryConfigurationStorage.class)
 public class RestConfigurationSchema {
     /** */
     @Min(1024)
diff --git a/modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java b/modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
index abe070f..dc2f956 100644
--- a/modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
+++ b/modules/rest/src/main/java/org/apache/ignite/rest/presentation/json/JsonConverter.java
@@ -151,7 +151,7 @@ public class JsonConverter implements FormatConverter {
     /** */
     public static ConfigurationSource jsonSource(JsonElement jsonElement) {
         //TODO IGNITE-14372 Finish this implementation.
-        return null;
+        return new JsonObjectConfigurationSource(new ArrayList<>(), jsonElement.getAsJsonObject());
     }
 
     private static class JsonObjectConfigurationSource implements ConfigurationSource {
@@ -174,8 +174,6 @@ public class JsonConverter implements FormatConverter {
             for (Map.Entry<String, JsonElement> entry : jsonObject.entrySet()) {
                 String key = entry.getKey();
 
-                path.add(key);
-
                 JsonElement jsonElement = entry.getValue();
 
                 try {
@@ -197,8 +195,6 @@ public class JsonConverter implements FormatConverter {
                 catch (NoSuchElementException e) {
                     throw new IllegalArgumentException(""); //TODO IGNITE-14372 Update comment.
                 }
-
-                path.remove(path.size() - 1);
             }
         }
     }
@@ -217,6 +213,9 @@ public class JsonConverter implements FormatConverter {
             if (clazz.isArray() != jsonLeaf.isJsonArray())
                 throw new IllegalArgumentException(""); //TODO IGNITE-14372 Update comment.
 
+            if (!clazz.isArray())
+                return unwrap(jsonLeaf.getAsJsonPrimitive(), clazz);
+
             return null;
         }
 
diff --git a/modules/runner/src/main/java/org/apache/ignite/app/IgniteRunner.java b/modules/runner/src/main/java/org/apache/ignite/app/IgniteRunner.java
index 007592a..15854b9 100644
--- a/modules/runner/src/main/java/org/apache/ignite/app/IgniteRunner.java
+++ b/modules/runner/src/main/java/org/apache/ignite/app/IgniteRunner.java
@@ -21,7 +21,6 @@ import java.io.BufferedReader;
 import java.io.FileReader;
 import java.io.IOException;
 import java.io.InputStreamReader;
-import java.io.StringReader;
 import java.util.Arrays;
 import java.util.stream.Collectors;
 import org.apache.ignite.configuration.ConfigurationModule;
@@ -103,9 +102,9 @@ public class IgniteRunner {
                 bldr.append(str);
             }
 
-            restModule.prepareStart(confModule.configurationRegistry(), new StringReader(bldr.toString()));
+            restModule.prepareStart(confModule.configurationRegistry());
 
-            confModule.bootstrap(new StringReader(bldr.toString()));
+            confModule.bootstrap(bldr.toString());
         }
         finally {
             if (confReader != null)
diff --git a/modules/runner/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java b/modules/runner/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java
index d6c9195..ccec3d0 100644
--- a/modules/runner/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java
+++ b/modules/runner/src/main/java/org/apache/ignite/configuration/ConfigurationModule.java
@@ -17,7 +17,13 @@
 
 package org.apache.ignite.configuration;
 
-import java.io.Reader;
+import com.google.gson.JsonObject;
+import com.google.gson.JsonParser;
+import java.util.Collections;
+import java.util.concurrent.ExecutionException;
+import org.apache.ignite.configuration.extended.LocalConfiguration;
+import org.apache.ignite.rest.configuration.InMemoryConfigurationStorage;
+import org.apache.ignite.rest.presentation.json.JsonConverter;
 
 /**
  * Module is responsible for preparing configuration when module is started.
@@ -25,21 +31,34 @@ import java.io.Reader;
  * Preparing configuration includes reading it from configuration file, parsing it and initializing
  * {@link ConfigurationRegistry} object.
  */
+@SuppressWarnings("PMD.UnusedPrivateField")
 public class ConfigurationModule {
-//    /** */
-//    private Configurator<LocalConfigurationImpl> localConfigurator;
+    /** */
+    private LocalConfiguration localConfigurator;
 
     /** */
     private final ConfigurationRegistry confRegistry = new ConfigurationRegistry();
 
-    /** */
-    public void bootstrap(Reader confReader) {
-//        FormatConverter converter = new JsonConverter();
-//
-//        Configurator<LocalConfigurationImpl> configurator =
-//            Configurator.create(LocalConfigurationImpl::new, converter.convertFrom(confReader, "local", InitLocal.class));
-//
-//        confRegistry.registerConfigurator(configurator);
+    /**
+     * @param jsonStr
+     */
+    public void bootstrap(String jsonStr) throws InterruptedException {
+        confRegistry.registerRootKey(LocalConfiguration.KEY);
+
+        InMemoryConfigurationStorage storage = new InMemoryConfigurationStorage();
+
+        confRegistry.registerStorage(storage);
+
+        JsonObject jsonCfg = JsonParser.parseString(jsonStr).getAsJsonObject();
+
+        try {
+            confRegistry.change(Collections.emptyList(), JsonConverter.jsonSource(jsonCfg), storage).get();
+        }
+        catch (ExecutionException e) {
+            throw new RuntimeException(e.getCause());
+        }
+
+        localConfigurator = confRegistry.getConfiguration(LocalConfiguration.KEY);
     }
 
     /** */
diff --git a/modules/runner/src/main/java/org/apache/ignite/configuration/extended/LocalConfigurationSchema.java b/modules/runner/src/main/java/org/apache/ignite/configuration/extended/LocalConfigurationSchema.java
index bfe9a21..346ae43 100644
--- a/modules/runner/src/main/java/org/apache/ignite/configuration/extended/LocalConfigurationSchema.java
+++ b/modules/runner/src/main/java/org/apache/ignite/configuration/extended/LocalConfigurationSchema.java
@@ -19,12 +19,13 @@ package org.apache.ignite.configuration.extended;
 
 import org.apache.ignite.configuration.annotation.ConfigValue;
 import org.apache.ignite.configuration.annotation.ConfigurationRoot;
+import org.apache.ignite.rest.configuration.InMemoryConfigurationStorage;
 
 /**
  *
  */
 @SuppressWarnings("PMD.UnusedPrivateField")
-@ConfigurationRoot(rootName = "local")
+@ConfigurationRoot(rootName = "local", storage = InMemoryConfigurationStorage.class)
 public class LocalConfigurationSchema {
     /** */
     @ConfigValue