From 4cc328ef78c7b8108d7623fdd4fcae5294317f2e Mon Sep 17 00:00:00 2001 From: Frederick Muriuki Muriithi Date: Wed, 18 Jan 2023 14:59:35 +0300 Subject: auth: Fix tests after enforcing FOREIGN KEY constraint Fix a number of tests and fixtures that were not conforming to the FOREIGN KEY constraints: * Each test that creates a new "object" needs to clean up after itself * Each fixture that sets up test data needs to clean up after itself --- gn3/api/search.py | 2 +- .../20221110_01_WtZ1I-create-resources-table.py | 3 +- ...0221110_02_z1dWf-create-mrna-resources-table.py | 5 ++- ...10_03_ka3W0-create-phenotype-resources-table.py | 5 ++- ...110_04_6PRFQ-create-genotype-resources-table.py | 5 ++- ...1_RDlfx-modify-group-roles-add-group-role-id.py | 1 + mypy.ini | 3 ++ tests/unit/auth/fixtures/group_fixtures.py | 51 +++++++++++++++------- tests/unit/auth/fixtures/resource_fixtures.py | 9 +++- tests/unit/auth/test_groups.py | 7 ++- tests/unit/auth/test_resources.py | 9 +++- tests/unit/auth/test_roles.py | 3 +- tests/unit/auth/test_token.py | 9 +++- 13 files changed, 85 insertions(+), 27 deletions(-) diff --git a/gn3/api/search.py b/gn3/api/search.py index 4ba2590..3cdee67 100644 --- a/gn3/api/search.py +++ b/gn3/api/search.py @@ -153,7 +153,7 @@ def apply_si_suffix(location: str) -> int: return int(float(location[:-1])*10**suffixes.get(location[-1].lower(), 0)) -def parse_location_field(lifted_species: str, species_prefix: str, +def parse_location_field(lifted_species: str, species_prefix: str, # pylint: disable=[too-many-arguments] chromosome_prefix: str, location_slot: int, liftover_function: IntervalLiftoverFunction, query: bytes) -> xapian.Query: diff --git a/migrations/auth/20221110_01_WtZ1I-create-resources-table.py b/migrations/auth/20221110_01_WtZ1I-create-resources-table.py index f99d482..09ed891 100644 --- a/migrations/auth/20221110_01_WtZ1I-create-resources-table.py +++ b/migrations/auth/20221110_01_WtZ1I-create-resources-table.py @@ -11,9 +11,10 @@ steps = [ """ CREATE TABLE IF NOT EXISTS resources( group_id TEXT NOT NULL, - resource_id TEXT PRIMARY KEY, + resource_id TEXT NOT NULL, resource_name TEXT NOT NULL, resource_category_id TEXT NOT NULL, + PRIMARY KEY(group_id, resource_id), FOREIGN KEY(group_id) REFERENCES groups(group_id), FOREIGN KEY(resource_category_id) REFERENCES resource_categories(resource_category_id) ) WITHOUT ROWID diff --git a/migrations/auth/20221110_02_z1dWf-create-mrna-resources-table.py b/migrations/auth/20221110_02_z1dWf-create-mrna-resources-table.py index 0959116..d3f97ac 100644 --- a/migrations/auth/20221110_02_z1dWf-create-mrna-resources-table.py +++ b/migrations/auth/20221110_02_z1dWf-create-mrna-resources-table.py @@ -13,9 +13,12 @@ steps = [ step( """ CREATE TABLE IF NOT EXISTS mrna_resources( + group_id TEXT NOT NULL, resource_id TEXT PRIMARY KEY, dataset_id TEXT NOT NULL UNIQUE, - FOREIGN KEY(resource_id) REFERENCES resources(resource_id) + FOREIGN KEY(group_id, resource_id) + REFERENCES resources(group_id, resource_id) + ON UPDATE CASCADE ON DELETE RESTRICT ) WITHOUT ROWID """, "DROP TABLE IF EXISTS mrna_resources") diff --git a/migrations/auth/20221110_03_ka3W0-create-phenotype-resources-table.py b/migrations/auth/20221110_03_ka3W0-create-phenotype-resources-table.py index d241f77..7dde67e 100644 --- a/migrations/auth/20221110_03_ka3W0-create-phenotype-resources-table.py +++ b/migrations/auth/20221110_03_ka3W0-create-phenotype-resources-table.py @@ -13,10 +13,13 @@ steps = [ step( """ CREATE TABLE IF NOT EXISTS phenotype_resources( + group_id TEXT NOT NULL, resource_id TEXT NOT NULL, trait_id TEXT NOT NULL UNIQUE, PRIMARY KEY(resource_id, trait_id), - FOREIGN KEY(resource_id) REFERENCES resources(resource_id) + FOREIGN KEY(group_id, resource_id) + REFERENCES resources(group_id, resource_id) + ON UPDATE CASCADE ON DELETE RESTRICT ) WITHOUT ROWID """, "DROP TABLE IF EXISTS phenotype_resources") diff --git a/migrations/auth/20221110_04_6PRFQ-create-genotype-resources-table.py b/migrations/auth/20221110_04_6PRFQ-create-genotype-resources-table.py index 9200774..ed6e87d 100644 --- a/migrations/auth/20221110_04_6PRFQ-create-genotype-resources-table.py +++ b/migrations/auth/20221110_04_6PRFQ-create-genotype-resources-table.py @@ -13,10 +13,13 @@ steps = [ step( """ CREATE TABLE IF NOT EXISTS genotype_resources( + group_id TEXT NOT NULL, resource_id TEXT NOT NULL, trait_id TEXT NOT NULL UNIQUE, PRIMARY KEY(resource_id, trait_id), - FOREIGN KEY(resource_id) REFERENCES resources(resource_id) + FOREIGN KEY(group_id, resource_id) + REFERENCES resources(group_id, resource_id) + ON UPDATE CASCADE ON DELETE RESTRICT ) WITHOUT ROWID """, "DROP TABLE IF EXISTS genotype_resources") diff --git a/migrations/auth/20221117_01_RDlfx-modify-group-roles-add-group-role-id.py b/migrations/auth/20221117_01_RDlfx-modify-group-roles-add-group-role-id.py index 7115bc3..2deff10 100644 --- a/migrations/auth/20221117_01_RDlfx-modify-group-roles-add-group-role-id.py +++ b/migrations/auth/20221117_01_RDlfx-modify-group-roles-add-group-role-id.py @@ -33,6 +33,7 @@ steps = [ group_role_id TEXT PRIMARY KEY, group_id TEXT NOT NULL, role_id TEXT NOT NULL, + UNIQUE (group_id, role_id), FOREIGN KEY(group_id) REFERENCES groups(group_id), FOREIGN KEY(role_id) REFERENCES roles(role_id) ) WITHOUT ROWID diff --git a/mypy.ini b/mypy.ini index dfedb13..1fcf703 100644 --- a/mypy.ini +++ b/mypy.ini @@ -54,3 +54,6 @@ ignore_missing_imports = True [mypy-authlib.*] ignore_missing_imports = True + +[mypy-pymonad.*] +ignore_missing_imports = True diff --git a/tests/unit/auth/fixtures/group_fixtures.py b/tests/unit/auth/fixtures/group_fixtures.py index 72b96d1..1830374 100644 --- a/tests/unit/auth/fixtures/group_fixtures.py +++ b/tests/unit/auth/fixtures/group_fixtures.py @@ -7,6 +7,8 @@ from gn3.auth import db from gn3.auth.authorisation.groups import Group, GroupRole from gn3.auth.authorisation.resources import Resource, ResourceCategory +from .role_fixtures import RESOURCE_EDITOR_ROLE, RESOURCE_READER_ROLE + TEST_GROUP_01 = Group(uuid.UUID("9988c21d-f02f-4d45-8966-22c968ac2fbf"), "TheTestGroup") TEST_GROUP_02 = Group(uuid.UUID("e37d59d7-c05e-4d67-b479-81e627d8d634"), @@ -45,6 +47,9 @@ TEST_RESOURCES_GROUP_02 = ( TEST_RESOURCES = TEST_RESOURCES_GROUP_01 + TEST_RESOURCES_GROUP_02 TEST_RESOURCES_PUBLIC = (TEST_RESOURCES_GROUP_01[0], TEST_RESOURCES_GROUP_02[1]) +def __gtuple__(cursor): + return tuple(dict(row) for row in cursor.fetchall()) + @pytest.fixture(scope="function") def fxtr_group(conn_after_auth_migrations):# pylint: disable=[redefined-outer-name] """Fixture: setup a test group.""" @@ -57,6 +62,11 @@ def fxtr_group(conn_after_auth_migrations):# pylint: disable=[redefined-outer-na yield (conn_after_auth_migrations, TEST_GROUPS[0]) + with db.cursor(conn_after_auth_migrations) as cursor: + cursor.executemany( + "DELETE FROM groups WHERE group_id=?", + ((str(group.group_id),) for group in TEST_GROUPS)) + @pytest.fixture(scope="function") def fxtr_users_in_group(fxtr_group, fxtr_users):# pylint: disable=[redefined-outer-name, unused-argument] """Link the users to the groups.""" @@ -78,9 +88,8 @@ def fxtr_users_in_group(fxtr_group, fxtr_users):# pylint: disable=[redefined-out query_params) @pytest.fixture(scope="function") -def fxtr_group_roles(fxtr_group):# pylint: disable=[redefined-outer-name] +def fxtr_group_roles(fxtr_group, fxtr_roles):# pylint: disable=[redefined-outer-name,unused-argument] """Link roles to group""" - from .role_fixtures import RESOURCE_EDITOR_ROLE, RESOURCE_READER_ROLE# pylint: disable=[import-outside-toplevel] group_roles = ( GroupRole(uuid.UUID("9c25efb2-b477-4918-a95c-9914770cbf4d"), TEST_GROUP_01, RESOURCE_EDITOR_ROLE), @@ -96,11 +105,20 @@ def fxtr_group_roles(fxtr_group):# pylint: disable=[redefined-outer-name] yield conn, groups, group_roles + with db.cursor(conn) as cursor: + cursor.execute("SELECT * FROM group_user_roles_on_resources") + cursor.executemany( + ("DELETE FROM group_roles " + "WHERE group_role_id=? AND group_id=? AND role_id=?"), + ((str(role.group_role_id), str(role.group.group_id), + str(role.role.role_id)) + for role in group_roles)) + @pytest.fixture(scope="function") -def fxtr_group_user_roles(fxtr_users_in_group, fxtr_group_roles, fxtr_resources):#pylint: disable=[redefined-outer-name,unused-argument] +def fxtr_group_user_roles(fxtr_resources, fxtr_group_roles, fxtr_users_in_group):#pylint: disable=[redefined-outer-name,unused-argument] """Assign roles to users.""" - from .role_fixtures import RESOURCE_EDITOR_ROLE # pylint: disable=[import-outside-toplevel] - conn, _groups, _group_roles = fxtr_group_roles + conn, _groups, group_roles = fxtr_group_roles + _conn, group_resources = fxtr_resources _conn, _group, group_users = fxtr_users_in_group users = tuple(user for user in group_users if user.email not in ("unaff@iliated.user", "group@lead.er")) @@ -108,19 +126,22 @@ def fxtr_group_user_roles(fxtr_users_in_group, fxtr_group_roles, fxtr_resources) (user, RESOURCE_EDITOR_ROLE, TEST_RESOURCES_GROUP_01[1]) for user in users if user.email == "group@mem.ber01") with db.cursor(conn) as cursor: + params = tuple({ + "group_id": str(resource.group.group_id), + "user_id": str(user.user_id), + "role_id": str(role.role_id), + "resource_id": str(resource.resource_id) + } for user, role, resource in users_roles_resources) cursor.executemany( - ("INSERT INTO group_user_roles_on_resources VALUES (?, ?, ?, ?)"), - ((str(TEST_GROUP_01.group_id), str(user.user_id), str(role.role_id), - str(resource.resource_id)) - for user, role, resource in users_roles_resources)) + ("INSERT INTO group_user_roles_on_resources " + "VALUES (:group_id, :user_id, :role_id, :resource_id)"), + params) - yield conn + yield conn, group_users, group_roles, group_resources with db.cursor(conn) as cursor: cursor.executemany( ("DELETE FROM group_user_roles_on_resources WHERE " - "group_id=? AND user_id=? AND role_id=? AND " - "resource_id=?"), - ((str(TEST_GROUP_01.group_id), str(user.user_id), str(role.role_id), - str(resource.resource_id)) - for user, role, resource in users_roles_resources)) + "group_id=:group_id AND user_id=:user_id AND role_id=:role_id AND " + "resource_id=:resource_id"), + params) diff --git a/tests/unit/auth/fixtures/resource_fixtures.py b/tests/unit/auth/fixtures/resource_fixtures.py index 12d8bce..117b4f4 100644 --- a/tests/unit/auth/fixtures/resource_fixtures.py +++ b/tests/unit/auth/fixtures/resource_fixtures.py @@ -15,4 +15,11 @@ def fxtr_resources(fxtr_group):# pylint: disable=[redefined-outer-name] ((str(res.group.group_id), str(res.resource_id), res.resource_name, str(res.resource_category.resource_category_id), 1 if res.public else 0) for res in TEST_RESOURCES)) - return (conn, TEST_RESOURCES) + + yield (conn, TEST_RESOURCES) + + with db.cursor(conn) as cursor: + cursor.executemany( + "DELETE FROM resources WHERE group_id=? AND resource_id=?", + ((str(res.group.group_id), str(res.resource_id),) + for res in TEST_RESOURCES)) diff --git a/tests/unit/auth/test_groups.py b/tests/unit/auth/test_groups.py index 53e0543..158360e 100644 --- a/tests/unit/auth/test_groups.py +++ b/tests/unit/auth/test_groups.py @@ -73,10 +73,15 @@ def test_create_group_role(mocker, fxtr_users_in_group, fxtr_app, user, expected mocker.patch("gn3.auth.authorisation.groups.uuid4", uuid_fn) mocker.patch("gn3.auth.authorisation.roles.uuid4", uuid_fn) conn, _group, _users = fxtr_users_in_group - with fxtr_app.app_context() as flask_context: + with fxtr_app.app_context() as flask_context, db.cursor(conn) as cursor: flask_context.g.user = user assert create_group_role( conn, GROUP, "ResourceEditor", PRIVILEGES) == expected + # cleanup + cursor.execute( + ("DELETE FROM group_roles " + "WHERE group_role_id=? AND group_id=? AND role_id=?"), + (str(uuid_fn()), str(GROUP.group_id), str(uuid_fn()))) @pytest.mark.unit_test def test_create_multiple_groups(mocker, fxtr_app, fxtr_users): diff --git a/tests/unit/auth/test_resources.py b/tests/unit/auth/test_resources.py index b756cd9..e6ebeb9 100644 --- a/tests/unit/auth/test_resources.py +++ b/tests/unit/auth/test_resources.py @@ -3,6 +3,7 @@ import uuid import pytest +from gn3.auth import db from gn3.auth.authorisation.groups import Group from gn3.auth.authorisation.resources import ( Resource, user_resources, create_resource, ResourceCategory, @@ -35,10 +36,14 @@ def test_create_resource(mocker, fxtr_app, fxtr_users_in_group, user, expected): """Test that resource creation works as expected.""" mocker.patch("gn3.auth.authorisation.resources.uuid4", uuid_fn) conn, _group, _users = fxtr_users_in_group - with fxtr_app.app_context() as flask_context: + with fxtr_app.app_context() as flask_context, db.cursor(conn) as cursor: flask_context.g.user = user assert create_resource(conn, "test_resource", resource_category) == expected + # Cleanup + cursor.execute( + "DELETE FROM resources WHERE resource_id=?", (str(uuid_fn()),)) + SORTKEY = lambda resource: resource.resource_id @pytest.mark.unit_test @@ -74,5 +79,5 @@ def test_user_resources(fxtr_group_user_roles, user, expected): WHEN: a particular user's resources are requested THEN: list only the resources for which the user can access """ - conn = fxtr_group_user_roles + conn, *_others = fxtr_group_user_roles assert sorted(user_resources(conn, user), key=SORTKEY) == expected diff --git a/tests/unit/auth/test_roles.py b/tests/unit/auth/test_roles.py index 56ad39e..bee03fc 100644 --- a/tests/unit/auth/test_roles.py +++ b/tests/unit/auth/test_roles.py @@ -99,4 +99,5 @@ def test_user_roles(fxtr_group_user_roles, user, expected): WHEN: we request the user's privileges THEN: return **ALL** the privileges attached to the user """ - assert user_roles(fxtr_group_user_roles, user) == expected + conn, *_others = fxtr_group_user_roles + assert user_roles(conn, user) == expected diff --git a/tests/unit/auth/test_token.py b/tests/unit/auth/test_token.py index a1709bf..76316ea 100644 --- a/tests/unit/auth/test_token.py +++ b/tests/unit/auth/test_token.py @@ -2,6 +2,8 @@ import pytest +from gn3.auth import db + SUCCESS_RESULT = { "status_code": 200, "result": { @@ -42,7 +44,7 @@ def test_token(fxtr_app, fxtr_oauth2_clients, test_data, expected): back c) TODO: when user tries to use wrong client, we get error message back """ - _conn, oa2clients = fxtr_oauth2_clients + conn, oa2clients = fxtr_oauth2_clients email, password, client_idx = test_data data = { "grant_type": "password", "scope": "profile nonexistent-scope", @@ -50,8 +52,11 @@ def test_token(fxtr_app, fxtr_oauth2_clients, test_data, expected): "client_secret": oa2clients[client_idx].client_secret, "username": email, "password": password} - with fxtr_app.test_client() as client: + with fxtr_app.test_client() as client, db.cursor(conn) as cursor: res = client.post("/api/oauth2/token", data=data) + # cleanup db + cursor.execute("DELETE FROM oauth2_tokens WHERE access_token=?", + (gen_token(None, None, None, None),)) assert res.status_code == expected["status_code"] for key in expected["result"]: assert res.json[key] == expected["result"][key] -- cgit v1.2.3