diff --git a/.DS_Store b/.DS_Store new file mode 100644 index 00000000000..4e230fdc439 Binary files /dev/null and b/.DS_Store differ diff --git a/handwritten/firestore/dev/src/index.ts b/handwritten/firestore/dev/src/index.ts index d5a74afd5eb..e57a25b8911 100644 --- a/handwritten/firestore/dev/src/index.ts +++ b/handwritten/firestore/dev/src/index.ts @@ -783,6 +783,13 @@ export class Firestore implements firestore.Firestore { validateBoolean('settings.ssl', settings.ssl); } + if (settings.alwaysUseImplicitOrderBy !== undefined) { + validateBoolean( + 'settings.alwaysUseImplicitOrderBy', + settings.alwaysUseImplicitOrderBy, + ); + } + if (settings.maxIdleChannels !== undefined) { validateInteger('settings.maxIdleChannels', settings.maxIdleChannels, { minValue: 0, @@ -845,6 +852,14 @@ export class Firestore implements firestore.Firestore { return this._databaseId || DEFAULT_DATABASE_ID; } + /** + * Whether to always use implicit order by clauses. + * @internal + */ + get alwaysUseImplicitOrderBy(): boolean { + return this._settings.alwaysUseImplicitOrderBy ?? false; + } + /** * Returns the root path of the database. Validates that * `initializeIfNeeded()` was called before. diff --git a/handwritten/firestore/dev/src/reference/query.ts b/handwritten/firestore/dev/src/reference/query.ts index 49e8e99353f..ec2678ef718 100644 --- a/handwritten/firestore/dev/src/reference/query.ts +++ b/handwritten/firestore/dev/src/reference/query.ts @@ -1475,6 +1475,7 @@ export class Query< toProto( transactionOrReadTime?: Uint8Array | Timestamp | api.ITransactionOptions, explainOptions?: firestore.ExplainOptions, + forceImplicitOrderBy?: boolean, ): api.IRunQueryRequest { const projectId = this.firestore.projectId; const databaseId = this.firestore.databaseId; @@ -1483,18 +1484,18 @@ export class Query< databaseId, ); - const structuredQuery = this.toStructuredQuery(); + const structuredQuery = this.toStructuredQuery(forceImplicitOrderBy); // For limitToLast queries, the structured query has to be translated to a version with // reversed ordered, and flipped startAt/endAt to work properly. if (this._queryOptions.limitType === LimitType.Last) { - if (!this._queryOptions.hasFieldOrders()) { - throw new Error( - 'limitToLast() queries require specifying at least one orderBy() clause.', - ); - } + const forceImplicit = + forceImplicitOrderBy || this._firestore.alwaysUseImplicitOrderBy; + const fieldOrders = forceImplicit + ? this.createImplicitOrderBy() + : this._queryOptions.fieldOrders; - structuredQuery.orderBy = this._queryOptions.fieldOrders!.map(order => { + structuredQuery.orderBy = fieldOrders.map(order => { // Flip the orderBy directions since we want the last results const dir = order.direction === 'DESCENDING' ? 'ASCENDING' : 'DESCENDING'; @@ -1564,7 +1565,9 @@ export class Query< return bundledQuery; } - private toStructuredQuery(): api.IStructuredQuery { + private toStructuredQuery( + forceImplicitOrderBy?: boolean, + ): api.IStructuredQuery { const structuredQuery: api.IStructuredQuery = { from: [{}], }; @@ -1586,9 +1589,19 @@ export class Query< ).toProto(); } - if (this._queryOptions.hasFieldOrders()) { - structuredQuery.orderBy = this._queryOptions.fieldOrders.map(o => - o.toProto(), + // orders + const forceImplicit = + forceImplicitOrderBy || this._firestore.alwaysUseImplicitOrderBy; + let fieldOrders = this._queryOptions.fieldOrders; + if (forceImplicit) { + fieldOrders = this.createImplicitOrderBy(); + } + + if (fieldOrders.length > 0) { + structuredQuery.orderBy = fieldOrders.map(o => o.toProto()); + } else if (this._queryOptions.limitType === LimitType.Last) { + throw new Error( + 'limitToLast() queries require specifying at least one orderBy() clause.', ); } diff --git a/handwritten/firestore/dev/src/watch.ts b/handwritten/firestore/dev/src/watch.ts index 72e3a4d2043..a95d531fba8 100644 --- a/handwritten/firestore/dev/src/watch.ts +++ b/handwritten/firestore/dev/src/watch.ts @@ -941,7 +941,7 @@ export class QueryWatch< } getTarget(resumeToken?: Uint8Array): google.firestore.v1.ITarget { - const query = this.query.toProto(); + const query = this.query.toProto(undefined, undefined, true); return {query, targetId: WATCH_TARGET_ID, resumeToken}; } } diff --git a/handwritten/firestore/dev/system-test/query.ts b/handwritten/firestore/dev/system-test/query.ts index 14acbe69bce..8def91adfaa 100644 --- a/handwritten/firestore/dev/system-test/query.ts +++ b/handwritten/firestore/dev/system-test/query.ts @@ -2291,5 +2291,55 @@ describe.skipClassic('Query and Pipeline Compare - Enterprise DB', () => { expectDocs(await compareQueryAndPipeline(query), 'doc2', 'doc3', 'doc4'); expectDocs(await queryWithCursor.get(), 'doc2', 'doc3', 'doc4'); }); + + it('alwaysUseImplicitOrderBy returns same results', async () => { + const collection = getTestRoot(); + const docs = { + doc01: {sort: 1}, + doc02: {sort: 2}, + doc03: {sort: 3}, + doc04: {sort: 4}, + doc05: {sort: 5}, + doc06: {sort: 6}, + doc07: {sort: 7}, + doc08: {sort: 8}, + doc09: {sort: 9}, + doc10: {sort: 10}, + }; + + for (const [id, data] of Object.entries(docs)) { + await collection.doc(id).set(data); + } + + const expectedOrder = [ + 'doc02', + 'doc03', + 'doc04', + 'doc05', + 'doc06', + 'doc07', + 'doc08', + 'doc09', + 'doc10', + ]; + + // TODO: This test should run against both standard and enterprise + // and verify the results respectively + // const originalQuery = collection.where('sort', '>', 1); + // const originalSnapshot = await originalQuery.get(); + // const originalResult = originalSnapshot.docs.map(d => d.id); + + const modifiedFirestore = new Firestore({ + ...firestore.settings, + alwaysUseImplicitOrderBy: true, + }); + const modifiedCollection = modifiedFirestore.collection(collection.id); + const query = modifiedCollection.where('sort', '>', 1); + const snapshot = await query.get(); + const result = snapshot.docs.map(d => d.id); + + // since alwaysUseImplicitOrderBy is true, we expect strict ordering. + expect(result).to.deep.equal(expectedOrder); + }); }); }); diff --git a/handwritten/firestore/dev/test/aggregateQuery.ts b/handwritten/firestore/dev/test/aggregateQuery.ts index 72b75cbe8b1..0a4b83fa16b 100644 --- a/handwritten/firestore/dev/test/aggregateQuery.ts +++ b/handwritten/firestore/dev/test/aggregateQuery.ts @@ -25,6 +25,8 @@ import {google} from '../protos/firestore_v1_proto_api'; import api = google.firestore.v1; import * as chaiAsPromised from 'chai-as-promised'; import {setTimeoutHandler} from '../src/backoff'; +import * as extend from 'extend'; + use(chaiAsPromised); describe('aggregate query interface', () => { @@ -100,6 +102,57 @@ describe('aggregate query interface', () => { }); }); + it('supports alwaysUseImplicitOrderBy', async () => { + const result: api.IRunAggregationQueryResponse = { + result: { + aggregateFields: { + aggregate_0: {integerValue: '99'}, + }, + }, + readTime: {seconds: 5, nanos: 6}, + }; + const overrides: ApiOverride = { + runAggregationQuery: request => { + let actualStructuredQuery = + request!.structuredAggregationQuery?.structuredQuery; + actualStructuredQuery = extend(true, {}, actualStructuredQuery); + expect(actualStructuredQuery).to.deep.equal({ + from: [{collectionId: 'collectionId'}], + where: { + fieldFilter: { + field: {fieldPath: 'foo'}, + op: 'GREATER_THAN' as api.StructuredQuery.FieldFilter.Operator, + value: {stringValue: 'bar'}, + }, + }, + orderBy: [ + { + direction: 'ASCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: 'foo'}, + }, + { + direction: 'ASCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: '__name__'}, + }, + ], + }); + return stream(result); + }, + }; + + firestore = await createInstance(overrides, { + alwaysUseImplicitOrderBy: true, + }); + + const query = firestore + .collection('collectionId') + .where('foo', '>', 'bar') + .count(); + return query.get().then(results => { + expect(results.data().count).to.be.equal(99); + }); + }); + it('handles stream exception at initialization', async () => { let attempts = 0; const query = firestore.collection('collectionId').count(); diff --git a/handwritten/firestore/dev/test/query.ts b/handwritten/firestore/dev/test/query.ts index b3ee1595bee..cce2ebe4837 100644 --- a/handwritten/firestore/dev/test/query.ts +++ b/handwritten/firestore/dev/test/query.ts @@ -609,6 +609,48 @@ describe('query interface', () => { expect(results.readTime.isEqual(new Timestamp(5, 6))).to.be.true; }); + it('supports alwaysUseImplicitOrderBy with limitToLast', async () => { + const overrides: ApiOverride = { + runQuery: request => { + queryEquals( + request, + where(fieldFilters('foo', 'GREATER_THAN_OR_EQUAL', 'bar')), + { + orderBy: [ + { + field: {fieldPath: 'foo'}, + direction: 'DESCENDING', + }, + { + field: {fieldPath: '__name__'}, + direction: 'DESCENDING', + }, + ], + limit: {value: 1}, + }, + ); + return stream({readTime: {seconds: 5, nanos: 6}}); + }, + }; + + firestore = await createInstance(overrides, { + alwaysUseImplicitOrderBy: true, + }); + const query = firestore + .collection('collectionId') + .where('foo', '>=', 'bar') + .limitToLast(1); + await query.get(); + }); + + it('throws for limitToLast without orderBy', async () => { + firestore = await createInstance(); + const query = firestore.collection('collectionId').limitToLast(1); + expect(() => query.toProto()).to.throw( + 'limitToLast() queries require specifying at least one orderBy() clause.', + ); + }); + it('retries on stream failure', async () => { let attempts = 0; const overrides: ApiOverride = { diff --git a/handwritten/firestore/dev/test/watch.ts b/handwritten/firestore/dev/test/watch.ts index 348f7f0898b..155061a19dc 100644 --- a/handwritten/firestore/dev/test/watch.ts +++ b/handwritten/firestore/dev/test/watch.ts @@ -597,6 +597,12 @@ describe('Query watch', () => { parent: `projects/${PROJECT_ID}/databases/(default)/documents`, structuredQuery: { from: [{collectionId: 'col'}], + orderBy: [ + { + direction: 'ASCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: '__name__'}, + }, + ], }, }, targetId, @@ -628,6 +634,12 @@ describe('Query watch', () => { }, }, }, + orderBy: [ + { + direction: 'ASCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: '__name__'}, + }, + ], }, }, targetId, @@ -646,6 +658,14 @@ describe('Query watch', () => { parent: `projects/${PROJECT_ID}/databases/(default)/documents`, structuredQuery: { from: [{collectionId: 'col'}], + orderBy: [ + { + direction: 'ASCENDING', + field: { + fieldPath: '__name__', + }, + }, + ], }, }, targetId, @@ -667,7 +687,16 @@ describe('Query watch', () => { parent: `projects/${PROJECT_ID}/databases/(default)/documents`, structuredQuery: { from: [{collectionId: 'col'}], - orderBy: [{direction: 'DESCENDING', field: {fieldPath: 'foo'}}], + orderBy: [ + { + direction: 'DESCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: 'foo'}, + }, + { + direction: 'DESCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: '__name__'}, + }, + ], }, }, targetId, @@ -786,6 +815,55 @@ describe('Query watch', () => { ); }); + it('supports alwaysUseImplicitOrderBy', async () => { + // Re-initialize with alwaysUseImplicitOrderBy: true + firestore = await createInstance( + {listen: () => listenCallback()}, + { + alwaysUseImplicitOrderBy: true, + }, + ); + const query = firestore.collection('col').where('foo', '>', 'bar'); + watchHelper = new WatchHelper(streamHelper, query, targetId); + + const expectedQueryJSON = { + database: `projects/${PROJECT_ID}/databases/(default)`, + addTarget: { + query: { + parent: `projects/${PROJECT_ID}/databases/(default)/documents`, + structuredQuery: { + from: [{collectionId: 'col'}], + where: { + fieldFilter: { + field: {fieldPath: 'foo'}, + op: 'GREATER_THAN' as api.StructuredQuery.FieldFilter.Operator, + value: {stringValue: 'bar'}, + }, + }, + orderBy: [ + { + direction: 'ASCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: 'foo'}, + }, + { + direction: 'ASCENDING' as api.StructuredQuery.Direction, + field: {fieldPath: '__name__'}, + }, + ], + }, + }, + targetId, + }, + }; + + return watchHelper.runTest(expectedQueryJSON, () => { + watchHelper.sendAddTarget(targetId); + watchHelper.sendCurrent(); + watchHelper.sendSnapshot(1); + return watchHelper.await('snapshot'); + }); + }); + it('rejects an unknown target', () => { return watchHelper.runFailedTest( collQueryJSON(), @@ -1280,10 +1358,6 @@ describe('Query watch', () => { // Add FieldPath.documentId() sorting query = query.orderBy(FieldPath.documentId(), 'desc'); - expectedJson.addTarget!.query!.structuredQuery!.orderBy!.push({ - direction: 'DESCENDING', - field: {fieldPath: '__name__'}, - }); watchHelper = new WatchHelper(streamHelper, query, targetId); diff --git a/handwritten/firestore/types/firestore.d.ts b/handwritten/firestore/types/firestore.d.ts index 69a3554988e..c1e73c63457 100644 --- a/handwritten/firestore/types/firestore.d.ts +++ b/handwritten/firestore/types/firestore.d.ts @@ -505,6 +505,10 @@ declare namespace FirebaseFirestore { * @beta */ openTelemetry?: FirestoreOpenTelemetryOptions; + /** + * Whether to always use implicit order by clauses. + */ + alwaysUseImplicitOrderBy?: boolean; [key: string]: any; // Accept other properties, such as GRPC settings. } /** @@ -570,6 +574,10 @@ declare namespace FirebaseFirestore { * Returns the Database ID for this Firestore instance. */ get databaseId(): string; + /** + * Whether to always use implicit order by clauses. + */ + get alwaysUseImplicitOrderBy(): boolean; /** * Gets a `CollectionReference` instance that refers to the collection at * the specified path.