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<Object>> elementProperties = normalize
?
+ final Iterator<Property<Object>> 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 [email protected] or file a JIRA ticket
with INFRA.
---
