diff --git a/internal/platform/credential_storage_impl.cc b/internal/platform/credential_storage_impl.cc index 4be1e4fd..f8a10102 100644 --- a/internal/platform/credential_storage_impl.cc +++ b/internal/platform/credential_storage_impl.cc @@ -23,7 +23,6 @@ namespace nearby { using ::nearby::internal::PrivateCredential; using ::nearby::internal::PublicCredential; using ::nearby::presence::CredentialSelector; -using ::nearby::presence::GenerateCredentialsCallback; using ::nearby::presence::GetPrivateCredentialsResultCallback; using ::nearby::presence::GetPublicCredentialsResultCallback; using ::nearby::presence::PublicCredentialType; diff --git a/internal/platform/credential_storage_impl_test.cc b/internal/platform/credential_storage_impl_test.cc index bfcb84e4..6b60c74e 100644 --- a/internal/platform/credential_storage_impl_test.cc +++ b/internal/platform/credential_storage_impl_test.cc @@ -36,7 +36,6 @@ using ::nearby::internal::IdentityType; using ::nearby::internal::PrivateCredential; using ::nearby::internal::PublicCredential; using ::nearby::presence::CredentialSelector; -using ::nearby::presence::GenerateCredentialsCallback; using ::nearby::presence::GetPrivateCredentialsResultCallback; using ::nearby::presence::GetPublicCredentialsResultCallback; using ::nearby::presence::PublicCredentialType; @@ -91,21 +90,15 @@ absl::StatusOr> GetPrivateCredentials( CredentialSelector selector = {.manager_app_id = std::string(manager_app_id), .account_name = std::string(account_name), .identity_type = identity_type}; - std::vector private_credentials; - absl::Status get_status = absl::OkStatus(); + absl::StatusOr> private_credentials; credential_storage.GetPrivateCredentials( - selector, GetPrivateCredentialsResultCallback{ - .credentials_fetched_cb = - [&](std::vector credentials) { - private_credentials = std::move(credentials); - }, - .get_credentials_failed_cb = - [&](absl::Status status) { get_status = status; }}); - if (get_status.ok()) { - return private_credentials; - } else { - return get_status; - } + selector, + GetPrivateCredentialsResultCallback{ + .credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + private_credentials = std::move(credentials); + }}); + return private_credentials; } absl::StatusOr> GetPublicCredentials( @@ -116,22 +109,15 @@ absl::StatusOr> GetPublicCredentials( CredentialSelector selector = {.manager_app_id = std::string(manager_app_id), .account_name = std::string(account_name), .identity_type = identity_type}; - std::vector public_credentials; - absl::Status get_status = absl::OkStatus(); + absl::StatusOr> public_credentials; credential_storage.GetPublicCredentials( selector, credential_type, GetPublicCredentialsResultCallback{ .credentials_fetched_cb = - [&](std::vector credentials) { + [&](absl::StatusOr> credentials) { public_credentials = std::move(credentials); - }, - .get_credentials_failed_cb = - [&](absl::Status status) { get_status = status; }}); - if (get_status.ok()) { - return public_credentials; - } else { - return get_status; - } + }}); + return public_credentials; } absl::Status SaveCredentials(CredentialStorageImpl& credential_storage, diff --git a/internal/platform/implementation/credential_callbacks.h b/internal/platform/implementation/credential_callbacks.h index 404e758e..ec26bb45 100644 --- a/internal/platform/implementation/credential_callbacks.h +++ b/internal/platform/implementation/credential_callbacks.h @@ -22,6 +22,7 @@ #include "absl/functional/any_invocable.h" #include "absl/status/status.h" +#include "absl/status/statusor.h" #include "absl/strings/str_format.h" #include "absl/strings/string_view.h" #include "internal/proto/credential.pb.h" @@ -53,8 +54,9 @@ struct SaveCredentialsResultCallback { absl::AnyInvocable credentials_saved_cb; }; -struct GenerateCredentialsCallback { - absl::AnyInvocable)> +struct GenerateCredentialsResultCallback { + absl::AnyInvocable>)> credentials_generated_cb; }; @@ -63,15 +65,15 @@ struct UpdateRemotePublicCredentialsCallback { }; struct GetPrivateCredentialsResultCallback { - absl::AnyInvocable)> + absl::AnyInvocable>)> credentials_fetched_cb; - absl::AnyInvocable get_credentials_failed_cb; }; struct GetPublicCredentialsResultCallback { - absl::AnyInvocable)> + absl::AnyInvocable>)> credentials_fetched_cb; - absl::AnyInvocable get_credentials_failed_cb; }; inline std::ostream& operator<<(std::ostream& os, diff --git a/internal/platform/implementation/g3/credential_storage_impl.cc b/internal/platform/implementation/g3/credential_storage_impl.cc index f1a446fc..8c53ec29 100644 --- a/internal/platform/implementation/g3/credential_storage_impl.cc +++ b/internal/platform/implementation/g3/credential_storage_impl.cc @@ -113,7 +113,7 @@ void CredentialStorageImpl::GetPrivateCredentials( if (private_credentials_map_.find(key) == private_credentials_map_.end()) { NEARBY_LOGS(WARNING) << "There are no Private Credentials stored for key:" << std::get<0>(key) << ", " << std::get<1>(key); - std::move(callback.get_credentials_failed_cb)(absl::NotFoundError( + std::move(callback.credentials_fetched_cb)(absl::NotFoundError( absl::StrFormat("No private credentials for %v", credential_selector))); return; } @@ -121,7 +121,7 @@ void CredentialStorageImpl::GetPrivateCredentials( private_credentials_map_[key]; FilterIdentityType(private_credentials, credential_selector.identity_type); if (private_credentials.empty()) { - std::move(callback.get_credentials_failed_cb)(absl::NotFoundError( + std::move(callback.credentials_fetched_cb)(absl::NotFoundError( absl::StrFormat("No private credentials for %v", credential_selector))); return; } @@ -141,7 +141,7 @@ void CredentialStorageImpl::GetPublicCredentials( NEARBY_LOGS(WARNING) << "There are no Public Credentials stored for key:" << std::get<0>(key) << ", " << std::get<1>(key) << ", " << std::get<2>(key); - std::move(callback.get_credentials_failed_cb)(absl::NotFoundError( + std::move(callback.credentials_fetched_cb)(absl::NotFoundError( absl::StrFormat("No public credentials for %v", credential_selector))); return; } @@ -149,7 +149,7 @@ void CredentialStorageImpl::GetPublicCredentials( public_credentials_map_[key]; FilterIdentityType(public_credentials, credential_selector.identity_type); if (public_credentials.empty()) { - std::move(callback.get_credentials_failed_cb)(absl::NotFoundError( + std::move(callback.credentials_fetched_cb)(absl::NotFoundError( absl::StrFormat("No public credentials for %v", credential_selector))); return; } diff --git a/presence/implementation/broadcast_manager.cc b/presence/implementation/broadcast_manager.cc index b938c912..1beff863 100644 --- a/presence/implementation/broadcast_manager.cc +++ b/presence/implementation/broadcast_manager.cc @@ -68,21 +68,23 @@ void BroadcastManager::FetchCredentials( GetPrivateCredentialsResultCallback{ .credentials_fetched_cb = [this, id, broadcast_request = std::move(broadcast_request)]( - std::vector<::nearby::internal::PrivateCredential> + absl::StatusOr< + std::vector<::nearby::internal::PrivateCredential>> credentials) { + if (!credentials.ok()) { + NEARBY_LOGS(WARNING) + << "Failed to fetch credentials, status: " + << credentials.status(); + NotifyStartCallbackStatus(id, credentials.status()); + return; + } RunOnServiceControllerThread( "advertise-non-public", [this, id, broadcast_request = std::move(broadcast_request), - credentials = std::move(credentials)]() + credentials = std::move(*credentials)]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(executor_) { Advertise(id, broadcast_request, credentials); }); - }, - .get_credentials_failed_cb = - [this, id](absl::Status status) { - NEARBY_LOGS(WARNING) - << "Failed to fetch credentials, status: " << status; - NotifyStartCallbackStatus(id, status); }}); } diff --git a/presence/implementation/credential_manager.h b/presence/implementation/credential_manager.h index 0d12318b..f16876d2 100644 --- a/presence/implementation/credential_manager.h +++ b/presence/implementation/credential_manager.h @@ -47,7 +47,7 @@ class CredentialManager { const nearby::internal::DeviceMetadata& device_metadata, const std::vector& credential_selectors, int credential_life_cycle_days, int contiguous_copy_of_credentials, - GenerateCredentialsCallback credentials_generated_cb) = 0; + GenerateCredentialsResultCallback credentials_generated_cb) = 0; // Update remote public credentials. virtual void UpdateRemotePublicCredentials( diff --git a/presence/implementation/credential_manager_impl.cc b/presence/implementation/credential_manager_impl.cc index ec2c05ef..7c16c05b 100644 --- a/presence/implementation/credential_manager_impl.cc +++ b/presence/implementation/credential_manager_impl.cc @@ -58,7 +58,7 @@ void CredentialManagerImpl::GenerateCredentials( const DeviceMetadata& device_metadata, const std::vector& credential_selectors, int credential_life_cycle_days, int contiguous_copy_of_credentials, - GenerateCredentialsCallback credentials_generated_cb) { + GenerateCredentialsResultCallback credentials_generated_cb) { std::vector public_credentials; std::vector private_credentials; @@ -99,7 +99,7 @@ void CredentialManagerImpl::GenerateCredentials( } else { NEARBY_LOGS(WARNING) << "Save credentials failed with: " << status; - std::move(callback.credentials_generated_cb)({}); + std::move(callback.credentials_generated_cb)(status); } }}); } @@ -289,16 +289,15 @@ CredentialManagerImpl::GetPrivateCredentialsSync( Future> result; GetPrivateCredentials( credential_selector, - { - .credentials_fetched_cb = - [result](std::vector credentials) mutable { - result.Set(credentials); - }, - .get_credentials_failed_cb = - [result](absl::Status status) mutable { - result.SetException({Exception::kFailed}); - }, - }); + {.credentials_fetched_cb = + [result](absl::StatusOr> + credentials) mutable { + if (!credentials.ok()) { + result.SetException({Exception::kFailed}); + } else { + result.Set(std::move(*credentials)); + } + }}); return result.Get(timeout); } @@ -309,16 +308,15 @@ CredentialManagerImpl::GetPublicCredentialsSync( Future> result; GetPublicCredentials( credential_selector, public_credential_type, - { - .credentials_fetched_cb = - [result](std::vector credentials) mutable { - result.Set(credentials); - }, - .get_credentials_failed_cb = - [result](absl::Status status) mutable { - result.SetException({Exception::kFailed}); - }, - }); + {.credentials_fetched_cb = + [result](absl::StatusOr> + credentials) mutable { + if (!credentials.ok()) { + result.SetException({Exception::kFailed}); + } else { + result.Set(std::move(*credentials)); + } + }}); return result.Get(timeout); } diff --git a/presence/implementation/credential_manager_impl.h b/presence/implementation/credential_manager_impl.h index dc31a66f..141fa65a 100644 --- a/presence/implementation/credential_manager_impl.h +++ b/presence/implementation/credential_manager_impl.h @@ -55,7 +55,7 @@ class CredentialManagerImpl : public CredentialManager { const nearby::internal::DeviceMetadata& device_metadata, const std::vector& credential_selectors, int credential_life_cycle_days, int contiguous_copy_of_credentials, - GenerateCredentialsCallback credentials_generated_cb) override; + GenerateCredentialsResultCallback credentials_generated_cb) override; void UpdateRemotePublicCredentials( absl::string_view manager_app_id, absl::string_view account_name, diff --git a/presence/implementation/credential_manager_impl_test.cc b/presence/implementation/credential_manager_impl_test.cc index df52c4ac..747acbea 100644 --- a/presence/implementation/credential_manager_impl_test.cc +++ b/presence/implementation/credential_manager_impl_test.cc @@ -167,30 +167,23 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) { TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - CredentialManagerImpl credential_manager; - - GenerateCredentialsCallback credentials_generated_cb; - - std::vector publicCredentials; - - auto create_creds_callback_lambda = - [&publicCredentials]( - std::vector credentials) { - publicCredentials = credentials; - }; - - credentials_generated_cb.credentials_generated_cb = - create_creds_callback_lambda; + absl::StatusOr> + public_credentials; std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; credential_manager.GenerateCredentials( device_metadata, BuildCredentialSelectors("TEST_MANAGER_APP", identityTypes), 1, 2, - std::move(credentials_generated_cb)); + {.credentials_generated_cb = + [&](absl::StatusOr> + credentials) { + public_credentials = std::move(credentials); + }}); - EXPECT_EQ(publicCredentials.size(), 2); - for (auto& public_credential : publicCredentials) { + EXPECT_OK(public_credentials); + EXPECT_EQ(public_credentials->size(), 2); + for (auto& public_credential : *public_credentials) { EXPECT_EQ(public_credential.identity_type(), IDENTITY_TYPE_PRIVATE); EXPECT_FALSE(public_credential.secret_id().empty()); EXPECT_EQ(public_credential.end_time_millis() - @@ -202,10 +195,8 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) { TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - auto credential_storage_ptr = std::make_unique(); - EXPECT_CALL(*credential_storage_ptr, SaveCredentials) .WillOnce(::testing::Invoke( [](absl::string_view manager_app_id, absl::string_view account_name, @@ -213,24 +204,24 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { const std::vector& public_credentials, PublicCredentialType public_credential_type, SaveCredentialsResultCallback callback) { - // Do nothing! Testing failed SaveCredentials call. + callback.credentials_saved_cb( + absl::FailedPreconditionError("Expected failure")); })); CredentialManagerImpl credential_manager(std::move(credential_storage_ptr)); - - GenerateCredentialsCallback credentials_generated_cb; - std::vector publicCredentials; - credentials_generated_cb.credentials_generated_cb = - [&publicCredentials]( - std::vector credentials) { - publicCredentials = credentials; - }; + absl::StatusOr> + public_credentials; std::vector identityTypes{IDENTITY_TYPE_PRIVATE}; credential_manager.GenerateCredentials( device_metadata, BuildCredentialSelectors("TEST_MANAGER_APP", identityTypes), 1, 2, - std::move(credentials_generated_cb)); - EXPECT_TRUE(publicCredentials.empty()); + {.credentials_generated_cb = + [&](absl::StatusOr> + credentials) { + public_credentials = std::move(credentials); + }}); + EXPECT_THAT(public_credentials, + StatusIs(absl::StatusCode::kFailedPrecondition)); } TEST(CredentialManagerImpl, UpdateRemotePublicCredentialsSuccessfully) { @@ -261,124 +252,75 @@ TEST(CredentialManagerImpl, UpdateRemotePublicCredentialsSuccessfully) { } TEST(CredentialManagerImpl, GetPrivateCredentialsFailed) { - std::vector private_credentials; - auto get_credentials_fetched_cb = - [&private_credentials](std::vector credentials) { - private_credentials = credentials; - }; - absl::Status get_credentials_status = absl::OkStatus(); - auto get_credentials_failed_cb = - [&get_credentials_status](absl::Status status) { - get_credentials_status = status; - }; - - GetPrivateCredentialsResultCallback get_private_credentials_result_callback; - get_private_credentials_result_callback.get_credentials_failed_cb = - get_credentials_failed_cb; - get_private_credentials_result_callback.credentials_fetched_cb = - get_credentials_fetched_cb; - + absl::StatusOr> private_credentials; CredentialSelector credential_selector; credential_selector.manager_app_id = "TEST_MANAGER_APP"; credential_selector.account_name = "test_account"; credential_selector.identity_type = IDENTITY_TYPE_PRIVATE; - CredentialManagerImpl credential_manager; + credential_manager.GetPrivateCredentials( - credential_selector, std::move(get_private_credentials_result_callback)); - EXPECT_THAT(get_credentials_status, StatusIs(absl::StatusCode::kNotFound)); - EXPECT_TRUE(private_credentials.empty()); + credential_selector, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + private_credentials = std::move(credentials); + }}); + + EXPECT_THAT(private_credentials, StatusIs(absl::StatusCode::kNotFound)); } TEST(CredentialManagerImpl, GetPublicCredentialsFailed) { - std::vector public_credentials; - auto get_credentials_fetched_cb = - [&public_credentials](std::vector credentials) { - public_credentials = credentials; - }; - absl::Status get_credentials_status = absl::OkStatus(); - auto get_credentials_failed_cb = - [&get_credentials_status](absl::Status status) { - get_credentials_status = status; - }; - - GetPublicCredentialsResultCallback get_public_credentials_result_callback; - get_public_credentials_result_callback.get_credentials_failed_cb = - get_credentials_failed_cb; - get_public_credentials_result_callback.credentials_fetched_cb = - get_credentials_fetched_cb; - + absl::StatusOr> public_credentials; CredentialSelector credential_selector; credential_selector.manager_app_id = "TEST_MANAGER_APP"; credential_selector.account_name = "test_account"; credential_selector.identity_type = IDENTITY_TYPE_PRIVATE; - CredentialManagerImpl credential_manager; + credential_manager.GetPublicCredentials( credential_selector, PublicCredentialType::kLocalPublicCredential, - std::move(get_public_credentials_result_callback)); - EXPECT_THAT(get_credentials_status, StatusIs(absl::StatusCode::kNotFound)); - EXPECT_TRUE(public_credentials.empty()); + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + public_credentials = std::move(credentials); + }}); + + EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kNotFound)); } TEST(CredentialManagerImpl, GetCredentialsSuccessfully) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - - std::vector publicCredentials; - - auto create_creds_callback_lambda = - [&publicCredentials]( - std::vector credentials) { - publicCredentials = credentials; - }; - GenerateCredentialsCallback generate_credentials_callback; - generate_credentials_callback.credentials_generated_cb = - create_creds_callback_lambda; - + absl::StatusOr> + public_credentials; CredentialManagerImpl credential_manager; std::vector identity_types{IDENTITY_TYPE_PRIVATE}; + absl::StatusOr> private_credentials; + CredentialSelector credential_selector = BuildDefaultCredentialSelector(); + credential_manager.GenerateCredentials( device_metadata, BuildCredentialSelectors("TEST_MANAGER_APP", identity_types), 1, 1, - std::move(generate_credentials_callback)); - EXPECT_EQ(publicCredentials.size(), 1); - - std::vector private_credentials; - auto get_credentials_fetched_cb = - [&private_credentials](std::vector credentials) { - private_credentials = credentials; - }; - absl::Status get_credentials_status = absl::OkStatus(); - auto get_credentials_failed_cb = - [&get_credentials_status](absl::Status status) { - get_credentials_status = status; - }; - - GetPrivateCredentialsResultCallback get_private_credentials_result_callback; - get_private_credentials_result_callback.get_credentials_failed_cb = - get_credentials_failed_cb; - get_private_credentials_result_callback.credentials_fetched_cb = - get_credentials_fetched_cb; - - CredentialSelector credential_selector = BuildDefaultCredentialSelector(); + {.credentials_generated_cb = + [&](absl::StatusOr> + credentials) { + public_credentials = std::move(credentials); + }}); credential_manager.GetPrivateCredentials( - credential_selector, std::move(get_private_credentials_result_callback)); - EXPECT_EQ(get_credentials_status, absl::OkStatus()); - EXPECT_FALSE(private_credentials.empty()); + credential_selector, + {.credentials_fetched_cb = + [&](absl::StatusOr> credentials) { + private_credentials = std::move(credentials); + }}); + + EXPECT_OK(public_credentials); + EXPECT_EQ(public_credentials->size(), 1); + EXPECT_OK(private_credentials); + EXPECT_FALSE(private_credentials->empty()); } TEST(CredentialManagerImpl, PublicCredentialsFailEncryption) { DeviceMetadata device_metadata = CreateTestDeviceMetadata(); - - std::vector publicCredentials; - - GenerateCredentialsCallback generate_credentials_callback; - generate_credentials_callback.credentials_generated_cb = - [&publicCredentials]( - std::vector credentials) { - publicCredentials = credentials; - }; - + absl::StatusOr> + public_credentials; auto credential_manager_ptr = std::make_unique(); EXPECT_CALL(*credential_manager_ptr, EncryptDeviceMetadata) @@ -386,13 +328,18 @@ TEST(CredentialManagerImpl, PublicCredentialsFailEncryption) { [](absl::string_view device_metadata_encryption_key, absl::string_view authenticity_key, absl::string_view device_metadata_string) { return ""; })); - std::vector identity_types{IDENTITY_TYPE_PRIVATE}; + credential_manager_ptr->GenerateCredentials( device_metadata, BuildCredentialSelectors("TEST_MANAGER_APP", identity_types), 1, 1, - std::move(generate_credentials_callback)); - EXPECT_TRUE(publicCredentials.empty()); + {.credentials_generated_cb = + [&](absl::StatusOr> + credentials) { + public_credentials = std::move(credentials); + }}); + + EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kInvalidArgument)); } } // namespace diff --git a/presence/implementation/scan_manager.cc b/presence/implementation/scan_manager.cc index 2cb9f4ba..8e12550e 100644 --- a/presence/implementation/scan_manager.cc +++ b/presence/implementation/scan_manager.cc @@ -127,21 +127,22 @@ void ScanManager::FetchCredentials(ScanSessionId id, selector, PublicCredentialType::kRemotePublicCredential, {.credentials_fetched_cb = [this, id, identity_type = selector.identity_type]( - std::vector<::nearby::internal::PublicCredential> + absl::StatusOr< + std::vector<::nearby::internal::PublicCredential>> credentials) { + if (!credentials.ok()) { + NEARBY_LOGS(WARNING) + << "Failed to fetch credentials: " << credentials.status(); + return; + } RunOnServiceControllerThread( "update-credentials", [this, id, identity_type, - credentials = std::move(credentials)]() + credentials = std::move(*credentials)]() ABSL_EXCLUSIVE_LOCKS_REQUIRED(*executor_) { UpdateCredentials(id, identity_type, std::move(credentials)); }); - }, - .get_credentials_failed_cb = - [](absl::Status status) { - NEARBY_LOGS(WARNING) - << "Failed to fetch credentials: " << status; }}); } }