diff --git a/internal/platform/credential_storage_impl.cc b/internal/platform/credential_storage_impl.cc index a78b19ef..c7ba4334 100644 --- a/internal/platform/credential_storage_impl.cc +++ b/internal/platform/credential_storage_impl.cc @@ -34,7 +34,7 @@ void CredentialStorageImpl::SaveCredentials( const std::vector& private_credentials, const std::vector& public_credentials, PublicCredentialType public_credential_type, - GenerateCredentialsCallback callback) { + SaveCredentialsResultCallback callback) { return impl_->SaveCredentials(manager_app_id, account_name, private_credentials, public_credentials, public_credential_type, std::move(callback)); diff --git a/internal/platform/credential_storage_impl.h b/internal/platform/credential_storage_impl.h index 79e80109..e2a1e73c 100644 --- a/internal/platform/credential_storage_impl.h +++ b/internal/platform/credential_storage_impl.h @@ -43,24 +43,21 @@ class CredentialStorageImpl : public api::CredentialStorage { void SaveCredentials( absl::string_view manager_app_id, absl::string_view account_name, - const std::vector<::nearby::internal::PrivateCredential>& - private_credentials, - const std::vector<::nearby::internal::PublicCredential>& - public_credentials, - ::nearby::presence::PublicCredentialType public_credential_type, - ::nearby::presence::GenerateCredentialsCallback callback) override; + const std::vector& private_credentials, + const std::vector& public_credentials, + PublicCredentialType public_credential_type, + SaveCredentialsResultCallback callback) override; // Used to fetch private creds when broadcasting. void GetPrivateCredentials( - const ::nearby::presence::CredentialSelector& credential_selector, - ::nearby::presence::GetPrivateCredentialsResultCallback callback) - override; + const CredentialSelector& credential_selector, + GetPrivateCredentialsResultCallback callback) override; // Used to fetch remote public creds when scanning. void GetPublicCredentials( - const ::nearby::presence::CredentialSelector& credential_selector, - ::nearby::presence::PublicCredentialType public_credential_type, - ::nearby::presence::GetPublicCredentialsResultCallback callback) override; + const CredentialSelector& credential_selector, + PublicCredentialType public_credential_type, + GetPublicCredentialsResultCallback callback) override; private: std::unique_ptr impl_; diff --git a/internal/platform/credential_storage_impl_test.cc b/internal/platform/credential_storage_impl_test.cc index 2ad5e4dc..9cb89c62 100644 --- a/internal/platform/credential_storage_impl_test.cc +++ b/internal/platform/credential_storage_impl_test.cc @@ -32,12 +32,12 @@ namespace { using ::nearby::internal::PrivateCredential; using ::nearby::internal::PublicCredential; -using ::nearby::presence::CredentialOperationStatus; using ::nearby::presence::CredentialSelector; using ::nearby::presence::GenerateCredentialsCallback; using ::nearby::presence::GetPrivateCredentialsResultCallback; using ::nearby::presence::GetPublicCredentialsResultCallback; using ::nearby::presence::PublicCredentialType; +using ::nearby::presence::SaveCredentialsResultCallback; std::vector BuildDefaultPrivateCreds() { PrivateCredential private_credential; @@ -89,8 +89,8 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPrivateCredentials) { }; bool get_private_cred_failed = false; get_private_creds_callback.get_credentials_failed_cb = - [&get_private_cred_failed](CredentialOperationStatus status) { - if (status == CredentialOperationStatus::kFailed) { + [&get_private_cred_failed](absl::Status status) { + if (!status.ok()) { get_private_cred_failed = true; } }; @@ -100,23 +100,41 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPrivateCredentials) { PublicCredentialType public_credential_type = PublicCredentialType::kLocalPublicCredential; CredentialStorageImpl creds_storage; - creds_storage.GetPrivateCredentials(cred_selector, - get_private_creds_callback); + creds_storage.GetPrivateCredentials( + cred_selector, + GetPrivateCredentialsResultCallback{ + .credentials_fetched_cb = + [&](const std::vector& private_creds) { + retrieved_creds = private_creds; + }, + .get_credentials_failed_cb = + [&](absl::Status status) { + get_private_cred_failed = !status.ok(); + }}); EXPECT_TRUE(get_private_cred_failed); - creds_storage.SaveCredentials(cred_selector.manager_app_id, - cred_selector.account_name, empty_private_creds, - empty_public_creds, public_credential_type, - generate_creds_callback); + creds_storage.SaveCredentials( + cred_selector.manager_app_id, cred_selector.account_name, + empty_private_creds, empty_public_creds, public_credential_type, + SaveCredentialsResultCallback{ + .credentials_saved_cb = [&](absl::Status status) { + successfull_save = status.ok(); + }}); EXPECT_FALSE(successfull_save); creds_storage.SaveCredentials( cred_selector.manager_app_id, cred_selector.account_name, default_private_creds, empty_public_creds, public_credential_type, - generate_creds_callback); + SaveCredentialsResultCallback{ + .credentials_saved_cb = [&](absl::Status status) { + successfull_save = status.ok(); + }}); EXPECT_FALSE(successfull_save); creds_storage.SaveCredentials( cred_selector.manager_app_id, cred_selector.account_name, default_private_creds, default_public_creds, public_credential_type, - std::move(generate_creds_callback)); + SaveCredentialsResultCallback{ + .credentials_saved_cb = [&](absl::Status status) { + successfull_save = status.ok(); + }}); EXPECT_TRUE(successfull_save); creds_storage.GetPrivateCredentials(cred_selector, std::move(get_private_creds_callback)); @@ -148,8 +166,8 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPublicCredentials) { }; bool get_public_cred_failed = false; get_public_creds_callback.get_credentials_failed_cb = - [&get_public_cred_failed](CredentialOperationStatus status) { - if (status == CredentialOperationStatus::kFailed) { + [&get_public_cred_failed](absl::Status status) { + if (!status.ok()) { get_public_cred_failed = true; } }; @@ -159,132 +177,47 @@ TEST(CredentialStorageImplTest, CanSaveAndGetPublicCredentials) { PublicCredentialType public_credential_type = PublicCredentialType::kLocalPublicCredential; CredentialStorageImpl creds_storage; - creds_storage.GetPublicCredentials(cred_selector, public_credential_type, - get_public_creds_callback); + creds_storage.GetPublicCredentials( + cred_selector, public_credential_type, + GetPublicCredentialsResultCallback{ + .credentials_fetched_cb = + [&](const std::vector& public_creds) { + retrieved_creds = public_creds; + }, + .get_credentials_failed_cb = + [&](absl::Status status) { + get_public_cred_failed = !status.ok(); + }}); EXPECT_TRUE(get_public_cred_failed); - creds_storage.SaveCredentials(cred_selector.manager_app_id, - cred_selector.account_name, empty_private_creds, - empty_public_creds, public_credential_type, - generate_creds_callback); + creds_storage.SaveCredentials( + cred_selector.manager_app_id, cred_selector.account_name, + empty_private_creds, empty_public_creds, public_credential_type, + SaveCredentialsResultCallback{ + .credentials_saved_cb = [&](absl::Status status) { + successfull_save = status.ok(); + }}); EXPECT_FALSE(successfull_save); creds_storage.SaveCredentials( cred_selector.manager_app_id, cred_selector.account_name, default_private_creds, empty_public_creds, public_credential_type, - generate_creds_callback); + SaveCredentialsResultCallback{ + .credentials_saved_cb = [&](absl::Status status) { + successfull_save = status.ok(); + }}); EXPECT_FALSE(successfull_save); creds_storage.SaveCredentials( cred_selector.manager_app_id, cred_selector.account_name, default_private_creds, default_public_creds, public_credential_type, - std::move(generate_creds_callback)); + SaveCredentialsResultCallback{ + .credentials_saved_cb = [&](absl::Status status) { + successfull_save = status.ok(); + }}); EXPECT_TRUE(successfull_save); creds_storage.GetPublicCredentials(cred_selector, public_credential_type, std::move(get_public_creds_callback)); EXPECT_EQ(retrieved_creds.size(), 1); } -TEST(CredentialStorageImplTest, OverwritePrivateCredentials) { - std::vector default_priv_creds = - BuildDefaultPrivateCreds(); - std::vector default_pub_creds = BuildDefaultPublicCreds(); - PrivateCredential overwrite_priv_cred; - overwrite_priv_cred.set_secret_id("overwrite_secret_id"); - std::vector overwrite_priv_creds; - overwrite_priv_creds.push_back(overwrite_priv_cred); - std::vector generated_creds; - GenerateCredentialsCallback generate_creds_callback; - generate_creds_callback.credentials_generated_cb = - [&generated_creds](const std::vector& public_creds) { - generated_creds = public_creds; - }; - std::vector retrieved_creds; - GetPrivateCredentialsResultCallback get_private_creds_callback; - get_private_creds_callback.credentials_fetched_cb = - [&retrieved_creds](const std::vector& private_creds) { - retrieved_creds = private_creds; - }; - bool get_private_cred_failed = false; - get_private_creds_callback.get_credentials_failed_cb = - [&get_private_cred_failed](CredentialOperationStatus status) { - if (status == CredentialOperationStatus::kFailed) { - get_private_cred_failed = true; - } - }; - // Create CredentialStorageImpl object to test SaveCredentials & - // GetPrivateCredentials - CredentialSelector cred_selector = BuildDefaultCredentialSelector(); - PublicCredentialType public_credential_type = - PublicCredentialType::kLocalPublicCredential; - CredentialStorageImpl creds_storage; - creds_storage.SaveCredentials(cred_selector.manager_app_id, - cred_selector.account_name, default_priv_creds, - default_pub_creds, public_credential_type, - generate_creds_callback); - EXPECT_EQ(generated_creds[0].secret_id(), default_pub_creds[0].secret_id()); - creds_storage.GetPrivateCredentials(cred_selector, - get_private_creds_callback); - EXPECT_EQ(retrieved_creds[0].secret_id(), default_priv_creds[0].secret_id()); - creds_storage.SaveCredentials( - cred_selector.manager_app_id, cred_selector.account_name, - overwrite_priv_creds, default_pub_creds, public_credential_type, - generate_creds_callback); - EXPECT_EQ(generated_creds[0].secret_id(), default_pub_creds[0].secret_id()); - creds_storage.GetPrivateCredentials(cred_selector, - get_private_creds_callback); - EXPECT_EQ(retrieved_creds[0].secret_id(), - overwrite_priv_creds[0].secret_id()); -} - -TEST(CredentialStorageImplTest, OverwritePublicCredentials) { - std::vector default_priv_creds = - BuildDefaultPrivateCreds(); - std::vector default_pub_creds = BuildDefaultPublicCreds(); - PublicCredential overwrite_pub_cred; - overwrite_pub_cred.set_secret_id("overwrite_secret_id"); - std::vector overwrite_pub_creds; - overwrite_pub_creds.push_back(overwrite_pub_cred); - std::vector generated_creds; - GenerateCredentialsCallback generate_creds_callback; - generate_creds_callback.credentials_generated_cb = - [&generated_creds](const std::vector& public_creds) { - generated_creds = public_creds; - }; - std::vector retrieved_creds; - GetPublicCredentialsResultCallback get_public_creds_callback; - get_public_creds_callback.credentials_fetched_cb = - [&retrieved_creds](const std::vector& public_creds) { - retrieved_creds = public_creds; - }; - bool get_public_cred_failed = false; - get_public_creds_callback.get_credentials_failed_cb = - [&get_public_cred_failed](CredentialOperationStatus status) { - if (status == CredentialOperationStatus::kFailed) { - get_public_cred_failed = true; - } - }; - // Create CredentialStorageImpl object to test SaveCredentials & - // GetPublicCredentials - CredentialSelector cred_selector = BuildDefaultCredentialSelector(); - PublicCredentialType public_credential_type = - PublicCredentialType::kLocalPublicCredential; - CredentialStorageImpl creds_storage; - creds_storage.SaveCredentials(cred_selector.manager_app_id, - cred_selector.account_name, default_priv_creds, - default_pub_creds, public_credential_type, - generate_creds_callback); - EXPECT_EQ(generated_creds[0].secret_id(), default_pub_creds[0].secret_id()); - creds_storage.GetPublicCredentials(cred_selector, public_credential_type, - get_public_creds_callback); - EXPECT_EQ(retrieved_creds[0].secret_id(), default_pub_creds[0].secret_id()); - creds_storage.SaveCredentials(cred_selector.manager_app_id, - cred_selector.account_name, default_priv_creds, - overwrite_pub_creds, public_credential_type, - generate_creds_callback); - EXPECT_EQ(generated_creds[0].secret_id(), overwrite_pub_creds[0].secret_id()); - creds_storage.GetPublicCredentials(cred_selector, public_credential_type, - get_public_creds_callback); - EXPECT_EQ(retrieved_creds[0].secret_id(), overwrite_pub_creds[0].secret_id()); -} - } // namespace } // namespace nearby } // namespace location diff --git a/internal/platform/implementation/BUILD b/internal/platform/implementation/BUILD index 5a91f916..fb58ef45 100644 --- a/internal/platform/implementation/BUILD +++ b/internal/platform/implementation/BUILD @@ -80,6 +80,7 @@ cc_library( "//internal/platform:uuid", "//internal/proto:credential_cc_proto", "@com_google_absl//absl/container:flat_hash_map", + "@com_google_absl//absl/functional:any_invocable", "@com_google_absl//absl/strings", ], ) diff --git a/internal/platform/implementation/credential_callbacks.h b/internal/platform/implementation/credential_callbacks.h index c9df5238..5b6bdb9d 100644 --- a/internal/platform/implementation/credential_callbacks.h +++ b/internal/platform/implementation/credential_callbacks.h @@ -20,22 +20,26 @@ #include #include +#include "absl/functional/any_invocable.h" #include "absl/strings/string_view.h" #include "internal/proto/credential.pb.h" namespace nearby { namespace presence { -enum class CredentialOperationStatus { - kUnknown = 0, - kFailed = 1, - kSucceeded = 2, -}; - struct CredentialSelector { std::string manager_app_id; std::string account_name; ::nearby::internal::IdentityType identity_type; + + template + friend void AbslStringify(Sink& sink, + const CredentialSelector& credential_selector) { + absl::Format(&sink, "CredentialSelector(%v, %v, IdentityType(%v))", + credential_selector.manager_app_id, + credential_selector.account_name, + static_cast(credential_selector.identity_type)); + } }; enum class PublicCredentialType { @@ -43,25 +47,29 @@ enum class PublicCredentialType { kRemotePublicCredential = 2, }; +struct SaveCredentialsResultCallback { + absl::AnyInvocable credentials_saved_cb; +}; + struct GenerateCredentialsCallback { - std::function)> + absl::AnyInvocable)> credentials_generated_cb; }; struct UpdateRemotePublicCredentialsCallback { - std::function credentials_updated_cb; + absl::AnyInvocable credentials_updated_cb; }; struct GetPrivateCredentialsResultCallback { - std::function)> + absl::AnyInvocable)> credentials_fetched_cb; - std::function get_credentials_failed_cb; + absl::AnyInvocable get_credentials_failed_cb; }; struct GetPublicCredentialsResultCallback { - std::function)> + absl::AnyInvocable)> credentials_fetched_cb; - std::function get_credentials_failed_cb; + absl::AnyInvocable get_credentials_failed_cb; }; inline std::ostream& operator<<(std::ostream& os, @@ -70,6 +78,13 @@ inline std::ostream& operator<<(std::ostream& os, << ")"; } +inline std::ostream& operator<<(std::ostream& os, + const CredentialSelector& credential_selector) { + return os << "CredentialSelector(" << credential_selector.manager_app_id + << ", " << credential_selector.account_name << ", IdentityType(" + << static_cast(credential_selector.identity_type) << "))"; +} + } // namespace presence } // namespace nearby diff --git a/internal/platform/implementation/credential_storage.h b/internal/platform/implementation/credential_storage.h index 6372236b..3b086f14 100644 --- a/internal/platform/implementation/credential_storage.h +++ b/internal/platform/implementation/credential_storage.h @@ -27,36 +27,57 @@ namespace location { namespace nearby { namespace api { -/* - * This class specifies the virtual functions for native platforms to implement. - */ +// Credential Storage interface class CredentialStorage { public: + using PrivateCredential = ::nearby::internal::PrivateCredential; + using PublicCredential = ::nearby::internal::PublicCredential; + using PublicCredentialType = ::nearby::presence::PublicCredentialType; + using SaveCredentialsResultCallback = + ::nearby::presence::SaveCredentialsResultCallback; + using CredentialSelector = ::nearby::presence::CredentialSelector; + using GetPrivateCredentialsResultCallback = + ::nearby::presence::GetPrivateCredentialsResultCallback; + using GetPublicCredentialsResultCallback = + ::nearby::presence::GetPublicCredentialsResultCallback; + virtual ~CredentialStorage() = default; - // Used for - // 1. Save private creds after (re)generate credentials invoked by manager app - // 2. Update remote public creds after manager app update the public creds. - // Skip the save/update if the provided vector is empty. - // Another way is to break this into two APIs for save and update separately. + + // Saves the credentials in the storage. + // + // If `private_credentials` is not empty, then the private credentials in the + // storage, associated with `manager_app_id`/`account_name` pair, are replaced + // with given credentials. + // + // If `public_credentials` is not empty, then the public credentials of + // `public_credential_type` type in the storage, associated with + // `manager_app_id`/`account_name` pair, are replaced with given credentials. + // + // Note, both private and public credentials have a `identity_type` field, + // which is used for querying credentials. virtual void SaveCredentials( absl::string_view manager_app_id, absl::string_view account_name, - const std::vector<::nearby::internal::PrivateCredential>& - private_credentials, - const std::vector<::nearby::internal::PublicCredential>& - public_credentials, - ::nearby::presence::PublicCredentialType public_credential_type, - ::nearby::presence::GenerateCredentialsCallback callback) = 0; + const std::vector& private_credentials, + const std::vector& public_credentials, + PublicCredentialType public_credential_type, + SaveCredentialsResultCallback callback) = 0; - // Used to fetch private creds when broadcasting. + // Fetches private credentials. + // + // When `credential_selector.identity_type` is not set (unspecified), then + // private credentials with any identity type should be returned. virtual void GetPrivateCredentials( - const ::nearby::presence::CredentialSelector& credential_selector, - ::nearby::presence::GetPrivateCredentialsResultCallback callback) = 0; + const CredentialSelector& credential_selector, + GetPrivateCredentialsResultCallback callback) = 0; - // Used to fetch remote public creds when scanning. + // Fetches public credentials. + // + // When `credential_selector.identity_type` is not set (unspecified), then + // public credentials with any identity type should be returned. virtual void GetPublicCredentials( - const ::nearby::presence::CredentialSelector& credential_selector, - ::nearby::presence::PublicCredentialType public_credential_type, - ::nearby::presence::GetPublicCredentialsResultCallback callback) = 0; + const CredentialSelector& credential_selector, + PublicCredentialType public_credential_type, + GetPublicCredentialsResultCallback callback) = 0; }; } // namespace api diff --git a/internal/platform/implementation/g3/BUILD b/internal/platform/implementation/g3/BUILD index 785f7dbe..efeea4da 100644 --- a/internal/platform/implementation/g3/BUILD +++ b/internal/platform/implementation/g3/BUILD @@ -88,6 +88,7 @@ cc_library( "@com_google_absl//absl/base:core_headers", "@com_google_absl//absl/container:flat_hash_map", "@com_google_absl//absl/container:flat_hash_set", + "@com_google_absl//absl/status", "@com_google_absl//absl/strings", "@com_google_absl//absl/strings:str_format", "@com_google_absl//absl/synchronization", diff --git a/internal/platform/implementation/g3/credential_storage_impl.cc b/internal/platform/implementation/g3/credential_storage_impl.cc index 2d92dfae..fc63d3a8 100644 --- a/internal/platform/implementation/g3/credential_storage_impl.cc +++ b/internal/platform/implementation/g3/credential_storage_impl.cc @@ -19,6 +19,8 @@ #include #include +#include "absl/status/status.h" +#include "absl/strings/str_format.h" #include "internal/platform/logging.h" #include "internal/proto/credential.pb.h" @@ -34,7 +36,7 @@ void CredentialStorageImpl::SaveCredentials( const std::vector& private_credentials, const std::vector& public_credentials, PublicCredentialType public_credential_type, - ::nearby::presence::GenerateCredentialsCallback callback) { + SaveCredentialsResultCallback callback) { if (private_credentials.empty() && public_credentials.empty()) { NEARBY_LOGS(INFO) << "G3 Save Credentials but seeing private and public " "both empty, skipping"; @@ -80,28 +82,25 @@ void CredentialStorageImpl::SaveCredentials( public_credentials_map_[key] = public_credentials; } } - - callback.credentials_generated_cb(public_credentials); + std::move(callback.credentials_saved_cb)(absl::OkStatus()); } void CredentialStorageImpl::GetPrivateCredentials( const ::nearby::presence::CredentialSelector& credential_selector, ::nearby::presence::GetPrivateCredentialsResultCallback callback) { - NEARBY_LOGS(INFO) << "G3 Get Private Credentials for account: " - << credential_selector.account_name << "], manager app ID:[" - << credential_selector.manager_app_id << "]"; + NEARBY_LOGS(INFO) << "G3 Get Private Credentials for " << credential_selector; absl::MutexLock lock(&private_mutex_); PrivateCredentialKey key = CreatePrivateCredentialKey( credential_selector.manager_app_id, credential_selector.account_name); 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); - callback.get_credentials_failed_cb( - ::nearby::presence::CredentialOperationStatus::kFailed); + std::move(callback.get_credentials_failed_cb)(absl::NotFoundError( + absl::StrFormat("No private credentials for %v", credential_selector))); } else { std::vector private_credentials = private_credentials_map_[key]; - callback.credentials_fetched_cb(private_credentials); + std::move(callback.credentials_fetched_cb)(private_credentials); } } @@ -109,9 +108,7 @@ void CredentialStorageImpl::GetPublicCredentials( const ::nearby::presence::CredentialSelector& credential_selector, ::nearby::presence::PublicCredentialType public_credential_type, ::nearby::presence::GetPublicCredentialsResultCallback callback) { - NEARBY_LOGS(INFO) << "G3 Get Public Credentials for account: " - << credential_selector.account_name << "], manager app ID:[" - << credential_selector.manager_app_id << "]"; + NEARBY_LOGS(INFO) << "G3 Get Public Credentials for " << credential_selector; absl::MutexLock lock(&public_mutex_); PublicCredentialKey key = CreatePublicCredentialKey( credential_selector.manager_app_id, credential_selector.account_name, @@ -120,12 +117,12 @@ 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); - callback.get_credentials_failed_cb( - ::nearby::presence::CredentialOperationStatus::kFailed); + std::move(callback.get_credentials_failed_cb)(absl::NotFoundError( + absl::StrFormat("No public credentials for %v", credential_selector))); } else { std::vector public_credentials = public_credentials_map_[key]; - callback.credentials_fetched_cb(public_credentials); + std::move(callback.credentials_fetched_cb)(public_credentials); } } } // namespace g3 diff --git a/internal/platform/implementation/g3/credential_storage_impl.h b/internal/platform/implementation/g3/credential_storage_impl.h index cac689a1..55f67529 100644 --- a/internal/platform/implementation/g3/credential_storage_impl.h +++ b/internal/platform/implementation/g3/credential_storage_impl.h @@ -55,19 +55,18 @@ class CredentialStorageImpl : public api::CredentialStorage { const std::vector& private_credentials, const std::vector& public_credentials, PublicCredentialType public_credential_type, - ::nearby::presence::GenerateCredentialsCallback callback) override; + SaveCredentialsResultCallback callback) override; // Used to fetch private creds when broadcasting. void GetPrivateCredentials( - const ::nearby::presence::CredentialSelector& credential_selector, - ::nearby::presence::GetPrivateCredentialsResultCallback callback) - override; + const CredentialSelector& credential_selector, + GetPrivateCredentialsResultCallback callback) override; // Used to fetch remote public creds when scanning. void GetPublicCredentials( - const ::nearby::presence::CredentialSelector& credential_selector, + const CredentialSelector& credential_selector, PublicCredentialType public_credential_type, - ::nearby::presence::GetPublicCredentialsResultCallback callback) override; + GetPublicCredentialsResultCallback callback) override; private: PrivateCredentialKey CreatePrivateCredentialKey( diff --git a/presence/implementation/credential_manager_impl.cc b/presence/implementation/credential_manager_impl.cc index 5716a217..bad595ba 100644 --- a/presence/implementation/credential_manager_impl.cc +++ b/presence/implementation/credential_manager_impl.cc @@ -85,7 +85,19 @@ void CredentialManagerImpl::GenerateCredentials( credential_storage_ptr_->SaveCredentials( manager_app_id, device_metadata.account_name(), private_credentials, public_credentials, PublicCredentialType::kLocalPublicCredential, - std::move(credentials_generated_cb)); + SaveCredentialsResultCallback{ + .credentials_saved_cb = + [callback = std::move(credentials_generated_cb), + public_credentials](absl::Status status) mutable { + if (status.ok()) { + std::move(callback.credentials_generated_cb)( + std::move(public_credentials)); + } else { + NEARBY_LOGS(WARNING) + << "Save credentials failed with: " << status; + std::move(callback.credentials_generated_cb)({}); + } + }}); } void CredentialManagerImpl::UpdateRemotePublicCredentials( @@ -95,19 +107,16 @@ void CredentialManagerImpl::UpdateRemotePublicCredentials( credential_storage_ptr_->SaveCredentials( manager_app_id, account_name, /* private_credentials */ {}, remote_public_creds, PublicCredentialType::kRemotePublicCredential, - GenerateCredentialsCallback{ - .credentials_generated_cb = - [credentials_updated_cb = std::move(credentials_updated_cb)]( - std::vector creds) { - if (!creds.empty()) { - credentials_updated_cb.credentials_updated_cb( - CredentialOperationStatus::kSucceeded); - } else { - credentials_updated_cb.credentials_updated_cb( - CredentialOperationStatus::kFailed); - } - }, - }); + SaveCredentialsResultCallback{ + .credentials_saved_cb = [callback = + std::move(credentials_updated_cb)]( + absl::Status status) mutable { + if (!status.ok()) { + NEARBY_LOGS(WARNING) + << "Update remote credentials failed with: " << status; + } + std::move(callback.credentials_updated_cb)(status); + }}); } std::pair @@ -124,11 +133,11 @@ CredentialManagerImpl::CreatePrivateCredential( Encryption::GenerateRandomByteArray(kAuthenticityKeyByteSize); private_credential.set_authenticity_key(secret_key); - // Uses SHA-256 algorithm to generate the credential ID from the authenticity - // key + // Uses SHA-256 algorithm to generate the credential ID from the + // authenticity key auto secret_id = Crypto::Sha256(secret_key); - // Does not expect to fail here since Crypto::Sha256 should not return empty - // ByteArray. + // Does not expect to fail here since Crypto::Sha256 should not return + // empty ByteArray. CHECK(!secret_id.Empty()) << "Crypto::Sha256 failed!"; private_credential.set_secret_id(std::string(secret_id.AsStringView())); @@ -282,7 +291,7 @@ CredentialManagerImpl::GetPrivateCredentialsSync( result.Set(credentials); }, .get_credentials_failed_cb = - [result](CredentialOperationStatus status) mutable { + [result](absl::Status status) mutable { result.SetException({Exception::kFailed}); }, }); @@ -302,7 +311,7 @@ CredentialManagerImpl::GetPublicCredentialsSync( result.Set(credentials); }, .get_credentials_failed_cb = - [result](CredentialOperationStatus status) mutable { + [result](absl::Status status) mutable { result.SetException({Exception::kFailed}); }, }); diff --git a/presence/implementation/credential_manager_impl_test.cc b/presence/implementation/credential_manager_impl_test.cc index ecdf570b..af358d6d 100644 --- a/presence/implementation/credential_manager_impl_test.cc +++ b/presence/implementation/credential_manager_impl_test.cc @@ -40,6 +40,7 @@ using ::nearby::internal::PublicCredential; using ::nearby::internal::IdentityType::IDENTITY_TYPE_PRIVATE; using ::proto2::contrib::parse_proto::ParseTestProto; using ::protobuf_matchers::EqualsProto; +using ::testing::status::StatusIs; DeviceMetadata CreateTestDeviceMetadata() { DeviceMetadata device_metadata; @@ -72,7 +73,7 @@ class CredentialManagerImplTest : public ::testing::Test { const std::vector<::nearby::internal::PublicCredential>& public_credentials, PublicCredentialType public_credential_type, - GenerateCredentialsCallback callback), + SaveCredentialsResultCallback callback), (override)); MOCK_METHOD( void, GetPublicCredentials, @@ -171,7 +172,7 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) { credential_manager.GenerateCredentials( device_metadata, /* manager_app_id= */ "TEST_MANAGER_APP", identityTypes, 1, 2, - credentials_generated_cb); + std::move(credentials_generated_cb)); EXPECT_EQ(publicCredentials.size(), 2); for (auto& public_credential : publicCredentials) { @@ -196,7 +197,7 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { const std::vector& private_credentials, const std::vector& public_credentials, PublicCredentialType public_credential_type, - GenerateCredentialsCallback callback) { + SaveCredentialsResultCallback callback) { // Do nothing! Testing failed SaveCredentials call. })); CredentialManagerImpl credential_manager(std::move(credential_storage_ptr)); @@ -213,7 +214,7 @@ TEST(CredentialManagerImpl, GenerateCredentialsSuccessfullyButStoreFailed) { credential_manager.GenerateCredentials( device_metadata, /* manager_app_id= */ "TEST_MANAGER_APP", identityTypes, 1, 2, - credentials_generated_cb); + std::move(credentials_generated_cb)); EXPECT_TRUE(publicCredentials.empty()); } @@ -227,8 +228,8 @@ TEST(CredentialManagerImpl, UpdateRemotePublicCredentialsSuccessfully) { location::nearby::CountDownLatch updated_latch(1); UpdateRemotePublicCredentialsCallback update_credentials_cb{ .credentials_updated_cb = - [&updated_latch](CredentialOperationStatus status) { - if (status == CredentialOperationStatus::kSucceeded) { + [&updated_latch](absl::Status status) { + if (status.ok()) { updated_latch.CountDown(); } }, @@ -239,7 +240,7 @@ TEST(CredentialManagerImpl, UpdateRemotePublicCredentialsSuccessfully) { credential_manager.UpdateRemotePublicCredentials( /* manager_app_id= */ "TEST_MANAGER_APP", /* account_name= */ "test_account", publicCredentials, - update_credentials_cb); + std::move(update_credentials_cb)); EXPECT_TRUE(updated_latch.Await().Ok()); } @@ -250,10 +251,9 @@ TEST(CredentialManagerImpl, GetPrivateCredentialsFailed) { [&private_credentials](std::vector credentials) { private_credentials = credentials; }; - CredentialOperationStatus get_credentials_status = - CredentialOperationStatus::kSucceeded; + absl::Status get_credentials_status = absl::OkStatus(); auto get_credentials_failed_cb = - [&get_credentials_status](CredentialOperationStatus status) { + [&get_credentials_status](absl::Status status) { get_credentials_status = status; }; @@ -270,8 +270,8 @@ TEST(CredentialManagerImpl, GetPrivateCredentialsFailed) { CredentialManagerImpl credential_manager; credential_manager.GetPrivateCredentials( - credential_selector, get_private_credentials_result_callback); - EXPECT_EQ(get_credentials_status, CredentialOperationStatus::kFailed); + credential_selector, std::move(get_private_credentials_result_callback)); + EXPECT_THAT(get_credentials_status, StatusIs(absl::StatusCode::kNotFound)); EXPECT_TRUE(private_credentials.empty()); } @@ -281,10 +281,9 @@ TEST(CredentialManagerImpl, GetPublicCredentialsFailed) { [&public_credentials](std::vector credentials) { public_credentials = credentials; }; - CredentialOperationStatus get_credentials_status = - CredentialOperationStatus::kSucceeded; + absl::Status get_credentials_status = absl::OkStatus(); auto get_credentials_failed_cb = - [&get_credentials_status](CredentialOperationStatus status) { + [&get_credentials_status](absl::Status status) { get_credentials_status = status; }; @@ -302,8 +301,8 @@ TEST(CredentialManagerImpl, GetPublicCredentialsFailed) { CredentialManagerImpl credential_manager; credential_manager.GetPublicCredentials( credential_selector, PublicCredentialType::kLocalPublicCredential, - get_public_credentials_result_callback); - EXPECT_EQ(get_credentials_status, CredentialOperationStatus::kFailed); + std::move(get_public_credentials_result_callback)); + EXPECT_THAT(get_credentials_status, StatusIs(absl::StatusCode::kNotFound)); EXPECT_TRUE(public_credentials.empty()); } @@ -333,10 +332,9 @@ TEST(CredentialManagerImpl, GetCredentialsSuccessfully) { [&private_credentials](std::vector credentials) { private_credentials = credentials; }; - CredentialOperationStatus get_credentials_status = - CredentialOperationStatus::kSucceeded; + absl::Status get_credentials_status = absl::OkStatus(); auto get_credentials_failed_cb = - [&get_credentials_status](CredentialOperationStatus status) { + [&get_credentials_status](absl::Status status) { get_credentials_status = status; }; @@ -349,7 +347,7 @@ TEST(CredentialManagerImpl, GetCredentialsSuccessfully) { CredentialSelector credential_selector = BuildDefaultCredentialSelector(); credential_manager.GetPrivateCredentials( credential_selector, std::move(get_private_credentials_result_callback)); - EXPECT_EQ(get_credentials_status, CredentialOperationStatus::kSucceeded); + EXPECT_EQ(get_credentials_status, absl::OkStatus()); EXPECT_FALSE(private_credentials.empty()); } diff --git a/presence/implementation/scan_manager.cc b/presence/implementation/scan_manager.cc index 12f0a135..dba1023a 100644 --- a/presence/implementation/scan_manager.cc +++ b/presence/implementation/scan_manager.cc @@ -149,8 +149,9 @@ void ScanManager::FetchCredentials(ScanSessionId id, }); }, .get_credentials_failed_cb = - [](CredentialOperationStatus status) { - NEARBY_LOGS(WARNING) << "Failed to fetch credentials"; + [](absl::Status status) { + NEARBY_LOGS(WARNING) + << "Failed to fetch credentials: " << status; }}); } } diff --git a/presence/implementation/service_controller_impl.cc b/presence/implementation/service_controller_impl.cc index e7f16630..2a7fed74 100644 --- a/presence/implementation/service_controller_impl.cc +++ b/presence/implementation/service_controller_impl.cc @@ -104,9 +104,9 @@ void ServiceControllerImpl::FetchCredentials( }); }, .get_credentials_failed_cb = - [this, id](CredentialOperationStatus status) { - NEARBY_LOGS(WARNING) << "Failed to fetch credentials, status: " - << static_cast(status); + [this, id](absl::Status status) { + NEARBY_LOGS(WARNING) + << "Failed to fetch credentials, status: " << status; NotifyStartCallbackStatus(id, Status{Status::Value::kError}); }}); }