[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user asfgit closed the pull request at: https://github.com/apache/tinkerpop/pull/710 ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/710#discussion_r138366459 --- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs --- @@ -102,9 +102,9 @@ public void g_V_HasXname_markoX_ValueMap_Next() var connection = _connectionFactory.CreateRemoteConnection(); var g = graph.Traversal().WithRemote(connection); -var receivedValueMap = g.V().Has("name", "marko").ValueMap().Next(); +var receivedValueMap = g.V().Has("name", "marko").ValueMap().Next(); --- End diff -- Ah ok, I saw the thread on the mailing list, but somehow I didn't draw the connection to this change. ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user jorgebay commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/710#discussion_r138317662 --- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs --- @@ -102,9 +102,9 @@ public void g_V_HasXname_markoX_ValueMap_Next() var connection = _connectionFactory.CreateRemoteConnection(); var g = graph.Traversal().WithRemote(connection); -var receivedValueMap = g.V().Has("name", "marko").ValueMap().Next(); +var receivedValueMap = g.V().Has("name", "marko").ValueMap().Next(); --- End diff -- The serialization format for maps changed (see JIRA tickets 1427 and 1658), the result from this traversal was originally a js object: ```js { "marko": { "@type": "g:Int32", "@value": 29 } } ``` And changed in GraphSON3 to a type "g:Map" for which we use `IDictionary`. I've started a discussion on the mailing list regarding collections child types: https://lists.apache.org/thread.html/3918353aaa63aa07b69214da24fa7aa0760004227fc57fa2b3bcae86@%3Cdev.tinkerpop.apache.org%3E ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/710#discussion_r138113216 --- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/SideEffectTests.cs --- @@ -129,10 +139,8 @@ public void ShouldReturnBothSideEffectForTraversalWithTwoSideEffects_() Assert.Equal(2, keys.Count); Assert.Contains("m", keys); Assert.Contains("n", keys); -var n = (Dictionary) t.SideEffects.Get("n"); -Assert.Equal(2, n.Count); -Assert.Equal(3, n["lop"]); -Assert.Equal(1, n["ripple"]); +var n = (IList) t.SideEffects.Get("n"); +Assert.Equal(n.Select(tr => ((Traverser)tr).Object), new[] {"lop", "ripple"}); --- End diff -- Please switch the sides of the arguments here as the expected value should be the first argument and the actual value the second. Otherwise the messages for a failed test can become misleading. ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/710#discussion_r138116451 --- Diff: gremlin-dotnet/src/Gremlin.Net/Driver/Messages/ResponseResult.cs --- @@ -23,13 +23,14 @@ using System.Collections.Generic; using Newtonsoft.Json; +using Newtonsoft.Json.Linq; namespace Gremlin.Net.Driver.Messages { internal class ResponseResult { [JsonProperty(PropertyName = "data")] -public List Data { get; set; } +public JToken Data { get; set; } --- End diff -- You can completely remove the type parameter `T` from the class when you change the type here to `JToken`. ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/710#discussion_r138111430 --- Diff: gremlin-dotnet/test/Gremlin.Net.IntegrationTest/Process/Traversal/DriverRemoteConnection/GraphTraversalTests.cs --- @@ -102,9 +102,9 @@ public void g_V_HasXname_markoX_ValueMap_Next() var connection = _connectionFactory.CreateRemoteConnection(); var g = graph.Traversal().WithRemote(connection); -var receivedValueMap = g.V().Has("name", "marko").ValueMap().Next(); +var receivedValueMap = g.V().Has("name", "marko").ValueMap().Next(); --- End diff -- Shouldn't this also work when `string` is used as the generic type parameter? ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
Github user FlorianHockmann commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/710#discussion_r138109326 --- Diff: gremlin-dotnet/src/Gremlin.Net/Structure/IO/GraphSON/Path3Deserializer.cs --- @@ -0,0 +1,45 @@ +#region License + +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +#endregion + +using System.Collections.Generic; +using System.Linq; +using Newtonsoft.Json.Linq; + +namespace Gremlin.Net.Structure.IO.GraphSON +{ +internal class Path3Deserializer : IGraphSONDeserializer +{ +public dynamic Objectify(JToken graphsonObject, GraphSONReader reader) +{ +// "labels" is a object[] where each item is ISet +var labelProperty = (object[])reader.ToObject(graphsonObject["labels"]); +var labels = labelProperty +.Select(x => new HashSet(((ISet)x).Cast())) +.ToList>(); +// "objects" is an object[] +object[] objectsProperty = reader.ToObject(graphsonObject["objects"]); +var objects = objectsProperty.ToArray(); --- End diff -- Isn't `ToArray()` redundant here when `objectsProperty` is already of type `object[]`? ---
[GitHub] tinkerpop pull request #710: TINKERPOP-1730 Gremlin .NET: add support for Gr...
GitHub user jorgebay opened a pull request: https://github.com/apache/tinkerpop/pull/710 TINKERPOP-1730 Gremlin .NET: add support for GraphSON3 https://issues.apache.org/jira/browse/TINKERPOP-1730 You can merge this pull request into a Git repository by running: $ git pull https://github.com/apache/tinkerpop TINKERPOP-1730 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/710.patch To close this pull request, make a commit to your master/trunk branch with (at least) the following in the commit message: This closes #710 commit abc1dbfd7e0cae7c326b630b4309c24848e1012b Author: Jorge Bay Gondra Date: 2017-09-08T15:32:29Z Gremlin .NET Support GraphSON3 ---