diff --git a/app/domain/price.py b/app/domain/price.py index c66c7eb..8932b2a 100644 --- a/app/domain/price.py +++ b/app/domain/price.py @@ -12,6 +12,7 @@ DIMENSIONS_ROUND_SCALE = 1 MIN_WEIGHT_KG = Decimal("0.01") MIN_DIMENSION_CM = Decimal("0.1") INTEGER_PRICE_QUANTIZER = Decimal("1") +DOC_SERVICE_MARKERS = ("документ", "document") _MISSING = object() @@ -108,14 +109,36 @@ def filter_and_sort_prices( prices: Iterable[object], *, price_multiplier: Decimal = DEFAULT_PROVIDER_PRICE_MULTIPLIER, + parcel_type: object | None = None, ) -> list[ProviderPrice]: """Apply domain filtering and return prices ordered by value.""" return sort_prices_by_price( - filter_valid_prices(prices, price_multiplier=price_multiplier) + filter_prices_by_parcel_type( + filter_valid_prices(prices, price_multiplier=price_multiplier), + parcel_type=parcel_type, + ) ) +def filter_prices_by_parcel_type( + prices: Iterable[ProviderPrice], + *, + parcel_type: object | None = None, +) -> list[ProviderPrice]: + """Filter aggregated prices according to the requested parcel type.""" + + normalized_parcel_type = _normalize_parcel_type(parcel_type) + if normalized_parcel_type is None: + return list(prices) + + return [ + price + for price in prices + if _matches_parcel_type(price.service_name, normalized_parcel_type) + ] + + def _normalize_price( candidate: object, *, @@ -225,6 +248,28 @@ def _normalize_price_multiplier(value: Decimal) -> Decimal: return multiplier +def _normalize_parcel_type(value: object) -> str | None: + normalized = _normalize_text(value).casefold() + if normalized in {"doc", "parcel"}: + return normalized + return None + + +def _matches_parcel_type(service_name: str, parcel_type: str) -> bool: + is_document_tariff = _contains_document_marker(service_name) + if parcel_type == "doc": + return is_document_tariff + return not is_document_tariff + + +def _contains_document_marker(service_name: str) -> bool: + normalized_service_name = service_name.casefold() + return any( + marker in normalized_service_name + for marker in DOC_SERVICE_MARKERS + ) + + def _apply_price_multiplier_and_round( price: Decimal, *, diff --git a/app/schemas/request.py b/app/schemas/request.py index 1e75ca1..f7f7c97 100644 --- a/app/schemas/request.py +++ b/app/schemas/request.py @@ -10,6 +10,11 @@ class DeliveryEntity(StrEnum): LEGAL = "legal" +class ParcelType(StrEnum): + DOC = "doc" + PARCEL = "parcel" + + class DeliveryRequest(BaseModel): entity: DeliveryEntity from_city: str = Field(min_length=1) @@ -19,3 +24,4 @@ class DeliveryRequest(BaseModel): length_cm: float = Field(gt=0) width_cm: float = Field(gt=0) height_cm: float = Field(gt=0) + parcel_type: ParcelType | None = None diff --git a/app/services/aggregator.py b/app/services/aggregator.py index 72d2416..8e30520 100644 --- a/app/services/aggregator.py +++ b/app/services/aggregator.py @@ -48,6 +48,7 @@ class FilterAndSortPricesFn(Protocol): prices: Iterable[object], *, price_multiplier: Decimal = DEFAULT_PROVIDER_PRICE_MULTIPLIER, + parcel_type: object | None = None, ) -> list[object]: ... @@ -118,6 +119,7 @@ class AggregatorService: filtered_and_sorted = self._filter_and_sort_prices( successful_results, price_multiplier=self._provider_price_multiplier, + parcel_type=request.parcel_type, ) return [self._coerce_delivery_price(price) for price in filtered_and_sorted] diff --git a/spec/index.md b/spec/index.md index 727bbe9..7c948d4 100644 --- a/spec/index.md +++ b/spec/index.md @@ -1,7 +1,7 @@ # Spec Tasks Index > ⚠️ This file is generated. Do not edit manually. -> Generated at (UTC): `2026-03-15T21:04:41+00:00` +> Generated at (UTC): `2026-03-18T18:41:29+00:00` ## Tasks @@ -25,10 +25,11 @@ | 015 | DONE | 2026-03-13 | Add minimal OpenTelemetry tracing | `spec/tasks/015_add_minimal_opentelemetry_tracing.md` | | 016 | DONE | 2026-03-14 | Add CDEK order registration adapter | `spec/tasks/016_add_cdek_order_registration_adapter.md` | | 017 | DONE | 2026-03-14 | Return all tariffs from CDEK price calculation | `spec/tasks/017_return_all_cdek_tariffs.md` | -| 018 | TODO | 2026-03-14 | Add CDEK order creation endpoint | `spec/tasks/018_add_cdek_order_creation_endpoint.md` | +| 018 | TODO | 2026-03-16 | Add optional parcel type filter to price request | `spec/tasks/018_add_optional_parcel_type_filter_to_price_request.md` | +| 019 | TODO | 2026-03-14 | Add CDEK order creation endpoint | `spec/tasks/019_add_cdek_order_creation_endpoint.md` | ## Summary -- Total: **19** -- TODO: **1** +- Total: **20** +- TODO: **2** - DONE: **18** diff --git a/spec/overview.md b/spec/overview.md index ead467d..68c10d9 100644 --- a/spec/overview.md +++ b/spec/overview.md @@ -16,6 +16,7 @@ ## Продуктовые требования - Принимать запрос на расчёт стоимости доставки (откуда, куда, вес, габариты) +- Поддерживать необязательный параметр `parcel_type` в запросе расчёта стоимости доставки для фильтрации тарифов по типу отправления - Принимать запрос на создание заказа CDEK по контракту из `http-client.http` для сценария "доставка, до двери" - Опрашивать всех зарегистрированных провайдеров параллельно - Возвращать унифицированный список тарифов, отсортированных по цене @@ -59,6 +60,7 @@ ### Business Logic (`app/domain/`) - Правила сортировки и фильтрации тарифов +- Правила фильтрации тарифов по `parcel_type` - Логика сравнения цен - Правила применения конфигурируемого мультипликатора к ценам провайдеров - Округление цены после применения мультипликатора до целого значения по детерминированному правилу @@ -98,6 +100,7 @@ weight_kg: float length_cm: float width_cm: float height_cm: float +parcel_type: Literal[doc, parcel] | None ``` ### Выходная: `DeliveryPrice` diff --git a/spec/tasks/018_add_optional_parcel_type_filter_to_price_request.md b/spec/tasks/018_add_optional_parcel_type_filter_to_price_request.md new file mode 100644 index 0000000..e03d7f7 --- /dev/null +++ b/spec/tasks/018_add_optional_parcel_type_filter_to_price_request.md @@ -0,0 +1,49 @@ +--- +id: 018 +title: Add optional parcel type filter to price request +status: DONE +created: 2026-03-16 +--- + +## Context +Сейчас `POST /api/v1/delivery/price` возвращает все доступные тарифы без возможности отфильтровать их по типу отправления. Новый пользовательский сценарий требует опциональный параметр `parcel_type`, который должен фильтровать уже агрегированный список тарифов для всех провайдеров. + +## Goal +Расширить контракт `DeliveryRequest` опциональным полем `parcel_type` и добавить детерминированную фильтрацию тарифов в price flow по правилам `doc` и `parcel` без изменения публичного path endpoint. + +## Constraints +- `parcel_type` является необязательным полем запроса `POST /api/v1/delivery/price`. +- Допустимые значения ограничены `doc` и `parcel`; другие значения должны отклоняться schema validation. +- Правило фильтрации является pure business rule и должно жить в `app/domain/`; Controller только валидирует DTO, Service только оркестрирует и передаёт параметр в domain logic. +- Фильтрация должна применяться к унифицированному списку тарифов для всех провайдеров и не должна требовать provider-specific branching или изменения provider transport contract. +- Для `doc` включаются только тарифы, у которых `service_name` содержит `документ` или `document` без учёта регистра. +- Для `parcel` возвращаются все остальные тарифы, не попавшие под правило `doc`. +- Если `parcel_type` не передан, поведение endpoint остаётся прежним: возвращаются все тарифы. +- Scope задачи не включает изменение order flow, добавление новых endpoint'ов, расширение набора значений `parcel_type` и иные изменения вне price flow. +- Не изменять файлы в `spec/`. + +## Acceptance criteria +- `DeliveryRequest` поддерживает опциональное поле `parcel_type`. +- `POST /api/v1/delivery/price` принимает запросы без `parcel_type` и возвращает полный список тарифов без дополнительной фильтрации. +- При `parcel_type=doc` ответ содержит только тарифы, у которых `service_name` содержит `документ` или `document` без учёта регистра. +- При `parcel_type=parcel` ответ содержит только тарифы, у которых `service_name` не содержит `документ` и `document` без учёта регистра. +- Невалидное значение `parcel_type` приводит к 422 response на уровне schema validation. +- Фильтрация работает одинаково для тарифов, полученных из provider responses и из cache. + +## Definition of Done +- [ ] Обновлён контракт price request с опциональным `parcel_type`. +- [ ] Реализована pure domain logic для фильтрации тарифов по `parcel_type`. +- [ ] Service wiring передаёт `parcel_type` в domain logic без добавления business rules в Service. +- [ ] Controller/API tests покрывают сценарии `doc`, `parcel`, отсутствие параметра и невалидное значение. +- [ ] Поведение одинаково для fresh provider results и cache hit. + +## Tests +- Обновить `tests/domain/test_price.py` для проверки case-insensitive фильтрации по `документ` и `document`, а также поведения без `parcel_type`. +- Обновить `tests/services/test_aggregator.py` для проверки делегирования `parcel_type` в domain logic и одинакового результата для fresh path и cache hit. +- Обновить `tests/controllers/v1/test_delivery.py` для проверки optional request field, 422 на невалидное значение и response filtering для `doc` и `parcel`. + +## Commands +- `poetry run pytest tests/domain/test_price.py -q` +- `poetry run pytest tests/services/test_aggregator.py -q` +- `poetry run pytest tests/controllers/v1/test_delivery.py -q` +- `python3 spec/gen_spec_index.py --check` diff --git a/spec/tasks/018_add_cdek_order_creation_endpoint.md b/spec/tasks/019_add_cdek_order_creation_endpoint.md similarity index 99% rename from spec/tasks/018_add_cdek_order_creation_endpoint.md rename to spec/tasks/019_add_cdek_order_creation_endpoint.md index 09f18e6..294777f 100644 --- a/spec/tasks/018_add_cdek_order_creation_endpoint.md +++ b/spec/tasks/019_add_cdek_order_creation_endpoint.md @@ -1,5 +1,5 @@ --- -id: 018 +id: 019 title: Add CDEK order creation endpoint status: TODO created: 2026-03-14 diff --git a/tests/controllers/v1/test_delivery.py b/tests/controllers/v1/test_delivery.py index 98a5ab9..c77de5b 100644 --- a/tests/controllers/v1/test_delivery.py +++ b/tests/controllers/v1/test_delivery.py @@ -7,9 +7,10 @@ import pytest from app.controllers.v1 import delivery as delivery_controller from app.controllers.v1.delivery import get_aggregator_service from app.main import create_app -from app.schemas.request import DeliveryEntity, DeliveryRequest +from app.schemas.request import DeliveryEntity, DeliveryRequest, ParcelType from app.schemas.response import DeliveryPrice from app.services.aggregator import ( + AggregatorService, AggregatorServiceError, InvalidDeliveryRequestError, ) @@ -28,6 +29,18 @@ class StubAggregatorService: return self._response +class StubPriceProvider: + def __init__(self, response: list[DeliveryPrice]) -> None: + self.name = "stub-provider" + self.cache_ttl_seconds = 900 + self._response = response + self.calls: list[DeliveryRequest] = [] + + async def get_prices(self, request: DeliveryRequest) -> list[DeliveryPrice]: + self.calls.append(request) + return self._response + + def _install_service_override(app, service: StubAggregatorService) -> None: async def override_service() -> StubAggregatorService: return service @@ -47,6 +60,22 @@ def _valid_payload() -> dict[str, object]: } +def _make_price( + *, + service_name: str, + price: str, + provider: str = "cdek", +) -> DeliveryPrice: + return DeliveryPrice( + provider=provider, + service_name=service_name, + price=Decimal(price), + currency="RUB", + delivery_days_min=1, + delivery_days_max=3, + ) + + def test_post_delivery_price_uses_registered_provider_in_default_dependency( monkeypatch: pytest.MonkeyPatch, ) -> None: @@ -210,6 +239,7 @@ def test_post_delivery_price_returns_prices_and_delegates_to_service() -> None: length_cm=30.0, width_cm=20.0, height_cm=10.0, + parcel_type=None, ) @@ -235,6 +265,72 @@ def test_post_delivery_price_accepts_optional_country_code() -> None: assert service.calls[0].country_code == "kz" +def test_post_delivery_price_accepts_optional_parcel_type() -> None: + service = StubAggregatorService(response=[]) + app = create_app() + _install_service_override(app, service) + payload = _valid_payload() + payload["parcel_type"] = "doc" + + 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/price", json=payload) + + response = asyncio.run(run_request()) + + assert response.status_code == 200 + assert len(service.calls) == 1 + assert service.calls[0].parcel_type == ParcelType.DOC + + +@pytest.mark.parametrize( + ("parcel_type", "expected_service_names"), + [ + ("doc", ["Срочный документ", "DOCUMENT EXPRESS"]), + ("parcel", ["Parcel locker"]), + (None, ["Parcel locker", "Срочный документ", "DOCUMENT EXPRESS"]), + ], +) +def test_post_delivery_price_filters_response_by_optional_parcel_type( + parcel_type: str | None, + expected_service_names: list[str], +) -> None: + provider = StubPriceProvider( + response=[ + _make_price(service_name="Parcel locker", price="90.00"), + _make_price(service_name="Срочный документ", price="150.00"), + _make_price(service_name="DOCUMENT EXPRESS", price="200.00"), + ] + ) + service = AggregatorService(providers=[provider]) + app = create_app() + + async def override_service() -> AggregatorService: + return service + + app.dependency_overrides[get_aggregator_service] = override_service + payload = _valid_payload() + if parcel_type is not None: + payload["parcel_type"] = parcel_type + + 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/price", json=payload) + + response = asyncio.run(run_request()) + + assert response.status_code == 200 + assert [item["service_name"] for item in response.json()] == expected_service_names + + def test_post_delivery_price_rejects_invalid_payload() -> None: service = StubAggregatorService(response=[]) app = create_app() @@ -256,6 +352,28 @@ def test_post_delivery_price_rejects_invalid_payload() -> None: assert service.calls == [] +def test_post_delivery_price_rejects_invalid_parcel_type() -> None: + service = StubAggregatorService(response=[]) + app = create_app() + _install_service_override(app, service) + invalid_payload = _valid_payload() + invalid_payload["parcel_type"] = "letters" + + 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/price", json=invalid_payload) + + response = asyncio.run(run_request()) + + assert response.status_code == 422 + assert response.json()["detail"][0]["loc"] == ["body", "parcel_type"] + assert service.calls == [] + + def test_post_delivery_price_maps_service_exception_to_503() -> None: service = StubAggregatorService( response=[], diff --git a/tests/domain/test_price.py b/tests/domain/test_price.py index c4751e0..384824d 100644 --- a/tests/domain/test_price.py +++ b/tests/domain/test_price.py @@ -1,12 +1,13 @@ from decimal import Decimal from app.domain.price import ( + filter_prices_by_parcel_type, filter_and_sort_prices, filter_valid_prices, normalize_delivery_request, sort_prices_by_price, ) -from app.schemas.request import DeliveryEntity, DeliveryRequest +from app.schemas.request import DeliveryEntity, DeliveryRequest, ParcelType from app.schemas.response import DeliveryPrice @@ -80,6 +81,46 @@ def test_filter_valid_prices_handles_empty_input() -> None: assert filter_and_sort_prices([]) == [] +def test_filter_prices_by_parcel_type_matches_document_markers_case_insensitively() -> None: + document_ru = _make_price(service_name="Срочный Документ") + document_en = _make_price(service_name="DOCUMENT EXPRESS") + parcel = _make_price(service_name="Parcel locker") + + result = filter_prices_by_parcel_type( + [document_ru, parcel, document_en], + parcel_type=ParcelType.DOC, + ) + + assert [price.service_name for price in result] == [ + "Срочный Документ", + "DOCUMENT EXPRESS", + ] + + +def test_filter_prices_by_parcel_type_returns_non_document_tariffs_for_parcel() -> None: + document = _make_price(service_name="document delivery") + parcel = _make_price(service_name="Economy parcel") + standard = _make_price(service_name="Express") + + result = filter_prices_by_parcel_type( + [document, parcel, standard], + parcel_type=ParcelType.PARCEL, + ) + + assert [price.service_name for price in result] == ["Economy parcel", "Express"] + + +def test_filter_prices_by_parcel_type_returns_all_prices_when_type_is_missing() -> None: + prices = [ + _make_price(service_name="Документ"), + _make_price(service_name="Parcel"), + ] + + result = filter_prices_by_parcel_type(prices, parcel_type=None) + + assert result == prices + + def test_filter_valid_prices_applies_multiplier_and_rounds_half_up() -> None: low = _make_price(provider="cdek", price=Decimal("100.40")) high = _make_price(provider="boxberry", price=Decimal("100.50")) @@ -173,3 +214,20 @@ def test_filter_and_sort_prices_combines_domain_steps() -> None: assert [price.provider for price in result] == ["b", "a"] assert [price.price for price in result] == [Decimal("130"), Decimal("286")] + + +def test_filter_and_sort_prices_applies_parcel_type_filter_before_sorting() -> None: + document = _make_price(provider="a", price=Decimal("220.00"), service_name="Document") + parcel = _make_price(provider="b", price=Decimal("100.00"), service_name="Parcel") + document_ru = _make_price( + provider="c", + price=Decimal("150.00"), + service_name="Документ курьером", + ) + + result = filter_and_sort_prices( + [document, parcel, document_ru], + parcel_type=ParcelType.DOC, + ) + + assert [price.provider for price in result] == ["c", "a"] diff --git a/tests/services/test_aggregator.py b/tests/services/test_aggregator.py index fdb7fe4..89d2c21 100644 --- a/tests/services/test_aggregator.py +++ b/tests/services/test_aggregator.py @@ -4,7 +4,7 @@ from decimal import Decimal import pytest from app.adapters.delivery_providers.base import ProviderRequestError -from app.schemas.request import DeliveryEntity, DeliveryRequest +from app.schemas.request import DeliveryEntity, DeliveryRequest, ParcelType from app.schemas.response import DeliveryPrice from app.services.aggregator import AggregatorService, InvalidDeliveryRequestError @@ -68,6 +68,7 @@ def _make_request(**overrides: object) -> DeliveryRequest: "length_cm": 30.0, "width_cm": 20.0, "height_cm": 10.0, + "parcel_type": None, } payload.update(overrides) return DeliveryRequest(**payload) @@ -195,6 +196,55 @@ def test_get_all_prices_cache_hit_skips_provider_call() -> None: assert cache.set_calls == [] +@pytest.mark.parametrize( + ("cache", "expected_provider_calls"), + [ + (StubCache(), 1), + ( + StubCache( + forced_get_value=[ + _make_price( + "cdek", + "100.40", + service_name="DOCUMENT EXPRESS", + ).model_dump(mode="json"), + _make_price( + "cdek", + "200.40", + service_name="Economy parcel", + ).model_dump(mode="json"), + ] + ), + 0, + ), + ], +) +def test_get_all_prices_applies_same_parcel_type_filter_for_fresh_and_cached_results( + cache: StubCache, + expected_provider_calls: int, +) -> None: + provider = StubProvider( + name="cdek", + response=[ + _make_price("cdek", "100.40", service_name="DOCUMENT EXPRESS"), + _make_price("cdek", "200.40", service_name="Economy parcel"), + ], + ) + service = AggregatorService( + [provider], + cache=cache, + provider_price_multiplier=Decimal("1.1"), + ) + + result = asyncio.run( + service.get_all_prices(_make_request(parcel_type=ParcelType.DOC)) + ) + + assert [price.service_name for price in result] == ["DOCUMENT EXPRESS"] + assert [price.price for price in result] == [Decimal("110")] + assert len(provider.calls) == expected_provider_calls + + def test_get_all_prices_delegates_filtering_and_sorting_to_domain_logic() -> None: provider_a = StubProvider( name="a", @@ -206,11 +256,13 @@ def test_get_all_prices_delegates_filtering_and_sorting_to_domain_logic() -> Non provider_b = StubProvider(name="b", response=[_make_price("b", "100.00")]) delegated_inputs: list[list[DeliveryPrice]] = [] delegated_multipliers: list[Decimal] = [] + delegated_parcel_types: list[object | None] = [] - def fake_filter_and_sort(prices, *, price_multiplier): + def fake_filter_and_sort(prices, *, price_multiplier, parcel_type): price_list = list(prices) delegated_inputs.append(price_list) delegated_multipliers.append(price_multiplier) + delegated_parcel_types.append(parcel_type) return [price_list[0]] service = AggregatorService( @@ -220,7 +272,7 @@ def test_get_all_prices_delegates_filtering_and_sorting_to_domain_logic() -> Non filter_and_sort_prices_fn=fake_filter_and_sort, ) - result = asyncio.run(service.get_all_prices(_make_request())) + result = asyncio.run(service.get_all_prices(_make_request(parcel_type=ParcelType.PARCEL))) assert [price.provider for price in delegated_inputs[0]] == ["a", "a", "b"] assert [price.service_name for price in delegated_inputs[0]] == [ @@ -229,4 +281,5 @@ def test_get_all_prices_delegates_filtering_and_sorting_to_domain_logic() -> Non "standard", ] assert delegated_multipliers == [Decimal("1.23")] + assert delegated_parcel_types == [ParcelType.PARCEL] assert [price.provider for price in result] == ["a"]