From 007c45c639bc791804047a1e18cc2ff98ed15482 Mon Sep 17 00:00:00 2001 From: Stephen Mallette Date: Sat, 19 Sep 2020 08:29:03 -0400 Subject: [PATCH] TINKERPOP-2421 Added requestOption capabilities for javascript This is a bit messy at the moment but all tests pass. --- .../glv/GraphTraversalSource.template | 22 +++++- gremlin-javascript/glv/generate.groovy | 1 + .../gremlin-javascript/lib/driver/client.js | 8 +-- .../lib/driver/connection.js | 23 +++--- .../lib/driver/driver-remote-connection.js | 17 ++++- .../lib/process/graph-traversal.js | 32 ++++++--- .../lib/process/traversal-strategy.js | 70 ++++++------------- .../lib/structure/io/type-serializers.js | 9 ++- .../test/integration/traversal-test.js | 14 ++-- .../test/unit/traversal-test.js | 49 ++++--------- 10 files changed, 129 insertions(+), 116 deletions(-) diff --git a/gremlin-javascript/glv/GraphTraversalSource.template b/gremlin-javascript/glv/GraphTraversalSource.template index fc553161b31..c5e490ce8a1 100644 --- a/gremlin-javascript/glv/GraphTraversalSource.template +++ b/gremlin-javascript/glv/GraphTraversalSource.template @@ -26,7 +26,7 @@ const { Traversal } = require('./traversal'); const remote = require('../driver/remote-connection'); const utils = require('../utils'); const Bytecode = require('./bytecode'); -const { TraversalStrategies, VertexProgramStrategy } = require('./traversal-strategy'); +const { TraversalStrategies, VertexProgramStrategy, OptionsStrategy } = require('./traversal-strategy'); /** @@ -76,6 +76,26 @@ class GraphTraversalSource { configuration: configuration})); } + /** + * Graph Traversal Source with method. + * @param {String} key + * @param {Object} value if not specified, the value with default to {@code true} + * @returns {GraphTraversalSource} + */ + with_(key, value=undefined) { + const val = value === undefined ? true : value; + let optionsStrategy = this.bytecode.sourceInstructions.find( + i => i[0] === "withStrategies" && i[1] instanceof OptionsStrategy); + if (optionsStrategy === undefined) { + optionsStrategy = new OptionsStrategy({[key]: val}); + return this.withStrategies(optionsStrategy); + } else { + optionsStrategy[1].configuration[key] = val; + return new this.graphTraversalSourceClass(this.graph, new TraversalStrategies(this.traversalStrategies), + this.bytecode, this.graphTraversalSourceClass, this.graphTraversalClass); + } + } + /** * Returns the string representation of the GraphTraversalSource. * @returns {string} diff --git a/gremlin-javascript/glv/generate.groovy b/gremlin-javascript/glv/generate.groovy index cd3e8998d6f..cfd1be5c070 100644 --- a/gremlin-javascript/glv/generate.groovy +++ b/gremlin-javascript/glv/generate.groovy @@ -87,6 +87,7 @@ def binding = ["enums": CoreImports.getClassImports() !it.name.equals("clone") && // Use hardcoded name to be for forward-compatibility !it.name.equals("withBindings") && + !it.name.equals(TraversalSource.Symbols.with) && !it.name.equals(TraversalSource.Symbols.withRemote) && !it.name.equals(TraversalSource.Symbols.withComputer) }. diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js index eaa9db1dad5..e8d1bafa0d7 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/client.js @@ -69,9 +69,10 @@ class Client { * Send a request to the Gremlin Server, can send a script or bytecode steps. * @param {Bytecode|string} message The bytecode or script to send * @param {Object} [bindings] The script bindings, if any. + * @param {Object} [requestOptions] Configuration specific to the current request. * @returns {Promise} */ - submit(message, bindings) { + submit(message, bindings, requestOptions=undefined) { if (typeof message === 'string') { const args = { 'gremlin': message, @@ -83,12 +84,11 @@ class Client { if (this._options.session && this._options.processor === 'session') { args['session'] = this._options.session; } - - return this._connection.submit(null, 'eval', args, null, this._options.processor || ''); + return this._connection.submit(null, 'eval', Object.assign(args, requestOptions), null, this._options.processor || ''); } if (message instanceof Bytecode) { - return this._connection.submit(message); + return this._connection.submit(message, 'bytecode', requestOptions); } } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js index 67dce3af489..28cb1d12ca9 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js @@ -156,16 +156,19 @@ class Connection extends EventEmitter { /** @override */ submit(bytecode, op, args, requestId, processor) { + const requestIdDefined = (args !== undefined && args !== null && 'requestId' in args) + const rid = requestId === undefined || requestId === null ? (requestIdDefined ? args['requestId'] : utils.getUuid()) : requestId; + if (requestIdDefined) delete args['requestId']; + return this.open().then(() => new Promise((resolve, reject) => { - if (requestId === null || requestId === undefined) { - requestId = utils.getUuid(); - this._responseHandlers[requestId] = { + if (op !== 'authentication') { + this._responseHandlers[rid] = { callback: (err, result) => err ? reject(err) : resolve(result), result: null }; } - const message = Buffer.from(this._header + JSON.stringify(this._getRequest(requestId, bytecode, op, args, processor))); + const message = Buffer.from(this._header + JSON.stringify(this._getRequest(rid, bytecode, op, args, processor))); this._ws.send(message); })); } @@ -185,6 +188,13 @@ class Connection extends EventEmitter { _getRequest(id, bytecode, op, args, processor) { if (args) { args = this._adaptArgs(args, true); + } else { + args = {}; + } + + if (bytecode) { + args['gremlin'] = this._writer.adaptObject(bytecode); + args['aliases'] = { 'g': this.traversalSource } } return ({ @@ -192,10 +202,7 @@ class Connection extends EventEmitter { 'op': op || 'bytecode', // if using op eval need to ensure processor stays unset if caller didn't set it. 'processor': (!processor && op !== 'eval') ? 'traversal' : processor, - 'args': args || { - 'gremlin': this._writer.adaptObject(bytecode), - 'aliases': { 'g': this.traversalSource } - } + 'args': args }); } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js index 1632fda8557..62cbdb58cb5 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/driver-remote-connection.js @@ -26,6 +26,7 @@ const rcModule = require('./remote-connection'); const RemoteConnection = rcModule.RemoteConnection; const RemoteTraversal = rcModule.RemoteTraversal; const Client = require('./client'); +const OptionsStrategy = require('../process/traversal-strategy').OptionsStrategy; /** * Represents the default {@link RemoteConnection} implementation. @@ -60,7 +61,21 @@ class DriverRemoteConnection extends RemoteConnection { /** @override */ submit(bytecode) { - return this._client.submit(bytecode).then(result => new RemoteTraversal(result.toArray())); + let optionsStrategy = bytecode.sourceInstructions.find( + i => i[0] === "withStrategies" && i[1] instanceof OptionsStrategy); + const allowedKeys = ['evaluationTimeout', 'scriptEvaluationTimeout', 'batchSize', 'requestId', 'userAgent']; + + let requestOptions = undefined; + if (optionsStrategy !== undefined) { + requestOptions = {}; + const conf = optionsStrategy[1].configuration; + for (let key in conf) { + if (conf.hasOwnProperty(key) && allowedKeys.indexOf(key) > -1) { + requestOptions[key] = conf[key]; + } + } + } + return this._client.submit(bytecode, null, requestOptions).then(result => new RemoteTraversal(result.toArray())); } /** @override */ diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js index b0e91f2eb2d..1e1960949f1 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/graph-traversal.js @@ -26,7 +26,7 @@ const { Traversal } = require('./traversal'); const remote = require('../driver/remote-connection'); const utils = require('../utils'); const Bytecode = require('./bytecode'); -const { TraversalStrategies, VertexProgramStrategy } = require('./traversal-strategy'); +const { TraversalStrategies, VertexProgramStrategy, OptionsStrategy } = require('./traversal-strategy'); /** @@ -76,6 +76,26 @@ class GraphTraversalSource { configuration: configuration})); } + /** + * Graph Traversal Source with method. + * @param {String} key + * @param {Object} value if not specified, the value with default to {@code true} + * @returns {GraphTraversalSource} + */ + with_(key, value=undefined) { + const val = value === undefined ? true : value; + let optionsStrategy = this.bytecode.sourceInstructions.find( + i => i[0] === "withStrategies" && i[1] instanceof OptionsStrategy); + if (optionsStrategy === undefined) { + optionsStrategy = new OptionsStrategy({[key]: val}); + return this.withStrategies(optionsStrategy); + } else { + optionsStrategy[1].configuration[key] = val; + return new this.graphTraversalSourceClass(this.graph, new TraversalStrategies(this.traversalStrategies), + this.bytecode, this.graphTraversalSourceClass, this.graphTraversalClass); + } + } + /** * Returns the string representation of the GraphTraversalSource. * @returns {string} @@ -84,16 +104,6 @@ class GraphTraversalSource { return 'graphtraversalsource[' + this.graph.toString() + ']'; } - /** - * Graph Traversal Source with method. - * @param {...Object} args - * @returns {GraphTraversalSource} - */ - with_(...args) { - const b = new Bytecode(this.bytecode).addSource('with', args); - return new this.graphTraversalSourceClass(this.graph, new TraversalStrategies(this.traversalStrategies), b, this.graphTraversalSourceClass, this.graphTraversalClass); - } - /** * Graph Traversal Source withBulk method. * @param {...Object} args diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal-strategy.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal-strategy.js index 301eb077a2e..37816bd90e0 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal-strategy.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/process/traversal-strategy.js @@ -22,6 +22,8 @@ */ 'use strict'; +const Traversal = require('./traversal').Traversal; + class TraversalStrategies { /** * Creates a new instance of TraversalStrategies. @@ -60,9 +62,9 @@ class TraversalStrategy { /** * @param {String} fqcn fully qualified class name in Java of the strategy - * @param {Map} configuration for the strategy + * @param {Object} configuration for the strategy */ - constructor(fqcn, configuration = new Map()) { + constructor(fqcn, configuration = {}) { this.fqcn = fqcn; this.configuration = configuration; } @@ -97,12 +99,12 @@ class HaltedTraverserStrategy extends TraversalStrategy { constructor(haltedTraverserFactory) { super("org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.HaltedTraverserStrategy"); if (haltedTraverserFactory !== undefined) - this.configuration.set("haltedTraverserFactory", haltedTraverserFactory); + this.configuration["haltedTraverserFactory"] = haltedTraverserFactory; } } class OptionsStrategy extends TraversalStrategy { - constructor(options = new Map()) { + constructor(options) { super("org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.OptionsStrategy", options); } } @@ -116,15 +118,7 @@ class PartitionStrategy extends TraversalStrategy { * @param {boolean} [options.includeMetaProperties] determines if meta-properties should be included in partitioning defaulting to false */ constructor(options) { - super("org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.PartitionStrategy"); - if (options.partitionKey !== undefined) - this.configuration.set("partitionKey", options.partitionKey); - if (options.writePartition !== undefined) - this.configuration.set("writePartition", options.writePartition); - if (options.readPartitions !== undefined) - this.configuration.set("readPartitions", options.readPartitions); - if (options.includeMetaProperties !== undefined) - this.configuration.set("includeMetaProperties", options.includeMetaProperties); + super("org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.PartitionStrategy", options); } } @@ -137,39 +131,20 @@ class SubgraphStrategy extends TraversalStrategy { * @param {boolean} [options.checkAdjacentVertices] enables the strategy to apply the {@code vertices} filter to the adjacent vertices of an edge. */ constructor(options) { - super("org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy"); - if (options.vertices !== undefined) - this.configuration.set("vertices", options.vertices); - if (options.edges !== undefined) - this.configuration.set("edges", options.edges); - if (options.vertexProperties !== undefined) - this.configuration.set("vertexProperties", options.vertexProperties); - if (options.checkAdjacentVertices !== undefined) - this.configuration.set("checkAdjacentVertices", options.checkAdjacentVertices); + super("org.apache.tinkerpop.gremlin.process.traversal.strategy.decoration.SubgraphStrategy", options); + if (this.configuration.vertices instanceof Traversal) + this.configuration.vertices = this.configuration.vertices.bytecode; + if (this.configuration.edges instanceof Traversal) + this.configuration.edges = this.configuration.edges.bytecode; + if (this.configuration.vertexProperties instanceof Traversal) + this.configuration.vertexProperties = this.configuration.vertexProperties.bytecode; } } class VertexProgramStrategy extends TraversalStrategy { constructor(options) { - super("org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.decoration.VertexProgramStrategy"); - this.configuration = new Map(); - if (options.graphComputer !== undefined) - this.configuration.set("graphComputer", options.graphComputer); - if (options.workers !== undefined) - this.configuration.set("workers", options.workers); - if (options.persist !== undefined) - this.configuration.set("persist", options.persist); - if (options.result !== undefined) - this.configuration.set("result", options.result); - if (options.vertices !== undefined) - this.configuration.set("vertices", options.vertices); - if (options.edges !== undefined) - this.configuration.set("edges", options.edges); - if (options.configuration !== undefined) - options.configuration.forEach(function(k,v) { - this.configuration.set(k, v); - }); + super("org.apache.tinkerpop.gremlin.process.computer.traversal.strategy.decoration.VertexProgramStrategy", options); } } @@ -179,8 +154,8 @@ class MatchAlgorithmStrategy extends TraversalStrategy { */ constructor(matchAlgorithm) { super("org.apache.tinkerpop.gremlin.process.traversal.strategy.finalization.MatchAlgorithmStrategy"); - if (graphComputer !== undefined) - this.configuration.set("matchAlgorithm", matchAlgorithm); + if (matchAlgorithm !== undefined) + this.configuration["matchAlgorithm"] = matchAlgorithm; } } @@ -286,9 +261,8 @@ class EdgeLabelVerificationStrategy extends TraversalStrategy { * @param {boolean} throwException determines if exceptions should be thrown when verifications fails */ constructor(logWarnings = false, throwException=false) { - super("org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.EdgeLabelVerificationStrategy"); - this.configuration.set("logWarnings", logWarnings); - this.configuration.set("throwException", throwException); + super("org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.EdgeLabelVerificationStrategy", + {logWarnings: logWarnings, throwException: throwException}); } } @@ -299,10 +273,8 @@ class ReservedKeysVerificationStrategy extends TraversalStrategy { * @param {Array} keys the list of reserved keys to verify */ constructor(logWarnings = false, throwException=false, keys=["id", "label"]) { - super("org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.EdgeLabelVerificationStrategy"); - this.configuration.set("logWarnings", logWarnings); - this.configuration.set("throwException", throwException); - this.configuration.set("keys", keys); + super("org.apache.tinkerpop.gremlin.process.traversal.strategy.verification.EdgeLabelVerificationStrategy", + {logWarnings: logWarnings, throwException: throwException, keys: keys}); } } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/type-serializers.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/type-serializers.js index b172749ccd8..47e940746ee 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/type-serializers.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/structure/io/type-serializers.js @@ -256,9 +256,16 @@ class TraverserSerializer extends TypeSerializer { class TraversalStrategySerializer extends TypeSerializer { /** @param {TraversalStrategy} item */ serialize(item) { + const conf = {}; + for (let k in item.configuration) { + if (item.configuration.hasOwnProperty(k)) { + conf[k] = this.writer.adaptObject(item.configuration[k]); + } + } + return { [typeKey]: 'g:' + item.constructor.name, - [valueKey]: item.configuration + [valueKey]: conf }; } diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js index cfa99dd2809..39c675fa881 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/integration/traversal-test.js @@ -153,19 +153,19 @@ describe('Traversal', function () { g.V().count().next().then(function (item1) { assert.ok(item1); assert.strictEqual(item1.value, 4); - }); + }, (err) => assert.fail("tanked: " + err)); g.E().count().next().then(function (item1) { assert.ok(item1); assert.strictEqual(item1.value, 0); - }); + }, (err) => assert.fail("tanked: " + err)); g.V().label().dedup().count().next().then(function (item1) { assert.ok(item1); assert.strictEqual(item1.value, 1); - }); + }, (err) => assert.fail("tanked: " + err)); g.V().label().dedup().next().then(function (item1) { assert.ok(item1); assert.strictEqual(item1.value, "person"); - }); + }, (err) => assert.fail("tanked: " + err)); }); it('should allow ReadOnlyStrategy', function() { const g = traversal().withRemote(connection).withStrategies(new ReadOnlyStrategy()); @@ -181,7 +181,11 @@ describe('Traversal', function () { assert.ok(item1); assert.strictEqual(item1.value, 6); }); - return g.V().out().iterate().then(() => assert.fail("should have tanked"), (err) => assert.ok(err)); + return g.V().out().iterate().then(() => assert.fail("should have tanked"), (err) => assert.strictEqual(err.statusCode, 500)); + }); + it('should allow with_(evaluationTimeout,10)', function() { + const g = traversal().withRemote(connection).with_('x').with_('evaluationTimeout', 10); + return g.V().repeat(__.both()).iterate().then(() => assert.fail("should have tanked"), (err) => assert.strictEqual(err.statusCode, 598)); }); }); }); \ No newline at end of file diff --git a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js index acdcdd4502b..2ab18eec5d9 100644 --- a/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js +++ b/gremlin-javascript/src/main/javascript/gremlin-javascript/test/unit/traversal-test.js @@ -57,43 +57,20 @@ describe('Traversal', function () { assert.strictEqual(bytecode.stepInstructions[2][2].typeName, 'Order'); assert.strictEqual(bytecode.stepInstructions[2][2].elementName, 'desc'); }); - }); - // describe('#hasNext()', function() { - // it('should apply strategies and determine if there is anything left to iterate in the traversal') - // const strategyMock = { - // apply: function (traversal) { - // traversal.traversers = [ new t.Traverser(1, 1), new t.Traverser(2, 1) ]; - // return Promise.resolve(); - // } - // }; - // const strategies = new TraversalStrategies(); - // strategies.addStrategy(strategyMock); - // const traversal = new t.Traversal(null, strategies, null); - // return traversal.hasNext() - // .then(function (more) { - // assert.strictEqual(more, true); - // return traversal.next(); - // }) - // .then(function (item) { - // assert.strictEqual(item.value, 1); - // assert.strictEqual(item.done, false); - // return traversal.next(); - // }) - // .then(function (item) { - // assert.strictEqual(item.value, 2); - // assert.strictEqual(item.done, false); - // return traversal.next(); - // }) - // .then(function (item) { - // assert.strictEqual(item.value, null); - // assert.strictEqual(item.done, true); - // return traversal.hasNext(); - // }) - // .then(function (more) { - // assert.strictEqual(more, false); - // }); - // }); + it('should configure OptionStrategy for with_()', function () { + const g = new graph.Graph().traversal(); + const bytecode = g.with_('x','test').with_('y').V().getBytecode(); + assert.ok(bytecode); + assert.strictEqual(bytecode.sourceInstructions.length, 1); + assert.strictEqual(bytecode.sourceInstructions[0][0], 'withStrategies'); + const conf = bytecode.sourceInstructions[0][1].configuration; + assert.strictEqual(conf.x, 'test'); + assert.strictEqual(conf.y, true); + assert.strictEqual(bytecode.stepInstructions.length, 1); + assert.strictEqual(bytecode.stepInstructions[0][0], 'V'); + }); + }); describe('#next()', function () { it('should apply the strategies and return a Promise with the iterator item', function () {