Change the signature of RunTransaction callback to Error (Transaction&, std::string&).

Mutable references are preferable in this case to pointers because they clearly indicate that the given argument is always valid (not null), potentially saving our users the trouble of checking explicitly.

The C++ style guide now (cr/312129146) allows using mutable references as function parameters. Note that our users never invoke the callback (this is done by Firestore), so the absence of an ampersand at the call site will not affect readability for them in any way.

PiperOrigin-RevId: 312197508

Author: var-const

firestore/src/android/firestore_android.cc

@@ -370,7 +370,7 @@ Future<void> FirestoreInternal::RunTransaction(TransactionFunction* update,
 
 #if defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
 Future<void> FirestoreInternal::RunTransaction(
-    std::function<Error(Transaction*, std::string*)> update) {
+    std::function<Error(Transaction&, std::string&)> update) {
   LambdaTransactionFunction* lambda_update =
       new LambdaTransactionFunction(firebase::Move(update));
   return RunTransaction(lambda_update, /*is_lambda=*/true);

firestore/src/android/firestore_android.h

@@ -91,7 +91,7 @@ class FirestoreInternal {
                               bool is_lambda = false);
 #if defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
   Future<void> RunTransaction(
-      std::function<Error(Transaction*, std::string*)> update);
+      std::function<Error(Transaction&, std::string&)> update);
 #endif  // defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
   Future<void> RunTransactionLastResult();
 

firestore/src/android/lambda_transaction_function.h

@@ -28,13 +28,13 @@ class LambdaTransactionFunction
       public Promise<void, void, FirestoreFn>::Completion<void> {
  public:
   LambdaTransactionFunction(
-      std::function<Error(Transaction*, std::string*)> update)
+      std::function<Error(Transaction&, std::string&)> update)
       : update_(firebase::Move(update)) {
     FIREBASE_ASSERT(update_);
   }
 
   // Override TransactionFunction::Apply().
-  Error Apply(Transaction* transaction, std::string* error_message) override {
+  Error Apply(Transaction& transaction, std::string& error_message) override {
     Error result = update_(transaction, error_message);
     return result;
   }
@@ -46,7 +46,7 @@ class LambdaTransactionFunction
   }
 
  private:
-  std::function<Error(Transaction*, std::string*)> update_;
+  std::function<Error(Transaction&, std::string&)> update_;
 };
 
 }  // namespace firestore

firestore/src/android/transaction_android.cc

@@ -215,7 +215,7 @@ jobject TransactionInternal::TransactionFunctionNativeApply(
   Transaction cpp_transaction(new TransactionInternal{firestore, transaction});
   cpp_transaction.internal_->ClearException();
   std::string message;
-  Error code = transaction_function->Apply(&cpp_transaction, &message);
+  Error code = transaction_function->Apply(cpp_transaction, message);
 
   jobject first_exception =
       env->NewLocalRef(*(cpp_transaction.internal_->first_exception_));

firestore/src/common/firestore.cc

@@ -230,7 +230,7 @@ Future<void> Firestore::RunTransaction(TransactionFunction* update) {
 
 #if defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
 Future<void> Firestore::RunTransaction(
-    std::function<Error(Transaction*, std::string*)> update) {
+    std::function<Error(Transaction&, std::string&)> update) {
   FIREBASE_ASSERT_MESSAGE(update, "invalid update parameter is passed in.");
   if (!internal_) return FailedFuture<void>();
   return internal_->RunTransaction(firebase::Move(update));

firestore/src/include/firebase/csharp/unity_transaction_function.cc

@@ -10,12 +10,12 @@ ::firebase::Mutex* UnityTransactionFunction::mutex_ = new ::firebase::Mutex();
 UnityTransactionFunctionCallback
     UnityTransactionFunction::transaction_function_callback_ = nullptr;
 
-Error UnityTransactionFunction::Apply(Transaction* transaction,
-                                      std::string* error_message) {
+Error UnityTransactionFunction::Apply(Transaction& transaction,
+                                      std::string& error_message) {
   MutexLock lock(*mutex_);
   if (transaction_function_callback_ != nullptr) {
-    return transaction_function_callback_(callback_id_, transaction,
-                                          error_message);
+    return transaction_function_callback_(callback_id_, &transaction,
+                                          &error_message);
   } else {
     FIREBASE_ASSERT_MESSAGE(
         false, "C++ transaction callback called before C# registered.");
@@ -42,8 +42,8 @@ Future<void> UnityTransactionFunction::RunTransactionOn(int32_t callback_id,
                                                         Firestore* firestore) {
   UnityTransactionFunction unity_transaction_function(callback_id);
   return firestore->RunTransaction(
-      [unity_transaction_function](Transaction* transaction,
-                                   std::string* error_message) mutable {
+      [unity_transaction_function](Transaction& transaction,
+                                   std::string& error_message) mutable {
         return unity_transaction_function.Apply(transaction, error_message);
       });
 }

firestore/src/include/firebase/csharp/unity_transaction_function.h

@@ -52,7 +52,7 @@ class UnityTransactionFunction : public TransactionFunction {
    * Implementation of TransactionFunction::Apply() that forwards to the C#
    * global delegate, passing along this->callback_id_ for context.
    */
-  Error Apply(Transaction* transaction, std::string* error_message) override;
+  Error Apply(Transaction& transaction, std::string& error_message) override;
 
  private:
   explicit UnityTransactionFunction(int32_t callback_id)

firestore/src/include/firebase/firestore.h

@@ -242,14 +242,15 @@ class Firestore {
    * 5 attempts, the transaction will fail.
    *
    * @param update function or lambda to execute within the transaction context.
+   * The string reference parameter can be used to set the error message.
    *
    * @return A Future that will be resolved when the transaction finishes.
    *
    * @note This method is not available when using the STLPort C++ runtime
    * library.
    */
   virtual Future<void> RunTransaction(
-      std::function<Error(Transaction*, std::string*)> update);
+      std::function<Error(Transaction&, std::string&)> update);
 #endif  // defined(FIREBASE_USE_STD_FUNCTION) || defined(DOXYGEN)
 
   /**

firestore/src/include/firebase/firestore/transaction.h

@@ -153,7 +153,7 @@ class TransactionFunction {
    * @return Either Error::kOk if successful or the error code from Error that
    * most closely matches the failure.
    */
-  virtual Error Apply(Transaction* transaction, std::string* error_message) = 0;
+  virtual Error Apply(Transaction& transaction, std::string& error_message) = 0;
 };
 
 }  // namespace firestore

firestore/src/ios/firestore_ios.cc

@@ -123,13 +123,13 @@ WriteBatch FirestoreInternal::batch() const {
 
 Future<void> FirestoreInternal::RunTransaction(TransactionFunction* update) {
   return RunTransaction(
-      [update](Transaction* transaction, std::string* error_message) {
+      [update](Transaction& transaction, std::string& error_message) {
         return update->Apply(transaction, error_message);
       });
 }
 
 Future<void> FirestoreInternal::RunTransaction(
-    std::function<Error(Transaction*, std::string*)> update) {
+    std::function<Error(Transaction&, std::string&)> update) {
   auto executor = absl::ShareUniquePtr(Executor::CreateConcurrent(
       "com.google.firebase.firestore.transaction", /*threads=*/5));
   auto promise =
@@ -151,7 +151,7 @@ Future<void> FirestoreInternal::RunTransaction(
                   new TransactionInternal(core_transaction, this);
               Transaction transaction{transaction_internal};
 
-              Error error_code = update(&transaction, &error_message);
+              Error error_code = update(transaction, error_message);
               if (error_code == Error::kOk) {
                 eventual_result_callback(Status::OK());
               } else {