Merge

Author: schmidt-sebastian

ci/fireci/fireci/commands.py

@@ -98,3 +98,4 @@ def api_information(auth_token, repo_name, issue_number):
     repo = github_client.get_repo(repo_name)
     pr = repo.get_pull(int(issue_number))
     pr.create_issue_comment(comment_string)
+    exit(1)

docs/README.md

@@ -25,6 +25,7 @@ in your app:
 
   * [`firebase-common`](ktx/common.md)
   * [`firebase-firestore`](ktx/firestore.md)
+  * [`firebase-functions`](ktx/functions.md)
 
 [android-setup]: https://firebase.google.com/docs/android/setup
 [main-readme]: https://github.com/firebase/firebase-android-sdk/blob/master/README.md

docs/ktx/functions.md

@@ -0,0 +1,62 @@
+# Cloud Functions Kotlin Extensions
+
+## Getting Started
+
+To use the Cloud Functions Android SDK with Kotlin Extensions, add the following
+to your app's `build.gradle` file:
+
+```groovy
+// See maven.google.com for the latest versions
+// This library transitively includes the firebase-functions library
+implementation 'com.google.firebase:firebase-functions-ktx:$VERSION'
+```
+
+## Features
+
+### Get the FirebaseFunctions instance of the default app
+
+**Kotlin**
+```kotlin
+val functions = FirebaseFunctions.getInstance()
+```
+
+**Kotlin + KTX**
+```kotlin
+val functions = Firebase.functions
+```
+
+### Get the FirebaseFunctions of a given region
+
+**Kotlin**
+```kotlin
+val functions = FirebaseFunctions.getInstance(region)
+```
+
+**Kotlin + KTX**
+```kotlin
+val functions = Firebase.functions(region)
+```
+
+### Get the FirebaseFunctions of a given FirebaseApp
+
+**Kotlin**
+```kotlin
+val functions = FirebaseFunctions.getInstance(app)
+```
+
+**Kotlin + KTX**
+```kotlin
+val functions = Firebase.functions(app)
+```
+
+### Get the FirebaseFunctions of a given region and FirebaseApp
+
+**Kotlin**
+```kotlin
+val functions = FirebaseFunctions.getInstance(app, region)
+```
+
+**Kotlin + KTX**
+```kotlin
+val functions = Firebase.functions(app, region)
+```

firebase-common/firebase-common.gradle

@@ -55,6 +55,9 @@ android {
             includeAndroidResources = true
         }
     }
+    lintOptions {
+        baseline file("lint-baseline.xml")
+    }
 }
 
 dependencies {

firebase-common/lint-baseline.xml

@@ -0,0 +1,15 @@
+<?xml version="1.0" encoding="UTF-8"?>
+<issues format="5" by="lint 3.4.1" client="gradle" variant="all" version="3.4.1">
+  <!-- TODO: remove when bazel support tools:targetApi -->
+    <issue
+        id="UnusedAttribute"
+        message="Attribute `directBootAware` is only used in API level 24 and higher (current min is 14)"
+        errorLine1="        android:directBootAware=&quot;true&quot; android:exported=&quot;false&quot;/>"
+        errorLine2="        ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~">
+        <location
+            file="src/main/AndroidManifest.xml"
+            line="11"
+            column="9"/>
+    </issue>
+
+</issues>

firebase-common/src/main/AndroidManifest.xml

@@ -8,8 +8,7 @@
   <application>
 
     <service android:name="com.google.firebase.components.ComponentDiscoveryService"
-        android:directBootAware="true" android:exported="false"
-        tools:targetApi="n" />
+        android:directBootAware="true" android:exported="false"/>
 
     <provider
         android:name="com.google.firebase.provider.FirebaseInitProvider"

firebase-firestore/api.txt

@@ -165,10 +165,6 @@ package com.google.firebase.firestore {
     method public static void setLoggingEnabled(boolean);
   }
 
-  public static interface FirebaseFirestore.InstanceRegistry {
-    method public void remove(@NonNull String);
-  }
-
   public class FirebaseFirestoreException {
     ctor public FirebaseFirestoreException(@NonNull String, @NonNull com.google.firebase.firestore.FirebaseFirestoreException.Code);
     ctor public FirebaseFirestoreException(@NonNull String, @NonNull com.google.firebase.firestore.FirebaseFirestoreException.Code, @Nullable Throwable);

firebase-firestore/src/main/java/com/google/firebase/firestore/local/LocalStore.java

@@ -326,56 +326,43 @@ public SnapshotVersion getLastRemoteSnapshotVersion() {
    * <p>LocalDocuments are re-calculated if there are remaining mutations in the queue.
    */
   public ImmutableSortedMap<DocumentKey, MaybeDocument> applyRemoteEvent(RemoteEvent remoteEvent) {
+    SnapshotVersion remoteVersion = remoteEvent.getSnapshotVersion();
+
     // TODO: Call queryEngine.handleDocumentChange() appropriately.
     return persistence.runTransaction(
         "Apply remote event",
         () -> {
+          Map<Integer, TargetChange> targetChanges = remoteEvent.getTargetChanges();
           long sequenceNumber = persistence.getReferenceDelegate().getCurrentSequenceNumber();
-          Set<DocumentKey> authoritativeUpdates = new HashSet<>();
 
-          Map<Integer, TargetChange> targetChanges = remoteEvent.getTargetChanges();
           for (Map.Entry<Integer, TargetChange> entry : targetChanges.entrySet()) {
             Integer boxedTargetId = entry.getKey();
             int targetId = boxedTargetId;
             TargetChange change = entry.getValue();
 
-            // Do not ref/unref unassigned targetIds - it may lead to leaks.
-            QueryData queryData = targetIds.get(targetId);
-            if (queryData == null) {
+            QueryData oldQueryData = targetIds.get(targetId);
+            if (oldQueryData == null) {
+              // We don't update the remote keys if the query is not active. This ensures that
+              // we persist the updated query data along with the updated assignment.
               continue;
             }
 
-            // When a global snapshot contains updates (either add or modify) we can completely
-            // trust these updates as authoritative and blindly apply them to our cache (as a
-            // defensive measure to promote self-healing in the unfortunate case that our cache
-            // is ever somehow corrupted / out-of-sync).
-            //
-            // If the document is only updated while removing it from a target then watch isn't
-            // obligated to send the absolute latest version: it can send the first version that
-            // caused the document not to match.
-            for (DocumentKey key : change.getAddedDocuments()) {
-              authoritativeUpdates.add(key);
-            }
-            for (DocumentKey key : change.getModifiedDocuments()) {
-              authoritativeUpdates.add(key);
-            }
-
             queryCache.removeMatchingKeys(change.getRemovedDocuments(), targetId);
             queryCache.addMatchingKeys(change.getAddedDocuments(), targetId);
 
-            // Update the resume token if the change includes one. Don't clear any preexisting
-            // value.
             ByteString resumeToken = change.getResumeToken();
+            // Update the resume token if the change includes one.
             if (!resumeToken.isEmpty()) {
-              QueryData oldQueryData = queryData;
-              queryData =
-                  queryData
+              QueryData newQueryData =
+                  oldQueryData
                       .withResumeToken(resumeToken, remoteEvent.getSnapshotVersion())
                       .withSequenceNumber(sequenceNumber);
-              targetIds.put(boxedTargetId, queryData);
+              targetIds.put(boxedTargetId, newQueryData);
 
-              if (shouldPersistQueryData(oldQueryData, queryData, change)) {
-                queryCache.updateQueryData(queryData);
+              // Update the query data if there are target changes (or if sufficient time has
+              // passed since the last update).
+              if (shouldPersistQueryData(oldQueryData, newQueryData, change)) {
+                queryCache.updateQueryData(newQueryData);
               }
             }
           }
@@ -394,10 +381,9 @@ public ImmutableSortedMap<DocumentKey, MaybeDocument> applyRemoteEvent(RemoteEve
             MaybeDocument existingDoc = existingDocs.get(key);
 
             if (existingDoc == null
-                || (authoritativeUpdates.contains(doc.getKey()) && !existingDoc.hasPendingWrites())
-                || doc.getVersion().compareTo(existingDoc.getVersion()) >= 0) {
-              // If a document update isn't authoritative, make sure we don't apply an old document
-              // version to the remote cache.
+                || doc.getVersion().compareTo(existingDoc.getVersion()) > 0
+                || (doc.getVersion().compareTo(existingDoc.getVersion()) == 0
+                    && existingDoc.hasPendingWrites())) {
               remoteDocuments.add(doc, remoteEvent.getSnapshotVersion());
               changedDocs.put(key, doc);
             } else if (doc instanceof NoDocument && doc.getVersion().equals(SnapshotVersion.NONE)) {
@@ -425,7 +411,6 @@ public ImmutableSortedMap<DocumentKey, MaybeDocument> applyRemoteEvent(RemoteEve
           // remote events when we get permission denied errors while trying to resolve the
           // state of a locally cached document that is in limbo.
           SnapshotVersion lastRemoteVersion = queryCache.getLastRemoteSnapshotVersion();
-          SnapshotVersion remoteVersion = remoteEvent.getSnapshotVersion();
           if (!remoteVersion.equals(SnapshotVersion.NONE)) {
             hardAssert(
                 remoteVersion.compareTo(lastRemoteVersion) >= 0,
@@ -451,10 +436,11 @@ public ImmutableSortedMap<DocumentKey, MaybeDocument> applyRemoteEvent(RemoteEve
    */
   private static boolean shouldPersistQueryData(
       QueryData oldQueryData, QueryData newQueryData, TargetChange change) {
-    // Avoid clearing any existing value
-    if (newQueryData.getResumeToken().isEmpty()) return false;
+    hardAssert(
+        !newQueryData.getResumeToken().isEmpty(),
+        "Attempted to persist query data with empty resume token");
 
-    // Any resume token is interesting if there isn't one already.
+    // Always persist query data if we don't already have a resume token.
     if (oldQueryData.getResumeToken().isEmpty()) return true;
 
     // Don't allow resume token changes to be buffered indefinitely. This allows us to be reasonably

firebase-firestore/src/test/java/com/google/firebase/firestore/local/LocalStoreTestCase.java

@@ -1386,4 +1386,27 @@ public void testGetHighestUnacknowledgedBatchId() {
     rejectMutation();
     assertEquals(MutationBatch.UNKNOWN, localStore.getHighestUnacknowledgedBatchId());
   }
+
+  @Test
+  public void testOnlyPersistsUpdatesForDocumentsWhenVersionChanges() {
+    Query query = Query.atPath(ResourcePath.fromString("foo"));
+    allocateQuery(query);
+    assertTargetId(2);
+
+    applyRemoteEvent(
+        addedRemoteEvent(doc("foo/bar", 1, map("val", "old")), asList(2), emptyList()));
+    assertChanged(doc("foo/bar", 1, map("val", "old"), Document.DocumentState.SYNCED));
+    assertContains(doc("foo/bar", 1, map("val", "old"), Document.DocumentState.SYNCED));
+
+    applyRemoteEvent(
+        addedRemoteEvent(
+            asList(doc("foo/bar", 1, map("val", "new")), doc("foo/baz", 2, map("val", "new"))),
+            asList(2),
+            emptyList()));
+
+    assertChanged(doc("foo/baz", 2, map("val", "new"), Document.DocumentState.SYNCED));
+    // The update for foo/bar is ignored.
+    assertContains(doc("foo/bar", 1, map("val", "old"), Document.DocumentState.SYNCED));
+    assertContains(doc("foo/baz", 2, map("val", "new"), Document.DocumentState.SYNCED));
+  }
 }

firebase-firestore/src/test/resources/json/limbo_spec_test.json

@@ -611,7 +611,7 @@
           "docs": [
             {
               "key": "collection/a",
-              "version": 1000,
+              "version": 1002,
               "value": {
                 "key": "b"
               },
@@ -948,7 +948,7 @@
           "docs": [
             {
               "key": "collection/a",
-              "version": 1000,
+              "version": 1002,
               "value": {
                 "key": "b"
               },
@@ -1055,7 +1055,7 @@
             "added": [
               {
                 "key": "collection/a",
-                "version": 1000,
+                "version": 1002,
                 "value": {
                   "key": "b"
                 },