Make AbstractPingPongTransactionChain.close() idempotent

[mdsal.git] / dom / mdsal-dom-spi / src / test / java / org / opendaylight / mdsal / dom / spi / PingPongTransactionChainTest.java

diff --git a/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChainTest.java b/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChainTest.java
index a33f4929496bc6a95e73f929a96d9db386ed13d8..30f187821201e3b1b72bdfc5f5515d2c9d8923ad 100644 (file)
--- a/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChainTest.java
+++ b/dom/mdsal-dom-spi/src/test/java/org/opendaylight/mdsal/dom/spi/PingPongTransactionChainTest.java
@@ -7,8 +7,14 @@
  */
 package org.opendaylight.mdsal.dom.spi;
 
+import static org.hamcrest.CoreMatchers.allOf;
+import static org.hamcrest.CoreMatchers.containsString;
+import static org.hamcrest.CoreMatchers.endsWith;
+import static org.hamcrest.CoreMatchers.startsWith;
+import static org.hamcrest.MatcherAssert.assertThat;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
+import static org.junit.Assert.assertNotNull;
 import static org.junit.Assert.assertSame;
 import static org.junit.Assert.assertThrows;
 import static org.mockito.ArgumentMatchers.any;
@@ -16,7 +22,9 @@ import static org.mockito.Mockito.doAnswer;
 import static org.mockito.Mockito.doNothing;
 import static org.mockito.Mockito.doReturn;
 import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
 import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.verifyNoMoreInteractions;
 
 import com.google.common.util.concurrent.FluentFuture;
 import com.google.common.util.concurrent.Futures;
@@ -52,6 +60,10 @@ public class PingPongTransactionChainTest {
     public DOMTransactionChain chain;
     @Mock
     public DOMDataTreeReadWriteTransaction rwTx;
+    @Mock
+    public DOMDataTreeReadWriteTransaction rwTx1;
+    @Mock
+    public DOMDataTreeReadWriteTransaction rwTx2;
 
     public DOMTransactionChainListener pingPongListener;
     public PingPongTransactionChain pingPong;
@@ -84,6 +96,7 @@ public class PingPongTransactionChainTest {
     public void testIdleFailure() {
         final var cause = new Throwable();
         doNothing().when(listener).onTransactionChainFailed(pingPong, null, cause);
+        doReturn("mock").when(chain).toString();
         pingPongListener.onTransactionChainFailed(chain, rwTx, cause);
         verify(listener).onTransactionChainFailed(pingPong, null, cause);
     }
@@ -203,6 +216,130 @@ public class PingPongTransactionChainTest {
         verify(rwTx).cancel();
     }
 
+    @Test
+    public void testNewAfterSuccessfulCancel() {
+        doReturn(true).when(rwTx).cancel();
+        pingPong.newWriteOnlyTransaction().cancel();
+        assertNotNull(pingPong.newWriteOnlyTransaction());
+    }
+
+    @Test
+    public void testNewAfterNew() {
+        assertNotNull(pingPong.newWriteOnlyTransaction());
+        doReturn(true).when(rwTx).cancel();
+        doReturn("mock").when(rwTx).toString();
+        final var ex = assertThrows(IllegalStateException.class, () -> pingPong.newWriteOnlyTransaction());
+        assertThat(ex.getMessage(), allOf(
+            startsWith("New transaction PingPongTransaction"),
+            containsString(" raced with transaction PingPongTransaction")));
+    }
+
+    @Test
+    public void testReadWriteReuse() {
+        final var tx = pingPong.newReadWriteTransaction();
+        final var rwTxFuture = SettableFuture.<CommitInfo>create();
+        doReturn(FluentFuture.from(rwTxFuture)).when(rwTx).commit();
+        // Now rwTx is inflight, but does not commit immediately
+        final var txFuture = tx.commit();
+        verify(rwTx).commit();
+
+        // Assert identity without delving into details
+        final var id = mock(Object.class);
+        doReturn(id).when(rwTx).getIdentifier();
+        assertSame(tx.getIdentifier(), id);
+
+        doReturn(rwTx1).when(chain).newReadWriteTransaction();
+        final var tx1 = pingPong.newWriteOnlyTransaction();
+        // now rwTx1 is ready, waiting for inflight to be completed
+        final var tx1Future = tx1.commit();
+
+        final var id1 = mock(Object.class);
+        doReturn(id1).when(rwTx1).getIdentifier();
+        assertSame(tx1.getIdentifier(), id1);
+
+        // Ready transaction is picked up by fast path allocation
+        final var tx2 = pingPong.newWriteOnlyTransaction();
+        assertSame(tx2.getIdentifier(), id1);
+
+        // Complete inflight transaction...
+        rwTxFuture.set(CommitInfo.empty());
+        assertDone(txFuture);
+        // ... but we are still holding the follow-up frontend transaction ...
+        assertFalse(tx1Future.isDone());
+        verify(rwTx1, never()).commit();
+
+        // ... and it will commit once we commit tx2 ...
+        doReturn(CommitInfo.emptyFluentFuture()).when(rwTx1).commit();
+        final var tx2Future = tx2.commit();
+        // ... at which point both complete
+        assertDone(tx1Future);
+        assertDone(tx2Future);
+    }
+
+    @Test
+    public void commitWhileInflight() {
+        final var tx = pingPong.newReadWriteTransaction();
+
+        final var rwTxFuture = SettableFuture.<CommitInfo>create();
+        doReturn(FluentFuture.from(rwTxFuture)).when(rwTx).commit();
+        // rwTxFuture is inflight
+        final var txFuture = tx.commit();
+        verify(rwTx).commit();
+        assertFalse(txFuture.isDone());
+
+        doReturn(rwTx1).when(chain).newReadWriteTransaction();
+        final var rwTxFuture1 = SettableFuture.<CommitInfo>create();
+        final var tx1 = pingPong.newWriteOnlyTransaction();
+        final var tx1Future = tx1.commit();
+
+        doReturn(FluentFuture.from(rwTxFuture1)).when(rwTx1).commit();
+        rwTxFuture.set(CommitInfo.empty());
+        assertDone(txFuture);
+        verify(rwTx1).commit();
+
+        rwTxFuture1.set(CommitInfo.empty());
+        assertDone(tx1Future);
+    }
+
+    @Test
+    public void testNewAfterAsyncShutdown() {
+        // Setup inflight transaction
+        final var tx = pingPong.newReadWriteTransaction();
+        final var rwTxFuture = SettableFuture.<CommitInfo>create();
+        doReturn(FluentFuture.from(rwTxFuture)).when(rwTx).commit();
+        final var txFuture = tx.commit();
+        assertFalse(txFuture.isDone());
+
+        // Setup ready transaction
+        doReturn(rwTx1).when(chain).newReadWriteTransaction();
+        final var rwTx1Future = SettableFuture.<CommitInfo>create();
+        doReturn(FluentFuture.from(rwTx1Future)).when(rwTx1).commit();
+
+        final var tx1Future = pingPong.newReadWriteTransaction().commit();
+        assertFalse(tx1Future.isDone());
+
+        pingPong.close();
+
+        final var ex = assertThrows(IllegalStateException.class, pingPong::newWriteOnlyTransaction);
+        assertThat(ex.getMessage(), allOf(startsWith("Transaction chain "), endsWith(" has been shut down")));
+        doNothing().when(chain).close();
+        rwTxFuture.set(CommitInfo.empty());
+        assertDone(txFuture);
+        verify(chain).close();
+
+        rwTx1Future.set(CommitInfo.empty());
+        assertDone(tx1Future);
+    }
+
+    @Test
+    public void testIdempotentClose() {
+        doNothing().when(chain).close();
+        pingPong.close();
+        verify(chain).close();
+        pingPong.close();
+        verifyNoMoreInteractions(chain);
+    }
+
     private static <T> T assertDone(final FluentFuture<T> future) {
         try {
             return Futures.getDone(future);