Merge 29d9e28 into 479f95f

wikimedia · Jun 21, 2018 · 1a643e8 · 1a643e8
2 parents 479f95f + 29d9e28
commit 1a643e8
Show file tree

Hide file tree

Showing 6 changed files with 112 additions and 7 deletions.
diff --git a/lib/language_variants_filter.js b/lib/language_variants_filter.js
@@ -87,7 +87,10 @@ module.exports = (hyper, req, next, options, specInfo) => {
                     return res;
                 }));
             } else {
-                // TODO: It's something else, so just forward to MCS
+                // TODO: Eventually we want to think of a better way to support
+                // other content pure-fetching, but right now let's use a magic
+                // header.
+                req.headers['cache-control'] = 'no-cache,no-store';
                 return next(hyper, req);
             }
         });

diff --git a/lib/mwUtil.js b/lib/mwUtil.js
@@ -156,9 +156,17 @@ mwUtil.normalizeContentType = (res) => {
 /**
  * Checks whether the request is a 'no-cache' request
  * @param {Object} req
+ * @return {boolean}
  */
 mwUtil.isNoCacheRequest = req => req.headers && /no-cache/i.test(req.headers['cache-control']);
 
+/**
+ * Checks whether the request is a 'no-store' request
+ * @param {Object} req
+ * @return {boolean}
+ */
+mwUtil.isNoStoreRequest = req => req.headers && /no-store/i.test(req.headers['cache-control']);
+
 mwUtil.parseRevision = (rev, bucketName) => {
     if (!/^[0-9]+/.test(rev)) {
         throw new HTTPError({

diff --git a/sys/key_value.js b/sys/key_value.js
@@ -142,6 +142,15 @@ class KVBucket {
             tid = tid || uuid.now().toString();
         }
 
+        if (mwUtil.isNoStoreRequest(req)) {
+            return {
+                status: 202,
+                headers: {
+                    etag: req.headers && req.headers.etag || mwUtil.makeETag('0', tid)
+                }
+            };
+        }
+
         const doPut = () => hyper.put({
             uri: new URI([rp.domain, 'sys', 'table', rp.bucket, '']),
             body: {

diff --git a/test/features/pagecontent/language_variants.js b/test/features/pagecontent/language_variants.js
@@ -106,4 +106,82 @@ describe('Language variants', function() {
             assert.deepEqual(/2\. Ovo je testna stranica/.test(res.body), true);
         });
     });
+
+    it('should request summary with no variant and store it', () => {
+        let storedEtag;
+        return preq.get({
+            uri: `${server.config.variantsWikiBucketURL}/summary/${variantsPageTitle}`
+        })
+        .then((res) => {
+            storedEtag = res.headers.etag;
+            assert.deepEqual(res.status, 200);
+            assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
+            assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control_with_client_caching');
+            // TODO: Pass in MCS assert.deepEqual(res.headers['content-language'], 'sr');
+            assert.checkString(res.headers.etag, /^"\d+\/[a-f0-9-]+"$/);
+            assert.deepEqual('1. Ово је тестна страница', res.body.extract);
+            // Not try fetching again with a default variant and see if etag matches
+            return preq.get({
+                uri: `${server.config.variantsWikiBucketURL}/summary/${variantsPageTitle}`,
+                headers: {
+                    'accept-language': 'sr'
+                }
+            });
+        })
+        .then((res) => {
+            assert.deepEqual(res.status, 200);
+            assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
+            assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control_with_client_caching');
+            // TODO: Pass in MCS, store in RB assert.deepEqual(res.headers['content-language'], 'sr');
+            assert.deepEqual(res.headers.etag, storedEtag);
+            assert.deepEqual('1. Ово је тестна страница', res.body.extract);
+            // Now try the impossible variant and see that stored one is served again.
+            return preq.get({
+                uri: `${server.config.variantsWikiBucketURL}/summary/${variantsPageTitle}`,
+                headers: {
+                    'accept-language': 'sr-this-is-no-a-variant'
+                }
+            });
+        })
+        .then((res) => {
+            assert.deepEqual(res.status, 200);
+            assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
+            assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control_with_client_caching');
+            // TODO: Pass in MCS, store in RB assert.deepEqual(res.headers['content-language'], 'sr');
+            assert.deepEqual(res.headers.etag, storedEtag);
+            assert.deepEqual('1. Ово је тестна страница', res.body.extract);
+        });
+    });
+
+    it('should request summary with latin variant and not store it', () => {
+        return preq.get({
+            uri: `${server.config.variantsWikiBucketURL}/summary/${variantsPageTitle}`,
+            headers: {
+                'accept-language': 'sr-el'
+            }
+        })
+        .then((res) => {
+            assert.deepEqual(res.status, 200);
+            // TODO: Pass in MCS assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
+            assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control_with_client_caching');
+            // TODO: Pass in MCS assert.deepEqual(res.headers['content-language'], 'sr-el');
+            assert.checkString(res.headers.etag, /^"\d+\/[a-f0-9-]+"$/);
+            assert.deepEqual('1. Ovo je testna stranica', res.body.extract);
+            // Try again without variant to see that stored didn't change
+            return preq.get({
+                uri: `${server.config.variantsWikiBucketURL}/summary/${variantsPageTitle}`,
+                headers: {
+                    'accept-language': 'sr'
+                }
+            });
+        })
+        .then((res) => {
+            assert.deepEqual(res.status, 200);
+            // TODO: Pass in MCS, store in RB assert.deepEqual(res.headers.vary.toLowerCase(), 'accept-language');
+            assert.deepEqual(res.headers['cache-control'], 'test_purged_cache_control_with_client_caching');
+            // TODO: Pass in MCS, store in RB assert.deepEqual(res.headers['content-language'], 'sr');
+            assert.checkString(res.headers.etag, /^"\d+\/[a-f0-9-]+"$/);
+            assert.deepEqual('1. Ово је тестна страница', res.body.extract);
+        });
+    });
 });
diff --git a/v1/summary.yaml b/v1/summary.yaml
@@ -145,15 +145,16 @@ paths:
             request:
               method: put
               uri: /{domain}/sys/key_value/page_summary/{request.params.title}
-              headers: '{{merge({"if-none-hash-match": "*"}, extract.headers)}}'
+              headers: '{{merge({"if-none-hash-match": "*"}, extract.headers, "cache-control": request.headers.cache-control)}}'
               body: '{{extract.body}}'
             # With the if-none-hash-match header the storage will return 412
             # if the content is not changed. In that case, return from the
             # handler completely, and avoid issuing purges.
+            # Also if we didn't really store anything, return as well.
             catch:
-              status: 412
+              status: [ 412, 202 ]
             return_if:
-              status: 412
+              status: [ 412, 202 ]
             return:
               status: 200
               headers:
@@ -164,6 +165,8 @@ paths:
         - emit_change_event:
             request:
               method: post
+              headers:
+                'cache-control': '{{request.headers.cache-control}}'
               uri: /{domain}/sys/events/
               body:
                 - meta:

diff --git a/v1/summary_new.yaml b/v1/summary_new.yaml
@@ -18,6 +18,7 @@ paths:
         options:
           redirect_cache_control: '{{options.response_cache_control}}'
       - path: ./lib/ensure_content_type.js
+      - path: ./lib/language_variants_filter.js
     get:
       tags:
         - Page content
@@ -112,6 +113,8 @@ paths:
         - extract:
             request:
               method: get
+              headers:
+                accept-language: '{{accept-language}}'
               uri: '{{options.host}}/{domain}/v1/page/summary/{title}'
             response:
               status: '{{extract.status}}'
@@ -121,15 +124,16 @@ paths:
             request:
               method: put
               uri: /{domain}/sys/key_value/page_summary/{request.params.title}
-              headers: '{{merge({"if-none-hash-match": "*"}, extract.headers)}}'
+              headers: '{{merge({"if-none-hash-match": "*", "cache-control": request.headers.cache-control}, extract.headers)}}'
               body: '{{extract.body}}'
             # With the if-none-hash-match header the storage will return 412
             # if the content is not changed. In that case, return from the
             # handler completely, and avoid issuing purges.
+            # Also don't emit an event if nothing new was actually stored (code 202)
             catch:
-              status: 412
+              status: [ 202, 412 ]
             return_if:
-              status: 412
+              status: [ 202, 412 ]
             return:
               status: 200
               headers: