diff --git a/NEWS.md b/NEWS.md index c8cd0412..d77f3db2 100644 --- a/NEWS.md +++ b/NEWS.md @@ -1,5 +1,6 @@ 1.6.0 ----- +* Fix shapefile export for non-linestring results starting with NULLs (#111) * Fix missing .prj in shapefile export (#110) * Improve recognition of non-standard field types names by db lookup (#112) diff --git a/app/models/formats/ogr.js b/app/models/formats/ogr.js index 3d26f9a6..f46b907d 100644 --- a/app/models/formats/ogr.js +++ b/app/models/formats/ogr.js @@ -109,20 +109,22 @@ ogr.prototype.toOGR = function(dbname, user_id, gcol, sql, skipfields, out_forma var next = this; - var sridsql = 'SELECT ST_Srid(' + pg.quoteIdentifier(geocol) + - ') as srid FROM (' + sql + ') as _cartodbsqlapi WHERE ' + - pg.quoteIdentifier(geocol) + ' is not null limit 1'; + var qgeocol = pg.quoteIdentifier(geocol); + var sridsql = 'SELECT ST_Srid(' + qgeocol + ') as srid, GeometryType(' + + qgeocol + ') as type FROM (' + sql + ') as _cartodbsqlapi WHERE ' + + qgeocol + ' is not null limit 1'; pg.query(sridsql, function(err, result) { if ( err ) { next(err); return; } if ( result.rows.length ) { var srid = result.rows[0].srid; - next(null, srid); + var type = result.rows[0].type; + next(null, srid, type); } }); }, - function spawnDumper(err, srid) { + function spawnDumper(err, srid, type) { if (err) throw err; var next = this; @@ -150,6 +152,10 @@ ogr.prototype.toOGR = function(dbname, user_id, gcol, sql, skipfields, out_forma ogrargs.push('-a_srs', 'EPSG:'+srid); } + if ( type ) { + ogrargs.push('-nlt', type); + } + var child = spawn(ogr2ogr, ogrargs); /* diff --git a/test/acceptance/export/shapefile.js b/test/acceptance/export/shapefile.js index 4fa41446..d8af4599 100644 --- a/test/acceptance/export/shapefile.js +++ b/test/acceptance/export/shapefile.js @@ -36,8 +36,6 @@ test('SHP format, unauthenticated', function(done){ 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); - // This will fail with < GDAL-0.10.2 - // https://github.com/CartoDB/CartoDB-SQL-API/issues/110 assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); // TODO: check DBF contents fs.unlinkSync(tmpfile); @@ -97,8 +95,6 @@ test('SHP format, unauthenticated, with custom filename', function(done){ 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); - // This will fail with < GDAL-0.10.2 - // https://github.com/CartoDB/CartoDB-SQL-API/issues/110 assert.ok(_.contains(zf.names, 'myshape.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); fs.unlinkSync(tmpfile); done(); @@ -124,8 +120,6 @@ test('SHP format, unauthenticated, with custom, dangerous filename', function(do 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); - // This will fail with < GDAL-0.10.2 - // https://github.com/CartoDB/CartoDB-SQL-API/issues/110 assert.ok(_.contains(zf.names, fname+ '.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); fs.unlinkSync(tmpfile); done(); @@ -149,8 +143,6 @@ test('SHP format, authenticated', function(done){ 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); - // This will fail with < GDAL-0.10.2 - // https://github.com/CartoDB/CartoDB-SQL-API/issues/110 assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); // TODO: check contents of the DBF fs.unlinkSync(tmpfile); @@ -280,8 +272,6 @@ test('SHP format, concurrently', function(done){ 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); - // This will fail with < GDAL-0.10.2 - // https://github.com/CartoDB/CartoDB-SQL-API/issues/110 assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); // TODO: check DBF contents fs.unlinkSync(tmpfile); @@ -290,4 +280,33 @@ test('SHP format, concurrently', function(done){ } }); +// See https://github.com/CartoDB/CartoDB-SQL-API/issues/111 +test('point with null first', function(done){ + var query = querystring.stringify({ + q: "SELECT null::geometry as g UNION ALL SELECT 'SRID=4326;POINT(0 0)'::geometry", + format: 'shp' + }); + assert.response(app, { + url: '/api/v1/sql?' + query, + 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)); + 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); + assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names); + // TODO: check contents of the DBF + fs.unlinkSync(tmpfile); + done(); + }); +}); + });