Skip to content

Commit 9dc36a7

Browse files
kjozwiakkjozwiak
authored
Merge pull request #5837 from /issues/10240-1.11.x
Fixes ad conversions fail after a creative has expired - 1.11.x
2 parents 8f85998 + 114542d commit 9dc36a7

File tree

7 files changed

+82
-16
lines changed

7 files changed

+82
-16
lines changed

‎components/brave_ads/browser/bundle_state_database.cc

+44-13
Original file line numberDiff line numberDiff line change
@@ -25,8 +25,8 @@ namespace brave_ads {
2525

2626
namespace {
2727

28-
const int kCurrentVersionNumber = 6;
29-
const int kCompatibleVersionNumber = 6;
28+
const int kCurrentVersionNumber = 7;
29+
const int kCompatibleVersionNumber = 7;
3030

3131
} // namespace
3232

@@ -342,24 +342,27 @@ bool BundleStateDatabase::CreateAdConversionsTable() {
342342
// new constraints to the schema
343343
const std::string sql = base::StringPrintf(
344344
"CREATE TABLE %s "
345-
"(id INTEGER PRIMARY KEY, "
346-
"creative_set_id LONGVARCHAR NOT NULL, "
345+
"(creative_set_id LONGVARCHAR NOT NULL, "
347346
"type LONGVARCHAR NOT NULL, "
348347
"url_pattern LONGVARCHAR NOT NULL, "
349-
"observation_window INTEGER NOT NULL)",
348+
"observation_window INTEGER NOT NULL, "
349+
"expiry_timestamp TIMESTAMP, "
350+
"UNIQUE(creative_set_id, type, url_pattern), "
351+
"PRIMARY KEY(creative_set_id, type, url_pattern))",
350352
table_name);
351353

352354
return GetDB().Execute(sql.c_str());
353355
}
354356

355-
bool BundleStateDatabase::TruncateAdConversionsTable() {
357+
bool BundleStateDatabase::PurgeExpiredAdConversions() {
356358
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);
357359

358360
const bool is_initialized = Init();
359361
DCHECK(is_initialized);
360362

361363
const std::string sql =
362-
"DELETE FROM ad_conversions";
364+
"DELETE FROM ad_conversions "
365+
"WHERE strftime('%s','now') >= expiry_timestamp";
363366

364367
sql::Statement statement(GetDB().GetUniqueStatement(sql.c_str()));
365368

@@ -378,16 +381,18 @@ bool BundleStateDatabase::InsertOrUpdateAdConversion(
378381
"(creative_set_id, "
379382
"type, "
380383
"url_pattern, "
381-
"observation_window) VALUES (%s)",
382-
CreateBindingParameterPlaceholders(4).c_str());
384+
"observation_window, "
385+
"expiry_timestamp) VALUES (%s)",
386+
CreateBindingParameterPlaceholders(5).c_str());
383387

384388
sql::Statement statement(GetDB().GetUniqueStatement(sql.c_str()));
385389

386390
statement.BindString(0, info.creative_set_id);
387391
statement.BindString(1, info.type);
388392
statement.BindString(2, info.url_pattern);
389-
// Use BindInt64 for uint32_t types to avoid uint32_t to int32_t cast.
393+
// Use BindInt64 for uint32_t types to avoid uint32_t to int32_t cast
390394
statement.BindInt64(3, info.observation_window);
395+
statement.BindInt64(4, info.expiry_timestamp);
391396

392397
return statement.Run();
393398
}
@@ -403,11 +408,10 @@ bool BundleStateDatabase::SaveBundleState(
403408
return false;
404409
}
405410

406-
// We are completely replacing the database here so truncate all the tables
407411
if (!TruncateCategoriesTable() ||
408412
!TruncateCreativeAdNotificationCategoriesTable() ||
409413
!TruncateCreativeAdNotificationsTable() ||
410-
!TruncateAdConversionsTable()) {
414+
!PurgeExpiredAdConversions()) {
411415
GetDB().RollbackTransaction();
412416
return false;
413417
}
@@ -525,7 +529,8 @@ bool BundleStateDatabase::GetAdConversions(
525529
"ac.creative_set_id, "
526530
"ac.type, "
527531
"ac.url_pattern, "
528-
"ac.observation_window "
532+
"ac.observation_window, "
533+
"ac.expiry_timestamp "
529534
"FROM ad_conversions AS ac";
530535

531536
sql::Statement statement(db_.GetUniqueStatement(sql.c_str()));
@@ -536,6 +541,7 @@ bool BundleStateDatabase::GetAdConversions(
536541
info.type = statement.ColumnString(1);
537542
info.url_pattern = statement.ColumnString(2);
538543
info.observation_window = statement.ColumnInt(3);
544+
info.expiry_timestamp = statement.ColumnInt64(4);
539545
ad_conversions->emplace_back(info);
540546
}
541547

@@ -642,6 +648,11 @@ bool BundleStateDatabase::Migrate() {
642648
break;
643649
}
644650

651+
case 6: {
652+
success = MigrateV6toV7();
653+
break;
654+
}
655+
645656
default: {
646657
NOTREACHED();
647658
break;
@@ -781,4 +792,24 @@ bool BundleStateDatabase::MigrateV5toV6() {
781792
return GetDB().Execute(create_ad_info_table_sql.c_str());
782793
}
783794

795+
bool BundleStateDatabase::MigrateV6toV7() {
796+
const std::string drop_ad_conversions_table_sql =
797+
"DROP TABLE IF EXISTS ad_conversions";
798+
if (!GetDB().Execute(drop_ad_conversions_table_sql.c_str())) {
799+
return false;
800+
}
801+
802+
const std::string create_ad_conversions_table_sql =
803+
"CREATE TABLE ad_conversions "
804+
"(creative_set_id LONGVARCHAR NOT NULL, "
805+
"type LONGVARCHAR NOT NULL, "
806+
"url_pattern LONGVARCHAR NOT NULL, "
807+
"observation_window INTEGER NOT NULL, "
808+
"expiry_timestamp TIMESTAMP, "
809+
"UNIQUE(creative_set_id, type, url_pattern), "
810+
"PRIMARY KEY(creative_set_id, type, url_pattern))";
811+
812+
return GetDB().Execute(create_ad_conversions_table_sql.c_str());
813+
}
814+
784815
} // namespace brave_ads

‎components/brave_ads/browser/bundle_state_database.h

+2-1
Original file line numberDiff line numberDiff line change
@@ -87,7 +87,7 @@ class BundleStateDatabase {
8787
bool CreateCreativeAdNotificationCategoriesCategoryIndex();
8888

8989
bool CreateAdConversionsTable();
90-
bool TruncateAdConversionsTable();
90+
bool PurgeExpiredAdConversions();
9191
bool InsertOrUpdateAdConversion(
9292
const ads::AdConversionInfo& info);
9393

@@ -103,6 +103,7 @@ class BundleStateDatabase {
103103
bool MigrateV3toV4();
104104
bool MigrateV4toV5();
105105
bool MigrateV5toV6();
106+
bool MigrateV6toV7();
106107

107108
sql::Database db_;
108109
sql::MetaTable meta_table_;

‎vendor/bat-native-ads/include/bat/ads/ad_conversion_info.h

+3
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66
#ifndef BAT_ADS_AD_CONVERSION_INFO_H_
77
#define BAT_ADS_AD_CONVERSION_INFO_H_
88

9+
#include <stdint.h>
10+
911
#include <string>
1012
#include <vector>
1113

@@ -41,6 +43,7 @@ struct ADS_EXPORT AdConversionInfo {
4143
std::string type;
4244
std::string url_pattern;
4345
unsigned int observation_window = 0;
46+
uint64_t expiry_timestamp = 0;
4447
};
4548

4649
using AdConversionList = std::vector<AdConversionInfo>;

‎vendor/bat-native-ads/resources/bundle-schema.json

+5-1
Original file line numberDiff line numberDiff line change
@@ -80,7 +80,8 @@
8080
"creativeSetId",
8181
"urlPattern",
8282
"type",
83-
"observationWindow"
83+
"observationWindow",
84+
"expiryTimestamp"
8485
],
8586
"properties": {
8687
"creativeSetId": {
@@ -94,6 +95,9 @@
9495
},
9596
"observationWindow": {
9697
"type": "number"
98+
},
99+
"expiryTimestamp": {
100+
"type": "number"
97101
}
98102
}
99103
}

‎vendor/bat-native-ads/src/bat/ads/ad_conversion_info.cc

+9-1
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,8 @@ bool AdConversionInfo::operator==(
2121
return creative_set_id == rhs.creative_set_id &&
2222
type == rhs.type &&
2323
url_pattern == rhs.url_pattern &&
24-
observation_window == rhs.observation_window;
24+
observation_window == rhs.observation_window &&
25+
expiry_timestamp == rhs.expiry_timestamp;
2526
}
2627

2728
bool AdConversionInfo::operator!=(
@@ -65,6 +66,10 @@ Result AdConversionInfo::FromJson(
6566
observation_window = document["observation_window"].GetUint();
6667
}
6768

69+
if (document.HasMember("expiry_timestamp")) {
70+
expiry_timestamp = document["expiry_timestamp"].GetUint64();
71+
}
72+
6873
return SUCCESS;
6974
}
7075

@@ -83,6 +88,9 @@ void SaveToJson(JsonWriter* writer, const AdConversionInfo& info) {
8388
writer->String("observation_window");
8489
writer->Uint(info.observation_window);
8590

91+
writer->String("expiry_timestamp");
92+
writer->Uint64(info.expiry_timestamp);
93+
8694
writer->EndObject();
8795
}
8896

‎vendor/bat-native-ads/src/bat/ads/bundle_state.cc

+9
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55

66
#include "bat/ads/bundle_state.h"
77

8+
#include "base/time/time.h"
89
#include "bat/ads/internal/json_helper.h"
910
#include "bat/ads/internal/url_util.h"
1011

@@ -139,6 +140,11 @@ Result BundleState::FromJson(
139140
ad_conversion["observationWindow"].GetUint();
140141
}
141142

143+
if (ad_conversion.HasMember("expiryTimestamp")) {
144+
info.expiry_timestamp =
145+
ad_conversion["expiryTimestamp"].GetUint64();
146+
}
147+
142148
new_ad_conversions.push_back(info);
143149
}
144150
}
@@ -239,6 +245,9 @@ void SaveToJson(
239245
writer->String("observationWindow");
240246
writer->Uint(ad_conversion.observation_window);
241247

248+
writer->String("expiryTimestamp");
249+
writer->Uint64(ad_conversion.expiry_timestamp);
250+
242251
writer->EndObject();
243252
}
244253

‎vendor/bat-native-ads/src/bat/ads/internal/catalog_state.cc

+10
Original file line numberDiff line numberDiff line change
@@ -132,6 +132,16 @@ Result CatalogState::FromJson(
132132
ad_conversion.observation_window =
133133
conversion["observationWindow"].GetUint();
134134

135+
base::Time end_at_timestamp;
136+
if (!base::Time::FromUTCString(campaign_info.end_at.c_str(),
137+
&end_at_timestamp)) {
138+
continue;
139+
}
140+
141+
base::Time expiry_timestamp = end_at_timestamp +
142+
base::TimeDelta::FromDays(ad_conversion.observation_window);
143+
ad_conversion.expiry_timestamp = expiry_timestamp.ToDoubleT();
144+
135145
creative_set_info.ad_conversions.push_back(ad_conversion);
136146
}
137147

0 commit comments

Comments
 (0)