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 2017/01/05 11:09:24 UTC
tinkerpop git commit: TINKERPOP-1589 Re-introduced CloseableIterator
Repository: tinkerpop
Updated Branches:
refs/heads/TINKERPOP-1589 [created] 3597dc5f0
TINKERPOP-1589 Re-introduced CloseableIterator
Made it so that the CloseableIterator is closed by GraphStep if it is provided by the iterator supplier. Furthermore, any steps in a Traversal implement AutoCloseable then its close() method will be called.
Project: http://git-wip-us.apache.org/repos/asf/tinkerpop/repo
Commit: http://git-wip-us.apache.org/repos/asf/tinkerpop/commit/3597dc5f
Tree: http://git-wip-us.apache.org/repos/asf/tinkerpop/tree/3597dc5f
Diff: http://git-wip-us.apache.org/repos/asf/tinkerpop/diff/3597dc5f
Branch: refs/heads/TINKERPOP-1589
Commit: 3597dc5f06407a8b1ada0e49a52b36b2f31b8a34
Parents: 3064b93
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Wed Jan 4 16:14:23 2017 -0500
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Wed Jan 4 16:14:23 2017 -0500
----------------------------------------------------------------------
CHANGELOG.asciidoc | 1 +
.../upgrade/release-3.2.x-incubating.asciidoc | 18 +++++
.../gremlin/process/traversal/Traversal.java | 7 +-
.../process/traversal/step/map/GraphStep.java | 18 ++++-
.../structure/util/CloseableIterator.java | 49 +++++++++++
.../util/DefaultCloseableIterator.java | 45 +++++++++++
.../process/traversal/TraversalTest.java | 44 +++++++++-
.../structure/util/CloseableIteratorTest.java | 85 ++++++++++++++++++++
8 files changed, 260 insertions(+), 7 deletions(-)
----------------------------------------------------------------------
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 34e81c4..7316d80 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,7 @@ image::https://raw.githubusercontent.com/apache/tinkerpop/master/docs/static/ima
TinkerPop 3.2.4 (Release Date: NOT OFFICIALLY RELEASED YET)
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
+* Added `CloseableIterator` to allow `Graph` providers who open expensive resources a way to let users release them.
* Fixed minor bug in `gremlin-driver` where closing a session-based `Client` without initializing it could generate an error.
* `TinkerGraph` Gryo and GraphSON deserialization is now configured to use multi-properties.
* Changed behavior of `ElementHelper.areEqual(Property, Property)` to not throw exceptions with `null` arguments.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/docs/src/upgrade/release-3.2.x-incubating.asciidoc
----------------------------------------------------------------------
diff --git a/docs/src/upgrade/release-3.2.x-incubating.asciidoc b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
index 6dcb0a3..8d73baed 100644
--- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
@@ -147,6 +147,24 @@ Upgrading for Providers
Graph Database Providers
^^^^^^^^^^^^^^^^^^^^^^^^
+CloseableIterator
++++++++++++++++++
+
+Prior to TinkerPop 3.x, Blueprints had the notion of a `CloseableIterable` which exposed a way for Graph Providers
+to offer a way to release resources that might have been opened when returning vertices and edges. That interface was
+never exposed in TinkerPop 3.x, but has now been made available via the new `CloseableIterator`. Providers may choose
+to use this interface or not when returning values from `Graph.vertices()` and `Graph.edges()`.
+
+It will be up to users to know whether or not they need to call `close()`. Of course, users should typically not be
+operating with the Graph Structure API, so it's unlikely that they would be calling these methods directly in the
+first place. It is more likely that users will be calling `Traversal.close()`. This method will essentially iterate
+the steps of the `Traversal` and simply call `close()` on any steps that implement `AutoCloseable`. By default,
+`GraphStep` now implements `AutoCloseable` which most Graph Providers will extend upon (as was done with TinkerGraph's
+`TinkerGraphStep`), so the integration should largely come for free if the provider simply returns a
+`CloseableIterator` from `Graph.vertices()` and `Graph.edges()`.
+
+See: https://issues.apache.org/jira/browse/TINKERPOP-1589[TINKERPOP-1589]
+
HasContainer AndP Splitting
+++++++++++++++++++++++++++
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
index 04f5127..13a2b7d 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/Traversal.java
@@ -254,9 +254,14 @@ public interface Traversal<S, E> extends Iterator<E>, Serializable, Cloneable, A
}
}
+ /**
+ * Releases resources opened in any steps that implement {@link AutoCloseable}.
+ */
@Override
public default void close() throws Exception {
- // do nothing by default
+ for (AutoCloseable closeableStep : TraversalHelper.getStepsOfAssignableClassRecursively(AutoCloseable.class, asAdmin())) {
+ closeableStep.close();
+ }
}
/**
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
index 3f169b0..87935d8 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/process/traversal/step/map/GraphStep.java
@@ -32,9 +32,12 @@ import org.apache.tinkerpop.gremlin.structure.Edge;
import org.apache.tinkerpop.gremlin.structure.Element;
import org.apache.tinkerpop.gremlin.structure.T;
import org.apache.tinkerpop.gremlin.structure.Vertex;
+import org.apache.tinkerpop.gremlin.structure.util.CloseableIterator;
import org.apache.tinkerpop.gremlin.structure.util.StringFactory;
import org.apache.tinkerpop.gremlin.util.iterator.EmptyIterator;
+import java.io.Closeable;
+import java.io.IOException;
import java.util.Arrays;
import java.util.Collection;
import java.util.Collections;
@@ -45,7 +48,7 @@ import java.util.function.Supplier;
* @author Marko A. Rodriguez (http://markorodriguez.com)
* @author Pieter Martin
*/
-public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implements GraphComputing {
+public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implements GraphComputing, AutoCloseable {
protected final Class<E> returnClass;
protected Object[] ids;
@@ -151,7 +154,6 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
this.iterator = EmptyIterator.instance();
}
-
@Override
public int hashCode() {
int result = super.hashCode() ^ this.returnClass.hashCode();
@@ -162,6 +164,18 @@ public class GraphStep<S, E extends Element> extends AbstractStep<S, E> implemen
}
/**
+ * Attemps to close an underlying iterator if it is of type {@link CloseableIterator}. Graph providers may choose
+ * to return this interface containing their vertices and edges if there are expensive resources that might need to
+ * be released at some point.
+ */
+ @Override
+ public void close() throws Exception {
+ if (iterator != null && iterator instanceof CloseableIterator) {
+ ((CloseableIterator) iterator).close();
+ }
+ }
+
+ /**
* Helper method for providers that want to "fold in" {@link HasContainer}'s based on id checking into the ids of the {@link GraphStep}.
*
* @param graphStep the GraphStep to potentially {@link GraphStep#addIds(Object...)}.
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java
new file mode 100644
index 0000000..5f334a5
--- /dev/null
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIterator.java
@@ -0,0 +1,49 @@
+/*
+ * 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.
+ */
+package org.apache.tinkerpop.gremlin.structure.util;
+
+import org.apache.tinkerpop.gremlin.structure.Graph;
+
+import java.io.Closeable;
+import java.util.Iterator;
+
+/**
+ * An extension of {@code Iterator} that implements {@code Closeable} which allows a {@link Graph} implementation
+ * that hold open resources to provide the user the option to release those resources.
+ *
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+public interface CloseableIterator<T> extends Iterator<T>, Closeable {
+
+ /**
+ * Wraps an existing {@code Iterator} in a {@code CloseableIterator}. If the {@code Iterator} is already of that
+ * type then it will simply be returned as-is.
+ */
+ public static <T> CloseableIterator<T> asCloseable(final Iterator<T> iterator) {
+ if (iterator instanceof CloseableIterator)
+ return (CloseableIterator<T>) iterator;
+
+ return new DefaultCloseableIterator<T>(iterator);
+ }
+
+ @Override
+ public default void close() {
+ // do nothing by default
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java
new file mode 100644
index 0000000..7e4fb7c
--- /dev/null
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/DefaultCloseableIterator.java
@@ -0,0 +1,45 @@
+/*
+ * 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.
+ */
+package org.apache.tinkerpop.gremlin.structure.util;
+
+import java.util.Iterator;
+
+/**
+ * A default implementation of {@link CloseableIterator} that simply wraps an existing {@code Iterator}. This
+ * implementation has a "do nothing" implementation of {@link #close()}.
+ *
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+public class DefaultCloseableIterator<T> implements CloseableIterator<T> {
+ protected Iterator<T> iterator;
+
+ public DefaultCloseableIterator(final Iterator<T> iterator) {
+ this.iterator = iterator;
+ }
+
+ @Override
+ public boolean hasNext() {
+ return iterator.hasNext();
+ }
+
+ @Override
+ public T next() {
+ return iterator.next();
+ }
+}
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
index c427d8e..020ed72 100644
--- a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/process/traversal/TraversalTest.java
@@ -112,6 +112,39 @@ public class TraversalTest {
assertThat(t.hasNext(), is(false));
}
+ @Test
+ public void shouldCloseWithoutACloseableStep() throws Exception {
+ final MockTraversal<Integer> t = new MockTraversal<>(1, 2, 3, 4, 5, 6, 7);
+ assertThat(t.hasNext(), is(true));
+ t.close();
+ assertThat(t.isClosed(), is(false));
+ }
+
+ @Test
+ public void shouldCloseWithCloseableStep() throws Exception {
+ final MockTraversal<Integer> t = new MockTraversal<>(Arrays.asList(1, 2, 3, 4, 5, 6, 7).iterator(), true);
+ assertThat(t.hasNext(), is(true));
+ t.close();
+ assertThat(t.isClosed(), is(true));
+ }
+
+ private static class MockCloseStep<E> extends MockStep<E> implements AutoCloseable {
+ private boolean closed = false;
+
+ public MockCloseStep(final Iterator<E> itty) {
+ super(itty);
+ }
+
+ boolean isClosed() {
+ return closed;
+ }
+
+ @Override
+ public void close() throws Exception {
+ closed = true;
+ }
+ }
+
private static class MockStep<E> implements Step<E,E> {
private final Iterator<E> itty;
@@ -206,7 +239,6 @@ public class TraversalTest {
}
}
-
private static class MockTraversal<T> implements Traversal.Admin<T,T> {
private Iterator<T> itty;
@@ -220,15 +252,19 @@ public class TraversalTest {
}
MockTraversal(final List<T> list) {
- this(list.iterator());
+ this(list.iterator(), false);
}
- MockTraversal(final Iterator<T> itty) {
+ MockTraversal(final Iterator<T> itty, final boolean asCloseable) {
this.itty = itty;
- mockEndStep = new MockStep<>(itty);
+ mockEndStep = asCloseable ? new MockCloseStep<>(itty) : new MockStep<>(itty);
steps = Collections.singletonList(mockEndStep);
}
+ boolean isClosed() {
+ return mockEndStep instanceof MockCloseStep && ((MockCloseStep) mockEndStep).isClosed();
+ }
+
@Override
public Bytecode getBytecode() {
return null;
http://git-wip-us.apache.org/repos/asf/tinkerpop/blob/3597dc5f/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java
new file mode 100644
index 0000000..c30896e
--- /dev/null
+++ b/gremlin-core/src/test/java/org/apache/tinkerpop/gremlin/structure/util/CloseableIteratorTest.java
@@ -0,0 +1,85 @@
+/*
+ * 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.
+ */
+package org.apache.tinkerpop.gremlin.structure.util;
+
+import org.junit.Test;
+
+import java.util.Arrays;
+import java.util.List;
+
+import static org.hamcrest.MatcherAssert.assertThat;
+import static org.hamcrest.core.Is.is;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertSame;
+
+/**
+ * @author Stephen Mallette (http://stephen.genoprime.com)
+ */
+public class CloseableIteratorTest {
+ @Test
+ public void shouldWrapIterator() {
+ final List<String> stuff = Arrays.asList("this stuff", "that stuff", "other stuff");
+ final CloseableIterator<String> itty = new DefaultCloseableIterator<>(stuff.iterator());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("this stuff", itty.next());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("that stuff", itty.next());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("other stuff", itty.next());
+ assertThat(itty.hasNext(), is(false));
+
+ // this is a do-nothing, but we should still be able to call it
+ itty.close();
+ }
+
+ @Test
+ public void shouldWrapIteratorWithHelper() {
+ final List<String> stuff = Arrays.asList("this stuff", "that stuff", "other stuff");
+ final CloseableIterator<String> itty = CloseableIterator.asCloseable(stuff.iterator());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("this stuff", itty.next());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("that stuff", itty.next());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("other stuff", itty.next());
+ assertThat(itty.hasNext(), is(false));
+
+ // this is a do-nothing, but we should still be able to call it
+ itty.close();
+ }
+
+ @Test
+ public void shouldReturnSameInstance() {
+ final List<String> stuff = Arrays.asList("this stuff", "that stuff", "other stuff");
+ final CloseableIterator<String> itty = new DefaultCloseableIterator<>(stuff.iterator());
+ final CloseableIterator<String> same = CloseableIterator.asCloseable(itty);
+ assertSame(itty, same);
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("this stuff", itty.next());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("that stuff", itty.next());
+ assertThat(itty.hasNext(), is(true));
+ assertEquals("other stuff", itty.next());
+ assertThat(itty.hasNext(), is(false));
+ assertThat(same.hasNext(), is(false));
+
+ // this is a do-nothing, but we should still be able to call it
+ itty.close();
+ }
+}