From 576d8e04e4606ccb85964e483b5f3bc000c77ff6 Mon Sep 17 00:00:00 2001 From: Florent Biville <445792+fbiville@users.noreply.github.com> Date: Fri, 25 Nov 2022 15:06:03 +0100 Subject: [PATCH] Update bookmark manager API (#410) As explained in ADR 011, Fabric combines bookmarks when multiple databases are involved in a single transaction so there is no need on the driver side to track databases at all. --- neo4j/bookmarks.go | 110 +++++------------- neo4j/bookmarks_test.go | 153 ++++++-------------------- neo4j/internal/bolt/bolt3.go | 7 +- neo4j/internal/bolt/bolt3_test.go | 14 +-- neo4j/internal/bolt/bolt4.go | 50 ++++----- neo4j/internal/bolt/bolt4_test.go | 18 +-- neo4j/internal/bolt/bolt5.go | 50 ++++----- neo4j/internal/bolt/bolt5_test.go | 18 +-- neo4j/internal/db/connection.go | 2 +- neo4j/internal/testutil/connfake.go | 4 +- neo4j/session_bookmarks.go | 15 +-- neo4j/session_bookmarks_test.go | 35 ++---- neo4j/session_with_context.go | 39 ++----- neo4j/test-integration/dbconn_test.go | 8 +- testkit-backend/backend.go | 48 ++------ 15 files changed, 177 insertions(+), 394 deletions(-) diff --git a/neo4j/bookmarks.go b/neo4j/bookmarks.go index 9bd111fd..e886333d 100644 --- a/neo4j/bookmarks.go +++ b/neo4j/bookmarks.go @@ -34,104 +34,69 @@ type Bookmarks = []string // BookmarkManager centralizes bookmark manager supply and notification // This API is experimental and may be changed or removed without prior notice type BookmarkManager interface { - // UpdateBookmarks updates the bookmark for the specified database + // UpdateBookmarks updates the bookmark tracked by this bookmark manager // previousBookmarks are the initial bookmarks of the bookmark holder (like a Session) // newBookmarks are the bookmarks that are received after completion of the bookmark holder operation (like the end of a Session) - UpdateBookmarks(ctx context.Context, database string, previousBookmarks, newBookmarks Bookmarks) error + UpdateBookmarks(ctx context.Context, previousBookmarks, newBookmarks Bookmarks) error - // GetAllBookmarks returns all the bookmarks tracked by this bookmark manager - // Note: the order of the returned bookmark slice is not guaranteed - GetAllBookmarks(ctx context.Context) (Bookmarks, error) - - // GetBookmarks returns all the bookmarks associated with the specified database + // GetBookmarks returns all the bookmarks tracked by this bookmark manager // Note: the order of the returned bookmark slice does not need to be deterministic - GetBookmarks(ctx context.Context, database string) (Bookmarks, error) - - // Forget removes all databases' bookmarks - // Note: it is the driver user's responsibility to call this - Forget(ctx context.Context, databases ...string) error + GetBookmarks(ctx context.Context) (Bookmarks, error) } // BookmarkManagerConfig is an experimental API and may be changed or removed // without prior notice type BookmarkManagerConfig struct { // Initial bookmarks per database - InitialBookmarks map[string]Bookmarks + InitialBookmarks Bookmarks // Supplier providing external bookmarks - BookmarkSupplier BookmarkSupplier + BookmarkSupplier func(context.Context) (Bookmarks, error) - // Hook called whenever bookmarks for a given database get updated + // Hook called whenever bookmarks get updated // The hook is called with the database and the new bookmarks // Note: the order of the supplied bookmark slice is not guaranteed - BookmarkConsumer func(ctx context.Context, database string, bookmarks Bookmarks) error -} - -type BookmarkSupplier interface { - // GetAllBookmarks returns all known bookmarks to the bookmark manager - GetAllBookmarks(ctx context.Context) (Bookmarks, error) - - // GetBookmarks returns all the bookmarks of the specified database to the bookmark manager - GetBookmarks(ctx context.Context, database string) (Bookmarks, error) + BookmarkConsumer func(ctx context.Context, bookmarks Bookmarks) error } type bookmarkManager struct { - bookmarks *sync.Map - supplier BookmarkSupplier - consumer func(context.Context, string, Bookmarks) error + bookmarks collection.Set[string] + supplyBookmarks func(context.Context) (Bookmarks, error) + consumeBookmarks func(context.Context, Bookmarks) error + mutex sync.RWMutex } -func (b *bookmarkManager) UpdateBookmarks(ctx context.Context, database string, previousBookmarks, newBookmarks Bookmarks) error { +func (b *bookmarkManager) UpdateBookmarks(ctx context.Context, previousBookmarks, newBookmarks Bookmarks) error { if len(newBookmarks) == 0 { return nil } + b.mutex.Lock() + defer b.mutex.Unlock() var bookmarksToNotify Bookmarks - storedNewBookmarks := collection.NewSet(newBookmarks) - if rawCurrentBookmarks, loaded := b.bookmarks.LoadOrStore(database, storedNewBookmarks); !loaded { - bookmarksToNotify = storedNewBookmarks.Values() - } else { - currentBookmarks := (rawCurrentBookmarks).(collection.Set[string]) - currentBookmarks.RemoveAll(previousBookmarks) - currentBookmarks.AddAll(newBookmarks) - bookmarksToNotify = currentBookmarks.Values() - } - if b.consumer != nil { - return b.consumer(ctx, database, bookmarksToNotify) + b.bookmarks.RemoveAll(previousBookmarks) + b.bookmarks.AddAll(newBookmarks) + bookmarksToNotify = b.bookmarks.Values() + if b.consumeBookmarks != nil { + return b.consumeBookmarks(ctx, bookmarksToNotify) } return nil } -func (b *bookmarkManager) GetAllBookmarks(ctx context.Context) (Bookmarks, error) { - allBookmarks := collection.NewSet([]string{}) - if b.supplier != nil { - bookmarks, err := b.supplier.GetAllBookmarks(ctx) - if err != nil { - return nil, err - } - allBookmarks.AddAll(bookmarks) - } - b.bookmarks.Range(func(db, rawBookmarks any) bool { - bookmarks := rawBookmarks.(collection.Set[string]) - allBookmarks.Union(bookmarks) - return true - }) - return allBookmarks.Values(), nil -} - -func (b *bookmarkManager) GetBookmarks(ctx context.Context, database string) (Bookmarks, error) { +func (b *bookmarkManager) GetBookmarks(ctx context.Context) (Bookmarks, error) { var extraBookmarks Bookmarks - if b.supplier != nil { - bookmarks, err := b.supplier.GetBookmarks(ctx, database) + if b.supplyBookmarks != nil { + bookmarks, err := b.supplyBookmarks(ctx) if err != nil { return nil, err } extraBookmarks = bookmarks } - rawBookmarks, found := b.bookmarks.Load(database) - if !found { + b.mutex.RLock() + defer b.mutex.RUnlock() + if len(b.bookmarks) == 0 { return extraBookmarks, nil } - bookmarks := rawBookmarks.(collection.Set[string]).Copy() + bookmarks := b.bookmarks.Copy() if extraBookmarks == nil { return bookmarks.Values(), nil } @@ -139,27 +104,12 @@ func (b *bookmarkManager) GetBookmarks(ctx context.Context, database string) (Bo return bookmarks.Values(), nil } -func (b *bookmarkManager) Forget(ctx context.Context, databases ...string) error { - for _, db := range databases { - b.bookmarks.Delete(db) - } - return nil -} - func NewBookmarkManager(config BookmarkManagerConfig) BookmarkManager { return &bookmarkManager{ - bookmarks: initializeBookmarks(config.InitialBookmarks), - supplier: config.BookmarkSupplier, - consumer: config.BookmarkConsumer, - } -} - -func initializeBookmarks(allBookmarks map[string]Bookmarks) *sync.Map { - var initialBookmarks sync.Map - for db, bookmarks := range allBookmarks { - initialBookmarks.Store(db, collection.NewSet(bookmarks)) + bookmarks: collection.NewSet(config.InitialBookmarks), + supplyBookmarks: config.BookmarkSupplier, + consumeBookmarks: config.BookmarkConsumer, } - return &initialBookmarks } // CombineBookmarks is a helper method to combine []Bookmarks into a single Bookmarks instance. diff --git a/neo4j/bookmarks_test.go b/neo4j/bookmarks_test.go index 41c58e9f..2109c3b3 100644 --- a/neo4j/bookmarks_test.go +++ b/neo4j/bookmarks_test.go @@ -41,27 +41,19 @@ func TestBookmarkManager(outer *testing.T) { outer.Run("deduplicates initial bookmarks", func(t *testing.T) { bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{ - "db1": {"a", "a", "b"}, - "db2": {"b", "c", "b"}, - }, + InitialBookmarks: neo4j.Bookmarks{"a", "a", "b"}, }) - bookmarks1, err := bookmarkManager.GetBookmarks(ctx, "db1") - AssertNoError(t, err) - expected1 := []string{"a", "b"} - AssertEqualsInAnyOrder(t, bookmarks1, expected1) + bookmarks, err := bookmarkManager.GetBookmarks(ctx) - bookmarks2, err := bookmarkManager.GetBookmarks(ctx, "db2") AssertNoError(t, err) - expected2 := []string{"b", "c"} - AssertEqualsInAnyOrder(t, bookmarks2, expected2) + AssertEqualsInAnyOrder(t, bookmarks, []string{"a", "b"}) }) outer.Run("gets no bookmarks by default", func(t *testing.T) { bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{}) getBookmarks := func(db string) bool { - bookmarks, err := bookmarkManager.GetBookmarks(ctx, db) + bookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) return bookmarks == nil } @@ -74,16 +66,13 @@ func TestBookmarkManager(outer *testing.T) { outer.Run("gets bookmarks along with user-supplied bookmarks", func(t *testing.T) { expectedBookmarks := neo4j.Bookmarks{"a", "b", "c"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{"db1": {"a", "b"}}, - BookmarkSupplier: &simpleBookmarkSupplier{databaseBookmarks: func(db string) (neo4j.Bookmarks, error) { - if db != "db1" { - t.Errorf("expected to supply bookmarks for db1, but got %s", db) - } + InitialBookmarks: neo4j.Bookmarks{"a", "b"}, + BookmarkSupplier: func(context.Context) (neo4j.Bookmarks, error) { return neo4j.Bookmarks{"b", "c"}, nil - }}, + }, }) - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, expectedBookmarks) }) @@ -92,8 +81,8 @@ func TestBookmarkManager(outer *testing.T) { calls := 0 expectedBookmarks := []string{"a"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{"db1": {"a"}}, - BookmarkSupplier: &simpleBookmarkSupplier{databaseBookmarks: func(db string) (neo4j.Bookmarks, error) { + InitialBookmarks: neo4j.Bookmarks{"a"}, + BookmarkSupplier: func(ctx2 context.Context) (neo4j.Bookmarks, error) { defer func() { calls++ }() @@ -101,12 +90,12 @@ func TestBookmarkManager(outer *testing.T) { return neo4j.Bookmarks{"b"}, nil } return nil, nil - }}, + }, }) - _, err := bookmarkManager.GetBookmarks(ctx, "db1") + _, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, expectedBookmarks) }) @@ -114,13 +103,13 @@ func TestBookmarkManager(outer *testing.T) { outer.Run("returned bookmarks are copies", func(t *testing.T) { expectedBookmarks := []string{"a"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{"db1": {"a"}}, + InitialBookmarks: neo4j.Bookmarks{"a"}, }) - bookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + bookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) bookmarks[0] = "changed" - bookmarks, err = bookmarkManager.GetBookmarks(ctx, "db1") + bookmarks, err = bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, bookmarks, expectedBookmarks) @@ -128,16 +117,14 @@ func TestBookmarkManager(outer *testing.T) { outer.Run("updates bookmarks", func(t *testing.T) { bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{ - "db1": {"a", "b", "c"}, - }, + InitialBookmarks: neo4j.Bookmarks{"a", "b", "c"}, }) - err := bookmarkManager.UpdateBookmarks(ctx, "db1", []string{"b", "c"}, []string{"d", "a"}) + err := bookmarkManager.UpdateBookmarks(ctx, []string{"b", "c"}, []string{"d", "a"}) AssertNoError(t, err) expectedBookmarks := []string{"a", "d"} - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, expectedBookmarks) }) @@ -146,20 +133,17 @@ func TestBookmarkManager(outer *testing.T) { notifyHookCalled := false expectedBookmarks := []string{"a", "d"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - BookmarkConsumer: func(_ context.Context, db string, bookmarks neo4j.Bookmarks) error { + BookmarkConsumer: func(_ context.Context, bookmarks neo4j.Bookmarks) error { notifyHookCalled = true - if db != "db1" { - t.Errorf("expected to receive notifications for DB db1 but received notifications for %s", db) - } AssertEqualsInAnyOrder(t, bookmarks, expectedBookmarks) return nil }, }) - err := bookmarkManager.UpdateBookmarks(ctx, "db1", nil, []string{"d", "a"}) + err := bookmarkManager.UpdateBookmarks(ctx, nil, []string{"d", "a"}) AssertNoError(t, err) - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, expectedBookmarks) if !notifyHookCalled { @@ -170,17 +154,17 @@ func TestBookmarkManager(outer *testing.T) { outer.Run("does not notify updated bookmarks when empty", func(t *testing.T) { initialBookmarks := []string{"a", "b"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{"db1": initialBookmarks}, - BookmarkConsumer: func(_ context.Context, db string, bookmarks neo4j.Bookmarks) error { + InitialBookmarks: initialBookmarks, + BookmarkConsumer: func(_ context.Context, bookmarks neo4j.Bookmarks) error { t.Error("I must not be called") return nil }, }) - err := bookmarkManager.UpdateBookmarks(ctx, "db1", initialBookmarks, nil) + err := bookmarkManager.UpdateBookmarks(ctx, initialBookmarks, nil) AssertNoError(t, err) - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, initialBookmarks) }) @@ -189,21 +173,18 @@ func TestBookmarkManager(outer *testing.T) { notifyHookCalled := false expectedBookmarks := []string{"a", "d"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{"db1": {}}, - BookmarkConsumer: func(_ context.Context, db string, bookmarks neo4j.Bookmarks) error { + InitialBookmarks: nil, + BookmarkConsumer: func(_ context.Context, bookmarks neo4j.Bookmarks) error { notifyHookCalled = true - if db != "db1" { - t.Errorf("expected to receive notifications for DB db1 but received notifications for %s", db) - } AssertEqualsInAnyOrder(t, bookmarks, expectedBookmarks) return nil }, }) - err := bookmarkManager.UpdateBookmarks(ctx, "db1", nil, []string{"d", "a"}) + err := bookmarkManager.UpdateBookmarks(ctx, nil, []string{"d", "a"}) AssertNoError(t, err) - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, expectedBookmarks) if !notifyHookCalled { @@ -215,88 +196,22 @@ func TestBookmarkManager(outer *testing.T) { notifyHookCalled := false expectedBookmarks := []string{"a", "d"} bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{"db1": {"a", "b", "c"}}, - BookmarkConsumer: func(_ context.Context, db string, bookmarks neo4j.Bookmarks) error { + InitialBookmarks: neo4j.Bookmarks{"a", "b", "c"}, + BookmarkConsumer: func(_ context.Context, bookmarks neo4j.Bookmarks) error { notifyHookCalled = true - if db != "db1" { - t.Errorf("expected to receive notifications for DB db1 but received notifications for %s", db) - } AssertEqualsInAnyOrder(t, bookmarks, expectedBookmarks) return nil }, }) - err := bookmarkManager.UpdateBookmarks(ctx, "db1", []string{"b", "c"}, []string{"d", "a"}) + err := bookmarkManager.UpdateBookmarks(ctx, []string{"b", "c"}, []string{"d", "a"}) AssertNoError(t, err) - actualBookmarks, err := bookmarkManager.GetBookmarks(ctx, "db1") + actualBookmarks, err := bookmarkManager.GetBookmarks(ctx) AssertNoError(t, err) AssertEqualsInAnyOrder(t, actualBookmarks, expectedBookmarks) if !notifyHookCalled { t.Errorf("notify hook should have been called") } }) - - outer.Run("forgets bookmarks of specified databases", func(t *testing.T) { - bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{ - "db": {"z", "cooper"}, - "foo": {"bar", "fighters"}, - "par": {"rot", "don my French"}, - }, - }) - - err := bookmarkManager.Forget(ctx, "db", "par") - - AssertNoError(t, err) - allBookmarks, err := bookmarkManager.GetAllBookmarks(ctx) - AssertNoError(t, err) - AssertEqualsInAnyOrder(t, allBookmarks, []string{"bar", "fighters"}) - bookmarks, err := bookmarkManager.GetBookmarks(ctx, "db") - AssertNoError(t, err) - AssertIntEqual(t, len(bookmarks), 0) - bookmarks, err = bookmarkManager.GetBookmarks(ctx, "foo") - AssertNoError(t, err) - AssertEqualsInAnyOrder(t, bookmarks, []string{"bar", "fighters"}) - bookmarks, err = bookmarkManager.GetBookmarks(ctx, "par") - AssertNoError(t, err) - AssertIntEqual(t, len(bookmarks), 0) - }) - - outer.Run("can forget untracked databases", func(t *testing.T) { - bookmarkManager := neo4j.NewBookmarkManager(neo4j.BookmarkManagerConfig{ - InitialBookmarks: map[string]neo4j.Bookmarks{ - "db": {"z", "cooper"}, - }, - }) - - err := bookmarkManager.Forget(ctx, "wat", "nope") - - AssertNoError(t, err) - allBookmarks, err := bookmarkManager.GetAllBookmarks(ctx) - AssertNoError(t, err) - AssertEqualsInAnyOrder(t, allBookmarks, []string{"z", "cooper"}) - bookmarks, err := bookmarkManager.GetBookmarks(ctx, "db") - AssertNoError(t, err) - AssertEqualsInAnyOrder(t, bookmarks, []string{"z", "cooper"}) - bookmarks, err = bookmarkManager.GetBookmarks(ctx, "wat") - AssertNoError(t, err) - AssertIntEqual(t, len(bookmarks), 0) - bookmarks, err = bookmarkManager.GetBookmarks(ctx, "nope") - AssertNoError(t, err) - AssertIntEqual(t, len(bookmarks), 0) - }) -} - -type simpleBookmarkSupplier struct { - allBookmarks func() (neo4j.Bookmarks, error) - databaseBookmarks func(string) (neo4j.Bookmarks, error) -} - -func (s *simpleBookmarkSupplier) GetAllBookmarks(context.Context) (neo4j.Bookmarks, error) { - return s.allBookmarks() -} - -func (s *simpleBookmarkSupplier) GetBookmarks(_ context.Context, db string) (neo4j.Bookmarks, error) { - return s.databaseBookmarks(db) } diff --git a/neo4j/internal/bolt/bolt3.go b/neo4j/internal/bolt/bolt3.go index 57815d37..e19dc632 100644 --- a/neo4j/internal/bolt/bolt3.go +++ b/neo4j/internal/bolt/bolt3.go @@ -617,11 +617,8 @@ func (b *bolt3) receiveNext(ctx context.Context) (*db.Record, *db.Summary, error } } -func (b *bolt3) Bookmark() (string, string) { - // the bookmark database is empty here since multi-tenancy (aka multi-database) - // is a Neo4j 4.x EE feature - // only Bolt 4.0+ connections may return a non-empty value - return b.bookmark, "" +func (b *bolt3) Bookmark() string { + return b.bookmark } func (b *bolt3) IsAlive() bool { diff --git a/neo4j/internal/bolt/bolt3_test.go b/neo4j/internal/bolt/bolt3_test.go index 8df4d6fa..9b7b4f5b 100644 --- a/neo4j/internal/bolt/bolt3_test.go +++ b/neo4j/internal/bolt/bolt3_test.go @@ -204,7 +204,7 @@ func TestBolt3(outer *testing.T) { bolt.TxCommit(context.Background(), tx) assertBoltState(t, bolt3_ready, bolt) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, committedBookmark, bookmark) }) @@ -226,7 +226,7 @@ func TestBolt3(outer *testing.T) { assertBoltState(t, bolt3_streamingtx, bolt) bolt.TxCommit(context.Background(), tx) assertBoltState(t, bolt3_ready, bolt) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, committedBookmark, bookmark) }) @@ -243,7 +243,7 @@ func TestBolt3(outer *testing.T) { idb.TxConfig{Mode: idb.ReadMode, Bookmarks: []string{"bm1"}}) assertBoltState(t, bolt3_failed, bolt) AssertError(t, err) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, "", bookmark) }) @@ -386,7 +386,7 @@ func TestBolt3(outer *testing.T) { err := bolt.Buffer(context.Background(), stream) AssertNoError(t, err) // The bookmark should be set - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, runBookmark) // Server closed connection and bolt will go into failed state @@ -436,7 +436,7 @@ func TestBolt3(outer *testing.T) { rec, sum, err = bolt.Next(context.Background(), stream) AssertNextOnlyError(t, rec, sum, err) // Should be no bookmark since we failed - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, "") }) @@ -467,7 +467,7 @@ func TestBolt3(outer *testing.T) { AssertNoError(t, err) AssertNotNil(t, sum) // The bookmark should be set - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, runBookmark) AssertStringEqual(t, sum.Bookmark, runBookmark) @@ -503,7 +503,7 @@ func TestBolt3(outer *testing.T) { sum, err := bolt.Consume(context.Background(), stream) AssertNeo4jError(t, err) AssertNil(t, sum) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, "") // Should not get the summary since there was an error diff --git a/neo4j/internal/bolt/bolt4.go b/neo4j/internal/bolt/bolt4.go index af316c24..0c8a5f00 100644 --- a/neo4j/internal/bolt/bolt4.go +++ b/neo4j/internal/bolt/bolt4.go @@ -80,26 +80,25 @@ func (i *internalTx4) toMeta() map[string]any { } type bolt4 struct { - state int - txId idb.TxHandle - streams openstreams - conn net.Conn - serverName string - out outgoing - in incoming - connId string - logId string - serverVersion string - tfirst int64 // Time that server started streaming - bookmark string // Last bookmark - bookmarkDatabase string // Last bookmark's database - birthDate time.Time - log log.Logger - databaseName string - err error // Last fatal error - minor int - lastQid int64 // Last seen qid - idleDate time.Time + state int + txId idb.TxHandle + streams openstreams + conn net.Conn + serverName string + out outgoing + in incoming + connId string + logId string + serverVersion string + tfirst int64 // Time that server started streaming + bookmark string // Last bookmark + birthDate time.Time + log log.Logger + databaseName string + err error // Last fatal error + minor int + lastQid int64 // Last seen qid + idleDate time.Time } func NewBolt4(serverName string, conn net.Conn, logger log.Logger, boltLog log.BoltLogger) *bolt4 { @@ -388,9 +387,6 @@ func (b *bolt4) TxCommit(ctx context.Context, txh idb.TxHandle) error { // Keep track of bookmark if len(succ.bookmark) > 0 { b.bookmark = succ.bookmark - // SUCCESS can include the DB the bookmarks belong to when SSR is on - // and queries like `USE myDb MATCH (...)` are executed - b.bookmarkDatabase = succ.db } // Transition into ready state @@ -805,9 +801,6 @@ func (b *bolt4) receiveNext(ctx context.Context) (*db.Record, bool, *db.Summary) sum.TFirst = b.tfirst if len(sum.Bookmark) > 0 { b.bookmark = sum.Bookmark - // SUCCESS can include the DB the bookmarks belong to when SSR is on - // and queries like `USE myDb MATCH (...)` are executed - b.bookmarkDatabase = sum.Database } // Done with this stream b.streams.detach(sum, nil) @@ -823,8 +816,8 @@ func (b *bolt4) receiveNext(ctx context.Context) (*db.Record, bool, *db.Summary) } } -func (b *bolt4) Bookmark() (string, string) { - return b.bookmark, b.bookmarkDatabase +func (b *bolt4) Bookmark() string { + return b.bookmark } func (b *bolt4) IsAlive() bool { @@ -848,7 +841,6 @@ func (b *bolt4) Reset(ctx context.Context) { b.log.Debugf(log.Bolt4, b.logId, "Resetting connection internal state") b.txId = 0 b.bookmark = "" - b.bookmarkDatabase = "" b.databaseName = idb.DefaultDatabase b.err = nil b.lastQid = -1 diff --git a/neo4j/internal/bolt/bolt4_test.go b/neo4j/internal/bolt/bolt4_test.go index 3c958f8b..d313da94 100644 --- a/neo4j/internal/bolt/bolt4_test.go +++ b/neo4j/internal/bolt/bolt4_test.go @@ -391,7 +391,7 @@ func TestBolt4(outer *testing.T) { bolt.TxCommit(context.Background(), tx) assertBoltState(t, bolt4_ready, bolt) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, committedBookmark, bookmark) }) @@ -501,7 +501,7 @@ func TestBolt4(outer *testing.T) { assertBoltState(t, bolt4_streamingtx, bolt) bolt.TxCommit(context.Background(), tx) assertBoltState(t, bolt4_ready, bolt) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, committedBookmark, bookmark) }) @@ -518,7 +518,7 @@ func TestBolt4(outer *testing.T) { idb.TxConfig{Mode: idb.ReadMode, Bookmarks: []string{"bm1"}}) assertBoltState(t, bolt4_failed, bolt) AssertError(t, err) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, "", bookmark) }) @@ -683,7 +683,7 @@ func TestBolt4(outer *testing.T) { err := bolt.Buffer(context.Background(), stream) AssertNoError(t, err) // The bookmark should be set - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, runBookmark) // Server closed connection and bolt will go into failed state @@ -732,7 +732,7 @@ func TestBolt4(outer *testing.T) { err = bolt.Buffer(context.Background(), stream) AssertNoError(t, err) // The bookmark should be set - bookmark, _ = bolt.Bookmark() + bookmark = bolt.Bookmark() AssertStringEqual(t, bookmark, bookmark) for i := 0; i < 4; i++ { @@ -778,7 +778,7 @@ func TestBolt4(outer *testing.T) { rec, sum, err = bolt.Next(context.Background(), stream) AssertNextOnlyError(t, rec, sum, err) // Should be no bookmark since we failed - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, "") }) @@ -810,7 +810,7 @@ func TestBolt4(outer *testing.T) { AssertNotNil(t, sum) assertBoltState(t, bolt4_ready, bolt) // The bookmark should be set - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, runBookmark) AssertStringEqual(t, sum.Bookmark, runBookmark) @@ -856,7 +856,7 @@ func TestBolt4(outer *testing.T) { assertBoltState(t, bolt4_ready, bolt) // The bookmark should be set - bookmark, _ = bolt.Bookmark() + bookmark = bolt.Bookmark() AssertStringEqual(t, bookmark, bookmark) AssertStringEqual(t, sum.Bookmark, bookmark) @@ -892,7 +892,7 @@ func TestBolt4(outer *testing.T) { sum, err := bolt.Consume(context.Background(), stream) AssertNeo4jError(t, err) AssertNil(t, sum) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, "") // Should not get the summary since there was an error diff --git a/neo4j/internal/bolt/bolt5.go b/neo4j/internal/bolt/bolt5.go index 6c33fd71..fa65f32f 100644 --- a/neo4j/internal/bolt/bolt5.go +++ b/neo4j/internal/bolt/bolt5.go @@ -79,26 +79,25 @@ func (i *internalTx5) toMeta() map[string]any { } type bolt5 struct { - state int - txId idb.TxHandle - streams openstreams - conn net.Conn - serverName string - out outgoing - in incoming - connId string - logId string - serverVersion string - tfirst int64 // Time that server started streaming - bookmark string // Last bookmark - bookmarkDatabase string // Last bookmark's database - birthDate time.Time - log log.Logger - databaseName string - err error // Last fatal error - minor int - lastQid int64 // Last seen qid - idleDate time.Time + state int + txId idb.TxHandle + streams openstreams + conn net.Conn + serverName string + out outgoing + in incoming + connId string + logId string + serverVersion string + tfirst int64 // Time that server started streaming + bookmark string // Last bookmark + birthDate time.Time + log log.Logger + databaseName string + err error // Last fatal error + minor int + lastQid int64 // Last seen qid + idleDate time.Time } func NewBolt5(serverName string, conn net.Conn, logger log.Logger, boltLog log.BoltLogger) *bolt5 { @@ -366,9 +365,6 @@ func (b *bolt5) TxCommit(ctx context.Context, txh idb.TxHandle) error { // Keep track of bookmark if len(succ.bookmark) > 0 { b.bookmark = succ.bookmark - // SUCCESS can include the DB the bookmarks belong to when SSR is on - // and queries like `USE myDb MATCH (...)` are executed - b.bookmarkDatabase = succ.db } // Transition into ready state @@ -778,9 +774,6 @@ func (b *bolt5) receiveNext(ctx context.Context) (*db.Record, bool, *db.Summary) sum.TFirst = b.tfirst if len(sum.Bookmark) > 0 { b.bookmark = sum.Bookmark - // SUCCESS can include the DB the bookmarks belong to when SSR is on - // and queries like `USE myDb MATCH (...)` are executed - b.bookmarkDatabase = sum.Database } // Done with this stream b.streams.detach(sum, nil) @@ -796,8 +789,8 @@ func (b *bolt5) receiveNext(ctx context.Context) (*db.Record, bool, *db.Summary) } } -func (b *bolt5) Bookmark() (string, string) { - return b.bookmark, b.bookmarkDatabase +func (b *bolt5) Bookmark() string { + return b.bookmark } func (b *bolt5) IsAlive() bool { @@ -821,7 +814,6 @@ func (b *bolt5) Reset(ctx context.Context) { b.log.Debugf(log.Bolt5, b.logId, "Resetting connection internal state") b.txId = 0 b.bookmark = "" - b.bookmarkDatabase = "" b.databaseName = idb.DefaultDatabase b.err = nil b.lastQid = -1 diff --git a/neo4j/internal/bolt/bolt5_test.go b/neo4j/internal/bolt/bolt5_test.go index 419aec1a..3738f80a 100644 --- a/neo4j/internal/bolt/bolt5_test.go +++ b/neo4j/internal/bolt/bolt5_test.go @@ -344,7 +344,7 @@ func TestBolt5(outer *testing.T) { _ = bolt.TxCommit(context.Background(), tx) assertBoltState(t, bolt5Ready, bolt) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, committedBookmark, bookmark) }) @@ -454,7 +454,7 @@ func TestBolt5(outer *testing.T) { assertBoltState(t, bolt5StreamingTx, bolt) _ = bolt.TxCommit(context.Background(), tx) assertBoltState(t, bolt5Ready, bolt) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, committedBookmark, bookmark) }) @@ -471,7 +471,7 @@ func TestBolt5(outer *testing.T) { idb.TxConfig{Mode: idb.ReadMode, Bookmarks: []string{"bm1"}}) assertBoltState(t, bolt5Failed, bolt) AssertError(t, err) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, "", bookmark) }) @@ -636,7 +636,7 @@ func TestBolt5(outer *testing.T) { err := bolt.Buffer(context.Background(), stream) AssertNoError(t, err) // The bookmark should be set - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, runBookmark) // Server closed connection and bolt will go into failed state @@ -685,7 +685,7 @@ func TestBolt5(outer *testing.T) { err = bolt.Buffer(context.Background(), stream) AssertNoError(t, err) // The bookmark should be set - bookmark, _ = bolt.Bookmark() + bookmark = bolt.Bookmark() AssertStringEqual(t, bookmark, bookmark) for i := 0; i < 4; i++ { @@ -731,7 +731,7 @@ func TestBolt5(outer *testing.T) { rec, sum, err = bolt.Next(context.Background(), stream) AssertNextOnlyError(t, rec, sum, err) // Should be no bookmark since we failed - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, "") }) @@ -763,7 +763,7 @@ func TestBolt5(outer *testing.T) { AssertNotNil(t, sum) assertBoltState(t, bolt5Ready, bolt) // The bookmark should be set - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, runBookmark) AssertStringEqual(t, sum.Bookmark, runBookmark) @@ -809,7 +809,7 @@ func TestBolt5(outer *testing.T) { assertBoltState(t, bolt5Ready, bolt) // The bookmark should be set - bookmark, _ = bolt.Bookmark() + bookmark = bolt.Bookmark() AssertStringEqual(t, bookmark, bookmark) AssertStringEqual(t, sum.Bookmark, bookmark) @@ -845,7 +845,7 @@ func TestBolt5(outer *testing.T) { sum, err := bolt.Consume(context.Background(), stream) AssertNeo4jError(t, err) AssertNil(t, sum) - bookmark, _ := bolt.Bookmark() + bookmark := bolt.Bookmark() AssertStringEqual(t, bookmark, "") // Should not get the summary since there was an error diff --git a/neo4j/internal/db/connection.go b/neo4j/internal/db/connection.go index a1453874..95e98455 100644 --- a/neo4j/internal/db/connection.go +++ b/neo4j/internal/db/connection.go @@ -84,7 +84,7 @@ type Connection interface { // Note that if there is an ongoing auto-commit transaction (stream active) the bookmark // from that is not included, use Buffer or Consume to end the stream with a bookmark. // Empty string if no bookmark. - Bookmark() (string, string) + Bookmark() string // ServerName returns the name of the remote server ServerName() string // ServerVersion returns the server version on pattern Neo4j/1.2.3 diff --git a/neo4j/internal/testutil/connfake.go b/neo4j/internal/testutil/connfake.go index e4536721..b1ac52cd 100644 --- a/neo4j/internal/testutil/connfake.go +++ b/neo4j/internal/testutil/connfake.go @@ -107,8 +107,8 @@ func (c *ConnFake) IdleDate() time.Time { return c.Idle } -func (c *ConnFake) Bookmark() (string, string) { - return c.Bookm, "" +func (c *ConnFake) Bookmark() string { + return c.Bookm } func (c *ConnFake) ServerVersion() string { diff --git a/neo4j/session_bookmarks.go b/neo4j/session_bookmarks.go index 4b5cfd71..b8d5ed0c 100644 --- a/neo4j/session_bookmarks.go +++ b/neo4j/session_bookmarks.go @@ -46,12 +46,12 @@ func (sb *sessionBookmarks) lastBookmark() string { return bookmarks[count-1] } -func (sb *sessionBookmarks) replaceBookmarks(ctx context.Context, database string, previousBookmarks []string, newBookmark string) error { +func (sb *sessionBookmarks) replaceBookmarks(ctx context.Context, previousBookmarks []string, newBookmark string) error { if len(newBookmark) == 0 { return nil } if sb.bookmarkManager != nil { - if err := sb.bookmarkManager.UpdateBookmarks(ctx, database, previousBookmarks, []string{newBookmark}); err != nil { + if err := sb.bookmarkManager.UpdateBookmarks(ctx, previousBookmarks, []string{newBookmark}); err != nil { return err } } @@ -66,18 +66,11 @@ func (sb *sessionBookmarks) replaceSessionBookmarks(newBookmark string) { sb.bookmarks = []string{newBookmark} } -func (sb *sessionBookmarks) bookmarksOfDatabase(ctx context.Context, db string) (Bookmarks, error) { +func (sb *sessionBookmarks) getBookmarks(ctx context.Context) (Bookmarks, error) { if sb.bookmarkManager == nil { return nil, nil } - return sb.bookmarkManager.GetBookmarks(ctx, db) -} - -func (sb *sessionBookmarks) allBookmarks(ctx context.Context) (Bookmarks, error) { - if sb.bookmarkManager == nil { - return nil, nil - } - return sb.bookmarkManager.GetAllBookmarks(ctx) + return sb.bookmarkManager.GetBookmarks(ctx) } // Remove empty string bookmarks to check for "bad" callers diff --git a/neo4j/session_bookmarks_test.go b/neo4j/session_bookmarks_test.go index 75d1dc48..76b6b9ba 100644 --- a/neo4j/session_bookmarks_test.go +++ b/neo4j/session_bookmarks_test.go @@ -52,7 +52,7 @@ func TestSessionBookmarks(outer *testing.T) { "", "bookmark", "", "deutschmark", "", }) - err := sessionBookmarks.replaceBookmarks(ctx, "db", nil, "booking mark") + err := sessionBookmarks.replaceBookmarks(ctx, nil, "booking mark") if err != nil { t.Errorf("expected nil error, got: %v", err) @@ -70,7 +70,7 @@ func TestSessionBookmarks(outer *testing.T) { outer.Run("does not replace set bookmarks when new bookmark is empty", func(t *testing.T) { sessionBookmarks := newSessionBookmarks(nil, []string{"book marking"}) - err := sessionBookmarks.replaceBookmarks(ctx, "db", nil, "") + err := sessionBookmarks.replaceBookmarks(ctx, nil, "") if err != nil { t.Errorf("expected nil error, got: %v", err) @@ -100,34 +100,23 @@ func TestSessionBookmarks(outer *testing.T) { bookmarkManager := &fakeBookmarkManager{} sessionBookmarks := newSessionBookmarks(bookmarkManager, nil) - err := sessionBookmarks.replaceBookmarks(ctx, "dbz", []string{"b1", "b2"}, "b3") + err := sessionBookmarks.replaceBookmarks(ctx, []string{"b1", "b2"}, "b3") if err != nil { t.Errorf("expected nil error, got: %v", err) } - if !bookmarkManager.called(1, "UpdateBookmarks", ctx, "dbz", []string{"b1", "b2"}, []string{"b3"}) { + if !bookmarkManager.called(1, "UpdateBookmarks", ctx, []string{"b1", "b2"}, []string{"b3"}) { t.Errorf("Expected UpdateBookmarks to be called once but was not") } }) - inner.Run("retrieves the specified database's bookmarks", func(t *testing.T) { + inner.Run("retrieves bookmarks", func(t *testing.T) { bookmarkManager := &fakeBookmarkManager{} sessionBookmarks := newSessionBookmarks(bookmarkManager, nil) - _, _ = sessionBookmarks.bookmarksOfDatabase(ctx, "dbz") + _, _ = sessionBookmarks.getBookmarks(ctx) - if !bookmarkManager.called(1, "GetBookmarks", ctx, "dbz") { - t.Errorf("Expected GetBookmarks to be called once but was not") - } - }) - - inner.Run("retrieves all databases' bookmarks", func(t *testing.T) { - bookmarkManager := &fakeBookmarkManager{} - sessionBookmarks := newSessionBookmarks(bookmarkManager, nil) - - _, _ = sessionBookmarks.allBookmarks(ctx) - - if !bookmarkManager.called(1, "GetAllBookmarks", ctx) { + if !bookmarkManager.called(1, "GetBookmarks", ctx) { t.Errorf("Expected GetBookmarks with the provided arguments to be called once but was not") } }) @@ -143,18 +132,18 @@ type fakeBookmarkManager struct { recordedCalls []invocation } -func (f *fakeBookmarkManager) UpdateBookmarks(ctx context.Context, database string, previousBookmarks, newBookmarks Bookmarks) error { +func (f *fakeBookmarkManager) UpdateBookmarks(ctx context.Context, previousBookmarks, newBookmarks Bookmarks) error { f.recordedCalls = append(f.recordedCalls, invocation{ function: "UpdateBookmarks", - arguments: []any{ctx, database, previousBookmarks, newBookmarks}, + arguments: []any{ctx, previousBookmarks, newBookmarks}, }) return nil } -func (f *fakeBookmarkManager) GetBookmarks(ctx context.Context, database string) (Bookmarks, error) { +func (f *fakeBookmarkManager) GetBookmarks(ctx context.Context) (Bookmarks, error) { f.recordedCalls = append(f.recordedCalls, invocation{ function: "GetBookmarks", - arguments: []any{ctx, database}, + arguments: []any{ctx}, }) return nil, nil } @@ -167,7 +156,7 @@ func (f *fakeBookmarkManager) GetAllBookmarks(ctx context.Context) (Bookmarks, e return nil, nil } -func (f *fakeBookmarkManager) Forget(context.Context, ...string) error { +func (f *fakeBookmarkManager) Forget(context.Context) error { return nil } diff --git a/neo4j/session_with_context.go b/neo4j/session_with_context.go index f628c0af..5b24277a 100644 --- a/neo4j/session_with_context.go +++ b/neo4j/session_with_context.go @@ -243,7 +243,7 @@ func (s *sessionWithContext) BeginTransaction(ctx context.Context, configurers . } // Begin transaction - beginBookmarks, err := s.transactionBookmarks(ctx) + beginBookmarks, err := s.getBookmarks(ctx) if err != nil { s.pool.Return(ctx, conn) return nil, wrapError(err) @@ -377,7 +377,7 @@ func (s *sessionWithContext) executeTransactionFunction( // handle transaction function panic as well defer s.pool.Return(ctx, conn) - beginBookmarks, err := s.transactionBookmarks(ctx) + beginBookmarks, err := s.getBookmarks(ctx) if err != nil { state.OnFailure(ctx, conn, err, false) return true, nil @@ -421,11 +421,10 @@ func (s *sessionWithContext) executeTransactionFunction( } func (s *sessionWithContext) getServers(ctx context.Context, mode idb.AccessMode) ([]string, error) { - bookmarksFn := s.routingBookmarks if mode == idb.ReadMode { - return s.router.Readers(ctx, bookmarksFn, s.databaseName, s.boltLogger) + return s.router.Readers(ctx, s.getBookmarks, s.databaseName, s.boltLogger) } else { - return s.router.Writers(ctx, bookmarksFn, s.databaseName, s.boltLogger) + return s.router.Writers(ctx, s.getBookmarks, s.databaseName, s.boltLogger) } } @@ -474,20 +473,14 @@ func (s *sessionWithContext) retrieveBookmarks(ctx context.Context, conn idb.Con if conn == nil { return nil } - bookmark, bookmarkDatabase := conn.Bookmark() - db := s.databaseName - if bookmarkDatabase != "" { - db = bookmarkDatabase - } - return s.bookmarks.replaceBookmarks(ctx, db, sentBookmarks, bookmark) + return s.bookmarks.replaceBookmarks(ctx, sentBookmarks, conn.Bookmark()) } func (s *sessionWithContext) retrieveSessionBookmarks(conn idb.Connection) { if conn == nil { return } - bookmark, _ := conn.Bookmark() - s.bookmarks.replaceSessionBookmarks(bookmark) + s.bookmarks.replaceSessionBookmarks(conn.Bookmark()) } func (s *sessionWithContext) Run(ctx context.Context, @@ -516,7 +509,7 @@ func (s *sessionWithContext) Run(ctx context.Context, return nil, wrapError(err) } - runBookmarks, err := s.transactionBookmarks(ctx) + runBookmarks, err := s.getBookmarks(ctx) if err != nil { s.pool.Return(ctx, conn) return nil, wrapError(err) @@ -608,8 +601,7 @@ func (s *sessionWithContext) resolveHomeDatabase(ctx context.Context) error { return nil } - // the actual database may not be known yet so the session initial bookmarks might actually belong to system - bookmarks, err := s.routingBookmarks(ctx) + bookmarks, err := s.getBookmarks(ctx) if err != nil { return err } @@ -623,8 +615,8 @@ func (s *sessionWithContext) resolveHomeDatabase(ctx context.Context) error { return nil } -func (s *sessionWithContext) transactionBookmarks(ctx context.Context) (Bookmarks, error) { - bookmarks, err := s.bookmarks.allBookmarks(ctx) +func (s *sessionWithContext) getBookmarks(ctx context.Context) (Bookmarks, error) { + bookmarks, err := s.bookmarks.getBookmarks(ctx) if err != nil { return nil, err } @@ -633,17 +625,6 @@ func (s *sessionWithContext) transactionBookmarks(ctx context.Context) (Bookmark return result.Values(), nil } -func (s *sessionWithContext) routingBookmarks(ctx context.Context) (Bookmarks, error) { - systemBookmarks, err := s.bookmarks.bookmarksOfDatabase(ctx, "system") - if err != nil { - return nil, err - } - sessionBookmarks := s.bookmarks.currentBookmarks() - bookmarks := collection.NewSet(systemBookmarks) - bookmarks.AddAll(sessionBookmarks) - return bookmarks.Values(), nil -} - type erroredSessionWithContext struct { err error } diff --git a/neo4j/test-integration/dbconn_test.go b/neo4j/test-integration/dbconn_test.go index 09d226a3..3a1a9835 100644 --- a/neo4j/test-integration/dbconn_test.go +++ b/neo4j/test-integration/dbconn_test.go @@ -267,7 +267,7 @@ func TestConnectionConformance(outer *testing.T) { } err = c.TxCommit(context.Background(), tx) AssertNoError(t, err) - bookmark, _ := c.Bookmark() + bookmark := c.Bookmark() AssertStringNotEmpty(t, bookmark) }, }, @@ -520,11 +520,11 @@ func TestConnectionConformance(outer *testing.T) { // Bookmark tests outer.Run("Bookmarks", func(tt *testing.T) { boltConn.Reset(context.Background()) - lastBookmark, _ := boltConn.Bookmark() + lastBookmark := boltConn.Bookmark() assertNewBookmark := func(t *testing.T) { t.Helper() - bookmark, _ := boltConn.Bookmark() + bookmark := boltConn.Bookmark() if len(bookmark) == 0 { t.Fatal("No bookmark") } @@ -536,7 +536,7 @@ func TestConnectionConformance(outer *testing.T) { assertNoNewBookmark := func(t *testing.T) { t.Helper() - bookmark, _ := boltConn.Bookmark() + bookmark := boltConn.Bookmark() if bookmark != lastBookmark { t.Fatal("New bookmark") } diff --git a/testkit-backend/backend.go b/testkit-backend/backend.go index 1a16d1c5..44cafc65 100644 --- a/testkit-backend/backend.go +++ b/testkit-backend/backend.go @@ -1099,16 +1099,14 @@ func patchNumbersInMap(dictionary map[string]any) error { func (b *backend) bookmarkManagerConfig(bookmarkManagerId string, config map[string]any) neo4j.BookmarkManagerConfig { - var initialBookmarks map[string]neo4j.Bookmarks + var initialBookmarks neo4j.Bookmarks if config["initialBookmarks"] != nil { - initialBookmarks = convertInitialBookmarks(config["initialBookmarks"].(map[string]any)) + initialBookmarks = convertInitialBookmarks(config["initialBookmarks"].([]any)) } result := neo4j.BookmarkManagerConfig{InitialBookmarks: initialBookmarks} supplierRegistered := config["bookmarksSupplierRegistered"] if supplierRegistered != nil && supplierRegistered.(bool) { - result.BookmarkSupplier = &testkitBookmarkSupplier{ - supplier: b.supplyBookmarks(bookmarkManagerId), - } + result.BookmarkSupplier = b.supplyBookmarks(bookmarkManagerId) } consumerRegistered := config["bookmarksConsumerRegistered"] if consumerRegistered != nil && consumerRegistered.(bool) { @@ -1117,16 +1115,10 @@ func (b *backend) bookmarkManagerConfig(bookmarkManagerId string, return result } -func (b *backend) supplyBookmarks(bookmarkManagerId string) func(...string) (neo4j.Bookmarks, error) { - return func(databases ...string) (neo4j.Bookmarks, error) { - if len(databases) > 1 { - panic("at most 1 database should be specified") - } +func (b *backend) supplyBookmarks(bookmarkManagerId string) func(context.Context) (neo4j.Bookmarks, error) { + return func(ctx context.Context) (neo4j.Bookmarks, error) { id := b.nextId() msg := map[string]any{"id": id, "bookmarkManagerId": bookmarkManagerId} - if len(databases) == 1 { - msg["database"] = databases[0] - } b.writeResponse("BookmarksSupplierRequest", msg) for { b.process() @@ -1135,13 +1127,12 @@ func (b *backend) supplyBookmarks(bookmarkManagerId string) func(...string) (neo } } -func (b *backend) consumeBookmarks(bookmarkManagerId string) func(context.Context, string, neo4j.Bookmarks) error { - return func(_ context.Context, database string, bookmarks neo4j.Bookmarks) error { +func (b *backend) consumeBookmarks(bookmarkManagerId string) func(context.Context, neo4j.Bookmarks) error { + return func(_ context.Context, bookmarks neo4j.Bookmarks) error { id := b.nextId() b.writeResponse("BookmarksConsumerRequest", map[string]any{ "id": id, "bookmarkManagerId": bookmarkManagerId, - "database": database, "bookmarks": bookmarks, }) for { @@ -1153,27 +1144,10 @@ func (b *backend) consumeBookmarks(bookmarkManagerId string) func(context.Contex } } -type testkitBookmarkSupplier struct { - supplier func(...string) (neo4j.Bookmarks, error) -} - -func (t *testkitBookmarkSupplier) GetAllBookmarks(context.Context) (neo4j.Bookmarks, error) { - return t.supplier() -} - -func (t *testkitBookmarkSupplier) GetBookmarks(_ context.Context, database string) (neo4j.Bookmarks, error) { - return t.supplier(database) -} - -func convertInitialBookmarks(bookmarks map[string]any) map[string]neo4j.Bookmarks { - result := make(map[string]neo4j.Bookmarks, len(bookmarks)) - for db, rawBookmarks := range bookmarks { - bookmarks := rawBookmarks.([]any) - storedBookmarks := make(neo4j.Bookmarks, len(bookmarks)) - for i, bookmark := range bookmarks { - storedBookmarks[i] = bookmark.(string) - } - result[db] = storedBookmarks +func convertInitialBookmarks(bookmarks []any) neo4j.Bookmarks { + result := make(neo4j.Bookmarks, len(bookmarks)) + for i, bookmark := range bookmarks { + result[i] = bookmark.(string) } return result }