Skip to content

Commit

Permalink
Comply to ExecuteQuery ADR latest revision (neo4j#1059)
Browse files Browse the repository at this point in the history 
Align naming of the default bookmark manager getter with the prescribed one in the latest version of the ADR.

Allow filtering of records by skipping `undefined` results. 
This avoid loading all mapped records into memory when some should be discarded.
  • Loading branch information
@fbiville @bigmontz
fbiville authored and bigmontz committed Mar 6, 2023
1 parent 5cf514c commit a76a8b5
Show file tree
Hide file tree
Showing 6 changed files with 52 additions and 19 deletions.
12 changes: 6 additions & 6 deletions packages/core/src/driver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ class QueryConfig<T = EagerResult> {
* 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}
Expand All @@ -361,7 +361,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

/**
Expand Down Expand Up @@ -392,7 +392,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))

/**
Expand All @@ -412,8 +412,8 @@ class Driver {
* @type {BookmarkManager}
* @returns {BookmarkManager}
*/
get queryBookmarkManager (): BookmarkManager {
return this._queryBookmarkManager
get defaultExecuteQueryBookmarkManager (): BookmarkManager {
return this._defaultExecuteQueryBookmarkManager
}

/**
Expand Down Expand Up @@ -495,7 +495,7 @@ class Driver {
* @see https://github.com/neo4j/neo4j-javascript-driver/discussions/1052
*/
async executeQuery<T = EagerResult> (query: Query, parameters?: any, config: QueryConfig<T> = {}): Promise<T> {
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<T>
const routingConfig: string = config.routing ?? routing.WRITERS

Expand Down
7 changes: 5 additions & 2 deletions packages/core/src/result-transformers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
>(config: { map?: (rec: Record) => R | undefined, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer<T> {
if (config == null || (config.collect == null && config.map == null)) {
throw newError('Requires a map or/and a collect functions.')
}
Expand All @@ -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)
}
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/driver.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -372,7 +372,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
Expand All @@ -398,7 +398,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
Expand Down Expand Up @@ -522,7 +522,7 @@ describe('Driver', () => {
return () => {
const defaultConfig = {
resultTransformer: resultTransformers.eagerResultTransformer(),
bookmarkManager: driver?.queryBookmarkManager,
bookmarkManager: driver?.defaultExecuteQueryBookmarkManager,
routing: routing.WRITERS,
database: undefined,
impersonatedUser: undefined
Expand Down
27 changes: 27 additions & 0 deletions packages/core/test/result-transformers.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
12 changes: 6 additions & 6 deletions packages/neo4j-driver-deno/lib/core/driver.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -334,7 +334,7 @@ class QueryConfig<T = EagerResult> {
* 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}
Expand All @@ -361,7 +361,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

/**
Expand Down Expand Up @@ -392,7 +392,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))

/**
Expand All @@ -412,8 +412,8 @@ class Driver {
* @type {BookmarkManager}
* @returns {BookmarkManager}
*/
get queryBookmarkManager (): BookmarkManager {
return this._queryBookmarkManager
get defaultExecuteQueryBookmarkManager (): BookmarkManager {
return this._defaultExecuteQueryBookmarkManager
}

/**
Expand Down Expand Up @@ -495,7 +495,7 @@ class Driver {
* @see https://github.com/neo4j/neo4j-javascript-driver/discussions/1052
*/
async executeQuery<T = EagerResult> (query: Query, parameters?: any, config: QueryConfig<T> = {}): Promise<T> {
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<T>
const routingConfig: string = config.routing ?? routing.WRITERS

Expand Down
7 changes: 5 additions & 2 deletions packages/neo4j-driver-deno/lib/core/result-transformers.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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<T> {
>(config: { map?: (rec: Record) => R | undefined, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer<T> {
if (config == null || (config.collect == null && config.map == null)) {
throw newError('Requires a map or/and a collect functions.')
}
Expand All @@ -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)
}
Expand Down

0 comments on commit a76a8b5

Please sign in to comment.