Skip to content

Commit 989d97f

Browse files
authored
🐛 filetypes in webserver API are case insensitive (ITISFoundation#4041)
1 parent 3424370 commit 989d97f

File tree

8 files changed

+122
-59
lines changed

8 files changed

+122
-59
lines changed

packages/postgres-database/src/simcore_postgres_database/errors.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -28,7 +28,12 @@
2828
OperationalError,
2929
ProgrammingError,
3030
)
31-
from psycopg2.errors import ForeignKeyViolation, NotNullViolation, UniqueViolation
31+
from psycopg2.errors import (
32+
CheckViolation,
33+
ForeignKeyViolation,
34+
NotNullViolation,
35+
UniqueViolation,
36+
)
3237

3338
assert issubclass(UniqueViolation, IntegrityError) # nosec
3439

@@ -44,6 +49,7 @@
4449

4550

4651
__all__: tuple[str, ...] = (
52+
"CheckViolation",
4753
"DatabaseError",
4854
"DataError",
4955
"DBAPIError",
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,29 @@
1+
"""Checkconstraint filetype
2+
3+
Revision ID: 9014ae5fd6e5
4+
Revises: 4f9c8738178b
5+
Create Date: 2023-03-29 13:52:47.611065+00:00
6+
7+
"""
8+
from alembic import op
9+
10+
# revision identifiers, used by Alembic.
11+
revision = "9014ae5fd6e5"
12+
down_revision = "4f9c8738178b"
13+
branch_labels = None
14+
depends_on = None
15+
16+
17+
def upgrade():
18+
op.execute("UPDATE services_consume_filetypes SET filetype = UPPER(filetype)")
19+
op.create_check_constraint(
20+
"ck_filetype_is_upper",
21+
"services_consume_filetypes",
22+
"filetype = upper(filetype)",
23+
)
24+
25+
26+
def downgrade():
27+
op.drop_constraint(
28+
"ck_filetype_is_upper", "services_consume_filetypes", type_="check"
29+
)

packages/postgres-database/src/simcore_postgres_database/models/services_consume_filetypes.py

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@
2020
services_consume_filetypes = sa.Table(
2121
"services_consume_filetypes",
2222
metadata,
23-
# TODO: add regex to key and version?
2423
sa.Column(
2524
"service_key",
2625
sa.String,
@@ -48,8 +47,12 @@
4847
sa.Column(
4948
"filetype",
5049
sa.String,
50+
sa.CheckConstraint(
51+
"filetype = upper(filetype)",
52+
name="ck_filetype_is_upper",
53+
),
5154
nullable=False,
52-
doc="A filetype supported by this service, e.g. CSV, Excel, ect."
55+
doc="An extension supported by this service, e.g. CSV, VTK, etc."
5356
"The filetype identifiers are not well defined, so we avoided using enums"
5457
"Temptative list in https://en.wikipedia.org/wiki/List_of_file_formats",
5558
),

packages/postgres-database/tests/test_services__meta_data.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -116,7 +116,7 @@ def services_fixture(faker: Faker, pg_sa_engine: sa.engine.Engine) -> ServicesFi
116116
service_version=version,
117117
service_display_name=service_name,
118118
service_input_port=f"input_{i}",
119-
filetype=filetype,
119+
filetype=filetype.upper(),
120120
is_guest_allowed=is_public,
121121
)
122122

@@ -134,7 +134,6 @@ def services_fixture(faker: Faker, pg_sa_engine: sa.engine.Engine) -> ServicesFi
134134
def test_trial_queries_for_service_metadata(
135135
services_fixture: ServicesFixture, pg_sa_engine: sa.engine.Engine
136136
):
137-
138137
# check if service exists and whether is public or not
139138
with pg_sa_engine.connect() as conn:
140139
query = sa.select(

packages/postgres-database/tests/test_services_consume_filetypes.py

Lines changed: 37 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,31 @@
44
# pylint: disable=no-value-for-parameter
55

66

7+
from typing import Callable
8+
79
import pytest
810
import sqlalchemy as sa
9-
from aiopg.sa.engine import Engine
11+
from aiopg.sa.connection import SAConnection
1012
from aiopg.sa.exc import ResourceClosedError
1113
from aiopg.sa.result import ResultProxy, RowProxy
1214
from pytest_simcore.helpers.utils_services import (
1315
FAKE_FILE_CONSUMER_SERVICES,
1416
list_supported_filetypes,
1517
)
18+
from simcore_postgres_database.errors import CheckViolation
1619
from simcore_postgres_database.models.services import services_meta_data
1720
from simcore_postgres_database.models.services_consume_filetypes import (
1821
services_consume_filetypes,
1922
)
2023

2124

2225
@pytest.fixture
23-
def make_table():
24-
async def _make(conn):
26+
def make_table() -> Callable:
27+
async def _make(connection: SAConnection):
2528

2629
for service in FAKE_FILE_CONSUMER_SERVICES:
2730

28-
await conn.execute(
31+
await connection.execute(
2932
services_meta_data.insert().values(
3033
key=service["key"],
3134
version=service["version"],
@@ -37,7 +40,7 @@ async def _make(conn):
3740
for n, consumable in enumerate(service["consumes"]):
3841
filetype, port, *_ = consumable.split(":") + ["input_1"]
3942

40-
result: ResultProxy = await conn.execute(
43+
result: ResultProxy = await connection.execute(
4144
services_consume_filetypes.insert().values(
4245
service_key=service["key"],
4346
service_version=service["version"],
@@ -57,13 +60,28 @@ async def _make(conn):
5760

5861

5962
@pytest.fixture
60-
async def conn(pg_engine: Engine, make_table):
61-
async with pg_engine.acquire() as conn:
62-
await make_table(conn)
63-
yield conn
63+
async def connection(connection: SAConnection, make_table: Callable):
64+
# EXTENDS
65+
await make_table(connection)
66+
yield connection
67+
68+
69+
async def test_check_constraint(connection: SAConnection):
70+
stmt_create_services_consume_filetypes = sa.text(
71+
'INSERT INTO "services_consume_filetypes" ("service_key", "service_version", "service_display_name", "service_input_port", "filetype", "preference_order", "is_guest_allowed") VALUES'
72+
"('simcore/services/dynamic/bio-formats-web', '1.0.20', 'bio-formats', 'input_1', 'PNG', 0, '1'),"
73+
"('simcore/services/dynamic/raw-graphs', '2.11.20', 'RAWGraphs', 'input_1', 'lowerUpper', 0, '1');"
74+
)
75+
76+
with pytest.raises(CheckViolation) as error_info:
77+
await connection.execute(stmt_create_services_consume_filetypes)
78+
79+
error = error_info.value
80+
assert error.pgcode == "23514"
81+
assert "ck_filetype_is_upper" in error.pgerror
6482

6583

66-
async def test_get_compatible_services(conn):
84+
async def test_get_compatible_services(connection: SAConnection):
6785
# given a filetype, get sorted services
6886
# test sorting of services given a filetype
6987
# 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):
7290
.where(services_consume_filetypes.c.filetype == "DCM")
7391
.order_by("preference_order")
7492
)
75-
result: ResultProxy = await conn.execute(stmt)
93+
result: ResultProxy = await connection.execute(stmt)
7694

7795
assert result.returns_rows
7896

@@ -86,7 +104,7 @@ async def test_get_compatible_services(conn):
86104
assert rows[-1].service_version == "2.0.0"
87105

88106

89-
async def test_get_supported_filetypes(conn):
107+
async def test_get_supported_filetypes(connection: SAConnection):
90108
# given a service, get supported filetypes
91109

92110
stmt = (
@@ -103,12 +121,12 @@ async def test_get_supported_filetypes(conn):
103121
.distinct()
104122
)
105123

106-
result: ResultProxy = await conn.execute(stmt)
124+
result: ResultProxy = await connection.execute(stmt)
107125
rows: list[RowProxy] = await result.fetchall()
108-
assert [v for row in rows for v in row.values()] == ["DCM", "S4LCacheData"]
126+
assert [v for row in rows for v in row.values()] == ["DCM", "S4LCACHEDATA"]
109127

110128

111-
async def test_list_supported_filetypes(conn):
129+
async def test_list_supported_filetypes(connection: SAConnection):
112130
# given a service, get supported filetypes
113131

114132
stmt = (
@@ -121,32 +139,15 @@ async def test_list_supported_filetypes(conn):
121139
.distinct()
122140
)
123141

124-
result: ResultProxy = await conn.execute(stmt)
142+
result: ResultProxy = await connection.execute(stmt)
125143
rows: list[RowProxy] = await result.fetchall()
126144
assert [v for row in rows for v in row.values()] == list_supported_filetypes()
127145

128146

129-
@pytest.mark.skip(reason="Under DEV")
130-
async def test_list_default_compatible_services():
131-
stmt = (
132-
sa.select(
133-
[
134-
services_consume_filetypes,
135-
]
136-
)
137-
.group_by(services_consume_filetypes.c.filetype)
138-
.order_by(
139-
services_consume_filetypes.c.key, services_consume_filetypes.c.version
140-
)
141-
.distinct()
142-
)
143-
raise NotImplementedError()
144-
145-
146-
async def test_contraints(conn):
147+
async def test_contraints(connection: SAConnection):
147148
# test foreign key contraints with service metadata table
148149

149-
await conn.execute(
150+
await connection.execute(
150151
services_meta_data.delete().where(
151152
services_meta_data.c.key == "simcore/services/dynamic/sim4life"
152153
)
@@ -163,17 +164,6 @@ async def test_contraints(conn):
163164
.where(services_consume_filetypes.c.filetype == "DCM")
164165
.scalar_subquery()
165166
)
166-
result: ResultProxy = await conn.execute(stmt)
167+
result: ResultProxy = await connection.execute(stmt)
167168
num_services = await result.scalar()
168169
assert num_services == 0
169-
170-
171-
@pytest.mark.skip(reason="Under DEV")
172-
async def test_get_compatible_services_available_to_everyone(conn):
173-
# TODO: resolve when this logic is moved to catalog
174-
175-
# given a filetype, get sorted services
176-
# test sorting of services given a filetype
177-
# https://docs.sqlalchemy.org/en/13/core/tutorial.html#ordering-or-grouping-by-a-label
178-
179-
raise NotImplementedError()

packages/pytest-simcore/src/pytest_simcore/helpers/utils_services.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,7 @@
2727
"key": "simcore/services/dynamic/sim4life",
2828
"version": "2.0.0",
2929
"display_name": "Sim4Life",
30-
"consumes": ["DCM", "S4LCacheData"],
30+
"consumes": ["DCM", "S4LCACHEDATA"],
3131
},
3232
# another service with multiple format support (preferred for CSV)
3333
{

services/web/server/src/simcore_service_webserver/studies_dispatcher/handlers_redirects.py

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,15 @@ def from_viewer(viewer: ViewerInfo) -> "ViewerQueryParams":
4242
viewer_version=viewer.version,
4343
)
4444

45+
@validator("file_type")
46+
@classmethod
47+
def ensure_extension_upper_and_dotless(cls, v):
48+
# NOTE: see filetype constraint-check
49+
if v and isinstance(v, str):
50+
w = urllib.parse.unquote(v)
51+
return w.upper().lstrip(".")
52+
return v
53+
4554

4655
class RedirectionQueryParams(ViewerQueryParams):
4756
file_name: str = "unknown"
@@ -78,6 +87,20 @@ def file_params_required(cls, values):
7887

7988
raise ValueError("One or more file parameters missing")
8089

90+
class Config:
91+
schema_extra = {
92+
"examples": [
93+
{
94+
"viewer_key": "simcore/services/comp/foo",
95+
"viewer_version": "1.2.3",
96+
"file_type": "lowerUPPER",
97+
"file_name": "filename",
98+
"file_size": "12",
99+
"download_link": "https://download.io/file123",
100+
}
101+
]
102+
}
103+
81104

82105
def compose_dispatcher_prefix_url(request: web.Request, viewer: ViewerInfo) -> HttpUrl:
83106
"""This is denoted PREFIX URL because it needs to append extra query

services/web/server/tests/unit/with_dbs/01/studies_dispatcher/test_studies_dispatcher_handlers.py

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -4,19 +4,22 @@
44

55
import re
66
import urllib.parse
7-
from typing import AsyncIterator
7+
from typing import Any, AsyncIterator
88

99
import pytest
10+
import simcore_service_webserver.studies_dispatcher.handlers_redirects
1011
import sqlalchemy as sa
1112
from aiohttp import ClientResponse, ClientSession, web
1213
from aiohttp.test_utils import TestClient, TestServer
1314
from aioresponses import aioresponses
1415
from models_library.projects_state import ProjectLocked, ProjectStatus
15-
from pydantic import parse_obj_as
16+
from pydantic import BaseModel, parse_obj_as
1617
from pytest import FixtureRequest, MonkeyPatch
1718
from pytest_mock import MockerFixture
1819
from pytest_simcore.helpers.utils_assert import assert_status
1920
from pytest_simcore.helpers.utils_login import UserRole
21+
from pytest_simcore.pydantic_models import iter_model_examples_in_module
22+
from servicelib.json_serialization import json_dumps
2023
from settings_library.redis import RedisSettings
2124
from simcore_service_webserver import catalog
2225
from simcore_service_webserver.studies_dispatcher._core import ViewerInfo
@@ -162,7 +165,6 @@ def _get_base_url(client: TestClient) -> str:
162165

163166

164167
async def test_api_get_viewer_for_file(client: TestClient):
165-
166168
resp = await client.get("/v0/viewers/default?file_type=JPEG")
167169
data, _ = await assert_status(resp, web.HTTPOk)
168170

@@ -184,7 +186,6 @@ async def test_api_get_viewer_for_unsupported_type(client: TestClient):
184186

185187

186188
async def test_api_list_supported_filetypes(client: TestClient):
187-
188189
resp = await client.get("/v0/viewers/default")
189190
data, _ = await assert_status(resp, web.HTTPOk)
190191

@@ -233,7 +234,20 @@ async def test_api_list_supported_filetypes(client: TestClient):
233234
]
234235

235236

236-
@pytest.mark.testit
237+
@pytest.mark.parametrize(
238+
"model_cls, example_name, example_data",
239+
iter_model_examples_in_module(
240+
simcore_service_webserver.studies_dispatcher.handlers_redirects
241+
),
242+
)
243+
def test_model_examples(
244+
model_cls: type[BaseModel], example_name: int, example_data: Any
245+
):
246+
print(example_name, ":", json_dumps(example_data))
247+
model = model_cls.parse_obj(example_data)
248+
assert model
249+
250+
237251
async def test_api_get_services(client: TestClient):
238252
assert client.app
239253

@@ -253,7 +267,6 @@ async def test_api_get_services(client: TestClient):
253267

254268
@pytest.fixture
255269
async def catalog_subsystem_mock(monkeypatch: MonkeyPatch) -> None:
256-
257270
services_in_project = [
258271
{"key": "simcore/services/frontend/file-picker", "version": "1.0.0"}
259272
] + [{"key": s.key, "version": s.version} for s in FAKE_VIEWS_LIST]

0 commit comments

Comments
 (0)