Skip to content

Commit 54ff05e

Browse files
dlarocquedlarocque
authored
fix(firestore): read and write user data on pipeline execute (#9715)
1 parent c44bb9a commit 54ff05e

File tree

9 files changed

+122
-260
lines changed

9 files changed

+122
-260
lines changed

‎.changeset/eleven-carrots-relate.md‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@firebase/firestore': patch
3+
---
4+
5+
**Beta API Breaking change**: Defer pipeline user data validation from initialization to `execute()`. This breaking change is allowed in a non-major release since the Firestore Pipelines API is currently in Public Preview.

‎common/api-review/firestore-lite-pipelines.api.md‎

Lines changed: 17 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1156,11 +1156,6 @@ export class Ordering {
11561156

11571157
// @beta
11581158
export class Pipeline {
1159-
/* Excluded from this release type: _db */
1160-
/* Excluded from this release type: userDataReader */
1161-
/* Excluded from this release type: _userDataWriter */
1162-
/* Excluded from this release type: stages */
1163-
/* Excluded from this release type: __constructor */
11641159
addFields(field: Selectable, ...additionalFields: Selectable[]): Pipeline;
11651160
addFields(options: AddFieldsStageOptions): Pipeline;
11661161
aggregate(accumulator: AliasedAggregate, ...additionalAccumulators: AliasedAggregate[]): Pipeline;
@@ -1217,15 +1212,31 @@ export class PipelineSnapshot {
12171212
// @beta
12181213
export class PipelineSource<PipelineType> {
12191214
collection(collection: string | Query): PipelineType;
1215+
/* Excluded from this release type: _createPipeline */
1216+
/* Excluded from this release type: __constructor */
12201217
collection(options: CollectionStageOptions): PipelineType;
1218+
/* Excluded from this release type: _createPipeline */
1219+
/* Excluded from this release type: __constructor */
12211220
collectionGroup(collectionId: string): PipelineType;
1221+
/* Excluded from this release type: _createPipeline */
1222+
/* Excluded from this release type: __constructor */
12221223
collectionGroup(options: CollectionGroupStageOptions): PipelineType;
1224+
/* Excluded from this release type: _createPipeline */
1225+
/* Excluded from this release type: __constructor */
12231226
createFrom(query: Query): Pipeline;
1227+
/* Excluded from this release type: _createPipeline */
1228+
/* Excluded from this release type: __constructor */
12241229
database(): PipelineType;
1230+
/* Excluded from this release type: _createPipeline */
1231+
/* Excluded from this release type: __constructor */
12251232
database(options: DatabaseStageOptions): PipelineType;
1233+
/* Excluded from this release type: _createPipeline */
1234+
/* Excluded from this release type: __constructor */
12261235
documents(docs: Array<string | DocumentReference>): PipelineType;
1236+
/* Excluded from this release type: _createPipeline */
1237+
/* Excluded from this release type: __constructor */
12271238
documents(options: DocumentsStageOptions): PipelineType;
1228-
}
1239+
}
12291240

12301241
// @beta
12311242
export function pow(base: Expression, exponent: Expression): FunctionExpression;

‎common/api-review/firestore-pipelines.api.md‎

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1254,15 +1254,31 @@ export class PipelineSnapshot {
12541254
// @beta
12551255
export class PipelineSource<PipelineType> {
12561256
collection(collection: string | Query): PipelineType;
1257+
/* Excluded from this release type: _createPipeline */
1258+
/* Excluded from this release type: __constructor */
12571259
collection(options: CollectionStageOptions): PipelineType;
1260+
/* Excluded from this release type: _createPipeline */
1261+
/* Excluded from this release type: __constructor */
12581262
collectionGroup(collectionId: string): PipelineType;
1263+
/* Excluded from this release type: _createPipeline */
1264+
/* Excluded from this release type: __constructor */
12591265
collectionGroup(options: CollectionGroupStageOptions): PipelineType;
1266+
/* Excluded from this release type: _createPipeline */
1267+
/* Excluded from this release type: __constructor */
12601268
createFrom(query: Query): Pipeline;
1269+
/* Excluded from this release type: _createPipeline */
1270+
/* Excluded from this release type: __constructor */
12611271
database(): PipelineType;
1272+
/* Excluded from this release type: _createPipeline */
1273+
/* Excluded from this release type: __constructor */
12621274
database(options: DatabaseStageOptions): PipelineType;
1275+
/* Excluded from this release type: _createPipeline */
1276+
/* Excluded from this release type: __constructor */
12631277
documents(docs: Array<string | DocumentReference>): PipelineType;
1278+
/* Excluded from this release type: _createPipeline */
1279+
/* Excluded from this release type: __constructor */
12641280
documents(options: DocumentsStageOptions): PipelineType;
1265-
}
1281+
}
12661282

12671283
// @beta
12681284
export function pow(base: Expression, exponent: Expression): FunctionExpression;

‎packages/firestore/src/api/pipeline.ts‎

Lines changed: 2 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,6 @@
1717

1818
import { Pipeline as LitePipeline } from '../lite-api/pipeline';
1919
import { Stage } from '../lite-api/stage';
20-
import { UserDataReader } from '../lite-api/user_data_reader';
21-
import { AbstractUserDataWriter } from '../lite-api/user_data_writer';
2220

2321
import { Firestore } from './database';
2422

@@ -30,18 +28,11 @@ export class Pipeline extends LitePipeline {
3028
* @internal
3129
* @private
3230
* @param db
33-
* @param userDataReader
3431
* @param userDataWriter
3532
* @param stages
36-
* @param converter
3733
* @protected
3834
*/
39-
protected newPipeline(
40-
db: Firestore,
41-
userDataReader: UserDataReader,
42-
userDataWriter: AbstractUserDataWriter,
43-
stages: Stage[]
44-
): Pipeline {
45-
return new Pipeline(db, userDataReader, userDataWriter, stages);
35+
protected newPipeline(db: Firestore, stages: Stage[]): Pipeline {
36+
return new Pipeline(db, stages);
4637
}
4738
}

‎packages/firestore/src/api/pipeline_impl.ts‎

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,6 @@ import { PipelineExecuteOptions } from '../lite-api/pipeline_options';
2828
import { Stage } from '../lite-api/stage';
2929
import {
3030
newUserDataReader,
31-
UserDataReader,
3231
UserDataSource
3332
} from '../lite-api/user_data_reader';
3433
import { cast } from '../util/input_validation';
@@ -140,11 +139,14 @@ export function execute(
140139
const firestore = cast(pipeline._db, Firestore);
141140
const client = ensureFirestoreConfigured(firestore);
142141

143-
const udr = new UserDataReader(
144-
firestore._databaseId,
145-
/* ignoreUndefinedProperties */ true
142+
const userDataReader = newUserDataReader(firestore);
143+
const context = userDataReader.createContext(
144+
UserDataSource.Argument,
145+
'execute'
146146
);
147-
const context = udr.createContext(UserDataSource.Argument, 'execute');
147+
148+
pipeline._readUserData(context);
149+
const userDataWriter = new ExpUserDataWriter(firestore);
148150

149151
const structuredPipelineOptions = new StructuredPipelineOptions(
150152
rest,
@@ -172,7 +174,7 @@ export function execute(
172174
.map(
173175
element =>
174176
new PipelineResult(
175-
pipeline._userDataWriter,
177+
userDataWriter,
176178
element.fields!,
177179
element.key?.path
178180
? new DocumentReference(firestore, null, element.key)
@@ -198,17 +200,7 @@ export function execute(
198200
*/
199201
// Augment the Firestore class with the pipeline() factory method
200202
Firestore.prototype.pipeline = function (): PipelineSource<Pipeline> {
201-
const userDataReader = newUserDataReader(this);
202-
return new PipelineSource<Pipeline>(
203-
this._databaseId,
204-
userDataReader,
205-
(stages: Stage[]) => {
206-
return new Pipeline(
207-
this,
208-
userDataReader,
209-
new ExpUserDataWriter(this),
210-
stages
211-
);
212-
}
213-
);
203+
return new PipelineSource<Pipeline>(this._databaseId, (stages: Stage[]) => {
204+
return new Pipeline(this, stages);
205+
});
214206
};

‎packages/firestore/src/lite-api/pipeline-source.ts‎

Lines changed: 0 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,6 @@ import {
4040
DatabaseStageOptions,
4141
DocumentsStageOptions
4242
} from './stage_options';
43-
import { UserDataReader, UserDataSource } from './user_data_reader';
4443

4544
/**
4645
* @beta
@@ -55,12 +54,10 @@ export class PipelineSource<PipelineType> {
5554
* @internal
5655
* @private
5756
* @param databaseId
58-
* @param userDataReader
5957
* @param _createPipeline
6058
*/
6159
constructor(
6260
private databaseId: DatabaseId,
63-
private userDataReader: UserDataReader,
6461
/**
6562
* @internal
6663
* @private
@@ -108,14 +105,6 @@ export class PipelineSource<PipelineType> {
108105
// Create stage object
109106
const stage = new CollectionSource(normalizedCollection, options);
110107

111-
// User data must be read in the context of the API method to
112-
// provide contextual errors
113-
const parseContext = this.userDataReader.createContext(
114-
UserDataSource.Argument,
115-
'collection'
116-
);
117-
stage._readUserData(parseContext);
118-
119108
// Add stage to the pipeline
120109
return this._createPipeline([stage]);
121110
}
@@ -148,14 +137,6 @@ export class PipelineSource<PipelineType> {
148137
// Create stage object
149138
const stage = new CollectionGroupSource(collectionId, options);
150139

151-
// User data must be read in the context of the API method to
152-
// provide contextual errors
153-
const parseContext = this.userDataReader.createContext(
154-
UserDataSource.Argument,
155-
'collectionGroup'
156-
);
157-
stage._readUserData(parseContext);
158-
159140
// Add stage to the pipeline
160141
return this._createPipeline([stage]);
161142
}
@@ -178,14 +159,6 @@ export class PipelineSource<PipelineType> {
178159
// Create stage object
179160
const stage = new DatabaseSource(options);
180161

181-
// User data must be read in the context of the API method to
182-
// provide contextual errors
183-
const parseContext = this.userDataReader.createContext(
184-
UserDataSource.Argument,
185-
'database'
186-
);
187-
stage._readUserData(parseContext);
188-
189162
// Add stage to the pipeline
190163
return this._createPipeline([stage]);
191164
}
@@ -236,14 +209,6 @@ export class PipelineSource<PipelineType> {
236209
// Create stage object
237210
const stage = new DocumentsSource(normalizedDocs, options);
238211

239-
// User data must be read in the context of the API method to
240-
// provide contextual errors
241-
const parseContext = this.userDataReader.createContext(
242-
UserDataSource.Argument,
243-
'documents'
244-
);
245-
stage._readUserData(parseContext);
246-
247212
// Add stage to the pipeline
248213
return this._createPipeline([stage]);
249214
}

0 commit comments

Comments
 (0)