Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Issues/2601 #1134

Merged
merged 4 commits into from
Dec 19, 2018
Merged

Issues/2601 #1134

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b6f670b
add migration
bridiver Dec 17, 2018
82fb01d
add support for region in the query
bridiver Dec 18, 2018
d0d1982
add new fields to updates/queries
bridiver Dec 18, 2018
ff607a4
Resolve issue where not all Ads were returned
tmancey Dec 19, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ deps = {
"vendor/bat-native-tweetnacl": "https://github.com/brave-intl/bat-native-tweetnacl.git@1b4362968c8f22720bfb75af6f506d4ecc0f3116",
"components/brave_sync/extension/brave-sync": "https://github.com/brave/sync.git@bf82f868e1eff9f7d94b84a09d3e70236c1c86af",
"components/brave_sync/extension/brave-crypto": "https://github.com/brave/crypto@7e391cec6975106fa9f686016f494cb8a782afcd",
"vendor/bat-native-ads": "https://github.com/brave-intl/bat-native-ads.git@6a2b8459c2a52e9649b6e498101d56f4131c5e6f",
"vendor/bat-native-ads": "https://github.com/brave-intl/bat-native-ads.git@81e0e7f0e050ab4efc879153c506df376ad341c6",
"vendor/bat-native-usermodel": "https://github.com/brave-intl/bat-native-usermodel.git@c3b6111aa862c5c452c84be8a225d5f1df32b284",
}

Expand Down
4 changes: 3 additions & 1 deletion components/brave_ads/browser/ads_service_impl.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -203,13 +203,14 @@ std::string LoadOnFileTaskRunner(
}

std::vector<ads::AdInfo> GetAdsForCategoryOnFileTaskRunner(
const std::string region,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how granular are the regions? we want to avoid having users uniquely identifiable by region + category

const std::string category,
BundleStateDatabase* backend) {
std::vector<ads::AdInfo> ads;
if (!backend)
return ads;

backend->GetAdsForCategory(category, ads);
backend->GetAdsForCategory(region, category, ads);

return ads;
}
Expand Down Expand Up @@ -646,6 +647,7 @@ void AdsServiceImpl::GetAds(
ads::OnGetAdsCallback callback) {
base::PostTaskAndReplyWithResult(file_task_runner_.get(), FROM_HERE,
base::BindOnce(&GetAdsForCategoryOnFileTaskRunner,
region,
category,
bundle_state_backend_.get()),
base::BindOnce(&AdsServiceImpl::OnGetAdsForCategory,
Expand Down
97 changes: 86 additions & 11 deletions components/brave_ads/browser/bundle_state_database.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,8 +19,8 @@ namespace brave_ads {

namespace {

const int kCurrentVersionNumber = 1;
const int kCompatibleVersionNumber = 1;
const int kCurrentVersionNumber = 2;
const int kCompatibleVersionNumber = 2;

} // namespace

Expand Down Expand Up @@ -123,6 +123,10 @@ bool BundleStateDatabase::CreateAdInfoTable() {
"end_timestamp DATETIME,"
"uuid LONGVARCHAR,"
"region VARCHAR,"
"campaign_id LONGVARCHAR,"
"daily_cap INTEGER DEFAULT 0 NOT NULL,"
"per_day INTEGER DEFAULT 0 NOT NULL,"
"total_max INTEGER DEFAULT 0 NOT NULL,"
"PRIMARY KEY(region, uuid))");
return GetDB().Execute(sql.c_str());
}
Expand Down Expand Up @@ -274,8 +278,9 @@ bool BundleStateDatabase::InsertOrUpdateAdInfo(const ads::AdInfo& info) {
GetDB().GetCachedStatement(SQL_FROM_HERE,
"INSERT OR REPLACE INTO ad_info "
"(creative_set_id, advertiser, notification_text, "
"notification_url, start_timestamp, end_timestamp, uuid, region) "
"VALUES (?, ?, ?, ?, datetime(?), datetime(?), ?, ?)"));
"notification_url, start_timestamp, end_timestamp, uuid, "
"campaign_id, daily_cap, per_day, total_max, region) "
"VALUES (?, ?, ?, ?, datetime(?), datetime(?), ?, ?, ?, ?, ?, ?)"));

ad_info_statement.BindString(0, info.creative_set_id);
ad_info_statement.BindString(1, info.advertiser);
Expand All @@ -284,7 +289,11 @@ bool BundleStateDatabase::InsertOrUpdateAdInfo(const ads::AdInfo& info) {
ad_info_statement.BindString(4, info.start_timestamp);
ad_info_statement.BindString(5, info.end_timestamp);
ad_info_statement.BindString(6, info.uuid);
ad_info_statement.BindString(7, *it);
ad_info_statement.BindString(7, info.campaign_id);
ad_info_statement.BindInt(8, info.daily_cap);
ad_info_statement.BindInt(9, info.per_day);
ad_info_statement.BindInt(10, info.total_max);
ad_info_statement.BindString(11, *it);
if (!ad_info_statement.Run()) {
return false;
}
Expand Down Expand Up @@ -316,7 +325,8 @@ bool BundleStateDatabase::InsertOrUpdateAdInfoCategory(
return ad_info_statement.Run();
}

bool BundleStateDatabase::GetAdsForCategory(const std::string& category,
bool BundleStateDatabase::GetAdsForCategory(const std::string& region,
const std::string& category,
std::vector<ads::AdInfo>& ads) {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

Expand All @@ -329,20 +339,35 @@ bool BundleStateDatabase::GetAdsForCategory(const std::string& category,
sql::Statement info_sql(
db_.GetUniqueStatement("SELECT ai.creative_set_id, ai.advertiser, "
"ai.notification_text, ai.notification_url, "
"ai.uuid FROM ad_info AS ai "
"ai.start_timestamp, ai.end_timestamp, "
"ai.uuid, ai.campaign_id, ai.daily_cap, "
"ai.per_day, ai.total_max FROM ad_info AS ai "
"INNER JOIN ad_info_category AS aic "
"ON aic.ad_info_uuid = ai.uuid "
"WHERE aic.category_name = ?"));

"WHERE aic.category_name = ? and "
// TODO(tmancey) - use region in the query
// "ai.region = ? and "
"ai.start_timestamp <= strftime('%Y-%m-%d %H:%M', "
"datetime('now','localtime')) and "
"ai.end_timestamp >= strftime('%Y-%m-%d %H:%M', "
"datetime('now','localtime'));"));
info_sql.BindString(0, category);
// TODO(tmancey) - use region in the query
// info_sql.BindString(1, region);

if (info_sql.Step()) {
while (info_sql.Step()) {
ads::AdInfo info;
info.creative_set_id = info_sql.ColumnString(0);
info.advertiser = info_sql.ColumnString(1);
info.notification_text = info_sql.ColumnString(2);
info.notification_url = info_sql.ColumnString(3);
info.uuid = info_sql.ColumnString(4);
info.start_timestamp = info_sql.ColumnString(4);
info.end_timestamp = info_sql.ColumnString(5);
info.uuid = info_sql.ColumnString(6);
info.campaign_id = info_sql.ColumnString(7);
info.daily_cap = info_sql.ColumnInt(8);
info.per_day = info_sql.ColumnInt(9);
info.total_max = info_sql.ColumnInt(10);
ads.emplace_back(info);
}

Expand Down Expand Up @@ -391,6 +416,44 @@ sql::MetaTable& BundleStateDatabase::GetMetaTable() {
return meta_table_;
}

bool BundleStateDatabase::MigrateV1toV2() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

if (!GetDB().BeginTransaction())
return false;

std::string sql = "ALTER TABLE ad_info ADD campaign_id LONGVARCHAR;";
if (!GetDB().Execute(sql.c_str())) {
GetDB().RollbackTransaction();
return false;
}

sql = "ALTER TABLE ad_info ADD daily_cap INTEGER DEFAULT 0 NOT NULL;";
if (!GetDB().Execute(sql.c_str())) {
GetDB().RollbackTransaction();
return false;
}

sql = "ALTER TABLE ad_info ADD per_day INTEGER DEFAULT 0 NOT NULL;";
if (!GetDB().Execute(sql.c_str())) {
GetDB().RollbackTransaction();
return false;
}

sql = "ALTER TABLE ad_info ADD total_max INTEGER DEFAULT 0 NOT NULL;";
if (!GetDB().Execute(sql.c_str())) {
GetDB().RollbackTransaction();
return false;
}

if (GetDB().CommitTransaction()) {
Vacuum();
return true;
}

return false;
}

sql::InitStatus BundleStateDatabase::EnsureCurrentVersion() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

Expand All @@ -400,6 +463,18 @@ sql::InitStatus BundleStateDatabase::EnsureCurrentVersion() {
return sql::INIT_TOO_NEW;
}

const int old_version = meta_table_.GetVersionNumber();
const int cur_version = GetCurrentVersion();

// Migration from version 1 to version 2
if (old_version == 1 && cur_version == 2) {
if (!MigrateV1toV2()) {
LOG(ERROR) << "DB: Error with MigrateV1toV2";
}

meta_table_.SetVersionNumber(cur_version);
}

return sql::INIT_OK;
}

Expand Down
4 changes: 3 additions & 1 deletion components/brave_ads/browser/bundle_state_database.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,8 @@ class BundleStateDatabase {
}

bool SaveBundleState(const ads::BundleState& bundle_state);
bool GetAdsForCategory(const std::string& category,
bool GetAdsForCategory(const std::string& region,
const std::string& category,
std::vector<ads::AdInfo>& ads);

// Returns the current version of the publisher info database
Expand Down Expand Up @@ -68,6 +69,7 @@ class BundleStateDatabase {
sql::Database& GetDB();
sql::MetaTable& GetMetaTable();

bool MigrateV1toV2();
sql::InitStatus EnsureCurrentVersion();

sql::Database db_;
Expand Down