Move bookmark manager configuration to session

Author: injectives

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

@@ -20,6 +20,7 @@
 
 import java.io.Serializable;
 import java.util.Set;
+import org.neo4j.driver.util.Experimental;
 
 /**
  * Keeps track of database bookmarks and is used by the driver to ensure causal consistency between sessions and query executions.
@@ -28,8 +29,9 @@
  * <p>
  * Implementations must avoid calling driver.
  *
- * @see org.neo4j.driver.Config.ConfigBuilder#withBookmarkManager(BookmarkManager)
+ * @see org.neo4j.driver.SessionConfig.Builder#withBookmarkManager(BookmarkManager)
  */
+@Experimental
 public interface BookmarkManager extends Serializable {
     /**
      * Updates database bookmarks by deleting the given previous bookmarks and adding the new bookmarks.

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

@@ -19,14 +19,16 @@
 package org.neo4j.driver;
 
 import org.neo4j.driver.internal.Neo4jBookmarkManager;
+import org.neo4j.driver.util.Experimental;
 
 /**
  * Setups new instances of {@link BookmarkManager}.
  */
+@Experimental
 public final class BookmarkManagers {
     private BookmarkManagers() {}
     /**
-     * Setups a new instance of bookmark manager that can be used in {@link org.neo4j.driver.Config.ConfigBuilder#withBookmarkManager(BookmarkManager)}.
+     * Setups a new instance of bookmark manager that can be used in {@link org.neo4j.driver.SessionConfig.Builder#withBookmarkManager(BookmarkManager)}.
      *
      * @param config the bookmark manager configuration
      * @return the bookmark manager

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

@@ -30,7 +30,6 @@
 import java.util.Collections;
 import java.util.List;
 import java.util.Objects;
-import java.util.Optional;
 import java.util.concurrent.TimeUnit;
 import java.util.logging.Level;
 import org.neo4j.driver.internal.SecuritySettings;
@@ -99,7 +98,6 @@ public final class Config implements Serializable {
     private final int eventLoopThreads;
     private final String userAgent;
     private final MetricsAdapter metricsAdapter;
-    private final BookmarkManager bookmarkManager;
 
     private Config(ConfigBuilder builder) {
         this.logging = builder.logging;
@@ -121,8 +119,6 @@ private Config(ConfigBuilder builder) {
 
         this.eventLoopThreads = builder.eventLoopThreads;
         this.metricsAdapter = builder.metricsAdapter;
-
-        this.bookmarkManager = builder.bookmarkManager;
     }
 
     /**
@@ -256,15 +252,6 @@ public String userAgent() {
         return userAgent;
     }
 
-    /**
-     * A {@link BookmarkManager} implementation for the driver to use.
-     *
-     * @return bookmark implementation
-     */
-    public Optional<BookmarkManager> bookmarkManager() {
-        return Optional.ofNullable(bookmarkManager);
-    }
-
     /**
      * Used to build new config instances
      */
@@ -285,7 +272,6 @@ public static final class ConfigBuilder {
         private MetricsAdapter metricsAdapter = MetricsAdapter.DEV_NULL;
         private long fetchSize = FetchSizeUtil.DEFAULT_FETCH_SIZE;
         private int eventLoopThreads = 0;
-        private BookmarkManager bookmarkManager;
 
         private ConfigBuilder() {}
 
@@ -659,19 +645,6 @@ public ConfigBuilder withUserAgent(String userAgent) {
             return this;
         }
 
-        /**
-         * Sets a {@link BookmarkManager} implementation for the driver to use.
-         * <p>
-         * By default, bookmark manager is effectively disabled.
-         *
-         * @param bookmarkManager bookmark manager implementation. Providing {@code null} effectively disables bookmark manager.
-         * @return this builder.
-         */
-        public ConfigBuilder withBookmarkManager(BookmarkManager bookmarkManager) {
-            this.bookmarkManager = bookmarkManager;
-            return this;
-        }
-
         /**
          * Extracts the driver version from the driver jar MANIFEST.MF file.
          */

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

@@ -29,6 +29,7 @@
 import org.neo4j.driver.async.AsyncSession;
 import org.neo4j.driver.reactive.ReactiveSession;
 import org.neo4j.driver.reactive.RxSession;
+import org.neo4j.driver.util.Experimental;
 import org.reactivestreams.Subscription;
 
 /**
@@ -45,15 +46,15 @@ public final class SessionConfig implements Serializable {
     private final String database;
     private final Long fetchSize;
     private final String impersonatedUser;
-    private final boolean ignoreBookmarkManager;
+    private final BookmarkManager bookmarkManager;
 
     private SessionConfig(Builder builder) {
         this.bookmarks = builder.bookmarks;
         this.defaultAccessMode = builder.defaultAccessMode;
         this.database = builder.database;
         this.fetchSize = builder.fetchSize;
         this.impersonatedUser = builder.impersonatedUser;
-        this.ignoreBookmarkManager = builder.ignoreBookmarkManager;
+        this.bookmarkManager = builder.bookmarkManager;
     }
 
     /**
@@ -133,12 +134,13 @@ public Optional<String> impersonatedUser() {
     }
 
     /**
-     * Determines if {@link BookmarkManager} configured at driver level should be ignored.
+     * A {@link BookmarkManager} implementation for the session to use.
      *
-     * @return {@code true} if bookmark manager should be ignored and not otherwise.
+     * @return bookmark implementation
      */
-    public boolean ignoreBookmarkManager() {
-        return ignoreBookmarkManager;
+    @Experimental
+    public Optional<BookmarkManager> bookmarkManager() {
+        return Optional.ofNullable(bookmarkManager);
     }
 
     @Override
@@ -155,19 +157,22 @@ public boolean equals(Object o) {
                 && Objects.equals(database, that.database)
                 && Objects.equals(fetchSize, that.fetchSize)
                 && Objects.equals(impersonatedUser, that.impersonatedUser)
-                && Objects.equals(ignoreBookmarkManager, that.ignoreBookmarkManager);
+                && Objects.equals(bookmarkManager, that.bookmarkManager);
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(bookmarks, defaultAccessMode, database, impersonatedUser, ignoreBookmarkManager);
+        return Objects.hash(bookmarks, defaultAccessMode, database, impersonatedUser, bookmarkManager);
     }
 
     @Override
     public String toString() {
-        return "SessionParameters{" + "bookmarks=" + bookmarks + ", defaultAccessMode=" + defaultAccessMode
-                + ", database='" + database + '\'' + ", fetchSize=" + fetchSize + "impersonatedUser=" + impersonatedUser
-                + '}';
+        return String.format(
+                """
+                SessionParameters{bookmarks=%s, defaultAccessMode=%s, database='%s', fetchSize=%d, impersonatedUser=%s, \
+                bookmarkManager=%s}\
+                """,
+                bookmarks, defaultAccessMode, database, fetchSize, impersonatedUser, bookmarkManager);
     }
 
     /**
@@ -179,7 +184,7 @@ public static final class Builder {
         private AccessMode defaultAccessMode = AccessMode.WRITE;
         private String database = null;
         private String impersonatedUser = null;
-        private boolean ignoreBookmarkManager = false;
+        private BookmarkManager bookmarkManager;
 
         private Builder() {}
 
@@ -306,13 +311,16 @@ public Builder withImpersonatedUser(String impersonatedUser) {
         }
 
         /**
-         * Ignore {@link BookmarkManager} configured at driver level using {@link org.neo4j.driver.Config.ConfigBuilder#withBookmarkManager(BookmarkManager)}.
+         * Sets a {@link BookmarkManager} implementation for the session to use.
+         * <p>
+         * By default, bookmark manager is effectively disabled.
          *
-         * @param ignore ignore if {@code true}, use otherwise.
+         * @param bookmarkManager bookmark manager implementation. Providing {@code null} effectively disables bookmark manager.
          * @return this builder.
          */
-        public Builder withIgnoredBookmarkManager(boolean ignore) {
-            this.ignoreBookmarkManager = ignore;
+        @Experimental
+        public Builder withBookmarkManager(BookmarkManager bookmarkManager) {
+            this.bookmarkManager = bookmarkManager;
             return this;
         }
 

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

@@ -260,12 +260,7 @@ protected InternalDriver createRoutingDriver(
      */
     protected InternalDriver createDriver(
             SecurityPlan securityPlan, SessionFactory sessionFactory, MetricsProvider metricsProvider, Config config) {
-        return new InternalDriver(
-                securityPlan,
-                sessionFactory,
-                metricsProvider,
-                config.logging(),
-                config.bookmarkManager().orElse(null));
+        return new InternalDriver(securityPlan, sessionFactory, metricsProvider, config.logging());
     }
 
     /**

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

@@ -22,7 +22,6 @@
 
 import java.util.concurrent.CompletionStage;
 import java.util.concurrent.atomic.AtomicBoolean;
-import org.neo4j.driver.BookmarkManager;
 import org.neo4j.driver.Driver;
 import org.neo4j.driver.Logger;
 import org.neo4j.driver.Logging;
@@ -48,22 +47,18 @@ public class InternalDriver implements Driver {
     private final SessionFactory sessionFactory;
     private final Logger log;
 
-    private AtomicBoolean closed = new AtomicBoolean(false);
+    private final AtomicBoolean closed = new AtomicBoolean(false);
     private final MetricsProvider metricsProvider;
 
-    private final BookmarkManager bookmarkManager;
-
     InternalDriver(
             SecurityPlan securityPlan,
             SessionFactory sessionFactory,
             MetricsProvider metricsProvider,
-            Logging logging,
-            BookmarkManager bookmarkManager) {
+            Logging logging) {
         this.securityPlan = securityPlan;
         this.sessionFactory = sessionFactory;
         this.metricsProvider = metricsProvider;
         this.log = logging.getLog(getClass());
-        this.bookmarkManager = bookmarkManager != null ? bookmarkManager : new NoOpBookmarkManager();
     }
 
     @Override
@@ -169,8 +164,7 @@ private static RuntimeException driverCloseException() {
 
     public NetworkSession newSession(SessionConfig config) {
         assertOpen();
-        var bookmarkManager = config.ignoreBookmarkManager() ? new NoOpBookmarkManager() : this.bookmarkManager;
-        NetworkSession session = sessionFactory.newInstance(config, bookmarkManager);
+        NetworkSession session = sessionFactory.newInstance(config);
         if (closed.get()) {
             // session does not immediately acquire connection, it is fine to just throw
             throw driverCloseException();

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

@@ -19,12 +19,11 @@
 package org.neo4j.driver.internal;
 
 import java.util.concurrent.CompletionStage;
-import org.neo4j.driver.BookmarkManager;
 import org.neo4j.driver.SessionConfig;
 import org.neo4j.driver.internal.async.NetworkSession;
 
 public interface SessionFactory {
-    NetworkSession newInstance(SessionConfig sessionConfig, BookmarkManager bookmarkManager);
+    NetworkSession newInstance(SessionConfig sessionConfig);
 
     CompletionStage<Void> verifyConnectivity();
 

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

@@ -51,8 +51,7 @@ public class SessionFactoryImpl implements SessionFactory {
     }
 
     @Override
-    public NetworkSession newInstance(SessionConfig sessionConfig, BookmarkManager bookmarkManager) {
-        Objects.requireNonNull(bookmarkManager, "bookmarkManager may not be null");
+    public NetworkSession newInstance(SessionConfig sessionConfig) {
         return createSession(
                 connectionProvider,
                 retryLogic,
@@ -62,7 +61,7 @@ public NetworkSession newInstance(SessionConfig sessionConfig, BookmarkManager b
                 parseFetchSize(sessionConfig),
                 sessionConfig.impersonatedUser().orElse(null),
                 logging,
-                bookmarkManager);
+                sessionConfig.bookmarkManager().orElse(new NoOpBookmarkManager()));
     }
 
     private Set<Bookmark> toDistinctSet(Iterable<Bookmark> bookmarks) {

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

@@ -110,9 +110,7 @@ void usesStandardSessionFactoryWhenNothingConfigured(String uri) {
         createDriver(uri, factory, config);
 
         SessionFactory capturedFactory = factory.capturedSessionFactory;
-        assertThat(
-                capturedFactory.newInstance(SessionConfig.defaultConfig(), new NoOpBookmarkManager()),
-                instanceOf(NetworkSession.class));
+        assertThat(capturedFactory.newInstance(SessionConfig.defaultConfig()), instanceOf(NetworkSession.class));
     }
 
     @ParameterizedTest
@@ -125,7 +123,7 @@ void usesLeakLoggingSessionFactoryWhenConfigured(String uri) {
 
         SessionFactory capturedFactory = factory.capturedSessionFactory;
         assertThat(
-                capturedFactory.newInstance(SessionConfig.defaultConfig(), new NoOpBookmarkManager()),
+                capturedFactory.newInstance(SessionConfig.defaultConfig()),
                 instanceOf(LeakLoggingNetworkSession.class));
     }
 

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

@@ -113,7 +113,7 @@ void shouldReturnMetricsIfMetricsEnabled() {
 
     private static InternalDriver newDriver(SessionFactory sessionFactory) {
         return new InternalDriver(
-                SecurityPlanImpl.insecure(), sessionFactory, DevNullMetricsProvider.INSTANCE, DEV_NULL_LOGGING, null);
+                SecurityPlanImpl.insecure(), sessionFactory, DevNullMetricsProvider.INSTANCE, DEV_NULL_LOGGING);
     }
 
     private static SessionFactory sessionFactoryMock() {
@@ -130,6 +130,6 @@ private static InternalDriver newDriver(boolean isMetricsEnabled) {
         }
 
         MetricsProvider metricsProvider = DriverFactory.getOrCreateMetricsProvider(config, Clock.SYSTEM);
-        return new InternalDriver(SecurityPlanImpl.insecure(), sessionFactory, metricsProvider, DEV_NULL_LOGGING, null);
+        return new InternalDriver(SecurityPlanImpl.insecure(), sessionFactory, metricsProvider, DEV_NULL_LOGGING);
     }
 }

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

@@ -26,7 +26,6 @@
 
 import org.junit.jupiter.api.Test;
 import org.neo4j.driver.AccessMode;
-import org.neo4j.driver.BookmarkManager;
 import org.neo4j.driver.Config;
 import org.neo4j.driver.internal.async.LeakLoggingNetworkSession;
 import org.neo4j.driver.internal.async.NetworkSession;
@@ -40,11 +39,11 @@ void createsNetworkSessions() {
         SessionFactory factory = newSessionFactory(config);
 
         NetworkSession readSession = factory.newInstance(
-                builder().withDefaultAccessMode(AccessMode.READ).build(), mock(BookmarkManager.class));
+                builder().withDefaultAccessMode(AccessMode.READ).build());
         assertThat(readSession, instanceOf(NetworkSession.class));
 
         NetworkSession writeSession = factory.newInstance(
-                builder().withDefaultAccessMode(AccessMode.WRITE).build(), mock(BookmarkManager.class));
+                builder().withDefaultAccessMode(AccessMode.WRITE).build());
         assertThat(writeSession, instanceOf(NetworkSession.class));
     }
 
@@ -57,11 +56,11 @@ void createsLeakLoggingNetworkSessions() {
         SessionFactory factory = newSessionFactory(config);
 
         NetworkSession readSession = factory.newInstance(
-                builder().withDefaultAccessMode(AccessMode.READ).build(), mock(BookmarkManager.class));
+                builder().withDefaultAccessMode(AccessMode.READ).build());
         assertThat(readSession, instanceOf(LeakLoggingNetworkSession.class));
 
         NetworkSession writeSession = factory.newInstance(
-                builder().withDefaultAccessMode(AccessMode.WRITE).build(), mock(BookmarkManager.class));
+                builder().withDefaultAccessMode(AccessMode.WRITE).build());
         assertThat(writeSession, instanceOf(LeakLoggingNetworkSession.class));
     }