From ef9e9f8c78938b6e0d9739caa53164c6d6ffa23a Mon Sep 17 00:00:00 2001
From: Javier Goizueta <jgoizueta@gmail.com>
Date: Tue, 26 Jan 2016 11:38:21 +0100
Subject: [PATCH] Adapt to changes in CDB_Overviews SQL function

Now data for multiple tables is obtained in one call, simplifying the
use of this function. Also base table is returned as an oid, so we
now have the overview base table names with schema only when needed.
---
 lib/cartodb/api/overviews_api.js              | 96 ++++++++-----------
 test/acceptance/multilayer_server.js          |  2 +-
 test/acceptance/overviews.js                  | 14 +--
 .../mapconfig_overviews_adapter.js            | 14 +--
 test/integration/overviews-api.js             |  2 +-
 test/support/sql/CDB_Overviews.sql            | 10 +-
 6 files changed, 61 insertions(+), 77 deletions(-)

diff --git a/lib/cartodb/api/overviews_api.js b/lib/cartodb/api/overviews_api.js
index 2a677210..305d6422 100644
--- a/lib/cartodb/api/overviews_api.js
+++ b/lib/cartodb/api/overviews_api.js
@@ -1,61 +1,45 @@
-var queue = require('queue-async');
-var QueryTablesApi = require('./query_tables_api');
-
-function OverviewsApi(pgQueryRunner) {
-   if (pgQueryRunner.pgQueryRunner !== undefined) {
-       this.queryTablesApi = pgQueryRunner;
-       this.pgQueryRunner = this.queryTablesApi.pgQueryRunner;
-   } else {
-       this.pgQueryRunner = pgQueryRunner;
-       this.queryTablesApi = new QueryTablesApi(pgQueryRunner);
-  }
+// TODO: use pgQueryRunner parameter directly
+function OverviewsApi(queryTablesApi) {
+   this.pgQueryRunner = queryTablesApi.pgQueryRunner;
 }
 
 module.exports = OverviewsApi;
 
-OverviewsApi.prototype.getOverviewsMetadata = function (username, sql, callback) {
-    var self = this;
-    this.queryTablesApi.getAffectedTablesInQuery(username, sql, function(err, tableNames){
-        if (err) {
-            callback(err);
-        } else {
-            var parallelism = 2;
-            var q = queue(parallelism);
-
-            tableNames.forEach(function(tableName) {
-                q.defer(function(done){
-                    var query = "SELECT * FROM CDB_Overviews('" + tableName + "');";
-                    self.pgQueryRunner.run(username, query, function handleOverviewsRows(err, rows) {
-                        if (err){
-                            var msg = err.message ? err.message : err;
-                            done(new Error('could not get overviews metadata: ' + msg));
-                            return;
-                        }
-                        done(null, [tableName, rows]);
-                    });
-                });
-            });
-
-            var metadata = {};
-            q.awaitAll(function(err, results){
-              if (err) {
-                  return callback(err);
-              } else {
-                  results.forEach(function(table_rows) {
-                      var tableName = table_rows[0];
-                      var rows = table_rows[1];
-                      if ( rows.length > 0 ) {
-                          var table_metadata = {};
-                          for ( var i=0; i<rows.length; ++i ) {
-                              var row = rows[i];
-                              table_metadata[row.z] = { table: row.overview_table };
-                          }
-                          metadata[tableName] = table_metadata;
-                      }
-                  });
-                  return callback(null, metadata);
-              }
-            });
-        }
-    });
+// TODO: share this with OverviewsApi? ... or maintain independence?
+var affectedTableRegexCache = {
+    bbox: /!bbox!/g,
+    scale_denominator: /!scale_denominator!/g,
+    pixel_width: /!pixel_width!/g,
+    pixel_height: /!pixel_height!/g
+};
+
+function prepareSql(sql) {
+    return sql
+        .replace(affectedTableRegexCache.bbox, 'ST_MakeEnvelope(0,0,0,0)')
+        .replace(affectedTableRegexCache.scale_denominator, '0')
+        .replace(affectedTableRegexCache.pixel_width, '1')
+        .replace(affectedTableRegexCache.pixel_height, '1')
+    ;
+}
+
+OverviewsApi.prototype.getOverviewsMetadata = function (username, sql, callback) {
+    var query = 'SELECT * FROM CDB_Overviews(CDB_QueryTablesText($windshaft$' + prepareSql(sql) + '$windshaft$))';
+    this.pgQueryRunner.run(username, query, function handleOverviewsRows(err, rows) {
+        if (err){
+            var msg = err.message ? err.message : err;
+            callback(new Error('could not get overviews metadata: ' + msg));
+            return;
+        }
+        var metadata = {};
+        rows.forEach(function(row) {
+            var table = row.base_table;
+            var table_metadata = metadata[table];
+            if ( !table_metadata ) {
+              table_metadata = metadata[table] = {};
+            }
+            table_metadata[row.z] = { table: row.overview_table };
+        });
+        return callback(null, metadata);
+    });
+
 };
diff --git a/test/acceptance/multilayer_server.js b/test/acceptance/multilayer_server.js
index a3ec8300..544aea6c 100644
--- a/test/acceptance/multilayer_server.js
+++ b/test/acceptance/multilayer_server.js
@@ -334,7 +334,7 @@ describe('tests from old api translated to multilayer', function() {
 
                 var parsed = JSON.parse(res.body);
                 assert.deepEqual(parsed, {
-                    errors: ["could not fetch source tables: fake error message"]
+                    errors: ["could not get overviews metadata: fake error message"]
                 });
 
                 done();
diff --git a/test/acceptance/overviews.js b/test/acceptance/overviews.js
index 01d2a994..12b23fe0 100644
--- a/test/acceptance/overviews.js
+++ b/test/acceptance/overviews.js
@@ -88,17 +88,17 @@ describe('overviews', function() {
               assert.deepEqual(non_overviews_layer, mapConfig._cfg.layers[1]);
               assert.equal(mapConfig._cfg.layers[0].type, 'cartodb');
               assert.ok(mapConfig._cfg.layers[0].options.overviews);
-              assert.ok(mapConfig._cfg.layers[0].options.overviews['public.test_table_overviews']);
-              assert.deepEqual(_.keys(mapConfig._cfg.layers[0].options.overviews), ['public.test_table_overviews']);
-              assert.equal(_.keys(mapConfig._cfg.layers[0].options.overviews['public.test_table_overviews']).length, 2);
-              assert.ok(mapConfig._cfg.layers[0].options.overviews['public.test_table_overviews'][1]);
-              assert.ok(mapConfig._cfg.layers[0].options.overviews['public.test_table_overviews'][2]);
+              assert.ok(mapConfig._cfg.layers[0].options.overviews.test_table_overviews);
+              assert.deepEqual(_.keys(mapConfig._cfg.layers[0].options.overviews), ['test_table_overviews']);
+              assert.equal(_.keys(mapConfig._cfg.layers[0].options.overviews.test_table_overviews).length, 2);
+              assert.ok(mapConfig._cfg.layers[0].options.overviews.test_table_overviews[1]);
+              assert.ok(mapConfig._cfg.layers[0].options.overviews.test_table_overviews[2]);
               assert.equal(
-                  mapConfig._cfg.layers[0].options.overviews['public.test_table_overviews'][1].table,
+                  mapConfig._cfg.layers[0].options.overviews.test_table_overviews[1].table,
                   'test_table_overviews_ov1'
               );
               assert.equal(
-                  mapConfig._cfg.layers[0].options.overviews['public.test_table_overviews'][2].table,
+                  mapConfig._cfg.layers[0].options.overviews.test_table_overviews[2].table,
                   'test_table_overviews_ov2'
               );
             });
diff --git a/test/integration/mapconfig_overviews_adapter.js b/test/integration/mapconfig_overviews_adapter.js
index 10a466a2..e6178d1b 100644
--- a/test/integration/mapconfig_overviews_adapter.js
+++ b/test/integration/mapconfig_overviews_adapter.js
@@ -75,17 +75,17 @@ describe('MapConfigOverviewsAdapter', function() {
             assert.equal(layers[0].options.cartocss, cartocss);
             assert.equal(layers[0].options.cartocss_version, cartocss_version);
             assert.ok(layers[0].options.overviews);
-            assert.ok(layers[0].options.overviews['public.test_table_overviews']);
-            assert.deepEqual(_.keys(layers[0].options.overviews), ['public.test_table_overviews']);
-            assert.equal(_.keys(layers[0].options.overviews['public.test_table_overviews']).length, 2);
-            assert.ok(layers[0].options.overviews['public.test_table_overviews'][1]);
-            assert.ok(layers[0].options.overviews['public.test_table_overviews'][2]);
+            assert.ok(layers[0].options.overviews.test_table_overviews);
+            assert.deepEqual(_.keys(layers[0].options.overviews), ['test_table_overviews']);
+            assert.equal(_.keys(layers[0].options.overviews.test_table_overviews).length, 2);
+            assert.ok(layers[0].options.overviews.test_table_overviews[1]);
+            assert.ok(layers[0].options.overviews.test_table_overviews[2]);
             assert.equal(
-                layers[0].options.overviews['public.test_table_overviews'][1].table,
+                layers[0].options.overviews.test_table_overviews[1].table,
                 'test_table_overviews_ov1'
               );
             assert.equal(
-                layers[0].options.overviews['public.test_table_overviews'][2].table,
+                layers[0].options.overviews.test_table_overviews[2].table,
                 'test_table_overviews_ov2'
               );
             done();
diff --git a/test/integration/overviews-api.js b/test/integration/overviews-api.js
index 4940955f..c7dface6 100644
--- a/test/integration/overviews-api.js
+++ b/test/integration/overviews-api.js
@@ -41,7 +41,7 @@ describe('OverviewsApi', function() {
             assert.ok(!err, err);
 
             assert.deepEqual(result, {
-                'public.test_table_overviews': {
+                'test_table_overviews': {
                   1: { table: 'test_table_overviews_ov1' },
                   2: { table: 'test_table_overviews_ov2' }
                 }
diff --git a/test/support/sql/CDB_Overviews.sql b/test/support/sql/CDB_Overviews.sql
index b5843f98..0acb16b9 100644
--- a/test/support/sql/CDB_Overviews.sql
+++ b/test/support/sql/CDB_Overviews.sql
@@ -1,13 +1,13 @@
 -- Mockup for CDB_Overviews
-CREATE OR REPLACE FUNCTION CDB_Overviews(table_name regclass)
-RETURNS TABLE(z integer, overview_table regclass)
+CREATE OR REPLACE FUNCTION CDB_Overviews(table_names regclass[])
+RETURNS TABLE(base_table regclass, z integer, overview_table regclass)
 AS $$
   BEGIN
-    IF table_name::text = 'test_table_overviews' THEN
+    IF (SELECT 'test_table_overviews'::regclass = ANY (table_names)) THEN
       RETURN QUERY
-        SELECT 1 AS z, 'test_table_overviews_ov1'::regclass AS overviw_table
+        SELECT 'test_table_overviews'::regclass AS base_table, 1 AS z, 'test_table_overviews_ov1'::regclass AS overviw_table
         UNION ALL
-        SELECT 2 AS z, 'test_table_overviews_ov2'::regclass AS overviw_table;
+        SELECT 'test_table_overviews'::regclass AS base_table, 2 AS z, 'test_table_overviews_ov2'::regclass AS overviw_table;
     ELSE
       RETURN;
     END IF;