diff --git a/docs/coding-conventions.md b/docs/coding-conventions.md index 70d27fa46b4..958797f9940 100644 --- a/docs/coding-conventions.md +++ b/docs/coding-conventions.md @@ -15,7 +15,7 @@ Have a look at `ESLint`'s configuration files [.eslintrc.json](.eslintrc.json) a Have a look at `Pylint`'s configuration file [.pylintrc](.pylintrc). -## Posgres +## Postgres ### Foreign keys diff --git a/packages/postgres-database/scripts/copy_database_volume.sh b/packages/postgres-database/scripts/copy_database_volume.sh index 227c1e46d4e..92faf7aaf24 100755 --- a/packages/postgres-database/scripts/copy_database_volume.sh +++ b/packages/postgres-database/scripts/copy_database_volume.sh @@ -53,13 +53,13 @@ IFS=$(printf '\n\t') if [ ! -z $folder ]; then - #folder mode + #from folder to target volume ssh $host \ "tar -cf - $folder " \ | \ docker run --rm -i -v "$target":/to alpine ash -c "cd /to ; tar -xpvf - " else - #docker mode + #from docker volume to target volume ssh $host \ "docker run --rm -v $volume:/from alpine ash -c 'cd /from ; tar -cf - . '" \ | \ diff --git a/packages/postgres-database/scripts/erd/Dockerfile b/packages/postgres-database/scripts/erd/Dockerfile index a43612425b6..f3a849330cd 100644 --- a/packages/postgres-database/scripts/erd/Dockerfile +++ b/packages/postgres-database/scripts/erd/Dockerfile @@ -24,5 +24,4 @@ RUN pip --no-cache-dir install --upgrade \ RUN pip install --no-cache-dir \ pyparsing \ pydot \ - eralchemy \ sqlalchemy_schemadisplay diff --git a/packages/postgres-database/scripts/erd/main.py b/packages/postgres-database/scripts/erd/main.py index af19ead0964..199455938bf 100644 --- a/packages/postgres-database/scripts/erd/main.py +++ b/packages/postgres-database/scripts/erd/main.py @@ -1,50 +1,47 @@ # # ERD (Entity Relationship Diagram) is used to visualize these relationships # - +# - Using sqlalchemy_schemadisplay which is maintained by sqlalchemy +# - Already tried 'eralchemy' but fails with latest version and not maintained anymore +# +# SEE https://github.com/sqlalchemy/sqlalchemy/wiki/SchemaDisplay +# SEE https://github.com/Alexis-benoist/eralchemy # pylint: disable=wildcard-import # pylint: disable=unused-wildcard-import -# -import sys + from pathlib import Path +from typing import Any, Optional from simcore_postgres_database.models import * # registers all schemas in metadata from simcore_postgres_database.models.base import metadata +from sqlalchemy_schemadisplay import create_schema_graph -CURRENT_DIR = Path(sys.argv[0] if __name__ == "__main__" else __file__).resolve().parent - - -def create_with_sqlalchemy_schemadisplay(image_path: Path): - # SEE https://github.com/sqlalchemy/sqlalchemy/wiki/SchemaDisplay - from sqlalchemy_schemadisplay import create_schema_graph +def create_erd(image_path: Path, tables: Optional[list[str]] = None): + """ + create the pydot graph object by autoloading all tables via a bound metadata object + """ - # create the pydot graph object by autoloading all tables via a bound metadata object - - graph = create_schema_graph( - metadata=metadata, + kwargs: dict[str, Any] = dict( show_datatypes=True, # The image would get nasty big if we'd show the datatypes show_indexes=False, # ditto for indexes rankdir="LR", # From left to right (instead of top to bottom) concentrate=False, # Don't try to join the relation lines together ) - graph.write_svg(str(image_path.with_suffix(".svg"))) - graph.write_png(str(image_path.with_suffix(".png"))) - - -def create_with_eralchemy(image_path: Path): - # SEE https://github.com/Alexis-benoist/eralchemy - from eralchemy import render_er + if tables: + kwargs["tables"] = [metadata.tables[t] for t in tables] + else: + kwargs["metadata"] = metadata - for ext in (".png", ".svg"): - render_er(metadata, str(image_path.with_suffix(ext))) + graph = create_schema_graph(kwargs) + # pylint: disable=no-member + graph.write_svg(f'{image_path.with_suffix(".svg")}') + graph.write_png(f'{image_path.with_suffix(".png")}') if __name__ == "__main__": - - # FIXME: sqlalchemy_schemadisplay failes with json columns - # create_with_sqlalchemy_schemadisplay( output_dir / "postgres-database-models.svg") - - create_with_eralchemy(Path.cwd() / "postgres-database-models.ignore.ext") + path = Path("postgres-database-erd-ignore.svg") + create_erd(path) + print("Created", path) diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/3aa309471ff8_add_tag_to_groups_table_and_rm_tag_user_.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/3aa309471ff8_add_tag_to_groups_table_and_rm_tag_user_.py new file mode 100644 index 00000000000..759bfa167b8 --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/3aa309471ff8_add_tag_to_groups_table_and_rm_tag_user_.py @@ -0,0 +1,104 @@ +"""add tag_to_groups table and rm tag.user_id + +Revision ID: 3aa309471ff8 +Revises: c3c564121364 +Create Date: 2022-11-17 23:21:49.290958+00:00 + +""" +import sqlalchemy as sa +from alembic import op + +# revision identifiers, used by Alembic. +revision = "3aa309471ff8" +down_revision = "c3c564121364" +branch_labels = None +depends_on = None + + +def upgrade(): + op.create_table( + "tags_to_groups", + sa.Column("tag_id", sa.BigInteger(), nullable=False), + sa.Column("group_id", sa.BigInteger(), nullable=False), + sa.Column("read", sa.Boolean(), server_default=sa.text("true"), nullable=False), + sa.Column( + "write", sa.Boolean(), server_default=sa.text("false"), nullable=False + ), + sa.Column( + "delete", sa.Boolean(), server_default=sa.text("false"), nullable=False + ), + sa.Column( + "created", sa.DateTime(), server_default=sa.text("now()"), nullable=False + ), + sa.Column( + "modified", sa.DateTime(), server_default=sa.text("now()"), nullable=False + ), + sa.ForeignKeyConstraint( + ["group_id"], + ["groups.gid"], + name="fk_tag_to_group_group_id", + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.ForeignKeyConstraint( + ["tag_id"], + ["tags.id"], + name="fk_tag_to_group_tag_id", + onupdate="CASCADE", + ondelete="CASCADE", + ), + sa.UniqueConstraint("tag_id", "group_id"), + ) + + # transform every tags.user_id to its PRIMARY group_id in tags_to_groups + op.execute( + sa.DDL( + """ +INSERT INTO tags_to_groups (tag_id, group_id) SELECT tags.id, user_to_groups.gid +FROM tags JOIN users ON users.id = tags.user_id JOIN user_to_groups ON users.id = user_to_groups.uid JOIN groups ON groups.gid = user_to_groups.gid AND groups.type = 'PRIMARY' + """ + ) + ) + + # set full access for PRIMARY group_id + op.execute( + sa.DDL( + """ +UPDATE tags_to_groups SET write='True', delete='True', modified=now() + """ + ) + ) + + # Drop old tags.user_id + op.drop_constraint("tags_user_id_fkey", "tags", type_="foreignkey") + op.drop_column("tags", "user_id") + + +def downgrade(): + op.add_column( + "tags", + sa.Column("user_id", sa.BIGINT(), autoincrement=False, default=1), + ) + op.create_foreign_key( + "tags_user_id_fkey", + "tags", + "users", + ["user_id"], + ["id"], + onupdate="CASCADE", + ondelete="CASCADE", + ) + + # Sets tags.user_id + op.execute( + sa.DDL( + """ + UPDATE tags SET user_id=(SELECT users.id + FROM users JOIN tags_to_groups ON tags_to_groups.group_id = users.primary_gid + WHERE tags.id = tags_to_groups.tag_id) + """ + ) + ) + op.alter_column("tags", "user_id", nullable=False) + + op.drop_table("tags_to_groups") diff --git a/packages/postgres-database/src/simcore_postgres_database/models/_common.py b/packages/postgres-database/src/simcore_postgres_database/models/_common.py new file mode 100644 index 00000000000..313812a0c01 --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/models/_common.py @@ -0,0 +1,22 @@ +import sqlalchemy as sa + + +def column_created_datetime() -> sa.Column: + return sa.Column( + "created", + sa.DateTime(), + nullable=False, + server_default=sa.sql.func.now(), + doc="Timestamp auto-generated upon creation", + ) + + +def column_modified_datetime() -> sa.Column: + return sa.Column( + "modified", + sa.DateTime(), + nullable=False, + server_default=sa.sql.func.now(), + onupdate=sa.sql.func.now(), + doc="Timestamp with last row update", + ) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/projects_to_products.py b/packages/postgres-database/src/simcore_postgres_database/models/projects_to_products.py index c52990b43ec..3faaa96d3ec 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/projects_to_products.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/projects_to_products.py @@ -1,5 +1,6 @@ import sqlalchemy as sa +from ._common import column_created_datetime, column_modified_datetime from .base import metadata projects_to_products = sa.Table( @@ -29,21 +30,8 @@ nullable=False, doc="Products unique name", ), - # ----- - sa.Column( - "created", - sa.DateTime(), - nullable=False, - server_default=sa.sql.func.now(), - doc="Timestamp auto-generated upon creation", - ), - sa.Column( - "modified", - sa.DateTime(), - nullable=False, - server_default=sa.sql.func.now(), - onupdate=sa.sql.func.now(), - doc="Timestamp with last row update", - ), + # TIME STAMPS ---- + column_created_datetime(), + column_modified_datetime(), sa.UniqueConstraint("project_uuid", "product_name"), ) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/tags.py b/packages/postgres-database/src/simcore_postgres_database/models/tags.py index bce9eb39966..0806e268435 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/tags.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/tags.py @@ -1,36 +1,120 @@ import sqlalchemy as sa +from ._common import column_created_datetime, column_modified_datetime from .base import metadata -study_tags = sa.Table( - "study_tags", +# +# tags: a way to mark any entity (e.g. a project, ...) +# this can be used to perform operations as filter, select, compare, etc +# +tags = sa.Table( + "tags", metadata, sa.Column( - "study_id", - sa.BigInteger, - sa.ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), + "id", + sa.BigInteger(), + nullable=False, + primary_key=True, + ), + sa.Column( + "name", + sa.String(), + nullable=False, + doc="display name", + ), + sa.Column( + "description", + sa.String(), + nullable=True, + doc="description displayed", + ), + sa.Column( + "color", + sa.String(), nullable=False, + doc="Hex color (see https://www.color-hex.com/)", ), +) + + +# +# tags_to_groups: Maps tags with groups to define the level of access +# of a group (group_id) for the corresponding tag (tag_id) +# +tags_to_groups = sa.Table( + "tags_to_groups", + metadata, sa.Column( "tag_id", + sa.BigInteger(), + sa.ForeignKey( + tags.c.id, + onupdate="CASCADE", + ondelete="CASCADE", + name="fk_tag_to_group_tag_id", + ), + nullable=False, + doc="Tag unique ID", + ), + sa.Column( + "group_id", sa.BigInteger, - sa.ForeignKey("tags.id", onupdate="CASCADE", ondelete="CASCADE"), + sa.ForeignKey( + "groups.gid", + onupdate="CASCADE", + ondelete="CASCADE", + name="fk_tag_to_group_group_id", + ), nullable=False, + doc="Group unique ID", ), - sa.UniqueConstraint("study_id", "tag_id"), + # ACCESS RIGHTS --- + sa.Column( + "read", + sa.Boolean(), + nullable=False, + server_default=sa.sql.expression.true(), + doc="If true, group can *read* a tag." + "This column can be used to set the tag invisible", + ), + sa.Column( + "write", + sa.Boolean(), + nullable=False, + server_default=sa.sql.expression.false(), + doc="If true, group can *create* and *update* a tag", + ), + sa.Column( + "delete", + sa.Boolean(), + nullable=False, + server_default=sa.sql.expression.false(), + doc="If true, group can *delete* the tag", + ), + # TIME STAMPS ---- + column_created_datetime(), + column_modified_datetime(), + sa.UniqueConstraint("tag_id", "group_id"), ) -tags = sa.Table( - "tags", + +# +# study_tags: projects marked with tags +# +study_tags = sa.Table( + "study_tags", metadata, - sa.Column("id", sa.BigInteger, nullable=False, primary_key=True), sa.Column( - "user_id", + "study_id", sa.BigInteger, - sa.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), + sa.ForeignKey("projects.id", onupdate="CASCADE", ondelete="CASCADE"), nullable=False, ), - sa.Column("name", sa.String, nullable=False), - sa.Column("description", sa.String, nullable=True), - sa.Column("color", sa.String, nullable=False), + sa.Column( + "tag_id", + sa.BigInteger, + sa.ForeignKey("tags.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ), + sa.UniqueConstraint("study_id", "tag_id"), ) diff --git a/packages/postgres-database/src/simcore_postgres_database/utils_tags.py b/packages/postgres-database/src/simcore_postgres_database/utils_tags.py new file mode 100644 index 00000000000..69059482ead --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/utils_tags.py @@ -0,0 +1,238 @@ +""" Repository pattern, errors and data structures for models.tags +""" + +import functools +from dataclasses import dataclass +from typing import Optional, TypedDict + +import sqlalchemy as sa +from aiopg.sa.connection import SAConnection +from simcore_postgres_database.models.groups import user_to_groups +from simcore_postgres_database.models.tags import tags, tags_to_groups +from simcore_postgres_database.models.users import users + + +# +# Errors +# +class BaseTagError(Exception): + pass + + +class TagNotFoundError(BaseTagError): + pass + + +class TagOperationNotAllowed(BaseTagError): # maps to AccessForbidden + pass + + +# +# Repository: interface layer over pg database +# +class TagDict(TypedDict, total=True): + # NOTE: ONLY used as returned value, otherwise used + id: int + name: int + description: str + color: str + + +_TAG_COLUMNS = [tags.c.id, tags.c.name, tags.c.description, tags.c.color] + + +@dataclass(frozen=True) +class TagsRepo: + user_id: int + + def _join_user_groups_tag( + self, + access_condition, + tag_id: int, + ): + j = user_to_groups.join( + tags_to_groups, + (user_to_groups.c.uid == self.user_id) + & (user_to_groups.c.gid == tags_to_groups.c.group_id) + & (access_condition) + & (tags_to_groups.c.tag_id == tag_id), + ) + return j + + def _join_user_to_given_tag(self, access_condition, tag_id: int): + j = self._join_user_groups_tag( + access_condition=access_condition, + tag_id=tag_id, + ).join(tags) + return j + + def _join_user_to_tags( + self, + access_condition, + ): + j = user_to_groups.join( + tags_to_groups, + (user_to_groups.c.uid == self.user_id) + & (user_to_groups.c.gid == tags_to_groups.c.group_id) + & (access_condition), + ).join(tags) + return j + + async def access_count( + self, + conn: SAConnection, + tag_id: int, + *, + read: Optional[bool] = None, + write: Optional[bool] = None, + delete: Optional[bool] = None, + ) -> int: + """ + Returns 0 if tag does not match access + Returns >0 if it does and represents the number of groups granting this access to the user + """ + access = [] + if read is not None: + access.append(tags_to_groups.c.read == read) + if write is not None: + access.append(tags_to_groups.c.write == write) + if delete is not None: + access.append(tags_to_groups.c.delete == delete) + + if not access: + raise ValueError("Undefined access") + + j = self._join_user_groups_tag( + access_condition=functools.reduce(sa.and_, access), + tag_id=tag_id, + ) + stmt = sa.select(sa.func.count(user_to_groups.c.uid)).select_from(j) + + # The number of occurrences of the user_id = how many groups are giving this access permission + permissions_count: Optional[int] = await conn.scalar(stmt) + return permissions_count if permissions_count else 0 + + # + # CRUD operations + # + + async def create( + self, + conn: SAConnection, + *, + name: str, + color: str, + description: Optional[str] = None, + read: bool = True, + write: bool = True, + delete: bool = True, + ) -> TagDict: + values = {"name": name, "color": color} + if description: + values["description"] = description + + async with conn.begin(): + # insert new tag + insert_stmt = tags.insert().values(**values).returning(*_TAG_COLUMNS) + result = await conn.execute(insert_stmt) + row = await result.first() + assert row # nosec + + # take tag ownership + scalar_subq = ( + sa.select(users.c.primary_gid) + .where(users.c.id == self.user_id) + .scalar_subquery() + ) + await conn.execute( + tags_to_groups.insert().values( + tag_id=row.id, + group_id=scalar_subq, + read=read, + write=write, + delete=delete, + ) + ) + return TagDict(row.items()) # type: ignore + + async def list(self, conn: SAConnection) -> list[TagDict]: + select_stmt = ( + sa.select(_TAG_COLUMNS) + .select_from(self._join_user_to_tags(tags_to_groups.c.read == True)) + .order_by(tags.c.id) + ) + return [TagDict(row.items()) async for row in conn.execute(select_stmt)] # type: ignore + + async def get(self, conn: SAConnection, tag_id: int) -> TagDict: + select_stmt = sa.select(_TAG_COLUMNS).select_from( + self._join_user_to_given_tag(tags_to_groups.c.read == True, tag_id=tag_id) + ) + + result = await conn.execute(select_stmt) + row = await result.first() + if not row: + raise TagNotFoundError( + f"{tag_id=} not found: either no access or does not exists" + ) + return TagDict(row.items()) # type: ignore + + async def update( + self, + conn: SAConnection, + tag_id: int, + *, + name: Optional[str] = None, + color: Optional[str] = None, + description: Optional[str] = None, + ) -> TagDict: + updates = {} + if name: + updates["name"] = name + if color: + updates["color"] = color + if description: + updates["description"] = description + + update_stmt = ( + tags.update() + .where(tags.c.id == tag_id) + .where( + (tags.c.id == tags_to_groups.c.tag_id) + & (tags_to_groups.c.write == True) + ) + .where( + (tags_to_groups.c.group_id == user_to_groups.c.gid) + & (user_to_groups.c.uid == self.user_id) + ) + .values(**updates) + .returning(*_TAG_COLUMNS) + ) + + result = await conn.execute(update_stmt) + row = await result.first() + if not row: + raise TagOperationNotAllowed( + f"{tag_id=} not updated: either no access or not found" + ) + + return TagDict(row.items()) # type: ignore + + async def delete(self, conn: SAConnection, tag_id: int) -> None: + delete_stmt = ( + tags.delete() + .where(tags.c.id == tag_id) + .where( + (tags_to_groups.c.tag_id == tag_id) & (tags_to_groups.c.delete == True) + ) + .where( + (tags_to_groups.c.group_id == user_to_groups.c.gid) + & (user_to_groups.c.uid == self.user_id) + ) + .returning(tags_to_groups.c.delete) + ) + + deleted = await conn.scalar(delete_stmt) + if not deleted: + raise TagOperationNotAllowed( + f"Could not delete {tag_id=}. Not found or insuficient access." + ) diff --git a/packages/postgres-database/tests/conftest.py b/packages/postgres-database/tests/conftest.py index d85337521a6..91c9e5e3f45 100644 --- a/packages/postgres-database/tests/conftest.py +++ b/packages/postgres-database/tests/conftest.py @@ -1,15 +1,26 @@ # pylint: disable=no-value-for-parameter -# pylint:disable=unused-variable -# pylint:disable=unused-argument -# pylint:disable=redefined-outer-name +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable -from typing import AsyncIterator, Awaitable, Callable, Union +from typing import AsyncIterator, Awaitable, Callable, Iterator, Optional, Union import aiopg.sa +import aiopg.sa.exc import pytest import sqlalchemy as sa import yaml +from aiopg.sa.connection import SAConnection from aiopg.sa.engine import Engine +from aiopg.sa.result import ResultProxy, RowProxy +from pytest_simcore.helpers.rawdata_fakers import random_group, random_user +from simcore_postgres_database.webserver_models import ( + GroupType, + groups, + user_to_groups, + users, +) +from sqlalchemy import literal_column pytest_plugins = [ "pytest_simcore.repository_paths", @@ -42,7 +53,9 @@ def postgres_service(docker_services, docker_ip, docker_compose_file) -> str: @pytest.fixture -def make_engine(postgres_service: str) -> Callable: +def make_engine( + postgres_service: str, +) -> Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]]: dsn = postgres_service def maker(*, is_async=True) -> Union[Awaitable[Engine], sa.engine.base.Engine]: @@ -71,7 +84,7 @@ def db_metadata(): @pytest.fixture -async def pg_engine(make_engine, db_metadata) -> AsyncIterator[Engine]: +async def pg_engine(make_engine: Callable, db_metadata) -> AsyncIterator[Engine]: async_engine = await make_engine(is_async=True) # NOTE: Using migration to upgrade/downgrade is not @@ -95,3 +108,72 @@ async def pg_engine(make_engine, db_metadata) -> AsyncIterator[Engine]: # NOTE: ALL is deleted after db_metadata.drop_all(sync_engine) sync_engine.dispose() + + +# +# FACTORY FIXTURES +# + + +@pytest.fixture +def create_fake_group( + make_engine: Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]] +) -> Iterator[Callable]: + """factory to create standard group""" + created_ids = [] + + async def _create_group(conn: SAConnection, **overrides) -> RowProxy: + result: ResultProxy = await conn.execute( + groups.insert() + .values(**random_group(type=GroupType.STANDARD, **overrides)) + .returning(literal_column("*")) + ) + group = await result.fetchone() + assert group + created_ids.append(group.gid) + return group + + yield _create_group + + sync_engine = make_engine(is_async=False) + sync_engine.execute(groups.delete().where(groups.c.gid.in_(created_ids))) + + +@pytest.fixture +def create_fake_user( + make_engine: Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]] +) -> Iterator[Callable]: + """factory to create a user w/ or w/o a standard group""" + + created_ids = [] + + async def _create_user( + conn, group: Optional[RowProxy] = None, **overrides + ) -> RowProxy: + user_id = await conn.scalar( + users.insert().values(**random_user(**overrides)).returning(users.c.id) + ) + assert user_id is not None + + # This is done in two executions instead of one (e.g. returning(literal_column("*")) ) + # to allow triggering function in db that + # insert primary_gid column + r = await conn.execute(users.select().where(users.c.id == user_id)) + assert r.rowcount == 1 + user = await r.first() + assert user + + created_ids.append(user.id) + + if group: + assert group.type == GroupType.STANDARD.name + result = await conn.execute( + user_to_groups.insert().values(uid=user.id, gid=group.gid) + ) + assert result + return user + + yield _create_user + + sync_engine = make_engine(is_async=False) + sync_engine.execute(users.delete().where(users.c.id.in_(created_ids))) diff --git a/packages/postgres-database/tests/test_groups.py b/packages/postgres-database/tests/test_groups.py index 36c84ebaf0f..78bfc6d8638 100644 --- a/packages/postgres-database/tests/test_groups.py +++ b/packages/postgres-database/tests/test_groups.py @@ -2,13 +2,15 @@ # pylint: disable=no-value-for-parameter -from typing import Optional +from typing import Awaitable, Callable, Optional, Union import aiopg.sa.exc import pytest +import sqlalchemy as sa +from aiopg.sa.engine import Engine from aiopg.sa.result import ResultProxy, RowProxy from psycopg2.errors import ForeignKeyViolation, RaiseException, UniqueViolation -from pytest_simcore.helpers.rawdata_fakers import random_group, random_user +from pytest_simcore.helpers.rawdata_fakers import random_user from simcore_postgres_database.models.base import metadata from simcore_postgres_database.webserver_models import ( GroupType, @@ -19,35 +21,19 @@ from sqlalchemy import func, literal_column, select -async def _create_group(conn, **overrides) -> RowProxy: - result = await conn.execute( - groups.insert() - .values(**random_group(**overrides)) - .returning(literal_column("*")) - ) - return await result.fetchone() - - -async def _create_user(conn, name: str, group: RowProxy) -> RowProxy: - result = await conn.execute( - users.insert().values(**random_user(name=name)).returning(literal_column("*")) - ) - user = await result.fetchone() - result = await conn.execute( - user_to_groups.insert().values(uid=user.id, gid=group.gid) - ) - return user - - -async def test_user_group_uniqueness(make_engine): +async def test_user_group_uniqueness( + make_engine: Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]], + create_fake_group: Callable, + create_fake_user: Callable, +): engine = await make_engine() sync_engine = make_engine(is_async=False) metadata.drop_all(sync_engine) metadata.create_all(sync_engine) async with engine.acquire() as conn: - rory_group = await _create_group(conn, name="Rory Storm and the Hurricanes") - ringo = await _create_user(conn, "Ringo", rory_group) + rory_group = await create_fake_group(conn, name="Rory Storm and the Hurricanes") + ringo = await create_fake_user(conn, name="Ringo", group=rory_group) # test unique user/group pair with pytest.raises(UniqueViolation, match="user_to_groups_uid_gid_key"): await conn.execute( @@ -66,7 +52,9 @@ async def test_user_group_uniqueness(make_engine): await res.fetchone() -async def test_all_group(make_engine): +async def test_all_group( + make_engine: Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]] +): engine = await make_engine() sync_engine = make_engine(is_async=False) metadata.drop_all(sync_engine) @@ -117,7 +105,9 @@ async def test_all_group(make_engine): assert all_group_gid == 1 # it's the first group so it gets a 1 -async def test_own_group(make_engine): +async def test_own_group( + make_engine: Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]] +): engine = await make_engine() sync_engine = make_engine(is_async=False) metadata.drop_all(sync_engine) @@ -167,19 +157,23 @@ async def test_own_group(make_engine): assert relations_count == (users_count + users_count) -async def test_group(make_engine): +async def test_group( + make_engine: Callable[[bool], Union[Awaitable[Engine], sa.engine.base.Engine]], + create_fake_group: Callable, + create_fake_user: Callable, +): engine = await make_engine() sync_engine = make_engine(is_async=False) metadata.drop_all(sync_engine) metadata.create_all(sync_engine) async with engine.acquire() as conn: - rory_group = await _create_group(conn, name="Rory Storm and the Hurricanes") - quarrymen_group = await _create_group(conn, name="The Quarrymen") - await _create_user(conn, "John", quarrymen_group) - await _create_user(conn, "Paul", quarrymen_group) - await _create_user(conn, "Georges", quarrymen_group) - pete = await _create_user(conn, "Pete", quarrymen_group) - ringo = await _create_user(conn, "Ringo", rory_group) + rory_group = await create_fake_group(conn, name="Rory Storm and the Hurricanes") + quarrymen_group = await create_fake_group(conn, name="The Quarrymen") + await create_fake_user(conn, name="John", group=quarrymen_group) + await create_fake_user(conn, name="Paul", group=quarrymen_group) + await create_fake_user(conn, name="Georges", group=quarrymen_group) + pete = await create_fake_user(conn, name="Pete", group=quarrymen_group) + ringo = await create_fake_user(conn, name="Ringo", group=rory_group) # rationale: following linux user/group system, each user has its own group (primary group) + whatever other group (secondary groups) # check DB contents diff --git a/packages/postgres-database/tests/test_models_tags.py b/packages/postgres-database/tests/test_models_tags.py new file mode 100644 index 00000000000..8a9caf9aa2d --- /dev/null +++ b/packages/postgres-database/tests/test_models_tags.py @@ -0,0 +1,49 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + + +import pytest +import sqlalchemy as sa +from simcore_postgres_database.models.base import metadata +from simcore_postgres_database.models.tags import tags_to_groups +from simcore_postgres_database.models.users import users + + +@pytest.mark.skip(reason="DEV only") +def test_migration_downgrade_script(): + # NOTE: This test keeps for the record how the downgrade expression in + # migration/versions/3aa309471ff8_add_tag_to_groups_table_and_rm_tag_user_.py + # was deduced. + old_tags = sa.Table( + "old_tags", + metadata, + sa.Column("id", sa.BigInteger, nullable=False, primary_key=True), + sa.Column( + "user_id", + sa.BigInteger, + sa.ForeignKey("users.id", onupdate="CASCADE", ondelete="CASCADE"), + nullable=False, + ), + sa.Column("name", sa.String, nullable=False), + sa.Column("description", sa.String, nullable=True), + sa.Column("color", sa.String, nullable=False), + ) + + j = users.join(tags_to_groups, tags_to_groups.c.group_id == users.c.primary_gid) + + scalar_subq = ( + sa.select(users.c.id) + .select_from(j) + .where(old_tags.c.id == tags_to_groups.c.tag_id) + .scalar_subquery() + ) + + update_stmt = old_tags.update().values(user_id=scalar_subq) + + assert str(update_stmt).split("\n") == [ + "UPDATE old_tags SET user_id=(SELECT users.id ", + "FROM users JOIN tags_to_groups ON tags_to_groups.group_id = users.primary_gid ", + "WHERE old_tags.id = tags_to_groups.tag_id)", + ] diff --git a/packages/postgres-database/tests/test_utils_tags.py b/packages/postgres-database/tests/test_utils_tags.py new file mode 100644 index 00000000000..4c0a25a3c3f --- /dev/null +++ b/packages/postgres-database/tests/test_utils_tags.py @@ -0,0 +1,513 @@ +# pylint: disable=redefined-outer-name +# pylint: disable=unused-argument +# pylint: disable=unused-variable +# pylint: disable=too-many-arguments + +from typing import Any, AsyncIterator, Awaitable, Callable + +import pytest +import sqlalchemy as sa +from aiopg.sa.connection import SAConnection +from aiopg.sa.engine import Engine +from aiopg.sa.result import RowProxy +from simcore_postgres_database.models.tags import tags, tags_to_groups +from simcore_postgres_database.models.users import UserRole, UserStatus +from simcore_postgres_database.utils_tags import ( + TagNotFoundError, + TagOperationNotAllowed, + TagsRepo, +) + + +@pytest.fixture +async def connection(pg_engine: Engine) -> AsyncIterator[SAConnection]: + async with pg_engine.acquire() as _conn: + yield _conn + + +@pytest.fixture +async def group( + create_fake_group: Callable[[SAConnection], Awaitable[RowProxy]], + connection: SAConnection, +) -> RowProxy: + group_ = await create_fake_group(connection) + assert group_ + assert group_.type == "STANDARD" + return group_ + + +@pytest.fixture +async def user( + create_fake_user: Callable[[SAConnection, RowProxy, Any], Awaitable[RowProxy]], + group: RowProxy, + connection: SAConnection, +) -> RowProxy: + user_ = await create_fake_user( + connection, + group=group, + status=UserStatus.ACTIVE, + role=UserRole.USER, + ) + + # note that this user belongs to two groups! + assert user_.primary_gid != group.gid + + return user_ + + +@pytest.fixture +async def other_user( + create_fake_user: Callable[[SAConnection, RowProxy, Any], RowProxy], + connection: SAConnection, +) -> RowProxy: + user_ = await create_fake_user( + connection, + status=UserStatus.ACTIVE, + role=UserRole.USER, + ) + return user_ + + +async def _create_tag_access( + conn: SAConnection, + *, + tag_id, + group_id, + read, + write, + delete, +) -> int: + await conn.execute( + tags_to_groups.insert().values( + tag_id=tag_id, group_id=group_id, read=read, write=write, delete=delete + ) + ) + return tag_id + + +async def _create_tag( + conn: SAConnection, + *, + name, + description, + color, + group_id, + read, + write, + delete, +) -> int: + """helper to create a tab by inserting rows in two different tables""" + tag_id = await conn.scalar( + tags.insert() + .values(name=name, description=description, color=color) + .returning(tags.c.id) + ) + assert tag_id + await _create_tag_access( + conn, tag_id=tag_id, group_id=group_id, read=read, write=write, delete=delete + ) + return tag_id + + +async def test_tags_access_with_primary_groups( + connection: SAConnection, user: RowProxy, group: RowProxy, other_user: RowProxy +): + conn = connection + + (tag_id, other_tag_id) = [ + await _create_tag( + conn, + name="T1", + description="tag 1", + color="blue", + group_id=user.primary_gid, + read=True, + write=True, + delete=True, + ), + await _create_tag( + conn, + name="T2", + description="tag for other_user", + color="yellow", + group_id=other_user.primary_gid, + read=True, + write=True, + delete=True, + ), + ] + + tags_repo = TagsRepo(user_id=user.id) + + # repo has access + assert ( + await tags_repo.access_count(conn, tag_id, read=True, write=True, delete=True) + == 1 + ) + assert await tags_repo.access_count(conn, tag_id, read=True, write=True) == 1 + assert await tags_repo.access_count(conn, tag_id, read=True) == 1 + assert await tags_repo.access_count(conn, tag_id, write=True) == 1 + + # changing access conditions + assert ( + await tags_repo.access_count( + conn, + tag_id, + read=True, + write=True, + delete=False, # <--- + ) + == 0 + ) + + # user will have NO access to other user's tags even matching access rights + assert ( + await tags_repo.access_count( + conn, other_tag_id, read=True, write=True, delete=True + ) + == 0 + ) + + +async def test_tags_access_with_multiple_groups( + connection: SAConnection, user: RowProxy, group: RowProxy, other_user: RowProxy +): + conn = connection + + (tag_id, other_tag_id, group_tag_id, everyone_tag_id) = [ + await _create_tag( + conn, + name="T1", + description="tag 1", + color="blue", + group_id=user.primary_gid, + read=True, + write=True, + delete=True, + ), + await _create_tag( + conn, + name="T2", + description="tag for other_user", + color="yellow", + group_id=other_user.primary_gid, + read=True, + write=True, + delete=True, + ), + await _create_tag( + conn, + name="TG", + description="read-write tag shared in a GROUP ( currently only user)", + color="read", + group_id=group.gid, + read=True, + write=True, + delete=False, + ), + await _create_tag( + conn, + name="TE", + description="read-only tag shared with EVERYONE", + color="pink", + group_id=1, + read=True, + write=False, + delete=False, + ), + ] + + tags_repo = TagsRepo(user_id=user.id) + other_repo = TagsRepo(user_id=other_user.id) + + # tag_id + assert ( + await tags_repo.access_count(conn, tag_id, read=True, write=True, delete=True) + == 1 + ) + assert ( + await other_repo.access_count(conn, tag_id, read=True, write=True, delete=True) + == 0 + ) + + # other_tag_id + assert await tags_repo.access_count(conn, other_tag_id, read=True) == 0 + assert await other_repo.access_count(conn, other_tag_id, read=True) == 1 + + # group_tag_id + assert await tags_repo.access_count(conn, group_tag_id, read=True) == 1 + assert await other_repo.access_count(conn, group_tag_id, read=True) == 0 + + # everyone_tag_id + assert await tags_repo.access_count(conn, everyone_tag_id, read=True) == 1 + assert await other_repo.access_count(conn, everyone_tag_id, read=True) == 1 + + # now group adds read for all tags + for t in (tag_id, other_tag_id, everyone_tag_id): + await _create_tag_access( + conn, + group_id=group.gid, + tag_id=t, + read=True, + write=False, + delete=False, + ) + + assert await tags_repo.access_count(conn, tag_id, read=True) == 2 + assert await tags_repo.access_count(conn, other_tag_id, read=True) == 1 + assert await tags_repo.access_count(conn, everyone_tag_id, read=True) == 2 + + +async def test_tags_repo_list_and_get( + connection: SAConnection, user: RowProxy, group: RowProxy, other_user: RowProxy +): + conn = connection + tags_repo = TagsRepo(user_id=user.id) + + # (1) no tags + listed_tags = await tags_repo.list(conn) + assert not listed_tags + + # (2) one tag + expected_tags_ids = [ + await _create_tag( + conn, + name="T1", + description=f"tag for {user.id}", + color="blue", + group_id=user.primary_gid, + read=True, + write=False, + delete=False, + ) + ] + + listed_tags = await tags_repo.list(conn) + assert listed_tags + assert [t["id"] for t in listed_tags] == expected_tags_ids + + # (3) another tag via its standard group + expected_tags_ids.append( + await _create_tag( + conn, + name="T2", + description="tag via std group", + color="red", + group_id=group.gid, + read=True, + write=False, + delete=False, + ) + ) + + listed_tags = await tags_repo.list(conn) + assert {t["id"] for t in listed_tags} == set(expected_tags_ids) + + # (4) add another tag from a differnt user + await _create_tag( + conn, + name="T3", + description=f"tag for {other_user.id}", + color="green", + group_id=other_user.primary_gid, + read=True, + write=False, + delete=False, + ) + + # same as before + prev_listed_tags = listed_tags + listed_tags = await tags_repo.list(conn) + assert listed_tags == prev_listed_tags + + # (5) add a global tag + tag_id = await _create_tag( + conn, + name="TG", + description="tag for EVERYBODY", + color="pink", + group_id=1, + read=True, + write=False, + delete=False, + ) + + listed_tags = await tags_repo.list(conn) + assert listed_tags == [ + {"id": 1, "name": "T1", "description": "tag for 1", "color": "blue"}, + {"id": 2, "name": "T2", "description": "tag via std group", "color": "red"}, + {"id": 4, "name": "TG", "description": "tag for EVERYBODY", "color": "pink"}, + ] + + other_repo = TagsRepo(user_id=other_user.id) + assert await other_repo.list(conn) == [ + {"id": 3, "name": "T3", "description": "tag for 2", "color": "green"}, + {"id": 4, "name": "TG", "description": "tag for EVERYBODY", "color": "pink"}, + ] + + # exclusive to user + assert await tags_repo.get(conn, tag_id=2) == { + "id": 2, + "name": "T2", + "description": "tag via std group", + "color": "red", + } + + # exclusive ot other user + with pytest.raises(TagNotFoundError): + assert await tags_repo.get(conn, tag_id=3) + + assert await other_repo.get(conn, tag_id=3) == { + "id": 3, + "name": "T3", + "description": "tag for 2", + "color": "green", + } + + # a common tag + assert await tags_repo.get(conn, tag_id=4) == await other_repo.get(conn, tag_id=4) + + +async def test_tags_repo_update( + connection: SAConnection, user: RowProxy, group: RowProxy, other_user: RowProxy +): + conn = connection + tags_repo = TagsRepo(user_id=user.id) + + # Tags with different access rights + readonly_tid, readwrite_tid, other_tid = [ + await _create_tag( + conn, + name="T1", + description="read only", + color="blue", + group_id=user.primary_gid, + read=True, + write=False, # <--- read only + delete=False, + ), + await _create_tag( + conn, + name="T2", + description="read/write", + color="green", + group_id=user.primary_gid, + read=True, + write=True, # <--- can write + delete=False, + ), + await _create_tag( + conn, + name="T3", + description="read/write but a other user", + color="blue", + group_id=other_user.primary_gid, + read=True, + write=True, # <--- can write but other user + delete=False, + ), + ] + + with pytest.raises(TagOperationNotAllowed): + await tags_repo.update(conn, tag_id=readonly_tid, description="modified") + + assert await tags_repo.update( + conn, tag_id=readwrite_tid, description="modified" + ) == { + "id": readwrite_tid, + "name": "T2", + "description": "modified", + "color": "green", + } + + with pytest.raises(TagOperationNotAllowed): + await tags_repo.update(conn, tag_id=other_tid, description="modified") + + +async def test_tags_repo_delete( + connection: SAConnection, user: RowProxy, group: RowProxy, other_user: RowProxy +): + conn = connection + tags_repo = TagsRepo(user_id=user.id) + + # Tags with different access rights + readonly_tid, delete_tid, other_tid = [ + await _create_tag( + conn, + name="T1", + description="read only", + color="blue", + group_id=user.primary_gid, + read=True, + write=False, # <--- read only + delete=False, + ), + await _create_tag( + conn, + name="T2", + description="read/write", + color="green", + group_id=user.primary_gid, + read=True, + write=True, + delete=True, # <-- can delete + ), + await _create_tag( + conn, + name="T3", + description="read/write but a other user", + color="blue", + group_id=other_user.primary_gid, + read=True, + write=True, + delete=True, # <-- can delete but other user + ), + ] + + # cannot delete + with pytest.raises(TagOperationNotAllowed): + await tags_repo.delete(conn, tag_id=readonly_tid) + + # can delete + await tags_repo.get(conn, tag_id=delete_tid) + await tags_repo.delete(conn, tag_id=delete_tid) + + with pytest.raises(TagNotFoundError): + await tags_repo.get(conn, tag_id=delete_tid) + + # cannot delete + with pytest.raises(TagOperationNotAllowed): + await tags_repo.delete(conn, tag_id=other_tid) + + +async def test_tags_repo_create( + connection: SAConnection, user: RowProxy, group: RowProxy, other_user: RowProxy +): + conn = connection + tags_repo = TagsRepo(user_id=user.id) + + tag_1 = await tags_repo.create( + conn, + name="T1", + description="my first tag", + color="pink", + read=True, + write=True, + delete=True, + ) + assert tag_1 == { + "id": 1, + "name": "T1", + "description": "my first tag", + "color": "pink", + } + + # assigned primary group + assert ( + await conn.scalar( + sa.select([tags_to_groups.c.group_id]).where( + tags_to_groups.c.tag_id == tag_1["id"] + ) + ) + == user.primary_gid + ) diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/rawdata_fakers.py b/packages/pytest-simcore/src/pytest_simcore/helpers/rawdata_fakers.py index f4d15882c9d..f6b41e2cda2 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/rawdata_fakers.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/rawdata_fakers.py @@ -22,7 +22,7 @@ from simcore_postgres_database.models.comp_pipeline import StateType from simcore_postgres_database.models.projects import projects from simcore_postgres_database.models.users import users -from simcore_postgres_database.webserver_models import ProjectType, UserStatus +from simcore_postgres_database.webserver_models import GroupType, UserStatus STATES = [ StateType.NOT_STARTED, @@ -97,7 +97,7 @@ def random_group(**overrides) -> dict[str, Any]: data = dict( name=FAKE.company(), description=FAKE.text(), - type=ProjectType.STANDARD.name, + type=GroupType.STANDARD.name, ) data.update(overrides) return data diff --git a/scripts/docker_clone_volume.bash b/scripts/docker_clone_volume.bash new file mode 100755 index 00000000000..6457468b23c --- /dev/null +++ b/scripts/docker_clone_volume.bash @@ -0,0 +1,45 @@ +#!/bin/bash + +# Taken from https://github.com/gdiepen/docker-convenience-scripts (⭐) +# wget https://raw.githubusercontent.com/gdiepen/docker-convenience-scripts/master/docker_clone_volume.sh + +#Author: Guido Diepen + +#Convenience script that can help me to easily create a clone of a given +#data volume. The script is mainly useful if you are using named volumes + +#First check if the user provided all needed arguments +if [ "$1" = "" ]; then + echo "Please provide a source volume name" + exit +fi + +if [ "$2" = "" ]; then + echo "Please provide a destination volume name" + exit +fi + +#Check if the source volume name does exist +docker volume inspect "$1" >/dev/null 2>&1 +if [ "$?" != "0" ]; then + echo "The source volume \"$1\" does not exist" + exit +fi + +#Now check if the destinatin volume name does not yet exist +docker volume inspect "$2" >/dev/null 2>&1 + +if [ "$?" = "0" ]; then + echo "The destination volume \"$2\" already exists" + exit +fi + +echo "Creating destination volume \"$2\"..." +docker volume create --name "$2" +echo "Copying data from source volume \"$1\" to destination volume \"$2\"..." +docker run --rm \ + --interactive \ + --tty \ + --volume "$1":/from \ + --volume "$2":/to \ + alpine ash -c "cd /from ; cp -av . /to" diff --git a/services/web/server/src/simcore_service_webserver/tags_handlers.py b/services/web/server/src/simcore_service_webserver/tags_handlers.py index 63aed49c476..bc2f18d3f68 100644 --- a/services/web/server/src/simcore_service_webserver/tags_handlers.py +++ b/services/web/server/src/simcore_service_webserver/tags_handlers.py @@ -1,84 +1,89 @@ -import sqlalchemy as sa +import functools + from aiohttp import web from servicelib.aiohttp.application_keys import APP_DB_ENGINE_KEY -from sqlalchemy import and_ +from servicelib.aiohttp.typing_extension import Handler +from servicelib.mimetype_constants import MIMETYPE_APPLICATION_JSON +from simcore_postgres_database.utils_tags import ( + TagNotFoundError, + TagOperationNotAllowed, + TagsRepo, +) -from .db_models import tags from .login.decorators import RQT_USERID_KEY, login_required from .security_api import check_permission +def _handle_tags_exceptions(handler: Handler): + @functools.wraps(handler) + async def wrapper(request: web.Request) -> web.Response: + try: + return await handler(request) + + except (KeyError, TypeError, ValueError) as exc: + # NOTE: will be replaced by more robust pydantic-based validation + # Bad match_info[*] -> KeyError + # Bad int(param) -> ValueError + # Bad update(**tag_update) -> TypeError + raise web.HTTPBadRequest(reason=f"{exc}") from exc + + except TagNotFoundError as exc: + raise web.HTTPNotFound(reason=f"{exc}") from exc + + except TagOperationNotAllowed as exc: + raise web.HTTPUnauthorized(reason=f"{exc}") from exc + + return wrapper + + @login_required +@_handle_tags_exceptions async def list_tags(request: web.Request): await check_permission(request, "tag.crud.*") uid, engine = request[RQT_USERID_KEY], request.app[APP_DB_ENGINE_KEY] + + repo = TagsRepo(user_id=uid) async with engine.acquire() as conn: - # pylint: disable=not-an-iterable - columns = [col for col in tags.columns if col.key != "user_id"] - query = sa.select(columns).where(tags.c.user_id == uid) - result = [] - async for row_proxy in conn.execute(query): - row_dict = dict(row_proxy.items()) - result.append(row_dict) - return result + tags = await repo.list(conn) + return tags @login_required +@_handle_tags_exceptions async def update_tag(request: web.Request): await check_permission(request, "tag.crud.*") uid, engine = request[RQT_USERID_KEY], request.app[APP_DB_ENGINE_KEY] - tag_id = request.match_info.get("tag_id") + tag_id = int(request.match_info["tag_id"]) tag_data = await request.json() + + repo = TagsRepo(user_id=uid) async with engine.acquire() as conn: - # pylint: disable=no-value-for-parameter - query = ( - tags.update() - .values( - name=tag_data["name"], - description=tag_data["description"], - color=tag_data["color"], - ) - .where(and_(tags.c.id == tag_id, tags.c.user_id == uid)) - .returning(tags.c.id, tags.c.name, tags.c.description, tags.c.color) - ) - async with conn.execute(query) as result: - if result.rowcount == 1: - row_proxy = await result.first() - return dict(row_proxy.items()) - raise web.HTTPInternalServerError() + tag = await repo.update(conn, tag_id, **tag_data) + return tag @login_required +@_handle_tags_exceptions async def create_tag(request: web.Request): await check_permission(request, "tag.crud.*") uid, engine = request[RQT_USERID_KEY], request.app[APP_DB_ENGINE_KEY] tag_data = await request.json() + + repo = TagsRepo(user_id=uid) async with engine.acquire() as conn: - # pylint: disable=no-value-for-parameter - query = ( - tags.insert() - .values( - user_id=uid, - name=tag_data["name"], - description=tag_data["description"], - color=tag_data["color"], - ) - .returning(tags.c.id, tags.c.name, tags.c.description, tags.c.color) - ) - async with conn.execute(query) as result: - if result.rowcount == 1: - row_proxy = await result.first() - return dict(row_proxy.items()) - raise web.HTTPInternalServerError() + tag = await repo.create(conn, read=True, write=True, delete=True, **tag_data) + return tag @login_required +@_handle_tags_exceptions async def delete_tag(request: web.Request): await check_permission(request, "tag.crud.*") uid, engine = request[RQT_USERID_KEY], request.app[APP_DB_ENGINE_KEY] - tag_id = request.match_info.get("tag_id") + tag_id = int(request.match_info["tag_id"]) + + repo = TagsRepo(user_id=uid) async with engine.acquire() as conn: - # pylint: disable=no-value-for-parameter - query = tags.delete().where(and_(tags.c.id == tag_id, tags.c.user_id == uid)) - async with conn.execute(query): - raise web.HTTPNoContent(content_type="application/json") + await repo.delete(conn, tag_id=tag_id) + + raise web.HTTPNoContent(content_type=MIMETYPE_APPLICATION_JSON) diff --git a/services/web/server/tests/data/test_tags_data.json b/services/web/server/tests/data/test_tags_data.json deleted file mode 100644 index 483b9b77294..00000000000 --- a/services/web/server/tests/data/test_tags_data.json +++ /dev/null @@ -1,14 +0,0 @@ -{ - "added_tags": [ - { - "name": "tag1", - "description": "description1", - "color": "#f00" - }, - { - "name": "tag2", - "description": "description2", - "color": "#00f" - } - ] -} \ No newline at end of file diff --git a/services/web/server/tests/unit/conftest.py b/services/web/server/tests/unit/conftest.py index c86370008a2..24361afdc04 100644 --- a/services/web/server/tests/unit/conftest.py +++ b/services/web/server/tests/unit/conftest.py @@ -12,7 +12,7 @@ import logging import sys from pathlib import Path -from typing import Any, Callable, Dict, Iterable +from typing import Any, Callable, Iterable import pytest import yaml @@ -54,7 +54,7 @@ def default_app_config_unit_file(tests_data_dir: Path) -> Path: def default_app_cfg(default_app_config_unit_file: Path) -> ConfigDict: # NOTE: ONLY used at the session scopes # TODO: create instead a loader function and return a Callable - config: Dict = yaml.safe_load(default_app_config_unit_file.read_text()) + config: dict = yaml.safe_load(default_app_config_unit_file.read_text()) return config @@ -76,17 +76,11 @@ def project_schema_file(api_version_prefix) -> Path: @pytest.fixture -def activity_data(fake_data_dir: Path) -> Iterable[Dict[str, Any]]: +def activity_data(fake_data_dir: Path) -> Iterable[dict[str, Any]]: with (fake_data_dir / "test_activity_data.json").open() as fp: yield json.load(fp) -@pytest.fixture -def test_tags_data(fake_data_dir: Path) -> Iterable[Dict[str, Any]]: - with (fake_data_dir / "test_tags_data.json").open() as fp: - yield json.load(fp).get("added_tags") - - @pytest.fixture def mock_orphaned_services(mocker): remove_orphaned_services = mocker.patch( diff --git a/services/web/server/tests/unit/with_dbs/03/tags/test_tags.py b/services/web/server/tests/unit/with_dbs/03/tags/test_tags.py index 0d852a960b2..94ad13d9e3d 100644 --- a/services/web/server/tests/unit/with_dbs/03/tags/test_tags.py +++ b/services/web/server/tests/unit/with_dbs/03/tags/test_tags.py @@ -3,8 +3,12 @@ # pylint:disable=redefined-outer-name +from pathlib import Path +from typing import Any, Callable + import pytest from aiohttp import web +from aiohttp.test_utils import TestClient from models_library.projects_state import ( ProjectLocked, ProjectRunningState, @@ -14,29 +18,47 @@ ) from models_library.utils.fastapi_encoders import jsonable_encoder from pytest_simcore.helpers.utils_assert import assert_status +from pytest_simcore.helpers.utils_login import UserInfoDict from pytest_simcore.helpers.utils_projects import assert_get_same_project from simcore_service_webserver.db_models import UserRole +@pytest.fixture +def fake_tags(fake_data_dir: Path) -> list[dict[str, Any]]: + return [ + {"name": "tag1", "description": "description1", "color": "#f00"}, + {"name": "tag2", "description": "description2", "color": "#00f"}, + ] + + @pytest.mark.parametrize("user_role,expected", [(UserRole.USER, web.HTTPOk)]) async def test_tags_to_studies( - client, logged_user, user_project, expected, test_tags_data, catalog_subsystem_mock + client: TestClient, + logged_user: UserInfoDict, + user_project, + expected: type[web.HTTPException], + fake_tags: dict[str, Any], + catalog_subsystem_mock: Callable, ): catalog_subsystem_mock([user_project]) + assert client.app + # Add test tags - tags = test_tags_data added_tags = [] - for tag in tags: + + for tag in fake_tags: url = client.app.router["create_tag"].url_for() - resp = await client.post(url, json=tag) + resp = await client.post(f"{url}", json=tag) added_tag, _ = await assert_status(resp, expected) added_tags.append(added_tag) + # Add tag to study url = client.app.router["add_tag"].url_for( study_uuid=user_project.get("uuid"), tag_id=str(added_tag.get("id")) ) - resp = await client.put(url) + resp = await client.put(f"{url}") data, _ = await assert_status(resp, expected) + # Tag is included in response assert added_tag["id"] in data["tags"] @@ -53,8 +75,9 @@ async def test_tags_to_studies( # Delete tag0 url = client.app.router["delete_tag"].url_for(tag_id=str(added_tags[0].get("id"))) - resp = await client.delete(url) + resp = await client.delete(f"{url}") await assert_status(resp, web.HTTPNoContent) + # Get project and check that tag is no longer there user_project["tags"].remove(added_tags[0]["id"]) data = await assert_get_same_project(client, user_project, expected) @@ -64,7 +87,7 @@ async def test_tags_to_studies( url = client.app.router["remove_tag"].url_for( study_uuid=user_project.get("uuid"), tag_id=str(added_tags[1].get("id")) ) - resp = await client.delete(url) + resp = await client.delete(f"{url}") await assert_status(resp, expected) # Get project and check that tag is no longer there user_project["tags"].remove(added_tags[1]["id"]) @@ -73,5 +96,5 @@ async def test_tags_to_studies( # Delete tag1 url = client.app.router["delete_tag"].url_for(tag_id=str(added_tags[1].get("id"))) - resp = await client.delete(url) + resp = await client.delete(f"{url}") await assert_status(resp, web.HTTPNoContent)