From 5772c815900b683c8725f068ceba7887d16b4690 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 16 Jan 2014 17:20:30 +0100 Subject: [PATCH] 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); };