[
https://issues.apache.org/jira/browse/TINKERPOP-1438?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15928100#comment-15928100
]
ASF GitHub Bot commented on TINKERPOP-1438:
-------------------------------------------
Github user dpitera commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/569#discussion_r106424327
--- Diff:
gremlin-server/src/main/java/org/apache/tinkerpop/gremlin/server/GraphManager.java
---
@@ -21,139 +21,87 @@
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 Graph getGraph(String gName);
+
+ /**
+ * Add {@link Graph} g with name {@link String} gName to
+ * {@link Map<String, Graph>} returned by call to getGraphs()
*/
- public Map<String, Graph> getGraphs() {
- return graphs;
- }
+ public void addGraph(String gName, Graph g);
/**
- * Get a list of the {@link TraversalSource} instances and their
binding names as defined by Gremlin Server
- * initialization scripts.
+ * 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(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(String tsName, 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);
/**
- * Selectively close transactions on the specified graphs or the
graphs of traversal sources.
+ * Implementation that allows for custom graph-opening implementations.
*/
- private 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.
- // might be possible that it could have been removed dynamically,
but that i'm not sure how one would do
- // that as of right now unless they were embedded in which case
they'd need to know what they were doing
- // anyway
- graphSourceNamesToCloseTxOn.forEach(r -> {
- if (graphs.containsKey(r))
- graphsToCloseTxOn.add(graphs.get(r));
- else
- graphsToCloseTxOn.add(traversalSources.get(r).getGraph());
- });
+ public Graph openGraph(String graphName, Supplier<Graph> supplier);
- graphsToCloseTxOn.forEach(graph -> {
- if (graph.features().graph().supportsTransactions() &&
graph.tx().isOpen()) {
- if (tx == Transaction.Status.COMMIT)
- graph.tx().commit();
- else
- graph.tx().rollback();
- }
- });
- }
-}
\ No newline at end of file
+ /**
+ * Implementation that allows for custom graph-closing implementations.
+ */
+ public void closeGraph(Graph graph);
--- End diff --
I think I will add both methods and allow the implementor to decide which
to use. Personally, I think it makes sense to define a close() method on the
object to be closed itself. However, this means the implementor needs to have a
`name` parameter associated with their graph, which I don't want to force them
to do.
> Consider GraphManager as an interface
> -------------------------------------
>
> Key: TINKERPOP-1438
> URL: https://issues.apache.org/jira/browse/TINKERPOP-1438
> Project: TinkerPop
> Issue Type: Improvement
> Components: server
> Affects Versions: 3.2.2
> Reporter: stephen mallette
> Priority: Minor
> Labels: breaking
>
> If {{GraphManager}} were an interface it would make embedding Gremlin Server
> easier as {{Graph}} instances could be more easily supplied by the host
> application. In doing this, It also might be good to force a
> {{TraversalSource}} to be referred to by both the {{Graph}} name and
> {{TraversalSource}} name.
--
This message was sent by Atlassian JIRA
(v6.3.15#6346)
