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>