[ 
https://issues.apache.org/jira/browse/TINKERPOP-1438?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=15943311#comment-15943311
 ] 

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_r108175942
  
    --- 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 --
    
    Sounds good to me.


> 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)

Reply via email to