Skip to content

Commit

Permalink
Merge pull request #26866 from brave/issues/42598
Browse files Browse the repository at this point in the history 
[ads] Optimize database queries serialization/deserialization overhead
  • Loading branch information
@aseren
aseren authored Dec 10, 2024
2 parents 9684ea1 + 6216a05 commit 55e552b
Show file tree
Hide file tree
Showing 101 changed files with 1,752 additions and 676 deletions.
24 changes: 1 addition & 23 deletions components/brave_ads/browser/ads_service_impl.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@
#include "brave/components/brave_ads/core/public/ad_units/notification_ad/notification_ad_value_util.h"
#include "brave/components/brave_ads/core/public/ads_constants.h"
#include "brave/components/brave_ads/core/public/ads_feature.h"
#include "brave/components/brave_ads/core/public/database/database.h"
#include "brave/components/brave_ads/core/public/flags/flags_util.h"
#include "brave/components/brave_ads/core/public/history/site_history.h"
#include "brave/components/brave_ads/core/public/prefs/pref_names.h"
Expand Down Expand Up @@ -349,6 +348,7 @@ void AdsServiceImpl::StartBatAdsService() {
}

bat_ads_service_remote_->Create(
ads_service_path_,
bat_ads_client_associated_receiver_.BindNewEndpointAndPassRemote(),
bat_ads_associated_remote_.BindNewEndpointAndPassReceiver(),
std::move(bat_ads_client_notifier_pending_receiver_),
Expand Down Expand Up @@ -408,18 +408,9 @@ void AdsServiceImpl::Initialize(size_t current_start_number) {
return;
}

InitializeDatabase();

InitializeRewardsWallet(current_start_number);
}

void AdsServiceImpl::InitializeDatabase() {
CHECK(!database_);

database_ = base::SequenceBound<Database>(
file_task_runner_, ads_service_path_.AppendASCII(kDatabaseFilename));
}

void AdsServiceImpl::InitializeRewardsWallet(size_t current_start_number) {
rewards_service_observation_.GetSource()->GetRewardsWallet(
base::BindOnce(&AdsServiceImpl::InitializeRewardsWalletCallback,
Expand Down Expand Up @@ -1096,8 +1087,6 @@ void AdsServiceImpl::ShutdownAdsService() {

CloseAdaptiveCaptcha();

database_.Reset();

if (is_bat_ads_initialized_) {
VLOG(2) << "Shutdown Bat Ads Service";
}
Expand Down Expand Up @@ -1733,17 +1722,6 @@ void AdsServiceImpl::ShowScheduledCaptcha(const std::string& payment_id,
#endif // !BUILDFLAG(IS_ANDROID)
}

void AdsServiceImpl::RunDBTransaction(
mojom::DBTransactionInfoPtr mojom_db_transaction,
RunDBTransactionCallback callback) {
CHECK(mojom_db_transaction);
CHECK(database_);

database_.AsyncCall(&Database::RunDBTransaction)
.WithArgs(std::move(mojom_db_transaction))
.Then(std::move(callback));
}

void AdsServiceImpl::RecordP2AEvents(const std::vector<std::string>& events) {
for (const auto& event : events) {
RecordAndEmitP2AHistogramName(prefs_,
Expand Down
8 changes: 0 additions & 8 deletions components/brave_ads/browser/ads_service_impl.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
#include "base/memory/scoped_refptr.h"
#include "base/memory/weak_ptr.h"
#include "base/task/cancelable_task_tracker.h"
#include "base/threading/sequence_bound.h"
#include "base/time/time.h"
#include "base/timer/timer.h"
#include "brave/components/brave_adaptive_captcha/brave_adaptive_captcha_service.h"
Expand Down Expand Up @@ -61,7 +60,6 @@ namespace brave_ads {
class AdsServiceObserver;
class AdsTooltipsDelegate;
class BatAdsServiceFactory;
class Database;
class DeviceId;
class ResourceComponent;
struct NewTabPageAdInfo;
Expand Down Expand Up @@ -124,7 +122,6 @@ class AdsServiceImpl final : public AdsService,
void InitializeBasePathDirectoryCallback(size_t current_start_number,
bool success);
void Initialize(size_t current_start_number);
void InitializeDatabase();
void InitializeRewardsWallet(size_t current_start_number);
void InitializeRewardsWalletCallback(
size_t current_start_number,
Expand Down Expand Up @@ -357,9 +354,6 @@ class AdsServiceImpl final : public AdsService,
void ShowScheduledCaptcha(const std::string& payment_id,
const std::string& captcha_id) override;

void RunDBTransaction(mojom::DBTransactionInfoPtr mojom_db_transaction,
RunDBTransactionCallback callback) override;

// TODO(https://github.com/brave/brave-browser/issues/14666) Decouple P2A
// business logic.
void RecordP2AEvents(const std::vector<std::string>& events) override;
Expand Down Expand Up @@ -427,8 +421,6 @@ class AdsServiceImpl final : public AdsService,

mojom::SysInfo sys_info_;

base::SequenceBound<Database> database_;

base::RepeatingTimer idle_state_timer_;
ui::IdleState last_idle_state_ = ui::IdleState::IDLE_STATE_ACTIVE;
base::TimeDelta last_idle_time_;
Expand Down
1 change: 1 addition & 0 deletions components/brave_ads/core/internal/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -583,6 +583,7 @@ static_library("internal") {
"creatives/segments_database_util.cc",
"creatives/segments_database_util.h",
"database/database.cc",
"database/database.h",
"database/database_maintenance.cc",
"database/database_maintenance.h",
"database/database_manager.cc",
Expand Down
97 changes: 78 additions & 19 deletions components/brave_ads/core/internal/account/account_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
#include "brave/components/brave_ads/core/internal/account/account.h"

#include "base/memory/raw_ptr.h"
#include "base/run_loop.h"
#include "base/test/gmock_callback_support.h"
#include "base/test/mock_callback.h"
#include "brave/components/brave_ads/core/internal/account/account_observer_mock.h"
#include "brave/components/brave_ads/core/internal/account/issuers/issuers_test_util.h"
Expand Down Expand Up @@ -180,8 +182,11 @@ TEST_F(BraveAdsAccountTest, GetStatementForRewardsUser) {
{mojom::AdType::kNotificationAd, 3}};

base::MockCallback<GetStatementOfAccountsCallback> callback;
EXPECT_CALL(callback, Run(::testing::Eq(std::ref(expected_mojom_statement))));
base::RunLoop run_loop;
EXPECT_CALL(callback, Run(::testing::Eq(std::ref(expected_mojom_statement))))
.WillOnce(base::test::RunOnceClosure(run_loop.QuitClosure()));
GetAccount().GetStatement(callback.Get());
run_loop.Run();
}

TEST_F(BraveAdsAccountTest, DoNotGetStatementForNonRewardsUser) {
Expand All @@ -190,8 +195,11 @@ TEST_F(BraveAdsAccountTest, DoNotGetStatementForNonRewardsUser) {

// Act & Assert
base::MockCallback<GetStatementOfAccountsCallback> callback;
EXPECT_CALL(callback, Run(/*statement=*/::testing::IsFalse()));
base::RunLoop run_loop;
EXPECT_CALL(callback, Run(/*statement=*/::testing::IsFalse()))
.WillOnce(base::test::RunOnceClosure(run_loop.QuitClosure()));
GetAccount().GetStatement(callback.Get());
run_loop.Run();
}

TEST_F(BraveAdsAccountTest, DepositForCash) {
Expand All @@ -214,18 +222,24 @@ TEST_F(BraveAdsAccountTest, DepositForCash) {
database::SaveCreativeNotificationAds({creative_ad});

// Act & Assert
base::RunLoop run_loop1;
EXPECT_CALL(account_observer_mock_,
OnDidProcessDeposit(/*transaction=*/::testing::FieldsAre(
/*id*/ ::testing::_, /*created_at*/ test::Now(),
test::kCreativeInstanceId, test::kSegment, test::kValue,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression,
/*reconciled_at*/ std::nullopt)));
/*reconciled_at*/ std::nullopt)))
.WillOnce(base::test::RunOnceClosure(run_loop1.QuitClosure()));
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit).Times(0);
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange);
base::RunLoop run_loop2;
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange)
.WillOnce(base::test::RunOnceClosure(run_loop2.QuitClosure()));
GetAccount().Deposit(test::kCreativeInstanceId, test::kSegment,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression);
run_loop1.Run();
run_loop2.Run();
}

TEST_F(BraveAdsAccountTest, DepositForCashWithUserData) {
Expand All @@ -248,57 +262,75 @@ TEST_F(BraveAdsAccountTest, DepositForCashWithUserData) {
database::SaveCreativeNotificationAds({creative_ad});

// Act & Assert
base::RunLoop run_loop1;
EXPECT_CALL(account_observer_mock_,
OnDidProcessDeposit(/*transaction=*/::testing::FieldsAre(
/*id*/ ::testing::_, /*created_at*/ test::Now(),
test::kCreativeInstanceId, test::kSegment, test::kValue,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression,
/*reconciled_at*/ std::nullopt)));
/*reconciled_at*/ std::nullopt)))
.WillOnce(base::test::RunOnceClosure(run_loop1.QuitClosure()));
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit).Times(0);
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange);
base::RunLoop run_loop2;
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange)
.WillOnce(base::test::RunOnceClosure(run_loop2.QuitClosure()));
GetAccount().DepositWithUserData(test::kCreativeInstanceId, test::kSegment,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression,
/*user_data=*/{});
run_loop1.Run();
run_loop2.Run();
}

TEST_F(BraveAdsAccountTest, DepositForNonCash) {
// Arrange
test::MockTokenGenerator(/*count=*/1);

// Act & Assert
base::RunLoop run_loop1;
EXPECT_CALL(
account_observer_mock_,
OnDidProcessDeposit(/*transaction=*/::testing::FieldsAre(
/*id*/ ::testing::_, /*created_at*/ test::Now(),
test::kCreativeInstanceId, test::kSegment, /*value*/ 0.0,
mojom::AdType::kNotificationAd, mojom::ConfirmationType::kClicked,
/*reconciled_at*/ std::nullopt)));
/*reconciled_at*/ std::nullopt)))
.WillOnce(base::test::RunOnceClosure(run_loop1.QuitClosure()));
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit).Times(0);
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange);
base::RunLoop run_loop2;
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange)
.WillOnce(base::test::RunOnceClosure(run_loop2.QuitClosure()));
GetAccount().Deposit(test::kCreativeInstanceId, test::kSegment,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kClicked);
run_loop1.Run();
run_loop2.Run();
}

TEST_F(BraveAdsAccountTest, DepositForNonCashWithUserData) {
// Arrange
test::MockTokenGenerator(/*count=*/1);

// Act & Assert
base::RunLoop run_loop1;
EXPECT_CALL(
account_observer_mock_,
OnDidProcessDeposit(/*transaction=*/::testing::FieldsAre(
/*id*/ ::testing::_, /*created_at*/ test::Now(),
test::kCreativeInstanceId, test::kSegment, /*value*/ 0.0,
mojom::AdType::kNotificationAd, mojom::ConfirmationType::kClicked,
/*reconciled_at*/ std::nullopt)));
/*reconciled_at*/ std::nullopt)))
.WillOnce(base::test::RunOnceClosure(run_loop1.QuitClosure()));
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit).Times(0);
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange);
base::RunLoop run_loop2;
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange)
.WillOnce(base::test::RunOnceClosure(run_loop2.QuitClosure()));
GetAccount().DepositWithUserData(
test::kCreativeInstanceId, test::kSegment, mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kClicked, /*user_data=*/{});
run_loop1.Run();
run_loop2.Run();
}

TEST_F(BraveAdsAccountTest, DoNotDepositCashIfCreativeInstanceIdDoesNotExist) {
Expand All @@ -310,12 +342,15 @@ TEST_F(BraveAdsAccountTest, DoNotDepositCashIfCreativeInstanceIdDoesNotExist) {
database::SaveCreativeNotificationAds({creative_ad});

// Act & Assert
base::RunLoop run_loop;
EXPECT_CALL(account_observer_mock_, OnDidProcessDeposit).Times(0);
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit);
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit)
.WillOnce(base::test::RunOnceClosure(run_loop.QuitClosure()));
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange).Times(0);
GetAccount().Deposit(test::kMissingCreativeInstanceId, test::kSegment,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression);
run_loop.Run();
}

TEST_F(BraveAdsAccountTest, AddTransactionWhenDepositingCashForRewardsUser) {
Expand All @@ -327,28 +362,37 @@ TEST_F(BraveAdsAccountTest, AddTransactionWhenDepositingCashForRewardsUser) {
database::SaveCreativeNotificationAds({creative_ad});

// Act
base::RunLoop run_loop1;
EXPECT_CALL(account_observer_mock_,
OnDidProcessDeposit(/*transaction=*/::testing::FieldsAre(
/*id*/ ::testing::_, /*created_at*/ test::Now(),
test::kCreativeInstanceId, test::kSegment, test::kValue,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression,
/*reconciled_at*/ std::nullopt)));
/*reconciled_at*/ std::nullopt)))
.WillOnce(base::test::RunOnceClosure(run_loop1.QuitClosure()));
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit).Times(0);
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange);
base::RunLoop run_loop2;
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange)
.WillOnce(base::test::RunOnceClosure(run_loop2.QuitClosure()));

GetAccount().Deposit(test::kCreativeInstanceId, test::kSegment,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kViewedImpression);
run_loop1.Run();
run_loop2.Run();

// Assert
base::MockCallback<database::table::GetTransactionsCallback> callback;
base::RunLoop run_loop3;
EXPECT_CALL(callback,
Run(/*success=*/true, /*transactions=*/::testing::SizeIs(1)));
Run(/*success=*/true, /*transactions=*/::testing::SizeIs(1)))
.WillOnce(base::test::RunOnceClosure(run_loop3.QuitClosure()));
const database::table::Transactions database_table;
database_table.GetForDateRange(/*from_time=*/test::DistantPast(),
/*to_time=*/test::DistantFuture(),
callback.Get());
run_loop3.Run();
}

TEST_F(BraveAdsAccountTest, AddTransactionWhenDepositingNonCashForRewardsUser) {
Expand All @@ -360,28 +404,37 @@ TEST_F(BraveAdsAccountTest, AddTransactionWhenDepositingNonCashForRewardsUser) {
database::SaveCreativeNotificationAds({creative_ad});

// Act
base::RunLoop run_loop1;
EXPECT_CALL(
account_observer_mock_,
OnDidProcessDeposit(/*transaction=*/::testing::FieldsAre(
/*id*/ ::testing::_, /*created_at*/ test::Now(),
test::kCreativeInstanceId, test::kSegment, /*value*/ 0.0,
mojom::AdType::kNotificationAd, mojom::ConfirmationType::kClicked,
/*reconciled_at*/ std::nullopt)));
/*reconciled_at*/ std::nullopt)))
.WillOnce(base::test::RunOnceClosure(run_loop1.QuitClosure()));
EXPECT_CALL(account_observer_mock_, OnFailedToProcessDeposit).Times(0);
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange);
base::RunLoop run_loop2;
EXPECT_CALL(*ads_observer_mock_, OnAdRewardsDidChange)
.WillOnce(base::test::RunOnceClosure(run_loop2.QuitClosure()));

GetAccount().Deposit(test::kCreativeInstanceId, test::kSegment,
mojom::AdType::kNotificationAd,
mojom::ConfirmationType::kClicked);
run_loop1.Run();
run_loop2.Run();

// Assert
base::MockCallback<database::table::GetTransactionsCallback> callback;
base::RunLoop run_loop3;
EXPECT_CALL(callback,
Run(/*success=*/true, /*transactions=*/::testing::SizeIs(1)));
Run(/*success=*/true, /*transactions=*/::testing::SizeIs(1)))
.WillOnce(base::test::RunOnceClosure(run_loop3.QuitClosure()));
const database::table::Transactions database_table;
database_table.GetForDateRange(/*from_time=*/test::DistantPast(),
/*to_time=*/test::DistantFuture(),
callback.Get());
run_loop3.Run();
}

TEST_F(BraveAdsAccountTest,
Expand All @@ -403,12 +456,15 @@ TEST_F(BraveAdsAccountTest,

// Assert
base::MockCallback<database::table::GetTransactionsCallback> callback;
base::RunLoop run_loop;
EXPECT_CALL(callback,
Run(/*success=*/true, /*transactions=*/::testing::IsEmpty()));
Run(/*success=*/true, /*transactions=*/::testing::IsEmpty()))
.WillOnce(base::test::RunOnceClosure(run_loop.QuitClosure()));
const database::table::Transactions database_table;
database_table.GetForDateRange(/*from_time=*/test::DistantPast(),
/*to_time=*/test::DistantFuture(),
callback.Get());
run_loop.Run();
}

TEST_F(BraveAdsAccountTest,
Expand All @@ -430,12 +486,15 @@ TEST_F(BraveAdsAccountTest,

// Assert
base::MockCallback<database::table::GetTransactionsCallback> callback;
base::RunLoop run_loop;
EXPECT_CALL(callback,
Run(/*success=*/true, /*transactions=*/::testing::IsEmpty()));
Run(/*success=*/true, /*transactions=*/::testing::IsEmpty()))
.WillOnce(base::test::RunOnceClosure(run_loop.QuitClosure()));
const database::table::Transactions database_table;
database_table.GetForDateRange(/*from_time=*/test::DistantPast(),
/*to_time=*/test::DistantFuture(),
callback.Get());
run_loop.Run();
}

} // namespace brave_ads
Loading

0 comments on commit 55e552b

Please sign in to comment.