From 29d9e283498f55e7a00c79e3886f9e3e02d951e4 Mon Sep 17 00:00:00 2001 From: Petr Pchelko Date: Thu, 21 Jun 2018 15:06:01 +0300 Subject: [PATCH] Support language variants for summary Change-Id: I4901a437444f3805bbca82df1b1a3c66c41a3d96 --- lib/language_variants_filter.js | 5 +- lib/mwUtil.js | 8 ++ sys/key_value.js | 9 +++ .../features/pagecontent/language_variants.js | 78 +++++++++++++++++++ v1/summary.yaml | 9 ++- v1/summary_new.yaml | 10 ++- 6 files changed, 112 insertions(+), 7 deletions(-) diff --git a/lib/language_variants_filter.js b/lib/language_variants_filter.js index 7060f6841..ba7459ace 100644 --- 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 index 068c72bb7..9cc8036c8 100644 --- 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 index 918e21920..064075e7a 100644 --- 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 index 9ef0f3466..638b982fb 100644 --- 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); + }); + }); }); \ No newline at end of file diff --git a/v1/summary.yaml b/v1/summary.yaml index 19b4a6d89..e94abfd73 100644 --- 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 index 41fd78d68..3c06243c1 100644 --- 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: