spmallette commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r727955226
##########
File path:
gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
##########
@@ -138,9 +143,11 @@ public void readGraph(final InputStream graphInputStream,
final Graph graphToWri
case GraphMLTokens.DATA:
final String key = reader.getAttributeValue(null,
GraphMLTokens.KEY);
final String dataAttributeName = keyIdMap.get(key);
+ final String defaultValue = defaultValues.get(key);
if (dataAttributeName != null) {
- final String value = reader.getElementText();
+ String elementValue = reader.getElementText();
+ final String value = elementValue.length() ==
0 && defaultValue.length() != 0 ? defaultValue : elementValue;
Review comment:
> This does not look right to me, key should be d_n.
sorry - i guess i'm used to making id/attr.name/key the same. didn't mean to
be confusing in my comment. My point was more that `<data key="d_n"/>` should
not trigger use of the default. It should simply be empty string.
The case for default usage is when the there is no `<data>` element with
`key="d_n"` at all. In that case it should inject the property
"modification=add". You mention that this situation currently leads to:
> there is no default value therefore related events won't be emitted at
all....My implementation will generate NPE in such cases
I think the goal here is to determine if the property key is present or not
in the GraphML for the particular node/edge and then adding it if its not. In
that sense, I think that the bulk of your changes for this should neatly fit
into `XMLEvent.END_ELEMENT` where all the property keys have already been
collected and you just before the vertex is created. Does that all make sense?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
