From 7fe09935cd10e242eedc34d1f767345246f160ae Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 9 Oct 2012 18:40:17 +0200 Subject: [PATCH 01/49] Restrict listening host to configured ``node_host`` --- app.js | 5 +++-- cluster.js | 6 ++++-- config/environments/development.js.example | 3 ++- package.json | 2 +- 4 files changed, 10 insertions(+), 6 deletions(-) diff --git a/app.js b/app.js index 21de716b..58c9cf6b 100755 --- a/app.js +++ b/app.js @@ -26,5 +26,6 @@ _.extend(global.settings, env); // kick off controller var app = require(global.settings.app_root + '/app/controllers/app'); -app.listen(global.settings.node_port); -console.log("CartoDB SQL API listening on port " + global.settings.node_port); \ No newline at end of file +app.listen(global.settings.node_port, global.settings.node_host, function() { + console.log("CartoDB SQL API listening on " + global.settings.node_host + ":" + global.settings.node_port); +}); diff --git a/cluster.js b/cluster.js index bc705630..30417979 100755 --- a/cluster.js +++ b/cluster.js @@ -30,11 +30,13 @@ var app = require(global.settings.app_root + '/app/controllers/app'); var cluster = new Cluster({ port: global.settings.node_port, + host: global.settings.node_host, + monHost: global.settings.node_host, monPort: global.settings.node_port+1 }); cluster.listen(function(cb) { cb(app); +}, function() { + console.log("CartoDB SQL API listening on " + global.settings.node_host + ':' + global.settings.node_port); }); - -console.log("CartoDB SQL API listening on port " + global.settings.node_port); diff --git a/config/environments/development.js.example b/config/environments/development.js.example index e18f3a33..17a81cbd 100644 --- a/config/environments/development.js.example +++ b/config/environments/development.js.example @@ -1,4 +1,5 @@ module.exports.node_port = 8080; +module.exports.node_host = '127.0.0.1'; module.exports.environment = 'development'; module.exports.db_base_name = 'cartodb_dev_user_<%= user_id %>_db'; module.exports.db_user = 'development_cartodb_user_<%= user_id %>'; @@ -9,4 +10,4 @@ module.exports.redis_port = 6379; module.exports.redisPool = 50; module.exports.redisIdleTimeoutMillis = 100; module.exports.redisReapIntervalMillis = 10; -module.exports.redisLog = false; \ No newline at end of file +module.exports.redisLog = false; diff --git a/package.json b/package.json index c09f3a2c..fc18f7ed 100644 --- a/package.json +++ b/package.json @@ -9,7 +9,7 @@ "email": "simon@vizzuality.com, strk@vizzuality.com" }, "dependencies": { - "cluster2": "git://github.com/CartoDB/cluster2.git#28cde11", + "cluster2": "git://github.com/CartoDB/cluster2.git#cdb_production", "express": "~2.5.11", "underscore" : "1.1.x", "underscore.string": "1.1.5", From b6f19eef493dfd06887bcbdc8d3c89048ddb46a6 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 9 Oct 2012 18:42:10 +0200 Subject: [PATCH 02/49] Shrinkwrapped current deps --- npm-shrinkwrap.json | 247 ++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 247 insertions(+) create mode 100644 npm-shrinkwrap.json diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json new file mode 100644 index 00000000..2862185d --- /dev/null +++ b/npm-shrinkwrap.json @@ -0,0 +1,247 @@ +{ + "name": "cartodb_api", + "version": "1.1.0", + "dependencies": { + "cluster2": { + "version": "0.3.5-cdb01", + "from": "git://github.com/CartoDB/cluster2.git#cdb_production", + "dependencies": { + "ejs": { + "version": "0.8.3" + }, + "npm": { + "version": "1.1.62", + "dependencies": { + "semver": { + "version": "1.0.14" + }, + "ini": { + "version": "1.0.4" + }, + "slide": { + "version": "1.1.3" + }, + "abbrev": { + "version": "1.0.3" + }, + "graceful-fs": { + "version": "1.1.14" + }, + "minimatch": { + "version": "0.2.6" + }, + "nopt": { + "version": "2.0.0" + }, + "rimraf": { + "version": "2.0.2" + }, + "request": { + "version": "2.9.203", + "from": "git://github.com/isaacs/request" + }, + "which": { + "version": "1.0.5" + }, + "tar": { + "version": "0.1.13" + }, + "fstream": { + "version": "0.1.19" + }, + "block-stream": { + "version": "0.0.6" + }, + "inherits": { + "version": "1.0.0", + "from": "git://github.com/isaacs/inherits" + }, + "mkdirp": { + "version": "0.3.4" + }, + "read": { + "version": "1.0.4", + "dependencies": { + "mute-stream": { + "version": "0.0.3" + } + } + }, + "lru-cache": { + "version": "2.0.4" + }, + "node-gyp": { + "version": "0.6.11" + }, + "fstream-npm": { + "version": "0.1.2", + "dependencies": { + "fstream-ignore": { + "version": "0.0.5" + } + } + }, + "uid-number": { + "version": "0.0.3" + }, + "archy": { + "version": "0.0.2" + }, + "chownr": { + "version": "0.0.1" + }, + "npmlog": { + "version": "0.0.2" + }, + "ansi": { + "version": "0.1.2" + }, + "npm-registry-client": { + "version": "0.2.7" + }, + "read-package-json": { + "version": "0.1.5" + }, + "read-installed": { + "version": "0.0.2" + }, + "glob": { + "version": "3.1.12" + }, + "init-package-json": { + "version": "0.0.5", + "dependencies": { + "promzard": { + "version": "0.2.0" + } + } + }, + "osenv": { + "version": "0.0.3" + }, + "lockfile": { + "version": "0.2.1" + }, + "retry": { + "version": "0.6.0" + }, + "couch-login": { + "version": "0.1.12" + }, + "once": { + "version": "1.1.1" + }, + "npmconf": { + "version": "0.0.16", + "dependencies": { + "config-chain": { + "version": "1.1.2", + "dependencies": { + "proto-list": { + "version": "1.2.2" + } + } + } + } + }, + "opener": { + "version": "1.3.0" + } + } + } + } + }, + "express": { + "version": "2.5.11", + "dependencies": { + "connect": { + "version": "1.9.2", + "dependencies": { + "formidable": { + "version": "1.0.11" + } + } + }, + "mime": { + "version": "1.2.4" + }, + "qs": { + "version": "0.4.2" + }, + "mkdirp": { + "version": "0.3.0" + } + } + }, + "underscore": { + "version": "1.1.7" + }, + "underscore.string": { + "version": "1.1.5", + "dependencies": { + "underscore": { + "version": "1.1.6" + } + } + }, + "pg": { + "version": "0.6.14", + "dependencies": { + "generic-pool": { + "version": "1.0.9" + } + } + }, + "generic-pool": { + "version": "1.0.12" + }, + "redis": { + "version": "0.7.1" + }, + "hiredis": { + "version": "0.1.14" + }, + "step": { + "version": "0.0.5" + }, + "oauth-client": { + "version": "0.2.0", + "dependencies": { + "node-uuid": { + "version": "1.1.0" + } + } + }, + "node-uuid": { + "version": "1.3.3" + }, + "csv": { + "version": "0.0.13" + }, + "mocha": { + "version": "1.2.1", + "dependencies": { + "commander": { + "version": "0.6.1" + }, + "growl": { + "version": "1.5.1" + }, + "jade": { + "version": "0.26.3", + "dependencies": { + "mkdirp": { + "version": "0.3.0" + } + } + }, + "diff": { + "version": "1.0.2" + }, + "debug": { + "version": "0.7.0" + } + } + } + } +} From 55d540a7bd657fec9454c55d5d93d3a35360ba1f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 10 Oct 2012 18:51:47 +0200 Subject: [PATCH 03/49] Add SVG to the list of supported formats --- doc/API.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/doc/API.md b/doc/API.md index 880e01cb..ceb72c09 100644 --- a/doc/API.md +++ b/doc/API.md @@ -12,7 +12,7 @@ Supported query string parameters: 'format': Specifies which format to use for the response. Supported formats: JSON (the default), GeoJSON, - csv + CSV, SVG 'dp': Number of digits after the decimal point. Only affects format=GeoJSON. From 0d91ab2c6a195f8a8c12028a8f1ba3f15f2e3896 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 12 Oct 2012 11:42:03 +0200 Subject: [PATCH 04/49] Survive multiple "format" parameters, only using last one --- app/controllers/app.js | 3 +-- test/acceptance/app.test.js | 13 +++++++++++++ 2 files changed, 14 insertions(+), 2 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 9e9f33bc..5a2c3b97 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -48,13 +48,12 @@ function handleQuery(req, res) { var database = req.query.database; // TODO: Depricate var limit = parseInt(req.query.rows_per_page); var offset = parseInt(req.query.page); - var format = req.query.format; + var format = _.isArray(req.query.format) ? req.query.format[req.query.format.length-1] : req.query.format; var dp = req.query.dp; // decimal point digits (defaults to 6) var gn = "the_geom"; // TODO: read from configuration file var svg_width = 1024.0; var svg_height = 768.0; - // sanitize and apply defaults to input dp = (dp === "" || _.isUndefined(dp)) ? '6' : dp; format = (format === "" || _.isUndefined(format)) ? null : format.toLowerCase(); diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 6dace8a6..8379b9d9 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -274,6 +274,19 @@ test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-di }); }); +test('the last format parameter is used, when multiple are used', function(done){ + assert.response(app, { + url: '/api/v1/sql?format=csv&q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /filename=cartodb-query.geojson/gi.test(cd)); + done(); + }); +}); + test('GET /api/v1/sql with SVG format', function(done){ var query = querystring.stringify({ q: "SELECT 1 as cartodb_id, ST_MakeLine(ST_MakePoint(10, 10), ST_MakePoint(1034, 778)) AS the_geom ", From ac837008109604862c9099fa14e7fa3201964996 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 12 Oct 2012 12:17:35 +0200 Subject: [PATCH 05/49] Send a 404 on unsupported format requested --- app/controllers/app.js | 17 ++++++++++++----- test/acceptance/app.test.js | 14 +++++++++++++- 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 5a2c3b97..e7206fbe 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -41,6 +41,10 @@ app.get('/api/v1/cachestatus', function(req, res) { handleCacheStatus(req, res) // request handlers function handleQuery(req, res) { + var supportedFormats = ['json', 'geojson', 'csv', 'svg']; + var svg_width = 1024.0; + var svg_height = 768.0; + // extract input var body = (req.body) ? req.body : {}; var sql = req.query.q || body.q; // HTTP GET and POST store in different vars @@ -48,24 +52,27 @@ function handleQuery(req, res) { var database = req.query.database; // TODO: Depricate var limit = parseInt(req.query.rows_per_page); var offset = parseInt(req.query.page); - var format = _.isArray(req.query.format) ? req.query.format[req.query.format.length-1] : req.query.format; + var format = _.isArray(req.query.format) ? _.last(req.query.format) : req.query.format; var dp = req.query.dp; // decimal point digits (defaults to 6) var gn = "the_geom"; // TODO: read from configuration file - var svg_width = 1024.0; - var svg_height = 768.0; // sanitize and apply defaults to input dp = (dp === "" || _.isUndefined(dp)) ? '6' : dp; - format = (format === "" || _.isUndefined(format)) ? null : format.toLowerCase(); + format = (format === "" || _.isUndefined(format)) ? 'json' : format.toLowerCase(); sql = (sql === "" || _.isUndefined(sql)) ? null : sql; database = (database === "" || _.isUndefined(database)) ? null : database; limit = (_.isNumber(limit)) ? limit : null; offset = (_.isNumber(offset)) ? offset * limit : null; + // setup step run var start = new Date().getTime(); try { + + if ( -1 === supportedFormats.indexOf(format) ) + throw new Error("Invalid format: " + format); + if (!_.isString(sql)) throw new Error("You must indicate a sql query"); // initialise MD5 key of sql for cache lookups @@ -132,7 +139,7 @@ function handleQuery(req, res) { // TODO: refactor formats to external object if (format === 'geojson'){ sql = ['SELECT *, ST_AsGeoJSON(the_geom,',dp,') as the_geom FROM (', sql, ') as foo'].join(""); - } else if (format === 'svg'){ + } else if (format === 'svg') { var svg_ratio = svg_width/svg_height; sql = 'WITH source AS ( ' + sql + '), extent AS ( ' + ' SELECT ST_Extent(' + gn + ') AS e FROM source ' diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 8379b9d9..7b488d38 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -274,7 +274,7 @@ test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-di }); }); -test('the last format parameter is used, when multiple are used', function(done){ +test('uses the last format parameter when multiple are used', function(done){ assert.response(app, { url: '/api/v1/sql?format=csv&q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', headers: {host: 'vizzuality.cartodb.com'}, @@ -287,6 +287,18 @@ test('the last format parameter is used, when multiple are used', function(done) }); }); +test('sends a 400 when an unsupported format is requested', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=unknown', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 400, res.body); + assert.deepEqual(JSON.parse(res.body), {"error":[ "Invalid format: unknown" ]}); + done(); + }); +}); + test('GET /api/v1/sql with SVG format', function(done){ var query = querystring.stringify({ q: "SELECT 1 as cartodb_id, ST_MakeLine(ST_MakePoint(10, 10), ST_MakePoint(1034, 778)) AS the_geom ", From 7b7a48103d45866461714544e452b570dd8a1a1e Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 12 Oct 2012 12:21:37 +0200 Subject: [PATCH 06/49] Updated with "format" parameter related changes --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index 183bb98c..ad691c6f 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ 1.1.0 (DD/MM/YY) ----- +* Only use last format parameter when multiple are requested +* Return a 400 response on unsupported format request * Fixed problem in cluster2 with pidfile name * SVG output format * Enhancement to the cdbsql tool: From 553146e6dd69a9c7e139d67aecfe73611f407fee Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Fri, 12 Oct 2012 12:57:03 +0200 Subject: [PATCH 07/49] Add consistency checking in packageResult about format value --- app/controllers/app.js | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index e7206fbe..b1faa5a2 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -191,8 +191,7 @@ function handleQuery(req, res) { toSVG(result.rows, gn, this); } else if (format === 'csv'){ toCSV(result, res, this); - } else { - // TODO: error out if 'format' resolves to an unsupported format ! + } else if ( format === 'json'){ var end = new Date().getTime(); var json_result = {'time' : (end - start)/1000}; @@ -201,6 +200,7 @@ function handleQuery(req, res) { return json_result; } + else throw new Error("Unexpected format in packageResults: " + format); }, function sendResults(err, out){ if (err) throw err; From 70616b5bea768c38b5ef22aa34910ffcdc3e8857 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 15 Oct 2012 13:06:28 +0200 Subject: [PATCH 08/49] Grant db owner privs on public table too --- test/test.sql | 1 + 1 file changed, 1 insertion(+) diff --git a/test/test.sql b/test/test.sql index c9db9360..e6c80902 100644 --- a/test/test.sql +++ b/test/test.sql @@ -133,6 +133,7 @@ CREATE INDEX test_table_the_geom_webmercator_idx_p ON private_table USING gist ( CREATE USER publicuser WITH PASSWORD ''; CREATE USER test_cartodb_user_1 WITH PASSWORD ''; +GRANT ALL ON TABLE untitle_table_4 TO test_cartodb_user_1; GRANT SELECT ON TABLE untitle_table_4 TO publicuser; GRANT ALL ON TABLE private_table TO test_cartodb_user_1; GRANT ALL ON SEQUENCE test_table_cartodb_id_seq_p TO test_cartodb_user_1 From 9c72f66fb355eac88e8a2e6cd04f77f378dc81a6 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 15 Oct 2012 13:15:48 +0200 Subject: [PATCH 09/49] Put a copy of CDB_* functions from cartodb to this repo One day we should source those script directly from the cartodb repo --- test/prepare_db.sh | 2 ++ test/support/CDB_QueryStatements.sql | 13 +++++++ test/support/CDB_QueryTables.sql | 54 ++++++++++++++++++++++++++++ test/test.sql | 25 ------------- 4 files changed, 69 insertions(+), 25 deletions(-) create mode 100644 test/support/CDB_QueryStatements.sql create mode 100644 test/support/CDB_QueryTables.sql diff --git a/test/prepare_db.sh b/test/prepare_db.sh index 909d7e95..b271dd01 100755 --- a/test/prepare_db.sh +++ b/test/prepare_db.sh @@ -34,6 +34,8 @@ echo "preparing postgres..." dropdb ${TEST_DB} # 2> /dev/null # error expected if doesn't exist, but not otherwise createdb -Ttemplate_postgis -EUTF8 ${TEST_DB} || die "Could not create test database" psql -f test.sql ${TEST_DB} +psql -f support/CDB_QueryStatements.sql ${TEST_DB} +psql -f support/CDB_QueryTables.sql ${TEST_DB} echo "preparing redis..." echo "HSET rails:users:vizzuality id 1" | redis-cli -p ${REDIS_PORT} -n 5 diff --git a/test/support/CDB_QueryStatements.sql b/test/support/CDB_QueryStatements.sql new file mode 100644 index 00000000..9aecc18b --- /dev/null +++ b/test/support/CDB_QueryStatements.sql @@ -0,0 +1,13 @@ +-- Return an array of statements found in the given query text +-- +-- Curtesy of Hubert Lubaczewski (depesz) +-- +CREATE OR REPLACE FUNCTION CDB_QueryStatements(query text) +RETURNS SETOF TEXT AS $$ + SELECT stmt FROM ( + SELECT btrim(q[1], E' \n\t\r;') as stmt FROM ( + SELECT regexp_matches( $1, $REG$((?:[^'"$;]+|"[^"]*"|'(?:[^']*|'')*'|(\$[^$]*\$).*?\2)+)$REG$, 'g' ) as q + ) i + ) j + WHERE stmt <> ''; +$$ language sql; diff --git a/test/support/CDB_QueryTables.sql b/test/support/CDB_QueryTables.sql new file mode 100644 index 00000000..e504c293 --- /dev/null +++ b/test/support/CDB_QueryTables.sql @@ -0,0 +1,54 @@ +-- Return an array of table names scanned by a given query +-- +-- Requires PostgreSQL 9.x+ +-- +CREATE OR REPLACE FUNCTION CDB_QueryTables(query text) +RETURNS name[] +AS $$ +DECLARE + exp XML; + tables NAME[]; + rec RECORD; + rec2 RECORD; +BEGIN + + tables := '{}'; + + FOR rec IN SELECT CDB_QueryStatements(query) q LOOP + + BEGIN + EXECUTE 'EXPLAIN (FORMAT XML) ' || rec.q INTO STRICT exp; + EXCEPTION WHEN others THEN + RAISE WARNING 'Cannot explain query: % (%)', rec.q, SQLERRM; + CONTINUE; + END; + + -- Now need to extract all values of + + --RAISE DEBUG 'Explain: %', exp; + + FOR rec2 IN WITH + inp AS ( SELECT xpath('//x:Relation-Name/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]) as x ) + SELECT unnest(x)::name as p from inp + LOOP + --RAISE DEBUG 'tab: %', rec2.p; + tables := array_append(tables, rec2.p); + END LOOP; + + -- RAISE DEBUG 'Tables: %', tables; + + END LOOP; + + -- RAISE DEBUG 'Tables: %', tables; + + -- Remove duplicates and sort by name + IF array_upper(tables, 1) > 0 THEN + WITH dist as ( SELECT DISTINCT unnest(tables)::text as p ORDER BY p ) + SELECT array_agg(p) from dist into tables; + END IF; + + --RAISE DEBUG 'Tables: %', tables; + + return tables; +END +$$ LANGUAGE 'plpgsql' VOLATILE STRICT; diff --git a/test/test.sql b/test/test.sql index e6c80902..11ad0c1f 100644 --- a/test/test.sql +++ b/test/test.sql @@ -20,31 +20,6 @@ SET search_path = public, pg_catalog; SET default_tablespace = ''; SET default_with_oids = false; - --- Return an array of table names used by a given query -CREATE OR REPLACE FUNCTION CDB_QueryTables(query text) -RETURNS name[] -AS $$ -DECLARE - exp XML; - tables NAME[]; -BEGIN - - EXECUTE 'EXPLAIN (FORMAT XML) ' || query INTO STRICT exp; - - -- Now need to extract all values of - - --RAISE DEBUG 'Explain: %', exp; - - tables := xpath('//x:Relation-Name/text()', exp, ARRAY[ARRAY['x', 'http://www.postgresql.org/2009/explain']]); - - --RAISE DEBUG 'Tables: %', tables; - - return tables; -END -$$ LANGUAGE 'plpgsql' VOLATILE STRICT; - - -- first table DROP TABLE IF EXISTS untitle_table_4; CREATE TABLE untitle_table_4 ( From d23416cc6064570cd4996e8a41d42fc4dd24bcdc Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 15 Oct 2012 13:20:37 +0200 Subject: [PATCH 10/49] Set X-Cache-Channel to NONE when the SQL may write to the database Note that "may write" allows for false positive, so there could be less cache hits than possibly allowable. If this will be a problem for any real use case we could still improve the regular expression used to detect "writing" queries. Automated tests are added to check for the X-Cache-Channel header with both writing and read-only queries performed by authenticated requests. Closes #27 Closes #43 --- app/controllers/app.js | 36 +++++++++-- test/acceptance/app.test.js | 126 +++++++++++++++++++++++++++++++++++- 2 files changed, 154 insertions(+), 8 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index b1faa5a2..bc7f6752 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -38,6 +38,21 @@ app.all('/api/v1/sql', function(req, res) { handleQuery(req, res) } ); app.all('/api/v1/sql.:f', function(req, res) { handleQuery(req, res) } ); app.get('/api/v1/cachestatus', function(req, res) { handleCacheStatus(req, res) } ); +// Return true of the given query may write to the database +// +// NOTE: this is a fuzzy check, the return could be true even +// if the query doesn't really write anything. +// But you can be pretty sure of a false return. +// +function queryMayWrite(sql) { + var mayWrite = false; + var pattern = RegExp("(insert|update|delete|create|drop)", "i"); + if ( pattern.test(sql) ) { + mayWrite = true; + } + return mayWrite; +} + // request handlers function handleQuery(req, res) { @@ -81,6 +96,8 @@ function handleQuery(req, res) { // placeholder for connection var pg; + var authenticated; + // 1. Get database from redis via the username stored in the host header subdomain // 2. Run the request through OAuth to get R/W user id if signed // 3. Get the list of tables affected by the query @@ -119,12 +136,14 @@ function handleQuery(req, res) { // store postgres connection pg = new PSQL(user_id, database, limit, offset); + authenticated = ! _.isNull(user_id); + // get all the tables from Cache or SQL if (!_.isNull(tableCache[sql_md5]) && !_.isUndefined(tableCache[sql_md5])){ tableCache[sql_md5].hits++; return true; - } else{ - pg.query("SELECT CDB_QueryTables($quotesql$" + sql + "$quotesql$)", this); + } else { + pg.query("SELECT CDB_QueryTables($quotesql$" + sql + "$quotesql$)", this); } }, function queryResult(err, result){ @@ -133,6 +152,7 @@ function handleQuery(req, res) { // store explain result in local Cache if (_.isUndefined(tableCache[sql_md5])){ tableCache[sql_md5] = result; + tableCache[sql_md5].may_write = queryMayWrite(sql); tableCache[sql_md5].hits = 1; //initialise hit counter } @@ -176,8 +196,8 @@ function handleQuery(req, res) { // set cache headers res.header('Last-Modified', new Date().toUTCString()); - res.header('Cache-Control', 'no-cache,max-age=3600,must-revalidate, public'); - res.header('X-Cache-Channel', generateCacheKey(database, tableCache[sql_md5])); + res.header('Cache-Control', 'no-cache,max-age=3600,must-revalidate,public'); + res.header('X-Cache-Channel', generateCacheKey(database, tableCache[sql_md5], authenticated)); return result; }, @@ -392,8 +412,12 @@ function setCrossDomain(res){ res.header("Access-Control-Allow-Headers", "X-Requested-With"); } -function generateCacheKey(database,tables){ - return database + ":" + tables.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; +function generateCacheKey(database,tables,is_authenticated){ + if ( is_authenticated && tables.may_write ) { + return "NONE"; + } else { + return database + ":" + tables.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; + } } function generateMD5(data){ diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 7b488d38..6bff0872 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -24,7 +24,7 @@ app.setMaxListeners(0); suite('app.test', function() { -var real_oauth_header = 'OAuth realm="http://vizzuality.testhost.lan/",oauth_consumer_key="fZeNGv5iYayvItgDYHUbot1Ukb5rVyX6QAg8GaY2",oauth_token="l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR",oauth_signature_method="HMAC-SHA1", oauth_signature="o4hx4hWP6KtLyFwggnYB4yPK8xI%3D",oauth_timestamp="1313581372",oauth_nonce="W0zUmvyC4eVL8cBd4YwlH1nnPTbxW0QBYcWkXTwe4",oauth_version="1.0"'; +var expected_cache_control = 'no-cache,max-age=3600,must-revalidate,public'; // use dec_sep for internationalization var checkDecimals = function(x, dec_sep){ @@ -54,11 +54,14 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. No oAuth included ', fu method: 'GET' },{ }, function(res) { assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); - test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4', @@ -70,6 +73,22 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just }); }); +test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers. Authenticated.', +function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20cartodb_id*2%20FROM%20untitle_table_4&api_key=1234', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + test('POST /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers', function(done){ assert.response(app, { @@ -129,6 +148,10 @@ test('INSERT returns affected rows', function(done){ assert.ok(out.hasOwnProperty('time')); assert.equal(out.total_rows, 2); assert.equal(out.rows.length, 0); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); @@ -151,6 +174,10 @@ test('UPDATE returns affected rows', function(done){ assert.ok(out.hasOwnProperty('time')); assert.equal(out.total_rows, 2); assert.equal(out.rows.length, 0); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); @@ -173,6 +200,10 @@ test('DELETE returns affected rows', function(done){ assert.ok(out.hasOwnProperty('time')); assert.equal(out.total_rows, 2); assert.equal(out.rows.length, 0); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); done(); }); }); @@ -261,6 +292,97 @@ test('GET /api/v1/sql with SQL parameter on DROP DATABASE only.header based db - }); }); +test('CREATE TABLE with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'CREATE TABLE create_table_test(a int)', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + +// TODO: test COPY +//test('COPY TABLE with GET and auth', function(done){ +// assert.response(app, { +// url: "/api/v1/sql?" + querystring.stringify({ +// q: 'COPY TABLE create_table_test FROM stdin; 1\n\\.\n', +// api_key: 1234 +// }), +// headers: {host: 'vizzuality.cartodb.com'}, +// method: 'GET' +// },{}, function(res) { +// assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); +// // Check cache headers +// // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 +// assert.equal(res.headers['x-cache-channel'], 'NONE'); +// assert.equal(res.headers['cache-control'], expected_cache_control); +// done(); +// }); +//}); + +test('DROP TABLE with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'DROP TABLE create_table_test', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + +test('CREATE FUNCTION with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'CREATE FUNCTION create_func_test(a int) RETURNS INT AS \'SELECT 1\' LANGUAGE \'sql\'', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + +test('DROP FUNCTION with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'DROP FUNCTION create_func_test(a int)', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-disposition set to geojson', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', From d9b733e5c64663289bfa6d7015dd2b84e93b5c24 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 15 Oct 2012 13:40:04 +0200 Subject: [PATCH 11/49] Recognize ALTER as a writing query --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 24 +++++++++++++++++++++--- 2 files changed, 22 insertions(+), 4 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index bc7f6752..7cc47c3f 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -46,7 +46,7 @@ app.get('/api/v1/cachestatus', function(req, res) { handleCacheStatus(req, res) // function queryMayWrite(sql) { var mayWrite = false; - var pattern = RegExp("(insert|update|delete|create|drop)", "i"); + var pattern = RegExp("(alter|insert|update|delete|create|drop)", "i"); if ( pattern.test(sql) ) { mayWrite = true; } diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 6bff0872..89396a93 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -295,7 +295,7 @@ test('GET /api/v1/sql with SQL parameter on DROP DATABASE only.header based db - test('CREATE TABLE with GET and auth', function(done){ assert.response(app, { url: "/api/v1/sql?" + querystring.stringify({ - q: 'CREATE TABLE create_table_test(a int)', + q: 'CREATE TABLE test_table(a int)', api_key: 1234 }), headers: {host: 'vizzuality.cartodb.com'}, @@ -314,7 +314,7 @@ test('CREATE TABLE with GET and auth', function(done){ //test('COPY TABLE with GET and auth', function(done){ // assert.response(app, { // url: "/api/v1/sql?" + querystring.stringify({ -// q: 'COPY TABLE create_table_test FROM stdin; 1\n\\.\n', +// q: 'COPY TABLE test_table FROM stdin; 1\n\\.\n', // api_key: 1234 // }), // headers: {host: 'vizzuality.cartodb.com'}, @@ -329,10 +329,28 @@ test('CREATE TABLE with GET and auth', function(done){ // }); //}); +test('ALTER TABLE with GET and auth', function(done){ + assert.response(app, { + url: "/api/v1/sql?" + querystring.stringify({ + q: 'ALTER TABLE test_table ADD b int', + api_key: 1234 + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'NONE'); + assert.equal(res.headers['cache-control'], expected_cache_control); + done(); + }); +}); + test('DROP TABLE with GET and auth', function(done){ assert.response(app, { url: "/api/v1/sql?" + querystring.stringify({ - q: 'DROP TABLE create_table_test', + q: 'DROP TABLE test_table', api_key: 1234 }), headers: {host: 'vizzuality.cartodb.com'}, From d399d2153f73e182418d7717fa97fa218dcccdf5 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 18 Oct 2012 11:32:08 +0200 Subject: [PATCH 12/49] Improve input data control in test for "dp" parameter --- test/acceptance/app.test.js | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 89396a93..048464b4 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -544,7 +544,10 @@ test('GET /api/v1/sql ensure cross domain set on errors', function(done){ test('GET /api/v1/sql as geojson limiting decimal places', function(done){ assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson&dp=1', + url: '/api/v1/sql?' + querystring.stringify({ + q: 'SELECT ST_MakePoint(0.123,2.3456) as the_geom', + format: 'geojson', + dp: '1'}), headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{ }, function(res){ @@ -557,7 +560,9 @@ test('GET /api/v1/sql as geojson limiting decimal places', function(done){ test('GET /api/v1/sql as geojson with default dp as 6', function(done){ assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', + url: '/api/v1/sql?' + querystring.stringify({ + q: 'SELECT ST_MakePoint(0.12345678,2.3456787654) as the_geom', + format: 'geojson'}), headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{ }, function(res){ From 8574517ab830aa1fa9602e26bfe01c7a2562524f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 18 Oct 2012 11:33:35 +0200 Subject: [PATCH 13/49] Add a userid_to_dbuser function --- app/controllers/app.js | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/app/controllers/app.js b/app/controllers/app.js index 7cc47c3f..6dafd9a0 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -53,6 +53,15 @@ function queryMayWrite(sql) { return mayWrite; } +// Return database username from user_id +// NOTE: a "null" user_id is a request to use the public user +function userid_to_dbuser(user_id) { + if ( _.isString(user_id) ) + return _.template(global.settings.db_user, {user_id: user_id}); + return "publicuser" // FIXME: make configurable +}; + + // request handlers function handleQuery(req, res) { From 729462b500722843ddadecf13086ca3ae6376f0f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 18 Oct 2012 11:34:39 +0200 Subject: [PATCH 14/49] Fix throw in async function (verifyRequest) --- app/models/apikey_auth.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/app/models/apikey_auth.js b/app/models/apikey_auth.js index 9cc54846..3d41dd97 100644 --- a/app/models/apikey_auth.js +++ b/app/models/apikey_auth.js @@ -88,7 +88,7 @@ module.exports = (function() { * Get privacy for cartodb table * * @param req - standard req object. Importantly contains table and host information - * @param callback - user_id if ok, null if auth fails + * @param callback - err, user_id (null if no auth) */ me.verifyRequest = function(req, callback) { var that = this; @@ -108,8 +108,8 @@ module.exports = (function() { } }, function (err, user_id){ - if (err) throw err; - callback(false, user_id); + if (err) callback(err); + else callback(false, user_id); } ); }; From b038419abdd03bd35be0f9adc58f2f20391c7e90 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 18 Oct 2012 11:35:06 +0200 Subject: [PATCH 15/49] Add missing newline --- app/models/metadata.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/metadata.js b/app/models/metadata.js index 36abf647..518d3a5f 100644 --- a/app/models/metadata.js +++ b/app/models/metadata.js @@ -72,4 +72,4 @@ module.exports = function() { }; return me; -}(); \ No newline at end of file +}(); From d0ae7e08a6e2c1539d586f902275b6248f2064db Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 15 Oct 2012 10:13:39 +0200 Subject: [PATCH 16/49] Initial support for Shapefile output --- app/controllers/app.js | 197 +++++++++++++++++++++++++++++++++++- doc/API.md | 2 +- test/acceptance/app.test.js | 30 ++++++ 3 files changed, 225 insertions(+), 4 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 6dafd9a0..4b9198a8 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -23,6 +23,9 @@ var express = require('express') , Step = require('step') , csv = require('csv') , crypto = require('crypto') + , fs = require('fs') + , zlib = require('zlib') + , spawn = require('child_process').spawn , Meta = require(global.settings.app_root + '/app/models/metadata') , oAuth = require(global.settings.app_root + '/app/models/oauth') , PSQL = require(global.settings.app_root + '/app/models/psql') @@ -65,7 +68,7 @@ function userid_to_dbuser(user_id) { // request handlers function handleQuery(req, res) { - var supportedFormats = ['json', 'geojson', 'csv', 'svg']; + var supportedFormats = ['json', 'geojson', 'csv', 'svg', 'shp']; var svg_width = 1024.0; var svg_height = 768.0; @@ -79,6 +82,7 @@ function handleQuery(req, res) { var format = _.isArray(req.query.format) ? _.last(req.query.format) : req.query.format; var dp = req.query.dp; // decimal point digits (defaults to 6) var gn = "the_geom"; // TODO: read from configuration file + var user_id; // sanitize and apply defaults to input dp = (dp === "" || _.isUndefined(dp)) ? '6' : dp; @@ -140,8 +144,9 @@ function handleQuery(req, res) { oAuth.verifyRequest(req, this); } }, - function queryExplain(err, user_id){ + function queryExplain(err, data){ if (err) throw err; + user_id = data; // store postgres connection pg = new PSQL(user_id, database, limit, offset); @@ -168,6 +173,8 @@ function handleQuery(req, res) { // TODO: refactor formats to external object if (format === 'geojson'){ sql = ['SELECT *, ST_AsGeoJSON(the_geom,',dp,') as the_geom FROM (', sql, ') as foo'].join(""); + } else if (format === 'shp') { + return null; } else if (format === 'svg') { var svg_ratio = svg_width/svg_height; sql = 'WITH source AS ( ' + sql + '), extent AS ( ' @@ -220,6 +227,8 @@ function handleQuery(req, res) { toSVG(result.rows, gn, this); } else if (format === 'csv'){ toCSV(result, res, this); + } else if ( format === 'shp'){ + toSHP(database, user_id, gn, sql, res, this); } else if ( format === 'json'){ var end = new Date().getTime(); @@ -235,7 +244,7 @@ function handleQuery(req, res) { if (err) throw err; // return to browser - res.send(out); + if ( out ) res.send(out); }, function errorHandle(err, result){ handleException(err, res); @@ -390,6 +399,182 @@ function toCSV(data, res, callback){ } } +function toSHP(dbname, user_id, gcol, sql, res, callback) { + var zip = 'zip'; // FIXME: make configurable + var ogr2ogr = 'ogr2ogr'; // FIXME: make configurable + var tmpdir = '/tmp'; // FIXME: make configurable + var dbhost = global.settings.db_host; + var dbport = global.settings.db_port; + var dbpass = ''; // turn into a parameter.. + var outdirpath = tmpdir + '/shapefile-' + generateMD5(sql); + var shapefile = outdirpath + '/cartodb-query.shp'; + var dbuser = userid_to_dbuser(user_id); + var columns = []; + + // TODO: following tests: + // - fetch with no auth [done] + // - fetch with auth [done] + // - fetch same query concurrently + // - fetch query with no "the_geom" column + + // TODO: Check if the file already exists + // (should mean another export of the same query is in progress) + + Step ( + + function createOutDir() { + fs.mkdir(outdirpath, 0777, this); + + }, + function fetchColumns(err) { + if ( err ) throw err; + var next = this; + var colsql = 'SELECT * FROM (' + sql + ') as _cartodbsqlapi LIMIT 1'; + var pg = new PSQL(user_id, dbname, 1, 0); + pg.query(colsql, this); + }, + function spawnDumper(err, result) { + if (err) throw err; + + if ( ! result.rows.length ) + throw new Error("Query returns no rows"); + + // Skip system columns + for (var k in result.rows[0]) { + if ( k === "the_geom_webmercator" ) continue; + columns.push('"' + k + '"'); + } + console.log(columns.join(',')); + + var next = this; + + // TODO: force epsg:4326 SRID as we know that's what we want anyway + sql = 'SELECT ' + columns.join(',') + + ' FROM (' + sql + ') as _cartodbsqlapi'; + + var child = spawn(ogr2ogr, [ + '-f', 'ESRI Shapefile', + shapefile, + "PG:host=" + dbhost + + " user=" + dbuser + + " dbname=" + dbname + + " password=" + dbpass + + " tables=fake" // trick to skip query to geometry_columns + + "", + '-sql', sql // WARNING! should we quote the sql ? + ]); + +/* +console.log(['ogr2ogr', + '-f', '"ESRI Shapefile"', + shapefile, + "'PG:host=" + dbhost + + " user=" + dbuser + + " dbname=" + dbname + + " password=" + dbpass + + " tables=fake" // trick to skip query to geometry_columns + + "'", + '-sql "', sql, '"'].join(' ')); +*/ + + + var stdout = ''; + child.stdout.on('data', function(data) { + stdout += data; + //console.log('stdout: ' + data); + }); + + var stderr = ''; + child.stderr.on('data', function(data) { + stderr += data; + console.log('ogr2ogr stderr: ' + data); + }); + + child.on('exit', function(code) { + if ( code ) { + next(new Error("ogr2ogr returned an error (error code " + code + ")\n" + stderr)); + } else { + next(null, outdirpath); + } + }); + }, + function zipAndSendDump(err, dir) { + if ( err ) throw err; + + var next = this; + + var zipfile = dir + '.zip'; + + var child = spawn(zip, ['-qrj', '-', dir ]); + + child.stdout.on('data', function(data) { + res.write(data); + }); + + var stderr = ''; + child.stderr.on('data', function(data) { + stderr += data; + console.log('zip stderr: ' + data); + }); + + child.on('exit', function(code) { + if (code) { + res.statusCode = 500; + //res.send(stderr); + } + //console.log("Zip complete, zip return code was " + code); + next(null); + }); + + }, + function cleanupDir(topError) { + + var next = this; + + //console.log("Cleaning up " + outdirpath); + + // Unlink the dir content + var unlinkall = function(dir, files, finish) { + var f = files.shift(); + if ( ! f ) { finish(null); return; } + var fn = dir + '/' + f; + fs.unlink(fn, function(err) { + if ( err ) { + console.log("Unlinking " + fn + ": " + err); + finish(err); + } + else unlinkall(dir, files, finish) + }); + } + fs.readdir(outdirpath, function(err, files) { + if ( err ) { + if ( err.code != 'ENOENT' ) { + next(new Error([topError, err].join('\n'))); + } else { + next(topError); + } + } else { + unlinkall(outdirpath, files, function(err) { + fs.rmdir(outdirpath, function(err) { + if ( err ) console.log("Removing dir " + path + ": " + err); + next(topError); + }); + }); + } + }); + }, + function finish(err) { + if ( err ) callback(err); + else { + res.end(); + callback(null); + } + + } + ); + +} + function getContentDisposition(format){ var ext = 'json'; if (format === 'geojson'){ @@ -401,6 +586,9 @@ function getContentDisposition(format){ else if (format === 'svg'){ ext = 'svg'; } + else if (format === 'shp'){ + ext = 'zip'; + } var time = new Date().toUTCString(); return 'inline; filename=cartodb-query.' + ext + '; modification-date="' + time + '";'; } @@ -413,6 +601,9 @@ function getContentType(format){ else if (format === 'svg'){ type = "image/svg+xml; charset=utf-8"; } + else if (format === 'shp'){ + type = "application/zip; charset=utf-8"; + } return type; } diff --git a/doc/API.md b/doc/API.md index ceb72c09..751f8ad5 100644 --- a/doc/API.md +++ b/doc/API.md @@ -12,7 +12,7 @@ Supported query string parameters: 'format': Specifies which format to use for the response. Supported formats: JSON (the default), GeoJSON, - CSV, SVG + CSV, SVG, SHP 'dp': Number of digits after the decimal point. Only affects format=GeoJSON. diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 048464b4..8f12aab0 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -653,4 +653,34 @@ test('CSV format', function(done){ }); }); +// SHP tests + +test('SHP format, unauthenticated', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /filename=cartodb-query.zip/gi.test(cd)); + // TODO: check for actual content, at least try to uncompress.. + done(); + }); +}); + +test('SHP format, authenticated', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp&api_key=1234', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /filename=cartodb-query.zip/gi.test(cd)); + // TODO: check for actual content, at least try to uncompress.. + done(); + }); +}); + }); From e7c172499939db23e8f6655a98ccaf4696ffb3db Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 18 Oct 2012 12:56:16 +0200 Subject: [PATCH 17/49] Advertise new SHP format in NEWS file (closes #53) --- NEWS.md | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index ad691c6f..e9bbc6c6 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,9 +1,11 @@ 1.1.0 (DD/MM/YY) ----- +* New output formats: + * ESRI Shapefile (format=shp) + * SVG (format=svg) * Only use last format parameter when multiple are requested * Return a 400 response on unsupported format request * Fixed problem in cluster2 with pidfile name -* SVG output format * Enhancement to the cdbsql tool: - New switches: --format, --key, --dp - Interactive mode From 26bdccf54141a45a9f941e368b8dfb14d929e9a9 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 18 Oct 2012 13:19:08 +0200 Subject: [PATCH 18/49] Remove debugging output --- app/controllers/app.js | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 4b9198a8..2dde1f76 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -444,11 +444,10 @@ function toSHP(dbname, user_id, gcol, sql, res, callback) { if ( k === "the_geom_webmercator" ) continue; columns.push('"' + k + '"'); } - console.log(columns.join(',')); + //console.log(columns.join(',')); var next = this; - // TODO: force epsg:4326 SRID as we know that's what we want anyway sql = 'SELECT ' + columns.join(',') + ' FROM (' + sql + ') as _cartodbsqlapi'; From a560a3782395b82386b9f89800f887cc8a8eb6fe Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 23 Oct 2012 17:45:21 +0200 Subject: [PATCH 19/49] Generalize OGR output function --- app/controllers/app.js | 74 +++++++++++++++++++++++++----------------- 1 file changed, 44 insertions(+), 30 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 2dde1f76..865418c7 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -399,36 +399,21 @@ function toCSV(data, res, callback){ } } -function toSHP(dbname, user_id, gcol, sql, res, callback) { - var zip = 'zip'; // FIXME: make configurable +// Internal function usable by all OGR-driven outputs +function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callback) { var ogr2ogr = 'ogr2ogr'; // FIXME: make configurable - var tmpdir = '/tmp'; // FIXME: make configurable var dbhost = global.settings.db_host; var dbport = global.settings.db_port; - var dbpass = ''; // turn into a parameter.. - var outdirpath = tmpdir + '/shapefile-' + generateMD5(sql); - var shapefile = outdirpath + '/cartodb-query.shp'; var dbuser = userid_to_dbuser(user_id); + var dbpass = ''; // turn into a parameter.. + + var tmpdir = '/tmp'; // FIXME: make configurable + var outdirpath = tmpdir + '/shapefile-' + generateMD5(sql); var columns = []; - // TODO: following tests: - // - fetch with no auth [done] - // - fetch with auth [done] - // - fetch same query concurrently - // - fetch query with no "the_geom" column - - // TODO: Check if the file already exists - // (should mean another export of the same query is in progress) - Step ( - function createOutDir() { - fs.mkdir(outdirpath, 0777, this); - - }, - function fetchColumns(err) { - if ( err ) throw err; - var next = this; + function fetchColumns() { var colsql = 'SELECT * FROM (' + sql + ') as _cartodbsqlapi LIMIT 1'; var pg = new PSQL(user_id, dbname, 1, 0); pg.query(colsql, this); @@ -452,21 +437,21 @@ function toSHP(dbname, user_id, gcol, sql, res, callback) { + ' FROM (' + sql + ') as _cartodbsqlapi'; var child = spawn(ogr2ogr, [ - '-f', 'ESRI Shapefile', - shapefile, + '-f', out_format, + out_filename, "PG:host=" + dbhost + " user=" + dbuser + " dbname=" + dbname + " password=" + dbpass + " tables=fake" // trick to skip query to geometry_columns + "", - '-sql', sql // WARNING! should we quote the sql ? + '-sql', sql ]); /* console.log(['ogr2ogr', - '-f', '"ESRI Shapefile"', - shapefile, + '-f', out_format, + out_filename, "'PG:host=" + dbhost + " user=" + dbuser + " dbname=" + dbname @@ -476,7 +461,6 @@ console.log(['ogr2ogr', '-sql "', sql, '"'].join(' ')); */ - var stdout = ''; child.stdout.on('data', function(data) { stdout += data; @@ -497,10 +481,41 @@ console.log(['ogr2ogr', } }); }, - function zipAndSendDump(err, dir) { + function finish(err) { + callback(err); + } + ); +} + +function toSHP(dbname, user_id, gcol, sql, res, callback) { + var zip = 'zip'; // FIXME: make configurable + var tmpdir = '/tmp'; // FIXME: make configurable + var outdirpath = tmpdir + '/shapefile-' + generateMD5(sql); + var shapefile = outdirpath + '/cartodb-query.shp'; + + // TODO: following tests: + // - fetch with no auth [done] + // - fetch with auth [done] + // - fetch same query concurrently + // - fetch query with no "the_geom" column + + // TODO: Check if the file already exists + // (should mean another export of the same query is in progress) + + Step ( + + function createOutDir() { + fs.mkdir(outdirpath, 0777, this); + }, + function spawnDumper(err) { + if ( err ) throw err; + toOGR(dbname, user_id, gcol, sql, res, 'ESRI Shapefile', shapefile, this); + }, + function zipAndSendDump(err) { if ( err ) throw err; var next = this; + var dir = outdirpath; var zipfile = dir + '.zip'; @@ -571,7 +586,6 @@ console.log(['ogr2ogr', } ); - } function getContentDisposition(format){ From 5d8eccc81e03849d7d84b750f464064f2b7ad269 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 25 Oct 2012 13:05:37 +0200 Subject: [PATCH 20/49] Fix CSV output with no rows. Closes #60 --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 14 ++++++++++++++ 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 865418c7..177832b2 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -387,7 +387,7 @@ function toSVG(rows, gn, callback){ function toCSV(data, res, callback){ try{ // pull out keys for column headers - var columns = _.keys(data.rows[0]); + var columns = data.rows.length ? _.keys(data.rows[0]) : []; // stream the csv out over http csv() diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 8f12aab0..2ca0a916 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -586,6 +586,20 @@ test('GET /api/v1/sql as csv', function(done){ }); }); +// See https://github.com/Vizzuality/CartoDB-SQL-API/issues/60 +test('GET /api/v1/sql as csv with no rows', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20true%20WHERE%20false&format=csv', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var body = ""; + assert.equal(body, res.body); + done(); + }); +}); + test('GET /api/v1/sql as csv, properly escaped', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20cartodb_id,%20address%20FROM%20untitle_table_4%20LIMIT%201&format=csv', From a6837573c5f3b6891ae27031cd402709a31631c1 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 25 Oct 2012 13:34:06 +0200 Subject: [PATCH 21/49] Use "attachment" Content-Disposition for all output formats. Closes #61, includes tests NOTE: this includes the default "json" format. --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 323 +++++++++++++++++++----------------- 2 files changed, 169 insertions(+), 156 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 177832b2..3fbc5c11 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -603,7 +603,7 @@ function getContentDisposition(format){ ext = 'zip'; } var time = new Date().toUTCString(); - return 'inline; filename=cartodb-query.' + ext + '; modification-date="' + time + '";'; + return 'attachment; filename=cartodb-query.' + ext + '; modification-date="' + time + '";'; } function getContentType(format){ diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 2ca0a916..47d49332 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -401,6 +401,88 @@ test('DROP FUNCTION with GET and auth', function(done){ }); }); +test('sends a 400 when an unsupported format is requested', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=unknown', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 400, res.body); + assert.deepEqual(JSON.parse(res.body), {"error":[ "Invalid format: unknown" ]}); + done(); + }); +}); + +test('GET /api/v1/sql with SQL parameter and no format, ensuring content-disposition set to json', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'JSON is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.json/gi.test(cd)); + done(); + }); +}); + +test('GET /api/v1/sql ensure cross domain set on errors', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*gadfgadfg%20FROM%20untitle_table_4', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ + status: 400 + }, function(res){ + var cd = res.header('Access-Control-Allow-Origin'); + assert.equal(cd, '*'); + done(); + }); +}); + +test('cannot GET system tables', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20pg_attribute', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ + status: 403 + }, function() { done(); }); +}); + +test('GET decent error if domain is incorrect', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', + headers: {host: 'vizzualinot.cartodb.com'}, + method: 'GET' + },{ + status: 404 + }, function(res){ + var result = JSON.parse(res.body); + assert.equal(result.error[0],"Sorry, we can't find this CartoDB. Please check that you have entered the correct domain."); + done(); + }); +}); + +test('GET decent error if SQL is broken', function(done){ + assert.response(app, { + url: '/api/v1/sql?' + querystring.stringify({q: + 'SELECT star FROM this and that' + }), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{}, function(res){ + assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); + var result = JSON.parse(res.body); + // NOTE: actual error message may be slighly different, possibly worth a regexp here + assert.equal(result.error[0], 'syntax error at or near "and"'); + done(); + }); +}); + +// GEOJSON tests + test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-disposition set to geojson', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', @@ -409,6 +491,7 @@ test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-di },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'GEOJSON is not disposed as attachment: ' + cd); assert.equal(true, /filename=cartodb-query.geojson/gi.test(cd)); done(); }); @@ -427,18 +510,97 @@ test('uses the last format parameter when multiple are used', function(done){ }); }); -test('sends a 400 when an unsupported format is requested', function(done){ + +test('GET /api/v1/sql as geojson limiting decimal places', function(done){ assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=unknown', + url: '/api/v1/sql?' + querystring.stringify({ + q: 'SELECT ST_MakePoint(0.123,2.3456) as the_geom', + format: 'geojson', + dp: '1'}), headers: {host: 'vizzuality.cartodb.com'}, method: 'GET' },{ }, function(res){ - assert.equal(res.statusCode, 400, res.body); - assert.deepEqual(JSON.parse(res.body), {"error":[ "Invalid format: unknown" ]}); + assert.equal(res.statusCode, 200, res.body); + var result = JSON.parse(res.body); + assert.equal(1, checkDecimals(result.features[0].geometry.coordinates[0], '.')); done(); }); }); +test('GET /api/v1/sql as geojson with default dp as 6', function(done){ + assert.response(app, { + url: '/api/v1/sql?' + querystring.stringify({ + q: 'SELECT ST_MakePoint(0.12345678,2.3456787654) as the_geom', + format: 'geojson'}), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var result = JSON.parse(res.body); + assert.equal(6, checkDecimals(result.features[0].geometry.coordinates[0], '.')); + done(); + }); +}); + + +// CSV tests + +test('CSV format', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=csv', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'CSV is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.csv/gi.test(cd)); + done(); + }); +}); + +test('GET /api/v1/sql as csv', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20cartodb_id,ST_AsEWKT(the_geom)%20as%20geom%20FROM%20untitle_table_4%20LIMIT%201&format=csv', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var body = "cartodb_id,geom\r\n1,SRID=4326;POINT(-3.699732 40.423012)"; + assert.equal(body, res.body); + done(); + }); +}); + +// See https://github.com/Vizzuality/CartoDB-SQL-API/issues/60 +test('GET /api/v1/sql as csv with no rows', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20true%20WHERE%20false&format=csv', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var body = ""; + assert.equal(body, res.body); + done(); + }); +}); + +test('GET /api/v1/sql as csv, properly escaped', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20cartodb_id,%20address%20FROM%20untitle_table_4%20LIMIT%201&format=csv', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var body = 'cartodb_id,address\r\n1,"Calle de Pérez Galdós 9, Madrid, Spain"'; + assert.equal(body, res.body); + done(); + }); +}); + +// SVG tests + test('GET /api/v1/sql with SVG format', function(done){ var query = querystring.stringify({ q: "SELECT 1 as cartodb_id, ST_MakeLine(ST_MakePoint(10, 10), ST_MakePoint(1034, 778)) AS the_geom ", @@ -506,6 +668,7 @@ test('GET /api/v1/sql with SVG format and trimmed decimals', function(done){ },{}, function(res) { assert.equal(res.statusCode, 200, res.body); var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'SVG is not disposed as attachment: ' + cd); assert.ok(/filename=cartodb-query.svg/gi.test(cd), cd); assert.equal(res.header('Content-Type'), 'image/svg+xml; charset=utf-8'); assert.ok( res.body.indexOf('') > 0, res.body ); @@ -515,157 +678,6 @@ test('GET /api/v1/sql with SVG format and trimmed decimals', function(done){ }); }); -test('GET /api/v1/sql with SQL parameter and no format, ensuring content-disposition set to json', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var cd = res.header('Content-Disposition'); - assert.equal(true, /filename=cartodb-query.json/gi.test(cd)); - done(); - }); -}); - -test('GET /api/v1/sql ensure cross domain set on errors', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*gadfgadfg%20FROM%20untitle_table_4', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ - status: 400 - }, function(res){ - var cd = res.header('Access-Control-Allow-Origin'); - assert.equal(cd, '*'); - done(); - }); -}); - -test('GET /api/v1/sql as geojson limiting decimal places', function(done){ - assert.response(app, { - url: '/api/v1/sql?' + querystring.stringify({ - q: 'SELECT ST_MakePoint(0.123,2.3456) as the_geom', - format: 'geojson', - dp: '1'}), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var result = JSON.parse(res.body); - assert.equal(1, checkDecimals(result.features[0].geometry.coordinates[0], '.')); - done(); - }); -}); - -test('GET /api/v1/sql as geojson with default dp as 6', function(done){ - assert.response(app, { - url: '/api/v1/sql?' + querystring.stringify({ - q: 'SELECT ST_MakePoint(0.12345678,2.3456787654) as the_geom', - format: 'geojson'}), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var result = JSON.parse(res.body); - assert.equal(6, checkDecimals(result.features[0].geometry.coordinates[0], '.')); - done(); - }); -}); - -test('GET /api/v1/sql as csv', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20cartodb_id,ST_AsEWKT(the_geom)%20as%20geom%20FROM%20untitle_table_4%20LIMIT%201&format=csv', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var body = "cartodb_id,geom\r\n1,SRID=4326;POINT(-3.699732 40.423012)"; - assert.equal(body, res.body); - done(); - }); -}); - -// See https://github.com/Vizzuality/CartoDB-SQL-API/issues/60 -test('GET /api/v1/sql as csv with no rows', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20true%20WHERE%20false&format=csv', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var body = ""; - assert.equal(body, res.body); - done(); - }); -}); - -test('GET /api/v1/sql as csv, properly escaped', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20cartodb_id,%20address%20FROM%20untitle_table_4%20LIMIT%201&format=csv', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var body = 'cartodb_id,address\r\n1,"Calle de Pérez Galdós 9, Madrid, Spain"'; - assert.equal(body, res.body); - done(); - }); -}); - -test('cannot GET system tables', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20pg_attribute', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ - status: 403 - }, function() { done(); }); -}); - -test('GET decent error if domain is incorrect', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', - headers: {host: 'vizzualinot.cartodb.com'}, - method: 'GET' - },{ - status: 404 - }, function(res){ - var result = JSON.parse(res.body); - assert.equal(result.error[0],"Sorry, we can't find this CartoDB. Please check that you have entered the correct domain."); - done(); - }); -}); - -test('GET decent error if SQL is broken', function(done){ - assert.response(app, { - url: '/api/v1/sql?' + querystring.stringify({q: - 'SELECT star FROM this and that' - }), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{}, function(res){ - assert.equal(res.statusCode, 400, res.statusCode + ': ' + res.body); - var result = JSON.parse(res.body); - // NOTE: actual error message may be slighly different, possibly worth a regexp here - assert.equal(result.error[0], 'syntax error at or near "and"'); - done(); - }); -}); - -// CSV tests -test('CSV format', function(done){ - assert.response(app, { - url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=csv', - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); - var cd = res.header('Content-Disposition'); - assert.equal(true, /filename=cartodb-query.csv/gi.test(cd)); - done(); - }); -}); // SHP tests @@ -677,6 +689,7 @@ test('SHP format, unauthenticated', function(done){ },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'SHP is not disposed as attachment: ' + cd); assert.equal(true, /filename=cartodb-query.zip/gi.test(cd)); // TODO: check for actual content, at least try to uncompress.. done(); From 978c0b4cbe8428de12df4703403f9ccdc12624ca Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 25 Oct 2012 13:40:21 +0200 Subject: [PATCH 22/49] Advertise header presence in CSV Content-Type --- app/controllers/app.js | 4 ++-- test/acceptance/app.test.js | 2 ++ 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 3fbc5c11..4535e7c0 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -203,7 +203,7 @@ function handleQuery(req, res) { function setHeaders(err, result){ if (err) throw err; - // configure headers for geojson/CSV + // configure headers for given format res.header("Content-Disposition", getContentDisposition(format)); res.header("Content-Type", getContentType(format)); @@ -609,7 +609,7 @@ function getContentDisposition(format){ function getContentType(format){ var type = "application/json; charset=utf-8"; if (format === 'csv'){ - type = "text/csv; charset=utf-8"; + type = "text/csv; charset=utf-8; header=present"; } else if (format === 'svg'){ type = "image/svg+xml; charset=utf-8"; diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 47d49332..2375e2c8 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -555,6 +555,8 @@ test('CSV format', function(done){ var cd = res.header('Content-Disposition'); assert.equal(true, /^attachment/.test(cd), 'CSV is not disposed as attachment: ' + cd); assert.equal(true, /filename=cartodb-query.csv/gi.test(cd)); + var ct = res.header('Content-Type'); + assert.equal(true, /header=present/.test(ct), "CSV doesn't advertise header presence: " + ct); done(); }); }); From 578edf29dbb113fa43b3a66ad6280723e1a00f26 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 25 Oct 2012 18:07:51 +0200 Subject: [PATCH 23/49] Update NEWS file (the 1.1.0 section, being still open) --- NEWS.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/NEWS.md b/NEWS.md index e9bbc6c6..d67799df 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,9 @@ * New output formats: * ESRI Shapefile (format=shp) * SVG (format=svg) +* Advertise header presence in CSV Content-Type +* Fix CSV output with no rows (#60) +* Use "attachment" Content-Disposition for all output formats (#61) * Only use last format parameter when multiple are requested * Return a 400 response on unsupported format request * Fixed problem in cluster2 with pidfile name From 9aa28c5bea607db403e0596cbb35b95fe2b2c5c9 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Thu, 25 Oct 2012 12:38:45 +0200 Subject: [PATCH 24/49] Initial support for KML output format. Closes #54. --- NEWS.md | 1 + app/controllers/app.js | 106 +++++++++++++++++++++++++++++++++++- test/acceptance/app.test.js | 32 +++++++++++ 3 files changed, 137 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index d67799df..d82142f2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -3,6 +3,7 @@ * New output formats: * ESRI Shapefile (format=shp) * SVG (format=svg) + * KML (format=kml) * Advertise header presence in CSV Content-Type * Fix CSV output with no rows (#60) * Use "attachment" Content-Disposition for all output formats (#61) diff --git a/app/controllers/app.js b/app/controllers/app.js index 4535e7c0..5b6de26d 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -25,6 +25,7 @@ var express = require('express') , crypto = require('crypto') , fs = require('fs') , zlib = require('zlib') + , util = require('util') , spawn = require('child_process').spawn , Meta = require(global.settings.app_root + '/app/models/metadata') , oAuth = require(global.settings.app_root + '/app/models/oauth') @@ -68,7 +69,7 @@ function userid_to_dbuser(user_id) { // request handlers function handleQuery(req, res) { - var supportedFormats = ['json', 'geojson', 'csv', 'svg', 'shp']; + var supportedFormats = ['json', 'geojson', 'csv', 'svg', 'shp', 'kml']; var svg_width = 1024.0; var svg_height = 768.0; @@ -229,6 +230,8 @@ function handleQuery(req, res) { toCSV(result, res, this); } else if ( format === 'shp'){ toSHP(database, user_id, gn, sql, res, this); + } else if ( format === 'kml'){ + toKML(database, user_id, gn, sql, res, this); } else if ( format === 'json'){ var end = new Date().getTime(); @@ -508,7 +511,16 @@ function toSHP(dbname, user_id, gcol, sql, res, callback) { fs.mkdir(outdirpath, 0777, this); }, function spawnDumper(err) { - if ( err ) throw err; + if ( err ) { + if ( err.code == 'EEXIST' ) { + // TODO: this could mean another request for the same + // resource is in progress, in which case we might want + // to queue the response to after it's completed... + console.log("Reusing existing SHP output directory for query: " + sql); + } else { + throw err; + } + } toOGR(dbname, user_id, gcol, sql, res, 'ESRI Shapefile', shapefile, this); }, function zipAndSendDump(err) { @@ -588,6 +600,90 @@ function toSHP(dbname, user_id, gcol, sql, res, callback) { ); } +function toKML(dbname, user_id, gcol, sql, res, callback) { + var zip = 'zip'; // FIXME: make configurable + var tmpdir = '/tmp'; // FIXME: make configurable + var outdirpath = tmpdir + '/sqlapi-kmloutput-' + generateMD5(sql); + var dumpfile = outdirpath + '/cartodb-query.kml'; + + // TODO: following tests: + // - fetch with no auth + // - fetch with auth + // - fetch same query concurrently + // - fetch query with no "the_geom" column + + Step ( + + function createOutDir() { + fs.mkdir(outdirpath, 0777, this); + }, + function spawnDumper(err) { + if ( err ) { + if ( err.code == 'EEXIST' ) { + // TODO: this could mean another request for the same + // resource is in progress, in which case we might want + // to queue the response to after it's completed... + console.log("Reusing existing KML output directory for query: " + sql); + } else { + throw err; + } + } + toOGR(dbname, user_id, gcol, sql, res, 'KML', dumpfile, this); + }, + function sendResults(err) { + + if ( ! err ) { + var stream = fs.createReadStream(dumpfile); + util.pump(stream, res); + } + + // cleanup output dir (should be safe to unlink) + var topError = err; + var next = this; + + //console.log("Cleaning up " + outdirpath); + + // Unlink the dir content + var unlinkall = function(dir, files, finish) { + var f = files.shift(); + if ( ! f ) { finish(null); return; } + var fn = dir + '/' + f; + fs.unlink(fn, function(err) { + if ( err ) { + console.log("Unlinking " + fn + ": " + err); + finish(err); + } + else unlinkall(dir, files, finish) + }); + } + fs.readdir(outdirpath, function(err, files) { + if ( err ) { + if ( err.code != 'ENOENT' ) { + next(new Error([topError, err].join('\n'))); + } else { + next(topError); + } + } else { + unlinkall(outdirpath, files, function(err) { + fs.rmdir(outdirpath, function(err) { + if ( err ) console.log("Removing dir " + path + ": " + err); + next(topError); + }); + }); + } + }); + }, + function finish(err) { + if ( err ) callback(err); + else { + res.end(); + callback(null); + } + + } + ); +} + function getContentDisposition(format){ var ext = 'json'; if (format === 'geojson'){ @@ -602,6 +698,9 @@ function getContentDisposition(format){ else if (format === 'shp'){ ext = 'zip'; } + else if (format === 'kml'){ + ext = 'kml'; + } var time = new Date().toUTCString(); return 'attachment; filename=cartodb-query.' + ext + '; modification-date="' + time + '";'; } @@ -617,6 +716,9 @@ function getContentType(format){ else if (format === 'shp'){ type = "application/zip; charset=utf-8"; } + else if (format === 'kml'){ + type = "application/kml; charset=utf-8"; + } return type; } diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 2375e2c8..67dbae6c 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -712,4 +712,36 @@ test('SHP format, authenticated', function(done){ }); }); +// KML tests + +test('KML format, unauthenticated', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=kml', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'KML is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.kml/gi.test(cd), 'Unexpected KML filename: ' + cd); + // TODO: check for actual content, at least try to uncompress.. + done(); + }); +}); + +test('KML format, authenticated', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=kml&api_key=1234', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /filename=cartodb-query.kml/gi.test(cd), 'Unexpected KML filename: ' + cd); + // TODO: check for actual content, at least try to uncompress.. + done(); + }); +}); + + }); From 4e0346a5a1b918515902f501c3890dc3b21c623e Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Tue, 30 Oct 2012 18:56:11 +0100 Subject: [PATCH 25/49] target version 1.1.1 --- NEWS.md | 6 +++++- package.json | 2 +- 2 files changed, 6 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 6ed93d68..52e3a3e3 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -1.1.0 (30/10/12) +1.1.1 (DD/MM/YY) ----- * New output formats: * ESRI Shapefile (format=shp) @@ -9,7 +9,11 @@ * Use "attachment" Content-Disposition for all output formats (#61) * Only use last format parameter when multiple are requested * Return a 400 response on unsupported format request + +1.1.0 (30/10/12) +----- * Fixed problem in cluster2 with pidfile name +* SVG output format * Enhancement to the cdbsql tool: - New switches: --format, --key, --dp - Interactive mode diff --git a/package.json b/package.json index fc18f7ed..c7709bab 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.1.0", + "version": "1.1.1", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", From 110af3e9d9dd820c6c5ae52a3ee7f197d3cd1cd3 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 31 Oct 2012 11:56:55 +0100 Subject: [PATCH 26/49] Add "sqlapi" prefix to temporary dir for shapefile output --- app/controllers/app.js | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 5b6de26d..a4bfe55a 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -411,7 +411,6 @@ function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callba var dbpass = ''; // turn into a parameter.. var tmpdir = '/tmp'; // FIXME: make configurable - var outdirpath = tmpdir + '/shapefile-' + generateMD5(sql); var columns = []; Step ( @@ -480,7 +479,7 @@ console.log(['ogr2ogr', if ( code ) { next(new Error("ogr2ogr returned an error (error code " + code + ")\n" + stderr)); } else { - next(null, outdirpath); + next(null); } }); }, @@ -493,7 +492,7 @@ console.log(['ogr2ogr', function toSHP(dbname, user_id, gcol, sql, res, callback) { var zip = 'zip'; // FIXME: make configurable var tmpdir = '/tmp'; // FIXME: make configurable - var outdirpath = tmpdir + '/shapefile-' + generateMD5(sql); + var outdirpath = tmpdir + '/sqlapi-shapefile-' + generateMD5(sql); var shapefile = outdirpath + '/cartodb-query.shp'; // TODO: following tests: From 765dc101b0f6323ff0999107700a426a0d253aaf Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 31 Oct 2012 12:12:17 +0100 Subject: [PATCH 27/49] Target 1.2.0, due to addition of new features since 1.1.0 NOTE: I haven't touched npm-shrinkwrap.json as that file will get version from package.json on next regeneration NOTE2: I added -dev suffix to the version so you can tell if you're running a development version or the "official" release (will get -dev removed) --- NEWS.md | 2 +- package.json | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index 52e3a3e3..7401fff4 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,4 @@ -1.1.1 (DD/MM/YY) +1.2.0-dev (DD/MM/YY) ----- * New output formats: * ESRI Shapefile (format=shp) diff --git a/package.json b/package.json index c7709bab..8980be19 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.1.1", + "version": "1.2.0-dev", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", From cea7e8c365358e034173ccded7489d947c43ef40 Mon Sep 17 00:00:00 2001 From: javi Date: Thu, 1 Nov 2012 13:16:46 +0100 Subject: [PATCH 28/49] added the needed headers for CORS --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 35 +++++++++++++++++++++++++++++++++++ 2 files changed, 36 insertions(+), 1 deletion(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index a4bfe55a..afc0599a 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -723,7 +723,7 @@ function getContentType(format){ function setCrossDomain(res){ res.header("Access-Control-Allow-Origin", "*"); - res.header("Access-Control-Allow-Headers", "X-Requested-With"); + res.header("Access-Control-Allow-Headers", "X-Requested-With, X-Prototype-Version, X-CSRF-Token"); } function generateCacheKey(database,tables,is_authenticated){ diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 67dbae6c..730032ac 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -743,5 +743,40 @@ test('KML format, authenticated', function(done){ }); }); +/** + * CORS + */ +test('GET /api/v1/sql with SQL parameter on SELECT only should return CORS headers ', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&database=cartodb_test_user_1_db', + method: 'GET' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); + assert.equal(res.headers['access-control-allow-origin'], '*'); + assert.equal(res.headers['access-control-allow-headers'], "X-Requested-With, X-Prototype-Version, X-CSRF-Token"); + done(); + }); +}); + +test('OPTIONS /api/v1/sql with SQL parameter on SELECT only should return CORS headers ', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&database=cartodb_test_user_1_db', + method: 'OPTIONS' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); + assert.equal(res.headers['access-control-allow-origin'], '*'); + assert.equal(res.headers['access-control-allow-headers'], "X-Requested-With, X-Prototype-Version, X-CSRF-Token"); + done(); + }); +}); + }); From 3fd3c5fabcc726df43daf473f5b06fec2aecf448 Mon Sep 17 00:00:00 2001 From: javi Date: Thu, 1 Nov 2012 13:16:46 +0100 Subject: [PATCH 29/49] added the needed headers for CORS --- NEWS.md | 1 + app/controllers/app.js | 2 +- test/acceptance/app.test.js | 35 +++++++++++++++++++++++++++++++++++ 3 files changed, 37 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 7401fff4..dd08c74e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -9,6 +9,7 @@ * Use "attachment" Content-Disposition for all output formats (#61) * Only use last format parameter when multiple are requested * Return a 400 response on unsupported format request +* Added X-Prototype-Version, X-CSRF-Token to Access-Control-Allow-Headers 1.1.0 (30/10/12) ----- diff --git a/app/controllers/app.js b/app/controllers/app.js index a4bfe55a..afc0599a 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -723,7 +723,7 @@ function getContentType(format){ function setCrossDomain(res){ res.header("Access-Control-Allow-Origin", "*"); - res.header("Access-Control-Allow-Headers", "X-Requested-With"); + res.header("Access-Control-Allow-Headers", "X-Requested-With, X-Prototype-Version, X-CSRF-Token"); } function generateCacheKey(database,tables,is_authenticated){ diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 67dbae6c..730032ac 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -743,5 +743,40 @@ test('KML format, authenticated', function(done){ }); }); +/** + * CORS + */ +test('GET /api/v1/sql with SQL parameter on SELECT only should return CORS headers ', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&database=cartodb_test_user_1_db', + method: 'GET' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); + assert.equal(res.headers['access-control-allow-origin'], '*'); + assert.equal(res.headers['access-control-allow-headers'], "X-Requested-With, X-Prototype-Version, X-CSRF-Token"); + done(); + }); +}); + +test('OPTIONS /api/v1/sql with SQL parameter on SELECT only should return CORS headers ', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&database=cartodb_test_user_1_db', + method: 'OPTIONS' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control); + assert.equal(res.headers['access-control-allow-origin'], '*'); + assert.equal(res.headers['access-control-allow-headers'], "X-Requested-With, X-Prototype-Version, X-CSRF-Token"); + done(); + }); +}); + }); From 62b22bb9f366557bcb5751da48a65effcec7bf4b Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Fri, 2 Nov 2012 15:38:24 +0100 Subject: [PATCH 30/49] target version 1.2.1 --- NEWS.md | 5 ++++- package.json | 2 +- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index dd08c74e..f6629808 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,4 +1,7 @@ -1.2.0-dev (DD/MM/YY) +1.2.1 (DD/MM/YY) +----- + +1.2.0 (DD/MM/YY) ----- * New output formats: * ESRI Shapefile (format=shp) diff --git a/package.json b/package.json index 8980be19..6e38f385 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.2.0-dev", + "version": "1.2.1", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", From ef4a379e0522c433a0ae61970e13f2ddab975988 Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Fri, 2 Nov 2012 23:19:36 +0100 Subject: [PATCH 31/49] set 600 timeout in cluster.js --- cluster.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/cluster.js b/cluster.js index 30417979..04b3e0bd 100755 --- a/cluster.js +++ b/cluster.js @@ -32,7 +32,8 @@ var cluster = new Cluster({ port: global.settings.node_port, host: global.settings.node_host, monHost: global.settings.node_host, - monPort: global.settings.node_port+1 + monPort: global.settings.node_port+1, + timeout: 600 }); cluster.listen(function(cb) { From c0568b59d9739cad7a165a169e98ceaa84806ada Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Fri, 2 Nov 2012 23:47:01 +0100 Subject: [PATCH 32/49] timeout is in miliseconds, not in seconds --- cluster.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/cluster.js b/cluster.js index 04b3e0bd..97f1dd98 100755 --- a/cluster.js +++ b/cluster.js @@ -33,7 +33,7 @@ var cluster = new Cluster({ host: global.settings.node_host, monHost: global.settings.node_host, monPort: global.settings.node_port+1, - timeout: 600 + timeout: 600000 }); cluster.listen(function(cb) { From dd5a2a38c3b9405ea214cc5b601fa4d06b361ece Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Mon, 12 Nov 2012 12:23:06 +0100 Subject: [PATCH 33/49] Updated NEWS.md for 1.2.1 --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index f6629808..d33713eb 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ 1.2.1 (DD/MM/YY) ----- +* Added timeout default to 600 miliseconds in cluster.js 1.2.0 (DD/MM/YY) ----- From 755fe738cae7c2773d485b4c162ece4fc9f09f16 Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Mon, 12 Nov 2012 12:27:37 +0100 Subject: [PATCH 34/49] target v1.2.2 --- NEWS.md | 3 +++ package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index d33713eb..03fa5036 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,6 @@ +1.2.2 (DD/MM/YY) +----- + 1.2.1 (DD/MM/YY) ----- * Added timeout default to 600 miliseconds in cluster.js diff --git a/package.json b/package.json index 6e38f385..77f06bcc 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.2.1", + "version": "1.2.2", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", From 46cec7a0e5bda946801b8f2cae756ccd4af9b47e Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 12 Nov 2012 12:37:34 +0100 Subject: [PATCH 35/49] Add support for specifying a filename for exports. Closes #64 Sets release target to 1.3.0, due to parameter addition --- NEWS.md | 3 +- app/controllers/app.js | 24 ++++-- npm-shrinkwrap.json | 7 +- package.json | 5 +- test/acceptance/app.test.js | 148 +++++++++++++++++++++++++++++++++++- 5 files changed, 171 insertions(+), 16 deletions(-) diff --git a/NEWS.md b/NEWS.md index 03fa5036..7e3e4062 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ -1.2.2 (DD/MM/YY) +1.3.0 (DD/MM/YY) ----- +* Support for specifying a filename for exports (#64) 1.2.1 (DD/MM/YY) ----- diff --git a/app/controllers/app.js b/app/controllers/app.js index afc0599a..93ef2deb 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -14,6 +14,9 @@ // eg. vizzuality.cartodb.com/api/v1/?sql=SELECT * from my_table // // + +var path = require('path'); + var express = require('express') , app = express.createServer( express.logger({ @@ -65,6 +68,12 @@ function userid_to_dbuser(user_id) { return "publicuser" // FIXME: make configurable }; +function sanitize_filename(filename) { + filename = path.basename(filename, path.extname(filename)); + filename = filename.replace(/[;()\[\]<>'"\s]/g, '_'); + //console.log("Sanitized: " + filename); + return filename; +} // request handlers function handleQuery(req, res) { @@ -81,6 +90,7 @@ function handleQuery(req, res) { var limit = parseInt(req.query.rows_per_page); var offset = parseInt(req.query.page); var format = _.isArray(req.query.format) ? _.last(req.query.format) : req.query.format; + var filename = req.query.filename; var dp = req.query.dp; // decimal point digits (defaults to 6) var gn = "the_geom"; // TODO: read from configuration file var user_id; @@ -88,12 +98,12 @@ function handleQuery(req, res) { // sanitize and apply defaults to input dp = (dp === "" || _.isUndefined(dp)) ? '6' : dp; format = (format === "" || _.isUndefined(format)) ? 'json' : format.toLowerCase(); + filename = (filename === "" || _.isUndefined(filename)) ? 'cartodb-query' : sanitize_filename(filename); sql = (sql === "" || _.isUndefined(sql)) ? null : sql; database = (database === "" || _.isUndefined(database)) ? null : database; limit = (_.isNumber(limit)) ? limit : null; offset = (_.isNumber(offset)) ? offset * limit : null; - // setup step run var start = new Date().getTime(); @@ -205,7 +215,7 @@ function handleQuery(req, res) { if (err) throw err; // configure headers for given format - res.header("Content-Disposition", getContentDisposition(format)); + res.header("Content-Disposition", getContentDisposition(format, filename)); res.header("Content-Type", getContentType(format)); // allow cross site post @@ -229,7 +239,7 @@ function handleQuery(req, res) { } else if (format === 'csv'){ toCSV(result, res, this); } else if ( format === 'shp'){ - toSHP(database, user_id, gn, sql, res, this); + toSHP(database, user_id, gn, sql, filename, res, this); } else if ( format === 'kml'){ toKML(database, user_id, gn, sql, res, this); } else if ( format === 'json'){ @@ -489,11 +499,11 @@ console.log(['ogr2ogr', ); } -function toSHP(dbname, user_id, gcol, sql, res, callback) { +function toSHP(dbname, user_id, gcol, sql, filename, res, callback) { var zip = 'zip'; // FIXME: make configurable var tmpdir = '/tmp'; // FIXME: make configurable var outdirpath = tmpdir + '/sqlapi-shapefile-' + generateMD5(sql); - var shapefile = outdirpath + '/cartodb-query.shp'; + var shapefile = outdirpath + '/' + filename + '.shp'; // TODO: following tests: // - fetch with no auth [done] @@ -683,7 +693,7 @@ function toKML(dbname, user_id, gcol, sql, res, callback) { ); } -function getContentDisposition(format){ +function getContentDisposition(format, filename) { var ext = 'json'; if (format === 'geojson'){ ext = 'geojson'; @@ -701,7 +711,7 @@ function getContentDisposition(format){ ext = 'kml'; } var time = new Date().toUTCString(); - return 'attachment; filename=cartodb-query.' + ext + '; modification-date="' + time + '";'; + return 'attachment; filename=' + filename + '.' + ext + '; modification-date="' + time + '";'; } function getContentType(format){ diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index 2862185d..1fdebc86 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,9 +1,9 @@ { "name": "cartodb_api", - "version": "1.1.0", + "version": "1.3.0-dev", "dependencies": { "cluster2": { - "version": "0.3.5-cdb01", + "version": "0.3.5-cdb02", "from": "git://github.com/CartoDB/cluster2.git#cdb_production", "dependencies": { "ejs": { @@ -218,6 +218,9 @@ "csv": { "version": "0.0.13" }, + "zipfile": { + "version": "0.3.2" + }, "mocha": { "version": "1.2.1", "dependencies": { diff --git a/package.json b/package.json index 77f06bcc..e9da332c 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.2.2", + "version": "1.3.0-dev", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", @@ -23,7 +23,8 @@ "csv":"0.0.13" }, "devDependencies": { - "mocha": "1.2.1" + "mocha": "1.2.1", + "zipfile": "~0.3.2" }, "scripts": { "test": "test/run_tests.sh" diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 730032ac..1ccb4188 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -14,10 +14,14 @@ require('../helper'); require('../support/assert'); + var app = require(global.settings.app_root + '/app/controllers/app') , assert = require('assert') , querystring = require('querystring') - , _ = require('underscore'); + , _ = require('underscore') + , zipfile = require('zipfile') + , fs = require('fs') + ; // allow lots of emitters to be set to silence warning app.setMaxListeners(0); @@ -422,7 +426,7 @@ test('GET /api/v1/sql with SQL parameter and no format, ensuring content-disposi assert.equal(res.statusCode, 200, res.body); var cd = res.header('Content-Disposition'); assert.equal(true, /^attachment/.test(cd), 'JSON is not disposed as attachment: ' + cd); - assert.equal(true, /filename=cartodb-query.json/gi.test(cd)); + assert.equal(true, /filename=cartodb-query.json/gi.test(cd), 'Unexpected JSON filename: ' + cd); done(); }); }); @@ -510,6 +514,19 @@ test('uses the last format parameter when multiple are used', function(done){ }); }); +test('uses custom filename', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&format=geojson&filename=x', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /filename=x.geojson/gi.test(cd), cd); + done(); + }); +}); + test('GET /api/v1/sql as geojson limiting decimal places', function(done){ assert.response(app, { @@ -561,6 +578,22 @@ test('CSV format', function(done){ }); }); +test('CSV format, custom filename', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=csv&filename=mycsv.csv', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'CSV is not disposed as attachment: ' + cd); + assert.equal(true, /filename=mycsv.csv/gi.test(cd), cd); + var ct = res.header('Content-Type'); + assert.equal(true, /header=present/.test(ct), "CSV doesn't advertise header presence: " + ct); + done(); + }); +}); + test('GET /api/v1/sql as csv', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20cartodb_id,ST_AsEWKT(the_geom)%20as%20geom%20FROM%20untitle_table_4%20LIMIT%201&format=csv', @@ -623,6 +656,27 @@ test('GET /api/v1/sql with SVG format', function(done){ }); }); +test('GET /api/v1/sql with SVG format and custom filename', function(done){ + var query = querystring.stringify({ + q: "SELECT 1 as cartodb_id, ST_MakeLine(ST_MakePoint(10, 10), ST_MakePoint(1034, 778)) AS the_geom ", + format: "svg", + filename: 'mysvg' + }); + assert.response(app, { + url: '/api/v1/sql?' + query, + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.ok(/filename=mysvg.svg/gi.test(cd), cd); + assert.equal(res.header('Content-Type'), 'image/svg+xml; charset=utf-8'); + assert.ok( res.body.indexOf('') > 0, res.body ); + // TODO: test viewBox + done(); + }); +}); + test('GET /api/v1/sql with SVG format and centered point', function(done){ var query = querystring.stringify({ q: "SELECT 1 as cartodb_id, ST_MakePoint(5000, -54) AS the_geom ", @@ -687,13 +741,74 @@ test('SHP format, unauthenticated', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp', headers: {host: 'vizzuality.cartodb.com'}, + encoding: 'binary', method: 'GET' },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); var cd = res.header('Content-Disposition'); assert.equal(true, /^attachment/.test(cd), 'SHP is not disposed as attachment: ' + cd); assert.equal(true, /filename=cartodb-query.zip/gi.test(cd)); - // TODO: check for actual content, at least try to uncompress.. + var tmpfile = '/tmp/myshape.zip'; + var err = fs.writeFileSync(tmpfile, res.body, 'binary'); + if (err) { done(err); return } + var zf = new zipfile.ZipFile(tmpfile); + assert.ok(_.contains(zf.names, 'cartodb-query.shp'), 'SHP zipfile does not contain .shp: ' + zf.names); + assert.ok(_.contains(zf.names, 'cartodb-query.shx'), 'SHP zipfile does not contain .shx: ' + zf.names); + assert.ok(_.contains(zf.names, 'cartodb-query.dbf'), 'SHP zipfile does not contain .dbf: ' + zf.names); + // missing SRID, so no PRJ (TODO: add ?) + //assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); + fs.unlinkSync(tmpfile); + done(); + }); +}); + +test('SHP format, unauthenticated, with custom filename', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp&filename=myshape', + headers: {host: 'vizzuality.cartodb.com'}, + encoding: 'binary', + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'SHP is not disposed as attachment: ' + cd); + assert.equal(true, /filename=myshape.zip/gi.test(cd)); + var tmpfile = '/tmp/myshape.zip'; + var err = fs.writeFileSync(tmpfile, res.body, 'binary'); + if (err) { done(err); return } + var zf = new zipfile.ZipFile(tmpfile); + assert.ok(_.contains(zf.names, 'myshape.shp'), 'SHP zipfile does not contain .shp: ' + zf.names); + assert.ok(_.contains(zf.names, 'myshape.shx'), 'SHP zipfile does not contain .shx: ' + zf.names); + assert.ok(_.contains(zf.names, 'myshape.dbf'), 'SHP zipfile does not contain .dbf: ' + zf.names); + // missing SRID, so no PRJ (TODO: add ?) + //assert.ok(_.contains(zf.names, 'myshape.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); + fs.unlinkSync(tmpfile); + done(); + }); +}); + +test('SHP format, unauthenticated, with custom, dangerous filename', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp&filename=b;"%20()[]a', + headers: {host: 'vizzuality.cartodb.com'}, + encoding: 'binary', + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var fname = "b_______a"; + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'SHP is not disposed as attachment: ' + cd); + assert.equal(true, /filename=b_______a.zip/gi.test(cd), 'Unexpected SHP filename: ' + cd); + var tmpfile = '/tmp/myshape.zip'; + var err = fs.writeFileSync(tmpfile, res.body, 'binary'); + if (err) { done(err); return } + var zf = new zipfile.ZipFile(tmpfile); + assert.ok(_.contains(zf.names, fname + '.shp'), 'SHP zipfile does not contain .shp: ' + zf.names); + assert.ok(_.contains(zf.names, fname + '.shx'), 'SHP zipfile does not contain .shx: ' + zf.names); + assert.ok(_.contains(zf.names, fname + '.dbf'), 'SHP zipfile does not contain .dbf: ' + zf.names); + // missing SRID, so no PRJ (TODO: add ?) + //assert.ok(_.contains(zf.names, fname+ '.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); + fs.unlinkSync(tmpfile); done(); }); }); @@ -702,12 +817,22 @@ test('SHP format, authenticated', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp&api_key=1234', headers: {host: 'vizzuality.cartodb.com'}, + encoding: 'binary', method: 'GET' },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); var cd = res.header('Content-Disposition'); assert.equal(true, /filename=cartodb-query.zip/gi.test(cd)); - // TODO: check for actual content, at least try to uncompress.. + var tmpfile = '/tmp/myshape.zip'; + var err = fs.writeFileSync(tmpfile, res.body, 'binary'); + if (err) { done(err); return } + var zf = new zipfile.ZipFile(tmpfile); + assert.ok(_.contains(zf.names, 'cartodb-query.shp'), 'SHP zipfile does not contain .shp: ' + zf.names); + assert.ok(_.contains(zf.names, 'cartodb-query.shx'), 'SHP zipfile does not contain .shx: ' + zf.names); + assert.ok(_.contains(zf.names, 'cartodb-query.dbf'), 'SHP zipfile does not contain .dbf: ' + zf.names); + // missing SRID, so no PRJ (TODO: add ?) + //assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); + fs.unlinkSync(tmpfile); done(); }); }); @@ -729,6 +854,21 @@ test('KML format, unauthenticated', function(done){ }); }); +test('KML format, unauthenticated, custom filename', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=kml&filename=kmltest', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'KML is not disposed as attachment: ' + cd); + assert.equal(true, /filename=kmltest.kml/gi.test(cd), 'Unexpected KML filename: ' + cd); + // TODO: check for actual content, at least try to uncompress.. + done(); + }); +}); + test('KML format, authenticated', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=kml&api_key=1234', From f7c64b88865fdfca2c3fdb283b8dcbc622e52dcc Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 12 Nov 2012 13:21:46 +0100 Subject: [PATCH 36/49] Add the 'filename' parameter to API doc --- doc/API.md | 3 +++ 1 file changed, 3 insertions(+) diff --git a/doc/API.md b/doc/API.md index 751f8ad5..0c05f760 100644 --- a/doc/API.md +++ b/doc/API.md @@ -14,6 +14,9 @@ Supported query string parameters: Supported formats: JSON (the default), GeoJSON, CSV, SVG, SHP + 'filename': Sets the filename to use for the query result + file attachment + 'dp': Number of digits after the decimal point. Only affects format=GeoJSON. By default this is 6. From ab9c739995261428d00f18488f9dad495f8d43ee Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Mon, 12 Nov 2012 13:23:34 +0100 Subject: [PATCH 37/49] removed -dev suffix from package.json version --- package.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/package.json b/package.json index e9da332c..5de43e94 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.3.0-dev", + "version": "1.3.0", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", From 005ae48e3aea698eb6d35ccc839fadb55c8ef635 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 12 Nov 2012 17:10:16 +0100 Subject: [PATCH 38/49] Support for specifying a list of fields to skip from output. Closes #63 --- NEWS.md | 1 + app/controllers/app.js | 16 +++- doc/API.md | 4 + npm-shrinkwrap.json | 10 ++- package.json | 3 +- test/acceptance/app.test.js | 149 +++++++++++++++++++++++++++++++++++- 6 files changed, 178 insertions(+), 5 deletions(-) diff --git a/NEWS.md b/NEWS.md index 7e3e4062..dd281572 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,7 @@ 1.3.0 (DD/MM/YY) ----- * Support for specifying a filename for exports (#64) +* Support for specifying a list of fields to skip from output (#63) 1.2.1 (DD/MM/YY) ----- diff --git a/app/controllers/app.js b/app/controllers/app.js index 93ef2deb..54208df6 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -91,6 +91,8 @@ function handleQuery(req, res) { var offset = parseInt(req.query.page); var format = _.isArray(req.query.format) ? _.last(req.query.format) : req.query.format; var filename = req.query.filename; + var skipfields = req.query.skipfields ? req.query.skipfields.split(',') : []; + req.query.skipfields = skipfields; // save back, for toOGR use var dp = req.query.dp; // decimal point digits (defaults to 6) var gn = "the_geom"; // TODO: read from configuration file var user_id; @@ -231,6 +233,14 @@ function handleQuery(req, res) { function packageResults(err, result){ if (err) throw err; + if ( skipfields.length ){ + for ( var i=0; i Date: Mon, 12 Nov 2012 19:14:20 +0100 Subject: [PATCH 39/49] Add 'cache_policy' parameter. Closes #62 --- NEWS.md | 1 + app/controllers/app.js | 12 ++++++++++-- doc/API.md | 7 +++++++ test/acceptance/app.test.js | 15 +++++++++++++++ 4 files changed, 33 insertions(+), 2 deletions(-) diff --git a/NEWS.md b/NEWS.md index dd281572..55a172f0 100644 --- a/NEWS.md +++ b/NEWS.md @@ -2,6 +2,7 @@ ----- * Support for specifying a filename for exports (#64) * Support for specifying a list of fields to skip from output (#63) +* Add 'cache_policy' parameter (#62) 1.2.1 (DD/MM/YY) ----- diff --git a/app/controllers/app.js b/app/controllers/app.js index 54208df6..612b2513 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -224,9 +224,17 @@ function handleQuery(req, res) { setCrossDomain(res); // set cache headers - res.header('Last-Modified', new Date().toUTCString()); - res.header('Cache-Control', 'no-cache,max-age=3600,must-revalidate,public'); res.header('X-Cache-Channel', generateCacheKey(database, tableCache[sql_md5], authenticated)); + var cache_policy = req.query.cache_policy; + if ( cache_policy == 'persist' ) { + res.header('Cache-Control', 'public,max-age=31536000'); // 1 year + } else { + // TODO: set ttl=0 when tableCache[sql_md5].may_write is true ? + var ttl = 3600; + res.header('Last-Modified', new Date().toUTCString()); + res.header('Cache-Control', 'no-cache,max-age='+ttl+',must-revalidate,public'); + } + return result; }, diff --git a/doc/API.md b/doc/API.md index e4fa0378..d61198a4 100644 --- a/doc/API.md +++ b/doc/API.md @@ -27,6 +27,13 @@ Supported query string parameters: 'api_key': Needed to authenticate in order to modify the database. + 'cache_policy': + Set to "persist" to have the server send an Cache-Control + header requesting caching devices to keep the response + cached as much as possible. This is best used with a + timestamp value in cache_buster for manual control of + updates. + Response formats ---------------- diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 3afce54a..c9e520b2 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -30,6 +30,7 @@ app.setMaxListeners(0); suite('app.test', function() { var expected_cache_control = 'no-cache,max-age=3600,must-revalidate,public'; +var expected_cache_control_persist = 'public,max-age=31536000'; // use dec_sep for internationalization var checkDecimals = function(x, dec_sep){ @@ -67,6 +68,20 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. No oAuth included ', fu }); }); +test('cache_policy=persist', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&database=cartodb_test_user_1_db&cache_policy=persist', + method: 'GET' + },{ }, function(res) { + assert.equal(res.statusCode, 200, res.body); + // Check cache headers + // See https://github.com/Vizzuality/CartoDB-SQL-API/issues/43 + assert.equal(res.headers['x-cache-channel'], 'cartodb_test_user_1_db:untitle_table_4'); + assert.equal(res.headers['cache-control'], expected_cache_control_persist); + done(); + }); +}); + test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4', From 0ef13f08c2d9ed7f5bbdf812bb36cf1e8666cb25 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Mon, 12 Nov 2012 19:44:16 +0100 Subject: [PATCH 40/49] Use inline disposition when no format and no filename are given See #61 --- app/controllers/app.js | 7 ++++--- test/acceptance/app.test.js | 20 +++++++++++++++++++- 2 files changed, 23 insertions(+), 4 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 612b2513..80c8da3e 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -217,7 +217,8 @@ function handleQuery(req, res) { if (err) throw err; // configure headers for given format - res.header("Content-Disposition", getContentDisposition(format, filename)); + var use_inline = !req.query.hasOwnProperty('format') && !req.query.hasOwnProperty('filename'); + res.header("Content-Disposition", getContentDisposition(format, filename, use_inline)); res.header("Content-Type", getContentType(format)); // allow cross site post @@ -713,7 +714,7 @@ function toKML(dbname, user_id, gcol, sql, res, callback) { ); } -function getContentDisposition(format, filename) { +function getContentDisposition(format, filename, inline) { var ext = 'json'; if (format === 'geojson'){ ext = 'geojson'; @@ -731,7 +732,7 @@ function getContentDisposition(format, filename) { ext = 'kml'; } var time = new Date().toUTCString(); - return 'attachment; filename=' + filename + '.' + ext + '; modification-date="' + time + '";'; + return ( inline ? 'inline' : 'attachment' ) +'; filename=' + filename + '.' + ext + '; modification-date="' + time + '";'; } function getContentType(format){ diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index c9e520b2..56b22867 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -440,13 +440,31 @@ test('GET /api/v1/sql with SQL parameter and no format, ensuring content-disposi method: 'GET' },{ }, function(res){ assert.equal(res.statusCode, 200, res.body); + var ct = res.header('Content-Type'); + assert.ok(/json/.test(ct), 'Default format is not JSON: ' + ct); var cd = res.header('Content-Disposition'); - assert.equal(true, /^attachment/.test(cd), 'JSON is not disposed as attachment: ' + cd); + assert.equal(true, /^inline/.test(cd), 'Default format is not disposed inline: ' + cd); assert.equal(true, /filename=cartodb-query.json/gi.test(cd), 'Unexpected JSON filename: ' + cd); done(); }); }); +test('GET /api/v1/sql with SQL parameter and no format, but a filename', function(done){ + assert.response(app, { + url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&filename=x', + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var ct = res.header('Content-Type'); + assert.ok(/json/.test(ct), 'Default format is not JSON: ' + ct); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'Format with filename is not disposed as attachment: ' + cd); + assert.equal(true, /filename=x.json/gi.test(cd), 'Unexpected JSON filename: ' + cd); + done(); + }); +}); + test('field named "the_geom_webmercator" is not skipped by default', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4', From 1914ab919635ff74d6aec5a4593b915efa708302 Mon Sep 17 00:00:00 2001 From: Luis Bosque Date: Tue, 13 Nov 2012 11:42:18 +0100 Subject: [PATCH 41/49] target v1.3.1 --- NEWS.md | 3 +++ package.json | 2 +- 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 55a172f0..aa8482a7 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,3 +1,6 @@ +1.3.1 (DD/MM/YY) +----- + 1.3.0 (DD/MM/YY) ----- * Support for specifying a filename for exports (#64) diff --git a/package.json b/package.json index bab52d7c..4dcfc82b 100644 --- a/package.json +++ b/package.json @@ -2,7 +2,7 @@ "private": true, "name": "cartodb_api", "description": "high speed SQL api for cartodb", - "version": "1.3.0", + "version": "1.3.1", "author": { "name": "Simon Tokumine, Sandro Santilli, Vizzuality", "url": "http://vizzuality.com", From e64c3f57f7f175a09212c0f7a325559c84a36158 Mon Sep 17 00:00:00 2001 From: johnhackworth Date: Tue, 13 Nov 2012 16:47:04 +0100 Subject: [PATCH 42/49] changes in the way the parameters are collected to support both POST and GET --- app/controllers/app.js | 53 ++++++++++++++++++++++-------------------- 1 file changed, 28 insertions(+), 25 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 80c8da3e..5fc97342 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -52,7 +52,7 @@ app.get('/api/v1/cachestatus', function(req, res) { handleCacheStatus(req, res) // But you can be pretty sure of a false return. // function queryMayWrite(sql) { - var mayWrite = false; + var mayWrite = false; var pattern = RegExp("(alter|insert|update|delete|create|drop)", "i"); if ( pattern.test(sql) ) { mayWrite = true; @@ -63,7 +63,7 @@ function queryMayWrite(sql) { // Return database username from user_id // NOTE: a "null" user_id is a request to use the public user function userid_to_dbuser(user_id) { - if ( _.isString(user_id) ) + if ( _.isString(user_id) ) return _.template(global.settings.db_user, {user_id: user_id}); return "publicuser" // FIXME: make configurable }; @@ -86,15 +86,18 @@ function handleQuery(req, res) { var body = (req.body) ? req.body : {}; var sql = req.query.q || body.q; // HTTP GET and POST store in different vars var api_key = req.query.api_key || body.api_key; - var database = req.query.database; // TODO: Depricate + var database = req.query.database; // TODO: Deprecate var limit = parseInt(req.query.rows_per_page); var offset = parseInt(req.query.page); - var format = _.isArray(req.query.format) ? _.last(req.query.format) : req.query.format; - var filename = req.query.filename; - var skipfields = req.query.skipfields ? req.query.skipfields.split(',') : []; + var requestedFormat = req.query.format || body.format; + var format = _.isArray(requestedFormat) ? _.last(requestedFormat) : requestedFormat; + var requestedFilename = req.query.filename || body.filename + var filename = requestedFilename; + var requestedSkipfields = req.query.skipfields || body.skipfields; + var skipfields = requestedSkipfields ? requestedSkipfields.split(',') : []; req.query.skipfields = skipfields; // save back, for toOGR use - var dp = req.query.dp; // decimal point digits (defaults to 6) - var gn = "the_geom"; // TODO: read from configuration file + var dp = req.query.dp || body.dp; // decimal point digits (defaults to 6) + var gn = "the_geom"; // TODO: read from configuration file var user_id; // sanitize and apply defaults to input @@ -190,7 +193,7 @@ function handleQuery(req, res) { return null; } else if (format === 'svg') { var svg_ratio = svg_width/svg_height; - sql = 'WITH source AS ( ' + sql + '), extent AS ( ' + sql = 'WITH source AS ( ' + sql + '), extent AS ( ' + ' SELECT ST_Extent(' + gn + ') AS e FROM source ' + '), extent_info AS ( SELECT e, ' + 'st_xmin(e) as ex0, st_ymax(e) as ey0, ' @@ -356,7 +359,7 @@ function toSVG(rows, gn, callback){ polys.push(''); } - if ( ! bbox ) { + if ( ! bbox ) { // Parse layer extent: "BOX(x y, X Y)" // NOTE: the name of the extent field is // determined by the same code adding the @@ -366,9 +369,9 @@ function toSVG(rows, gn, callback){ bbox = bbox.match(/BOX\(([^ ]*) ([^ ,]*),([^ ]*) ([^)]*)\)/); bbox = { xmin: parseFloat(bbox[1]), - ymin: parseFloat(bbox[2]), + ymin: parseFloat(bbox[2]), xmax: parseFloat(bbox[3]), - ymax: parseFloat(bbox[4]) + ymax: parseFloat(bbox[4]) }; } }); @@ -395,7 +398,7 @@ function toSVG(rows, gn, callback){ bbox.width = bbox.xmax - bbox.xmin; bbox.height = bbox.ymax - bbox.ymin; root_tag += 'viewBox="' + bbox.xmin + ' ' + (-bbox.ymax) + ' ' - + bbox.width + ' ' + bbox.height + '" '; + + bbox.width + ' ' + bbox.height + '" '; } root_tag += 'style="fill-opacity:' + fill_opacity + '; stroke:' + stroke_color @@ -433,8 +436,8 @@ function toCSV(data, res, callback){ // Internal function usable by all OGR-driven outputs function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callback) { var ogr2ogr = 'ogr2ogr'; // FIXME: make configurable - var dbhost = global.settings.db_host; - var dbport = global.settings.db_port; + var dbhost = global.settings.db_host; + var dbport = global.settings.db_port; var dbuser = userid_to_dbuser(user_id); var dbpass = ''; // turn into a parameter.. @@ -454,7 +457,7 @@ function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callba function spawnDumper(err, result) { if (err) throw err; - if ( ! result.rows.length ) + if ( ! result.rows.length ) throw new Error("Query returns no rows"); // Skip system columns @@ -470,15 +473,15 @@ function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callba + ' FROM (' + sql + ') as _cartodbsqlapi'; var child = spawn(ogr2ogr, [ - '-f', out_format, + '-f', out_format, out_filename, "PG:host=" + dbhost + " user=" + dbuser + " dbname=" + dbname + " password=" + dbpass - + " tables=fake" // trick to skip query to geometry_columns + + " tables=fake" // trick to skip query to geometry_columns + "", - '-sql', sql + '-sql', sql ]); /* @@ -515,7 +518,7 @@ console.log(['ogr2ogr', }); }, function finish(err) { - callback(err); + callback(err); } ); } @@ -620,7 +623,7 @@ function toSHP(dbname, user_id, gcol, sql, filename, res, callback) { }); }, function finish(err) { - if ( err ) callback(err); + if ( err ) callback(err); else { res.end(); callback(null); @@ -637,8 +640,8 @@ function toKML(dbname, user_id, gcol, sql, res, callback) { var dumpfile = outdirpath + '/cartodb-query.kml'; // TODO: following tests: - // - fetch with no auth - // - fetch with auth + // - fetch with no auth + // - fetch with auth // - fetch same query concurrently // - fetch query with no "the_geom" column @@ -704,7 +707,7 @@ function toKML(dbname, user_id, gcol, sql, res, callback) { }); }, function finish(err) { - if ( err ) callback(err); + if ( err ) callback(err); else { res.end(); callback(null); @@ -761,7 +764,7 @@ function generateCacheKey(database,tables,is_authenticated){ if ( is_authenticated && tables.may_write ) { return "NONE"; } else { - return database + ":" + tables.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; + return database + ":" + tables.rows[0].cdb_querytables.split(/^\{(.*)\}$/)[1]; } } From 6b7cada97dc57b92ec7eb0cea0fe21fddb6a3cc0 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 13 Nov 2012 19:26:36 +0100 Subject: [PATCH 43/49] Never dispose "inline" when using POST. --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 101 ++++++++++++++++++++++++++++++++++++ 2 files changed, 102 insertions(+), 1 deletion(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 5fc97342..997d009c 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -220,7 +220,7 @@ function handleQuery(req, res) { if (err) throw err; // configure headers for given format - var use_inline = !req.query.hasOwnProperty('format') && !req.query.hasOwnProperty('filename'); + var use_inline = req.method == 'GET' && !req.query.hasOwnProperty('format') && !req.query.hasOwnProperty('filename'); res.header("Content-Disposition", getContentDisposition(format, filename, use_inline)); res.header("Content-Type", getContentType(format)); diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 56b22867..7bcd6b0e 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -449,6 +449,23 @@ test('GET /api/v1/sql with SQL parameter and no format, ensuring content-disposi }); }); +test('POST /api/v1/sql with SQL parameter and no format, ensuring content-disposition set to json', function(done){ + assert.response(app, { + url: '/api/v1/sql', + data: querystring.stringify({q: "SELECT * FROM untitle_table_4" }), + headers: {host: 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' }, + method: 'POST' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var ct = res.header('Content-Type'); + assert.ok(/json/.test(ct), 'Default format is not JSON: ' + ct); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'Default format is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.json/gi.test(cd), 'Unexpected JSON filename: ' + cd); + done(); + }); +}); + test('GET /api/v1/sql with SQL parameter and no format, but a filename', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4&filename=x', @@ -575,6 +592,21 @@ test('GET /api/v1/sql with SQL parameter and geojson format, ensuring content-di }); }); +test('POST /api/v1/sql with SQL parameter and geojson format, ensuring content-disposition set to geojson', function(done){ + assert.response(app, { + url: '/api/v1/sql', + data: querystring.stringify({q: "SELECT * FROM untitle_table_4", format: 'geojson' }), + headers: {host: 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' }, + method: 'POST' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'GEOJSON is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.geojson/gi.test(cd)); + done(); + }); +}); + test('uses the last format parameter when multiple are used', function(done){ assert.response(app, { url: '/api/v1/sql?format=csv&q=SELECT%20*%20FROM%20untitle_table_4&format=geojson', @@ -694,6 +726,23 @@ test('CSV format', function(done){ }); }); +test('CSV format from POST', function(done){ + assert.response(app, { + url: '/api/v1/sql', + data: querystring.stringify({q: "SELECT * FROM untitle_table_4 LIMIT 1", format: 'csv'}), + headers: {host: 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' }, + method: 'POST' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'CSV is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.csv/gi.test(cd)); + var ct = res.header('Content-Type'); + assert.equal(true, /header=present/.test(ct), "CSV doesn't advertise header presence: " + ct); + done(); + }); +}); + test('CSV format, custom filename', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=csv&filename=mycsv.csv', @@ -803,6 +852,28 @@ test('GET /api/v1/sql with SVG format', function(done){ }); }); +test('POST /api/v1/sql with SVG format', function(done){ + var query = querystring.stringify({ + q: "SELECT 1 as cartodb_id, ST_MakeLine(ST_MakePoint(10, 10), ST_MakePoint(1034, 778)) AS the_geom ", + format: "svg" + }); + assert.response(app, { + url: '/api/v1/sql', + data: query, + headers: {host: 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' }, + method: 'POST' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'SVG is not disposed as attachment: ' + cd); + assert.ok(/filename=cartodb-query.svg/gi.test(cd), cd); + assert.equal(res.header('Content-Type'), 'image/svg+xml; charset=utf-8'); + assert.ok( res.body.indexOf('') > 0, res.body ); + // TODO: test viewBox + done(); + }); +}); + test('GET /api/v1/sql with SVG format and custom filename', function(done){ var query = querystring.stringify({ q: "SELECT 1 as cartodb_id, ST_MakeLine(ST_MakePoint(10, 10), ST_MakePoint(1034, 778)) AS the_geom ", @@ -910,6 +981,21 @@ test('SHP format, unauthenticated', function(done){ }); }); +test('SHP format, unauthenticated, POST', function(done){ + assert.response(app, { + url: '/api/v1/sql', + data: 'q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp', + headers: {host: 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' }, + method: 'POST' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'SHP is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.zip/gi.test(cd), 'Unexpected SHP filename: ' + cd); + done(); + }); +}); + test('SHP format, unauthenticated, with custom filename', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=shp&filename=myshape', @@ -1011,6 +1097,21 @@ test('KML format, unauthenticated', function(done){ }); }); +test('KML format, unauthenticated, POST', function(done){ + assert.response(app, { + url: '/api/v1/sql', + data: 'q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=kml', + headers: {host: 'vizzuality.cartodb.com', 'Content-Type': 'application/x-www-form-urlencoded' }, + method: 'POST' + },{ }, function(res){ + assert.equal(res.statusCode, 200, res.body); + var cd = res.header('Content-Disposition'); + assert.equal(true, /^attachment/.test(cd), 'KML is not disposed as attachment: ' + cd); + assert.equal(true, /filename=cartodb-query.kml/gi.test(cd), 'Unexpected KML filename: ' + cd); + done(); + }); +}); + test('KML format, skipfields', function(done){ assert.response(app, { url: '/api/v1/sql?q=SELECT%20*%20FROM%20untitle_table_4%20LIMIT%201&format=kml&skipfields=address,cartodb_id', From cceeb72f4e0e11b8a03ad4a2b65c500525106fcd Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Tue, 13 Nov 2012 19:28:09 +0100 Subject: [PATCH 44/49] Update NEWS --- NEWS.md | 2 ++ 1 file changed, 2 insertions(+) diff --git a/NEWS.md b/NEWS.md index aa8482a7..86afd916 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,7 @@ 1.3.1 (DD/MM/YY) ----- +* Support 'format' and 'filename' params in POST +* Always dispose response as attachment when using POST 1.3.0 (DD/MM/YY) ----- From bd08eb4addcfae986a86b6332954f0ba3e5c2e9f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 14 Nov 2012 16:30:18 +0100 Subject: [PATCH 45/49] Use inline attachment also with POST, if format isn't given --- app/controllers/app.js | 2 +- test/acceptance/app.test.js | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 997d009c..286acf7a 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -220,7 +220,7 @@ function handleQuery(req, res) { if (err) throw err; // configure headers for given format - var use_inline = req.method == 'GET' && !req.query.hasOwnProperty('format') && !req.query.hasOwnProperty('filename'); + var use_inline = !requestedFormat && !requestedFilename; res.header("Content-Disposition", getContentDisposition(format, filename, use_inline)); res.header("Content-Type", getContentType(format)); diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js index 7bcd6b0e..08f8b578 100644 --- a/test/acceptance/app.test.js +++ b/test/acceptance/app.test.js @@ -460,7 +460,7 @@ test('POST /api/v1/sql with SQL parameter and no format, ensuring content-dispos var ct = res.header('Content-Type'); assert.ok(/json/.test(ct), 'Default format is not JSON: ' + ct); var cd = res.header('Content-Disposition'); - assert.equal(true, /^attachment/.test(cd), 'Default format is not disposed as attachment: ' + cd); + assert.equal(true, /^inline/.test(cd), 'Default format is not disposed inline: ' + cd); assert.equal(true, /filename=cartodb-query.json/gi.test(cd), 'Unexpected JSON filename: ' + cd); done(); }); From 34587782b501d173ccba008d56749590d7d4fce2 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 14 Nov 2012 16:32:52 +0100 Subject: [PATCH 46/49] Remove reverted change from NEWS --- NEWS.md | 1 - 1 file changed, 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 86afd916..43ee9f41 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,6 @@ 1.3.1 (DD/MM/YY) ----- * Support 'format' and 'filename' params in POST -* Always dispose response as attachment when using POST 1.3.0 (DD/MM/YY) ----- From 75fcd5ae9ce1eb6eccd8604cd154e80a71e26890 Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 14 Nov 2012 18:04:38 +0100 Subject: [PATCH 47/49] Do not write back to request.query as it breaks oAuth signature NOTE: this breakage misses a testcase (overcomplex at the moment) --- app/controllers/app.js | 19 ++++++++----------- 1 file changed, 8 insertions(+), 11 deletions(-) diff --git a/app/controllers/app.js b/app/controllers/app.js index 286acf7a..21599e5f 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -95,7 +95,6 @@ function handleQuery(req, res) { var filename = requestedFilename; var requestedSkipfields = req.query.skipfields || body.skipfields; var skipfields = requestedSkipfields ? requestedSkipfields.split(',') : []; - req.query.skipfields = skipfields; // save back, for toOGR use var dp = req.query.dp || body.dp; // decimal point digits (defaults to 6) var gn = "the_geom"; // TODO: read from configuration file var user_id; @@ -261,9 +260,9 @@ function handleQuery(req, res) { } else if (format === 'csv'){ toCSV(result, res, this); } else if ( format === 'shp'){ - toSHP(database, user_id, gn, sql, filename, res, this); + toSHP(database, user_id, gn, sql, skipfields, filename, res, this); } else if ( format === 'kml'){ - toKML(database, user_id, gn, sql, res, this); + toKML(database, user_id, gn, sql, skipfields, res, this); } else if ( format === 'json'){ var end = new Date().getTime(); @@ -434,7 +433,7 @@ function toCSV(data, res, callback){ } // Internal function usable by all OGR-driven outputs -function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callback) { +function toOGR(dbname, user_id, gcol, sql, skipfields, res, out_format, out_filename, callback) { var ogr2ogr = 'ogr2ogr'; // FIXME: make configurable var dbhost = global.settings.db_host; var dbport = global.settings.db_port; @@ -444,9 +443,6 @@ function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callba var tmpdir = '/tmp'; // FIXME: make configurable var columns = []; - var skipfields = res.req.query.skipfields; - skipfields.push( "the_geom_webmercator" ); - Step ( function fetchColumns() { @@ -463,6 +459,7 @@ function toOGR(dbname, user_id, gcol, sql, res, out_format, out_filename, callba // Skip system columns for (var k in result.rows[0]) { if ( skipfields.indexOf(k) != -1 ) continue; + if ( k == "the_geom_webmercator" ) continue; columns.push('"' + k + '"'); } //console.log(columns.join(',')); @@ -523,7 +520,7 @@ console.log(['ogr2ogr', ); } -function toSHP(dbname, user_id, gcol, sql, filename, res, callback) { +function toSHP(dbname, user_id, gcol, sql, skipfields, filename, res, callback) { var zip = 'zip'; // FIXME: make configurable var tmpdir = '/tmp'; // FIXME: make configurable var outdirpath = tmpdir + '/sqlapi-shapefile-' + generateMD5(sql); @@ -554,7 +551,7 @@ function toSHP(dbname, user_id, gcol, sql, filename, res, callback) { throw err; } } - toOGR(dbname, user_id, gcol, sql, res, 'ESRI Shapefile', shapefile, this); + toOGR(dbname, user_id, gcol, sql, skipfields, res, 'ESRI Shapefile', shapefile, this); }, function zipAndSendDump(err) { if ( err ) throw err; @@ -633,7 +630,7 @@ function toSHP(dbname, user_id, gcol, sql, filename, res, callback) { ); } -function toKML(dbname, user_id, gcol, sql, res, callback) { +function toKML(dbname, user_id, gcol, sql, skipfields, res, callback) { var zip = 'zip'; // FIXME: make configurable var tmpdir = '/tmp'; // FIXME: make configurable var outdirpath = tmpdir + '/sqlapi-kmloutput-' + generateMD5(sql); @@ -661,7 +658,7 @@ function toKML(dbname, user_id, gcol, sql, res, callback) { throw err; } } - toOGR(dbname, user_id, gcol, sql, res, 'KML', dumpfile, this); + toOGR(dbname, user_id, gcol, sql, skipfields, res, 'KML', dumpfile, this); }, function sendResults(err) { From 4cc92838740f92a9086a4dbe9e9e11298837804f Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 14 Nov 2012 18:06:05 +0100 Subject: [PATCH 48/49] Update with oAuth bugfix --- NEWS.md | 1 + 1 file changed, 1 insertion(+) diff --git a/NEWS.md b/NEWS.md index 43ee9f41..00eeef6e 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,6 +1,7 @@ 1.3.1 (DD/MM/YY) ----- * Support 'format' and 'filename' params in POST +* Fix oAuth bug introduced by 'skipfields' param in 1.3.0 1.3.0 (DD/MM/YY) ----- From 0223ff83e9fadcafa7ec948514111b9bb7d1466c Mon Sep 17 00:00:00 2001 From: Sandro Santilli Date: Wed, 14 Nov 2012 18:06:46 +0100 Subject: [PATCH 49/49] Reference the oAuth breakage ticket (#69) in NEWS --- NEWS.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/NEWS.md b/NEWS.md index 00eeef6e..60d1c33c 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,7 +1,7 @@ 1.3.1 (DD/MM/YY) ----- * Support 'format' and 'filename' params in POST -* Fix oAuth bug introduced by 'skipfields' param in 1.3.0 +* Fix oAuth bug introduced by 'skipfields' param in 1.3.0 (#69) 1.3.0 (DD/MM/YY) -----