[GitHub] tinkerpop issue #923: TINKERPOP-2028: Register GremlinServerModule to GraphS...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/923 @spmallette I added the changelog entry, please let me know if it's appropriate thanks ---
[GitHub] tinkerpop issue #923: TINKERPOP-2028: Register GremlinServerModule to GraphS...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/923 The reason this is targeting master is because although I did not break backward compatibility and put previous outdated methods as Deprecated I wasn't sure what the strategy is here for retiring things in patch versions. Also there would be a slight change of behavior if anybody were to have also extended `AbstractGraphSON2MessageSerializer`. Let me know if that's acceptable to put in a patch or not, if so I don't any other reason not to do it in tp33. ---
[GitHub] tinkerpop pull request #924: Add self loop example to Cycle Detection recipe...
Github user newkek closed the pull request at: https://github.com/apache/tinkerpop/pull/924 ---
[GitHub] tinkerpop issue #924: Add self loop example to Cycle Detection recipe.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/924 sorry - I messed up - shouldn't have opened that ---
[GitHub] tinkerpop pull request #924: Add self loop example to Cycle Detection recipe...
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/924 Add self loop example to Cycle Detection recipe. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop cycles-recipe-self-loops Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/924.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 #924 commit 3afc57653a9ca81ba560b1b9a6be8c3ad7b1251d Author: Kevin Gallardo Date: 2018-08-24T21:18:43Z Add self loop example to Cycle Detection recipe. ---
[GitHub] tinkerpop pull request #921: Add self loop example to Cycle Detection recipe...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/921#discussion_r213448492
--- Diff: docs/src/recipes/cycle-detection.asciidoc ---
@@ -48,6 +48,31 @@ the length of the cycle is known to be three and there
is no need to exceed that
cycle. It returned three, because there was one for each vertex that
started the cycle (i.e. one for `A`, one for `B`
and one for `C`). This next line introduce deduplication to only return
unique cycles.
+Note that these traversals won't detect self-loops (vertices directly
connected to themselves).
+To do so, you would need to `.emit()` a Traverser before the repeat()-loop.
+
+[gremlin-groovy]
+
+g.addV().property(id,'a').as('a').
+ addV().property(id,'b').as('b').
+ addV().property(id,'c').as('c').
+ addV().property(id,'d').as('d').
+ addE('knows').from('a').to('b').
+ addE('knows').from('b').to('c').
+ addE('knows').from('c').to('a').
+ addE('knows').from('a').to('d').
+ addE('knows').from('c').to('d').
+ addE('self').from('a').to('a').iterate()
+g.V().as('a').
+emit().
+repeat(outE().inV().simplePath()).
+times(2).
+outE().inV().where(eq('a')).
+path().
+ by(id).
+ by(label)
+
+
--- End diff --
Done
---
[GitHub] tinkerpop issue #923: TINKERPOP-2028: Register GremlinServerModule to GraphS...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/923 Left the current behavior unchanged but deprecated the current method, in favor of another `GraphSONMessageSerializer` constructor that gives the opportunity to child classes of `AbstractGraphSON2MessageSerializer` to register their own modules when a `GraphSONMapper.Builder` is provided. ---
[GitHub] tinkerpop pull request #923: TINKERPOP-2028: Register GremlinServerModule to...
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/923 TINKERPOP-2028: Register GremlinServerModule to GraphSON message seri⦠â¦alizer You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop TINKERPOP-2028 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/923.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 #923 commit 6513ce68c37fbdcb57bfa54f68b739d6e8f176cc Author: Kevin Gallardo Date: 2018-08-28T19:34:30Z TINKERPOP-2028: Register GremlinServerModule to GraphSON message serializer ---
[GitHub] tinkerpop pull request #921: Add self loop example to Cycle Detection recipe...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/921#discussion_r213426961
--- Diff: docs/src/recipes/cycle-detection.asciidoc ---
@@ -48,6 +48,31 @@ the length of the cycle is known to be three and there
is no need to exceed that
cycle. It returned three, because there was one for each vertex that
started the cycle (i.e. one for `A`, one for `B`
and one for `C`). This next line introduce deduplication to only return
unique cycles.
+Note that these traversals won't detect self-loops (vertices directly
connected to themselves).
+To do so, you would need to `.emit()` a Traverser before the repeat()-loop.
+
+[gremlin-groovy]
+
+g.addV().property(id,'a').as('a').
+ addV().property(id,'b').as('b').
+ addV().property(id,'c').as('c').
+ addV().property(id,'d').as('d').
+ addE('knows').from('a').to('b').
+ addE('knows').from('b').to('c').
+ addE('knows').from('c').to('a').
+ addE('knows').from('a').to('d').
+ addE('knows').from('c').to('d').
+ addE('self').from('a').to('a').iterate()
+g.V().as('a').
+emit().
+repeat(outE().inV().simplePath()).
+times(2).
+outE().inV().where(eq('a')).
+path().
+ by(id).
+ by(label)
+
+
--- End diff --
Okay. well no strong opinion here I can change it for that
---
[GitHub] tinkerpop pull request #921: Add self loop example to Cycle Detection recipe...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/921#discussion_r213372950
--- Diff: docs/src/recipes/cycle-detection.asciidoc ---
@@ -48,6 +48,31 @@ the length of the cycle is known to be three and there
is no need to exceed that
cycle. It returned three, because there was one for each vertex that
started the cycle (i.e. one for `A`, one for `B`
and one for `C`). This next line introduce deduplication to only return
unique cycles.
+Note that these traversals won't detect self-loops (vertices directly
connected to themselves).
+To do so, you would need to `.emit()` a Traverser before the repeat()-loop.
+
+[gremlin-groovy]
+
+g.addV().property(id,'a').as('a').
+ addV().property(id,'b').as('b').
+ addV().property(id,'c').as('c').
+ addV().property(id,'d').as('d').
+ addE('knows').from('a').to('b').
+ addE('knows').from('b').to('c').
+ addE('knows').from('c').to('a').
+ addE('knows').from('a').to('d').
+ addE('knows').from('c').to('d').
+ addE('self').from('a').to('a').iterate()
+g.V().as('a').
+emit().
+repeat(outE().inV().simplePath()).
+times(2).
+outE().inV().where(eq('a')).
+path().
+ by(id).
+ by(label)
+
+
--- End diff --
```
The above case assumed that the need was to only detect cycles over a path
length of three.
It also respected the directionality of the edges by only considering
outgoing ones.
Also note that this traversal won't detect self-loops []
--
[self-loop code example]
--
What would need to change to detect cycles of arbitrary length over both
incoming and
outgoing edges, in the modern graph?
--
[bi-directional code example]
--
[...]
```
Is that what you meant?
---
[GitHub] tinkerpop pull request #921: Add self loop example to Cycle Detection recipe...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/921#discussion_r212996121
--- Diff: docs/src/recipes/cycle-detection.asciidoc ---
@@ -48,6 +48,25 @@ the length of the cycle is known to be three and there
is no need to exceed that
cycle. It returned three, because there was one for each vertex that
started the cycle (i.e. one for `A`, one for `B`
and one for `C`). This next line introduce deduplication to only return
unique cycles.
+Note that these traversals won't detect self-loops (vertices directly
connected to themselves).
+To do so, you would need to `.emit()` a Traverser before the repeat()-loop.
+
+[gremlin-groovy]
+
+g.addV().property(id,'a').as('a').
+ addV().property(id,'b').as('b').
+ addV().property(id,'c').as('c').
+ addV().property(id,'d').as('d').
+ addE('knows').from('a').to('b').
+ addE('knows').from('b').to('c').
+ addE('knows').from('c').to('a').
+ addE('knows').from('a').to('d').
+ addE('knows').from('c').to('d').
+ addE('self').from('a').to('a').iterate()
+g.V().as('a').emit().repeat(out().simplePath()).times(2).
+ where(out().as('a')).path()
--- End diff --
Good point!
---
[GitHub] tinkerpop pull request #921: Add self loop example to Cycle Detection recipe...
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/921 Add self loop example to Cycle Detection recipe. Not sure if it is worth it but I figured out that self loops weren't detected in the given example. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop cycles-recipe-self-loops Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/921.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 #921 commit 87ce35824dac27f6bba9329aeb88b49940187c55 Author: Kevin Gallardo Date: 2018-08-24T21:18:43Z Add self loop example to Cycle Detection recipe. ---
[GitHub] tinkerpop issue #914: Do not format and reparse eval timeout arg when evalua...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/914 I've pushed a fix for tests that used to send the Eval_timeout as an int instead of a long, but also made the server robust in case an int is still sent. Not sure if we want to do that or to enforce the `long` by failing. ---
[GitHub] tinkerpop pull request #914: Do not format and reparse eval timeout arg when...
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/914 Do not format and reparse eval timeout arg when evaluating request. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop eval-timeout-request-message Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/914.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 #914 ---
[GitHub] tinkerpop issue #585: TINEKRPOP-1654 tp32 Bumped to Jackson 2.8.7
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/585 Cool, thanks - #586 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #586: TINKERPOP-1654: use deserializatinoContext in `...
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/586 TINKERPOP-1654: use deserializatinoContext in `typeFromId()`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop TINKERPOP-1654-tp32-newkek Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/586.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 #586 commit ad268ef4f784d8de0be7650e93e6ab750e3010d5 Author: Kevin Gallardo <kevin.galla...@datastax.com> Date: 2017-03-28T14:10:14Z TINKERPOP-1654: use deserializatinoContext in `typeFromId()`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #585: TINEKRPOP-1654 tp32 Bumped to Jackson 2.8.7
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/585 @spmallette I actually had a fix slightly different in mind, I just tested and it works and the tests pass, mind if I use that different solution? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #523: Add ResponseMessage deserializer for GraphSON2.
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/523 Add ResponseMessage deserializer for GraphSON2. Motivation: `ResponseMessage` used to be deserialized manually via a method on GraphSONMessageSerializerV2d0 (deserializeResponse). Which prevented from using the `Mapper` directly to deserialize a response. Modification: Deserialization of `ResponseMessage` is done in a deserializer registered by the `GremlinServerModule` and the GraphSONMessageSerializerV2d0 has been changed accordingly. Also added tests of ser/de for `RequestMessage` and `ResponseMessage` from the `ObjectMapper`. Result: Calls like `objectMapper.readValue(jsonString, ResponseMessage.class)` work now. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop fix-graphson2-responsemessage Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/523.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 #523 commit 5ba605bf67fe6c46cdb874b63703cf65aa25fa6a Author: Kevin Gallardo <kevin.galla...@datastax.com> Date: 2017-01-05T16:57:32Z Add ResponseMessageDeserializer for GraphSON2. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #517: Fix numbers deserialization for GraphSON2.
GitHub user newkek opened a pull request:
https://github.com/apache/tinkerpop/pull/517
Fix numbers deserialization for GraphSON2.
Issue:
There was a small issue with `ObjectMapper` that was preventing one from
doing `objectMapper.readValue(jsonString, Integer.class/Double.class)`. Turns
out the issue come from the fact that Jackson normally doesn't uses the
`GraphSONTypeResolverBuilder` hook I've introduced for the typing system - and
call directly the default deserializers - for numbers values like `Integer` or
`Double`, which led the default deserializers to fail since they are facing a
Typed object (`{"@type":..., "@value":}`) and not simply the value.
Fix:
Adding simple deserializers for `Integer` and `Double` causes Jackson to
use the hook for the typing system and hence deserialization works normally.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/newkek/incubator-tinkerpop
fix-graphson2-numbers
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/tinkerpop/pull/517.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 #517
commit c66bbdf477c1576eb700271d3473e3d67ff4ab5f
Author: Kevin Gallardo <kevin.galla...@datastax.com>
Date: 2016-12-19T15:56:42Z
Fix numbers deserialization for GraphSON2.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #499: TINKERPOP-1520: Difference between 'has' step generate...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/499 > If we do this, then we would have to change Edge back to the same model to be consistent Indeed. > why change it back? Because currently it is not consistent, (Detached)Edge/Vertex/VertexProperty all derive from (Detached)Element, for which `properties` have the same structure, but with what's currently on the PR, Edge and VertexProperty don't have their `properties` serialized in the same format than Vertex. If we unify all we can make generalize the deserialization of an object that derives from Element and make the deserialization code more readable. In my opinion I don't believe verbosity should be chosen over consistency here, especially since GraphSON 2 is already very very verbose and not really meant to be compact. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #499: TINKERPOP-1520: Difference between 'has' step generate...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/499 https://gist.github.com/newkek/bfc976712f1adcbe76bcea4e34435ea2 - the rest doesn't change --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r89014561
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -153,13 +152,16 @@ public void serialize(final Edge edge, final
JsonGenerator jsonGenerator, final
}
private void writeProperties(final Edge edge, final JsonGenerator
jsonGenerator) throws IOException {
-final Iterator<Property> elementProperties = normalize
?
+final Iterator<Property> edgeProperties = normalize ?
IteratorUtils.list(edge.properties(),
Comparators.PROPERTY_COMPARATOR).iterator() : edge.properties();
-if (elementProperties.hasNext()) {
+if (edgeProperties.hasNext()) {
jsonGenerator.writeFieldName(GraphSONTokens.PROPERTIES);
jsonGenerator.writeStartObject();
-elementProperties.forEachRemaining(prop ->
safeWriteObjectField(jsonGenerator, prop.key(), prop));
+while (edgeProperties.hasNext()) {
+final Property property = edgeProperties.next();
+jsonGenerator.writeObjectField(property.key(),
property.value());
--- End diff --
What you suggest in the PR here is
[this](https://gist.github.com/newkek/15e9933dc4b43674c2c60bc9487fc80e/revisions#diff-5a08d02157ba6ec13bacc40ab3a3a7bc)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r89014281
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -153,13 +152,16 @@ public void serialize(final Edge edge, final
JsonGenerator jsonGenerator, final
}
private void writeProperties(final Edge edge, final JsonGenerator
jsonGenerator) throws IOException {
-final Iterator<Property> elementProperties = normalize
?
+final Iterator<Property> edgeProperties = normalize ?
IteratorUtils.list(edge.properties(),
Comparators.PROPERTY_COMPARATOR).iterator() : edge.properties();
-if (elementProperties.hasNext()) {
+if (edgeProperties.hasNext()) {
jsonGenerator.writeFieldName(GraphSONTokens.PROPERTIES);
jsonGenerator.writeStartObject();
-elementProperties.forEachRemaining(prop ->
safeWriteObjectField(jsonGenerator, prop.key(), prop));
+while (edgeProperties.hasNext()) {
+final Property property = edgeProperties.next();
+jsonGenerator.writeObjectField(property.key(),
property.value());
--- End diff --
Basically I had written [this for some
specs](https://gist.github.com/newkek/15e9933dc4b43674c2c60bc9487fc80e#file-graph-types-asciidoc)
(note: not sure the Metrics and TraversalMetrics ones are still up to date but
the rest should be).
What I suggest is [the revision
here](https://gist.github.com/newkek/15e9933dc4b43674c2c60bc9487fc80e/revisions#diff-5a08d02157ba6ec13bacc40ab3a3a7bc).
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r89010812
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -153,13 +152,16 @@ public void serialize(final Edge edge, final
JsonGenerator jsonGenerator, final
}
private void writeProperties(final Edge edge, final JsonGenerator
jsonGenerator) throws IOException {
-final Iterator<Property> elementProperties = normalize
?
+final Iterator<Property> edgeProperties = normalize ?
IteratorUtils.list(edge.properties(),
Comparators.PROPERTY_COMPARATOR).iterator() : edge.properties();
-if (elementProperties.hasNext()) {
+if (edgeProperties.hasNext()) {
jsonGenerator.writeFieldName(GraphSONTokens.PROPERTIES);
jsonGenerator.writeStartObject();
-elementProperties.forEachRemaining(prop ->
safeWriteObjectField(jsonGenerator, prop.key(), prop));
+while (edgeProperties.hasNext()) {
+final Property property = edgeProperties.next();
+jsonGenerator.writeObjectField(property.key(),
property.value());
--- End diff --
Also I had waited before correcting the discrepancy mentioned above for the
VertexProperty's properties because it's a breaking change and it seems
dangerous to make it part of GraphSON 2.**0** in my opinion
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r89010235
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -153,13 +152,16 @@ public void serialize(final Edge edge, final
JsonGenerator jsonGenerator, final
}
private void writeProperties(final Edge edge, final JsonGenerator
jsonGenerator) throws IOException {
-final Iterator<Property> elementProperties = normalize
?
+final Iterator<Property> edgeProperties = normalize ?
IteratorUtils.list(edge.properties(),
Comparators.PROPERTY_COMPARATOR).iterator() : edge.properties();
-if (elementProperties.hasNext()) {
+if (edgeProperties.hasNext()) {
jsonGenerator.writeFieldName(GraphSONTokens.PROPERTIES);
jsonGenerator.writeStartObject();
-elementProperties.forEachRemaining(prop ->
safeWriteObjectField(jsonGenerator, prop.key(), prop));
+while (edgeProperties.hasNext()) {
+final Property property = edgeProperties.next();
+jsonGenerator.writeObjectField(property.key(),
property.value());
--- End diff --
I had noticed the discrepancy in that VertexProperty's Properties are not
serialized as Properties but instead the values directly are serialized and
that's an oversight from when I implemented GraphSON2. I understand that the
same thing is applied here and it makes a smaller footprint but I think we
should rather fix the VertexProperty's properties to be serialized as Map of
Properties and not Map of the values, because it reflects directly what is
going on underneath and so the deserialization process does not need to any
transformation, just set the fields of the newly created object with the rest
of the deserialized data, rather than creating. Also it is not consistent
because each Vertex, Edge, and VertexProperty derive from Element, which has
the same properties field, a list of ? extends Property so we should come up
with the same format for all I believe and here a Vertex would have its
properties serialized objects, but the others would have their properties
serialized a
s the values, not sure if that's a major concern but I would advocate for
consistency as much as possible
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r89010143
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -153,13 +152,16 @@ public void serialize(final Edge edge, final
JsonGenerator jsonGenerator, final
}
private void writeProperties(final Edge edge, final JsonGenerator
jsonGenerator) throws IOException {
-final Iterator<Property> elementProperties = normalize
?
+final Iterator<Property> edgeProperties = normalize ?
IteratorUtils.list(edge.properties(),
Comparators.PROPERTY_COMPARATOR).iterator() : edge.properties();
-if (elementProperties.hasNext()) {
+if (edgeProperties.hasNext()) {
jsonGenerator.writeFieldName(GraphSONTokens.PROPERTIES);
jsonGenerator.writeStartObject();
-elementProperties.forEachRemaining(prop ->
safeWriteObjectField(jsonGenerator, prop.key(), prop));
+while (edgeProperties.hasNext()) {
--- End diff --
I had noticed the discrepancy in that VertexProperty's Properties are not
serialized as Properties but instead the values directly are serialized and
that's an oversight from when I implemented GraphSON2. I understand that the
same thing is applied here and it makes a smaller footprint but I think we
should rather fix the VertexProperty's properties to be serialized as Map of
Properties and not Map of the values, because it reflects directly what is
going on underneath and so the deserialization process does not need to any
transformation, just set the fields of the newly created object with the rest
of the deserialized data, rather than creating. Also it is not consistent
because each Vertex, Edge, and VertexProperty derive from Element, which has
the same `properties` field, a list of `? extends Property` so we should come
up with the same format for all I believe and here a Vertex would have its
properties serialized objects, but the others would have their properties
seriali
zed as the values, not sure if that's a major concern but I would advocate for
consistency as much as possible
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request: https://github.com/apache/tinkerpop/pull/499#discussion_r88769838 --- Diff: gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java --- @@ -299,6 +325,7 @@ public void serialize(final TraversalExplanation traversalExplanation, final Jso intermediate.put(GraphSONTokens.STRATEGY, pair.getValue0().toString()); intermediate.put(GraphSONTokens.CATEGORY, pair.getValue0().getTraversalCategory().getSimpleName()); intermediate.put(GraphSONTokens.TRAVERSAL, getStepsAsList(pair.getValue1())); +intermediate.put(GraphSONTokens.TRAVERSAL, getStepsAsList(pair.getValue1())); --- End diff -- Is this line supposed to be duplicated? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r88769492
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -175,8 +177,30 @@ public PropertyJacksonSerializer() {
public void serialize(final Property property, final JsonGenerator
jsonGenerator, final SerializerProvider serializerProvider)
throws IOException {
jsonGenerator.writeStartObject();
-jsonGenerator.writeObjectField(GraphSONTokens.KEY,
property.key());
+jsonGenerator.writeStringField(GraphSONTokens.KEY,
property.key());
jsonGenerator.writeObjectField(GraphSONTokens.VALUE,
property.value());
+if (property.element() instanceof VertexProperty) {
+VertexProperty vertexProperty = (VertexProperty)
property.element();
+
jsonGenerator.writeObjectFieldStart(GraphSONTokens.ELEMENT);
+jsonGenerator.writeStringField(GraphSONTokens.VALUETYPE,
"g:VertexProperty");
+
jsonGenerator.writeObjectFieldStart(GraphSONTokens.VALUEPROP);
+jsonGenerator.writeObjectField(GraphSONTokens.ID,
vertexProperty.id());
+jsonGenerator.writeStringField(GraphSONTokens.LABEL,
vertexProperty.label());
+jsonGenerator.writeObjectField(GraphSONTokens.VERTEX,
vertexProperty.element().id());
+jsonGenerator.writeEndObject();
+jsonGenerator.writeEndObject();
+} else if (property.element() instanceof Edge) {
+Edge edge = (Edge) property.element();
+
jsonGenerator.writeObjectFieldStart(GraphSONTokens.ELEMENT);
+jsonGenerator.writeStringField(GraphSONTokens.VALUETYPE,
"g:Edge");
--- End diff --
This will write an object of type "g:Edge" but will not contain the same
content than what a Edge serializer would have serialized, and so the contents
are inconsistent. Here what is missing is the out/inVLabel
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #499: TINKERPOP-1520: Difference between 'has' step g...
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/499#discussion_r88769301
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONSerializersV2d0.java
---
@@ -175,8 +177,30 @@ public PropertyJacksonSerializer() {
public void serialize(final Property property, final JsonGenerator
jsonGenerator, final SerializerProvider serializerProvider)
throws IOException {
jsonGenerator.writeStartObject();
-jsonGenerator.writeObjectField(GraphSONTokens.KEY,
property.key());
+jsonGenerator.writeStringField(GraphSONTokens.KEY,
property.key());
jsonGenerator.writeObjectField(GraphSONTokens.VALUE,
property.value());
+if (property.element() instanceof VertexProperty) {
+VertexProperty vertexProperty = (VertexProperty)
property.element();
+
jsonGenerator.writeObjectFieldStart(GraphSONTokens.ELEMENT);
+jsonGenerator.writeStringField(GraphSONTokens.VALUETYPE,
"g:VertexProperty");
--- End diff --
It isn't great to bypass the typing system.. As the type system is dynamic,
hardcoding the type can lead to inconsistent types if users overrides the
Gremlin standard serializers and types as it is currently possible. But I see
why we can't just jsonGenerator.writeObjectField(GraphSONTokens.ELEMENT,
property.element()), because that would, well, re-write VertexProperty's
properties and so on. We can maybe introduce a "g:Element" type? Or maybe just
write a simple Map here where the deserializers are aware of what to do with a
"element" field? wdyt?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #499: TINKERPOP-1520: Difference between 'has' step generate...
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/499 Indeed there seems to be some breaking changes in the protocol. It seems ok for pure TinkerPop since the parsing code is updated accordingly but will cause problems to implementors, just as a note (maybe only for my own remembering)... --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/478 I think the `promise()` method is more elegant as well, as it avoids adding many new methods in the Traversal API --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #478: TINKERPOP-1490 Implemented promise API for Traversal
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/478 The issue with using a secondary thread pool that will start a new thread for each Traversal execution would be more important when the Traversal is backed by a RemoteConnection. Where each Traversal represents a "query" that is sent to a server, if the driver behind the RemoteConnection is able to handle tens of thousands of requests, the performance will still be limited to the number of threads the async thread pool can handle simultaneously, and there would be a big waste of CPU/Threads. Ideally as @jorgebay suggests with the strategies more of the TinkerPop lib would be changed to become fully async, where maybe synchronous methods would only be blocking calls to the async execution method(s) (`next()` calls `promise().get()`). The thread pool is still needed at some point though, since executing against a local TinkerGraph is currently done in the same thread, as a sequential operation so Returning with a Future, and Continuing to process the operation in the background has to be done in 2 separate threads simultaneously. _just thinking out loud and I probably miss a lot of details_ There may be a way to have a method `submitAsync()` on the `RemoteConnection` that returns a Future of Traverser (instead of Traverser) that, associated with what @jorgebay suggests for the Strategies and _some other changes_ would allow the `promise()` call, when associated with a `RemoteConnection` not to create a new thread each time by default and simply return the Future returned with `RemoteConnection#submitAsync()`, and thus delegate the asynchronous execution to the RemoteConnection --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #403: Fix InetAddress serialization with GraphSON 2.0...
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/403 Fix InetAddress serialization with GraphSON 2.0. InetAddress serialization/deserialization wasn't working with GraphSON 2.0. "InetAddress" type has been added to the `GraphSONXModule`. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop graphson-inetaddress-fix Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/403.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 #403 commit a0c677c0d49a9396709eba19baa6bf645043a77b Author: Kevin Gallardo <kevin.galla...@datastax.com> Date: 2016-09-09T18:03:49Z Fix serialization of InetAddress with GraphSON 2.0. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #390: GraphSON 2.0 Deser tweaks and improvements
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/390 Seems like it is only on my machine... I think Stephen tried on the docker image ? Yes what I have seen so far is it happens at the moment we call `deserializationContext.findContextualValueDeserializer(typeFromId)` in `GraphSONTypeDeserializer` where the `typeFromId` is the type that has been constructed at init time with `TypeFactory.defaultInstance().constructType(Tree.class)`. I also see that for some reason I don't get, it happens to me only when the [Enums types](https://github.com/apache/tinkerpop/blob/TINKERPOP-1278/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphson/GraphSONModule.java#L131-L140) from the Traversal serialzers have been constructed as well. If I comment those lines, I've got no problem when doing constructType(Tree.class) like all the rest.. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #390: GraphSON 2.0 Deser tweaks and improvements
GitHub user newkek opened a pull request: https://github.com/apache/tinkerpop/pull/390 GraphSON 2.0 Deser tweaks and improvements There's 2 commits: - the first brings a massive improvement to the GraphSON 2.0 deserialization, avoiding a lot of useless copies done on the JSON parser when parsing to detect a type in the payload. There's in almost every case no copy done anymore, thus reducing the memory impact and improves performances. - second fixes a bug I had on my machine that was causing a StackOverflow exception when deserializing a `Tree` from GraphSON2.0. The issue has been pinned down to understanding that there could be a never-ending recursive reference when creating a `JavaType` to be indexed in the GraphSONTypeIdResolver. All detailed in comments in the code but now I have a special case for when a `Tree` is registered that force a non-cyclic reference type. It is still unclear to me why this happens to me but not all the time for @spmallette for example. In any case this fixes it for me and should be fixing for everybody.. You can merge this pull request into a Git repository by running: $ git pull https://github.com/newkek/incubator-tinkerpop TINKERPOP-1278-imp Alternatively you can review and apply these changes as the patch at: https://github.com/apache/tinkerpop/pull/390.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 #390 commit bbdfc0c5583f58355f91202e0e7870a3cb22e4bd Author: Kevin Gallardo <kevin.galla...@datastax.com> Date: 2016-08-25T09:40:01Z Improved performance on GraphSON 2.0 deserialization. commit 13c80cb22c4a847079b0b1c81f20e5958ec1f6b3 Author: Kevin Gallardo <kevin.galla...@datastax.com> Date: 2016-08-25T13:08:44Z Fix an issue with Tree deserialization and the type system. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #386: TINKERPOP-1274: GraphSON 2.0 [revised]
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/386#discussion_r75709109
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/MutableMetrics.java
---
@@ -42,6 +42,18 @@ public MutableMetrics(final String id, final String
name) {
this.name = name;
}
+// create a MutableMetrics from an Immutable one.
+// needed that for tests, don't know if it is worth keeping it public.
+// TODO: see if it's ok to add this
+public MutableMetrics(Metrics other) {
--- End diff --
Same here I added a convenience constructor that takes any kind of Metrics
and returns a `MutableMetrics`. It was convenient for tests, as I wanted to
serialize Metrics that I am sure have nested Metrics, and make sure the ser/de
works for those.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #386: TINKERPOP-1274: GraphSON 2.0 [revised]
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/386#discussion_r75708714
--- Diff:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/util/DefaultTraversalMetrics.java
---
@@ -54,6 +54,14 @@
public DefaultTraversalMetrics() {
}
+// This is only a convenient constructor needed for GraphSON
deserialization.
+// TODO: see if that's ok to add that.
+public DefaultTraversalMetrics(long totalStepDurationNs,
List metricsMap) {
--- End diff --
Not sure who I should ask about that, I decided to add a public constructor
here for convenience during the deserialization, it takes the total duration,
and a list of all the Metrics its composed of.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #386: TINKERPOP-1274: GraphSON 2.0 [revised]
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/386 I just pushed corrected docs, and renamed everything "domain" to "namespace". Waiting for more consensus to change the default gremlin namespace. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #386: TINKERPOP-1274: GraphSON 2.0 [revised]
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/386 A remark: @okram requested `TraversalExplanation` to be included in the list of Graph objects. I've noticed that the current `TraversalExplanation` Graphson serializer serializes a `Traversal`. In order to follow the requirement of this GraphSON2.0 type system, we would have to write a deserializer for the `TraversalExplanation` (there isn't currently). However, the current 1.0 serializer serializes the Traversal as the `toString()` of the Traversal's step list, which means that it is not possible to deserialize to a Traversal with only such info. So I believe it would be more appropriate to wait for TP1278 before implementing the `TraversalExplanation` ser/de and leave it out of the scope of this PR. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #386: TINKERPOP-1274: GraphSON 2.0 [revised]
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/386 *NB: the conflicts for merge are caused by the CHANGELOG* --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/351 Closing this in favor of #386. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek closed the pull request at: https://github.com/apache/tinkerpop/pull/351 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue:
https://github.com/apache/tinkerpop/pull/351
@robertdale the format you suggest would lead to the same inconsistencies
as in GraphSON 1.0. Since the type is at the same level than the data itself,
whether the container is an Array or an Object, the type format would not be
the same. I just pushed a change in the format that is the one @PommeVerte
suggested, which gives a consistent format, without the concern of unordered
Lists (for reference the new format is `{"@type" : "typeName", "@value" :
value}`.
> please correct me if I'm wrong, but it doesn't look like the code does
any dynamic serializing.
The `TypeIdResolver` which is the object that the serializers will call to
get a TypeID from a java `Object` is dynamic in a way, in the sense that it
returns `o.getClass().getSimpleName()`. So there is no `object` -> typeID index
reference. However for the Deser, as explained in the description, Java by
default doesn't offer a way to get a Class by its simple name, so the
`TypeIdResolver` needs to keep a reference index of typeID(which is a class's
simple name) -> Java `Class`. Don't know if that answers your question..
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue:
https://github.com/apache/tinkerpop/pull/351
Well, I agree we cannot prevent some parsers and drivers to read the whole
JSON content and check the created object only afterward. I would not recommend
it though, because it is not great for memory consumption, but it is maybe
something we have the opportunity to avoid only thanks to Jackson and some
libraries may not offer that capability. So I guess I'll switch to the
`{"@type":"...", "value": ...}` format instead of the current one. Does that
sound good @spmallette @PommeVerte ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/351 Just pushed the change for `@class` to `@type`. All tests pass. 'twas a 1 line fix, isn't that wonderful. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/351 Yes the `@class` bothered me a little as well, since now it doesn't really describe a "Class". But I did not want to change, for consistency for GraphSON v1.0 and I reused the `GraphSONTokens.CLASS`. So if it is ok, I'd be +1 for `@type` actually. Concerning BSON (or MsgPack) it probably is indeed a more efficient serialization solution, however the goal of this PR is to optimize and improve JSON. Implementing Graph-BSON is probably another topic adjacent to this one, on which btw I'd be happy to collaborate as well. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue:
https://github.com/apache/tinkerpop/pull/351
@spmallette yes, maybe some code will help explain.
```
ObjectMapper mapper =
GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create().createMapper();
Map<String, Integer> map = new HashMap<>();
map.put("helo", 2);
String s = mapper.writeValueAsString(map);
// prints 's = {"helo":2}'
System.out.println("s = " + s);
Map read = mapper.readValue(s, Map.class);
// prints 'read.get("helo") = class java.lang.Integer'
System.out.println("read.get(\"helo\") = " +
read.get("helo").getClass());
Map<String, Long> map3 = new HashMap<>();
map3.put("helo", 2L);
String s2 = mapper.writeValueAsString(map3);
// prints 's2 = {"helo":[{"@class":"Long"},2]}'
System.out.println("s2 = " + s2);
Map read2 = mapper.readValue(s2, Map.class);
// prints 'read2.get("helo") = class java.lang.Long'
System.out.println("read2.get(\"helo\") = " +
read2.get("helo").getClass());
```
> number with decimal and no type = Java Float
Almost, by default decimals are Double, not Floats.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue:
https://github.com/apache/tinkerpop/pull/351
> I thought that java integer would be implied.
Yes with the current PR serializing an Integer will result in no type
added. Serializing a Long or a Short will result in an explicit type added. To
be very explicit, in JSON everything is Doubles and Longs, it does that because
it's the bigger container format that contains the others less precise ones.
Jackson however, considers that when an Integer is to be serialized, there is
no need for an explicit type because the precision will be kept since for JSON
it's in a Long. When it comes to serializing a Long, still no precision loss
but the format explicitly indicates to the Deserializer that what has been
serialized initially was a Long. So everything is as defined as if the Integer
was typed, because by default the reader assumes everything's an Integer, and
if not there will be a type to specify what it is. However we have save a large
payload size by not typing the integer. Same concept for Float VS Double, for
JSON everything is a Double, if a value is a Float, it will be t
yped, if not, by default it's a Float.
So as I said in the description I think the outcome of the mail
conversation was to not type Simple values. Mostly because we're not sure if
this is going to be useful or not. However, adding those types in the future,
for Simple values, can be very easily done and I've left detailed comments in
the `TypeSerializer` on how to add them when we deem it necessary. Also, adding
them would not break existing code since the format would be the same, it's
just that _every_ value would follow the format for typed values. Since there's
no possibility to mix-up for the Numeric values as explained above with the
Shorts and Longs and etc... I still think we should wait somebody explicitly
requires it.
> Perhaps {"@class":"java.lang.Integer", "value": 1} is a better option.
As I see it for how I implemented the TypeDeserializer, it acts as a meta
deserializer that will read the raw text JSON sequentially, so there's no
chance there can be a mixup in the order, it does not deserialize the whole
structure and creates a `List<Map<>>` object. Same for the TypeSerializer, it
does not create a Java `List` object to write the type, but it writes directly
in JSON "write a start_array token, write a start_map token, write a property
name, write a text field (the type name), write a end_array token, etc" so same
thing, there is no chance there can be a mix up in the order.
I don't know what it could look like for parsers of other languages, but it
would seem like doing something else than that would be quite inefficient in
terms of performance because it would that for every typed value you would
instantiate a `new List<Map<>>` just to read a simple value. Does that makes
sense ?
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69327810
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
So to sum up on that samples issue :
- the grateful-dead V1 samples have changed because for some reason, some
of the `inE` of some vertices were not written in the same order. I'm almost
sure the fix here has nothing to do with that order change, so I pushed the
changed ones. It also doesn't concern the `normalize` option of the GraphSON
mapper, since the `inE` are generally not ordered. So, quite a mystery but I
definitely don't that's something introduced by this branch.
- The `data/` folder now has the new `-v2d0` and `-v2d0-typed` graphs, but
not the `normalized` ones.
- The `gremlin-test/src/main/ressources/etc...` has all the new v2 graphs
and the normalized ones.
- I don't know what's up with the `sample.kryo` but there's near to
0.1% chances it's related to that branch. But I pushed the change though.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69314893
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
I'll try to find the differences for the grateful-dead in json but I'm not
sure I'd be able to tell concerning the diff for the `sample.kryo` as I'm not
fluent in bianry (yet) :)
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69288856
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
So, since now the build creates the new v2d0 graphs, and it seems like the
other ones are pushed in the repo, should I push the new ones too? Also, it
seems that the already pushed ones have modifications. Should I push all of as
well ? Here's the diff :
https://gist.github.com/newkek/4e0488268f9bb78e0d3597821ae6b357
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69288319
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
Apparently the TestHelper behaviour has changed in commit da2eb7e, but the
pom wasn't changed with regards to that change.
Ok I'll change the pom as explained earlier
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69283289
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
This is also broken on master
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69283048
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
Ah, looking at the pom, I'm getting this warning during the build :
```
[INFO] --- maven-resources-plugin:2.6:copy-resources
(copy-graphson-from-tmp-to-resources) @ tinkergraph-gremlin ---
[INFO] Using 'UTF-8' encoding to copy filtered resources.
[INFO] skip non existing resourceDirectory
/Users/kevingallardo/Documents/workspace/newkek-incubator-tinkerpop/tinkergraph-gremlin/target/tinkerpop-io
```
Seems like the pom is going to search for the ressources in
`${project.build.directory}/tinkerpop-io` and
`${project.build.directory}` is defined as
`${basedir}/target`.
But the `tempPath` in IoDataGenerationTest` is `tempPath =
TestHelper.makeTestDataPath(TinkerGraphTest.class, "tinkerpop-io").getPath() +
File.separator;`
If I change the pom to search in the right directory :
`${project.build.directory}/test-case-data/TinkerGraphTest/tinkerpop-io`,
it works.
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69276332
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
Oh I think I see what you mean, I don't see them in the root's `data/` dir,
I don't know why
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r69276176
--- Diff:
tinkergraph-gremlin/src/test/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/IoDataGenerationTest.java
---
@@ -275,4 +290,78 @@ else if (e.label().equals("writtenBy"))
GraphSONWriter.build().mapper(GraphSONMapper.build().embedTypes(true).create()).create().writeGraph(os4,
g);
os4.close();
}
+
+@Test
+public void shouldWriteGratefulDeadGraphSONV2d0() throws IOException {
+final TinkerGraph g = TinkerGraph.open();
+final TinkerGraph readG = TinkerGraph.open();
+
+final GraphReader reader = GryoReader.build().create();
+try (final InputStream stream =
AbstractGremlinTest.class.getResourceAsStream("/org/apache/tinkerpop/gremlin/structure/io/gryo/grateful-dead.kryo"))
{
+reader.readGraph(stream, g);
+}
+final OutputStream os2 = new FileOutputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONWriter.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().writeGraph(os2,
g);
+os2.close();
+
+final InputStream is = new FileInputStream(tempPath +
"grateful-dead-V2d0-typed.json");
+
GraphSONReader.build().mapper(GraphSONMapper.build().version(GraphSONVersion.V2_0).typeInfo(GraphSONMapper.TypeInfo.PARTIAL_TYPES).create()).create().readGraph(is,
readG);
+is.close();
+
+assertEquals(approximateGraphsCheck(g, readG), true);
+}
+
+/**
+ * Checks sequentially vertices and egdes of both graphs. Will check
sequentially Vertex IDs, Vertex Properties IDs
+ * and values and classes. Then same for edges. To use when
serializing a Graph and deserializing the supposedly
+ * same Graph.
+ */
+private boolean approximateGraphsCheck(Graph g1, Graph g2) {
--- End diff --
https://gist.github.com/newkek/643c907e54e93ab36594295281c3e4e6
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/351 Re-conflicts with Changelog against master.. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/351 @spmallette - I've written all the docs, please don't hesitate to correct them if they're not written well. - Updated the tests, parameterized them as much as possible, and focused the 2.0 tests on the 2.0 specific functionalities. - Rebased on current `master`. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on a diff in the pull request:
https://github.com/apache/tinkerpop/pull/351#discussion_r68951292
--- Diff:
gremlin-driver/src/test/java/org/apache/tinkerpop/gremlin/driver/ser/GraphSONMessageSerializerV2d0Test.java
---
@@ -0,0 +1,474 @@
+/*
+ * 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.
+ */
+package org.apache.tinkerpop.gremlin.driver.ser;
+
+import org.apache.tinkerpop.gremlin.driver.message.RequestMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseMessage;
+import org.apache.tinkerpop.gremlin.driver.message.ResponseStatusCode;
+import
org.apache.tinkerpop.gremlin.process.traversal.dsl.graph.GraphTraversalSource;
+import org.apache.tinkerpop.gremlin.process.traversal.step.util.Tree;
+import org.apache.tinkerpop.gremlin.structure.*;
+import org.apache.tinkerpop.gremlin.structure.io.AbstractIoRegistry;
+import org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONIo;
+import org.apache.tinkerpop.gremlin.structure.io.graphson.GraphSONTokens;
+import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerFactory;
+import org.apache.tinkerpop.gremlin.tinkergraph.structure.TinkerGraph;
+import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
+import org.apache.tinkerpop.shaded.jackson.core.JsonGenerationException;
+import org.apache.tinkerpop.shaded.jackson.core.JsonGenerator;
+import org.apache.tinkerpop.shaded.jackson.databind.JsonNode;
+import org.apache.tinkerpop.shaded.jackson.databind.ObjectMapper;
+import org.apache.tinkerpop.shaded.jackson.databind.SerializerProvider;
+import org.apache.tinkerpop.shaded.jackson.databind.module.SimpleModule;
+import org.apache.tinkerpop.shaded.jackson.databind.node.NullNode;
+import org.apache.tinkerpop.shaded.jackson.databind.ser.std.StdSerializer;
+import org.apache.tinkerpop.shaded.jackson.databind.util.StdDateFormat;
+import org.junit.Test;
+
+import java.awt.*;
+import java.io.IOException;
+import java.util.*;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.*;
+
+/**
+ * These tests focus on message serialization and not "result"
serialization as test specific to results (e.g.
+ * vertices, edges, annotated values, etc.) are handled in the IO packages.
+ *
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+public class GraphSONMessageSerializerV2d0Test {
--- End diff --
I could change some of the tests to take into account the version otherwise
---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at infrastruct...@apache.org or file a JIRA ticket
with INFRA.
---
[GitHub] tinkerpop issue #351: TINKERPOP-1274: GraphSON 2.0.
Github user newkek commented on the issue: https://github.com/apache/tinkerpop/pull/351 > tests are failing Ah I changed something quickly this morning and did not run again those tests. Will correct that. Ok for the docs, will do. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] tinkerpop pull request #351: TINKERPOP-1274: GraphSON 2.0.
GitHub user newkek opened a pull request:
https://github.com/apache/tinkerpop/pull/351
TINKERPOP-1274: GraphSON 2.0.
https://issues.apache.org/jira/browse/TINKERPOP-1274
# Summary of the changes :
Implementation of a format for value types serialization which is uniform
and not Java centric. As a reminder the new format is as follows :
- A value not typed : `value`
- A value typed : `[{"@class":"typeName"}, value]`
The content of `value` can be either a simple value, or a more complex
structure. The type prefix allows to call the right _Deserializer_ to
deserialize the `value` whatever its content is.
The default's GraphSON's 2.0 format will include types for every `value`
that is not a JSON native type
(`String/int/double/null/boolean/Map/Collection`) - called `PARTIAL_TYPES`.
This allows to significantly reduce the size of the JSON payload where those
aren't needed by the Jackson library.
GraphSON serialization _without_ types is not affected. To enable it, need
to use `NO_TYPES`.
## Quick walkthrough for the review
There are new components involved in the ser/de process that are extended
from the Jackson library :
- _TypeIdResolver_ : performs the conversion of a object's `Class` ->
`typeID`, and from a `typeID` -> `Class`.
- _TypeResolverBuilder_ : creates a _TypeIdResolver_ and instantiates a
_TypeDeserializer_ or _TypeSerializer_ with the _TypeIdResolver_ as a param.
- _TypeSerializer_ : writes the prefix and suffix for a typeID. The
_TypeSerializer_ is provided to the _Serializers_ and handles the type
serialization that respects the format put in place.
- _TypeDeserializer_ : is called before the deserializers are called. Its
role is to detect a type, and call the right _Deserializer_ to deserialize the
value.
Most of the serializers already existing for Graph had to be modified,
because most of them were *manually hardcoding* types without calling the
_TypeSerializer_ and hence, those were not respecting the format. Now all
Serializers respect the format because they call the _TypeSerializer_ given in
parameter. I followed the frame put in place by @spmallette to implement those
new serializers (prefixed with `V2d0`) without breaking existing clients code.
In _TypeDeserializer_, `baseType` represents the class given in parameter
by the user for the ser/de. If the user calls
`mapper.readValueAsString(jsonString, UUID.class)` the `baseType` in
_TypeDeserializer_ will be a _JavaType_ (which is the Jackson's custom class
for Java classes) that represents a _UUID_ class. The *wildcard* in our
deserialization mechanism is `Object.class`.
The deserialization path is as follow :
- _TypeDeserializer_ is called only for non simple JSON values (non
`String/int/double/null/boolean`).
- When called, if a type is detected, the _TypeDeserializer_ will read the
typeID, convert it to a JavaType (thanks to the _TypeIdResolver_), check that
the `baseType` is the same than what was read in the payload (only if
`baseType` is not the *wildcard*), and call the `deserialize()` method of the
_JsonDeserializer_ registered for that type.
- If a type is not detected, detect Maps or Arrays and call appropriate
Deserializer.
## Some results
GraphSON 2.0 shows a significant reduction of the payload's size for typed
serialization. And the consequence in performance is that since there's less to
process, the ser/de is faster. Results show a reduction of at least 50% in the
payload's size and evolving linearly (the bigger the payload the bigger the
difference) :
```
â ls -lh
tinkergraph-gremlin/target/test-case-data/TinkerGraphTest/tinkerpop-io/
-rw-r--r-- 1 kevingallardo staff 890K 28 Jun 21:50
grateful-dead-V2d0-typed.json
-rw-r--r-- 1 kevingallardo staff 1.9M 28 Jun 21:50
grateful-dead-typed.json
-rw-r--r-- 1 kevingallardo staff 851K 28 Jun 21:50 grateful-dead.json
-rw-r--r-- 1 kevingallardo staff 1.5K 28 Jun 21:50
tinkerpop-classic-V2d0-typed.json
-rw-r--r-- 1 kevingallardo staff 3.6K 28 Jun 21:50
tinkerpop-classic-typed.json
-rw-r--r-- 1 kevingallardo staff 1.3K 28 Jun 21:50
tinkerpop-classic.json
```
## Tests
Tests cover the same functionalities covered by GraphSON 1.0 typed, plus
additional features brought by GraphSON 2.0.
## Tradeoffs
- Some tricks were implemented in order to provide the types without
packages names. Since Java does not provide a way to search a class by its
simple name, the `TypeIdResolver` has to have an index that it can refer to,
and that has been correctly filled. The _TinkerpopJackosnModule_ class will
handle providing those new indexes to the _TypeIdResolver_ when custom
_Deserializers_ are added, but _without_ doing that we have no way to pr
