fix: avoid adding duplicate orderBys in startAfter()

googleapis · Mar 22, 2021 · e8ff476 · e8ff476
1 parent 9cea104
commit e8ff476
Show file tree

Hide file tree

Showing 3 changed files with 62 additions and 5 deletions.
diff --git a/dev/src/reference.ts b/dev/src/reference.ts
@@ -1521,6 +1521,9 @@ export class Query<T = firestore.DocumentData> implements firestore.Query<T> {
       // If no explicit ordering is specified, use the first inequality to
       // define an implicit order.
       for (const fieldFilter of this._queryOptions.fieldFilters) {
+        if (FieldPath.documentId().isEqual(fieldFilter.field)) {
+          hasDocumentId = true;
+        }
         if (fieldFilter.isInequalityFilter()) {
           fieldOrders.push(new FieldOrder(fieldFilter.field));
           break;

diff --git a/dev/test/query.ts b/dev/test/query.ts
@@ -49,6 +49,7 @@ import {
 } from './util/helpers';
 
 import api = google.firestore.v1;
+import protobuf = google.protobuf;
 
 const PROJECT_ID = 'test-project';
 const DATABASE_ROOT = `projects/${PROJECT_ID}/databases/(default)`;
@@ -285,9 +286,10 @@ function endAt(
   return {endAt: cursor};
 }
 
-export function queryEqualsWithParent(
+export function queryEqualsWithParentAndReadTime(
   actual: api.IRunQueryRequest | undefined,
   parent: string,
+  readTime?: protobuf.ITimestamp,
   ...protoComponents: api.IStructuredQuery[]
 ): void {
   expect(actual).to.not.be.undefined;
@@ -315,6 +317,10 @@ export function queryEqualsWithParent(
     ];
   }
 
+  if (readTime) {
+    query.readTime = readTime;
+  }
+
   // 'extend' removes undefined fields in the request object. The backend
   // ignores these fields, but we need to manually strip them before we compare
   // the expected and the actual request.
@@ -326,7 +332,12 @@ export function queryEquals(
   actual: api.IRunQueryRequest | undefined,
   ...protoComponents: api.IStructuredQuery[]
 ): void {
-  queryEqualsWithParent(actual, '', ...protoComponents);
+  queryEqualsWithParentAndReadTime(
+    actual,
+    '',
+    /* readTime= */ undefined,
+    ...protoComponents
+  );
 }
 
 function bundledQueryEquals(

diff --git a/dev/test/recursive-delete.ts b/dev/test/recursive-delete.ts
@@ -32,10 +32,12 @@ import {
 import {
   allDescendants,
   fieldFilters,
+  orderBy,
   queryEquals,
-  queryEqualsWithParent,
+  queryEqualsWithParentAndReadTime,
   result,
   select,
+  startAt as queryStartAt,
 } from './query';
 import {
   createRequest,
@@ -147,9 +149,10 @@ describe('recursiveDelete() method:', () => {
     it('for nested collections', async () => {
       const overrides: ApiOverride = {
         runQuery: req => {
-          queryEqualsWithParent(
+          queryEqualsWithParentAndReadTime(
             req,
             'root/doc',
+            /* readTime= */ undefined,
             select('__name__'),
             allDescendants(/* kindless= */ true),
             fieldFilters(
@@ -173,9 +176,10 @@ describe('recursiveDelete() method:', () => {
     it('documents', async () => {
       const overrides: ApiOverride = {
         runQuery: req => {
-          queryEqualsWithParent(
+          queryEqualsWithParentAndReadTime(
             req,
             'root/doc',
+            /* readTime= */ undefined,
             select('__name__'),
             allDescendants(/* kindless= */ true)
           );
@@ -187,6 +191,45 @@ describe('recursiveDelete() method:', () => {
       firestore = await createInstance(overrides);
       return firestore.recursiveDelete(firestore.doc('root/doc'));
     });
+
+    it('creates retry query after stream exception with last received doc', async () => {
+      let callCount = 0;
+      const overrides: ApiOverride = {
+        runQuery: request => {
+          callCount++;
+          if (callCount === 1) {
+            return stream(result('doc1'), new Error('failed in test'));
+          } else {
+            queryEqualsWithParentAndReadTime(
+              request,
+              /* parent= */ '',
+              /* readTime= */ {seconds: '5', nanos: 6},
+              select('__name__'),
+              allDescendants(/* kindless= */ true),
+              orderBy('__name__', 'ASCENDING'),
+              queryStartAt(false, {
+                referenceValue:
+                  `projects/${PROJECT_ID}/databases/(default)/` +
+                  'documents/collectionId/doc1',
+              }),
+              fieldFilters(
+                '__name__',
+                'GREATER_THAN_OR_EQUAL',
+                startAt('root'),
+                '__name__',
+                'LESS_THAN',
+                endAt('root')
+              )
+            );
+            return stream();
+          }
+        },
+        batchWrite: () => response(successResponse(1)),
+      };
+
+      const firestore = await createInstance(overrides);
+      await firestore.recursiveDelete(firestore.collection('root'));
+    });
   });
 
   describe('deletes', () => {