You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by sp...@apache.org on 2019/06/17 20:30:31 UTC
[tinkerpop] 01/01: TINKERPOP-2238 Fixed iterator leaks for
GraphMLWriter
This is an automated email from the ASF dual-hosted git repository.
spmallette pushed a commit to branch TINKERPOP-2238
in repository https://gitbox.apache.org/repos/asf/tinkerpop.git
commit c13e8fdd76cd1b0966f210f71305cf85c0cd94d9
Author: Stephen Mallette <sp...@genoprime.com>
AuthorDate: Mon Jun 17 16:29:55 2019 -0400
TINKERPOP-2238 Fixed iterator leaks for GraphMLWriter
---
.../structure/io/graphml/GraphMLWriter.java | 169 +++++++++++----------
.../tinkerpop/gremlin/structure/io/IoTest.java | 2 -
.../tinkergraph/structure/TinkerGraphIterator.java | 12 +-
3 files changed, 94 insertions(+), 89 deletions(-)
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
index 21fe416..6d31cc8 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/io/graphml/GraphMLWriter.java
@@ -27,6 +27,7 @@ import org.apache.tinkerpop.gremlin.structure.Vertex;
import org.apache.tinkerpop.gremlin.structure.VertexProperty;
import org.apache.tinkerpop.gremlin.structure.io.GraphWriter;
import org.apache.tinkerpop.gremlin.structure.io.Io;
+import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator;
import org.apache.tinkerpop.gremlin.structure.util.Comparators;
import org.apache.tinkerpop.gremlin.util.iterator.IteratorUtils;
@@ -218,7 +219,7 @@ public final class GraphMLWriter implements GraphWriter {
final Map<String, String> identifiedEdgeKeyTypes,
final XMLStreamWriter writer) throws XMLStreamException {
// <key id="weight" for="edge" attr.name="weight" attr.type="float"/>
- final Collection<String> vertexKeySet = getVertexKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
+ final Collection<String> vertexKeySet = getKeysAndNormalizeIfRequired(identifiedVertexKeyTypes);
for (String key : vertexKeySet) {
writer.writeStartElement(GraphMLTokens.KEY);
writer.writeAttribute(GraphMLTokens.ID, key);
@@ -228,7 +229,7 @@ public final class GraphMLWriter implements GraphWriter {
writer.writeEndElement();
}
- final Collection<String> edgeKeySet = getEdgeKeysAndNormalizeIfRequired(identifiedEdgeKeyTypes);
+ final Collection<String> edgeKeySet = getKeysAndNormalizeIfRequired(identifiedEdgeKeyTypes);
for (String key : edgeKeySet) {
writer.writeStartElement(GraphMLTokens.KEY);
writer.writeAttribute(GraphMLTokens.ID, key);
@@ -240,58 +241,62 @@ public final class GraphMLWriter implements GraphWriter {
}
private void writeEdges(final XMLStreamWriter writer, final Graph graph) throws XMLStreamException {
- if (normalize) {
- final List<Edge> edges = IteratorUtils.list(graph.edges());
- Collections.sort(edges, Comparators.ELEMENT_COMPARATOR);
-
- for (Edge edge : edges) {
- writer.writeStartElement(GraphMLTokens.EDGE);
- writer.writeAttribute(GraphMLTokens.ID, edge.id().toString());
- writer.writeAttribute(GraphMLTokens.SOURCE, edge.outVertex().id().toString());
- writer.writeAttribute(GraphMLTokens.TARGET, edge.inVertex().id().toString());
-
- writer.writeStartElement(GraphMLTokens.DATA);
- writer.writeAttribute(GraphMLTokens.KEY, this.edgeLabelKey);
- writer.writeCharacters(edge.label());
- writer.writeEndElement();
+ final Iterator<Edge> iterator = graph.edges();
+ try {
+ if (normalize) {
+ final List<Edge> edges = IteratorUtils.list(iterator);
+ Collections.sort(edges, Comparators.ELEMENT_COMPARATOR);
- final List<String> keys = new ArrayList<>(edge.keys());
- Collections.sort(keys);
+ for (Edge edge : edges) {
+ writer.writeStartElement(GraphMLTokens.EDGE);
+ writer.writeAttribute(GraphMLTokens.ID, edge.id().toString());
+ writer.writeAttribute(GraphMLTokens.SOURCE, edge.outVertex().id().toString());
+ writer.writeAttribute(GraphMLTokens.TARGET, edge.inVertex().id().toString());
- for (String key : keys) {
writer.writeStartElement(GraphMLTokens.DATA);
- writer.writeAttribute(GraphMLTokens.KEY, key);
- // technically there can't be a null here as gremlin structure forbids that occurrence even if Graph
- // implementations support it, but out to empty string just in case.
- writer.writeCharacters(edge.property(key).orElse("").toString());
+ writer.writeAttribute(GraphMLTokens.KEY, this.edgeLabelKey);
+ writer.writeCharacters(edge.label());
writer.writeEndElement();
- }
- writer.writeEndElement();
- }
- } else {
- final Iterator<Edge> iterator = graph.edges();
- while (iterator.hasNext()) {
- final Edge edge = iterator.next();
- writer.writeStartElement(GraphMLTokens.EDGE);
- writer.writeAttribute(GraphMLTokens.ID, edge.id().toString());
- writer.writeAttribute(GraphMLTokens.SOURCE, edge.outVertex().id().toString());
- writer.writeAttribute(GraphMLTokens.TARGET, edge.inVertex().id().toString());
- writer.writeStartElement(GraphMLTokens.DATA);
- writer.writeAttribute(GraphMLTokens.KEY, this.edgeLabelKey);
- writer.writeCharacters(edge.label());
- writer.writeEndElement();
+ final List<String> keys = new ArrayList<>(edge.keys());
+ Collections.sort(keys);
+
+ for (String key : keys) {
+ writer.writeStartElement(GraphMLTokens.DATA);
+ writer.writeAttribute(GraphMLTokens.KEY, key);
+ // technically there can't be a null here as gremlin structure forbids that occurrence even if Graph
+ // implementations support it, but out to empty string just in case.
+ writer.writeCharacters(edge.property(key).orElse("").toString());
+ writer.writeEndElement();
+ }
+ writer.writeEndElement();
+ }
+ } else {
+ while (iterator.hasNext()) {
+ final Edge edge = iterator.next();
+ writer.writeStartElement(GraphMLTokens.EDGE);
+ writer.writeAttribute(GraphMLTokens.ID, edge.id().toString());
+ writer.writeAttribute(GraphMLTokens.SOURCE, edge.outVertex().id().toString());
+ writer.writeAttribute(GraphMLTokens.TARGET, edge.inVertex().id().toString());
- for (String key : edge.keys()) {
writer.writeStartElement(GraphMLTokens.DATA);
- writer.writeAttribute(GraphMLTokens.KEY, key);
- // technically there can't be a null here as gremlin structure forbids that occurrence even if Graph
- // implementations support it, but out to empty string just in case.
- writer.writeCharacters(edge.property(key).orElse("").toString());
+ writer.writeAttribute(GraphMLTokens.KEY, this.edgeLabelKey);
+ writer.writeCharacters(edge.label());
+ writer.writeEndElement();
+
+ for (String key : edge.keys()) {
+ writer.writeStartElement(GraphMLTokens.DATA);
+ writer.writeAttribute(GraphMLTokens.KEY, key);
+ // technically there can't be a null here as gremlin structure forbids that occurrence even if Graph
+ // implementations support it, but out to empty string just in case.
+ writer.writeCharacters(edge.property(key).orElse("").toString());
+ writer.writeEndElement();
+ }
writer.writeEndElement();
}
- writer.writeEndElement();
}
+ } finally {
+ CloseableIterator.closeIterator(iterator);
}
}
@@ -334,40 +339,34 @@ public final class GraphMLWriter implements GraphWriter {
}
private Iterable<Vertex> getVerticesAndNormalizeIfRequired(final Graph graph) {
- final Iterable<Vertex> vertices;
- if (normalize) {
- vertices = new ArrayList<>();
- final Iterator<Vertex> vertexIterator = graph.vertices();
- while (vertexIterator.hasNext()) {
- ((Collection<Vertex>) vertices).add(vertexIterator.next());
- }
- Collections.sort((List<Vertex>) vertices, Comparators.ELEMENT_COMPARATOR);
- } else
- vertices = IteratorUtils.list(graph.vertices());
-
- return vertices;
- }
+ final Iterator<Vertex> iterator = graph.vertices();
- private Collection<String> getEdgeKeysAndNormalizeIfRequired(final Map<String, String> identifiedEdgeKeyTypes) {
- final Collection<String> edgeKeySet;
- if (normalize) {
- edgeKeySet = new ArrayList<>();
- edgeKeySet.addAll(identifiedEdgeKeyTypes.keySet());
- Collections.sort((List<String>) edgeKeySet);
- } else
- edgeKeySet = identifiedEdgeKeyTypes.keySet();
+ try {
+ final Iterable<Vertex> vertices;
+ if (normalize) {
+ vertices = new ArrayList<>();
+ while (iterator.hasNext()) {
+ ((Collection<Vertex>) vertices).add(iterator.next());
+ }
+ Collections.sort((List<Vertex>) vertices, Comparators.ELEMENT_COMPARATOR);
+ } else {
+ vertices = IteratorUtils.list(iterator);
+ }
- return edgeKeySet;
+ return vertices;
+ } finally {
+ CloseableIterator.closeIterator(iterator);
+ }
}
- private Collection<String> getVertexKeysAndNormalizeIfRequired(final Map<String, String> identifiedVertexKeyTypes) {
+ private Collection<String> getKeysAndNormalizeIfRequired(final Map<String, String> identifiedKeyTypes) {
final Collection<String> keyset;
if (normalize) {
keyset = new ArrayList<>();
- keyset.addAll(identifiedVertexKeyTypes.keySet());
+ keyset.addAll(identifiedKeyTypes.keySet());
Collections.sort((List<String>) keyset);
} else
- keyset = identifiedVertexKeyTypes.keySet();
+ keyset = identifiedKeyTypes.keySet();
return keyset;
}
@@ -386,15 +385,19 @@ public final class GraphMLWriter implements GraphWriter {
private static Map<String, String> determineVertexTypes(final Graph graph) {
final Map<String, String> vertexKeyTypes = new HashMap<>();
final Iterator<Vertex> vertices = graph.vertices();
- while (vertices.hasNext()) {
- final Vertex vertex = vertices.next();
- for (String key : vertex.keys()) {
- if (!vertexKeyTypes.containsKey(key)) {
- final VertexProperty<Object> currentValue = getCheckedVertexProperty(vertex, key);
-
- vertexKeyTypes.put(key, GraphMLWriter.getStringType(currentValue.value()));
+ try {
+ while (vertices.hasNext()) {
+ final Vertex vertex = vertices.next();
+ for (String key : vertex.keys()) {
+ if (!vertexKeyTypes.containsKey(key)) {
+ final VertexProperty<Object> currentValue = getCheckedVertexProperty(vertex, key);
+
+ vertexKeyTypes.put(key, GraphMLWriter.getStringType(currentValue.value()));
+ }
}
}
+ } finally {
+ CloseableIterator.closeIterator(vertices);
}
return vertexKeyTypes;
@@ -412,12 +415,16 @@ public final class GraphMLWriter implements GraphWriter {
private static Map<String, String> determineEdgeTypes(final Graph graph) {
final Map<String, String> edgeKeyTypes = new HashMap<>();
final Iterator<Edge> edges = graph.edges();
- while (edges.hasNext()) {
- final Edge edge = edges.next();
- for (String key : edge.keys()) {
- if (!edgeKeyTypes.containsKey(key))
- edgeKeyTypes.put(key, GraphMLWriter.getStringType(edge.property(key).value()));
+ try {
+ while (edges.hasNext()) {
+ final Edge edge = edges.next();
+ for (String key : edge.keys()) {
+ if (!edgeKeyTypes.containsKey(key))
+ edgeKeyTypes.put(key, GraphMLWriter.getStringType(edge.property(key).value()));
+ }
}
+ } finally {
+ CloseableIterator.closeIterator(edges);
}
return edgeKeyTypes;
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 55bc362..2303e79 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
@@ -103,11 +103,9 @@ import static org.junit.Assume.assumeThat;
* @author Marko A. Rodriguez (http://markorodriguez.com)
*/
@RunWith(Enclosed.class)
-@IgnoreIteratorLeak
public class IoTest {
private static final Logger logger = LoggerFactory.getLogger(IoTest.class);
- @IgnoreIteratorLeak
public static class GraphMLTest extends AbstractGremlinTest {
@Test
@FeatureRequirement(featureClass = Graph.Features.EdgeFeatures.class, feature = Graph.Features.EdgeFeatures.FEATURE_ADD_EDGES)
diff --git a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphIterator.java b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphIterator.java
index cb796cc..4d6cc5d 100644
--- a/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphIterator.java
+++ b/tinkergraph-gremlin/src/main/java/org/apache/tinkerpop/gremlin/tinkergraph/structure/TinkerGraphIterator.java
@@ -42,6 +42,12 @@ public class TinkerGraphIterator<E> implements Iterator<E>, AutoCloseable {
*/
private boolean finished;
+ public TinkerGraphIterator(final Iterator<E> orig) {
+ this.orig = orig;
+ StoreIteratorCounter.INSTANCE.increment();
+ finished = false;
+ }
+
@Override
public boolean hasNext() {
if (next != null) return true;
@@ -77,12 +83,6 @@ public class TinkerGraphIterator<E> implements Iterator<E>, AutoCloseable {
}
}
- public TinkerGraphIterator(final Iterator<E> orig) {
- this.orig = orig;
- StoreIteratorCounter.INSTANCE.increment();
- finished = false;
- }
-
@Override
public void close() {
if (!finished) {