From 920cb350cc15715c386ef57ac69db7243eb193e6 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Mon, 7 Sep 2015 18:09:42 +0200 Subject: [PATCH 1/2] x-cache-channel tests --- test/acceptance/x-cache-channel.js | 154 +++++++++++++---------------- 1 file changed, 68 insertions(+), 86 deletions(-) diff --git a/test/acceptance/x-cache-channel.js b/test/acceptance/x-cache-channel.js index ea35fb7f..b06bb420 100644 --- a/test/acceptance/x-cache-channel.js +++ b/test/acceptance/x-cache-channel.js @@ -8,98 +8,80 @@ var _ = require('underscore'); // allow lots of emitters to be set to silence warning app.setMaxListeners(0); -describe('x_cache_channel', function() { +describe('X-Cache-Channel header', function() { -assert.contains = function(ary, elem) { - assert.ok(_.contains(ary,elem), 'missing "' + elem +'" from x-cache-channel: '+ ary); -}; + function createGetRequest(sqlQuery) { + var query = querystring.stringify({ + q: sqlQuery, + api_key: 1234 + }); + return { + url: '/api/v1/sql?' + query, + headers: { + host: 'vizzuality.cartodb.com' + }, + method: 'GET' + }; + } -it('supports joins', function(done) { - var query = querystring.stringify({ - q: "SELECT a.name as an, b.name as bn FROM untitle_table_4 a " + - "left join private_table b ON (a.cartodb_id = b.cartodb_id)", - api_key: 1234 - }); - assert.response(app, { - url: '/api/v1/sql?' + query, - headers: {host: 'vizzuality.cartodb.com' }, - method: 'GET' - },{ }, function(res) { - assert.equal(res.statusCode, 200, res.body); - // Check x-cache headers - var cc = res.headers['x-cache-channel'].split(':'); - assert.equal(cc[0], 'cartodb_test_user_1_db'); - var tt = cc[1].split(','); - assert.equal(tt.length, 2); - assert.contains(tt, 'public.private_table'); - assert.contains(tt, 'public.untitle_table_4'); - done(); - }); -}); + var RESPONSE_OK = { + statusCode: 200 + }; -it('supports multistatements', function(done) { - var query = querystring.stringify({ - q: "SELECT * FROM untitle_table_4; SELECT * FROM private_table", - api_key: 1234 - }); - assert.response(app, { - url: '/api/v1/sql?' + query, - headers: {host: 'vizzuality.cartodb.com' }, - method: 'GET' - },{ }, function(res) { - assert.equal(res.statusCode, 200, res.body); - // Check x-cache headers - var cc = res.headers['x-cache-channel'].split(':'); - assert.equal(cc[0], 'cartodb_test_user_1_db'); - var tt = cc[1].split(','); - assert.equal(tt.length, 2); - assert.contains(tt, 'public.private_table'); - assert.contains(tt, 'public.untitle_table_4'); - done(); - }); -}); + function xCacheChannelHeaderHasTables(xCacheChannel, expectedTablesNames) { + var databaseAndTables = xCacheChannel.split(':'); + var databaseName = databaseAndTables[0]; -it('supports explicit transactions', function(done) { - var query = querystring.stringify({ - q: "BEGIN; SELECT * FROM untitle_table_4; COMMIT; BEGIN; SELECT * FROM private_table; COMMIT;", - api_key: 1234 - }); - assert.response(app, { - url: '/api/v1/sql?' + query, - headers: {host: 'vizzuality.cartodb.com' }, - method: 'GET' - },{ }, function(res) { - assert.equal(res.statusCode, 200, res.body); - // Check x-cache headers - var cc = res.headers['x-cache-channel'].split(':'); - assert.equal(cc[0], 'cartodb_test_user_1_db'); - var tt = cc[1].split(','); - assert.equal(tt.length, 2); - assert.contains(tt, 'public.private_table'); - assert.contains(tt, 'public.untitle_table_4'); - done(); - }); -}); + assert.equal(databaseName, 'cartodb_test_user_1_db'); -it('survives partial transactions', function(done) { - var query = querystring.stringify({ - q: "BEGIN; SELECT * FROM untitle_table_4", - api_key: 1234 + var headerTableNames = databaseAndTables[1].split(','); + assert.equal(headerTableNames.length, expectedTablesNames.length); + + var tablesDiff = _.difference(expectedTablesNames, headerTableNames); + assert.equal(tablesDiff.length, 0, 'X-Cache-Channel header missing tables: ' + tablesDiff.join(',')); + } + + function tableNamesInCacheChannelHeader(expectedTableNames, done) { + return function(res) { + xCacheChannelHeaderHasTables(res.headers['x-cache-channel'], expectedTableNames); + done(); + }; + } + + it('supports joins', function(done) { + var sql = "SELECT a.name as an, b.name as bn FROM untitle_table_4 a " + + "left join private_table b ON (a.cartodb_id = b.cartodb_id)"; + + assert.response(app, createGetRequest(sql), RESPONSE_OK, tableNamesInCacheChannelHeader([ + 'public.private_table', + 'public.untitle_table_4' + ], done)); }); - assert.response(app, { - url: '/api/v1/sql?' + query, - headers: {host: 'vizzuality.cartodb.com' }, - method: 'GET' - },{ }, function(res) { - assert.equal(res.statusCode, 200, res.body); - // Check x-cache headers - var cc = res.headers['x-cache-channel'].split(':'); - assert.equal(cc[0], 'cartodb_test_user_1_db'); - var tt = cc[1].split(','); - assert.equal(tt.length, 1); - assert.contains(tt, 'public.untitle_table_4'); - done(); + + it('supports multistatements', function(done) { + var sql = "SELECT * FROM untitle_table_4; SELECT * FROM private_table"; + + assert.response(app, createGetRequest(sql), RESPONSE_OK, tableNamesInCacheChannelHeader([ + 'public.private_table', + 'public.untitle_table_4' + ], done)); + }); + + it('supports explicit transactions', function(done) { + var sql = "BEGIN; SELECT * FROM untitle_table_4; COMMIT; BEGIN; SELECT * FROM private_table; COMMIT;"; + + assert.response(app, createGetRequest(sql), RESPONSE_OK, tableNamesInCacheChannelHeader([ + 'public.private_table', + 'public.untitle_table_4' + ], done)); + }); + + it('survives partial transactions', function(done) { + var sql = "BEGIN; SELECT * FROM untitle_table_4"; + + assert.response(app, createGetRequest(sql), RESPONSE_OK, tableNamesInCacheChannelHeader([ + 'public.untitle_table_4' + ], done)); }); -}); }); From 875eccba62131a7632b2657b178551e0cebb7902 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Mon, 7 Sep 2015 18:22:24 +0200 Subject: [PATCH 2/2] Stop adding X-Cache-Channel header when no tables were identified in SQL query Fixes #238 and fixes #157 --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 2 +- test/acceptance/x-cache-channel.js | 26 ++++++++++++++++++++++++++ 3 files changed, 28 insertions(+), 2 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 158b2827..46636ac8 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -488,7 +488,7 @@ function handleQuery(req, res) { } // Only set an X-Cache-Channel for responses we want Varnish to cache. - if ( tableCacheItem && ! tableCacheItem.may_write ) { + if ( tableCacheItem && tableCacheItem.affected_tables.length > 0 && !tableCacheItem.may_write ) { res.header('X-Cache-Channel', generateCacheKey(dbopts.dbname, tableCacheItem, authenticated)); } diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 78644c90..6db04df3 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -1082,7 +1082,7 @@ it('numeric arrays are rendered as such', function(done){ assert.equal(out.rows[0].x.length, 2); assert.equal(out.rows[0].x[0], '8.7'); assert.equal(out.rows[0].x[1], '4.3'); - assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:'); // keep forever + assert.equal(res.headers.hasOwnProperty('x-cache-channel'), false); done(); }); }); diff --git a/test/acceptance/x-cache-channel.js b/test/acceptance/x-cache-channel.js index b06bb420..53c58cc1 100644 --- a/test/acceptance/x-cache-channel.js +++ b/test/acceptance/x-cache-channel.js @@ -84,4 +84,30 @@ describe('X-Cache-Channel header', function() { ], done)); }); + it('should not add header for functions', function(done) { + var sql = "SELECT format('%s', 'wadus')"; + assert.response(app, createGetRequest(sql), RESPONSE_OK, function(res) { + assert.ok(!res.headers.hasOwnProperty('x-cache-channel'), res.headers['x-cache-channel']); + done(); + }); + }); + + it('should not add header for CDB_QueryTables', function(done) { + var sql = "SELECT CDB_QueryTablesText('select * from untitle_table_4')"; + assert.response(app, createGetRequest(sql), RESPONSE_OK, function(res) { + assert.ok(!res.headers.hasOwnProperty('x-cache-channel'), res.headers['x-cache-channel']); + done(); + }); + }); + + it('should not add header for non table results', function(done) { + var sql = "SELECT 'wadus'::text"; + assert.response(app, createGetRequest(sql), RESPONSE_OK, function(res) { + assert.ok(!res.headers.hasOwnProperty('x-cache-channel'), res.headers['x-cache-channel']); + done(); + }); + }); + + + });