Collapse success&failure callbacks into one

Instead of two callbacks:
- void on_success(Result result)
- void on_failure(Status status)
use just one callback:
void on_result(StatusOr<Result> result)

PiperOrigin-RevId: 501912034
This commit is contained in:
Janusz Sobczak
2023-01-13 12:28:47 -08:00
committed by Copybara-Service
parent 454d22b19a
commit 0e2111afc2
10 changed files with 132 additions and 197 deletions
@@ -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;
@@ -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<std::vector<PrivateCredential>> GetPrivateCredentials(
CredentialSelector selector = {.manager_app_id = std::string(manager_app_id),
.account_name = std::string(account_name),
.identity_type = identity_type};
std::vector<PrivateCredential> private_credentials;
absl::Status get_status = absl::OkStatus();
absl::StatusOr<std::vector<PrivateCredential>> private_credentials;
credential_storage.GetPrivateCredentials(
selector, GetPrivateCredentialsResultCallback{
.credentials_fetched_cb =
[&](std::vector<PrivateCredential> 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<std::vector<PrivateCredential>> credentials) {
private_credentials = std::move(credentials);
}});
return private_credentials;
}
absl::StatusOr<std::vector<PublicCredential>> GetPublicCredentials(
@@ -116,22 +109,15 @@ absl::StatusOr<std::vector<PublicCredential>> GetPublicCredentials(
CredentialSelector selector = {.manager_app_id = std::string(manager_app_id),
.account_name = std::string(account_name),
.identity_type = identity_type};
std::vector<PublicCredential> public_credentials;
absl::Status get_status = absl::OkStatus();
absl::StatusOr<std::vector<PublicCredential>> public_credentials;
credential_storage.GetPublicCredentials(
selector, credential_type,
GetPublicCredentialsResultCallback{
.credentials_fetched_cb =
[&](std::vector<PublicCredential> credentials) {
[&](absl::StatusOr<std::vector<PublicCredential>> 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,
@@ -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<void(absl::Status)> credentials_saved_cb;
};
struct GenerateCredentialsCallback {
absl::AnyInvocable<void(std::vector<nearby::internal::PublicCredential>)>
struct GenerateCredentialsResultCallback {
absl::AnyInvocable<void(
absl::StatusOr<std::vector<nearby::internal::PublicCredential>>)>
credentials_generated_cb;
};
@@ -63,15 +65,15 @@ struct UpdateRemotePublicCredentialsCallback {
};
struct GetPrivateCredentialsResultCallback {
absl::AnyInvocable<void(std::vector<::nearby::internal::PrivateCredential>)>
absl::AnyInvocable<void(
absl::StatusOr<std::vector<nearby::internal::PrivateCredential>>)>
credentials_fetched_cb;
absl::AnyInvocable<void(absl::Status)> get_credentials_failed_cb;
};
struct GetPublicCredentialsResultCallback {
absl::AnyInvocable<void(std::vector<::nearby::internal::PublicCredential>)>
absl::AnyInvocable<void(
absl::StatusOr<std::vector<nearby::internal::PublicCredential>>)>
credentials_fetched_cb;
absl::AnyInvocable<void(absl::Status)> get_credentials_failed_cb;
};
inline std::ostream& operator<<(std::ostream& os,
@@ -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;
}
+10 -8
View File
@@ -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);
}});
}
+1 -1
View File
@@ -47,7 +47,7 @@ class CredentialManager {
const nearby::internal::DeviceMetadata& device_metadata,
const std::vector<CredentialSelector>& 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(
@@ -58,7 +58,7 @@ void CredentialManagerImpl::GenerateCredentials(
const DeviceMetadata& device_metadata,
const std::vector<CredentialSelector>& credential_selectors,
int credential_life_cycle_days, int contiguous_copy_of_credentials,
GenerateCredentialsCallback credentials_generated_cb) {
GenerateCredentialsResultCallback credentials_generated_cb) {
std::vector<PublicCredential> public_credentials;
std::vector<PrivateCredential> 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<std::vector<PrivateCredential>> result;
GetPrivateCredentials(
credential_selector,
{
.credentials_fetched_cb =
[result](std::vector<PrivateCredential> credentials) mutable {
result.Set(credentials);
},
.get_credentials_failed_cb =
[result](absl::Status status) mutable {
result.SetException({Exception::kFailed});
},
});
{.credentials_fetched_cb =
[result](absl::StatusOr<std::vector<PrivateCredential>>
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<std::vector<PublicCredential>> result;
GetPublicCredentials(
credential_selector, public_credential_type,
{
.credentials_fetched_cb =
[result](std::vector<PublicCredential> credentials) mutable {
result.Set(credentials);
},
.get_credentials_failed_cb =
[result](absl::Status status) mutable {
result.SetException({Exception::kFailed});
},
});
{.credentials_fetched_cb =
[result](absl::StatusOr<std::vector<PublicCredential>>
credentials) mutable {
if (!credentials.ok()) {
result.SetException({Exception::kFailed});
} else {
result.Set(std::move(*credentials));
}
}});
return result.Get(timeout);
}
@@ -55,7 +55,7 @@ class CredentialManagerImpl : public CredentialManager {
const nearby::internal::DeviceMetadata& device_metadata,
const std::vector<CredentialSelector>& 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,
@@ -167,30 +167,23 @@ TEST(CredentialManagerImpl, CreateOneCredentialSuccessfully) {
TEST(CredentialManagerImpl, GenerateCredentialsSuccessfully) {
DeviceMetadata device_metadata = CreateTestDeviceMetadata();
CredentialManagerImpl credential_manager;
GenerateCredentialsCallback credentials_generated_cb;
std::vector<nearby::internal::PublicCredential> publicCredentials;
auto create_creds_callback_lambda =
[&publicCredentials](
std::vector<nearby::internal::PublicCredential> credentials) {
publicCredentials = credentials;
};
credentials_generated_cb.credentials_generated_cb =
create_creds_callback_lambda;
absl::StatusOr<std::vector<nearby::internal::PublicCredential>>
public_credentials;
std::vector<IdentityType> 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<std::vector<nearby::internal::PublicCredential>>
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<CredentialManagerImplTest::MockCredentialStorage>();
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<PublicCredential>& 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<nearby::internal::PublicCredential> publicCredentials;
credentials_generated_cb.credentials_generated_cb =
[&publicCredentials](
std::vector<nearby::internal::PublicCredential> credentials) {
publicCredentials = credentials;
};
absl::StatusOr<std::vector<nearby::internal::PublicCredential>>
public_credentials;
std::vector<IdentityType> 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<std::vector<nearby::internal::PublicCredential>>
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<PrivateCredential> private_credentials;
auto get_credentials_fetched_cb =
[&private_credentials](std::vector<PrivateCredential> 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<std::vector<PrivateCredential>> 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<std::vector<PrivateCredential>> credentials) {
private_credentials = std::move(credentials);
}});
EXPECT_THAT(private_credentials, StatusIs(absl::StatusCode::kNotFound));
}
TEST(CredentialManagerImpl, GetPublicCredentialsFailed) {
std::vector<PublicCredential> public_credentials;
auto get_credentials_fetched_cb =
[&public_credentials](std::vector<PublicCredential> 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<std::vector<PublicCredential>> 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<std::vector<PublicCredential>> credentials) {
public_credentials = std::move(credentials);
}});
EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kNotFound));
}
TEST(CredentialManagerImpl, GetCredentialsSuccessfully) {
DeviceMetadata device_metadata = CreateTestDeviceMetadata();
std::vector<nearby::internal::PublicCredential> publicCredentials;
auto create_creds_callback_lambda =
[&publicCredentials](
std::vector<nearby::internal::PublicCredential> credentials) {
publicCredentials = credentials;
};
GenerateCredentialsCallback generate_credentials_callback;
generate_credentials_callback.credentials_generated_cb =
create_creds_callback_lambda;
absl::StatusOr<std::vector<nearby::internal::PublicCredential>>
public_credentials;
CredentialManagerImpl credential_manager;
std::vector<IdentityType> identity_types{IDENTITY_TYPE_PRIVATE};
absl::StatusOr<std::vector<PrivateCredential>> 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<PrivateCredential> private_credentials;
auto get_credentials_fetched_cb =
[&private_credentials](std::vector<PrivateCredential> 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<std::vector<nearby::internal::PublicCredential>>
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<std::vector<PrivateCredential>> 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<nearby::internal::PublicCredential> publicCredentials;
GenerateCredentialsCallback generate_credentials_callback;
generate_credentials_callback.credentials_generated_cb =
[&publicCredentials](
std::vector<nearby::internal::PublicCredential> credentials) {
publicCredentials = credentials;
};
absl::StatusOr<std::vector<nearby::internal::PublicCredential>>
public_credentials;
auto credential_manager_ptr =
std::make_unique<CredentialManagerImplTest::MockCredentialManager>();
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<IdentityType> 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<std::vector<nearby::internal::PublicCredential>>
credentials) {
public_credentials = std::move(credentials);
}});
EXPECT_THAT(public_credentials, StatusIs(absl::StatusCode::kInvalidArgument));
}
} // namespace
+8 -7
View File
@@ -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;
}});
}
}