From b6f670bc976b45315b7ca3e97bfcbeabe3e46c80 Mon Sep 17 00:00:00 2001 From: bridiver Date: Mon, 17 Dec 2018 16:04:15 -0700 Subject: [PATCH 1/4] add migration --- .../browser/bundle_state_database.cc | 54 ++++++++++++++++++- .../brave_ads/browser/bundle_state_database.h | 1 + 2 files changed, 53 insertions(+), 2 deletions(-) diff --git a/components/brave_ads/browser/bundle_state_database.cc b/components/brave_ads/browser/bundle_state_database.cc index 253827c672f5..2049b78d18c1 100644 --- a/components/brave_ads/browser/bundle_state_database.cc +++ b/components/brave_ads/browser/bundle_state_database.cc @@ -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 @@ -391,6 +391,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_); @@ -400,6 +438,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; } diff --git a/components/brave_ads/browser/bundle_state_database.h b/components/brave_ads/browser/bundle_state_database.h index 37ccdeea4c9e..73a72d16c60e 100644 --- a/components/brave_ads/browser/bundle_state_database.h +++ b/components/brave_ads/browser/bundle_state_database.h @@ -68,6 +68,7 @@ class BundleStateDatabase { sql::Database& GetDB(); sql::MetaTable& GetMetaTable(); + bool MigrateV1toV2(); sql::InitStatus EnsureCurrentVersion(); sql::Database db_; From 82fb01dba4d823745b00aec21f1ae6570f966e77 Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 18 Dec 2018 14:24:59 -0700 Subject: [PATCH 2/4] add support for region in the query --- components/brave_ads/browser/ads_service_impl.cc | 4 +++- components/brave_ads/browser/bundle_state_database.cc | 7 ++++++- components/brave_ads/browser/bundle_state_database.h | 3 ++- 3 files changed, 11 insertions(+), 3 deletions(-) diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc index caf11da7dab9..13c8e3966034 100644 --- a/components/brave_ads/browser/ads_service_impl.cc +++ b/components/brave_ads/browser/ads_service_impl.cc @@ -203,13 +203,14 @@ std::string LoadOnFileTaskRunner( } std::vector GetAdsForCategoryOnFileTaskRunner( + const std::string region, const std::string category, BundleStateDatabase* backend) { std::vector ads; if (!backend) return ads; - backend->GetAdsForCategory(category, ads); + backend->GetAdsForCategory(region, category, ads); return ads; } @@ -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, diff --git a/components/brave_ads/browser/bundle_state_database.cc b/components/brave_ads/browser/bundle_state_database.cc index 2049b78d18c1..4ada8df217cc 100644 --- a/components/brave_ads/browser/bundle_state_database.cc +++ b/components/brave_ads/browser/bundle_state_database.cc @@ -316,7 +316,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) { DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_); @@ -334,7 +335,11 @@ bool BundleStateDatabase::GetAdsForCategory(const std::string& category, "ON aic.ad_info_uuid = ai.uuid " "WHERE aic.category_name = ?")); + // TODO(tmancey) - use region in the query + // "ai.region = ? and " info_sql.BindString(0, category); + // TODO(tmancey) - use region in the query + // info_sql.BindString(1, region); if (info_sql.Step()) { ads::AdInfo info; diff --git a/components/brave_ads/browser/bundle_state_database.h b/components/brave_ads/browser/bundle_state_database.h index 73a72d16c60e..dfd1edd5b39c 100644 --- a/components/brave_ads/browser/bundle_state_database.h +++ b/components/brave_ads/browser/bundle_state_database.h @@ -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); // Returns the current version of the publisher info database From d0d1982b21b4828828507f5d1ab37ed6b69dfe8d Mon Sep 17 00:00:00 2001 From: bridiver Date: Tue, 18 Dec 2018 14:26:02 -0700 Subject: [PATCH 3/4] add new fields to updates/queries fix https://github.com/brave/brave-browser/issues/2601 --- DEPS | 2 +- .../browser/bundle_state_database.cc | 34 +++++++++++++++---- 2 files changed, 28 insertions(+), 8 deletions(-) diff --git a/DEPS b/DEPS index 9874811f6c3a..0416f2c03110 100644 --- a/DEPS +++ b/DEPS @@ -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@016dfaf7034e639a57089af9f5c2929c54afe8c1", "vendor/bat-native-usermodel": "https://github.com/brave-intl/bat-native-usermodel.git@c3b6111aa862c5c452c84be8a225d5f1df32b284", } diff --git a/components/brave_ads/browser/bundle_state_database.cc b/components/brave_ads/browser/bundle_state_database.cc index 4ada8df217cc..474e08d9824b 100644 --- a/components/brave_ads/browser/bundle_state_database.cc +++ b/components/brave_ads/browser/bundle_state_database.cc @@ -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()); } @@ -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); @@ -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; } @@ -330,13 +339,18 @@ bool BundleStateDatabase::GetAdsForCategory(const std::string& region, 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); @@ -347,7 +361,13 @@ bool BundleStateDatabase::GetAdsForCategory(const std::string& region, 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); } From ff607a4ceea1d2a08045b8be8b448d0de286ae70 Mon Sep 17 00:00:00 2001 From: Terry Mancey Date: Wed, 19 Dec 2018 13:26:16 +0000 Subject: [PATCH 4/4] Resolve issue where not all Ads were returned --- DEPS | 2 +- components/brave_ads/browser/bundle_state_database.cc | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/DEPS b/DEPS index 0416f2c03110..162ff3c3722a 100644 --- a/DEPS +++ b/DEPS @@ -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@016dfaf7034e639a57089af9f5c2929c54afe8c1", + "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", } diff --git a/components/brave_ads/browser/bundle_state_database.cc b/components/brave_ads/browser/bundle_state_database.cc index 474e08d9824b..667f005b0b48 100644 --- a/components/brave_ads/browser/bundle_state_database.cc +++ b/components/brave_ads/browser/bundle_state_database.cc @@ -355,7 +355,7 @@ bool BundleStateDatabase::GetAdsForCategory(const std::string& region, // 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);