Github user spmallette commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/569#discussion_r107671911
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
---
@@ -21,139 +21,98 @@
import org.apache.tinkerpop.gremlin.process.traversal.TraversalSource;
import org.apache.tinkerpop.gremlin.structure.Graph;
import org.apache.tinkerpop.gremlin.structure.Transaction;
-import org.apache.tinkerpop.gremlin.structure.util.GraphFactory;
-import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
-import org.slf4j.Logger;
-import org.slf4j.LoggerFactory;
import javax.script.Bindings;
-import javax.script.SimpleBindings;
-import java.util.HashSet;
import java.util.Map;
import java.util.Set;
-import java.util.concurrent.ConcurrentHashMap;
-import java.util.function.Predicate;
-
-/**
- * Holder for {@link Graph} and {@link TraversalSource} instances
configured for the server to be passed to script
- * engine bindings. The {@link Graph} instances are read from the {@link
Settings} for Gremlin Server as defined in
- * the configuration file. The {@link TraversalSource} instances are
rebound to the {@code GraphManager} once
- * initialization scripts construct them.
- */
-public final class GraphManager {
- private static final Logger logger =
LoggerFactory.getLogger(GremlinServer.class);
-
- private final Map<String, Graph> graphs = new ConcurrentHashMap<>();
- private final Map<String, TraversalSource> traversalSources = new
ConcurrentHashMap<>();
+import java.util.function.Supplier;
+public interface GraphManager {
/**
- * Create a new instance using the {@link Settings} from Gremlin
Server.
+ * Get a list of the {@link Graph} instances and their binding names
+ *
+ * @return a {@link Map} where the key is the name of the {@link
Graph} and the value is the {@link Graph} itself
*/
- public GraphManager(final Settings settings) {
- settings.graphs.entrySet().forEach(e -> {
- try {
- final Graph newGraph = GraphFactory.open(e.getValue());
- graphs.put(e.getKey(), newGraph);
- logger.info("Graph [{}] was successfully configured via
[{}].", e.getKey(), e.getValue());
- } catch (RuntimeException re) {
- logger.warn(String.format("Graph [%s] configured at [%s]
could not be instantiated and will not be available in Gremlin Server.
GraphFactory message: %s",
- e.getKey(), e.getValue(), re.getMessage()), re);
- if (re.getCause() != null) logger.debug("GraphFactory
exception", re.getCause());
- }
- });
- }
+ public Map<String, Graph> getGraphs();
/**
- * Get a list of the {@link Graph} instances and their binding names
as defined in the Gremlin Server
- * configuration file.
+ * Get {@link Graph} instance whose name matches {@link gName}
*
- * @return a {@link Map} where the key is the name of the {@link
Graph} and the value is the {@link Graph} itself
+ * @return {@link Graph} if exists, else null
*/
- public Map<String, Graph> getGraphs() {
- return graphs;
- }
+ public Graph getGraph(final String gName);
/**
- * Get a list of the {@link TraversalSource} instances and their
binding names as defined by Gremlin Server
- * initialization scripts.
+ * Add {@link Graph} g with name {@link String} gName to
+ * {@link Map<String, Graph>} returned by call to getGraphs()
+ */
+ public void addGraph(final String gName, final Graph g);
+
+ /**
+ * Get a list of the {@link TraversalSource} instances and their
binding names
*
* @return a {@link Map} where the key is the name of the {@link
TraversalSource} and the value is the
* {@link TraversalSource} itself
*/
- public Map<String, TraversalSource> getTraversalSources() {
- return traversalSources;
- }
+ public Map<String, TraversalSource> getTraversalSources();
+
+ /**
+ * Get {@link TraversalSource} instance whose name matches {@link
tsName}
+ *
+ * @return {@link TraversalSource} if exists, else null
+ */
+ public TraversalSource getTraversalSource(final String tsName);
/**
* Get the {@link Graph} and {@link TraversalSource} list as a set of
bindings.
*/
- public Bindings getAsBindings() {
- final Bindings bindings = new SimpleBindings();
- graphs.forEach(bindings::put);
- traversalSources.forEach(bindings::put);
- return bindings;
- }
+
+ /**
+ * Add {@link TraversalSource} ts with name {@link String} tsName to
+ * {@link Map<String, TraversalSource>} returned by call to
getTraversalSources()
+ */
+ public void addTraversalSource(final String tsName, final
TraversalSource ts);
+
+ public Bindings getAsBindings();
/**
* Rollback transactions across all {@link Graph} objects.
*/
- public void rollbackAll() {
- graphs.entrySet().forEach(e -> {
- final Graph graph = e.getValue();
- if (graph.features().graph().supportsTransactions() &&
graph.tx().isOpen())
- graph.tx().rollback();
- });
- }
+ public void rollbackAll();
/**
* Selectively rollback transactions on the specified graphs or the
graphs of traversal sources.
*/
- public void rollback(final Set<String> graphSourceNamesToCloseTxOn) {
- closeTx(graphSourceNamesToCloseTxOn, Transaction.Status.ROLLBACK);
- }
+ public void rollback(final Set<String> graphSourceNamesToCloseTxOn);
/**
* Commit transactions across all {@link Graph} objects.
*/
- public void commitAll() {
- graphs.entrySet().forEach(e -> {
- final Graph graph = e.getValue();
- if (graph.features().graph().supportsTransactions() &&
graph.tx().isOpen())
- graph.tx().commit();
- });
- }
+ public void commitAll();
/**
* Selectively commit transactions on the specified graphs or the
graphs of traversal sources.
*/
- public void commit(final Set<String> graphSourceNamesToCloseTxOn) {
- closeTx(graphSourceNamesToCloseTxOn, Transaction.Status.COMMIT);
- }
+ public void commit(final Set<String> graphSourceNamesToCloseTxOn);
+
+ /**
+ * Implementation that allows for custom graph-opening
implementations; if the {@link Map}
+ * tracking graph references has a {@link Graph} object corresponding
to the {@link String} graphName,
+ * then we return that {@link Graph}-- otherwise, we use the custom
{@link Supplier} to instantiate a
+ * a new {@link Graph}, add it to the {@link Map} tracking graph
references, and return said {@link Graph}.
+ */
+ public Graph openGraph(final String graphName, final Supplier<Graph>
supplier);
--- End diff --
I'm feeling like the second argument here could offer a little more
flexibility if it were `BiSupplier<String,Graph>` where the first argument was
the `graphName` passed back in on itself (or ultimately whatever name the
`GraphManager` implementation used to key the created `Graph` instance. That
could be a useful configuration parameter to something like Neo4j where it
could use the graph name to uniquely create a data directory for example.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [email protected] or file a JIRA ticket
with INFRA.
---
