From fcd17692eee1b0523938e4f100bfbbeead4f7415 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 9 Jan 2014 15:36:16 +0100 Subject: [PATCH 01/16] Fix username extraction in another two places. Thanks @demimismo. Closes #100 (again) --- lib/cartodb/server_options.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index 7769a83e..786aac0d 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -188,7 +188,7 @@ module.exports = function(){ } var dbName = req.params.dbname; - var username = req.headers.host.split('.')[0]; + var username = this.userByReq(req); // strip out windshaft/mapnik inserted sql if present var sql = req.params.sql.match(/^\((.*)\)\sas\scdbq$/); @@ -577,7 +577,7 @@ console.log("Checking authorization from signer " + signer + " for resource " + if ( tksplit.length > 1 ) req.params.cache_buster= tksplit[1]; tksplit = req.params.token.split('@'); if ( tksplit.length > 1 ) { - req.params.signer = req.headers.host.split('.')[0]; + req.params.signer = this.userByReq(req); if ( tksplit[0] ) req.params.signer = tksplit[0]; req.params.token = tksplit[1]; //console.log("Request for token " + req.params.token + " with signature from " + req.params.signer); From 4ee4492490c6966e0fd5e37db41e0237b6ba5b2e Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 9 Jan 2014 16:46:47 +0100 Subject: [PATCH 02/16] Yet another username extraction fix. Thanks again @demimismo. Closes #100 (yet again) --- lib/cartodb/server_options.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index 786aac0d..957deb66 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -281,7 +281,7 @@ module.exports = function(){ sql = sql.join(';'); var dbName = req.params.dbname; - var usr = req.headers.host.split('.')[0]; + var usr = this.userByReq(req); var key = req.params.map_key; var cacheKey = dbName + ':' + token; From d849ae216d767adaf0d3079d317025e9d3b99b3e Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 9 Jan 2014 18:01:22 +0100 Subject: [PATCH 03/16] Keep build status line within 80 cols (and use http) --- README.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 3014d09d..ed6dea27 100644 --- a/README.md +++ b/README.md @@ -1,7 +1,8 @@ Windshaft-CartoDB ================== -[![Build Status](https://travis-ci.org/CartoDB/Windshaft-cartodb.png)](http://travis-ci.org/CartoDB/Windshaft-cartodb) +[![Build Status](http://travis-ci.org/CartoDB/Windshaft-cartodb.png)] +(http://travis-ci.org/CartoDB/Windshaft-cartodb) This is the CartoDB map tiler. It extends Windshaft with some extra functionality and custom filters for authentication From c1b6b865a76f13f21efab540c05d5af4d918c557 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 10 Jan 2014 11:30:10 +0100 Subject: [PATCH 04/16] Release 1.6.0 --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index aa794ca7..cbab098e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -1.6.0 -- 20YY-MM-DD +1.6.0 -- 2014-01-10 ------------------- New features: From 90e0a5dc30f23e4447881f15a44f15376bc586a5 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 10 Jan 2014 11:32:03 +0100 Subject: [PATCH 05/16] Prepare for 1.6.1 --- NEWS.md | 3 +++ package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index cbab098e..fec5f42e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,6 @@ +1.6.1 -- 2014-01-DD +------------------- + 1.6.0 -- 2014-01-10 ------------------- diff --git a/package.json b/package.json index 9e1ce7fe..fd7b19a0 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "1.6.0", + "version": "1.6.1", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From ae82d0ab47d21a8e5aef7f332b74244d94072545 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 10 Jan 2014 13:20:26 +0100 Subject: [PATCH 06/16] Expect overrides of mapnik_version to be honoured Reported on http://gis.stackexchange.com/questions/81450/cartodb-windshaft-error --- test/acceptance/server.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 33248fb5..8798114d 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -107,7 +107,7 @@ suite('server', function() { }, function(res) { var parsed = JSON.parse(res.body); assert.equal(parsed.style, _.template(default_style, {table: 'my_table'})); - assert.equal(parsed.style_version, mapnik.versions.mapnik); + assert.equal(parsed.style_version, mapnik_version); done(); }); }); @@ -158,7 +158,7 @@ suite('server', function() { var parsed = JSON.parse(res.body); var style = _.template(default_style, {table: 'test_table_private_1'}); assert.equal(parsed.style, style); - assert.equal(parsed.style_version, mapnik.versions.mapnik); + assert.equal(parsed.style_version, mapnik_version); done(); }); }); @@ -351,7 +351,7 @@ suite('server', function() { assert.equal(res.statusCode, 200, res.body); var parsed = JSON.parse(res.body); assert.equal(parsed.style, style); - assert.equal(parsed.style_version, mapnik.versions.mapnik); + assert.equal(parsed.style_version, mapnik_version); done(); }); }); @@ -379,7 +379,7 @@ suite('server', function() { var parsed = JSON.parse(res.body); // NOTE: no transform expected for the specific style assert.equal(parsed.style, style); - assert.equal(parsed.style_version, mapnik.versions.mapnik); + assert.equal(parsed.style_version, mapnik_version); done(); }); }); From 2690ef3f0541859a59cec9e655045269c185d196 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 13 Jan 2014 11:20:02 +0100 Subject: [PATCH 07/16] Drop cache headers from error responses. Closes #107 (github), #resolve CDB-1423 (JIRA) --- NEWS.md | 4 ++++ lib/cartodb/cartodb_windshaft.js | 10 ++++++++++ test/acceptance/server.js | 17 +++++++++++++++-- 3 files changed, 29 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index fec5f42e..70a27813 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,10 @@ 1.6.1 -- 2014-01-DD ------------------- +Bug fixes: + +* Drop cache headers from error responses (#107) + 1.6.0 -- 2014-01-10 ------------------- diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index 29737d1d..cc3151d0 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -42,6 +42,16 @@ 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) { + delete res._headers['cache-control']; + delete res._headers['last-modified']; + delete res._headers['x-cache-channel']; + ws_sendError.apply(this, arguments); + }; + /** * Helper to allow access to the layer to be used in the maps infowindow popup. */ diff --git a/test/acceptance/server.js b/test/acceptance/server.js index 8798114d..06d06f10 100644 --- a/test/acceptance/server.js +++ b/test/acceptance/server.js @@ -125,6 +125,7 @@ suite('server', function() { assert.equal(res.statusCode, 400, res.body); assert.deepEqual(JSON.parse(res.body), {error: 'Sorry, you are unauthorized (permission denied)'}); + assert.ok(!res.headers.hasOwnProperty('cache-control')); done(); }); }); @@ -142,6 +143,7 @@ suite('server', function() { assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(JSON.parse(res.body), {error:"missing unknown_user's database_name in redis (try CARTODB/script/restore_redis)"}); + assert.ok(!res.headers.hasOwnProperty('cache-control')); done(); }); }); @@ -212,9 +214,12 @@ suite('server', function() { url: '/tiles/my_table/style', method: 'POST' },{ - status: 400, body: '{"error":"must send style information"}' - }, function() { done(); }); + }, function(res) { + assert.equal(res.statusCode, 400); + assert.ok(!res.headers.hasOwnProperty('cache-control')); + done(); + }); }); test("post'ing bad style returns 400 with error", function(done){ @@ -766,6 +771,8 @@ suite('server', function() { assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); assert.deepEqual(JSON.parse(res.body), {error:"missing unknown_user's database_name in redis (try CARTODB/script/restore_redis)"}); + assert.ok(!res.headers.hasOwnProperty('cache-control'), + "Unexpected Cache-Control: " + res.headers['cache-control']); done(); }); }); @@ -786,6 +793,9 @@ suite('server', function() { }, function(res) { // 401 Unauthorized assert.equal(res.statusCode, 401, res.statusCode + ': ' + res.body); + // Failed in 1.6.0 of https://github.com/CartoDB/Windshaft-cartodb/issues/107 + assert.ok(!res.headers.hasOwnProperty('cache-control'), + "Unexpected Cache-Control: " + res.headers['cache-control']); done(); }); }); @@ -1179,6 +1189,7 @@ suite('server', function() { method: 'DELETE' },{}, function(res) { assert.equal(res.statusCode, 404, res.statusCode + ': ' + res.body); + assert.ok(!res.headers.hasOwnProperty('cache-control')); done(); }); }); @@ -1210,6 +1221,7 @@ suite('server', function() { },{}, function(res) { // FIXME: should be 401 instead assert.equal(res.statusCode, 500, res.statusCode + ': ' + res.body); + assert.ok(!res.headers.hasOwnProperty('cache-control')); done(); }); }); @@ -1262,6 +1274,7 @@ suite('server', function() { method: 'DELETE' },{}, function(res) { assert.equal(res.statusCode, 404, res.statusCode + ': ' + res.body); + assert.ok(!res.headers.hasOwnProperty('cache-control')); done(); }); }); From d6fe5339cf6e43ff9f79afe26e52d8a299c4e834 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 13 Jan 2014 18:56:09 +0100 Subject: [PATCH 08/16] Do not choke on headers cleanup when response headers are not set Raise a WARNING instead. See #107 (github) and CDB-1438 (JIRA) --- lib/cartodb/cartodb_windshaft.js | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index cc3151d0..d18cf4e3 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -46,9 +46,13 @@ var CartodbWindshaft = function(serverOptions) { // See http://github.com/CartoDB/Windshaft-cartodb/issues/107 var ws_sendError = ws.sendError; ws.sendError = function(res) { - delete res._headers['cache-control']; - delete res._headers['last-modified']; - delete res._headers['x-cache-channel']; + if ( res._headers ) { + delete res._headers['cache-control']; + delete res._headers['last-modified']; + delete res._headers['x-cache-channel']; + } else { + console.log("WARNING: response has no _headers: "); console.dir(res); + } ws_sendError.apply(this, arguments); }; From 18ccd3cbafddc422835d8ad5d0079af712f4dde1 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 14 Jan 2014 16:20:06 +0100 Subject: [PATCH 09/16] Localize external CartoCSS resources at renderer creation time Closes #108. JIRA CDB-1422 #resolve --- NEWS.md | 1 + package.json | 2 +- 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 70a27813..1a98667b 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ Bug fixes: * Drop cache headers from error responses (#107) +* Localize external CartoCSS resources at renderer creation time (#108) 1.6.0 -- 2014-01-10 ------------------- diff --git a/package.json b/package.json index fd7b19a0..46114702 100644 --- a/package.json +++ b/package.json @@ -24,7 +24,7 @@ "dependencies": { "node-varnish": "0.1.1", "underscore" : "~1.3.3", - "windshaft" : "~0.14.5", + "windshaft" : "~0.15.0", "step": "0.0.x", "request": "2.9.202", "cartodb-redis": "~0.3.0", From b01ce9d4cccbcb26ee634636127c6a0a920f1bd5 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 14 Jan 2014 18:09:36 +0100 Subject: [PATCH 10/16] Regenerate shrinkwrap for 1.6.1 --- npm-shrinkwrap.json | 172 ++++++++++++++++++++++++++++++++------------ 1 file changed, 127 insertions(+), 45 deletions(-) diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 4c6c37e2..c086f7e6 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "windshaft-cartodb", - "version": "1.6.0", + "version": "1.6.1", "dependencies": { "node-varnish": { "version": "0.1.1" @@ -9,14 +9,14 @@ "version": "1.3.3" }, "windshaft": { - "version": "0.14.5", + "version": "0.15.0", "dependencies": { "grainstore": { - "version": "0.15.2", + "version": "0.16.0", "dependencies": { "carto": { "version": "0.9.5-cdb2", - "from": "git://github.com/CartoDB/carto.git#0.9.5-cdb2", + "from": "http://github.com/CartoDB/carto/tarball/0.9.5-cdb2", "dependencies": { "underscore": { "version": "1.4.4" @@ -25,7 +25,7 @@ "version": "0.2.8", "dependencies": { "sax": { - "version": "0.5.5" + "version": "0.5.8" } } }, @@ -131,46 +131,136 @@ "version": "0.3.8" }, "zipfile": { - "version": "0.4.2" + "version": "0.4.3" }, "sqlite3": { - "version": "2.1.19", + "version": "2.2.0", "dependencies": { - "tar.gz": { - "version": "0.1.1", + "node-pre-gyp": { + "version": "0.2.5", "dependencies": { - "fstream": { - "version": "0.1.25", + "nopt": { + "version": "2.1.2", "dependencies": { - "rimraf": { - "version": "2.2.4" - }, - "graceful-fs": { - "version": "2.0.1" - }, - "inherits": { - "version": "2.0.1" + "abbrev": { + "version": "1.0.4" } } }, + "npmlog": { + "version": "0.0.6", + "dependencies": { + "ansi": { + "version": "0.2.1" + } + } + }, + "semver": { + "version": "2.1.0" + }, "tar": { - "version": "0.1.18", + "version": "0.1.19", "dependencies": { "inherits": { "version": "2.0.1" }, "block-stream": { "version": "0.0.7" + }, + "fstream": { + "version": "0.1.25", + "dependencies": { + "graceful-fs": { + "version": "2.0.1" + } + } } } }, - "commander": { - "version": "1.1.1", + "tar-pack": { + "version": "2.0.0", "dependencies": { - "keypress": { - "version": "0.1.0" + "uid-number": { + "version": "0.0.3" + }, + "once": { + "version": "1.1.1" + }, + "debug": { + "version": "0.7.4" + }, + "fstream": { + "version": "0.1.25", + "dependencies": { + "graceful-fs": { + "version": "2.0.1" + }, + "inherits": { + "version": "2.0.1" + } + } + }, + "fstream-ignore": { + "version": "0.0.7", + "dependencies": { + "minimatch": { + "version": "0.2.14", + "dependencies": { + "sigmund": { + "version": "1.0.0" + } + } + }, + "inherits": { + "version": "2.0.1" + } + } + }, + "readable-stream": { + "version": "1.0.24" + }, + "graceful-fs": { + "version": "1.2.3" } } + }, + "aws-sdk": { + "version": "2.0.0-rc8", + "dependencies": { + "xml2js": { + "version": "0.2.4", + "dependencies": { + "sax": { + "version": "0.6.0" + } + } + }, + "xmlbuilder": { + "version": "0.4.2" + } + } + }, + "rc": { + "version": "0.3.2", + "dependencies": { + "optimist": { + "version": "0.3.7", + "dependencies": { + "wordwrap": { + "version": "0.0.2" + } + } + }, + "deep-extend": { + "version": "0.2.6" + }, + "ini": { + "version": "1.1.0" + } + } + }, + "rimraf": { + "version": "2.2.5" } } } @@ -239,7 +329,7 @@ } }, "tilelive-mapnik": { - "version": "0.6.4", + "version": "0.6.5", "dependencies": { "eio": { "version": "0.2.2" @@ -267,10 +357,18 @@ "version": "0.3.0" }, "redis-mpool": { - "version": "0.0.2", + "version": "0.0.3", "dependencies": { "generic-pool": { "version": "2.0.4" + }, + "hiredis": { + "version": "0.1.16", + "dependencies": { + "bindings": { + "version": "1.1.1" + } + } } } }, @@ -283,27 +381,11 @@ "strftime": { "version": "0.6.2" }, - "semver": { - "version": "1.1.4" - }, "redis": { "version": "0.8.6" }, - "redis-mpool": { - "version": "0.0.2", - "dependencies": { - "generic-pool": { - "version": "2.0.4" - } - } - }, - "hiredis": { - "version": "0.1.15", - "dependencies": { - "bindings": { - "version": "1.1.0" - } - } + "semver": { + "version": "1.1.4" }, "mocha": { "version": "1.14.0", @@ -338,7 +420,7 @@ "version": "3.2.3", "dependencies": { "minimatch": { - "version": "0.2.12", + "version": "0.2.14", "dependencies": { "lru-cache": { "version": "2.5.0" From f89fd98ed73cf4cecea47878bc2f8754a637c56a Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 15 Jan 2014 11:53:19 +0100 Subject: [PATCH 11/16] Expect malformed response objects (#109) Include test for sql errors on layergroup creation Closes #109 --- lib/cartodb/cartodb_windshaft.js | 8 ++++--- test/acceptance/multilayer.js | 37 ++++++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 3 deletions(-) diff --git a/lib/cartodb/cartodb_windshaft.js b/lib/cartodb/cartodb_windshaft.js index d18cf4e3..6ba8e031 100644 --- a/lib/cartodb/cartodb_windshaft.js +++ b/lib/cartodb/cartodb_windshaft.js @@ -46,13 +46,15 @@ var CartodbWindshaft = function(serverOptions) { // 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']; - } else { - console.log("WARNING: response has no _headers: "); console.dir(res); - } + } ws_sendError.apply(this, arguments); }; diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index c7d6f3bd..c5d30856 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -19,6 +19,14 @@ var serverOptions = require(__dirname + '/../../lib/cartodb/server_options'); 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); @@ -460,6 +468,35 @@ suite('multilayer', function() { }); }); + // Also tests that server doesn't crash: + // see http://github.com/CartoDB/Windshaft-cartodb/issues/109 + test("layergroup creation fails if sql is bogus", function(done) { + var layergroup = { + stat_tag: 'random_tag', + version: '1.0.0', + layers: [ + { options: { + sql: 'select bogus(0,0) as the_geom_webmercator', + cartocss: '#layer { polygon-fill:red; }', + cartocss_version: '2.0.1' + } } + ] + }; + assert.response(server, { + url: '/tiles/layergroup', + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: JSON.stringify(layergroup) + }, {}, function(res) { + assert.equal(res.statusCode, 400, res.body); + var parsed = JSON.parse(res.body); + var msg = parsed.errors[0]; + assert.ok(msg.match(/bogus.*exist/), msg); + checkNoCache(res); + done(); + }); + }); + test("layergroup with 2 private-table layers", function(done) { var layergroup = { From d22f399f1827553e784018fd2787e75cd42499db Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 15 Jan 2014 19:23:20 +0100 Subject: [PATCH 12/16] Release 1.6.1 --- NEWS.md | 2 +- npm-shrinkwrap.json | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/NEWS.md b/NEWS.md index 1a98667b..772214d3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -1.6.1 -- 2014-01-DD +1.6.1 -- 2014-01-15 ------------------- Bug fixes: diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index c086f7e6..3f7371c7 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -378,15 +378,15 @@ "lzma": { "version": "1.2.3" }, + "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": { From 09d4467e22e571ab50133cf8197c423b19189e8c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 16 Jan 2014 17:19:55 +0100 Subject: [PATCH 13/16] Prepare for 1.6.2 --- NEWS.md | 5 +++++ package.json | 2 +- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 772214d3..9b5547a0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,8 @@ +1.6.2 -- 2014-MM-DD +------------------- + +Bug fixes: + 1.6.1 -- 2014-01-15 ------------------- diff --git a/package.json b/package.json index 46114702..9a622089 100644 --- a/package.json +++ b/package.json @@ -1,7 +1,7 @@ { "private": true, "name": "windshaft-cartodb", - "version": "1.6.1", + "version": "1.6.2", "description": "A map tile server for CartoDB", "keywords": [ "cartodb" From 5772c815900b683c8725f068ceba7887d16b4690 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 16 Jan 2014 17:20:30 +0100 Subject: [PATCH 14/16] Fix support for long (>64k chars) queries in layergroup creation Closes #111. Includes testcase. --- NEWS.md | 2 + lib/cartodb/server_options.js | 10 ++++- test/acceptance/multilayer.js | 65 ++++++++++++++++++++++++++++++++ test/support/SQLAPIEmu.js | 71 ++++++++++++++++++++++++----------- 4 files changed, 126 insertions(+), 22 deletions(-) diff --git a/NEWS.md b/NEWS.md index 9b5547a0..957a752e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,8 @@ Bug fixes: +* Fix support for long (>64k chars) queries in layergroup creation (#111) + 1.6.1 -- 2014-01-15 ------------------- diff --git a/lib/cartodb/server_options.js b/lib/cartodb/server_options.js index 957deb66..b533336b 100644 --- a/lib/cartodb/server_options.js +++ b/lib/cartodb/server_options.js @@ -78,7 +78,15 @@ module.exports = function(){ if (_.isString(api_key) && api_key != '') { qs.api_key = api_key; } // call sql api - request.get({url:sqlapi, qs:qs, json:true}, function(err, res, body){ + // + // NOTE: using POST to avoid size limits: + // Seehttp://github.com/CartoDB/Windshaft-cartodb/issues/111 + // + // TODO: use "host" header to allow IP based specification + // of sqlapi address (and avoid a DNS lookup) + // + request.post({url:sqlapi, body:qs, json:true}, + function(err, res, body){ if (err){ console.log('ERROR connecting to SQL API on ' + sqlapi + ': ' + err); callback(err); diff --git a/test/acceptance/multilayer.js b/test/acceptance/multilayer.js index c5d30856..2a136ef2 100644 --- a/test/acceptance/multilayer.js +++ b/test/acceptance/multilayer.js @@ -900,6 +900,71 @@ suite('multilayer', function() { ); }); + // SQL strings can be of arbitrary length, when using POST + // See https://github.com/CartoDB/Windshaft-cartodb/issues/111 + test("sql string can be very long", function(done){ + var long_val = 'pretty'; + for (var i=0; i<1024; ++i) long_val += ' long' + long_val += ' string'; + var sql = "SELECT "; + for (var i=0; i<16; ++i) + sql += "'" + long_val + "'::text as pretty_long_field_name_" + i + ", "; + sql += "cartodb_id, the_geom_webmercator FROM gadm4 g"; + var layergroup = { + version: '1.0.0', + layers: [ + { options: { + sql: sql, + cartocss: '#layer { marker-fill:red; }', + cartocss_version: '2.0.1' + } } + ] + }; + var errors = []; + var expected_token; + Step( + function do_post() + { + var data = JSON.stringify(layergroup); + assert.ok(data.length > 1024*64); + var next = this; + assert.response(server, { + url: '/tiles/layergroup?api_key=1234', + method: 'POST', + headers: {host: 'localhost', 'Content-Type': 'application/json' }, + data: data + }, {}, function(res) { next(null, res); }); + }, + function check_result(err, res) { + if ( err ) throw err; + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + var parsedBody = JSON.parse(res.body); + var token_components = parsedBody.layergroupid.split(':'); + expected_token = token_components[0]; + return null; + }, + function cleanup(err) { + if ( err ) errors.push(err.message); + if ( ! expected_token ) return null; + var next = this; + redis_client.keys("map_style|test_cartodb_user_1_db|~" + 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); + next(); + }); + }); + }, + function finish(err) { + if ( err ) errors.push('' + err); + if ( errors.length ) done(new Error(errors.join(','))); + else done(null); + } + ); + }); + + suiteTeardown(function(done) { // This test will add map_style records, like diff --git a/test/support/SQLAPIEmu.js b/test/support/SQLAPIEmu.js index 99e9b8c6..8781325d 100644 --- a/test/support/SQLAPIEmu.js +++ b/test/support/SQLAPIEmu.js @@ -5,31 +5,60 @@ var o = function(port, cb) { this.queries = []; var that = this; + this.sqlapi_server = http.createServer(function(req,res) { - var query = url.parse(req.url, true).query; - that.queries.push(query); - if ( query.q.match('SQLAPIERROR') ) { - res.statusCode = 400; - res.write(JSON.stringify({'error':'Some error occurred'})); - } else if ( query.q.match('EPOCH.* as max') ) { - // This is the structure of the known query sent by tiler - var row = { - 'max': 1234567890.123 - }; - res.write(JSON.stringify({rows: [ row ]})); - } else { - var qs = JSON.stringify(query); - var row = { - // This is the structure of the known query sent by tiler - 'cdb_querytables': '{' + qs + '}', - 'max': qs - }; - res.write(JSON.stringify({rows: [ row ]})); - } - res.end(); + //console.log("server got request with method " + req.method); + var query; + if ( req.method == 'GET' ) { + query = url.parse(req.url, true).query; + that.handleQuery(query, res); + } + else if ( req.method == 'POST') { + var data = ''; + req.on('data', function(chunk) { + //console.log("GOT Chunk " + chunk); + data += chunk; + }); + req.on('end', function() { + //console.log("Data is: "); console.dir(data); + query = JSON.parse(data); + //console.log("Parsed is: "); console.dir(query); + //console.log("handleQuery is " + that.handleQuery); + that.handleQuery(query, res); + }); + } + else { + that.handleQuery('SQLAPIEmu does not support method' + req.method, res); + } }).listen(port, cb); }; +o.prototype.handleQuery = function(query, res) { + this.queries.push(query); + if ( query.q.match('SQLAPIERROR') ) { + res.statusCode = 400; + res.write(JSON.stringify({'error':'Some error occurred'})); + } else if ( query.q.match('EPOCH.* as max') ) { + // This is the structure of the known query sent by tiler + var row = { + 'max': 1234567890.123 + }; + res.write(JSON.stringify({rows: [ row ]})); + } else { + var qs = JSON.stringify(query); + var row = { + // This is the structure of the known query sent by tiler + 'cdb_querytables': '{' + qs + '}', + 'max': qs + }; + var out_obj = {rows: [ row ]}; + var out = JSON.stringify(out_obj); + res.write(out); + } + res.end(); + }; + + o.prototype.close = function(cb) { this.sqlapi_server.close(cb); }; From 8b80ad8ba18a618f6f6a6009c8369be6a49bc8bb Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 16 Jan 2014 18:51:02 +0100 Subject: [PATCH 15/16] Restore XML print from the show_style tool Closes #110 --- NEWS.md | 5 ++++ tools/show_style | 64 ++++++++++++++++++++++++++++++++++++++++++------ 2 files changed, 62 insertions(+), 7 deletions(-) diff --git a/NEWS.md b/NEWS.md index 957a752e..92cc691a 100644 --- a/NEWS.md +++ b/NEWS.md @@ -5,6 +5,11 @@ Bug fixes: * Fix support for long (>64k chars) queries in layergroup creation (#111) +Enhancements: + +* Enhance tools/show_style to accept an environment parameter and + print XML style now it is not in redis anymore (#110) + 1.6.1 -- 2014-01-15 ------------------- diff --git a/tools/show_style b/tools/show_style index b073f78a..d52ac9a9 100755 --- a/tools/show_style +++ b/tools/show_style @@ -2,28 +2,78 @@ # TODO: port to node, if you really need it -REDIS_PORT=6379 # default port - +ENV='development'; +BASEDIR=`cd $(dirname $0)/../; pwd` if test -z "$1"; then - echo "Usage: $0 [|~]" >&2 + echo "Usage: $0 [--env ] [|~]" >&2 + echo " environment defaults to 'development'" + exit 1 +fi + +username="" +token="" + +while test -n "$1"; do + if test "$1" = "--env"; then + shift; ENV="$1"; shift + elif test -z "$username"; then + username="$1"; shift + elif test -z "$token"; then + token="$1"; shift + else + echo "Unused option $1" >&2 + shift + fi +done + +echo "Using environment '${ENV}'" + +CONFIG="${BASEDIR}/config/environments/${ENV}.js" +REDIS_PORT=`node -e "console.log(require('${CONFIG}').redis.port)"` +if test $? -ne 0; then exit 1 fi -username="$1" -token="$2" dbname=`redis-cli -p ${REDIS_PORT} -n 5 hget "rails:users:${username}" "database_name"` if test $? -ne 0; then exit 1 fi if test -z "${dbname}"; then - echo "Username ${username} unknown by redis (try CARTODB/script/restore_redis?)" >&2 + echo "Username ${username} unknown by redis on port ${REDIS_PORT} (try CARTODB/script/restore_redis?)" >&2 exit 1 fi echo "Database name for user ${username}: ${dbname}" # only if verbose? if test -n "$token"; then - redis-cli get "map_style|${dbname}|${token}" | sed -e 's/\\n/\n/g' -e 's/\\//g' + rec=`redis-cli get "map_style|${dbname}|${token}"` + if test -z "${rec}"; then + echo "${token}: no such map style known by redis on port ${REDIS_PORT}" >&2 + exit 1 + fi + #echo "${rec}" + escrec=`echo "${rec}" | sed -e 's/\\\\/\\\\\\\\/g'` + #echo "${escrec}" + node < Date: Fri, 17 Jan 2014 17:47:37 +0100 Subject: [PATCH 16/16] Fix XML print from in show_style for token styles (#110) --- tools/show_style | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/tools/show_style b/tools/show_style index d52ac9a9..f2048906 100755 --- a/tools/show_style +++ b/tools/show_style @@ -63,7 +63,13 @@ global.environment = require('${CONFIG}'); var serverOptions = require('${BASEDIR}/lib/cartodb/server_options'); // _after_ setting global.environment var grainstore = require('${BASEDIR}/node_modules/windshaft/node_modules/grainstore/lib/grainstore'); var mml_store = new grainstore.MMLStore(serverOptions.redis, serverOptions.grainstore); -var mml_builder = mml_store.mml_builder({dbname:'${dbname}', table:'${token}'}, +var builderconfig = {dbname:'${dbname}'}; +if ( '${token}'.match(/^~/) ) { + builderconfig.token = '${token}'.substring(1); +} else { + builderconfig.table = '${token}'; +} +var mml_builder = mml_store.mml_builder(builderconfig, function(err, payload) { if ( err ) throw err; mml_builder.toXML(function(err, xml) {