Expire Brave Ads catalog after last_updated + ping

brave · Mar 12, 2023 · 4ac9467 · 4ac9467
1 parent 97e803b
commit 4ac9467
Show file tree

Hide file tree

Showing 5 changed files with 13 additions and 11 deletions.
diff --git a/.../ads/ad_events/new_tab_page_ads/new_tab_page_ad_event_handler_if_ads_disabled_unittest.cc b/.../ads/ad_events/new_tab_page_ads/new_tab_page_ad_event_handler_if_ads_disabled_unittest.cc
@@ -16,6 +16,7 @@
 #include "brave/components/brave_ads/core/internal/ads/ad_events/new_tab_page_ads/new_tab_page_ad_event_handler_observer.h"
 #include "brave/components/brave_ads/core/internal/ads/serving/permission_rules/permission_rules_unittest_util.h"
 #include "brave/components/brave_ads/core/internal/ads/serving/serving_features.h"
+#include "brave/components/brave_ads/core/internal/catalog/catalog_util.h"
 #include "brave/components/brave_ads/core/internal/common/unittest/unittest_base.h"
 #include "brave/components/brave_ads/core/internal/common/unittest/unittest_time_util.h"
 #include "brave/components/brave_ads/core/internal/creatives/new_tab_page_ads/creative_new_tab_page_ad_info.h"
@@ -333,7 +334,7 @@ TEST_F(BatAdsNewTabPageAdEventHandlerIfAdsDisabledTest,
       creative_ad, AdType::kNewTabPageAd, ConfirmationType::kViewed, Now());
   FireAdEvents(viewed_ad_event, ads_per_day - 1);
 
-  AdvanceClockBy(base::Days(1) - base::Seconds(1));
+  AdvanceClockBy(GetCatalogPing() - base::Seconds(1));
 
   const std::string placement_id =
       base::GUID::GenerateRandomV4().AsLowercaseString();
@@ -367,7 +368,7 @@ TEST_F(BatAdsNewTabPageAdEventHandlerIfAdsDisabledTest,
       creative_ad, AdType::kNewTabPageAd, ConfirmationType::kViewed, Now());
   FireAdEvents(viewed_ad_event, ads_per_day);
 
-  AdvanceClockBy(base::Days(1) - base::Seconds(1));
+  AdvanceClockBy(GetCatalogPing() - base::Seconds(1));
 
   const std::string placement_id =
       base::GUID::GenerateRandomV4().AsLowercaseString();

diff --git a/...ents/brave_ads/core/internal/ads/serving/permission_rules/catalog_permission_rule_test.cc b/...ents/brave_ads/core/internal/ads/serving/permission_rules/catalog_permission_rule_test.cc
@@ -39,9 +39,9 @@ TEST_F(BatAdsCatalogPermissionRuleIntegrationTest, AllowAd) {
 }
 
 TEST_F(BatAdsCatalogPermissionRuleIntegrationTest,
-       AllowAdIfCatalogWasLastUpdated23HoursAnd59MinutesAgo) {
+       AllowAdIfCatalogWasLastUpdatedOnOrBeforeCatalogPing) {
   // Arrange
-  AdvanceClockBy(base::Days(1) - base::Seconds(1));
+  AdvanceClockBy(GetCatalogPing() - base::Seconds(1));
 
   // Act
   CatalogPermissionRule permission_rule;
@@ -52,9 +52,9 @@ TEST_F(BatAdsCatalogPermissionRuleIntegrationTest,
 }
 
 TEST_F(BatAdsCatalogPermissionRuleIntegrationTest,
-       DoNotAllowAdIfCatalogWasLastUpdated1DayAgo) {
+       DoNotAllowAdIfCatalogWasNotUpdatedAfterCatalogPing) {
   // Arrange
-  AdvanceClockBy(base::Days(1));
+  AdvanceClockBy(GetCatalogPing());
 
   // Act
   CatalogPermissionRule permission_rule;

diff --git a/components/brave_ads/core/internal/catalog/catalog.cc b/components/brave_ads/core/internal/catalog/catalog.cc
@@ -122,16 +122,17 @@ void Catalog::OnFetch(const mojom::UrlResponseInfo& url_response) {
     return;
   }
 
-  SetCatalogLastUpdated(base::Time::Now());
-
   if (!HasCatalogChanged(catalog->id)) {
     BLOG(1, "Catalog id " << catalog->id << " is up to date");
     FetchAfterDelay();
     return;
   }
 
+  SetCatalogLastUpdated(base::Time::Now());
+
   SaveCatalog(*catalog);
   NotifyDidUpdateCatalog(*catalog);
+
   FetchAfterDelay();
 }
 

diff --git a/components/brave_ads/core/internal/catalog/catalog_util.cc b/components/brave_ads/core/internal/catalog/catalog_util.cc
@@ -29,8 +29,6 @@ namespace ads {
 
 namespace {
 
-constexpr base::TimeDelta kCatalogLifespan = base::Days(1);
-
 void Delete() {
   database::DeleteCampaigns();
   database::DeleteCreativeNotificationAds();
@@ -122,7 +120,7 @@ bool HasCatalogChanged(const std::string& catalog_id) {
 }
 
 bool HasCatalogExpired() {
-  return base::Time::Now() >= GetCatalogLastUpdated() + kCatalogLifespan;
+  return base::Time::Now() >= GetCatalogLastUpdated() + GetCatalogPing();
 }
 
 }  // namespace ads
diff --git a/components/brave_ads/core/internal/catalog/catalog_util_unittest.cc b/components/brave_ads/core/internal/catalog/catalog_util_unittest.cc
@@ -81,6 +81,7 @@ TEST_F(BatAdsCatalogUtilTest, CatalogHasNotChanged) {
 
 TEST_F(BatAdsCatalogUtilTest, CatalogHasExpired) {
   // Arrange
+  SetCatalogPing(base::Days(1));
   SetCatalogLastUpdated(Now());
 
   // Act
@@ -93,6 +94,7 @@ TEST_F(BatAdsCatalogUtilTest, CatalogHasExpired) {
 
 TEST_F(BatAdsCatalogUtilTest, CatalogHasNotExpired) {
   // Arrange
+  SetCatalogPing(base::Days(1));
   SetCatalogLastUpdated(Now());
 
   // Act