diff --git a/packages/postgres-database/src/simcore_postgres_database/errors.py b/packages/postgres-database/src/simcore_postgres_database/errors.py index 64f29cdebf4..9a3b996c153 100644 --- a/packages/postgres-database/src/simcore_postgres_database/errors.py +++ b/packages/postgres-database/src/simcore_postgres_database/errors.py @@ -28,7 +28,12 @@ OperationalError, ProgrammingError, ) -from psycopg2.errors import ForeignKeyViolation, NotNullViolation, UniqueViolation +from psycopg2.errors import ( + CheckViolation, + ForeignKeyViolation, + NotNullViolation, + UniqueViolation, +) assert issubclass(UniqueViolation, IntegrityError) # nosec @@ -44,6 +49,7 @@ __all__: tuple[str, ...] = ( + "CheckViolation", "DatabaseError", "DataError", "DBAPIError", diff --git a/packages/postgres-database/src/simcore_postgres_database/migration/versions/9014ae5fd6e5_checkconstraint_filetype.py b/packages/postgres-database/src/simcore_postgres_database/migration/versions/9014ae5fd6e5_checkconstraint_filetype.py new file mode 100644 index 00000000000..cfb030a157a --- /dev/null +++ b/packages/postgres-database/src/simcore_postgres_database/migration/versions/9014ae5fd6e5_checkconstraint_filetype.py @@ -0,0 +1,29 @@ +"""Checkconstraint filetype + +Revision ID: 9014ae5fd6e5 +Revises: 4f9c8738178b +Create Date: 2023-03-29 13:52:47.611065+00:00 + +""" +from alembic import op + +# revision identifiers, used by Alembic. +revision = "9014ae5fd6e5" +down_revision = "4f9c8738178b" +branch_labels = None +depends_on = None + + +def upgrade(): + op.execute("UPDATE services_consume_filetypes SET filetype = UPPER(filetype)") + op.create_check_constraint( + "ck_filetype_is_upper", + "services_consume_filetypes", + "filetype = upper(filetype)", + ) + + +def downgrade(): + op.drop_constraint( + "ck_filetype_is_upper", "services_consume_filetypes", type_="check" + ) diff --git a/packages/postgres-database/src/simcore_postgres_database/models/services_consume_filetypes.py b/packages/postgres-database/src/simcore_postgres_database/models/services_consume_filetypes.py index db242ed3de8..efbae9b9d32 100644 --- a/packages/postgres-database/src/simcore_postgres_database/models/services_consume_filetypes.py +++ b/packages/postgres-database/src/simcore_postgres_database/models/services_consume_filetypes.py @@ -20,7 +20,6 @@ services_consume_filetypes = sa.Table( "services_consume_filetypes", metadata, - # TODO: add regex to key and version? sa.Column( "service_key", sa.String, @@ -48,8 +47,12 @@ sa.Column( "filetype", sa.String, + sa.CheckConstraint( + "filetype = upper(filetype)", + name="ck_filetype_is_upper", + ), nullable=False, - doc="A filetype supported by this service, e.g. CSV, Excel, ect." + doc="An extension supported by this service, e.g. CSV, VTK, etc." "The filetype identifiers are not well defined, so we avoided using enums" "Temptative list in https://en.wikipedia.org/wiki/List_of_file_formats", ), diff --git a/packages/postgres-database/tests/test_services__meta_data.py b/packages/postgres-database/tests/test_services__meta_data.py index c456ee3dfc8..a4fa923fd97 100644 --- a/packages/postgres-database/tests/test_services__meta_data.py +++ b/packages/postgres-database/tests/test_services__meta_data.py @@ -116,7 +116,7 @@ def services_fixture(faker: Faker, pg_sa_engine: sa.engine.Engine) -> ServicesFi service_version=version, service_display_name=service_name, service_input_port=f"input_{i}", - filetype=filetype, + filetype=filetype.upper(), is_guest_allowed=is_public, ) @@ -134,7 +134,6 @@ def services_fixture(faker: Faker, pg_sa_engine: sa.engine.Engine) -> ServicesFi def test_trial_queries_for_service_metadata( services_fixture: ServicesFixture, pg_sa_engine: sa.engine.Engine ): - # check if service exists and whether is public or not with pg_sa_engine.connect() as conn: query = sa.select( diff --git a/packages/postgres-database/tests/test_services_consume_filetypes.py b/packages/postgres-database/tests/test_services_consume_filetypes.py index bca20e799fc..f1530b764f6 100644 --- a/packages/postgres-database/tests/test_services_consume_filetypes.py +++ b/packages/postgres-database/tests/test_services_consume_filetypes.py @@ -4,15 +4,18 @@ # pylint: disable=no-value-for-parameter +from typing import Callable + import pytest import sqlalchemy as sa -from aiopg.sa.engine import Engine +from aiopg.sa.connection import SAConnection from aiopg.sa.exc import ResourceClosedError from aiopg.sa.result import ResultProxy, RowProxy from pytest_simcore.helpers.utils_services import ( FAKE_FILE_CONSUMER_SERVICES, list_supported_filetypes, ) +from simcore_postgres_database.errors import CheckViolation from simcore_postgres_database.models.services import services_meta_data from simcore_postgres_database.models.services_consume_filetypes import ( services_consume_filetypes, @@ -20,12 +23,12 @@ @pytest.fixture -def make_table(): - async def _make(conn): +def make_table() -> Callable: + async def _make(connection: SAConnection): for service in FAKE_FILE_CONSUMER_SERVICES: - await conn.execute( + await connection.execute( services_meta_data.insert().values( key=service["key"], version=service["version"], @@ -37,7 +40,7 @@ async def _make(conn): for n, consumable in enumerate(service["consumes"]): filetype, port, *_ = consumable.split(":") + ["input_1"] - result: ResultProxy = await conn.execute( + result: ResultProxy = await connection.execute( services_consume_filetypes.insert().values( service_key=service["key"], service_version=service["version"], @@ -57,13 +60,28 @@ async def _make(conn): @pytest.fixture -async def conn(pg_engine: Engine, make_table): - async with pg_engine.acquire() as conn: - await make_table(conn) - yield conn +async def connection(connection: SAConnection, make_table: Callable): + # EXTENDS + await make_table(connection) + yield connection + + +async def test_check_constraint(connection: SAConnection): + stmt_create_services_consume_filetypes = sa.text( + 'INSERT INTO "services_consume_filetypes" ("service_key", "service_version", "service_display_name", "service_input_port", "filetype", "preference_order", "is_guest_allowed") VALUES' + "('simcore/services/dynamic/bio-formats-web', '1.0.20', 'bio-formats', 'input_1', 'PNG', 0, '1')," + "('simcore/services/dynamic/raw-graphs', '2.11.20', 'RAWGraphs', 'input_1', 'lowerUpper', 0, '1');" + ) + + with pytest.raises(CheckViolation) as error_info: + await connection.execute(stmt_create_services_consume_filetypes) + + error = error_info.value + assert error.pgcode == "23514" + assert "ck_filetype_is_upper" in error.pgerror -async def test_get_compatible_services(conn): +async def test_get_compatible_services(connection: SAConnection): # given a filetype, get sorted services # test sorting of services given a filetype # https://docs.sqlalchemy.org/en/13/core/tutorial.html#ordering-or-grouping-by-a-label @@ -72,7 +90,7 @@ async def test_get_compatible_services(conn): .where(services_consume_filetypes.c.filetype == "DCM") .order_by("preference_order") ) - result: ResultProxy = await conn.execute(stmt) + result: ResultProxy = await connection.execute(stmt) assert result.returns_rows @@ -86,7 +104,7 @@ async def test_get_compatible_services(conn): assert rows[-1].service_version == "2.0.0" -async def test_get_supported_filetypes(conn): +async def test_get_supported_filetypes(connection: SAConnection): # given a service, get supported filetypes stmt = ( @@ -103,12 +121,12 @@ async def test_get_supported_filetypes(conn): .distinct() ) - result: ResultProxy = await conn.execute(stmt) + result: ResultProxy = await connection.execute(stmt) rows: list[RowProxy] = await result.fetchall() - assert [v for row in rows for v in row.values()] == ["DCM", "S4LCacheData"] + assert [v for row in rows for v in row.values()] == ["DCM", "S4LCACHEDATA"] -async def test_list_supported_filetypes(conn): +async def test_list_supported_filetypes(connection: SAConnection): # given a service, get supported filetypes stmt = ( @@ -121,32 +139,15 @@ async def test_list_supported_filetypes(conn): .distinct() ) - result: ResultProxy = await conn.execute(stmt) + result: ResultProxy = await connection.execute(stmt) rows: list[RowProxy] = await result.fetchall() assert [v for row in rows for v in row.values()] == list_supported_filetypes() -@pytest.mark.skip(reason="Under DEV") -async def test_list_default_compatible_services(): - stmt = ( - sa.select( - [ - services_consume_filetypes, - ] - ) - .group_by(services_consume_filetypes.c.filetype) - .order_by( - services_consume_filetypes.c.key, services_consume_filetypes.c.version - ) - .distinct() - ) - raise NotImplementedError() - - -async def test_contraints(conn): +async def test_contraints(connection: SAConnection): # test foreign key contraints with service metadata table - await conn.execute( + await connection.execute( services_meta_data.delete().where( services_meta_data.c.key == "simcore/services/dynamic/sim4life" ) @@ -163,17 +164,6 @@ async def test_contraints(conn): .where(services_consume_filetypes.c.filetype == "DCM") .scalar_subquery() ) - result: ResultProxy = await conn.execute(stmt) + result: ResultProxy = await connection.execute(stmt) num_services = await result.scalar() assert num_services == 0 - - -@pytest.mark.skip(reason="Under DEV") -async def test_get_compatible_services_available_to_everyone(conn): - # TODO: resolve when this logic is moved to catalog - - # given a filetype, get sorted services - # test sorting of services given a filetype - # https://docs.sqlalchemy.org/en/13/core/tutorial.html#ordering-or-grouping-by-a-label - - raise NotImplementedError() diff --git a/packages/pytest-simcore/src/pytest_simcore/helpers/utils_services.py b/packages/pytest-simcore/src/pytest_simcore/helpers/utils_services.py index 6da74838b84..32d91d46c78 100644 --- a/packages/pytest-simcore/src/pytest_simcore/helpers/utils_services.py +++ b/packages/pytest-simcore/src/pytest_simcore/helpers/utils_services.py @@ -27,7 +27,7 @@ "key": "simcore/services/dynamic/sim4life", "version": "2.0.0", "display_name": "Sim4Life", - "consumes": ["DCM", "S4LCacheData"], + "consumes": ["DCM", "S4LCACHEDATA"], }, # another service with multiple format support (preferred for CSV) { diff --git a/services/web/server/src/simcore_service_webserver/studies_dispatcher/handlers_redirects.py b/services/web/server/src/simcore_service_webserver/studies_dispatcher/handlers_redirects.py index ba49d921757..9e6817baa84 100644 --- a/services/web/server/src/simcore_service_webserver/studies_dispatcher/handlers_redirects.py +++ b/services/web/server/src/simcore_service_webserver/studies_dispatcher/handlers_redirects.py @@ -42,6 +42,15 @@ def from_viewer(viewer: ViewerInfo) -> "ViewerQueryParams": viewer_version=viewer.version, ) + @validator("file_type") + @classmethod + def ensure_extension_upper_and_dotless(cls, v): + # NOTE: see filetype constraint-check + if v and isinstance(v, str): + w = urllib.parse.unquote(v) + return w.upper().lstrip(".") + return v + class RedirectionQueryParams(ViewerQueryParams): file_name: str = "unknown" @@ -78,6 +87,20 @@ def file_params_required(cls, values): raise ValueError("One or more file parameters missing") + class Config: + schema_extra = { + "examples": [ + { + "viewer_key": "simcore/services/comp/foo", + "viewer_version": "1.2.3", + "file_type": "lowerUPPER", + "file_name": "filename", + "file_size": "12", + "download_link": "https://download.io/file123", + } + ] + } + def compose_dispatcher_prefix_url(request: web.Request, viewer: ViewerInfo) -> HttpUrl: """This is denoted PREFIX URL because it needs to append extra query diff --git a/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py b/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py index 45d78fbd9e4..a1a0327c2b2 100644 --- a/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py +++ b/services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py @@ -4,19 +4,22 @@ import re import urllib.parse -from typing import AsyncIterator +from typing import Any, AsyncIterator import pytest +import simcore_service_webserver.studies_dispatcher.handlers_redirects import sqlalchemy as sa from aiohttp import ClientResponse, ClientSession, web from aiohttp.test_utils import TestClient, TestServer from aioresponses import aioresponses from models_library.projects_state import ProjectLocked, ProjectStatus -from pydantic import parse_obj_as +from pydantic import BaseModel, parse_obj_as from pytest import FixtureRequest, MonkeyPatch from pytest_mock import MockerFixture from pytest_simcore.helpers.utils_assert import assert_status from pytest_simcore.helpers.utils_login import UserRole +from pytest_simcore.pydantic_models import iter_model_examples_in_module +from servicelib.json_serialization import json_dumps from settings_library.redis import RedisSettings from simcore_service_webserver import catalog from simcore_service_webserver.studies_dispatcher._core import ViewerInfo @@ -162,7 +165,6 @@ def _get_base_url(client: TestClient) -> str: async def test_api_get_viewer_for_file(client: TestClient): - resp = await client.get("/v0/viewers/default?file_type=JPEG") data, _ = await assert_status(resp, web.HTTPOk) @@ -184,7 +186,6 @@ async def test_api_get_viewer_for_unsupported_type(client: TestClient): async def test_api_list_supported_filetypes(client: TestClient): - resp = await client.get("/v0/viewers/default") data, _ = await assert_status(resp, web.HTTPOk) @@ -233,7 +234,20 @@ async def test_api_list_supported_filetypes(client: TestClient): ] -@pytest.mark.testit +@pytest.mark.parametrize( + "model_cls, example_name, example_data", + iter_model_examples_in_module( + simcore_service_webserver.studies_dispatcher.handlers_redirects + ), +) +def test_model_examples( + model_cls: type[BaseModel], example_name: int, example_data: Any +): + print(example_name, ":", json_dumps(example_data)) + model = model_cls.parse_obj(example_data) + assert model + + async def test_api_get_services(client: TestClient): assert client.app @@ -253,7 +267,6 @@ async def test_api_get_services(client: TestClient): @pytest.fixture async def catalog_subsystem_mock(monkeypatch: MonkeyPatch) -> None: - services_in_project = [ {"key": "simcore/services/frontend/file-picker", "version": "1.0.0"} ] + [{"key": s.key, "version": s.version} for s in FAKE_VIEWS_LIST]