Bug 1494973 - Add clear cache method for ASRouter

content-src/components/ASRouterAdmin/ASRouterAdmin.jsx

@@ -51,6 +51,10 @@ export class ASRouterAdmin extends React.PureComponent {
     return () => ASRouterUtils.overrideMessage(id);
   }
 
+  expireCache() {
+    ASRouterUtils.sendMessage({type: "EXPIRE_QUERY_CACHE"});
+  }
+
   renderMessageItem(msg) {
     const isCurrent = msg.id === this.state.lastMessageId;
     const isBlocked = this.state.messageBlockList.includes(msg.id);
@@ -113,6 +117,8 @@ export class ASRouterAdmin extends React.PureComponent {
   render() {
     return (<div className="asrouter-admin outer-wrapper">
       <h1>AS Router Admin</h1>
+      <h2>Targeting Utilities</h2>
+      <button className="button" onClick={this.expireCache}>Expire Cache</button> (This expires the cache in ASR Targeting for bookmarks and top sites)
       <h2>Message Providers</h2>
       {this.state.providers ? this.renderProviders() : null}
       <h2>Messages</h2>

lib/ASRouter.jsm

@@ -20,6 +20,8 @@ ChromeUtils.defineModuleGetter(this, "ASRouterPreferences",
   "resource://activity-stream/lib/ASRouterPreferences.jsm");
 ChromeUtils.defineModuleGetter(this, "ASRouterTargeting",
   "resource://activity-stream/lib/ASRouterTargeting.jsm");
+ChromeUtils.defineModuleGetter(this, "QueryCache",
+  "resource://activity-stream/lib/ASRouterTargeting.jsm");
 ChromeUtils.defineModuleGetter(this, "ASRouterTriggerListeners",
   "resource://activity-stream/lib/ASRouterTriggerListeners.jsm");
 
@@ -1018,6 +1020,9 @@ class _ASRouter {
           this.dispatchToAS(ac.ASRouterUserEvent(action.data));
         }
         break;
+      case "EXPIRE_QUERY_CACHE":
+        QueryCache.expireAll();
+        break;
     }
   }
 }

lib/ASRouterTargeting.jsm

@@ -27,48 +27,59 @@ const FRECENT_SITES_IGNORE_BLOCKED = false;
 const FRECENT_SITES_NUM_ITEMS = 25;
 const FRECENT_SITES_MIN_FRECENCY = 100;
 
+/**
+ * CachedTargetingGetter
+ * @param property {string} Name of the method called on ActivityStreamProvider
+ * @param options {{}?} Options object passsed to ActivityStreamProvider method
+ * @param updateInterval {number?} Update interval for query. Defaults to FRECENT_SITES_UPDATE_INTERVAL
+ */
 function CachedTargetingGetter(property, options = null, updateInterval = FRECENT_SITES_UPDATE_INTERVAL) {
-  const targetingGetter = {
+  return {
     _lastUpdated: 0,
     _value: null,
     // For testing
     expire() {
       this._lastUpdated = 0;
       this._value = null;
     },
-  };
-
-  Object.defineProperty(targetingGetter, property, {
-    get: () => new Promise(async (resolve, reject) => {
-      const now = Date.now();
-      if (now - targetingGetter._lastUpdated >= updateInterval) {
-        try {
-          targetingGetter._value = await asProvider[property](options);
-          targetingGetter._lastUpdated = now;
-        } catch (e) {
-          Cu.reportError(e);
-          reject(e);
+    get() {
+      return new Promise(async (resolve, reject) => {
+        const now = Date.now();
+        if (now - this._lastUpdated >= updateInterval) {
+          try {
+            this._value = await asProvider[property](options);
+            this._lastUpdated = now;
+          } catch (e) {
+            Cu.reportError(e);
+            reject(e);
+          }
         }
-      }
-      resolve(targetingGetter._value);
-    }),
-  });
-
-  return targetingGetter;
+        resolve(this._value);
+      });
+    },
+  };
 }
 
-const TopFrecentSitesCache = new CachedTargetingGetter(
-  "getTopFrecentSites",
-  {
-    ignoreBlocked: FRECENT_SITES_IGNORE_BLOCKED,
-    numItems: FRECENT_SITES_NUM_ITEMS,
-    topsiteFrecency: FRECENT_SITES_MIN_FRECENCY,
-    onePerDomain: true,
-    includeFavicon: false,
-  }
-);
-
-const TotalBookmarksCountCache = new CachedTargetingGetter("getTotalBookmarksCount");
+const QueryCache = {
+  expireAll() {
+    Object.keys(this.queries).forEach(query => {
+      this.queries[query].expire();
+    });
+  },
+  queries: {
+    TopFrecentSites: new CachedTargetingGetter(
+      "getTopFrecentSites",
+      {
+        ignoreBlocked: FRECENT_SITES_IGNORE_BLOCKED,
+        numItems: FRECENT_SITES_NUM_ITEMS,
+        topsiteFrecency: FRECENT_SITES_MIN_FRECENCY,
+        onePerDomain: true,
+        includeFavicon: false,
+      }
+    ),
+    TotalBookmarksCount: new CachedTargetingGetter("getTotalBookmarksCount"),
+  },
+};
 
 /**
  * sortMessagesByWeightedRank
@@ -179,7 +190,7 @@ const TargetingGetters = {
     return Services.prefs.getIntPref("devtools.selfxss.count");
   },
   get topFrecentSites() {
-    return TopFrecentSitesCache.getTopFrecentSites.then(sites => sites.map(site => (
+    return QueryCache.queries.TopFrecentSites.get().then(sites => sites.map(site => (
       {
         url: site.url,
         host: (new URL(site.url)).hostname,
@@ -200,7 +211,7 @@ const TargetingGetters = {
     }, {});
   },
   get totalBookmarksCount() {
-    return TotalBookmarksCountCache.getTotalBookmarksCount;
+    return QueryCache.queries.TotalBookmarksCount.get();
   },
   get firefoxVersion() {
     return parseInt(AppConstants.MOZ_APP_VERSION.match(/\d+/), 10);
@@ -295,7 +306,6 @@ this.ASRouterTargeting = {
 };
 
 // Export for testing
-this.TopFrecentSitesCache = TopFrecentSitesCache;
-this.TotalBookmarksCountCache = TotalBookmarksCountCache;
+this.QueryCache = QueryCache;
 this.CachedTargetingGetter = CachedTargetingGetter;
-this.EXPORTED_SYMBOLS = ["ASRouterTargeting", "TopFrecentSitesCache", "TotalBookmarksCountCache", "CachedTargetingGetter"];
+this.EXPORTED_SYMBOLS = ["ASRouterTargeting", "QueryCache", "CachedTargetingGetter"];

test/browser/browser_asrouter_targeting.js

@@ -1,4 +1,4 @@
-const {ASRouterTargeting, TopFrecentSitesCache, TotalBookmarksCountCache} =
+const {ASRouterTargeting, QueryCache} =
   ChromeUtils.import("resource://activity-stream/lib/ASRouterTargeting.jsm", {});
 const {AddonTestUtils} =
   ChromeUtils.import("resource://testing-common/AddonTestUtils.jsm", {});
@@ -153,16 +153,17 @@ add_task(async function check_totalBookmarksCount() {
   await clearHistoryAndBookmarks();
   const message = {id: "foo", targeting: "totalBookmarksCount > 0"};
 
-  is(await ASRouterTargeting.findMatchingMessage({messages: [message]}), undefined,
-    "Should not select any message because");
+  const results = await ASRouterTargeting.findMatchingMessage({messages: [message]});
+  is(results ? JSON.stringify(results) : results, undefined,
+    "Should not select any message because bookmarks count is not 0");
 
   const bookmark = await PlacesUtils.bookmarks.insert({
     parentGuid: PlacesUtils.bookmarks.unfiledGuid,
     title: "foo",
     url: "https://mozilla1.com/nowNew",
   });
 
-  TotalBookmarksCountCache.expire();
+  QueryCache.queries.TotalBookmarksCount.expire();
 
   is(await ASRouterTargeting.findMatchingMessage({messages: [message]}), message,
     "Should select correct item after bookmarks are added.");
@@ -323,7 +324,6 @@ add_task(async function checkFrecentSites() {
 
   // Cleanup
   await clearHistoryAndBookmarks();
-  TopFrecentSitesCache.expire();
 });
 
 add_task(async function check_firefox_version() {

test/browser/head.js

@@ -2,6 +2,8 @@
 
 ChromeUtils.defineModuleGetter(this, "PlacesTestUtils",
   "resource://testing-common/PlacesTestUtils.jsm");
+ChromeUtils.defineModuleGetter(this, "QueryCache",
+  "resource://activity-stream/lib/ASRouterTargeting.jsm");
 
 function popPrefs() {
   return SpecialPowers.popPrefEnv();
@@ -22,6 +24,7 @@ async function setDefaultTopSites() { // eslint-disable-line no-unused-vars
 async function clearHistoryAndBookmarks() { // eslint-disable-line no-unused-vars
   await PlacesUtils.bookmarks.eraseEverything();
   await PlacesUtils.history.clear();
+  QueryCache.expireAll();
 }
 
 /**

test/unit/asrouter/ASRouter.test.js

@@ -16,6 +16,7 @@ import {ASRouterTriggerListeners} from "lib/ASRouterTriggerListeners.jsm";
 import {CFRPageActions} from "lib/CFRPageActions.jsm";
 import {GlobalOverrider} from "test/unit/utils";
 import ProviderResponseSchema from "content-src/asrouter/schemas/provider-response.schema.json";
+import {QueryCache} from "lib/ASRouterTargeting.jsm";
 
 const MESSAGE_PROVIDER_PREF_NAME = "browser.newtabpage.activity-stream.asrouter.messageProviders";
 const ONBOARDING_FINISHED_PREF = "browser.onboarding.notification.finished";
@@ -863,6 +864,17 @@ describe("ASRouter", () => {
     });
   });
 
+  describe("#onMessage: EXPIRE_QUERY_CACHE", () => {
+    it("should clear all QueryCache getters", async () => {
+      const msg = fakeAsyncMessage({type: "EXPIRE_QUERY_CACHE"});
+      sandbox.stub(QueryCache, "expireAll");
+
+      await Router.onMessage(msg);
+
+      assert.calledOnce(QueryCache.expireAll);
+    });
+  });
+
   describe("_triggerHandler", () => {
     it("should call #onMessage with the correct trigger", () => {
       sinon.spy(Router, "onMessage");

test/unit/asrouter/ASRouterTargeting.test.js

@@ -65,14 +65,14 @@ describe("#CachedTargetingGetter", () => {
     frecentStub.resolves();
     clock.tick(sixHours);
 
-    await topsitesCache.getTopFrecentSites; // eslint-disable-line no-unused-expressions
-    await topsitesCache.getTopFrecentSites; // eslint-disable-line no-unused-expressions
+    await topsitesCache.get();
+    await topsitesCache.get();
 
     assert.calledOnce(global.NewTabUtils.activityStreamProvider.getTopFrecentSites);
 
     clock.tick(sixHours);
 
-    await topsitesCache.getTopFrecentSites; // eslint-disable-line no-unused-expressions
+    await topsitesCache.get();
 
     assert.calledTwice(global.NewTabUtils.activityStreamProvider.getTopFrecentSites);
   });
@@ -83,7 +83,7 @@ describe("#CachedTargetingGetter", () => {
     // assert.throws expect a function as the first parameter, try/catch is a
     // workaround
     try {
-      await topsitesCache.getTopFrecentSites; // eslint-disable-line no-unused-expressions
+      await topsitesCache.get();
       assert.isTrue(false);
     } catch (e) {
       assert.calledOnce(global.Cu.reportError);