Skip to content

Commit 38f9ccc

Browse files
fbivillefbiville
authored
Comply to ExecuteQuery ADR latest revision (#1059)
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.
1 parent 15386b7 commit 38f9ccc

File tree

6 files changed

+52
-19
lines changed

6 files changed

+52
-19
lines changed

‎packages/core/src/driver.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ class QueryConfig<T = EagerResult> {
311311
* A BookmarkManager is a piece of software responsible for keeping casual consistency between different pieces of work by sharing bookmarks
312312
* between the them.
313313
*
314-
* By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.queryBookmarkManager}
314+
* By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.defaultExecuteQueryBookmarkManager}
315315
*
316316
* Can be set to null to disable causal chaining.
317317
* @type {BookmarkManager|null}
@@ -338,7 +338,7 @@ class Driver {
338338
private readonly _createConnectionProvider: CreateConnectionProvider
339339
private _connectionProvider: ConnectionProvider | null
340340
private readonly _createSession: CreateSession
341-
private readonly _queryBookmarkManager: BookmarkManager
341+
private readonly _defaultExecuteQueryBookmarkManager: BookmarkManager
342342
private readonly _queryExecutor: QueryExecutor
343343

344344
/**
@@ -369,7 +369,7 @@ class Driver {
369369
this._log = log
370370
this._createConnectionProvider = createConnectionProvider
371371
this._createSession = createSession
372-
this._queryBookmarkManager = bookmarkManager()
372+
this._defaultExecuteQueryBookmarkManager = bookmarkManager()
373373
this._queryExecutor = createQueryExecutor(this.session.bind(this))
374374

375375
/**
@@ -389,8 +389,8 @@ class Driver {
389389
* @type {BookmarkManager}
390390
* @returns {BookmarkManager}
391391
*/
392-
get queryBookmarkManager (): BookmarkManager {
393-
return this._queryBookmarkManager
392+
get defaultExecuteQueryBookmarkManager (): BookmarkManager {
393+
return this._defaultExecuteQueryBookmarkManager
394394
}
395395

396396
/**
@@ -472,7 +472,7 @@ class Driver {
472472
* @see https://github.com/neo4j/neo4j-javascript-driver/discussions/1052
473473
*/
474474
async executeQuery<T = EagerResult> (query: Query, parameters?: any, config: QueryConfig<T> = {}): Promise<T> {
475-
const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.queryBookmarkManager)
475+
const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.defaultExecuteQueryBookmarkManager)
476476
const resultTransformer = (config.resultTransformer ?? resultTransformers.eagerResultTransformer()) as ResultTransformer<T>
477477
const routingConfig: string = config.routing ?? routing.WRITERS
478478

‎packages/core/src/result-transformers.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class ResultTransformers {
137137
*/
138138
mappedResultTransformer <
139139
R = Record, T = { records: R[], keys: string[], summary: ResultSummary }
140-
>(config: { map?: (rec: Record) => R, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer<T> {
140+
>(config: { map?: (rec: Record) => R | undefined, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer<T> {
141141
if (config == null || (config.collect == null && config.map == null)) {
142142
throw newError('Requires a map or/and a collect functions.')
143143
}
@@ -151,7 +151,10 @@ class ResultTransformers {
151151
},
152152
onNext (record: Record) {
153153
if (config.map != null) {
154-
state.records.push(config.map(record))
154+
const mappedRecord = config.map(record)
155+
if (mappedRecord !== undefined) {
156+
state.records.push(mappedRecord)
157+
}
155158
} else {
156159
state.records.push(record as unknown as R)
157160
}

‎packages/core/test/driver.test.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -335,7 +335,7 @@ describe('Driver', () => {
335335
expect(eagerResult).toEqual(expected)
336336
expect(spiedExecute).toBeCalledWith({
337337
resultTransformer: resultTransformers.eagerResultTransformer(),
338-
bookmarkManager: driver?.queryBookmarkManager,
338+
bookmarkManager: driver?.defaultExecuteQueryBookmarkManager,
339339
routing: routing.WRITERS,
340340
database: undefined,
341341
impersonatedUser: undefined
@@ -361,7 +361,7 @@ describe('Driver', () => {
361361
expect(summary).toEqual(expected.summary)
362362
expect(spiedExecute).toBeCalledWith({
363363
resultTransformer: resultTransformers.eagerResultTransformer(),
364-
bookmarkManager: driver?.queryBookmarkManager,
364+
bookmarkManager: driver?.defaultExecuteQueryBookmarkManager,
365365
routing: routing.WRITERS,
366366
database: undefined,
367367
impersonatedUser: undefined
@@ -485,7 +485,7 @@ describe('Driver', () => {
485485
return () => {
486486
const defaultConfig = {
487487
resultTransformer: resultTransformers.eagerResultTransformer(),
488-
bookmarkManager: driver?.queryBookmarkManager,
488+
bookmarkManager: driver?.defaultExecuteQueryBookmarkManager,
489489
routing: routing.WRITERS,
490490
database: undefined,
491491
impersonatedUser: undefined

‎packages/core/test/result-transformers.test.ts‎

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -191,6 +191,33 @@ describe('resultTransformers', () => {
191191
expect(collect).toHaveBeenCalledWith(rawRecords.map(rec => new Record(keys, rec)), new ResultSummary(query, params, meta), keys)
192192
})
193193

194+
it('should skip the undefined records', async () => {
195+
const {
196+
rawRecords,
197+
result,
198+
keys
199+
} = scenario()
200+
let firstCall = true
201+
const map = jest.fn((record) => {
202+
if (firstCall) {
203+
firstCall = false
204+
return undefined
205+
}
206+
return record.get('a') as number
207+
})
208+
209+
const transform = resultTransformers.mappedResultTransformer({ map })
210+
211+
const { records: as }: { records: number[] } = await transform(result)
212+
213+
const [,...tailRecords] = rawRecords
214+
expect(as).toEqual(tailRecords.map(record => record[keys.indexOf('a')]))
215+
expect(map).toHaveBeenCalledTimes(rawRecords.length)
216+
for (const rawRecord of rawRecords) {
217+
expect(map).toHaveBeenCalledWith(new Record(keys, rawRecord))
218+
}
219+
})
220+
194221
it.each([
195222
undefined,
196223
null,

‎packages/neo4j-driver-deno/lib/core/driver.ts‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -311,7 +311,7 @@ class QueryConfig<T = EagerResult> {
311311
* A BookmarkManager is a piece of software responsible for keeping casual consistency between different pieces of work by sharing bookmarks
312312
* between the them.
313313
*
314-
* By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.queryBookmarkManager}
314+
* By default, it uses the driver's non mutable driver level bookmark manager. See, {@link Driver.defaultExecuteQueryBookmarkManager}
315315
*
316316
* Can be set to null to disable causal chaining.
317317
* @type {BookmarkManager|null}
@@ -338,7 +338,7 @@ class Driver {
338338
private readonly _createConnectionProvider: CreateConnectionProvider
339339
private _connectionProvider: ConnectionProvider | null
340340
private readonly _createSession: CreateSession
341-
private readonly _queryBookmarkManager: BookmarkManager
341+
private readonly _defaultExecuteQueryBookmarkManager: BookmarkManager
342342
private readonly _queryExecutor: QueryExecutor
343343

344344
/**
@@ -369,7 +369,7 @@ class Driver {
369369
this._log = log
370370
this._createConnectionProvider = createConnectionProvider
371371
this._createSession = createSession
372-
this._queryBookmarkManager = bookmarkManager()
372+
this._defaultExecuteQueryBookmarkManager = bookmarkManager()
373373
this._queryExecutor = createQueryExecutor(this.session.bind(this))
374374

375375
/**
@@ -389,8 +389,8 @@ class Driver {
389389
* @type {BookmarkManager}
390390
* @returns {BookmarkManager}
391391
*/
392-
get queryBookmarkManager (): BookmarkManager {
393-
return this._queryBookmarkManager
392+
get defaultExecuteQueryBookmarkManager (): BookmarkManager {
393+
return this._defaultExecuteQueryBookmarkManager
394394
}
395395

396396
/**
@@ -472,7 +472,7 @@ class Driver {
472472
* @see https://github.com/neo4j/neo4j-javascript-driver/discussions/1052
473473
*/
474474
async executeQuery<T = EagerResult> (query: Query, parameters?: any, config: QueryConfig<T> = {}): Promise<T> {
475-
const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.queryBookmarkManager)
475+
const bookmarkManager = config.bookmarkManager === null ? undefined : (config.bookmarkManager ?? this.defaultExecuteQueryBookmarkManager)
476476
const resultTransformer = (config.resultTransformer ?? resultTransformers.eagerResultTransformer()) as ResultTransformer<T>
477477
const routingConfig: string = config.routing ?? routing.WRITERS
478478

‎packages/neo4j-driver-deno/lib/core/result-transformers.ts‎

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -137,7 +137,7 @@ class ResultTransformers {
137137
*/
138138
mappedResultTransformer <
139139
R = Record, T = { records: R[], keys: string[], summary: ResultSummary }
140-
>(config: { map?: (rec: Record) => R, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer<T> {
140+
>(config: { map?: (rec: Record) => R | undefined, collect?: (records: R[], summary: ResultSummary, keys: string[]) => T }): ResultTransformer<T> {
141141
if (config == null || (config.collect == null && config.map == null)) {
142142
throw newError('Requires a map or/and a collect functions.')
143143
}
@@ -151,7 +151,10 @@ class ResultTransformers {
151151
},
152152
onNext (record: Record) {
153153
if (config.map != null) {
154-
state.records.push(config.map(record))
154+
const mappedRecord = config.map(record)
155+
if (mappedRecord !== undefined) {
156+
state.records.push(mappedRecord)
157+
}
155158
} else {
156159
state.records.push(record as unknown as R)
157160
}

0 commit comments

Comments
 (0)