Simplify bookmark impl by exposing internal values

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

@@ -18,7 +18,9 @@
  */
 package org.neo4j.driver;
 
-import java.io.Serializable;
+import java.util.Set;
+
+import org.neo4j.driver.internal.InternalBookmark;
 
 /**
  * Causal chaining is carried out by passing bookmarks between transactions.
@@ -31,6 +33,27 @@
  *
  * To opt out of this mechanism for unrelated units of work, applications can use multiple sessions.
  */
-public interface Bookmark extends Serializable
+public interface Bookmark
 {
+    /**
+     * Returns a read-only set of bookmark strings that this bookmark instance identifies.
+     * @return a read-only set of bookmark strings that this bookmark instance identifies.
+     */
+    Set<String> values();
+
+    /**
+     * Reconstruct bookmark from \bookmarks string values.
+     * @param values values obtained from a previous bookmark.
+     * @return A bookmark.
+     */
+    static Bookmark from( Set<String> values )
+    {
+        return InternalBookmark.parse( values );
+    }
+
+    /**
+     * Return true if the bookmark is empty.
+     * @return true if the bookmark is empty.
+     */
+    boolean isEmpty();
 }

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

@@ -18,22 +18,24 @@
  */
 package org.neo4j.driver.internal;
 
+import org.neo4j.driver.Bookmark;
+
 public interface BookmarkHolder
 {
-    InternalBookmark getBookmark();
+    Bookmark getBookmark();
 
-    void setBookmark( InternalBookmark bookmark );
+    void setBookmark( Bookmark bookmark );
 
     BookmarkHolder NO_OP = new BookmarkHolder()
     {
         @Override
-        public InternalBookmark getBookmark()
+        public Bookmark getBookmark()
         {
             return InternalBookmark.empty();
         }
 
         @Override
-        public void setBookmark( InternalBookmark bookmark )
+        public void setBookmark( Bookmark bookmark )
         {
         }
     };

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

@@ -18,31 +18,33 @@
  */
 package org.neo4j.driver.internal;
 
+import org.neo4j.driver.Bookmark;
+
 /**
  * @since 2.0
  */
 public class DefaultBookmarkHolder implements BookmarkHolder
 {
-    private volatile InternalBookmark bookmark;
+    private volatile Bookmark bookmark;
 
     public DefaultBookmarkHolder()
     {
         this( InternalBookmark.empty() );
     }
 
-    public DefaultBookmarkHolder( InternalBookmark bookmark )
+    public DefaultBookmarkHolder( Bookmark bookmark )
     {
         this.bookmark = bookmark;
     }
 
     @Override
-    public InternalBookmark getBookmark()
+    public Bookmark getBookmark()
     {
         return bookmark;
     }
 
     @Override
-    public void setBookmark( InternalBookmark bookmark )
+    public void setBookmark( Bookmark bookmark )
     {
         if ( bookmark != null && !bookmark.isEmpty() )
         {

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

@@ -18,58 +18,48 @@
  */
 package org.neo4j.driver.internal;
 
-import java.io.Serializable;
-import java.util.Collection;
 import java.util.Collections;
 import java.util.HashSet;
 import java.util.Objects;
 import java.util.Set;
 
 import org.neo4j.driver.Bookmark;
+import org.neo4j.driver.internal.util.Iterables;
 
 import static java.util.Objects.requireNonNull;
 
 public final class InternalBookmark implements Bookmark
 {
     private static final InternalBookmark EMPTY = new InternalBookmark( Collections.emptySet() );
 
-    private final Collection<String> values;
+    private final Set<String> values;
 
-    private InternalBookmark( Collection<String> values )
+    private InternalBookmark( Set<String> values )
     {
         requireNonNull( values );
-        if ( !(values instanceof Serializable) )
-        {
-            // The Collection interface does not enforce Serializable, but all built-in Collection implementations actually are Serializable.
-            // This check ensures that we always provide values using these java built-in Collection objects.
-            throw new IllegalArgumentException( "The bookmark value should only be of Java built-in types such as ArrayList, HashSet which are serializable." );
-        }
         this.values = values;
     }
 
-    public static InternalBookmark empty()
+    public static Bookmark empty()
     {
         return EMPTY;
     }
 
-    public static InternalBookmark from( Iterable<Bookmark> bookmarks )
+    public static Bookmark from( Iterable<Bookmark> bookmarks )
     {
         if ( bookmarks == null )
         {
             return empty();
         }
 
-        if ( bookmarks instanceof Collection )
+        int size = Iterables.count( bookmarks );
+        if ( size == 0 )
         {
-            int size = ((Collection) bookmarks).size();
-            if ( size == 0 )
-            {
-                return empty();
-            }
-            else if ( size == 1 )
-            {
-                return from( bookmarks.iterator().next() );
-            }
+            return empty();
+        }
+        else if ( size == 1 )
+        {
+            return from( bookmarks.iterator().next() );
         }
 
         Set<String> newValues = new HashSet<>();
@@ -79,43 +69,37 @@ else if ( size == 1 )
             {
                 continue; // skip any null bookmark value
             }
-            assertInternalBookmark( value );
-            newValues.addAll( ((InternalBookmark) value).values );
+            newValues.addAll( value.values() );
         }
         return new InternalBookmark( newValues );
     }
 
-    private static InternalBookmark from( Bookmark bookmark )
+    private static Bookmark from( Bookmark bookmark )
     {
         if ( bookmark == null )
         {
             return empty();
         }
-        assertInternalBookmark( bookmark );
-        return (InternalBookmark) bookmark; // we directly return the same bookmark back
+        // it is safe to return the given bookmark as bookmarks values can not be modified once it is created.
+        return bookmark;
     }
 
-    private static void assertInternalBookmark( Bookmark bookmark )
-    {
-        if ( !(bookmark instanceof InternalBookmark) )
-        {
-            throw new IllegalArgumentException( String.format( "Received bookmark '%s' is not generated by a driver session.", bookmark ) );
-        }
-    }
-
-    public static InternalBookmark parse( String value )
+    /**
+     * Used to extract bookmark from metadata from server.
+     */
+    public static Bookmark parse( String value )
     {
         if ( value == null )
         {
             return empty();
         }
-        return parse( Collections.singletonList( value ) );
+        return new InternalBookmark( Collections.singleton( value ) );
     }
 
     /**
-     * Used for test only
+     * Used to reconstruct bookmark from values.
      */
-    public static InternalBookmark parse( Collection<String> values )
+    public static Bookmark parse( Set<String> values )
     {
         if ( values == null )
         {
@@ -124,14 +108,16 @@ public static InternalBookmark parse( Collection<String> values )
         return new InternalBookmark( values );
     }
 
+    @Override
     public boolean isEmpty()
     {
         return values.isEmpty();
     }
 
-    public Iterable<String> values()
+    @Override
+    public Set<String> values()
     {
-        return values;
+        return Collections.unmodifiableSet( values );
     }
 
     @Override

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

@@ -18,26 +18,28 @@
  */
 package org.neo4j.driver.internal;
 
+import org.neo4j.driver.Bookmark;
+
 /**
  * @since 2.0
  */
 public class ReadOnlyBookmarkHolder implements BookmarkHolder
 {
-    private final InternalBookmark bookmark;
+    private final Bookmark bookmark;
 
-    public ReadOnlyBookmarkHolder( InternalBookmark bookmark )
+    public ReadOnlyBookmarkHolder( Bookmark bookmark )
     {
         this.bookmark = bookmark;
     }
 
     @Override
-    public InternalBookmark getBookmark()
+    public Bookmark getBookmark()
     {
         return bookmark;
     }
 
     @Override
-    public void setBookmark( InternalBookmark bookmark )
+    public void setBookmark( Bookmark bookmark )
     {
         // NO_OP
     }

driver/src/main/java/org/neo4j/driver/internal/async/ConnectionContext.java

@@ -19,14 +19,14 @@
 package org.neo4j.driver.internal.async;
 
 import org.neo4j.driver.AccessMode;
+import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.internal.DatabaseName;
-import org.neo4j.driver.internal.InternalBookmark;
 
 public interface ConnectionContext
 {
     DatabaseName databaseName();
 
     AccessMode mode();
 
-    InternalBookmark rediscoveryBookmark();
+    Bookmark rediscoveryBookmark();
 }

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

@@ -22,13 +22,13 @@
 import java.util.concurrent.CompletionStage;
 import java.util.function.BiFunction;
 
+import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.Session;
 import org.neo4j.driver.Statement;
 import org.neo4j.driver.TransactionConfig;
 import org.neo4j.driver.async.StatementResultCursor;
 import org.neo4j.driver.exceptions.ClientException;
 import org.neo4j.driver.internal.BookmarkHolder;
-import org.neo4j.driver.internal.InternalBookmark;
 import org.neo4j.driver.internal.cursor.AsyncStatementResultCursor;
 import org.neo4j.driver.internal.cursor.RxStatementResultCursor;
 import org.neo4j.driver.internal.messaging.BoltProtocol;
@@ -75,7 +75,7 @@ public ExplicitTransaction( Connection connection, BookmarkHolder bookmarkHolder
         this.fetchSize = fetchSize;
     }
 
-    public CompletionStage<ExplicitTransaction> beginAsync( InternalBookmark initialBookmark, TransactionConfig config )
+    public CompletionStage<ExplicitTransaction> beginAsync( Bookmark initialBookmark, TransactionConfig config )
     {
         return protocol.beginTransaction( connection, initialBookmark, config )
                 .handle( ( ignore, beginError ) ->

driver/src/main/java/org/neo4j/driver/internal/async/ImmutableConnectionContext.java

@@ -19,8 +19,8 @@
 package org.neo4j.driver.internal.async;
 
 import org.neo4j.driver.AccessMode;
+import org.neo4j.driver.Bookmark;
 import org.neo4j.driver.internal.DatabaseName;
-import org.neo4j.driver.internal.InternalBookmark;
 import org.neo4j.driver.internal.spi.Connection;
 
 import static org.neo4j.driver.internal.DatabaseNameUtil.defaultDatabase;
@@ -35,9 +35,9 @@ public class ImmutableConnectionContext implements ConnectionContext
 
     private final DatabaseName databaseName;
     private final AccessMode mode;
-    private final InternalBookmark rediscoveryBookmark;
+    private final Bookmark rediscoveryBookmark;
 
-    public ImmutableConnectionContext( DatabaseName databaseName, InternalBookmark bookmark, AccessMode mode )
+    public ImmutableConnectionContext( DatabaseName databaseName, Bookmark bookmark, AccessMode mode )
     {
         this.databaseName = databaseName;
         this.rediscoveryBookmark = bookmark;
@@ -57,7 +57,7 @@ public AccessMode mode()
     }
 
     @Override
-    public InternalBookmark rediscoveryBookmark()
+    public Bookmark rediscoveryBookmark()
     {
         return rediscoveryBookmark;
     }

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

@@ -30,15 +30,14 @@
 import org.neo4j.driver.TransactionConfig;
 import org.neo4j.driver.async.StatementResultCursor;
 import org.neo4j.driver.exceptions.ClientException;
+import org.neo4j.driver.exceptions.TransactionNestingException;
 import org.neo4j.driver.internal.BookmarkHolder;
 import org.neo4j.driver.internal.DatabaseName;
 import org.neo4j.driver.internal.FailableCursor;
-import org.neo4j.driver.internal.InternalBookmark;
 import org.neo4j.driver.internal.cursor.AsyncStatementResultCursor;
 import org.neo4j.driver.internal.cursor.RxStatementResultCursor;
 import org.neo4j.driver.internal.cursor.StatementResultCursorFactory;
 import org.neo4j.driver.internal.logging.PrefixedLogger;
-import org.neo4j.driver.exceptions.TransactionNestingException;
 import org.neo4j.driver.internal.retry.RetryLogic;
 import org.neo4j.driver.internal.spi.Connection;
 import org.neo4j.driver.internal.spi.ConnectionProvider;
@@ -356,9 +355,9 @@ private static class NetworkSessionConnectionContext implements ConnectionContex
         // This bookmark is only used for rediscovery.
         // It has to be the initial bookmark given at the creation of the session.
         // As only that bookmark could carry extra system bookmarks
-        private final InternalBookmark rediscoveryBookmark;
+        private final Bookmark rediscoveryBookmark;
 
-        private NetworkSessionConnectionContext( DatabaseName databaseName, InternalBookmark bookmark )
+        private NetworkSessionConnectionContext( DatabaseName databaseName, Bookmark bookmark )
         {
             this.databaseName = databaseName;
             this.rediscoveryBookmark = bookmark;
@@ -383,7 +382,7 @@ public AccessMode mode()
         }
 
         @Override
-        public InternalBookmark rediscoveryBookmark()
+        public Bookmark rediscoveryBookmark()
         {
             return rediscoveryBookmark;
         }