[TINKERPOP-2841] Test and Fix Per-Request Settings in Go GLV

CHANGELOG.asciidoc

@@ -36,6 +36,8 @@ This release also includes changes from <<release-3-6-XXX, 3.6.XXX>>.
 * Moves `SimpleSocketServer` and its initializers to a new `gremlin-tools/gremlin-socket-server` module.
 * Configures `gremlin-socket-server` to build a docker image which can be used for testing GLV's. (Can be skipped with -DskipImageBuild)
 * Reduces dependency from `gremlin-server` onto `gremlin-driver` to a test scope only.
+* Added `RequestOptions` and `RequestOptionsBuilder` types to Go GLV to encapsulate per-request settings and bindings.
+* Added `SubmitWithOptions()` methods to `Client` and `DriverRemoteConnection` in Go GLV to pass `RequestOptions` to the server.
 
 == TinkerPop 3.6.0 (Tinkerheart)
 

docs/src/reference/gremlin-applications.asciidoc

@@ -612,7 +612,8 @@ list = g.V().has("person","name","marko").out("knows").toList()
 ----
 // script
 client, err := NewClient("ws://localhost:8182/gremlin")
-resultSet, err := client.Submit("g.V().has('person','name',name).out('knows')", map[string]interface{}{"name": "marko"})
+resultSet, err := client.SubmitWithOptions("g.V().has('person','name',name).out('knows')",
+	new(RequestOptionsBuilder).AddBinding("name", "marko").Create())
 result, err := resultSet.All()
 
 // bytecode

docs/src/reference/gremlin-variants.asciidoc

@@ -293,32 +293,39 @@ fmt.Println(result[0].GetString()) <3>
 ----
 
 <1> Submit a script that simply returns a Count of vertexes.
-<2> Get results from resultSet. Block until the the script is evaluated and results are sent back by the server.
+<2> Get results from resultSet. Block until the script is evaluated and results are sent back by the server.
 <3> Use the result.
 
 ==== Per Request Settings
 
-The `client.Submit()` functions accept a `bindings` which expects a map. The `bindings` provide a way to include options
-that are specific to the request made with the call to `Submit()`. A good use-case for this feature is to set a per-request
-override to the `evaluationTimeout` so that it only applies to the current request.
+Both the `Client` and `DriverRemoteConnection` types have a `SubmitWithOptions(traversalString, requestOptions)` variant
+of the standard `Submit()` method. These methods allow a `RequestOptions` struct to be passed in which will augment the
+execution on the server. `RequestOptions` can be constructed
+using `RequestOptionsBuilder`. A good use-case for this feature is to set a per-request override to the
+`evaluationTimeout` so that it only applies to the current request.
 
 [source,go]
 ----
-resultSet, err := client.Submit("g.V().repeat(both()).times(100)", map[string]interface{}{"evaluationTimeout": 5000})
+options := new(RequestOptionsBuilder).
+			SetEvaluationTimeout(5000).
+			AddBinding("x", 100).
+			Create()
+resultSet, err := client.SubmitWithOptions("g.V(x).count()", options)
 ----
 
 The following options are allowed on a per-request basis in this fashion: `batchSize`, `requestId`, `userAgent` and
-`evaluationTimeout`.
+`evaluationTimeout`. `RequestOptions` may also contain a map of variable `bindings` to be applied to the supplied
+traversal string.
 
 IMPORTANT: The preferred method for setting a per-request timeout for scripts is demonstrated above, but those familiar
 with bytecode may try `g.with("evaluationTimeout", 500)` within a script. Scripts with multiple traversals and multiple
 timeouts will be interpreted as a sum of all timeouts identified in the script for that request.
 
 [source,go]
 ----
-resultSet, err := client.Submit("g.with('evaluationTimeout', 500).addV().iterate();"+
+resultSet, err := client.SubmitWithOptions("g.with('evaluationTimeout', 500).addV().iterate();"+
   "g.addV().iterate();"+
-  "g.with('evaluationTimeout', 500).addV();", map[string]interface{}{"evaluationTimeout": 500})
+  "g.with('evaluationTimeout', 500).addV();", new(RequestOptionsBuilder).SetEvaluationTimeout(500).Create())
 results, err := resultSet.All()
 ----
 

gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Util/SocketServerSettings.cs

@@ -87,7 +87,7 @@ public class SocketServerSettings
 
     public static SocketServerSettings FromYaml(String path)
     {
-        var deserializer = new YamlDotNet.Serialization.DeserializerBuilder().Build();
+        var deserializer = new YamlDotNet.Serialization.DeserializerBuilder().IgnoreUnmatchedProperties().Build();
 
         return deserializer.Deserialize<SocketServerSettings>(File.ReadAllText(path));
     }

gremlin-go/driver/client.go

@@ -147,17 +147,28 @@ func (client *Client) Close() {
 	client.connections.close()
 }
 
-// Submit submits a Gremlin script to the server and returns a ResultSet.
-func (client *Client) Submit(traversalString string, bindings ...map[string]interface{}) (ResultSet, error) {
+// SubmitWithOptions submits a Gremlin script to the server with specified RequestOptions and returns a ResultSet.
+func (client *Client) SubmitWithOptions(traversalString string, requestOptions RequestOptions) (ResultSet, error) {
 	client.logHandler.logf(Debug, submitStartedString, traversalString)
-	request := makeStringRequest(traversalString, client.traversalSource, client.session, bindings...)
+	request := makeStringRequest(traversalString, client.traversalSource, client.session, requestOptions)
 	result, err := client.connections.write(&request)
 	if err != nil {
 		client.logHandler.logf(Error, logErrorGeneric, "Client.Submit()", err.Error())
 	}
 	return result, err
 }
 
+// Submit submits a Gremlin script to the server and returns a ResultSet. Submit can optionally accept a map of bindings
+// to be applied to the traversalString, it is preferred however to instead wrap any bindings into a RequestOptions
+// struct and use SubmitWithOptions().
+func (client *Client) Submit(traversalString string, bindings ...map[string]interface{}) (ResultSet, error) {
+	requestOptionsBuilder := new(RequestOptionsBuilder)
+	if len(bindings) > 0 {
+		requestOptionsBuilder.SetBindings(bindings[0])
+	}
+	return client.SubmitWithOptions(traversalString, requestOptionsBuilder.Create())
+}
+
 // submitBytecode submits Bytecode to the server to execute and returns a ResultSet.
 func (client *Client) submitBytecode(bytecode *Bytecode) (ResultSet, error) {
 	client.logHandler.logf(Debug, submitStartedBytecode, *bytecode)

gremlin-go/driver/client_test.go

@@ -37,13 +37,14 @@ func TestClient(t *testing.T) {
 	testNoAuthAuthInfo := &AuthInfo{}
 	testNoAuthTlsConfig := &tls.Config{}
 
-	t.Run("Test client.submit()", func(t *testing.T) {
+	t.Run("Test client.Submit()", func(t *testing.T) {
 		skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable)
 		client, err := NewClient(testNoAuthUrl,
 			func(settings *ClientSettings) {
 				settings.TlsConfig = testNoAuthTlsConfig
 				settings.AuthInfo = testNoAuthAuthInfo
 			})
+		defer client.Close()
 		assert.Nil(t, err)
 		assert.NotNil(t, client)
 		resultSet, err := client.Submit("g.V().count()")
@@ -53,7 +54,25 @@ func TestClient(t *testing.T) {
 		assert.Nil(t, err)
 		assert.True(t, ok)
 		assert.NotNil(t, result)
-		client.Close()
+	})
+
+	t.Run("Test client.SubmitWithOptions()", func(t *testing.T) {
+		skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable)
+		client, err := NewClient(testNoAuthUrl,
+			func(settings *ClientSettings) {
+				settings.TlsConfig = testNoAuthTlsConfig
+				settings.AuthInfo = testNoAuthAuthInfo
+			})
+		defer client.Close()
+		assert.Nil(t, err)
+		assert.NotNil(t, client)
+		resultSet, err := client.SubmitWithOptions("g.V().count()", *new(RequestOptions))
+		assert.Nil(t, err)
+		assert.NotNil(t, resultSet)
+		result, ok, err := resultSet.One()
+		assert.Nil(t, err)
+		assert.True(t, ok)
+		assert.NotNil(t, result)
 	})
 }
 
@@ -91,6 +110,12 @@ type SocketServerSettings struct {
 	 * during the web socket handshake.
 	 */
 	USER_AGENT_REQUEST_ID uuid.UUID `yaml:"USER_AGENT_REQUEST_ID"`
+	/**
+	 * If a request with this ID comes to the server, the server responds with a string containing all overridden
+	 * per request settings from the request message. String will be of the form
+	 * "requestId=19436d9e-f8fc-4b67-8a76-deec60918424 evaluationTimeout=1234, batchSize=12, userAgent=testUserAgent"
+	 */
+	PER_REQUEST_SETTINGS_REQUEST_ID uuid.UUID `yaml:"PER_REQUEST_SETTINGS_REQUEST_ID"`
 }
 
 func FromYaml(path string) *SocketServerSettings {
@@ -121,16 +146,17 @@ func TestClientAgainstSocketServer(t *testing.T) {
 	t.Run("Should get single vertex response from gremlin socket server", func(t *testing.T) {
 		skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable)
 		client, err := NewClient(testSocketServerUrl)
+		defer client.Close()
 		assert.Nil(t, err)
 		assert.NotNil(t, client)
-		resultSet, err := client.Submit("1", map[string]interface{}{"requestId": settings.SINGLE_VERTEX_REQUEST_ID})
+		resultSet, err := client.SubmitWithOptions("1", new(RequestOptionsBuilder).
+			SetRequestId(settings.SINGLE_VERTEX_REQUEST_ID).Create())
 		assert.Nil(t, err)
 		assert.NotNil(t, resultSet)
 		result, ok, err := resultSet.One()
 		assert.Nil(t, err)
 		assert.True(t, ok)
 		assert.NotNil(t, result)
-		client.Close()
 	})
 
 	/**
@@ -140,10 +166,12 @@ func TestClientAgainstSocketServer(t *testing.T) {
 	t.Run("Should include user agent in handshake request", func(t *testing.T) {
 		skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable)
 		client, err := NewClient(testSocketServerUrl)
+		defer client.Close()
 		assert.Nil(t, err)
 		assert.NotNil(t, client)
 
-		resultSet, err := client.Submit("1", map[string]interface{}{"requestId": settings.USER_AGENT_REQUEST_ID})
+		resultSet, err := client.SubmitWithOptions("1", new(RequestOptionsBuilder).
+			SetRequestId(settings.USER_AGENT_REQUEST_ID).Create())
 		assert.Nil(t, err)
 		assert.NotNil(t, resultSet)
 
@@ -154,8 +182,6 @@ func TestClientAgainstSocketServer(t *testing.T) {
 
 		userAgentResponse := result.GetString()
 		assert.Equal(t, userAgent, userAgentResponse)
-
-		client.Close()
 	})
 
 	/**
@@ -168,10 +194,12 @@ func TestClientAgainstSocketServer(t *testing.T) {
 			func(settings *ClientSettings) {
 				settings.EnableUserAgentOnConnect = false
 			})
+		defer client.Close()
 		assert.Nil(t, err)
 		assert.NotNil(t, client)
 
-		resultSet, err := client.Submit("1", map[string]interface{}{"requestId": settings.USER_AGENT_REQUEST_ID})
+		resultSet, err := client.SubmitWithOptions("1", new(RequestOptionsBuilder).
+			SetRequestId(settings.USER_AGENT_REQUEST_ID).Create())
 		assert.Nil(t, err)
 		assert.NotNil(t, resultSet)
 
@@ -184,8 +212,33 @@ func TestClientAgainstSocketServer(t *testing.T) {
 		//If the gremlin user agent is disabled, the underlying web socket library reverts to sending its default user agent
 		//during connection requests.
 		assert.Contains(t, userAgentResponse, "Go-http-client/")
+	})
+
+	/**
+	 * Tests that client is correctly sending all overridable per request settings (requestId, batchSize,
+	 * evaluationTimeout, and userAgent) to the server.
+	 */
+	t.Run("Should Send Per Request Settings To Server", func(t *testing.T) {
+		skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable)
+		client, err := NewClient(testSocketServerUrl)
+		defer client.Close()
+		assert.Nil(t, err)
+		assert.NotNil(t, client)
 
-		client.Close()
+		resultSet, err := client.SubmitWithOptions("1", new(RequestOptionsBuilder).
+			SetRequestId(settings.PER_REQUEST_SETTINGS_REQUEST_ID).
+			SetEvaluationTimeout(1234).
+			SetBatchSize(12).
+			SetUserAgent("helloWorld").
+			Create())
+		assert.Nil(t, err)
+		assert.NotNil(t, resultSet)
+		result, ok, err := resultSet.One()
+		assert.Nil(t, err)
+		assert.True(t, ok)
+		expectedResult := fmt.Sprintf("requestId=%v evaluationTimeout=%v, batchSize=%v, userAgent=%v",
+			settings.PER_REQUEST_SETTINGS_REQUEST_ID, 1234, 12, "helloWorld")
+		assert.Equal(t, expectedResult, result.Data)
 	})
 
 	/**
@@ -197,24 +250,26 @@ func TestClientAgainstSocketServer(t *testing.T) {
 		t.Run("Should try create new connection if closed by server", func(t *testing.T) {
 			skipTestsIfNotEnabled(t, integrationTestSuiteName, testNoAuthEnable)
 			client, err := NewClient(testSocketServerUrl)
+			defer client.Close()
 			assert.Nil(t, err)
 			assert.NotNil(t, client)
-			resultSet, err := client.Submit("1", map[string]interface{}{"requestId": settings.CLOSE_CONNECTION_REQUEST_ID})
+			resultSet, err := client.SubmitWithOptions("1", new(RequestOptionsBuilder).
+				SetRequestId(settings.CLOSE_CONNECTION_REQUEST_ID).Create())
 			assert.Nil(t, err)
 			assert.NotNil(t, resultSet)
 
 			result, ok, err := resultSet.One()
 
 			assert.EqualError(t, err, "websocket: close 1005 (no status)")
 
-			resultSet, err = client.Submit("1", map[string]interface{}{"requestId": settings.SINGLE_VERTEX_REQUEST_ID})
+			resultSet, err = client.SubmitWithOptions("1", new(RequestOptionsBuilder).
+				SetRequestId(settings.SINGLE_VERTEX_REQUEST_ID).Create())
 			assert.Nil(t, err)
 			assert.NotNil(t, resultSet)
 			result, ok, err = resultSet.One()
 			assert.Nil(t, err)
 			assert.True(t, ok)
 			assert.NotNil(t, result)
-			client.Close()
 		})
 	*/
 }

gremlin-go/driver/connection_test.go

@@ -381,7 +381,7 @@ func TestConnection(t *testing.T) {
 		assert.NotNil(t, connection)
 		assert.Equal(t, established, connection.state)
 		defer deferredCleanup(t, connection)
-		request := makeStringRequest("g.V().count()", "g", "")
+		request := makeStringRequest("g.V().count()", "g", "", *new(RequestOptions))
 		resultSet, err := connection.write(&request)
 		assert.Nil(t, err)
 		assert.NotNil(t, resultSet)
@@ -399,7 +399,7 @@ func TestConnection(t *testing.T) {
 		assert.NotNil(t, connection)
 		assert.Equal(t, established, connection.state)
 		defer deferredCleanup(t, connection)
-		request := makeStringRequest("g.V().count()", "g", "")
+		request := makeStringRequest("g.V().count()", "g", "", *new(RequestOptions))
 		resultSet, err := connection.write(&request)
 		assert.Nil(t, err)
 		assert.NotNil(t, resultSet)
@@ -435,7 +435,7 @@ func TestConnection(t *testing.T) {
 		err = connection.close()
 		assert.Nil(t, err)
 		assert.Equal(t, closed, connection.state)
-		request := makeStringRequest("g.V().count()", "g", "")
+		request := makeStringRequest("g.V().count()", "g", "", *new(RequestOptions))
 		resultSet, err := connection.write(&request)
 		assert.Nil(t, resultSet)
 		assert.Equal(t, newError(err0102WriteConnectionClosedError), err)
@@ -451,7 +451,7 @@ func TestConnection(t *testing.T) {
 		assert.Equal(t, established, connection.state)
 		assert.Nil(t, err)
 		time.Sleep(120 * time.Second)
-		request := makeStringRequest("g.V().count()", "g", "")
+		request := makeStringRequest("g.V().count()", "g", "", *new(RequestOptions))
 		resultSet, err := connection.write(&request)
 		assert.Nil(t, resultSet)
 		assert.NotNil(t, err)

gremlin-go/driver/driverRemoteConnection.go

@@ -163,15 +163,20 @@ func (driver *DriverRemoteConnection) Close() {
 	driver.isClosed = true
 }
 
-// Submit sends a string traversal to the server.
-func (driver *DriverRemoteConnection) Submit(traversalString string) (ResultSet, error) {
-	result, err := driver.client.Submit(traversalString)
+// SubmitWithOptions sends a string traversal to the server along with specified RequestOptions.
+func (driver *DriverRemoteConnection) SubmitWithOptions(traversalString string, requestOptions RequestOptions) (ResultSet, error) {
+	result, err := driver.client.SubmitWithOptions(traversalString, requestOptions)
 	if err != nil {
 		driver.client.logHandler.logf(Error, logErrorGeneric, "Driver.Submit()", err.Error())
 	}
 	return result, err
 }
 
+// Submit sends a string traversal to the server.
+func (driver *DriverRemoteConnection) Submit(traversalString string) (ResultSet, error) {
+	return driver.SubmitWithOptions(traversalString, *new(RequestOptions))
+}
+
 // submitBytecode sends a Bytecode traversal to the server.
 func (driver *DriverRemoteConnection) submitBytecode(bytecode *Bytecode) (ResultSet, error) {
 	if driver.isClosed {

gremlin-go/driver/request.go

@@ -20,7 +20,6 @@ under the License.
 package gremlingo
 
 import (
-	"fmt"
 	"github.com/google/uuid"
 )
 
@@ -37,8 +36,7 @@ const sessionProcessor = "session"
 const stringOp = "eval"
 const stringProcessor = ""
 
-// Bindings should be a key-object map (different from Binding class in Bytecode).
-func makeStringRequest(stringGremlin string, traversalSource string, sessionId string, bindings ...map[string]interface{}) (req request) {
+func makeStringRequest(stringGremlin string, traversalSource string, sessionId string, requestOptions RequestOptions) (req request) {
 	newProcessor := stringProcessor
 	newArgs := map[string]interface{}{
 		"gremlin": stringGremlin,
@@ -50,13 +48,27 @@ func makeStringRequest(stringGremlin string, traversalSource string, sessionId s
 		newProcessor = sessionProcessor
 		newArgs["session"] = sessionId
 	}
-	requestId := uuid.New()
-	if len(bindings) > 0 {
-		newArgs["bindings"] = bindings[0]
-		customRequestId, err := uuid.Parse(fmt.Sprintf("%v", bindings[0]["requestId"]))
-		if err == nil {
-			requestId = customRequestId
-		}
+	var requestId uuid.UUID
+	if requestOptions.requestID == uuid.Nil {
+		requestId = uuid.New()
+	} else {
+		requestId = requestOptions.requestID
+	}
+
+	if requestOptions.bindings != nil {
+		newArgs["bindings"] = requestOptions.bindings
+	}
+
+	if requestOptions.evaluationTimeout != 0 {
+		newArgs["evaluationTimeout"] = requestOptions.evaluationTimeout
+	}
+
+	if requestOptions.batchSize != 0 {
+		newArgs["batchSize"] = requestOptions.batchSize
+	}
+
+	if requestOptions.userAgent != "" {
+		newArgs["userAgent"] = requestOptions.userAgent
 	}
 
 	return request{