From 41de6e445051e22cdef2e65aa647b60f18bca85f Mon Sep 17 00:00:00 2001 From: Anthony Rueda Date: Tue, 13 Sep 2022 16:50:35 -0700 Subject: [PATCH] Remove nested callback function logic from CredentialStorage g3 implementation. Change instances of passing callback by value to moving callback. PiperOrigin-RevId: 474157705 --- internal/platform/credential_storage_impl.cc | 12 ++-- .../platform/credential_storage_impl_test.cc | 48 ++++++-------- .../g3/credential_storage_impl.cc | 63 +++---------------- .../g3/credential_storage_impl.h | 14 +---- 4 files changed, 35 insertions(+), 102 deletions(-) diff --git a/internal/platform/credential_storage_impl.cc b/internal/platform/credential_storage_impl.cc index 1cf9daa1..584f5319 100644 --- a/internal/platform/credential_storage_impl.cc +++ b/internal/platform/credential_storage_impl.cc @@ -11,9 +11,11 @@ // 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 "internal/platform/credential_storage_impl.h" +#include +#include + namespace location { namespace nearby { @@ -28,21 +30,21 @@ void CredentialStorageImpl::SaveCredentials( api::SaveCredentialsResultCallback callback) { return impl_->SaveCredentials(manager_app_id, account_name, private_credentials, public_credentials, - public_credential_type, callback); + public_credential_type, std::move(callback)); } void CredentialStorageImpl::GetPrivateCredentials( const api::CredentialSelector& credential_selector, api::GetPrivateCredentialsResultCallback callback) { - return impl_->GetPrivateCredentials(credential_selector, callback); + return impl_->GetPrivateCredentials(credential_selector, std::move(callback)); } void CredentialStorageImpl::GetPublicCredentials( const api::CredentialSelector& credential_selector, api::PublicCredentialType public_credential_type, api::GetPublicCredentialsResultCallback callback) { - return impl_->GetPublicCredentials(credential_selector, - public_credential_type, callback); + return impl_->GetPublicCredentials( + credential_selector, public_credential_type, std::move(callback)); } } // namespace nearby diff --git a/internal/platform/credential_storage_impl_test.cc b/internal/platform/credential_storage_impl_test.cc index c5531b90..7a97eeb9 100644 --- a/internal/platform/credential_storage_impl_test.cc +++ b/internal/platform/credential_storage_impl_test.cc @@ -49,17 +49,17 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPrivateCredentials) { public_credentials.push_back(public_credential); // Define SaveCredentialsResultCallback bool successfull_save{false}; - auto save_creds_lambda = + api::SaveCredentialsResultCallback save_creds_callback; + save_creds_callback.credentials_saved_cb = [&successfull_save](api::CredentialOperationStatus status) { if (status == api::CredentialOperationStatus::kSucceeded) { successfull_save = true; } }; - api::SaveCredentialsResultCallback save_creds_callback; - save_creds_callback.credentials_saved_cb = save_creds_lambda; // Define GetPrivateCredentialsResultCallback bool get_private_cred_succeeded{false}; - auto credentials_fetched_lambda = + api::GetPrivateCredentialsResultCallback get_private_creds_callback; + get_private_creds_callback.credentials_fetched_cb = [&private_credentials, &get_private_cred_succeeded]( const std::vector &private_creds) { auto private_cred = private_creds[0]; @@ -68,29 +68,24 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPrivateCredentials) { get_private_cred_succeeded = true; } }; - auto get_credentials_failed_lambda = + get_private_creds_callback.get_credentials_failed_cb = [&get_private_cred_succeeded](api::CredentialOperationStatus status) { if (status == api::CredentialOperationStatus::kFailed) { get_private_cred_succeeded = false; } }; - api::GetPrivateCredentialsResultCallback get_private_creds_callback; - get_private_creds_callback.credentials_fetched_cb = - credentials_fetched_lambda; - get_private_creds_callback.get_credentials_failed_cb = - get_credentials_failed_lambda; // Create CredentialStorageImpl object to test SaveCredentials & // GetPrivateCredentials CredentialStorageImpl creds_storage; creds_storage.GetPrivateCredentials(credential_selector, get_private_creds_callback); EXPECT_FALSE(get_private_cred_succeeded); - creds_storage.SaveCredentials(manager_app_id, account_name, - private_credentials, public_credentials, - public_credential_type, save_creds_callback); + creds_storage.SaveCredentials( + manager_app_id, account_name, private_credentials, public_credentials, + public_credential_type, std::move(save_creds_callback)); EXPECT_TRUE(successfull_save); creds_storage.GetPrivateCredentials(credential_selector, - get_private_creds_callback); + std::move(get_private_creds_callback)); EXPECT_TRUE(get_private_cred_succeeded); } @@ -114,17 +109,17 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPublicCredentials) { public_credentials.push_back(public_credential); // Define SaveCredentialsResultCallback bool successfull_save{false}; - auto save_creds_lambda = + api::SaveCredentialsResultCallback save_creds_callback; + save_creds_callback.credentials_saved_cb = [&successfull_save](api::CredentialOperationStatus status) { if (status == api::CredentialOperationStatus::kSucceeded) { successfull_save = true; } }; - api::SaveCredentialsResultCallback save_creds_callback; - save_creds_callback.credentials_saved_cb = save_creds_lambda; // Define GetPublicCredentialsResultCallback bool get_public_cred_succeeded{false}; - auto credentials_fetched_lambda = + api::GetPublicCredentialsResultCallback get_public_creds_callback; + get_public_creds_callback.credentials_fetched_cb = [&public_credentials, &get_public_cred_succeeded]( const std::vector &public_creds) { auto public_cred = public_creds[0]; @@ -133,28 +128,25 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPublicCredentials) { get_public_cred_succeeded = true; } }; - auto get_credentials_failed_lambda = + get_public_creds_callback.get_credentials_failed_cb = [&get_public_cred_succeeded](api::CredentialOperationStatus status) { if (status == api::CredentialOperationStatus::kFailed) { get_public_cred_succeeded = false; } }; - api::GetPublicCredentialsResultCallback get_public_creds_callback; - get_public_creds_callback.credentials_fetched_cb = credentials_fetched_lambda; - get_public_creds_callback.get_credentials_failed_cb = - get_credentials_failed_lambda; // Create CredentialStorageImpl object to test SaveCredentials & // GetPublicCredentials CredentialStorageImpl creds_storage; creds_storage.GetPublicCredentials( credential_selector, public_credential_type, get_public_creds_callback); EXPECT_FALSE(get_public_cred_succeeded); - creds_storage.SaveCredentials(manager_app_id, account_name, - private_credentials, public_credentials, - public_credential_type, save_creds_callback); + creds_storage.SaveCredentials( + manager_app_id, account_name, private_credentials, public_credentials, + public_credential_type, std::move(save_creds_callback)); EXPECT_TRUE(successfull_save); - creds_storage.GetPublicCredentials( - credential_selector, public_credential_type, get_public_creds_callback); + creds_storage.GetPublicCredentials(credential_selector, + public_credential_type, + std::move(get_public_creds_callback)); EXPECT_TRUE(get_public_cred_succeeded); } diff --git a/internal/platform/implementation/g3/credential_storage_impl.cc b/internal/platform/implementation/g3/credential_storage_impl.cc index ac1fb8b1..46f2fbdd 100644 --- a/internal/platform/implementation/g3/credential_storage_impl.cc +++ b/internal/platform/implementation/g3/credential_storage_impl.cc @@ -34,74 +34,25 @@ void CredentialStorageImpl::SaveCredentials( const std::vector& public_credentials, api::PublicCredentialType public_credential_type, api::SaveCredentialsResultCallback callback) { - NEARBY_LOGS(INFO) << "G3 Save Credentials for account: " << account_name; - auto save_public_creds_lambda = - [&callback](api::CredentialOperationStatus status) { - if (status == api::CredentialOperationStatus::kSucceeded) { - NEARBY_LOGS(INFO) << "Saving Public Credentials succeeded!"; - } else if (status == api::CredentialOperationStatus::kFailed) { - NEARBY_LOGS(WARNING) << "Saving Public Credentials failed!"; - } else { - NEARBY_LOGS(WARNING) << "Saving Public Credentials status unknown!"; - } - callback.credentials_saved_cb(status); - }; - api::SaveCredentialsResultCallback save_public_creds_cb; - save_public_creds_cb.credentials_saved_cb = save_public_creds_lambda; - auto save_private_creds_lambda = [manager_app_id, account_name, - &public_credentials, public_credential_type, - &save_public_creds_cb, &callback, this]( - api::CredentialOperationStatus status) { - if (status == api::CredentialOperationStatus::kSucceeded) { - NEARBY_LOGS(INFO) << "Saving Private Credentials succeeded!"; - SavePublicCredentials(manager_app_id, account_name, public_credentials, - public_credential_type, save_public_creds_cb); - } else if (status == api::CredentialOperationStatus::kFailed) { - NEARBY_LOGS(WARNING) << "Saving Private Credentials failed!"; - callback.credentials_saved_cb(status); - } else { - NEARBY_LOGS(WARNING) << "Saving Private Credentials status unknown!"; - callback.credentials_saved_cb(status); - } - }; - api::SaveCredentialsResultCallback save_private_creds_cb; - save_private_creds_cb.credentials_saved_cb = save_private_creds_lambda; - - // Invoke SavePrivateCredentials to kick off chain of credential storage. - SavePrivateCredentials(manager_app_id, account_name, private_credentials, - save_private_creds_cb); -} - -void CredentialStorageImpl::SavePrivateCredentials( - absl::string_view manager_app_id, absl::string_view account_name, - const std::vector& private_credentials, - api::SaveCredentialsResultCallback callback) { NEARBY_LOGS(INFO) << "G3 Save Private Credentials for account: " << account_name; - auto key_value = std::make_pair(std::make_pair(manager_app_id, account_name), - private_credentials); - auto res = private_credentials_map_.insert(key_value); - if (!res.second) { + auto private_key_value = std::make_pair( + std::make_pair(manager_app_id, account_name), private_credentials); + auto private_res = private_credentials_map_.insert(private_key_value); + if (!private_res.second) { NEARBY_LOGS(WARNING) << "Credentials already saved in map. Overriding previous creds!"; private_credentials_map_[std::make_pair(manager_app_id, account_name)] = private_credentials; } - callback.credentials_saved_cb(api::CredentialOperationStatus::kSucceeded); -} -void CredentialStorageImpl::SavePublicCredentials( - absl::string_view manager_app_id, absl::string_view account_name, - const std::vector& public_credentials, - api::PublicCredentialType public_credential_type, - api::SaveCredentialsResultCallback callback) { NEARBY_LOGS(INFO) << "G3 Save Public Credentials for account: " << account_name; - auto key_value = std::make_pair( + auto public_key_value = std::make_pair( std::make_tuple(manager_app_id, account_name, public_credential_type), public_credentials); - auto res = public_credentials_map_.insert(key_value); - if (!res.second) { + auto public_res = public_credentials_map_.insert(public_key_value); + if (!public_res.second) { NEARBY_LOGS(WARNING) << "Credentials already saved in map. Overriding previous creds!"; public_credentials_map_[std::make_tuple(manager_app_id, account_name, diff --git a/internal/platform/implementation/g3/credential_storage_impl.h b/internal/platform/implementation/g3/credential_storage_impl.h index 28c95ebb..1c0e7bcf 100644 --- a/internal/platform/implementation/g3/credential_storage_impl.h +++ b/internal/platform/implementation/g3/credential_storage_impl.h @@ -39,6 +39,7 @@ class CredentialStorageImpl : public api::CredentialStorage { explicit CredentialStorageImpl() = default; ~CredentialStorageImpl() override = default; + // Used to save private and public credentials. void SaveCredentials(absl::string_view manager_app_id, absl::string_view account_name, const std::vector<::nearby::internal::PrivateCredential>& @@ -48,19 +49,6 @@ class CredentialStorageImpl : public api::CredentialStorage { api::PublicCredentialType public_credential_type, api::SaveCredentialsResultCallback callback) override; - void SavePrivateCredentials( - absl::string_view manager_app_id, absl::string_view account_name, - const std::vector<::nearby::internal::PrivateCredential>& - private_credentials, - api::SaveCredentialsResultCallback callback); - - void SavePublicCredentials( - absl::string_view manager_app_id, absl::string_view account_name, - const std::vector<::nearby::internal::PublicCredential>& - public_credentials, - api::PublicCredentialType public_credential_type, - api::SaveCredentialsResultCallback callback); - // Used to fetch private creds when broadcasting. void GetPrivateCredentials( const api::CredentialSelector& credential_selector,