about summary refs log tree commit diff
diff options
context:
space:
mode:
authorFrederick Muriuki Muriithi2024-09-16 15:14:40 -0500
committerFrederick Muriuki Muriithi2024-09-16 15:19:21 -0500
commit3c47081696e5a81f70e4ed509267725bc904434c (patch)
tree0a5290f789c3765ac3b228af7b4eddb12acbec4d
parente829074e99fd5bec033765d18d5efa55e1edce44 (diff)
downloadgn-auth-3c47081696e5a81f70e4ed509267725bc904434c.tar.gz
Pass cursor rather than connection to create_resource function
In order to decouple the `create_resource` function from the related
functions that assign roles to users, this commit changes the code to
pass in a cursor rather than a connection.

The cursor will be the same cursor passed into the role assignment
functions ensuring that the resource creation and role assignment
happen in a single transaction.
-rw-r--r--gn_auth/auth/authorisation/resources/models.py64
-rw-r--r--gn_auth/auth/authorisation/resources/views.py14
-rw-r--r--tests/unit/auth/test_resources.py17
3 files changed, 53 insertions, 42 deletions
diff --git a/gn_auth/auth/authorisation/resources/models.py b/gn_auth/auth/authorisation/resources/models.py
index 501ab91..fa7797b 100644
--- a/gn_auth/auth/authorisation/resources/models.py
+++ b/gn_auth/auth/authorisation/resources/models.py
@@ -17,7 +17,7 @@ from gn_auth.auth.errors import NotFoundError, AuthorisationError
 
 from .checks import authorised_for
 from .base import Resource, ResourceCategory
-from .groups.models import Group, user_group, is_group_leader
+from .groups.models import Group, is_group_leader
 from .mrna import (
     resource_data as mrna_resource_data,
     attach_resources_data as mrna_attach_resources_data,
@@ -34,8 +34,6 @@ from .phenotype import (
     link_data_to_resource as phenotype_link_data_to_resource,
     unlink_data_from_resource as phenotype_unlink_data_from_resource)
 
-from .errors import MissingGroupError
-
 def __assign_resource_owner_role__(cursor, resource, user):
     """Assign `user` the 'Resource Owner' role for `resource`."""
     cursor.execute("SELECT * FROM roles WHERE role_name='resource-owner'")
@@ -66,38 +64,36 @@ def resource_from_dbrow(row: sqlite3.Row):
 @authorised_p(("group:resource:create-resource",),
               error_description="Insufficient privileges to create a resource",
               oauth2_scope="profile resource")
-def create_resource(
-        conn: db.DbConnection, resource_name: str,
-        resource_category: ResourceCategory, user: User,
-        public: bool) -> Resource:
+def create_resource(# pylint: disable=[too-many-arguments]
+        cursor: sqlite3.Cursor,
+        resource_name: str,
+        resource_category: ResourceCategory,
+        user: User,
+        group: Group,
+        public: bool
+) -> Resource:
     """Create a resource item."""
-    with db.cursor(conn) as cursor:
-        group = user_group(conn, user).maybe(
-            False, lambda grp: grp)# type: ignore[misc, arg-type]
-        if not group:
-            raise MissingGroupError(# Not all resources require an owner group
-                "User with no group cannot create a resource.")
-        resource = Resource(uuid4(), resource_name, resource_category, public)
-        cursor.execute(
-            "INSERT INTO resources VALUES (?, ?, ?, ?)",
-            (str(resource.resource_id),
-             resource_name,
-             str(resource.resource_category.resource_category_id),
-             1 if resource.public else 0))
-        # TODO: @fredmanglis,@rookie101
-        # 1. Move the actions below into a (the?) hooks system
-        # 2. Do more checks: A resource can have varying hooks depending on type
-        #    e.g. if mRNA, pheno or geno resource, assign:
-        #           - "resource-owner"
-        #         if inbredset-group, assign:
-        #           - "resource-owner",
-        #           - "inbredset-group-owner" etc.
-        #         if resource is of type "group", assign:
-        #           - group-leader
-        cursor.execute("INSERT INTO resource_ownership (group_id, resource_id) "
-                       "VALUES (?, ?)",
-                       (str(group.group_id), str(resource.resource_id)))
-        __assign_resource_owner_role__(cursor, resource, user)
+    resource = Resource(uuid4(), resource_name, resource_category, public)
+    cursor.execute(
+        "INSERT INTO resources VALUES (?, ?, ?, ?)",
+        (str(resource.resource_id),
+         resource_name,
+         str(resource.resource_category.resource_category_id),
+         1 if resource.public else 0))
+    # TODO: @fredmanglis,@rookie101
+    # 1. Move the actions below into a (the?) hooks system
+    # 2. Do more checks: A resource can have varying hooks depending on type
+    #    e.g. if mRNA, pheno or geno resource, assign:
+    #           - "resource-owner"
+    #         if inbredset-group, assign:
+    #           - "resource-owner",
+    #           - "inbredset-group-owner" etc.
+    #         if resource is of type "group", assign:
+    #           - group-leader
+    cursor.execute("INSERT INTO resource_ownership (group_id, resource_id) "
+                   "VALUES (?, ?)",
+                   (str(group.group_id), str(resource.resource_id)))
+    __assign_resource_owner_role__(cursor, resource, user)
 
     return resource
 
diff --git a/gn_auth/auth/authorisation/resources/views.py b/gn_auth/auth/authorisation/resources/views.py
index 494fde9..23399e5 100644
--- a/gn_auth/auth/authorisation/resources/views.py
+++ b/gn_auth/auth/authorisation/resources/views.py
@@ -40,13 +40,14 @@ from gn_auth.auth.authentication.oauth2.resource_server import require_oauth
 from gn_auth.auth.authentication.users import User, user_by_id, user_by_email
 
 from .checks import authorised_for
+from .errors import MissingGroupError
+from .groups.models import Group, user_group
 from .models import (
     Resource, resource_data, resource_by_id, public_resources,
     resource_categories, assign_resource_user, link_data_to_resource,
     unassign_resource_user, resource_category_by_id, user_roles_on_resources,
     unlink_data_from_resource, create_resource as _create_resource,
     get_resource_id)
-from .groups.models import Group
 
 resources = Blueprint("resources", __name__)
 
@@ -68,13 +69,20 @@ def create_resource() -> Response:
         resource_name = form.get("resource_name")
         resource_category_id = UUID(form.get("resource_category"))
         db_uri = app.config["AUTH_DB"]
-        with db.connection(db_uri) as conn:
+        with (db.connection(db_uri) as conn,
+              db.cursor(conn) as cursor):
             try:
+                group = user_group(conn, the_token.user).maybe(
+                    False, lambda grp: grp)# type: ignore[misc, arg-type]
+                if not group:
+                    raise MissingGroupError(# Not all resources require an owner group
+                        "User with no group cannot create a resource.")
                 resource = _create_resource(
-                    conn,
+                    cursor,
                     resource_name,
                     resource_category_by_id(conn, resource_category_id),
                     the_token.user,
+                    group,
                     (form.get("public") == "on"))
                 return jsonify(asdict(resource))
             except sqlite3.IntegrityError as sql3ie:
diff --git a/tests/unit/auth/test_resources.py b/tests/unit/auth/test_resources.py
index 9b45b68..7f0b43d 100644
--- a/tests/unit/auth/test_resources.py
+++ b/tests/unit/auth/test_resources.py
@@ -47,11 +47,11 @@ def test_create_resource(# pylint: disable=[too-many-arguments, unused-argument]
             user,
             tuple(client for client in clients if client.user == user)[0]))
     conn, _group, _users = fxtr_users_in_group
-    resource = create_resource(
-        conn, "test_resource", resource_category, user, False)
-    assert resource == expected
 
     with db.cursor(conn) as cursor:
+        resource = create_resource(
+            cursor, "test_resource", resource_category, user, _group, False)
+        assert resource == expected
         # Cleanup
         cursor.execute(
             "DELETE FROM user_roles WHERE resource_id=?",
@@ -82,8 +82,15 @@ def test_create_resource_raises_for_unauthorised_users(
             tuple(client for client in clients if client.user == user)[0]))
     conn, _group, _users = fxtr_users_in_group
     with pytest.raises(AuthorisationError):
-        assert create_resource(
-            conn, "test_resource", resource_category, user, False) == expected
+        with db.cursor(conn) as cursor:
+            assert create_resource(
+                cursor,
+                "test_resource",
+                resource_category,
+                user,
+                _group,
+                False
+            ) == expected
 
 def sort_key_resources(resource):
     """Sort-key for resources."""