diff --git a/driver/src/main/java/org/neo4j/driver/internal/Bookmark.java b/driver/src/main/java/org/neo4j/driver/internal/Bookmark.java new file mode 100644 index 0000000000..a7729f15dc --- /dev/null +++ b/driver/src/main/java/org/neo4j/driver/internal/Bookmark.java @@ -0,0 +1,154 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal; + +import java.util.Collections; +import java.util.HashMap; +import java.util.Iterator; +import java.util.Map; + +import org.neo4j.driver.v1.Value; + +import static java.util.Collections.emptyMap; +import static java.util.Collections.singleton; +import static org.neo4j.driver.v1.Values.value; + +public final class Bookmark +{ + private static final String BOOKMARK_KEY = "bookmark"; + private static final String BOOKMARKS_KEY = "bookmarks"; + private static final String BOOKMARK_PREFIX = "neo4j:bookmark:v1:tx"; + + private static final long UNKNOWN_BOOKMARK_VALUE = -1; + + private static final Bookmark EMPTY = new Bookmark( Collections.emptySet() ); + + private final Iterable values; + private final String maxValue; + + private Bookmark( Iterable values ) + { + this.values = values; + this.maxValue = maxBookmark( values ); + } + + public static Bookmark empty() + { + return EMPTY; + } + + public static Bookmark from( String value ) + { + if ( value == null ) + { + return empty(); + } + return from( singleton( value ) ); + } + + public static Bookmark from( Iterable values ) + { + if ( values == null ) + { + return empty(); + } + return new Bookmark( values ); + } + + public boolean isEmpty() + { + return maxValue == null; + } + + public String maxBookmarkAsString() + { + return maxValue; + } + + public Map asBeginTransactionParameters() + { + if ( isEmpty() ) + { + return emptyMap(); + } + + // Driver sends {bookmark: "max", bookmarks: ["one", "two", "max"]} instead of simple + // {bookmarks: ["one", "two", "max"]} for backwards compatibility reasons. Old servers can only accept single + // bookmark that is why driver has to parse and compare given list of bookmarks. This functionality will + // eventually be removed. + Map parameters = new HashMap<>( 4 ); + parameters.put( BOOKMARK_KEY, value( maxValue ) ); + parameters.put( BOOKMARKS_KEY, value( values ) ); + return parameters; + } + + @Override + public String toString() + { + return "Bookmark{values=" + values + "}"; + } + + private static String maxBookmark( Iterable bookmarks ) + { + if ( bookmarks == null ) + { + return null; + } + + Iterator iterator = bookmarks.iterator(); + + if ( !iterator.hasNext() ) + { + return null; + } + + String maxBookmark = iterator.next(); + long maxValue = bookmarkValue( maxBookmark ); + + while ( iterator.hasNext() ) + { + String bookmark = iterator.next(); + long value = bookmarkValue( bookmark ); + + if ( value > maxValue ) + { + maxBookmark = bookmark; + maxValue = value; + } + } + + return maxBookmark; + } + + private static long bookmarkValue( String value ) + { + if ( value != null && value.startsWith( BOOKMARK_PREFIX ) ) + { + try + { + return Long.parseLong( value.substring( BOOKMARK_PREFIX.length() ) ); + } + catch ( NumberFormatException e ) + { + return UNKNOWN_BOOKMARK_VALUE; + } + } + return UNKNOWN_BOOKMARK_VALUE; + } +} diff --git a/driver/src/main/java/org/neo4j/driver/internal/BookmarkCollector.java b/driver/src/main/java/org/neo4j/driver/internal/BookmarkCollector.java index adb0aa2f48..ae8529f247 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/BookmarkCollector.java +++ b/driver/src/main/java/org/neo4j/driver/internal/BookmarkCollector.java @@ -30,7 +30,7 @@ class BookmarkCollector extends NoOperationCollector } @Override - public void bookmark( String bookmark ) + public void bookmark( Bookmark bookmark ) { transaction.setBookmark( bookmark ); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java b/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java index 12f33c3cce..4701ec2314 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java +++ b/driver/src/main/java/org/neo4j/driver/internal/ExplicitTransaction.java @@ -34,8 +34,6 @@ import org.neo4j.driver.v1.exceptions.Neo4jException; import org.neo4j.driver.v1.types.TypeSystem; -import static java.util.Collections.emptyMap; -import static java.util.Collections.singletonMap; import static org.neo4j.driver.v1.Values.ofValue; import static org.neo4j.driver.v1.Values.value; @@ -68,19 +66,19 @@ private enum State private final SessionResourcesHandler resourcesHandler; private final Connection conn; - private String bookmark = null; + private Bookmark bookmark = Bookmark.empty(); private State state = State.ACTIVE; public ExplicitTransaction( Connection conn, SessionResourcesHandler resourcesHandler ) { - this( conn, resourcesHandler, null ); + this( conn, resourcesHandler, Bookmark.empty() ); } - ExplicitTransaction( Connection conn, SessionResourcesHandler resourcesHandler, String bookmark ) + ExplicitTransaction( Connection conn, SessionResourcesHandler resourcesHandler, Bookmark initialBookmark ) { this.conn = conn; this.resourcesHandler = resourcesHandler; - runBeginStatement( conn, bookmark ); + runBeginStatement( conn, initialBookmark ); } @Override @@ -238,32 +236,28 @@ public synchronized void markToClose() state = State.FAILED; } - public String bookmark() + public Bookmark bookmark() { return bookmark; } - void setBookmark( String bookmark ) + void setBookmark( Bookmark bookmark ) { - this.bookmark = bookmark; + if ( bookmark != null && !bookmark.isEmpty() ) + { + this.bookmark = bookmark; + } } - private static void runBeginStatement( Connection connection, String bookmark ) + private static void runBeginStatement( Connection connection, Bookmark bookmark ) { - Map parameters; - if ( bookmark != null ) - { - parameters = singletonMap( "bookmark", value( bookmark ) ); - } - else - { - parameters = emptyMap(); - } + Bookmark initialBookmark = bookmark == null ? Bookmark.empty() : bookmark; + Map parameters = initialBookmark.asBeginTransactionParameters(); connection.run( "BEGIN", parameters, Collector.NO_OP ); connection.pullAll( Collector.NO_OP ); - if ( bookmark != null ) + if ( !initialBookmark.isEmpty() ) { connection.sync(); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/InternalDriver.java b/driver/src/main/java/org/neo4j/driver/internal/InternalDriver.java index 094ff8ec19..97474bd529 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/InternalDriver.java +++ b/driver/src/main/java/org/neo4j/driver/internal/InternalDriver.java @@ -18,6 +18,7 @@ */ package org.neo4j.driver.internal; +import java.util.Collection; import java.util.concurrent.atomic.AtomicBoolean; import org.neo4j.driver.internal.security.SecurityPlan; @@ -62,7 +63,7 @@ public final Session session() @Override public final Session session( AccessMode mode ) { - return session( mode, null ); + return newSession( mode, Bookmark.empty() ); } @Override @@ -73,6 +74,23 @@ public final Session session( String bookmark ) @Override public final Session session( AccessMode mode, String bookmark ) + { + return newSession( mode, Bookmark.from( bookmark ) ); + } + + @Override + public Session session( Iterable bookmarks ) + { + return session( AccessMode.WRITE, bookmarks ); + } + + @Override + public Session session( AccessMode mode, Iterable bookmarks ) + { + return newSession( mode, Bookmark.from( bookmarks ) ); + } + + private Session newSession( AccessMode mode, Bookmark bookmark ) { assertOpen(); Session session = sessionFactory.newInstance( mode, bookmark ); diff --git a/driver/src/main/java/org/neo4j/driver/internal/InternalStatementResult.java b/driver/src/main/java/org/neo4j/driver/internal/InternalStatementResult.java index c6701209bc..23f2c785d5 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/InternalStatementResult.java +++ b/driver/src/main/java/org/neo4j/driver/internal/InternalStatementResult.java @@ -138,7 +138,7 @@ public void notifications( List notifications ) } @Override - public void bookmark( String bookmark ) + public void bookmark( Bookmark bookmark ) { if ( transaction != null ) { diff --git a/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java b/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java index 43607c1b49..8f2c78fe7d 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java +++ b/driver/src/main/java/org/neo4j/driver/internal/NetworkSession.java @@ -53,7 +53,7 @@ public class NetworkSession implements Session, SessionResourcesHandler private final RetryLogic retryLogic; protected final Logger logger; - private String bookmark; + private Bookmark bookmark = Bookmark.empty(); private PooledConnection currentConnection; private ExplicitTransaction currentTransaction; @@ -179,7 +179,7 @@ public synchronized Transaction beginTransaction() @Override public synchronized Transaction beginTransaction( String bookmark ) { - setBookmark( bookmark ); + setBookmark( Bookmark.from( bookmark ) ); return beginTransaction(); } @@ -195,12 +195,9 @@ public T writeTransaction( TransactionWork work ) return transaction( AccessMode.WRITE, work ); } - // Internal method for setting the bookmark explicitly, mainly for testing. - // This method does not prevent setting the bookmark to null since that - // is a valid requirement for some test scenarios. - void setBookmark( String bookmark ) + void setBookmark( Bookmark bookmark ) { - if( bookmark != null ) + if ( bookmark != null && !bookmark.isEmpty() ) { this.bookmark = bookmark; } @@ -209,7 +206,7 @@ void setBookmark( String bookmark ) @Override public String lastBookmark() { - return bookmark; + return bookmark == null ? null : bookmark.maxBookmarkAsString(); } @Override diff --git a/driver/src/main/java/org/neo4j/driver/internal/SessionFactory.java b/driver/src/main/java/org/neo4j/driver/internal/SessionFactory.java index c4b2e6c6b5..fefc799536 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/SessionFactory.java +++ b/driver/src/main/java/org/neo4j/driver/internal/SessionFactory.java @@ -23,5 +23,5 @@ public interface SessionFactory extends AutoCloseable { - Session newInstance( AccessMode mode, String bookmark ); + Session newInstance( AccessMode mode, Bookmark bookmark ); } diff --git a/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java b/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java index 431d133b4f..6d633019ec 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java +++ b/driver/src/main/java/org/neo4j/driver/internal/SessionFactoryImpl.java @@ -27,10 +27,10 @@ public class SessionFactoryImpl implements SessionFactory { - protected final ConnectionProvider connectionProvider; - protected final RetryLogic retryLogic; - protected final Logging logging; - protected final boolean leakedSessionsLoggingEnabled; + private final ConnectionProvider connectionProvider; + private final RetryLogic retryLogic; + private final Logging logging; + private final boolean leakedSessionsLoggingEnabled; SessionFactoryImpl( ConnectionProvider connectionProvider, RetryLogic retryLogic, Config config ) { @@ -41,23 +41,23 @@ public class SessionFactoryImpl implements SessionFactory } @Override - public Session newInstance( AccessMode mode, String bookmark ) + public final Session newInstance( AccessMode mode, Bookmark bookmark ) { - NetworkSession session; - if ( leakedSessionsLoggingEnabled ) - { - session = new LeakLoggingNetworkSession( connectionProvider, mode, retryLogic, logging ); - } - else - { - session = new NetworkSession( connectionProvider, mode, retryLogic, logging ); - } + NetworkSession session = createSession( connectionProvider, retryLogic, mode, logging ); session.setBookmark( bookmark ); return session; } + protected NetworkSession createSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, + AccessMode mode, Logging logging ) + { + return leakedSessionsLoggingEnabled ? + new LeakLoggingNetworkSession( connectionProvider, mode, retryLogic, logging ) : + new NetworkSession( connectionProvider, mode, retryLogic, logging ); + } + @Override - public void close() throws Exception + public final void close() throws Exception { connectionProvider.close(); } @@ -69,7 +69,7 @@ public void close() throws Exception * * @return the connection provider used by this factory. */ - public ConnectionProvider getConnectionProvider() + public final ConnectionProvider getConnectionProvider() { return connectionProvider; } diff --git a/driver/src/main/java/org/neo4j/driver/internal/net/SocketResponseHandler.java b/driver/src/main/java/org/neo4j/driver/internal/net/SocketResponseHandler.java index 2ca4e0f089..b7dda67019 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/net/SocketResponseHandler.java +++ b/driver/src/main/java/org/neo4j/driver/internal/net/SocketResponseHandler.java @@ -22,7 +22,7 @@ import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; -import org.neo4j.driver.v1.exceptions.AuthenticationException; +import org.neo4j.driver.internal.Bookmark; import org.neo4j.driver.internal.messaging.MessageHandler; import org.neo4j.driver.internal.spi.Collector; import org.neo4j.driver.internal.summary.InternalNotification; @@ -30,6 +30,7 @@ import org.neo4j.driver.internal.summary.InternalProfiledPlan; import org.neo4j.driver.internal.summary.InternalSummaryCounters; import org.neo4j.driver.v1.Value; +import org.neo4j.driver.v1.exceptions.AuthenticationException; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.DatabaseException; import org.neo4j.driver.v1.exceptions.Neo4jException; @@ -206,7 +207,7 @@ private void collectBookmark( Collector collector, Value bookmark ) { if ( bookmark != null ) { - collector.bookmark( bookmark.asString() ); + collector.bookmark( Bookmark.from( bookmark.asString() ) ); } } diff --git a/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java b/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java index 4cdfbcbd39..dd056f4d05 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java +++ b/driver/src/main/java/org/neo4j/driver/internal/spi/Collector.java @@ -20,6 +20,7 @@ import java.util.List; +import org.neo4j.driver.internal.Bookmark; import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.Neo4jException; @@ -130,7 +131,9 @@ public void profile( ProfiledPlan plan ) {} public void notifications( List notifications ) {} @Override - public void bookmark( String bookmark ) {} + public void bookmark( Bookmark bookmark ) + { + } @Override public void done() {} @@ -179,7 +182,7 @@ public void serverVersion( String server ){} void notifications( List notifications ); - void bookmark( String bookmark ); + void bookmark( Bookmark bookmark ); void done(); diff --git a/driver/src/main/java/org/neo4j/driver/internal/summary/SummaryBuilder.java b/driver/src/main/java/org/neo4j/driver/internal/summary/SummaryBuilder.java index dfcfb6e054..cd1b4dc577 100644 --- a/driver/src/main/java/org/neo4j/driver/internal/summary/SummaryBuilder.java +++ b/driver/src/main/java/org/neo4j/driver/internal/summary/SummaryBuilder.java @@ -22,6 +22,7 @@ import java.util.List; import java.util.concurrent.TimeUnit; +import org.neo4j.driver.internal.Bookmark; import org.neo4j.driver.internal.spi.Collector; import org.neo4j.driver.v1.Statement; import org.neo4j.driver.v1.Value; @@ -131,7 +132,7 @@ public void notifications( List notifications ) } @Override - public void bookmark( String bookmark ) + public void bookmark( Bookmark bookmark ) { // intentionally empty } diff --git a/driver/src/main/java/org/neo4j/driver/v1/Driver.java b/driver/src/main/java/org/neo4j/driver/v1/Driver.java index ef9f5e1d39..679126dd90 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/Driver.java +++ b/driver/src/main/java/org/neo4j/driver/v1/Driver.java @@ -77,7 +77,7 @@ public interface Driver extends AutoCloseable * Alias to {@code session(mode, null)}. * * @param mode the type of access required by units of work in this session, - * e.g. {@link AccessMode#READ read access}. + * e.g. {@link AccessMode#READ read access} or {@link AccessMode#WRITE write access}. * @return a new {@link Session} object. */ Session session( AccessMode mode ); @@ -101,13 +101,41 @@ public interface Driver extends AutoCloseable * transaction referenced by the supplied bookmark. * * @param mode the type of access required by units of work in this session, - * e.g. {@link AccessMode#READ read access}. + * e.g. {@link AccessMode#READ read access} or {@link AccessMode#WRITE write access}. * @param bookmark the initial reference to some previous transaction. A {@code null} value is permitted, and * indicates that the bookmark does not exist or is unknown. * @return a new {@link Session} object. */ Session session( AccessMode mode, String bookmark ); + /** + * Create a new {@link AccessMode#WRITE write} {@link Session} with specified initial bookmarks. + * First transaction in the created session will ensure that server hosting is at least as up-to-date as the + * latest transaction referenced by the supplied iterable of bookmarks. + *

+ * Alias to {@code session(AccessMode.WRITE, bookmarks)}. + * + * @param bookmarks initial references to some previous transactions. Both {@code null} value and empty iterable + * are permitted, and indicate that the bookmarks do not exist or are unknown. + * @return a new {@link Session} object. + */ + Session session( Iterable bookmarks ); + + /** + * Create a new {@link AccessMode#WRITE write} {@link Session} with specified initial bookmarks. + * First transaction in the created session will ensure that server hosting is at least as up-to-date as the + * latest transaction referenced by the supplied iterable of bookmarks. + *

+ * Alias to {@code session(AccessMode.WRITE, bookmarks)}. + * + * @param mode the type of access required by units of work in this session, + * e.g. {@link AccessMode#READ read access} or {@link AccessMode#WRITE write access}. + * @param bookmarks initial references to some previous transactions. Both {@code null} value and empty iterable + * are permitted, and indicate that the bookmarks do not exist or are unknown. + * @return a new {@link Session} object. + */ + Session session( AccessMode mode, Iterable bookmarks ); + /** * Close all the resources assigned to this driver, including any open connections. */ diff --git a/driver/src/main/java/org/neo4j/driver/v1/Session.java b/driver/src/main/java/org/neo4j/driver/v1/Session.java index 63f6f0986a..2c4e3e1609 100644 --- a/driver/src/main/java/org/neo4j/driver/v1/Session.java +++ b/driver/src/main/java/org/neo4j/driver/v1/Session.java @@ -18,6 +18,8 @@ */ package org.neo4j.driver.v1; +import java.util.Collection; + import org.neo4j.driver.v1.util.Resource; /** @@ -75,7 +77,7 @@ public interface Session extends Resource, StatementRunner * * @param bookmark a reference to a previous transaction * @return a new {@link Transaction} - * @deprecated This method is deprecated in favour of {@link Driver#session(String)} that accepts an initial + * @deprecated This method is deprecated in favour of {@link Driver#session(Collection)} that accepts an initial * bookmark. Session will ensure that all nested transactions are chained with bookmarks to guarantee * causal consistency. This method will be removed in the next major release. */ diff --git a/driver/src/test/java/org/neo4j/driver/internal/BookmarkTest.java b/driver/src/test/java/org/neo4j/driver/internal/BookmarkTest.java new file mode 100644 index 0000000000..c5ab4fcf53 --- /dev/null +++ b/driver/src/test/java/org/neo4j/driver/internal/BookmarkTest.java @@ -0,0 +1,165 @@ +/* + * Copyright (c) 2002-2017 "Neo Technology," + * Network Engine for Objects in Lund AB [http://neotechnology.com] + * + * This file is part of Neo4j. + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.neo4j.driver.internal; + +import org.junit.Test; + +import java.util.Collections; +import java.util.HashMap; +import java.util.List; +import java.util.Map; + +import org.neo4j.driver.v1.Value; + +import static java.util.Arrays.asList; +import static java.util.Collections.emptyMap; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; +import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; +import static org.neo4j.driver.v1.Values.value; + +public class BookmarkTest +{ + @Test + public void isEmptyForEmptyBookmark() + { + Bookmark bookmark = Bookmark.empty(); + assertTrue( bookmark.isEmpty() ); + } + + @Test + public void maxBookmarkAsStringForEmptyBookmark() + { + Bookmark bookmark = Bookmark.empty(); + assertNull( bookmark.maxBookmarkAsString() ); + } + + @Test + public void asBeginTransactionParametersForEmptyBookmark() + { + Bookmark bookmark = Bookmark.empty(); + assertEquals( emptyMap(), bookmark.asBeginTransactionParameters() ); + } + + @Test + public void isEmptyForNonEmptyBookmark() + { + Bookmark bookmark = Bookmark.from( "SomeBookmark" ); + assertFalse( bookmark.isEmpty() ); + } + + @Test + public void maxBookmarkAsStringForNonEmptyBookmark() + { + Bookmark bookmark = Bookmark.from( "SomeBookmark" ); + assertEquals( "SomeBookmark", bookmark.maxBookmarkAsString() ); + } + + @Test + public void asBeginTransactionParametersForNonEmptyBookmark() + { + Bookmark bookmark = Bookmark.from( "SomeBookmark" ); + verifyParameters( bookmark, "SomeBookmark", "SomeBookmark" ); + } + + @Test + public void bookmarkFromString() + { + Bookmark bookmark = Bookmark.from( "Cat" ); + assertEquals( "Cat", bookmark.maxBookmarkAsString() ); + verifyParameters( bookmark, "Cat", "Cat" ); + } + + @Test + public void bookmarkFromNullString() + { + Bookmark bookmark = Bookmark.from( (String) null ); + assertTrue( bookmark.isEmpty() ); + } + + @Test + public void bookmarkFromIterable() + { + Bookmark bookmark = Bookmark.from( asList( + "neo4j:bookmark:v1:tx42", "neo4j:bookmark:v1:tx10", "neo4j:bookmark:v1:tx12" ) ); + assertEquals( "neo4j:bookmark:v1:tx42", bookmark.maxBookmarkAsString() ); + verifyParameters( bookmark, + "neo4j:bookmark:v1:tx42", + "neo4j:bookmark:v1:tx42", "neo4j:bookmark:v1:tx10", "neo4j:bookmark:v1:tx12" ); + } + + @Test + public void bookmarkFromNullIterable() + { + Bookmark bookmark = Bookmark.from( (Iterable) null ); + assertTrue( bookmark.isEmpty() ); + } + + @Test + public void bookmarkFromEmptyIterable() + { + Bookmark bookmark = Bookmark.from( Collections.emptyList() ); + assertTrue( bookmark.isEmpty() ); + } + + @Test + public void asBeginTransactionParametersForBookmarkWithInvalidValue() + { + Bookmark bookmark = Bookmark.from( asList( + "neo4j:bookmark:v1:tx1", "neo4j:bookmark:v1:txcat", "neo4j:bookmark:v1:tx3" ) ); + assertEquals( "neo4j:bookmark:v1:tx3", bookmark.maxBookmarkAsString() ); + verifyParameters( bookmark, + "neo4j:bookmark:v1:tx3", + "neo4j:bookmark:v1:tx1", "neo4j:bookmark:v1:txcat", "neo4j:bookmark:v1:tx3" ); + } + + @Test + public void asBeginTransactionParametersForBookmarkWithEmptyStringValue() + { + Bookmark bookmark = Bookmark.from( asList( "neo4j:bookmark:v1:tx9", "", "neo4j:bookmark:v1:tx3" ) ); + assertEquals( "neo4j:bookmark:v1:tx9", bookmark.maxBookmarkAsString() ); + verifyParameters( bookmark, + "neo4j:bookmark:v1:tx9", + "neo4j:bookmark:v1:tx9", "", "neo4j:bookmark:v1:tx3" ); + } + + @Test + public void asBeginTransactionParametersForBookmarkWithNullValue() + { + Bookmark bookmark = Bookmark.from( asList( "neo4j:bookmark:v1:tx41", null, "neo4j:bookmark:v1:tx42" ) ); + assertEquals( "neo4j:bookmark:v1:tx42", bookmark.maxBookmarkAsString() ); + verifyParameters( bookmark, + "neo4j:bookmark:v1:tx42", + asList( "neo4j:bookmark:v1:tx41", null, "neo4j:bookmark:v1:tx42" ) ); + } + + private static void verifyParameters( Bookmark bookmark, String expectedMaxValue, String... expectedValues ) + { + verifyParameters( bookmark, expectedMaxValue, asList( expectedValues ) ); + } + + private static void verifyParameters( Bookmark bookmark, String expectedMaxValue, List expectedValues ) + { + Map expectedParameters = new HashMap<>(); + expectedParameters.put( "bookmark", value( expectedMaxValue ) ); + expectedParameters.put( "bookmarks", value( expectedValues ) ); + assertEquals( expectedParameters, bookmark.asBeginTransactionParameters() ); + } +} diff --git a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java index 90dcf93eed..7984d66e83 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/DirectDriverTest.java @@ -23,6 +23,8 @@ import org.junit.Test; import java.net.URI; +import java.util.Arrays; +import java.util.List; import org.neo4j.driver.internal.net.BoltServerAddress; import org.neo4j.driver.internal.util.ServerVersion; @@ -30,11 +32,13 @@ import org.neo4j.driver.v1.GraphDatabase; import org.neo4j.driver.v1.Record; import org.neo4j.driver.v1.Session; +import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.util.StubServer; import org.neo4j.driver.v1.util.TestNeo4j; import static org.hamcrest.Matchers.is; import static org.hamcrest.core.IsEqual.equalTo; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertThat; import static org.junit.Assert.fail; import static org.junit.Assume.assumeTrue; @@ -98,9 +102,9 @@ public void shouldRejectInvalidAddress() try { driver = GraphDatabase.driver( uri, neo4j.authToken() ); - fail("Expecting error for wrong uri"); + fail( "Expecting error for wrong uri" ); } - catch( IllegalArgumentException e ) + catch ( IllegalArgumentException e ) { assertThat( e.getMessage(), equalTo( "Invalid URI format `*`" ) ); } @@ -145,6 +149,32 @@ public void shouldBeAbleRunCypher() throws Exception assertThat( server.exitStatus(), equalTo( 0 ) ); } + @Test + public void shouldSendMultipleBookmarks() throws Exception + { + StubServer server = StubServer.start( "multiple_bookmarks.script", 9001 ); + + List bookmarks = Arrays.asList( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", + "neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16", + "neo4j:bookmark:v1:tx68" ); + + try ( Driver driver = GraphDatabase.driver( "bolt://localhost:9001", INSECURE_CONFIG ); + Session session = driver.session( bookmarks ) ) + { + try ( Transaction tx = session.beginTransaction() ) + { + tx.run( "CREATE (n {name:'Bob'})" ); + tx.success(); + } + + assertEquals( "neo4j:bookmark:v1:tx95", session.lastBookmark() ); + } + finally + { + assertEquals( 0, server.exitStatus() ); + } + } + /** * Check if running test neo4j instance supports {@value org.neo4j.driver.v1.util.Neo4jSettings#LISTEN_ADDR} * configuration option. Only 3.1+ versions support it. diff --git a/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java b/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java index 465fbc4097..ba41de32c5 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/ExplicitTransactionTest.java @@ -29,6 +29,7 @@ import org.neo4j.driver.v1.Transaction; import org.neo4j.driver.v1.Value; +import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertTrue; import static org.mockito.Matchers.any; @@ -39,7 +40,6 @@ import static org.mockito.Mockito.verify; import static org.mockito.Mockito.verifyNoMoreInteractions; import static org.mockito.Mockito.when; -import static org.neo4j.driver.v1.Values.value; public class ExplicitTransactionTest { @@ -135,12 +135,12 @@ public void shouldOnlyQueueMessagesWhenNoBookmarkGiven() @Test public void shouldSyncWhenBookmarkGiven() { - String bookmark = "hi, I'm bookmark"; + Bookmark bookmark = Bookmark.from( "hi, I'm bookmark" ); Connection connection = mock( Connection.class ); new ExplicitTransaction( connection, mock( SessionResourcesHandler.class ), bookmark ); - Map expectedParams = Collections.singletonMap( "bookmark", value( bookmark ) ); + Map expectedParams = bookmark.asBeginTransactionParameters(); InOrder inOrder = inOrder( connection ); inOrder.verify( connection ).run( "BEGIN", expectedParams, Collector.NO_OP ); @@ -219,6 +219,41 @@ public void shouldBeClosedWhenMarkedToCloseAndClosed() assertFalse( tx.isOpen() ); } + @Test + public void shouldHaveEmptyBookmarkInitially() + { + ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( SessionResourcesHandler.class ) ); + assertTrue( tx.bookmark().isEmpty() ); + } + + @Test + public void shouldNotKeepInitialBookmark() + { + ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( SessionResourcesHandler.class ), + Bookmark.from( "Dog" ) ); + assertTrue( tx.bookmark().isEmpty() ); + } + + @Test + public void shouldNotOverwriteBookmarkWithNull() + { + ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( SessionResourcesHandler.class ) ); + tx.setBookmark( Bookmark.from( "Cat" ) ); + assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() ); + tx.setBookmark( null ); + assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() ); + } + + @Test + public void shouldNotOverwriteBookmarkWithEmptyBookmark() + { + ExplicitTransaction tx = new ExplicitTransaction( openConnectionMock(), mock( SessionResourcesHandler.class ) ); + tx.setBookmark( Bookmark.from( "Cat" ) ); + assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() ); + tx.setBookmark( Bookmark.empty() ); + assertEquals( "Cat", tx.bookmark().maxBookmarkAsString() ); + } + private static Connection openConnectionMock() { Connection connection = mock( Connection.class ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java b/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java index 6d5f2a1a12..2cc85b6579 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/NetworkSessionTest.java @@ -44,7 +44,6 @@ import org.neo4j.driver.v1.exceptions.ServiceUnavailableException; import org.neo4j.driver.v1.exceptions.SessionExpiredException; -import static java.util.Collections.singletonMap; import static org.hamcrest.CoreMatchers.containsString; import static org.hamcrest.CoreMatchers.equalTo; import static org.hamcrest.CoreMatchers.instanceOf; @@ -72,7 +71,6 @@ import static org.neo4j.driver.internal.spi.Collector.NO_OP; import static org.neo4j.driver.v1.AccessMode.READ; import static org.neo4j.driver.v1.AccessMode.WRITE; -import static org.neo4j.driver.v1.Values.value; public class NetworkSessionTest { @@ -362,7 +360,7 @@ public void updatesBookmarkWhenTxIsClosed() NetworkSession session = newSession( connectionProvider, READ ); Transaction tx = session.beginTransaction(); - setBookmark( tx, "TheBookmark" ); + setBookmark( tx, Bookmark.from( "TheBookmark" ) ); assertNull( session.lastBookmark() ); @@ -519,7 +517,7 @@ public void closesConnectionWhenResultIsBuffered() @Test public void bookmarkIsPropagatedFromSession() { - String bookmark = "Bookmark"; + Bookmark bookmark = Bookmark.from( "Bookmark" ); ConnectionProvider connectionProvider = mock( ConnectionProvider.class ); PooledConnection connection = mock( PooledConnection.class ); @@ -535,7 +533,7 @@ public void bookmarkIsPropagatedFromSession() @Test public void bookmarkIsPropagatedInBeginTransaction() { - String bookmark = "Bookmark"; + Bookmark bookmark = Bookmark.from( "Bookmark" ); ConnectionProvider connectionProvider = mock( ConnectionProvider.class ); PooledConnection connection = mock( PooledConnection.class ); @@ -552,8 +550,8 @@ public void bookmarkIsPropagatedInBeginTransaction() @Test public void bookmarkIsPropagatedBetweenTransactions() { - String bookmark1 = "Bookmark1"; - String bookmark2 = "Bookmark2"; + Bookmark bookmark1 = Bookmark.from( "Bookmark1" ); + Bookmark bookmark2 = Bookmark.from( "Bookmark2" ); ConnectionProvider connectionProvider = mock( ConnectionProvider.class ); PooledConnection connection = mock( PooledConnection.class ); @@ -565,16 +563,16 @@ public void bookmarkIsPropagatedBetweenTransactions() setBookmark( tx, bookmark1 ); } - assertEquals( bookmark1, session.lastBookmark() ); + assertEquals( bookmark1.maxBookmarkAsString(), session.lastBookmark() ); try ( Transaction tx = session.beginTransaction() ) { verifyBeginTx( connection, bookmark1 ); - assertNull( getBookmark( tx ) ); + assertTrue( getBookmark( tx ).isEmpty() ); setBookmark( tx, bookmark2 ); } - assertEquals( bookmark2, session.lastBookmark() ); + assertEquals( bookmark2.maxBookmarkAsString(), session.lastBookmark() ); } @Test @@ -596,7 +594,7 @@ public void setLastBookmark() { NetworkSession session = newSession( mock( ConnectionProvider.class ), WRITE ); - session.setBookmark( "TheBookmark" ); + session.setBookmark( Bookmark.from( "TheBookmark" ) ); assertEquals( "TheBookmark", session.lastBookmark() ); } @@ -608,7 +606,7 @@ public void testPassingNoBookmarkShouldRetainBookmark() PooledConnection connection = openConnectionMock(); when( connectionProvider.acquireConnection( READ ) ).thenReturn( connection ); NetworkSession session = newSession( connectionProvider, READ ); - session.setBookmark( "X" ); + session.setBookmark( Bookmark.from( "X" ) ); session.beginTransaction(); assertThat( session.lastBookmark(), equalTo( "X" ) ); } @@ -621,7 +619,7 @@ public void testPassingNullBookmarkShouldRetainBookmark() PooledConnection connection = openConnectionMock(); when( connectionProvider.acquireConnection( READ ) ).thenReturn( connection ); NetworkSession session = newSession( connectionProvider, READ ); - session.setBookmark( "X" ); + session.setBookmark( Bookmark.from( "X" ) ); session.beginTransaction( null ); assertThat( session.lastBookmark(), equalTo( "X" ) ); } @@ -735,6 +733,7 @@ public void writeTxRetriedUntilFailureWhenTxCloseThrows() } @Test + @SuppressWarnings( "deprecation" ) public void transactionShouldBeOpenAfterSessionReset() { ConnectionProvider connectionProvider = mock( ConnectionProvider.class ); @@ -750,6 +749,7 @@ public void transactionShouldBeOpenAfterSessionReset() } @Test + @SuppressWarnings( "deprecation" ) public void transactionShouldBeClosedAfterSessionResetAndClose() { ConnectionProvider connectionProvider = mock( ConnectionProvider.class ); @@ -767,6 +767,31 @@ public void transactionShouldBeClosedAfterSessionResetAndClose() assertFalse( tx.isOpen() ); } + @Test + public void shouldHaveNullLastBookmarkInitially() + { + NetworkSession session = newSession( mock( ConnectionProvider.class ), READ ); + assertNull( session.lastBookmark() ); + } + + @Test + public void shouldNotOverwriteBookmarkWithNull() + { + NetworkSession session = newSession( mock( ConnectionProvider.class ), READ, Bookmark.from( "Cat" ) ); + assertEquals( "Cat", session.lastBookmark() ); + session.setBookmark( null ); + assertEquals( "Cat", session.lastBookmark() ); + } + + @Test + public void shouldNotOverwriteBookmarkWithEmptyBookmark() + { + NetworkSession session = newSession( mock( ConnectionProvider.class ), READ, Bookmark.from( "Cat" ) ); + assertEquals( "Cat", session.lastBookmark() ); + session.setBookmark( Bookmark.empty() ); + assertEquals( "Cat", session.lastBookmark() ); + } + private static void testConnectionAcquisition( AccessMode sessionMode, AccessMode transactionMode ) { ConnectionProvider connectionProvider = mock( ConnectionProvider.class ); @@ -968,21 +993,22 @@ else if ( mode == WRITE ) private static NetworkSession newSession( ConnectionProvider connectionProvider, AccessMode mode ) { - return newSession( connectionProvider, mode, null ); + return newSession( connectionProvider, mode, Bookmark.empty() ); } private static NetworkSession newSession( ConnectionProvider connectionProvider, RetryLogic retryLogic ) { - return newSession( connectionProvider, WRITE, retryLogic, null ); + return newSession( connectionProvider, WRITE, retryLogic, Bookmark.empty() ); } - private static NetworkSession newSession( ConnectionProvider connectionProvider, AccessMode mode, String bookmark ) + private static NetworkSession newSession( ConnectionProvider connectionProvider, AccessMode mode, + Bookmark bookmark ) { return newSession( connectionProvider, mode, new FixedRetryLogic( 0 ), bookmark ); } private static NetworkSession newSession( ConnectionProvider connectionProvider, AccessMode mode, - RetryLogic retryLogic, String bookmark ) + RetryLogic retryLogic, Bookmark bookmark ) { NetworkSession session = new NetworkSession( connectionProvider, mode, retryLogic, DEV_NULL_LOGGING ); session.setBookmark( bookmark ); @@ -1028,9 +1054,9 @@ private static void verifyBeginTx( PooledConnection connectionMock, Verification verifyRun( connectionMock, "BEGIN", mode ); } - private static void verifyBeginTx( PooledConnection connectionMock, String bookmark ) + private static void verifyBeginTx( PooledConnection connectionMock, Bookmark bookmark ) { - verify( connectionMock ).run( "BEGIN", singletonMap( "bookmark", value( bookmark ) ), NO_OP ); + verify( connectionMock ).run( "BEGIN", bookmark.asBeginTransactionParameters(), NO_OP ); } private static void verifyCommitTx( PooledConnection connectionMock, VerificationMode mode ) @@ -1053,12 +1079,12 @@ private static Map anyParams() return anyMapOf( String.class, Value.class ); } - private static String getBookmark( Transaction tx ) + private static Bookmark getBookmark( Transaction tx ) { return ((ExplicitTransaction) tx).bookmark(); } - private static void setBookmark( Transaction tx, String bookmark ) + private static void setBookmark( Transaction tx, Bookmark bookmark ) { ((ExplicitTransaction) tx).setBookmark( bookmark ); } diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java index d893fbeac3..8c6ea1f5a3 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverBoltKitTest.java @@ -25,6 +25,7 @@ import java.io.IOException; import java.net.URI; import java.util.ArrayList; +import java.util.Arrays; import java.util.List; import java.util.concurrent.atomic.AtomicInteger; import java.util.logging.Level; @@ -1007,6 +1008,34 @@ public void shouldTreatRoutingTableWithSingleRouterAsValid() throws Exception } } + @Test + public void shouldSendMultipleBookmarks() throws Exception + { + StubServer router = StubServer.start( "acquire_endpoints.script", 9001 ); + StubServer writer = StubServer.start( "multiple_bookmarks.script", 9007 ); + + List bookmarks = Arrays.asList( "neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", + "neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16", + "neo4j:bookmark:v1:tx68" ); + + try ( Driver driver = GraphDatabase.driver( "bolt+routing://localhost:9001", config ); + Session session = driver.session( bookmarks ) ) + { + try ( Transaction tx = session.beginTransaction() ) + { + tx.run( "CREATE (n {name:'Bob'})" ); + tx.success(); + } + + assertEquals( "neo4j:bookmark:v1:tx95", session.lastBookmark() ); + } + finally + { + assertEquals( 0, router.exitStatus() ); + assertEquals( 0, writer.exitStatus() ); + } + } + private static Driver newDriverWithSleeplessClock( String uriString ) { DriverFactory driverFactory = new DriverFactoryWithClock( new SleeplessClock() ); diff --git a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java index 97403a5115..ec857b03e0 100644 --- a/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java +++ b/driver/src/test/java/org/neo4j/driver/internal/RoutingDriverTest.java @@ -33,6 +33,7 @@ import org.neo4j.driver.internal.cluster.RoutingSettings; import org.neo4j.driver.internal.net.BoltServerAddress; import org.neo4j.driver.internal.retry.FixedRetryLogic; +import org.neo4j.driver.internal.retry.RetryLogic; import org.neo4j.driver.internal.spi.Collector; import org.neo4j.driver.internal.spi.ConnectionPool; import org.neo4j.driver.internal.spi.ConnectionProvider; @@ -45,7 +46,6 @@ import org.neo4j.driver.v1.EventLogger; import org.neo4j.driver.v1.GraphDatabase; import org.neo4j.driver.v1.Logging; -import org.neo4j.driver.v1.Session; import org.neo4j.driver.v1.Value; import org.neo4j.driver.v1.exceptions.ClientException; import org.neo4j.driver.v1.exceptions.ProtocolException; @@ -440,11 +440,10 @@ private static class NetworkSessionWithAddressFactory extends SessionFactoryImpl } @Override - public Session newInstance( AccessMode mode, String bookmark ) + protected NetworkSession createSession( ConnectionProvider connectionProvider, RetryLogic retryLogic, + AccessMode mode, Logging logging ) { - NetworkSessionWithAddress session = new NetworkSessionWithAddress( connectionProvider, mode, logging ); - session.setBookmark( bookmark ); - return session; + return new NetworkSessionWithAddress( connectionProvider, mode, logging ); } } diff --git a/driver/src/test/resources/multiple_bookmarks.script b/driver/src/test/resources/multiple_bookmarks.script new file mode 100644 index 0000000000..5e7344103c --- /dev/null +++ b/driver/src/test/resources/multiple_bookmarks.script @@ -0,0 +1,16 @@ +!: AUTO INIT +!: AUTO RESET +!: AUTO PULL_ALL + +C: RUN "BEGIN" {"bookmark": "neo4j:bookmark:v1:tx94", "bookmarks": ["neo4j:bookmark:v1:tx5", "neo4j:bookmark:v1:tx29", "neo4j:bookmark:v1:tx94", "neo4j:bookmark:v1:tx56", "neo4j:bookmark:v1:tx16", "neo4j:bookmark:v1:tx68"]} + PULL_ALL +S: SUCCESS {} + SUCCESS {} +C: RUN "CREATE (n {name:'Bob'})" {} + PULL_ALL +S: SUCCESS {} + SUCCESS {"bookmark": "neo4j:bookmark:v1:tx95"} +C: RUN "COMMIT" {} + PULL_ALL +S: SUCCESS {} + SUCCESS {} diff --git a/driver/src/test/resources/read_tx_with_bookmarks.script b/driver/src/test/resources/read_tx_with_bookmarks.script index a48ee65f9a..47e9244506 100644 --- a/driver/src/test/resources/read_tx_with_bookmarks.script +++ b/driver/src/test/resources/read_tx_with_bookmarks.script @@ -2,7 +2,7 @@ !: AUTO RESET !: AUTO PULL_ALL -C: RUN "BEGIN" {"bookmark": "OldBookmark"} +C: RUN "BEGIN" {"bookmark": "OldBookmark", "bookmarks": ["OldBookmark"]} PULL_ALL S: SUCCESS {} SUCCESS {} diff --git a/driver/src/test/resources/write_read_tx_with_bookmarks.script b/driver/src/test/resources/write_read_tx_with_bookmarks.script index cb689a1627..f01feff289 100644 --- a/driver/src/test/resources/write_read_tx_with_bookmarks.script +++ b/driver/src/test/resources/write_read_tx_with_bookmarks.script @@ -2,7 +2,7 @@ !: AUTO RESET !: AUTO PULL_ALL -C: RUN "BEGIN" {"bookmark": "BookmarkA"} +C: RUN "BEGIN" {"bookmark": "BookmarkA", "bookmarks": ["BookmarkA"]} PULL_ALL S: SUCCESS {} SUCCESS {} @@ -14,7 +14,7 @@ C: RUN "COMMIT" {} PULL_ALL S: SUCCESS {} SUCCESS {} -C: RUN "BEGIN" {"bookmark": "BookmarkB"} +C: RUN "BEGIN" {"bookmark": "BookmarkB", "bookmarks": ["BookmarkB"]} PULL_ALL S: SUCCESS {} SUCCESS {} diff --git a/driver/src/test/resources/write_tx_with_bookmarks.script b/driver/src/test/resources/write_tx_with_bookmarks.script index 08247a9e2a..4413266b16 100644 --- a/driver/src/test/resources/write_tx_with_bookmarks.script +++ b/driver/src/test/resources/write_tx_with_bookmarks.script @@ -2,7 +2,7 @@ !: AUTO RESET !: AUTO PULL_ALL -C: RUN "BEGIN" {"bookmark": "OldBookmark"} +C: RUN "BEGIN" {"bookmark": "OldBookmark", "bookmarks": ["OldBookmark"]} PULL_ALL S: SUCCESS {} SUCCESS {}