diff --git a/app/adapters/delivery_providers/cdek/order_mapper.py b/app/adapters/delivery_providers/cdek/order_mapper.py index 9ce7f1c..ddc38b5 100644 --- a/app/adapters/delivery_providers/cdek/order_mapper.py +++ b/app/adapters/delivery_providers/cdek/order_mapper.py @@ -2,7 +2,12 @@ from typing import Any -from app.schemas.order import OrderCreateRequest, OrderCreateResponse +from app.schemas.order import ( + OrderCreateRequest, + OrderCreateResponse, + OrderPackage, + OrderParty, +) class CDEKOrderMappingError(ValueError): @@ -10,7 +15,22 @@ class CDEKOrderMappingError(ValueError): def map_cdek_order_request(request: OrderCreateRequest) -> dict[str, Any]: - return request.model_dump(mode="python", exclude_none=True) + payload: dict[str, Any] = { + "type": request.type, + "tariff_code": request.tariff_code, + "sender": _map_order_party(request.sender), + "recipient": _map_order_party(request.recipient), + "from_location": request.from_location.model_dump(mode="python"), + "to_location": request.to_location.model_dump(mode="python"), + "packages": [_map_order_package(package) for package in request.packages], + } + if request.comment is not None: + payload["comment"] = request.comment + if request.services is not None: + payload["services"] = [ + service.model_dump(mode="python") for service in request.services + ] + return payload def map_cdek_order_response(payload: dict[str, Any]) -> OrderCreateResponse: @@ -23,3 +43,17 @@ def map_cdek_order_response(payload: dict[str, Any]) -> OrderCreateResponse: raise CDEKOrderMappingError("CDEK order response must include entity.uuid.") return OrderCreateResponse(provider="cdek", order_uuid=order_uuid) + + +def _map_order_party(party: OrderParty) -> dict[str, Any]: + return { + "name": party.name, + "email": party.email, + "phones": [party.phone.model_dump(mode="python")], + } + + +def _map_order_package(package: OrderPackage) -> dict[str, Any]: + payload = package.model_dump(mode="python", exclude_none=True) + payload["weight"] = package.weight * 1000 + return payload diff --git a/app/schemas/order.py b/app/schemas/order.py index d373e75..7e715a0 100644 --- a/app/schemas/order.py +++ b/app/schemas/order.py @@ -2,7 +2,7 @@ from typing import Literal -from pydantic import BaseModel, Field, model_validator +from pydantic import BaseModel, ConfigDict, Field, model_validator class OrderPhone(BaseModel): @@ -10,9 +10,11 @@ class OrderPhone(BaseModel): class OrderParty(BaseModel): + model_config = ConfigDict(extra="forbid") + name: str = Field(min_length=1) email: str = Field(min_length=1) - phones: list[OrderPhone] = Field(min_length=1) + phone: OrderPhone @model_validator(mode="before") @classmethod @@ -50,7 +52,7 @@ class OrderCreateRequest(BaseModel): recipient: OrderParty from_location: OrderLocation to_location: OrderLocation - services: list[OrderService] = Field(min_length=1) + services: list[OrderService] | None = Field(default=None, min_length=1) packages: list[OrderPackage] = Field(min_length=1) diff --git a/http-client.http b/http-client.http index b23c884..e5f11bf 100644 --- a/http-client.http +++ b/http-client.http @@ -24,20 +24,16 @@ Content-Type: application/json "sender": { "name": "Петр Петров", "email": "sender@example.com", - "phones": [ - { - "number": "+79009876543" - } - ] + "phone": { + "number": "+79009876543" + } }, "recipient": { "name": "Иван Иванов", "email": "ivan@example.com", - "phones": [ - { - "number": "+79001234567" - } - ] + "phone": { + "number": "+79001234567" + } }, "from_location": { "address": "ул. Ленина, 1", @@ -49,16 +45,10 @@ Content-Type: application/json "city": "Новосибирск", "country_code": "RU" }, - "services": [ - { - "code": "INSURANCE", - "parameter": "1000" - } - ], "packages": [ { "number": "1", - "weight": 1000, + "weight": 1, "length": 20, "width": 15, "height": 10, diff --git a/spec/index.md b/spec/index.md index 81523f9..bba3dbe 100644 --- a/spec/index.md +++ b/spec/index.md @@ -32,9 +32,10 @@ | 023 | DONE | 2026-03-29 | Add Yandex Geosuggest address suggestion adapter and CIS routing | `spec/tasks/023_add_yandex_geosuggest_address_suggestion_adapter.md` | | 024 | DONE | 2026-03-29 | Add TomTom address suggestion adapter and Europe routing | `spec/tasks/024_add_tomtom_address_suggestion_adapter.md` | | 025 | DONE | 2026-04-03 | Remove company from Create Delivery Order parties | `spec/tasks/025_remove_company_from_create_delivery_order.md` | +| 026 | DONE | 2026-04-05 | Align CDEK order contract with single phone and kilogram package weight | `spec/tasks/026_align_cdek_order_contract_single_phone_and_weight_units.md` | ## Summary -- Total: **26** +- Total: **27** - TODO: **0** -- DONE: **26** +- DONE: **27** diff --git a/spec/overview.md b/spec/overview.md index 0ce768a..1adbdc5 100644 --- a/spec/overview.md +++ b/spec/overview.md @@ -97,6 +97,9 @@ - Для расчёта тарифа CDEK adapter принимает city identifiers из `DeliveryCalculationRequest`, находит запись в `cities_map`, берёт `cdek.code` и передаёт его в CDEK API Для сценария создания заказа CDEK adapter принимает валидированную order model, отправляет контракт `Регистрация заказа (тип "доставка", до двери)` из `http-client.http` и возвращает внутреннюю response model без утечки HTTP-деталей в Service. +Во внутреннем order flow `sender` и `recipient` содержат ровно одно поле `phone`, а CDEK adapter сериализует его в provider payload `phones` с одним элементом. +Поле `services` в order flow является необязательным; при отсутствии значения adapter не отправляет `services` в CDEK payload. +Поле `packages[*].weight` во входном order request задаётся в килограммах, а CDEK adapter конвертирует его в граммы перед отправкой в provider API. ### Adapter (`app/adapters/address_suggestions`) - `base.py` — абстрактный интерфейс `AddressSuggestionProvider`: @@ -173,11 +176,11 @@ comment: str | None sender: name: str email: str - phones: list[{number: str}] + phone: {number: str} recipient: name: str email: str - phones: list[{number: str}] + phone: {number: str} from_location: address: str city: str @@ -186,11 +189,13 @@ to_location: address: str city: str country_code: str -services: list[{code: str, parameter: str}] +services: list[{code: str, parameter: str}] | None packages: list[{number: str, weight: int, length: int, width: int, height: int, comment: str | None}] ``` `from_location.address` и `to_location.address` должны содержать точные значения адреса, выбранные клиентом; order flow не выполняет address suggestion lookup. +`sender.phone` и `recipient.phone` представляют единственный телефон для соответствующей стороны заказа; передача нескольких телефонов во входном API не поддерживается. +`packages[*].weight` в `OrderCreateRequest` задаётся в килограммах, а в payload CDEK должен передаваться в граммах. ### Выходная: `OrderCreateResponse` ``` diff --git a/spec/tasks/026_align_cdek_order_contract_single_phone_and_weight_units.md b/spec/tasks/026_align_cdek_order_contract_single_phone_and_weight_units.md new file mode 100644 index 0000000..bb6d769 --- /dev/null +++ b/spec/tasks/026_align_cdek_order_contract_single_phone_and_weight_units.md @@ -0,0 +1,52 @@ +--- +id: 026 +title: Align CDEK order contract with single phone and kilogram package weight +status: DONE +created: 2026-04-05 +--- + +## Context +Текущий order flow принимает `sender.phones` и `recipient.phones` как список, требует обязательное поле `services` и пробрасывает `packages.weight` в CDEK payload без явной фиксации единиц измерения. Новый контракт должен принимать один телефон на сторону, разрешать отсутствие `services` и гарантировать, что во входном API вес упаковки задаётся в килограммах, а в CDEK отправляется в граммах. + +## Goal +Обновить flow `POST /api/v1/delivery/order` по слоям Controller, Service и Adapter так, чтобы public/internal request contract использовал `phone` вместо `phones`, поле `services` было необязательным, а CDEK order mapper конвертировал `packages.weight` из килограммов во входном запросе в граммы во внешнем provider payload. + +## Constraints +- Изменения ограничены существующими модулями order flow: `app/schemas/order.py`, `app/controllers/v1/delivery.py`, `app/services/aggregator.py`, `app/adapters/delivery_providers/cdek/`, `http-client.http` и связанными тестами. +- `POST /api/v1/delivery/order` должен оставаться в существующем controller и по-прежнему вызывать ровно один метод Service: `AggregatorService.create_order()`. +- Service остаётся orchestration layer и не получает новую business logic; он только принимает обновлённую order model и делегирует её adapter. +- Во внутреннем order flow и публичном API поле `phones` должно быть удалено; передача нескольких телефонов больше не поддерживается. +- CDEK adapter может сериализовать внутренний `phone` в provider-specific поле `phones`, если это требуется внешним контрактом CDEK, но множественность телефонов не должна возвращаться во внутренние модели и controller contract. +- `services` должно быть необязательным полем request schema; при отсутствии значения нельзя подставлять фиктивные service entries. +- Конвертация единиц `packages.weight` должна выполняться только на границе CDEK adapter mapping: входной order request использует килограммы, исходящий payload в CDEK использует граммы. +- Scope задачи не включает изменение response contract, price flow, address suggestion flow, provider routing, кеширование, новые провайдеры и расширение поддерживаемых `type`/`tariff_code`. +- Не изменять файлы в `spec/`. + +## Acceptance criteria +- `OrderCreateRequest` использует `sender.phone` и `recipient.phone` вместо `sender.phones` и `recipient.phones`. +- Если request payload содержит `sender.phones` или `recipient.phones`, endpoint возвращает 422 на уровне schema validation. +- `POST /api/v1/delivery/order` принимает валидный payload без поля `services`. +- Если `services` отсутствует или равен `null`, service и adapter flow успешно обрабатывают запрос без добавления `services` в исходящий CDEK payload. +- `AggregatorService.create_order()` продолжает только оркестрировать вызов injected order adapter и не содержит преобразования `phone`/`phones` или килограммов в граммы. +- CDEK order mapper формирует provider payload с полями `sender.phones` и `recipient.phones`, каждое из которых содержит ровно один элемент, полученный из соответствующего внутреннего поля `phone`. +- Для каждого элемента `packages` исходящий payload CDEK содержит `weight`, равный значению входного `packages.weight`, умноженному на `1000`. +- `http-client.http` содержит актуальный пример Create Delivery Order с `phone` вместо `phones`, без обязательного `services` и с весом, отражающим controller contract в килограммах. + +## Definition of Done +- [ ] Обновлён public/internal order request contract на `phone` вместо `phones`. +- [ ] `services` сделано необязательным без изменения endpoint path и service orchestration. +- [ ] Реализован mapping одного телефона в provider payload `phones` и конвертация `packages.weight` из килограммов в граммы. +- [ ] Обновлены controller, service и adapter tests под новый контракт и unit conversion. +- [ ] Обновлён пример запроса в `http-client.http`. + +## Tests +- Обновить `tests/controllers/v1/test_order.py` для success case с `phone`, сценариев 422 при передаче `sender.phones` и `recipient.phones`, а также для запроса без `services`. +- Обновить `tests/services/test_order.py` для проверки, что service принимает обновлённую order model, делегирует её adapter без новой логики и корректно работает с `services=None`. +- Обновить `tests/adapters/delivery_providers/cdek/test_order_client.py` для проверки mapping `phone -> phones[0]`, отсутствия `services` в исходящем payload при `None` и конвертации `packages.weight` из килограммов в граммы. +- При необходимости обновить другие order-related tests и fixtures, завязанные на старые поля `phones` и обязательность `services`. + +## Commands +- `poetry run pytest tests/controllers/v1/test_order.py -q` +- `poetry run pytest tests/services/test_order.py -q` +- `poetry run pytest tests/adapters/delivery_providers/cdek/test_order_client.py -q` +- `python3 spec/gen_spec_index.py --check` diff --git a/tests/adapters/delivery_providers/cdek/test_order_client.py b/tests/adapters/delivery_providers/cdek/test_order_client.py index a73c474..09b439d 100644 --- a/tests/adapters/delivery_providers/cdek/test_order_client.py +++ b/tests/adapters/delivery_providers/cdek/test_order_client.py @@ -59,12 +59,12 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest: "sender": { "name": "Petr Petrov", "email": "sender@example.com", - "phones": [{"number": "+79009876543"}], + "phone": {"number": "+79009876543"}, }, "recipient": { "name": "Ivan Ivanov", "email": "ivan@example.com", - "phones": [{"number": "+79001234567"}], + "phone": {"number": "+79001234567"}, }, "from_location": { "address": "Lenina 1", @@ -80,7 +80,7 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest: "packages": [ { "number": "1", - "weight": 1000, + "weight": 1, "length": 20, "width": 15, "height": 10, @@ -160,6 +160,70 @@ def test_provider_register_order_posts_cdek_contract_payload_and_maps_response() ] +def test_provider_register_order_omits_services_when_none_and_converts_package_weights() -> None: + response = httpx.Response( + 200, + json={"entity": {"uuid": "cdek-order-uuid"}}, + request=httpx.Request("POST", "https://api.cdek.test/v2/orders"), + ) + http_client = SequenceHTTPClient([response]) + provider = CDEKProvider( + CDEKClient( + http_client=http_client, # type: ignore[arg-type] + auth_client=StubAuthClient(), # type: ignore[arg-type] + base_url="https://api.cdek.test/v2", + timeout_seconds=7.5, + retry_attempts=0, + ) + ) + request = _make_order_request( + services=None, + packages=[ + { + "number": "1", + "weight": 1, + "length": 20, + "width": 15, + "height": 10, + "comment": "Package 1", + }, + { + "number": "2", + "weight": 2, + "length": 25, + "width": 18, + "height": 12, + "comment": "Package 2", + }, + ], + ) + + asyncio.run(provider.register_order(request)) + + payload = http_client.calls[0]["json"] + assert "services" not in payload + assert payload["sender"]["phones"] == [{"number": "+79009876543"}] + assert payload["recipient"]["phones"] == [{"number": "+79001234567"}] + assert payload["packages"] == [ + { + "number": "1", + "weight": 1000, + "length": 20, + "width": 15, + "height": 10, + "comment": "Package 1", + }, + { + "number": "2", + "weight": 2000, + "length": 25, + "width": 18, + "height": 12, + "comment": "Package 2", + }, + ] + + def test_cdek_client_register_order_maps_4xx_to_request_error() -> None: rejected_response = httpx.Response( 422, diff --git a/tests/controllers/v1/test_order.py b/tests/controllers/v1/test_order.py index 8ef7a9f..fd8b07e 100644 --- a/tests/controllers/v1/test_order.py +++ b/tests/controllers/v1/test_order.py @@ -39,12 +39,12 @@ def _valid_payload() -> dict[str, object]: "sender": { "name": "Petr Petrov", "email": "sender@example.com", - "phones": [{"number": "+79009876543"}], + "phone": {"number": "+79009876543"}, }, "recipient": { "name": "Ivan Ivanov", "email": "ivan@example.com", - "phones": [{"number": "+79001234567"}], + "phone": {"number": "+79001234567"}, }, "from_location": { "address": "Lenina 1", @@ -60,7 +60,7 @@ def _valid_payload() -> dict[str, object]: "packages": [ { "number": "1", - "weight": 1000, + "weight": 1, "length": 20, "width": 15, "height": 10, @@ -136,6 +136,30 @@ def test_post_delivery_order_rejects_sender_company_field() -> None: assert service.calls == [] +def test_post_delivery_order_rejects_sender_phones_field() -> None: + service = StubAggregatorService(response=None) + app = create_app() + _install_service_override(app, service) + invalid_payload = _valid_payload() + invalid_payload["sender"] = { + **invalid_payload["sender"], # type: ignore[arg-type] + "phones": [{"number": "+79009876543"}], + } + + async def run_request() -> httpx.Response: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, + base_url="http://testserver", + ) as client: + return await client.post("/api/v1/delivery/order", json=invalid_payload) + + response = asyncio.run(run_request()) + + assert response.status_code == 422 + assert service.calls == [] + + def test_post_delivery_order_rejects_recipient_company_field() -> None: service = StubAggregatorService(response=None) app = create_app() @@ -160,6 +184,53 @@ def test_post_delivery_order_rejects_recipient_company_field() -> None: assert service.calls == [] +def test_post_delivery_order_rejects_recipient_phones_field() -> None: + service = StubAggregatorService(response=None) + app = create_app() + _install_service_override(app, service) + invalid_payload = _valid_payload() + invalid_payload["recipient"] = { + **invalid_payload["recipient"], # type: ignore[arg-type] + "phones": [{"number": "+79001234567"}], + } + + async def run_request() -> httpx.Response: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, + base_url="http://testserver", + ) as client: + return await client.post("/api/v1/delivery/order", json=invalid_payload) + + response = asyncio.run(run_request()) + + assert response.status_code == 422 + assert service.calls == [] + + +def test_post_delivery_order_accepts_request_without_services() -> None: + expected_response = OrderCreateResponse(provider="cdek", order_uuid="order-uuid-1") + service = StubAggregatorService(response=expected_response) + app = create_app() + _install_service_override(app, service) + payload = _valid_payload() + del payload["services"] + + async def run_request() -> httpx.Response: + transport = httpx.ASGITransport(app=app) + async with httpx.AsyncClient( + transport=transport, + base_url="http://testserver", + ) as client: + return await client.post("/api/v1/delivery/order", json=payload) + + response = asyncio.run(run_request()) + + assert response.status_code == 200 + assert response.json() == expected_response.model_dump(mode="json") + assert service.calls == [OrderCreateRequest(**payload)] + + def test_post_delivery_order_maps_invalid_request_to_400() -> None: service = StubAggregatorService( response=None, diff --git a/tests/services/test_order.py b/tests/services/test_order.py index 73d570d..0f3a8f0 100644 --- a/tests/services/test_order.py +++ b/tests/services/test_order.py @@ -39,12 +39,12 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest: "sender": { "name": "Petr Petrov", "email": "sender@example.com", - "phones": [{"number": "+79009876543"}], + "phone": {"number": "+79009876543"}, }, "recipient": { "name": "Ivan Ivanov", "email": "ivan@example.com", - "phones": [{"number": "+79001234567"}], + "phone": {"number": "+79001234567"}, }, "from_location": { "address": "Lenina 1", @@ -60,7 +60,7 @@ def _make_order_request(**overrides: object) -> OrderCreateRequest: "packages": [ { "number": "1", - "weight": 1000, + "weight": 1, "length": 20, "width": 15, "height": 10, @@ -96,6 +96,20 @@ def test_create_order_maps_provider_request_errors_to_invalid_order_error() -> N assert adapter.calls == [request] +def test_create_order_delegates_request_with_services_none_without_new_logic() -> None: + request = _make_order_request(services=None) + adapter = StubOrderAdapter( + response=OrderCreateResponse(provider="cdek", order_uuid="order-uuid-2") + ) + service = AggregatorService(providers=[], order_adapter=adapter) + + result = asyncio.run(service.create_order(request)) + + assert result == OrderCreateResponse(provider="cdek", order_uuid="order-uuid-2") + assert adapter.calls == [request] + assert adapter.calls[0].services is None + + def test_create_order_maps_transport_failures_to_unavailable_error() -> None: request = _make_order_request() adapter = StubOrderAdapter(error=RuntimeError("transport down"))