Do not cache feeds for the current day inn cassandra

Change-Id: I55016bb26ae60efd6692cbcbe693adb012692ac2
wikimedia · Sep 22, 2017 · b5f0a2a · b5f0a2a
1 parent 0030f8a
commit b5f0a2a
Show file tree

Hide file tree

Showing 4 changed files with 24 additions and 54 deletions.
diff --git a/lib/base_feed.js b/lib/base_feed.js
@@ -7,12 +7,10 @@ const uuid = require('cassandra-uuid').TimeUuid;
 const mwUtil = require('./mwUtil');
 const URI = HyperSwitch.URI;
 
-const DEFAULT_TTL = 3600;
 const DEFAULT_CACHE_CONTROL = 's-maxage=30, max-age=15';
 
 class BaseFeed {
     constructor(options) {
-        options.ttl = options.ttl || DEFAULT_TTL;
         options.feed_cache_control = options.feed_cache_control || DEFAULT_CACHE_CONTROL;
         if (!options.host) {
             throw new Error('feed module: host option missing');
@@ -71,26 +69,18 @@ class BaseFeed {
                 body: res.body
             });
         };
-        const getCurrentContent = () => {
-            return getContent(this._getCurrentBucketName(), true)
-            .catch({ status: 404 }, () =>
-                // it's a cache miss, so we need to request all
-                // of the components and store them
-                this._makeFeedRequests(hyper, req, false)
-                .then(result => this._assembleResult(result, dateKey))
-                .tap((res) => {
-                    const storeReqs = [ storeContent(res, this._getCurrentBucketName()) ];
-                    if (this.options.storeHistory) {
-                        storeReqs.push(storeContent(res, this._getHistoricBucketName()));
-                    }
-                    // Store async
-                    P.all(storeReqs);
-                }));
-        };
-        const requestHistoricContentFromMCS = () => {
-            return this._makeFeedRequests(hyper, req, true)
-            .then(result => this._assembleResult(result, dateKey));
-        };
+        const requesCurrentContentFromMCS = () => this._makeFeedRequests(hyper, req, false)
+        .then(result => this._assembleResult(result, dateKey))
+        .tap((res) => {
+            if (this.options.storeHistory) {
+                // Store async
+                storeContent(res, this._getHistoricBucketName());
+            }
+        });
+
+        const requestHistoricContentFromMCS = () => this._makeFeedRequests(hyper, req, true)
+        .then(result => this._assembleResult(result, dateKey));
+
         const getHistoricContent = () => {
             if (mwUtil.isNoCacheRequest(req)) {
                 // Need to update only the parts of content
@@ -129,32 +119,16 @@ class BaseFeed {
         if (this.options.storeHistory && mwUtil.isHistoric(date)) {
             return getHistoricContent().then(populateSummaries);
         } else {
-            return getCurrentContent().then(populateSummaries);
+            return requesCurrentContentFromMCS().then(populateSummaries);
         }
     }
 
-    _getCurrentBucketName() {
-        return this.options.name;
-    }
-
     _getHistoricBucketName() {
         return `${this.options.name}.historic`;
     }
 
     getModuleDeclaration() {
-        const resources = [
-            {
-                uri: `/{domain}/sys/key_value/${this._getCurrentBucketName()}`,
-                body: {
-                    version: 2,
-                    valueType: 'json',
-                    retention_policy: {
-                        type: 'ttl',
-                        ttl: this.options.ttl
-                    }
-                }
-            }
-        ];
+        const resources = [];
         if (this.options.storeHistory) {
             resources.push({
                 uri: `/{domain}/sys/key_value/${this._getHistoricBucketName()}`,

diff --git a/projects/wmf_default.yaml b/projects/wmf_default.yaml
@@ -83,11 +83,11 @@ paths:
             /feed:
               x-modules:
                 - path: v1/feed.js
-                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 7200}, options.mobileapps)}}'
+                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 300}, options.mobileapps)}}'
                 - path: v1/announcements.yaml
                   options: '{{merge({"announcement_cache_control": "s-maxage=86400, max-age=86400"}, options.mobileapps)}}'
                 - path: v1/onthisday.js
-                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 7200}, options.mobileapps)}}'
+                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 300}, options.mobileapps)}}'
             /transform:
               x-modules:
                 - path: v1/transform.yaml

diff --git a/projects/wmf_enwiki.yaml b/projects/wmf_enwiki.yaml
@@ -83,13 +83,13 @@ paths:
             /feed:
               x-modules:
                 - path: v1/feed.js
-                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 7200}, options.mobileapps)}}'
+                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 300}, options.mobileapps)}}'
                 - path: v1/announcements.yaml
                   options: '{{merge({"announcement_cache_control": "s-maxage=86400, max-age=86400"}, options.mobileapps)}}'
                 - path: v1/trending.js
                   options: '{{options.trending}}'
                 - path: v1/onthisday.js
-                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 7200}, options.mobileapps)}}'
+                  options: '{{merge({"feed_cache_control": "s-maxage=300, max-age=60", "ttl": 300}, options.mobileapps)}}'
             /transform:
               x-modules:
                 - path: v1/transform.yaml

diff --git a/test/features/feed.js b/test/features/feed.js
@@ -113,8 +113,6 @@ describe('Featured feed', () => {
         .then(() => {
             slice.halt();
             const requests = slice.get().map(JSON.parse);
-            assertStorageRequest(requests, 'get', 'feed.aggregated', true);
-            assertStorageRequest(requests, 'put', 'feed.aggregated', true);
             assertStorageRequest(requests, 'put', 'feed.aggregated.historic', true);
             assertMCSRequest(requests, 'page/featured', date, true);
             assertMCSRequest(requests, 'page/most-read', date, true);
@@ -123,7 +121,7 @@ describe('Featured feed', () => {
         });
     });
 
-    it('Should not rerender available current content', () => {
+    it('Should rerender available current content', () => {
         const now = new Date();
         const date = now.toISOString().split('T').shift().split('-').join('/');
         const slice = server.config.logStream.slice();
@@ -134,11 +132,10 @@ describe('Featured feed', () => {
         .then(() => {
             slice.halt();
             const requests = slice.get().map(JSON.parse);
-            assertStorageRequest(requests, 'get', 'feed.aggregated', true);
-            assertMCSRequest(requests, 'page/featured', date, false);
-            assertMCSRequest(requests, 'page/most-read', date, false);
-            assertMCSRequest(requests, 'media/image/featured', date, false);
-            assertMCSRequest(requests, 'page/news', undefined, false);
+            assertMCSRequest(requests, 'page/featured', date, true);
+            assertMCSRequest(requests, 'page/most-read', date, true);
+            assertMCSRequest(requests, 'media/image/featured', date, true);
+            assertMCSRequest(requests, 'page/news', undefined, true);
         });
     });
 
@@ -156,7 +153,6 @@ describe('Featured feed', () => {
         .then(() => {
             slice.halt();
             const requests = slice.get().map(JSON.parse);
-            assertStorageRequest(requests, 'put', 'feed.aggregated', true);
             assertStorageRequest(requests, 'put', 'feed.aggregated.historic', true);
             assertMCSRequest(requests, 'page/featured', date, true);
             assertMCSRequest(requests, 'page/most-read', date, true);
@@ -243,6 +239,6 @@ describe('Trending feed', () => {
             assert.deepEqual(page.normalizedtitle, 'Trending article');
         })
         .then(() => api.done())
-        .finally(() => nock.cleanAll())
+        .finally(() => nock.cleanAll());
     });
 });