set bloom filter application status

Author: milaGGL

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/RemoteSerializer.java

@@ -124,8 +124,7 @@ public SnapshotVersion decodeVersion(com.google.protobuf.Timestamp proto) {
   // Names and Keys
 
   /**
-   * Encodes the given document key as a fully qualified name. This includes the
-   * databaseId from the
+   * Encodes the given document key as a fully qualified name. This includes the databaseId from the
    * constructor and the key path.
    */
   public String encodeKey(DocumentKey key) {
@@ -170,10 +169,8 @@ private String encodeResourceName(DatabaseId databaseId, ResourcePath path) {
   }
 
   /**
-   * Decodes a fully qualified resource name into a resource path and validates
-   * that there is a
-   * project and database encoded in the path. There are no guarantees that a
-   * local path is also
+   * Decodes a fully qualified resource name into a resource path and validates that there is a
+   * project and database encoded in the path. There are no guarantees that a local path is also
    * encoded in this resource name.
    */
   private ResourcePath decodeResourceName(String encoded) {
@@ -183,19 +180,15 @@ private ResourcePath decodeResourceName(String encoded) {
     return resource;
   }
 
-  /**
-   * Creates the prefix for a fully qualified resource path, without a local path
-   * on the end.
-   */
+  /** Creates the prefix for a fully qualified resource path, without a local path on the end. */
   private static ResourcePath encodedDatabaseId(DatabaseId databaseId) {
     return ResourcePath.fromSegments(
         Arrays.asList(
             "projects", databaseId.getProjectId(), "databases", databaseId.getDatabaseId()));
   }
 
   /**
-   * Decodes a fully qualified resource name into a resource path and validates
-   * that there is a
+   * Decodes a fully qualified resource name into a resource path and validates that there is a
    * project and database encoded in the path along with a local path.
    */
   private static ResourcePath extractLocalPathFromResourceName(ResourcePath resourceName) {
@@ -206,10 +199,7 @@ private static ResourcePath extractLocalPathFromResourceName(ResourcePath resour
     return resourceName.popFirst(5);
   }
 
-  /**
-   * Validates that a path has a prefix that looks like a valid encoded
-   * databaseId.
-   */
+  /** Validates that a path has a prefix that looks like a valid encoded databaseId. */
   private static boolean isValidResourceName(ResourcePath path) {
     // Resource names have at least 4 components (project ID, database ID)
     // and commonly the (root) resource type, e.g. documents
@@ -231,7 +221,8 @@ public String databaseName() {
   // Documents
 
   public com.google.firestore.v1.Document encodeDocument(DocumentKey key, ObjectValue value) {
-    com.google.firestore.v1.Document.Builder builder = com.google.firestore.v1.Document.newBuilder();
+    com.google.firestore.v1.Document.Builder builder =
+        com.google.firestore.v1.Document.newBuilder();
     builder.setName(encodeKey(key));
     builder.putAllFields(value.getFieldsMap());
     return builder.build();
@@ -300,9 +291,10 @@ public com.google.firestore.v1.Write encodeMutation(Mutation mutation) {
   }
 
   public Mutation decodeMutation(com.google.firestore.v1.Write mutation) {
-    Precondition precondition = mutation.hasCurrentDocument()
-        ? decodePrecondition(mutation.getCurrentDocument())
-        : Precondition.NONE;
+    Precondition precondition =
+        mutation.hasCurrentDocument()
+            ? decodePrecondition(mutation.getCurrentDocument())
+            : Precondition.NONE;
 
     List<FieldTransform> fieldTransforms = new ArrayList<>();
     for (DocumentTransform.FieldTransform fieldTransform : mutation.getUpdateTransformsList()) {
@@ -339,7 +331,8 @@ public Mutation decodeMutation(com.google.firestore.v1.Write mutation) {
 
   private com.google.firestore.v1.Precondition encodePrecondition(Precondition precondition) {
     hardAssert(!precondition.isNone(), "Can't serialize an empty precondition");
-    com.google.firestore.v1.Precondition.Builder builder = com.google.firestore.v1.Precondition.newBuilder();
+    com.google.firestore.v1.Precondition.Builder builder =
+        com.google.firestore.v1.Precondition.newBuilder();
     if (precondition.getUpdateTime() != null) {
       return builder.setUpdateTime(encodeVersion(precondition.getUpdateTime())).build();
     } else if (precondition.getExists() != null) {
@@ -399,7 +392,8 @@ private DocumentTransform.FieldTransform encodeFieldTransform(FieldTransform fie
           .setRemoveAllFromArray(ArrayValue.newBuilder().addAllValues(remove.getElements()))
           .build();
     } else if (transform instanceof NumericIncrementTransformOperation) {
-      NumericIncrementTransformOperation incrementOperation = (NumericIncrementTransformOperation) transform;
+      NumericIncrementTransformOperation incrementOperation =
+          (NumericIncrementTransformOperation) transform;
       return DocumentTransform.FieldTransform.newBuilder()
           .setFieldPath(fieldTransform.getFieldPath().canonicalString())
           .setIncrement(incrementOperation.getOperand())
@@ -413,7 +407,8 @@ private FieldTransform decodeFieldTransform(DocumentTransform.FieldTransform fie
     switch (fieldTransform.getTransformTypeCase()) {
       case SET_TO_SERVER_VALUE:
         hardAssert(
-            fieldTransform.getSetToServerValue() == DocumentTransform.FieldTransform.ServerValue.REQUEST_TIME,
+            fieldTransform.getSetToServerValue()
+                == DocumentTransform.FieldTransform.ServerValue.REQUEST_TIME,
             "Unknown transform setToServerValue: %s",
             fieldTransform.getSetToServerValue());
         return new FieldTransform(
@@ -464,8 +459,7 @@ public MutationResult decodeMutationResult(
 
   @Nullable
   public Map<String, String> encodeListenRequestLabels(TargetData targetData) {
-    @Nullable
-    String value = encodeLabel(targetData.getPurpose());
+    @Nullable String value = encodeLabel(targetData.getPurpose());
     if (value == null) {
       return null;
     }
@@ -667,7 +661,8 @@ private List<Filter> decodeFilters(StructuredQuery.Filter proto) {
     // containing F1,
     // F2, ... to stay consistent with the older SDK versions.
     if (result instanceof com.google.firebase.firestore.core.CompositeFilter) {
-      com.google.firebase.firestore.core.CompositeFilter compositeFilter = (com.google.firebase.firestore.core.CompositeFilter) result;
+      com.google.firebase.firestore.core.CompositeFilter compositeFilter =
+          (com.google.firebase.firestore.core.CompositeFilter) result;
       if (compositeFilter.isFlatConjunction()) {
         return compositeFilter.getFilters();
       }
@@ -925,8 +920,9 @@ public WatchChange decodeWatchChange(ListenResponse protoChange) {
           default:
             throw new IllegalArgumentException("Unknown target change type");
         }
-        watchChange = new WatchTargetChange(
-            changeType, targetChange.getTargetIdsList(), targetChange.getResumeToken(), cause);
+        watchChange =
+            new WatchTargetChange(
+                changeType, targetChange.getTargetIdsList(), targetChange.getResumeToken(), cause);
         break;
       case DOCUMENT_CHANGE:
         DocumentChange docChange = protoChange.getDocumentChange();
@@ -947,7 +943,8 @@ public WatchChange decodeWatchChange(ListenResponse protoChange) {
         // Note that version might be unset in which case we use SnapshotVersion.NONE
         version = decodeVersion(docDelete.getReadTime());
         MutableDocument doc = MutableDocument.newNoDocument(key, version);
-        watchChange = new WatchChange.DocumentChange(Collections.emptyList(), removed, doc.getKey(), doc);
+        watchChange =
+            new WatchChange.DocumentChange(Collections.emptyList(), removed, doc.getKey(), doc);
         break;
       case DOCUMENT_REMOVE:
         DocumentRemove docRemove = protoChange.getDocumentRemove();
@@ -957,7 +954,8 @@ public WatchChange decodeWatchChange(ListenResponse protoChange) {
         break;
       case FILTER:
         com.google.firestore.v1.ExistenceFilter protoFilter = protoChange.getFilter();
-        ExistenceFilter filter = new ExistenceFilter(protoFilter.getCount(), protoFilter.getUnchangedNames());
+        ExistenceFilter filter =
+            new ExistenceFilter(protoFilter.getCount(), protoFilter.getUnchangedNames());
         int targetId = protoFilter.getTargetId();
         watchChange = new ExistenceFilterWatchChange(targetId, filter);
         break;

firebase-firestore/src/main/java/com/google/firebase/firestore/remote/WatchChangeAggregator.java

@@ -83,6 +83,13 @@ public interface TargetMetadataProvider {
   /** The log tag to use for this class. */
   private static final String LOG_TAG = "WatchChangeAggregator";
 
+  /** The bloom filter application status while handling existence filter mismatch. */
+  private enum BloomFilterApplicationStatus {
+    SUCCESS,
+    SKIPPED,
+    FALSE_POSITIVE
+  }
+
   public WatchChangeAggregator(TargetMetadataProvider targetMetadataProvider) {
     this.targetMetadataProvider = targetMetadataProvider;
   }
@@ -208,27 +215,34 @@ public void handleExistenceFilter(ExistenceFilterWatchChange watchChange) {
         if (currentSize != expectedCount) {
 
           // Apply bloom filter to identify and mark removed documents.
-          boolean bloomFilterApplied = this.applyBloomFilter(watchChange, currentSize);
+          BloomFilterApplicationStatus status = this.applyBloomFilter(watchChange, currentSize);
 
-          if (!bloomFilterApplied) {
+          if (status != BloomFilterApplicationStatus.SUCCESS) {
             // If bloom filter application fails, we reset the mapping and
             // trigger re-run of the query.
             resetTarget(targetId);
-            pendingTargetResets.put(targetId, QueryPurpose.EXISTENCE_FILTER_MISMATCH);
+
+            QueryPurpose purpose =
+                status == BloomFilterApplicationStatus.FALSE_POSITIVE
+                    ? QueryPurpose.EXISTENCE_FILTER_MISMATCH_BLOOM
+                    : QueryPurpose.EXISTENCE_FILTER_MISMATCH;
+
+            pendingTargetResets.put(targetId, purpose);
           }
         }
       }
     }
   }
 
-  /** Returns whether a bloom filter removed the deleted documents successfully. */
-  private boolean applyBloomFilter(ExistenceFilterWatchChange watchChange, int currentCount) {
+  /** Apply bloom filter to remove the deleted documents, and return the application status. */
+  private BloomFilterApplicationStatus applyBloomFilter(
+      ExistenceFilterWatchChange watchChange, int currentCount) {
     int expectedCount = watchChange.getExistenceFilter().getCount();
     com.google.firestore.v1.BloomFilter unchangedNames =
         watchChange.getExistenceFilter().getUnchangedNames();
 
     if (unchangedNames == null || !unchangedNames.hasBits()) {
-      return false;
+      return BloomFilterApplicationStatus.SKIPPED;
     }
 
     byte[] bitmap = unchangedNames.getBits().getBitmap().toByteArray();
@@ -244,12 +258,20 @@ private boolean applyBloomFilter(ExistenceFilterWatchChange watchChange, int cur
           "Decoding the base64 bloom filter in existence filter failed ("
               + e.getMessage()
               + "); ignoring the bloom filter and falling back to full re-query.");
-      return false;
+      return BloomFilterApplicationStatus.SKIPPED;
+    }
+
+    if (bloomFilter.getBitCount() == 0) {
+      return BloomFilterApplicationStatus.SKIPPED;
     }
 
     int removedDocumentCount = this.filterRemovedDocuments(bloomFilter, watchChange.getTargetId());
 
-    return expectedCount == (currentCount - removedDocumentCount);
+    if (expectedCount != (currentCount - removedDocumentCount)) {
+      return BloomFilterApplicationStatus.FALSE_POSITIVE;
+    }
+
+    return BloomFilterApplicationStatus.SUCCESS;
   }
 
   /**

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

@@ -1166,7 +1166,7 @@
                 }
               ],
               "resumeToken": "",
-              "targetPurpose": 2
+              "targetPurpose": 3
             },
             "2": {
               "queries": [