no-cache requests write to cache even when storage is disabled. (#1334)

To allow us to safely switch back from disabled_storage, we need to keep
pre-generated cache entries up to date. We can do this by simply
continuing to handle no-cache requests as we did before, allowing them
to write updated renderings to the cache.

config.fullstack.test.yaml

@@ -137,6 +137,17 @@ wikipedia_project: &wikipedia_project
               - path: projects/proxy.yaml
                 options: *proxy_options
           /{api:sys}: *default_sys
+          /sys_passthrough/{+syspath}:
+            get:
+              x-request-handler:
+                - passthrough_sys:
+                    request:
+                      method: get
+                      uri: /{domain}/sys/{+syspath}
+                    return:
+                      status: 200
+                      headers: '{{passthrough_sys.headers}}'
+                      body: '{{passthrough_sys.body}}'
 
 wikipedia_project_no_storage: &wikipedia_project_no_storage
   x-modules:

sys/parsoid.js

@@ -551,17 +551,19 @@ class ParsoidService {
         let contentReq;
 
         const disabledStorage = this._isStorageDisabled(req.params.domain);
+        const isNoCacheRequest = mwUtil.isNoCacheRequest(req);
 
-        if (!disabledStorage) {
+        if (disabledStorage && !isNoCacheRequest) {
+            contentReq = this._getPageBundleFromParsoid(hyper, req);
+        } else {
             contentReq = this._getContentWithFallback(
                 hyper, rp.domain, rp.title, rp.revision, rp.tid
             ).then((res) => {
                 res.headers['x-restbase-cache'] = res.headers['x-restbase-cache'] || 'hit'; // FIXME: miss?
                 return res;
             });
 
-            if (mwUtil.isNoCacheRequest(req)) {
-
+            if (isNoCacheRequest) {
                 // Check content generation either way
                 contentReq = contentReq.then((res) => {
                     res.headers['x-restbase-cache'] = 'nocache';
@@ -581,8 +583,6 @@ class ParsoidService {
                 // Only (possibly) generate content if there was an error
                 contentReq = contentReq.catch(generateContent);
             }
-        } else {
-            contentReq = this._getPageBundleFromParsoid(hyper, req);
         }
 
         return contentReq

test/features/pagecontent/pagecontent.js

@@ -9,16 +9,17 @@ describe('item requests', function() {
     this.timeout(20000);
     let pagingToken = '';
     let contentTypes;
-    let disabledStorage;
+    let hostPort;
     const title = 'Earth';
     const revision = '358126';
     const prevRevisions = ['358125', '214592', '214591']
+    const domainWithStorageDisabled = "es.wikipedia.beta.wmflabs.org";
 
     const server = new Server();
     before(() => server.start()
     .then(() => {
+        hostPort = server.config.hostPort;
         contentTypes = server.config.conf.test.content_types;
-        disabledStorage = server.config.conf.test.parsoid.disabled_storage;
     }));
     after(() => server.stop());
 
@@ -28,6 +29,17 @@ describe('item requests', function() {
     function deniedContentUri(format) {
         return [server.config.bucketURL(), format, encodeURIComponent(deniedTitle), deniedRev].join('/');
     }
+
+    const sysURL = (domain) => {
+        domain = domain || server.config.defaultDomain;
+        return `${hostPort}/${domain}/sys_passthrough`;
+    };
+
+    const sysGet = (domain, path, opt = {}) => {
+        const uri = `${sysURL(domain)}/${path}`;
+        return preq.get( { uri, ...opt } );
+    };
+
     const assertCORS = (res) => {
         assert.deepEqual(res.headers['access-control-allow-origin'], '*');
         assert.deepEqual(res.headers['access-control-allow-methods'], 'GET,HEAD');
@@ -80,11 +92,35 @@ describe('item requests', function() {
     });
     it('should not cache HTML for Main_Page when storage is disabled', function () {
         return preq.get({
-            uri: `${server.config.bucketURL("es.wikipedia.beta.wmflabs.org")}/html/Página_principal`,
+            uri: `${server.config.bucketURL(domainWithStorageDisabled)}/html/Página_principal`,
+        })
+        .then((res) => {
+            assert.deepEqual(res.status, 200);
+            assert.deepEqual(res.headers['x-restbase-cache'], 'disabled');
+
+            // should still not be cached
+            return sysGet( domainWithStorageDisabled, `key_value/parsoidphp/Página_principal` );
+        }).then((res) => {
+            // if this mails, make sure you are resetting the database between tests
+            assert.deepEqual(res.status, 404);
+        }).catch((res) => {
+            assert.deepEqual(res.status, 404);
+        })
+    });
+    it('should cache HTML for Main_Page even when storage is disabled (cache-control: no-cache)', function () {
+        return preq.get({
+            uri: `${server.config.bucketURL(domainWithStorageDisabled)}/html/Página_principal`,
+            headers: {
+                'cache-control': 'no-cache'
+            }
         })
           .then((res) => {
               assert.deepEqual(res.status, 200);
-              assert.deepEqual(res.headers['x-restbase-cache'], 'disabled');
+
+              // should now be cached
+              return sysGet( domainWithStorageDisabled, `key_value/parsoidphp/Página_principal` );
+          }).then((res) => {
+              assert.deepEqual(res.status, 200);
           })
     });
     it(`should transparently create a new HTML revision with id ${prevRevisions[0]}`, () => {
@@ -175,7 +211,7 @@ describe('item requests', function() {
             assert.deepEqual(res.status, 200);
             assert.contentType(res, 'application/json');
             assert.deepEqual(res.body, {
-                items: ['v1' ]
+                items: ['sys_passthrough', 'v1']
             });
         });
     });