Release LimitToLast (#4616)

* [1/4] Split Query and Target, and make LocalStore/RemoteStore work with Target. (#4413)

* [2/4]LimitToLast: Make SyncEngine work with Target (#4491)

* [3/4] Implement LimitToLast (#4505)

Author: wu-hui

Firestore/CHANGELOG.md

@@ -1,4 +1,6 @@
 # Unreleased
+- [feature] Added a `limit(toLast:)` query operator, which returns the last
+  matching documents up to the given limit.
 
 # v1.8.3
 - [changed] Internal improvements.

Firestore/Example/Tests/Integration/API/FIRQueryTests.mm

@@ -21,6 +21,7 @@
 #import "Firestore/Source/API/FIRQuery+Internal.h"
 
 #import "Firestore/Example/Tests/Util/FSTEventAccumulator.h"
+#import "Firestore/Example/Tests/Util/FSTHelpers.h"
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 
 @interface FIRQueryTests : FSTIntegrationTestCase
@@ -56,6 +57,84 @@ - (void)testLimitQueriesWithDescendingSortOrder {
                         (@[ @{@"k" : @"d", @"sort" : @2}, @{@"k" : @"c", @"sort" : @1} ]));
 }
 
+- (void)testLimitToLastMustAlsoHaveExplicitOrderBy {
+  FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{}];
+  FIRQuery *query = [collRef queryLimitedToLast:2];
+  FSTAssertThrows(
+      [query getDocumentsWithCompletion:^(FIRQuerySnapshot *documentSet, NSError *error){
+      }],
+      @"limit(toLast:) queries require specifying at least one OrderBy() clause.");
+}
+
+// Two queries that mapped to the same target ID are referred to as
+// "mirror queries". An example for a mirror query is a limitToLast()
+// query and a limit() query that share the same backend Target ID.
+// Since limitToLast() queries are sent to the backend with a modified
+// orderBy() clause, they can map to the same target representation as
+// limit() query, even if both queries appear separate to the user.
+- (void)testListenUnlistenRelistenSequenceOfMirrorQueries {
+  FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
+    @"a" : @{@"k" : @"a", @"sort" : @0},
+    @"b" : @{@"k" : @"b", @"sort" : @1},
+    @"c" : @{@"k" : @"c", @"sort" : @1},
+    @"d" : @{@"k" : @"d", @"sort" : @2},
+  }];
+
+  // Setup a `limit` query.
+  FIRQuery *limit = [[collRef queryOrderedByField:@"sort" descending:NO] queryLimitedTo:2];
+  FSTEventAccumulator *limitAccumulator = [FSTEventAccumulator accumulatorForTest:self];
+  id<FIRListenerRegistration> limitRegistration =
+      [limit addSnapshotListener:limitAccumulator.valueEventHandler];
+
+  // Setup a mirroring `limitToLast` query.
+  FIRQuery *limitToLast = [[collRef queryOrderedByField:@"sort"
+                                             descending:YES] queryLimitedToLast:2];
+  FSTEventAccumulator *limitToLastAccumulator = [FSTEventAccumulator accumulatorForTest:self];
+  id<FIRListenerRegistration> limitToLastRegistration =
+      [limitToLast addSnapshotListener:limitToLastAccumulator.valueEventHandler];
+
+  // Verify both queries get expected result.
+  FIRQuerySnapshot *snapshot = [limitAccumulator awaitEventWithName:@"Snapshot"];
+  NSArray *expected = @[ @{@"k" : @"a", @"sort" : @0}, @{@"k" : @"b", @"sort" : @1} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+  snapshot = [limitToLastAccumulator awaitEventWithName:@"Snapshot"];
+  expected = @[ @{@"k" : @"b", @"sort" : @1}, @{@"k" : @"a", @"sort" : @0} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+
+  // Unlisten then re-listen limit query.
+  [limitRegistration remove];
+  [limit addSnapshotListener:[limitAccumulator valueEventHandler]];
+
+  // Verify limit query still works.
+  snapshot = [limitAccumulator awaitEventWithName:@"Snapshot"];
+  expected = @[ @{@"k" : @"a", @"sort" : @0}, @{@"k" : @"b", @"sort" : @1} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+
+  // Add a document that would change the result set.
+  [self addDocumentRef:collRef data:@{@"k" : @"e", @"sort" : @-1}];
+
+  // Verify both queries get expected result.
+  snapshot = [limitAccumulator awaitEventWithName:@"Snapshot"];
+  expected = @[ @{@"k" : @"e", @"sort" : @-1}, @{@"k" : @"a", @"sort" : @0} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+  snapshot = [limitToLastAccumulator awaitEventWithName:@"Snapshot"];
+  expected = @[ @{@"k" : @"a", @"sort" : @0}, @{@"k" : @"e", @"sort" : @-1} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+
+  // Unlisten to limitToLast, update a doc, then relisten to limitToLast
+  [limitToLastRegistration remove];
+  [self updateDocumentRef:[collRef documentWithPath:@"a"] data:@{@"k" : @"a", @"sort" : @-2}];
+  [limitToLast addSnapshotListener:[limitToLastAccumulator valueEventHandler]];
+
+  // Verify both queries get expected result.
+  snapshot = [limitAccumulator awaitEventWithName:@"Snapshot"];
+  expected = @[ @{@"k" : @"a", @"sort" : @-2}, @{@"k" : @"e", @"sort" : @-1} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+  snapshot = [limitToLastAccumulator awaitEventWithName:@"Snapshot"];
+  expected = @[ @{@"k" : @"e", @"sort" : @-1}, @{@"k" : @"a", @"sort" : @-2} ];
+  XCTAssertEqualObjects(FIRQuerySnapshotGetData(snapshot), expected);
+}
+
 - (void)testKeyOrderIsDescendingForDescendingInequality {
   FIRCollectionReference *collRef = [self collectionRefWithDocuments:@{
     @"a" : @{@"foo" : @42},

Firestore/Example/Tests/Integration/API/FIRValidationTests.mm

@@ -410,6 +410,10 @@ - (void)testQueryWithNonPositiveLimitFails {
                   @"Invalid Query. Query limit (0) is invalid. Limit must be positive.");
   FSTAssertThrows([[self collectionRef] queryLimitedTo:-1],
                   @"Invalid Query. Query limit (-1) is invalid. Limit must be positive.");
+  FSTAssertThrows([[self collectionRef] queryLimitedToLast:0],
+                  @"Invalid Query. Query limit (0) is invalid. Limit must be positive.");
+  FSTAssertThrows([[self collectionRef] queryLimitedToLast:-1],
+                  @"Invalid Query. Query limit (-1) is invalid. Limit must be positive.");
 }
 
 - (void)testNonEqualityQueriesOnNullOrNaNFail {

Firestore/Example/Tests/SpecTests/FSTMockDatastore.mm

@@ -102,7 +102,7 @@ bool IsOpen() const override {
   }
 
   void WatchQuery(const QueryData& query) override {
-    LOG_DEBUG("WatchQuery: %s: %s, %s", query.target_id(), query.query().ToString(),
+    LOG_DEBUG("WatchQuery: %s: %s, %s", query.target_id(), query.target().ToString(),
               query.resume_token().ToString());
 
     // Snapshot version is ignored on the wire

Firestore/Example/Tests/SpecTests/FSTSpecTests.mm

@@ -86,7 +86,6 @@
 using firebase::firestore::remote::ExistenceFilter;
 using firebase::firestore::remote::DocumentWatchChange;
 using firebase::firestore::remote::ExistenceFilterWatchChange;
-using firebase::firestore::remote::InvalidQuery;
 using firebase::firestore::remote::WatchTargetChange;
 using firebase::firestore::remote::WatchTargetChangeState;
 using firebase::firestore::util::MakeString;
@@ -214,11 +213,18 @@ - (Query)parseQuery:(id)querySpec {
     std::shared_ptr<const std::string> collectionGroup =
         util::MakeStringPtr(queryDict[@"collectionGroup"]);
     Query query(std::move(resource_path), std::move(collectionGroup));
+
     if (queryDict[@"limit"]) {
       NSNumber *limitNumber = queryDict[@"limit"];
       auto limit = static_cast<int32_t>(limitNumber.integerValue);
-      query = query.WithLimit(limit);
+      NSString *limitType = queryDict[@"limitType"];
+      if ([limitType isEqualToString:@"LimitToFirst"]) {
+        query = query.WithLimitToFirst(limit);
+      } else {
+        query = query.WithLimitToLast(limit);
+      }
     }
+
     if (queryDict[@"filters"]) {
       NSArray<NSArray<id> *> *filters = queryDict[@"filters"];
       for (NSArray<id> *filter in filters) {
@@ -228,6 +234,7 @@ - (Query)parseQuery:(id)querySpec {
         query = query.AddingFilter(Filter(key, op, value));
       }
     }
+
     if (queryDict[@"orderBys"]) {
       NSArray *orderBys = queryDict[@"orderBys"];
       for (NSArray<NSString *> *orderBy in orderBys) {
@@ -239,7 +246,7 @@ - (Query)parseQuery:(id)querySpec {
     return query;
   } else {
     XCTFail(@"Invalid query: %@", querySpec);
-    return InvalidQuery();
+    return Query();
   }
 }
 
@@ -651,21 +658,26 @@ - (void)validateExpectedState:(nullable NSDictionary *)expectedState {
       [self.driver setExpectedLimboDocuments:std::move(expectedLimboDocuments)];
     }
     if (expectedState[@"activeTargets"]) {
-      __block std::unordered_map<TargetId, QueryData> expectedActiveTargets;
+      __block ActiveTargetMap expectedActiveTargets;
       [expectedState[@"activeTargets"]
           enumerateKeysAndObjectsUsingBlock:^(NSString *targetIDString, NSDictionary *queryData,
                                               BOOL *stop) {
             TargetId targetID = [targetIDString intValue];
-            Query query = [self parseQuery:queryData[@"query"]];
             ByteString resumeToken = MakeResumeToken(queryData[@"resumeToken"]);
-            // TODO(mcg): populate the purpose of the target once it's possible to encode that in
-            // the spec tests. For now, hard-code that it's a listen despite the fact that it's not
-            // always the right value.
-            expectedActiveTargets[targetID] =
-                QueryData(std::move(query), targetID, 0, QueryPurpose::Listen,
-                          SnapshotVersion::None(), SnapshotVersion::None(), std::move(resumeToken));
+            NSArray *queriesJson = queryData[@"queries"];
+            std::vector<QueryData> queries;
+            for (id queryJson in queriesJson) {
+              Query query = [self parseQuery:queryJson];
+              // TODO(mcg): populate the purpose of the target once it's possible to encode that in
+              // the spec tests. For now, hard-code that it's a listen despite the fact that it's
+              // not always the right value.
+              queries.push_back(QueryData(query.ToTarget(), targetID, 0, QueryPurpose::Listen,
+                                          SnapshotVersion::None(), SnapshotVersion::None(),
+                                          std::move(resumeToken)));
+            }
+            expectedActiveTargets[targetID] = std::make_pair(std::move(queries), resumeToken);
           }];
-      [self.driver setExpectedActiveTargets:expectedActiveTargets];
+      [self.driver setExpectedActiveTargets:std::move(expectedActiveTargets)];
     }
   }
 
@@ -726,9 +738,10 @@ - (void)validateActiveTargets {
   // Create a copy so we can modify it below
   std::unordered_map<TargetId, QueryData> actualTargets = [self.driver activeTargets];
 
-  for (const auto &kv : [self.driver activeTargets]) {
+  for (const auto &kv : [self.driver expectedActiveTargets]) {
     TargetId targetID = kv.first;
-    const QueryData &queryData = kv.second;
+    const std::pair<std::vector<QueryData>, ByteString> &queries = kv.second;
+    const QueryData &queryData = queries.first[0];
 
     auto found = actualTargets.find(targetID);
     XCTAssertNotEqual(found, actualTargets.end(), @"Expected active target not found: %s",
@@ -739,7 +752,7 @@ - (void)validateActiveTargets {
     // XCTAssertEqualObjects(actualTargets[targetID], queryData);
 
     const QueryData &actual = found->second;
-    XCTAssertEqual(actual.query(), queryData.query());
+    XCTAssertEqual(actual.target(), queryData.target());
     XCTAssertEqual(actual.target_id(), queryData.target_id());
     XCTAssertEqual(actual.snapshot_version(), queryData.snapshot_version());
     XCTAssertEqual(actual.resume_token(), queryData.resume_token());

Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.h

@@ -19,6 +19,7 @@
 #include <map>
 #include <memory>
 #include <unordered_map>
+#include <utility>
 #include <vector>
 
 #include "Firestore/core/src/firebase/firestore/auth/user.h"
@@ -31,6 +32,8 @@
 #include "Firestore/core/src/firebase/firestore/model/mutation.h"
 #include "Firestore/core/src/firebase/firestore/model/snapshot_version.h"
 #include "Firestore/core/src/firebase/firestore/model/types.h"
+#include "Firestore/core/src/firebase/firestore/nanopb/byte_string.h"
+#include "Firestore/core/src/firebase/firestore/nanopb/nanopb_util.h"
 #include "Firestore/core/src/firebase/firestore/remote/watch_change.h"
 #include "Firestore/core/src/firebase/firestore/util/async_queue.h"
 #include "Firestore/core/src/firebase/firestore/util/empty.h"
@@ -48,6 +51,14 @@ class Persistence;
 namespace core = firebase::firestore::core;
 namespace local = firebase::firestore::local;
 namespace model = firebase::firestore::model;
+namespace nanopb = firebase::firestore::nanopb;
+
+// A map holds expected information about currently active targets. The keys are
+// target ID, and the values are a vector of `QueryData`s mapped to the target and
+// the target's resume token.
+using ActiveTargetMap =
+    std::unordered_map<model::TargetId,
+                       std::pair<std::vector<local::QueryData>, nanopb::ByteString>>;
 
 NS_ASSUME_NONNULL_BEGIN
 
@@ -337,11 +348,9 @@ typedef std::unordered_map<firebase::firestore::auth::User,
 - (const std::unordered_map<firebase::firestore::model::TargetId, local::QueryData> &)activeTargets;
 
 /** The expected set of active targets, keyed by target ID. */
-- (const std::unordered_map<firebase::firestore::model::TargetId, local::QueryData> &)
-    expectedActiveTargets;
+- (const ActiveTargetMap &)expectedActiveTargets;
 
-- (void)setExpectedActiveTargets:
-    (const std::unordered_map<firebase::firestore::model::TargetId, local::QueryData> &)targets;
+- (void)setExpectedActiveTargets:(ActiveTargetMap)targets;
 
 @end
 

Firestore/Example/Tests/SpecTests/FSTSyncEngineTestDriver.mm

@@ -165,7 +165,9 @@ @implementation FSTSyncEngineTestDriver {
 
   DelayedConstructor<EventManager> _eventManager;
 
-  std::unordered_map<TargetId, QueryData> _expectedActiveTargets;
+  // Set of active targets, keyed by target Id, mapped to corresponding resume token,
+  // and list of `QueryData`.
+  ActiveTargetMap _expectedActiveTargets;
 
   // ivar is declared as mutable.
   std::unordered_map<User, NSMutableArray<FSTOutstandingWrite *> *, HashUser> _outstandingWrites;
@@ -478,12 +480,12 @@ - (void)receiveWatchStreamError:(int)errorCode userInfo:(NSDictionary<NSString *
   return _datastore->ActiveTargets();
 }
 
-- (const std::unordered_map<TargetId, QueryData> &)expectedActiveTargets {
+- (const ActiveTargetMap &)expectedActiveTargets {
   return _expectedActiveTargets;
 }
 
-- (void)setExpectedActiveTargets:(const std::unordered_map<TargetId, QueryData> &)targets {
-  _expectedActiveTargets = targets;
+- (void)setExpectedActiveTargets:(ActiveTargetMap)targets {
+  _expectedActiveTargets = std::move(targets);
 }
 
 #pragma mark - Helper Methods

Firestore/Example/Tests/SpecTests/json/collection_spec_test.json

@@ -20,11 +20,13 @@
         "expectedState": {
           "activeTargets": {
             "2": {
-              "query": {
-                "path": "collection",
-                "filters": [],
-                "orderBys": []
-              },
+              "queries": [
+                {
+                  "path": "collection",
+                  "filters": [],
+                  "orderBys": []
+                }
+              ],
               "resumeToken": ""
             }
           }
@@ -117,11 +119,13 @@
         "expectedState": {
           "activeTargets": {
             "2": {
-              "query": {
-                "path": "collection",
-                "filters": [],
-                "orderBys": []
-              },
+              "queries": [
+                {
+                  "path": "collection",
+                  "filters": [],
+                  "orderBys": []
+                }
+              ],
               "resumeToken": ""
             }
           }