From cd77bbb0b9895b77a72087a1edcde3bdf858d0aa Mon Sep 17 00:00:00 2001 From: Florent Biville Date: Tue, 14 Feb 2023 13:57:38 +0100 Subject: [PATCH 1/2] Comply to ExecuteQuery ADR latest revision --- packages/core/src/driver.ts | 12 ++++----- packages/core/src/result-transformers.ts | 7 +++-- packages/core/test/driver.test.ts | 6 ++--- .../core/test/result-transformers.test.ts | 27 +++++++++++++++++++ 4 files changed, 41 insertions(+), 11 deletions(-) diff --git a/packages/core/src/driver.ts b/packages/core/src/driver.ts index f9745f35b..11129d090 100644 --- a/packages/core/src/driver.ts +++ b/packages/core/src/driver.ts @@ -311,7 +311,7 @@ class QueryConfig { * A BookmarkManager is a piece of software responsible for keeping casual consistency between different pieces of work by sharing bookmarks * between the them. * - * By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.queryBookmarkManager} + * By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.defaultExecuteQueryBookmarkManager} * * Can be set to null to disable causal chaining. * @type {BookmarkManager|null} @@ -338,7 +338,7 @@ class Driver { private readonly _createConnectionProvider: CreateConnectionProvider private _connectionProvider: ConnectionProvider | null private readonly _createSession: CreateSession - private readonly _queryBookmarkManager: BookmarkManager + private readonly _defaultExecuteQueryBookmarkManager: BookmarkManager private readonly _queryExecutor: QueryExecutor /** @@ -369,7 +369,7 @@ class Driver { this._log = log this._createConnectionProvider = createConnectionProvider this._createSession = createSession - this._queryBookmarkManager = bookmarkManager() + this._defaultExecuteQueryBookmarkManager = bookmarkManager() this._queryExecutor = createQueryExecutor(this.session.bind(this)) /** @@ -389,8 +389,8 @@ class Driver { * @type {BookmarkManager} * @returns {BookmarkManager} */ - get queryBookmarkManager (): BookmarkManager { - return this._queryBookmarkManager + get defaultExecuteQueryBookmarkManager (): BookmarkManager { + return this._defaultExecuteQueryBookmarkManager } /** @@ -472,7 +472,7 @@ class Driver { * @see https://github.com/neo4j/neo4j-javascript-driver/discussions/1052 */ async executeQuery (query: Query, parameters?: any, config: QueryConfig = {}): Promise { - const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.queryBookmarkManager) + const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.defaultExecuteQueryBookmarkManager) const resultTransformer = (config.resultTransformer ?? resultTransformers.eagerResultTransformer()) as ResultTransformer const routingConfig: string = config.routing ?? routing.WRITERS diff --git a/packages/core/src/result-transformers.ts b/packages/core/src/result-transformers.ts index c11b66ccc..041df5540 100644 --- a/packages/core/src/result-transformers.ts +++ b/packages/core/src/result-transformers.ts @@ -137,7 +137,7 @@ class ResultTransformers { */ mappedResultTransformer < R = Record, T = { records: R[], keys: string[], summary: ResultSummary } - >(config: { map?: (rec: Record) => R, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer { + >(config: { map?: (rec: Record) => R | undefined, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer { if (config == null || (config.collect == null && config.map == null)) { throw newError('Requires a map or/and a collect functions.') } @@ -151,7 +151,10 @@ class ResultTransformers { }, onNext (record: Record) { if (config.map != null) { - state.records.push(config.map(record)) + const mappedRecord = config.map(record) + if (mappedRecord !== undefined) { + state.records.push(mappedRecord) + } } else { state.records.push(record as unknown as R) } diff --git a/packages/core/test/driver.test.ts b/packages/core/test/driver.test.ts index 2ac292125..d78148a04 100644 --- a/packages/core/test/driver.test.ts +++ b/packages/core/test/driver.test.ts @@ -335,7 +335,7 @@ describe('Driver', () => { expect(eagerResult).toEqual(expected) expect(spiedExecute).toBeCalledWith({ resultTransformer: resultTransformers.eagerResultTransformer(), - bookmarkManager: driver?.queryBookmarkManager, + bookmarkManager: driver?.defaultExecuteQueryBookmarkManager, routing: routing.WRITERS, database: undefined, impersonatedUser: undefined @@ -361,7 +361,7 @@ describe('Driver', () => { expect(summary).toEqual(expected.summary) expect(spiedExecute).toBeCalledWith({ resultTransformer: resultTransformers.eagerResultTransformer(), - bookmarkManager: driver?.queryBookmarkManager, + bookmarkManager: driver?.defaultExecuteQueryBookmarkManager, routing: routing.WRITERS, database: undefined, impersonatedUser: undefined @@ -485,7 +485,7 @@ describe('Driver', () => { return () => { const defaultConfig = { resultTransformer: resultTransformers.eagerResultTransformer(), - bookmarkManager: driver?.queryBookmarkManager, + bookmarkManager: driver?.defaultExecuteQueryBookmarkManager, routing: routing.WRITERS, database: undefined, impersonatedUser: undefined diff --git a/packages/core/test/result-transformers.test.ts b/packages/core/test/result-transformers.test.ts index 30f1f3855..609c6a0bb 100644 --- a/packages/core/test/result-transformers.test.ts +++ b/packages/core/test/result-transformers.test.ts @@ -191,6 +191,33 @@ describe('resultTransformers', () => { expect(collect).toHaveBeenCalledWith(rawRecords.map(rec => new Record(keys, rec)), new ResultSummary(query, params, meta), keys) }) + it('should skip the undefined records', async () => { + const { + rawRecords, + result, + keys + } = scenario() + let firstCall = true + const map = jest.fn((record) => { + if (firstCall) { + firstCall = false + return undefined + } + return record.get('a') as number + }) + + const transform = resultTransformers.mappedResultTransformer({ map }) + + const { records: as }: { records: number[] } = await transform(result) + + const [,...tailRecords] = rawRecords + expect(as).toEqual(tailRecords.map(record => record[keys.indexOf('a')])) + expect(map).toHaveBeenCalledTimes(rawRecords.length) + for (const rawRecord of rawRecords) { + expect(map).toHaveBeenCalledWith(new Record(keys, rawRecord)) + } + }) + it.each([ undefined, null, From 08c7979101815da0a3eca2517f287a7b65251d94 Mon Sep 17 00:00:00 2001 From: Florent Biville Date: Wed, 15 Feb 2023 11:53:18 +0100 Subject: [PATCH 2/2] Add deno changes --- packages/neo4j-driver-deno/lib/core/driver.ts | 12 ++++++------ .../lib/core/result-transformers.ts | 7 +++++-- 2 files changed, 11 insertions(+), 8 deletions(-) diff --git a/packages/neo4j-driver-deno/lib/core/driver.ts b/packages/neo4j-driver-deno/lib/core/driver.ts index 7c5cb12c5..d79341a71 100644 --- a/packages/neo4j-driver-deno/lib/core/driver.ts +++ b/packages/neo4j-driver-deno/lib/core/driver.ts @@ -311,7 +311,7 @@ class QueryConfig { * A BookmarkManager is a piece of software responsible for keeping casual consistency between different pieces of work by sharing bookmarks * between the them. * - * By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.queryBookmarkManager} + * By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.defaultExecuteQueryBookmarkManager} * * Can be set to null to disable causal chaining. * @type {BookmarkManager|null} @@ -338,7 +338,7 @@ class Driver { private readonly _createConnectionProvider: CreateConnectionProvider private _connectionProvider: ConnectionProvider | null private readonly _createSession: CreateSession - private readonly _queryBookmarkManager: BookmarkManager + private readonly _defaultExecuteQueryBookmarkManager: BookmarkManager private readonly _queryExecutor: QueryExecutor /** @@ -369,7 +369,7 @@ class Driver { this._log = log this._createConnectionProvider = createConnectionProvider this._createSession = createSession - this._queryBookmarkManager = bookmarkManager() + this._defaultExecuteQueryBookmarkManager = bookmarkManager() this._queryExecutor = createQueryExecutor(this.session.bind(this)) /** @@ -389,8 +389,8 @@ class Driver { * @type {BookmarkManager} * @returns {BookmarkManager} */ - get queryBookmarkManager (): BookmarkManager { - return this._queryBookmarkManager + get defaultExecuteQueryBookmarkManager (): BookmarkManager { + return this._defaultExecuteQueryBookmarkManager } /** @@ -472,7 +472,7 @@ class Driver { * @see https://github.com/neo4j/neo4j-javascript-driver/discussions/1052 */ async executeQuery (query: Query, parameters?: any, config: QueryConfig = {}): Promise { - const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.queryBookmarkManager) + const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.defaultExecuteQueryBookmarkManager) const resultTransformer = (config.resultTransformer ?? resultTransformers.eagerResultTransformer()) as ResultTransformer const routingConfig: string = config.routing ?? routing.WRITERS diff --git a/packages/neo4j-driver-deno/lib/core/result-transformers.ts b/packages/neo4j-driver-deno/lib/core/result-transformers.ts index f0e5f636b..30fbb6a20 100644 --- a/packages/neo4j-driver-deno/lib/core/result-transformers.ts +++ b/packages/neo4j-driver-deno/lib/core/result-transformers.ts @@ -137,7 +137,7 @@ class ResultTransformers { */ mappedResultTransformer < R = Record, T = { records: R[], keys: string[], summary: ResultSummary } - >(config: { map?: (rec: Record) => R, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer { + >(config: { map?: (rec: Record) => R | undefined, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer { if (config == null || (config.collect == null && config.map == null)) { throw newError('Requires a map or/and a collect functions.') } @@ -151,7 +151,10 @@ class ResultTransformers { }, onNext (record: Record) { if (config.map != null) { - state.records.push(config.map(record)) + const mappedRecord = config.map(record) + if (mappedRecord !== undefined) { + state.records.push(mappedRecord) + } } else { state.records.push(record as unknown as R) }