From f7707141d6df5105dd6df624aab0b6335f7ca159 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 8 Oct 2019 17:22:00 +0200 Subject: [PATCH 01/11] Rename variable --- lib/models/mapconfig/provider/base-mapconfig-adapter.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/models/mapconfig/provider/base-mapconfig-adapter.js b/lib/models/mapconfig/provider/base-mapconfig-adapter.js index 301ea3b5..e94f9630 100644 --- a/lib/models/mapconfig/provider/base-mapconfig-adapter.js +++ b/lib/models/mapconfig/provider/base-mapconfig-adapter.js @@ -1,6 +1,6 @@ 'use strict'; -const QueryTables = require('cartodb-query-tables').queryTables; +const { queryTables } = require('cartodb-query-tables'); module.exports = class BaseMapConfigProvider { createAffectedTables (callback) { @@ -34,7 +34,7 @@ module.exports = class BaseMapConfigProvider { return callback(err); } - QueryTables.getQueryMetadataModel(connection, sql, (err, affectedTables) => { + queryTables.getQueryMetadataModel(connection, sql, (err, affectedTables) => { if (err) { return callback(err); } From c4054f0ac9cb351e7f8982b71750ace471acf249 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 15 Oct 2019 10:39:31 +0200 Subject: [PATCH 02/11] Use develop branch of query-tables --- .../provider/base-mapconfig-adapter.js | 15 +++--- package-lock.json | 51 +++++++++---------- package.json | 2 +- test/acceptance/multilayer-server-test.js | 4 +- test/integration/query-tables-test.js | 37 +++++--------- 5 files changed, 48 insertions(+), 61 deletions(-) diff --git a/lib/models/mapconfig/provider/base-mapconfig-adapter.js b/lib/models/mapconfig/provider/base-mapconfig-adapter.js index e94f9630..a606d0cb 100644 --- a/lib/models/mapconfig/provider/base-mapconfig-adapter.js +++ b/lib/models/mapconfig/provider/base-mapconfig-adapter.js @@ -34,15 +34,12 @@ module.exports = class BaseMapConfigProvider { return callback(err); } - queryTables.getQueryMetadataModel(connection, sql, (err, affectedTables) => { - if (err) { - return callback(err); - } - - this.affectedTablesCache.set(dbname, token, affectedTables); - - callback(null, affectedTables); - }); + queryTables.getQueryMetadataModel(connection, sql) + .then(affectedTables => { + this.affectedTablesCache.set(dbname, token, affectedTables); + callback(null, affectedTables); + }) + .catch(err => callback(err)); }); }); } diff --git a/package-lock.json b/package-lock.json index b254d14f..7899ea6d 100644 --- a/package-lock.json +++ b/package-lock.json @@ -166,7 +166,7 @@ }, "async": { "version": "1.5.2", - "resolved": "https://registry.npmjs.org/async/-/async-1.5.2.tgz", + "resolved": "http://registry.npmjs.org/async/-/async-1.5.2.tgz", "integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=" }, "asynckit": { @@ -427,9 +427,8 @@ } }, "cartodb-query-tables": { - "version": "0.6.3", - "resolved": "https://registry.npmjs.org/cartodb-query-tables/-/cartodb-query-tables-0.6.3.tgz", - "integrity": "sha512-ijHl2Roh+0B1pP8SL3guEAu8tE6yNN3J/oxdUWCFOSKjHmXjwTzyJdjO+tONGcERmlWfS594SCFYElGIweSnQg==", + "version": "github:cartodb/node-cartodb-query-tables#ea2734da8f3c6d1c37e48f438f2bb8d1c5c832b5", + "from": "github:cartodb/node-cartodb-query-tables#coherent-ttl-refactor", "requires": { "decimal.js": "10.2.0" } @@ -812,7 +811,7 @@ }, "entities": { "version": "1.0.0", - "resolved": "https://registry.npmjs.org/entities/-/entities-1.0.0.tgz", + "resolved": "http://registry.npmjs.org/entities/-/entities-1.0.0.tgz", "integrity": "sha1-sph6o4ITR/zeZCsk/fyeT7cSvyY=", "dev": true }, @@ -826,7 +825,7 @@ }, "es6-promise": { "version": "3.1.2", - "resolved": "https://registry.npmjs.org/es6-promise/-/es6-promise-3.1.2.tgz", + "resolved": "http://registry.npmjs.org/es6-promise/-/es6-promise-3.1.2.tgz", "integrity": "sha1-eV4lzrR/e6uyY9FRr77dktGOagc=" }, "escape-html": { @@ -914,7 +913,7 @@ }, "express": { "version": "4.16.3", - "resolved": "https://registry.npmjs.org/express/-/express-4.16.3.tgz", + "resolved": "http://registry.npmjs.org/express/-/express-4.16.3.tgz", "integrity": "sha1-avilAjUNsyRuzEvs9rWjTSL37VM=", "requires": { "accepts": "~1.3.5", @@ -1037,7 +1036,7 @@ }, "fast-deep-equal": { "version": "1.1.0", - "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-1.1.0.tgz", + "resolved": "http://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-1.1.0.tgz", "integrity": "sha1-wFNHeBfIa1HaqFPIHgWbcz0CNhQ=" }, "fast-json-stable-stringify": { @@ -1066,7 +1065,7 @@ }, "finalhandler": { "version": "1.1.1", - "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.1.1.tgz", + "resolved": "http://registry.npmjs.org/finalhandler/-/finalhandler-1.1.1.tgz", "integrity": "sha512-Y1GUDo39ez4aHAw7MysnUD5JzYX+WaIj8I57kO3aEPT1fFRL4sr7mjei97FgnwhAyyzRYmQZaTHb2+9uZ1dPtg==", "requires": { "debug": "2.6.9", @@ -2008,7 +2007,7 @@ }, "get-stream": { "version": "3.0.0", - "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", + "resolved": "http://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=" }, "getpass": { @@ -2229,7 +2228,7 @@ }, "htmlparser2": { "version": "3.8.3", - "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-3.8.3.tgz", + "resolved": "http://registry.npmjs.org/htmlparser2/-/htmlparser2-3.8.3.tgz", "integrity": "sha1-mWwosZFRaovoZQGn15dX5ccMEGg=", "dev": true, "requires": { @@ -2242,7 +2241,7 @@ "dependencies": { "readable-stream": { "version": "1.1.14", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-1.1.14.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-1.1.14.tgz", "integrity": "sha1-fPTFTvZI44EwhMY23SB54WbAgdk=", "dev": true, "requires": { @@ -2256,7 +2255,7 @@ }, "http-errors": { "version": "1.6.3", - "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.6.3.tgz", + "resolved": "http://registry.npmjs.org/http-errors/-/http-errors-1.6.3.tgz", "integrity": "sha1-i1VoC7S+KDoLW/TqLjhYC+HZMg0=", "requires": { "depd": "~1.1.2", @@ -2580,12 +2579,12 @@ "dependencies": { "async": { "version": "0.2.10", - "resolved": "https://registry.npmjs.org/async/-/async-0.2.10.tgz", + "resolved": "http://registry.npmjs.org/async/-/async-0.2.10.tgz", "integrity": "sha1-trvgsGdLnXGXCMo43owjfLUmw9E=" }, "semver": { "version": "4.3.6", - "resolved": "https://registry.npmjs.org/semver/-/semver-4.3.6.tgz", + "resolved": "http://registry.npmjs.org/semver/-/semver-4.3.6.tgz", "integrity": "sha1-MAvG4OhjdPe6YQaLWx7NV/xlMto=" }, "underscore": { @@ -2620,7 +2619,7 @@ }, "media-typer": { "version": "0.3.0", - "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", + "resolved": "http://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", "integrity": "sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g=" }, "mem": { @@ -2706,7 +2705,7 @@ }, "minimist": { "version": "0.0.8", - "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" }, "minipass": { @@ -2740,7 +2739,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "requires": { "minimist": "0.0.8" @@ -3238,7 +3237,7 @@ }, "postcss": { "version": "5.0.19", - "resolved": "https://registry.npmjs.org/postcss/-/postcss-5.0.19.tgz", + "resolved": "http://registry.npmjs.org/postcss/-/postcss-5.0.19.tgz", "integrity": "sha1-tjQqAdx1uMq36Wiv2pau/Gf4iK8=", "requires": { "js-base64": "^2.1.9", @@ -3277,7 +3276,7 @@ }, "postcss-value-parser": { "version": "3.3.0", - "resolved": "https://registry.npmjs.org/postcss-value-parser/-/postcss-value-parser-3.3.0.tgz", + "resolved": "http://registry.npmjs.org/postcss-value-parser/-/postcss-value-parser-3.3.0.tgz", "integrity": "sha1-h/OPnxj3dKSrTIojL1xc6IcqnRU=" }, "postgres-array": { @@ -3404,7 +3403,7 @@ }, "readable-stream": { "version": "1.0.34", - "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-1.0.34.tgz", + "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-1.0.34.tgz", "integrity": "sha1-Elgg40vIQtLyqq+v5MKRbuMsFXw=", "requires": { "core-util-is": "~1.0.0", @@ -3780,7 +3779,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "requires": { "ansi-regex": "^2.0.0" @@ -3796,7 +3795,7 @@ }, "strip-eof": { "version": "1.0.0", - "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", + "resolved": "http://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=" }, "strip-json-comments": { @@ -3840,7 +3839,7 @@ }, "through": { "version": "2.3.8", - "resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz", + "resolved": "http://registry.npmjs.org/through/-/through-2.3.8.tgz", "integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=" }, "torque.js": { @@ -4067,7 +4066,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "requires": { "string-width": "^1.0.1", @@ -4096,7 +4095,7 @@ }, "yargs": { "version": "11.1.0", - "resolved": "https://registry.npmjs.org/yargs/-/yargs-11.1.0.tgz", + "resolved": "http://registry.npmjs.org/yargs/-/yargs-11.1.0.tgz", "integrity": "sha512-NwW69J42EsCSanF8kyn5upxvjp5ds+t3+udGBeTbFnERA+lF541DDpMawzo4z6W/QrzNM18D+BPMiOBibnFV5A==", "requires": { "cliui": "^4.0.0", diff --git a/package.json b/package.json index c983c5ab..d6af65a1 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "body-parser": "1.18.3", "camshaft": "^0.64.2", "cartodb-psql": "0.14.0", - "cartodb-query-tables": "^0.6.3", + "cartodb-query-tables": "github:cartodb/node-cartodb-query-tables#coherent-ttl-refactor", "cartodb-redis": "2.1.0", "debug": "3.1.0", "dot": "1.1.2", diff --git a/test/acceptance/multilayer-server-test.js b/test/acceptance/multilayer-server-test.js index 7a7b7e72..cb26c8f6 100644 --- a/test/acceptance/multilayer-server-test.js +++ b/test/acceptance/multilayer-server-test.js @@ -368,8 +368,8 @@ describe('tests from old api translated to multilayer', function() { keysToDelete['user:localhost:mapviews:global'] = 5; var affectedFn = QueryTables.getQueryMetadataModel; - QueryTables.getQueryMetadataModel = function(pg, sql, callback) { - return callback(new Error('fake error message')); + QueryTables.getQueryMetadataModel = async function (pg, sql) { + throw new Error('fake error message'); }; // reset internal cacheChannel cache diff --git a/test/integration/query-tables-test.js b/test/integration/query-tables-test.js index e44d66b9..04206b58 100644 --- a/test/integration/query-tables-test.js +++ b/test/integration/query-tables-test.js @@ -32,36 +32,27 @@ describe('QueryTables', function() { // Check test/support/sql/windshaft.test.sql to understand where the values come from. - it('should return an object with affected tables array and last updated time', function(done) { + it('should return an object with affected tables array and last updated time', async function () { var query = 'select * from test_table'; - QueryTables.getQueryMetadataModel(connection, query, function(err, result) { - assert.ok(!err, err); + const result = await QueryTables.getQueryMetadataModel(connection, query); - assert.equal(result.getLastUpdatedAt(), 1234567890123); + assert.equal(result.getLastUpdatedAt(), 1234567890123); - assert.equal(result.tables.length, 1); - assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); - assert.equal(result.tables[0].schema_name, 'public'); - assert.equal(result.tables[0].table_name, 'test_table'); - - done(); - }); + assert.equal(result.tables.length, 1); + assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); + assert.equal(result.tables[0].schema_name, 'public'); + assert.equal(result.tables[0].table_name, 'test_table'); }); - it('should work with private tables', function(done) { + it('should work with private tables', async function () { var query = 'select * from test_table_private_1'; - QueryTables.getQueryMetadataModel(connection, query, function(err, result) { - assert.ok(!err, err); + const result = await QueryTables.getQueryMetadataModel(connection, query); - assert.equal(result.getLastUpdatedAt(), 1234567890123); + assert.equal(result.getLastUpdatedAt(), 1234567890123); - assert.equal(result.tables.length, 1); - assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); - assert.equal(result.tables[0].schema_name, 'public'); - assert.equal(result.tables[0].table_name, 'test_table_private_1'); - - done(); - }); + assert.equal(result.tables.length, 1); + assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); + assert.equal(result.tables[0].schema_name, 'public'); + assert.equal(result.tables[0].table_name, 'test_table_private_1'); }); - }); From f8e117a7b7755d92f55ea3da3a193d2a49459036 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 15 Oct 2019 11:46:44 +0200 Subject: [PATCH 03/11] JSHint is not ready for modern javascript --- test/acceptance/multilayer-server-test.js | 4 +-- test/integration/query-tables-test.js | 33 ++++++++++++----------- 2 files changed, 20 insertions(+), 17 deletions(-) diff --git a/test/acceptance/multilayer-server-test.js b/test/acceptance/multilayer-server-test.js index cb26c8f6..2b5558c6 100644 --- a/test/acceptance/multilayer-server-test.js +++ b/test/acceptance/multilayer-server-test.js @@ -368,8 +368,8 @@ describe('tests from old api translated to multilayer', function() { keysToDelete['user:localhost:mapviews:global'] = 5; var affectedFn = QueryTables.getQueryMetadataModel; - QueryTables.getQueryMetadataModel = async function (pg, sql) { - throw new Error('fake error message'); + QueryTables.getQueryMetadataModel = function () { + return Promise.reject(new Error('fake error message')); }; // reset internal cacheChannel cache diff --git a/test/integration/query-tables-test.js b/test/integration/query-tables-test.js index 04206b58..dcc3aad8 100644 --- a/test/integration/query-tables-test.js +++ b/test/integration/query-tables-test.js @@ -32,27 +32,30 @@ describe('QueryTables', function() { // Check test/support/sql/windshaft.test.sql to understand where the values come from. - it('should return an object with affected tables array and last updated time', async function () { + it('should return an object with affected tables array and last updated time', function () { var query = 'select * from test_table'; - const result = await QueryTables.getQueryMetadataModel(connection, query); + return QueryTables.getQueryMetadataModel(connection, query) + .then(result => { + assert.equal(result.getLastUpdatedAt(), 1234567890123); - assert.equal(result.getLastUpdatedAt(), 1234567890123); + assert.equal(result.tables.length, 1); + assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); + assert.equal(result.tables[0].schema_name, 'public'); + assert.equal(result.tables[0].table_name, 'test_table'); + }); - assert.equal(result.tables.length, 1); - assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); - assert.equal(result.tables[0].schema_name, 'public'); - assert.equal(result.tables[0].table_name, 'test_table'); }); - it('should work with private tables', async function () { + it('should work with private tables', function () { var query = 'select * from test_table_private_1'; - const result = await QueryTables.getQueryMetadataModel(connection, query); + return QueryTables.getQueryMetadataModel(connection, query) + .then(result => { + assert.equal(result.getLastUpdatedAt(), 1234567890123); - assert.equal(result.getLastUpdatedAt(), 1234567890123); - - assert.equal(result.tables.length, 1); - assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); - assert.equal(result.tables[0].schema_name, 'public'); - assert.equal(result.tables[0].table_name, 'test_table_private_1'); + assert.equal(result.tables.length, 1); + assert.equal(result.tables[0].dbname, 'test_windshaft_cartodb_user_1_db'); + assert.equal(result.tables[0].schema_name, 'public'); + assert.equal(result.tables[0].table_name, 'test_table_private_1'); + }); }); }); From 5a7ffcf499202d57266c71597c9ed7461ecfcdd8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 15 Oct 2019 12:48:50 +0200 Subject: [PATCH 04/11] Be able to synchronize the TTL of cache-control header to expire in a coherent way --- lib/api/middlewares/cache-control-header.js | 35 +++++++++++++++++-- .../cache/cache-control-header-test.js | 14 ++++++-- 2 files changed, 44 insertions(+), 5 deletions(-) diff --git a/lib/api/middlewares/cache-control-header.js b/lib/api/middlewares/cache-control-header.js index d15b259c..a72bdea3 100644 --- a/lib/api/middlewares/cache-control-header.js +++ b/lib/api/middlewares/cache-control-header.js @@ -1,14 +1,35 @@ 'use strict'; -const ONE_YEAR_IN_SECONDS = 60 * 60 * 24 * 365; -const FIVE_MINUTES_IN_SECONDS = 60 * 5; +const ONE_MINUTE_IN_SECONDS = 60; +const THREE_MINUTE_IN_SECONDS = 60 * 3; +const FIVE_MINUTES_IN_SECONDS = ONE_MINUTE_IN_SECONDS * 5; +const TEN_MINUTES_IN_SECONDS = ONE_MINUTE_IN_SECONDS * 10; +const FIFTEEN_MINUTES_IN_SECONDS = ONE_MINUTE_IN_SECONDS * 15; +const THIRTY_MINUTES_IN_SECONDS = ONE_MINUTE_IN_SECONDS * 30; +const ONE_HOUR_IN_SECONDS = ONE_MINUTE_IN_SECONDS * 60; +const ONE_YEAR_IN_SECONDS = ONE_HOUR_IN_SECONDS * 24 * 365; + const FALLBACK_TTL = global.environment.varnish.fallbackTtl || FIVE_MINUTES_IN_SECONDS; +const validFallbackTTL = [ + ONE_MINUTE_IN_SECONDS, + THREE_MINUTE_IN_SECONDS, + FIVE_MINUTES_IN_SECONDS, + TEN_MINUTES_IN_SECONDS, + FIFTEEN_MINUTES_IN_SECONDS, + THIRTY_MINUTES_IN_SECONDS, + ONE_HOUR_IN_SECONDS +]; + module.exports = function setCacheControlHeader ({ ttl = ONE_YEAR_IN_SECONDS, fallbackTtl = FALLBACK_TTL, revalidate = false } = {}) { + if (!validFallbackTTL.includes(fallbackTtl)) { + throw new Error(`Invalid fallback TTL value for Cache-Control header. Got ${fallbackTtl}, expected ${validFallbackTTL.join(', ')}`); + } + return function setCacheControlHeaderMiddleware (req, res, next) { if (req.method !== 'GET') { return next(); @@ -27,7 +48,7 @@ module.exports = function setCacheControlHeader ({ if (everyAffectedTableCanBeInvalidated(affectedTables)) { directives.push(`max-age=${ttl}`); } else { - directives.push(`max-age=${fallbackTtl}`); + directives.push(`max-age=${computeNextTTL({ ttlInSeconds: fallbackTtl })}`); } if (revalidate) { @@ -49,3 +70,11 @@ function everyAffectedTableCanBeInvalidated (affectedTables) { affectedTables.getTables(skipNotUpdatedAtTables, skipAnalysisCachedTables) .every(table => table.updated_at !== null); } + +function computeNextTTL ({ ttlInSeconds } = {}) { + const nowInSeconds = Math.ceil(Date.now() / 1000); + const secondsAfterPreviousTTLStep = nowInSeconds % ttlInSeconds; + const secondsToReachTheNextTTLStep = ttlInSeconds - secondsAfterPreviousTTLStep; + + return secondsToReachTheNextTTLStep; +} diff --git a/test/acceptance/cache/cache-control-header-test.js b/test/acceptance/cache/cache-control-header-test.js index d3b89863..d761b6ef 100644 --- a/test/acceptance/cache/cache-control-header-test.js +++ b/test/acceptance/cache/cache-control-header-test.js @@ -79,7 +79,12 @@ describe('cache-control header', function () { return done(err); } - assert.equal(res.headers['cache-control'], `public,max-age=${ttl}`); + const cacheControl = res.headers['cache-control']; + const [ type, maxAge ] = cacheControl.split(','); + const [ key, value ] = maxAge.split('='); + + assert.ok(Number(value) <= ttl); + testClient.drain(done); }); }); @@ -114,7 +119,12 @@ describe('cache-control header', function () { return done(err); } - assert.equal(res.headers['cache-control'], `public,max-age=${ttl}`); + const cacheControl = res.headers['cache-control']; + const [ type, maxAge ] = cacheControl.split(','); + const [ key, value ] = maxAge.split('='); + + assert.ok(Number(value) <= ttl); + testClient.drain(done); }); }); From ebff2ac9f25eb281c707ec61c3fbbdcc936e81a8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Tue, 15 Oct 2019 13:27:40 +0200 Subject: [PATCH 05/11] Please JSHint --- lib/api/middlewares/cache-control-header.js | 7 ++++++- test/acceptance/cache/cache-control-header-test.js | 8 ++++---- 2 files changed, 10 insertions(+), 5 deletions(-) diff --git a/lib/api/middlewares/cache-control-header.js b/lib/api/middlewares/cache-control-header.js index a72bdea3..84b7a360 100644 --- a/lib/api/middlewares/cache-control-header.js +++ b/lib/api/middlewares/cache-control-header.js @@ -27,7 +27,12 @@ module.exports = function setCacheControlHeader ({ revalidate = false } = {}) { if (!validFallbackTTL.includes(fallbackTtl)) { - throw new Error(`Invalid fallback TTL value for Cache-Control header. Got ${fallbackTtl}, expected ${validFallbackTTL.join(', ')}`); + const message = [ + 'Invalid fallback TTL value for Cache-Control header.', + `Got ${fallbackTtl}, expected ${validFallbackTTL.join(', ')}` + ].join(' '); + + throw new Error(message); } return function setCacheControlHeaderMiddleware (req, res, next) { diff --git a/test/acceptance/cache/cache-control-header-test.js b/test/acceptance/cache/cache-control-header-test.js index d761b6ef..3094c8a2 100644 --- a/test/acceptance/cache/cache-control-header-test.js +++ b/test/acceptance/cache/cache-control-header-test.js @@ -80,8 +80,8 @@ describe('cache-control header', function () { } const cacheControl = res.headers['cache-control']; - const [ type, maxAge ] = cacheControl.split(','); - const [ key, value ] = maxAge.split('='); + const [ , maxAge ] = cacheControl.split(','); + const [ , value ] = maxAge.split('='); assert.ok(Number(value) <= ttl); @@ -120,8 +120,8 @@ describe('cache-control header', function () { } const cacheControl = res.headers['cache-control']; - const [ type, maxAge ] = cacheControl.split(','); - const [ key, value ] = maxAge.split('='); + const [ , maxAge ] = cacheControl.split(','); + const [ , value ] = maxAge.split('='); assert.ok(Number(value) <= ttl); From 8ed187b0f5f36996d494b4e2ef3b4933ff09073e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 21 Oct 2019 11:01:05 +0200 Subject: [PATCH 06/11] Do not set Last-Modifed to January 1st 1970 when cache buster ins layergroup token is 0. In this case, 0 means we don't know when the resource was updated for the last time. --- lib/api/middlewares/last-modified-header.js | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/lib/api/middlewares/last-modified-header.js b/lib/api/middlewares/last-modified-header.js index 3f6c6e6a..4d1fad52 100644 --- a/lib/api/middlewares/last-modified-header.js +++ b/lib/api/middlewares/last-modified-header.js @@ -10,7 +10,9 @@ module.exports = function setLastModifiedHeader () { if (cache_buster) { const cacheBuster = parseInt(cache_buster, 10); - const lastModifiedDate = Number.isFinite(cacheBuster) ? new Date(cacheBuster) : new Date(); + const lastModifiedDate = Number.isFinite(cacheBuster) && cacheBuster !== 0 + ? new Date(cacheBuster) + : new Date(); res.set('Last-Modified', lastModifiedDate.toUTCString()); From 0c9cfefcd0125cd7886ae370c3cbb857f6614127 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 21 Oct 2019 11:13:54 +0200 Subject: [PATCH 07/11] Please jshint, can you be a regular linter? --- lib/api/middlewares/last-modified-header.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/lib/api/middlewares/last-modified-header.js b/lib/api/middlewares/last-modified-header.js index 4d1fad52..e80d142a 100644 --- a/lib/api/middlewares/last-modified-header.js +++ b/lib/api/middlewares/last-modified-header.js @@ -10,9 +10,9 @@ module.exports = function setLastModifiedHeader () { if (cache_buster) { const cacheBuster = parseInt(cache_buster, 10); - const lastModifiedDate = Number.isFinite(cacheBuster) && cacheBuster !== 0 - ? new Date(cacheBuster) - : new Date(); + const lastModifiedDate = Number.isFinite(cacheBuster) && cacheBuster !== 0 ? + new Date(cacheBuster) : + new Date(); res.set('Last-Modified', lastModifiedDate.toUTCString()); From f844d70275ed6ab042a1d6afb39c23dcf6523864 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 21 Oct 2019 11:32:56 +0200 Subject: [PATCH 08/11] Replace http --> https. I swear, I'm not a spy --- package-lock.json | 46 +++++++++++++++++++++++----------------------- 1 file changed, 23 insertions(+), 23 deletions(-) diff --git a/package-lock.json b/package-lock.json index 7899ea6d..9307290a 100644 --- a/package-lock.json +++ b/package-lock.json @@ -166,7 +166,7 @@ }, "async": { "version": "1.5.2", - "resolved": "http://registry.npmjs.org/async/-/async-1.5.2.tgz", + "resolved": "https://registry.npmjs.org/async/-/async-1.5.2.tgz", "integrity": "sha1-7GphrlZIDAw8skHJVhjiCJL5Zyo=" }, "asynckit": { @@ -811,7 +811,7 @@ }, "entities": { "version": "1.0.0", - "resolved": "http://registry.npmjs.org/entities/-/entities-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/entities/-/entities-1.0.0.tgz", "integrity": "sha1-sph6o4ITR/zeZCsk/fyeT7cSvyY=", "dev": true }, @@ -825,7 +825,7 @@ }, "es6-promise": { "version": "3.1.2", - "resolved": "http://registry.npmjs.org/es6-promise/-/es6-promise-3.1.2.tgz", + "resolved": "https://registry.npmjs.org/es6-promise/-/es6-promise-3.1.2.tgz", "integrity": "sha1-eV4lzrR/e6uyY9FRr77dktGOagc=" }, "escape-html": { @@ -913,7 +913,7 @@ }, "express": { "version": "4.16.3", - "resolved": "http://registry.npmjs.org/express/-/express-4.16.3.tgz", + "resolved": "https://registry.npmjs.org/express/-/express-4.16.3.tgz", "integrity": "sha1-avilAjUNsyRuzEvs9rWjTSL37VM=", "requires": { "accepts": "~1.3.5", @@ -1036,7 +1036,7 @@ }, "fast-deep-equal": { "version": "1.1.0", - "resolved": "http://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-1.1.0.tgz", + "resolved": "https://registry.npmjs.org/fast-deep-equal/-/fast-deep-equal-1.1.0.tgz", "integrity": "sha1-wFNHeBfIa1HaqFPIHgWbcz0CNhQ=" }, "fast-json-stable-stringify": { @@ -1065,7 +1065,7 @@ }, "finalhandler": { "version": "1.1.1", - "resolved": "http://registry.npmjs.org/finalhandler/-/finalhandler-1.1.1.tgz", + "resolved": "https://registry.npmjs.org/finalhandler/-/finalhandler-1.1.1.tgz", "integrity": "sha512-Y1GUDo39ez4aHAw7MysnUD5JzYX+WaIj8I57kO3aEPT1fFRL4sr7mjei97FgnwhAyyzRYmQZaTHb2+9uZ1dPtg==", "requires": { "debug": "2.6.9", @@ -2007,7 +2007,7 @@ }, "get-stream": { "version": "3.0.0", - "resolved": "http://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", + "resolved": "https://registry.npmjs.org/get-stream/-/get-stream-3.0.0.tgz", "integrity": "sha1-jpQ9E1jcN1VQVOy+LtsFqhdO3hQ=" }, "getpass": { @@ -2228,7 +2228,7 @@ }, "htmlparser2": { "version": "3.8.3", - "resolved": "http://registry.npmjs.org/htmlparser2/-/htmlparser2-3.8.3.tgz", + "resolved": "https://registry.npmjs.org/htmlparser2/-/htmlparser2-3.8.3.tgz", "integrity": "sha1-mWwosZFRaovoZQGn15dX5ccMEGg=", "dev": true, "requires": { @@ -2241,7 +2241,7 @@ "dependencies": { "readable-stream": { "version": "1.1.14", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-1.1.14.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-1.1.14.tgz", "integrity": "sha1-fPTFTvZI44EwhMY23SB54WbAgdk=", "dev": true, "requires": { @@ -2255,7 +2255,7 @@ }, "http-errors": { "version": "1.6.3", - "resolved": "http://registry.npmjs.org/http-errors/-/http-errors-1.6.3.tgz", + "resolved": "https://registry.npmjs.org/http-errors/-/http-errors-1.6.3.tgz", "integrity": "sha1-i1VoC7S+KDoLW/TqLjhYC+HZMg0=", "requires": { "depd": "~1.1.2", @@ -2579,12 +2579,12 @@ "dependencies": { "async": { "version": "0.2.10", - "resolved": "http://registry.npmjs.org/async/-/async-0.2.10.tgz", + "resolved": "https://registry.npmjs.org/async/-/async-0.2.10.tgz", "integrity": "sha1-trvgsGdLnXGXCMo43owjfLUmw9E=" }, "semver": { "version": "4.3.6", - "resolved": "http://registry.npmjs.org/semver/-/semver-4.3.6.tgz", + "resolved": "https://registry.npmjs.org/semver/-/semver-4.3.6.tgz", "integrity": "sha1-MAvG4OhjdPe6YQaLWx7NV/xlMto=" }, "underscore": { @@ -2619,7 +2619,7 @@ }, "media-typer": { "version": "0.3.0", - "resolved": "http://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", + "resolved": "https://registry.npmjs.org/media-typer/-/media-typer-0.3.0.tgz", "integrity": "sha1-hxDXrwqmJvj/+hzgAWhUUmMlV0g=" }, "mem": { @@ -2705,7 +2705,7 @@ }, "minimist": { "version": "0.0.8", - "resolved": "http://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", + "resolved": "https://registry.npmjs.org/minimist/-/minimist-0.0.8.tgz", "integrity": "sha1-hX/Kv8M5fSYluCKCYuhqp6ARsF0=" }, "minipass": { @@ -2739,7 +2739,7 @@ }, "mkdirp": { "version": "0.5.1", - "resolved": "http://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", + "resolved": "https://registry.npmjs.org/mkdirp/-/mkdirp-0.5.1.tgz", "integrity": "sha1-MAV0OOrGz3+MR2fzhkjWaX11yQM=", "requires": { "minimist": "0.0.8" @@ -3237,7 +3237,7 @@ }, "postcss": { "version": "5.0.19", - "resolved": "http://registry.npmjs.org/postcss/-/postcss-5.0.19.tgz", + "resolved": "https://registry.npmjs.org/postcss/-/postcss-5.0.19.tgz", "integrity": "sha1-tjQqAdx1uMq36Wiv2pau/Gf4iK8=", "requires": { "js-base64": "^2.1.9", @@ -3276,7 +3276,7 @@ }, "postcss-value-parser": { "version": "3.3.0", - "resolved": "http://registry.npmjs.org/postcss-value-parser/-/postcss-value-parser-3.3.0.tgz", + "resolved": "https://registry.npmjs.org/postcss-value-parser/-/postcss-value-parser-3.3.0.tgz", "integrity": "sha1-h/OPnxj3dKSrTIojL1xc6IcqnRU=" }, "postgres-array": { @@ -3403,7 +3403,7 @@ }, "readable-stream": { "version": "1.0.34", - "resolved": "http://registry.npmjs.org/readable-stream/-/readable-stream-1.0.34.tgz", + "resolved": "https://registry.npmjs.org/readable-stream/-/readable-stream-1.0.34.tgz", "integrity": "sha1-Elgg40vIQtLyqq+v5MKRbuMsFXw=", "requires": { "core-util-is": "~1.0.0", @@ -3779,7 +3779,7 @@ }, "strip-ansi": { "version": "3.0.1", - "resolved": "http://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", + "resolved": "https://registry.npmjs.org/strip-ansi/-/strip-ansi-3.0.1.tgz", "integrity": "sha1-ajhfuIU9lS1f8F0Oiq+UJ43GPc8=", "requires": { "ansi-regex": "^2.0.0" @@ -3795,7 +3795,7 @@ }, "strip-eof": { "version": "1.0.0", - "resolved": "http://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", + "resolved": "https://registry.npmjs.org/strip-eof/-/strip-eof-1.0.0.tgz", "integrity": "sha1-u0P/VZim6wXYm1n80SnJgzE2Br8=" }, "strip-json-comments": { @@ -3839,7 +3839,7 @@ }, "through": { "version": "2.3.8", - "resolved": "http://registry.npmjs.org/through/-/through-2.3.8.tgz", + "resolved": "https://registry.npmjs.org/through/-/through-2.3.8.tgz", "integrity": "sha1-DdTJ/6q8NXlgsbckEV1+Doai4fU=" }, "torque.js": { @@ -4066,7 +4066,7 @@ }, "wrap-ansi": { "version": "2.1.0", - "resolved": "http://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", + "resolved": "https://registry.npmjs.org/wrap-ansi/-/wrap-ansi-2.1.0.tgz", "integrity": "sha1-2Pw9KE3QV5T+hJc8rs3Rz4JP3YU=", "requires": { "string-width": "^1.0.1", @@ -4095,7 +4095,7 @@ }, "yargs": { "version": "11.1.0", - "resolved": "http://registry.npmjs.org/yargs/-/yargs-11.1.0.tgz", + "resolved": "https://registry.npmjs.org/yargs/-/yargs-11.1.0.tgz", "integrity": "sha512-NwW69J42EsCSanF8kyn5upxvjp5ds+t3+udGBeTbFnERA+lF541DDpMawzo4z6W/QrzNM18D+BPMiOBibnFV5A==", "requires": { "cliui": "^4.0.0", From 736d3460d989eefd64c38afc1adb561d880d193f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 21 Oct 2019 13:46:29 +0200 Subject: [PATCH 09/11] Update development branch --- package-lock.json | 4 ++-- package.json | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package-lock.json b/package-lock.json index 9307290a..13eb1e2b 100644 --- a/package-lock.json +++ b/package-lock.json @@ -427,8 +427,8 @@ } }, "cartodb-query-tables": { - "version": "github:cartodb/node-cartodb-query-tables#ea2734da8f3c6d1c37e48f438f2bb8d1c5c832b5", - "from": "github:cartodb/node-cartodb-query-tables#coherent-ttl-refactor", + "version": "github:cartodb/node-cartodb-query-tables#760aa889197da49626018c2a54b45894fdcb6c71", + "from": "github:cartodb/node-cartodb-query-tables#coherent-ttl", "requires": { "decimal.js": "10.2.0" } diff --git a/package.json b/package.json index d6af65a1..bdeab731 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "body-parser": "1.18.3", "camshaft": "^0.64.2", "cartodb-psql": "0.14.0", - "cartodb-query-tables": "github:cartodb/node-cartodb-query-tables#coherent-ttl-refactor", + "cartodb-query-tables": "github:cartodb/node-cartodb-query-tables#coherent-ttl", "cartodb-redis": "2.1.0", "debug": "3.1.0", "dot": "1.1.2", From be4d610de113cc7837b4d6c73138a1d0f6329d64 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 21 Oct 2019 16:14:07 +0200 Subject: [PATCH 10/11] Use released version of cartodb-query-tables 0.7.0 --- NEWS.md | 3 +++ package-lock.json | 5 +++-- package.json | 2 +- 3 files changed, 7 insertions(+), 3 deletions(-) diff --git a/NEWS.md b/NEWS.md index 058c65bc..5212f51b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -10,6 +10,9 @@ Announcements: - Added mechanism to inject custom middlewares through configuration. - Stop requiring unused config properties: "base_url", "base_url_mapconfig", and "base_url_templated". +- Upgraded cartodb-query-tables to version [0.7.0](https://github.com/CartoDB/node-cartodb-query-tables/blob/0.7.0/NEWS.md#version-0.7.0). +- Be able to set a coherent TTL in Cache-Control header to expire all resources belonging a map simultaneously. +- When `cache buster` in request path is `0` set header `Last-Modified` to now, it avoids stalled content in 3rd party cache providers when they add `If-Modified-Since` header into the request. ## 7.2.0 Released 2019-09-30 diff --git a/package-lock.json b/package-lock.json index 13eb1e2b..f7ab113e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -427,8 +427,9 @@ } }, "cartodb-query-tables": { - "version": "github:cartodb/node-cartodb-query-tables#760aa889197da49626018c2a54b45894fdcb6c71", - "from": "github:cartodb/node-cartodb-query-tables#coherent-ttl", + "version": "0.7.0", + "resolved": "https://registry.npmjs.org/cartodb-query-tables/-/cartodb-query-tables-0.7.0.tgz", + "integrity": "sha512-DNLP14IR9xp/nlyM2ivGG8ml1VLu1WWzNqGFUIAkEG8oTehAm8aHbfaUKkDVCOSzhcN4jqAr4YCUKdG7FuqQaA==", "requires": { "decimal.js": "10.2.0" } diff --git a/package.json b/package.json index bdeab731..8db23c86 100644 --- a/package.json +++ b/package.json @@ -29,7 +29,7 @@ "body-parser": "1.18.3", "camshaft": "^0.64.2", "cartodb-psql": "0.14.0", - "cartodb-query-tables": "github:cartodb/node-cartodb-query-tables#coherent-ttl", + "cartodb-query-tables": "^0.7.0", "cartodb-redis": "2.1.0", "debug": "3.1.0", "dot": "1.1.2", From 8a781d241c4e3b084747ec14e4278ab6623eccb5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 21 Oct 2019 16:22:26 +0200 Subject: [PATCH 11/11] Typo --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 5212f51b..f3cde6ab 100644 --- a/NEWS.md +++ b/NEWS.md @@ -11,7 +11,7 @@ Announcements: - Added mechanism to inject custom middlewares through configuration. - Stop requiring unused config properties: "base_url", "base_url_mapconfig", and "base_url_templated". - Upgraded cartodb-query-tables to version [0.7.0](https://github.com/CartoDB/node-cartodb-query-tables/blob/0.7.0/NEWS.md#version-0.7.0). -- Be able to set a coherent TTL in Cache-Control header to expire all resources belonging a map simultaneously. +- Be able to set a coherent TTL in Cache-Control header to expire all resources belonging to a map simultaneously. - When `cache buster` in request path is `0` set header `Last-Modified` to now, it avoids stalled content in 3rd party cache providers when they add `If-Modified-Since` header into the request. ## 7.2.0