spmallette commented on a change in pull request #1485:
URL: https://github.com/apache/tinkerpop/pull/1485#discussion_r726101651
##########
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:
I don't recall how `getElementText()` works. If you have `<data
key="d_n"/>` does that return an empty string or `null`? If it's the former,
which your code seems to indicate, doesn't this mean that GraphML can't have
empty strings as values anymore (which may be what the user wants)? I think the
point of the default specification is to deal with a situation like:
```xml
<key id="d_n" for="node" attr.name="modification" attr.type="string">
<default>add</default>
</key>
...
<node id="6315"></node>
```
where the "modification" is not present at all for the `<node><data>` in
which case it would include the default property of "modification" with an
"add" value. otoh, if it were the following:
```xml
<key id="d_n" for="node" attr.name="modification" attr.type="string">
<default>add</default>
</key>
...
<node id="6315">
<data key="modification"/>
</node>
```
it would just include the "modification" property with an empty string. Does
that 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]
