Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

🐛 filetypes in webserver API are case insensitive #4041

Merged
merged 5 commits into from
Mar 30, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -44,6 +49,7 @@


__all__: tuple[str, ...] = (
"CheckViolation",
"DatabaseError",
"DataError",
"DBAPIError",
Expand Down
Original file line number Diff line number Diff line change
@@ -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"
)
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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",
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)

Expand All @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,28 +4,31 @@
# 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,
)


@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"],
Expand All @@ -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"],
Expand All @@ -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
Expand All @@ -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

Expand All @@ -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 = (
Expand All @@ -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 = (
Expand All @@ -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"
)
Expand All @@ -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()
Original file line number Diff line number Diff line change
Expand Up @@ -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)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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):
pcrespov marked this conversation as resolved.
Show resolved Hide resolved
# 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"
Expand Down Expand Up @@ -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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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)

Expand All @@ -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)

Expand Down Expand Up @@ -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

Expand All @@ -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]
Expand Down