diff --git a/NEWS.md b/NEWS.md index 27f51b6a..d774985d 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ Released 2019-mm-dd Announcements: +* Cache control header fine tuning. Set a shorter value for "max-age" directive if there is no way to know when to trigger the invalidation. * Upgrade devel dependency `sqlite3` to version `4.0.6` * Log queries (https://github.com/CartoDB/CartoDB-SQL-API/pull/574) * Improve batch-queries draining while exiting the process #582 diff --git a/app/controllers/query_controller.js b/app/controllers/query_controller.js index 5bd02d51..f570542e 100644 --- a/app/controllers/query_controller.js +++ b/app/controllers/query_controller.js @@ -23,7 +23,8 @@ const { RATE_LIMIT_ENDPOINTS_GROUPS } = rateLimitsMiddleware; const handleQueryMiddleware = require('../middlewares/handle-query'); const logMiddleware = require('../middlewares/log'); -var ONE_YEAR_IN_SECONDS = 31536000; // 1 year time to live by default +const ONE_YEAR_IN_SECONDS = 31536000; // ttl in cache provider +const FIVE_MINUTES_IN_SECONDS = 60 * 5; // ttl in cache provider function QueryController(metadataBackend, userDatabaseService, tableCache, statsd_client, userLimitsService) { this.metadataBackend = metadataBackend; @@ -190,8 +191,13 @@ QueryController.prototype.handleQuery = function (req, res, next) { if (cachePolicy === 'persist') { res.header('Cache-Control', 'public,max-age=' + ONE_YEAR_IN_SECONDS); } else { - var maxAge = (mayWrite) ? 0 : ONE_YEAR_IN_SECONDS; - res.header('Cache-Control', 'no-cache,max-age='+maxAge+',must-revalidate,public'); + if (affectedTables && affectedTables.getTables().every(table => table.updated_at !== null)) { + const maxAge = mayWrite ? 0 : (global.settings.cache.ttl || ONE_YEAR_IN_SECONDS); + res.header('Cache-Control', `no-cache,max-age=${maxAge},must-revalidate,public`); + } else { + const maxAge = global.settings.cache.fallbackTtl || FIVE_MINUTES_IN_SECONDS; + res.header('Cache-Control', `no-cache,max-age=${maxAge},must-revalidate,public`); + } } // Only set an X-Cache-Channel for responses we want Varnish to cache. diff --git a/config/environments/development.js.example b/config/environments/development.js.example index 19c4b9e2..6ffd9ff5 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -131,3 +131,8 @@ module.exports.validatePGEntitiesAccess = false; module.exports.dataIngestionLogPath = 'logs/data-ingestion.log'; module.exports.logQueries = true; module.exports.maxQueriesLogLength = 2000; + +module.exports.cache = { + ttl: 60 * 60 * 24 * 365, // one year in seconds + fallbackTtl: 60 * 5 // five minutes in seconds +}; diff --git a/config/environments/production.js.example b/config/environments/production.js.example index 2987d2fc..077e2795 100644 --- a/config/environments/production.js.example +++ b/config/environments/production.js.example @@ -135,3 +135,8 @@ module.exports.validatePGEntitiesAccess = false; module.exports.dataIngestionLogPath = 'logs/data-ingestion.log'; module.exports.logQueries = true; module.exports.maxQueriesLogLength = 1024; + +module.exports.cache = { + ttl: 60 * 60 * 24 * 365, // one year in seconds + fallbackTtl: 60 * 5 // five minutes in seconds +}; diff --git a/config/environments/staging.js.example b/config/environments/staging.js.example index ee7ab0d2..ca3b6771 100644 --- a/config/environments/staging.js.example +++ b/config/environments/staging.js.example @@ -132,3 +132,8 @@ module.exports.validatePGEntitiesAccess = false; module.exports.dataIngestionLogPath = 'logs/data-ingestion.log'; module.exports.logQueries = true; module.exports.maxQueriesLogLength = 1024; + +module.exports.cache = { + ttl: 60 * 60 * 24 * 365, // one year in seconds + fallbackTtl: 60 * 5 // five minutes in seconds +}; diff --git a/config/environments/test.js.example b/config/environments/test.js.example index 031493bb..2ef95be4 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -132,3 +132,8 @@ module.exports.validatePGEntitiesAccess = false; module.exports.dataIngestionLogPath = 'logs/data-ingestion.log'; module.exports.logQueries = true; module.exports.maxQueriesLogLength = 1024; + +module.exports.cache = { + ttl: 60 * 60 * 24 * 365, // one year in seconds + fallbackTtl: 60 * 5 // five minutes in seconds +}; diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 5e67d378..8e334c80 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -246,7 +246,7 @@ it('TRUNCATE TABLE with GET and auth', function(done){ assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); // table should not get a cache channel as it won't get invalidated assert.ok(!res.headers.hasOwnProperty('x-cache-channel')); - assert.equal(res.headers['cache-control'], expected_cache_control); + assert.equal(res.headers['cache-control'], 'no-cache,max-age=300,must-revalidate,public'); var pbody = JSON.parse(res.body); assert.equal(pbody.total_rows, 1); assert.equal(pbody.rows[0].count, 0); diff --git a/test/acceptance/cache-headers.js b/test/acceptance/cache-headers.js new file mode 100644 index 00000000..8d1b6da5 --- /dev/null +++ b/test/acceptance/cache-headers.js @@ -0,0 +1,112 @@ +'use strict'; + +const server = require('../../app/server')(); +const assert = require('../support/assert'); +const qs = require('querystring'); + +describe('cache headers', function () { + it('should return a Vary header', function (done) { + assert.response(server, { + url: `/api/v1/sql?${qs.encode({ + api_key: '1234', + q: 'select * from untitle_table_4' + })}`, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }, + {}, + function (err, res) { + assert.equal(res.headers.vary, 'Authorization'); + done(); + }); + }); + + it('should return a proper max-age when CDB_TableMetadata table includes the last updated time', function (done) { + const ONE_YEAR_IN_SECONDS = 60 * 60 * 24 * 365; + const noTtl = 0; + const fallbackTtl = global.settings.cache.fallbackTtl; + const ttl = global.settings.cache.ttl || ONE_YEAR_IN_SECONDS; + const tableName = `wadus_table_${Date.now()}`; + + assert.response(server, { + url: `/api/v1/sql?${qs.encode({ + api_key: '1234', + q: `create table ${tableName}()` + })}`, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }, + {}, + function(err, res) { + assert.equal(res.headers['cache-control'], `no-cache,max-age=${noTtl},must-revalidate,public`); + + assert.response(server, { + url: `/api/v1/sql?${qs.encode({ + api_key: '1234', + q: `select * from ${tableName}` + })}`, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }, {}, + function(err, res) { + assert.equal(res.headers['cache-control'], `no-cache,max-age=${fallbackTtl},must-revalidate,public`); + + assert.response(server, { + url: `/api/v1/sql?${qs.encode({ + api_key: '1234', + q: `select CDB_TableMetadataTouch('${tableName}'::regclass)` + })}`, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }, {}, + function(err, res) { + assert.equal(res.headers['cache-control'], `no-cache,max-age=${ttl},must-revalidate,public`); + + assert.response(server, { + url: `/api/v1/sql?${qs.encode({ + api_key: '1234', + q: `select * from ${tableName}` + })}`, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }, {}, + function(err, res) { + assert.equal(res.headers['cache-control'], `no-cache,max-age=${ttl},must-revalidate,public`); + done(); + }); + }); + }); + }); + }); + + it('should return a proper max-age when the query doesn\'t use any table', function (done) { + const ONE_YEAR_IN_SECONDS = 60 * 60 * 24 * 365; + const ttl = global.settings.cache.ttl || ONE_YEAR_IN_SECONDS; + + assert.response(server, { + url: `/api/v1/sql?${qs.encode({ + api_key: '1234', + q: `select 1` + })}`, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }, + {}, + function (err, res) { + assert.equal(res.headers['cache-control'], `no-cache,max-age=${ttl},must-revalidate,public`); + done(); + }); + }); +}); diff --git a/test/acceptance/cache.js b/test/acceptance/cache.js deleted file mode 100644 index f8914ca9..00000000 --- a/test/acceptance/cache.js +++ /dev/null @@ -1,21 +0,0 @@ -'use strict'; - -var server = require('../../app/server')(); -const assert = require('../support/assert'); - -describe('Cache', function () { - it('should return a Vary header', function (done) { - assert.response(server, { - url: '/api/v1/sql?api_key=1234&g=select%20*%20from%20untitle_table_4', - headers: { - host: 'vizzuality.cartodb.com' - }, - method: 'GET' - }, - {}, - function(err, res) { - assert.equal(res.headers.vary, 'Authorization'); - done(); - }); - }); -});