From bcfaf2a16857071da6639d04781de4d75be92bc8 Mon Sep 17 00:00:00 2001 From: Mark Duckworth <1124037+MarkDuckworth@users.noreply.github.com> Date: Wed, 13 Nov 2024 13:24:51 -0700 Subject: [PATCH] Modular-style pipeline and execute methods for experimentation. Plus cleanup. --- common/api-review/firestore-lite.api.md | 17 ++++- common/api-review/firestore.api.md | 65 ++++++++----------- packages/firestore/lite/index.ts | 9 ++- packages/firestore/src/api.ts | 4 +- packages/firestore/src/api/pipeline.ts | 21 +++++- packages/firestore/src/core/pipeline-util.ts | 7 +- .../src/lite-api/database_augmentation.ts | 12 ++-- packages/firestore/src/lite-api/overloads.ts | 25 ++----- packages/firestore/src/lite-api/pipeline.ts | 21 +++++- .../firestore/src/lite-api/pipeline_impl.ts | 13 ++++ .../src/remote/internal_serializer.ts | 4 +- .../test/integration/api/pipeline.test.ts | 47 +++++++++++++- 12 files changed, 162 insertions(+), 83 deletions(-) create mode 100644 packages/firestore/src/lite-api/pipeline_impl.ts diff --git a/common/api-review/firestore-lite.api.md b/common/api-review/firestore-lite.api.md index 4d8f428ad06..3acb768aa40 100644 --- a/common/api-review/firestore-lite.api.md +++ b/common/api-review/firestore-lite.api.md @@ -639,6 +639,9 @@ export function euclideanDistance(expr: Constant, other: VectorValue): Euclidean // @beta export function euclideanDistance(expr: Constant, other: Constant): EuclideanDistance; +// @beta +export function execute(pipeline: Pipeline): Promise>>; + // @beta (undocumented) export class Exists extends FirestoreFunction implements FilterCondition { constructor(expr: Constant); @@ -1612,6 +1615,14 @@ export class Pipeline { where(condition: FilterCondition & Constant): Pipeline; } +// Warning: (ae-incompatible-release-tags) The symbol "pipeline" is marked as @public, but its signature references "PipelineSource" which is marked as @beta +// +// @public +export function pipeline(firestore: Firestore): PipelineSource; + +// @public +export function pipeline(query: Query): Pipeline; + // @beta export class PipelineResult { /* Excluded from this release type: _ref */ @@ -2203,8 +2214,8 @@ export function xor(left: FilterExpr, ...right: FilterExpr[]): Xor; // Warnings were encountered during analysis: // -// /home/runner/work/firebase-js-sdk/firebase-js-sdk/packages/firestore/dist/lite/index.d.ts:9237:9 - (ae-incompatible-release-tags) The symbol "accumulators" is marked as @public, but its signature references "AccumulatorTarget" which is marked as @beta -// /home/runner/work/firebase-js-sdk/firebase-js-sdk/packages/firestore/dist/lite/index.d.ts:9238:9 - (ae-incompatible-release-tags) The symbol "groups" is marked as @public, but its signature references "Selectable" which is marked as @beta -// /home/runner/work/firebase-js-sdk/firebase-js-sdk/packages/firestore/dist/lite/index.d.ts:9267:9 - (ae-incompatible-release-tags) The symbol "orderings" is marked as @public, but its signature references "Ordering" which is marked as @beta +// /Users/markduckworth/projects/firebase-js-sdk/packages/firestore/dist/lite/index.d.ts:9243:9 - (ae-incompatible-release-tags) The symbol "accumulators" is marked as @public, but its signature references "AccumulatorTarget" which is marked as @beta +// /Users/markduckworth/projects/firebase-js-sdk/packages/firestore/dist/lite/index.d.ts:9244:9 - (ae-incompatible-release-tags) The symbol "groups" is marked as @public, but its signature references "Selectable" which is marked as @beta +// /Users/markduckworth/projects/firebase-js-sdk/packages/firestore/dist/lite/index.d.ts:9273:9 - (ae-incompatible-release-tags) The symbol "orderings" is marked as @public, but its signature references "Ordering" which is marked as @beta ``` diff --git a/common/api-review/firestore.api.md b/common/api-review/firestore.api.md index 720dd83d983..5f17b27219f 100644 --- a/common/api-review/firestore.api.md +++ b/common/api-review/firestore.api.md @@ -686,6 +686,9 @@ export function euclideanDistance(expr: Constant, other: VectorValue): Euclidean // @beta export function euclideanDistance(expr: Constant, other: Constant): EuclideanDistance; +// @beta +export function execute(pipeline: Pipeline): Promise>>; + // @beta (undocumented) export class Exists extends FirestoreFunction implements FilterCondition { constructor(expr: Constant); @@ -1819,65 +1822,53 @@ export interface PersistentSingleTabManagerSettings { // @public export type PersistentTabManager = PersistentSingleTabManager | PersistentMultipleTabManager; -// @public +// @public (undocumented) export class Pipeline { - /* Excluded from this release type: _db */ + /* Excluded from this release type: __constructor */ // Warning: (ae-incompatible-release-tags) The symbol "addFields" is marked as @public, but its signature references "Selectable" which is marked as @beta addFields(...fields: Selectable[]): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ // Warning: (ae-incompatible-release-tags) The symbol "aggregate" is marked as @public, but its signature references "AccumulatorTarget" which is marked as @beta aggregate(...accumulators: AccumulatorTarget[]): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ - aggregate(options: { - accumulators: AccumulatorTarget[]; - groups?: Array; - }): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ + aggregate(options: { accumulators: AccumulatorTarget[]; groups?: Array; }): Pipeline; + // (undocumented) + converter: any; // Warning: (ae-incompatible-release-tags) The symbol "distinct" is marked as @public, but its signature references "Selectable" which is marked as @beta distinct(...groups: Array): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ // Warning: (ae-incompatible-release-tags) The symbol "execute" is marked as @public, but its signature references "PipelineResult" which is marked as @beta execute(): Promise>>; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ // Warning: (ae-incompatible-release-tags) The symbol "findNearest" is marked as @public, but its signature references "FindNearestOptions" which is marked as @beta // // (undocumented) findNearest(options: FindNearestOptions): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ genericStage(name: string, params: any[]): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ limit(limit: number): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ offset(offset: number): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ + readUserData: any; // Warning: (ae-incompatible-release-tags) The symbol "select" is marked as @public, but its signature references "Selectable" which is marked as @beta select(...selections: Array): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ + // (undocumented) + selectablesToMap: any; // Warning: (ae-incompatible-release-tags) The symbol "sort" is marked as @public, but its signature references "Ordering" which is marked as @beta sort(...orderings: Ordering[]): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ - // (undocumented) - sort(options: { - orderings: Ordering[]; - }): Pipeline; - /* Excluded from this release type: userDataWriter */ - /* Excluded from this release type: documentReferenceFactory */ + // (undocumented) + sort(options: { orderings: Ordering[]; }): Pipeline; + // (undocumented) + stages: any; + // (undocumented) + userDataReader: any; // Warning: (ae-incompatible-release-tags) The symbol "where" is marked as @public, but its signature references "FilterCondition" which is marked as @beta // Warning: (ae-incompatible-release-tags) The symbol "where" is marked as @public, but its signature references "Constant" which is marked as @beta where(condition: FilterCondition & Constant): Pipeline; } +// Warning: (ae-incompatible-release-tags) The symbol "pipeline" is marked as @public, but its signature references "PipelineSource" which is marked as @beta +// +// @public +export function pipeline(firestore: Firestore): PipelineSource; + +// @public +export function pipeline(query: Query): Pipeline; + // @beta export class PipelineResult { /* Excluded from this release type: _ref */ @@ -2499,8 +2490,8 @@ export function xor(left: FilterExpr, ...right: FilterExpr[]): Xor; // Warnings were encountered during analysis: // -// /home/runner/work/firebase-js-sdk/firebase-js-sdk/packages/firestore/dist/index.d.ts:10116:9 - (ae-incompatible-release-tags) The symbol "accumulators" is marked as @public, but its signature references "AccumulatorTarget" which is marked as @beta -// /home/runner/work/firebase-js-sdk/firebase-js-sdk/packages/firestore/dist/index.d.ts:10117:9 - (ae-incompatible-release-tags) The symbol "groups" is marked as @public, but its signature references "Selectable" which is marked as @beta -// /home/runner/work/firebase-js-sdk/firebase-js-sdk/packages/firestore/dist/index.d.ts:10146:9 - (ae-incompatible-release-tags) The symbol "orderings" is marked as @public, but its signature references "Ordering" which is marked as @beta +// /Users/markduckworth/projects/firebase-js-sdk/packages/firestore/dist/index.d.ts:10106:26 - (ae-incompatible-release-tags) The symbol "accumulators" is marked as @public, but its signature references "AccumulatorTarget" which is marked as @beta +// /Users/markduckworth/projects/firebase-js-sdk/packages/firestore/dist/index.d.ts:10106:61 - (ae-incompatible-release-tags) The symbol "groups" is marked as @public, but its signature references "Selectable" which is marked as @beta +// /Users/markduckworth/projects/firebase-js-sdk/packages/firestore/dist/index.d.ts:10133:21 - (ae-incompatible-release-tags) The symbol "orderings" is marked as @public, but its signature references "Ordering" which is marked as @beta ``` diff --git a/packages/firestore/lite/index.ts b/packages/firestore/lite/index.ts index 6e30bdd4e0c..7bd1b2ce3db 100644 --- a/packages/firestore/lite/index.ts +++ b/packages/firestore/lite/index.ts @@ -31,10 +31,12 @@ export { PipelineSource } from '../src/lite-api/pipeline-source'; export { PipelineResult } from '../src/lite-api/pipeline-result'; -export { Pipeline } from '../src/lite-api/pipeline'; +export { Pipeline, pipeline } from '../src/lite-api/pipeline'; export { useFirestorePipelines } from '../src/lite-api/database_augmentation'; +export { execute } from '../src/lite-api/pipeline_impl'; + export { Stage, FindNearestOptions, @@ -238,10 +240,7 @@ export { queryEqual } from '../src/lite-api/reference'; -export { - and, - or, -} from '../src/lite-api/overloads'; +export { and, or } from '../src/lite-api/overloads'; export { endAt, diff --git a/packages/firestore/src/api.ts b/packages/firestore/src/api.ts index 73b725f184b..9b77311666b 100644 --- a/packages/firestore/src/api.ts +++ b/packages/firestore/src/api.ts @@ -19,10 +19,12 @@ export { PipelineSource } from './lite-api/pipeline-source'; export { PipelineResult } from './lite-api/pipeline-result'; -export { Pipeline } from './lite-api/pipeline'; +export { Pipeline, pipeline } from './api/pipeline'; export { useFirestorePipelines } from './api/database_augmentation'; +export { execute } from './lite-api/pipeline_impl'; + export { Stage, FindNearestOptions, diff --git a/packages/firestore/src/api/pipeline.ts b/packages/firestore/src/api/pipeline.ts index 9aeccae9ad0..3122fc36412 100644 --- a/packages/firestore/src/api/pipeline.ts +++ b/packages/firestore/src/api/pipeline.ts @@ -1,7 +1,8 @@ import { firestoreClientExecutePipeline } from '../core/firestore_client'; import { Pipeline as LitePipeline } from '../lite-api/pipeline'; import { PipelineResult } from '../lite-api/pipeline-result'; -import { DocumentData, DocumentReference } from '../lite-api/reference'; +import { PipelineSource } from '../lite-api/pipeline-source'; +import { DocumentData, DocumentReference, Query } from '../lite-api/reference'; import { Stage } from '../lite-api/stage'; import { UserDataReader } from '../lite-api/user_data_reader'; import { AbstractUserDataWriter } from '../lite-api/user_data_writer'; @@ -97,3 +98,21 @@ export class Pipeline< }); } } + +/** + * Experimental Modular API for console testing. + * @param firestore + */ +export function pipeline(firestore: Firestore): PipelineSource; + +/** + * Experimental Modular API for console testing. + * @param query + */ +export function pipeline(query: Query): Pipeline; + +export function pipeline( + firestoreOrQuery: Firestore | Query +): PipelineSource | Pipeline { + return firestoreOrQuery.pipeline(); +} diff --git a/packages/firestore/src/core/pipeline-util.ts b/packages/firestore/src/core/pipeline-util.ts index 8a6ef9e17a7..49704e06381 100644 --- a/packages/firestore/src/core/pipeline-util.ts +++ b/packages/firestore/src/core/pipeline-util.ts @@ -17,12 +17,9 @@ import { Expr, Field, FilterCondition, - not, + not } from '../lite-api/expressions'; -import { - and, - or -} from '../lite-api/overloads'; +import { and, or } from '../lite-api/overloads'; import { isNanValue, isNullValue } from '../model/values'; import { ArrayValue as ProtoArrayValue, diff --git a/packages/firestore/src/lite-api/database_augmentation.ts b/packages/firestore/src/lite-api/database_augmentation.ts index d1140260b20..cce1214138a 100644 --- a/packages/firestore/src/lite-api/database_augmentation.ts +++ b/packages/firestore/src/lite-api/database_augmentation.ts @@ -1,13 +1,13 @@ import { DocumentKey } from '../model/document_key'; import { Firestore } from './database'; +import { And, FilterExpr, Or } from './expressions'; +import { or, and } from './overloads'; import { Pipeline } from './pipeline'; import { PipelineSource } from './pipeline-source'; import { DocumentReference, Query } from './reference'; import { LiteUserDataWriter } from './reference_impl'; import { newUserDataReader } from './user_data_reader'; -import {or, and} from "./overloads"; -import {And, Avg, Expr, Field, FilterExpr, Or, Sum} from "./expressions"; export function useFirestorePipelines(): void { Firestore.prototype.pipeline = function (): PipelineSource { @@ -41,11 +41,11 @@ export function useFirestorePipelines(): void { return pipeline; }; - and._andFunction = function(left: FilterExpr, ...right: FilterExpr[]): And { + and._andFunction = function (left: FilterExpr, ...right: FilterExpr[]): And { return new And([left, ...right]); - } + }; - or._orFunction = function(left: FilterExpr, ...right: FilterExpr[]): Or { + or._orFunction = function (left: FilterExpr, ...right: FilterExpr[]): Or { return new Or([left, ...right]); - } + }; } diff --git a/packages/firestore/src/lite-api/overloads.ts b/packages/firestore/src/lite-api/overloads.ts index c17ea7ee46a..2b3675db521 100644 --- a/packages/firestore/src/lite-api/overloads.ts +++ b/packages/firestore/src/lite-api/overloads.ts @@ -1,20 +1,12 @@ import { CompositeOperator } from '../core/filter'; -import { - And, Avg, Expr, Field, - FilterExpr, - Or, Sum -} from './expressions'; - +import { And, FilterExpr, Or } from './expressions'; import { QueryCompositeFilterConstraint, QueryConstraint, QueryFilterConstraint, validateQueryFilterConstraint } from './query'; -import {FieldPath} from "./field_path"; -import {AggregateField} from "./aggregate_types"; -import {fieldPathFromArgument} from "./user_data_reader"; /** * @beta @@ -53,8 +45,7 @@ export function or( ): Or | QueryCompositeFilterConstraint { if (leftFilterExprOrQueryConstraint === undefined) { return or._orFilters(); - } - else if ( + } else if ( leftFilterExprOrQueryConstraint instanceof QueryConstraint || leftFilterExprOrQueryConstraint instanceof QueryCompositeFilterConstraint || leftFilterExprOrQueryConstraint === undefined @@ -65,7 +56,7 @@ export function or( ); } else { // @ts-ignore - return or._orFunction(leftFilterExprOrQueryConstraint, ...(right)); + return or._orFunction(leftFilterExprOrQueryConstraint, ...right); } } @@ -83,10 +74,7 @@ or._orFilters = function ( ); }; -or._orFunction = function ( - left: FilterExpr, - ...right: FilterExpr[] -): Or { +or._orFunction = function (left: FilterExpr, ...right: FilterExpr[]): Or { throw new Error( 'Pipelines not initialized. Your application must call `useFirestorePipelines()` before using Firestore Pipeline features.' ); @@ -160,10 +148,7 @@ and._andFilters = function ( ); }; -and._andFunction = function ( - left: FilterExpr, - ...right: FilterExpr[] -): And { +and._andFunction = function (left: FilterExpr, ...right: FilterExpr[]): And { throw new Error( 'Pipelines not initialized. Your application must call `useFirestorePipelines()` before using Firestore Pipeline features.' ); diff --git a/packages/firestore/src/lite-api/pipeline.ts b/packages/firestore/src/lite-api/pipeline.ts index 41a3b4590d8..5b9c061c1b9 100644 --- a/packages/firestore/src/lite-api/pipeline.ts +++ b/packages/firestore/src/lite-api/pipeline.ts @@ -42,7 +42,8 @@ import { Selectable } from './expressions'; import { PipelineResult } from './pipeline-result'; -import { DocumentData, DocumentReference } from './reference'; +import { PipelineSource } from './pipeline-source'; +import { DocumentData, DocumentReference, Query } from './reference'; import { AddFields, Aggregate, @@ -829,3 +830,21 @@ export class Pipeline }; } } + +/** + * Experimental Modular API for console testing. + * @param firestore + */ +export function pipeline(firestore: Firestore): PipelineSource; + +/** + * Experimental Modular API for console testing. + * @param query + */ +export function pipeline(query: Query): Pipeline; + +export function pipeline( + firestoreOrQuery: Firestore | Query +): PipelineSource | Pipeline { + return firestoreOrQuery.pipeline(); +} diff --git a/packages/firestore/src/lite-api/pipeline_impl.ts b/packages/firestore/src/lite-api/pipeline_impl.ts new file mode 100644 index 00000000000..30ccc04cbd4 --- /dev/null +++ b/packages/firestore/src/lite-api/pipeline_impl.ts @@ -0,0 +1,13 @@ +import { Pipeline } from './pipeline'; +import { PipelineResult } from './pipeline-result'; + +/** + * Modular API for console experimentation. + * @param pipeline Execute this pipeline. + * @beta + */ +export function execute( + pipeline: Pipeline +): Promise>> { + return pipeline.execute(); +} diff --git a/packages/firestore/src/remote/internal_serializer.ts b/packages/firestore/src/remote/internal_serializer.ts index c0e688585ef..dafdea85efc 100644 --- a/packages/firestore/src/remote/internal_serializer.ts +++ b/packages/firestore/src/remote/internal_serializer.ts @@ -19,13 +19,13 @@ import { ensureFirestoreConfigured, Firestore } from '../api/database'; import { AggregateImpl } from '../core/aggregate'; import { queryToAggregateTarget, queryToTarget } from '../core/query'; import { AggregateSpec } from '../lite-api/aggregate_types'; +import { getDatastore } from '../lite-api/components'; import { Pipeline } from '../lite-api/pipeline'; import { Query } from '../lite-api/reference'; import { cast } from '../util/input_validation'; import { mapToArray } from '../util/obj'; import { toQueryTarget, toRunAggregationQueryRequest } from './serializer'; -import {getDatastore} from "../lite-api/components"; /** * @internal @@ -105,9 +105,9 @@ export function _internalAggregationQueryToProtoRunAggregationQueryRequest< // eslint-disable-next-line @typescript-eslint/no-explicit-any export function _internalPipelineToExecutePipelineRequestProto( pipeline: Pipeline + // eslint-disable-next-line @typescript-eslint/no-explicit-any ): any { const firestore = cast(pipeline._db, Firestore); - const client = ensureFirestoreConfigured(firestore); const datastore = getDatastore(firestore); const serializer = datastore.serializer; if (serializer === undefined) { diff --git a/packages/firestore/test/integration/api/pipeline.test.ts b/packages/firestore/test/integration/api/pipeline.test.ts index 57a5121ed37..aa68528d124 100644 --- a/packages/firestore/test/integration/api/pipeline.test.ts +++ b/packages/firestore/test/integration/api/pipeline.test.ts @@ -15,6 +15,8 @@ import { expect, use } from 'chai'; import chaiAsPromised from 'chai-as-promised'; +import { pipeline } from '../../../src/api/pipeline'; +import { execute } from '../../../src/lite-api/pipeline_impl'; import { addEqualityMatcher } from '../../util/equality_matcher'; import { Deferred } from '../../util/promise'; import { @@ -57,7 +59,7 @@ import { apiDescribe, withTestCollection } from '../util/helpers'; use(chaiAsPromised); useFirestorePipelines(); -apiDescribe.only('Pipelines', persistence => { +apiDescribe('Pipelines', persistence => { addEqualityMatcher(); let firestore: Firestore; let randomCol: CollectionReference; @@ -807,7 +809,6 @@ apiDescribe.only('Pipelines', persistence => { const proto = _internalPipelineToExecutePipelineRequestProto(pipeline); expect(proto).not.to.be.null; - console.log(JSON.stringify(proto, null, 2)) }); // TODO(pipeline) support converter @@ -843,4 +844,46 @@ apiDescribe.only('Pipelines', persistence => { // myTitle: 'Crime and Punishment', // }); // }); + describe('modular API', () => { + it('works when creating a pipeline from a Firestore instance', async () => { + const myPipeline = pipeline(firestore) + .collection(randomCol.path) + .where(lt(Field.of('published'), 1984)) + .aggregate({ + accumulators: [avg('rating').as('avgRating')], + groups: ['genre'] + }) + .where(gt('avgRating', 4.3)) + .sort(Field.of('avgRating').descending()); + + const results = await execute(myPipeline); + + expectResults( + results, + { avgRating: 4.7, genre: 'Fantasy' }, + { avgRating: 4.5, genre: 'Romance' }, + { avgRating: 4.4, genre: 'Science Fiction' } + ); + }); + + it('works when creating a pipeline from a collection', async () => { + const myPipeline = pipeline(randomCol) + .where(lt(Field.of('published'), 1984)) + .aggregate({ + accumulators: [avg('rating').as('avgRating')], + groups: ['genre'] + }) + .where(gt('avgRating', 4.3)) + .sort(Field.of('avgRating').descending()); + + const results = await execute(myPipeline); + + expectResults( + results, + { avgRating: 4.7, genre: 'Fantasy' }, + { avgRating: 4.5, genre: 'Romance' }, + { avgRating: 4.4, genre: 'Science Fiction' } + ); + }); + }); });