Merge pull request #15546 from brave/issues/26139

Cleanup Brave Ads part deux!
2022-10-20 07:52:56 -05:00
parent 68fd880be4 5365f33e65
commit f25cd49c66
7 changed files with 51 additions and 29 deletions
@@ -5,6 +5,9 @@

 #include "bat/ads/internal/base/crypto/crypto_unittest_util.h"

+#include <algorithm>
+#include <iterator>
+
 #include "tweetnacl.h"  // NOLINT

 namespace ads::security {
@@ -18,9 +21,9 @@ std::vector<uint8_t> Decrypt(const std::vector<uint8_t>& ciphertext,
                  ciphertext.size(), &nonce.front(),
                  &ephemeral_public_key.front(), &secret_key.front());

-  std::vector<uint8_t> plaintext(
-      padded_plaintext.cbegin() + crypto_box_ZEROBYTES,
-      padded_plaintext.cend());
+  std::vector<uint8_t> plaintext;
+  std::copy(padded_plaintext.cbegin() + crypto_box_ZEROBYTES,
+            padded_plaintext.cend(), std::back_inserter(plaintext));

  return plaintext;
 }
@@ -89,6 +89,10 @@ bool DoesConfirmationTypeMatchConversionType(
      return false;
    }
  }
+
+  NOTREACHED() << "Unexpected value for ConfirmationType: "
+               << static_cast<int>(confirmation_type.value());
+  return false;
 }

 std::string ExtractConversionIdFromText(
@@ -26,8 +26,6 @@ NotificationAdManager* g_notification_ad_manager_instance = nullptr;

 #if BUILDFLAG(IS_ANDROID)
 constexpr int kMaximumNotificationAds = 3;
-#else   // !BUILDFLAG(IS_ANDROID)
-constexpr int kMaximumNotificationAds = 0;  // Unlimited
 #endif  // BUILDFLAG(IS_ANDROID)

 }  // namespace
@@ -76,12 +74,14 @@ void NotificationAdManager::Add(const NotificationAdInfo& ad) {

  ads_.push_back(ad);

-  if (kMaximumNotificationAds > 0 && ads_.size() > kMaximumNotificationAds) {
+#if BUILDFLAG(IS_ANDROID)
+  if (ads_.size() > kMaximumNotificationAds) {
    AdsClientHelper::GetInstance()->CloseNotificationAd(
        ads_.front().placement_id);

    ads_.pop_front();
  }
+#endif  // BUILDFLAG(IS_ANDROID)

  AdsClientHelper::GetInstance()->SetListPref(prefs::kNotificationAds,
                                              NotificationAdsToValue(ads_));
@@ -15,6 +15,7 @@
 #include "base/time/time.h"
 #include "bat/ads/ad_info.h"
 #include "bat/ads/ad_type.h"
+#include "bat/ads/history_item_info.h"
 #include "bat/ads/internal/ads_client_helper.h"
 #include "bat/ads/internal/base/logging_util.h"
 #include "bat/ads/internal/deprecated/client/client_info.h"
@@ -256,13 +257,17 @@ AdContentLikeActionType ClientStateManager::ToggleAdThumbDown(
 AdContentLikeActionType
 ClientStateManager::GetAdContentLikeActionTypeForAdvertiser(
    const std::string& advertiser_id) {
-  for (const auto& item : client_->history_items) {
-    if (item.ad_content.advertiser_id == advertiser_id) {
-      return item.ad_content.like_action_type;
-    }
+  const auto iter = base::ranges::find_if(
+      client_->history_items,
+      [&advertiser_id](const HistoryItemInfo& history_item) -> bool {
+        return history_item.ad_content.advertiser_id == advertiser_id;
+      });
+
+  if (iter == client_->history_items.cend()) {
+    return AdContentLikeActionType::kNeutral;
  }

-  return AdContentLikeActionType::kNeutral;
+  return iter->ad_content.like_action_type;
 }

 CategoryContentOptActionType ClientStateManager::ToggleAdOptIn(
@@ -327,13 +332,17 @@ CategoryContentOptActionType ClientStateManager::ToggleAdOptOut(
 CategoryContentOptActionType
 ClientStateManager::GetCategoryContentOptActionTypeForSegment(
    const std::string& segment) {
-  for (const auto& item : client_->history_items) {
-    if (item.category_content.category == segment) {
-      return item.category_content.opt_action_type;
-    }
+  const auto iter = base::ranges::find_if(
+      client_->history_items,
+      [&segment](const HistoryItemInfo& history_item) -> bool {
+        return history_item.category_content.category == segment;
+      });
+
+  if (iter == client_->history_items.cend()) {
+    return CategoryContentOptActionType::kNone;
  }

-  return CategoryContentOptActionType::kNone;
+  return iter->category_content.opt_action_type;
 }

 bool ClientStateManager::ToggleSavedAd(const AdContentInfo& ad_content) {
@@ -201,9 +201,9 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,

  ConfirmationList new_failed_confirmations;

-  for (const auto& item : *failed_confirmations) {
-    const base::Value::Dict* const dict = item.GetIfDict();
-    if (!dict) {
+  for (const auto& value : *failed_confirmations) {
+    const base::Value::Dict* const failed_confirmation_dict = value.GetIfDict();
+    if (!failed_confirmation_dict) {
      BLOG(0, "Confirmation should be a dictionary");
      continue;
    }
@@ -211,7 +211,8 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,
    ConfirmationInfo confirmation;

    // Transaction id
-    if (const std::string* const value = dict->FindString("transaction_id")) {
+    if (const std::string* const value =
+            failed_confirmation_dict->FindString("transaction_id")) {
      confirmation.transaction_id = *value;
    } else {
      // Migrate legacy confirmations
@@ -221,7 +222,7 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,

    // Creative instance id
    if (const std::string* const value =
-            dict->FindString("creative_instance_id")) {
+            failed_confirmation_dict->FindString("creative_instance_id")) {
      confirmation.creative_instance_id = *value;
    } else {
      BLOG(0, "Missing confirmation creative instance id");
@@ -229,7 +230,8 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,
    }

    // Type
-    if (const std::string* const value = dict->FindString("type")) {
+    if (const std::string* const value =
+            failed_confirmation_dict->FindString("type")) {
      confirmation.type = ConfirmationType(*value);
    } else {
      BLOG(0, "Missing confirmation type");
@@ -237,7 +239,8 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,
    }

    // Ad type
-    if (const std::string* const value = dict->FindString("ad_type")) {
+    if (const std::string* const value =
+            failed_confirmation_dict->FindString("ad_type")) {
      confirmation.ad_type = AdType(*value);
    } else {
      // Migrate legacy confirmations, this value is not used right now so safe
@@ -247,7 +250,7 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,

    // Created at
    if (const std::string* const value =
-            dict->FindString("timestamp_in_seconds")) {
+            failed_confirmation_dict->FindString("timestamp_in_seconds")) {
      double timestamp_as_double;
      if (!base::StringToDouble(*value, &timestamp_as_double)) {
        continue;
@@ -257,11 +260,12 @@ bool GetFailedConfirmationsFromDictionary(const base::Value::Dict& dict,
    }

    // Was created
-    const absl::optional<bool> was_created = dict->FindBool("created");
+    const absl::optional<bool> was_created =
+        failed_confirmation_dict->FindBool("created");
    confirmation.was_created = was_created.value_or(true);

    // Opted-in
-    confirmation.opted_in = GetOptedIn(*dict);
+    confirmation.opted_in = GetOptedIn(*failed_confirmation_dict);

    if (!IsValid(confirmation)) {
      BLOG(0, "Invalid confirmation");
@@ -37,6 +37,10 @@ bool ShouldFilterConfirmationType(const ConfirmationType& confirmation_type) {
      return true;
    }
  }
+
+  NOTREACHED() << "Unexpected value for ConfirmationType: "
+               << static_cast<int>(confirmation_type.value());
+  return true;
 }

 std::map<std::string, HistoryItemInfo> BuildBuckets(
@@ -70,9 +70,7 @@ HistoryItemList HistoryManager::Get(const HistoryFilterType filter_type,

  const auto date_range_filter =
      std::make_unique<DateRangeHistoryFilter>(from_time, to_time);
-  if (date_range_filter) {
-    history_items = date_range_filter->Apply(history_items);
-  }
+  history_items = date_range_filter->Apply(history_items);

  const auto filter = HistoryFilterFactory::Build(filter_type);
  if (filter) {