Allow removal of configuration properties

Signed-off-by: Clement de Groc <[email protected]>

docs/changelog.md

@@ -162,6 +162,23 @@ GraphBinary is now used as the default MessageSerializer.
 We are dropping support for old serialization format of JanusGraph predicates. The old predicates serialization format is only used by client older than 0.6.
 The change only affects GraphSON.
 
+##### Allow removal of configuration keys
+
+Users can now remove configuration keys in the ConfiguredGraphFactory's configuration:
+
+```groovy
+ConfiguredGraphFactory.removeConfiguration("<graph_name>", Collections.singleton("<config_key>"))
+```
+
+Or the global configuration:
+```groovy
+mgmt = graph.openManagement()
+mgmt.remove("<config_key>")
+mgmt.commit()
+```
+
+Note that the above commands should be used with care. They cannot be used to drop an external index backend if it has mixed indexes for instance.
+
 ##### New index management
 
 The index management has received an overhaul which enables proper index removal.

janusgraph-backend-testutils/src/main/java/org/janusgraph/core/AbstractConfiguredGraphFactoryTest.java

@@ -29,6 +29,7 @@
 import org.junit.jupiter.api.Test;
 import org.mockito.Mockito;
 
+import java.util.Collections;
 import java.util.HashMap;
 import java.util.Map;
 import java.util.Set;
@@ -37,6 +38,7 @@
 
 import static org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration.GRAPH_NAME;
 import static org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration.STORAGE_BACKEND;
+import static org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration.STORAGE_HOSTS;
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertFalse;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -247,14 +249,14 @@ public void updateConfigurationShouldRemoveGraphFromCache() throws Exception {
 
             // string-delimited hosts are recognized
             final Map<String, Object> map = graphConfig.getMap();
-            map.put("storage.hostname", "localhost,localhost");
+            map.put(STORAGE_HOSTS.toStringWithoutRoot(), "localhost,localhost");
             ConfiguredGraphFactory.updateConfiguration(graphName, new MapConfiguration(map));
             assertNull(gm.getGraph(graphName));
             assertNotNull(ConfiguredGraphFactory.open(graphName));
 
             // bogus backend will prevent the graph from being opened
             final Map<String, Object> map2 = graphConfig.getMap();
-            map2.put("storage.backend", "bogusBackend");
+            map2.put(STORAGE_HOSTS.toStringWithoutRoot(), "bogusBackend");
             ConfiguredGraphFactory.updateConfiguration(graphName, new MapConfiguration(map2));
             assertNull(gm.getGraph(graphName));
             // we should throw an error since the config has been updated and we are attempting
@@ -286,6 +288,37 @@ public void removeConfigurationShouldRemoveGraphFromCache() throws Exception {
         }
     }
 
+    @Test
+    public void removeConfigurationShouldAllowRemovingKeysExceptGraphName() throws Exception {
+        final MapConfiguration graphConfig = getGraphConfig();
+        final String graphName = graphConfig.getString(GRAPH_NAME.toStringWithoutRoot());
+
+        try {
+            ConfiguredGraphFactory.createConfiguration(graphConfig);
+            final StandardJanusGraph graph = (StandardJanusGraph) ConfiguredGraphFactory.open(graphName);
+            assertNotNull(graph);
+
+            // Set cannot contain the "graph.graphname" property
+            assertThrows(IllegalArgumentException.class,
+                () -> ConfiguredGraphFactory.removeConfiguration(graphName, Collections.singleton(GRAPH_NAME.toStringWithoutRoot())));
+
+            // Add a STORAGE_HOSTS configuration key to the graph
+            final Map<String, Object> map = graphConfig.getMap();
+            map.put(STORAGE_HOSTS.toStringWithoutRoot(), "localhost");
+            ConfiguredGraphFactory.updateConfiguration(graphName, new MapConfiguration(map));
+            assertNotNull(ConfiguredGraphFactory.open(graphName));
+
+            // After checking that the STORAGE_HOSTS configuration key exists, delete it and check that it's unset
+            assertEquals("localhost", ConfiguredGraphFactory.getConfiguration(graphName).get(STORAGE_HOSTS.toStringWithoutRoot()));
+            ConfiguredGraphFactory.removeConfiguration(graphName, Collections.singleton(STORAGE_HOSTS.toStringWithoutRoot()));
+            assertNotNull(ConfiguredGraphFactory.open(graphName));
+            assertFalse(ConfiguredGraphFactory.getConfiguration(graphName).containsKey(STORAGE_HOSTS.toStringWithoutRoot()));
+        } finally {
+            ConfiguredGraphFactory.removeConfiguration(graphName);
+            ConfiguredGraphFactory.close(graphName);
+        }
+    }
+
     @Test
     public void dropGraphShouldRemoveGraphFromCache() throws Exception {
         final MapConfiguration graphConfig = getGraphConfig();

janusgraph-core/src/main/java/org/janusgraph/core/ConfiguredGraphFactory.java

@@ -277,6 +277,18 @@ public static void removeConfiguration(final String graphName) {
         configManagementGraph.removeConfiguration(graphName);
     }
 
+    /**
+     * Remove configuration corresponding to supplied graphName; we remove supplied existing
+     * properties.
+     * NOTE: The updated configuration is only guaranteed to take effect if the {@link Graph} corresponding to
+     * graphName has been closed and reopened on every JanusGraph Node.
+     */
+    public static void removeConfiguration(final String graphName, final Set<String> configKeys) {
+        final ConfigurationManagementGraph configManagementGraph = getConfigGraphManagementInstance();
+        removeGraphFromCache(graphName);
+        configManagementGraph.removeConfiguration(graphName, configKeys);
+    }
+
     private static void removeGraphFromCache(final String graphName) {
 
         try {
@@ -314,9 +326,9 @@ public static void removeTemplateConfiguration() {
      *
      * @return Map&lt;String, Object&gt;
      */
-    public static Map<String, Object> getConfiguration(final String configName) {
+    public static Map<String, Object> getConfiguration(final String graphName) {
         final ConfigurationManagementGraph configManagementGraph = getConfigGraphManagementInstance();
-        return configManagementGraph.getConfiguration(configName);
+        return configManagementGraph.getConfiguration(graphName);
     }
 
     /**

janusgraph-core/src/main/java/org/janusgraph/core/schema/JanusGraphConfiguration.java

@@ -40,4 +40,11 @@ public interface JanusGraphConfiguration {
      */
     JanusGraphConfiguration set(String path, Object value);
 
+    /**
+     * Remove the configuration option identified by the provided path.
+     *
+     * @param path
+     */
+    JanusGraphConfiguration remove(String path);
+
 }

janusgraph-core/src/main/java/org/janusgraph/diskstorage/configuration/UserModifiableConfiguration.java

@@ -117,6 +117,16 @@ public UserModifiableConfiguration set(String path, Object value) {
         return this;
     }
 
+    @Override
+    public JanusGraphConfiguration remove(String path) {
+        ConfigElement.PathIdentifier pp = ConfigElement.parse(config.getRootNamespace(),path);
+        Preconditions.checkArgument(pp.element.isOption(),"Need to provide configuration option - not namespace: %s",path);
+        ConfigOption option = (ConfigOption)pp.element;
+        verifier.verifyModification(option);
+        config.remove(option, pp.umbrellaElements);
+        return this;
+    }
+
     /**
      * Closes this configuration handler
      */

janusgraph-core/src/main/java/org/janusgraph/graphdb/database/management/ManagementSystem.java

@@ -1566,4 +1566,10 @@ public synchronized JanusGraphConfiguration set(String path, Object value) {
         ensureOpen();
         return userConfig.set(path, value);
     }
+
+    @Override
+    public JanusGraphConfiguration remove(String path) {
+        ensureOpen();
+        return userConfig.remove(path);
+    }
 }

janusgraph-core/src/main/java/org/janusgraph/graphdb/management/ConfigurationManagementGraph.java

@@ -205,6 +205,22 @@ public void removeConfiguration(final String graphName) {
         removeVertex(PROPERTY_GRAPH_NAME, graphName);
     }
 
+    /**
+     * Remove configuration corresponding to supplied graphName; we remove supplied existing
+     * properties.
+     * NOTE: The updated configuration is only guaranteed to take effect if the {@link Graph} corresponding to
+     * graphName has been closed and reopened on every JanusGraph Node.
+     */
+    public void removeConfiguration(final String graphName, final Set<String> configKeys) {
+        Preconditions.checkArgument(!configKeys.contains(PROPERTY_GRAPH_NAME),
+            "The list of configuration keys to remove cannot contain the property \"%s\".", PROPERTY_GRAPH_NAME);
+        log.warn("Configuration {} is only guaranteed to take effect when graph {} has been closed and reopened on all Janus Graph Nodes.",
+            graphName,
+            graphName
+        );
+        removeVertexProperties(PROPERTY_GRAPH_NAME, graphName, configKeys);
+    }
+
     /**
      * Remove template configuration
      */
@@ -335,6 +351,15 @@ private void updateVertexWithProperties(String propertyKey, Object propertyValue
         }
     }
 
+    private void removeVertexProperties(String propertyKey, Object propertyValue, Set<String> set) {
+        final GraphTraversalSource g = getTraversal();
+        if (g.V().has(propertyKey, propertyValue).hasNext()) {
+            final Vertex v = g.V().has(propertyKey, propertyValue).next();
+            set.forEach(key -> v.property(key, null));
+            graph.tx().commit();
+        }
+    }
+
     private Map<String, Object> deserializeVertexProperties(Map<Object, Object> map) {
         HashMap<String, Object> deserializedProperties = new HashMap<>();
         map.forEach((key, value) -> {

janusgraph-test/src/test/java/org/janusgraph/diskstorage/configuration/UserModifiableConfigurationTest.java

@@ -0,0 +1,56 @@
+// Copyright 2022 JanusGraph Authors
+// Licensed 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.janusgraph.diskstorage.configuration;
+
+import org.apache.commons.configuration2.BaseConfiguration;
+import org.janusgraph.diskstorage.configuration.backend.CommonsConfiguration;
+import org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration;
+import org.janusgraph.util.system.ConfigurationUtil;
+import org.junit.jupiter.api.Test;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
+
+public class UserModifiableConfigurationTest {
+
+    private UserModifiableConfiguration newBaseConfiguration() {
+        final BaseConfiguration base = ConfigurationUtil.createBaseConfiguration();
+        final CommonsConfiguration config = new CommonsConfiguration(base);
+        return new UserModifiableConfiguration(new ModifiableConfiguration(GraphDatabaseConfiguration.ROOT_NS, config.copy(), BasicConfiguration.Restriction.NONE));
+    }
+
+    @Test
+    public void testRemove() {
+        UserModifiableConfiguration configuration = newBaseConfiguration();
+        configuration.set("storage.backend", "inmemory");
+        assertEquals("inmemory", configuration.get("storage.backend"));
+
+        configuration.remove("storage.backend");
+        assertEquals("null", configuration.get("storage.backend"));
+
+        configuration.set("index.search.backend", "lucene");
+        assertEquals("lucene", configuration.get("index.search.backend"));
+        assertEquals("+ search\n", configuration.get("index"));
+
+        configuration.remove("index.search.backend");
+        assertEquals("", configuration.get("index"));
+
+        IllegalArgumentException e1 = assertThrows(IllegalArgumentException.class, () -> configuration.remove("index.search"));
+        assertTrue(e1.getMessage().startsWith("Need to provide configuration option - not namespace"));
+
+        IllegalArgumentException e2 = assertThrows(IllegalArgumentException.class, () -> configuration.remove("non_existing"));
+        assertEquals("Unknown configuration element in namespace [root]: non_existing", e2.getMessage());
+    }
+}