Merge branch 'main' into feature/enable-tvos-packaging

vimanyu · vimanyu · commit 2d9cc0f02daa · 2021-07-21T09:23:51.000-07:00
diff --git a/.github/workflows/integration_tests.yml b/.github/workflows/integration_tests.yml
@@ -175,16 +175,20 @@ jobs:
           else
             # If running in a PR, diff against the PR's base.
             # "git merge-base main branch_name" will give the common ancestor of both branches.
-            MERGE_BASE=$(git merge-base origin/${{github.event.pull_request.head.ref}} origin/${{github.event.pull_request.base.ref}})
-            echo "::warning ::Auto-diff origin/${{github.event.pull_request.head.ref}}..${MERGE_BASE}"
-            git diff --name-only origin/${{github.event.pull_request.head.ref}}..${MERGE_BASE}
-            echo "AUTO_DIFF_PARAM=--auto_diff origin/${{github.event.pull_request.head.ref}}..${MERGE_BASE}" >> $GITHUB_ENV
+            MERGE_BASE=$(git merge-base origin/${{github.event.pull_request.head.ref}} origin/${{github.event.pull_request.base.ref}} || true)
+            # If origin/<branch> is no longer valid, then just run all tests.
+            if [[ -n "${MERGE_BASE}" ]]; then
+              echo "::warning ::Auto-diff origin/${{github.event.pull_request.head.ref}}..${MERGE_BASE}"
+              git diff --name-only origin/${{github.event.pull_request.head.ref}}..${MERGE_BASE}
+              echo "AUTO_DIFF_PARAM=--auto_diff origin/${{github.event.pull_request.head.ref}}..${MERGE_BASE}" >> $GITHUB_ENV
+            fi
           fi
       - id: export-result
         # e.g. 'ubuntu-latest,macos-latest' -> '["ubuntu-latest","macos-latest"]'
         run: |
-          echo "::set-output name=apis::$( python scripts/gha/print_matrix_configuration.py -c -w integration_tests -k apis -o "${{github.event.inputs.apis}}" ${AUTO_DIFF_PARAM})"
-          echo "::set-output name=matrix_platform::$( python scripts/gha/print_matrix_configuration.py -w integration_tests ${EXPANDED_MATRIX_PARAM} -k platform -o "${{github.event.inputs.platforms}}" ${AUTO_DIFF_PARAM})"
+          apis=$( python scripts/gha/print_matrix_configuration.py -c -w integration_tests -k apis -o "${{github.event.inputs.apis}}" ${AUTO_DIFF_PARAM})
+          echo "::set-output name=apis::${apis}"
+          echo "::set-output name=matrix_platform::$( python scripts/gha/print_matrix_configuration.py -w integration_tests ${EXPANDED_MATRIX_PARAM} -k platform -o "${{github.event.inputs.platforms}}" --apis ${apis} ${AUTO_DIFF_PARAM})"
           echo "::set-output name=matrix_os::$( python scripts/gha/print_matrix_configuration.py -w integration_tests ${EXPANDED_MATRIX_PARAM} -k os -o "${{github.event.inputs.operating_systems}}" ${AUTO_DIFF_PARAM})"
           # If building against a packaged SDK, only use boringssl.
           if [[ -n "${{ github.event.inputs.test_packaged_sdk }}" ]]; then
@@ -808,6 +812,7 @@ jobs:
         run: |
           echo "::set-output name=device_type::$( python scripts/gha/print_matrix_configuration.py -d -k ${{ matrix.android_device }} )"
       - name: Run Android integration tests on Emulator locally
+        timeout-minutes: 60
         if: steps.get-device-type.outputs.device_type == 'virtual'
         run: |
           python scripts/gha/test_simulator.py --testapp_dir testapps \
@@ -892,6 +897,7 @@ jobs:
         run: |
           echo "::set-output name=device_type::$( python scripts/gha/print_matrix_configuration.py -d -k ${{ matrix.ios_device }} )"
       - name: Run iOS integration tests on Simulator locally
+        timeout-minutes: 60
         if: steps.get-device-type.outputs.device_type == 'virtual'
         run: |
           python scripts/gha/test_simulator.py --testapp_dir testapps \
@@ -973,6 +979,7 @@ jobs:
       - name: Install python deps
         run: pip install -r scripts/gha/requirements.txt
       - name: Run tvOS integration tests on Simulator locally
+        timeout-minutes: 60
         run: |
           python scripts/gha/test_simulator.py --testapp_dir testapps \
             --tvos_device "${{ matrix.tvos_device }}" \
diff --git a/firestore/CMakeLists.txt b/firestore/CMakeLists.txt
@@ -342,6 +342,8 @@ add_custom_target(
   COMMENT "Copying internal Firestore headers"
 )
 
+# This is needed due to Firestore's dependence on some firebase::app APIs that
+# are guarded by this flag, such as GetUserAgent and function_registry.
 set(FIREBASE_FIRESTORE_CPP_DEFINES -DINTERNAL_EXPERIMENTAL=1)
 
 if (WIN32 AND NOT ANDROID AND NOT IOS)
diff --git a/firestore/integration_test_internal/src/bundle_test.cc b/firestore/integration_test_internal/src/bundle_test.cc
@@ -65,6 +65,15 @@ std::string CreateTestBundle(Firestore* db) {
   return CreateBundle(db->app()->options().project_id());
 }
 
+void SetPromiseValueWhenUpdateIsFinal(
+    LoadBundleTaskProgress progress,
+    std::promise<void>& final_update_received) {
+  if (progress.state() == LoadBundleTaskProgress::State::kError ||
+      progress.state() == LoadBundleTaskProgress::State::kSuccess) {
+    final_update_received.set_value();
+  }
+}
+
 class BundleTest : public FirestoreIntegrationTest {
  protected:
   void SetUp() override {
@@ -133,15 +142,19 @@ TEST_F(BundleTest, CanLoadBundlesWithProgressUpdates) {
   auto bundle = CreateTestBundle(db);
 
   std::vector<LoadBundleTaskProgress> progresses;
+  std::promise<void> final_update;
   Future<LoadBundleTaskProgress> result = db->LoadBundle(
-      bundle, [&progresses](const LoadBundleTaskProgress& progress) {
+      bundle,
+      [&progresses, &final_update](const LoadBundleTaskProgress& progress) {
         progresses.push_back(progress);
+        SetPromiseValueWhenUpdateIsFinal(progress, final_update);
       });
 
   auto final_progress = AwaitResult(result);
 
   // 4 progresses will be reported: initial, document 1, document 2, final
   // success.
+  final_update.get_future().wait();
   ASSERT_EQ(progresses.size(), 4);
   EXPECT_THAT(progresses[0], InProgressWithLoadedDocuments(0));
   EXPECT_THAT(progresses[1], InProgressWithLoadedDocuments(1));
@@ -195,12 +208,16 @@ TEST_F(BundleTest, LoadBundlesForASecondTimeSkips) {
   VerifySuccessProgress(first_load);
 
   std::vector<LoadBundleTaskProgress> progresses;
+  std::promise<void> final_update;
   LoadBundleTaskProgress second_load = AwaitResult(db->LoadBundle(
-      bundle, [&progresses](const LoadBundleTaskProgress& progress) {
+      bundle,
+      [&progresses, &final_update](const LoadBundleTaskProgress& progress) {
         progresses.push_back(progress);
+        SetPromiseValueWhenUpdateIsFinal(progress, final_update);
       }));
 
   // There will be 4 progress updates if it does not skip loading.
+  final_update.get_future().wait();
   ASSERT_EQ(progresses.size(), 1);
   VerifySuccessProgress(progresses[0]);
   EXPECT_EQ(progresses[0], second_load);
@@ -220,13 +237,18 @@ TEST_F(BundleTest, LoadInvalidBundlesShouldFail) {
   };
   for (const auto& bundle : invalid_bundles) {
     std::vector<LoadBundleTaskProgress> progresses;
+    std::promise<void> final_update;
     Future<LoadBundleTaskProgress> result = db->LoadBundle(
-        bundle, [&progresses](const LoadBundleTaskProgress& progress) {
+        bundle,
+        [&progresses, &final_update](const LoadBundleTaskProgress& progress) {
           progresses.push_back(progress);
+          SetPromiseValueWhenUpdateIsFinal(progress, final_update);
         });
-    Await(result);
 
+    Await(result);
     EXPECT_NE(result.error(), Error::kErrorOk);
+
+    final_update.get_future().wait();
     ASSERT_EQ(progresses.size(), 1);
     VerifyErrorProgress(progresses[0]);
   }
@@ -306,13 +328,17 @@ TEST_F(BundleTest, LoadDocumentsFromOtherProjectsShouldFail) {
   Firestore* db = TestFirestore();
   auto bundle = CreateBundle("other-project");
   std::vector<LoadBundleTaskProgress> progresses;
+  std::promise<void> final_update;
   Future<LoadBundleTaskProgress> result = db->LoadBundle(
-      bundle, [&progresses](const LoadBundleTaskProgress& progress) {
+      bundle,
+      [&progresses, &final_update](const LoadBundleTaskProgress& progress) {
         progresses.push_back(progress);
+        SetPromiseValueWhenUpdateIsFinal(progress, final_update);
       });
   Await(result);
-
   EXPECT_NE(result.error(), Error::kErrorOk);
+
+  final_update.get_future().wait();
   ASSERT_EQ(progresses.size(), 2);
   EXPECT_THAT(progresses[0], InProgressWithLoadedDocuments(0));
   VerifyErrorProgress(progresses[1]);
diff --git a/firestore/integration_test_internal/src/util/integration_test_util.cc b/firestore/integration_test_internal/src/util/integration_test_util.cc
@@ -9,7 +9,12 @@
 #include "firebase/app.h"
 #include "firebase/firestore.h"
 #include "firestore/src/common/hard_assert_common.h"
+
+#if !defined(__ANDROID__)
 #include "firestore/src/main/firestore_main.h"
+#else
+#include "firestore/src/android/firestore_android.h"
+#endif  // !defined(__ANDROID__)
 
 namespace firebase {
 namespace firestore {
diff --git a/firestore/src/android/firestore_android.cc b/firestore/src/android/firestore_android.cc
@@ -440,25 +440,18 @@ WriteBatch FirestoreInternal::batch() const {
   return WriteBatch(new WriteBatchInternal(mutable_this(), result));
 }
 
-Future<void> FirestoreInternal::RunTransaction(TransactionFunction* update,
-                                               bool is_lambda) {
+Future<void> FirestoreInternal::RunTransaction(
+    std::function<Error(Transaction&, std::string&)> update) {
+  auto* lambda_update = new LambdaTransactionFunction(Move(update));
   Env env = GetEnv();
   Local<Object> transaction_function =
-      TransactionInternal::Create(env, this, update);
+      TransactionInternal::Create(env, this, lambda_update);
   Local<Task> task = env.Call(obj_, kRunTransaction, transaction_function);
 
   if (!env.ok()) return {};
 
-  auto* completion =
-      static_cast<LambdaTransactionFunction*>(is_lambda ? update : nullptr);
   return promises_->NewFuture<void>(env, AsyncFn::kRunTransaction, task,
-                                    completion);
-}
-
-Future<void> FirestoreInternal::RunTransaction(
-    std::function<Error(Transaction&, std::string&)> update) {
-  auto* lambda_update = new LambdaTransactionFunction(Move(update));
-  return RunTransaction(lambda_update, /*is_lambda=*/true);
+                                    lambda_update);
 }
 
 Future<void> FirestoreInternal::DisableNetwork() {
diff --git a/firestore/src/android/firestore_android.h b/firestore/src/android/firestore_android.h
@@ -29,7 +29,6 @@ namespace firestore {
 class Firestore;
 class ListenerRegistrationInternal;
 class Transaction;
-class TransactionFunction;
 class WriteBatch;
 
 template <typename EnumT>
@@ -93,11 +92,6 @@ class FirestoreInternal {
 
   WriteBatch batch() const;
 
-  // Runs transaction atomically.
-  // TODO(b/191969448): Remove the is_lambda parameter if possible.
-  Future<void> RunTransaction(TransactionFunction* update,
-                              bool is_lambda = false);
-
   Future<void> RunTransaction(
       std::function<Error(Transaction&, std::string&)> update);
 
diff --git a/firestore/src/android/lambda_transaction_function.h b/firestore/src/android/lambda_transaction_function.h
@@ -10,6 +10,7 @@
 #include "app/src/assert.h"
 #include "firestore/src/android/firestore_android.h"
 #include "firestore/src/android/promise_android.h"
+#include "firestore/src/common/transaction_function.h"
 #include "firestore/src/include/firebase/firestore/transaction.h"
 
 namespace firebase {
diff --git a/firestore/src/android/transaction_android.h b/firestore/src/android/transaction_android.h
@@ -9,6 +9,7 @@
 #include "app/meta/move.h"
 #include "app/src/embedded_file.h"
 #include "firestore/src/android/wrapper.h"
+#include "firestore/src/common/transaction_function.h"
 #include "firestore/src/include/firebase/firestore/document_reference.h"
 #include "firestore/src/include/firebase/firestore/field_value.h"
 #include "firestore/src/include/firebase/firestore/map_field_value.h"
diff --git a/firestore/src/common/firestore.cc b/firestore/src/common/firestore.cc
@@ -260,11 +260,6 @@ WriteBatch Firestore::batch() const {
   return internal_->batch();
 }
 
-Future<void> Firestore::RunTransaction(TransactionFunction* update) {
-  if (!internal_) return FailedFuture<void>();
-  return internal_->RunTransaction(update);
-}
-
 Future<void> Firestore::RunTransaction(
     std::function<Error(Transaction&, std::string&)> update) {
   FIREBASE_ASSERT_MESSAGE(update, "invalid update parameter is passed in.");
diff --git a/firestore/src/common/transaction_function.h b/firestore/src/common/transaction_function.h
@@ -0,0 +1,37 @@
+// Copyright 2021 Google LLC
+
+#ifndef FIREBASE_FIRESTORE_SRC_COMMON_TRANSACTION_FUNCTION_H_
+#define FIREBASE_FIRESTORE_SRC_COMMON_TRANSACTION_FUNCTION_H_
+
+#include <string>
+
+#include "firebase/firestore/firestore_errors.h"
+
+namespace firebase {
+namespace firestore {
+
+class Transaction;
+
+/**
+ * An interface for providing code to be executed within a transaction
+ * context.
+ */
+class TransactionFunction {
+ public:
+  virtual ~TransactionFunction() {}
+
+  /**
+   * Subclass should override this method and put the transaction logic here.
+   *
+   * @param[in] transaction The transaction to run this function with.
+   * @param[out] error_message You can set error message with this parameter.
+   * @return Either Error::kErrorOk if successful or the error code from Error
+   * that most closely matches the failure.
+   */
+  virtual Error Apply(Transaction& transaction, std::string& error_message) = 0;
+};
+
+}  // namespace firestore
+}  // namespace firebase
+
+#endif  // FIREBASE_FIRESTORE_SRC_COMMON_TRANSACTION_FUNCTION_H_
diff --git a/firestore/src/include/firebase/firestore.h b/firestore/src/include/firebase/firestore.h
@@ -253,26 +253,6 @@ class Firestore {
   virtual Future<void> RunTransaction(
       std::function<Error(Transaction&, std::string&)> update);
 
-  /**
-   * Executes the given update and then attempts to commit the changes applied
-   * within the transaction. If any document read within the transaction has
-   * changed, the update function will be retried. If it fails to commit after 5
-   * attempts, the transaction will fail.
-   *
-   * @param update The function to execute within the transaction context.
-   * (Ownership is not transferred; you are responsible for making sure that
-   * transaction is valid as long as the Future has not yet completed.)
-   *
-   * @return A Future that will be resolved when the transaction finishes.
-   *
-   * @deprecated This method was originally added to support the STLPort C++
-   * runtime on Android. Firebase support for STLPort has been removed, and
-   * consequently this method is deprecated and will be removed in a future
-   * release.
-   */
-  FIREBASE_DEPRECATED virtual Future<void> RunTransaction(
-      TransactionFunction* update);
-
   /**
    * Sets the log verbosity of all Firestore instances.
    *
diff --git a/firestore/src/include/firebase/firestore/field_value.h b/firestore/src/include/firebase/firestore/field_value.h
@@ -325,7 +325,6 @@ class FieldValue final {
    */
   static FieldValue ArrayRemove(std::vector<FieldValue> elements);
 
-#if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
   /**
    * Returns a special value that can be used with `Set()` or `Update()` that
    * tells the server to increment the field's current value by the given
@@ -355,9 +354,6 @@ class FieldValue final {
     return IntegerIncrement(static_cast<int64_t>(by_value));
   }
 
-#endif  // if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
-
-#if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
   /**
    * Returns a special value that can be used with `Set()` or `Update()` that
    * tells the server to increment the field's current value by the given
@@ -388,8 +384,6 @@ class FieldValue final {
     return DoubleIncrement(static_cast<double>(by_value));
   }
 
-#endif  // if defined(INTERNAL_EXPERIMENTAL) || defined(SWIG)
-
   /**
    * Returns a string representation of this `FieldValue` for logging/debugging
    * purposes.
diff --git a/firestore/src/include/firebase/firestore/transaction.h b/firestore/src/include/firebase/firestore/transaction.h
@@ -137,27 +137,6 @@ class Transaction {
   mutable TransactionInternal* internal_ = nullptr;
 };
 
-/**
- * An interface for providing code to be executed within a transaction
- * context.
- *
- * @see Firestore::RunTransaction(TransactionFunction*)
- */
-class TransactionFunction {
- public:
-  virtual ~TransactionFunction() {}
-
-  /**
-   * Subclass should override this method and put the transaction logic here.
-   *
-   * @param[in] transaction The transaction to run this function with.
-   * @param[out] error_message You can set error message with this parameter.
-   * @return Either Error::kErrorOk if successful or the error code from Error
-   * that most closely matches the failure.
-   */
-  virtual Error Apply(Transaction& transaction, std::string& error_message) = 0;
-};
-
 }  // namespace firestore
 }  // namespace firebase
 
diff --git a/firestore/src/main/firestore_main.cc b/firestore/src/main/firestore_main.cc
@@ -151,13 +151,6 @@ WriteBatch FirestoreInternal::batch() const {
   return MakePublic(firestore_core_->GetBatch());
 }
 
-Future<void> FirestoreInternal::RunTransaction(TransactionFunction* update) {
-  return RunTransaction(
-      [update](Transaction& transaction, std::string& error_message) {
-        return update->Apply(transaction, error_message);
-      });
-}
-
 Future<void> FirestoreInternal::RunTransaction(
     std::function<Error(Transaction&, std::string&)> update) {
   auto executor = transaction_executor_;
diff --git a/firestore/src/main/firestore_main.h b/firestore/src/main/firestore_main.h
@@ -28,7 +28,6 @@ namespace firestore {
 class Firestore;
 class ListenerRegistrationInternal;
 class Transaction;
-class TransactionFunction;
 class WriteBatch;
 
 namespace util {
@@ -69,7 +68,6 @@ class FirestoreInternal {
 
   Future<void> RunTransaction(
       std::function<Error(Transaction&, std::string&)> update);
-  Future<void> RunTransaction(TransactionFunction* update);
 
   Future<void> DisableNetwork();
 
diff --git a/messaging/integration_test/integration_test.xcodeproj/project.pbxproj b/messaging/integration_test/integration_test.xcodeproj/project.pbxproj
diff --git a/messaging/integration_test/src/integration_test.cc b/messaging/integration_test/src/integration_test.cc
diff --git a/release_build_files/readme.md b/release_build_files/readme.md
diff --git a/scripts/gha/build_ios_tvos.py b/scripts/gha/build_ios_tvos.py
diff --git a/scripts/gha/print_matrix_configuration.py b/scripts/gha/print_matrix_configuration.py
diff --git a/scripts/gha/test_simulator.py b/scripts/gha/test_simulator.py

Original file line number	Diff line number	Diff line change
`@@ -342,6 +342,8 @@ add_custom_target(`
`342`	`342`	`COMMENT "Copying internal Firestore headers"`
`343`	`343`	`)`
`344`	`344`
	`345`	`+# This is needed due to Firestore's dependence on some firebase::app APIs that`
	`346`	`+# are guarded by this flag, such as GetUserAgent and function_registry.`
`345`	`347`	`set(FIREBASE_FIRESTORE_CPP_DEFINES -DINTERNAL_EXPERIMENTAL=1)`
`346`	`348`
`347`	`349`	`if (WIN32 AND NOT ANDROID AND NOT IOS)`