[firestore] Fix FirestoreQuery/FirestoreQueryModifiers incorrectly mutating previous query instances when chaining (fixes invertase#2691)

Author: Salakar

packages/firestore/e2e/Query/query.e2e.js

@@ -0,0 +1,30 @@
+/*
+ *  Copyright (c) 2016-present Invertase Limited & Contributors
+ *
+ *  Licensed under the Apache License, Version 2.0 (the "License");
+ *  you may not use this library except in compliance with the License.
+ *  You may obtain a copy of the License at
+ *
+ *    http://www.apache.org/licenses/LICENSE-2.0
+ *
+ *  Unless required by applicable law or agreed to in writing, software
+ *  distributed under the License is distributed on an "AS IS" BASIS,
+ *  WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ *  See the License for the specific language governing permissions and
+ *  limitations under the License.
+ */
+
+describe('FirestoreQuery/FirestoreQueryModifiers', () => {
+  it('should not mutate previous queries (#2691)', async () => {
+    const queryBefore = firebase
+      .firestore()
+      .collection('users')
+      .where('age', '>', 30);
+    const queryAfter = queryBefore.orderBy('age');
+    queryBefore._modifiers._orders.length.should.equal(0);
+    queryBefore._modifiers._filters.length.should.equal(1);
+
+    queryAfter._modifiers._orders.length.should.equal(1);
+    queryAfter._modifiers._filters.length.should.equal(1);
+  });
+});

packages/firestore/lib/FirestoreQuery.js

@@ -36,6 +36,8 @@ export default class FirestoreQuery {
   }
 
   _handleQueryCursor(cursor, docOrField, fields) {
+    const modifiers = this._modifiers._copy();
+
     if (isUndefined(docOrField)) {
       throw new Error(
         `firebase.firestore().collection().${cursor}(*) Expected a DocumentSnapshot or list of field values but got undefined.`,
@@ -58,7 +60,7 @@ export default class FirestoreQuery {
         );
       }
 
-      const currentOrders = this._modifiers.orders;
+      const currentOrders = modifiers.orders;
 
       const values = [];
 
@@ -77,14 +79,14 @@ export default class FirestoreQuery {
         values.push(value);
       }
 
-      this._modifiers._orders.push({
+      modifiers._orders.push({
         fieldPath: '__name__',
         direction: 'ASCENDING',
       });
 
       values.push(documentSnapshot.id);
 
-      return this._modifiers.setFieldsCursor(cursor, values);
+      return modifiers.setFieldsCursor(cursor, values);
     }
 
     /**
@@ -93,13 +95,13 @@ export default class FirestoreQuery {
 
     const allFields = [docOrField].concat(fields);
 
-    if (allFields.length > this._modifiers.orders.length) {
+    if (allFields.length > modifiers.orders.length) {
       throw new Error(
         `firebase.firestore().collection().${cursor}(*) Too many arguments provided. The number of arguments must be less than or equal to the number of orderBy() clauses.`,
       );
     }
 
-    return this._modifiers.setFieldsCursor(cursor, allFields);
+    return modifiers.setFieldsCursor(cursor, allFields);
   }
 
   endAt(docOrField, ...fields) {
@@ -187,7 +189,7 @@ export default class FirestoreQuery {
       );
     }
 
-    const modifiers = this._modifiers.limit(limit);
+    const modifiers = this._modifiers._copy().limit(limit);
 
     return new FirestoreQuery(this._firestore, this._collectionPath, modifiers);
   }
@@ -291,7 +293,7 @@ export default class FirestoreQuery {
       );
     }
 
-    const modifiers = this._modifiers.orderBy(path, directionStr);
+    const modifiers = this._modifiers._copy().orderBy(path, directionStr);
 
     try {
       modifiers.validateOrderBy();
@@ -355,7 +357,7 @@ export default class FirestoreQuery {
       );
     }
 
-    const modifiers = this._modifiers.where(path, opStr, value);
+    const modifiers = this._modifiers._copy().where(path, opStr, value);
 
     try {
       modifiers.validateWhere();

packages/firestore/lib/FirestoreQueryModifiers.js

@@ -52,6 +52,19 @@ export default class FirestoreQueryModifiers {
     this._endBefore = undefined;
   }
 
+  _copy() {
+    const newInstance = new FirestoreQueryModifiers();
+    newInstance._limit = this._limit;
+    newInstance._filters = [...this._filters];
+    newInstance._orders = [...this._orders];
+    newInstance._type = this._type;
+    newInstance._startAt = this._startAt;
+    newInstance._startAfter = this._startAfter;
+    newInstance._endAt = this._endAt;
+    newInstance._endBefore = this._endBefore;
+    return newInstance;
+  }
+
   get filters() {
     return this._filters;
   }