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 2016/05/13 12:48:11 UTC

[26/28] incubator-tinkerpop git commit: Added some test to enforce some existing transactional semantics.

Added some test to enforce some existing transactional semantics.

Covers TINKERPOP-1059 and TINKERPOP-947. While these were labelled as "breaking", that's a bit of a stretch of the definition. Existing expected semantics are now being enforced by tests so it's possible that providers could see a failure in their test suite over this change, but it is highly unlikely given the semantics being asserted. CTR


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

Branch: refs/heads/TINKERPOP-1297
Commit: 08720716cbb2d777fc5200e8afd3843802ed4ac3
Parents: aeebae5
Author: Stephen Mallette <sp...@genoprime.com>
Authored: Thu May 12 13:47:18 2016 -0400
Committer: Stephen Mallette <sp...@genoprime.com>
Committed: Thu May 12 13:51:42 2016 -0400

----------------------------------------------------------------------
 CHANGELOG.asciidoc                              |  2 +
 .../upgrade/release-3.2.x-incubating.asciidoc   | 21 +++++++
 .../util/AbstractThreadedTransaction.java       |  3 +-
 .../gremlin/structure/TransactionTest.java      | 60 ++++++++++++++++++++
 4 files changed, 85 insertions(+), 1 deletion(-)
----------------------------------------------------------------------


http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/08720716/CHANGELOG.asciidoc
----------------------------------------------------------------------
diff --git a/CHANGELOG.asciidoc b/CHANGELOG.asciidoc
index 31ae001..fcd0a1b 100644
--- a/CHANGELOG.asciidoc
+++ b/CHANGELOG.asciidoc
@@ -26,6 +26,8 @@ image::https://raw.githubusercontent.com/apache/incubator-tinkerpop/master/docs/
 TinkerPop 3.2.1 (NOT OFFICIALLY RELEASED YET)
 ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+* Added tests to validate the status of a transaction immediately following calls to close.
+* Added tests to ensure that threaded transactions cannot be re-used.
 * `GraphFilter` helper methods are now more intelligent when determining edge direction/label legality.
 * Added `GraphFilterStrategy` to automatically construct `GraphFilters` via traversal introspection in OLAP.
 * Increased the testing and scope of `TraversalHelper.isLocalStarGraph()`.

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/08720716/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 04eb8f5..cc50c43 100644
--- a/docs/src/upgrade/release-3.2.x-incubating.asciidoc
+++ b/docs/src/upgrade/release-3.2.x-incubating.asciidoc
@@ -135,6 +135,25 @@ in a future release along with the "JUnit Benchmarks" dependency.
 
 See: link:https://issues.apache.org/jira/browse/TINKERPOP-1294[TINKERPOP-1294]
 
+Graph Database Providers
+^^^^^^^^^^^^^^^^^^^^^^^^
+
+Transaction Tests
++++++++++++++++++
+
+Tests and assertions were added to the structure test suite to validate that transaction status was in the appropriate
+state following calls to close the transaction with `commit()` or `rollback()`. It is unlikely that this change would
+cause test breaks for providers, unless the transaction status was inherently disconnected from calls to close the
+transaction somehow.
+
+In addition, other tests were added to enforce the expected semantics for threaded transactions. Threaded transactions
+are expected to behave like manual transactions. They should be open automatically when they are created and once
+closed should no longer be used. This behavior is not new and is the typical expected method for working with these
+types of transactions. The test suite just requires that the provider implementation conform to these semantics.
+
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-947[TINKERPOP-947],
+link:https://issues.apache.org/jira/browse/TINKERPOP-1059[TINKERPOP-1059]
+
 GraphFilter and GraphFilterStrategy
 +++++++++++++++++++++++++++++++++++
 
@@ -153,6 +172,8 @@ static {
 }
 ----
 
+See: link:https://issues.apache.org/jira/browse/TINKERPOP-1293[TINKERPOP-1293]
+
 Graph Language Providers
 ^^^^^^^^^^^^^^^^^^^^^^^^
 

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/08720716/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/AbstractThreadedTransaction.java
----------------------------------------------------------------------
diff --git a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/AbstractThreadedTransaction.java b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/AbstractThreadedTransaction.java
index 2fdc7df..98bdd3e 100644
--- a/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/AbstractThreadedTransaction.java
+++ b/gremlin-core/src/main/java/org/apache/tinkerpop/gremlin/structure/util/AbstractThreadedTransaction.java
@@ -71,7 +71,8 @@ public abstract class AbstractThreadedTransaction extends AbstractTransaction {
     }
 
     /**
-     * Most implementations should do nothing with this as the tx is already open on creation.
+     * Threaded transactions should be open immediately upon creation so most implementations should do nothing with
+     * this method.
      */
     @Override
     protected void doReadWrite() {

http://git-wip-us.apache.org/repos/asf/incubator-tinkerpop/blob/08720716/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/TransactionTest.java
----------------------------------------------------------------------
diff --git a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/TransactionTest.java b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/TransactionTest.java
index a16dd09..9ec7e04 100644
--- a/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/TransactionTest.java
+++ b/gremlin-test/src/main/java/org/apache/tinkerpop/gremlin/structure/TransactionTest.java
@@ -41,6 +41,8 @@ import static org.apache.tinkerpop.gremlin.structure.Graph.Features.EdgeProperty
 import static org.apache.tinkerpop.gremlin.structure.Graph.Features.GraphFeatures.FEATURE_TRANSACTIONS;
 import static org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_DOUBLE_VALUES;
 import static org.apache.tinkerpop.gremlin.structure.Graph.Features.VertexPropertyFeatures.FEATURE_INTEGER_VALUES;
+import static org.hamcrest.core.Is.is;
+import static org.hamcrest.core.IsInstanceOf.instanceOf;
 import static org.junit.Assert.*;
 
 /**
@@ -174,8 +176,11 @@ public class TransactionTest extends AbstractGremlinTest {
     public void shouldAllowAutoTransactionToWorkWithoutMutationByDefault() {
         // expecting no exceptions to be thrown here
         g.tx().commit();
+        assertThat(g.tx().isOpen(), is(false));
         g.tx().rollback();
+        assertThat(g.tx().isOpen(), is(false));
         g.tx().commit();
+        assertThat(g.tx().isOpen(), is(false));
     }
 
     @Test
@@ -243,19 +248,25 @@ public class TransactionTest extends AbstractGremlinTest {
         assertVertexEdgeCounts(graph, 1, 1);
         assertEquals(v1.id(), graph.vertices(v1.id()).next().id());
         assertEquals(e1.id(), graph.edges(e1.id()).next().id());
+        assertThat(g.tx().isOpen(), is(true));
         g.tx().commit();
+        assertThat(g.tx().isOpen(), is(false));
         assertVertexEdgeCounts(graph, 1, 1);
+        assertThat(g.tx().isOpen(), is(true));
         assertEquals(v1.id(), graph.vertices(v1.id()).next().id());
         assertEquals(e1.id(), graph.edges(e1.id()).next().id());
 
         graph.vertices(v1.id()).forEachRemaining(Element::remove);
         assertVertexEdgeCounts(graph, 0, 0);
         g.tx().rollback();
+        assertThat(g.tx().isOpen(), is(false));
         assertVertexEdgeCounts(graph, 1, 1);
+        assertThat(g.tx().isOpen(), is(true));
 
         graph.vertices(v1.id()).forEachRemaining(Element::remove);
         assertVertexEdgeCounts(graph, 0, 0);
         g.tx().commit();
+        assertThat(g.tx().isOpen(), is(false));
         assertVertexEdgeCounts(graph, 0, 0);
     }
 
@@ -590,6 +601,55 @@ public class TransactionTest extends AbstractGremlinTest {
         assertVertexEdgeCounts(graph, numberOfThreads, 0);
     }
 
+    @Test
+    @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)
+    @FeatureRequirement(featureClass = Graph.Features.GraphFeatures.class, feature = Graph.Features.GraphFeatures.FEATURE_TRANSACTIONS)
+    @FeatureRequirement(featureClass = Graph.Features.GraphFeatures.class, feature = Graph.Features.GraphFeatures.FEATURE_THREADED_TRANSACTIONS)
+    public void shouldOpenTxWhenThreadedTransactionIsCreated() throws Exception {
+        // threaded transactions should be immediately open on creation
+        final Graph threadedG = g.tx().createThreadedTx();
+        assertThat(threadedG.tx().isOpen(), is(true));
+
+        threadedG.tx().rollback();
+        assertThat(threadedG.tx().isOpen(), is(false));
+    }
+
+    @Test
+    @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)
+    @FeatureRequirement(featureClass = Graph.Features.GraphFeatures.class, feature = Graph.Features.GraphFeatures.FEATURE_TRANSACTIONS)
+    @FeatureRequirement(featureClass = Graph.Features.GraphFeatures.class, feature = Graph.Features.GraphFeatures.FEATURE_THREADED_TRANSACTIONS)
+    public void shouldNotReuseThreadedTransaction() throws Exception {
+        final int numberOfThreads = 10;
+        final CountDownLatch latch = new CountDownLatch(numberOfThreads);
+
+        final Graph threadedG = g.tx().createThreadedTx();
+
+        for (int ix = 0; ix < numberOfThreads; ix++) {
+            new Thread(() -> {
+                threadedG.addVertex();
+                latch.countDown();
+            }).start();
+        }
+
+        latch.await(10000, TimeUnit.MILLISECONDS);
+
+        // threaded transaction is not yet committed so g should not reflect any change
+        assertVertexEdgeCounts(graph, 0, 0);
+        threadedG.tx().commit();
+
+        // there should be one vertex for each thread
+        assertVertexEdgeCounts(graph, numberOfThreads, 0);
+
+        try {
+            assertThat(threadedG.tx().isOpen(), is(false));
+            threadedG.addVertex();
+            fail("Shouldn't be able to re-use a threaded transaction");
+        } catch (Exception ex) {
+            assertThat(ex, instanceOf(IllegalStateException.class));
+        } finally {
+            threadedG.tx().rollback();
+        }
+    }
 
     @Test
     @FeatureRequirement(featureClass = Graph.Features.VertexFeatures.class, feature = Graph.Features.VertexFeatures.FEATURE_ADD_VERTICES)