Fix missing .prj file in shapefile format

Finds srid, when needed, with an additional query.
Closes #110.
Includes testcases.

NEWS.md

@@ -1,5 +1,6 @@
 1.6.0
 -----
+* Fix missing .prj in shapefile export (#110)
 * Improve recognition of non-standard field types names by db lookup (#112)
 
 1.5.4 - 2013-10-01

app/models/formats/ogr.js

@@ -65,7 +65,11 @@ ogr.prototype.toOGR = function(dbname, user_id, gcol, sql, skipfields, out_forma
   var dbuser = userid_to_dbuser(user_id);
   var dbpass = ''; // turn into a parameter..
 
+  var that = this;
+
   var columns = [];
+  var geocol;
+  var pg;
 
   // Drop ending semicolon (ogr doens't like it)
   sql = sql.replace(/;\s*$/, ''); 
@@ -73,32 +77,60 @@ ogr.prototype.toOGR = function(dbname, user_id, gcol, sql, skipfields, out_forma
   Step (
 
     function fetchColumns() {
-      var colsql = 'SELECT * FROM (' + sql + ') as _cartodbsqlapi LIMIT 1';
+      var colsql = 'SELECT * FROM (' + sql + ') as _cartodbsqlapi LIMIT 0';
-      var pg = new PSQL(user_id, dbname, 1, 0);
+      pg = new PSQL(user_id, dbname, 1, 0);
       pg.query(colsql, this);
     },
-    function spawnDumper(err, result) {
+    function findSRS(err, result) {
       if (err) throw err;
 
       //if ( ! result.rows.length ) throw new Error("Query returns no rows");
 
-      // Skip system columns
+      var needSRS = that._needSRS;
-      if ( result.rows.length ) {
+
-        for (var k in result.rows[0]) {
+      // Skip system columns, find geom column
+      for (var i=0; i<result.fields.length; ++i) {
+        var field = result.fields[i];
+        var k = field.name;
         if ( skipfields.indexOf(k) != -1 ) continue;
         if ( out_format != 'CSV' && k == "the_geom_webmercator" ) continue; // TODO: drop ?
-          if ( out_format == 'CSV' ) columns.push('"' + k + '"::text');
+        if ( out_format == 'CSV' ) columns.push(pg.quoteIdentifier(k)+'::text');
-          else columns.push('"' + k + '"');
+        else columns.push(pg.quoteIdentifier(k));
+
+        if ( needSRS ) {
+          if ( ! geocol && pg.typeName(field.dataTypeID) == 'geometry' ) {
+            geocol = k
+          }
+        }
       }
-      } else columns.push('*');
       //console.log(columns.join(','));
 
+      if ( ! needSRS || ! geocol ) return null;
+
       var next = this;
 
-      sql = 'SELECT ' + columns.join(',')
+      var sridsql = 'SELECT ST_Srid(' + pg.quoteIdentifier(geocol) +
+                   ') as srid FROM (' + sql + ') as _cartodbsqlapi WHERE ' +
+                   pg.quoteIdentifier(geocol) + ' 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);
+        }
+      });
+
+    },
+    function spawnDumper(err, srid) {
+      if (err) throw err;
+
+      var next = this;
+
+      var ogrsql = 'SELECT ' + columns.join(',')
           + ' FROM (' + sql + ') as _cartodbsqlapi';
 
-      var child = spawn(ogr2ogr, [
+      var ogrargs = [
         '-f', out_format,
         '-lco', 'ENCODING=UTF-8',
         '-lco', 'LINEFORMAT=CRLF',
@@ -111,20 +143,17 @@ ogr.prototype.toOGR = function(dbname, user_id, gcol, sql, skipfields, out_forma
                           // in turn breaks knowing SRID with gdal-0.10.1:
                           // http://github.com/CartoDB/CartoDB-SQL-API/issues/110
          + "",
-        '-sql', sql
+        '-sql', ogrsql
-      ]);
+      ];
+
+      if ( srid ) {
+        ogrargs.push('-a_srs', 'EPSG:'+srid);
+      }
+
+      var child = spawn(ogr2ogr, ogrargs);
 
 /*
-console.log(['ogr2ogr',
+console.log('ogr2ogr' + ogrargs);
-        '-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, "'"].join(' '));
 */
 
       var stdout = '';

app/models/formats/shp.js

@@ -14,6 +14,7 @@ var p = shp.prototype;
 
 p._contentType = "application/zip; charset=utf-8";
 p._fileExtension = "zip";
+p._needSRS = true;
 
 p.generate = function(options, callback) {
   var o = options;

app/models/psql.js

@@ -174,7 +174,34 @@ var PSQL = function(user_id, db) {
                 callback(err, query)
             }
         );
-    },
+    };
+
+    // Ported from PostgreSQL 9.2.4 source code in src/interfaces/libpq/fe-exec.c
+    me.quoteIdentifier = function(str) {
+
+      // TODO: delegate to node-postgresl (needs node-pg 2.2.0+!)
+
+      var escaped = '"';
+
+      for(var i = 0; i < str.length; i++) {
+        var c = str[i];
+        if(c === '"') {
+          escaped += c + c;
+        } else {
+          escaped += c;
+        }
+      }
+
+      escaped += '"';
+
+      return escaped;
+    };
+
+/*
+    me.escapeLiteral = function(s) {
+      return this.client.escapeLiteral(s);
+    };
+*/
 
     me.query = function(sql, callback){
         var that = this;

test/acceptance/export/shapefile.js

@@ -36,8 +36,9 @@ 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);
-        // missing SRID, so no PRJ (TODO: add ?)
+        // This will fail with < GDAL-0.10.2
-        //assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names);
+        // 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);
         done();
@@ -96,8 +97,9 @@ 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);
-        // missing SRID, so no PRJ (TODO: add ?)
+        // This will fail with < GDAL-0.10.2
-        //assert.ok(_.contains(zf.names, 'myshape.prj'), 'SHP zipfile does not contain .prj: ' + zf.names);
+        // 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();
     });
@@ -122,8 +124,9 @@ 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);
-        // missing SRID, so no PRJ (TODO: add ?)
+        // This will fail with < GDAL-0.10.2
-        //assert.ok(_.contains(zf.names, fname+ '.prj'), 'SHP zipfile does not contain .prj: ' + zf.names);
+        // 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();
     });
@@ -146,8 +149,9 @@ 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);
-        // missing SRID, so no PRJ (TODO: add ?)
+        // This will fail with < GDAL-0.10.2
-        //assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names);
+        // 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);
         done();
@@ -276,8 +280,9 @@ 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);
-          // missing SRID, so no PRJ (TODO: add ?)
+          // This will fail with < GDAL-0.10.2
-          //assert.ok(_.contains(zf.names, 'cartodb-query.prj'), 'SHP zipfile does not contain .prj: ' + zf.names);
+          // 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);
           if ( ! --waiting ) done();

test/test.sql

@@ -113,4 +113,6 @@ 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
+GRANT ALL ON SEQUENCE test_table_cartodb_id_seq_p TO test_cartodb_user_1;
+
+GRANT ALL ON TABLE spatial_ref_sys TO test_cartodb_user_1, publicuser;