Skip to content

Commit

Permalink
Followup to "Decouple Brave Ads conversions and resolve #16090 and #2…
Browse files Browse the repository at this point in the history
  • Loading branch information
tmancey committed Jul 10, 2023
1 parent ab59dcd commit 6da9087
Show file tree
Hide file tree
Showing 18 changed files with 68 additions and 52 deletions.
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 @@ -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
6 changes: 3 additions & 3 deletions components/brave_ads/core/test/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -313,11 +313,11 @@ source_set("brave_ads_unit_tests") {
"//brave/components/brave_ads/core/internal/conversions/queue/queue_item/conversion_queue_item_unittest_util.h",
"//brave/components/brave_ads/core/internal/conversions/queue/queue_item/conversion_queue_item_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/default_conversion/creative_set_conversion_url_pattern/creative_set_conversion_url_pattern_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_unittest_util.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_unittest_util.h",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/envelope/verifiable_conversion_envelope_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_builder_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_builder_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_unittest_util.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_unittest_util.h",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_envelope_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_meta_tag_parser_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_html_parser_util_unittest.cc",
"//brave/components/brave_ads/core/internal/conversions/types/verifiable_conversion/verifiable_conversion_id_pattern/parsers/verifiable_conversion_id_url_redirects_parser_util_unittest.cc",
Expand Down

0 comments on commit 6da9087

Please sign in to comment.