Comply to ExecuteQuery ADR latest revision (#1059)

fbiville · web-flow · commit 38f9ccc69f57 · 2023-02-15T15:51:09.000+01:00
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.
diff --git a/packages/core/src/driver.ts b/packages/core/src/driver.ts
@@ -311,7 +311,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}
@@ -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<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
 
diff --git 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<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.')
     }
@@ -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
@@ -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
@@ -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,
diff --git a/packages/neo4j-driver-deno/lib/core/driver.ts b/packages/neo4j-driver-deno/lib/core/driver.ts
@@ -311,7 +311,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}
@@ -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<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
 
diff --git 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<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.')
     }
@@ -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)
             }
