tinkerpop-commits mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From spmalle...@apache.org
Subject [5/7] tinkerpop git commit: Update code according to PR comments/suggestions
Date Mon, 17 Apr 2017 13:30:07 GMT
Update code according to PR comments/suggestions


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/410707bf
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/410707bf
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/410707bf

Branch: refs/heads/master
Commit: 410707bf386d7736620f248668c1d6b082fbe11f
Parents: 32374d9
Author: dpitera <dpitera@us.ibm.com>
Authored: Thu Mar 16 11:32:47 2017 -0400
Committer: dpitera <dpitera@us.ibm.com>
Committed: Thu Mar 30 15:12:23 2017 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  8 ++-
 .../src/reference/gremlin-applications.asciidoc |  2 +-
 .../upgrade/release-3.2.x-incubating.asciidoc   | 13 ++--
 .../gremlin/server/AbstractChannelizer.java     |  2 +-
 .../tinkerpop/gremlin/server/GraphManager.java  | 70 +++++++++++++-----
 .../tinkerpop/gremlin/server/GremlinServer.java | 10 +--
 .../gremlin/server/op/session/Session.java      |  8 +--
 .../op/traversal/TraversalOpProcessor.java      |  6 +-
 .../server/util/DefaultGraphManager.java        | 69 ++++++++++++------
 .../server/util/ServerGremlinExecutor.java      | 36 +++++-----
 .../driver/remote/RemoteGraphProvider.java      |  2 +-
 .../server/GremlinServerIntegrateTest.java      |  2 +-
 .../server/util/DefaultGraphManagerTest.java    | 74 ++++++++++++++++----
 13 files changed, 206 insertions(+), 96 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 1e85497..7e774cb 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -56,9 +56,11 @@ TinkerPop 3.2.5 (Release Date: NOT OFFICIALLY RELEASED YET)
 * Improved error handling of compilation failures for very large or highly parameterized
script sent to Gremlin Server.
 * Fixed a bug in `RangeByIsCountStrategy` that changed the meaning of inner traversals.
 * Improved Gremlin-Python Driver implementation by adding a threaded client with basic connection
pooling and support for pluggable websocket clients.
-* Changed GraphManager from a final class implementation to an interface.
-* Updated GraphManager interface to include methods for opening/instantiating a graph and
closing a graph.
-* Implemented DefaultGraphManager to include previous GraphManager functionality and adhere
to updated interface.
+* Changed `GraphManager` from a final class implementation to an interface.
+* Updated `GraphManager` interface to include methods for opening/instantiating a graph and
closing a graph.
+* Implemented `DefaultGraphManager` to include previous `GraphManager` functionality and
adhere to updated interface.
+* Deprecated `GraphManager.getGraphs()` and added `GraphManager.getGraphNames()`.
+* Deprecated `GraphManager.getTraversalSources()` and added `GraphManager.getTraversalSourceNames()`.
 
 [[release-3-2-4]]
 TinkerPop 3.2.4 (Release Date: February 8, 2017)

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/docs/src/reference/gremlin-applications.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/reference/gremlin-applications.asciidoc b/docs/src/reference/gremlin-applications.asciidoc
index 3f4f72f..a748f79 100644
--- a/docs/src/reference/gremlin-applications.asciidoc
+++ b/docs/src/reference/gremlin-applications.asciidoc
@@ -1075,7 +1075,7 @@ The following table describes the various configuration options that
Gremlin Ser
 |authentication.className |The fully qualified classname of an `Authenticator` implementation
to use.  If this setting is not present, then authentication is effectively disabled. |`AllowAllAuthenticator`
 |authentication.config |A `Map` of configuration settings to be passes to the `Authenticator`
when it is constructed.  The settings available are dependent on the implementation. |_none_
 |channelizer |The fully qualified classname of the `Channelizer` implementation to use. 
A `Channelizer` is a "channel initializer" which Gremlin Server uses to define the type of
processing pipeline to use.  By allowing different `Channelizer` implementations, Gremlin
Server can support different communication protocols (e.g. Websockets, Java NIO, etc.). |`WebSocketChannelizer`
-|graphManager |The fully qualified classname of the `GraphManager` implementation to use.
 A `GraphManager` is a class that adheres to the Tinkerpop GraphManager interface, allowing
custom implementations for storing and managing graph references, as well as defining custom
methods to open and close graphs instantiations. It is important to note that the Tinkerpop
Http and WebSocketChannelizers auto-commit and auto-rollback based on the graphs stored in
the graphManager upon script execution completion. |`DefaultGraphManager`
+|graphManager |The fully qualified classname of the `GraphManager` implementation to use.
 A `GraphManager` is a class that adheres to the Tinkerpop `GraphManager` interface, allowing
custom implementations for storing and managing graph references, as well as defining custom
methods to open and close graphs instantiations. It is important to note that the Tinkerpop
Http and WebSocketChannelizers auto-commit and auto-rollback based on the graphs stored in
the graphManager upon script execution completion. |`DefaultGraphManager`
 |graphs |A `Map` of `Graph` configuration files where the key of the `Map` becomes the name
to which the `Graph` will be bound and the value is the file name of a `Graph` configuration
file. |_none_
 |gremlinPool |The number of "Gremlin" threads available to execute actual scripts in a `ScriptEngine`.
This pool represents the workers available to handle blocking operations in Gremlin Server.
When set to `0`, Gremlin Server will use the value provided by `Runtime.availableProcessors()`.
|0
 |host |The name of the host to bind the server to. |localhost

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/docs/src/upgrade/release-3.2.x-incubating.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
index 7fa9e90..02959d1 100644
--- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
@@ -68,11 +68,14 @@ See: link:https://issues.apache.org/jira/browse/TINKERPOP-1387[TINKERPOP-1387]
 
 GraphManager versus DefaultGraphManager
 ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-The Gremlin-Server previously implemented its own final GraphManager class. Now, the GraphManager
has been 
-updated to an interface, and users can supply their own GraphManager implementations in their
YAML. The
-previous GraphManager class was traditionally only to be used by internal Gremlin-Server
classes; however,
-if you previously used the GraphManager, then this changeset will be `breaking` for you.
To fix the break,
-you can upgrade your code to use the new DefaultGraphManager.
+The Gremlin-Server previously implemented its own final `GraphManager` class. Now, the `GraphManager`
has been 
+updated to an interface, and users can supply their own `GraphManager` implementations in
their YAML. The
+previous `GraphManager` class was traditionally only to be used by internal Gremlin-Server
classes; however,
+if you previously used the `GraphManager`, then this changeset will be `breaking` for you.
To fix the break,
+you can upgrade your code to use the new `DefaultGraphManager`.
+
+The `GraphManager` also deprecated the usage of `getGraphs()` so please use `getGraphNames()`
instead.
+The `GraphManager` also deprecated the usage of `getTraversalSources()` so please use `getTraversalSourceNames()`
instead.
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-1438[TINKERPOP-1438]
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java
index 2a5ca55..c5bf010 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/AbstractChannelizer.java
@@ -181,7 +181,7 @@ public abstract class AbstractChannelizer extends ChannelInitializer<SocketChann
                 }
 
                 final MessageSerializer serializer = (MessageSerializer) clazz.newInstance();
-                Map<String, Graph> graphsDefinedAtStartup = new HashMap<String,
Graph>();
+                final Map<String, Graph> graphsDefinedAtStartup = new HashMap<String,
Graph>();
                 for (String graphName : settings.graphs.keySet()) {
                     graphsDefinedAtStartup.put(graphName, graphManager.getGraph(graphName));
                 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
index 18ef175..23c019b 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
@@ -25,35 +25,55 @@ import org.apache.tinkerpop.gremlin.structure.Transaction;
 import javax.script.Bindings;
 import java.util.Map;
 import java.util.Set;
-import java.util.function.Supplier;
+import java.util.function.Function;
 
+/**
+ * The {@link GraphManager} interface allows for reference tracking of Graph references through
+ * a {@link Map<String, Graph>}; the interface plugs into the lifeline of gremlin script
+ * executions, meaning that commit() and rollback() will be called on all graphs stored in
the
+ * graph reference tracker at the end of the script executions; you may want to implement
+ * this interface if you want to define a custom graph instantiation/closing mechanism; note
that
+ * the interface also defines similar features for {@link TraversalSource} objects.
+ */
 public interface GraphManager {
     /**
+     * @Deprecated This returns a {@link Map} that should be immutable. Please refer to
+     * getGraphNames() for replacement.
+     *
      * 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
      */
+    @Deprecated
     public Map<String, Graph> getGraphs();
-    
+
+    /**
+     * Get a {@link Set} of {@link String} graphNames corresponding to names stored in the
graph's
+     * reference tracker.
+     */
+    public Set<String> getGraphNames();
     /**
      * Get {@link Graph} instance whose name matches {@link gName}
      *
-     * @return {@link Graph} if exists, else null 
+     * @return {@link Graph} if exists, else null
      */
-    public Graph getGraph(String gName);
+    public Graph getGraph(final String gName);
 
     /**
-     * Add {@link Graph} g with name {@link String} gName to 
-     * {@link Map<String, Graph>} returned by call to getGraphs()
+     * Add or update {@link Graph} g with name {@link String} gName to
+     * {@link Map<String, Graph>}
      */
-    public void addGraph(String gName, Graph g);
+    public void putGraph(final String gName, final Graph g);
 
     /**
+     * @Deprecated Please treat as immutable {@link Map} and refer to getTraversalSourceNames()
+     *
      * 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
      */
+    @Deprecated
     public Map<String, TraversalSource> getTraversalSources();
 
     /**
@@ -61,18 +81,29 @@ public interface GraphManager {
      *
      * @return {@link TraversalSource} if exists, else null
      */
-    
-    public TraversalSource getTraversalSource(String tsName);
+
+    /**
+     * Get a {@link Set} of {@link String} traversalSourceNames to names stored in the
+     * traversalSources's reference tracker.
+     */
+    public Set<String> getTraversalSourceNames();
+
+    public TraversalSource getTraversalSource(final String tsName);
     /**
      * Get the {@link Graph} and {@link TraversalSource} list as a set of bindings.
      */
-    
+
     /**
-     * Add {@link TraversalSource} ts with name {@link String} tsName to 
+     * Add or update {@link TraversalSource} ts with name {@link String} tsName to
      * {@link Map<String, TraversalSource>} returned by call to getTraversalSources()
      */
-    public void addTraversalSource(String tsName, TraversalSource ts);
-    
+    public void putTraversalSource(final String tsName, final TraversalSource ts);
+
+    /**
+     * Remove {@link TraversalSource} with tsName from {@link Map<String, TraversalSource}.
+     */
+    public TraversalSource removeTraversalSource(final String tsName);
+
     public Bindings getAsBindings();
 
     /**
@@ -96,12 +127,17 @@ public interface GraphManager {
     public void commit(final Set<String> graphSourceNamesToCloseTxOn);
 
     /**
-     * Implementation that allows for custom graph-opening implementations.
+     * 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(String graphName, Supplier<Graph> supplier);
+    public Graph openGraph(final String graphName, final Function<String, Graph> supplier);
 
     /**
-     * Implementation that allows for custom graph-closing implementations.
+     * Implementation that allows for custom graph-closing implementations;
+     * this method should remove the {@link Graph} graph from the {@link Object}
+     * tracking {@link Graph} references.
      */
-    public void closeGraph(Graph graph);
+    public Graph removeGraph(final String graphName) throws Exception;
 }

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
index 49b2f28..db1caae 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GremlinServer.java
@@ -301,14 +301,14 @@ public class GremlinServer {
                 logger.warn("Timeout waiting for boss/worker thread pools to shutdown - continuing
with shutdown process.");
             }
 
-            serverGremlinExecutor.getGraphManager().getGraphs().forEach((k, v) -> {
-                logger.debug("Closing Graph instance [{}]", k);
+            serverGremlinExecutor.getGraphManager().getGraphNames().stream().forEach(gName
-> {
+                logger.debug("Closing Graph instance [{}]", gName);
                 try {
-                    v.close();
+                    serverGremlinExecutor.getGraphManager().getGraph(gName).close();
                 } catch (Exception ex) {
-                    logger.warn(String.format("Exception while closing Graph instance [%s]",
k), ex);
+                    logger.warn(String.format("Exception while closing Graph instance [%s]",
gName), ex);
                 } finally {
-                    logger.info("Closed Graph instance [{}]", k);
+                    logger.info("Closed Graph instance [{}]", gName);
                 }
             });
 

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
index 6961339..93defd2 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/session/Session.java
@@ -194,20 +194,20 @@ public class Session {
 
         if (!force) {
             // when the session is killed open transaction should be rolled back
-            graphManager.getGraphs().entrySet().forEach(kv -> {
-                final Graph g = kv.getValue();
+            graphManager.getGraphNames().stream().forEach(gName -> {
+                final Graph g = graphManager.getGraph(gName);
                 if (g.features().graph().supportsTransactions()) {
                     // have to execute the rollback in the executor because the transaction
is associated with
                     // that thread of execution from this session
                     try {
                         executor.submit(() -> {
                             if (g.tx().isOpen()) {
-                                logger.info("Rolling back open transactions on {} before
killing session: {}", kv.getKey(), session);
+                                logger.info("Rolling back open transactions on {} before
killing session: {}", gName, session);
                                 g.tx().rollback();
                             }
                         }).get(configuredPerGraphCloseTimeout, TimeUnit.MILLISECONDS);
                     } catch (Exception ex) {
-                        logger.warn(String.format("An error occurred while attempting rollback
on %s when closing session: %s", kv.getKey(), session), ex);
+                        logger.warn(String.format("An error occurred while attempting rollback
on %s when closing session: %s", gName, session), ex);
                     }
                 }
             });

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
index c5a05e4..99e9d9b 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/op/traversal/TraversalOpProcessor.java
@@ -223,7 +223,7 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
 
     private static void validateTraversalSourceAlias(final Context ctx, final RequestMessage
message, final Map<String, String> aliases) throws OpProcessorException {
         final String traversalSourceBindingForAlias = aliases.values().iterator().next();
-        if (!ctx.getGraphManager().getTraversalSources().containsKey(traversalSourceBindingForAlias))
{
+        if (null == ctx.getGraphManager().getTraversalSource(traversalSourceBindingForAlias))
{
             final String msg = String.format("The traversal source [%s] for alias [%s] is
not configured on the server.", traversalSourceBindingForAlias, Tokens.VAL_TRAVERSAL_SOURCE_ALIAS);
             throw new OpProcessorException(msg, ResponseMessage.build(message).code(ResponseStatusCode.REQUEST_ERROR_INVALID_REQUEST_ARGUMENTS).statusMessage(msg).create());
         }
@@ -265,7 +265,7 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
 
         final GraphManager graphManager = context.getGraphManager();
         final String traversalSourceName = aliases.entrySet().iterator().next().getValue();
-        final TraversalSource g = graphManager.getTraversalSources().get(traversalSourceName);
+        final TraversalSource g = graphManager.getTraversalSource(traversalSourceName);
 
         final Timer.Context timerContext = traversalOpTimer.time();
         try {
@@ -341,7 +341,7 @@ public class TraversalOpProcessor extends AbstractOpProcessor {
 
         final GraphManager graphManager = context.getGraphManager();
         final String traversalSourceName = aliases.entrySet().iterator().next().getValue();
-        final TraversalSource g = graphManager.getTraversalSources().get(traversalSourceName);
+        final TraversalSource g = graphManager.getTraversalSource(traversalSourceName);
 
         final Traversal.Admin<?, ?> traversal;
         try {

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManager.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManager.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManager.java
index 807213a..54f424c 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManager.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManager.java
@@ -36,7 +36,7 @@ import java.util.Map;
 import java.util.Set;
 import java.util.concurrent.ConcurrentHashMap;
 import java.util.function.Predicate;
-import java.util.function.Supplier;
+import java.util.function.Function;
 
 /**
  * Holder for {@link Graph} and {@link TraversalSource} instances configured for the server
to be passed to script
@@ -68,46 +68,65 @@ public final class DefaultGraphManager implements GraphManager {
     }
 
     /**
+     * @Deprecated The {@link Map} returned should be immutable. Please refer to
+     * getGraphNames().
+     *
      * Get a list of the {@link Graph} instances and their binding names as defined in the
Gremlin Server
      * configuration file.
      *
      * @return a {@link Map} where the key is the name of the {@link Graph} and the value
is the {@link Graph} itself
      */
-    public Map<String, Graph> getGraphs() {
+    @Deprecated
+    public final Map<String, Graph> getGraphs() {
         return graphs;
     }
 
-    public Graph getGraph(String gName) {
+    public final Set<String> getGraphNames() {
+        return graphs.keySet();
+    }
+
+    public final Graph getGraph(final String gName) {
         return graphs.get(gName);
     }
 
-    public void addGraph(String gName, Graph g) {
+    public final void putGraph(final String gName, final Graph g) {
         graphs.put(gName, g);
     }
 
     /**
+     * @Deprecated
+     *
      * Get a list of the {@link TraversalSource} instances and their binding names as defined
by Gremlin Server
      * initialization scripts.
      *
      * @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() {
+    @Deprecated
+    public final Map<String, TraversalSource> getTraversalSources() {
         return traversalSources;
     }
 
-    public TraversalSource getTraversalSource(String tsName) {
+    public final Set<String> getTraversalSourceNames() {
+        return traversalSources.keySet();
+    }
+
+    public final TraversalSource getTraversalSource(final String tsName) {
         return traversalSources.get(tsName);
     }
 
-    public void addTraversalSource(String tsName, TraversalSource ts) {
+    public final void putTraversalSource(final String tsName, final TraversalSource ts) {
         traversalSources.put(tsName, ts);
     }
 
+    public final TraversalSource removeTraversalSource(final String tsName) {
+        return traversalSources.remove(tsName);
+    }
+
     /**
      * Get the {@link Graph} and {@link TraversalSource} list as a set of bindings.
      */
-    public Bindings getAsBindings() {
+    public final Bindings getAsBindings() {
         final Bindings bindings = new SimpleBindings();
         graphs.forEach(bindings::put);
         traversalSources.forEach(bindings::put);
@@ -117,7 +136,7 @@ public final class DefaultGraphManager implements GraphManager {
     /**
      * Rollback transactions across all {@link Graph} objects.
      */
-    public void rollbackAll() {
+    public final void rollbackAll() {
         graphs.entrySet().forEach(e -> {
             final Graph graph = e.getValue();
             if (graph.features().graph().supportsTransactions() && graph.tx().isOpen())
@@ -128,14 +147,14 @@ public final class DefaultGraphManager implements GraphManager {
     /**
      * Selectively rollback transactions on the specified graphs or the graphs of traversal
sources.
      */
-    public void rollback(final Set<String> graphSourceNamesToCloseTxOn) {
+    public final void rollback(final Set<String> graphSourceNamesToCloseTxOn) {
         closeTx(graphSourceNamesToCloseTxOn, Transaction.Status.ROLLBACK);
     }
 
     /**
      * Commit transactions across all {@link Graph} objects.
      */
-    public void commitAll() {
+    public final void commitAll() {
         graphs.entrySet().forEach(e -> {
             final Graph graph = e.getValue();
             if (graph.features().graph().supportsTransactions() && graph.tx().isOpen())
@@ -146,36 +165,40 @@ public final class DefaultGraphManager implements GraphManager {
     /**
      * Selectively commit transactions on the specified graphs or the graphs of traversal
sources.
      */
-    public void commit(final Set<String> graphSourceNamesToCloseTxOn) {
+    public final void commit(final Set<String> graphSourceNamesToCloseTxOn) {
         closeTx(graphSourceNamesToCloseTxOn, Transaction.Status.COMMIT);
     }
 
     /**
-     * Basic graphManager has basic openGraph function.
+     * If {@link Map} containing {@link Graph} references contains one corresponding to
+     * {@link String} graphName, then we return that {@link Graph}; otherwise, instantiate
a
+     * new {@link Graph} using the {@link Supplier}, add it to the {@link Map} tracking {@link
Graph}
+     * references, and return that {@link Graph}.
      */
-    public Graph openGraph(String graphName, Supplier<Graph> supplier) {
+    public final Graph openGraph(final String graphName, final Function<String, Graph>
supplier) {
         final Graph graph = graphs.get(graphName);
         if (null != graph) {
             return graph;
         }
-        return supplier.get();
+        final Graph newGraph = supplier.apply(graphName);
+        putGraph(graphName, newGraph);
+        return newGraph;
     }
 
     /**
-     *  Close graph
+     * Remove {@link Graph} corresponding to {@link String} graphName from {@link Map}
+     * tracking graph references and close the {@link Graph}.
      */
-    public void closeGraph(Graph graph) {
-        try {
-            graph.close();
-        } catch (Exception e) {
-            throw new RuntimeException(e);
-        }
+    public final Graph removeGraph(final String graphName) throws Exception {
+        Graph graph = graphs.remove(graphName);
+        graph.close();
+        return graph;
     }
 
     /**
      * Selectively close transactions on the specified graphs or the graphs of traversal
sources.
      */
-    private void closeTx(final Set<String> graphSourceNamesToCloseTxOn, final Transaction.Status
tx) {
+    private final void closeTx(final Set<String> graphSourceNamesToCloseTxOn, final
Transaction.Status tx) {
         final Set<Graph> graphsToCloseTxOn = new HashSet<>();
 
         // by the time this method has been called, it should be validated that the source/graph
is present.

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/ServerGremlinExecutor.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/ServerGremlinExecutor.java
b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/ServerGremlinExecutor.java
index 9fa28d7..8398dbb 100644
--- a/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/ServerGremlinExecutor.java
+++ b/gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/util/ServerGremlinExecutor.java
@@ -105,22 +105,22 @@ public class ServerGremlinExecutor<T extends ScheduledExecutorService>
{
         this.settings = settings;
 
         if (null == graphManager) {
-          try {
-            final Class<?> clazz = Class.forName(settings.graphManager);
-            final Constructor c = clazz.getConstructor(Settings.class);
-            graphManager = (GraphManager) c.newInstance(settings);
-          } catch (ClassNotFoundException e) {
-            logger.error("Could not find GraphManager implementation "
-                         + "defined by the 'graphManager' setting as: {}",
-                         settings.graphManager);
-            throw new RuntimeException(e);
-          } catch (Exception e) {
-            logger.error("Could not invoke constructor on class {} (defined by "
-                         + "the 'graphManager' setting) with one argument of "
-                         + "class Settings",
-                         settings.graphManager);
-            throw new RuntimeException(e);
-          }
+            try {
+                final Class<?> clazz = Class.forName(settings.graphManager);
+                final Constructor c = clazz.getConstructor(Settings.class);
+                graphManager = (GraphManager) c.newInstance(settings);
+            } catch (ClassNotFoundException e) {
+                logger.error("Could not find GraphManager implementation "
+                             + "defined by the 'graphManager' setting as: {}",
+                             settings.graphManager);
+                throw new RuntimeException(e);
+            } catch (Exception e) {
+                logger.error("Could not invoke constructor on class {} (defined by "
+                             + "the 'graphManager' setting) with one argument of "
+                             + "class Settings",
+                             settings.graphManager);
+                throw new RuntimeException(e);
+            }
         }
 
         if (null == gremlinExecutorService) {
@@ -189,14 +189,14 @@ public class ServerGremlinExecutor<T extends ScheduledExecutorService>
{
         // re-apply those references back
         gremlinExecutor.getGlobalBindings().entrySet().stream()
                 .filter(kv -> kv.getValue() instanceof Graph)
-                .forEach(kv -> this.graphManager.addGraph(kv.getKey(), (Graph) kv.getValue()));
+                .forEach(kv -> this.graphManager.putGraph(kv.getKey(), (Graph) kv.getValue()));
 
         // script engine init may have constructed the TraversalSource bindings - store them
in Graphs object
         gremlinExecutor.getGlobalBindings().entrySet().stream()
                 .filter(kv -> kv.getValue() instanceof TraversalSource)
                 .forEach(kv -> {
                     logger.info("A {} is now bound to [{}] with {}", kv.getValue().getClass().getSimpleName(),
kv.getKey(), kv.getValue());
-                    this.graphManager.addTraversalSource(kv.getKey(), (TraversalSource) kv.getValue());
+                    this.graphManager.putTraversalSource(kv.getKey(), (TraversalSource) kv.getValue());
                 });
 
         // determine if the initialization scripts introduced LifeCycleHook objects - if
so we need to gather them

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java
index a19ecc8..ab0093d 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/driver/remote/RemoteGraphProvider.java
@@ -83,7 +83,7 @@ public class RemoteGraphProvider extends AbstractGraphProvider implements
AutoCl
                                                     final LoadGraphWith.GraphData loadGraphWith)
{
         final String serverGraphName = getServerGraphName(loadGraphWith);
 
-        final Supplier<Graph> graphGetter = () -> server.getServerGremlinExecutor().getGraphManager().getGraphs().get(serverGraphName);
+        final Supplier<Graph> graphGetter = () -> server.getServerGremlinExecutor().getGraphManager().getGraph(serverGraphName);
         return new HashMap<String, Object>() {{
             put(Graph.GRAPH, RemoteGraph.class.getName());
             put(RemoteGraph.GREMLIN_REMOTE_GRAPH_REMOTE_CONNECTION_CLASS, DriverRemoteConnection.class.getName());

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
index af25be5..9f827d0 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/GremlinServerIntegrateTest.java
@@ -110,7 +110,7 @@ public class GremlinServerIntegrateTest extends AbstractGremlinServerIntegration
     private static final String CLIENT_CRT = "src/test/resources/client.crt";
 
     private Log4jRecordingAppender recordingAppender = null;
-    private final Supplier<Graph> graphGetter = () -> server.getServerGremlinExecutor().getGraphManager().getGraphs().get("graph");
+    private final Supplier<Graph> graphGetter = () -> server.getServerGremlinExecutor().getGraphManager().getGraph("graph");
     private final Configuration conf = new BaseConfiguration() {{
         setProperty(Graph.GRAPH, RemoteGraph.class.getName());
         setProperty(GREMLIN_REMOTE_CONNECTION_CLASS, DriverRemoteConnection.class.getName());

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/410707bf/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManagerTest.java
----------------------------------------------------------------------
diff --git a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManagerTest.java
b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManagerTest.java
index a2ec75d..ff73236 100644
--- a/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManagerTest.java
+++ b/gremlin-server/src/test/java/org/apache/tinkerpop/gremlin/server/util/DefaultGraphManagerTest.java
@@ -26,6 +26,7 @@ import org.junit.Test;
 
 import javax.script.Bindings;
 import java.util.Map;
+import java.util.Set;
 
 import static org.hamcrest.CoreMatchers.instanceOf;
 import static org.hamcrest.CoreMatchers.is;
@@ -42,12 +43,13 @@ public class DefaultGraphManagerTest {
     public void shouldReturnGraphs() {
         final Settings settings = Settings.read(DefaultGraphManagerTest.class.getResourceAsStream("../gremlin-server-integration.yaml"));
         final GraphManager graphManager = new DefaultGraphManager(settings);
-        final Map<String, Graph> m = graphManager.getGraphs();
+        final Set<String> graphNames = graphManager.getGraphNames();
 
-        assertNotNull(m);
-        assertEquals(1, m.size());
-        assertThat(m.containsKey("graph"), is(true));
-        assertThat(m.get("graph"), instanceOf(TinkerGraph.class));
+        assertNotNull(graphNames);
+        assertEquals(1, graphNames.size());
+
+        assertEquals(graphNames.toArray()[0], "graph");
+        assertThat(graphManager.getGraph("graph"), instanceOf(TinkerGraph.class));
     }
 
     @Test
@@ -61,7 +63,7 @@ public class DefaultGraphManagerTest {
         assertThat(bindings.get("graph"), instanceOf(TinkerGraph.class));
         assertThat(bindings.containsKey("graph"), is(true));
     }
-    
+
     @Test
     public void shouldGetGraph() {
         final Settings settings = Settings.read(DefaultGraphManagerTest.class.getResourceAsStream("../gremlin-server-integration.yaml"));
@@ -71,18 +73,62 @@ public class DefaultGraphManagerTest {
         assertNotNull(graph);
         assertThat(graph, instanceOf(TinkerGraph.class));
     }
-    
+
     @Test
     public void shouldGetDynamicallyAddedGraph() {
         final Settings settings = Settings.read(DefaultGraphManagerTest.class.getResourceAsStream("../gremlin-server-integration.yaml"));
         final GraphManager graphManager = new DefaultGraphManager(settings);
         final Graph graph = graphManager.getGraph("graph"); //fake out a graph instance
-        graphManager.addGraph("newGraph", graph);
-        
-        final Map<String, Graph> m = graphManager.getGraphs();
-        assertNotNull(m);
-        assertEquals(2, m.size());
-        assertThat(m.containsKey("newGraph"), is(true));
-        assertThat(m.get("newGraph"), instanceOf(TinkerGraph.class));
+        graphManager.putGraph("newGraph", graph);
+
+        final Set<String> graphNames = graphManager.getGraphNames();
+        assertNotNull(graphNames);
+        assertEquals(2, graphNames.size());
+        assertThat(graphNames.contains("newGraph"), is(true));
+        assertThat(graphManager.getGraph("newGraph"), instanceOf(TinkerGraph.class));
+    }
+
+    @Test
+    public void shouldNotGetRemovedGraph() throws Exception {
+        final Settings settings = Settings.read(DefaultGraphManagerTest.class.getResourceAsStream("../gremlin-server-integration.yaml"));
+        final GraphManager graphManager = new DefaultGraphManager(settings);
+        final Graph graph = graphManager.getGraph("graph"); //fake out a graph instance
+        graphManager.putGraph("newGraph", graph);
+        final Set<String> graphNames = graphManager.getGraphNames();
+        assertNotNull(graphNames);
+        assertEquals(2, graphNames.size());
+        assertThat(graphNames.contains("newGraph"), is(true));
+        assertThat(graphManager.getGraph("newGraph"), instanceOf(TinkerGraph.class));
+
+        graphManager.removeGraph("newGraph");
+
+        final Set<String> graphNames2 = graphManager.getGraphNames();
+        assertEquals(1, graphNames2.size());
+        assertThat(graphNames2.contains("newGraph"), is(false));
+    }
+
+    @Test
+    public void openGraphShouldReturnExistingGraph() {
+        final Settings settings = Settings.read(DefaultGraphManagerTest.class.getResourceAsStream("../gremlin-server-integration.yaml"));
+        final GraphManager graphManager = new DefaultGraphManager(settings);
+
+        Graph graph = graphManager.openGraph("graph", null);
+        assertNotNull(graph);
+        assertThat(graph, instanceOf(TinkerGraph.class));
+    }
+
+    @Test
+    public void openGraphShouldReturnNewGraphUsingThunk() {
+        final Settings settings = Settings.read(DefaultGraphManagerTest.class.getResourceAsStream("../gremlin-server-integration.yaml"));
+        final GraphManager graphManager = new DefaultGraphManager(settings);
+
+        Graph graph = graphManager.getGraph("graph"); //fake out graph instance
+
+        Graph newGraph = graphManager.openGraph("newGraph", (String gName) -> {
+            return graph;
+        });
+
+        assertNotNull(graph);
+        assertThat(graph, instanceOf(TinkerGraph.class));
     }
 }


Mime
View raw message