diff --git a/NEWS.md b/NEWS.md index 058c65bc..f3cde6ab 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 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 Released 2019-09-30 diff --git a/lib/api/middlewares/cache-control-header.js b/lib/api/middlewares/cache-control-header.js index d15b259c..84b7a360 100644 --- a/lib/api/middlewares/cache-control-header.js +++ b/lib/api/middlewares/cache-control-header.js @@ -1,14 +1,40 @@ '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)) { + 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) { if (req.method !== 'GET') { return next(); @@ -27,7 +53,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 +75,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/lib/api/middlewares/last-modified-header.js b/lib/api/middlewares/last-modified-header.js index 3f6c6e6a..e80d142a 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()); diff --git a/lib/models/mapconfig/provider/base-mapconfig-adapter.js b/lib/models/mapconfig/provider/base-mapconfig-adapter.js index 301ea3b5..a606d0cb 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,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..f7ab113e 100644 --- a/package-lock.json +++ b/package-lock.json @@ -427,9 +427,9 @@ } }, "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": "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 c983c5ab..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": "^0.6.3", + "cartodb-query-tables": "^0.7.0", "cartodb-redis": "2.1.0", "debug": "3.1.0", "dot": "1.1.2", diff --git a/test/acceptance/cache/cache-control-header-test.js b/test/acceptance/cache/cache-control-header-test.js index d3b89863..3094c8a2 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 [ , maxAge ] = cacheControl.split(','); + const [ , 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 [ , maxAge ] = cacheControl.split(','); + const [ , value ] = maxAge.split('='); + + assert.ok(Number(value) <= ttl); + testClient.drain(done); }); }); diff --git a/test/acceptance/multilayer-server-test.js b/test/acceptance/multilayer-server-test.js index 7a7b7e72..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 = function(pg, sql, callback) { - return callback(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 e44d66b9..dcc3aad8 100644 --- a/test/integration/query-tables-test.js +++ b/test/integration/query-tables-test.js @@ -32,36 +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', function(done) { + it('should return an object with affected tables array and last updated time', function () { var query = 'select * from test_table'; - QueryTables.getQueryMetadataModel(connection, query, function(err, result) { - assert.ok(!err, err); + 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'); - - done(); - }); }); - it('should work with private tables', function(done) { + it('should work with private tables', function () { var query = 'select * from test_table_private_1'; - QueryTables.getQueryMetadataModel(connection, query, function(err, result) { - assert.ok(!err, err); + 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'); - - 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'); + }); }); - });