Skip to content

Commit 8756a42

Browse files
bigmontzbigmontz
authored
Update Bookmark Manager for no longer tracking per database (#1335)
* Update Bookmark Manager for no longer tracking per database BookmarkManager interface changes: * The methods `getBookmarks` and `getAllBookmarks` were merge into `getBookmarks`. * `forget` was removed. * The database param was removed from the `updateBookmarks` method. BookmarkManagerConfig changes: * The BookmarksSupplier interface was replaced by Supplier<Set<Bookmark>>. * The bookmarksConsumer changed its type to Consumer<Set<Bookmark>>. * `initialBookmarks` changed its type to Set<Bookmark> * Address suggestions from the review
1 parent 8632e1a commit 8756a42

File tree

13 files changed

+90
-243
lines changed

13 files changed

+90
-243
lines changed

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

Lines changed: 6 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@
2323
import org.neo4j.driver.util.Experimental;
2424

2525
/**
26-
* Keeps track of database bookmarks and is used by the driver to ensure causal consistency between sessions and query executions.
26+
* Keeps track of bookmarks and is used by the driver to ensure causal consistency between sessions and query executions.
2727
* <p>
2828
* Please note that implementations of this interface MUST NOT block for extended periods of time.
2929
* <p>
@@ -34,35 +34,17 @@
3434
@Experimental
3535
public interface BookmarkManager extends Serializable {
3636
/**
37-
* Updates database bookmarks by deleting the given previous bookmarks and adding the new bookmarks.
37+
* Updates bookmarks by deleting the given previous bookmarks and adding the new bookmarks.
3838
*
39-
* @param database the database name, this might be an empty string when session has no database name configured and database discovery is unavailable
4039
* @param previousBookmarks the previous bookmarks
4140
* @param newBookmarks the new bookmarks
4241
*/
43-
void updateBookmarks(String database, Set<Bookmark> previousBookmarks, Set<Bookmark> newBookmarks);
42+
void updateBookmarks(Set<Bookmark> previousBookmarks, Set<Bookmark> newBookmarks);
4443

4544
/**
46-
* Gets an immutable set of bookmarks for a given database.
45+
* Gets an immutable set of bookmarks.
4746
*
48-
* @param database the database name
49-
* @return the set of bookmarks or an empty set if the database name is unknown to the bookmark manager
47+
* @return the set of bookmarks.
5048
*/
51-
Set<Bookmark> getBookmarks(String database);
52-
53-
/**
54-
* Gets an immutable set of bookmarks for all databases.
55-
*
56-
* @return the set of bookmarks or an empty set
57-
*/
58-
Set<Bookmark> getAllBookmarks();
59-
60-
/**
61-
* Deletes bookmarks for the given databases.
62-
* <p>
63-
* This method should be called by driver users if data deletion is desired when bookmarks for the given databases are no longer needed.
64-
*
65-
* @param databases the set of database names
66-
*/
67-
void forget(Set<String> databases);
49+
Set<Bookmark> getBookmarks();
6850
}

‎driver/src/main/java/org/neo4j/driver/BookmarkManagerConfig.java

Lines changed: 20 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -19,19 +19,19 @@
1919
package org.neo4j.driver;
2020

2121
import java.util.Collections;
22-
import java.util.Map;
2322
import java.util.Objects;
2423
import java.util.Optional;
2524
import java.util.Set;
26-
import java.util.function.BiConsumer;
25+
import java.util.function.Consumer;
26+
import java.util.function.Supplier;
2727

2828
/**
2929
* Bookmark configuration used to configure bookmark manager supplied by {@link BookmarkManagers#defaultManager(BookmarkManagerConfig)}.
3030
*/
3131
public final class BookmarkManagerConfig {
32-
private final Map<String, Set<Bookmark>> initialBookmarks;
33-
private final BiConsumer<String, Set<Bookmark>> bookmarksConsumer;
34-
private final BookmarksSupplier bookmarksSupplier;
32+
private final Set<Bookmark> initialBookmarks;
33+
private final Consumer<Set<Bookmark>> bookmarksConsumer;
34+
private final Supplier<Set<Bookmark>> bookmarksSupplier;
3535

3636
private BookmarkManagerConfig(BookmarkManagerConfigBuilder builder) {
3737
this.initialBookmarks = builder.initialBookmarks;
@@ -53,16 +53,16 @@ public static BookmarkManagerConfigBuilder builder() {
5353
*
5454
* @return the map of bookmarks
5555
*/
56-
public Map<String, Set<Bookmark>> initialBookmarks() {
56+
public Set<Bookmark> initialBookmarks() {
5757
return initialBookmarks;
5858
}
5959

6060
/**
61-
* Returns bookmarks consumer that will be notified when database bookmarks are updated.
61+
* Returns bookmarks consumer that will be notified when bookmarks are updated.
6262
*
6363
* @return the bookmarks consumer
6464
*/
65-
public Optional<BiConsumer<String, Set<Bookmark>>> bookmarksConsumer() {
65+
public Optional<Consumer<Set<Bookmark>>> bookmarksConsumer() {
6666
return Optional.ofNullable(bookmarksConsumer);
6767
}
6868

@@ -71,29 +71,29 @@ public Optional<BiConsumer<String, Set<Bookmark>>> bookmarksConsumer() {
7171
*
7272
* @return the bookmark supplier
7373
*/
74-
public Optional<BookmarksSupplier> bookmarksSupplier() {
74+
public Optional<Supplier<Set<Bookmark>>> bookmarksSupplier() {
7575
return Optional.ofNullable(bookmarksSupplier);
7676
}
7777

7878
/**
7979
* Builder used to configure {@link BookmarkManagerConfig} which will be used to create a bookmark manager.
8080
*/
8181
public static final class BookmarkManagerConfigBuilder {
82-
private Map<String, Set<Bookmark>> initialBookmarks = Collections.emptyMap();
83-
private BiConsumer<String, Set<Bookmark>> bookmarksConsumer;
84-
private BookmarksSupplier bookmarksSupplier;
82+
private Set<Bookmark> initialBookmarks = Collections.emptySet();
83+
private Consumer<Set<Bookmark>> bookmarksConsumer;
84+
private Supplier<Set<Bookmark>> bookmarksSupplier;
8585

8686
private BookmarkManagerConfigBuilder() {}
8787

8888
/**
8989
* Provide a map of initial bookmarks to initialise the bookmark manager.
9090
*
91-
* @param databaseToBookmarks database to bookmarks map
91+
* @param initialBookmarks initial set of bookmarks
9292
* @return this builder
9393
*/
94-
public BookmarkManagerConfigBuilder withInitialBookmarks(Map<String, Set<Bookmark>> databaseToBookmarks) {
95-
Objects.requireNonNull(databaseToBookmarks, "databaseToBookmarks must not be null");
96-
this.initialBookmarks = databaseToBookmarks;
94+
public BookmarkManagerConfigBuilder withInitialBookmarks(Set<Bookmark> initialBookmarks) {
95+
Objects.requireNonNull(initialBookmarks, "initialBookmarks must not be null");
96+
this.initialBookmarks = initialBookmarks;
9797
return this;
9898
}
9999

@@ -105,22 +105,23 @@ public BookmarkManagerConfigBuilder withInitialBookmarks(Map<String, Set<Bookmar
105105
* @param bookmarksConsumer bookmarks consumer
106106
* @return this builder
107107
*/
108-
public BookmarkManagerConfigBuilder withBookmarksConsumer(BiConsumer<String, Set<Bookmark>> bookmarksConsumer) {
108+
public BookmarkManagerConfigBuilder withBookmarksConsumer(Consumer<Set<Bookmark>> bookmarksConsumer) {
109109
this.bookmarksConsumer = bookmarksConsumer;
110110
return this;
111111
}
112112

113113
/**
114114
* Provide bookmarks supplier.
115115
* <p>
116-
* The supplied bookmarks will be served alongside the bookmarks served by the bookmark manager. The supplied bookmarks will not be stored nor managed by the bookmark manager.
116+
* The supplied bookmarks will be served alongside the bookmarks served by the bookmark manager. The supplied bookmarks will not be stored nor managed
117+
* by the bookmark manager.
117118
* <p>
118119
* The supplier will be called outside bookmark manager's synchronisation lock.
119120
*
120121
* @param bookmarksSupplier the bookmarks supplier
121122
* @return this builder
122123
*/
123-
public BookmarkManagerConfigBuilder withBookmarksSupplier(BookmarksSupplier bookmarksSupplier) {
124+
public BookmarkManagerConfigBuilder withBookmarksSupplier(Supplier<Set<Bookmark>> bookmarksSupplier) {
124125
this.bookmarksSupplier = bookmarksSupplier;
125126
return this;
126127
}

‎driver/src/main/java/org/neo4j/driver/BookmarksSupplier.java

Lines changed: 0 additions & 43 deletions
This file was deleted.

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

Lines changed: 19 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -21,20 +21,16 @@
2121
import static org.neo4j.driver.internal.util.LockUtil.executeWithLock;
2222

2323
import java.io.Serial;
24-
import java.util.Collection;
2524
import java.util.Collections;
26-
import java.util.HashMap;
2725
import java.util.HashSet;
28-
import java.util.Map;
2926
import java.util.Objects;
3027
import java.util.Set;
3128
import java.util.concurrent.locks.ReadWriteLock;
3229
import java.util.concurrent.locks.ReentrantReadWriteLock;
33-
import java.util.function.BiConsumer;
34-
import java.util.stream.Collectors;
30+
import java.util.function.Consumer;
31+
import java.util.function.Supplier;
3532
import org.neo4j.driver.Bookmark;
3633
import org.neo4j.driver.BookmarkManager;
37-
import org.neo4j.driver.BookmarksSupplier;
3834

3935
/**
4036
* A basic {@link BookmarkManager} implementation.
@@ -45,66 +41,40 @@ public final class Neo4jBookmarkManager implements BookmarkManager {
4541

4642
private final ReadWriteLock rwLock = new ReentrantReadWriteLock();
4743

48-
private final Map<String, Set<Bookmark>> databaseToBookmarks = new HashMap<>();
49-
private final BiConsumer<String, Set<Bookmark>> updateListener;
50-
private final BookmarksSupplier bookmarksSupplier;
44+
private final Set<Bookmark> bookmarks;
45+
private final Consumer<Set<Bookmark>> updateListener;
46+
private final Supplier<Set<Bookmark>> bookmarksSupplier;
5147

5248
public Neo4jBookmarkManager(
53-
Map<String, Set<Bookmark>> initialBookmarks,
54-
BiConsumer<String, Set<Bookmark>> updateListener,
55-
BookmarksSupplier bookmarksSupplier) {
49+
Set<Bookmark> initialBookmarks,
50+
Consumer<Set<Bookmark>> updateListener,
51+
Supplier<Set<Bookmark>> bookmarksSupplier) {
5652
Objects.requireNonNull(initialBookmarks, "initialBookmarks must not be null");
57-
this.databaseToBookmarks.putAll(initialBookmarks);
53+
this.bookmarks = new HashSet<>(initialBookmarks);
5854
this.updateListener = updateListener;
5955
this.bookmarksSupplier = bookmarksSupplier;
6056
}
6157

6258
@Override
63-
public void updateBookmarks(String database, Set<Bookmark> previousBookmarks, Set<Bookmark> newBookmarks) {
64-
var immutableBookmarks = executeWithLock(
65-
rwLock.writeLock(),
66-
() -> databaseToBookmarks.compute(database, (ignored, bookmarks) -> {
67-
var updatedBookmarks = new HashSet<Bookmark>();
68-
if (bookmarks != null) {
69-
bookmarks.stream()
70-
.filter(bookmark -> !previousBookmarks.contains(bookmark))
71-
.forEach(updatedBookmarks::add);
72-
}
73-
updatedBookmarks.addAll(newBookmarks);
74-
return Collections.unmodifiableSet(updatedBookmarks);
75-
}));
59+
public void updateBookmarks(Set<Bookmark> previousBookmarks, Set<Bookmark> newBookmarks) {
60+
var immutableBookmarks = executeWithLock(rwLock.writeLock(), () -> {
61+
this.bookmarks.removeAll(previousBookmarks);
62+
this.bookmarks.addAll(newBookmarks);
63+
return Collections.unmodifiableSet(this.bookmarks);
64+
});
7665
if (updateListener != null) {
77-
updateListener.accept(database, immutableBookmarks);
66+
updateListener.accept(immutableBookmarks);
7867
}
7968
}
8069

8170
@Override
82-
public Set<Bookmark> getBookmarks(String database) {
83-
var immutableBookmarks = executeWithLock(
84-
rwLock.readLock(), () -> databaseToBookmarks.getOrDefault(database, Collections.emptySet()));
71+
public Set<Bookmark> getBookmarks() {
72+
var immutableBookmarks = executeWithLock(rwLock.readLock(), () -> Collections.unmodifiableSet(this.bookmarks));
8573
if (bookmarksSupplier != null) {
8674
var bookmarks = new HashSet<>(immutableBookmarks);
87-
bookmarks.addAll(bookmarksSupplier.getBookmarks(database));
75+
bookmarks.addAll(bookmarksSupplier.get());
8876
immutableBookmarks = Collections.unmodifiableSet(bookmarks);
8977
}
9078
return immutableBookmarks;
9179
}
92-
93-
@Override
94-
public Set<Bookmark> getAllBookmarks() {
95-
var immutableBookmarks = executeWithLock(rwLock.readLock(), () -> databaseToBookmarks.values().stream()
96-
.flatMap(Collection::stream))
97-
.collect(Collectors.toUnmodifiableSet());
98-
if (bookmarksSupplier != null) {
99-
var bookmarks = new HashSet<>(immutableBookmarks);
100-
bookmarks.addAll(bookmarksSupplier.getAllBookmarks());
101-
immutableBookmarks = Collections.unmodifiableSet(bookmarks);
102-
}
103-
return immutableBookmarks;
104-
}
105-
106-
@Override
107-
public void forget(Set<String> databases) {
108-
executeWithLock(rwLock.writeLock(), () -> databases.forEach(databaseToBookmarks::remove));
109-
}
11080
}

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

Lines changed: 3 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -34,22 +34,16 @@ public class NoOpBookmarkManager implements BookmarkManager {
3434
private static final Set<Bookmark> EMPTY = Collections.emptySet();
3535

3636
@Override
37-
public void updateBookmarks(String database, Set<Bookmark> previousBookmarks, Set<Bookmark> newBookmarks) {
37+
public void updateBookmarks(Set<Bookmark> previousBookmarks, Set<Bookmark> newBookmarks) {
3838
// ignored
3939
}
4040

4141
@Override
42-
public Set<Bookmark> getBookmarks(String database) {
42+
public Set<Bookmark> getBookmarks() {
4343
return EMPTY;
4444
}
4545

46-
@Override
47-
public Set<Bookmark> getAllBookmarks() {
46+
private Set<Bookmark> getAllBookmarks() {
4847
return EMPTY;
4948
}
50-
51-
@Override
52-
public void forget(Set<String> databases) {
53-
// ignored
54-
}
5549
}

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

Lines changed: 7 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@
4343
import org.neo4j.driver.exceptions.TransactionNestingException;
4444
import org.neo4j.driver.internal.DatabaseBookmark;
4545
import org.neo4j.driver.internal.DatabaseName;
46-
import org.neo4j.driver.internal.DatabaseNameUtil;
4746
import org.neo4j.driver.internal.FailableCursor;
4847
import org.neo4j.driver.internal.ImpersonationUtil;
4948
import org.neo4j.driver.internal.cursor.AsyncResultCursor;
@@ -100,7 +99,7 @@ public NetworkSession(
10099
this.bookmarkManager = bookmarkManager;
101100
this.lastReceivedBookmarks = bookmarks;
102101
this.connectionContext =
103-
new NetworkSessionConnectionContext(databaseNameFuture, determineBookmarks(true), impersonatedUser);
102+
new NetworkSessionConnectionContext(databaseNameFuture, determineBookmarks(false), impersonatedUser);
104103
this.fetchSize = fetchSize;
105104
}
106105

@@ -145,7 +144,7 @@ public CompletionStage<UnmanagedTransaction> beginTransactionAsync(
145144
ImpersonationUtil.ensureImpersonationSupport(connection, connection.impersonatedUser()))
146145
.thenCompose(connection -> {
147146
UnmanagedTransaction tx = new UnmanagedTransaction(connection, this::handleNewBookmark, fetchSize);
148-
return tx.beginAsync(determineBookmarks(false), config, txType);
147+
return tx.beginAsync(determineBookmarks(true), config, txType);
149148
});
150149

151150
// update the reference to the only known transaction
@@ -240,7 +239,7 @@ private CompletionStage<ResultCursorFactory> buildResultCursorFactory(Query quer
240239
.runInAutoCommitTransaction(
241240
connection,
242241
query,
243-
determineBookmarks(false),
242+
determineBookmarks(true),
244243
this::handleNewBookmark,
245244
config,
246245
fetchSize);
@@ -342,21 +341,14 @@ private void handleNewBookmark(DatabaseBookmark databaseBookmark) {
342341
var bookmark = databaseBookmark.bookmark();
343342
if (bookmark != null) {
344343
var bookmarks = Set.of(bookmark);
345-
String databaseName = databaseBookmark.databaseName();
346-
if (databaseName == null || databaseName.isEmpty()) {
347-
databaseName = getDatabaseNameNow().orElse(FALLBACK_DATABASE_NAME);
348-
}
349344
lastReceivedBookmarks = bookmarks;
350-
bookmarkManager.updateBookmarks(databaseName, lastUsedBookmarks, bookmarks);
345+
bookmarkManager.updateBookmarks(lastUsedBookmarks, bookmarks);
351346
}
352347
}
353348

354-
private Set<Bookmark> determineBookmarks(boolean useSystemOnly) {
355-
var bookmarks = new HashSet<Bookmark>();
356-
if (useSystemOnly) {
357-
bookmarks.addAll(bookmarkManager.getBookmarks(DatabaseNameUtil.SYSTEM_DATABASE_NAME));
358-
} else {
359-
bookmarks.addAll(bookmarkManager.getAllBookmarks());
349+
private Set<Bookmark> determineBookmarks(boolean updateLastUsed) {
350+
var bookmarks = new HashSet<>(bookmarkManager.getBookmarks());
351+
if (updateLastUsed) {
360352
lastUsedBookmarks = Collections.unmodifiableSet(bookmarks);
361353
}
362354
bookmarks.addAll(lastReceivedBookmarks);

0 commit comments

Comments
 (0)