From 95ab99be4dab164b196bac252c555c550c0cd935 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Tue, 9 Feb 2016 19:06:34 +0100 Subject: [PATCH 1/7] Use new CDB_QueryTablesUpdatedAt function --- lib/cartodb/api/query_tables_api.js | 26 +++++++------------ lib/cartodb/api/tables_extent_api.js | 10 +++---- .../cache/model/database_tables_entry.js | 14 +++++----- lib/cartodb/controllers/layergroup.js | 19 ++++++++------ lib/cartodb/controllers/map.js | 26 +++++++++---------- lib/cartodb/controllers/named_maps.js | 3 +-- test/acceptance/multilayer.js | 6 ++--- 7 files changed, 49 insertions(+), 55 deletions(-) diff --git a/lib/cartodb/api/query_tables_api.js b/lib/cartodb/api/query_tables_api.js index af2fc978..c32ace7f 100644 --- a/lib/cartodb/api/query_tables_api.js +++ b/lib/cartodb/api/query_tables_api.js @@ -29,32 +29,26 @@ QueryTablesApi.prototype.getAffectedTablesInQuery = function (username, sql, cal }; QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (username, sql, callback) { - var query = [ - 'WITH querytables AS (', - 'SELECT * FROM CDB_QueryTablesText($windshaft$' + prepareSql(sql) + '$windshaft$) as tablenames', - ')', - 'SELECT (SELECT tablenames FROM querytables), EXTRACT(EPOCH FROM max(updated_at)) as max', - 'FROM CDB_TableMetadata m', - 'WHERE m.tabname = any ((SELECT tablenames from querytables)::regclass[])' - ].join(' '); + var query = + 'SELECT * FROM CDB_QueryTablesUpdatedAt($windshaft$' + prepareSql(sql) + '$windshaft$)'; this.pgQueryRunner.run(username, query, function handleAffectedTablesAndLastUpdatedTimeRows (err, rows) { - if (err || rows.length === 0) { + if (err) { var msg = err.message ? err.message : err; callback(new Error('could not fetch affected tables or last updated time: ' + msg)); return; } - var result = rows[0]; + var affectedTables = rows; - // This is an Array, so no need to split into parts - var tableNames = result.tablenames; - - var lastUpdatedTime = result.max || 0; + var updatedTimes = affectedTables.map(function getUpdateDate(table) { + return table.updated_at; + }); + var lastUpdatedTime = (affectedTables.length === 0 ? 0 : Math.max.apply(null, updatedTimes)) || 0; callback(null, { - affectedTables: tableNames, - lastUpdatedTime: lastUpdatedTime * 1000 + affectedTables: affectedTables, + lastUpdatedTime: lastUpdatedTime }); }); }; diff --git a/lib/cartodb/api/tables_extent_api.js b/lib/cartodb/api/tables_extent_api.js index d4293ed7..7b534a3f 100644 --- a/lib/cartodb/api/tables_extent_api.js +++ b/lib/cartodb/api/tables_extent_api.js @@ -13,13 +13,9 @@ module.exports = TablesExtentApi; * `table_name` format as valid input * @param {Function} callback function(err, result) {Object} result with `west`, `south`, `east`, `north` */ -TablesExtentApi.prototype.getBounds = function (username, tableNames, callback) { - var estimatedExtentSQLs = tableNames.map(function(tableName) { - var schemaTable = tableName.split('.'); - if (schemaTable.length > 1) { - return "ST_EstimatedExtent('" + schemaTable[0] + "', '" + schemaTable[1] + "', 'the_geom_webmercator')"; - } - return "ST_EstimatedExtent('" + schemaTable[0] + "', 'the_geom_webmercator')"; +TablesExtentApi.prototype.getBounds = function (username, tables, callback) { + var estimatedExtentSQLs = tables.map(function(table) { + return "ST_EstimatedExtent('" + table.schema_name + "', '" + table.table_name + "', 'the_geom_webmercator')"; }); var query = [ diff --git a/lib/cartodb/cache/model/database_tables_entry.js b/lib/cartodb/cache/model/database_tables_entry.js index 4d269137..89bf88ab 100644 --- a/lib/cartodb/cache/model/database_tables_entry.js +++ b/lib/cartodb/cache/model/database_tables_entry.js @@ -1,22 +1,24 @@ var crypto = require('crypto'); -function DatabaseTables(dbName, tableNames) { +function DatabaseTables(tables) { this.namespace = 't'; - this.dbName = dbName; - this.tableNames = tableNames; + this.tables = tables; } module.exports = DatabaseTables; DatabaseTables.prototype.key = function() { - return this.tableNames.map(function(tableName) { - return this.namespace + ':' + shortHashKey(this.dbName + ':' + tableName); + return this.tables.map(function(table) { + return this.namespace + ':' + shortHashKey(table.db_name + ':' + table.table_name + '.' + table.schema_name); }.bind(this)); }; DatabaseTables.prototype.getCacheChannel = function() { - return this.dbName + ':' + this.tableNames.join(','); + var key = this.tables.map(function(table) { + return table.db_name + ':' + table.schema_name + "." + table.table_name; + }).join(";;"); + return key; }; function shortHashKey(target) { diff --git a/lib/cartodb/controllers/layergroup.js b/lib/cartodb/controllers/layergroup.js index 0a7e7698..e647a046 100644 --- a/lib/cartodb/controllers/layergroup.js +++ b/lib/cartodb/controllers/layergroup.js @@ -320,7 +320,7 @@ LayergroupController.prototype.sendResponse = function(req, res, body, status, h global.logger.warn('ERROR generating cache channel: ' + err); } if (!!affectedTables) { - var tablesCacheEntry = new TablesCacheEntry(dbName, affectedTables); + var tablesCacheEntry = new TablesCacheEntry(affectedTables); res.set('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); self.surrogateKeysCache.tag(res, tablesCacheEntry); } @@ -366,17 +366,20 @@ LayergroupController.prototype.getAffectedTables = function(user, dbName, layerg throw new Error("this request doesn't need an X-Cache-Channel generated"); } - self.queryTablesApi.getAffectedTablesInQuery(user, sql, this); // in addCacheChannel + self.queryTablesApi.getAffectedTablesAndLastUpdatedTime(user, sql, this); // in addCacheChannel }, - function buildCacheChannel(err, tableNames) { + function buildCacheChannel(err, tables) { assert.ifError(err); + self.layergroupAffectedTables.set(dbName, layergroupId, tables.affectedTables); - self.layergroupAffectedTables.set(dbName, layergroupId, tableNames); - - return tableNames; + return tables; }, - function finish(err, affectedTables) { - callback(err, affectedTables); + function finish(err, tables) { + if(tables === undefined){ + callback(err); + }else{ + callback(err, tables.affectedTables); + } } ); }; diff --git a/lib/cartodb/controllers/map.js b/lib/cartodb/controllers/map.js index 9fad27f6..dad77dbd 100644 --- a/lib/cartodb/controllers/map.js +++ b/lib/cartodb/controllers/map.js @@ -280,20 +280,20 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la function checkCachedAffectedTables() { return self.layergroupAffectedTables.hasAffectedTables(dbName, layergroupId); }, - function getAffectedTablesAndLastUpdatedTime(err, hasCache) { + function getAffectedTablesAndLastUpdatedTime(err) { assert.ifError(err); - if (hasCache) { - var next = this; - var affectedTables = self.layergroupAffectedTables.get(dbName, layergroupId); - self.queryTablesApi.getLastUpdatedTime(username, affectedTables, function(err, lastUpdatedTime) { - if (err) { - return next(err); - } - return next(null, { affectedTables: affectedTables, lastUpdatedTime: lastUpdatedTime }); - }); - } else { + // if (hasCache) { + // var next = this; + // var affectedTables = self.layergroupAffectedTables.get(dbName, layergroupId); + // self.queryTablesApi.getLastUpdatedTime(username, affectedTables, function(err, lastUpdatedTime) { + // if (err) { + // return next(err); + // } + // return next(null, { affectedTables: affectedTables, lastUpdatedTime: lastUpdatedTime }); + // }); + // } else { self.queryTablesApi.getAffectedTablesAndLastUpdatedTime(username, sql, this); - } + //} }, function handleAffectedTablesAndLastUpdatedTime(err, result) { if (req.profiler) { @@ -310,7 +310,7 @@ MapController.prototype.afterLayergroupCreate = function(req, res, mapconfig, la addWidgetsUrl(username, layergroup); if (req.method === 'GET') { - var tableCacheEntry = new TablesCacheEntry(dbName, result.affectedTables); + var tableCacheEntry = new TablesCacheEntry(result.affectedTables); var ttl = global.environment.varnish.layergroupTtl || 86400; res.set('Cache-Control', 'public,max-age='+ttl+',must-revalidate'); res.set('Last-Modified', (new Date()).toUTCString()); diff --git a/lib/cartodb/controllers/named_maps.js b/lib/cartodb/controllers/named_maps.js index 004282c9..3687dc39 100644 --- a/lib/cartodb/controllers/named_maps.js +++ b/lib/cartodb/controllers/named_maps.js @@ -44,7 +44,6 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header var self = this; - var dbName = req.params.dbname; step( function getAffectedTablesAndLastUpdatedTime() { namedMapProvider.getAffectedTablesAndLastUpdatedTime(this); @@ -66,7 +65,7 @@ NamedMapsController.prototype.sendResponse = function(req, res, resource, header } res.set('Last-Modified', lastModifiedDate.toUTCString()); - var tablesCacheEntry = new TablesCacheEntry(dbName, result.affectedTables); + var tablesCacheEntry = new TablesCacheEntry(result.affectedTables); res.set('X-Cache-Channel', tablesCacheEntry.getCacheChannel()); if (result.affectedTables.length > 0) { self.surrogateKeysCache.tag(res, tablesCacheEntry); diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 94df8bdc..d4eac5ef 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -262,9 +262,9 @@ describe(suiteName, function() { var parsedBody = JSON.parse(res.body); expected_token = parsedBody.layergroupid.split(':')[0]; helper.checkCache(res); - helper.checkSurrogateKey(res, new TablesCacheEntry('test_windshaft_cartodb_user_1_db', [ - 'public.test_table', - 'public.test_table_2' + helper.checkSurrogateKey(res, new TablesCacheEntry([ + {db_name: "test_windshaft_cartodb_user_1_db", table_name: "test_table", schema_name: "public"}, + {db_name: "test_windshaft_cartodb_user_1_db", table_name: "test_table_2", schema_name: "public"}, ]).key().join(' ')); From b7ff554209dc7a0fc90f8f0b511c21899ed573b9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Thu, 11 Feb 2016 11:45:09 +0100 Subject: [PATCH 2/7] Use new _Updated_At function and new names --- lib/cartodb/api/query_tables_api.js | 2 +- lib/cartodb/cache/model/database_tables_entry.js | 4 ++-- test/acceptance/multilayer.js | 4 ++-- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lib/cartodb/api/query_tables_api.js b/lib/cartodb/api/query_tables_api.js index c32ace7f..a94d6411 100644 --- a/lib/cartodb/api/query_tables_api.js +++ b/lib/cartodb/api/query_tables_api.js @@ -30,7 +30,7 @@ QueryTablesApi.prototype.getAffectedTablesInQuery = function (username, sql, cal QueryTablesApi.prototype.getAffectedTablesAndLastUpdatedTime = function (username, sql, callback) { var query = - 'SELECT * FROM CDB_QueryTablesUpdatedAt($windshaft$' + prepareSql(sql) + '$windshaft$)'; + 'SELECT * FROM CDB_QueryTables_Updated_At($windshaft$' + prepareSql(sql) + '$windshaft$)'; this.pgQueryRunner.run(username, query, function handleAffectedTablesAndLastUpdatedTimeRows (err, rows) { if (err) { diff --git a/lib/cartodb/cache/model/database_tables_entry.js b/lib/cartodb/cache/model/database_tables_entry.js index 89bf88ab..60629714 100644 --- a/lib/cartodb/cache/model/database_tables_entry.js +++ b/lib/cartodb/cache/model/database_tables_entry.js @@ -10,13 +10,13 @@ module.exports = DatabaseTables; DatabaseTables.prototype.key = function() { return this.tables.map(function(table) { - return this.namespace + ':' + shortHashKey(table.db_name + ':' + table.table_name + '.' + table.schema_name); + return this.namespace + ':' + shortHashKey(table.dbname + ':' + table.table_name + '.' + table.schema_name); }.bind(this)); }; DatabaseTables.prototype.getCacheChannel = function() { var key = this.tables.map(function(table) { - return table.db_name + ':' + table.schema_name + "." + table.table_name; + return table.dbname + ':' + table.schema_name + "." + table.table_name; }).join(";;"); return key; }; diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index d4eac5ef..2611bb7f 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -263,8 +263,8 @@ describe(suiteName, function() { expected_token = parsedBody.layergroupid.split(':')[0]; helper.checkCache(res); helper.checkSurrogateKey(res, new TablesCacheEntry([ - {db_name: "test_windshaft_cartodb_user_1_db", table_name: "test_table", schema_name: "public"}, - {db_name: "test_windshaft_cartodb_user_1_db", table_name: "test_table_2", schema_name: "public"}, + {dbname: "test_windshaft_cartodb_user_1_db", table_name: "test_table", schema_name: "public"}, + {dbname: "test_windshaft_cartodb_user_1_db", table_name: "test_table_2", schema_name: "public"}, ]).key().join(' ')); From a6562850011e73bc14d6bd5b6434dfc1e3b812ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Fri, 12 Feb 2016 17:25:57 +0100 Subject: [PATCH 3/7] Run tests against master cartodb-postgresql --- test/support/prepare_db.sh | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/test/support/prepare_db.sh b/test/support/prepare_db.sh index 4f001228..8ade522f 100755 --- a/test/support/prepare_db.sh +++ b/test/support/prepare_db.sh @@ -79,10 +79,19 @@ if test x"$PREPARE_PGSQL" = xyes; then psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 psql -c "CREATE LANGUAGE plpythonu;" ${TEST_DB} - curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/cdb/scripts-available/CDB_QueryStatements.sql -o sql/CDB_QueryStatements.sql - curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/cdb/scripts-available/CDB_QueryTables.sql -o sql/CDB_QueryTables.sql - cat sql/CDB_QueryStatements.sql sql/CDB_QueryTables.sql | + curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/master/scripts-available/CDB_QueryStatements.sql -o sql/CDB_QueryStatements.sql + curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/master/scripts-available/CDB_QueryTables.sql -o sql/CDB_QueryTables.sql + curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/master/scripts-available/CDB_TableMetadata.sql -o sql/CDB_TableMetadata.sql + curl -L -s https://github.com/CartoDB/cartodb-postgresql/raw/master/scripts-available/CDB_ForeignTable.sql -o sql/CDB_ForeignTable.sql + cat sql/CDB_QueryStatements.sql | + psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 + cat sql/CDB_QueryTables.sql | + psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 + cat sql/CDB_TableMetadata.sql| psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 + cat sql/CDB_ForeignTable.sql|sed -e 's/cartodb\./public./g' \ + -e "s/''cartodb''/''public''/g" | + psql -v ON_ERROR_STOP=1 ${TEST_DB} || exit 1 fi From 33ba629c6d05f651bf3b3d968bcbcf96293daf66 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Mon, 15 Feb 2016 13:00:21 +0100 Subject: [PATCH 4/7] Add publicuser creation to Travis config --- .travis.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.travis.yml b/.travis.yml index 8492e564..db5cd52f 100644 --- a/.travis.yml +++ b/.travis.yml @@ -13,6 +13,7 @@ addons: before_install: - npm install -g npm@2 - createdb template_postgis + - createuser publicuser - psql -c "CREATE EXTENSION postgis" template_postgis env: From 119846b56b188b4d04f89330712d3040215ec5f6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Mon, 15 Feb 2016 16:04:13 +0100 Subject: [PATCH 5/7] Fix specs --- test/acceptance/templates.js | 4 ++-- test/integration/query-tables-api.js | 5 ++--- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index d4eb304b..1db9cd90 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -1393,7 +1393,7 @@ describe('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 helper.checkCache(res); var expectedSurrogateKey = [ - new TablesCacheEntry('test_windshaft_cartodb_user_1_db', ['public.test_table_private_1']).key(), + new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', table_name: 'test_table_private_1'}]).key(), new NamedMapsCacheEntry('localhost', template_acceptance_open.name).key() ].join(' '); helper.checkSurrogateKey(res, expectedSurrogateKey); @@ -1476,7 +1476,7 @@ describe('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 helper.checkCache(res); var expectedSurrogateKey = [ - new TablesCacheEntry('test_windshaft_cartodb_user_1_db', ['public.test_table_private_1']).key(), + new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', table_name: 'test_table_private_1'}]).key(), new NamedMapsCacheEntry('localhost', template_acceptance_open.name).key() ].join(' '); helper.checkSurrogateKey(res, expectedSurrogateKey); diff --git a/test/integration/query-tables-api.js b/test/integration/query-tables-api.js index d05b1ed0..c4996f0b 100644 --- a/test/integration/query-tables-api.js +++ b/test/integration/query-tables-api.js @@ -28,9 +28,8 @@ describe('QueryTablesApi', function() { var query = 'select * from test_table'; queryTablesApi.getAffectedTablesAndLastUpdatedTime('localhost', query, function(err, result) { assert.ok(!err, err); - assert.deepEqual(result, { - affectedTables: [ 'public.test_table' ], + affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", "table_name": 'test_table', updated_at: new Date(1234567890123)}], lastUpdatedTime: 1234567890123 }); @@ -44,7 +43,7 @@ describe('QueryTablesApi', function() { assert.ok(!err, err); assert.deepEqual(result, { - affectedTables: [ 'public.test_table_private_1' ], + affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", "table_name": 'test_table_private_1', updated_at: new Date(1234567890123)}], lastUpdatedTime: 1234567890123 }); From 0e83420e24a333b3ae71846104c16017a3e924c6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Mon, 15 Feb 2016 16:15:43 +0100 Subject: [PATCH 6/7] Fix long line --- test/integration/query-tables-api.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/integration/query-tables-api.js b/test/integration/query-tables-api.js index c4996f0b..93ca0a07 100644 --- a/test/integration/query-tables-api.js +++ b/test/integration/query-tables-api.js @@ -29,7 +29,8 @@ describe('QueryTablesApi', function() { queryTablesApi.getAffectedTablesAndLastUpdatedTime('localhost', query, function(err, result) { assert.ok(!err, err); assert.deepEqual(result, { - affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", "table_name": 'test_table', updated_at: new Date(1234567890123)}], + affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", + "table_name": 'test_table', updated_at: new Date(1234567890123)}], lastUpdatedTime: 1234567890123 }); @@ -43,7 +44,8 @@ describe('QueryTablesApi', function() { assert.ok(!err, err); assert.deepEqual(result, { - affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", "table_name": 'test_table_private_1', updated_at: new Date(1234567890123)}], + affectedTables: [{dbname: "test_windshaft_cartodb_user_1_db", schema_name: "public", + "table_name": 'test_table_private_1', updated_at: new Date(1234567890123)}], lastUpdatedTime: 1234567890123 }); From 19596245b801f40e404d24612579bdf5e0739106 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Alejandro=20Mart=C3=ADnez?= Date: Mon, 15 Feb 2016 16:21:13 +0100 Subject: [PATCH 7/7] Fix long line --- test/acceptance/templates.js | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index 1db9cd90..137f5ce5 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -1393,7 +1393,8 @@ describe('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 helper.checkCache(res); var expectedSurrogateKey = [ - new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', table_name: 'test_table_private_1'}]).key(), + new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', + table_name: 'test_table_private_1'}]).key(), new NamedMapsCacheEntry('localhost', template_acceptance_open.name).key() ].join(' '); helper.checkSurrogateKey(res, expectedSurrogateKey); @@ -1476,7 +1477,8 @@ describe('template_api', function() { // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 helper.checkCache(res); var expectedSurrogateKey = [ - new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', table_name: 'test_table_private_1'}]).key(), + new TablesCacheEntry([{dbname: 'test_windshaft_cartodb_user_1_db', schema_name: 'public', + table_name: 'test_table_private_1'}]).key(), new NamedMapsCacheEntry('localhost', template_acceptance_open.name).key() ].join(' '); helper.checkSurrogateKey(res, expectedSurrogateKey);