Release limitToLast and query/target split. (#994)

* [1/3] Split Query and Target and make RemoteStore and LocalStore work with Targets (#963)

* Split Query and Target and make RemoteStore and LocalStore work with Targets mostly.

* Address comments 1

* [2/3] Make SyncEngine able to work with both Query and Target (#965)

* Make sync engine work with both Query and Target

* stopListening handles query:target mapping

* Address comments

* Merge limitToLast to correct remote branch (#984)

* Make sync engine work with both Query and Target

* stopListening handles query:target mapping

* Add converting for limit to last

* Implement limitToLast and added tests

* Address comments

* Address comments

* Disallow double-releasing.

* [3/3] Add spec tests for limitToLast and proper sync state update for new queries. (#990)

* Add spec test to verify mirror queries being rejected. Also, applying sync state to new query if there are already active queries to the same target.

* remove debug code.

* Add change log

* fix api.txt

* Fix type

Author: wu-hui

firebase-firestore/CHANGELOG.md

@@ -1,4 +1,8 @@
-# Unreleased (21.3.0)
+# Unreleased (21.3.1)
+- [feature] Added `Query.limitToLast(n: long)`, which returns the last `n`
+  documents as the result.
+
+# 21.3.0
 - [feature] Added `Query.whereIn()` and `Query.whereArrayContainsAny()` query
   operators. `Query.whereIn()` finds documents where a specified field’s value
   is IN a specified array. `Query.whereArrayContainsAny()` finds documents

firebase-firestore/api.txt

@@ -268,6 +268,7 @@ package com.google.firebase.firestore {
     method @NonNull public com.google.firebase.firestore.FirebaseFirestore getFirestore();
     method public int hashCode();
     method @NonNull public com.google.firebase.firestore.Query limit(long);
+    method @NonNull public com.google.firebase.firestore.Query limitToLast(long);
     method @NonNull public com.google.firebase.firestore.Query orderBy(@NonNull String);
     method @NonNull public com.google.firebase.firestore.Query orderBy(@NonNull com.google.firebase.firestore.FieldPath);
     method @NonNull public com.google.firebase.firestore.Query orderBy(@NonNull String, @NonNull com.google.firebase.firestore.Query.Direction);

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/QueryTest.java

@@ -20,6 +20,7 @@
 import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testCollectionWithDocs;
 import static com.google.firebase.firestore.testutil.IntegrationTestUtil.testFirestore;
 import static com.google.firebase.firestore.testutil.IntegrationTestUtil.waitFor;
+import static com.google.firebase.firestore.testutil.TestUtil.expectError;
 import static com.google.firebase.firestore.testutil.TestUtil.map;
 import static java.util.Arrays.asList;
 import static java.util.Collections.singletonList;
@@ -80,6 +81,78 @@ public void testLimitQueriesUsingDescendingSortOrder() {
     assertEquals(asList(map("k", "d", "sort", 2L), map("k", "c", "sort", 1L)), data);
   }
 
+  @Test
+  public void testLimitToLastMustAlsoHaveExplicitOrderBy() {
+    CollectionReference collection = testCollectionWithDocs(map());
+
+    Query query = collection.limitToLast(2);
+    expectError(
+        () -> waitFor(query.get()),
+        "limitToLast() 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.
+  @Test
+  public void testListenUnlistenRelistenSequenceOfMirrorQueries() {
+    CollectionReference collection =
+        testCollectionWithDocs(
+            map(
+                "a", map("k", "a", "sort", 0),
+                "b", map("k", "b", "sort", 1),
+                "c", map("k", "c", "sort", 1),
+                "d", map("k", "d", "sort", 2)));
+
+    // Setup `limit` query.
+    Query limit = collection.limit(2).orderBy("sort", Direction.ASCENDING);
+    EventAccumulator<QuerySnapshot> limitAccumulator = new EventAccumulator<>();
+    ListenerRegistration limitRegistration = limit.addSnapshotListener(limitAccumulator.listener());
+
+    // Setup mirroring `limitToLast` query.
+    Query limitToLast = collection.limitToLast(2).orderBy("sort", Direction.DESCENDING);
+    EventAccumulator<QuerySnapshot> limitToLastAccumulator = new EventAccumulator<>();
+    ListenerRegistration limitToLastRegistration =
+        limitToLast.addSnapshotListener(limitToLastAccumulator.listener());
+
+    // Verify both query get expected result.
+    List<Map<String, Object>> data = querySnapshotToValues(limitAccumulator.await());
+    assertEquals(asList(map("k", "a", "sort", 0L), map("k", "b", "sort", 1L)), data);
+    data = querySnapshotToValues(limitToLastAccumulator.await());
+    assertEquals(asList(map("k", "b", "sort", 1L), map("k", "a", "sort", 0L)), data);
+
+    // Unlisten then re-listen limit query.
+    limitRegistration.remove();
+    limit.addSnapshotListener(limitAccumulator.listener());
+
+    // Verify `limit` query still works.
+    data = querySnapshotToValues(limitAccumulator.await());
+    assertEquals(asList(map("k", "a", "sort", 0L), map("k", "b", "sort", 1L)), data);
+
+    // Add a document that would change the result set.
+    waitFor(collection.add(map("k", "e", "sort", -1)));
+
+    // Verify both query get expected result.
+    data = querySnapshotToValues(limitAccumulator.await());
+    assertEquals(asList(map("k", "e", "sort", -1L), map("k", "a", "sort", 0L)), data);
+    data = querySnapshotToValues(limitToLastAccumulator.await());
+    assertEquals(asList(map("k", "a", "sort", 0L), map("k", "e", "sort", -1L)), data);
+
+    // Unlisten to limitToLast, update a doc, then relisten to limitToLast
+    limitToLastRegistration.remove();
+    waitFor(collection.document("a").update(map("k", "a", "sort", -2)));
+    limitToLast.addSnapshotListener(limitToLastAccumulator.listener());
+
+    // Verify both query get expected result.
+    data = querySnapshotToValues(limitAccumulator.await());
+    assertEquals(asList(map("k", "a", "sort", -2L), map("k", "e", "sort", -1L)), data);
+    data = querySnapshotToValues(limitToLastAccumulator.await());
+    assertEquals(asList(map("k", "e", "sort", -1L), map("k", "a", "sort", -2L)), data);
+  }
+
   @Test
   public void testKeyOrderIsDescendingForDescendingInequality() {
     CollectionReference collection =

firebase-firestore/src/androidTest/java/com/google/firebase/firestore/ValidationTest.java

@@ -401,6 +401,12 @@ public void queriesWithNonPositiveLimitFail() {
     expectError(
         () -> collection.limit(-1),
         "Invalid Query. Query limit (-1) is invalid. Limit must be positive.");
+    expectError(
+        () -> collection.limitToLast(0),
+        "Invalid Query. Query limitToLast (0) is invalid. Limit must be positive.");
+    expectError(
+        () -> collection.limitToLast(-1),
+        "Invalid Query. Query limitToLast (-1) is invalid. Limit must be positive.");
   }
 
   @Test

firebase-firestore/src/main/java/com/google/firebase/firestore/Query.java

@@ -574,8 +574,8 @@ private Query orderBy(
   }
 
   /**
-   * Creates and returns a new {@code Query} that's additionally limited to only return up to the
-   * specified number of documents.
+   * Creates and returns a new {@code Query} that only returns the first matching documents up to
+   * the specified number.
    *
    * @param limit The maximum number of items to return.
    * @return The created {@code Query}.
@@ -586,7 +586,26 @@ public Query limit(long limit) {
       throw new IllegalArgumentException(
           "Invalid Query. Query limit (" + limit + ") is invalid. Limit must be positive.");
     }
-    return new Query(query.limit(limit), firestore);
+    return new Query(query.limitToFirst(limit), firestore);
+  }
+
+  /**
+   * Creates and returns a new {@code Query} that only returns the last matching documents up to the
+   * specified number.
+   *
+   * <p>You must specify at least one {@code orderBy} clause for {@code limitToLast} queries,
+   * otherwise an exception will be thrown during execution.
+   *
+   * @param limit The maximum number of items to return.
+   * @return The created {@code Query}.
+   */
+  @NonNull
+  public Query limitToLast(long limit) {
+    if (limit <= 0) {
+      throw new IllegalArgumentException(
+          "Invalid Query. Query limitToLast (" + limit + ") is invalid. Limit must be positive.");
+    }
+    return new Query(query.limitToLast(limit), firestore);
   }
 
   /**
@@ -832,6 +851,7 @@ public Task<QuerySnapshot> get() {
    */
   @NonNull
   public Task<QuerySnapshot> get(@NonNull Source source) {
+    validateHasExplicitOrderByForLimitToLast();
     if (source == Source.CACHE) {
       return firestore
           .getClient()
@@ -1010,6 +1030,7 @@ private ListenerRegistration addSnapshotListenerInternal(
       ListenOptions options,
       @Nullable Activity activity,
       EventListener<QuerySnapshot> userListener) {
+    validateHasExplicitOrderByForLimitToLast();
 
     // Convert from ViewSnapshots to QuerySnapshots.
     EventListener<ViewSnapshot> viewListener =
@@ -1035,6 +1056,13 @@ private ListenerRegistration addSnapshotListenerInternal(
         new ListenerRegistrationImpl(firestore.getClient(), queryListener, asyncListener));
   }
 
+  private void validateHasExplicitOrderByForLimitToLast() {
+    if (query.hasLimitToLast() && query.getExplicitOrderBy().isEmpty()) {
+      throw new IllegalStateException(
+          "limitToLast() queries require specifying at least one orderBy() clause");
+    }
+  }
+
   @Override
   public boolean equals(Object o) {
     if (this == o) {