Skip to content

Commit

Permalink
Addressing pull request findings.
Browse files Browse the repository at this point in the history 
Removing brave_rewards::prefs::kAnonTransferChecked.
Making UpholdAuthorization::GetAnonFunds() and UpholdAuthorization::LinkWallet() private (using FRIEND_TEST_ALL_PREFIXES).
Updating copyright year in uphold_authorization_unittest.cc.
Adjusting TEST_P's and INSTANTIATE_TEST_SUITE_P's arguments in uphold_authorization_unittest.cc.
Passing drain_id by reference-to-const in UpholdWallet::OnTransferTokens().
  • Loading branch information
@szilardszaloki
szilardszaloki committed Jun 15, 2021
1 parent 96e4218 commit fbf7d7c
Show file tree
Hide file tree
Showing 7 changed files with 46 additions and 45 deletions.
1 change: 0 additions & 1 deletion components/brave_rewards/browser/rewards_service.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,6 @@ void RewardsService::RegisterProfilePrefs(PrefRegistrySimple* registry) {
#endif
registry->RegisterUint64Pref(prefs::kPromotionLastFetchStamp, 0ull);
registry->RegisterBooleanPref(prefs::kPromotionCorruptedMigrated, false);
registry->RegisterBooleanPref(prefs::kAnonTransferChecked, false);
registry->RegisterIntegerPref(prefs::kVersion, 0);
registry->RegisterIntegerPref(prefs::kMinVisitTime, 8);
registry->RegisterIntegerPref(prefs::kMinVisits, 1);
Expand Down
1 change: 0 additions & 1 deletion components/brave_rewards/common/pref_names.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,6 @@ const char kPromotionLastFetchStamp[] =
"brave.rewards.promotion_last_fetch_stamp";
const char kPromotionCorruptedMigrated[] =
"brave.rewards.promotion_corrupted_migrated2";
const char kAnonTransferChecked[] = "brave.rewards.anon_transfer_checked";
const char kVersion[] = "brave.rewards.version";
const char kMinVisitTime[] = "brave.rewards.ac.min_visit_time";
const char kMinVisits[] = "brave.rewards.ac.min_visits";
Expand Down
1 change: 0 additions & 1 deletion components/brave_rewards/common/pref_names.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@ extern const char kServerPublisherListStamp[];
extern const char kUpholdAnonAddress[]; // DEPRECATED
extern const char kPromotionLastFetchStamp[];
extern const char kPromotionCorruptedMigrated[];
extern const char kAnonTransferChecked[]; // DEPRECATED
extern const char kVersion[];
extern const char kMinVisitTime[];
extern const char kMinVisits[];
Expand Down
5 changes: 4 additions & 1 deletion vendor/bat-native-ledger/src/bat/ledger/internal/uphold/uphold_authorization.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,13 +29,13 @@ class UpholdAuthorization {
void Authorize(const base::flat_map<std::string, std::string>& args,
ledger::ExternalWalletAuthorizationCallback callback) const;

private:
void GetAnonFunds(
endpoint::promotion::GetWalletBalanceCallback callback) const;

void LinkWallet(const double user_funds,
endpoint::promotion::PostClaimUpholdCallback callback) const;

private:
void OnAuthorize(const type::Result result,
const std::string& token,
ledger::ExternalWalletAuthorizationCallback callback) const;
Expand All @@ -59,6 +59,9 @@ class UpholdAuthorization {
LedgerImpl* ledger_; // NOT OWNED
std::unique_ptr<endpoint::PromotionServer> promotion_server_;
std::unique_ptr<endpoint::UpholdServer> uphold_server_;

FRIEND_TEST_ALL_PREFIXES(GetAnonFundsTest, AnonFundsFlow);
FRIEND_TEST_ALL_PREFIXES(LinkWalletTest, LinkWalletFlow);
};

} // namespace uphold
Expand Down
79 changes: 40 additions & 39 deletions vendor/bat-native-ledger/src/bat/ledger/internal/uphold/uphold_authorization_unittest.cc
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
/* Copyright (c) 2019 The Brave Authors. All rights reserved.
/* Copyright (c) 2021 The Brave Authors. All rights reserved.
* This Source Code Form is subject to the terms of the Mozilla Public
* License, v. 2.0. If a copy of the MPL was not distributed with this
* file, You can obtain one at http://mozilla.org/MPL/2.0/. */
Expand All @@ -25,17 +25,16 @@ using ::testing::Values;

namespace ledger {
namespace uphold {
namespace tests {

using GetAnonFundsParamType =
using GetAnonFundsTestParamType =
std::tuple<bool, // fetch old balance enabled
std::string, // Brave wallet
type::UrlResponse, // balance server response
std::pair<type::Result,
base::Optional<double>> // expected result
>;

class GetAnonFunds : public TestWithParam<GetAnonFundsParamType> {
class GetAnonFundsTest : public TestWithParam<GetAnonFundsTestParamType> {
private:
base::test::TaskEnvironment scoped_task_environment_;

Expand All @@ -46,7 +45,7 @@ class GetAnonFunds : public TestWithParam<GetAnonFundsParamType> {
std::unique_ptr<UpholdAuthorization> authorization_;

public:
GetAnonFunds()
GetAnonFundsTest()
: mock_ledger_client_{std::make_unique<MockLedgerClient>()},
mock_ledger_impl_{
std::make_unique<MockLedgerImpl>(mock_ledger_client_.get())},
Expand All @@ -57,50 +56,53 @@ class GetAnonFunds : public TestWithParam<GetAnonFundsParamType> {
};

std::string GetAnonFundsNameSuffixGenerator(
const TestParamInfo<GetAnonFunds::ParamType>& info) {
const TestParamInfo<GetAnonFundsTest::ParamType>& info) {
const bool fetch_old_balance_enabled = std::get<0>(info.param);
const std::string& brave_wallet = std::get<1>(info.param);
const type::UrlResponse& balance_server_response = std::get<2>(info.param);

if (!fetch_old_balance_enabled) {
return "fetch_old_balance_disabled";
return "FetchOldBalanceDisabled";
}

if (brave_wallet.empty()) {
return "brave_wallet_is_not_created";
return "BraveWalletIsNotCreated";
}

if (brave_wallet.find(R"("payment_id": "")") != std::string::npos) {
return "brave_wallet_payment_id_is_empty";
return "BraveWalletPaymentIdIsEmpty";
}

if (balance_server_response.status_code != net::HttpStatusCode::HTTP_OK) {
return "balance_server_error";
return "BalanceServerError";
}

if (balance_server_response.body.empty()) {
return "invalid_body_in_balance_server_response";
return "InvalidBodyInBalanceServerResponse";
}

return "happy_path";
return "HappyPath";
}

INSTANTIATE_TEST_SUITE_P(
UpholdAuthorizationTest,
GetAnonFunds,
UpholdAuthorization,
GetAnonFundsTest,
Values(
// "fetch_old_balance_disabled"
GetAnonFundsParamType{false, {}, {}, {type::Result::LEDGER_OK, 0.0}},
GetAnonFundsTestParamType{false,
{},
{},
{type::Result::LEDGER_OK, 0.0}},
// "brave_wallet_is_not_created"
GetAnonFundsParamType{true, {}, {}, {type::Result::LEDGER_OK, 0.0}},
GetAnonFundsTestParamType{true, {}, {}, {type::Result::LEDGER_OK, 0.0}},
// "brave_wallet_payment_id_is_empty"
GetAnonFundsParamType{
GetAnonFundsTestParamType{
true,
R"({ "payment_id": "", "recovery_seed": "OG2zYotDSeZ81qLtr/uq5k/GC6WE5/7BclT1lHi4l+w=" })",
{},
{type::Result::LEDGER_ERROR, {}}},
// "balance_server_error"
GetAnonFundsParamType{
GetAnonFundsTestParamType{
true,
R"({ "payment_id": "f375da3c-c206-4f09-9422-665b8e5952db", "recovery_seed": "OG2zYotDSeZ81qLtr/uq5k/GC6WE5/7BclT1lHi4l+w=" })",
type::UrlResponse{{},
Expand All @@ -110,13 +112,13 @@ INSTANTIATE_TEST_SUITE_P(
{}},
{type::Result::LEDGER_ERROR, {}}},
// "invalid_body_in_balance_server_response"
GetAnonFundsParamType{
GetAnonFundsTestParamType{
true,
R"({ "payment_id": "f375da3c-c206-4f09-9422-665b8e5952db", "recovery_seed": "OG2zYotDSeZ81qLtr/uq5k/GC6WE5/7BclT1lHi4l+w=" })",
type::UrlResponse{{}, {}, net::HttpStatusCode::HTTP_OK, {}, {}},
{type::Result::LEDGER_ERROR, 0.0}},
// "happy_path"
GetAnonFundsParamType{
GetAnonFundsTestParamType{
true,
R"({ "payment_id": "f375da3c-c206-4f09-9422-665b8e5952db", "recovery_seed": "OG2zYotDSeZ81qLtr/uq5k/GC6WE5/7BclT1lHi4l+w=" })",
type::UrlResponse{
Expand All @@ -128,7 +130,7 @@ INSTANTIATE_TEST_SUITE_P(
{type::Result::LEDGER_OK, 5.0}}),
GetAnonFundsNameSuffixGenerator);

TEST_P(GetAnonFunds, ) {
TEST_P(GetAnonFundsTest, AnonFundsFlow) {
const auto& params = GetParam();
const bool fetch_old_balance_enabled = std::get<0>(params);
const std::string& brave_wallet = std::get<1>(params);
Expand Down Expand Up @@ -165,13 +167,13 @@ TEST_P(GetAnonFunds, ) {
});
}

using LinkWalletParamType =
using LinkWalletTestParamType =
std::tuple<std::string, // Uphold wallet
type::UrlResponse, // rewards web services response
type::Result // expected result
>;

class LinkWallet : public TestWithParam<LinkWalletParamType> {
class LinkWalletTest : public TestWithParam<LinkWalletTestParamType> {
private:
base::test::TaskEnvironment scoped_task_environment_;

Expand All @@ -181,7 +183,7 @@ class LinkWallet : public TestWithParam<LinkWalletParamType> {
std::unique_ptr<UpholdAuthorization> authorization_;

public:
LinkWallet()
LinkWalletTest()
: mock_ledger_client_{std::make_unique<MockLedgerClient>()},
mock_ledger_impl_{
std::make_unique<MockLedgerImpl>(mock_ledger_client_.get())},
Expand All @@ -190,13 +192,13 @@ class LinkWallet : public TestWithParam<LinkWalletParamType> {
};

std::string LinkWalletNameSuffixGenerator(
const TestParamInfo<LinkWallet::ParamType>& info) {
const TestParamInfo<LinkWalletTest::ParamType>& info) {
const std::string& uphold_wallet = std::get<0>(info.param);
const type::UrlResponse& rewards_web_services_response =
std::get<1>(info.param);

if (uphold_wallet.empty()) {
return "uphold_wallet_is_null";
return "UpholdWalletIsNull";
}

if (rewards_web_services_response.status_code ==
Expand All @@ -206,15 +208,15 @@ std::string LinkWalletNameSuffixGenerator(

if (rewards_web_services_response.status_code ==
net::HttpStatusCode::HTTP_CONFLICT) {
return "wallet_device_limit_reached";
return "WalletDeviceLimitReached";
}

if (rewards_web_services_response.status_code !=
net::HttpStatusCode::HTTP_OK) {
return "rewards_web_services_error";
return "RewardsWebServicesError";
}

return "happy_path";
return "HappyPath";
}

const char uphold_wallet[]{R"(
Expand All @@ -234,14 +236,14 @@ const char uphold_wallet[]{R"(
)"};

INSTANTIATE_TEST_SUITE_P(
UpholdAuthorizationTest,
LinkWallet,
UpholdAuthorization,
LinkWalletTest,
Values(
// "uphold_wallet_is_null"
LinkWalletParamType{"", type::UrlResponse{},
type::Result::LEDGER_ERROR},
LinkWalletTestParamType{"", type::UrlResponse{},
type::Result::LEDGER_ERROR},
// "404"
LinkWalletParamType{
LinkWalletTestParamType{
uphold_wallet,
type::UrlResponse{{},
{},
Expand All @@ -250,7 +252,7 @@ INSTANTIATE_TEST_SUITE_P(
{}},
type::Result::NOT_FOUND},
// "wallet_device_limit_reached"
LinkWalletParamType{
LinkWalletTestParamType{
uphold_wallet,
type::UrlResponse{{},
{},
Expand All @@ -259,7 +261,7 @@ INSTANTIATE_TEST_SUITE_P(
{}},
type::Result::ALREADY_EXISTS},
// "rewards_web_services_error"
LinkWalletParamType{
LinkWalletTestParamType{
uphold_wallet,
type::UrlResponse{{},
{},
Expand All @@ -268,13 +270,13 @@ INSTANTIATE_TEST_SUITE_P(
{}},
type::Result::LEDGER_ERROR},
// "happy_path"
LinkWalletParamType{
LinkWalletTestParamType{
uphold_wallet,
type::UrlResponse{{}, {}, net::HttpStatusCode::HTTP_OK, {}, {}},
type::Result::LEDGER_OK}),
LinkWalletNameSuffixGenerator);

TEST_P(LinkWallet, ) {
TEST_P(LinkWalletTest, LinkWalletFlow) {
const auto& params = GetParam();
const std::string& uphold_wallet = std::get<0>(params);
const type::UrlResponse& rewards_web_services_response = std::get<1>(params);
Expand All @@ -294,6 +296,5 @@ TEST_P(LinkWallet, ) {
});
}

} // namespace tests
} // namespace uphold
} // namespace ledger
2 changes: 1 addition & 1 deletion vendor/bat-native-ledger/src/bat/ledger/internal/uphold/uphold_wallet.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ void UpholdWallet::OnCreateCard(
}

void UpholdWallet::OnTransferTokens(const type::Result result,
std::string drain_id,
const std::string& drain_id,
ledger::ResultCallback callback) {
if (result != type::Result::LEDGER_OK) {
BLOG(0, "Claiming tokens failed");
Expand Down
2 changes: 1 addition & 1 deletion vendor/bat-native-ledger/src/bat/ledger/internal/uphold/uphold_wallet.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ class UpholdWallet {
ledger::ResultCallback callback);

void OnTransferTokens(const type::Result result,
std::string drain_id,
const std::string& drain_id,
ledger::ResultCallback callback);

type::WalletStatus GetNewStatus(const type::WalletStatus old_status,
Expand Down

0 comments on commit fbf7d7c

Please sign in to comment.