Merge pull request #21114 from brave/issues/34529

[ads] Refactor LoadFileResource to LoadComponentResource
brave · Nov 29, 2023 · c84cb76 · c84cb76
2 parents 276c6d1 + c2637bd
commit c84cb76
Show file tree

Hide file tree

Showing 77 changed files with 420 additions and 348 deletions.
diff --git a/components/brave_ads/GLOSSARY.md b/components/brave_ads/GLOSSARY.md
@@ -4,6 +4,7 @@ A place to define all specific terms and vocabulary for the Brave Ads component,
 
 | term  | description  |
 |---|---|
+| Ad unit  | An ad unit is displayed to the user. Also known as an ad placement.  |
 | Advertiser  | An advertiser is someone or a company that promotes products, services, or ideas through various mediums to reach and persuade potential customers or target audiences.  |
 | Advertisement  | An advertisement is a promotional message or content created to promote a product, service, or idea to a target audience, also known as an Ad.  |
 | Analytics  | Examine data to uncover meaningful insights, trends, and patterns that can inform decision-making and improve understanding of a given subject or domain. See P2A and P3A.  |

diff --git a/components/brave_ads/README.md b/components/brave_ads/README.md
@@ -2,7 +2,7 @@
 
 Users earn tokens by viewing privacy-first Brave Ads. Ads presented are based on the user's interests, as inferred from the user's browsing behavior. No personal data or browsing history should ever leave the browser.
 
-For more details, refer to [glossary of terms](../../../brave/components/brave_ads/GLOSSARY.md).
+For more details, refer to [glossary of terms](GLOSSARY.md).
 
 Brave Ads is a [layered component](https://sites.google.com/a/chromium.org/dev/developers/design-documents/layered-components-design). It has the following structure:
 

diff --git a/components/brave_ads/browser/ads_service_impl.cc b/components/brave_ads/browser/ads_service_impl.cc
@@ -1625,9 +1625,10 @@ void AdsServiceImpl::Load(const std::string& name, LoadCallback callback) {
           std::move(callback)));
 }
 
-void AdsServiceImpl::LoadFileResource(const std::string& id,
-                                      const int version,
-                                      LoadFileResourceCallback callback) {
+void AdsServiceImpl::LoadComponentResource(
+    const std::string& id,
+    const int version,
+    LoadComponentResourceCallback callback) {
   absl::optional<base::FilePath> file_path =
       g_brave_browser_process->resource_component()->GetPath(id, version);
   if (!file_path) {
@@ -1647,7 +1648,7 @@ void AdsServiceImpl::LoadFileResource(const std::string& id,
           },
           std::move(*file_path), file_task_runner_),
       base::BindOnce(
-          [](LoadFileResourceCallback callback,
+          [](LoadComponentResourceCallback callback,
              std::unique_ptr<base::File, base::OnTaskRunnerDeleter> file) {
             CHECK(file);
             std::move(callback).Run(std::move(*file));

diff --git a/components/brave_ads/browser/ads_service_impl.h b/components/brave_ads/browser/ads_service_impl.h
@@ -330,9 +330,9 @@ class AdsServiceImpl : public AdsService,
 
   // TODO(https://github.com/brave/brave-browser/issues/26195) Decouple load
   // resources business logic.
-  void LoadFileResource(const std::string& id,
-                        int version,
-                        LoadFileResourceCallback callback) override;
+  void LoadComponentResource(const std::string& id,
+                             int version,
+                             LoadComponentResourceCallback callback) override;
   void LoadDataResource(const std::string& name,
                         LoadDataResourceCallback callback) override;
 

diff --git a/components/brave_ads/core/README.md b/components/brave_ads/core/README.md
@@ -2,4 +2,6 @@
 
 Platform independent implementation of the world's first global ad platform built on privacy, see [Brave Ads](https://brave.com/brave-ads-launch/).
 
+For more details, refer to [glossary of terms](../GLOSSARY.md).
+
 Please add to it!
diff --git a/components/brave_ads/core/internal/account/deposits/deposits_database_table.cc b/components/brave_ads/core/internal/account/deposits/deposits_database_table.cc
@@ -78,7 +78,7 @@ DepositInfo GetFromRecord(mojom::DBRecordInfo* record) {
 }
 
 void GetForCreativeInstanceIdCallback(
-    const std::string& /*creative_instance_id=*/,
+    const std::string& /*creative_instance_id*/,
     GetDepositsCallback callback,
     mojom::DBCommandResponseInfoPtr command_response) {
   if (!command_response ||

diff --git a/components/brave_ads/core/internal/account/deposits/non_cash_deposit.cc b/components/brave_ads/core/internal/account/deposits/non_cash_deposit.cc
@@ -11,7 +11,7 @@
 
 namespace brave_ads {
 
-void NonCashDeposit::GetValue(const std::string& /*creative_instance_id=*/,
+void NonCashDeposit::GetValue(const std::string& /*creative_instance_id*/,
                               GetDepositCallback callback) {
   std::move(callback).Run(/*success=*/true, /* value=*/0.0);
 }

diff --git a/components/brave_ads/core/internal/account/user_data/dynamic/system_timestamp_user_data.cc b/components/brave_ads/core/internal/account/user_data/dynamic/system_timestamp_user_data.cc
@@ -23,7 +23,7 @@ base::Value::Dict BuildSystemTimestampUserData() {
   }
 
   user_data.Set(kSystemTimestampKey,
-                TimeToPrivacyPreservingISO8601(base::Time::Now()));
+                TimeToPrivacyPreservingIso8601(base::Time::Now()));
 
   return user_data;
 }

diff --git a/components/brave_ads/core/internal/account/user_data/fixed/conversion_user_data.cc b/components/brave_ads/core/internal/account/user_data/fixed/conversion_user_data.cc
@@ -27,7 +27,7 @@ void BuildConversionUserData(const std::string& creative_instance_id,
       creative_instance_id,
       base::BindOnce(
           [](BuildUserDataCallback callback, const bool success,
-             const std::string& /*creative_instance_id=*/,
+             const std::string& /*creative_instance_id*/,
              const ConversionQueueItemList& conversion_queue_items) {
             if (!success) {
               return std::move(callback).Run(/*user_data=*/{});

diff --git a/components/brave_ads/core/internal/account/user_data/fixed/created_at_timestamp_user_data.cc b/components/brave_ads/core/internal/account/user_data/fixed/created_at_timestamp_user_data.cc
@@ -24,7 +24,7 @@ base::Value::Dict BuildCreatedAtTimestampUserData(
   }
 
   user_data.Set(kCreatedAtTimestampKey,
-                TimeToPrivacyPreservingISO8601(transaction.created_at));
+                TimeToPrivacyPreservingIso8601(transaction.created_at));
 
   return user_data;
 }

diff --git a/components/brave_ads/core/internal/catalog/catalog.cc b/components/brave_ads/core/internal/catalog/catalog.cc
@@ -138,8 +138,8 @@ void Catalog::OnFailedToFetchCatalog() {
   NotifyFailedToUpdateCatalog();
 }
 
-void Catalog::OnDidMigrateDatabase(const int /*from_version=*/,
-                                   const int /*to_version=*/) {
+void Catalog::OnDidMigrateDatabase(const int /*from_version*/,
+                                   const int /*to_version*/) {
   ResetCatalog();
 }
 

diff --git a/components/brave_ads/core/internal/catalog/catalog_url_request_json_reader_unittest.cc b/components/brave_ads/core/internal/catalog/catalog_url_request_json_reader_unittest.cc
@@ -23,7 +23,7 @@
 #include "brave/components/brave_ads/core/internal/catalog/catalog_info.h"
 #include "brave/components/brave_ads/core/internal/catalog/catalog_unittest_constants.h"
 #include "brave/components/brave_ads/core/internal/common/unittest/unittest_base.h"
-#include "brave/components/brave_ads/core/internal/common/unittest/unittest_file_path_util.h"
+#include "brave/components/brave_ads/core/internal/common/unittest/unittest_file_util.h"
 #include "brave/components/brave_ads/core/internal/common/unittest/unittest_time_util.h"
 #include "url/gurl.h"
 
@@ -225,8 +225,8 @@ CatalogCampaignInfo BuildCatalogCampaign1() {
   catalog_campaign.id = "27a624a1-9c80-494a-bf1b-af327b563f85";
   catalog_campaign.priority = 1;
   catalog_campaign.pass_through_rate = 1.0;
-  catalog_campaign.start_at = DistantPastAsISO8601();
-  catalog_campaign.end_at = DistantFutureAsISO8601();
+  catalog_campaign.start_at = DistantPastAsIso8601();
+  catalog_campaign.end_at = DistantFutureAsIso8601();
   catalog_campaign.daily_cap = 10;
   catalog_campaign.advertiser_id = "a437c7f3-9a48-4fe8-b37b-99321bea93fe";
   catalog_campaign.creative_sets = catalog_creative_sets;
@@ -413,8 +413,8 @@ CatalogCampaignInfo BuildCatalogCampaign2() {
   catalog_campaign.id = "856fc4bc-a21b-4582-bab7-a20d412359aa";
   catalog_campaign.priority = 2;
   catalog_campaign.pass_through_rate = 0.5;
-  catalog_campaign.start_at = DistantPastAsISO8601();
-  catalog_campaign.end_at = DistantFutureAsISO8601();
+  catalog_campaign.start_at = DistantPastAsIso8601();
+  catalog_campaign.end_at = DistantFutureAsIso8601();
   catalog_campaign.daily_cap = 25;
   catalog_campaign.advertiser_id = "7523854c-5f28-4153-9da8-d9da6804ed58";
   catalog_campaign.creative_sets = catalog_creative_sets;
@@ -431,27 +431,26 @@ class BraveAdsCatalogUrlRequestJsonReaderTest : public UnitTestBase {};
 TEST_F(BraveAdsCatalogUrlRequestJsonReaderTest,
        ParseCatalogWithSingleCampaign) {
   // Arrange
-  const absl::optional<std::string> json =
-      ReadFileFromTestPathAndParseTagsToString(
-          kCatalogWithSingleCampaignFilename);
-  ASSERT_TRUE(json);
+  const absl::optional<std::string> contents =
+      MaybeReadFileToStringAndReplaceTags(kCatalogWithSingleCampaignFilename);
+  ASSERT_TRUE(contents);
 
   // Act & Assert
   CatalogInfo expected_catalog;
   expected_catalog.id = kCatalogId;
   expected_catalog.version = 9;
   expected_catalog.ping = base::Milliseconds(7'200'000);
   expected_catalog.campaigns.push_back(BuildCatalogCampaign1());
-  EXPECT_EQ(expected_catalog, json::reader::ReadCatalog(*json));
+  EXPECT_EQ(expected_catalog, json::reader::ReadCatalog(*contents));
 }
 
 TEST_F(BraveAdsCatalogUrlRequestJsonReaderTest,
        ParseCatalogWithMultipleCampaigns) {
   // Arrange
-  const absl::optional<std::string> json =
-      ReadFileFromTestPathAndParseTagsToString(
+  const absl::optional<std::string> contents =
+      MaybeReadFileToStringAndReplaceTags(
           kCatalogWithMultipleCampaignsFilename);
-  ASSERT_TRUE(json);
+  ASSERT_TRUE(contents);
 
   // Act & Assert
   CatalogInfo expected_catalog;
@@ -460,21 +459,21 @@ TEST_F(BraveAdsCatalogUrlRequestJsonReaderTest,
   expected_catalog.ping = base::Milliseconds(7'200'000);
   expected_catalog.campaigns.push_back(BuildCatalogCampaign1());
   expected_catalog.campaigns.push_back(BuildCatalogCampaign2());
-  EXPECT_EQ(expected_catalog, json::reader::ReadCatalog(*json));
+  EXPECT_EQ(expected_catalog, json::reader::ReadCatalog(*contents));
 }
 
 TEST_F(BraveAdsCatalogUrlRequestJsonReaderTest, ParseEmptyCatalog) {
   // Arrange
-  const absl::optional<std::string> json =
-      ReadFileFromTestPathAndParseTagsToString(kEmptyCatalogFilename);
-  ASSERT_TRUE(json);
+  const absl::optional<std::string> contents =
+      MaybeReadFileToStringAndReplaceTags(kEmptyCatalogFilename);
+  ASSERT_TRUE(contents);
 
   // Act & Assert
   CatalogInfo expected_catalog;
   expected_catalog.id = kCatalogId;
   expected_catalog.version = 9;
   expected_catalog.ping = base::Milliseconds(7'200'000);
-  EXPECT_EQ(expected_catalog, json::reader::ReadCatalog(*json));
+  EXPECT_EQ(expected_catalog, json::reader::ReadCatalog(*contents));
 }
 
 TEST_F(BraveAdsCatalogUrlRequestJsonReaderTest, InvalidCatalog) {

diff --git a/components/brave_ads/core/internal/client/ads_client_mock.h b/components/brave_ads/core/internal/client/ads_client_mock.h
@@ -80,7 +80,7 @@ class AdsClientMock : public AdsClient {
                SaveCallback callback));
   MOCK_METHOD(void, Load, (const std::string& name, LoadCallback callback));
   MOCK_METHOD(void,
-              LoadFileResource,
+              LoadComponentResource,
               (const std::string& id,
                const int version,
                LoadFileCallback callback));

diff --git a/components/brave_ads/core/internal/client/ads_client_util.cc b/components/brave_ads/core/internal/client/ads_client_util.cc
@@ -115,10 +115,10 @@ void Load(const std::string& name, LoadCallback callback) {
   GetInstance()->Load(name, std::move(callback));
 }
 
-void LoadFileResource(const std::string& id,
-                      int version,
-                      LoadFileCallback callback) {
-  GetInstance()->LoadFileResource(id, version, std::move(callback));
+void LoadComponentResource(const std::string& id,
+                           int version,
+                           LoadFileCallback callback) {
+  GetInstance()->LoadComponentResource(id, version, std::move(callback));
 }
 
 std::string LoadDataResource(const std::string& name) {

diff --git a/components/brave_ads/core/internal/client/ads_client_util.h b/components/brave_ads/core/internal/client/ads_client_util.h
@@ -58,9 +58,11 @@ void Save(const std::string& name,
           const std::string& value,
           SaveCallback callback);
 void Load(const std::string& name, LoadCallback callback);
-void LoadFileResource(const std::string& id,
-                      int version,
-                      LoadFileCallback callback);
+
+void LoadComponentResource(const std::string& id,
+                           int version,
+                           LoadFileCallback callback);
+
 std::string LoadDataResource(const std::string& name);
 
 void GetScheduledCaptcha(const std::string& payment_id,

diff --git a/components/brave_ads/core/internal/common/resources/resources_util_impl.h b/components/brave_ads/core/internal/common/resources/resources_util_impl.h
@@ -53,8 +53,8 @@ base::expected<T, std::string> ReadFileAndParseResourceOnBackgroundThread(
 }
 
 template <typename T>
-void LoadFileResourceCallback(LoadAndParseResourceCallback<T> callback,
-                              base::File file) {
+void LoadComponentResourceCallback(LoadAndParseResourceCallback<T> callback,
+                                   base::File file) {
   base::ThreadPool::PostTaskAndReplyWithResult(
       FROM_HERE, {base::MayBlock()},
       base::BindOnce(&ReadFileAndParseResourceOnBackgroundThread<T>,
@@ -66,9 +66,9 @@ template <typename T>
 void LoadAndParseResource(const std::string& id,
                           const int version,
                           LoadAndParseResourceCallback<T> callback) {
-  LoadFileResource(
+  LoadComponentResource(
       id, version,
-      base::BindOnce(&LoadFileResourceCallback<T>, std::move(callback)));
+      base::BindOnce(&LoadComponentResourceCallback<T>, std::move(callback)));
 }
 
 }  // namespace brave_ads

diff --git a/components/brave_ads/core/internal/common/time/time_util.cc b/components/brave_ads/core/internal/common/time/time_util.cc
@@ -236,7 +236,7 @@ base::Time GetLocalTimeAtEndOfThisMonth() {
   return AdjustLocalTimeToEndOfMonth(now);
 }
 
-std::string TimeToPrivacyPreservingISO8601(const base::Time time) {
+std::string TimeToPrivacyPreservingIso8601(const base::Time time) {
   base::Time::Exploded exploded;
   time.UTCExplode(&exploded);
 

diff --git a/components/brave_ads/core/internal/common/time/time_util.h b/components/brave_ads/core/internal/common/time/time_util.h
@@ -28,7 +28,7 @@ base::Time GetLocalTimeAtEndOfLastMonth();
 base::Time GetLocalTimeAtBeginningOfThisMonth();
 base::Time GetLocalTimeAtEndOfThisMonth();
 
-std::string TimeToPrivacyPreservingISO8601(base::Time time);
+std::string TimeToPrivacyPreservingIso8601(base::Time time);
 
 // TODO(https://github.com/brave/brave-browser/issues/20169): Remove this
 // function when base::Time::FromLocalExploded for linux sandbox will be fixed.

diff --git a/components/brave_ads/core/internal/common/time/time_util_unittest.cc b/components/brave_ads/core/internal/common/time/time_util_unittest.cc
@@ -235,14 +235,14 @@ TEST_P(BraveAdsTimeUtilTest, GetLocalTimeAtEndOfThisMonth) {
   EXPECT_EQ(expected_adjusted_time, GetLocalTimeAtEndOfThisMonth());
 }
 
-TEST_P(BraveAdsTimeUtilTest, TimeToPrivacyPreservingISO8601) {
+TEST_P(BraveAdsTimeUtilTest, TimeToPrivacyPreservingIso8601) {
   // Arrange
   const base::Time time =
       TimeFromString("November 18 2020 23:45:12.345", /*is_local=*/false);
   AdvanceClockTo(time);
 
   // Act & Assert
-  EXPECT_EQ("2020-11-18T23:00:00.000Z", TimeToPrivacyPreservingISO8601(Now()));
+  EXPECT_EQ("2020-11-18T23:00:00.000Z", TimeToPrivacyPreservingIso8601(Now()));
 }
 
 #if BUILDFLAG(IS_LINUX)