diff --git a/packages/core/src/bookmark-manager.ts b/packages/core/src/bookmark-manager.ts index 3d2a36168..9ebee832f 100644 --- a/packages/core/src/bookmark-manager.ts +++ b/packages/core/src/bookmark-manager.ts @@ -36,54 +36,30 @@ export default class BookmarkManager { * Method called when the bookmarks get updated when a transaction finished. * * This method will be called when auto-commit queries finish and when explicit transactions - * get commited. + * get committed. * - * @param {string} database The database which the bookmarks belongs to * @param {Iterable} previousBookmarks The bookmarks used when starting the transaction * @param {Iterable} newBookmarks The new bookmarks received at the end of the transaction. * @returns {void} */ - async updateBookmarks (database: string, previousBookmarks: Iterable, newBookmarks: Iterable): Promise { + async updateBookmarks (previousBookmarks: Iterable, newBookmarks: Iterable): Promise { throw new Error('Not implemented') } /** - * Method called by the driver to get the bookmarks for one specific database + * Method called by the driver to get the bookmarks. * - * @param {string} database The database which the bookmarks belong to * @returns {Iterable} The set of bookmarks */ - async getBookmarks (database: string): Promise> { - throw new Error('Not implemented') - } - - /** - * Method called by the driver for getting all the bookmarks. - * - * This method should return all bookmarks for all databases present in the BookmarkManager. - * - * @returns {Iterable} The set of bookmarks - */ - async getAllBookmarks (): Promise> { - throw new Error('Not implemented') - } - - /** - * Forget the databases and its bookmarks - * - * This method is not called by the driver. Forgetting unused databases is the user's responsibility. - * - * @param {Iterable} databases The databases which the bookmarks will be removed for. - */ - async forget (databases: Iterable): Promise { + async getBookmarks (): Promise> { throw new Error('Not implemented') } } export interface BookmarkManagerConfig { - initialBookmarks?: Map> - bookmarksSupplier?: (database?: string) => Promise> - bookmarksConsumer?: (database: string, bookmarks: Iterable) => Promise + initialBookmarks?: Iterable + bookmarksSupplier?: () => Promise> + bookmarksConsumer?: (bookmarks: Iterable) => Promise } /** @@ -91,11 +67,9 @@ export interface BookmarkManagerConfig { * * @since 5.0 * @experimental - * @property {Map>} [initialBookmarks@experimental] Defines the initial set of bookmarks. The key is the database name and the values are the bookmarks. - * @property {function([database]: string):Promise>} [bookmarksSupplier] Called for supplying extra bookmarks to the BookmarkManager - * 1. supplying bookmarks from the given database when the default BookmarkManager's `.getBookmarks(database)` gets called. - * 2. supplying all the bookmarks when the default BookmarkManager's `.getAllBookmarks()` gets called - * @property {function(database: string, bookmarks: Iterable): Promise} [bookmarksConsumer] Called when the set of bookmarks for database get updated + * @property {Iterable} [initialBookmarks] Defines the initial set of bookmarks. The key is the database name and the values are the bookmarks. + * @property {function():Promise>} [bookmarksSupplier] Called for supplying extra bookmarks to the BookmarkManager + * @property {function(bookmarks: Iterable): Promise} [bookmarksConsumer] Called when the set of bookmarks get updated */ /** * Provides an configured {@link BookmarkManager} instance. @@ -106,9 +80,7 @@ export interface BookmarkManagerConfig { * @returns {BookmarkManager} */ export function bookmarkManager (config: BookmarkManagerConfig = {}): BookmarkManager { - const initialBookmarks = new Map>() - - config.initialBookmarks?.forEach((v, k) => initialBookmarks.set(k, new Set(v))) + const initialBookmarks = new Set(config.initialBookmarks) return new Neo4jBookmarkManager( initialBookmarks, @@ -119,15 +91,15 @@ export function bookmarkManager (config: BookmarkManagerConfig = {}): BookmarkMa class Neo4jBookmarkManager implements BookmarkManager { constructor ( - private readonly _bookmarksPerDb: Map>, - private readonly _bookmarksSupplier?: (database?: string) => Promise>, - private readonly _bookmarksConsumer?: (database: string, bookmark: Iterable) => Promise + private readonly _bookmarks: Set, + private readonly _bookmarksSupplier?: () => Promise>, + private readonly _bookmarksConsumer?: (bookmark: Iterable) => Promise ) { } - async updateBookmarks (database: string, previousBookmarks: Iterable, newBookmarks: Iterable): Promise { - const bookmarks = this._getOrInitializeBookmarks(database) + async updateBookmarks (previousBookmarks: Iterable, newBookmarks: Iterable): Promise { + const bookmarks = this._bookmarks for (const bm of previousBookmarks) { bookmarks.delete(bm) } @@ -135,40 +107,13 @@ class Neo4jBookmarkManager implements BookmarkManager { bookmarks.add(bm) } if (typeof this._bookmarksConsumer === 'function') { - await this._bookmarksConsumer(database, [...bookmarks]) - } - } - - private _getOrInitializeBookmarks (database: string): Set { - let maybeBookmarks = this._bookmarksPerDb.get(database) - if (maybeBookmarks === undefined) { - maybeBookmarks = new Set() - this._bookmarksPerDb.set(database, maybeBookmarks) - } - return maybeBookmarks - } - - async getBookmarks (database: string): Promise> { - const bookmarks = new Set(this._bookmarksPerDb.get(database)) - - if (typeof this._bookmarksSupplier === 'function') { - const suppliedBookmarks = await this._bookmarksSupplier(database) ?? [] - for (const bm of suppliedBookmarks) { - bookmarks.add(bm) - } + await this._bookmarksConsumer([...bookmarks]) } - - return [...bookmarks] } - async getAllBookmarks (): Promise> { - const bookmarks = new Set() + async getBookmarks (): Promise> { + const bookmarks = new Set(this._bookmarks) - for (const [, dbBookmarks] of this._bookmarksPerDb) { - for (const bm of dbBookmarks) { - bookmarks.add(bm) - } - } if (typeof this._bookmarksSupplier === 'function') { const suppliedBookmarks = await this._bookmarksSupplier() ?? [] for (const bm of suppliedBookmarks) { @@ -176,12 +121,6 @@ class Neo4jBookmarkManager implements BookmarkManager { } } - return bookmarks - } - - async forget (databases: Iterable): Promise { - for (const database of databases) { - this._bookmarksPerDb.delete(database) - } + return [...bookmarks] } } diff --git a/packages/core/src/driver.ts b/packages/core/src/driver.ts index c2174ebc9..8de1de9b2 100644 --- a/packages/core/src/driver.ts +++ b/packages/core/src/driver.ts @@ -194,10 +194,10 @@ class SessionConfig { * Enabling it is done by supplying an BookmarkManager implementation instance to this param. * A default implementation could be acquired by calling the factory function {@link bookmarkManager}. * - * **Warning**: Share the same BookmarkManager instance accross all session can have a negative impact + * **Warning**: Sharing the same BookmarkManager instance across multiple sessions can have a negative impact * on performance since all the queries will wait for the latest changes being propagated across the cluster. * For keeping consistency between a group of queries, use {@link Session} for grouping them. - * For keeping consistency between a group of sessions, use {@link BookmarkManager} instance for groupping them. + * For keeping consistency between a group of sessions, use {@link BookmarkManager} instance for grouping them. * * @example * const bookmarkManager = neo4j.bookmarkManager() @@ -214,7 +214,7 @@ class SessionConfig { * * // Reading Driver User will wait of the changes being propagated to the server before RUN the query * // So the 'Driver User' person should exist in the Result, unless deleted. - * const linkedSesssion2 = await linkedSession2.run('CREATE (p:Person {name: $name}) RETURN p', { name: 'Driver User'}) + * const linkedResult = await linkedSession2.run('CREATE (p:Person {name: $name}) RETURN p', { name: 'Driver User'}) * * await linkedSession1.close() * await linkedSession2.close() diff --git a/packages/core/src/session.ts b/packages/core/src/session.ts index 09c5a1892..feedd0cc1 100644 --- a/packages/core/src/session.ts +++ b/packages/core/src/session.ts @@ -343,7 +343,7 @@ class Session { } private async _bookmarks (): Promise { - const bookmarks = await this._bookmarkManager?.getAllBookmarks() + const bookmarks = await this._bookmarkManager?.getBookmarks() if (bookmarks === undefined) { return this._lastBookmarks } @@ -489,7 +489,7 @@ class Session { } private async _getConnectionAcquistionBookmarks (): Promise { - const bookmarks = await this._bookmarkManager?.getBookmarks('system') + const bookmarks = await this._bookmarkManager?.getBookmarks() if (bookmarks === undefined) { return this._lastBookmarks } @@ -505,7 +505,6 @@ class Session { _updateBookmarks (newBookmarks?: Bookmarks, previousBookmarks?: Bookmarks, database?: string): void { if ((newBookmarks != null) && !newBookmarks.isEmpty()) { this._bookmarkManager?.updateBookmarks( - database ?? this._database, previousBookmarks?.values() ?? [], newBookmarks?.values() ?? [] ) diff --git a/packages/core/test/bookmark-manager.test.ts b/packages/core/test/bookmark-manager.test.ts index 0ff6ae73f..1b71ad9a2 100644 --- a/packages/core/test/bookmark-manager.test.ts +++ b/packages/core/test/bookmark-manager.test.ts @@ -30,124 +30,72 @@ describe('BookmarkManager', () => { }) describe('getBookmarks()', () => { - it('should return empty if db doesnt exists', async () => { - const manager = bookmarkManager({}) - - const bookmarks = await manager.getBookmarks('neo4j') - - expect(bookmarks).toEqual([]) - }) - - it('should return bookmarks for the given db', async () => { - const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) - }) - - const bookmarks = await manager.getBookmarks('neo4j') - - expect(bookmarks).toBeSortedEqual(neo4jBookmarks) - }) - - it('should return get bookmarks from bookmarkSupplier', async () => { - const extraBookmarks = ['neo4j:bm03', 'neo4j:bm04'] - + it('should return all bookmarks ', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), - bookmarksSupplier: async () => await Promise.resolve(extraBookmarks) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) - const bookmarks = await manager.getBookmarks('neo4j') + const bookmarks = await manager.getBookmarks() - expect(bookmarks).toBeSortedEqual([...neo4jBookmarks, ...extraBookmarks]) + expect(bookmarks).toBeSortedEqual([...neo4jBookmarks, ...systemBookmarks]) }) - it('should return not duplicate bookmarks if bookmarkSupplier returns existing bm', async () => { - const extraBookmarks = ['neo4j:bm03', 'neo4j:bm04'] - - const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), - bookmarksSupplier: async () => await Promise.resolve([...extraBookmarks, ...neo4jBookmarks]) - }) + it('should return empty if there are no bookmarks', async () => { + const manager = bookmarkManager({}) - const bookmarks = await manager.getBookmarks('neo4j') + const bookmarks = await manager.getBookmarks() - expect(bookmarks).toBeSortedEqual([...neo4jBookmarks, ...extraBookmarks]) + expect(bookmarks).toBeSortedEqual([]) }) - it('should return call from bookmarkSupplier with correct database', async () => { - const bookmarksSupplier = jest.fn() - + it('should return enriched bookmarks list with supplied bookmarks', async () => { + const extraBookmarks = ['neo4j:bmextra', 'system:bmextra', 'adb:bmextra'] + const bookmarksSupplier = jest.fn(async () => await Promise.resolve(extraBookmarks)) const manager = bookmarkManager({ + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks], bookmarksSupplier }) - await manager.getBookmarks('neo4j') - - expect(bookmarksSupplier).toBeCalledWith('neo4j') - }) - }) - - describe('getAllBookmarks()', () => { - it('should return all bookmarks ', async () => { - const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) - }) + const bookmarks = await manager.getBookmarks() - const bookmarks = await manager.getAllBookmarks() - - expect(bookmarks).toBeSortedEqual([...neo4jBookmarks, ...systemBookmarks]) - }) - - it('should return empty if there are no bookmarks for any db', async () => { - const manager = bookmarkManager({}) - - const bookmarks = await manager.getAllBookmarks() - - expect(bookmarks).toBeSortedEqual([]) + expect(bookmarks).toBeSortedEqual( + [...neo4jBookmarks, ...systemBookmarks, ...extraBookmarks] + ) }) - it('should return enriched bookmarks list with supplied bookmarks', async () => { + it('should not leak bookmarks from bookmarks supplier to the internal state', async () => { const extraBookmarks = ['neo4j:bmextra', 'system:bmextra', 'adb:bmextra'] - const bookmarksSupplier = jest.fn(async (database?: string) => await Promise.resolve(extraBookmarks)) + const bookmarksSupplier = jest.fn() + bookmarksSupplier + .mockReturnValueOnce(Promise.resolve(extraBookmarks)) + .mockReturnValue(Promise.resolve([])) const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks], bookmarksSupplier }) - const bookmarks = await manager.getAllBookmarks() + const bookmarksWithExtraBookmarks = await manager.getBookmarks() - expect(bookmarks).toBeSortedEqual( + expect(bookmarksWithExtraBookmarks).toBeSortedEqual( [...neo4jBookmarks, ...systemBookmarks, ...extraBookmarks] ) + + const internalBookmarks = await manager.getBookmarks() + + expect(internalBookmarks).toBeSortedEqual( + [...neo4jBookmarks, ...systemBookmarks] + ) }) it('should return duplicate bookmarks if bookmarksSupplier returns already existing bm', async () => { const extraBookmarks = ['neo4j:bmextra', 'system:bmextra', 'adb:bmextra'] - const bookmarksSupplier = jest.fn(async (database?: string) => await Promise.resolve([...extraBookmarks, ...systemBookmarks])) + const bookmarksSupplier = jest.fn(async () => await Promise.resolve([...extraBookmarks, ...systemBookmarks])) const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks], bookmarksSupplier }) - const bookmarks = await manager.getAllBookmarks() + const bookmarks = await manager.getBookmarks() expect(bookmarks).toBeSortedEqual( [...neo4jBookmarks, ...systemBookmarks, ...extraBookmarks] @@ -157,136 +105,62 @@ describe('BookmarkManager', () => { it('should call bookmarkSupplier for getting all bookmarks', async () => { const bookmarksSupplier = jest.fn() const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks], bookmarksSupplier }) - await manager.getAllBookmarks() + await manager.getBookmarks() expect(bookmarksSupplier).toBeCalledWith() }) }) describe('updateBookmarks()', () => { - it('should remove previous bookmarks and new bookmarks for an existing db', async () => { + it('should replace previous bookmarks with new bookmarks', async () => { const newBookmarks = ['neo4j:bm03'] const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) await manager.updateBookmarks( - 'neo4j', - await manager.getAllBookmarks(), + + await manager.getBookmarks(), newBookmarks ) - await expect(manager.getBookmarks('neo4j')).resolves.toBeSortedEqual(newBookmarks) - await expect(manager.getBookmarks('system')).resolves.toBeSortedEqual(systemBookmarks) + await expect(manager.getBookmarks()).resolves.toBeSortedEqual(newBookmarks) }) it('should not remove bookmarks not present in the original list', async () => { const newBookmarks = ['neo4j:bm03'] const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) - const [bookmarkNotUsedInTx, ...bookmarksUsedInTx] = neo4jBookmarks + const [bookmarkNotUsedInTx, ...bookmarksUsedInTx] = [...neo4jBookmarks, ...systemBookmarks] await manager.updateBookmarks( - 'neo4j', bookmarksUsedInTx, newBookmarks ) - await expect(manager.getBookmarks('neo4j')) + await expect(manager.getBookmarks()) .resolves.toBeSortedEqual([bookmarkNotUsedInTx, ...newBookmarks]) - await expect(manager.getBookmarks('system')).resolves.toBeSortedEqual(systemBookmarks) - }) - - it('should add bookmarks to a non-existing database', async () => { - const newBookmarks = ['neo4j:bm03'] - const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['system', systemBookmarks] - ]) - }) - - await manager.updateBookmarks( - 'neo4j', - [], - newBookmarks - ) - - await expect(manager.getBookmarks('neo4j')).resolves.toBeSortedEqual(newBookmarks) - await expect(manager.getBookmarks('system')).resolves.toBeSortedEqual(systemBookmarks) }) it('should notify new bookmarks', async () => { const bookmarksConsumer = jest.fn() const newBookmarks = ['neo4j:bm03'] const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks], bookmarksConsumer }) await manager.updateBookmarks( - 'neo4j', - await manager.getAllBookmarks(), + await manager.getBookmarks(), newBookmarks ) - expect(bookmarksConsumer).toBeCalledWith('neo4j', newBookmarks) - }) - }) - - describe('forget()', () => { - it('should forget database', async () => { - const extraBookmarks = ['system:bmextra', 'adb:bmextra'] - const bookmarksSupplier = jest.fn(async () => extraBookmarks) - const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), - bookmarksSupplier - }) - - await manager.forget(['neo4j', 'adb']) - const bookmarks = await manager.getAllBookmarks() - - expect(bookmarks).toBeSortedEqual( - [...systemBookmarks, ...extraBookmarks] - ) - }) - - it('should forget what never reminded', async () => { - const extraBookmarks = ['system:bmextra', 'adb:bmextra'] - const bookmarksSupplier = jest.fn(async () => extraBookmarks) - const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]), - bookmarksSupplier - }) - - await manager.forget(['unexisting-db']) - const bookmarks = await manager.getAllBookmarks() - - expect(bookmarks).toBeSortedEqual( - [...systemBookmarks, ...neo4jBookmarks, ...extraBookmarks] - ) + expect(bookmarksConsumer).toBeCalledWith(newBookmarks) }) }) }) diff --git a/packages/core/test/session.test.ts b/packages/core/test/session.test.ts index 6d1d1f417..58795e4f4 100644 --- a/packages/core/test/session.test.ts +++ b/packages/core/test/session.test.ts @@ -315,12 +315,9 @@ describe('session', () => { expect(tx).toBeDefined() }) - it('should acquire connection with system bookmarks from the bookmark manager', async () => { + it('should acquire connection with bookmarks from the bookmark manager', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = mockBeginWithSuccess(newFakeConnection()) @@ -334,16 +331,13 @@ describe('session', () => { await session.beginTransaction() expect(connectionProvider.acquireConnection).toBeCalledWith( - expect.objectContaining({ bookmarks: new bookmarks.Bookmarks(systemBookmarks) }) + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...neo4jBookmarks, ...systemBookmarks]) }) ) }) - it('should acquire connection with system bookmarks from the bookmark manager + lastBookmarks', async () => { + it('should acquire connection with bookmarks from the bookmark manager + lastBookmarks', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = mockBeginWithSuccess(newFakeConnection()) @@ -358,7 +352,7 @@ describe('session', () => { await session.beginTransaction() expect(connectionProvider.acquireConnection).toBeCalledWith( - expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...systemBookmarks]) }) + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...neo4jBookmarks, ...systemBookmarks]) }) ) }) @@ -367,12 +361,9 @@ describe('session', () => { [customBookmarks] ])('should call getAllBookmarks for the relevant database', async (lastBookmarks) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) - const getAllBookmarksSpy = jest.spyOn(manager, 'getAllBookmarks') + const getAllBookmarksSpy = jest.spyOn(manager, 'getBookmarks') const connection = mockBeginWithSuccess(newFakeConnection()) @@ -394,10 +385,7 @@ describe('session', () => { [customBookmarks] ])('should call begin query with getAllBookmarks + lastBookmarks', async (lastBookmarks) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = mockBeginWithSuccess(newFakeConnection()) @@ -425,10 +413,7 @@ describe('session', () => { ['adb'] ])('should not call updateBookmarks when server returns no bookmarks', async (metaDb) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const updateBookmarksSpy = jest.spyOn(manager, 'updateBookmarks') @@ -455,10 +440,7 @@ describe('session', () => { ['adb', 'adb'] ])('should call updateBookmarks when server returns non-empty bookmarks', async (metaDb, updateDb) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const updateBookmarksSpy = jest.spyOn(manager, 'updateBookmarks') @@ -476,7 +458,7 @@ describe('session', () => { await tx.commit() expect(updateBookmarksSpy).toBeCalledTimes(1) - expect(updateBookmarksSpy).toBeCalledWith(updateDb, [...neo4jBookmarks, ...systemBookmarks], customBookmarks) + expect(updateBookmarksSpy).toBeCalledWith([...neo4jBookmarks, ...systemBookmarks], customBookmarks) }) it.each([ @@ -485,10 +467,7 @@ describe('session', () => { ['adb'] ])('should not call updateBookmarks when server returns no bookmarks', async (metaDb) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const updateBookmarksSpy = jest.spyOn(manager, 'updateBookmarks') @@ -548,12 +527,9 @@ describe('session', () => { }) describe('.run()', () => { - it('should acquire connection with system bookmarks from the bookmark manager', async () => { + it('should acquire connection with bookmarks from the bookmark manager', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -567,16 +543,13 @@ describe('session', () => { await session.run('query') expect(connectionProvider.acquireConnection).toBeCalledWith( - expect.objectContaining({ bookmarks: new bookmarks.Bookmarks(systemBookmarks) }) + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...neo4jBookmarks, ...systemBookmarks]) }) ) }) - it('should acquire connection with system bookmarks from the bookmark manager + lastBookmarks', async () => { + it('should acquire connection with bookmarks from the bookmark manager + lastBookmarks', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -591,16 +564,13 @@ describe('session', () => { await session.run('query') expect(connectionProvider.acquireConnection).toBeCalledWith( - expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...systemBookmarks]) }) + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...neo4jBookmarks, ...systemBookmarks]) }) ) }) - it('should acquire connection with system bookmarks from the bookmark manager when bookmarks already updated', async () => { + it('should acquire connection with bookmarks from the bookmark manager when bookmarks already updated', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -618,16 +588,17 @@ describe('session', () => { await session.run('query') - expect(connectionProvider.acquireConnection).toHaveBeenNthCalledWith(2, - expect.objectContaining({ bookmarks: new bookmarks.Bookmarks(systemBookmarks) })) + expect(connectionProvider.acquireConnection).toHaveBeenCalledTimes(2) + expect(connectionProvider.acquireConnection).toHaveBeenNthCalledWith(1, + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...neo4jBookmarks, ...systemBookmarks]) })) + + expect(connectionProvider.acquireConnection).toHaveBeenLastCalledWith( + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks(['other-bookmark']) })) }) - it('should acquire connection with system bookmarks from the bookmark manager + lastBookmarks when bookmarks not updated', async () => { + it('should acquire connection with bookmarks from the bookmark manager + lastBookmarks when bookmarks not updated', async () => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -646,21 +617,18 @@ describe('session', () => { await session.run('query') expect(connectionProvider.acquireConnection).toHaveBeenNthCalledWith(2, - expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...systemBookmarks]) }) + expect.objectContaining({ bookmarks: new bookmarks.Bookmarks([...customBookmarks, ...neo4jBookmarks, ...systemBookmarks]) }) ) }) it.each([ [[]], [customBookmarks] - ])('should call getAllBookmarks for the relevant database', async (lastBookmarks) => { + ])('should call getBookmarks', async (lastBookmarks) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) - const getAllBookmarksSpy = jest.spyOn(manager, 'getAllBookmarks') + const getAllBookmarksSpy = jest.spyOn(manager, 'getBookmarks') const connection = newFakeConnection() @@ -680,12 +648,9 @@ describe('session', () => { it.each([ [[]], [customBookmarks] - ])('should call run query with getAllBookmarks + lastBookmarks', async (lastBookmarks) => { + ])('should call run query with getBookmarks + lastBookmarks', async (lastBookmarks) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -710,12 +675,9 @@ describe('session', () => { it.each([ [[]], [customBookmarks] - ])('should call run query with getAllBookmarks + lastBookmarks when bookmarks not updated', async (lastBookmarks) => { + ])('should call run query with getBookmarks + lastBookmarks when bookmarks not updated', async (lastBookmarks) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -744,12 +706,9 @@ describe('session', () => { it.each([ [[]], [customBookmarks] - ])('should call run query with getAllBookmarks when bookmarks updated', async (lastBookmarks) => { + ])('should call run query with getBookmarks when bookmarks updated', async (lastBookmarks) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const connection = newFakeConnection() @@ -765,13 +724,13 @@ describe('session', () => { await session.run('query') const { afterComplete } = connection.seenProtocolOptions[0] afterComplete({ db: 'neo4j', bookmark: 'abc' }) - await manager.updateBookmarks('neo4j', ['abc'], neo4jBookmarks) + await manager.updateBookmarks(['abc'], neo4jBookmarks) await session.run('query') expect(connection.seenProtocolOptions[1]).toEqual( expect.objectContaining({ - bookmarks: new bookmarks.Bookmarks([...neo4jBookmarks, ...systemBookmarks]) + bookmarks: new bookmarks.Bookmarks([...neo4jBookmarks]) }) ) }) @@ -782,10 +741,7 @@ describe('session', () => { ['adb', 'adb'] ])('should call updateBookmarks when server returns non-empty bookmarks', async (metaDb, updateDb) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const updateBookmarksSpy = jest.spyOn(manager, 'updateBookmarks') @@ -805,7 +761,7 @@ describe('session', () => { afterComplete({ db: metaDb, bookmark: customBookmarks }) - expect(updateBookmarksSpy).toBeCalledWith(updateDb, [...neo4jBookmarks, ...systemBookmarks], customBookmarks) + expect(updateBookmarksSpy).toBeCalledWith([...neo4jBookmarks, ...systemBookmarks], customBookmarks) }) it.each([ @@ -814,10 +770,7 @@ describe('session', () => { ['adb'] ])('should not call updateBookmarks when server returns no bookmarks', async (metaDb) => { const manager = bookmarkManager({ - initialBookmarks: new Map([ - ['neo4j', neo4jBookmarks], - ['system', systemBookmarks] - ]) + initialBookmarks: [...neo4jBookmarks, ...systemBookmarks] }) const updateBookmarksSpy = jest.spyOn(manager, 'updateBookmarks') @@ -847,7 +800,7 @@ function mockBeginWithSuccess (connection: FakeConnection): FakeConnection { connection.protocol = () => { return { ...protocol, - beginTransaction: (params: { afterComplete: () => {}}, ...args: any[]) => { + beginTransaction: (params: { afterComplete: () => {} }, ...args: any[]) => { protocol.beginTransaction([params, ...args]) params.afterComplete() } @@ -861,7 +814,7 @@ function mockCommitWithSuccess (connection: FakeConnection, metadata: any): Fake connection.protocol = () => { return { ...protocol, - commitTransaction: (params: { afterComplete: (metadata: any) => {}}, ...args: any[]) => { + commitTransaction: (params: { afterComplete: (metadata: any) => {} }, ...args: any[]) => { const observer = protocol.commitTransaction(...[params, ...args]) params.afterComplete(metadata) return observer @@ -915,7 +868,7 @@ function setupSession ({ }) if (beginTx) { - session.beginTransaction().catch(e => {}) // force session to acquire new connection + session.beginTransaction().catch(e => { }) // force session to acquire new connection } return { session, connectionProvider } } diff --git a/packages/neo4j-driver-deno/lib/core/bookmark-manager.ts b/packages/neo4j-driver-deno/lib/core/bookmark-manager.ts index 3d2a36168..9ebee832f 100644 --- a/packages/neo4j-driver-deno/lib/core/bookmark-manager.ts +++ b/packages/neo4j-driver-deno/lib/core/bookmark-manager.ts @@ -36,54 +36,30 @@ export default class BookmarkManager { * Method called when the bookmarks get updated when a transaction finished. * * This method will be called when auto-commit queries finish and when explicit transactions - * get commited. + * get committed. * - * @param {string} database The database which the bookmarks belongs to * @param {Iterable} previousBookmarks The bookmarks used when starting the transaction * @param {Iterable} newBookmarks The new bookmarks received at the end of the transaction. * @returns {void} */ - async updateBookmarks (database: string, previousBookmarks: Iterable, newBookmarks: Iterable): Promise { + async updateBookmarks (previousBookmarks: Iterable, newBookmarks: Iterable): Promise { throw new Error('Not implemented') } /** - * Method called by the driver to get the bookmarks for one specific database + * Method called by the driver to get the bookmarks. * - * @param {string} database The database which the bookmarks belong to * @returns {Iterable} The set of bookmarks */ - async getBookmarks (database: string): Promise> { - throw new Error('Not implemented') - } - - /** - * Method called by the driver for getting all the bookmarks. - * - * This method should return all bookmarks for all databases present in the BookmarkManager. - * - * @returns {Iterable} The set of bookmarks - */ - async getAllBookmarks (): Promise> { - throw new Error('Not implemented') - } - - /** - * Forget the databases and its bookmarks - * - * This method is not called by the driver. Forgetting unused databases is the user's responsibility. - * - * @param {Iterable} databases The databases which the bookmarks will be removed for. - */ - async forget (databases: Iterable): Promise { + async getBookmarks (): Promise> { throw new Error('Not implemented') } } export interface BookmarkManagerConfig { - initialBookmarks?: Map> - bookmarksSupplier?: (database?: string) => Promise> - bookmarksConsumer?: (database: string, bookmarks: Iterable) => Promise + initialBookmarks?: Iterable + bookmarksSupplier?: () => Promise> + bookmarksConsumer?: (bookmarks: Iterable) => Promise } /** @@ -91,11 +67,9 @@ export interface BookmarkManagerConfig { * * @since 5.0 * @experimental - * @property {Map>} [initialBookmarks@experimental] Defines the initial set of bookmarks. The key is the database name and the values are the bookmarks. - * @property {function([database]: string):Promise>} [bookmarksSupplier] Called for supplying extra bookmarks to the BookmarkManager - * 1. supplying bookmarks from the given database when the default BookmarkManager's `.getBookmarks(database)` gets called. - * 2. supplying all the bookmarks when the default BookmarkManager's `.getAllBookmarks()` gets called - * @property {function(database: string, bookmarks: Iterable): Promise} [bookmarksConsumer] Called when the set of bookmarks for database get updated + * @property {Iterable} [initialBookmarks] Defines the initial set of bookmarks. The key is the database name and the values are the bookmarks. + * @property {function():Promise>} [bookmarksSupplier] Called for supplying extra bookmarks to the BookmarkManager + * @property {function(bookmarks: Iterable): Promise} [bookmarksConsumer] Called when the set of bookmarks get updated */ /** * Provides an configured {@link BookmarkManager} instance. @@ -106,9 +80,7 @@ export interface BookmarkManagerConfig { * @returns {BookmarkManager} */ export function bookmarkManager (config: BookmarkManagerConfig = {}): BookmarkManager { - const initialBookmarks = new Map>() - - config.initialBookmarks?.forEach((v, k) => initialBookmarks.set(k, new Set(v))) + const initialBookmarks = new Set(config.initialBookmarks) return new Neo4jBookmarkManager( initialBookmarks, @@ -119,15 +91,15 @@ export function bookmarkManager (config: BookmarkManagerConfig = {}): BookmarkMa class Neo4jBookmarkManager implements BookmarkManager { constructor ( - private readonly _bookmarksPerDb: Map>, - private readonly _bookmarksSupplier?: (database?: string) => Promise>, - private readonly _bookmarksConsumer?: (database: string, bookmark: Iterable) => Promise + private readonly _bookmarks: Set, + private readonly _bookmarksSupplier?: () => Promise>, + private readonly _bookmarksConsumer?: (bookmark: Iterable) => Promise ) { } - async updateBookmarks (database: string, previousBookmarks: Iterable, newBookmarks: Iterable): Promise { - const bookmarks = this._getOrInitializeBookmarks(database) + async updateBookmarks (previousBookmarks: Iterable, newBookmarks: Iterable): Promise { + const bookmarks = this._bookmarks for (const bm of previousBookmarks) { bookmarks.delete(bm) } @@ -135,40 +107,13 @@ class Neo4jBookmarkManager implements BookmarkManager { bookmarks.add(bm) } if (typeof this._bookmarksConsumer === 'function') { - await this._bookmarksConsumer(database, [...bookmarks]) - } - } - - private _getOrInitializeBookmarks (database: string): Set { - let maybeBookmarks = this._bookmarksPerDb.get(database) - if (maybeBookmarks === undefined) { - maybeBookmarks = new Set() - this._bookmarksPerDb.set(database, maybeBookmarks) - } - return maybeBookmarks - } - - async getBookmarks (database: string): Promise> { - const bookmarks = new Set(this._bookmarksPerDb.get(database)) - - if (typeof this._bookmarksSupplier === 'function') { - const suppliedBookmarks = await this._bookmarksSupplier(database) ?? [] - for (const bm of suppliedBookmarks) { - bookmarks.add(bm) - } + await this._bookmarksConsumer([...bookmarks]) } - - return [...bookmarks] } - async getAllBookmarks (): Promise> { - const bookmarks = new Set() + async getBookmarks (): Promise> { + const bookmarks = new Set(this._bookmarks) - for (const [, dbBookmarks] of this._bookmarksPerDb) { - for (const bm of dbBookmarks) { - bookmarks.add(bm) - } - } if (typeof this._bookmarksSupplier === 'function') { const suppliedBookmarks = await this._bookmarksSupplier() ?? [] for (const bm of suppliedBookmarks) { @@ -176,12 +121,6 @@ class Neo4jBookmarkManager implements BookmarkManager { } } - return bookmarks - } - - async forget (databases: Iterable): Promise { - for (const database of databases) { - this._bookmarksPerDb.delete(database) - } + return [...bookmarks] } } diff --git a/packages/neo4j-driver-deno/lib/core/driver.ts b/packages/neo4j-driver-deno/lib/core/driver.ts index 5244aa67a..b7850ddd8 100644 --- a/packages/neo4j-driver-deno/lib/core/driver.ts +++ b/packages/neo4j-driver-deno/lib/core/driver.ts @@ -194,10 +194,10 @@ class SessionConfig { * Enabling it is done by supplying an BookmarkManager implementation instance to this param. * A default implementation could be acquired by calling the factory function {@link bookmarkManager}. * - * **Warning**: Share the same BookmarkManager instance accross all session can have a negative impact + * **Warning**: Sharing the same BookmarkManager instance across multiple sessions can have a negative impact * on performance since all the queries will wait for the latest changes being propagated across the cluster. * For keeping consistency between a group of queries, use {@link Session} for grouping them. - * For keeping consistency between a group of sessions, use {@link BookmarkManager} instance for groupping them. + * For keeping consistency between a group of sessions, use {@link BookmarkManager} instance for grouping them. * * @example * const bookmarkManager = neo4j.bookmarkManager() @@ -214,7 +214,7 @@ class SessionConfig { * * // Reading Driver User will wait of the changes being propagated to the server before RUN the query * // So the 'Driver User' person should exist in the Result, unless deleted. - * const linkedSesssion2 = await linkedSession2.run('CREATE (p:Person {name: $name}) RETURN p', { name: 'Driver User'}) + * const linkedResult = await linkedSession2.run('CREATE (p:Person {name: $name}) RETURN p', { name: 'Driver User'}) * * await linkedSession1.close() * await linkedSession2.close() diff --git a/packages/neo4j-driver-deno/lib/core/session.ts b/packages/neo4j-driver-deno/lib/core/session.ts index fd6770345..ab8cd9750 100644 --- a/packages/neo4j-driver-deno/lib/core/session.ts +++ b/packages/neo4j-driver-deno/lib/core/session.ts @@ -343,7 +343,7 @@ class Session { } private async _bookmarks (): Promise { - const bookmarks = await this._bookmarkManager?.getAllBookmarks() + const bookmarks = await this._bookmarkManager?.getBookmarks() if (bookmarks === undefined) { return this._lastBookmarks } @@ -489,7 +489,7 @@ class Session { } private async _getConnectionAcquistionBookmarks (): Promise { - const bookmarks = await this._bookmarkManager?.getBookmarks('system') + const bookmarks = await this._bookmarkManager?.getBookmarks() if (bookmarks === undefined) { return this._lastBookmarks } @@ -505,7 +505,6 @@ class Session { _updateBookmarks (newBookmarks?: Bookmarks, previousBookmarks?: Bookmarks, database?: string): void { if ((newBookmarks != null) && !newBookmarks.isEmpty()) { this._bookmarkManager?.updateBookmarks( - database ?? this._database, previousBookmarks?.values() ?? [], newBookmarks?.values() ?? [] ) diff --git a/packages/neo4j-driver-lite/test/unit/index.test.ts b/packages/neo4j-driver-lite/test/unit/index.test.ts index c19f37ae6..8b7476756 100644 --- a/packages/neo4j-driver-lite/test/unit/index.test.ts +++ b/packages/neo4j-driver-lite/test/unit/index.test.ts @@ -93,16 +93,10 @@ describe('index', () => { it('should treat BookmarkManager as an interface', () => { const bookmarkManager: BookmarkManager = { - async getAllBookmarks (): Promise { + async getBookmarks (): Promise { return [] }, - async getBookmarks (database: string): Promise { - return [] - }, - async updateBookmarks (database: string, previousBookmarks: string[], newBookmarks: string[]): Promise { - - }, - async forget (databases: string[]): Promise { + async updateBookmarks (previousBookmarks: string[], newBookmarks: string[]): Promise { } } diff --git a/packages/testkit-backend/src/request-handlers.js b/packages/testkit-backend/src/request-handlers.js index 49b8fb0a2..3a030f7e4 100644 --- a/packages/testkit-backend/src/request-handlers.js +++ b/packages/testkit-backend/src/request-handlers.js @@ -428,21 +428,18 @@ export function NewBookmarkManager ( const id = context.addBookmarkManager((bookmarkManagerId) => { let bookmarksSupplier let bookmarksConsumer - if (initialBookmarks != null) { - initialBookmarks = new Map(Object.entries(initialBookmarks)) - } if (bookmarksSupplierRegistered === true) { - bookmarksSupplier = (database) => + bookmarksSupplier = () => new Promise((resolve, reject) => { const id = context.addBookmarkSupplierRequest(resolve, reject) - wire.writeResponse(responses.BookmarksSupplierRequest({ id, bookmarkManagerId, database })) + wire.writeResponse(responses.BookmarksSupplierRequest({ id, bookmarkManagerId })) }) } if (bookmarksConsumerRegistered === true) { - bookmarksConsumer = (database, bookmarks) => + bookmarksConsumer = (bookmarks) => new Promise((resolve, reject) => { const id = context.addNotifyBookmarksRequest(resolve, reject) - wire.writeResponse(responses.BookmarksConsumerRequest({ id, bookmarkManagerId, database, bookmarks })) + wire.writeResponse(responses.BookmarksConsumerRequest({ id, bookmarkManagerId, bookmarks })) }) } bookmarkManager = neo4j.bookmarkManager({