Merge ~arturo-seijas/postgresql-charm:fix-enabling-extensions into postgresql-charm:master

[Arturo Enrique Seijas Fernández]

Fix DB extensions installation when schema is specified

Fixes a bug preventing the database extensions to be installed when the target schema is also specified, in the format extension:schema

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

This merge proposal is being monitored by mergebot. Change the status to Approved to merge.

[Tom Haddon (mthaddon)]

Looks great to me, thanks.

review: Approve

[🤖 Canonical IS Merge Bot (canonical-is-mergebot)]

Change successfully merged at revision 06b06260a39efa096fa8ca5e2ad2a182455c14bf

diff --git a/reactive/postgresql/client.py b/reactive/postgresql/client.py
index d80f771..9df9db3 100644
--- a/reactive/postgresql/client.py
+++ b/reactive/postgresql/client.py
@@ -15,7 +15,6 @@
 # along with this program.  If not, see <http://www.gnu.org/licenses/>.
 import json
 from itertools import chain
-import re
 
 from charmhelpers.core import hookenv, host
 from charms import leadership
@@ -27,7 +26,7 @@ from reactive.postgresql import helpers
 from reactive.postgresql import replication
 from reactive.postgresql import postgresql
 from reactive.postgresql.service import incoming_addresses
-from relations.pgsql.requires import ConnectionString
+from relations import pgsql
 
 from everyhook import everyhook
 
@@ -283,7 +282,7 @@ def relinfo_to_cs(relinfo):
     )
     if not all(d.values()):
         return None
-    return ConnectionString(**d)
+    return pgsql.requires.ConnectionString(**d)
 
 
 def ping_standbys():
@@ -335,14 +334,15 @@ def ensure_db_relation_resources(rel):
     # Create requested extensions. We never drop extensions, as there
     # may be dependent objects.
     if "extensions" in master:
-        extensions = list(filter(None, master.get("extensions", "").split(",")))
-        # Convert to the (extension, schema) tuple expected by
+        extensions_list = list(filter(None, master.get("extensions", "").split(",")))
+        # Convert to the (extension, schema) tuple defaulting schema to public, as expected by
         # postgresql.ensure_extensions
-        for i in range(0, len(extensions)):
-            m = re.search(r"^\s*([^(\s]+)\s*(?:\((\w+)\))?", extensions[i])
-            if m is None:
-                raise RuntimeError("Invalid extension {}".format(extensions[i]))
-            extensions[i] = (m.group(1), m.group(2) or "public")
+        extensions = []
+        for ext in extensions_list:
+            ext_sch = ext.split(":")
+            if(len(ext_sch) < 2):
+                ext_sch.append("public")
+            extensions.append(tuple(ext_sch))
         postgresql.ensure_extensions(con, extensions)
 
     con.commit()  # Don't throw away our changes.
diff --git a/unit_tests/test_client.py b/unit_tests/test_client.py
new file mode 100644
index 0000000..d569a26
--- /dev/null
+++ b/unit_tests/test_client.py
@@ -0,0 +1,51 @@
+# Copyright 2022 Canonical Ltd.
+#
+# This file is part of the PostgreSQL Charm for Juju.
+#
+# This program is free software: you can redistribute it and/or modify
+# it under the terms of the GNU General Public License version 3, as
+# published by the Free Software Foundation.
+#
+# This program is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranties of
+# MERCHANTABILITY, SATISFACTORY QUALITY, or FITNESS FOR A PARTICULAR
+# PURPOSE.  See the GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+import os
+import sys
+import unittest
+from charmhelpers.core import hookenv
+from unittest.mock import ANY, Mock, patch
+
+ROOT = os.path.abspath(os.path.join(os.path.dirname(__file__), os.pardir))
+sys.path.insert(1, ROOT)
+sys.path.insert(2, os.path.join(ROOT, 'lib'))
+sys.path.insert(3, os.path.join(ROOT, 'lib', 'testdeps'))
+
+from reactive.postgresql import client
+from reactive.postgresql import postgresql
+
+
+class TestClient(unittest.TestCase):
+    @patch.object(hookenv, "config")
+    @patch.object(postgresql, "connect")
+    @patch.object(postgresql, "ensure_database")
+    @patch.object(postgresql, "ensure_user")
+    @patch.object(postgresql, "grant_database_privileges")
+    @patch("psycopg2.connect")
+    def test_ensure_db_relation_resources(self, *_):
+        rel = Mock()
+        rel.local = {
+            "database": "exampledb",
+            "extensions": "ext1:public,ext2,ext3:schema1",
+            "password": "whatever",
+            "schema_user": "someuser",
+            "schema_password": "somepwd",
+            "user": "sampleuser",
+        }
+        expected_extensions = [("ext1", "public"), ("ext2", "public"), ("ext3", "schema1")]
+        with patch.object(postgresql, "ensure_extensions") as mock_ensure_extensions:
+            client.ensure_db_relation_resources(rel)
+            mock_ensure_extensions.assert_any_call(ANY, expected_extensions)