Skip to content

Commit 4f59cc6

Browse files
committed
Do not keep bookmark in transaction
Pass it directly to the session, when the transaction is committed. This simplifies `ExplicitTransaction` a bit by removing all bookmark handling from it.
1 parent 86cf222 commit 4f59cc6

File tree

3 files changed

+18
-66
lines changed

3 files changed

+18
-66
lines changed

driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java

Lines changed: 1 addition & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,6 @@ private enum State
6868
private final NetworkSession session;
6969
private final ResultCursorsHolder resultCursors;
7070

71-
private volatile Bookmarks bookmarks = Bookmarks.empty();
7271
private volatile State state = State.ACTIVE;
7372

7473
public ExplicitTransaction( Connection connection, NetworkSession session )
@@ -229,19 +228,6 @@ public void markTerminated()
229228
state = State.TERMINATED;
230229
}
231230

232-
public Bookmarks bookmark()
233-
{
234-
return bookmarks;
235-
}
236-
237-
public void setBookmarks( Bookmarks bookmarks )
238-
{
239-
if ( bookmarks != null && !bookmarks.isEmpty() )
240-
{
241-
this.bookmarks = bookmarks;
242-
}
243-
}
244-
245231
private CompletionStage<Void> doCommitAsync()
246232
{
247233
if ( state == State.TERMINATED )
@@ -252,7 +238,7 @@ private CompletionStage<Void> doCommitAsync()
252238
return protocol.commitTransaction( connection )
253239
.thenApply( newBookmarks ->
254240
{
255-
setBookmarks( newBookmarks );
241+
session.setBookmarks( newBookmarks );
256242
return null;
257243
} );
258244
}
@@ -283,7 +269,6 @@ private void transactionClosed( State newState )
283269
{
284270
state = newState;
285271
connection.release(); // release in background
286-
session.setBookmarks( bookmarks );
287272
}
288273

289274
private void terminateConnectionOnThreadInterrupt( String reason )

driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -201,40 +201,6 @@ void shouldBeClosedWhenMarkedTerminatedAndClosed()
201201
assertFalse( tx.isOpen() );
202202
}
203203

204-
@Test
205-
void shouldHaveEmptyBookmarkInitially()
206-
{
207-
ExplicitTransaction tx = beginTx( connectionMock() );
208-
assertTrue( tx.bookmark().isEmpty() );
209-
}
210-
211-
@Test
212-
void shouldNotKeepInitialBookmark()
213-
{
214-
ExplicitTransaction tx = beginTx( connectionMock(), Bookmarks.from( "Dog" ) );
215-
assertTrue( tx.bookmark().isEmpty() );
216-
}
217-
218-
@Test
219-
void shouldNotOverwriteBookmarkWithNull()
220-
{
221-
ExplicitTransaction tx = beginTx( connectionMock() );
222-
tx.setBookmarks( Bookmarks.from( "Cat" ) );
223-
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
224-
tx.setBookmarks( null );
225-
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
226-
}
227-
228-
@Test
229-
void shouldNotOverwriteBookmarkWithEmptyBookmark()
230-
{
231-
ExplicitTransaction tx = beginTx( connectionMock() );
232-
tx.setBookmarks( Bookmarks.from( "Cat" ) );
233-
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
234-
tx.setBookmarks( Bookmarks.empty() );
235-
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
236-
}
237-
238204
@Test
239205
void shouldReleaseConnectionWhenBeginFails()
240206
{

driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java

Lines changed: 17 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727

2828
import java.util.Map;
2929

30+
import org.neo4j.driver.internal.messaging.BoltProtocol;
3031
import org.neo4j.driver.internal.messaging.request.PullAllMessage;
3132
import org.neo4j.driver.internal.messaging.request.RunMessage;
3233
import org.neo4j.driver.internal.retry.FixedRetryLogic;
@@ -62,6 +63,7 @@
6263
import static org.mockito.Mockito.RETURNS_MOCKS;
6364
import static org.mockito.Mockito.atLeastOnce;
6465
import static org.mockito.Mockito.doAnswer;
66+
import static org.mockito.Mockito.doReturn;
6567
import static org.mockito.Mockito.inOrder;
6668
import static org.mockito.Mockito.mock;
6769
import static org.mockito.Mockito.never;
@@ -243,11 +245,17 @@ void acquiresNewConnectionForBeginTx()
243245
@Test
244246
void updatesBookmarkWhenTxIsClosed()
245247
{
246-
Transaction tx = session.beginTransaction();
247-
setBookmarks( tx, Bookmarks.from( "TheBookmark" ) );
248+
Bookmarks bookmarkAfterCommit = Bookmarks.from( "TheBookmark" );
249+
250+
BoltProtocol protocol = spy( DEFAULT_TEST_PROTOCOL );
251+
doReturn( completedFuture( bookmarkAfterCommit ) ).when( protocol ).commitTransaction( any( Connection.class ) );
248252

253+
when( connection.protocol() ).thenReturn( protocol );
254+
255+
Transaction tx = session.beginTransaction();
249256
assertNull( session.lastBookmark() );
250257

258+
tx.success();
251259
tx.close();
252260
assertEquals( "TheBookmark", session.lastBookmark() );
253261
}
@@ -309,18 +317,21 @@ void bookmarkIsPropagatedBetweenTransactions()
309317

310318
NetworkSession session = newSession( connectionProvider, READ );
311319

320+
BoltProtocol protocol = spy( DEFAULT_TEST_PROTOCOL );
321+
doReturn( completedFuture( bookmarks1 ), completedFuture( bookmarks2 ) ).when( protocol ).commitTransaction( any( Connection.class ) );
322+
323+
when( connection.protocol() ).thenReturn( protocol );
324+
312325
try ( Transaction tx = session.beginTransaction() )
313326
{
314-
setBookmarks( tx, bookmarks1 );
327+
tx.success();
315328
}
316-
317329
assertEquals( bookmarks1, Bookmarks.from( session.lastBookmark() ) );
318330

319331
try ( Transaction tx = session.beginTransaction() )
320332
{
321333
verifyBeginTx( connection, bookmarks1 );
322-
assertTrue( getBookmarks( tx ).isEmpty() );
323-
setBookmarks( tx, bookmarks2 );
334+
tx.success();
324335
}
325336
assertEquals( bookmarks2, Bookmarks.from( session.lastBookmark() ) );
326337
}
@@ -898,16 +909,6 @@ private static void verifyRunAndFlush( Connection connectionMock, String stateme
898909
verify( connectionMock, mode ).writeAndFlush( eq( new RunMessage( statement ) ), any(), eq( PullAllMessage.PULL_ALL ), any() );
899910
}
900911

901-
private static Bookmarks getBookmarks( Transaction tx )
902-
{
903-
return ((ExplicitTransaction) tx).bookmark();
904-
}
905-
906-
private static void setBookmarks( Transaction tx, Bookmarks bookmarks )
907-
{
908-
((ExplicitTransaction) tx).setBookmarks( bookmarks );
909-
}
910-
911912
private static void setupFailingCommit( Connection connection, int times )
912913
{
913914
doAnswer( new Answer<Void>()

0 commit comments

Comments
 (0)