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.

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

Fixes ad conversions fail after a creative has expired #5834

Open
wants to merge 1 commit into
base: master
from
Open
Changes from all commits
Commits
File filter...
Filter file types
Jump to…
Jump to file
Failed to load files.

Always

Just for now

@@ -25,8 +25,8 @@ namespace brave_ads {

namespace {

const int kCurrentVersionNumber = 6;
const int kCompatibleVersionNumber = 6;
const int kCurrentVersionNumber = 7;
const int kCompatibleVersionNumber = 7;

} // namespace

@@ -342,24 +342,27 @@ bool BundleStateDatabase::CreateAdConversionsTable() {
// new constraints to the schema
const std::string sql = base::StringPrintf(
"CREATE TABLE %s "
"(id INTEGER PRIMARY KEY, "
"creative_set_id LONGVARCHAR NOT NULL, "
"(creative_set_id LONGVARCHAR NOT NULL, "
"type LONGVARCHAR NOT NULL, "
"url_pattern LONGVARCHAR NOT NULL, "
"observation_window INTEGER NOT NULL)",
"observation_window INTEGER NOT NULL, "
"expiry_timestamp TIMESTAMP, "
"UNIQUE(creative_set_id, type, url_pattern), "
"PRIMARY KEY(creative_set_id, type, url_pattern))",
table_name);

return GetDB().Execute(sql.c_str());
}

bool BundleStateDatabase::TruncateAdConversionsTable() {
bool BundleStateDatabase::PurgeExpiredAdConversions() {
DCHECK_CALLED_ON_VALID_SEQUENCE(sequence_checker_);

const bool is_initialized = Init();
DCHECK(is_initialized);

const std::string sql =
"DELETE FROM ad_conversions";
"DELETE FROM ad_conversions "
"WHERE strftime('%s','now') >= expiry_timestamp";

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

@@ -378,16 +381,18 @@ bool BundleStateDatabase::InsertOrUpdateAdConversion(
"(creative_set_id, "
"type, "
"url_pattern, "
"observation_window) VALUES (%s)",
CreateBindingParameterPlaceholders(4).c_str());
"observation_window, "
"expiry_timestamp) VALUES (%s)",
CreateBindingParameterPlaceholders(5).c_str());

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

statement.BindString(0, info.creative_set_id);
statement.BindString(1, info.type);
statement.BindString(2, info.url_pattern);
// Use BindInt64 for uint32_t types to avoid uint32_t to int32_t cast.
// Use BindInt64 for uint32_t types to avoid uint32_t to int32_t cast
statement.BindInt64(3, info.observation_window);
statement.BindInt64(4, info.expiry_timestamp);

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

// We are completely replacing the database here so truncate all the tables
if (!TruncateCategoriesTable() ||
!TruncateCreativeAdNotificationCategoriesTable() ||
!TruncateCreativeAdNotificationsTable() ||
!TruncateAdConversionsTable()) {
!PurgeExpiredAdConversions()) {
GetDB().RollbackTransaction();
return false;
}
@@ -525,7 +529,8 @@ bool BundleStateDatabase::GetAdConversions(
"ac.creative_set_id, "
"ac.type, "
"ac.url_pattern, "
"ac.observation_window "
"ac.observation_window, "
"ac.expiry_timestamp "
"FROM ad_conversions AS ac";

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

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

case 6: {
success = MigrateV6toV7();
break;
}

default: {
NOTREACHED();
break;
@@ -781,4 +792,24 @@ bool BundleStateDatabase::MigrateV5toV6() {
return GetDB().Execute(create_ad_info_table_sql.c_str());
}

bool BundleStateDatabase::MigrateV6toV7() {
const std::string drop_ad_conversions_table_sql =
"DROP TABLE IF EXISTS ad_conversions";
if (!GetDB().Execute(drop_ad_conversions_table_sql.c_str())) {
return false;
}

const std::string create_ad_conversions_table_sql =
"CREATE TABLE ad_conversions "
"(creative_set_id LONGVARCHAR NOT NULL, "
"type LONGVARCHAR NOT NULL, "
"url_pattern LONGVARCHAR NOT NULL, "
"observation_window INTEGER NOT NULL, "
"expiry_timestamp TIMESTAMP, "
"UNIQUE(creative_set_id, type, url_pattern), "
"PRIMARY KEY(creative_set_id, type, url_pattern))";

return GetDB().Execute(create_ad_conversions_table_sql.c_str());
}

} // namespace brave_ads
@@ -87,7 +87,7 @@ class BundleStateDatabase {
bool CreateCreativeAdNotificationCategoriesCategoryIndex();

bool CreateAdConversionsTable();
bool TruncateAdConversionsTable();
bool PurgeExpiredAdConversions();
bool InsertOrUpdateAdConversion(
const ads::AdConversionInfo& info);

@@ -103,6 +103,7 @@ class BundleStateDatabase {
bool MigrateV3toV4();
bool MigrateV4toV5();
bool MigrateV5toV6();
bool MigrateV6toV7();

sql::Database db_;
sql::MetaTable meta_table_;
@@ -6,6 +6,8 @@
#ifndef BAT_ADS_AD_CONVERSION_INFO_H_
#define BAT_ADS_AD_CONVERSION_INFO_H_

#include <stdint.h>

#include <string>
#include <vector>

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

using AdConversionList = std::vector<AdConversionInfo>;
@@ -80,7 +80,8 @@
"creativeSetId",
"urlPattern",
"type",
"observationWindow"
"observationWindow",
"expiryTimestamp"
],
"properties": {
"creativeSetId": {
@@ -94,6 +95,9 @@
},
"observationWindow": {
"type": "number"
},
"expiryTimestamp": {
"type": "number"
}
}
}
@@ -21,7 +21,8 @@ bool AdConversionInfo::operator==(
return creative_set_id == rhs.creative_set_id &&
type == rhs.type &&
url_pattern == rhs.url_pattern &&
observation_window == rhs.observation_window;
observation_window == rhs.observation_window &&
expiry_timestamp == rhs.expiry_timestamp;
}

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

if (document.HasMember("expiry_timestamp")) {
expiry_timestamp = document["expiry_timestamp"].GetUint64();
}

return SUCCESS;
}

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

writer->String("expiry_timestamp");
writer->Uint64(info.expiry_timestamp);

writer->EndObject();
}

@@ -5,6 +5,7 @@

#include "bat/ads/bundle_state.h"

#include "base/time/time.h"
#include "bat/ads/internal/json_helper.h"
#include "bat/ads/internal/url_util.h"

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

if (ad_conversion.HasMember("expiryTimestamp")) {
info.expiry_timestamp =
ad_conversion["expiryTimestamp"].GetUint64();
}

new_ad_conversions.push_back(info);
}
}
@@ -239,6 +245,9 @@ void SaveToJson(
writer->String("observationWindow");
writer->Uint(ad_conversion.observation_window);

writer->String("expiryTimestamp");
writer->Uint64(ad_conversion.expiry_timestamp);

writer->EndObject();
}

@@ -132,6 +132,16 @@ Result CatalogState::FromJson(
ad_conversion.observation_window =
conversion["observationWindow"].GetUint();

base::Time end_at_timestamp;
if (!base::Time::FromUTCString(campaign_info.end_at.c_str(),
&end_at_timestamp)) {
continue;
}

base::Time expiry_timestamp = end_at_timestamp +
base::TimeDelta::FromDays(ad_conversion.observation_window);
ad_conversion.expiry_timestamp = expiry_timestamp.ToDoubleT();

creative_set_info.ad_conversions.push_back(ad_conversion);
}

ProTip! Use n and p to navigate between commits in a pull request.
You can’t perform that action at this time.