From afd66d4df2d16763b1899a76408670773fa09939 Mon Sep 17 00:00:00 2001 From: Pedro Crespo-Valero <32402063+pcrespov@users.noreply.github.com> Date: Tue, 26 Nov 2024 09:45:47 +0100 Subject: [PATCH] =?UTF-8?q?=20=F0=9F=90=9B=20Fixes=20OrderBy=20serializati?= =?UTF-8?q?on=20error=20in=20pydanticv2=20(#6828)?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- .../src/models_library/rest_ordering.py | 4 +- .../tests/test_rest_ordering.py | 81 +++++++++++++++++-- .../tests/aiohttp/test_requests_validation.py | 7 +- .../folders/_folders_handlers.py | 5 +- .../folders/_models.py | 4 +- .../projects/_crud_handlers.py | 4 +- .../projects/_crud_handlers_models.py | 4 +- .../resource_usage/_service_runs_handlers.py | 8 +- .../workspaces/_models.py | 4 +- .../workspaces/_workspaces_handlers.py | 2 +- ...s_crud_handlers__list_with_query_params.py | 3 +- 11 files changed, 97 insertions(+), 29 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..eb2a5adf5f4 100644 --- a/packages/models-library/src/models_library/rest_ordering.py +++ b/packages/models-library/src/models_library/rest_ordering.py @@ -27,10 +27,10 @@ class OrderBy(BaseModel): class _BaseOrderQueryParams(RequestParameters): - order_by: OrderBy | None = None + 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 6db89d4bb48..b2e6e0765b5 100644 --- a/packages/models-library/tests/test_rest_ordering.py +++ b/packages/models-library/tests/test_rest_ordering.py @@ -1,16 +1,19 @@ +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, ConfigDict, Field, Json, + TypeAdapter, ValidationError, field_validator, ) @@ -47,8 +50,72 @@ 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_class( + 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( + 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"}, @@ -77,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), ) @@ -94,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), ) @@ -112,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"}, @@ -142,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"}, @@ -155,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 7f395bfd654..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") ) @@ -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 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..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 @@ -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.model_construct(**query_params.order_by.model_dump()), ) page = Page[FolderGet].model_validate( 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.py b/services/web/server/src/simcore_service_webserver/projects/_crud_handlers.py index 12ce6bb6c18..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 @@ -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.model_construct(**query_params.order_by.model_dump()), tag_ids_list=tag_ids_list, ) 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 73c75038d52..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", @@ -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.model_construct(**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/_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", 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( 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