Converter cleanup

googleapis · Aug 26, 2024 · 50f061b · 50f061b
1 parent e957017
commit 50f061b
Show file tree

Hide file tree

Showing 5 changed files with 39 additions and 60 deletions.
diff --git a/dev/src/pipeline-util.ts b/dev/src/pipeline-util.ts
@@ -42,10 +42,7 @@ import api = protos.google.firestore.v1;
  * @private
  * @internal
  */
-export class ExecutionUtil<
-  AppModelType,
-  DbModelType extends firestore.DocumentData,
-> {
+export class ExecutionUtil<AppModelType> {
   constructor(
     /** @private */
     readonly _firestore: Firestore,
@@ -55,15 +52,15 @@ export class ExecutionUtil<
   ) {}
 
   _getResponse(
-    pipeline: Pipeline<AppModelType, DbModelType>,
+    pipeline: Pipeline<AppModelType>,
     transactionOrReadTime?: Uint8Array | Timestamp | api.ITransactionOptions,
     explainOptions?: firestore.ExplainOptions
-  ): Promise<Array<PipelineResult<AppModelType, DbModelType>> | undefined> {
+  ): Promise<Array<PipelineResult<AppModelType>> | undefined> {
     // Capture the error stack to preserve stack tracing across async calls.
     const stack = Error().stack!;
 
     return new Promise((resolve, reject) => {
-      const results: Array<PipelineResult<AppModelType, DbModelType>> = [];
+      const results: Array<PipelineResult<AppModelType>> = [];
       const output: Omit<QueryResponse<never>, 'result'> & {
         executionTime?: Timestamp;
       } = {};
@@ -72,25 +69,22 @@ export class ExecutionUtil<
         .on('error', err => {
           reject(wrapError(err, stack));
         })
-        .on(
-          'data',
-          (data: PipelineStreamElement<AppModelType, DbModelType>[]) => {
-            for (const element of data) {
-              if (element.transaction) {
-                output.transaction = element.transaction;
-              }
-              if (element.executionTime) {
-                output.executionTime = element.executionTime;
-              }
-              if (element.explainMetrics) {
-                output.explainMetrics = element.explainMetrics;
-              }
-              if (element.result) {
-                results.push(element.result);
-              }
+        .on('data', (data: PipelineStreamElement<AppModelType>[]) => {
+          for (const element of data) {
+            if (element.transaction) {
+              output.transaction = element.transaction;
+            }
+            if (element.executionTime) {
+              output.executionTime = element.executionTime;
+            }
+            if (element.explainMetrics) {
+              output.explainMetrics = element.explainMetrics;
+            }
+            if (element.result) {
+              results.push(element.result);
             }
           }
-        )
+        })
         .on('end', () => {
           resolve(results);
         });
@@ -111,7 +105,7 @@ export class ExecutionUtil<
     return Date.now() - startTime >= totalTimeout;
   }
 
-  stream(pipeline: Pipeline<AppModelType, DbModelType>): NodeJS.ReadableStream {
+  stream(pipeline: Pipeline<AppModelType>): NodeJS.ReadableStream {
     const responseStream = this._stream(pipeline);
     const transform = new Transform({
       objectMode: true,
@@ -126,7 +120,7 @@ export class ExecutionUtil<
   }
 
   _stream(
-    pipeline: Pipeline<AppModelType, DbModelType>,
+    pipeline: Pipeline<AppModelType>,
     transactionOrReadTime?: Uint8Array | Timestamp | api.ITransactionOptions,
     explainOptions?: firestore.ExplainOptions
   ): NodeJS.ReadableStream {
@@ -149,7 +143,7 @@ export class ExecutionUtil<
         }
 
         if (proto.results && proto.results.length === 0) {
-          const output: PipelineStreamElement<AppModelType, DbModelType> = {};
+          const output: PipelineStreamElement<AppModelType> = {};
           if (proto.transaction?.length) {
             output.transaction = proto.transaction;
           }
@@ -161,8 +155,7 @@ export class ExecutionUtil<
           callback(
             undefined,
             proto.results.map(result => {
-              const output: PipelineStreamElement<AppModelType, DbModelType> =
-                {};
+              const output: PipelineStreamElement<AppModelType> = {};
               if (proto.transaction?.length) {
                 output.transaction = proto.transaction;
               }
@@ -176,7 +169,7 @@ export class ExecutionUtil<
                     QualifiedResourcePath.fromSlashSeparatedString(result.name)
                   )
                 : undefined;
-              output.result = new PipelineResult<AppModelType, DbModelType>(
+              output.result = new PipelineResult<AppModelType>(
                 this._serializer,
                 ref,
                 result.fields || undefined,

diff --git a/dev/src/pipeline.ts b/dev/src/pipeline.ts
@@ -105,10 +105,7 @@ export class PipelineSource {
  *     .execute();
  * ```
  */
-export class Pipeline<
-  AppModelType = firestore.DocumentData,
-  DbModelType extends firestore.DocumentData = firestore.DocumentData,
-> {
+export class Pipeline<AppModelType = firestore.DocumentData> {
   constructor(
     private db: Firestore,
     private stages: Stage[],
@@ -573,12 +570,9 @@ export class Pipeline<
   }
 
   withConverter(converter: null): Pipeline;
-  withConverter<
-    NewAppModelType,
-    NewDbModelType extends firestore.DocumentData = firestore.DocumentData,
-  >(
+  withConverter<NewAppModelType>(
     converter: firestore.FirestorePipelineConverter<NewAppModelType>
-  ): Pipeline<NewAppModelType, NewDbModelType>;
+  ): Pipeline<NewAppModelType>;
   /**
    * Applies a custom data converter to this Query, allowing you to use your
    * own custom model objects with Firestore. When you call get() on the
@@ -629,14 +623,11 @@ export class Pipeline<
    * from Firestore. Passing in `null` removes the current converter.
    * @return A Query that uses the provided converter.
    */
-  withConverter<
-    NewAppModelType,
-    NewDbModelType extends firestore.DocumentData = firestore.DocumentData,
-  >(
+  withConverter<NewAppModelType>(
     converter: firestore.FirestorePipelineConverter<NewAppModelType> | null
-  ): Pipeline<NewAppModelType, NewDbModelType> {
+  ): Pipeline<NewAppModelType> {
     const copy = this.stages.map(s => s);
-    return new Pipeline<NewAppModelType, NewDbModelType>(
+    return new Pipeline<NewAppModelType>(
       this.db,
       copy,
       converter ?? defaultPipelineConverter()
@@ -674,8 +665,8 @@ export class Pipeline<
    *
    * @return A Promise representing the asynchronous pipeline execution.
    */
-  execute(): Promise<Array<PipelineResult<AppModelType, DbModelType>>> {
-    const util = new ExecutionUtil<AppModelType, DbModelType>(
+  execute(): Promise<Array<PipelineResult<AppModelType>>> {
+    const util = new ExecutionUtil<AppModelType>(
       this.db,
       this.db._serializer!,
       this.converter
@@ -700,7 +691,7 @@ export class Pipeline<
    * ```
    */
   stream(): NodeJS.ReadableStream {
-    const util = new ExecutionUtil<AppModelType, DbModelType>(
+    const util = new ExecutionUtil<AppModelType>(
       this.db,
       this.db._serializer!,
       this.converter
@@ -730,10 +721,8 @@ export class Pipeline<
  * <p>If the PipelineResult represents a non-document result, `ref` will return a undefined
  * value.
  */
-export class PipelineResult<
-  AppModelType = firestore.DocumentData,
-  DbModelType extends firestore.DocumentData = firestore.DocumentData,
-> implements firestore.PipelineResult<AppModelType, DbModelType>
+export class PipelineResult<AppModelType = firestore.DocumentData>
+  implements firestore.PipelineResult<AppModelType>
 {
   private readonly _ref: DocumentReference | undefined;
   private _serializer: Serializer;
@@ -855,7 +844,7 @@ export class PipelineResult<
     // if a converter has been provided.
     if (!!this.converter && this.converter !== defaultPipelineConverter()) {
       return this.converter.fromFirestore(
-        new PipelineResult<DocumentData, DocumentData>(
+        new PipelineResult<DocumentData>(
           this._serializer,
           this.ref,
           this._fieldsProto,
@@ -946,7 +935,7 @@ export class PipelineResult<
    * @return {boolean} true if this `PipelineResult` is equal to the provided
    * value.
    */
-  isEqual(other: PipelineResult<AppModelType, DbModelType>): boolean {
+  isEqual(other: PipelineResult<AppModelType>): boolean {
     return (
       this === other ||
       (isOptionalEqual(this._ref, other._ref) &&

diff --git a/dev/src/reference/types.ts b/dev/src/reference/types.ts
@@ -67,7 +67,7 @@ export interface PipelineStreamElement<
   transaction?: Uint8Array;
   executionTime?: Timestamp;
   explainMetrics?: ExplainMetrics;
-  result?: PipelineResult<AppModelType, DbModelType>;
+  result?: PipelineResult<AppModelType>;
 }
 
 /**

diff --git a/dev/system-test/pipeline.ts b/dev/system-test/pipeline.ts
@@ -734,7 +734,7 @@ describe.only('Pipeline class', () => {
     );
   });
 
-  it('testPipelineConverters', async () => {
+  it('pipeline converter works', async () => {
     type AppModel = {myTitle: string; myAuthor: string; myPublished: number};
     const converter: FirestorePipelineConverter<AppModel> = {
       fromFirestore(result: FirebaseFirestore.PipelineResult): AppModel {

diff --git a/types/firestore.d.ts b/types/firestore.d.ts
@@ -1663,10 +1663,7 @@ declare namespace FirebaseFirestore {
     data(): AppModelType;
   }
 
-  export class PipelineResult<
-    AppModelType = DocumentData,
-    DbModelType extends DocumentData = DocumentData,
-  > {
+  export class PipelineResult<AppModelType = DocumentData> {
     private constructor();
 
     /**