Deprecate RunTransaction which takes a TransactionFunction. (#512)

* Deprecate RunTransaction which takes a TransactionFunction.

* Update comments.

* Don't use deprecated API in firestore TransactionTest

* Address code review comments.

Author: ehsannas

firestore/integration_test_internal/src/includes_test.cc

@@ -24,10 +24,6 @@ struct TestListener {
   void OnEvent(const int&, Error, const std::string&) {}
 };
 
-struct TestTransactionFunction : TransactionFunction {
-  Error Apply(Transaction&, std::string&) override { return Error::kErrorOk; }
-};
-
 }  // namespace
 
 // This test makes sure that all the objects in Firestore public API are
@@ -68,7 +64,6 @@ TEST_F(IncludesTest, TestIncludingFirestoreHeaderIsSufficient) {
       WriteBatch write_batch;
 
       TestListener test_listener;
-      TestTransactionFunction test_transaction_function;
 
       Timestamp timestamp;
       GeoPoint geo_point;

firestore/integration_test_internal/src/transaction_extra_test.cc

@@ -18,12 +18,6 @@
 namespace firebase {
 namespace firestore {
 
-// We will be using lambda in the test instead of defining a
-// TransactionFunction for each of the test case.
-//
-// We do have a TransactionFunction-version of the test
-// TestGetNonexistentDocumentThenCreate to test the non-lambda API.
-
 using TransactionExtraTest = FirestoreIntegrationTest;
 
 TEST_F(TransactionExtraTest,

firestore/integration_test_internal/src/transaction_test.cc

@@ -31,12 +31,6 @@ namespace firestore {
 
 using ::testing::HasSubstr;
 
-// We will be using lambda in the test instead of defining a
-// TransactionFunction for each of the test case.
-//
-// We do have a TransactionFunction-version of the test
-// TestGetNonexistentDocumentThenCreate to test the non-lambda API.
-
 class TransactionTest : public FirestoreIntegrationTest {
  protected:
   // We occasionally get transient error like "Could not reach Cloud Firestore
@@ -91,38 +85,25 @@ class TransactionTest : public FirestoreIntegrationTest {
   }
 };
 
-class TestTransactionFunction : public TransactionFunction {
- public:
-  TestTransactionFunction(DocumentReference doc) : doc_(doc) {}
-
-  Error Apply(Transaction& transaction, std::string& error_message) override {
-    Error error = Error::kErrorUnknown;
-    DocumentSnapshot snapshot = transaction.Get(doc_, &error, &error_message);
-    EXPECT_EQ(Error::kErrorOk, error);
-    EXPECT_FALSE(snapshot.exists());
-    transaction.Set(doc_, MapFieldValue{{key_, FieldValue::String(value_)}});
-    return error;
-  }
-
-  std::string key() { return key_; }
-  std::string value() { return value_; }
-
- private:
-  DocumentReference doc_;
-  const std::string key_{"foo"};
-  const std::string value_{"bar"};
-};
-
-TEST_F(TransactionTest, TestGetNonexistentDocumentThenCreatePortableVersion) {
+TEST_F(TransactionTest, TestGetNonexistentDocumentThenCreate) {
   DocumentReference doc = TestFirestore()->Collection("towns").Document();
-  TestTransactionFunction transaction{doc};
-  Future<void> future = TestFirestore()->RunTransaction(&transaction);
+  std::string key = "foo";
+  std::string value = "bar";
+  Future<void> future = TestFirestore()->RunTransaction(
+      [&](Transaction& transaction, std::string& error_message) {
+        Error error = Error::kErrorUnknown;
+        DocumentSnapshot snapshot =
+            transaction.Get(doc, &error, &error_message);
+        EXPECT_EQ(Error::kErrorOk, error);
+        EXPECT_FALSE(snapshot.exists());
+        transaction.Set(doc, MapFieldValue{{key, FieldValue::String(value)}});
+        return error;
+      });
   Await(future);
 
   EXPECT_EQ(Error::kErrorOk, future.error());
   DocumentSnapshot snapshot = ReadDocument(doc);
-  EXPECT_EQ(FieldValue::String(transaction.value()),
-            snapshot.Get(transaction.key()));
+  EXPECT_EQ(FieldValue::String(value), snapshot.Get(key));
 }
 
 class TransactionStage {

firestore/src/include/firebase/firestore.h

@@ -264,8 +264,14 @@ class Firestore {
    * 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.
    */
-  virtual Future<void> RunTransaction(TransactionFunction* update);
+  FIREBASE_DEPRECATED virtual Future<void> RunTransaction(
+      TransactionFunction* update);
 
   /**
    * Sets the log verbosity of all Firestore instances.