From 4a5c9b0eed5dc99aa4c14076a274e659699a81a6 Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@keybit.net>
Date: Fri, 15 Nov 2013 13:24:48 +0100
Subject: [PATCH] Refactor OGR class interface to improve flexibility

The toOGR and toOGR_SingleFile function now take an "options"
object rather than a long list of arguments. This allows for
easier extension toward per-user database host and password
(see tickets #120 and #121)
---
 app/models/formats/csv.js |  3 +--
 app/models/formats/kml.js |  3 +--
 app/models/formats/ogr.js | 24 ++++++++++++++++++++----
 app/models/formats/shp.js | 14 ++++++++++----
 4 files changed, 32 insertions(+), 12 deletions(-)

diff --git a/app/models/formats/csv.js b/app/models/formats/csv.js
index fbf1125b..48c2d111 100644
--- a/app/models/formats/csv.js
+++ b/app/models/formats/csv.js
@@ -10,8 +10,7 @@ p._contentType = "text/csv; charset=utf-8; header=present";
 p._fileExtension = "csv";
 
 p.generate = function(options, callback) {
-    var o = options;
-    this.toOGR_SingleFile(o.database, o.user_id, o.gn, o.sql, o.skipfields, 'CSV', 'csv', o.filename, callback);
+    this.toOGR_SingleFile(options, 'CSV', callback);
 };
 
 module.exports = csv;
diff --git a/app/models/formats/kml.js b/app/models/formats/kml.js
index e0fe0e0a..18c0bb4f 100644
--- a/app/models/formats/kml.js
+++ b/app/models/formats/kml.js
@@ -18,8 +18,7 @@ p._fileExtension = "kml";
 p._needSRS = true;
 
 p.generate = function(options, callback) {
-    var o = options;
-    this.toOGR_SingleFile(o.database, o.user_id, o.gn, o.sql, o.skipfields, 'KML', 'kml', o.filename, callback);
+    this.toOGR_SingleFile(options, 'KML', callback);
 };
 
 module.exports = kml;
diff --git a/app/models/formats/ogr.js b/app/models/formats/ogr.js
index e358f50f..cf1eae29 100644
--- a/app/models/formats/ogr.js
+++ b/app/models/formats/ogr.js
@@ -59,7 +59,15 @@ ogr.prototype = {
 };
 
 // Internal function usable by all OGR-driven outputs
-ogr.prototype.toOGR = function(dbname, user_id, gcol, sql, skipfields, out_format, out_filename, out_layername, callback) {
+ogr.prototype.toOGR = function(options, out_format, out_filename, callback) {
+
+  var dbname = options.database;
+  var user_id = options.user_id;
+  var gcol = options.gn;
+  var sql = options.sql;
+  var skipfields = options.skipfields;
+  var out_layername = options.filename;
+
   var ogr2ogr = 'ogr2ogr'; // FIXME: make configurable
   var dbhost = global.settings.db_host;
   var dbport = global.settings.db_port;
@@ -197,8 +205,16 @@ console.log('ogr2ogr ' + _.map(ogrargs, function(x) { return "'" + x + "'"; }).j
   );
 };
 
-// TODO: simplify to take an options object
-ogr.prototype.toOGR_SingleFile = function(dbname, user_id, gcol, sql, skipfields, fmt, ext, layername, callback) {
+ogr.prototype.toOGR_SingleFile = function(options, fmt, callback) {
+
+  var dbname = options.database;
+  var user_id = options.user_id;
+  var gcol = options.gcol;
+  var sql = options.sql;
+  var skipfields = options.skipfields;
+  var ext = this._fileExtension;
+  var layername = options.filename;
+
   var tmpdir = global.settings.tmpDir || '/tmp';
   var reqKey = [ fmt, dbname, user_id, gcol, this.generateMD5(layername), this.generateMD5(sql) ].concat(skipfields).join(':');
   var outdirpath = tmpdir + '/sqlapi-' + process.pid + '-' + reqKey;
@@ -206,7 +222,7 @@ ogr.prototype.toOGR_SingleFile = function(dbname, user_id, gcol, sql, skipfields
 
   // TODO: following tests:
   //  - fetch query with no "the_geom" column
-  this.toOGR(dbname, user_id, gcol, sql, skipfields, fmt, dumpfile, layername, callback);
+  this.toOGR(options, fmt, dumpfile, callback);
 };
 
 ogr.prototype.sendResponse = function(opts, callback) {
diff --git a/app/models/formats/shp.js b/app/models/formats/shp.js
index dbd1a0fa..dffe28df 100644
--- a/app/models/formats/shp.js
+++ b/app/models/formats/shp.js
@@ -23,11 +23,17 @@ p._fileExtension = "zip";
 p._needSRS = true;
 
 p.generate = function(options, callback) {
-  var o = options;
-  this.toSHP(o.database, o.user_id, o.gn, o.sql, o.skipfields, o.filename, callback);
+  this.toSHP(options, callback);
 };
 
-p.toSHP = function (dbname, user_id, gcol, sql, skipfields, filename, callback) {
+p.toSHP = function (options, callback) {
+  var dbname = options.database;
+  var user_id = options.user_id;
+  var gcol = options.gn;
+  var sql = options.sql;
+  var skipfields = options.skipfields;
+  var filename = options.filename;
+
   var fmtObj = this;
   var zip = 'zip'; // FIXME: make configurable
   var tmpdir = global.settings.tmpDir || '/tmp';
@@ -45,7 +51,7 @@ p.toSHP = function (dbname, user_id, gcol, sql, skipfields, filename, callback)
     },
     function spawnDumper(err) {
       if ( err ) throw err;
-      fmtObj.toOGR(dbname, user_id, gcol, sql, skipfields, 'ESRI Shapefile', shapefile, filename, this);
+      fmtObj.toOGR(options, 'ESRI Shapefile', shapefile, this);
     },
     function doZip(err) {
       if ( err ) throw err;