From 5e4d1d5c1c6d0fbe39608d9a2b60819f556cf9da Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Fri, 10 Mar 2017 11:40:48 +0100 Subject: [PATCH 1/8] Get affected tables and add it to the layergroup --- .../adapter/analysis-mapconfig-adapter.js | 9 ++++++ test/acceptance/analysis/analysis-layers.js | 30 +++++++++++++++++++ 2 files changed, 39 insertions(+) diff --git a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js index b59684c7..09467ea9 100644 --- a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js @@ -115,6 +115,7 @@ AnalysisMapConfigAdapter.prototype.getMapConfig = function(user, requestMapConfi } layer.options.sql = analysisSql; layer.options.columns = getDataviewsColumns(getLayerDataviews(layer, dataviews)); + layer.options.affected_tables = getAllAffectedTablesFromSourceNodes(layerNode); } else { missingNodesErrors.push( new Error('Missing analysis node.id="' + layerSourceId +'" for layer='+layerIndex) @@ -330,4 +331,12 @@ function AnalysisError(message) { this.message = message; } +function getAllAffectedTablesFromSourceNodes(node) { + var affectedTables = []; + var affectedTables = node.getAllInputNodes(function (node) { + return node.getType() === 'source'; + }).reduce(function(list, node) { return list.concat(node.getAffectedTables()); },[]); + return affectedTables; +} + require('util').inherits(AnalysisError, Error); diff --git a/test/acceptance/analysis/analysis-layers.js b/test/acceptance/analysis/analysis-layers.js index a745fb97..3a54ea2c 100644 --- a/test/acceptance/analysis/analysis-layers.js +++ b/test/acceptance/analysis/analysis-layers.js @@ -146,6 +146,36 @@ describe('analysis-layers', function() { }); }); + it('should have empty affected tables if it has only "source" node', function(done) { + var useCase = useCases[0]; + + var testClient = new TestClient(useCase.mapConfig, 1234); + + testClient.getLayergroup(function(err, layergroupResult) { + assert.ok(!err, err); + + var affected_tables = layergroupResult.metadata.layers[0].meta.affected_tables; + assert.equal(affected_tables.length, 0); + + testClient.drain(done); + }); + }); + + it('should have empty affected tables if it has a node other than "source"', function(done) { + var useCase = useCases[1]; + + var testClient = new TestClient(useCase.mapConfig, 1234); + + testClient.getLayergroup(function(err, layergroupResult) { + assert.ok(!err, err); + + var affected_tables = layergroupResult.metadata.layers[0].meta.affected_tables; + assert.equal(affected_tables[0], 'public.populated_places_simple_reduced'); + + testClient.drain(done); + }); + }); + it('should NOT fail for non-authenticated requests when it is just source', function(done) { var useCase = useCases[0]; From 0c387cf6d98882ddba7e4b3595d56ad8ef455c4d Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Fri, 10 Mar 2017 17:22:07 +0100 Subject: [PATCH 2/8] Add more tests for x-cache-channel but with analysis --- test/acceptance/x_cache_channel.js | 247 ++++++++++++++++++----------- 1 file changed, 152 insertions(+), 95 deletions(-) diff --git a/test/acceptance/x_cache_channel.js b/test/acceptance/x_cache_channel.js index 4ebb45e8..cd12e6ce 100644 --- a/test/acceptance/x_cache_channel.js +++ b/test/acceptance/x_cache_channel.js @@ -25,32 +25,92 @@ describe('get requests x-cache-channel', function() { status: 200 }; - var mapConfig = { - version: '1.3.0', - layers: [ + var mapConfigs = [ + { + "description": "header should be present", + "data": + { + version: '1.4.0', + layers: [ + { + options: { + source: { + id: "2570e105-7b37-40d2-bdf4-1af889598745" + }, + sql: 'select * from test_table limit 2', + cartocss: '#layer { marker-fill:red; }', + cartocss_version: '2.3.0', + attributes: { + id:'cartodb_id', + columns: [ + 'name', + 'address' + ] + } + } + } + ], + analyses: [ + { + "id": "2570e105-7b37-40d2-bdf4-1af889598745", + "type": "source", + "params": { + "query": "select * from test_table limit 2" + } + } + ] + }, + }, + { + "description": "header should be present and be composed with source table name", + "data": { - options: { - sql: 'select * from test_table limit 2', - cartocss: '#layer { marker-fill:red; }', - cartocss_version: '2.3.0', - attributes: { - id:'cartodb_id', - columns: [ - 'name', - 'address' - ] + version: '1.5.0', + layers: [ + { + options: { + source: { + id: "2570e105-7b37-40d2-bdf4-1af889598745" + }, + sql: 'select * from test_table limit 2', + cartocss: '#layer { marker-fill:red; }', + cartocss_version: '2.3.0', + attributes: { + id:'cartodb_id', + columns: [ + 'name', + 'address' + ] + } + } } - } + ], + analyses: [ + { + "id": "2570e105-7b37-40d2-bdf4-1af889598745", + "type": "buffer", + "params": { + "source": { + "type": "source", + "params": { + "query": "select * from test_table limit 2" + } + }, + "radius": 50000 + } + } + ] } - ] - }; + }]; - var layergroupRequest = { - url: '/api/v1/map?config=' + encodeURIComponent(JSON.stringify(mapConfig)), - method: 'GET', - headers: { - host: 'localhost' - } + var layergroupRequest = function(mapConfig) { + return { + url: '/api/v1/map?api_key=1234&config=' + encodeURIComponent(JSON.stringify(mapConfig)), + method: 'GET', + headers: { + host: 'localhost' + } + }; }; function getRequest(url, addApiKey, callbackName) { @@ -101,10 +161,10 @@ describe('get requests x-cache-channel', function() { }; } - function withLayergroupId(callback) { + function withLayergroupId(mapConfig, callback) { assert.response( server, - layergroupRequest, + layergroupRequest(mapConfig), statusOkResponse, function(res, err) { if (err) { @@ -118,75 +178,76 @@ describe('get requests x-cache-channel', function() { ); } - describe('header should be present', function() { + mapConfigs.forEach(function(mapConfigData) { + describe(mapConfigData.description, function() { + var mapConfig = mapConfigData.data; + it('/api/v1/map Map instantiation', function(done) { + var testFn = validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table'); + withLayergroupId(mapConfig, function(err, layergroupId, res) { + testFn(res); + }); + }); - it('/api/v1/map Map instantiation', function(done) { - var testFn = validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table'); - withLayergroupId(function(err, layergroupId, res) { - testFn(res); + it ('/api/v1/map/:token/:z/:x/:y@:scale_factor?x.:format Mapnik retina tiles', function(done) { + withLayergroupId(mapConfig, function(err, layergroupId) { + assert.response( + server, + getRequest('/api/v1/map/' + layergroupId + '/0/0/0@2x.png'), + validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + ); + }); + }); + + it ('/api/v1/map/:token/:z/:x/:y@:scale_factor?x.:format Mapnik tiles', function(done) { + withLayergroupId(mapConfig, function(err, layergroupId) { + assert.response( + server, + getRequest('/api/v1/map/' + layergroupId + '/0/0/0.png'), + validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + ); + }); + }); + + it ('/api/v1/map/:token/:layer/:z/:x/:y.(:format) Per :layer rendering', function(done) { + withLayergroupId(mapConfig, function(err, layergroupId) { + assert.response( + server, + getRequest('/api/v1/map/' + layergroupId + '/0/0/0/0.png'), + validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + ); + }); + }); + + it ('/api/v1/map/:token/:layer/attributes/:fid endpoint for info windows', function(done) { + withLayergroupId(mapConfig, function(err, layergroupId) { + assert.response( + server, + getRequest('/api/v1/map/' + layergroupId + '/0/attributes/1'), + validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + ); + }); + }); + + it ('/api/v1/map/static/center/:token/:z/:lat/:lng/:width/:height.:format static maps', function(done) { + withLayergroupId(mapConfig, function(err, layergroupId) { + assert.response( + server, + getRequest('/api/v1/map/static/center/' + layergroupId + '/0/0/0/400/300.png'), + validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + ); + }); + }); + + it ('/api/v1/map/static/bbox/:token/:bbox/:width/:height.:format static maps', function(done) { + withLayergroupId(mapConfig, function(err, layergroupId) { + assert.response( + server, + getRequest('/api/v1/map/static/bbox/' + layergroupId + '/-45,-45,45,45/400/300.png'), + validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + ); + }); }); }); - - it ('/api/v1/map/:token/:z/:x/:y@:scale_factor?x.:format Mapnik retina tiles', function(done) { - withLayergroupId(function(err, layergroupId) { - assert.response( - server, - getRequest('/api/v1/map/' + layergroupId + '/0/0/0@2x.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') - ); - }); - }); - - it ('/api/v1/map/:token/:z/:x/:y@:scale_factor?x.:format Mapnik tiles', function(done) { - withLayergroupId(function(err, layergroupId) { - assert.response( - server, - getRequest('/api/v1/map/' + layergroupId + '/0/0/0.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') - ); - }); - }); - - it ('/api/v1/map/:token/:layer/:z/:x/:y.(:format) Per :layer rendering', function(done) { - withLayergroupId(function(err, layergroupId) { - assert.response( - server, - getRequest('/api/v1/map/' + layergroupId + '/0/0/0/0.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') - ); - }); - }); - - it ('/api/v1/map/:token/:layer/attributes/:fid endpoint for info windows', function(done) { - withLayergroupId(function(err, layergroupId) { - assert.response( - server, - getRequest('/api/v1/map/' + layergroupId + '/0/attributes/1'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') - ); - }); - }); - - it ('/api/v1/map/static/center/:token/:z/:lat/:lng/:width/:height.:format static maps', function(done) { - withLayergroupId(function(err, layergroupId) { - assert.response( - server, - getRequest('/api/v1/map/static/center/' + layergroupId + '/0/0/0/400/300.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') - ); - }); - }); - - it ('/api/v1/map/static/bbox/:token/:bbox/:width/:height.:format static maps', function(done) { - withLayergroupId(function(err, layergroupId) { - assert.response( - server, - getRequest('/api/v1/map/static/bbox/' + layergroupId + '/-45,-45,45,45/400/300.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') - ); - }); - }); - }); describe('header should NOT be present', function() { @@ -238,7 +299,7 @@ describe('get requests x-cache-channel', function() { auth: { method: 'open' }, - layergroup: mapConfig + layergroup: mapConfigs[0].data }; var namedMapRequest = { @@ -298,10 +359,6 @@ describe('get requests x-cache-channel', function() { noXCacheChannelHeader(done) ); }); - }); - }); - - }); From fa6493ae444de4d30527f187a544c62b52458cdd Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Mon, 13 Mar 2017 17:28:29 +0100 Subject: [PATCH 3/8] Affected tables are now included in X-Cache-Channel --- lib/cartodb/controllers/layergroup.js | 16 +++++----- lib/cartodb/controllers/map.js | 14 ++++++--- .../adapter/analysis-mapconfig-adapter.js | 1 - test/acceptance/analysis/analysis-layers.js | 30 ------------------ test/acceptance/templates.js | 6 ++-- test/acceptance/x_cache_channel.js | 31 +++++++++++-------- 6 files changed, 41 insertions(+), 57 deletions(-) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 4119655f..10b5324b 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -391,13 +391,15 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg function getSQL(err, mapConfig) { assert.ifError(err); - var queries = mapConfig.getLayers() - .map(function(lyr) { - return lyr.options.sql; - }) - .filter(function(sql) { - return !!sql; - }); + var queries = []; + mapConfig.getLayers().map(function(layer) { + queries.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(function(table) { + queries.push('SELECT * FROM ' + table + ' LIMIT 0'); + }); + } + }); return queries.length ? queries.join(';') : null; }, diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index cfd9e0d6..3b06ac02 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -306,9 +306,15 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la done(); }); - var sql = mapconfig.getLayers().map(function(layer) { - return layer.options.sql; - }).join(';'); + var sql = []; + mapconfig.getLayers().map(function(layer) { + sql.push(layer.options.sql); + if (layer.options.affected_tables) { + layer.options.affected_tables.map(function(table) { + sql.push('SELECT * FROM ' + table + ' LIMIT 0'); + }); + } + }); var dbName = req.params.dbname; var layergroupId = layergroup.layergroupid; @@ -319,7 +325,7 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la }, function getAffectedTablesAndLastUpdatedTime(err, connection) { assert.ifError(err); - QueryTables.getAffectedTablesFromQuery(connection, sql, this); + QueryTables.getAffectedTablesFromQuery(connection, sql.join(';'), this); }, function handleAffectedTablesAndLastUpdatedTime(err, result) { req.profiler.done('queryTablesAndLastUpdated'); diff --git a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js index 09467ea9..c6467488 100644 --- a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js @@ -332,7 +332,6 @@ function AnalysisError(message) { } function getAllAffectedTablesFromSourceNodes(node) { - var affectedTables = []; var affectedTables = node.getAllInputNodes(function (node) { return node.getType() === 'source'; }).reduce(function(list, node) { return list.concat(node.getAffectedTables()); },[]); diff --git a/test/acceptance/analysis/analysis-layers.js b/test/acceptance/analysis/analysis-layers.js index 3a54ea2c..a745fb97 100644 --- a/test/acceptance/analysis/analysis-layers.js +++ b/test/acceptance/analysis/analysis-layers.js @@ -146,36 +146,6 @@ describe('analysis-layers', function() { }); }); - it('should have empty affected tables if it has only "source" node', function(done) { - var useCase = useCases[0]; - - var testClient = new TestClient(useCase.mapConfig, 1234); - - testClient.getLayergroup(function(err, layergroupResult) { - assert.ok(!err, err); - - var affected_tables = layergroupResult.metadata.layers[0].meta.affected_tables; - assert.equal(affected_tables.length, 0); - - testClient.drain(done); - }); - }); - - it('should have empty affected tables if it has a node other than "source"', function(done) { - var useCase = useCases[1]; - - var testClient = new TestClient(useCase.mapConfig, 1234); - - testClient.getLayergroup(function(err, layergroupResult) { - assert.ok(!err, err); - - var affected_tables = layergroupResult.metadata.layers[0].meta.affected_tables; - assert.equal(affected_tables[0], 'public.populated_places_simple_reduced'); - - testClient.drain(done); - }); - }); - it('should NOT fail for non-authenticated requests when it is just source', function(done) { var useCase = useCases[0]; diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index 7201b7fb..74d34d82 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -1052,8 +1052,9 @@ describe('template_api', function() { 'Unexpected error for authorized instance: ' + res.statusCode + ' -- ' + res.body); assert.equal(res.headers['content-type'], "application/json; charset=utf-8"); var cc = res.headers['x-cache-channel']; + var expectedCC = 'test_windshaft_cartodb_user_1_db:public.test_table_private_1'; assert.ok(cc); - assert.ok(cc.match, /ciao/, cc); + assert.equal(cc, expectedCC); // hack simulating restart... server.layergroupAffectedTablesCache.cache.reset(); // need to clean channel cache var get_request = { @@ -1072,8 +1073,9 @@ describe('template_api', function() { 'Unexpected error for authorized instance: ' + res.statusCode + ' -- ' + res.body); assert.equal(res.headers['content-type'], "application/json; charset=utf-8"); var cc = res.headers['x-cache-channel']; + var expectedCC = 'test_windshaft_cartodb_user_1_db:public.test_table_private_1'; assert.ok(cc, "Missing X-Cache-Channel on fetch-after-restart"); - assert.ok(cc.match, /ciao/, cc); + assert.equal(cc, expectedCC); return null; }, function deleteTemplate(err) diff --git a/test/acceptance/x_cache_channel.js b/test/acceptance/x_cache_channel.js index cd12e6ce..298c0ef0 100644 --- a/test/acceptance/x_cache_channel.js +++ b/test/acceptance/x_cache_channel.js @@ -28,6 +28,7 @@ describe('get requests x-cache-channel', function() { var mapConfigs = [ { "description": "header should be present", + "x_cache_channel": "test_windshaft_cartodb_user_1_db:public.test_table", "data": { version: '1.4.0', @@ -63,6 +64,9 @@ describe('get requests x-cache-channel', function() { }, { "description": "header should be present and be composed with source table name", + "x_cache_channel": "test_windshaft_cartodb_user_1_db:" + + "public.analysis_2f13a3dbd7_9eb239903a1afd8a69130d1ece0fc8b38de8592d" + + ",public.test_table", "data": { version: '1.5.0', @@ -181,8 +185,9 @@ describe('get requests x-cache-channel', function() { mapConfigs.forEach(function(mapConfigData) { describe(mapConfigData.description, function() { var mapConfig = mapConfigData.data; + var expectedCacheChannel = mapConfigData.x_cache_channel; it('/api/v1/map Map instantiation', function(done) { - var testFn = validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table'); + var testFn = validateXCacheChannel(done, expectedCacheChannel); withLayergroupId(mapConfig, function(err, layergroupId, res) { testFn(res); }); @@ -192,8 +197,8 @@ describe('get requests x-cache-channel', function() { withLayergroupId(mapConfig, function(err, layergroupId) { assert.response( server, - getRequest('/api/v1/map/' + layergroupId + '/0/0/0@2x.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + getRequest('/api/v1/map/' + layergroupId + '/0/0/0@2x.png', true), + validateXCacheChannel(done, expectedCacheChannel) ); }); }); @@ -202,8 +207,8 @@ describe('get requests x-cache-channel', function() { withLayergroupId(mapConfig, function(err, layergroupId) { assert.response( server, - getRequest('/api/v1/map/' + layergroupId + '/0/0/0.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + getRequest('/api/v1/map/' + layergroupId + '/0/0/0.png', true), + validateXCacheChannel(done, expectedCacheChannel) ); }); }); @@ -212,8 +217,8 @@ describe('get requests x-cache-channel', function() { withLayergroupId(mapConfig, function(err, layergroupId) { assert.response( server, - getRequest('/api/v1/map/' + layergroupId + '/0/0/0/0.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + getRequest('/api/v1/map/' + layergroupId + '/0/0/0/0.png', true), + validateXCacheChannel(done, expectedCacheChannel) ); }); }); @@ -222,8 +227,8 @@ describe('get requests x-cache-channel', function() { withLayergroupId(mapConfig, function(err, layergroupId) { assert.response( server, - getRequest('/api/v1/map/' + layergroupId + '/0/attributes/1'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + getRequest('/api/v1/map/' + layergroupId + '/0/attributes/1', true), + validateXCacheChannel(done, expectedCacheChannel) ); }); }); @@ -232,8 +237,8 @@ describe('get requests x-cache-channel', function() { withLayergroupId(mapConfig, function(err, layergroupId) { assert.response( server, - getRequest('/api/v1/map/static/center/' + layergroupId + '/0/0/0/400/300.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + getRequest('/api/v1/map/static/center/' + layergroupId + '/0/0/0/400/300.png', true), + validateXCacheChannel(done, expectedCacheChannel) ); }); }); @@ -242,8 +247,8 @@ describe('get requests x-cache-channel', function() { withLayergroupId(mapConfig, function(err, layergroupId) { assert.response( server, - getRequest('/api/v1/map/static/bbox/' + layergroupId + '/-45,-45,45,45/400/300.png'), - validateXCacheChannel(done, 'test_windshaft_cartodb_user_1_db:public.test_table') + getRequest('/api/v1/map/static/bbox/' + layergroupId + '/-45,-45,45,45/400/300.png', true), + validateXCacheChannel(done, expectedCacheChannel) ); }); }); From 9707881bf90dfaf94cfade2018aa2708a128e0cd Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Mon, 13 Mar 2017 18:36:13 +0100 Subject: [PATCH 4/8] Include check for surrogate-key header and renamed the test file --- lib/cartodb/controllers/layergroup.js | 2 +- lib/cartodb/controllers/map.js | 2 +- .../adapter/analysis-mapconfig-adapter.js | 4 +- .../cache_headers.js} | 76 +++++++++++-------- 4 files changed, 49 insertions(+), 35 deletions(-) rename test/acceptance/{x_cache_channel.js => cache/cache_headers.js} (81%) diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 10b5324b..79268d6e 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -392,7 +392,7 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg assert.ifError(err); var queries = []; - mapConfig.getLayers().map(function(layer) { + mapConfig.getLayers().forEach(function(layer) { queries.push(layer.options.sql); if (layer.options.affected_tables) { layer.options.affected_tables.map(function(table) { diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 3b06ac02..b3ca2b4a 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -307,7 +307,7 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la }); var sql = []; - mapconfig.getLayers().map(function(layer) { + mapconfig.getLayers().forEach(function(layer) { sql.push(layer.options.sql); if (layer.options.affected_tables) { layer.options.affected_tables.map(function(table) { diff --git a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js index c6467488..8cb63f48 100644 --- a/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js +++ b/lib/cartodb/models/mapconfig/adapter/analysis-mapconfig-adapter.js @@ -334,7 +334,9 @@ function AnalysisError(message) { function getAllAffectedTablesFromSourceNodes(node) { var affectedTables = node.getAllInputNodes(function (node) { return node.getType() === 'source'; - }).reduce(function(list, node) { return list.concat(node.getAffectedTables()); },[]); + }).reduce(function(list, node) { + return list.concat(node.getAffectedTables()); + },[]); return affectedTables; } diff --git a/test/acceptance/x_cache_channel.js b/test/acceptance/cache/cache_headers.js similarity index 81% rename from test/acceptance/x_cache_channel.js rename to test/acceptance/cache/cache_headers.js index 298c0ef0..13ca77dc 100644 --- a/test/acceptance/x_cache_channel.js +++ b/test/acceptance/cache/cache_headers.js @@ -1,16 +1,16 @@ -var testHelper = require('../support/test_helper'); +var testHelper = require('../../support/test_helper'); -var assert = require('../support/assert'); +var assert = require('../../support/assert'); var qs = require('querystring'); -var CartodbWindshaft = require('../../lib/cartodb/server'); -var serverOptions = require('../../lib/cartodb/server_options'); +var CartodbWindshaft = require('../../../lib/cartodb/server'); +var serverOptions = require('../../../lib/cartodb/server_options'); var server = new CartodbWindshaft(serverOptions); server.setMaxListeners(0); -var LayergroupToken = require('../support/layergroup-token'); +var LayergroupToken = require('../../support/layergroup-token'); -describe('get requests x-cache-channel', function() { +describe('get requests with cache headers', function() { var keysToDelete; beforeEach(function() { @@ -27,11 +27,14 @@ describe('get requests x-cache-channel', function() { var mapConfigs = [ { - "description": "header should be present", - "x_cache_channel": "test_windshaft_cartodb_user_1_db:public.test_table", + "description": "cache headers should be present", + "cache_headers": { + "x_cache_channel": "test_windshaft_cartodb_user_1_db:public.test_table", + "surrogate_keys": "t:77pJnX" + }, "data": { - version: '1.4.0', + version: '1.5.0', layers: [ { options: { @@ -63,10 +66,13 @@ describe('get requests x-cache-channel', function() { }, }, { - "description": "header should be present and be composed with source table name", - "x_cache_channel": "test_windshaft_cartodb_user_1_db:" + - "public.analysis_2f13a3dbd7_9eb239903a1afd8a69130d1ece0fc8b38de8592d" + - ",public.test_table", + "description": "cache headers should be present and be composed with source table name", + "cache_headers": { + "x_cache_channel": "test_windshaft_cartodb_user_1_db:" + + "public.analysis_2f13a3dbd7_9eb239903a1afd8a69130d1ece0fc8b38de8592d" + + ",public.test_table", + "surrogate_keys": "t:77pJnX t:iL4eth" + }, "data": { version: '1.5.0', @@ -136,22 +142,24 @@ describe('get requests x-cache-channel', function() { }; } - function validateXCacheChannel(done, expectedCacheChannel) { + function validateCacheHeaders(done, expectedCacheHeaders) { return function(res, err) { if (err) { return done(err); } assert.ok(res.headers['x-cache-channel']); - if (expectedCacheChannel) { - assert.equal(res.headers['x-cache-channel'], expectedCacheChannel); + assert.ok(res.headers['surrogate-key']); + if (expectedCacheHeaders) { + assert.equal(res.headers['x-cache-channel'], expectedCacheHeaders.x_cache_channel); + assert.equal(res.headers['surrogate-key'], expectedCacheHeaders.surrogate_keys); } done(); }; } - function noXCacheChannelHeader(done) { + function noCacheHeaders(done) { return function(res, err) { if (err) { return done(err); @@ -161,6 +169,10 @@ describe('get requests x-cache-channel', function() { !res.headers['x-cache-channel'], 'did not expect x-cache-channel header, got: `' + res.headers['x-cache-channel'] + '`' ); + assert.ok( + !res.headers['surrogate-key'], + 'did not expect surrogate-key header, got: `' + res.headers['surrogate-key'] + '`' + ); done(); }; } @@ -185,9 +197,9 @@ describe('get requests x-cache-channel', function() { mapConfigs.forEach(function(mapConfigData) { describe(mapConfigData.description, function() { var mapConfig = mapConfigData.data; - var expectedCacheChannel = mapConfigData.x_cache_channel; + var expectedCacheHeaders = mapConfigData.cache_headers; it('/api/v1/map Map instantiation', function(done) { - var testFn = validateXCacheChannel(done, expectedCacheChannel); + var testFn = validateCacheHeaders(done, expectedCacheHeaders); withLayergroupId(mapConfig, function(err, layergroupId, res) { testFn(res); }); @@ -198,7 +210,7 @@ describe('get requests x-cache-channel', function() { assert.response( server, getRequest('/api/v1/map/' + layergroupId + '/0/0/0@2x.png', true), - validateXCacheChannel(done, expectedCacheChannel) + validateCacheHeaders(done, expectedCacheHeaders) ); }); }); @@ -208,7 +220,7 @@ describe('get requests x-cache-channel', function() { assert.response( server, getRequest('/api/v1/map/' + layergroupId + '/0/0/0.png', true), - validateXCacheChannel(done, expectedCacheChannel) + validateCacheHeaders(done, expectedCacheHeaders) ); }); }); @@ -218,7 +230,7 @@ describe('get requests x-cache-channel', function() { assert.response( server, getRequest('/api/v1/map/' + layergroupId + '/0/0/0/0.png', true), - validateXCacheChannel(done, expectedCacheChannel) + validateCacheHeaders(done, expectedCacheHeaders) ); }); }); @@ -228,7 +240,7 @@ describe('get requests x-cache-channel', function() { assert.response( server, getRequest('/api/v1/map/' + layergroupId + '/0/attributes/1', true), - validateXCacheChannel(done, expectedCacheChannel) + validateCacheHeaders(done, expectedCacheHeaders) ); }); }); @@ -238,7 +250,7 @@ describe('get requests x-cache-channel', function() { assert.response( server, getRequest('/api/v1/map/static/center/' + layergroupId + '/0/0/0/400/300.png', true), - validateXCacheChannel(done, expectedCacheChannel) + validateCacheHeaders(done, expectedCacheHeaders) ); }); }); @@ -248,21 +260,21 @@ describe('get requests x-cache-channel', function() { assert.response( server, getRequest('/api/v1/map/static/bbox/' + layergroupId + '/-45,-45,45,45/400/300.png', true), - validateXCacheChannel(done, expectedCacheChannel) + validateCacheHeaders(done, expectedCacheHeaders) ); }); }); }); }); - describe('header should NOT be present', function() { + describe('cache headers should NOT be present', function() { it('/', function(done) { assert.response( server, getRequest('/'), statusOkResponse, - noXCacheChannelHeader(done) + noCacheHeaders(done) ); }); @@ -271,7 +283,7 @@ describe('get requests x-cache-channel', function() { server, getRequest('/version'), statusOkResponse, - noXCacheChannelHeader(done) + noCacheHeaders(done) ); }); @@ -280,7 +292,7 @@ describe('get requests x-cache-channel', function() { server, getRequest('/health'), statusOkResponse, - noXCacheChannelHeader(done) + noCacheHeaders(done) ); }); @@ -289,7 +301,7 @@ describe('get requests x-cache-channel', function() { server, getRequest('/api/v1/map/named', true), statusOkResponse, - noXCacheChannelHeader(done) + noCacheHeaders(done) ); }); @@ -352,7 +364,7 @@ describe('get requests x-cache-channel', function() { server, getRequest('/api/v1/map/named/' + templateName, true), statusOkResponse, - noXCacheChannelHeader(done) + noCacheHeaders(done) ); }); @@ -361,7 +373,7 @@ describe('get requests x-cache-channel', function() { server, getRequest('/api/v1/map/named/' + templateName, true, 'cb'), statusOkResponse, - noXCacheChannelHeader(done) + noCacheHeaders(done) ); }); }); From 4132bc755da715490538336bf72c0aad82a153ec Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Tue, 14 Mar 2017 14:06:50 +0100 Subject: [PATCH 5/8] Add cdb_invalidate_varnish function fixture to tests --- test/support/prepare_db.sh | 2 +- test/support/sql/cdb_invalidate_varnish.sql | 6 ++++++ 2 files changed, 7 insertions(+), 1 deletion(-) create mode 100644 test/support/sql/cdb_invalidate_varnish.sql diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 1a6d929c..4fbf4b7a 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -75,7 +75,7 @@ if test x"$PREPARE_PGSQL" = xyes; then dropdb "${TEST_DB}" createdb -Ttemplate_postgis -EUTF8 "${TEST_DB}" || die "Could not create test database" - LOCAL_SQL_SCRIPTS='analysis_catalog windshaft.test gadm4 ported/populated_places_simple_reduced cdb_analysis_check' + LOCAL_SQL_SCRIPTS='analysis_catalog windshaft.test gadm4 ported/populated_places_simple_reduced cdb_analysis_check cdb_invalidate_varnish' REMOTE_SQL_SCRIPTS='CDB_QueryStatements CDB_QueryTables CDB_CartodbfyTable CDB_TableMetadata CDB_ForeignTable CDB_UserTables CDB_ColumnNames CDB_ZoomFromScale CDB_OverviewsSupport CDB_Overviews CDB_QuantileBins CDB_JenksBins CDB_HeadsTailsBins CDB_EqualIntervalBins CDB_Hexagon CDB_XYZ' CURL_ARGS="" diff --git a/test/support/sql/cdb_invalidate_varnish.sql b/test/support/sql/cdb_invalidate_varnish.sql new file mode 100644 index 00000000..7cd2d8f1 --- /dev/null +++ b/test/support/sql/cdb_invalidate_varnish.sql @@ -0,0 +1,6 @@ +CREATE OR REPLACE FUNCTION CDB_Invalidate_Varnish(table_name TEXT) +RETURNS void AS +$$ +BEGIN +END; +$$ LANGUAGE PLPGSQL; \ No newline at end of file From 125c39967c30234f1ead15d2e33c5f82532e315a Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Tue, 14 Mar 2017 14:32:36 +0100 Subject: [PATCH 6/8] Make the cache headers tests idempotent --- test/acceptance/cache/cache_headers.js | 22 +++++++++++++++++----- 1 file changed, 17 insertions(+), 5 deletions(-) diff --git a/test/acceptance/cache/cache_headers.js b/test/acceptance/cache/cache_headers.js index 13ca77dc..2cd916af 100644 --- a/test/acceptance/cache/cache_headers.js +++ b/test/acceptance/cache/cache_headers.js @@ -29,7 +29,10 @@ describe('get requests with cache headers', function() { { "description": "cache headers should be present", "cache_headers": { - "x_cache_channel": "test_windshaft_cartodb_user_1_db:public.test_table", + "x_cache_channel": { + "db_name": "test_windshaft_cartodb_user_1_db", + "tables": ["public.test_table"] + }, "surrogate_keys": "t:77pJnX" }, "data": @@ -68,9 +71,11 @@ describe('get requests with cache headers', function() { { "description": "cache headers should be present and be composed with source table name", "cache_headers": { - "x_cache_channel": "test_windshaft_cartodb_user_1_db:" + - "public.analysis_2f13a3dbd7_9eb239903a1afd8a69130d1ece0fc8b38de8592d" + - ",public.test_table", + "x_cache_channel": { + "db_name": "test_windshaft_cartodb_user_1_db", + "tables": ["public.analysis_2f13a3dbd7_9eb239903a1afd8a69130d1ece0fc8b38de8592d", + "public.test_table"] + }, "surrogate_keys": "t:77pJnX t:iL4eth" }, "data": @@ -151,7 +156,7 @@ describe('get requests with cache headers', function() { assert.ok(res.headers['x-cache-channel']); assert.ok(res.headers['surrogate-key']); if (expectedCacheHeaders) { - assert.equal(res.headers['x-cache-channel'], expectedCacheHeaders.x_cache_channel); + validateXChannelHeaders(res.headers, expectedCacheHeaders); assert.equal(res.headers['surrogate-key'], expectedCacheHeaders.surrogate_keys); } @@ -159,6 +164,13 @@ describe('get requests with cache headers', function() { }; } + function validateXChannelHeaders(headers, expectedCacheHeaders) { + var dbName = headers['x-cache-channel'].split(':')[0]; + var tables = headers['x-cache-channel'].split(':')[1].split(',').sort(); + assert.equal(dbName, expectedCacheHeaders.x_cache_channel.db_name); + assert.deepEqual(tables, expectedCacheHeaders.x_cache_channel.tables.sort()); + } + function noCacheHeaders(done) { return function(res, err) { if (err) { From e8a0f6b7b672ae3947b2e5f7d6f979492dd0c2e0 Mon Sep 17 00:00:00 2001 From: Mario de Frutos Date: Tue, 14 Mar 2017 12:57:46 +0100 Subject: [PATCH 7/8] Point to camshaft branch to test properly --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index 6c665702..53c38c3f 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ ], "dependencies": { "body-parser": "~1.14.0", - "camshaft": "0.52.0", + "camshaft": "cartodb/camshaft#add_affected_tables", "cartodb-psql": "~0.7.1", "cartodb-query-tables": "0.2.0", "cartodb-redis": "0.13.2", From 92ec17218ba9713000cfa64c762a823d988511ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20Garc=C3=ADa=20Aubert?= Date: Mon, 10 Apr 2017 11:36:17 +0200 Subject: [PATCH 8/8] Upgrade camshaft to 0.53.0 --- package.json | 2 +- yarn.lock | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/package.json b/package.json index 53c38c3f..be83898e 100644 --- a/package.json +++ b/package.json @@ -20,7 +20,7 @@ ], "dependencies": { "body-parser": "~1.14.0", - "camshaft": "cartodb/camshaft#add_affected_tables", + "camshaft": "0.53.0", "cartodb-psql": "~0.7.1", "cartodb-query-tables": "0.2.0", "cartodb-redis": "0.13.2", diff --git a/yarn.lock b/yarn.lock index 8c739b00..92c3c673 100644 --- a/yarn.lock +++ b/yarn.lock @@ -194,9 +194,9 @@ camelcase@^3.0.0: version "3.0.0" resolved "https://registry.yarnpkg.com/camelcase/-/camelcase-3.0.0.tgz#32fc4b9fcdaf845fcdf7e73bb97cac2261f0ab0a" -camshaft@0.52.0: - version "0.52.0" - resolved "https://registry.yarnpkg.com/camshaft/-/camshaft-0.52.0.tgz#d7cac00884783c059fcaaa791879429b80c0e7b8" +camshaft@0.53.0: + version "0.53.0" + resolved "https://registry.yarnpkg.com/camshaft/-/camshaft-0.53.0.tgz#89708ca89bea93c4d94b0587b692935e23f1e9d6" dependencies: async "^1.5.2" bunyan "1.8.1"