From 2a2e47ebdb5342c30ebee2e9a0cca138e07fc58c Mon Sep 17 00:00:00 2001 From: Jorge Bay Gondra Date: Wed, 12 Sep 2018 11:17:54 +0200 Subject: [PATCH 1/2] TINKERPOP-1972 Fix test harness, extract child type in generic params array --- .../Gherkin/GherkinTestRunner.cs | 6 +++--- .../TraversalEvaluation/TraversalParser.cs | 21 +++++++++++++++++-- 2 files changed, 22 insertions(+), 5 deletions(-) diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs index 3802da5edde..27bbc42f712 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/GherkinTestRunner.cs @@ -38,10 +38,10 @@ namespace Gremlin.Net.IntegrationTest.Gherkin public class GherkinTestRunner { private static readonly IDictionary IgnoredScenarios = - new Dictionary() + new Dictionary { - { "g_injectX1X_chooseXisX1X__constantX10Xfold__foldX", IgnoreReason.NoReason }, - { "g_injectX2X_chooseXisX1X__constantX10Xfold__foldX", IgnoreReason.NoReason } + // Add here the name of scenarios to ignore and the reason, e.g. + // { "g_injectX2X_chooseXisX1X__constantX10Xfold__foldX", IgnoreReason.NoReason } }; private static class Keywords diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs index e3f6a3f8e3d..419ce8db3a0 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Gherkin/TraversalEvaluation/TraversalParser.cs @@ -245,6 +245,14 @@ private static object[] BuildParameters(MethodInfo method, Token token, // if we have the type of value we have the type of E2. genericParameterTypes.Add(info.ParameterType.Name, tokenParameter.GetParameterType()); } + else if (IsParamsArray(info) && info.ParameterType.GetElementType().IsGenericParameter) + { + // Its a method where the type parameter comes from an params Array + // e.g., Inject(params S[] value) + genericParameterTypes.Add(info.ParameterType.GetElementType().Name, + tokenParameter.GetParameterType()); + } + if (info.ParameterType != tokenParameter.GetParameterType() && IsNumeric(info.ParameterType) && IsNumeric(tokenParameter.GetParameterType())) { @@ -252,6 +260,7 @@ private static object[] BuildParameters(MethodInfo method, Token token, value = Convert.ChangeType(value, info.ParameterType); } } + if (IsParamsArray(info)) { // For `params type[] value` we should provide an empty array @@ -264,11 +273,19 @@ private static object[] BuildParameters(MethodInfo method, Token token, { // An array with the parameter values // No more method parameters after this one - var arr = Array.CreateInstance(info.ParameterType.GetElementType(), token.Parameters.Count - i); + var elementType = info.ParameterType.GetElementType(); + + if (elementType.IsGenericParameter) + { + // The Array element type is generic, so we use type of the value to specify it + elementType = value.GetType(); + } + + var arr = Array.CreateInstance(elementType, token.Parameters.Count - i); arr.SetValue(value, 0); for (var j = 1; j < token.Parameters.Count - i; j++) { - arr.SetValue(token.Parameters[i + j].GetValue(), j); + arr.SetValue(token.Parameters[i + j].GetValue(), j); } value = arr; } From dee155e50eab747afa1d302ed0c027c8c3629e2b Mon Sep 17 00:00:00 2001 From: Florian Hockmann Date: Fri, 5 Oct 2018 17:32:09 +0200 Subject: [PATCH 2/2] TINKERPOP-1972 Fix arg handling for inject step To be precise, this affects all steps that have a params array with a generic type parameter as the member type but currently inject is the only step in Gremlin.Net that has such an array as an argument. The arguments passed as such a params array were incorrectly wrapped in one array before instead of individual arguments. --- gremlin-dotnet/glv/GraphTraversal.template | 5 +++-- gremlin-dotnet/glv/GraphTraversalSource.template | 5 +++-- gremlin-dotnet/glv/generate.groovy | 16 +++++++--------- .../Process/Traversal/GraphTraversal.cs | 5 +++-- .../Process/Traversal/GraphTraversalSource.cs | 5 +++-- .../BytecodeGenerationTests.cs | 12 ++++++++++++ 6 files changed, 31 insertions(+), 17 deletions(-) diff --git a/gremlin-dotnet/glv/GraphTraversal.template b/gremlin-dotnet/glv/GraphTraversal.template index 67384674652..9e4b2844de2 100644 --- a/gremlin-dotnet/glv/GraphTraversal.template +++ b/gremlin-dotnet/glv/GraphTraversal.template @@ -22,6 +22,7 @@ #endregion using System.Collections.Generic; +using System.Linq; using Gremlin.Net.Structure; // THIS IS A GENERATED FILE - DO NOT MODIFY THIS FILE DIRECTLY - see pom.xml @@ -68,8 +69,8 @@ namespace Gremlin.Net.Process.Traversal public GraphTraversal<$method.t1, $method.t2> <%= toCSharpMethodName.call(method.methodName) %><%= method.tParam %> (<%= method.parameters %>) { <% if (method.parameters.contains("params ")) { - %> var args = new List<$method.argsListType>(<%= method.paramNames.init().size() %> + <%= method.paramNames.last() %>.Length) {<%= method.paramNames.init().join(", ") %>}; - args.AddRange(<%= method.paramNames.last() %>); + %> var args = new List(<%= method.paramNames.init().size() %> + <%= method.paramNames.last() %>.Length) {<%= method.paramNames.init().join(", ") %>}; + args.AddRange(<%= method.paramNames.last() %><% if (method.isArgsCastNecessary) { %>.Cast()<% } %>); Bytecode.AddStep("<%= method.methodName %>", args.ToArray());<% } else { diff --git a/gremlin-dotnet/glv/GraphTraversalSource.template b/gremlin-dotnet/glv/GraphTraversalSource.template index afa42973dee..bbc3684804e 100644 --- a/gremlin-dotnet/glv/GraphTraversalSource.template +++ b/gremlin-dotnet/glv/GraphTraversalSource.template @@ -23,6 +23,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Gremlin.Net.Process.Remote; using Gremlin.Net.Process.Traversal.Strategy.Decoration; using Gremlin.Net.Structure; @@ -131,8 +132,8 @@ namespace Gremlin.Net.Process.Traversal { var traversal = new GraphTraversal<$method.typeNameString>(TraversalStrategies, new Bytecode(Bytecode)); <% if (method.parameters.contains("params ")) { - %>var args = new List<$method.argsListType>(<%= method.paramNames.init().size() %> + <%= method.paramNames.last() %>.Length) {<%= method.paramNames.init().join(", ") %>}; - args.AddRange(<%= method.paramNames.last() %>); + %>var args = new List(<%= method.paramNames.init().size() %> + <%= method.paramNames.last() %>.Length) {<%= method.paramNames.init().join(", ") %>}; + args.AddRange(<%= method.paramNames.last() %><% if (method.isArgsCastNecessary) { %>.Cast()<% } %>); traversal.Bytecode.AddStep("<%= method.methodName %>", args.ToArray());<% } else { diff --git a/gremlin-dotnet/glv/generate.groovy b/gremlin-dotnet/glv/generate.groovy index 29f9ec75553..2d8b005adab 100644 --- a/gremlin-dotnet/glv/generate.groovy +++ b/gremlin-dotnet/glv/generate.groovy @@ -191,14 +191,12 @@ def getParamNames = { parameters -> } } -def getArgsListType = { parameterString -> - def argsListType = "object" +def isParamsArgCastNecessary = { parameterString -> if (parameterString.contains("params ")) { def paramsType = parameterString.substring(parameterString.indexOf("params ") + "params ".length(), parameterString.indexOf("[]")) - if (paramsType == "E" || paramsType == "S") - argsListType = paramsType + return paramsType == "E" || paramsType == "S" } - argsListType + return false } def hasMethodNoGenericCounterPartInGraphTraversal = { method -> @@ -270,8 +268,8 @@ def binding = ["pmethods": P.class.getMethods(). def tParam = getCSharpGenericTypeParam(t2) def parameters = getCSharpParamString(javaMethod, true) def paramNames = getParamNames(javaMethod.parameters) - def argsListType = getArgsListType(parameters) - return ["methodName": javaMethod.name, "typeNameString": typeNameString, "tParam":tParam, "parameters":parameters, "paramNames":paramNames, "argsListType":argsListType] + def isArgsCastNecessary = isParamsArgCastNecessary(parameters) + return ["methodName": javaMethod.name, "typeNameString": typeNameString, "tParam":tParam, "parameters":parameters, "paramNames":paramNames, "isArgsCastNecessary":isArgsCastNecessary] }, "graphStepMethods": GraphTraversal.getMethods(). findAll { GraphTraversal.class.equals(it.returnType) }. @@ -286,8 +284,8 @@ def binding = ["pmethods": P.class.getMethods(). def tParam = getCSharpGenericTypeParam(t2) def parameters = getCSharpParamString(javaMethod, true) def paramNames = getParamNames(javaMethod.parameters) - def argsListType = getArgsListType(parameters) - return ["methodName": javaMethod.name, "t1":t1, "t2":t2, "tParam":tParam, "parameters":parameters, "paramNames":paramNames, "argsListType":argsListType] + def isArgsCastNecessary = isParamsArgCastNecessary(parameters) + return ["methodName": javaMethod.name, "t1":t1, "t2":t2, "tParam":tParam, "parameters":parameters, "paramNames":paramNames, "isArgsCastNecessary":isArgsCastNecessary] }, "anonStepMethods": __.class.getMethods(). findAll { GraphTraversal.class.equals(it.returnType) }. diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs index c8708dfd8e8..81d3c623069 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversal.cs @@ -22,6 +22,7 @@ #endregion using System.Collections.Generic; +using System.Linq; using Gremlin.Net.Structure; // THIS IS A GENERATED FILE - DO NOT MODIFY THIS FILE DIRECTLY - see pom.xml @@ -870,8 +871,8 @@ public GraphTraversal InV () /// public GraphTraversal Inject (params E[] injections) { - var args = new List(0 + injections.Length) {}; - args.AddRange(injections); + var args = new List(0 + injections.Length) {}; + args.AddRange(injections.Cast()); Bytecode.AddStep("inject", args.ToArray()); return Wrap(this); } diff --git a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs index 7115016b61e..9f735f48ad2 100644 --- a/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs +++ b/gremlin-dotnet/src/Gremlin.Net/Process/Traversal/GraphTraversalSource.cs @@ -23,6 +23,7 @@ using System; using System.Collections.Generic; +using System.Linq; using Gremlin.Net.Process.Remote; using Gremlin.Net.Process.Traversal.Strategy.Decoration; using Gremlin.Net.Structure; @@ -307,8 +308,8 @@ public GraphTraversal AddV(string label) public GraphTraversal Inject(params S[] starts) { var traversal = new GraphTraversal(TraversalStrategies, new Bytecode(Bytecode)); - var args = new List(0 + starts.Length) {}; - args.AddRange(starts); + var args = new List(0 + starts.Length) {}; + args.AddRange(starts.Cast()); traversal.Bytecode.AddStep("inject", args.ToArray()); return traversal; } diff --git a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/BytecodeGeneration/BytecodeGenerationTests.cs b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/BytecodeGeneration/BytecodeGenerationTests.cs index 266e3c62d92..4b13bdec44f 100644 --- a/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/BytecodeGeneration/BytecodeGenerationTests.cs +++ b/gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/BytecodeGeneration/BytecodeGenerationTests.cs @@ -76,6 +76,18 @@ public void g_WithSackX1X_E_GroupCount_ByXweightX() Assert.Equal(1, bytecode.StepInstructions[2].Arguments.Length); } + [Fact] + public void g_InjectX1_2_3X() + { + var g = new Graph().Traversal(); + + var bytecode = g.Inject(1, 2, 3).Bytecode; + + Assert.Equal(1, bytecode.StepInstructions.Count); + Assert.Equal("inject", bytecode.StepInstructions[0].OperatorName); + Assert.Equal(3, bytecode.StepInstructions[0].Arguments.Length); + } + [Fact] public void AnonymousTraversal_Start_EmptyBytecode() {