diff --git a/NEWS.md b/NEWS.md index aa694e28..6079d79f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -7,6 +7,10 @@ Enhancements: - Respond with a permission denied on attempt to access map tiles waiving signature of someone who had not left any (#170) +Bug fixes: + + - Do not cache map creation responses (#176) + 1.8.4 -- 2014-03-03 ------------------- diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index 349f3189..ad51767e 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -40,6 +40,12 @@ var CartodbWindshaft = function(serverOptions) { callback(err, req); } + // This is for Templated maps + // + // "named" is the official, "template" is for backward compatibility up to 1.6.x + // + var template_baseurl = global.environment.base_url_templated || '(?:/maps/named|/tiles/template)'; + serverOptions.signedMaps = new SignedMaps(redisPool); var templateMapsOpts = { max_user_templates: global.environment.maxUserTemplates @@ -58,6 +64,13 @@ var CartodbWindshaft = function(serverOptions) { } var ws_sendResponse = ws.sendResponse; + // Routes used to create maps with GET, + // for which we don't want to request any caching. + // POST requests are never cached + var mapCreateRoutesGET = [ + serverOptions.base_url_mapconfig, + template_baseurl + '/:template_id/jsonp' + ]; ws.sendResponse = function(res, args) { var that = this; var thatArgs = arguments; @@ -85,9 +98,18 @@ var CartodbWindshaft = function(serverOptions) { // unsuccessful responses return false; } + if ( _.contains(mapCreateRoutesGET, req.route.path) ) { + // We do not want to cache + // map creation responses + // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 +//console.log("Skipping cache channel in route:\n" + req.route.path); + return false; + } +//console.log("Adding cache channel to route\n" + req.route.path + " not matching any in:\n" + mapCreateRoutes.join("\n")); serverOptions.addCacheChannel(that, req, this); }, function sendResponse(err, added) { + if ( err ) console.log(err + err.stack); ws_sendResponse.apply(that, thatArgs); } ); @@ -170,12 +192,6 @@ var CartodbWindshaft = function(serverOptions) { return serverOptions.userByReq(req); } - // This is for Templated maps - // - // "named" is the official, "template" is for backward compatibility up to 1.6.x - // - var template_baseurl = global.environment.base_url_templated || '(?:/maps/named|/tiles/template)'; - // Add a template ws.post(template_baseurl, function(req, res) { ws.doCORS(res); diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 7236e6a1..afc5c622 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -10,7 +10,7 @@ var strftime = require('strftime'); var SQLAPIEmu = require(__dirname + '/../support/SQLAPIEmu.js'); var redis_stats_db = 5; -require(__dirname + '/../support/test_helper'); +var helper = require(__dirname + '/../support/test_helper'); var windshaft_fixtures = __dirname + '/../../node_modules/windshaft/test/fixtures'; @@ -20,14 +20,6 @@ serverOptions = ServerOptions(); var server = new CartodbWindshaft(serverOptions); server.setMaxListeners(0); -// Check that the response headers do not request caching -// Throws on failure -function checkNoCache(res) { - assert.ok(!res.headers.hasOwnProperty('x-cache-channel')); - assert.ok(!res.headers.hasOwnProperty('cache-control')); // is this correct ? - assert.ok(!res.headers.hasOwnProperty('last-modified')); // is this correct ? -} - suite('multilayer', function() { var redis_client = redis.createClient(global.environment.redis.port); @@ -200,6 +192,59 @@ suite('multilayer', function() { ); }); + // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 + // NOTE: another test like this is in templates.js + test("get creation requests no cache", function(done) { + + var layergroup = { + version: '1.0.0', + layers: [ + { options: { + sql: 'select cartodb_id, ST_Translate(the_geom_webmercator, 5e6, 0) as the_geom_webmercator from test_table limit 2', + cartocss: '#layer { marker-fill:red; marker-width:32; marker-allow-overlap:true; }', + cartocss_version: '2.0.1' + } } + ] + }; + + var expected_token; + Step( + function do_create_get() + { + var next = this; + assert.response(server, { + url: '/tiles/layergroup?config=' + encodeURIComponent(JSON.stringify(layergroup)), + method: 'GET', + headers: {host: 'localhost'} + }, {}, function(res, err) { next(err, res); }); + }, + function do_check_create(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 200, res.body); + var parsedBody = JSON.parse(res.body); + expected_token = parsedBody.layergroupid.split(':')[0]; + helper.checkNoCache(res); + return null; + }, + function finish(err) { + var errors = []; + if ( err ) { + errors.push(err.message); + console.log("Error: " + err); + } + redis_client.keys("map_cfg|" + expected_token, function(err, matches) { + if ( err ) errors.push(err.message); + assert.equal(matches.length, 1, "Missing expected token " + expected_token + " from redis: " + matches); + redis_client.del(matches, function(err) { + if ( err ) errors.push(err.message); + if ( errors.length ) done(new Error(errors)); + else done(null); + }); + }); + } + ); + }); + test("layergroup can hold substitution tokens", function(done) { @@ -515,7 +560,7 @@ suite('multilayer', function() { var parsed = JSON.parse(res.body); var msg = parsed.errors[0]; assert.ok(msg.match(/bogus.*exist/), msg); - checkNoCache(res); + helper.checkNoCache(res); done(); }); }); diff --git a/test/acceptance/templates.js b/test/acceptance/templates.js index 563f5330..37754b76 100644 --- a/test/acceptance/templates.js +++ b/test/acceptance/templates.js @@ -1,5 +1,4 @@ var assert = require('../support/assert'); -var tests = module.exports = {}; var _ = require('underscore'); var redis = require('redis'); var querystring = require('querystring'); @@ -16,7 +15,7 @@ var redis_stats_db = 5; process.env['PGPORT'] = '666'; process.env['PGHOST'] = 'fake'; -require(__dirname + '/../support/test_helper'); +var helper = require(__dirname + '/../support/test_helper'); var windshaft_fixtures = __dirname + '/../../node_modules/windshaft/test/fixtures'; @@ -1445,6 +1444,7 @@ suite('template_api', function() { headers: {host: 'localhost', 'Content-Type': 'application/json' }, data: JSON.stringify(template_params) } + helper.checkNoCache(res); var next = this; assert.response(server, post_request, {}, function(res) { next(null, res); }); @@ -1516,13 +1516,16 @@ suite('template_api', function() { assert.response(server, post_request, {}, function(res) { next(null, res); }); }, - function instanciateAuth(err, res) + function checkInstanciation(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 200, - 'Unexpected success instanciating template with no auth: ' - + res.statusCode + ': ' + res.body); - done(); + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 + helper.checkNoCache(res); + return null; + }, + function finish(err) { + done(err); } ); }); @@ -1587,12 +1590,12 @@ suite('template_api', function() { assert.response(server, post_request, {}, function(res) { next(null, res); }); }, - function instanciateAuth(err, res) + function checkInstanciation(err, res) { if ( err ) throw err; - assert.equal(res.statusCode, 200, - 'Unexpected success instanciating template with no auth: ' - + res.statusCode + ': ' + res.body); + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // See https://github.com/CartoDB/Windshaft-cartodb/issues/176 + helper.checkNoCache(res); return null; }, function finish(err) { diff --git a/test/support/test_helper.js b/test/support/test_helper.js index 89873c6b..1e7f86ce 100644 --- a/test/support/test_helper.js +++ b/test/support/test_helper.js @@ -6,6 +6,7 @@ */ var _ = require('underscore'); +var assert = require('assert'); var LZMA = require('lzma/lzma_worker.js').LZMA; // set environment specific variables @@ -29,7 +30,17 @@ function lzma_compress_to_base64(payload, mode, callback) { ); } -module.exports = { - lzma_compress_to_base64: lzma_compress_to_base64 +// Check that the response headers do not request caching +// Throws on failure +function checkNoCache(res) { + assert.ok(!res.headers.hasOwnProperty('x-cache-channel')); + assert.ok(!res.headers.hasOwnProperty('cache-control')); // is this correct ? + assert.ok(!res.headers.hasOwnProperty('last-modified')); // is this correct ? +} + + +module.exports = { + lzma_compress_to_base64: lzma_compress_to_base64, + checkNoCache: checkNoCache }