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

Followup to "Decouple Brave Ads conversions and resolve #16090 and #26834" #19205

Merged
merged 2 commits into from
Jul 10, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
10 changes: 5 additions & 5 deletions components/brave_ads/core/internal/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -690,15 +690,15 @@ source_set("internal") {
"conversions/queue/queue_item/conversion_queue_item_util_constants.h",
"conversions/types/default_conversion/creative_set_conversion_url_pattern/creative_set_conversion_url_pattern_util.cc",
"conversions/types/default_conversion/creative_set_conversion_url_pattern/creative_set_conversion_url_pattern_util.h",
"conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.cc",
"conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.h",
"conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util.cc",
"conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util.h",
"conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util_constants.h",
"conversions/types/verifiable_conversion/verifiable_conversion_builder.cc",
"conversions/types/verifiable_conversion/verifiable_conversion_builder.h",
"conversions/types/verifiable_conversion/verifiable_conversion_builder_util.cc",
"conversions/types/verifiable_conversion/verifiable_conversion_builder_util.h",
"conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.cc",
"conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h",
"conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.cc",
"conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.h",
"conversions/types/verifiable_conversion/verifiable_conversion_envelope_util_constants.h",
"conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_meta_tag_parser_util.cc",
"conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_meta_tag_parser_util.h",
"conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_parser_util.cc",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,8 +12,8 @@
#include "brave/components/brave_ads/core/internal/conversions/actions/conversion_action_types.h"
#include "brave/components/brave_ads/core/internal/conversions/actions/conversion_action_types_util.h"
#include "brave/components/brave_ads/core/internal/conversions/queue/queue_item/conversion_queue_item_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_info.h"
#include "third_party/abseil-cpp/absl/types/optional.h"

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -217,25 +217,6 @@ void AdEvents::LogEvent(const AdEventInfo& ad_event, ResultCallback callback) {
RunTransaction(std::move(transaction), std::move(callback));
}

void AdEvents::GetIf(const std::string& condition,
GetAdEventsCallback callback) const {
mojom::DBTransactionInfoPtr transaction = mojom::DBTransactionInfo::New();
mojom::DBCommandInfoPtr command = mojom::DBCommandInfo::New();
command->type = mojom::DBCommandInfo::Type::READ;
command->sql = base::ReplaceStringPlaceholders(
"SELECT ae.placement_id, ae.type, ae.confirmation_type, ae.campaign_id, "
"ae.creative_set_id, ae.creative_instance_id, ae.advertiser_id, "
"ae.segment, ae.created_at FROM $1 AS ae WHERE $2 ORDER BY created_at "
"DESC;",
{GetTableName(), condition}, nullptr);
BindRecords(&*command);
transaction->commands.push_back(std::move(command));

AdsClientHelper::GetInstance()->RunDBTransaction(
std::move(transaction),
base::BindOnce(&GetCallback, std::move(callback)));
}

void AdEvents::GetAll(GetAdEventsCallback callback) const {
mojom::DBTransactionInfoPtr transaction = mojom::DBTransactionInfo::New();
mojom::DBCommandInfoPtr command = mojom::DBCommandInfo::New();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ class AdEvents final : public TableInterface {
public:
void LogEvent(const AdEventInfo& ad_event, ResultCallback callback);

void GetIf(const std::string& condition, GetAdEventsCallback callback) const;

void GetAll(GetAdEventsCallback callback) const;

void GetForType(mojom::AdType ad_type, GetAdEventsCallback callback) const;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,15 @@

namespace brave_ads {

namespace {

constexpr char kVerifiableConversion[] = "verifiable conversion";
constexpr char kDefaultConversion[] = "conversion";

} // namespace

std::string ConversionTypeToString(const ConversionInfo& conversion) {
return conversion.verifiable ? "verifiable conversion" : "conversion";
return conversion.verifiable ? kVerifiableConversion : kDefaultConversion;
}

} // namespace brave_ads
31 changes: 19 additions & 12 deletions components/brave_ads/core/internal/conversions/conversions.cc
Original file line number Diff line number Diff line change
Expand Up @@ -120,21 +120,23 @@ void Conversions::CheckForConversions(
const CreativeSetConversionList filtered_creative_set_conversions =
FilterConvertedAndNonMatchingCreativeSetConversions(
creative_set_conversions, ad_events, redirect_chain);

BLOG(1, "There are " << creative_set_conversions.size()
<< " eligible creative set conversions");
if (filtered_creative_set_conversions.empty()) {
return BLOG(1, "There are no eligible creative set conversions");
}

CreativeSetConversionBuckets creative_set_conversion_buckets =
SortCreativeSetConversionsIntoBuckets(filtered_creative_set_conversions);
if (creative_set_conversion_buckets.empty()) {
// There are no matching eligible creative set conversion URL patterns for
// the redirect chain.
return;
}

BLOG(1, filtered_creative_set_conversions.size()
<< " out of " << creative_set_conversions.size()
<< " eligible creative set conversions are sorted into "
<< creative_set_conversion_buckets.size() << " buckets");

bool did_convert = false;

// Click-through conversions should take priority over view-through
// conversions. Ad events are ordered in descending order by created at; click
// events are guaranteed to occur after view events.
// conversions. Ad events are ordered in descending order by |created_at|;
// click events are guaranteed to occur after view events.
for (const auto& ad_event : ad_events) {
// Do we have a bucket with creative set conversions for this ad event?
const auto iter =
Expand All @@ -161,15 +163,21 @@ void Conversions::CheckForConversions(
redirect_chain, html, resource_.get().id_patterns,
*creative_set_conversion));

did_convert = true;

// Remove the bucket for this creative set because we should not convert
// another ad from the same creative set.
// another ad event from the same creative set.
creative_set_conversion_buckets.erase(ad_event.creative_set_id);
if (creative_set_conversion_buckets.empty()) {
// All the buckets have drained, so they will no longer contain
// conversions; therefore, we should consider our job done!
break;
}
}

if (!did_convert) {
BLOG(1, "There are no matching conversions");
}
}

void Conversions::Convert(
Expand Down Expand Up @@ -199,7 +207,6 @@ void Conversions::ConvertCallback(

const ConversionInfo conversion =
BuildConversion(ad_event, verifiable_conversion);

queue_.Add(conversion);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ void ConversionQueue::Add(const ConversionInfo& conversion) {
BuildConversionQueueItem(conversion, ProcessQueueItemAt());
CHECK(conversion_queue_item.IsValid());

database::table::ConversionQueue database_table;
const database::table::ConversionQueue database_table;
database_table.Save(
{conversion_queue_item},
base::BindOnce(&ConversionQueue::AddCallback, weak_factory_.GetWeakPtr(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -386,7 +386,7 @@ ConversionQueue::ConversionQueue() : batch_size_(kDefaultBatchSize) {}

void ConversionQueue::Save(
const ConversionQueueItemList& conversion_queue_items,
ResultCallback callback) {
ResultCallback callback) const {
if (conversion_queue_items.empty()) {
return std::move(callback).Run(/*success*/ true);
}
Expand Down Expand Up @@ -569,7 +569,7 @@ void ConversionQueue::Migrate(mojom::DBTransactionInfo* transaction,

void ConversionQueue::InsertOrUpdate(
mojom::DBTransactionInfo* transaction,
const ConversionQueueItemList& conversion_queue_items) {
const ConversionQueueItemList& conversion_queue_items) const {
CHECK(transaction);

if (conversion_queue_items.empty()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ class ConversionQueue final : public TableInterface {
ConversionQueue();

void Save(const ConversionQueueItemList& conversion_queue_items,
ResultCallback callback);
ResultCallback callback) const;

void Delete(const ConversionQueueItemInfo& conversion_queue_item,
ResultCallback callback) const;
Expand Down Expand Up @@ -58,8 +58,9 @@ class ConversionQueue final : public TableInterface {
void Migrate(mojom::DBTransactionInfo* transaction, int to_version) override;

private:
void InsertOrUpdate(mojom::DBTransactionInfo* transaction,
const ConversionQueueItemList& conversion_queue_items);
void InsertOrUpdate(
mojom::DBTransactionInfo* transaction,
const ConversionQueueItemList& conversion_queue_items) const;

std::string BuildInsertOrUpdateSql(
mojom::DBCommandInfo* command,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ namespace brave_ads {

void SaveConversionQueueItems(
const ConversionQueueItemList& conversion_queue_items) {
database::table::ConversionQueue database_table;
const database::table::ConversionQueue database_table;
database_table.Save(
conversion_queue_items,
base::BindOnce([](const bool success) { CHECK(success); }));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.h"

#include <tuple>

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_INFO_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_INFO_H_
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_INFO_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_INFO_H_

#include <string>

Expand Down Expand Up @@ -38,4 +38,4 @@ bool operator!=(const VerifiableConversionEnvelopeInfo&,

} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_INFO_H_
#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_INFO_H_
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,15 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_unittest_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_unittest_util.h"

#include <cstdint>
#include <vector>

#include "base/base64.h"
#include "brave/components/brave_ads/core/internal/account/user_data/conversion_user_data_constants.h"
#include "brave/components/brave_ads/core/internal/common/crypto/crypto_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.h"
#include "tweetnacl.h" // NOLINT

namespace brave_ads {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UNITTEST_UTIL_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UNITTEST_UTIL_H_
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UNITTEST_UTIL_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UNITTEST_UTIL_H_

#include <string>

Expand All @@ -24,4 +24,4 @@ absl::optional<std::string> OpenVerifiableConversionEnvelope(

} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UNITTEST_UTIL_H_
#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UNITTEST_UTIL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util.h"

#include <cstdint>
#include <vector>
Expand All @@ -12,8 +12,8 @@
#include "base/check_op.h"
#include "brave/components/brave_ads/core/internal/common/crypto/crypto_util.h"
#include "brave/components/brave_ads/core/internal/common/crypto/key_pair_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_util_constants.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util_constants.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_info.h"
#include "third_party/re2/src/re2/re2.h"
#include "tweetnacl.h" // NOLINT
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_H_
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_H_

#include <string>

Expand All @@ -23,4 +23,4 @@ SealVerifiableConversionEnvelope(

} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_H_
#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_H_
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,8 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_CONSTANTS_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_CONSTANTS_H_
#ifndef BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_CONSTANTS_H_
#define BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_CONSTANTS_H_

#include <cstddef>

Expand All @@ -15,4 +15,4 @@ constexpr size_t kMaxVerifiableConversionEnvelopeMessageLength = 30;

} // namespace brave_ads

#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_CONSTANTS_H_
#endif // BRAVE_COMPONENTS_BRAVE_ADS_CORE_INTERNAL_CONVERSIONS_TYPES_VERIFIABLE_CONVERSION_ENVELOPE_VERIFIABLE_CONVERSION_ENVELOPE_UTIL_CONSTANTS_H_
Original file line number Diff line number Diff line change
Expand Up @@ -3,11 +3,11 @@
* License, v. 2.0. If a copy of the MPL was not distributed with this file,
* You can obtain one at https://mozilla.org/MPL/2.0/. */

#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util.h"

#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_unittest_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_util_constants.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_unittest_util.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util_constants.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_info.h"
#include "brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_unittest_constants.h"
#include "testing/gtest/include/gtest/gtest.h"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,6 +46,7 @@ CreativeSetConversionList FilterConvertedAndNonMatchingCreativeSetConversions(
GetConvertedCreativeSets(ad_events);

CreativeSetConversionList filtered_creative_set_conversions;

base::ranges::copy_if(
creative_set_conversions,
std::back_inserter(filtered_creative_set_conversions),
Expand Down
Loading