From 6c6f3d02f67328567fc074870961bdbac4fbd2d2 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 19 Feb 2014 10:08:25 +0100 Subject: [PATCH] Always generate X-Cache-Channel for token-based tile responses Closes #152 --- NEWS.md | 4 ++ lib/cartodb/cartodb_windshaft.js | 68 ++++++++++++++++++++------------ lib/cartodb/server_options.js | 38 +++++++++++++----- npm-shrinkwrap.json | 10 ++--- package.json | 2 +- test/acceptance/multilayer.js | 2 +- 6 files changed, 82 insertions(+), 42 deletions(-) diff --git a/NEWS.md b/NEWS.md index f0cb27b4..f2d2cb39 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,10 @@ Enhancements: * Use log4js logger (#138) +Bug fixes: + + * Always generate X-Cache-Channel for token-based tile responses (#152) + 1.8.0 -- 2014-02-18 ------------------- diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index 64e349b8..9a40725e 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -46,20 +46,36 @@ var CartodbWindshaft = function(serverOptions) { return version; } - // Override sendError to drop added cache headers (if any) - // See http://github.com/CartoDB/Windshaft-cartodb/issues/107 - var ws_sendError = ws.sendError; - ws.sendError = function(res) { - // NOTE: the "res" object will have no _headers when - // faked by Windshaft, see - // http://github.com/CartoDB/Windshaft-cartodb/issues/109 - // - if ( res._headers ) { - delete res._headers['cache-control']; - delete res._headers['last-modified']; - delete res._headers['x-cache-channel']; - } - ws_sendError.apply(this, arguments); + var ws_sendResponse = ws.sendResponse; + ws.sendResponse = function(res, args) { + var that = this; + var thatArgs = arguments; + var statusCode; + if ( args.length > 2 ) statusCode = args[2]; + else { + statusCode = args[1] || 200; + } + var req = res.req; + Step ( + function addCacheChannel() { + if ( ! req ) { + // having no associated request can happen when + // using fake response objects for testing layergroup + // creation + return false; + } + if ( statusCode != 200 ) { + // We do not want to cache + // unsuccessful responses + return false; + } + serverOptions.addCacheChannel(that, req, this); + }, + function sendResponse(err, added) { + if (added && req.profiler) req.profiler.done('addCacheChannel'); + ws_sendResponse.apply(that, thatArgs); + } + ); }; /** @@ -74,9 +90,9 @@ var CartodbWindshaft = function(serverOptions) { function(err, data){ if (err){ ws.sendError(res, {error: err.message}, 500, 'GET INFOWINDOW', err); - //res.send({error: err.message}, 500); + //ws.sendResponse(res, [{error: err.message}, 500]); } else { - res.send({infowindow: data}, 200); + ws.sendResponse(res, [{infowindow: data}, 200]); } } ); @@ -95,9 +111,9 @@ var CartodbWindshaft = function(serverOptions) { function(err, data){ if (err){ ws.sendError(res, {error: err.message}, 500, 'GET MAP_METADATA', err); - //res.send(err.message, 500); + //ws.sendResponse(res, [err.message, 500]); } else { - res.send({map_metadata: data}, 200); + ws.sendResponse(res, [{map_metadata: data}, 200]); } } ); @@ -116,9 +132,9 @@ var CartodbWindshaft = function(serverOptions) { function sendResponse(err, data){ if (err){ ws.sendError(res, {error: err.message}, 500, 'DELETE CACHE', err); - //res.send(500); + //ws.sendResponse(res, [500]); } else { - res.send({status: 'ok'}, 200); + ws.sendResponse(res, [{status: 'ok'}, 200]); } } ); @@ -177,7 +193,7 @@ var CartodbWindshaft = function(serverOptions) { } ws.sendError(res, response, statusCode, 'POST TEMPLATE', err); } else { - res.send(response, 200); + ws.sendResponse(res, [response, 200]); } } ); @@ -234,7 +250,7 @@ var CartodbWindshaft = function(serverOptions) { } ws.sendError(res, response, statusCode, 'PUT TEMPLATE', err); } else { - res.send(response, 200); + ws.sendResponse(res, [response, 200]); } } ); @@ -292,7 +308,7 @@ var CartodbWindshaft = function(serverOptions) { } ws.sendError(res, response, statusCode, 'GET TEMPLATE', err); } else { - res.send(response, 200); + ws.sendResponse(res, [response, 200]); } } ); @@ -342,7 +358,7 @@ var CartodbWindshaft = function(serverOptions) { } ws.sendError(res, response, statusCode, 'DELETE TEMPLATE', err); } else { - res.send('', 204); + ws.sendResponse(res, ['', 204]); } } ); @@ -382,7 +398,7 @@ var CartodbWindshaft = function(serverOptions) { } ws.sendError(res, response, statusCode, 'GET TEMPLATE LIST', err); } else { - res.send(response, statusCode); + ws.sendResponse(res, [response, statusCode]); } } ); @@ -531,7 +547,7 @@ var CartodbWindshaft = function(serverOptions) { } ws.sendError(res, response, statusCode, 'POST INSTANCE TEMPLATE', err); } else { - res.send(response, 200); + ws.sendResponse(res, [response, 200]); } if ( req.profiler && req.profiler.statsd_client) { req.profiler.sendStats(); diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index b77cfdd2..15f17e87 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -197,7 +197,7 @@ module.exports = function(){ return hash.digest('hex'); } - me.generateCacheChannel = function(req, callback){ + me.generateCacheChannel = function(app, req, callback){ // use key to call sql api with sql request if present, else // just return dbname and table name base key @@ -225,10 +225,33 @@ module.exports = function(){ // TODO: cached cache channel for token-based access should // be constructed at renderer cache creation time // See http://github.com/CartoDB/Windshaft-cartodb/issues/152 - throw new Error('missing channel cache for token ' + req.params.token); + if ( ! app.mapStore ) { + throw new Error('missing channel cache for token ' + req.params.token); + return; + } + var next = this; + var mapStore = app.mapStore; + Step( + function loadFromStore() { + mapStore.load(req.params.token, this); + }, + function getSQL(err, mapConfig) { + if ( err ) throw err; + var sql = []; + _.each(mapConfig.obj().layers, function(lyr) { + sql.push(lyr.options.sql); + }); + sql = sql.join(';'); + return sql; + }, + function finish(err, sql) { + next(err, sql); + } + ); return; } - else if ( ! req.params.sql ) { + + if ( ! req.params.sql ) { return null; // no sql } @@ -278,7 +301,7 @@ module.exports = function(){ // @param cb function(err, channel) will be called when ready. // the channel parameter will be null if nothing was added // - me.addCacheChannel = function(req, cb) { + me.addCacheChannel = function(app, req, cb) { // 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; @@ -302,7 +325,7 @@ module.exports = function(){ } res.header('Last-Modified', lastUpdated.toUTCString()); - me.generateCacheChannel(req, function(err, channel){ + me.generateCacheChannel(app, req, function(err, channel){ if ( ! err ) { res.header('X-Cache-Channel', channel); cb(null, channel); @@ -722,10 +745,7 @@ module.exports = function(){ dbport: global.environment.postgres.port }); - that.addCacheChannel(req, function(err) { - if (req.profiler) req.profiler.done('addCacheChannel'); - callback(err, req); - }); + callback(null, req); } ); }; diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index f7b0c4f7..97f1bedc 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -10,8 +10,8 @@ "version": "1.3.3" }, "windshaft": { - "version": "0.18.2", - "from": "http://github.com/CartoDB/Windshaft/tarball/0.18.2", + "version": "0.19.0", + "from": "http://github.com/CartoDB/Windshaft/tarball/0.19.0-rc1", "dependencies": { "grainstore": { "version": "0.18.0", @@ -429,15 +429,15 @@ } } }, + "semver": { + "version": "1.1.4" + }, "strftime": { "version": "0.6.2" }, "redis": { "version": "0.8.6" }, - "semver": { - "version": "1.1.4" - }, "mocha": { "version": "1.14.0", "dependencies": { diff --git a/package.json b/package.json index 7d2c26a6..1996ac67 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "dependencies": { "node-varnish": "http://github.com/Vizzuality/node-varnish/tarball/v0.2.0", "underscore" : "~1.3.3", - "windshaft" : "http://github.com/CartoDB/Windshaft/tarball/0.18.2", + "windshaft" : "http://github.com/CartoDB/Windshaft/tarball/0.19.0-rc1", "step": "0.0.x", "request": "2.9.202", "cartodb-redis": "~0.3.0", diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index 81a6065a..07f094b5 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -665,7 +665,7 @@ suite('multilayer', function() { }); // See https://github.com/CartoDB/Windshaft-cartodb/issues/152 - test.skip("x-cache-channel still works for GETs after tiler restart", function(done) { + test("x-cache-channel still works for GETs after tiler restart", function(done) { var layergroup = { version: '1.0.0',