Skip to content

Commit 9bcd4b2

Browse files
lutovichlutovich
committed
Address review comments
 * improve method naming in `Bookmark` * handle `null` bookmark in given `Iterable`
1 parent f186165 commit 9bcd4b2

File tree

6 files changed

+54
-28
lines changed

6 files changed

+54
-28
lines changed

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -76,12 +76,12 @@ public boolean isEmpty()
7676
return maxValue == null;
7777
}
7878

79-
public String asString()
79+
public String maxBookmarkAsString()
8080
{
8181
return maxValue;
8282
}
8383

84-
public Map<String,Value> asParameters()
84+
public Map<String,Value> asBeginTransactionParameters()
8585
{
8686
if ( isEmpty() )
8787
{
@@ -138,7 +138,7 @@ private static String maxBookmark( Iterable<String> bookmarks )
138138

139139
private static long bookmarkValue( String value )
140140
{
141-
if ( value.startsWith( BOOKMARK_PREFIX ) )
141+
if ( value != null && value.startsWith( BOOKMARK_PREFIX ) )
142142
{
143143
try
144144
{

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ void setBookmark( Bookmark bookmark )
252252
private static void runBeginStatement( Connection connection, Bookmark bookmark )
253253
{
254254
Bookmark initialBookmark = bookmark == null ? Bookmark.empty() : bookmark;
255-
Map<String,Value> parameters = initialBookmark.asParameters();
255+
Map<String,Value> parameters = initialBookmark.asBeginTransactionParameters();
256256

257257
connection.run( "BEGIN", parameters, Collector.NO_OP );
258258
connection.pullAll( Collector.NO_OP );

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

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ void setBookmark( Bookmark bookmark )
206206
@Override
207207
public String lastBookmark()
208208
{
209-
return bookmark == null ? null : bookmark.asString();
209+
return bookmark == null ? null : bookmark.maxBookmarkAsString();
210210
}
211211

212212
@Override

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

Lines changed: 41 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222

2323
import java.util.Collections;
2424
import java.util.HashMap;
25+
import java.util.List;
2526
import java.util.Map;
2627

2728
import org.neo4j.driver.v1.Value;
@@ -44,17 +45,17 @@ public void isEmptyForEmptyBookmark()
4445
}
4546

4647
@Test
47-
public void asStringForEmptyBookmark()
48+
public void maxBookmarkAsStringForEmptyBookmark()
4849
{
4950
Bookmark bookmark = Bookmark.empty();
50-
assertNull( bookmark.asString() );
51+
assertNull( bookmark.maxBookmarkAsString() );
5152
}
5253

5354
@Test
54-
public void asParametersForEmptyBookmark()
55+
public void asBeginTransactionParametersForEmptyBookmark()
5556
{
5657
Bookmark bookmark = Bookmark.empty();
57-
assertEquals( emptyMap(), bookmark.asParameters() );
58+
assertEquals( emptyMap(), bookmark.asBeginTransactionParameters() );
5859
}
5960

6061
@Test
@@ -65,14 +66,14 @@ public void isEmptyForNonEmptyBookmark()
6566
}
6667

6768
@Test
68-
public void asStringForNonEmptyBookmark()
69+
public void maxBookmarkAsStringForNonEmptyBookmark()
6970
{
7071
Bookmark bookmark = Bookmark.from( "SomeBookmark" );
71-
assertEquals( "SomeBookmark", bookmark.asString() );
72+
assertEquals( "SomeBookmark", bookmark.maxBookmarkAsString() );
7273
}
7374

7475
@Test
75-
public void asParametersForNonEmptyBookmark()
76+
public void asBeginTransactionParametersForNonEmptyBookmark()
7677
{
7778
Bookmark bookmark = Bookmark.from( "SomeBookmark" );
7879
verifyParameters( bookmark, "SomeBookmark", "SomeBookmark" );
@@ -82,7 +83,7 @@ public void asParametersForNonEmptyBookmark()
8283
public void bookmarkFromString()
8384
{
8485
Bookmark bookmark = Bookmark.from( "Cat" );
85-
assertEquals( "Cat", bookmark.asString() );
86+
assertEquals( "Cat", bookmark.maxBookmarkAsString() );
8687
verifyParameters( bookmark, "Cat", "Cat" );
8788
}
8889

@@ -98,7 +99,7 @@ public void bookmarkFromIterable()
9899
{
99100
Bookmark bookmark = Bookmark.from( asList(
100101
"neo4j:bookmark:v1:tx42", "neo4j:bookmark:v1:tx10", "neo4j:bookmark:v1:tx12" ) );
101-
assertEquals( "neo4j:bookmark:v1:tx42", bookmark.asString() );
102+
assertEquals( "neo4j:bookmark:v1:tx42", bookmark.maxBookmarkAsString() );
102103
verifyParameters( bookmark,
103104
"neo4j:bookmark:v1:tx42",
104105
"neo4j:bookmark:v1:tx42", "neo4j:bookmark:v1:tx10", "neo4j:bookmark:v1:tx12" );
@@ -119,21 +120,46 @@ public void bookmarkFromEmptyIterable()
119120
}
120121

121122
@Test
122-
public void asParametersForBookmarkWithInvalidValue()
123+
public void asBeginTransactionParametersForBookmarkWithInvalidValue()
123124
{
124125
Bookmark bookmark = Bookmark.from( asList(
125126
"neo4j:bookmark:v1:tx1", "neo4j:bookmark:v1:txcat", "neo4j:bookmark:v1:tx3" ) );
126-
assertEquals( "neo4j:bookmark:v1:tx3", bookmark.asString() );
127+
assertEquals( "neo4j:bookmark:v1:tx3", bookmark.maxBookmarkAsString() );
127128
verifyParameters( bookmark,
128129
"neo4j:bookmark:v1:tx3",
129130
"neo4j:bookmark:v1:tx1", "neo4j:bookmark:v1:txcat", "neo4j:bookmark:v1:tx3" );
130131
}
131132

132-
private static void verifyParameters( Bookmark bookmark, String bookmarkValue, String... bookmarkValues )
133+
@Test
134+
public void asBeginTransactionParametersForBookmarkWithEmptyStringValue()
135+
{
136+
Bookmark bookmark = Bookmark.from( asList( "neo4j:bookmark:v1:tx9", "", "neo4j:bookmark:v1:tx3" ) );
137+
assertEquals( "neo4j:bookmark:v1:tx9", bookmark.maxBookmarkAsString() );
138+
verifyParameters( bookmark,
139+
"neo4j:bookmark:v1:tx9",
140+
"neo4j:bookmark:v1:tx9", "", "neo4j:bookmark:v1:tx3" );
141+
}
142+
143+
@Test
144+
public void asBeginTransactionParametersForBookmarkWithNullValue()
145+
{
146+
Bookmark bookmark = Bookmark.from( asList( "neo4j:bookmark:v1:tx41", null, "neo4j:bookmark:v1:tx42" ) );
147+
assertEquals( "neo4j:bookmark:v1:tx42", bookmark.maxBookmarkAsString() );
148+
verifyParameters( bookmark,
149+
"neo4j:bookmark:v1:tx42",
150+
asList( "neo4j:bookmark:v1:tx41", null, "neo4j:bookmark:v1:tx42" ) );
151+
}
152+
153+
private static void verifyParameters( Bookmark bookmark, String expectedMaxValue, String... expectedValues )
154+
{
155+
verifyParameters( bookmark, expectedMaxValue, asList( expectedValues ) );
156+
}
157+
158+
private static void verifyParameters( Bookmark bookmark, String expectedMaxValue, List<String> expectedValues )
133159
{
134160
Map<String,Value> expectedParameters = new HashMap<>();
135-
expectedParameters.put( "bookmark", value( bookmarkValue ) );
136-
expectedParameters.put( "bookmarks", value( bookmarkValues ) );
137-
assertEquals( expectedParameters, bookmark.asParameters() );
161+
expectedParameters.put( "bookmark", value( expectedMaxValue ) );
162+
expectedParameters.put( "bookmarks", value( expectedValues ) );
163+
assertEquals( expectedParameters, bookmark.asBeginTransactionParameters() );
138164
}
139165
}

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

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -140,7 +140,7 @@ public void shouldSyncWhenBookmarkGiven()
140140

141141
new ExplicitTransaction( connection, mock( SessionResourcesHandler.class ), bookmark );
142142

143-
Map<String,Value> expectedParams = bookmark.asParameters();
143+
Map<String,Value> expectedParams = bookmark.asBeginTransactionParameters();
144144

145145
InOrder inOrder = inOrder( connection );
146146
inOrder.verify( connection ).run( "BEGIN", expectedParams, Collector.NO_OP );
@@ -239,19 +239,19 @@ public void shouldNotOverwriteBookmarkWithNull()
239239
{
240240
ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( SessionResourcesHandler.class ) );
241241
tx.setBookmark( Bookmark.from( "Cat" ) );
242-
assertEquals( "Cat", tx.bookmark().asString() );
242+
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
243243
tx.setBookmark( null );
244-
assertEquals( "Cat", tx.bookmark().asString() );
244+
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
245245
}
246246

247247
@Test
248248
public void shouldNotOverwriteBookmarkWithEmptyBookmark()
249249
{
250250
ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( SessionResourcesHandler.class ) );
251251
tx.setBookmark( Bookmark.from( "Cat" ) );
252-
assertEquals( "Cat", tx.bookmark().asString() );
252+
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
253253
tx.setBookmark( Bookmark.empty() );
254-
assertEquals( "Cat", tx.bookmark().asString() );
254+
assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() );
255255
}
256256

257257
private static Connection openConnectionMock()

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

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -563,7 +563,7 @@ public void bookmarkIsPropagatedBetweenTransactions()
563563
setBookmark( tx, bookmark1 );
564564
}
565565

566-
assertEquals( bookmark1.asString(), session.lastBookmark() );
566+
assertEquals( bookmark1.maxBookmarkAsString(), session.lastBookmark() );
567567

568568
try ( Transaction tx = session.beginTransaction() )
569569
{
@@ -572,7 +572,7 @@ public void bookmarkIsPropagatedBetweenTransactions()
572572
setBookmark( tx, bookmark2 );
573573
}
574574

575-
assertEquals( bookmark2.asString(), session.lastBookmark() );
575+
assertEquals( bookmark2.maxBookmarkAsString(), session.lastBookmark() );
576576
}
577577

578578
@Test
@@ -1056,7 +1056,7 @@ private static void verifyBeginTx( PooledConnection connectionMock, Verification
10561056

10571057
private static void verifyBeginTx( PooledConnection connectionMock, Bookmark bookmark )
10581058
{
1059-
verify( connectionMock ).run( "BEGIN", bookmark.asParameters(), NO_OP );
1059+
verify( connectionMock ).run( "BEGIN", bookmark.asBeginTransactionParameters(), NO_OP );
10601060
}
10611061

10621062
private static void verifyCommitTx( PooledConnection connectionMock, VerificationMode mode )

0 commit comments

Comments
 (0)