From af1f3daa690c6a2786d6ec6afa30ac1bb2700bce Mon Sep 17 00:00:00 2001
From: Sandro Santilli <strk@keybit.net>
Date: Mon, 18 Nov 2013 12:21:30 +0100
Subject: [PATCH] Optionally read user-specific database_host from redis

Follows CartoDB-2.5.0 model. Includes testcase.
Closes #121 -- Jire ref CDB-870
---
 NEWS.md                     |  2 ++
 app/controllers/app.js      | 28 ++++++++++++++++------------
 test/acceptance/app.test.js | 22 ++++++++++++++++++++++
 test/prepare_db.sh          | 11 ++++++++++-
 4 files changed, 50 insertions(+), 13 deletions(-)

diff --git a/NEWS.md b/NEWS.md
index e925aafc..44879cf9 100644
--- a/NEWS.md
+++ b/NEWS.md
@@ -1,6 +1,8 @@
 1.7.0 - 2013-MM-DD
 ------------------
 * CartoDB redis interaction delegated to "cartodb-redis" module
+* Optionally read user-specific database_host from redis
+  as per CartoDB-2.5.0 model (#121)
 
 1.6.3 - 2013-11-10
 ------------------
diff --git a/app/controllers/app.js b/app/controllers/app.js
index 5ad07227..229af80d 100755
--- a/app/controllers/app.js
+++ b/app/controllers/app.js
@@ -164,7 +164,9 @@ function handleQuery(req, res) {
         var pg;
 
         // Database options
-        var dbopts;
+        var dbopts = {
+          port: global.settings.db_port
+        };
 
         var authenticated;
 
@@ -195,6 +197,7 @@ function handleQuery(req, res) {
                 }
 
                 database = (data === "" || _.isNull(data) || _.isUndefined(data)) ? database : data;
+                dbopts.dbname = database;
 
                 if(api_key) {
                     ApiKeyAuth.verifyRequest(req, this);
@@ -202,24 +205,25 @@ function handleQuery(req, res) {
                     oAuth.verifyRequest(req, this, requestProtocol);
                 }
             },
-            function queryExplain(err, data){
+            function setUserGetDBHost(err, data){
                 if (err) throw err;
-                user_id = data;
+                user_id = data; // used to determine authentication later, TODO: use "authenticated" directly
 
-                // store postgres connection
                 var dbuser = user_id ? 
                   _.template(global.settings.db_user, {user_id: user_id})
                   :
                   global.settings.db_pubuser;
 
-                dbopts = {
-                  user: dbuser, 
-                  dbname: database,
-                  host: global.settings.db_host,
-                  port: global.settings.db_port,
-                };
-                // TODO: add password
-              
+                dbopts.user = dbuser;
+
+                Meta.getDatabaseHost(req, this);
+            },
+            function queryExplain(err, data){
+                if (err) throw err;
+
+                dbopts.host = data || global.settings.db_host;
+                //dbopts.pass = '' // TODO: add password
+
                 pg = new PSQL(dbopts);
 
                 authenticated = ! _.isNull(user_id);
diff --git a/test/acceptance/app.test.js b/test/acceptance/app.test.js
index 64450e6e..ac0af2e8 100644
--- a/test/acceptance/app.test.js
+++ b/test/acceptance/app.test.js
@@ -133,6 +133,28 @@ test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just
     });
 });
 
+test('SELECT from user-specific database', function(done){
+    var backupDBHost = global.settings.db_host;
+    global.settings.db_host = '6.6.6.6';
+    assert.response(app, {
+        url: '/api/v1/sql?q=SELECT+2+as+n',
+        headers: {host: 'cartodb250user.cartodb.com'},
+        method: 'GET'
+    },{}, function(res) {
+        global.settings.db_host = backupDBHost;
+        var err = null;
+        try {
+          assert.equal(res.statusCode, 200, res.statusCode + ": " + res.body);
+          var parsed = JSON.parse(res.body);
+          assert.equal(parsed.rows.length, 1);
+          assert.equal(parsed.rows[0].n, 2);
+        } catch (e) {
+          err = e;
+        }
+        done(err);
+    });
+});
+
 test('GET /api/v1/sql with SQL parameter on SELECT only. no database param, just id using headers. Authenticated.',
 function(done){
     assert.response(app, {
diff --git a/test/prepare_db.sh b/test/prepare_db.sh
index 7d7e3a51..8047b531 100755
--- a/test/prepare_db.sh
+++ b/test/prepare_db.sh
@@ -22,7 +22,8 @@ public_user=`grep \.db_pubuser ${TESTENV} | sed "s/.*= *'\([^']*\)'.*/\1/"`
 echo "PUBLICUSER: [${public_user}]"
 
 
-TEST_DB="cartodb_test_user_1_db"
+TESTUSERID=1
+TEST_DB="cartodb_test_user_1_db" # TODO: read from config ?
 REDIS_PORT=6333 # TODO: read from environment file
 
 export PGHOST PGPORT
@@ -45,6 +46,14 @@ echo "HSET rails:users:vizzuality id 1" | redis-cli -p ${REDIS_PORT} -n 5
 echo "HSET rails:users:vizzuality database_name ${TEST_DB}" | redis-cli -p ${REDIS_PORT} -n 5
 echo "HSET rails:users:vizzuality" "map_key" "1234" | redis-cli -p ${REDIS_PORT} -n 5
 echo "SADD rails:users:vizzuality:map_key 1235" | redis-cli -p ${REDIS_PORT} -n 5
+
+# A user configured as with cartodb-2.5.0+
+echo "HSET rails:users:cartodb250user id ${TESTUSERID}" | redis-cli -p ${REDIS_PORT} -n 5
+echo 'HSET rails:users:cartodb250user database_name "'${TEST_DB}'"' | redis-cli -p ${REDIS_PORT} -n 5
+echo 'HSET rails:users:cartodb250user database_host "localhost"' | redis-cli -p ${REDIS_PORT} -n 5
+#echo 'HSET rails:users:cartodb250user database_password "'${TESTPASS}'"' | redis-cli -p ${REDIS_PORT} -n 5
+echo "HSET rails:users:cartodb250user map_key 1235" | redis-cli -p ${REDIS_PORT} -n 5
+
 echo "hset rails:oauth_access_tokens:l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR consumer_key fZeNGv5iYayvItgDYHUbot1Ukb5rVyX6QAg8GaY2" | redis-cli -p ${REDIS_PORT} -n 3
 echo "hset rails:oauth_access_tokens:l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR consumer_secret IBLCvPEefxbIiGZhGlakYV4eM8AbVSwsHxwEYpzx" | redis-cli -p ${REDIS_PORT} -n 3
 echo "hset rails:oauth_access_tokens:l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR access_token_token l0lPbtP68ao8NfStCiA3V3neqfM03JKhToxhUQTR" | redis-cli -p ${REDIS_PORT} -n 3