From e22aaee5f5b6bf39c2573c1b222222822ed190c1 Mon Sep 17 00:00:00 2001
From: Raul Marin <git@rmr.ninja>
Date: Tue, 29 Oct 2019 16:47:35 +0100
Subject: [PATCH] Test re-registration of a server with different options

---
 scripts-available/CDB_FederatedServer.sql | 13 ++++---------
 test/CDB_FederatedServer.sql              | 19 ++++++++++++++++---
 test/CDB_FederatedServer_expect           |  6 ++++--
 3 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/scripts-available/CDB_FederatedServer.sql b/scripts-available/CDB_FederatedServer.sql
index d33c7fa..8328c5b 100644
--- a/scripts-available/CDB_FederatedServer.sql
+++ b/scripts-available/CDB_FederatedServer.sql
@@ -167,11 +167,11 @@ $$
 LANGUAGE PLPGSQL IMMUTABLE PARALLEL SAFE;
 
 
+-- Registers a new server
 CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Register_PG(server TEXT, config JSONB)
 RETURNS void
 AS $$
 DECLARE
-    -- TODO: Check and handle existing servers (if needed)
     server_internal name := @extschema@.__CDB_FS_Generate_Server_Name(input_name := server, check_existence := false);
     final_config json := @extschema@.__CDB_FS_credentials_to_user_mapping(@extschema@.__CDB_FS_add_default_options(config));
     role_name name;
@@ -200,14 +200,11 @@ BEGIN
                 USING HINT = 'Please clean the remaining objects"';
         END;
     END IF;
-    
-    -- TODO: Drop existing options in server ?
-    
+
     -- Add new options
     FOR row IN SELECT p.key, p.value from lateral json_each_text(final_config->'server') p
     LOOP
         IF NOT EXISTS (
-            -- TODO: Use pg_options_to_table
             WITH a AS (
                 SELECT split_part(unnest(srvoptions), '=', 1) AS options FROM pg_foreign_server WHERE srvname=server_internal
             ) SELECT * from a where options = row.key)
@@ -217,14 +214,11 @@ BEGIN
             EXECUTE FORMAT('ALTER SERVER %I OPTIONS (SET %I %L)', server_internal, row.key, row.value);
         END IF;
     END LOOP;
-    
-    -- TODO: Drop existing user mappings ?
-    
+
     -- Update user mapping settings
     FOR option IN SELECT o.key, o.value from lateral json_each_text(final_config->'user_mapping') o
     LOOP
         IF NOT EXISTS (
-            -- TODO: Use pg_options_to_table
             WITH a AS (
                 SELECT split_part(unnest(umoptions), '=', 1) as options from pg_user_mappings WHERE srvname = server_internal AND usename = 'public'
             ) SELECT * from a where options = option.key)
@@ -238,6 +232,7 @@ END
 $$
 LANGUAGE PLPGSQL VOLATILE PARALLEL UNSAFE;
 
+-- Drops a registered server and all the objects associated with it
 CREATE OR REPLACE FUNCTION @extschema@.CDB_Federated_Server_Unregister(server TEXT)
 RETURNS void
 AS $$
diff --git a/test/CDB_FederatedServer.sql b/test/CDB_FederatedServer.sql
index a445e73..58a033b 100644
--- a/test/CDB_FederatedServer.sql
+++ b/test/CDB_FederatedServer.sql
@@ -43,8 +43,22 @@ SELECT '2.3', cartodb.CDB_Federated_Server_List_Servers(server := 'myRemote');
 
 
 -- Re-register the second server
--- SELECT '3.1', cartodb.CDB_Federated_Server_Register_PG();
--- SELECT '3.2', cartodb.CDB_PG_Federated_Server_List_Servers();
+SELECT '3.1', cartodb.CDB_Federated_Server_Register_PG(server := 'myRemote2'::text, config := '{
+    "server": {
+        "dbname": "fdw_target",
+        "host": "localhost",
+        "port": @@PGPORT@@,
+        "extensions": "postgis",
+        "updatable": "false",
+        "use_remote_estimate": "true",
+        "fetch_size": "1000"
+    },
+    "credentials": {
+        "username": "other_remote_user",
+        "password": "foobarino"
+    }
+}'::jsonb);
+SELECT '3.2', cartodb.CDB_Federated_Server_List_Servers();
 
 -- Unregister #1
 SELECT '4.1', cartodb.CDB_Federated_Server_Unregister(server := 'myRemote'::text);
@@ -52,7 +66,6 @@ SELECT '4.2', cartodb.CDB_Federated_Server_List_Servers();
 
 -- Unregister a server that doesn't exist
 SELECT '5.1', cartodb.CDB_Federated_Server_Unregister(server := 'doesNotExist'::text);
-SELECT '5.2', cartodb.CDB_Federated_Server_List_Servers();
 
 -- Unregister #2
 SELECT '6.1', cartodb.CDB_Federated_Server_Unregister(server := 'myRemote2'::text);
diff --git a/test/CDB_FederatedServer_expect b/test/CDB_FederatedServer_expect
index 768b271..24c6479 100644
--- a/test/CDB_FederatedServer_expect
+++ b/test/CDB_FederatedServer_expect
@@ -4,8 +4,10 @@
 2.2|(myRemote,postgres_fdw,localhost,5432,,read-only,fdw_user)
 2.2|(myRemote2,postgres_fdw,localhost,5432,fdw_target,read-only,fdw_user)
 2.3|(myRemote,postgres_fdw,localhost,5432,,read-only,fdw_user)
+3.1|
+3.2|(myRemote,postgres_fdw,localhost,5432,,read-only,fdw_user)
+3.2|(myRemote2,postgres_fdw,localhost,5432,fdw_target,read-only,other_remote_user)
 4.1|
-4.2|(myRemote2,postgres_fdw,localhost,5432,fdw_target,read-only,fdw_user)
+4.2|(myRemote2,postgres_fdw,localhost,5432,fdw_target,read-only,other_remote_user)
 ERROR:  Server "doesNotExist" does not exist
-5.2|(myRemote2,postgres_fdw,localhost,5432,fdw_target,read-only,fdw_user)
 6.1|