Fixed an NPE in GraphMLReader where if the <edge> doesn't have an ID field and 
the base graph supports ids, then an NPE happens. Added a test case to verifiy 
working behavior. Super simple fix. CTR.


Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/61a3d125
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/61a3d125
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/61a3d125

Branch: refs/heads/TINKERPOP-1642
Commit: 61a3d125c48869c8e5fdaf195abbb46fbaaf0c30
Parents: 9ea319d
Author: Marko A. Rodriguez <okramma...@gmail.com>
Authored: Thu Mar 9 14:49:36 2017 -0700
Committer: Marko A. Rodriguez <okramma...@gmail.com>
Committed: Thu Mar 9 14:49:36 2017 -0700

----------------------------------------------------------------------
 .../structure/io/graphml/GraphMLReader.java     | 19 +++++++------
 .../tinkerpop/gremlin/structure/io/IoTest.java  | 25 ++++++++++++++--
 .../structure/io/graphml/graph-no-edge-ids.xml  | 30 ++++++++++++++++++++
 3 files changed, 62 insertions(+), 12 deletions(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/61a3d125/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
----------------------------------------------------------------------
diff --git 
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
 
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
index e161ba7..3289166 100644
--- 
a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
+++ 
b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLReader.java
@@ -19,10 +19,10 @@
 package org.apache.tinkerpop.gremlin.structure.io.graphml;
 
 import org.apache.tinkerpop.gremlin.structure.Direction;
-import org.apache.tinkerpop.gremlin.structure.Property;
-import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.structure.Edge;
 import org.apache.tinkerpop.gremlin.structure.Graph;
+import org.apache.tinkerpop.gremlin.structure.Property;
+import org.apache.tinkerpop.gremlin.structure.T;
 import org.apache.tinkerpop.gremlin.structure.Vertex;
 import org.apache.tinkerpop.gremlin.structure.VertexProperty;
 import org.apache.tinkerpop.gremlin.structure.io.GraphReader;
@@ -72,7 +72,7 @@ public final class GraphMLReader implements GraphReader {
 
     @Override
     public void readGraph(final InputStream graphInputStream, final Graph 
graphToWriteTo) throws IOException {
-        final Map<Object,Vertex> cache = new HashMap<>();
+        final Map<Object, Vertex> cache = new HashMap<>();
         final AtomicLong counter = new AtomicLong(0);
         final boolean supportsTx = 
graphToWriteTo.features().graph().supportsTransactions();
         final Graph.Features.EdgeFeatures edgeFeatures = 
graphToWriteTo.features().edge();
@@ -186,9 +186,9 @@ public final class GraphMLReader implements GraphReader {
                         isInVertex = false;
                     } else if (elementName.equals(GraphMLTokens.EDGE)) {
                         final Object[] propsAsArray = 
edgeProps.entrySet().stream().flatMap(e -> Stream.of(e.getKey(), 
e.getValue())).toArray();
-                        final Object[] propsReady = 
edgeFeatures.willAllowId(edgeId) ? ElementHelper.upsert(propsAsArray, T.id, 
edgeId) : propsAsArray;
-                        
-                       edgeOutVertex.addEdge(null == edgeLabel ? 
Edge.DEFAULT_LABEL : edgeLabel, edgeInVertex, propsReady);
+                        final Object[] propsReady = null != edgeId && 
edgeFeatures.willAllowId(edgeId) ? ElementHelper.upsert(propsAsArray, T.id, 
edgeId) : propsAsArray;
+
+                        edgeOutVertex.addEdge(null == edgeLabel ? 
Edge.DEFAULT_LABEL : edgeLabel, edgeInVertex, propsReady);
 
                         if (supportsTx && counter.incrementAndGet() % 
batchSize == 0)
                             graphToWriteTo.tx().commit();
@@ -292,7 +292,7 @@ public final class GraphMLReader implements GraphReader {
 
     private static Vertex findOrCreate(final Object id, final Graph 
graphToWriteTo,
                                        final Graph.Features.VertexFeatures 
features,
-                                       final Map<Object,Vertex> cache, final 
boolean asVertex, final Object... args) {
+                                       final Map<Object, Vertex> cache, final 
boolean asVertex, final Object... args) {
         if (cache.containsKey(id)) {
             // if the request to findOrCreate come from a vertex then AND the 
vertex was already created, that means
             // that the vertex was created by an edge that arrived first in 
the stream (allowable via GraphML
@@ -306,7 +306,7 @@ public final class GraphMLReader implements GraphReader {
                 return cache.get(id);
             }
         } else {
-            final Object [] argsReady = features.willAllowId(id) ? 
ElementHelper.upsert(args, T.id, id) : args;
+            final Object[] argsReady = features.willAllowId(id) ? 
ElementHelper.upsert(args, T.id, id) : args;
             final Vertex v = graphToWriteTo.addVertex(argsReady);
             cache.put(id, v);
             return v;
@@ -344,7 +344,8 @@ public final class GraphMLReader implements GraphReader {
         private boolean strict = true;
         private long batchSize = 10000;
 
-        private Builder() { }
+        private Builder() {
+        }
 
         /**
          * When set to true, exceptions will be thrown if a property value 
cannot be coerced to the expected data

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/61a3d125/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java
----------------------------------------------------------------------
diff --git 
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java
 
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java
index 17ffed2..a523b15 100644
--- 
a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java
+++ 
b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/io/IoTest.java
@@ -79,7 +79,12 @@ import java.util.UUID;
 import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.ElementFeatures.FEATURE_ANY_IDS;
 import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VariableFeatures.FEATURE_VARIABLES;
 import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexFeatures.FEATURE_USER_SUPPLIED_IDS;
-import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.*;
+import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_BOOLEAN_VALUES;
+import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_DOUBLE_VALUES;
+import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_FLOAT_VALUES;
+import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_INTEGER_VALUES;
+import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_LONG_VALUES;
+import static 
org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_STRING_VALUES;
 import static org.apache.tinkerpop.gremlin.structure.io.IoCore.graphson;
 import static org.hamcrest.core.Is.is;
 import static org.junit.Assert.assertEquals;
@@ -183,6 +188,19 @@ public class IoTest {
             assertEquals("junk", v.<String>value("n"));
         }
 
+        @Test
+        @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, 
feature = Graph.Features.EdgeFeatures.FEATURE_ADD_EDGES)
+        @FeatureRequirement(featureClass = 
Graph.Features.VertexFeatures.class, feature = 
Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)
+        public void shouldReadGraphMLWithoutEdgeIds() throws IOException {
+            final GraphReader reader = 
GraphMLReader.build().strict(false).create();
+            try (final InputStream stream = 
IoTest.class.getResourceAsStream(TestHelper.convertPackageToResourcePath(GraphMLResourceAccess.class)
 + "graph-no-edge-ids.xml")) {
+                reader.readGraph(stream, graph);
+            }
+            assertEquals(1, IteratorUtils.count(graph.edges()));
+            assertEquals(2, IteratorUtils.count(graph.vertices()));
+        }
+
+
         @Test(expected = NumberFormatException.class)
         @FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, 
feature = Graph.Features.EdgeFeatures.FEATURE_ADD_EDGES)
         @FeatureRequirement(featureClass = 
Graph.Features.VertexFeatures.class, feature = 
Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)
@@ -318,7 +336,8 @@ public class IoTest {
             v1.addEdge("SELFLOOP", v1);
 
             final Configuration targetConf = 
graphProvider.newGraphConfiguration("target", this.getClass(), 
name.getMethodName(), null);
-            final Graph target = graphProvider.openTestGraph(targetConf);;
+            final Graph target = graphProvider.openTestGraph(targetConf);
+            ;
             try (ByteArrayOutputStream os = new ByteArrayOutputStream()) {
                 source.io(IoCore.gryo()).writer().create().writeGraph(os, 
source);
                 try (ByteArrayInputStream is = new 
ByteArrayInputStream(os.toByteArray())) {
@@ -764,7 +783,7 @@ public class IoTest {
         final List<Edge> v1Edges = 
IteratorUtils.list(v1.edges(Direction.BOTH));
         assertEquals(1, v1Edges.size());
         v1Edges.forEach(e -> {
-               if (e.inVertex().value("name").equals("vadas")) {
+            if (e.inVertex().value("name").equals("vadas")) {
                 assertEquals(Edge.DEFAULT_LABEL, e.label());
                 if (assertDouble)
                     assertWeightLoosely(0.5d, e);

http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/61a3d125/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/graph-no-edge-ids.xml
----------------------------------------------------------------------
diff --git 
a/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/graph-no-edge-ids.xml
 
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/graph-no-edge-ids.xml
new file mode 100644
index 0000000..fe8f820
--- /dev/null
+++ 
b/gremlin-test/src/main/resources/org/apache/tinkerpop/gremlin/structure/io/graphml/graph-no-edge-ids.xml
@@ -0,0 +1,30 @@
+<!--
+  ~  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.
+  -->
+<graphml xmlns="http://graphml.graphdrawing.org/xmlns";
+         xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance";
+         xsi:schemaLocation="http://graphml.graphdrawing.org/xmlns
+        http://graphml.graphdrawing.org/xmlns/1.0/graphml.xsd";>
+    <graph id="G" edgedefault="directed">
+        <node id="1">
+        </node>
+        <node id="2">
+        </node>
+        <edge source="1" target="2"></edge>
+    </graph>
+</graphml>

Reply via email to