Apply Carl suggestions

Author: sinclert-canonical

common/common/mysql_shell/__init__.py

@@ -144,18 +144,13 @@ def _get_mysql_roles(self, name_pattern: str) -> typing.Set[str]:
         logger.debug(f"MySQL roles found for {name_pattern=}: {len(rows)}")
         return {row[0] for row in rows}
 
-    def create_application_database(self, *, database: str) -> str:
+    def _create_application_database(self, *, database: str, rolename: str) -> str:
         """Create database for related database_provides application."""
-        role_name = f"charmed_dba_{database}"
-        if len(role_name) >= _ROLE_MAX_LENGTH:
-            logger.exception("Failed to create application database")
-            raise ValueError("Role name longer than 32 characters")
-
         statements = [
             f"CREATE DATABASE IF NOT EXISTS `{database}`",
-            f"CREATE ROLE IF NOT EXISTS `{role_name}`",
-            f"GRANT SELECT, INSERT, DELETE, UPDATE, EXECUTE ON `{database}`.* TO {role_name}",
-            f"GRANT ALTER, ALTER ROUTINE, CREATE, CREATE ROUTINE, CREATE VIEW, DROP, INDEX, LOCK TABLES, REFERENCES, TRIGGER ON `{database}`.* TO {role_name}",
+            f"CREATE ROLE IF NOT EXISTS `{rolename}`",
+            f"GRANT SELECT, INSERT, DELETE, UPDATE, EXECUTE ON `{database}`.* TO {rolename}",
+            f"GRANT ALTER, ALTER ROUTINE, CREATE, CREATE ROUTINE, CREATE VIEW, DROP, INDEX, LOCK TABLES, REFERENCES, TRIGGER ON `{database}`.* TO {rolename}",
         ]
 
         mysql_roles = self._get_mysql_roles("charmed_%")
@@ -173,7 +168,7 @@ def create_application_database(self, *, database: str) -> str:
         logger.debug(f"Created {database=}")
         return database
 
-    def create_application_user(self, *, database: str, username: str) -> str:
+    def _create_application_user(self, *, database: str, username: str) -> str:
         """Create database user for related database_provides application."""
         attributes = self._get_attributes()
         password = utils.generate_password()
@@ -185,6 +180,16 @@ def create_application_user(self, *, database: str, username: str) -> str:
         logger.debug(f"Created {username=} with {attributes=}")
         return password
 
+    def create_application_database(self, *, database: str, username: str) -> str:
+        """Create both the database and the relation user, returning its password."""
+        rolename = f"charmed_dba_{database}"
+        if len(rolename) >= _ROLE_MAX_LENGTH:
+            raise ValueError("Database DBA role longer than 32 characters")
+
+        ________ = self._create_application_database(database=database, rolename=rolename)
+        password = self._create_application_user(database=database, username=username)
+        return password
+
     def add_attributes_to_mysql_router_user(
         self, *, username: str, router_id: str, unit_name: str
     ) -> None:

common/common/relations/database_provides.py

@@ -30,12 +30,25 @@ class _UnsupportedExtraUserRole(status_exception.StatusException):
 
     def __init__(self, *, app_name: str, endpoint_name: str) -> None:
         message = (
-            f"{app_name} app requested unsupported extra user role on {endpoint_name} endpoint"
+            f"{app_name} app requested unsupported extra user role on "
+            f"{endpoint_name} endpoint"
         )
         logger.warning(message)
         super().__init__(ops.BlockedStatus(message))
 
 
+class _InvalidDatabaseName(status_exception.StatusException):
+    """Application charm requested an invalid database name"""
+
+    def __init__(self, *, app_name: str, endpoint_name: str, exception_msg: str) -> None:
+        message = (
+            f"{app_name} app requested an invalid database name on "
+            f"{endpoint_name} endpoint: {exception_msg}"
+        )
+        logger.warning(message)
+        super().__init__(ops.BlockedStatus(exception_msg))
+
+
 class _Relation:
     """Relation to one application charm"""
 
@@ -79,9 +92,11 @@ def __init__(
         if isinstance(event, ops.RelationBrokenEvent) and event.relation.id == self._id:
             raise _RelationBreaking
         self._database: str = self._databag["database"]
+        self._app_name: str = relation.app.name
+        self._endpoint_name: str = relation.endpoint.name
         if self._databag.get("extra-user-roles"):
             raise _UnsupportedExtraUserRole(
-                app_name=relation.app.name, endpoint_name=relation.name
+                app_name=self._app_name, endpoint_name=self._endpoint_name
             )
 
     def _set_databag(
@@ -121,15 +136,24 @@ def create_database_and_user(
         shell.delete_user(username, must_exist=False)
         logger.debug("Deleted user if exists before creating user")
 
-        ________ = shell.create_application_database(database=self._database)
-        password = shell.create_application_user(database=self._database, username=username)
-
-        self._set_databag(
-            username=username,
-            password=password,
-            router_read_write_endpoints=router_read_write_endpoints,
-            router_read_only_endpoints=router_read_only_endpoints,
-        )
+        try:
+            password = shell.create_application_database(
+                database=self._database,
+                username=username,
+            )
+        except ValueError as exception:
+            raise _InvalidDatabaseName(
+                app_name=self._app_name,
+                endpoint_name=self._endpoint_name,
+                exception_msg=str(exception),
+            )
+        else:
+            self._set_databag(
+                username=username,
+                password=password,
+                router_read_write_endpoints=router_read_write_endpoints,
+                router_read_only_endpoints=router_read_only_endpoints,
+            )
 
 
 class _UserNotShared(Exception):

machines/src/relations/deprecated_shared_db_database_provides.py

@@ -147,9 +147,10 @@ def create_database_and_user(
         shell.delete_user(self._username, must_exist=False)
         logger.debug("Deleted user if exists before creating user")
 
-        ________ = shell.create_application_database(database=self._database)
-        password = shell.create_application_user(database=self._database, username=self._username)
-
+        password = shell.create_application_database(
+            database=self._database,
+            username=self._username,
+        )
         self._peer_app_databag[self.peer_databag_password_key] = password
         self.set_databag(password=password)