Skip to content

Commit f6d5401

Browse files
zhenlineozhenlineo
authored
Merge pull request #562 from michael-simons/feature/bookmarks-holder
Use only BookmarksHolder contract instead of concrete NetworkSession.
2 parents bb1ad0f + fcf3dbf commit f6d5401

13 files changed

+168
-141
lines changed

‎driver/src/main/java/org/neo4j/driver/internal/BookmarksHolder.java

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,8 @@ public interface BookmarksHolder
2424

2525
void setBookmarks( Bookmarks bookmarks );
2626

27+
String lastBookmark();
28+
2729
BookmarksHolder NO_OP = new BookmarksHolder()
2830
{
2931
@Override
@@ -36,5 +38,11 @@ public Bookmarks getBookmarks()
3638
public void setBookmarks( Bookmarks bookmarks )
3739
{
3840
}
41+
42+
@Override
43+
public String lastBookmark()
44+
{
45+
return null;
46+
}
3947
};
4048
}

‎driver/src/test/java/org/neo4j/driver/internal/SimpleBookmarksHolder.java renamed to ‎driver/src/main/java/org/neo4j/driver/internal/DefaultBookmarksHolder.java

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -18,11 +18,19 @@
1818
*/
1919
package org.neo4j.driver.internal;
2020

21-
public class SimpleBookmarksHolder implements BookmarksHolder
21+
/**
22+
* @since 2.0
23+
*/
24+
public class DefaultBookmarksHolder implements BookmarksHolder
2225
{
2326
private volatile Bookmarks bookmarks;
2427

25-
public SimpleBookmarksHolder( Bookmarks bookmarks )
28+
public DefaultBookmarksHolder()
29+
{
30+
this( Bookmarks.empty() );
31+
}
32+
33+
public DefaultBookmarksHolder( Bookmarks bookmarks )
2634
{
2735
this.bookmarks = bookmarks;
2836
}
@@ -36,6 +44,15 @@ public Bookmarks getBookmarks()
3644
@Override
3745
public void setBookmarks( Bookmarks bookmarks )
3846
{
39-
this.bookmarks = bookmarks;
47+
if ( bookmarks != null && !bookmarks.isEmpty() )
48+
{
49+
this.bookmarks = bookmarks;
50+
}
51+
}
52+
53+
@Override
54+
public String lastBookmark()
55+
{
56+
return bookmarks == null ? null : bookmarks.maxBookmarkAsString();
4057
}
4158
}

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

Lines changed: 4 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -67,16 +67,16 @@ private enum State
6767

6868
private final Connection connection;
6969
private final BoltProtocol protocol;
70-
private final NetworkSession session;
70+
private final BookmarksHolder bookmarksHolder;
7171
private final ResultCursorsHolder resultCursors;
7272

7373
private volatile State state = State.ACTIVE;
7474

75-
public ExplicitTransaction( Connection connection, NetworkSession session )
75+
public ExplicitTransaction( Connection connection, BookmarksHolder bookmarksHolder )
7676
{
7777
this.connection = connection;
7878
this.protocol = connection.protocol();
79-
this.session = session;
79+
this.bookmarksHolder = bookmarksHolder;
8080
this.resultCursors = new ResultCursorsHolder();
8181
}
8282

@@ -247,12 +247,7 @@ private CompletionStage<Void> doCommitAsync()
247247
return failedFuture( new ClientException( "Transaction can't be committed. " +
248248
"It has been rolled back either because of an error or explicit termination" ) );
249249
}
250-
return protocol.commitTransaction( connection )
251-
.thenApply( newBookmarks ->
252-
{
253-
session.setBookmarks( newBookmarks );
254-
return null;
255-
} );
250+
return protocol.commitTransaction( connection ).thenAccept( bookmarksHolder::setBookmarks );
256251
}
257252

258253
private CompletionStage<Void> doRollbackAsync()

‎driver/src/main/java/org/neo4j/driver/internal/LeakLoggingNetworkSession.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,9 +30,9 @@ class LeakLoggingNetworkSession extends NetworkSession
3030
{
3131
private final String stackTrace;
3232

33-
LeakLoggingNetworkSession( ConnectionProvider connectionProvider, AccessMode mode, RetryLogic retryLogic, Logging logging )
33+
LeakLoggingNetworkSession( ConnectionProvider connectionProvider, AccessMode mode, RetryLogic retryLogic, Logging logging, BookmarksHolder bookmarksHolder )
3434
{
35-
super( connectionProvider, mode, retryLogic, logging );
35+
super( connectionProvider, mode, retryLogic, logging, bookmarksHolder );
3636
this.stackTrace = captureStackTrace();
3737
}
3838

‎driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java

Lines changed: 9 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@
4949
import static org.neo4j.driver.internal.util.Futures.completedWithNull;
5050
import static org.neo4j.driver.internal.util.Futures.failedFuture;
5151

52-
public class NetworkSession extends AbstractStatementRunner implements Session, BookmarksHolder
52+
public class NetworkSession extends AbstractStatementRunner implements Session
5353
{
5454
private static final String LOG_NAME = "Session";
5555

@@ -58,19 +58,20 @@ public class NetworkSession extends AbstractStatementRunner implements Session,
5858
private final RetryLogic retryLogic;
5959
protected final Logger logger;
6060

61-
private volatile Bookmarks bookmarks = Bookmarks.empty();
61+
private final BookmarksHolder bookmarksHolder;
6262
private volatile CompletionStage<ExplicitTransaction> transactionStage = completedWithNull();
6363
private volatile CompletionStage<Connection> connectionStage = completedWithNull();
6464
private volatile CompletionStage<? extends FailableCursor> resultCursorStage = completedWithNull();
6565

6666
private final AtomicBoolean open = new AtomicBoolean( true );
6767

68-
public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, RetryLogic retryLogic, Logging logging )
68+
public NetworkSession( ConnectionProvider connectionProvider, AccessMode mode, RetryLogic retryLogic, Logging logging, BookmarksHolder bookmarksHolder )
6969
{
7070
this.connectionProvider = connectionProvider;
7171
this.mode = mode;
7272
this.retryLogic = retryLogic;
7373
this.logger = new PrefixedLogger( "[" + hashCode() + "]", logging.getLog( LOG_NAME ) );
74+
this.bookmarksHolder = bookmarksHolder;
7475
}
7576

7677
@Override
@@ -185,7 +186,7 @@ public Transaction beginTransaction( TransactionConfig config )
185186
@Override
186187
public Transaction beginTransaction( String bookmark )
187188
{
188-
setBookmarks( Bookmarks.from( bookmark ) );
189+
bookmarksHolder.setBookmarks( Bookmarks.from( bookmark ) );
189190
return beginTransaction();
190191
}
191192

@@ -250,25 +251,10 @@ public <T> CompletionStage<T> writeTransactionAsync( TransactionWork<CompletionS
250251
return transactionAsync( AccessMode.WRITE, work, config );
251252
}
252253

253-
@Override
254-
public Bookmarks getBookmarks()
255-
{
256-
return bookmarks;
257-
}
258-
259-
@Override
260-
public void setBookmarks( Bookmarks bookmarks )
261-
{
262-
if ( bookmarks != null && !bookmarks.isEmpty() )
263-
{
264-
this.bookmarks = bookmarks;
265-
}
266-
}
267-
268254
@Override
269255
public String lastBookmark()
270256
{
271-
return bookmarks == null ? null : bookmarks.maxBookmarkAsString();
257+
return bookmarksHolder.lastBookmark();
272258
}
273259

274260
@Override
@@ -470,7 +456,7 @@ private CompletionStage<StatementResultCursorFactory> buildResultCursorFactory(
470456
try
471457
{
472458
StatementResultCursorFactory factory = connection.protocol()
473-
.runInAutoCommitTransaction( connection, statement, this, config, waitForRunResponse );
459+
.runInAutoCommitTransaction( connection, statement, bookmarksHolder, config, waitForRunResponse );
474460
return completedFuture( factory );
475461
}
476462
catch ( Throwable e )
@@ -495,8 +481,8 @@ private CompletionStage<ExplicitTransaction> beginTransactionAsync( AccessMode m
495481
.thenCompose( ignore -> acquireConnection( mode ) )
496482
.thenCompose( connection ->
497483
{
498-
ExplicitTransaction tx = new ExplicitTransaction( connection, NetworkSession.this );
499-
return tx.beginAsync( bookmarks, config );
484+
ExplicitTransaction tx = new ExplicitTransaction( connection, bookmarksHolder );
485+
return tx.beginAsync( bookmarksHolder.getBookmarks(), config );
500486
} );
501487

502488
// update the reference to the only known transaction

‎driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -44,8 +44,8 @@ public class SessionFactoryImpl implements SessionFactory
4444
@Override
4545
public NetworkSession newInstance( AccessMode mode, Bookmarks bookmarks )
4646
{
47-
NetworkSession session = createSession( connectionProvider, retryLogic, mode, logging );
48-
session.setBookmarks( bookmarks );
47+
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder( bookmarks );
48+
NetworkSession session = createSession( connectionProvider, retryLogic, mode, logging, bookmarksHolder );
4949
return session;
5050
}
5151

@@ -74,10 +74,10 @@ public ConnectionProvider getConnectionProvider()
7474
}
7575

7676
private NetworkSession createSession( ConnectionProvider connectionProvider, RetryLogic retryLogic,
77-
AccessMode mode, Logging logging )
77+
AccessMode mode, Logging logging, BookmarksHolder bookmarksHolder )
7878
{
7979
return leakedSessionsLoggingEnabled
80-
? new LeakLoggingNetworkSession( connectionProvider, mode, retryLogic, logging )
81-
: new NetworkSession( connectionProvider, mode, retryLogic, logging );
80+
? new LeakLoggingNetworkSession( connectionProvider, mode, retryLogic, logging, bookmarksHolder )
81+
: new NetworkSession( connectionProvider, mode, retryLogic, logging, bookmarksHolder );
8282
}
8383
}

‎driver/src/test/java/org/neo4j/driver/internal/DefaultBookmarksHolderTest.java

Lines changed: 96 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,96 @@
1+
/*
2+
* Copyright (c) 2002-2019 "Neo4j,"
3+
* Neo4j Sweden AB [http://neo4j.com]
4+
*
5+
* This file is part of Neo4j.
6+
*
7+
* Licensed under the Apache License, Version 2.0 (the "License");
8+
* you may not use this file except in compliance with the License.
9+
* You may obtain a copy of the License at
10+
*
11+
* http://www.apache.org/licenses/LICENSE-2.0
12+
*
13+
* Unless required by applicable law or agreed to in writing, software
14+
* distributed under the License is distributed on an "AS IS" BASIS,
15+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
16+
* See the License for the specific language governing permissions and
17+
* limitations under the License.
18+
*/
19+
package org.neo4j.driver.internal;
20+
21+
import org.junit.jupiter.api.Test;
22+
23+
import static org.junit.jupiter.api.Assertions.assertEquals;
24+
25+
class DefaultBookmarksHolderTest
26+
{
27+
@Test
28+
void shouldAllowToGetAndSetBookmarks()
29+
{
30+
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder();
31+
assertEquals( Bookmarks.empty(), bookmarksHolder.getBookmarks() );
32+
33+
bookmarksHolder.setBookmarks( null );
34+
assertEquals( Bookmarks.empty(), bookmarksHolder.getBookmarks() );
35+
36+
bookmarksHolder.setBookmarks( Bookmarks.empty() );
37+
assertEquals( Bookmarks.empty(), bookmarksHolder.getBookmarks() );
38+
39+
Bookmarks bookmarks1 = Bookmarks.from( "neo4j:bookmark:v1:tx1" );
40+
bookmarksHolder.setBookmarks( bookmarks1 );
41+
assertEquals( bookmarks1, bookmarksHolder.getBookmarks() );
42+
43+
bookmarksHolder.setBookmarks( null );
44+
assertEquals( bookmarks1, bookmarksHolder.getBookmarks() );
45+
46+
bookmarksHolder.setBookmarks( Bookmarks.empty() );
47+
assertEquals( bookmarks1, bookmarksHolder.getBookmarks() );
48+
49+
Bookmarks bookmarks2 = Bookmarks.from( "neo4j:bookmark:v1:tx2" );
50+
bookmarksHolder.setBookmarks( bookmarks2 );
51+
assertEquals( bookmarks2, bookmarksHolder.getBookmarks() );
52+
53+
Bookmarks bookmarks3 = Bookmarks.from( "neo4j:bookmark:v1:tx42" );
54+
bookmarksHolder.setBookmarks( bookmarks3 );
55+
assertEquals( bookmarks3, bookmarksHolder.getBookmarks() );
56+
}
57+
58+
@Test
59+
void bookmarkCanBeSet()
60+
{
61+
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder();
62+
Bookmarks bookmarks = Bookmarks.from( "neo4j:bookmark:v1:tx100" );
63+
64+
bookmarksHolder.setBookmarks( bookmarks );
65+
66+
assertEquals( bookmarks.maxBookmarkAsString(), bookmarksHolder.lastBookmark() );
67+
}
68+
69+
@Test
70+
void shouldNotOverwriteBookmarkWithNull()
71+
{
72+
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder( Bookmarks.from( "Cat" ) );
73+
assertEquals( "Cat", bookmarksHolder.lastBookmark() );
74+
bookmarksHolder.setBookmarks( null );
75+
assertEquals( "Cat", bookmarksHolder.lastBookmark() );
76+
}
77+
78+
@Test
79+
void shouldNotOverwriteBookmarkWithEmptyBookmark()
80+
{
81+
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder( Bookmarks.from( "Cat" ) );
82+
assertEquals( "Cat", bookmarksHolder.lastBookmark() );
83+
bookmarksHolder.setBookmarks( Bookmarks.empty() );
84+
assertEquals( "Cat", bookmarksHolder.lastBookmark() );
85+
}
86+
87+
@Test
88+
void setLastBookmark()
89+
{
90+
BookmarksHolder bookmarksHolder = new DefaultBookmarksHolder();
91+
92+
bookmarksHolder.setBookmarks( Bookmarks.from( "TheBookmark" ) );
93+
94+
assertEquals( "TheBookmark", bookmarksHolder.lastBookmark() );
95+
}
96+
}

‎driver/src/test/java/org/neo4j/driver/internal/LeakLoggingNetworkSessionTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ private static void finalize( Session session ) throws Exception
9595

9696
private static LeakLoggingNetworkSession newSession( Logging logging, boolean openConnection )
9797
{
98-
return new LeakLoggingNetworkSession( connectionProviderMock( openConnection ), READ, new FixedRetryLogic( 0 ), logging );
98+
return new LeakLoggingNetworkSession( connectionProviderMock( openConnection ), READ, new FixedRetryLogic( 0 ), logging, new DefaultBookmarksHolder( ) );
9999
}
100100

101101
private static ConnectionProvider connectionProviderMock( boolean openConnection )

‎driver/src/test/java/org/neo4j/driver/internal/messaging/v2/ExplicitTransactionTest.java

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -24,8 +24,8 @@
2424
import java.util.function.Consumer;
2525

2626
import org.neo4j.driver.internal.Bookmarks;
27+
import org.neo4j.driver.internal.DefaultBookmarksHolder;
2728
import org.neo4j.driver.internal.ExplicitTransaction;
28-
import org.neo4j.driver.internal.NetworkSession;
2929
import org.neo4j.driver.internal.messaging.request.PullAllMessage;
3030
import org.neo4j.driver.internal.messaging.request.RunMessage;
3131
import org.neo4j.driver.internal.spi.Connection;
@@ -208,7 +208,7 @@ void shouldReleaseConnectionWhenBeginFails()
208208
{
209209
RuntimeException error = new RuntimeException( "Wrong bookmark!" );
210210
Connection connection = connectionWithBegin( handler -> handler.onFailure( error ) );
211-
ExplicitTransaction tx = new ExplicitTransaction( connection, mock( NetworkSession.class ) );
211+
ExplicitTransaction tx = new ExplicitTransaction( connection, new DefaultBookmarksHolder() );
212212

213213
Bookmarks bookmarks = Bookmarks.from( "SomeBookmark" );
214214
TransactionConfig txConfig = TransactionConfig.empty();
@@ -223,7 +223,7 @@ void shouldReleaseConnectionWhenBeginFails()
223223
void shouldNotReleaseConnectionWhenBeginSucceeds()
224224
{
225225
Connection connection = connectionWithBegin( handler -> handler.onSuccess( emptyMap() ) );
226-
ExplicitTransaction tx = new ExplicitTransaction( connection, mock( NetworkSession.class ) );
226+
ExplicitTransaction tx = new ExplicitTransaction( connection, new DefaultBookmarksHolder() );
227227

228228
Bookmarks bookmarks = Bookmarks.from( "SomeBookmark" );
229229
TransactionConfig txConfig = TransactionConfig.empty();
@@ -237,7 +237,7 @@ void shouldNotReleaseConnectionWhenBeginSucceeds()
237237
void shouldReleaseConnectionWhenTerminatedAndCommitted()
238238
{
239239
Connection connection = connectionMock();
240-
ExplicitTransaction tx = new ExplicitTransaction( connection, mock( NetworkSession.class ) );
240+
ExplicitTransaction tx = new ExplicitTransaction( connection, new DefaultBookmarksHolder() );
241241

242242
tx.markTerminated();
243243

@@ -251,7 +251,7 @@ void shouldReleaseConnectionWhenTerminatedAndCommitted()
251251
void shouldReleaseConnectionWhenTerminatedAndRolledBack()
252252
{
253253
Connection connection = connectionMock();
254-
ExplicitTransaction tx = new ExplicitTransaction( connection, mock( NetworkSession.class ) );
254+
ExplicitTransaction tx = new ExplicitTransaction( connection, new DefaultBookmarksHolder() );
255255

256256
tx.markTerminated();
257257
await( tx.rollbackAsync() );
@@ -266,12 +266,7 @@ private static ExplicitTransaction beginTx( Connection connection )
266266

267267
private static ExplicitTransaction beginTx( Connection connection, Bookmarks initialBookmarks )
268268
{
269-
return beginTx( connection, mock( NetworkSession.class ), initialBookmarks );
270-
}
271-
272-
private static ExplicitTransaction beginTx( Connection connection, NetworkSession session, Bookmarks initialBookmarks )
273-
{
274-
ExplicitTransaction tx = new ExplicitTransaction( connection, session );
269+
ExplicitTransaction tx = new ExplicitTransaction( connection, new DefaultBookmarksHolder() );
275270
return await( tx.beginAsync( initialBookmarks, TransactionConfig.empty() ) );
276271
}
277272

0 commit comments

Comments
 (0)