Skip to content

Commit 4b919e6

Browse files
committed
Refactor check_ordering_list to enhance parameter documentation and type annotations; remove unused functions and imports
1 parent b10606d commit 4b919e6

File tree

3 files changed

+12
-179
lines changed

3 files changed

+12
-179
lines changed

packages/models-library/src/models_library/list_operations.py

Lines changed: 12 additions & 73 deletions
Original file line numberDiff line numberDiff line change
@@ -6,8 +6,9 @@
66
"""
77

88
from enum import Enum
9-
from typing import TYPE_CHECKING, Any, Generic, Literal, TypeVar, get_args, get_origin
9+
from typing import TYPE_CHECKING, Annotated, Generic, TypeVar
1010

11+
from annotated_types import doc
1112
from pydantic.generics import GenericModel
1213

1314

@@ -29,34 +30,24 @@ def __str__(self) -> str: ...
2930
TField = TypeVar("TField", bound=str)
3031

3132

32-
def get_literal_values(tfield: Any) -> tuple[str, ...] | None:
33-
"""Return Literal values if TField is a Literal, else None."""
34-
if get_origin(tfield) is Literal:
35-
return get_args(tfield)
36-
return None
37-
38-
3933
class OrderClause(GenericModel, Generic[TField]):
4034
field: TField
4135
direction: OrderDirection = OrderDirection.ASC
4236

4337

4438
def check_ordering_list(
45-
order_by: list[tuple[TField, OrderDirection]],
46-
) -> list[tuple[TField, OrderDirection]]:
39+
order_by: Annotated[
40+
list[tuple[TField, OrderDirection]],
41+
doc(
42+
"Duplicates with same direction dropped, conflicting directions raise ValueError"
43+
),
44+
],
45+
) -> Annotated[
46+
list[tuple[TField, OrderDirection]],
47+
doc("Duplicates removed, preserving first occurrence order"),
48+
]:
4749
"""Validates ordering list and removes duplicate entries.
4850
49-
Ensures that each field appears at most once. If a field is repeated:
50-
- With the same direction: silently drops the duplicate
51-
- With different directions: raises ValueError
52-
53-
54-
Args:
55-
order_by: List of (field, direction) tuples
56-
57-
Returns:
58-
List with duplicates removed, preserving order of first occurrence
59-
6051
Raises:
6152
ValueError: If a field appears with conflicting directions
6253
"""
@@ -77,55 +68,3 @@ def check_ordering_list(
7768
unique_order_by.append((field, direction))
7869

7970
return unique_order_by
80-
81-
82-
def map_order_fields(
83-
order_clauses: list[OrderClause[TField]], field_mapping: dict[str, str]
84-
) -> list[tuple[str, OrderDirection]]:
85-
"""Map order clause fields using a field mapping dictionary.
86-
87-
Args:
88-
order_clauses: List of OrderClause objects with API field names
89-
field_mapping: Dictionary mapping API field names to domain/DB field names
90-
91-
Returns:
92-
List of tuples with mapped field names and directions
93-
94-
Example:
95-
>>> clauses = [OrderClause(field="email", direction=OrderDirection.ASC)]
96-
>>> mapping = {"email": "user_email", "created_at": "created"}
97-
>>> map_order_fields(clauses, mapping)
98-
[("user_email", OrderDirection.ASC)]
99-
"""
100-
return [
101-
(field_mapping[str(clause.field)], clause.direction) for clause in order_clauses
102-
]
103-
104-
105-
def validate_order_fields_with_literals(
106-
order_by: list[tuple[str, str]],
107-
valid_fields: set[str],
108-
) -> None:
109-
"""Validate order_by list with string field names and directions.
110-
111-
Args:
112-
order_by: List of (field_name, direction) tuples with string values
113-
valid_fields: Set of allowed field names
114-
valid_directions: Set of allowed direction values
115-
116-
Raises:
117-
ValueError: If any field or direction is invalid
118-
"""
119-
valid_directions = {OrderDirection.ASC.value, OrderDirection.DESC.value}
120-
121-
invalid_fields = {field for field, _ in order_by if field not in valid_fields}
122-
if invalid_fields:
123-
msg = f"Invalid order_by field(s): {invalid_fields}. Valid fields are: {valid_fields}"
124-
raise ValueError(msg)
125-
126-
invalid_directions = {
127-
direction for _, direction in order_by if direction not in valid_directions
128-
}
129-
if invalid_directions:
130-
msg = f"Invalid order direction(s): {invalid_directions}. Must be one of: {valid_directions}"
131-
raise ValueError(msg)
Lines changed: 0 additions & 103 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,7 @@
1-
from typing import Literal
2-
31
import pytest
42
from models_library.list_operations import (
5-
OrderClause,
63
OrderDirection,
74
check_ordering_list,
8-
map_order_fields,
9-
validate_order_fields_with_literals,
105
)
116

127

@@ -73,101 +68,3 @@ def test_check_ordering_list_no_duplicates():
7368

7469
# Should return the same list
7570
assert result == order_by
76-
77-
78-
def test_map_order_fields():
79-
"""Test that map_order_fields correctly maps field names using provided mapping"""
80-
81-
ValidField = Literal["email", "created_at", "name"]
82-
83-
order_clauses = [
84-
OrderClause[ValidField](field="email", direction=OrderDirection.ASC),
85-
OrderClause[ValidField](field="created_at", direction=OrderDirection.DESC),
86-
OrderClause[ValidField](field="name", direction=OrderDirection.ASC),
87-
]
88-
89-
field_mapping = {
90-
"email": "user_email",
91-
"created_at": "created_timestamp",
92-
"name": "display_name",
93-
}
94-
95-
result = map_order_fields(order_clauses, field_mapping)
96-
97-
expected = [
98-
("user_email", OrderDirection.ASC),
99-
("created_timestamp", OrderDirection.DESC),
100-
("display_name", OrderDirection.ASC),
101-
]
102-
103-
assert result == expected
104-
105-
106-
def test_map_order_fields_with_unmapped_field():
107-
"""Test that map_order_fields raises KeyError when field is not in mapping"""
108-
109-
ValidField = Literal["email", "unknown"]
110-
111-
order_clauses = [
112-
OrderClause[ValidField](field="email", direction=OrderDirection.ASC),
113-
OrderClause[ValidField](field="unknown", direction=OrderDirection.DESC),
114-
]
115-
116-
field_mapping = {
117-
"email": "user_email",
118-
# "unknown" is missing from mapping
119-
}
120-
121-
with pytest.raises(KeyError):
122-
map_order_fields(order_clauses, field_mapping)
123-
124-
125-
def test_validate_order_fields_with_literals_valid():
126-
"""Test that validate_order_fields_with_literals passes with valid fields and directions"""
127-
128-
order_by = [
129-
("email", "asc"),
130-
("created", "desc"),
131-
("name", "asc"),
132-
]
133-
134-
valid_fields = {"email", "created", "name"}
135-
136-
# Should not raise any exception
137-
validate_order_fields_with_literals(order_by, valid_fields)
138-
139-
140-
def test_validate_order_fields_with_literals_invalid_field():
141-
"""Test that validate_order_fields_with_literals raises ValueError for invalid fields"""
142-
143-
order_by = [
144-
("email", "asc"),
145-
("invalid_field", "desc"),
146-
]
147-
148-
valid_fields = {"email", "created"}
149-
150-
with pytest.raises(ValueError, match="Invalid order_by field") as exc_info:
151-
validate_order_fields_with_literals(order_by, valid_fields)
152-
153-
error_msg = str(exc_info.value)
154-
assert "invalid_field" in error_msg
155-
assert "Valid fields are" in error_msg
156-
157-
158-
def test_validate_order_fields_with_literals_invalid_direction():
159-
"""Test that validate_order_fields_with_literals raises ValueError for invalid directions"""
160-
161-
order_by = [
162-
("email", "ascending"), # Invalid direction
163-
("created", "desc"),
164-
]
165-
166-
valid_fields = {"email", "created"}
167-
168-
with pytest.raises(ValueError, match="Invalid order direction") as exc_info:
169-
validate_order_fields_with_literals(order_by, valid_fields)
170-
171-
error_msg = str(exc_info.value)
172-
assert "ascending" in error_msg
173-
assert "Must be one of" in error_msg

packages/models-library/tests/test_rest_ordering.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
import pytest
55
from common_library.json_serialization import json_dumps
66
from models_library.basic_types import IDStr
7-
from models_library.list_operations import get_literal_values
87
from models_library.rest_ordering import (
98
OrderBy,
109
OrderClause,
@@ -260,8 +259,6 @@ class TestOrderingParams(OrderingQueryParams[ValidField]):
260259
OrderClause[ValidField](field="gender", direction=OrderDirection.ASC),
261260
]
262261

263-
assert get_literal_values(ValidField) == ("created_at", "name", "gender")
264-
265262

266263
def test_ordering_query_params_validation_error_with_invalid_fields():
267264
"""Test that OrderingQueryParams raises ValidationError when invalid fields are used"""

0 commit comments

Comments
 (0)