From 74429f82e1fe0618737dfe0a5c742dd3303b7ab4 Mon Sep 17 00:00:00 2001 From: Raul Ochoa Date: Wed, 12 Nov 2014 11:36:59 +0100 Subject: [PATCH] Improve topojson output by streaming json --- NEWS.md | 1 + app/controllers/app.js | 2 + app/models/formats/topojson.js | 137 ++++++++++++++++++++++++----- test/acceptance/export/topojson.js | 101 ++++++++++++++++----- 4 files changed, 195 insertions(+), 46 deletions(-) diff --git a/NEWS.md b/NEWS.md index 8f35c89a..b3817b05 100644 --- a/NEWS.md +++ b/NEWS.md @@ -4,6 +4,7 @@ Enhancements: * Don't loop twice over svg rows * Improve statement timeout error messages + * Improve topojson output by streaming json 1.18.0 - 2014-10-14 diff --git a/app/controllers/app.js b/app/controllers/app.js index 7f95107a..e427c51f 100755 --- a/app/controllers/app.js +++ b/app/controllers/app.js @@ -487,6 +487,8 @@ function handleQuery(req, res) { formatter.sendResponse(opts, this); }, function errorHandle(err){ + formatter = null; + if ( err ) handleException(err, res); if ( req.profiler ) { req.profiler.sendStats(); // TODO: do on nextTick ? diff --git a/app/models/formats/topojson.js b/app/models/formats/topojson.js index bbff1eee..8c436677 100644 --- a/app/models/formats/topojson.js +++ b/app/models/formats/topojson.js @@ -3,7 +3,9 @@ var pg = require('./pg'), geojson = require('./geojson'), TopoJSON = require('topojson'); -function TopoJsonFormat() { } +function TopoJsonFormat() { + this.features = []; +} TopoJsonFormat.prototype = new pg('topojson'); @@ -11,33 +13,120 @@ TopoJsonFormat.prototype.getQuery = function(sql, options) { return geojson.prototype.getQuery(sql, options) + ' where ' + options.gn + ' is not null'; }; -TopoJsonFormat.prototype.transform = function(result, options, callback) { - toTopoJSON(result, options.gn, options.skipfields, callback); +TopoJsonFormat.prototype.handleQueryRow = function(row) { + var _geojson = { + type: "Feature" + }; + _geojson.geometry = JSON.parse(row[this.opts.gn]); + delete row[this.opts.gn]; + delete row["the_geom_webmercator"]; + _geojson.properties = row; + this.features.push(_geojson); }; -function toTopoJSON(data, gn, skipfields, callback){ - geojson.toGeoJSON(data, gn, function(err, geojson) { - if ( err ) { - callback(err, null); - return; +TopoJsonFormat.prototype.handleQueryEnd = function() { + if (this.error) { + this.callback(this.error); + return; } - var topology = TopoJSON.topology(geojson.features, { - /* TODO: expose option to API for requesting an identifier - "id": function(o) { - console.log("id called with obj: "); console.dir(o); - return o; - }, - */ - "quantization": 1e4, // TODO: expose option to API (use existing "dp" for this ?) - "force-clockwise": true, - "property-filter": function(d) { - // TODO: delegate skipfields handling to toGeoJSON - return skipfields.indexOf(d) != -1 ? null : d; - } + + if ( this.opts.profiler ) this.opts.profiler.done('gotRows'); + + var topology = TopoJSON.topology(this.features, { + "quantization": 1e4, + "force-clockwise": true, + "property-filter": function(d) { + return d; + } }); - callback(err, topology); - }); -} + + this.features = []; + + var stream = this.opts.sink; + var jsonpCallback = this.opts.callback; + var bufferedRows = this.opts.bufferedRows; + var buffer = ''; + + function streamObjectSubtree(obj, key, done) { + buffer += '"' + key + '":'; + + var isObject = _.isObject(obj[key]), + isArray = _.isArray(obj[key]), + isIterable = isArray || isObject; + + if (isIterable) { + buffer += isArray ? '[' : '{'; + var subtreeKeys = Object.keys(obj[key]); + var pos = 0; + function streamNext() { + setImmediate(function() { + var subtreeKey = subtreeKeys.shift(); + if (!isArray) { + buffer += '"' + subtreeKey + '":'; + } + buffer += JSON.stringify(obj[key][subtreeKey]); + + if (pos++ % (bufferedRows || 1000)) { + stream.write(buffer); + buffer = ''; + } + + if (subtreeKeys.length > 0) { + delete obj[key][subtreeKey]; + buffer += ','; + streamNext(); + } else { + buffer += isArray ? ']' : '}'; + stream.write(buffer); + buffer = ''; + done(); + } + }); + } + streamNext(); + } else { + buffer += JSON.stringify(obj[key]); + done(); + } + } + + if (jsonpCallback) { + buffer += jsonpCallback + '('; + } + buffer += '{'; + var keys = Object.keys(topology); + function sendResponse() { + setImmediate(function () { + var key = keys.shift(); + function done() { + if (keys.length > 0) { + delete topology[key]; + buffer += ','; + sendResponse(); + } else { + buffer += '}'; + if (jsonpCallback) { + buffer += ')'; + } + stream.write(buffer); + stream.end(); + topology = null; + } + } + streamObjectSubtree(topology, key, done); + }); + } + sendResponse(); + + this.callback(); +}; + +TopoJsonFormat.prototype.cancel = function() { + if (this.queryCanceller) { + this.queryCanceller.call(); + } +}; + module.exports = TopoJsonFormat; diff --git a/test/acceptance/export/topojson.js b/test/acceptance/export/topojson.js index a5a97b1f..8b5cbf15 100644 --- a/test/acceptance/export/topojson.js +++ b/test/acceptance/export/topojson.js @@ -19,18 +19,32 @@ suite('export.topojson', function() { // TOPOJSON tests + function getRequest(query, extraParams) { + var params = { + q: query, + format: 'topojson' + }; + + params = _.extend(params, extraParams || {}); + + return { + url: '/api/v1/sql?' + querystring.stringify(params), + headers: {host: 'vizzuality.cartodb.com'}, + method: 'GET' + }; + } + test('GET two polygons sharing an edge as topojson', function(done){ - assert.response(app, { - url: '/api/v1/sql?' + querystring.stringify({ - q: "SELECT 1 as gid, 'U' as name, 'POLYGON((-5 0,5 0,0 5,-5 0))'::geometry as the_geom " + - " UNION ALL " + - "SELECT 2, 'D', 'POLYGON((0 -5,0 5,-5 0,0 -5))'::geometry as the_geom ", - format: 'topojson' - }), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); + assert.response(app, + getRequest( + "SELECT 1 as gid, 'U' as name, 'POLYGON((-5 0,5 0,0 5,-5 0))'::geometry as the_geom " + + " UNION ALL " + + "SELECT 2, 'D', 'POLYGON((0 -5,0 5,-5 0,0 -5))'::geometry as the_geom " + ), + { + status: 200 + }, + function(res) { var cd = res.header('Content-Disposition'); assert.equal(true, /^attachment/.test(cd), 'TOPOJSON is not disposed as attachment: ' + cd); assert.equal(true, /filename=cartodb-query.topojson/gi.test(cd)); @@ -127,17 +141,15 @@ test('GET two polygons sharing an edge as topojson', function(done){ }); test('null geometries', function(done){ - assert.response(app, { - url: '/api/v1/sql?' + querystring.stringify({ - q: "SELECT 1 as gid, 'U' as name, 'POLYGON((-5 0,5 0,0 5,-5 0))'::geometry as the_geom " + - " UNION ALL " + - "SELECT 2, 'D', null::geometry as the_geom ", - format: 'topojson' - }), - headers: {host: 'vizzuality.cartodb.com'}, - method: 'GET' - },{ }, function(res){ - assert.equal(res.statusCode, 200, res.body); + assert.response(app, getRequest( + "SELECT 1 as gid, 'U' as name, 'POLYGON((-5 0,5 0,0 5,-5 0))'::geometry as the_geom " + + " UNION ALL " + + "SELECT 2, 'D', null::geometry as the_geom " + ), + { + status: 200 + }, + function(res) { var cd = res.header('Content-Disposition'); assert.equal(true, /^attachment/.test(cd), 'TOPOJSON is not disposed as attachment: ' + cd); assert.equal(true, /filename=cartodb-query.topojson/gi.test(cd)); @@ -186,4 +198,49 @@ test('null geometries', function(done){ }); }); + test('skipped fields are not returned', function(done) { + assert.response(app, + getRequest( + "SELECT 1 as gid, 'U' as name, 'POLYGON((-5 0,5 0,0 5,-5 0))'::geometry as the_geom", + { + skipfields: 'name' + } + ), + { + status: 200 + }, + function(res) { + var parsedBody = JSON.parse(res.body); + assert.equal(parsedBody.objects[0].properties.gid, 1, 'gid was expected property'); + assert.ok(!parsedBody.objects[0].properties.name); + done(); + } + ); + }); + + test('jsonp callback is invoked', function(done){ + assert.response( + app, + getRequest( + "SELECT 1 as gid, 'U' as name, 'POLYGON((-5 0,5 0,0 5,-5 0))'::geometry as the_geom", + { + callback: 'foo_jsonp' + } + ), + { + status: 200 + }, + function(res) { + assert.equal(res.statusCode, 200, res.statusCode + ': ' + res.body); + var didRunJsonCallback = false; + function foo_jsonp(body) { + didRunJsonCallback = true; + } + eval(res.body); + assert.ok(didRunJsonCallback); + done(); + } + ); + }); + });