FIRQueryTests.mm: improve the test that resumes a query with existence filter to actually validate the existence filter

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

@@ -22,6 +22,8 @@
 #import "Firestore/Example/Tests/Util/FSTHelpers.h"
 #import "Firestore/Example/Tests/Util/FSTIntegrationTestCase.h"
 
+#include "Firestore/core/test/unit/testutil/testing_hooks_util.h"
+
 namespace {
 
 NSArray<NSString *> *SortedStringsNotIn(NSSet<NSString *> *set, NSSet<NSString *> *remove) {
@@ -1191,6 +1193,10 @@ - (void)testOrderByEquality {
 }
 
 - (void)testResumingAQueryShouldUseExistenceFilterToDetectDeletes {
+  // Set this test to stop when the first failure occurs because some test assertion failures make
+  // the rest of the test not applicable or will even crash.
+  [self setContinueAfterFailure:NO];
+
   // Prepare the names and contents of the 100 documents to create.
   NSMutableDictionary<NSString *, NSDictionary<NSString *, id> *> *testDocs =
       [[NSMutableDictionary alloc] init];
@@ -1236,8 +1242,13 @@ - (void)testResumingAQueryShouldUseExistenceFilterToDetectDeletes {
   [NSThread sleepForTimeInterval:10.0f];
 
   // Resume the query and save the resulting snapshot for verification.
-  FIRQuerySnapshot *querySnapshot2 = [self readDocumentSetForRef:collRef
-                                                          source:FIRFirestoreSourceDefault];
+  // Use some internal testing hooks to "capture" the existence filter mismatches to verify them.
+  FIRQuerySnapshot *querySnapshot2;
+  std::vector<firebase::firestore::util::TestingHooks::ExistenceFilterMismatchInfo>
+      existence_filter_mismatches =
+          firebase::firestore::testutil::CaptureExistenceFilterMismatches([&] {
+            querySnapshot2 = [self readDocumentSetForRef:collRef source:FIRFirestoreSourceDefault];
+          });
 
   // Verify that the snapshot from the resumed query contains the expected documents; that is,
   // that it contains the 50 documents that were _not_ deleted.
@@ -1272,6 +1283,21 @@ - (void)testResumingAQueryShouldUseExistenceFilterToDetectDeletes {
               [missingDocumentIds componentsJoinedByString:@", "]);
     }
   }
+
+  // Skip the verification of the existence filter mismatch when testing against the Firestore
+  // emulator because the Firestore emulator fails to to send an existence filter at all.
+  // TODO(b/270731363): Enable the verification of the existence filter mismatch once the Firestore
+  // emulator is fixed to send an existence filter.
+  if ([FSTIntegrationTestCase isRunningAgainstEmulator]) {
+    return;
+  }
+
+  // Verify that Watch sent an existence filter with the correct counts when the query was resumed.
+  XCTAssertEqual(static_cast<int>(existence_filter_mismatches.size()), 1);
+  firebase::firestore::util::TestingHooks::ExistenceFilterMismatchInfo &info =
+      existence_filter_mismatches[0];
+  XCTAssertEqual(info.local_cache_count, 100);
+  XCTAssertEqual(info.existence_filter_count, 50);
 }
 
 @end

Firestore/core/src/remote/remote_event.cc

@@ -19,6 +19,7 @@
 #include <utility>
 
 #include "Firestore/core/src/local/target_data.h"
+#include "Firestore/core/src/util/testing_hooks.h"
 
 namespace firebase {
 namespace firestore {
@@ -34,6 +35,7 @@ using model::MutableDocument;
 using model::SnapshotVersion;
 using model::TargetId;
 using nanopb::ByteString;
+using util::TestingHooks;
 
 // TargetChange
 
@@ -239,6 +241,8 @@ void WatchChangeAggregator::HandleExistenceFilter(
         // snapshot with `isFromCache:true`.
         ResetTarget(target_id);
         pending_target_resets_.insert(target_id);
+        TestingHooks::GetInstance().NotifyOnExistenceFilterMismatch(
+            {current_size, expected_count});
       }
     }
   }

Firestore/core/src/util/testing_hooks.cc

@@ -0,0 +1,119 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file 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.
+ */
+
+#include "Firestore/core/src/util/testing_hooks.h"
+
+#include <functional>
+#include <mutex>  // NOLINT(build/c++11)
+#include <type_traits>
+#include <unordered_map>
+#include <utility>
+#include <vector>
+
+#include "Firestore/core/src/util/no_destructor.h"
+
+namespace firebase {
+namespace firestore {
+namespace util {
+
+namespace {
+
+/**
+ * An implementation of `ListenerRegistration` whose `Remove()` method simply
+ * invokes the function specified to the constructor. This allows easily
+ * creating `ListenerRegistration` objects that call a lambda.
+ */
+class RemoveDelegatingListenerRegistration final
+    : public api::ListenerRegistration {
+ public:
+  RemoveDelegatingListenerRegistration(std::function<void()> delegate)
+      : delegate_(std::move(delegate)) {
+  }
+
+  void Remove() override {
+    delegate_();
+  }
+
+ private:
+  std::function<void()> delegate_;
+};
+
+}  // namespace
+
+/** Returns the singleton instance of this class. */
+TestingHooks& TestingHooks::GetInstance() {
+  static NoDestructor<TestingHooks> instance;
+  return *instance;
+}
+
+std::shared_ptr<api::ListenerRegistration>
+TestingHooks::OnExistenceFilterMismatch(
+    ExistenceFilterMismatchCallback callback) {
+  // Register the callback.
+  std::unique_lock<std::mutex> lock(mutex_);
+  const int id = next_id_++;
+  existence_filter_mismatch_callbacks_.insert(
+      {id,
+       std::make_shared<ExistenceFilterMismatchCallback>(std::move(callback))});
+  lock.unlock();
+
+  // NOTE: Capturing `this` in the lambda below is safe because the destructor
+  // is deleted and, therefore, `this` can never be deleted. The static_assert
+  // statements below verify this invariant.
+  using this_type = std::remove_pointer<decltype(this)>::type;
+  static_assert(std::is_same<this_type, TestingHooks>::value, "");
+  static_assert(!std::is_destructible<this_type>::value, "");
+
+  // Create a ListenerRegistration that the caller can use to unregister the
+  // callback.
+  return std::make_shared<RemoveDelegatingListenerRegistration>([this, id]() {
+    std::lock_guard<std::mutex> lock(mutex_);
+    auto iter = existence_filter_mismatch_callbacks_.find(id);
+    if (iter != existence_filter_mismatch_callbacks_.end()) {
+      existence_filter_mismatch_callbacks_.erase(iter);
+    }
+  });
+}
+
+void TestingHooks::NotifyOnExistenceFilterMismatch(
+    const ExistenceFilterMismatchInfo& info) {
+  std::unique_lock<std::mutex> lock(mutex_);
+
+  // Short-circuit to avoid any unnecessary work if there is nothing to do.
+  if (existence_filter_mismatch_callbacks_.empty()) {
+    return;
+  }
+
+  // Copy the callbacks into a vector so that they can be invoked after
+  // releasing the lock.
+  std::vector<std::shared_ptr<ExistenceFilterMismatchCallback>> callbacks;
+  for (auto&& entry : existence_filter_mismatch_callbacks_) {
+    callbacks.push_back(entry.second);
+  }
+
+  // Release the lock so that the callback invocations are done _without_
+  // holding the lock. This avoids deadlock in the case that invocations are
+  // re-entrant.
+  lock.unlock();
+
+  for (std::shared_ptr<ExistenceFilterMismatchCallback> callback : callbacks) {
+    callback->operator()(info);
+  }
+}
+
+}  // namespace util
+}  // namespace firestore
+}  // namespace firebase

Firestore/core/src/util/testing_hooks.h

@@ -0,0 +1,121 @@
+/*
+ * Copyright 2023 Google LLC
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file 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.
+ */
+
+#ifndef FIRESTORE_CORE_SRC_UTIL_TESTING_HOOKS_H_
+#define FIRESTORE_CORE_SRC_UTIL_TESTING_HOOKS_H_
+
+#include <functional>
+#include <memory>
+#include <mutex>  // NOLINT(build/c++11)
+#include <unordered_map>
+
+#include "Firestore/core/src/api/listener_registration.h"
+#include "Firestore/core/src/util/no_destructor.h"
+
+namespace firebase {
+namespace firestore {
+namespace util {
+
+/**
+ * Manages "testing hooks", hooks into the internals of the SDK to verify
+ * internal state and events during integration tests. Do not use this class
+ * except for testing purposes.
+ */
+class TestingHooks final {
+ public:
+  /** Returns the singleton instance of this class. */
+  static TestingHooks& GetInstance();
+
+  /**
+   * Information about an existence filter mismatch, as specified to callbacks
+   * registered with `OnExistenceFilterMismatch()`.
+   */
+  struct ExistenceFilterMismatchInfo {
+    /** The number of documents that matched the query in the local cache. */
+    int local_cache_count = -1;
+
+    /**
+     * The number of documents that matched the query on the server, as
+     * specified in the `ExistenceFilter` message's `count` field.
+     */
+    int existence_filter_count = -1;
+  };
+
+  using ExistenceFilterMismatchCallback =
+      std::function<void(const TestingHooks::ExistenceFilterMismatchInfo&)>;
+
+  /**
+   * Registers a callback to be invoked when an existence filter mismatch occurs
+   * in the Watch listen stream.
+   *
+   * The relative order in which callbacks are notified is unspecified; do not
+   * rely on any particular ordering. If a given callback is registered multiple
+   * times then it will be notified multiple times, once per registration.
+   *
+   * The listener callbacks are performed synchronously in
+   * `NotifyOnExistenceFilterMismatch()`; therefore, listeners should perform
+   * their work as quickly as possible and return to avoid blocking any critical
+   * work. In particular, the listener callbacks should *not* block or perform
+   * long-running operations.
+   *
+   * The `ExistenceFilterMismatchInfo` reference specified to the callback is
+   * only valid during the lifetime of the callback. Once the callback returns
+   * then it must not use the given `ExistenceFilterMismatchInfo` reference
+   * again.
+   *
+   * @param callback the callback to invoke upon existence filter mismatch.
+   *
+   * @return an object whose `Remove()` member function unregisters the given
+   * callback; only the first invocation of `Remove()` does anything; all
+   * subsequent invocations do nothing. Note that due to inherent race
+   * conditions it is technically possible, although unlikely, that callbacks
+   * could still occur _after_ unregistering.
+   */
+  std::shared_ptr<api::ListenerRegistration> OnExistenceFilterMismatch(
+      ExistenceFilterMismatchCallback callback);
+
+  /**
+   * Invokes all currently-registered `OnExistenceFilterMismatch` callbacks
+   * synchronously.
+   * @param info Information about the existence filter mismatch.
+   */
+  void NotifyOnExistenceFilterMismatch(const ExistenceFilterMismatchInfo& info);
+
+ private:
+  TestingHooks() = default;
+
+  // Delete the destructor so that the singleton instance of this class can
+  // never be deleted.
+  ~TestingHooks() = delete;
+
+  TestingHooks(const TestingHooks&) = delete;
+  TestingHooks(TestingHooks&&) = delete;
+  TestingHooks& operator=(const TestingHooks&) = delete;
+  TestingHooks& operator=(TestingHooks&&) = delete;
+
+  friend class NoDestructor<TestingHooks>;
+
+  mutable std::mutex mutex_;
+  int next_id_ = 0;
+  std::unordered_map<int, std::shared_ptr<ExistenceFilterMismatchCallback>>
+      existence_filter_mismatch_callbacks_;
+};
+
+}  // namespace util
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIRESTORE_CORE_SRC_UTIL_TESTING_HOOKS_H_