diff --git a/docs/changelog.md b/docs/changelog.md index d8c1e71678..95ea7b8e1e 100644 --- a/docs/changelog.md +++ b/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("", Collections.singleton("")) +``` + +Or the global configuration: +```groovy +mgmt = graph.openManagement() +mgmt.remove("") +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. diff --git a/janusgraph-backend-testutils/src/main/java/org/janusgraph/core/AbstractConfiguredGraphFactoryTest.java b/janusgraph-backend-testutils/src/main/java/org/janusgraph/core/AbstractConfiguredGraphFactoryTest.java index 7ebf162fbf..e2a4d6bf27 100644 --- a/janusgraph-backend-testutils/src/main/java/org/janusgraph/core/AbstractConfiguredGraphFactoryTest.java +++ b/janusgraph-backend-testutils/src/main/java/org/janusgraph/core/AbstractConfiguredGraphFactoryTest.java @@ -29,14 +29,18 @@ import org.junit.jupiter.api.Test; import org.mockito.Mockito; +import java.util.Collections; import java.util.HashMap; +import java.util.HashSet; import java.util.Map; import java.util.Set; import javax.script.Bindings; import javax.script.SimpleBindings; +import static org.janusgraph.graphdb.configuration.GraphDatabaseConfiguration.CONNECTION_TIMEOUT; 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 +251,14 @@ public void updateConfigurationShouldRemoveGraphFromCache() throws Exception { // string-delimited hosts are recognized final Map 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 map2 = graphConfig.getMap(); - map2.put("storage.backend", "bogusBackend"); + map2.put(STORAGE_BACKEND.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 +290,85 @@ public void removeConfigurationShouldRemoveGraphFromCache() throws Exception { } } + @Test + public void removeConfigurationFailsToRemovingGraphNameKey() 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()))); + } finally { + ConfiguredGraphFactory.removeConfiguration(graphName); + ConfiguredGraphFactory.close(graphName); + } + } + + @Test + public void removeConfigurationShouldRemoveOneKey() 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); + + // Add a STORAGE_HOSTS configuration key to the graph + final Map 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 removeConfigurationShouldRemoveMultipleKeys() 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); + + // Add a STORAGE_HOSTS configuration key to the graph + final Map map = graphConfig.getMap(); + map.put(STORAGE_HOSTS.toStringWithoutRoot(), "localhost"); + map.put(CONNECTION_TIMEOUT.toStringWithoutRoot(), 20000L); + ConfiguredGraphFactory.updateConfiguration(graphName, new MapConfiguration(map)); + assertNotNull(ConfiguredGraphFactory.open(graphName)); + + // After checking that the STORAGE_HOSTS and CONNECTION_TIMEOUT configuration keys exist, delete them and check that they're unset + assertEquals("localhost", ConfiguredGraphFactory.getConfiguration(graphName).get(STORAGE_HOSTS.toStringWithoutRoot())); + assertEquals(20000L, ConfiguredGraphFactory.getConfiguration(graphName).get(CONNECTION_TIMEOUT.toStringWithoutRoot())); + Set configurationKeys = new HashSet<>(); + configurationKeys.add(STORAGE_HOSTS.toStringWithoutRoot()); + configurationKeys.add(CONNECTION_TIMEOUT.toStringWithoutRoot()); + ConfiguredGraphFactory.removeConfiguration(graphName, configurationKeys); + assertNotNull(ConfiguredGraphFactory.open(graphName)); + assertFalse(ConfiguredGraphFactory.getConfiguration(graphName).containsKey(STORAGE_HOSTS.toStringWithoutRoot())); + assertFalse(ConfiguredGraphFactory.getConfiguration(graphName).containsKey(CONNECTION_TIMEOUT.toStringWithoutRoot())); + } finally { + ConfiguredGraphFactory.removeConfiguration(graphName); + ConfiguredGraphFactory.close(graphName); + } + } + @Test public void dropGraphShouldRemoveGraphFromCache() throws Exception { final MapConfiguration graphConfig = getGraphConfig(); diff --git a/janusgraph-core/src/main/java/org/janusgraph/core/ConfiguredGraphFactory.java b/janusgraph-core/src/main/java/org/janusgraph/core/ConfiguredGraphFactory.java index 4801e8361b..b0e7b25b71 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/core/ConfiguredGraphFactory.java +++ b/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 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<String, Object> */ - public static Map getConfiguration(final String configName) { + public static Map getConfiguration(final String graphName) { final ConfigurationManagementGraph configManagementGraph = getConfigGraphManagementInstance(); - return configManagementGraph.getConfiguration(configName); + return configManagementGraph.getConfiguration(graphName); } /** diff --git a/janusgraph-core/src/main/java/org/janusgraph/core/schema/JanusGraphConfiguration.java b/janusgraph-core/src/main/java/org/janusgraph/core/schema/JanusGraphConfiguration.java index 80a052687f..aa82e7d780 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/core/schema/JanusGraphConfiguration.java +++ b/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); + } diff --git a/janusgraph-core/src/main/java/org/janusgraph/diskstorage/configuration/UserModifiableConfiguration.java b/janusgraph-core/src/main/java/org/janusgraph/diskstorage/configuration/UserModifiableConfiguration.java index 4a6b6a231b..4f3d32f933 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/diskstorage/configuration/UserModifiableConfiguration.java +++ b/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 */ diff --git a/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/management/ManagementSystem.java b/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/management/ManagementSystem.java index 42301c2ab5..0e43eb9e39 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/graphdb/database/management/ManagementSystem.java +++ b/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); + } } diff --git a/janusgraph-core/src/main/java/org/janusgraph/graphdb/management/ConfigurationManagementGraph.java b/janusgraph-core/src/main/java/org/janusgraph/graphdb/management/ConfigurationManagementGraph.java index 1275c7a20e..cc76af9af2 100644 --- a/janusgraph-core/src/main/java/org/janusgraph/graphdb/management/ConfigurationManagementGraph.java +++ b/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 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,14 @@ private void updateVertexWithProperties(String propertyKey, Object propertyValue } } + private void removeVertexProperties(String propertyKey, Object propertyValue, Set set) { + final GraphTraversalSource g = getTraversal(); + if (g.V().has(propertyKey, propertyValue).hasNext()) { + g.V().has(propertyKey, propertyValue).properties(set.toArray(new String[0])).drop().iterate(); + graph.tx().commit(); + } + } + private Map deserializeVertexProperties(Map map) { HashMap deserializedProperties = new HashMap<>(); map.forEach((key, value) -> { diff --git a/janusgraph-test/src/test/java/org/janusgraph/diskstorage/configuration/UserModifiableConfigurationTest.java b/janusgraph-test/src/test/java/org/janusgraph/diskstorage/configuration/UserModifiableConfigurationTest.java new file mode 100644 index 0000000000..035054dc48 --- /dev/null +++ b/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()); + } +}