From 31f762e188015a637b9e8650c79f8ab6aeffdde8 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 9 May 2022 12:57:57 -0400 Subject: [PATCH 01/20] TransactionOptions added --- Firestore/core/src/api/firestore.cc | 9 +- Firestore/core/src/api/firestore.h | 4 +- .../core/src/core/transaction_options.cc | 44 +++++++ Firestore/core/src/core/transaction_options.h | 47 ++++++++ .../unit/core/transaction_options_test.cc | 111 ++++++++++++++++++ 5 files changed, 210 insertions(+), 5 deletions(-) create mode 100644 Firestore/core/src/core/transaction_options.cc create mode 100644 Firestore/core/src/core/transaction_options.h create mode 100644 Firestore/core/test/unit/core/transaction_options_test.cc diff --git a/Firestore/core/src/api/firestore.cc b/Firestore/core/src/api/firestore.cc index 37756344c7a..8a6e82006cb 100644 --- a/Firestore/core/src/api/firestore.cc +++ b/Firestore/core/src/api/firestore.cc @@ -28,6 +28,7 @@ #include "Firestore/core/src/core/firestore_client.h" #include "Firestore/core/src/core/query.h" #include "Firestore/core/src/core/transaction.h" +#include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/credentials/empty_credentials_provider.h" #include "Firestore/core/src/local/leveldb_persistence.h" #include "Firestore/core/src/model/document_key.h" @@ -155,12 +156,12 @@ core::Query Firestore::GetCollectionGroup(std::string collection_id) { std::move(collection_id))); } -void Firestore::RunTransaction( - core::TransactionUpdateCallback update_callback, - core::TransactionResultCallback result_callback) { +void Firestore::RunTransaction(core::TransactionUpdateCallback update_callback, + core::TransactionResultCallback result_callback, + const core::TransactionOptions& options) { EnsureClientConfigured(); - client_->Transaction(5, std::move(update_callback), + client_->Transaction(options.max_attempts(), std::move(update_callback), std::move(result_callback)); } diff --git a/Firestore/core/src/api/firestore.h b/Firestore/core/src/api/firestore.h index f8f58ccde26..6e7d1816fca 100644 --- a/Firestore/core/src/api/firestore.h +++ b/Firestore/core/src/api/firestore.h @@ -25,6 +25,7 @@ #include "Firestore/core/src/api/load_bundle_task.h" #include "Firestore/core/src/api/settings.h" #include "Firestore/core/src/core/core_fwd.h" +#include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/credentials/credentials_fwd.h" #include "Firestore/core/src/model/database_id.h" #include "Firestore/core/src/util/byte_stream.h" @@ -92,7 +93,8 @@ class Firestore : public std::enable_shared_from_this { core::Query GetCollectionGroup(std::string collection_id); void RunTransaction(core::TransactionUpdateCallback update_callback, - core::TransactionResultCallback result_callback); + core::TransactionResultCallback result_callback, + const core::TransactionOptions& options = {}); void Terminate(util::StatusCallback callback); void ClearPersistence(util::StatusCallback callback); diff --git a/Firestore/core/src/core/transaction_options.cc b/Firestore/core/src/core/transaction_options.cc new file mode 100644 index 00000000000..ff6c28fc0fd --- /dev/null +++ b/Firestore/core/src/core/transaction_options.cc @@ -0,0 +1,44 @@ +/* + * Copyright 2020 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/core/transaction_options.h" + +#include "Firestore/core/src/util/hard_assert.h" +#include "Firestore/core/src/util/string_format.h" +#include "Firestore/core/src/util/to_string.h" + +namespace firebase { +namespace firestore { +namespace core { + +void TransactionOptions::set_max_attempts(int32_t max_attempts) { + HARD_ASSERT(max_attempts > 0, "invalid max_attempts: %s", + util::ToString(max_attempts)); + max_attempts_ = max_attempts; +} + +std::string TransactionOptions::ToString() const { + return util::StringFormat("TransactionOptions(max_attempts=%s)", + util::ToString(max_attempts_)); +} + +std::ostream& operator<<(std::ostream& os, const TransactionOptions& options) { + return os << options.ToString(); +} + +} // namespace core +} // namespace firestore +} // namespace firebase diff --git a/Firestore/core/src/core/transaction_options.h b/Firestore/core/src/core/transaction_options.h new file mode 100644 index 00000000000..a1e491c326a --- /dev/null +++ b/Firestore/core/src/core/transaction_options.h @@ -0,0 +1,47 @@ +/* + * Copyright 2020 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_CORE_TRANSACTION_OPTIONS_H_ +#define FIRESTORE_CORE_SRC_CORE_TRANSACTION_OPTIONS_H_ + +#include +#include + +namespace firebase { +namespace firestore { +namespace core { + +class TransactionOptions { + public: + int32_t max_attempts() const { + return max_attempts_; + } + + void set_max_attempts(int32_t max_attempts); + + std::string ToString() const; + + private: + int32_t max_attempts_ = 5; +}; + +std::ostream& operator<<(std::ostream&, const TransactionOptions&); + +} // namespace core +} // namespace firestore +} // namespace firebase + +#endif // FIRESTORE_CORE_SRC_CORE_TRANSACTION_OPTIONS_H_ diff --git a/Firestore/core/test/unit/core/transaction_options_test.cc b/Firestore/core/test/unit/core/transaction_options_test.cc new file mode 100644 index 00000000000..974e20c0940 --- /dev/null +++ b/Firestore/core/test/unit/core/transaction_options_test.cc @@ -0,0 +1,111 @@ +/* + * Copyright 2020 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 +#include + +#include "Firestore/core/src/core/transaction_options.h" + +#include "gtest/gtest.h" + +namespace firebase { +namespace firestore { +namespace core { +namespace { + +TEST(TransactionOptionsTest, ZeroArgConstructor) { + TransactionOptions options; + EXPECT_EQ(options.max_attempts(), 5); +} + +TEST(TransactionOptionsTest, CopyConstructor) { + TransactionOptions options1; + options1.set_max_attempts(999); + + TransactionOptions options2(options1); + + EXPECT_EQ(options2.max_attempts(), 999); +} + +TEST(TransactionOptionsTest, MoveConstructor) { + TransactionOptions options1; + options1.set_max_attempts(999); + + TransactionOptions options2(std::move(options1)); + + EXPECT_EQ(options2.max_attempts(), 999); +} + +TEST(TransactionOptionsTest, CopyAssignmentOperator) { + TransactionOptions options1; + options1.set_max_attempts(999); + TransactionOptions options2; + options2.set_max_attempts(123); + + options2 = options1; + + EXPECT_EQ(options2.max_attempts(), 999); +} + +TEST(TransactionOptionsTest, MoveAssignmentOperator) { + TransactionOptions options1; + options1.set_max_attempts(999); + TransactionOptions options2; + options2.set_max_attempts(123); + + options2 = std::move(options1); + + EXPECT_EQ(options2.max_attempts(), 999); +} + +TEST(TransactionOptionsTest, SetMaxAttempts) { + TransactionOptions options; + options.set_max_attempts(10); + EXPECT_EQ(options.max_attempts(), 10); + options.set_max_attempts(20); + EXPECT_EQ(options.max_attempts(), 20); + options.set_max_attempts(1); + EXPECT_EQ(options.max_attempts(), 1); +} + +TEST(TransactionOptionsTest, SetMaxAttemptsFailsOnInvalidMaxAttempts) { + TransactionOptions options; + EXPECT_ANY_THROW(options.set_max_attempts(0)); + EXPECT_ANY_THROW(options.set_max_attempts(-1)); + EXPECT_ANY_THROW(options.set_max_attempts(-999)); + EXPECT_ANY_THROW(options.set_max_attempts(INT32_MIN)); +} + +TEST(TransactionOptionsTest, ToString) { + TransactionOptions options; + options.set_max_attempts(999); + EXPECT_EQ(options.ToString(), "TransactionOptions(max_attempts=999)"); +} + +TEST(TransactionOptionsTest, WriteToOstream) { + TransactionOptions options; + options.set_max_attempts(999); + std::ostringstream out; + + out << options; + + EXPECT_EQ(out.str(), options.ToString()); +} + +} // namespace +} // namespace core +} // namespace firestore +} // namespace firebase From 8716c4304b034153ac10cb285e4b3e00352220aa Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 9 May 2022 16:27:29 -0400 Subject: [PATCH 02/20] FIRTransactionOptions added --- .../Firestore.xcodeproj/project.pbxproj | 28 +++++ .../Tests/API/FIRTransactionOptionsTests.mm | 112 ++++++++++++++++++ .../API/FIRTransactionOptions+Internal.h | 32 +++++ Firestore/Source/API/FIRTransactionOptions.mm | 79 ++++++++++++ .../FirebaseFirestore/FIRTransactionOptions.h | 37 ++++++ .../core/src/core/transaction_options.cc | 9 ++ Firestore/core/src/core/transaction_options.h | 9 ++ .../unit/core/transaction_options_test.cc | 18 +++ 8 files changed, 324 insertions(+) create mode 100644 Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm create mode 100644 Firestore/Source/API/FIRTransactionOptions+Internal.h create mode 100644 Firestore/Source/API/FIRTransactionOptions.mm create mode 100644 Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 2d68c73ad90..29f57c80ff8 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -63,6 +63,7 @@ 0963F6D7B0F9AE1E24B82866 /* path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 403DBF6EFB541DFD01582AA3 /* path_test.cc */; }; 096BA3A3703AC1491F281618 /* index.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 395E8B07639E69290A929695 /* index.pb.cc */; }; 098191405BA24F9A7E4F80C6 /* append_only_list_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 5477CDE922EE71C8000FCC1E /* append_only_list_test.cc */; }; + 0A074A769E0E892A200EAA89 /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 0A4E1B5E3E853763AE6ED7AE /* grpc_stream_tester.cc in Sources */ = {isa = PBXBuildFile; fileRef = 87553338E42B8ECA05BA987E /* grpc_stream_tester.cc */; }; 0A52B47C43B7602EE64F53A7 /* cc_compilation_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1B342370EAE3AA02393E33EB /* cc_compilation_test.cc */; }; 0A6FBE65A7FE048BAD562A15 /* FSTGoogleTestTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */; }; @@ -126,6 +127,7 @@ 15F54E9538839D56A40C5565 /* watch_change_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 2D7472BC70C024D736FF74D9 /* watch_change_test.cc */; }; 16791B16601204220623916C /* status_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0352C20A3B3D7003E0143 /* status_test.cc */; }; 16FE432587C1B40AF08613D2 /* objc_type_traits_apple_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = 2A0CF41BA5AED6049B0BEB2C /* objc_type_traits_apple_test.mm */; }; + 16FF9073CA381CA43CA9BF29 /* FIRTransactionOptionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */; }; 1733601ECCEA33E730DEAF45 /* autoid_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54740A521FC913E500713A1A /* autoid_test.cc */; }; 17473086EBACB98CDC3CC65C /* view_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = C7429071B33BDF80A7FA2F8A /* view_test.cc */; }; 17638F813B9B556FE7718C0C /* FIRQuerySnapshotTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04F202154AA00B64F25 /* FIRQuerySnapshotTests.mm */; }; @@ -172,6 +174,7 @@ 1F4930A8366F74288121F627 /* create_noop_connectivity_monitor.cc in Sources */ = {isa = PBXBuildFile; fileRef = CF39535F2C41AB0006FA6C0E /* create_noop_connectivity_monitor.cc */; }; 1F56F51EB6DF0951B1F4F85B /* lru_garbage_collector_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 277EAACC4DD7C21332E8496A /* lru_garbage_collector_test.cc */; }; 1F998DDECB54A66222CC66AA /* string_format_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54131E9620ADE678001DF3FF /* string_format_test.cc */; }; + 1FE21B06B7217E6615EC3FEC /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 2045517602D767BD01EA71D9 /* overlay_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = E1459FA70B8FC18DE4B80D0D /* overlay_test.cc */; }; 205601D1C6A40A4DD3BBAA04 /* target_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 526D755F65AC676234F57125 /* target_test.cc */; }; 20814A477D00EA11D0E76631 /* FIRDocumentSnapshotTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04B202154AA00B64F25 /* FIRDocumentSnapshotTests.mm */; }; @@ -249,6 +252,7 @@ 3056418E81BC7584FBE8AD6C /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = CCC9BD953F121B9E29F9AA42 /* user_test.cc */; }; 306E762DC6B829CED4FD995D /* target_id_generator_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380CF82019382300D97691 /* target_id_generator_test.cc */; }; 3095316962A00DD6A4A2A441 /* counting_query_engine.cc in Sources */ = {isa = PBXBuildFile; fileRef = 99434327614FEFF7F7DC88EC /* counting_query_engine.cc */; }; + 30BA616D48145CB97CDBDADA /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 314D231A9F33E0502611DD20 /* sorted_set_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4C20A36DBB00BCEB75 /* sorted_set_test.cc */; }; 31850B3D5232E8D3F8C4D90C /* memory_remote_document_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1CA9800A53669EFBFFB824E3 /* memory_remote_document_cache_test.cc */; }; 31A396C81A107D1DEFDF4A34 /* serializer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 61F72C5520BC48FD001A68CB /* serializer_test.cc */; }; @@ -263,6 +267,7 @@ 336E415DD06E719F9C9E2A14 /* grpc_stream_tester.cc in Sources */ = {isa = PBXBuildFile; fileRef = 87553338E42B8ECA05BA987E /* grpc_stream_tester.cc */; }; 338DFD5BCD142DF6C82A0D56 /* cc_compilation_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1B342370EAE3AA02393E33EB /* cc_compilation_test.cc */; }; 339CFFD1323BDCA61EAAFE31 /* query_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B9C261C26C5D311E1E3C0CB9 /* query_test.cc */; }; + 339D4DD13E1518BA79FF12EA /* FIRTransactionOptionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */; }; 340987A77D72C80A3E0FDADF /* view_snapshot_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = CC572A9168BBEF7B83E4BBC5 /* view_snapshot_test.cc */; }; 3409F2AEB7D6D95478D4344A /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; }; 34202A37E0B762386967AF3D /* grpc_stream_tester.cc in Sources */ = {isa = PBXBuildFile; fileRef = 87553338E42B8ECA05BA987E /* grpc_stream_tester.cc */; }; @@ -538,6 +543,7 @@ 57BDB8DBEDEC4C61DB497CB4 /* append_only_list_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 5477CDE922EE71C8000FCC1E /* append_only_list_test.cc */; }; 583DF65751B7BBD0A222CAB4 /* byte_stream_cpp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 01D10113ECC5B446DB35E96D /* byte_stream_cpp_test.cc */; }; 58693C153EC597BC25EE9648 /* firebase_auth_credentials_provider_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = F869D85E900E5AF6CD02E2FC /* firebase_auth_credentials_provider_test.mm */; }; + 58B84B550725D9812729C7F7 /* FIRTransactionOptionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */; }; 58E377DCCC64FE7D2C6B59A1 /* database_id_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB71064B201FA60300344F18 /* database_id_test.cc */; }; 5958E3E3A0446A88B815CB70 /* grpc_connection_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B6D9649021544D4F00EB9CFB /* grpc_connection_test.cc */; }; 59880AE766F7FBFF0C41A94E /* remote_event_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 584AE2C37A55B408541A6FF3 /* remote_event_test.cc */; }; @@ -777,6 +783,7 @@ 84E75527F3739131C09BEAA5 /* target_index_matcher_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 63136A2371C0C013EC7A540C /* target_index_matcher_test.cc */; }; 851346D66DEC223E839E3AA9 /* memory_mutation_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 74FBEFA4FE4B12C435011763 /* memory_mutation_queue_test.cc */; }; 856A1EAAD674ADBDAAEDAC37 /* bundle_builder.cc in Sources */ = {isa = PBXBuildFile; fileRef = 4F5B96F3ABCD2CA901DB1CD4 /* bundle_builder.cc */; }; + 85A33A9CE33207C2333DDD32 /* FIRTransactionOptionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */; }; 85B8918FC8C5DC62482E39C3 /* resource_path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = B686F2B02024FFD70028D6BE /* resource_path_test.cc */; }; 85BC2AB572A400114BF59255 /* limbo_spec_test.json in Resources */ = {isa = PBXBuildFile; fileRef = 54DA129E1F315EE100DD57A1 /* limbo_spec_test.json */; }; 85D61BDC7FB99B6E0DD3AFCA /* mutation.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 618BBE8220B89AAC00B5BCE7 /* mutation.pb.cc */; }; @@ -787,6 +794,7 @@ 86494278BE08F10A8AAF9603 /* iterator_adaptors_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0353420A3D8CB003E0143 /* iterator_adaptors_test.cc */; }; 867B370BF2DF84B6AB94B874 /* filesystem_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = BA02DA2FCD0001CFC6EB08DA /* filesystem_testing.cc */; }; 8683BBC3AC7B01937606A83B /* firestore.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 544129D421C2DDC800EFB9CC /* firestore.pb.cc */; }; + 86C341E67E2F60813BC1776B /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 86E6FC2B7657C35B342E1436 /* sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4E20A36DBB00BCEB75 /* sorted_map_test.cc */; }; 8705C4856498F66E471A0997 /* FIRWriteBatchTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E06F202154D600B64F25 /* FIRWriteBatchTests.mm */; }; 873B8AEB1B1F5CCA007FD442 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 873B8AEA1B1F5CCA007FD442 /* Main.storyboard */; }; @@ -823,6 +831,7 @@ 90B9302B082E6252AF4E7DC7 /* leveldb_migrations_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = EF83ACD5E1E9F25845A9ACED /* leveldb_migrations_test.cc */; }; 90FE088B8FD9EC06EEED1F39 /* memory_index_manager_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = DB5A1E760451189DA36028B3 /* memory_index_manager_test.cc */; }; 911931696309D2EABB325F17 /* strerror_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 358C3B5FE573B1D60A4F7592 /* strerror_test.cc */; }; + 913C2DB6951A2ED24778686C /* FIRTransactionOptionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */; }; 913F6E57AF18F84C5ECFD414 /* lru_garbage_collector_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 277EAACC4DD7C21332E8496A /* lru_garbage_collector_test.cc */; }; 915A9B8DB280DB4787D83FFE /* byte_stream_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 432C71959255C5DBDF522F52 /* byte_stream_test.cc */; }; 91AEFFEE35FBE15FEC42A1F4 /* memory_local_store_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F6CA0C5638AB6627CB5B4CF4 /* memory_local_store_test.cc */; }; @@ -896,6 +905,7 @@ A5175CA2E677E13CC5F23D72 /* document_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB6B908320322E4D00CC290A /* document_test.cc */; }; A55266E6C986251D283CE948 /* FIRCursorTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E070202154D600B64F25 /* FIRCursorTests.mm */; }; A5583822218F9D5B1E86FCAC /* overlay_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = E1459FA70B8FC18DE4B80D0D /* overlay_test.cc */; }; + A5739BC7DDF1A76E623AD76B /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; A57EC303CD2D6AA4F4745551 /* FIRFieldValueTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04A202154AA00B64F25 /* FIRFieldValueTests.mm */; }; A585BD0F31E90980B5F5FBCA /* local_serializer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F8043813A5D16963EC02B182 /* local_serializer_test.cc */; }; A5AB1815C45FFC762981E481 /* write.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 544129D921C2DDC800EFB9CC /* write.pb.cc */; }; @@ -938,6 +948,7 @@ AC03C4F1456FB1C0D88E94FF /* query_listener_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7C3F995E040E9E9C5E8514BB /* query_listener_test.cc */; }; AC6B856ACB12BB28D279693D /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; }; AC6C1E57B18730428CB15E03 /* executor_libdispatch_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4689208F9B9100554BA2 /* executor_libdispatch_test.mm */; }; + AC7F651D64F575650EC3FB7F /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; AC835157AD2BE7AA8D20FB5A /* ConditionalConformanceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3228F51DCDC2E90D5C58F97 /* ConditionalConformanceTests.swift */; }; ACC9369843F5ED3BD2284078 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; }; AD12205540893CEB48647937 /* filesystem_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = BA02DA2FCD0001CFC6EB08DA /* filesystem_testing.cc */; }; @@ -1156,6 +1167,7 @@ DB3ADDA51FB93E84142EA90D /* FIRBundlesTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 776530F066E788C355B78457 /* FIRBundlesTests.mm */; }; DB7E9C5A59CCCDDB7F0C238A /* path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 403DBF6EFB541DFD01582AA3 /* path_test.cc */; }; DBDC8E997E909804F1B43E92 /* log_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54C2294E1FECABAE007D065B /* log_test.cc */; }; + DBFE8B2E803C1D0DECB71FF6 /* FIRTransactionOptionsTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */; }; DC0B0E50DBAE916E6565AA18 /* string_win_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 79507DF8378D3C42F5B36268 /* string_win_test.cc */; }; DC0E186BDD221EAE9E4D2F41 /* sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4E20A36DBB00BCEB75 /* sorted_map_test.cc */; }; DC1C711290E12F8EF3601151 /* array_sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54EB764C202277B30088B8F3 /* array_sorted_map_test.cc */; }; @@ -1696,10 +1708,12 @@ CD422AF3E4515FB8E9BE67A0 /* equals_tester.h */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.c.h; path = equals_tester.h; sourceTree = ""; }; CE37875365497FFA8687B745 /* message_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; name = message_test.cc; path = nanopb/message_test.cc; sourceTree = ""; }; CF39535F2C41AB0006FA6C0E /* create_noop_connectivity_monitor.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = create_noop_connectivity_monitor.cc; sourceTree = ""; }; + CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */ = {isa = PBXFileReference; includeInIndex = 1; path = FIRTransactionOptionsTests.mm; sourceTree = ""; }; D0A6E9136804A41CEC9D55D4 /* delayed_constructor_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = sourcecode.cpp.cpp; path = delayed_constructor_test.cc; sourceTree = ""; }; D3CC3DC5338DCAF43A211155 /* README.md */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = net.daringfireball.markdown; name = README.md; path = ../README.md; sourceTree = ""; }; D5B2593BCB52957D62F1C9D3 /* perf_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = perf_spec_test.json; sourceTree = ""; }; D5B25E7E7D6873CBA4571841 /* FIRNumericTransformTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FIRNumericTransformTests.mm; sourceTree = ""; }; + D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = transaction_options_test.cc; sourceTree = ""; }; D7DF4A6F740086A2D8C0E28E /* Pods_Firestore_Tests_tvOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Tests_tvOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; DAFF0CF521E64AC30062958F /* Firestore_Example_macOS.app */ = {isa = PBXFileReference; explicitFileType = wrapper.application; includeInIndex = 0; path = Firestore_Example_macOS.app; sourceTree = BUILT_PRODUCTS_DIR; }; DAFF0CF721E64AC30062958F /* AppDelegate.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = AppDelegate.h; sourceTree = ""; }; @@ -2463,6 +2477,7 @@ B9C261C26C5D311E1E3C0CB9 /* query_test.cc */, AB380CF82019382300D97691 /* target_id_generator_test.cc */, 526D755F65AC676234F57125 /* target_test.cc */, + D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */, CC572A9168BBEF7B83E4BBC5 /* view_snapshot_test.cc */, C7429071B33BDF80A7FA2F8A /* view_test.cc */, ); @@ -2504,6 +2519,7 @@ FF73B39D04D1760190E6B84A /* FIRQueryUnitTests.mm */, 5492E04D202154AA00B64F25 /* FIRSnapshotMetadataTests.mm */, B65D34A7203C99090076A5E1 /* FIRTimestampTest.m */, + CF39ECA1293D21A0A2AB2626 /* FIRTransactionOptionsTests.mm */, 5492E047202154AA00B64F25 /* FSTAPIHelpers.h */, 5492E04E202154AA00B64F25 /* FSTAPIHelpers.mm */, 8D9892F204959C50613F16C8 /* FSTUserDataReaderTests.mm */, @@ -3523,6 +3539,7 @@ D39F0216BF1EA8CD54C76CF8 /* FIRQueryUnitTests.mm in Sources */, 2EAD77559EC654E6CA4D3E21 /* FIRSnapshotMetadataTests.mm in Sources */, CD0AA9E5D83C00CAAE7C2F67 /* FIRTimestampTest.m in Sources */, + 16FF9073CA381CA43CA9BF29 /* FIRTransactionOptionsTests.mm in Sources */, 9D71628E38D9F64C965DF29E /* FSTAPIHelpers.mm in Sources */, F4F00BF4E87D7F0F0F8831DB /* FSTEventAccumulator.mm in Sources */, 0A6FBE65A7FE048BAD562A15 /* FSTGoogleTestTests.mm in Sources */, @@ -3684,6 +3701,7 @@ 2AAEABFD550255271E3BAC91 /* to_string_apple_test.mm in Sources */, 1E2AE064CF32A604DC7BFD4D /* to_string_test.cc in Sources */, AAFA9D7A0A067F2D3D8D5487 /* token_test.cc in Sources */, + 0A074A769E0E892A200EAA89 /* transaction_options_test.cc in Sources */, 5D51D8B166D24EFEF73D85A2 /* transform_operation_test.cc in Sources */, 5F19F66D8B01BA2B97579017 /* tree_sorted_map_test.cc in Sources */, 124AAEE987451820F24EEA8E /* user_test.cc in Sources */, @@ -3722,6 +3740,7 @@ 518BF03D57FBAD7C632D18F8 /* FIRQueryUnitTests.mm in Sources */, ED420D8F49DA5C41EEF93913 /* FIRSnapshotMetadataTests.mm in Sources */, 36E174A66C323891AEA16A2A /* FIRTimestampTest.m in Sources */, + DBFE8B2E803C1D0DECB71FF6 /* FIRTransactionOptionsTests.mm in Sources */, 6E4854B19B120C6F0F8192CC /* FSTAPIHelpers.mm in Sources */, 73E42D984FB36173A2BDA57C /* FSTEventAccumulator.mm in Sources */, E375FBA0632EFB4D14C4E5A9 /* FSTGoogleTestTests.mm in Sources */, @@ -3883,6 +3902,7 @@ 5BE49546D57C43DDFCDB6FBD /* to_string_apple_test.mm in Sources */, E500AB82DF2E7F3AFDB1AB3F /* to_string_test.cc in Sources */, 5C9B5696644675636A052018 /* token_test.cc in Sources */, + AC7F651D64F575650EC3FB7F /* transaction_options_test.cc in Sources */, 5EE21E86159A1911E9503BC1 /* transform_operation_test.cc in Sources */, 627253FDEC6BB5549FE77F4E /* tree_sorted_map_test.cc in Sources */, 3056418E81BC7584FBE8AD6C /* user_test.cc in Sources */, @@ -3928,6 +3948,7 @@ FA7837C5CDFB273DE447E447 /* FIRServerTimestampTests.mm in Sources */, 67BC2B77C1CC47388E79D774 /* FIRSnapshotMetadataTests.mm in Sources */, 041CF73F67F6A22BF317625A /* FIRTimestampTest.m in Sources */, + 58B84B550725D9812729C7F7 /* FIRTransactionOptionsTests.mm in Sources */, 75D124966E727829A5F99249 /* FIRTypeTests.mm in Sources */, 12DB753599571E24DCED0C2C /* FIRValidationTests.mm in Sources */, BC0C98A9201E8F98B9A176A9 /* FIRWriteBatchTests.mm in Sources */, @@ -4096,6 +4117,7 @@ 95DCD082374F871A86EF905F /* to_string_apple_test.mm in Sources */, 9E656F4FE92E8BFB7F625283 /* to_string_test.cc in Sources */, 96D95E144C383459D4E26E47 /* token_test.cc in Sources */, + A5739BC7DDF1A76E623AD76B /* transaction_options_test.cc in Sources */, 15BF63DFF3A7E9A5376C4233 /* transform_operation_test.cc in Sources */, 54B91B921DA757C64CC67C90 /* tree_sorted_map_test.cc in Sources */, CDB5816537AB1B209C2B72A4 /* user_test.cc in Sources */, @@ -4141,6 +4163,7 @@ 27E46C94AAB087C80A97FF7F /* FIRServerTimestampTests.mm in Sources */, 59F512D155DE361095A04ED4 /* FIRSnapshotMetadataTests.mm in Sources */, FE1C0263F6570DAC54A60F5C /* FIRTimestampTest.m in Sources */, + 339D4DD13E1518BA79FF12EA /* FIRTransactionOptionsTests.mm in Sources */, 5F05A801B1EA44BC1264E55A /* FIRTypeTests.mm in Sources */, 8403D519C916C72B9C7F2FA1 /* FIRValidationTests.mm in Sources */, 8705C4856498F66E471A0997 /* FIRWriteBatchTests.mm in Sources */, @@ -4309,6 +4332,7 @@ F9705E595FC3818F13F6375A /* to_string_apple_test.mm in Sources */, 3BAFCABA851AE1865D904323 /* to_string_test.cc in Sources */, 1B9E54F4C4280A713B825981 /* token_test.cc in Sources */, + 1FE21B06B7217E6615EC3FEC /* transaction_options_test.cc in Sources */, 44EAF3E6EAC0CC4EB2147D16 /* transform_operation_test.cc in Sources */, 3D22F56C0DE7C7256C75DC06 /* tree_sorted_map_test.cc in Sources */, A80D38096052F928B17E1504 /* user_test.cc in Sources */, @@ -4357,6 +4381,7 @@ CB2C731116D6C9464220626F /* FIRQueryUnitTests.mm in Sources */, 5492E057202154AB00B64F25 /* FIRSnapshotMetadataTests.mm in Sources */, B65D34A9203C995B0076A5E1 /* FIRTimestampTest.m in Sources */, + 85A33A9CE33207C2333DDD32 /* FIRTransactionOptionsTests.mm in Sources */, 5492E058202154AB00B64F25 /* FSTAPIHelpers.mm in Sources */, 5492E03E2021401F00B64F25 /* FSTEventAccumulator.mm in Sources */, 54764FAF1FAA21B90085E60A /* FSTGoogleTestTests.mm in Sources */, @@ -4518,6 +4543,7 @@ B68B1E012213A765008977EF /* to_string_apple_test.mm in Sources */, B696858E2214B53900271095 /* to_string_test.cc in Sources */, D50232D696F19C2881AC01CE /* token_test.cc in Sources */, + 86C341E67E2F60813BC1776B /* transaction_options_test.cc in Sources */, D3CB03747E34D7C0365638F1 /* transform_operation_test.cc in Sources */, 549CCA5120A36DBC00BCEB75 /* tree_sorted_map_test.cc in Sources */, 1B816F48012524939CA57CB3 /* user_test.cc in Sources */, @@ -4582,6 +4608,7 @@ 5492E077202154D600B64F25 /* FIRServerTimestampTests.mm in Sources */, 716289F99B5316B3CC5E5CE9 /* FIRSnapshotMetadataTests.mm in Sources */, 02B83EB79020AE6CBA60A410 /* FIRTimestampTest.m in Sources */, + 913C2DB6951A2ED24778686C /* FIRTransactionOptionsTests.mm in Sources */, 5492E07A202154D600B64F25 /* FIRTypeTests.mm in Sources */, 5492E076202154D600B64F25 /* FIRValidationTests.mm in Sources */, 5492E078202154D600B64F25 /* FIRWriteBatchTests.mm in Sources */, @@ -4750,6 +4777,7 @@ 60260A06871DCB1A5F3448D3 /* to_string_apple_test.mm in Sources */, ECED3B60C5718B085AAB14FB /* to_string_test.cc in Sources */, F0EA84FB66813F2BC164EF7C /* token_test.cc in Sources */, + 30BA616D48145CB97CDBDADA /* transaction_options_test.cc in Sources */, 60186935E36CF79E48A0B293 /* transform_operation_test.cc in Sources */, 5DA343D28AE05B0B2FE9FFB3 /* tree_sorted_map_test.cc in Sources */, EF8C005DC4BEA6256D1DBC6F /* user_test.cc in Sources */, diff --git a/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm b/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm new file mode 100644 index 00000000000..39fe93866d2 --- /dev/null +++ b/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm @@ -0,0 +1,112 @@ +/* + * Copyright 2022 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. + */ + +#import + +#import + +#import "Firestore/Source/API/FIRTransactionOptions+Internal.h" + +using firebase::firestore::core::TransactionOptions; + +NS_ASSUME_NONNULL_BEGIN + +@interface FIRTransactionOptionsTests : XCTestCase +@end + +@implementation FIRTransactionOptionsTests + +- (void)testDefaults { + FIRTransactionOptions* options = [[FIRTransactionOptions alloc] init]; + XCTAssertEqual(options.maxAttempts, 5); +} + +- (void)testSetMaxAttempts { + FIRTransactionOptions* options = [[FIRTransactionOptions alloc] init]; + options.maxAttempts = 10; + XCTAssertEqual(options.maxAttempts, 10); + options.maxAttempts = 99; + XCTAssertEqual(options.maxAttempts, 99); +} + +- (void)testSetMaxAttemptsThrowsOnInvalidValue { + FIRTransactionOptions* options = [[FIRTransactionOptions alloc] init]; + XCTAssertThrows(options.maxAttempts = 0); + XCTAssertThrows(options.maxAttempts = -1); + XCTAssertThrows(options.maxAttempts = INT32_MIN); +} + +- (void)testInternalTransactionOptions { + FIRTransactionOptions* optionsPtr = [[FIRTransactionOptions alloc] init]; + optionsPtr.maxAttempts = 99; + TransactionOptions options = [optionsPtr internalTransactionOptions]; + XCTAssertEqual(options.max_attempts(), 99); +} + +- (void)testHash { + XCTAssertEqual([[[FIRTransactionOptions alloc] init] hash], + [[[FIRTransactionOptions alloc] init] hash]); + + FIRTransactionOptions* options1a = [[FIRTransactionOptions alloc] init]; + options1a.maxAttempts = 99; + FIRTransactionOptions* options1b = [[FIRTransactionOptions alloc] init]; + options1b.maxAttempts = 99; + FIRTransactionOptions* options2a = [[FIRTransactionOptions alloc] init]; + options2a.maxAttempts = 11; + FIRTransactionOptions* options2b = [[FIRTransactionOptions alloc] init]; + options2b.maxAttempts = 11; + + XCTAssertEqual(options1a.maxAttempts, options1b.maxAttempts); + XCTAssertEqual(options2a.maxAttempts, options2b.maxAttempts); + XCTAssertNotEqual(options1a.maxAttempts, options2a.maxAttempts); +} + +- (void)testIsEqual { + FIRTransactionOptions* options1a = [[FIRTransactionOptions alloc] init]; + options1a.maxAttempts = 99; + FIRTransactionOptions* options1b = [[FIRTransactionOptions alloc] init]; + options1b.maxAttempts = 99; + FIRTransactionOptions* options2a = [[FIRTransactionOptions alloc] init]; + options2a.maxAttempts = 11; + FIRTransactionOptions* options2b = [[FIRTransactionOptions alloc] init]; + options2b.maxAttempts = 11; + + XCTAssertTrue([options1a isEqual:options1a]); + + XCTAssertTrue([options1a isEqual:options1b]); + XCTAssertTrue([options2a isEqual:options2b]); + + XCTAssertFalse([options1a isEqual:options2a]); + + XCTAssertFalse([options1a isEqual:@"definitely not equal"]); +} + +- (void)testCopy { + FIRTransactionOptions* options1 = [[FIRTransactionOptions alloc] init]; + options1.maxAttempts = 99; + FIRTransactionOptions* options2 = [options1 copy]; + XCTAssertEqual(options2.maxAttempts, 99); + + // Verify that the copy is independent of the copied object. + options1.maxAttempts = 55; + XCTAssertEqual(options2.maxAttempts, 99); + options2.maxAttempts = 22; + XCTAssertEqual(options1.maxAttempts, 55); +} + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/API/FIRTransactionOptions+Internal.h b/Firestore/Source/API/FIRTransactionOptions+Internal.h new file mode 100644 index 00000000000..9203da9057e --- /dev/null +++ b/Firestore/Source/API/FIRTransactionOptions+Internal.h @@ -0,0 +1,32 @@ +/* + * Copyright 2022 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. + */ + +#import "FIRTransactionOptions.h" + +#import + +#include "Firestore/core/src/core/transaction_options.h" + +NS_ASSUME_NONNULL_BEGIN + +@interface FIRTransactionOptions (Internal) + +/** Converts this FIRTransactionOptions instance into a core::TransactionOptions object. */ +- (firebase::firestore::core::TransactionOptions)internalTransactionOptions; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/API/FIRTransactionOptions.mm b/Firestore/Source/API/FIRTransactionOptions.mm new file mode 100644 index 00000000000..00e91a81e98 --- /dev/null +++ b/Firestore/Source/API/FIRTransactionOptions.mm @@ -0,0 +1,79 @@ +/* + * Copyright 2022 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 +#include + +#import "FIRTransactionOptions.h" + +#import + +#include "Firestore/core/src/core/transaction_options.h" +#include "Firestore/core/src/util/exception.h" + +NS_ASSUME_NONNULL_BEGIN + +using firebase::firestore::core::TransactionOptions; +using firebase::firestore::util::MakeString; +using firebase::firestore::util::ThrowInvalidArgument; + +@implementation FIRTransactionOptions + +- (instancetype)init { + if (self = [super init]) { + TransactionOptions defaultOptions; + _maxAttempts = defaultOptions.max_attempts(); + } + return self; +} + +- (BOOL)isEqual:(id)other { + if (self == other) { + return YES; + } else if (![other isKindOfClass:[FIRTransactionOptions class]]) { + return NO; + } + + FIRTransactionOptions *otherOptions = (FIRTransactionOptions *)other; + return self.maxAttempts == otherOptions.maxAttempts; +} + +- (NSUInteger)hash { + return _maxAttempts * 31; +} + +- (id)copyWithZone:(__unused NSZone *_Nullable)zone { + FIRTransactionOptions *copy = [[FIRTransactionOptions alloc] init]; + copy.maxAttempts = self.maxAttempts; + return copy; +} + +- (void)setMaxAttempts:(NSInteger)maxAttempts { + if (maxAttempts <= 0 || maxAttempts > INT32_MAX) { + ThrowInvalidArgument("Invalid maxAttempts: %s", std::to_string(maxAttempts)); + } + _maxAttempts = maxAttempts; +} + +- (TransactionOptions)internalTransactionOptions { + TransactionOptions transactionOptions; + transactionOptions.set_max_attempts(_maxAttempts); + return transactionOptions; +} + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h b/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h new file mode 100644 index 00000000000..86da63b0fc8 --- /dev/null +++ b/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h @@ -0,0 +1,37 @@ +/* + * Copyright 2022 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. + */ + +#import + +NS_ASSUME_NONNULL_BEGIN + +/** Settings used to configure a `Firestore` instance. */ +NS_SWIFT_NAME(TransactionOptions) +@interface FIRTransactionOptions : NSObject + +/** + * Creates and returns an empty `FirestoreTransactionOptions` object. + * + * @return The created `FirestoreTransactionOptions` object. + */ +- (instancetype)init NS_DESIGNATED_INITIALIZER; + +/** The maximum number of attempts to commit, after which transaction fails. Default is 5. */ +@property(nonatomic) NSInteger maxAttempts; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/core/src/core/transaction_options.cc b/Firestore/core/src/core/transaction_options.cc index ff6c28fc0fd..516d29958a8 100644 --- a/Firestore/core/src/core/transaction_options.cc +++ b/Firestore/core/src/core/transaction_options.cc @@ -17,6 +17,7 @@ #include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/util/hard_assert.h" +#include "Firestore/core/src/util/hashing.h" #include "Firestore/core/src/util/string_format.h" #include "Firestore/core/src/util/to_string.h" @@ -35,10 +36,18 @@ std::string TransactionOptions::ToString() const { util::ToString(max_attempts_)); } +size_t TransactionOptions::Hash() const { + return util::Hash(max_attempts_); +} + std::ostream& operator<<(std::ostream& os, const TransactionOptions& options) { return os << options.ToString(); } +bool operator==(const TransactionOptions& lhs, const TransactionOptions& rhs) { + return lhs.max_attempts_ == rhs.max_attempts_; +} + } // namespace core } // namespace firestore } // namespace firebase diff --git a/Firestore/core/src/core/transaction_options.h b/Firestore/core/src/core/transaction_options.h index a1e491c326a..6a6b0d3b174 100644 --- a/Firestore/core/src/core/transaction_options.h +++ b/Firestore/core/src/core/transaction_options.h @@ -34,12 +34,21 @@ class TransactionOptions { std::string ToString() const; + friend bool operator==(const TransactionOptions&, const TransactionOptions&); + + size_t Hash() const; + private: int32_t max_attempts_ = 5; }; std::ostream& operator<<(std::ostream&, const TransactionOptions&); +inline bool operator!=(const TransactionOptions& lhs, + const TransactionOptions& rhs) { + return !(lhs == rhs); +} + } // namespace core } // namespace firestore } // namespace firebase diff --git a/Firestore/core/test/unit/core/transaction_options_test.cc b/Firestore/core/test/unit/core/transaction_options_test.cc index 974e20c0940..80b4b870e1f 100644 --- a/Firestore/core/test/unit/core/transaction_options_test.cc +++ b/Firestore/core/test/unit/core/transaction_options_test.cc @@ -18,6 +18,7 @@ #include #include "Firestore/core/src/core/transaction_options.h" +#include "Firestore/core/test/unit/testutil/equals_tester.h" #include "gtest/gtest.h" @@ -105,6 +106,23 @@ TEST(TransactionOptionsTest, WriteToOstream) { EXPECT_EQ(out.str(), options.ToString()); } +TEST(TransactionOptionsTest, EqualsAndHash) { + TransactionOptions options_max_attempts_1a; + options_max_attempts_1a.set_max_attempts(10); + TransactionOptions options_max_attempts_1b; + options_max_attempts_1b.set_max_attempts(10); + TransactionOptions options_max_attempts_2a; + options_max_attempts_2a.set_max_attempts(99); + TransactionOptions options_max_attempts_2b; + options_max_attempts_2b.set_max_attempts(99); + + testutil::EqualsTester() + .AddEqualityGroup(TransactionOptions(), TransactionOptions()) + .AddEqualityGroup(options_max_attempts_1a, options_max_attempts_1b) + .AddEqualityGroup(options_max_attempts_2a, options_max_attempts_2b) + .TestEquals(); +} + } // namespace } // namespace core } // namespace firestore From 3b2521b8b6fa89bdabac28d4f56179bc9d5d7ca6 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 11 May 2022 01:18:21 -0400 Subject: [PATCH 03/20] FIRFirestore.h: add support for TransactionOptions --- Firestore/Source/API/FIRFirestore.mm | 31 ++++++++++++++----- .../Public/FirebaseFirestore/FIRFirestore.h | 6 ++++ 2 files changed, 30 insertions(+), 7 deletions(-) diff --git a/Firestore/Source/API/FIRFirestore.mm b/Firestore/Source/API/FIRFirestore.mm index 5e8c3f142b7..7dd96c573fe 100644 --- a/Firestore/Source/API/FIRFirestore.mm +++ b/Firestore/Source/API/FIRFirestore.mm @@ -21,6 +21,7 @@ #include #import "FIRFirestoreSettings+Internal.h" +#import "FIRTransactionOptions+Internal.h" #import "FirebaseCore/Extension/FIRAppInternal.h" #import "FirebaseCore/Extension/FIRComponentContainer.h" @@ -42,6 +43,7 @@ #include "Firestore/core/src/core/database_info.h" #include "Firestore/core/src/core/event_listener.h" #include "Firestore/core/src/core/transaction.h" +#include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/credentials/credentials_provider.h" #include "Firestore/core/src/model/database_id.h" #include "Firestore/core/src/remote/firebase_metadata_provider.h" @@ -254,9 +256,10 @@ - (FIRWriteBatch *)batch { return [FIRWriteBatch writeBatchWithDataReader:self.dataReader writeBatch:_firestore->GetBatch()]; } -- (void)runTransactionWithBlock:(UserUpdateBlock)updateBlock - dispatchQueue:(dispatch_queue_t)queue - completion:(UserTransactionCompletion)completion { +- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options + block:(UserUpdateBlock)updateBlock + dispatchQueue:(dispatch_queue_t)queue + completion:(UserTransactionCompletion)completion { if (!updateBlock) { ThrowInvalidArgument("Transaction block cannot be nil."); } @@ -335,21 +338,35 @@ void HandleFinalStatus(const Status &status) { result_capture->HandleFinalStatus(status); }; - _firestore->RunTransaction(std::move(internalUpdateBlock), std::move(objcTranslator)); + firebase::firestore::core::TransactionOptions transaction_options; + if (options) { + transaction_options = [options internalTransactionOptions]; + } + + _firestore->RunTransaction(std::move(internalUpdateBlock), std::move(objcTranslator), + transaction_options); } - (void)runTransactionWithBlock:(id _Nullable (^)(FIRTransaction *, NSError **error))updateBlock completion: (void (^)(id _Nullable result, NSError *_Nullable error))completion { + [self runTransactionWithOptions:nil block:updateBlock completion:completion]; +} + +- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options + block:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock + completion: + (void (^)(id _Nullable result, NSError *_Nullable error))completion { static dispatch_queue_t transactionDispatchQueue; static dispatch_once_t onceToken; dispatch_once(&onceToken, ^{ transactionDispatchQueue = dispatch_queue_create("com.google.firebase.firestore.transaction", DISPATCH_QUEUE_CONCURRENT); }); - [self runTransactionWithBlock:updateBlock - dispatchQueue:transactionDispatchQueue - completion:completion]; + [self runTransactionWithOptions:options + block:updateBlock + dispatchQueue:transactionDispatchQueue + completion:completion]; } + (void)enableLogging:(BOOL)logging { diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h index 2af05021b3a..15633c38df8 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h @@ -26,6 +26,7 @@ @class FIRLoadBundleTaskProgress; @class FIRQuery; @class FIRTransaction; +@class FIRTransactionOptions; @class FIRWriteBatch; NS_ASSUME_NONNULL_BEGIN @@ -146,6 +147,11 @@ NS_SWIFT_NAME(Firestore) completion:(void (^)(id _Nullable result, NSError *_Nullable error))completion __attribute__((swift_async(none))); // Disable async import due to #9426. +- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options + block:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock + completion: + (void (^)(id _Nullable result, NSError *_Nullable error))completion + __attribute__((swift_async(none))); // Disable async import due to #9426. /** * Creates a write batch, used for performing multiple writes as a single * atomic operation. From e22217b537830464bfbc03241dd345586953b9da Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 13:49:58 -0400 Subject: [PATCH 04/20] FSTTransactionTests.mm: testTransactionOptionsMaxAttempts added --- .../Tests/Integration/FSTTransactionTests.mm | 49 +++++++++++++++++++ .../Public/FirebaseFirestore/FIRFirestore.h | 15 +++++- .../FirebaseFirestore/FIRTransactionOptions.h | 2 +- .../FirebaseFirestore/FirebaseFirestore.h | 1 + 4 files changed, 65 insertions(+), 2 deletions(-) diff --git a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm index 96ed3afc0c2..4ddd228a38f 100644 --- a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm +++ b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm @@ -795,4 +795,53 @@ - (void)testUpdateNestedFieldsTransactionally { [self awaitExpectations]; } +- (void)testTransactionOptionsMaxAttempts { + FIRTransactionOptions *options = [[FIRTransactionOptions alloc] init]; + options.maxAttempts = 4; + + // Note: The logic below to force retries is heavily based on + // testRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges. + + FIRFirestore *firestore = [self firestore]; + FIRDocumentReference *doc = [[firestore collectionWithPath:@"counters"] documentWithAutoID]; + auto attemptCount = std::make_shared(0); + attemptCount->store(0); + + [self writeDocumentRef:doc data:@{@"count" : @"initial value"}]; + + // Skip backoff delays. + [firestore workerQueue]->SkipDelaysForTimerId(TimerId::RetryTransaction); + + XCTestExpectation *expectation = [self expectationWithDescription:@"transaction"]; + [firestore + runTransactionWithOptions:options + block:^id _Nullable(FIRTransaction *transaction, NSError **error) { + ++(*attemptCount); + + [transaction getDocument:doc error:error]; + XCTAssertNil(*error); + + // Do a write outside of the transaction. This will force the transaction to be retried. + dispatch_semaphore_t writeSemaphore = dispatch_semaphore_create(0); + [doc setData:@{ + @"count" : @(attemptCount->load()) + } + completion:^(NSError *) { + dispatch_semaphore_signal(writeSemaphore); + }]; + dispatch_semaphore_wait(writeSemaphore, DISPATCH_TIME_FOREVER); + + // Now try to update the doc from within the transaction. + // This will fail since the document was modified outside of the transaction. + [transaction setData:@{@"count" : @"this write should fail"} forDocument:doc]; + return nil; + } + completion:^(id, NSError *_Nullable error) { + XCTAssertNil(error); + XCTAssertEqual(attemptCount->load(), 4); + [expectation fulfill]; + }]; + [self awaitExpectations]; +} + @end diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h index 15633c38df8..4679376cc16 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h @@ -147,11 +147,24 @@ NS_SWIFT_NAME(Firestore) completion:(void (^)(id _Nullable result, NSError *_Nullable error))completion __attribute__((swift_async(none))); // Disable async import due to #9426. +/** + * Executes the given updateBlock and then attempts to commit the changes applied within an atomic + * transaction. + * + * This is identical to `runTransactionWithBlock` except that it also accepts "options" that affect + * how the transaction is executed. + * + * @param options The options to use for the transaction. + * @param updateBlock The block to execute within the transaction context. + * @param completion The block to call with the result or error of the transaction. This + * block will run even if the client is offline, unless the process is killed. + */ - (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options - block:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock + block:(id _Nullable (^)(FIRTransaction *, NSError **))block completion: (void (^)(id _Nullable result, NSError *_Nullable error))completion __attribute__((swift_async(none))); // Disable async import due to #9426. + /** * Creates a write batch, used for performing multiple writes as a single * atomic operation. diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h b/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h index 86da63b0fc8..ca4bc6ff2a4 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h @@ -30,7 +30,7 @@ NS_SWIFT_NAME(TransactionOptions) - (instancetype)init NS_DESIGNATED_INITIALIZER; /** The maximum number of attempts to commit, after which transaction fails. Default is 5. */ -@property(nonatomic) NSInteger maxAttempts; +@property(nonatomic, assign) NSInteger maxAttempts; @end diff --git a/Firestore/Source/Public/FirebaseFirestore/FirebaseFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FirebaseFirestore.h index 02edf172439..67128e454af 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FirebaseFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FirebaseFirestore.h @@ -31,4 +31,5 @@ #import "FIRSnapshotMetadata.h" #import "FIRTimestamp.h" #import "FIRTransaction.h" +#import "FIRTransactionOptions.h" #import "FIRWriteBatch.h" From 610cc6280f4f3702293b5471c313ce8698bafc7d Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 14:37:34 -0400 Subject: [PATCH 05/20] un-hardcode maxAttempts --- Firestore/core/src/core/sync_engine.cc | 7 ++++--- Firestore/core/src/core/sync_engine.h | 4 ++-- Firestore/core/src/core/transaction_runner.cc | 9 ++++----- Firestore/core/src/core/transaction_runner.h | 3 ++- 4 files changed, 12 insertions(+), 11 deletions(-) diff --git a/Firestore/core/src/core/sync_engine.cc b/Firestore/core/src/core/sync_engine.cc index ff7ac468aaf..9e6a87c3a40 100644 --- a/Firestore/core/src/core/sync_engine.cc +++ b/Firestore/core/src/core/sync_engine.cc @@ -235,17 +235,18 @@ void SyncEngine::RegisterPendingWritesCallback(StatusCallback callback) { std::move(callback)); } -void SyncEngine::Transaction(int retries, +void SyncEngine::Transaction(int max_attempts, const std::shared_ptr& worker_queue, TransactionUpdateCallback update_callback, TransactionResultCallback result_callback) { worker_queue->VerifyIsCurrentQueue(); - HARD_ASSERT(retries >= 0, "Got negative number of retries for transaction"); + HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %d", max_attempts); // Allocate a shared_ptr so that the TransactionRunner can outlive this frame. auto runner = std::make_shared(worker_queue, remote_store_, std::move(update_callback), - std::move(result_callback)); + std::move(result_callback), + max_attempts); runner->Run(); } diff --git a/Firestore/core/src/core/sync_engine.h b/Firestore/core/src/core/sync_engine.h index c02685c3299..51ee0759884 100644 --- a/Firestore/core/src/core/sync_engine.h +++ b/Firestore/core/src/core/sync_engine.h @@ -126,14 +126,14 @@ class SyncEngine : public remote::RemoteStoreCallback, public QueryEventSource { * Runs the given transaction block up to retries times and then calls * completion. * - * @param retries The number of times to try before giving up. + * @param max_attempts The maximum number of times to try before giving up. * @param worker_queue The queue to dispatch sync engine calls to. * @param update_callback The callback to call to execute the user's * transaction. * @param result_callback The callback to call when the transaction is * finished or failed. */ - void Transaction(int retries, + void Transaction(int max_attempts, const std::shared_ptr& worker_queue, core::TransactionUpdateCallback update_callback, core::TransactionResultCallback result_callback); diff --git a/Firestore/core/src/core/transaction_runner.cc b/Firestore/core/src/core/transaction_runner.cc index 6ce1b1003ad..47877a45f02 100644 --- a/Firestore/core/src/core/transaction_runner.cc +++ b/Firestore/core/src/core/transaction_runner.cc @@ -31,9 +31,6 @@ using util::AsyncQueue; using util::Status; using util::TimerId; -/** Maximum number of times a transaction can be attempted before failing. */ -constexpr int kMaxAttemptsCount = 5; - bool IsRetryableTransactionError(const util::Status& error) { // In transactions, the backend will fail outdated reads with // FAILED_PRECONDITION and non-matching document versions with ABORTED. These @@ -48,13 +45,15 @@ bool IsRetryableTransactionError(const util::Status& error) { TransactionRunner::TransactionRunner(const std::shared_ptr& queue, RemoteStore* remote_store, TransactionUpdateCallback update_callback, - TransactionResultCallback result_callback) + TransactionResultCallback result_callback, + int max_attempts) : queue_{queue}, remote_store_{remote_store}, update_callback_{std::move(update_callback)}, result_callback_{std::move(result_callback)}, backoff_{queue_, TimerId::RetryTransaction}, - attempts_remaining_{kMaxAttemptsCount} { + attempts_remaining_{max_attempts} { + HARD_ASSERT(max_attempts > 0, "invalid max_attempts: %d", max_attempts); } void TransactionRunner::Run() { diff --git a/Firestore/core/src/core/transaction_runner.h b/Firestore/core/src/core/transaction_runner.h index 7e3501ab09d..15b08cb02ca 100644 --- a/Firestore/core/src/core/transaction_runner.h +++ b/Firestore/core/src/core/transaction_runner.h @@ -45,7 +45,8 @@ class TransactionRunner TransactionRunner(const std::shared_ptr& queue, remote::RemoteStore* remote_store, core::TransactionUpdateCallback update_callback, - core::TransactionResultCallback result_callback); + core::TransactionResultCallback result_callback, + int max_attempts); /** * Runs the transaction and calls the result_callback_ with the result. From f3411ab74a8fc756a2be1ee4a3f44a29b495ec37 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 14:40:58 -0400 Subject: [PATCH 06/20] CHANGELOG.md entry added --- Firestore/CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index 92388208613..a83deccdde6 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,3 +1,7 @@ +# Unreleased +- [feature] Added the ability to specify the maximum number of attempts to + execute a transaction (#XXXXXXXXXXXXX) + # 9.0.0 - [fixed] Fixed CMake build errors when building with Xcode 13.3.1 (#9702). - [fixed] **Breaking change:** Fixed an issue where returning `nil` from the From da2d8370ad0d80945261296590a297a9a8b99d6c Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 14:58:15 -0400 Subject: [PATCH 07/20] Fix retries/max_attempts assertions --- Firestore/Example/Tests/Integration/FSTTransactionTests.mm | 6 +++--- Firestore/core/src/core/firestore_client.cc | 6 +++--- Firestore/core/src/core/firestore_client.h | 4 ++-- Firestore/core/src/core/sync_engine.cc | 2 +- Firestore/core/src/core/transaction_runner.cc | 2 +- 5 files changed, 10 insertions(+), 10 deletions(-) diff --git a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm index 4ddd228a38f..b1a05c9dc5c 100644 --- a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm +++ b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm @@ -797,7 +797,7 @@ - (void)testUpdateNestedFieldsTransactionally { - (void)testTransactionOptionsMaxAttempts { FIRTransactionOptions *options = [[FIRTransactionOptions alloc] init]; - options.maxAttempts = 4; + options.maxAttempts = 7; // Note: The logic below to force retries is heavily based on // testRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges. @@ -837,8 +837,8 @@ - (void)testTransactionOptionsMaxAttempts { return nil; } completion:^(id, NSError *_Nullable error) { - XCTAssertNil(error); - XCTAssertEqual(attemptCount->load(), 4); + [self assertError:error message:@"the transaction should fail due to retries exhausted" code:FIRFirestoreErrorCodeFailedPrecondition]; + XCTAssertEqual(attemptCount->load(), 7); [expectation fulfill]; }]; [self awaitExpectations]; diff --git a/Firestore/core/src/core/firestore_client.cc b/Firestore/core/src/core/firestore_client.cc index 79952e44cf2..3ac97d8a241 100644 --- a/Firestore/core/src/core/firestore_client.cc +++ b/Firestore/core/src/core/firestore_client.cc @@ -491,7 +491,7 @@ void FirestoreClient::WriteMutations(std::vector&& mutations, }); } -void FirestoreClient::Transaction(int retries, +void FirestoreClient::Transaction(int max_attempts, TransactionUpdateCallback update_callback, TransactionResultCallback result_callback) { VerifyNotTerminated(); @@ -503,8 +503,8 @@ void FirestoreClient::Transaction(int retries, } }; - worker_queue_->Enqueue([this, retries, update_callback, async_callback] { - sync_engine_->Transaction(retries, worker_queue_, + worker_queue_->Enqueue([this, max_attempts, update_callback, async_callback] { + sync_engine_->Transaction(max_attempts, worker_queue_, std::move(update_callback), std::move(async_callback)); }); diff --git a/Firestore/core/src/core/firestore_client.h b/Firestore/core/src/core/firestore_client.h index 51db9f99912..c296c7051d4 100644 --- a/Firestore/core/src/core/firestore_client.h +++ b/Firestore/core/src/core/firestore_client.h @@ -143,9 +143,9 @@ class FirestoreClient : public std::enable_shared_from_this { util::StatusCallback callback); /** - * Tries to execute the transaction in update_callback up to retries times. + * Tries to execute the transaction in update_callback up to max_attempts times. */ - void Transaction(int retries, + void Transaction(int max_attempts, TransactionUpdateCallback update_callback, TransactionResultCallback result_callback); diff --git a/Firestore/core/src/core/sync_engine.cc b/Firestore/core/src/core/sync_engine.cc index 9e6a87c3a40..3e1b2671c1a 100644 --- a/Firestore/core/src/core/sync_engine.cc +++ b/Firestore/core/src/core/sync_engine.cc @@ -240,7 +240,7 @@ void SyncEngine::Transaction(int max_attempts, TransactionUpdateCallback update_callback, TransactionResultCallback result_callback) { worker_queue->VerifyIsCurrentQueue(); - HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %d", max_attempts); + HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %s", max_attempts); // Allocate a shared_ptr so that the TransactionRunner can outlive this frame. auto runner = std::make_shared(worker_queue, remote_store_, diff --git a/Firestore/core/src/core/transaction_runner.cc b/Firestore/core/src/core/transaction_runner.cc index 47877a45f02..1a106e07a9d 100644 --- a/Firestore/core/src/core/transaction_runner.cc +++ b/Firestore/core/src/core/transaction_runner.cc @@ -53,7 +53,7 @@ TransactionRunner::TransactionRunner(const std::shared_ptr& queue, result_callback_{std::move(result_callback)}, backoff_{queue_, TimerId::RetryTransaction}, attempts_remaining_{max_attempts} { - HARD_ASSERT(max_attempts > 0, "invalid max_attempts: %d", max_attempts); + HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %s", max_attempts); } void TransactionRunner::Run() { From b0a24a19823d65b51b4c4251fa5794e26544ee7c Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 15:40:15 -0400 Subject: [PATCH 08/20] FSTTransactionTests.mm: fix test refactoring --- .../Tests/Integration/FSTTransactionTests.mm | 18 +++++++++++++----- 1 file changed, 13 insertions(+), 5 deletions(-) diff --git a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm index b1a05c9dc5c..366228bd426 100644 --- a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm +++ b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm @@ -25,6 +25,7 @@ using firebase::firestore::util::TimerId; @interface FSTTransactionTests : FSTIntegrationTestCase +- (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nullable)options expectNumAttempts:(int)expectedNumAttempts; @end /** @@ -795,10 +796,7 @@ - (void)testUpdateNestedFieldsTransactionally { [self awaitExpectations]; } -- (void)testTransactionOptionsMaxAttempts { - FIRTransactionOptions *options = [[FIRTransactionOptions alloc] init]; - options.maxAttempts = 7; - +- (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nullable)options expectNumAttempts:(int)expectedNumAttempts { // Note: The logic below to force retries is heavily based on // testRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges. @@ -838,10 +836,20 @@ - (void)testTransactionOptionsMaxAttempts { } completion:^(id, NSError *_Nullable error) { [self assertError:error message:@"the transaction should fail due to retries exhausted" code:FIRFirestoreErrorCodeFailedPrecondition]; - XCTAssertEqual(attemptCount->load(), 7); + XCTAssertEqual(attemptCount->load(), expectedNumAttempts); [expectation fulfill]; }]; [self awaitExpectations]; } +- (void)testTransactionOptionsNil { + [self runFailedPreconditionTransactionWithOptions:nil expectNumAttempts:5]; +} + +- (void)testTransactionOptionsMaxAttempts { + FIRTransactionOptions *options = [[FIRTransactionOptions alloc] init]; + options.maxAttempts = 7; + [self runFailedPreconditionTransactionWithOptions:options expectNumAttempts:7]; +} + @end From 5ed74458a019fa51c95a27401842db620ebdeb83 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 16:37:40 -0400 Subject: [PATCH 09/20] Delete C++ TransactionOptions class --- .../Firestore.xcodeproj/project.pbxproj | 14 -- .../Tests/API/FIRTransactionOptionsTests.mm | 11 -- Firestore/Source/API/FIRFirestore.mm | 12 +- .../API/FIRTransactionOptions+Internal.h | 32 ----- Firestore/Source/API/FIRTransactionOptions.mm | 13 +- .../Public/FirebaseFirestore/FIRFirestore.h | 7 +- Firestore/core/src/api/firestore.cc | 9 +- Firestore/core/src/api/firestore.h | 5 +- Firestore/core/src/core/sync_engine.cc | 2 +- .../core/src/core/transaction_options.cc | 53 ------- Firestore/core/src/core/transaction_options.h | 56 -------- .../unit/core/transaction_options_test.cc | 129 ------------------ 12 files changed, 18 insertions(+), 325 deletions(-) delete mode 100644 Firestore/Source/API/FIRTransactionOptions+Internal.h delete mode 100644 Firestore/core/src/core/transaction_options.cc delete mode 100644 Firestore/core/src/core/transaction_options.h delete mode 100644 Firestore/core/test/unit/core/transaction_options_test.cc diff --git a/Firestore/Example/Firestore.xcodeproj/project.pbxproj b/Firestore/Example/Firestore.xcodeproj/project.pbxproj index 65c98e2f0c6..472ae57806c 100644 --- a/Firestore/Example/Firestore.xcodeproj/project.pbxproj +++ b/Firestore/Example/Firestore.xcodeproj/project.pbxproj @@ -65,7 +65,6 @@ 0963F6D7B0F9AE1E24B82866 /* path_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 403DBF6EFB541DFD01582AA3 /* path_test.cc */; }; 096BA3A3703AC1491F281618 /* index.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 395E8B07639E69290A929695 /* index.pb.cc */; }; 098191405BA24F9A7E4F80C6 /* append_only_list_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 5477CDE922EE71C8000FCC1E /* append_only_list_test.cc */; }; - 0A074A769E0E892A200EAA89 /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 0A4E1B5E3E853763AE6ED7AE /* grpc_stream_tester.cc in Sources */ = {isa = PBXBuildFile; fileRef = 87553338E42B8ECA05BA987E /* grpc_stream_tester.cc */; }; 0A52B47C43B7602EE64F53A7 /* cc_compilation_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1B342370EAE3AA02393E33EB /* cc_compilation_test.cc */; }; 0A6FBE65A7FE048BAD562A15 /* FSTGoogleTestTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 54764FAE1FAA21B90085E60A /* FSTGoogleTestTests.mm */; }; @@ -177,7 +176,6 @@ 1F4930A8366F74288121F627 /* create_noop_connectivity_monitor.cc in Sources */ = {isa = PBXBuildFile; fileRef = CF39535F2C41AB0006FA6C0E /* create_noop_connectivity_monitor.cc */; }; 1F56F51EB6DF0951B1F4F85B /* lru_garbage_collector_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 277EAACC4DD7C21332E8496A /* lru_garbage_collector_test.cc */; }; 1F998DDECB54A66222CC66AA /* string_format_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54131E9620ADE678001DF3FF /* string_format_test.cc */; }; - 1FE21B06B7217E6615EC3FEC /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 2045517602D767BD01EA71D9 /* overlay_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = E1459FA70B8FC18DE4B80D0D /* overlay_test.cc */; }; 205601D1C6A40A4DD3BBAA04 /* target_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 526D755F65AC676234F57125 /* target_test.cc */; }; 20814A477D00EA11D0E76631 /* FIRDocumentSnapshotTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04B202154AA00B64F25 /* FIRDocumentSnapshotTests.mm */; }; @@ -255,7 +253,6 @@ 3056418E81BC7584FBE8AD6C /* user_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = CCC9BD953F121B9E29F9AA42 /* user_test.cc */; }; 306E762DC6B829CED4FD995D /* target_id_generator_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB380CF82019382300D97691 /* target_id_generator_test.cc */; }; 3095316962A00DD6A4A2A441 /* counting_query_engine.cc in Sources */ = {isa = PBXBuildFile; fileRef = 99434327614FEFF7F7DC88EC /* counting_query_engine.cc */; }; - 30BA616D48145CB97CDBDADA /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 314D231A9F33E0502611DD20 /* sorted_set_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4C20A36DBB00BCEB75 /* sorted_set_test.cc */; }; 31850B3D5232E8D3F8C4D90C /* memory_remote_document_cache_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 1CA9800A53669EFBFFB824E3 /* memory_remote_document_cache_test.cc */; }; 31A396C81A107D1DEFDF4A34 /* serializer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 61F72C5520BC48FD001A68CB /* serializer_test.cc */; }; @@ -801,7 +798,6 @@ 86494278BE08F10A8AAF9603 /* iterator_adaptors_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 54A0353420A3D8CB003E0143 /* iterator_adaptors_test.cc */; }; 867B370BF2DF84B6AB94B874 /* filesystem_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = BA02DA2FCD0001CFC6EB08DA /* filesystem_testing.cc */; }; 8683BBC3AC7B01937606A83B /* firestore.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 544129D421C2DDC800EFB9CC /* firestore.pb.cc */; }; - 86C341E67E2F60813BC1776B /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; 86E6FC2B7657C35B342E1436 /* sorted_map_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 549CCA4E20A36DBB00BCEB75 /* sorted_map_test.cc */; }; 8705C4856498F66E471A0997 /* FIRWriteBatchTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E06F202154D600B64F25 /* FIRWriteBatchTests.mm */; }; 873B8AEB1B1F5CCA007FD442 /* Main.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 873B8AEA1B1F5CCA007FD442 /* Main.storyboard */; }; @@ -913,7 +909,6 @@ A5175CA2E677E13CC5F23D72 /* document_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = AB6B908320322E4D00CC290A /* document_test.cc */; }; A55266E6C986251D283CE948 /* FIRCursorTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E070202154D600B64F25 /* FIRCursorTests.mm */; }; A5583822218F9D5B1E86FCAC /* overlay_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = E1459FA70B8FC18DE4B80D0D /* overlay_test.cc */; }; - A5739BC7DDF1A76E623AD76B /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; A57EC303CD2D6AA4F4745551 /* FIRFieldValueTests.mm in Sources */ = {isa = PBXBuildFile; fileRef = 5492E04A202154AA00B64F25 /* FIRFieldValueTests.mm */; }; A585BD0F31E90980B5F5FBCA /* local_serializer_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = F8043813A5D16963EC02B182 /* local_serializer_test.cc */; }; A5AB1815C45FFC762981E481 /* write.pb.cc in Sources */ = {isa = PBXBuildFile; fileRef = 544129D921C2DDC800EFB9CC /* write.pb.cc */; }; @@ -957,7 +952,6 @@ AC03C4F1456FB1C0D88E94FF /* query_listener_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 7C3F995E040E9E9C5E8514BB /* query_listener_test.cc */; }; AC6B856ACB12BB28D279693D /* random_access_queue_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = 014C60628830D95031574D15 /* random_access_queue_test.cc */; }; AC6C1E57B18730428CB15E03 /* executor_libdispatch_test.mm in Sources */ = {isa = PBXBuildFile; fileRef = B6FB4689208F9B9100554BA2 /* executor_libdispatch_test.mm */; }; - AC7F651D64F575650EC3FB7F /* transaction_options_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */; }; AC835157AD2BE7AA8D20FB5A /* ConditionalConformanceTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = E3228F51DCDC2E90D5C58F97 /* ConditionalConformanceTests.swift */; }; ACC9369843F5ED3BD2284078 /* timestamp_test.cc in Sources */ = {isa = PBXBuildFile; fileRef = ABF6506B201131F8005F2C74 /* timestamp_test.cc */; }; AD12205540893CEB48647937 /* filesystem_testing.cc in Sources */ = {isa = PBXBuildFile; fileRef = BA02DA2FCD0001CFC6EB08DA /* filesystem_testing.cc */; }; @@ -1725,7 +1719,6 @@ D3CC3DC5338DCAF43A211155 /* README.md */ = {isa = PBXFileReference; includeInIndex = 1; lastKnownFileType = net.daringfireball.markdown; name = README.md; path = ../README.md; sourceTree = ""; }; D5B2593BCB52957D62F1C9D3 /* perf_spec_test.json */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = text.json; path = perf_spec_test.json; sourceTree = ""; }; D5B25E7E7D6873CBA4571841 /* FIRNumericTransformTests.mm */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.cpp.objcpp; path = FIRNumericTransformTests.mm; sourceTree = ""; }; - D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = transaction_options_test.cc; sourceTree = ""; }; D7DF4A6F740086A2D8C0E28E /* Pods_Firestore_Tests_tvOS.framework */ = {isa = PBXFileReference; explicitFileType = wrapper.framework; includeInIndex = 0; path = Pods_Firestore_Tests_tvOS.framework; sourceTree = BUILT_PRODUCTS_DIR; }; D8A6D52723B1BABE1B7B8D8F /* leveldb_overlay_migration_manager_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = leveldb_overlay_migration_manager_test.cc; sourceTree = ""; }; D9D94300B9C02F7069523C00 /* leveldb_snappy_test.cc */ = {isa = PBXFileReference; includeInIndex = 1; path = leveldb_snappy_test.cc; sourceTree = ""; }; @@ -2493,7 +2486,6 @@ B9C261C26C5D311E1E3C0CB9 /* query_test.cc */, AB380CF82019382300D97691 /* target_id_generator_test.cc */, 526D755F65AC676234F57125 /* target_test.cc */, - D7865A152060F11D2A0F3B01 /* transaction_options_test.cc */, CC572A9168BBEF7B83E4BBC5 /* view_snapshot_test.cc */, C7429071B33BDF80A7FA2F8A /* view_test.cc */, ); @@ -3719,7 +3711,6 @@ 2AAEABFD550255271E3BAC91 /* to_string_apple_test.mm in Sources */, 1E2AE064CF32A604DC7BFD4D /* to_string_test.cc in Sources */, AAFA9D7A0A067F2D3D8D5487 /* token_test.cc in Sources */, - 0A074A769E0E892A200EAA89 /* transaction_options_test.cc in Sources */, 5D51D8B166D24EFEF73D85A2 /* transform_operation_test.cc in Sources */, 5F19F66D8B01BA2B97579017 /* tree_sorted_map_test.cc in Sources */, 124AAEE987451820F24EEA8E /* user_test.cc in Sources */, @@ -3922,7 +3913,6 @@ 5BE49546D57C43DDFCDB6FBD /* to_string_apple_test.mm in Sources */, E500AB82DF2E7F3AFDB1AB3F /* to_string_test.cc in Sources */, 5C9B5696644675636A052018 /* token_test.cc in Sources */, - AC7F651D64F575650EC3FB7F /* transaction_options_test.cc in Sources */, 5EE21E86159A1911E9503BC1 /* transform_operation_test.cc in Sources */, 627253FDEC6BB5549FE77F4E /* tree_sorted_map_test.cc in Sources */, 3056418E81BC7584FBE8AD6C /* user_test.cc in Sources */, @@ -4139,7 +4129,6 @@ 95DCD082374F871A86EF905F /* to_string_apple_test.mm in Sources */, 9E656F4FE92E8BFB7F625283 /* to_string_test.cc in Sources */, 96D95E144C383459D4E26E47 /* token_test.cc in Sources */, - A5739BC7DDF1A76E623AD76B /* transaction_options_test.cc in Sources */, 15BF63DFF3A7E9A5376C4233 /* transform_operation_test.cc in Sources */, 54B91B921DA757C64CC67C90 /* tree_sorted_map_test.cc in Sources */, CDB5816537AB1B209C2B72A4 /* user_test.cc in Sources */, @@ -4356,7 +4345,6 @@ F9705E595FC3818F13F6375A /* to_string_apple_test.mm in Sources */, 3BAFCABA851AE1865D904323 /* to_string_test.cc in Sources */, 1B9E54F4C4280A713B825981 /* token_test.cc in Sources */, - 1FE21B06B7217E6615EC3FEC /* transaction_options_test.cc in Sources */, 44EAF3E6EAC0CC4EB2147D16 /* transform_operation_test.cc in Sources */, 3D22F56C0DE7C7256C75DC06 /* tree_sorted_map_test.cc in Sources */, A80D38096052F928B17E1504 /* user_test.cc in Sources */, @@ -4569,7 +4557,6 @@ B68B1E012213A765008977EF /* to_string_apple_test.mm in Sources */, B696858E2214B53900271095 /* to_string_test.cc in Sources */, D50232D696F19C2881AC01CE /* token_test.cc in Sources */, - 86C341E67E2F60813BC1776B /* transaction_options_test.cc in Sources */, D3CB03747E34D7C0365638F1 /* transform_operation_test.cc in Sources */, 549CCA5120A36DBC00BCEB75 /* tree_sorted_map_test.cc in Sources */, 1B816F48012524939CA57CB3 /* user_test.cc in Sources */, @@ -4805,7 +4792,6 @@ 60260A06871DCB1A5F3448D3 /* to_string_apple_test.mm in Sources */, ECED3B60C5718B085AAB14FB /* to_string_test.cc in Sources */, F0EA84FB66813F2BC164EF7C /* token_test.cc in Sources */, - 30BA616D48145CB97CDBDADA /* transaction_options_test.cc in Sources */, 60186935E36CF79E48A0B293 /* transform_operation_test.cc in Sources */, 5DA343D28AE05B0B2FE9FFB3 /* tree_sorted_map_test.cc in Sources */, EF8C005DC4BEA6256D1DBC6F /* user_test.cc in Sources */, diff --git a/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm b/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm index 39fe93866d2..32f5d017c03 100644 --- a/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm +++ b/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm @@ -18,10 +18,6 @@ #import -#import "Firestore/Source/API/FIRTransactionOptions+Internal.h" - -using firebase::firestore::core::TransactionOptions; - NS_ASSUME_NONNULL_BEGIN @interface FIRTransactionOptionsTests : XCTestCase @@ -49,13 +45,6 @@ - (void)testSetMaxAttemptsThrowsOnInvalidValue { XCTAssertThrows(options.maxAttempts = INT32_MIN); } -- (void)testInternalTransactionOptions { - FIRTransactionOptions* optionsPtr = [[FIRTransactionOptions alloc] init]; - optionsPtr.maxAttempts = 99; - TransactionOptions options = [optionsPtr internalTransactionOptions]; - XCTAssertEqual(options.max_attempts(), 99); -} - - (void)testHash { XCTAssertEqual([[[FIRTransactionOptions alloc] init] hash], [[[FIRTransactionOptions alloc] init] hash]); diff --git a/Firestore/Source/API/FIRFirestore.mm b/Firestore/Source/API/FIRFirestore.mm index 7dd96c573fe..24b46d7d671 100644 --- a/Firestore/Source/API/FIRFirestore.mm +++ b/Firestore/Source/API/FIRFirestore.mm @@ -21,7 +21,7 @@ #include #import "FIRFirestoreSettings+Internal.h" -#import "FIRTransactionOptions+Internal.h" +#import "FIRTransactionOptions.h" #import "FirebaseCore/Extension/FIRAppInternal.h" #import "FirebaseCore/Extension/FIRComponentContainer.h" @@ -43,7 +43,6 @@ #include "Firestore/core/src/core/database_info.h" #include "Firestore/core/src/core/event_listener.h" #include "Firestore/core/src/core/transaction.h" -#include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/credentials/credentials_provider.h" #include "Firestore/core/src/model/database_id.h" #include "Firestore/core/src/remote/firebase_metadata_provider.h" @@ -62,6 +61,7 @@ #include "Firestore/core/src/util/string_apple.h" #include "absl/memory/memory.h" +using firebase::firestore::api::kDefaultTransactionMaxAttempts; using firebase::firestore::api::DocumentReference; using firebase::firestore::api::Firestore; using firebase::firestore::api::ListenerRegistration; @@ -338,13 +338,9 @@ void HandleFinalStatus(const Status &status) { result_capture->HandleFinalStatus(status); }; - firebase::firestore::core::TransactionOptions transaction_options; - if (options) { - transaction_options = [options internalTransactionOptions]; - } - + int max_attempts = options ? options.maxAttempts : kDefaultTransactionMaxAttempts; _firestore->RunTransaction(std::move(internalUpdateBlock), std::move(objcTranslator), - transaction_options); + max_attempts); } - (void)runTransactionWithBlock:(id _Nullable (^)(FIRTransaction *, NSError **error))updateBlock diff --git a/Firestore/Source/API/FIRTransactionOptions+Internal.h b/Firestore/Source/API/FIRTransactionOptions+Internal.h deleted file mode 100644 index 9203da9057e..00000000000 --- a/Firestore/Source/API/FIRTransactionOptions+Internal.h +++ /dev/null @@ -1,32 +0,0 @@ -/* - * Copyright 2022 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. - */ - -#import "FIRTransactionOptions.h" - -#import - -#include "Firestore/core/src/core/transaction_options.h" - -NS_ASSUME_NONNULL_BEGIN - -@interface FIRTransactionOptions (Internal) - -/** Converts this FIRTransactionOptions instance into a core::TransactionOptions object. */ -- (firebase::firestore::core::TransactionOptions)internalTransactionOptions; - -@end - -NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/API/FIRTransactionOptions.mm b/Firestore/Source/API/FIRTransactionOptions.mm index 00e91a81e98..e96083f5e66 100644 --- a/Firestore/Source/API/FIRTransactionOptions.mm +++ b/Firestore/Source/API/FIRTransactionOptions.mm @@ -21,12 +21,12 @@ #import -#include "Firestore/core/src/core/transaction_options.h" +#include "Firestore/core/src/api/firestore.h" #include "Firestore/core/src/util/exception.h" NS_ASSUME_NONNULL_BEGIN -using firebase::firestore::core::TransactionOptions; +using firebase::firestore::api::kDefaultTransactionMaxAttempts; using firebase::firestore::util::MakeString; using firebase::firestore::util::ThrowInvalidArgument; @@ -34,8 +34,7 @@ @implementation FIRTransactionOptions - (instancetype)init { if (self = [super init]) { - TransactionOptions defaultOptions; - _maxAttempts = defaultOptions.max_attempts(); + _maxAttempts = kDefaultTransactionMaxAttempts; } return self; } @@ -68,12 +67,6 @@ - (void)setMaxAttempts:(NSInteger)maxAttempts { _maxAttempts = maxAttempts; } -- (TransactionOptions)internalTransactionOptions { - TransactionOptions transactionOptions; - transactionOptions.set_max_attempts(_maxAttempts); - return transactionOptions; -} - @end NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h index 4679376cc16..8965ebc582d 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h @@ -155,14 +155,11 @@ NS_SWIFT_NAME(Firestore) * how the transaction is executed. * * @param options The options to use for the transaction. - * @param updateBlock The block to execute within the transaction context. + * @param block The block to execute within the transaction context. * @param completion The block to call with the result or error of the transaction. This * block will run even if the client is offline, unless the process is killed. */ -- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options - block:(id _Nullable (^)(FIRTransaction *, NSError **))block - completion: - (void (^)(id _Nullable result, NSError *_Nullable error))completion +- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options block:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock completion:(void (^)(id _Nullable result, NSError *_Nullable error))completion __attribute__((swift_async(none))); // Disable async import due to #9426. /** diff --git a/Firestore/core/src/api/firestore.cc b/Firestore/core/src/api/firestore.cc index 8a6e82006cb..071ee4c8d46 100644 --- a/Firestore/core/src/api/firestore.cc +++ b/Firestore/core/src/api/firestore.cc @@ -28,7 +28,6 @@ #include "Firestore/core/src/core/firestore_client.h" #include "Firestore/core/src/core/query.h" #include "Firestore/core/src/core/transaction.h" -#include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/credentials/empty_credentials_provider.h" #include "Firestore/core/src/local/leveldb_persistence.h" #include "Firestore/core/src/model/document_key.h" @@ -58,6 +57,8 @@ using util::Empty; using util::Executor; using util::Status; +const int kDefaultTransactionMaxAttempts = 5; + Firestore::Firestore( model::DatabaseId database_id, std::string persistence_key, @@ -158,11 +159,11 @@ core::Query Firestore::GetCollectionGroup(std::string collection_id) { void Firestore::RunTransaction(core::TransactionUpdateCallback update_callback, core::TransactionResultCallback result_callback, - const core::TransactionOptions& options) { + int max_attempts) { + HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %s", max_attempts); EnsureClientConfigured(); - client_->Transaction(options.max_attempts(), std::move(update_callback), - std::move(result_callback)); + client_->Transaction(max_attempts, std::move(update_callback), std::move(result_callback)); } void Firestore::Terminate(util::StatusCallback callback) { diff --git a/Firestore/core/src/api/firestore.h b/Firestore/core/src/api/firestore.h index 6e7d1816fca..8c18d11d1b5 100644 --- a/Firestore/core/src/api/firestore.h +++ b/Firestore/core/src/api/firestore.h @@ -25,7 +25,6 @@ #include "Firestore/core/src/api/load_bundle_task.h" #include "Firestore/core/src/api/settings.h" #include "Firestore/core/src/core/core_fwd.h" -#include "Firestore/core/src/core/transaction_options.h" #include "Firestore/core/src/credentials/credentials_fwd.h" #include "Firestore/core/src/model/database_id.h" #include "Firestore/core/src/util/byte_stream.h" @@ -47,6 +46,8 @@ struct Empty; namespace api { +extern const int kDefaultTransactionMaxAttempts; + class Firestore : public std::enable_shared_from_this { public: Firestore() = default; @@ -94,7 +95,7 @@ class Firestore : public std::enable_shared_from_this { void RunTransaction(core::TransactionUpdateCallback update_callback, core::TransactionResultCallback result_callback, - const core::TransactionOptions& options = {}); + int max_attempts = kDefaultTransactionMaxAttempts); void Terminate(util::StatusCallback callback); void ClearPersistence(util::StatusCallback callback); diff --git a/Firestore/core/src/core/sync_engine.cc b/Firestore/core/src/core/sync_engine.cc index 3e1b2671c1a..b424e9ef91f 100644 --- a/Firestore/core/src/core/sync_engine.cc +++ b/Firestore/core/src/core/sync_engine.cc @@ -239,8 +239,8 @@ void SyncEngine::Transaction(int max_attempts, const std::shared_ptr& worker_queue, TransactionUpdateCallback update_callback, TransactionResultCallback result_callback) { - worker_queue->VerifyIsCurrentQueue(); HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %s", max_attempts); + worker_queue->VerifyIsCurrentQueue(); // Allocate a shared_ptr so that the TransactionRunner can outlive this frame. auto runner = std::make_shared(worker_queue, remote_store_, diff --git a/Firestore/core/src/core/transaction_options.cc b/Firestore/core/src/core/transaction_options.cc deleted file mode 100644 index 516d29958a8..00000000000 --- a/Firestore/core/src/core/transaction_options.cc +++ /dev/null @@ -1,53 +0,0 @@ -/* - * Copyright 2020 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/core/transaction_options.h" - -#include "Firestore/core/src/util/hard_assert.h" -#include "Firestore/core/src/util/hashing.h" -#include "Firestore/core/src/util/string_format.h" -#include "Firestore/core/src/util/to_string.h" - -namespace firebase { -namespace firestore { -namespace core { - -void TransactionOptions::set_max_attempts(int32_t max_attempts) { - HARD_ASSERT(max_attempts > 0, "invalid max_attempts: %s", - util::ToString(max_attempts)); - max_attempts_ = max_attempts; -} - -std::string TransactionOptions::ToString() const { - return util::StringFormat("TransactionOptions(max_attempts=%s)", - util::ToString(max_attempts_)); -} - -size_t TransactionOptions::Hash() const { - return util::Hash(max_attempts_); -} - -std::ostream& operator<<(std::ostream& os, const TransactionOptions& options) { - return os << options.ToString(); -} - -bool operator==(const TransactionOptions& lhs, const TransactionOptions& rhs) { - return lhs.max_attempts_ == rhs.max_attempts_; -} - -} // namespace core -} // namespace firestore -} // namespace firebase diff --git a/Firestore/core/src/core/transaction_options.h b/Firestore/core/src/core/transaction_options.h deleted file mode 100644 index 6a6b0d3b174..00000000000 --- a/Firestore/core/src/core/transaction_options.h +++ /dev/null @@ -1,56 +0,0 @@ -/* - * Copyright 2020 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_CORE_TRANSACTION_OPTIONS_H_ -#define FIRESTORE_CORE_SRC_CORE_TRANSACTION_OPTIONS_H_ - -#include -#include - -namespace firebase { -namespace firestore { -namespace core { - -class TransactionOptions { - public: - int32_t max_attempts() const { - return max_attempts_; - } - - void set_max_attempts(int32_t max_attempts); - - std::string ToString() const; - - friend bool operator==(const TransactionOptions&, const TransactionOptions&); - - size_t Hash() const; - - private: - int32_t max_attempts_ = 5; -}; - -std::ostream& operator<<(std::ostream&, const TransactionOptions&); - -inline bool operator!=(const TransactionOptions& lhs, - const TransactionOptions& rhs) { - return !(lhs == rhs); -} - -} // namespace core -} // namespace firestore -} // namespace firebase - -#endif // FIRESTORE_CORE_SRC_CORE_TRANSACTION_OPTIONS_H_ diff --git a/Firestore/core/test/unit/core/transaction_options_test.cc b/Firestore/core/test/unit/core/transaction_options_test.cc deleted file mode 100644 index 80b4b870e1f..00000000000 --- a/Firestore/core/test/unit/core/transaction_options_test.cc +++ /dev/null @@ -1,129 +0,0 @@ -/* - * Copyright 2020 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 -#include - -#include "Firestore/core/src/core/transaction_options.h" -#include "Firestore/core/test/unit/testutil/equals_tester.h" - -#include "gtest/gtest.h" - -namespace firebase { -namespace firestore { -namespace core { -namespace { - -TEST(TransactionOptionsTest, ZeroArgConstructor) { - TransactionOptions options; - EXPECT_EQ(options.max_attempts(), 5); -} - -TEST(TransactionOptionsTest, CopyConstructor) { - TransactionOptions options1; - options1.set_max_attempts(999); - - TransactionOptions options2(options1); - - EXPECT_EQ(options2.max_attempts(), 999); -} - -TEST(TransactionOptionsTest, MoveConstructor) { - TransactionOptions options1; - options1.set_max_attempts(999); - - TransactionOptions options2(std::move(options1)); - - EXPECT_EQ(options2.max_attempts(), 999); -} - -TEST(TransactionOptionsTest, CopyAssignmentOperator) { - TransactionOptions options1; - options1.set_max_attempts(999); - TransactionOptions options2; - options2.set_max_attempts(123); - - options2 = options1; - - EXPECT_EQ(options2.max_attempts(), 999); -} - -TEST(TransactionOptionsTest, MoveAssignmentOperator) { - TransactionOptions options1; - options1.set_max_attempts(999); - TransactionOptions options2; - options2.set_max_attempts(123); - - options2 = std::move(options1); - - EXPECT_EQ(options2.max_attempts(), 999); -} - -TEST(TransactionOptionsTest, SetMaxAttempts) { - TransactionOptions options; - options.set_max_attempts(10); - EXPECT_EQ(options.max_attempts(), 10); - options.set_max_attempts(20); - EXPECT_EQ(options.max_attempts(), 20); - options.set_max_attempts(1); - EXPECT_EQ(options.max_attempts(), 1); -} - -TEST(TransactionOptionsTest, SetMaxAttemptsFailsOnInvalidMaxAttempts) { - TransactionOptions options; - EXPECT_ANY_THROW(options.set_max_attempts(0)); - EXPECT_ANY_THROW(options.set_max_attempts(-1)); - EXPECT_ANY_THROW(options.set_max_attempts(-999)); - EXPECT_ANY_THROW(options.set_max_attempts(INT32_MIN)); -} - -TEST(TransactionOptionsTest, ToString) { - TransactionOptions options; - options.set_max_attempts(999); - EXPECT_EQ(options.ToString(), "TransactionOptions(max_attempts=999)"); -} - -TEST(TransactionOptionsTest, WriteToOstream) { - TransactionOptions options; - options.set_max_attempts(999); - std::ostringstream out; - - out << options; - - EXPECT_EQ(out.str(), options.ToString()); -} - -TEST(TransactionOptionsTest, EqualsAndHash) { - TransactionOptions options_max_attempts_1a; - options_max_attempts_1a.set_max_attempts(10); - TransactionOptions options_max_attempts_1b; - options_max_attempts_1b.set_max_attempts(10); - TransactionOptions options_max_attempts_2a; - options_max_attempts_2a.set_max_attempts(99); - TransactionOptions options_max_attempts_2b; - options_max_attempts_2b.set_max_attempts(99); - - testutil::EqualsTester() - .AddEqualityGroup(TransactionOptions(), TransactionOptions()) - .AddEqualityGroup(options_max_attempts_1a, options_max_attempts_1b) - .AddEqualityGroup(options_max_attempts_2a, options_max_attempts_2b) - .TestEquals(); -} - -} // namespace -} // namespace core -} // namespace firestore -} // namespace firebase From a740f0b81966fc6e886ed618ffda23a915234d92 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 16:43:26 -0400 Subject: [PATCH 10/20] format code --- .../Tests/Integration/FSTTransactionTests.mm | 15 +++++++++------ Firestore/Source/API/FIRTransactionOptions.mm | 4 ++-- .../Public/FirebaseFirestore/FIRFirestore.h | 5 ++++- Firestore/core/src/api/firestore.cc | 3 ++- Firestore/core/src/core/firestore_client.h | 3 ++- Firestore/core/src/core/sync_engine.cc | 7 +++---- 6 files changed, 22 insertions(+), 15 deletions(-) diff --git a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm index 366228bd426..3c53e9b02c6 100644 --- a/Firestore/Example/Tests/Integration/FSTTransactionTests.mm +++ b/Firestore/Example/Tests/Integration/FSTTransactionTests.mm @@ -25,7 +25,8 @@ using firebase::firestore::util::TimerId; @interface FSTTransactionTests : FSTIntegrationTestCase -- (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nullable)options expectNumAttempts:(int)expectedNumAttempts; +- (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nullable)options + expectNumAttempts:(int)expectedNumAttempts; @end /** @@ -796,8 +797,9 @@ - (void)testUpdateNestedFieldsTransactionally { [self awaitExpectations]; } -- (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nullable)options expectNumAttempts:(int)expectedNumAttempts { - // Note: The logic below to force retries is heavily based on +- (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nullable)options + expectNumAttempts:(int)expectedNumAttempts { + // Note: The logic below to force retries is heavily based on // testRetriesWhenDocumentThatWasReadWithoutBeingWrittenChanges. FIRFirestore *firestore = [self firestore]; @@ -811,8 +813,7 @@ - (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nul [firestore workerQueue]->SkipDelaysForTimerId(TimerId::RetryTransaction); XCTestExpectation *expectation = [self expectationWithDescription:@"transaction"]; - [firestore - runTransactionWithOptions:options + [firestore runTransactionWithOptions:options block:^id _Nullable(FIRTransaction *transaction, NSError **error) { ++(*attemptCount); @@ -835,7 +836,9 @@ - (void)runFailedPreconditionTransactionWithOptions:(FIRTransactionOptions *_Nul return nil; } completion:^(id, NSError *_Nullable error) { - [self assertError:error message:@"the transaction should fail due to retries exhausted" code:FIRFirestoreErrorCodeFailedPrecondition]; + [self assertError:error + message:@"the transaction should fail due to retries exhausted" + code:FIRFirestoreErrorCodeFailedPrecondition]; XCTAssertEqual(attemptCount->load(), expectedNumAttempts); [expectation fulfill]; }]; diff --git a/Firestore/Source/API/FIRTransactionOptions.mm b/Firestore/Source/API/FIRTransactionOptions.mm index e96083f5e66..33f31e6b2be 100644 --- a/Firestore/Source/API/FIRTransactionOptions.mm +++ b/Firestore/Source/API/FIRTransactionOptions.mm @@ -14,11 +14,11 @@ * limitations under the License. */ +#import "FIRTransactionOptions.h" + #include #include -#import "FIRTransactionOptions.h" - #import #include "Firestore/core/src/api/firestore.h" diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h index 8965ebc582d..c52bc7764e2 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h @@ -159,7 +159,10 @@ NS_SWIFT_NAME(Firestore) * @param completion The block to call with the result or error of the transaction. This * block will run even if the client is offline, unless the process is killed. */ -- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options block:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock completion:(void (^)(id _Nullable result, NSError *_Nullable error))completion +- (void)runTransactionWithOptions:(FIRTransactionOptions *_Nullable)options + block:(id _Nullable (^)(FIRTransaction *, NSError **))updateBlock + completion: + (void (^)(id _Nullable result, NSError *_Nullable error))completion __attribute__((swift_async(none))); // Disable async import due to #9426. /** diff --git a/Firestore/core/src/api/firestore.cc b/Firestore/core/src/api/firestore.cc index 071ee4c8d46..4ea47444dc6 100644 --- a/Firestore/core/src/api/firestore.cc +++ b/Firestore/core/src/api/firestore.cc @@ -163,7 +163,8 @@ void Firestore::RunTransaction(core::TransactionUpdateCallback update_callback, HARD_ASSERT(max_attempts >= 0, "invalid max_attempts: %s", max_attempts); EnsureClientConfigured(); - client_->Transaction(max_attempts, std::move(update_callback), std::move(result_callback)); + client_->Transaction(max_attempts, std::move(update_callback), + std::move(result_callback)); } void Firestore::Terminate(util::StatusCallback callback) { diff --git a/Firestore/core/src/core/firestore_client.h b/Firestore/core/src/core/firestore_client.h index c296c7051d4..3fb69d1aa32 100644 --- a/Firestore/core/src/core/firestore_client.h +++ b/Firestore/core/src/core/firestore_client.h @@ -143,7 +143,8 @@ class FirestoreClient : public std::enable_shared_from_this { util::StatusCallback callback); /** - * Tries to execute the transaction in update_callback up to max_attempts times. + * Tries to execute the transaction in update_callback up to max_attempts + * times. */ void Transaction(int max_attempts, TransactionUpdateCallback update_callback, diff --git a/Firestore/core/src/core/sync_engine.cc b/Firestore/core/src/core/sync_engine.cc index b424e9ef91f..bc47a08cb79 100644 --- a/Firestore/core/src/core/sync_engine.cc +++ b/Firestore/core/src/core/sync_engine.cc @@ -243,10 +243,9 @@ void SyncEngine::Transaction(int max_attempts, worker_queue->VerifyIsCurrentQueue(); // Allocate a shared_ptr so that the TransactionRunner can outlive this frame. - auto runner = std::make_shared(worker_queue, remote_store_, - std::move(update_callback), - std::move(result_callback), - max_attempts); + auto runner = std::make_shared( + worker_queue, remote_store_, std::move(update_callback), + std::move(result_callback), max_attempts); runner->Run(); } From 8c5e5bfd177391fea8fbdaca3138fa3d245accf7 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 16:43:26 -0400 Subject: [PATCH 11/20] format code fix in FIRTransactionOptions.mm --- Firestore/Source/API/FIRTransactionOptions.mm | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firestore/Source/API/FIRTransactionOptions.mm b/Firestore/Source/API/FIRTransactionOptions.mm index 33f31e6b2be..c0703184943 100644 --- a/Firestore/Source/API/FIRTransactionOptions.mm +++ b/Firestore/Source/API/FIRTransactionOptions.mm @@ -16,11 +16,11 @@ #import "FIRTransactionOptions.h" +#import + #include #include -#import - #include "Firestore/core/src/api/firestore.h" #include "Firestore/core/src/util/exception.h" From d55fa7772801dcb3b005aa162f0050bb6b6ddd1d Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Tue, 24 May 2022 22:54:54 -0400 Subject: [PATCH 12/20] Firestore/CHANGELOG.md: add PR number --- Firestore/CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index a83deccdde6..ad933959b2d 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,6 +1,6 @@ # Unreleased - [feature] Added the ability to specify the maximum number of attempts to - execute a transaction (#XXXXXXXXXXXXX) + execute a transaction (#9838) # 9.0.0 - [fixed] Fixed CMake build errors when building with Xcode 13.3.1 (#9702). From 10e200162ec492b053e4948d1cd31d37289b55fa Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 00:41:53 -0400 Subject: [PATCH 13/20] Firestore/CHANGELOG.md: Copied message from https://github.com/firebase/firebase-android-sdk/pull/3664 --- Firestore/CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firestore/CHANGELOG.md b/Firestore/CHANGELOG.md index ad933959b2d..331de784fca 100644 --- a/Firestore/CHANGELOG.md +++ b/Firestore/CHANGELOG.md @@ -1,6 +1,6 @@ # Unreleased -- [feature] Added the ability to specify the maximum number of attempts to - execute a transaction (#9838) +- [feature] Added `TransactionOptions` to control how many times a transaction + will retry commits before failing (#9838). # 9.0.0 - [fixed] Fixed CMake build errors when building with Xcode 13.3.1 (#9702). From ff4e7cac14596f5f62952bc4f107784c446a5ec6 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 00:49:44 -0400 Subject: [PATCH 14/20] FIRTransactionOptionsTests.mm: fix hash tests to actually call the hash method --- Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm b/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm index 32f5d017c03..875eabf33f0 100644 --- a/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm +++ b/Firestore/Example/Tests/API/FIRTransactionOptionsTests.mm @@ -43,6 +43,7 @@ - (void)testSetMaxAttemptsThrowsOnInvalidValue { XCTAssertThrows(options.maxAttempts = 0); XCTAssertThrows(options.maxAttempts = -1); XCTAssertThrows(options.maxAttempts = INT32_MIN); + XCTAssertThrows(options.maxAttempts = INT32_MAX + 1); } - (void)testHash { @@ -58,9 +59,9 @@ - (void)testHash { FIRTransactionOptions* options2b = [[FIRTransactionOptions alloc] init]; options2b.maxAttempts = 11; - XCTAssertEqual(options1a.maxAttempts, options1b.maxAttempts); - XCTAssertEqual(options2a.maxAttempts, options2b.maxAttempts); - XCTAssertNotEqual(options1a.maxAttempts, options2a.maxAttempts); + XCTAssertEqual([options1a hash], [options1b hash]); + XCTAssertEqual([options2a hash], [options2b hash]); + XCTAssertNotEqual([options1a hash], [options2a hash]); } - (void)testIsEqual { From 27b85c8c20e4618d6d2f4ccf66f380569325a159 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 01:01:43 -0400 Subject: [PATCH 15/20] Add [FIRTransactionOptions defaultMaxAttempts] --- Firestore/Source/API/FIRFirestore.mm | 8 +++-- .../API/FIRTransactionOptions+Internal.h | 29 +++++++++++++++++++ Firestore/Source/API/FIRTransactionOptions.mm | 7 ++++- 3 files changed, 41 insertions(+), 3 deletions(-) create mode 100644 Firestore/Source/API/FIRTransactionOptions+Internal.h diff --git a/Firestore/Source/API/FIRFirestore.mm b/Firestore/Source/API/FIRFirestore.mm index 24b46d7d671..96bdebe6dd1 100644 --- a/Firestore/Source/API/FIRFirestore.mm +++ b/Firestore/Source/API/FIRFirestore.mm @@ -21,6 +21,7 @@ #include #import "FIRFirestoreSettings+Internal.h" +#import "FIRTransactionOptions+Internal.h" #import "FIRTransactionOptions.h" #import "FirebaseCore/Extension/FIRAppInternal.h" @@ -61,7 +62,6 @@ #include "Firestore/core/src/util/string_apple.h" #include "absl/memory/memory.h" -using firebase::firestore::api::kDefaultTransactionMaxAttempts; using firebase::firestore::api::DocumentReference; using firebase::firestore::api::Firestore; using firebase::firestore::api::ListenerRegistration; @@ -338,7 +338,11 @@ void HandleFinalStatus(const Status &status) { result_capture->HandleFinalStatus(status); }; - int max_attempts = options ? options.maxAttempts : kDefaultTransactionMaxAttempts; + int max_attempts = [FIRTransactionOptions defaultMaxAttempts]; + if (options) { + max_attempts = options.maxAttempts; + } + _firestore->RunTransaction(std::move(internalUpdateBlock), std::move(objcTranslator), max_attempts); } diff --git a/Firestore/Source/API/FIRTransactionOptions+Internal.h b/Firestore/Source/API/FIRTransactionOptions+Internal.h new file mode 100644 index 00000000000..2264ef4c437 --- /dev/null +++ b/Firestore/Source/API/FIRTransactionOptions+Internal.h @@ -0,0 +1,29 @@ +/* + * Copyright 2022 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. + */ + +#import "FIRTransactionOptions.h" + +#import + +NS_ASSUME_NONNULL_BEGIN + +@interface FIRTransactionOptions (Internal) + ++ (int)defaultMaxAttempts; + +@end + +NS_ASSUME_NONNULL_END diff --git a/Firestore/Source/API/FIRTransactionOptions.mm b/Firestore/Source/API/FIRTransactionOptions.mm index c0703184943..8422a17a071 100644 --- a/Firestore/Source/API/FIRTransactionOptions.mm +++ b/Firestore/Source/API/FIRTransactionOptions.mm @@ -15,6 +15,7 @@ */ #import "FIRTransactionOptions.h" +#import "FIRTransactionOptions+Internal.h" #import @@ -34,11 +35,15 @@ @implementation FIRTransactionOptions - (instancetype)init { if (self = [super init]) { - _maxAttempts = kDefaultTransactionMaxAttempts; + _maxAttempts = [[self class] defaultMaxAttempts]; } return self; } ++ (int)defaultMaxAttempts { + return kDefaultTransactionMaxAttempts; +} + - (BOOL)isEqual:(id)other { if (self == other) { return YES; From 9ac926b0c1650b01fd7fe7d8a32499d338ad41d0 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 01:06:59 -0400 Subject: [PATCH 16/20] firestore.h: Add a TODO to remove the default value for max_attempts --- Firestore/core/src/api/firestore.h | 2 ++ 1 file changed, 2 insertions(+) diff --git a/Firestore/core/src/api/firestore.h b/Firestore/core/src/api/firestore.h index 8c18d11d1b5..2282fb68823 100644 --- a/Firestore/core/src/api/firestore.h +++ b/Firestore/core/src/api/firestore.h @@ -93,6 +93,8 @@ class Firestore : public std::enable_shared_from_this { WriteBatch GetBatch(); core::Query GetCollectionGroup(std::string collection_id); + // TODO: Remove the default value of `max_attempts` once the CPP SDK has + // been updated to specify an explicit value. void RunTransaction(core::TransactionUpdateCallback update_callback, core::TransactionResultCallback result_callback, int max_attempts = kDefaultTransactionMaxAttempts); From c195cadb8731397e5f268222ba878aab55461f48 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 01:14:43 -0400 Subject: [PATCH 17/20] firestore.h: update TODO comment --- Firestore/core/src/api/firestore.h | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/Firestore/core/src/api/firestore.h b/Firestore/core/src/api/firestore.h index 2282fb68823..0ce045c8f92 100644 --- a/Firestore/core/src/api/firestore.h +++ b/Firestore/core/src/api/firestore.h @@ -93,8 +93,8 @@ class Firestore : public std::enable_shared_from_this { WriteBatch GetBatch(); core::Query GetCollectionGroup(std::string collection_id); - // TODO: Remove the default value of `max_attempts` once the CPP SDK has - // been updated to specify an explicit value. + // TODO(dconeybe): Remove the default value of `max_attempts` once + // the firebase-cpp-sdk has been updated to specify an explicit value. void RunTransaction(core::TransactionUpdateCallback update_callback, core::TransactionResultCallback result_callback, int max_attempts = kDefaultTransactionMaxAttempts); From 0fe625cf72254bfb8b0a899dd2c006836fafe475 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 07:43:11 -0400 Subject: [PATCH 18/20] FIRFirestore.h: block -> updateBlock in @param tag --- Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h index c52bc7764e2..f9d6373ded7 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h @@ -155,7 +155,7 @@ NS_SWIFT_NAME(Firestore) * how the transaction is executed. * * @param options The options to use for the transaction. - * @param block The block to execute within the transaction context. + * @param updateBlock The block to execute within the transaction context. * @param completion The block to call with the result or error of the transaction. This * block will run even if the client is offline, unless the process is killed. */ From 6646bcaeca678ec679f03137412f7af8425440bd Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Wed, 25 May 2022 13:25:23 -0400 Subject: [PATCH 19/20] Update public API docs of FIRFirestore.h and FIRTransactionOptions.h --- .../Public/FirebaseFirestore/FIRFirestore.h | 30 +++++++++++++++++-- .../FirebaseFirestore/FIRTransactionOptions.h | 9 ++++-- 2 files changed, 33 insertions(+), 6 deletions(-) diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h index f9d6373ded7..348a44af657 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRFirestore.h @@ -151,10 +151,34 @@ NS_SWIFT_NAME(Firestore) * Executes the given updateBlock and then attempts to commit the changes applied within an atomic * transaction. * - * This is identical to `runTransactionWithBlock` except that it also accepts "options" that affect - * how the transaction is executed. + * The maximum number of writes allowed in a single transaction is 500, but note that each usage of + * `FieldValue.serverTimestamp()`, `FieldValue.arrayUnion()`, `FieldValue.arrayRemove()`, or + * `FieldValue.increment()` inside a transaction counts as an additional write. + * + * In the updateBlock, a set of reads and writes can be performed atomically using the + * `Transaction` object passed to the block. After the updateBlock is run, Firestore will attempt + * to apply the changes to the server. If any of the data read has been modified outside of this + * transaction since being read, then the transaction will be retried by executing the updateBlock + * again. If the transaction still fails after the attempting the number of times specified by the + * `max_attempts` property of the given `TransactionOptions` object, then the transaction will fail. + * If the given `TransactionOptions` is `nil`, then the default `max_attempts` of 5 will be used. + * + * Since the updateBlock may be executed multiple times, it should avoiding doing anything that + * would cause side effects. + * + * Any value maybe be returned from the updateBlock. If the transaction is successfully committed, + * then the completion block will be passed that value. The updateBlock also has an `NSErrorPointer` + * out parameter. If this is set, then the transaction will not attempt to commit, and the given + * error will be passed to the completion block. + * + * The `Transaction` object passed to the updateBlock contains methods for accessing documents + * and collections. Unlike other firestore access, data accessed with the transaction will not + * reflect local changes that have not been committed. For this reason, it is required that all + * reads are performed before any writes. Transactions must be performed while online. Otherwise, + * reads will fail, the final commit will fail, and the completion block will return an error. * - * @param options The options to use for the transaction. + * @param options The transaction options for controlling execution, or `nil` to use the default + * transaction options. * @param updateBlock The block to execute within the transaction context. * @param completion The block to call with the result or error of the transaction. This * block will run even if the client is offline, unless the process is killed. diff --git a/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h b/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h index ca4bc6ff2a4..2dc21575cc4 100644 --- a/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h +++ b/Firestore/Source/Public/FirebaseFirestore/FIRTransactionOptions.h @@ -18,14 +18,17 @@ NS_ASSUME_NONNULL_BEGIN -/** Settings used to configure a `Firestore` instance. */ +/** + * Options to customize the behavior of `Firestore.runTransactionWithOptions()`. + */ NS_SWIFT_NAME(TransactionOptions) @interface FIRTransactionOptions : NSObject /** - * Creates and returns an empty `FirestoreTransactionOptions` object. + * Creates and returns a new `TransactionOptions` object with all properties initialized to their + * default values. * - * @return The created `FirestoreTransactionOptions` object. + * @return The created `TransactionOptions` object. */ - (instancetype)init NS_DESIGNATED_INITIALIZER; From adf78534dced32b11e19c98469c4c1a0c2c347c2 Mon Sep 17 00:00:00 2001 From: Denver Coneybeare Date: Mon, 30 May 2022 11:08:37 -0400 Subject: [PATCH 20/20] FIRTransactionOptions.mm: remove unused using declaration for MakeString --- Firestore/Source/API/FIRTransactionOptions.mm | 1 - 1 file changed, 1 deletion(-) diff --git a/Firestore/Source/API/FIRTransactionOptions.mm b/Firestore/Source/API/FIRTransactionOptions.mm index 8422a17a071..6ca26c92b41 100644 --- a/Firestore/Source/API/FIRTransactionOptions.mm +++ b/Firestore/Source/API/FIRTransactionOptions.mm @@ -28,7 +28,6 @@ NS_ASSUME_NONNULL_BEGIN using firebase::firestore::api::kDefaultTransactionMaxAttempts; -using firebase::firestore::util::MakeString; using firebase::firestore::util::ThrowInvalidArgument; @implementation FIRTransactionOptions