Make bookmark to be Serializable so that the bookmark can be passed around without disclose the content of bookmark.

Author: Zhen Li

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

@@ -18,6 +18,8 @@
  */
 package org.neo4j.driver.internal;
 
+import java.io.Serializable;
+
 /**
  * Causal chaining is carried out by passing bookmarks between transactions.
  *
@@ -29,6 +31,6 @@
  *
  * To opt out of this mechanism for unrelated units of work, applications can use multiple sessions.
  */
-public interface Bookmark
+public interface Bookmark extends Serializable
 {
 }

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

@@ -18,20 +18,30 @@
  */
 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 static java.util.Objects.requireNonNull;
+
 public final class InternalBookmark implements Bookmark
 {
     private static final InternalBookmark EMPTY = new InternalBookmark( Collections.emptySet() );
 
-    private final Iterable<String> values;
+    private final Collection<String> values;
 
-    private InternalBookmark( Iterable<String> values )
+    private InternalBookmark( Collection<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;
     }
 
@@ -65,10 +75,10 @@ else if ( size == 1 )
         {
             if ( value == null )
             {
-                continue; // skip any null bookmark objects
+                continue; // skip any null bookmark value
             }
             assertInternalBookmark( value );
-            ((InternalBookmark) value).values.forEach( newValues::add );
+            newValues.addAll( ((InternalBookmark) value).values );
         }
         return new InternalBookmark( newValues );
     }
@@ -103,7 +113,7 @@ public static InternalBookmark parse( String value )
     /**
      * Used for test only
      */
-    public static InternalBookmark parse( Iterable<String> values )
+    public static InternalBookmark parse( Collection<String> values )
     {
         if ( values == null )
         {
@@ -114,11 +124,7 @@ public static InternalBookmark parse( Iterable<String> values )
 
     public boolean isEmpty()
     {
-        if ( values instanceof Collection )
-        {
-            return ((Collection) values).isEmpty();
-        }
-        return !values.iterator().hasNext();
+        return values.isEmpty();
     }
 
     public Iterable<String> values()

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

@@ -20,8 +20,11 @@
 
 import org.junit.jupiter.api.Test;
 
+import java.io.ByteArrayInputStream;
+import java.io.ByteArrayOutputStream;
+import java.io.ObjectInputStream;
+import java.io.ObjectOutputStream;
 import java.util.Arrays;
-import java.util.Collections;
 import java.util.List;
 
 import org.neo4j.driver.internal.util.Iterables;
@@ -104,7 +107,7 @@ void asBeginTransactionParametersForNonEmptyBookmark()
     void bookmarkFromString()
     {
         InternalBookmark bookmark = InternalBookmark.parse( "Cat" );
-        assertEquals( Collections.singletonList( "Cat" ), bookmark.values() );
+        assertEquals( singletonList( "Cat" ), bookmark.values() );
         verifyValues( bookmark, "Cat" );
     }
 
@@ -133,7 +136,7 @@ void bookmarkFromNullIterable()
     @Test
     void bookmarkFromEmptyIterable()
     {
-        InternalBookmark bookmark = InternalBookmark.parse( Collections.<String>emptyList() );
+        InternalBookmark bookmark = InternalBookmark.parse( emptyList() );
         assertTrue( bookmark.isEmpty() );
     }
 
@@ -169,6 +172,25 @@ void shouldReturnAllBookmarks()
         assertIterableEquals( bookmarks, InternalBookmark.parse( bookmarks ).values() );
     }
 
+    @Test
+    void objectShouldBeTheSameWhenSerializingAndDeserializing() throws Throwable
+    {
+        Bookmark bookmark = InternalBookmark.parse( Arrays.asList( "neo4j:1000", "neo4j:2000" ) );
+
+        ByteArrayOutputStream outputStream = new ByteArrayOutputStream();
+        ObjectOutputStream objectOutputStream = new ObjectOutputStream( outputStream );
+        objectOutputStream.writeObject( bookmark );
+        objectOutputStream.flush();
+        objectOutputStream.close();
+
+        ByteArrayInputStream inputStream = new ByteArrayInputStream( outputStream.toByteArray() );
+        ObjectInputStream objectInputStream = new ObjectInputStream( inputStream );
+        Bookmark readBookmark = (InternalBookmark) objectInputStream.readObject();
+        objectInputStream.close();
+
+        assertEquals( bookmark, readBookmark );
+    }
+
     private static void verifyValues( InternalBookmark bookmark, String... expectedValues )
     {
         verifyValues( bookmark, asList( expectedValues ) );