From a34113377820974db16a505366203fe91ef06ac0 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:23:24 +0100 Subject: [PATCH 1/6] tests cxonvertions --- .../tests/test_rest_ordering.py | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/packages/models-library/tests/test_rest_ordering.py b/packages/models-library/tests/test_rest_ordering.py index 6db89d4bb48..e293f75c808 100644 --- a/packages/models-library/tests/test_rest_ordering.py +++ b/packages/models-library/tests/test_rest_ordering.py @@ -11,6 +11,7 @@ ConfigDict, Field, Json, + TypeAdapter, ValidationError, field_validator, ) @@ -47,6 +48,47 @@ def _validate_order_by_field(cls, v): ) +def test_conversion_order_by_from_query_to_domain_model(): + OrderQueryParamsModel = create_ordering_query_model_classes( + ordering_fields={"modified_at", "name", "description"}, + default=OrderBy(field=IDStr("modified_at"), direction=OrderDirection.DESC), + ) + + # normal + data = {"order_by": {"field": "modified_at", "direction": "asc"}} + query_model = OrderQueryParamsModel.model_validate(data) + + expected_data = data["order_by"] + + assert type(query_model.order_by) is not OrderBy + assert isinstance(query_model.order_by, OrderBy) + + # NOTE: This does NOT convert to OrderBy but has correct data + order_by = TypeAdapter(OrderBy).validate_python( + query_model.order_by, from_attributes=True + ) + assert type(order_by) is not OrderBy + assert order_by.model_dump(mode="json") == expected_data + + order_by = OrderBy.model_validate(query_model.order_by.model_dump()) + assert type(order_by) is OrderBy + assert order_by.model_dump(mode="json") == expected_data + + # NOTE: This does NOT convert to OrderBy but has correct data + order_by = OrderBy.model_validate(query_model.order_by, from_attributes=True) + assert type(order_by) is not OrderBy + assert order_by.model_dump(mode="json") == expected_data + + order_by = OrderBy(**query_model.order_by.model_dump()) + assert type(order_by) is OrderBy + assert order_by.model_dump(mode="json") == expected_data + + # we should use this !!! + order_by = OrderBy.model_construct(**query_model.order_by.model_dump()) + assert type(order_by) is OrderBy + assert order_by.model_dump(mode="json") == expected_data + + def test_ordering_query_model_class_factory(): BaseOrderingQueryModel = create_ordering_query_model_classes( ordering_fields={"modified_at", "name", "description"}, From a31b9e727d13b36173772680e34ed8b9629aefba Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:24:51 +0100 Subject: [PATCH 2/6] order-by conversion --- .../simcore_service_webserver/folders/_folders_handlers.py | 5 ++--- .../src/simcore_service_webserver/projects/_crud_handlers.py | 4 ++-- .../resource_usage/_service_runs_handlers.py | 4 ++-- .../workspaces/_workspaces_handlers.py | 2 +- 4 files changed, 7 insertions(+), 8 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py index eb72dfeffee..921a955027a 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py @@ -10,7 +10,6 @@ from models_library.rest_ordering import OrderBy from models_library.rest_pagination import Page from models_library.rest_pagination_utils import paginate_data -from pydantic import TypeAdapter from servicelib.aiohttp import status from servicelib.aiohttp.requests_validation import ( parse_request_body_as, @@ -82,7 +81,7 @@ async def list_folders(request: web.Request): trashed=query_params.filters.trashed, offset=query_params.offset, limit=query_params.limit, - order_by=OrderBy.model_validate(query_params.order_by), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) page = Page[FolderGet].model_validate( @@ -121,7 +120,7 @@ async def list_folders_full_search(request: web.Request): trashed=query_params.filters.trashed, offset=query_params.offset, limit=query_params.limit, - order_by=TypeAdapter(OrderBy).validate_python(query_params.order_by), + order_by=OrderBy(**query_params.order_by.model_dump()), ) page = Page[FolderGet].model_validate( diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 12ce6bb6c18..d362f85fb81 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -203,7 +203,7 @@ async def list_projects(request: web.Request): limit=query_params.limit, offset=query_params.offset, search=query_params.search, - order_by=OrderBy.model_validate(query_params.order_by), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), folder_id=query_params.folder_id, workspace_id=query_params.workspace_id, ) @@ -241,7 +241,7 @@ async def list_projects_full_search(request: web.Request): limit=query_params.limit, offset=query_params.offset, text=query_params.text, - order_by=query_params.order_by, + order_by=OrderBy(**query_params.order_by.model_dump()), tag_ids_list=tag_ids_list, ) diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py b/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py index 73c75038d52..3f5e79e60ea 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py @@ -136,7 +136,7 @@ async def list_resource_usage_services(request: web.Request): wallet_id=query_params.wallet_id, offset=query_params.offset, limit=query_params.limit, - order_by=OrderBy.model_validate(query_params.order_by), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), filters=TypeAdapter(ServiceResourceUsagesFilters | None).validate_python( query_params.filters ), @@ -216,7 +216,7 @@ async def export_resource_usage_services(request: web.Request): user_id=req_ctx.user_id, product_name=req_ctx.product_name, wallet_id=query_params.wallet_id, - order_by=TypeAdapter(OrderBy | None).validate_python(query_params.order_by), + order_by=OrderBy(**query_params.order_by.model_dump()), filters=TypeAdapter(ServiceResourceUsagesFilters | None).validate_python( query_params.filters ), diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_handlers.py b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_handlers.py index fb2f7c9f1f6..9889e286dda 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_handlers.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_workspaces_handlers.py @@ -79,7 +79,7 @@ async def list_workspaces(request: web.Request): filter_trashed=query_params.filters.trashed, offset=query_params.offset, limit=query_params.limit, - order_by=OrderBy.model_validate(query_params.order_by), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) page = Page[WorkspaceGet].model_validate( From dd99d49ca7d9fc1fb0cb499f37bfad93ac0acc5f Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:29:59 +0100 Subject: [PATCH 3/6] drops none --- packages/models-library/src/models_library/rest_ordering.py | 2 +- .../service-library/tests/aiohttp/test_requests_validation.py | 3 ++- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/models-library/src/models_library/rest_ordering.py b/packages/models-library/src/models_library/rest_ordering.py index b042950c352..a84e0c443ad 100644 --- a/packages/models-library/src/models_library/rest_ordering.py +++ b/packages/models-library/src/models_library/rest_ordering.py @@ -27,7 +27,7 @@ class OrderBy(BaseModel): class _BaseOrderQueryParams(RequestParameters): - order_by: OrderBy | None = None + order_by: OrderBy def create_ordering_query_model_classes( diff --git a/packages/service-library/tests/aiohttp/test_requests_validation.py b/packages/service-library/tests/aiohttp/test_requests_validation.py index 7f395bfd654..ad804f9496a 100644 --- a/packages/service-library/tests/aiohttp/test_requests_validation.py +++ b/packages/service-library/tests/aiohttp/test_requests_validation.py @@ -376,4 +376,5 @@ def test_parse_request_query_parameters_as_with_order_by_query_models(): request = make_mocked_request("GET", path=f"{url}") query_params = parse_request_query_parameters_as(OrderQueryModel, request) - assert query_params.order_by.model_dump() == expected.model_dump() + + assert OrderBy.model_construct(**query_params.order_by.model_dump()) == expected From 776b223403ae4443683d092ceb899658d260bb53 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 18:31:54 +0100 Subject: [PATCH 4/6] costruct --- .../src/simcore_service_webserver/folders/_folders_handlers.py | 2 +- .../src/simcore_service_webserver/projects/_crud_handlers.py | 2 +- .../resource_usage/_service_runs_handlers.py | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) diff --git a/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py b/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py index 921a955027a..becf8af0054 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py +++ b/services/web/server/src/simcore_service_webserver/folders/_folders_handlers.py @@ -120,7 +120,7 @@ async def list_folders_full_search(request: web.Request): trashed=query_params.filters.trashed, offset=query_params.offset, limit=query_params.limit, - order_by=OrderBy(**query_params.order_by.model_dump()), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), ) page = Page[FolderGet].model_validate( diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index d362f85fb81..95e35582c9e 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py @@ -241,7 +241,7 @@ async def list_projects_full_search(request: web.Request): limit=query_params.limit, offset=query_params.offset, text=query_params.text, - order_by=OrderBy(**query_params.order_by.model_dump()), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), tag_ids_list=tag_ids_list, ) diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py b/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py index 3f5e79e60ea..ff550abd462 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py @@ -216,7 +216,7 @@ async def export_resource_usage_services(request: web.Request): user_id=req_ctx.user_id, product_name=req_ctx.product_name, wallet_id=query_params.wallet_id, - order_by=OrderBy(**query_params.order_by.model_dump()), + order_by=OrderBy.model_construct(**query_params.order_by.model_dump()), filters=TypeAdapter(ServiceResourceUsagesFilters | None).validate_python( query_params.filters ), From 57bfe2559cd62c7de3664bf26f1b22b5cbce4f15 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 21:42:20 +0100 Subject: [PATCH 5/6] rename and added new test --- .../src/models_library/rest_ordering.py | 2 +- .../tests/test_rest_ordering.py | 41 +++++++++++++++---- .../tests/aiohttp/test_requests_validation.py | 4 +- .../folders/_models.py | 4 +- .../projects/_crud_handlers_models.py | 4 +- .../resource_usage/_service_runs_handlers.py | 4 +- .../workspaces/_models.py | 4 +- 7 files changed, 44 insertions(+), 19 deletions(-) diff --git a/packages/models-library/src/models_library/rest_ordering.py b/packages/models-library/src/models_library/rest_ordering.py index a84e0c443ad..eb2a5adf5f4 100644 --- a/packages/models-library/src/models_library/rest_ordering.py +++ b/packages/models-library/src/models_library/rest_ordering.py @@ -30,7 +30,7 @@ class _BaseOrderQueryParams(RequestParameters): order_by: OrderBy -def create_ordering_query_model_classes( +def create_ordering_query_model_class( *, ordering_fields: set[str], default: OrderBy, diff --git a/packages/models-library/tests/test_rest_ordering.py b/packages/models-library/tests/test_rest_ordering.py index e293f75c808..b2e6e0765b5 100644 --- a/packages/models-library/tests/test_rest_ordering.py +++ b/packages/models-library/tests/test_rest_ordering.py @@ -1,10 +1,12 @@ +import pickle + import pytest from common_library.json_serialization import json_dumps from models_library.basic_types import IDStr from models_library.rest_ordering import ( OrderBy, OrderDirection, - create_ordering_query_model_classes, + create_ordering_query_model_class, ) from pydantic import ( BaseModel, @@ -48,8 +50,31 @@ def _validate_order_by_field(cls, v): ) +@pytest.mark.xfail( + reason="create_ordering_query_model_class.._OrderBy is still not pickable" +) +def test_pickle_ordering_query_model_class(): + OrderQueryParamsModel = create_ordering_query_model_class( + ordering_fields={"name", "description"}, + default=OrderBy(field=IDStr("name"), direction=OrderDirection.DESC), + ) + + data = {"order_by": {"field": "name", "direction": "asc"}} + query_model = OrderQueryParamsModel.model_validate(data) + + # https://docs.pydantic.dev/latest/concepts/serialization/#pickledumpsmodel + expected = query_model.order_by + + # see https://github.com/ITISFoundation/osparc-simcore/pull/6828 + # FAILURE: raises `AttributeError: Can't pickle local object 'create_ordering_query_model_class.._OrderBy'` + data = pickle.dumps(expected) + + loaded = pickle.loads(data) + assert loaded == expected + + def test_conversion_order_by_from_query_to_domain_model(): - OrderQueryParamsModel = create_ordering_query_model_classes( + OrderQueryParamsModel = create_ordering_query_model_class( ordering_fields={"modified_at", "name", "description"}, default=OrderBy(field=IDStr("modified_at"), direction=OrderDirection.DESC), ) @@ -90,7 +115,7 @@ def test_conversion_order_by_from_query_to_domain_model(): def test_ordering_query_model_class_factory(): - BaseOrderingQueryModel = create_ordering_query_model_classes( + BaseOrderingQueryModel = create_ordering_query_model_class( ordering_fields={"modified_at", "name", "description"}, default=OrderBy(field=IDStr("modified_at"), direction=OrderDirection.DESC), ordering_fields_api_to_column_map={"modified_at": "modified_column"}, @@ -119,7 +144,7 @@ class OrderQueryParamsModel(BaseOrderingQueryModel): def test_ordering_query_model_class__fails_with_invalid_fields(): - OrderQueryParamsModel = create_ordering_query_model_classes( + OrderQueryParamsModel = create_ordering_query_model_class( ordering_fields={"modified", "name", "description"}, default=OrderBy(field=IDStr("modified"), direction=OrderDirection.DESC), ) @@ -136,7 +161,7 @@ def test_ordering_query_model_class__fails_with_invalid_fields(): def test_ordering_query_model_class__fails_with_invalid_direction(): - OrderQueryParamsModel = create_ordering_query_model_classes( + OrderQueryParamsModel = create_ordering_query_model_class( ordering_fields={"modified", "name", "description"}, default=OrderBy(field=IDStr("modified"), direction=OrderDirection.DESC), ) @@ -154,7 +179,7 @@ def test_ordering_query_model_class__fails_with_invalid_direction(): def test_ordering_query_model_class__defaults(): - OrderQueryParamsModel = create_ordering_query_model_classes( + OrderQueryParamsModel = create_ordering_query_model_class( ordering_fields={"modified", "name", "description"}, default=OrderBy(field=IDStr("modified"), direction=OrderDirection.DESC), ordering_fields_api_to_column_map={"modified": "modified_at"}, @@ -184,7 +209,7 @@ def test_ordering_query_model_class__defaults(): def test_ordering_query_model_with_map(): - OrderQueryParamsModel = create_ordering_query_model_classes( + OrderQueryParamsModel = create_ordering_query_model_class( ordering_fields={"modified", "name", "description"}, default=OrderBy(field=IDStr("modified"), direction=OrderDirection.DESC), ordering_fields_api_to_column_map={"modified": "some_db_column_name"}, @@ -197,7 +222,7 @@ def test_ordering_query_model_with_map(): def test_ordering_query_parse_json_pre_validator(): - OrderQueryParamsModel = create_ordering_query_model_classes( + OrderQueryParamsModel = create_ordering_query_model_class( ordering_fields={"modified", "name"}, default=OrderBy(field=IDStr("modified"), direction=OrderDirection.DESC), ) diff --git a/packages/service-library/tests/aiohttp/test_requests_validation.py b/packages/service-library/tests/aiohttp/test_requests_validation.py index ad804f9496a..97c2b317b6a 100644 --- a/packages/service-library/tests/aiohttp/test_requests_validation.py +++ b/packages/service-library/tests/aiohttp/test_requests_validation.py @@ -15,7 +15,7 @@ from models_library.rest_ordering import ( OrderBy, OrderDirection, - create_ordering_query_model_classes, + create_ordering_query_model_class, ) from pydantic import BaseModel, ConfigDict, Field from servicelib.aiohttp import status @@ -365,7 +365,7 @@ async def test_parse_request_with_invalid_headers_params( def test_parse_request_query_parameters_as_with_order_by_query_models(): - OrderQueryModel = create_ordering_query_model_classes( + OrderQueryModel = create_ordering_query_model_class( ordering_fields={"modified", "name"}, default=OrderBy(field="name") ) diff --git a/services/web/server/src/simcore_service_webserver/folders/_models.py b/services/web/server/src/simcore_service_webserver/folders/_models.py index f2e23f58ea6..9cac8a2f1a1 100644 --- a/services/web/server/src/simcore_service_webserver/folders/_models.py +++ b/services/web/server/src/simcore_service_webserver/folders/_models.py @@ -8,7 +8,7 @@ from models_library.rest_ordering import ( OrderBy, OrderDirection, - create_ordering_query_model_classes, + create_ordering_query_model_class, ) from models_library.rest_pagination import PageQueryParameters from models_library.trash import RemoveQueryParams @@ -42,7 +42,7 @@ class FolderFilters(Filters): ) -_FolderOrderQueryParams: type[RequestParameters] = create_ordering_query_model_classes( +_FolderOrderQueryParams: type[RequestParameters] = create_ordering_query_model_class( ordering_fields={ "modified_at", "name", diff --git a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers_models.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers_models.py index ba0ef6ae78f..5d10df1ffe1 100644 --- a/services/web/server/src/simcore_service_webserver/projects/_crud_handlers_models.py +++ b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers_models.py @@ -15,7 +15,7 @@ from models_library.rest_ordering import ( OrderBy, OrderDirection, - create_ordering_query_model_classes, + create_ordering_query_model_class, ) from models_library.rest_pagination import PageQueryParameters from models_library.utils.common_validators import ( @@ -100,7 +100,7 @@ class ProjectFilters(Filters): ) -ProjectsListOrderParams = create_ordering_query_model_classes( +ProjectsListOrderParams = create_ordering_query_model_class( ordering_fields={ "type", "uuid", diff --git a/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py b/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py index ff550abd462..4f952a932de 100644 --- a/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py +++ b/services/web/server/src/simcore_service_webserver/resource_usage/_service_runs_handlers.py @@ -16,7 +16,7 @@ from models_library.rest_ordering import ( OrderBy, OrderDirection, - create_ordering_query_model_classes, + create_ordering_query_model_class, ) from models_library.rest_pagination import Page, PageQueryParameters from models_library.rest_pagination_utils import paginate_data @@ -53,7 +53,7 @@ async def wrapper(request: web.Request) -> web.StreamResponse: _ResorceUsagesListOrderQueryParams: type[ RequestParameters -] = create_ordering_query_model_classes( +] = create_ordering_query_model_class( ordering_fields={ "wallet_id", "wallet_name", diff --git a/services/web/server/src/simcore_service_webserver/workspaces/_models.py b/services/web/server/src/simcore_service_webserver/workspaces/_models.py index fec1d7d8fcb..362d68884c7 100644 --- a/services/web/server/src/simcore_service_webserver/workspaces/_models.py +++ b/services/web/server/src/simcore_service_webserver/workspaces/_models.py @@ -6,7 +6,7 @@ from models_library.rest_ordering import ( OrderBy, OrderDirection, - create_ordering_query_model_classes, + create_ordering_query_model_class, ) from models_library.rest_pagination import PageQueryParameters from models_library.trash import RemoveQueryParams @@ -31,7 +31,7 @@ class WorkspacesPathParams(StrictRequestParameters): _WorkspacesListOrderQueryParams: type[ RequestParameters -] = create_ordering_query_model_classes( +] = create_ordering_query_model_class( ordering_fields={ "modified_at", "name", From 80f1b461b8dac609df826cfc0a6d87ba16971a30 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Mon, 25 Nov 2024 21:53:16 +0100 Subject: [PATCH 6/6] minor --- .../02/test_projects_crud_handlers__list_with_query_params.py | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__list_with_query_params.py b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__list_with_query_params.py index abb26a3f3e3..16610b9c774 100644 --- a/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__list_with_query_params.py +++ b/services/web/server/tests/unit/with_dbs/02/test_projects_crud_handlers__list_with_query_params.py @@ -7,9 +7,10 @@ import json import random from collections import UserDict +from collections.abc import Iterator from copy import deepcopy from pathlib import Path -from typing import Any, Iterator +from typing import Any import pytest import sqlalchemy as sa