diff --git a/app/adapters/delivery_providers/base.py b/app/adapters/delivery_providers/base.py index 2e783f0..d1ffa88 100644 --- a/app/adapters/delivery_providers/base.py +++ b/app/adapters/delivery_providers/base.py @@ -14,3 +14,7 @@ class DeliveryProvider(ABC): """Fetch one quote from an external provider.""" raise NotImplementedError + + +class ProviderRequestError(RuntimeError): + """Raised when a provider rejects input request data.""" diff --git a/app/adapters/delivery_providers/cdek/client.py b/app/adapters/delivery_providers/cdek/client.py index cf907fb..f6e13cc 100644 --- a/app/adapters/delivery_providers/cdek/client.py +++ b/app/adapters/delivery_providers/cdek/client.py @@ -1,23 +1,38 @@ """CDEK HTTP client and provider adapter.""" import asyncio +import sys from collections.abc import Awaitable, Callable from typing import Any import httpx -from app.adapters.delivery_providers.base import DeliveryProvider +from app.adapters.delivery_providers.base import DeliveryProvider, ProviderRequestError from app.adapters.delivery_providers.cdek.auth import CDEKAuthClient from app.adapters.delivery_providers.cdek.mapper import map_cdek_response from app.config import AdapterConfig from app.schemas.request import DeliveryRequest from app.schemas.response import DeliveryPrice +import logging + + +logging.basicConfig( + level=logging.INFO, + format="%(asctime)s [%(levelname)s] %(name)s: %(message)s", + handlers=[logging.StreamHandler(sys.stdout)] # важно: stdout, не stderr +) +log = logging.getLogger(__name__) +log.info("test") class CDEKClientError(RuntimeError): """Raised when CDEK tariff request fails.""" +class CDEKRequestError(CDEKClientError, ProviderRequestError): + """Raised when CDEK rejects request data as invalid.""" + + class CDEKClient: def __init__( self, @@ -133,6 +148,7 @@ class CDEKClient: raise CDEKClientError( f"CDEK city lookup failed with status {response.status_code}." ) + log.info(response.json()) try: response.raise_for_status() body = response.json() @@ -143,7 +159,9 @@ class CDEKClient: raise CDEKClientError("CDEK city lookup failed unexpectedly.") if not isinstance(body, list) or not body: - raise CDEKClientError(f"CDEK city lookup returned no matches for '{city}'.") + raise CDEKRequestError( + f"CDEK city lookup returned no matches for '{city}'." + ) first_item = body[0] if not isinstance(first_item, dict): raise CDEKClientError("CDEK city lookup response entry must be an object.") diff --git a/app/controllers/v1/delivery.py b/app/controllers/v1/delivery.py index f69c4ef..4cdc886 100644 --- a/app/controllers/v1/delivery.py +++ b/app/controllers/v1/delivery.py @@ -8,7 +8,11 @@ from app.controllers.http_client import build_controller_http_client from app.repositories.cache.redis_cache import PriceCache from app.schemas.request import DeliveryRequest from app.schemas.response import DeliveryPrice -from app.services.aggregator import AggregatorService, AggregatorServiceError +from app.services.aggregator import ( + AggregatorService, + AggregatorServiceError, + InvalidDeliveryRequestError, +) router = APIRouter(prefix="/delivery", tags=["delivery"]) @@ -56,6 +60,14 @@ async def get_delivery_price( ) -> list[DeliveryPrice]: try: return await service.get_all_prices(delivery_request) + except InvalidDeliveryRequestError as exc: + raise HTTPException( + status_code=status.HTTP_400_BAD_REQUEST, + detail={ + "code": "invalid_delivery_request", + "message": "Delivery request contains unknown or unsupported location.", + }, + ) from exc except AggregatorServiceError as exc: raise HTTPException( status_code=status.HTTP_503_SERVICE_UNAVAILABLE, diff --git a/app/services/aggregator.py b/app/services/aggregator.py index 90e836c..78888c9 100644 --- a/app/services/aggregator.py +++ b/app/services/aggregator.py @@ -6,7 +6,7 @@ import json from collections.abc import Callable, Iterable, Sequence from typing import Protocol -from app.adapters.delivery_providers.base import DeliveryProvider +from app.adapters.delivery_providers.base import DeliveryProvider, ProviderRequestError from app.domain.price import ( DEFAULT_WEIGHT_ROUND_SCALE, NormalizedDeliveryRequest, @@ -21,6 +21,10 @@ class AggregatorServiceError(RuntimeError): """Base exception for AggregatorService failures.""" +class InvalidDeliveryRequestError(AggregatorServiceError): + """Raised when provider rejects delivery request as invalid.""" + + class PriceCacheProtocol(Protocol): async def get(self, key: str) -> object | None: ... @@ -69,6 +73,19 @@ class AggregatorService: for price in provider_results if isinstance(price, DeliveryPrice) ] + provider_errors = [ + error + for error in provider_results + if isinstance(error, Exception) + ] + if ( + not successful_results + and provider_errors + and any(isinstance(error, ProviderRequestError) for error in provider_errors) + ): + raise InvalidDeliveryRequestError( + "Delivery request is invalid for configured providers." + ) filtered_and_sorted = self._filter_and_sort_prices(successful_results) return [self._coerce_delivery_price(price) for price in filtered_and_sorted] diff --git a/tests/adapters/delivery_providers/cdek/test_client.py b/tests/adapters/delivery_providers/cdek/test_client.py index 6ca238b..790ebe7 100644 --- a/tests/adapters/delivery_providers/cdek/test_client.py +++ b/tests/adapters/delivery_providers/cdek/test_client.py @@ -8,6 +8,7 @@ from app.adapters.delivery_providers.cdek.client import ( CDEKClient, CDEKClientError, CDEKProvider, + CDEKRequestError, ) from app.config import AdapterConfig, Settings from app.schemas.request import DeliveryEntity, DeliveryRequest @@ -254,7 +255,7 @@ def test_cdek_client_raises_when_city_code_not_found() -> None: retry_attempts=0, ) - with pytest.raises(CDEKClientError, match="no matches"): + with pytest.raises(CDEKRequestError, match="no matches"): asyncio.run(client.get_raw_price(request)) diff --git a/tests/controllers/v1/test_delivery.py b/tests/controllers/v1/test_delivery.py index 18862b5..47ffa2f 100644 --- a/tests/controllers/v1/test_delivery.py +++ b/tests/controllers/v1/test_delivery.py @@ -9,7 +9,10 @@ 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.response import DeliveryPrice -from app.services.aggregator import AggregatorServiceError +from app.services.aggregator import ( + AggregatorServiceError, + InvalidDeliveryRequestError, +) class StubAggregatorService: @@ -229,6 +232,33 @@ def test_post_delivery_price_maps_service_exception_to_503() -> None: } +def test_post_delivery_price_maps_invalid_request_to_400() -> None: + service = StubAggregatorService( + response=[], + error=InvalidDeliveryRequestError("city not found"), + ) + app = create_app() + _install_service_override(app, service) + + async def run_request() -> httpx.Response: + transport = httpx.ASGITransport(app=app, raise_app_exceptions=False) + async with httpx.AsyncClient( + transport=transport, + base_url="http://testserver", + ) as client: + return await client.post("/api/v1/delivery/price", json=_valid_payload()) + + response = asyncio.run(run_request()) + + assert response.status_code == 400 + assert response.json() == { + "detail": { + "code": "invalid_delivery_request", + "message": "Delivery request contains unknown or unsupported location.", + } + } + + def test_delivery_price_endpoint_accepts_only_post_method() -> None: service = StubAggregatorService(response=[]) app = create_app() diff --git a/tests/services/test_aggregator.py b/tests/services/test_aggregator.py index 3e7274c..9d68b3d 100644 --- a/tests/services/test_aggregator.py +++ b/tests/services/test_aggregator.py @@ -1,9 +1,12 @@ import asyncio from decimal import Decimal +import pytest + +from app.adapters.delivery_providers.base import ProviderRequestError from app.schemas.request import DeliveryEntity, DeliveryRequest from app.schemas.response import DeliveryPrice -from app.services.aggregator import AggregatorService +from app.services.aggregator import AggregatorService, InvalidDeliveryRequestError _MISSING = object() @@ -117,6 +120,19 @@ def test_get_all_prices_all_failures_returns_empty_list() -> None: assert len(provider_b.calls) == 1 +def test_get_all_prices_raises_invalid_request_for_provider_request_errors() -> None: + provider = StubProvider( + name="cdek", + error=ProviderRequestError("city not found"), + ) + service = AggregatorService([provider], cache=StubCache()) + + with pytest.raises(InvalidDeliveryRequestError): + asyncio.run(service.get_all_prices(_make_request())) + + assert len(provider.calls) == 1 + + def test_get_all_prices_cache_hit_skips_provider_call() -> None: cached_payload = _make_price("cdek", "99.00").model_dump(mode="json") cache = StubCache(forced_get_value=cached_payload) @@ -152,4 +168,3 @@ def test_get_all_prices_delegates_filtering_and_sorting_to_domain_logic() -> Non assert [price.provider for price in delegated_inputs[0]] == ["a", "b"] assert [price.provider for price in result] == ["a"] -