diff --git a/config/environments/test.js.example b/config/environments/test.js.example index b97b9a6d..ff636581 100644 --- a/config/environments/test.js.example +++ b/config/environments/test.js.example @@ -48,8 +48,10 @@ var config = { } ,sqlapi: { protocol: 'http', - host: 'localhost.lan', - port: 8080, + host: '', + // This port will be used by "make check" for testing purposes + // It must be available + port: 1080, version: 'v1' } ,varnish: { diff --git a/lib/cartodb/cache_validator.js b/lib/cartodb/cache_validator.js index 479568f6..aa551df6 100644 --- a/lib/cartodb/cache_validator.js +++ b/lib/cartodb/cache_validator.js @@ -36,7 +36,7 @@ function generateCacheChannel(req, callback){ // use cache if present if (!_.isNull(channelCache[sql_md5]) && !_.isUndefined(channelCache[sql_md5])) { - callback(channelCache[sql_md5]); + callback(null, channelCache[sql_md5]); } else{ // strip out windshaft/mapnik inserted sql if present var sql = req.params.sql.match(/^\((.*)\)\sas\scdbq$/); @@ -54,21 +54,28 @@ function generateCacheChannel(req, callback){ } // call sql api - request.get({url:sqlapi, qs:qs, json:true}, function(err, response, body){ - if (!err && response.statusCode == 200) { - tableNames = body.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; - } else { - //oops, no SQL API. Just cache using fallback 'table' key - tableNames = 'table'; + request.get({url:sqlapi, qs:qs, json:true}, function(err, res, body){ + var epref = 'could not detect source tables using SQL api at ' + sqlapi; + if (err){ + var msg = err.message ? err.message : err; + callback(new Error(epref + ': ' + msg)); + return; } + if (res.statusCode != 200) { + var msg = res.body.error ? res.body.error : res.body; + callback(new Error(epref + ': ' + msg)); + return; + } + var qtables = body.rows[0].cdb_querytables; + tableNames = qtables.split(/^\{(.*)\}$/)[1]; cacheChannel = buildCacheChannel(dbName,tableNames); channelCache[sql_md5] = cacheChannel; // store for caching - callback(cacheChannel); + callback(null, cacheChannel); }); } } else { cacheChannel = buildCacheChannel(dbName,tableNames); - callback(cacheChannel); + callback(null, cacheChannel); } } diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index ac9c458f..ac1bc3bf 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -57,17 +57,27 @@ module.exports = function(){ // skip non-GET requests, or requests for which there's no response if ( req.method != 'GET' || ! req.res ) { cb(null, null); return; } var res = req.res; - var ttl = global.environment.varnish.ttl || 86400; - Cache.generateCacheChannel(req, function(channel){ - res.header('X-Cache-Channel', channel); - var cache_policy = req.query.cache_policy; - if ( cache_policy == 'persist' ) { - res.header('Cache-Control', 'public,max-age=31536000'); // 1 year + var cache_policy = req.query.cache_policy; + if ( cache_policy == 'persist' ) { + res.header('Cache-Control', 'public,max-age=31536000'); // 1 year + } else { + var ttl = global.environment.varnish.ttl || 86400; + res.header('Last-Modified', new Date().toUTCString()); + res.header('Cache-Control', 'no-cache,max-age='+ttl+',must-revalidate, public'); + } + Cache.generateCacheChannel(req, function(err, channel){ + if ( ! err ) { + res.header('X-Cache-Channel', channel); + cb(null, channel); } else { - res.header('Last-Modified', new Date().toUTCString()); - res.header('Cache-Control', 'no-cache,max-age='+ttl+',must-revalidate, public'); + // avoid caching this result + // (temptative, what Varnish does is out of our control) + res.header('Cache-Control', 'no-cache,no-store,max-age=0,must-revalidate'); + res.header('Pragma', 'no-cache'); + console.log('ERROR generating cache channel: ' + ( err.message ? err.message : err )); + // TODO: evaluate if we should bubble up the error instead + cb(null, 'ERROR'); } - cb(null, channel); // add last-modified too ? }); } @@ -127,7 +137,7 @@ module.exports = function(){ if (!_.isNull(data)) _.extend(req.params, {geom_type: data}); - that.addCacheChannel(req, function(err, chan) { + that.addCacheChannel(req, function(err) { callback(err, req); }); } diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 3e419015..262d7663 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -6,6 +6,8 @@ var querystring = require('querystring'); var semver = require('semver'); var mapnik = require('mapnik'); var Step = require('step'); +var http = require('http'); +var url = require('url'); require(__dirname + '/../support/test_helper'); @@ -17,6 +19,7 @@ server.setMaxListeners(0); suite('server', function() { var redis_client = redis.createClient(global.environment.redis.port); + var sqlapi_server; var default_style = semver.satisfies(mapnik.versions.mapnik, '<2.1.0') ? @@ -30,7 +33,19 @@ suite('server', function() { var test_style_black_200 = "#test_table{marker-fill:black;marker-line-color:red;marker-width:10}"; var test_style_black_210 = "#test_table{marker-fill:black;marker-line-color:red;marker-width:20}"; - suiteSetup(function(){ + suiteSetup(function(done){ + sqlapi_server = http.createServer(function(req,res) { + var query = url.parse(req.url, true).query; + if ( query.q.match('SQLAPIERROR') ) { + res.statusCode = 400; + res.write(JSON.stringify({'error':'Some error occurred'})); + } else { + res.write(JSON.stringify({rows: [ { 'cdb_querytables': '{' + + JSON.stringify(query) + '}' } ]})); + } + res.end(); + }); + sqlapi_server.listen(global.environment.sqlapi.port, done); }); ///////////////////////////////////////////////////////////////////////////////// @@ -916,6 +931,69 @@ suite('server', function() { ); }); + test("uses sqlapi to figure source data of query", function(done){ + var qo = { + sql: "SELECT g.cartodb_id, g.codineprov, t.the_geom_webmercator " + + "FROM gadm4 g, test_table t " + + "WHERE g.cartodb_id = t.cartodb_id", + map_key: 1234 + }; + var sqlapi; + Step( + function sendRequest(err) { + assert.response(server, { + headers: {host: 'localhost'}, + url: '/tiles/gadm4/6/31/24.png?' + querystring.stringify(qo), + method: 'GET' + },{}, this); + }, + function checkResponse(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + var ct = res.headers['content-type']; + assert.equal(ct, 'image/png'); + var cc = res.headers['x-cache-channel']; + var dbname = 'cartodb_test_user_1_db' + assert.equal(cc.substring(0, dbname.length), dbname); + var jsonquery = cc.substring(dbname.length+1); + var sentquery = JSON.parse(jsonquery); + assert.equal(sentquery.api_key, qo.map_key); + assert.equal(sentquery.q, 'SELECT CDB_QueryTables($windshaft$' + qo.sql + '$windshaft$)'); + done(); + } + ); + }); + + test("requests to skip cache on sqlapi error", function(done){ + var qo = { + sql: "SELECT g.cartodb_id, g.codineprov, t.the_geom_webmercator " + + ", 'SQLAPIERROR' is not null " + + "FROM gadm4 g, test_table t " + + "WHERE g.cartodb_id = t.cartodb_id", + map_key: 1234 + }; + var sqlapi; + Step( + function sendRequest(err) { + assert.response(server, { + headers: {host: 'localhost'}, + url: '/tiles/gadm4/6/31/24.png?' + querystring.stringify(qo), + method: 'GET' + },{}, this); + }, + function checkResponse(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + var ct = res.headers['content-type']; + assert.equal(ct, 'image/png'); + // does NOT send an x-cache-channel + assert.ok(!res.headers.hasOwnProperty('x-cache-channel')); + // attempts to tell varnish NOT to cache + assert.equal(res.headers['cache-control'], 'no-cache,no-store,max-age=0,must-revalidate'); + assert.equal(res.headers['pragma'], 'no-cache'); + done(); + } + ); + }); + ///////////////////////////////////////////////////////////////////////////////// // // DELETE CACHE @@ -1040,7 +1118,7 @@ suite('server', function() { // 'map_style|null|publicuser|my_table', redis_client.keys("map_style|*", function(err, matches) { _.each(matches, function(k) { redis_client.del(k); }); - done(); + sqlapi_server.close(done); }); }); diff --git a/test/support/sql/gadm4.sql b/test/support/sql/gadm4.sql index 4f257192..525053ae 100644 --- a/test/support/sql/gadm4.sql +++ b/test/support/sql/gadm4.sql @@ -97,8 +97,6 @@ ALTER TABLE ONLY gadm4 CREATE INDEX bdll25_provincias_4326_2_the_geom_webmercator_idx ON gadm4 USING gist (the_geom_webmercator); --- development_cartodb_user_3 role -CREATE USER development_cartodb_user_3; -GRANT ALL ON TABLE gadm4 TO development_cartodb_user_3; +GRANT ALL ON TABLE gadm4 TO test_cartodb_user_1; GRANT SELECT ON TABLE gadm4 TO publicuser;