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

🐛 Fixes OrderBy serialization error in pydanticv2 #6828

Merged
merged 7 commits into from
Nov 26, 2024
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
4 changes: 2 additions & 2 deletions packages/models-library/src/models_library/rest_ordering.py
Original file line number Diff line number Diff line change
Expand Up @@ -27,10 +27,10 @@ class OrderBy(BaseModel):


class _BaseOrderQueryParams(RequestParameters):
order_by: OrderBy | None = None
order_by: OrderBy
pcrespov marked this conversation as resolved.
Show resolved Hide resolved


def create_ordering_query_model_classes(
def create_ordering_query_model_class(
*,
ordering_fields: set[str],
default: OrderBy,
Expand Down
81 changes: 74 additions & 7 deletions packages/models-library/tests/test_rest_ordering.py
Original file line number Diff line number Diff line change
@@ -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,
)
Expand Down Expand Up @@ -47,8 +50,72 @@ def _validate_order_by_field(cls, v):
)


@pytest.mark.xfail(
reason="create_ordering_query_model_class.<locals>._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.<locals>._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"},
Expand Down Expand Up @@ -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),
)
Expand All @@ -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),
)
Expand All @@ -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"},
Expand Down Expand Up @@ -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"},
Expand All @@ -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),
)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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")
)

Expand All @@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand Down Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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,
)
Expand Down Expand Up @@ -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,
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -100,7 +100,7 @@ class ProjectFilters(Filters):
)


ProjectsListOrderParams = create_ordering_query_model_classes(
ProjectsListOrderParams = create_ordering_query_model_class(
ordering_fields={
"type",
"uuid",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down Expand Up @@ -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
),
Expand Down Expand Up @@ -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
),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading