From bd6f8c67fdc87b2e7cca0339119b4a11988fe3f2 Mon Sep 17 00:00:00 2001 From: Yiannis Giannelos Date: Thu, 11 Jul 2024 21:57:18 +0200 Subject: [PATCH] pcs/summary: Use if-unmodified-since to avoid unnecessary rendering * Compare tid with if-unmodified-since value to avoid rendering old content * Rename the function to avoid confusion with `if-modified-since` header --- lib/mwUtil.js | 27 +++++++++++++++++++++++++++ sys/key_value.js | 18 ++++++++++++++++-- sys/parsoid.js | 23 +---------------------- v1/pcs/stored_endpoint.js | 25 +++++++++++++------------ v1/summary_new.yaml | 1 + 5 files changed, 58 insertions(+), 36 deletions(-) diff --git a/lib/mwUtil.js b/lib/mwUtil.js index 1720c92c8..6f1a283bf 100644 --- a/lib/mwUtil.js +++ b/lib/mwUtil.js @@ -182,6 +182,33 @@ mwUtil.isNoCacheRequest = (req) => req.headers && /no-cache/i.test(req.headers[' */ mwUtil.isNoStoreRequest = (req) => req.headers && /no-store/i.test(req.headers['cache-control']); +/** + * Checks whether the request is an 'if-unmodified-since' request + * @param {Object} req + * @return {boolean} + */ +mwUtil.isUnmodifiedSinceRequest = (req) => Boolean(req.headers && req.headers['if-unmodified-since']); + +/** + * Checks whether the response has been modified since the timestamp + * in `if-unmodified-since` header of the request + * @param {Object} req the request + * @param {Object} res the response + * @return {boolean} true if content has beed modified + */ +mwUtil.isUnmodifiedSince = (req, res) => { + try { + if (req.headers['if-unmodified-since']) { + const jobTime = Date.parse(req.headers['if-unmodified-since']); + const revInfo = mwUtil.parseETag(res.headers.etag); + return revInfo && uuidUtils.getDate(revInfo.tid) < jobTime; + } + } catch (e) { + // Ignore errors from date parsing + } + return true; +}; + 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 4e1f50b81..abf1de4c9 100644 --- a/sys/key_value.js +++ b/sys/key_value.js @@ -57,7 +57,10 @@ class KVBucket { } getRevision(hyper, req) { - if (mwUtil.isNoCacheRequest(req)) { + const isNoCacheRequest = mwUtil.isNoCacheRequest(req); + const isUnmodifiedSinceRequest = mwUtil.isUnmodifiedSinceRequest(req); + + if (isNoCacheRequest && !isUnmodifiedSinceRequest) { throw new HTTPError({ status: 404 }); } @@ -75,11 +78,22 @@ class KVBucket { return hyper.get(storeReq).then((dbResult) => { if (dbResult.body && dbResult.body.items && dbResult.body.items.length) { const row = dbResult.body.items[0]; - return { + const result = { status: 200, headers: row.headers, body: row.value }; + + if (isNoCacheRequest && !mwUtil.isUnmodifiedSince(result)) { + throw new HTTPError({ + status: 412, + body: { + type: 'precondition_failed', + detail: 'The precondition failed' + } + }); + } + return result; } else { throw new HTTPError({ status: 404, diff --git a/sys/parsoid.js b/sys/parsoid.js index 183b60ec5..60184a4e8 100644 --- a/sys/parsoid.js +++ b/sys/parsoid.js @@ -6,7 +6,6 @@ const URI = HyperSwitch.URI; const HTTPError = HyperSwitch.HTTPError; const uuidv1 = require('uuid').v1; -const uuidUtils = require('../lib/uuidUtils'); const mwUtil = require('../lib/mwUtil'); @@ -45,26 +44,6 @@ function extractTidMeta(html) { return tidMatch && (tidMatch[1] || tidMatch[2]); } -/** - * Checks whether the content has been modified since the timestamp - * in `if-unmodified-since` header of the request - * @param {Object} req the request - * @param {Object} res the response - * @return {boolean} true if content has beed modified - */ -function isModifiedSince(req, res) { - try { - if (req.headers['if-unmodified-since']) { - const jobTime = Date.parse(req.headers['if-unmodified-since']); - const revInfo = mwUtil.parseETag(res.headers.etag); - return revInfo && uuidUtils.getDate(revInfo.tid) >= jobTime; - } - } catch (e) { - // Ignore errors from date parsing - } - return false; -} - /** HTML resource_change event emission * @param {HyperSwitch} hyper the hyperswitch router object * @param {Object} req the request @@ -568,7 +547,7 @@ class ParsoidService { contentReq = contentReq.then((res) => { res.headers['x-restbase-cache'] = 'nocache'; - if (isModifiedSince(req, res)) { // Already up to date, nothing to do. + if (!mwUtil.isUnmodifiedSince(req, res)) { // Already up to date, nothing to do. throw new HTTPError({ status: 412, body: { diff --git a/v1/pcs/stored_endpoint.js b/v1/pcs/stored_endpoint.js index 5981aba36..817f1b1f9 100644 --- a/v1/pcs/stored_endpoint.js +++ b/v1/pcs/stored_endpoint.js @@ -35,22 +35,23 @@ class PCSEndpoint { }); } - if (mwUtils.isNoCacheRequest(req)) { - return this._fetchFromPCSAndStore(hyper, req) - .tap((res) => { - this._injectCacheControl.bind(this)(res); - hyper.metrics.endTiming([ - 'pcs_getContent_latency', - 'pcs_getContent_latency_no_cache', - `pcs_getContent_latency_${rp.domain}` - ], startTime); - }); - } - return hyper.get({ uri: new URI([rp.domain, 'sys', 'key_value', this._options.name, rp.title]) }) .then((res) => { + if (mwUtils.isNoCacheRequest(req)) { + if (!mwUtils.isUnmodifiedSince(req, res)) { + throw new HyperSwitch.HTTPError({ + status: 412, + body: { + type: 'precondition_failed', + detail: 'The precondition failed' + } + }); + } + return this._fetchFromPCSAndStore(hyper, req); + } + if (!rp.revision || `${mwUtils.parseETag(res.headers.etag).rev}` === `${rp.revision}`) { return res; diff --git a/v1/summary_new.yaml b/v1/summary_new.yaml index c7fd3eea4..ab8a9ab91 100644 --- a/v1/summary_new.yaml +++ b/v1/summary_new.yaml @@ -107,6 +107,7 @@ paths: method: get headers: cache-control: '{{cache-control}}' + if-unmodified-since: '{{if-unmodified-since}}' uri: /{domain}/sys/key_value/page_summary/{request.params.title} catch: status: 404