diff --git a/oilpriceapi/async_client.py b/oilpriceapi/async_client.py index 58e6ee7..ee93289 100644 --- a/oilpriceapi/async_client.py +++ b/oilpriceapi/async_client.py @@ -9,7 +9,8 @@ import asyncio import logging import os -from typing import Any, AsyncGenerator, Dict, List, Optional, Union +from datetime import datetime +from typing import Any, AsyncGenerator, Dict, List, Optional, Union, cast from urllib.parse import urljoin import httpx @@ -249,7 +250,9 @@ async def request( await asyncio.sleep(wait_time) continue elif response.status_code >= 500: - if self._retry_strategy.should_retry(attempt, response.status_code, response.headers): + if self._retry_strategy.should_retry( + attempt, response.status_code, response.headers + ): wait_time = self._retry_strategy.calculate_wait_time(attempt) self._retry_strategy.log_retry( attempt, @@ -365,9 +368,48 @@ async def __aexit__(self, exc_type, exc_val, exc_tb): class AsyncPricesResource: """Async resource for current prices.""" + #: See PricesResource.MAX_CODES_PER_REQUEST — the API caps one request at + #: 20 codes and bills it once. Verified against production 2026-08-23. + MAX_CODES_PER_REQUEST = 20 + def __init__(self, client: AsyncOilPriceAPI): self.client = client + @staticmethod + def _to_price(price_data: Dict[str, Any], fallback_code: Optional[str] = None) -> Price: + """Map one API price row onto the Price model. + + Casts are for mypy; pydantic does the real validation. See + PricesResource._to_price. + """ + return Price( + commodity=cast(str, price_data.get("code", fallback_code)), + value=cast(float, price_data.get("price")), + currency=price_data.get("currency", "USD"), + unit=cast(str, price_data.get("unit", "barrel")), + timestamp=cast(datetime, price_data.get("created_at")), + ) + + async def _fetch_batch(self, codes: List[str]) -> List[Price]: + """One request for up to MAX_CODES_PER_REQUEST codes. + + One code returns a flat ``data`` object; two or more return + ``data.prices[]``. + """ + response = await self.client.request( + method="GET", + path="/v1/prices/latest", + params={"by_code": ",".join(codes)}, + ) + data = response.get("data", response) if isinstance(response, dict) else response + + if isinstance(data, dict) and "prices" in data: + rows = data["prices"] + else: + rows = [data] + + return [self._to_price(row, codes[0] if len(codes) == 1 else None) for row in rows] + async def get(self, commodity: str) -> Price: """Get current price for commodity.""" response = await self.client.request( @@ -409,20 +451,37 @@ async def get_multiple( OilPriceAPIError: If raise_on_error=True and any commodity fails """ - # Use gather for concurrent requests - tasks = [self.get(commodity) for commodity in commodities] - results = await asyncio.gather(*tasks, return_exceptions=True) + # api#7240: batch first, THEN fan out. Previously this issued one + # request per code concurrently, which spent quota per code and could + # trip the 60-per-60s rate limit instantly on a long list. Chunks of 20 + # are still gathered concurrently, so 25 codes cost 2 requests, not 25. + chunks = [ + commodities[i : i + self.MAX_CODES_PER_REQUEST] + for i in range(0, len(commodities), self.MAX_CODES_PER_REQUEST) + ] + results = await asyncio.gather( + *(self._fetch_batch(chunk) for chunk in chunks), return_exceptions=True + ) prices = [] failures = [] - for commodity, result in zip(commodities, results): - if isinstance(result, Price): - prices.append(result) - elif isinstance(result, Exception): - if raise_on_error: - raise result - failures.append((commodity, str(result))) + for chunk, result in zip(chunks, results): + if isinstance(result, list): + prices.extend(result) + continue + if raise_on_error: + raise result + # The API rejects the WHOLE request when any code in it is unknown, + # so retry this chunk per code to say which one was at fault. + retry = await asyncio.gather( + *(self.get(code) for code in chunk), return_exceptions=True + ) + for code, item in zip(chunk, retry): + if isinstance(item, Price): + prices.append(item) + else: + failures.append((code, str(item))) if return_failures: return prices, failures diff --git a/oilpriceapi/resources/prices.py b/oilpriceapi/resources/prices.py index 1cd3db9..cc8ed60 100644 --- a/oilpriceapi/resources/prices.py +++ b/oilpriceapi/resources/prices.py @@ -7,7 +7,7 @@ from __future__ import annotations from datetime import datetime -from typing import List, Optional, Set, Tuple, Union +from typing import Any, Dict, List, Optional, Set, Tuple, Union, cast from .._pagination import validate_page_size from ..models import Price @@ -16,9 +16,58 @@ class PricesResource: """Resource for current price operations.""" + #: The API accepts at most this many commodity codes in one request, and + #: that request counts ONCE against quota. Verified against production + #: 2026-08-23: 20 codes -> 200, 21 -> 400 "Too many commodity codes + #: requested (max: 20, requested: 21)". Raising this without a + #: corresponding API change turns every call into a 400. + MAX_CODES_PER_REQUEST = 20 + def __init__(self, client): self.client = client + @staticmethod + def _to_price(price_data: Dict[str, Any], fallback_code: Optional[str] = None) -> Price: + """Map one API price row onto the Price model. + + Shared by get() and the batched path so the two cannot drift. + + Values are passed to a pydantic model, which does the coercion and + raises on anything genuinely wrong. The casts here are for mypy: the + JSON payload is Dict[str, Any], and the previous Price(**mapped_data) + form simply hid that from the type checker. + """ + return Price( + commodity=cast(str, price_data.get("code", fallback_code)), + value=cast(float, price_data.get("price")), + currency=price_data.get("currency"), + # Retain the established oil-only fallback for legacy minimal + # responses; any unit actually supplied by the API wins. + unit=cast(str, price_data.get("unit", "barrel")), + timestamp=cast(datetime, price_data.get("created_at")), + ) + + def _fetch_batch(self, codes: List[str]) -> List[Price]: + """One request for up to MAX_CODES_PER_REQUEST codes. + + A single code returns a flat ``data`` object; two or more return + ``data.prices[]``. Both shapes are handled here so callers do not + have to care how many codes they asked for. + """ + response = self.client.request( + method="GET", + path="/v1/prices/latest", + params={"by_code": ",".join(codes)}, + ) + data = response.get("data", response) if isinstance(response, dict) else response + + if isinstance(data, dict) and "prices" in data: + rows = data["prices"] + else: + rows = [data] + + return [self._to_price(row, codes[0] if len(codes) == 1 else None) for row in rows] + def get(self, commodity: str) -> Price: """Get current price for a single commodity. @@ -36,24 +85,8 @@ def get(self, commodity: str) -> Price: method="GET", path="/v1/prices/latest", params={"by_code": commodity} ) - # Parse response - if "data" in response: - price_data = response["data"] - else: - price_data = response - - # Map API response to Price model without inventing source context. - mapped_data = { - "commodity": price_data.get("code", commodity), - "value": price_data.get("price"), - "currency": price_data.get("currency"), - # Retain the established oil-only fallback for legacy minimal - # responses; any unit actually supplied by the API wins. - "unit": price_data.get("unit", "barrel"), - "timestamp": price_data.get("created_at"), - } - - return Price(**mapped_data) + price_data = response["data"] if "data" in response else response + return self._to_price(price_data, commodity) def get_multiple( self, commodities: List[str], raise_on_error: bool = False, return_failures: bool = False @@ -93,15 +126,29 @@ def get_multiple( prices = [] failures = [] - for commodity in commodities: + for start in range(0, len(commodities), self.MAX_CODES_PER_REQUEST): + chunk = commodities[start : start + self.MAX_CODES_PER_REQUEST] try: - price = self.get(commodity) - prices.append(price) - except OilPriceAPIError as e: + prices.extend(self._fetch_batch(chunk)) + except OilPriceAPIError: if raise_on_error: raise - failures.append((commodity, str(e))) - continue + # The API rejects the WHOLE request when any code in it is + # unknown, so a chunk failure does not say which code was at + # fault. Retry just this chunk per code to preserve the + # per-code failure contract. A one-code chunk has nothing to + # narrow down, so it is recorded directly rather than refetched. + if len(chunk) == 1: + try: + prices.append(self.get(chunk[0])) + except OilPriceAPIError as exc: + failures.append((chunk[0], str(exc))) + continue + for commodity in chunk: + try: + prices.append(self.get(commodity)) + except OilPriceAPIError as exc: + failures.append((commodity, str(exc))) if return_failures: return prices, failures diff --git a/tests/test_client.py b/tests/test_client.py index f88b973..8df76dc 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -122,15 +122,38 @@ def create_response(code, price_value): } return response - # Set up side_effect to return different responses - mock_request.side_effect = [ - create_response("BRENT_CRUDE_USD", 75.50), - create_response("WTI_USD", 70.25), - ] + # api#7240: get_multiple now BATCHES — two codes are one request that + # returns data.prices[], not two requests each returning a flat object. + batch = Mock() + batch.status_code = 200 + batch.json.return_value = { + "status": "success", + "data": { + "prices": [ + { + "code": "BRENT_CRUDE_USD", + "price": 75.50, + "currency": "USD", + "created_at": "2024-01-15T10:00:00Z", + "type": "spot_price", + }, + { + "code": "WTI_USD", + "price": 70.25, + "currency": "USD", + "created_at": "2024-01-15T10:00:00Z", + "type": "spot_price", + }, + ] + }, + } + mock_request.return_value = batch client = OilPriceAPI(api_key="test_key") prices = client.prices.get_multiple(["BRENT_CRUDE_USD", "WTI_USD"]) + assert mock_request.call_count == 1, "two codes must cost ONE request" + assert len(prices) == 2 assert prices[0].commodity == "BRENT_CRUDE_USD" assert prices[0].value == 75.50 diff --git a/tests/unit/test_prices_resource.py b/tests/unit/test_prices_resource.py index 4e3bbf5..34af36a 100644 --- a/tests/unit/test_prices_resource.py +++ b/tests/unit/test_prices_resource.py @@ -37,37 +37,36 @@ def test_get_single_price(self, mock_request, api_key, mock_price_response): @patch("httpx.Client.request") def test_get_multiple_prices(self, mock_request, api_key, mock_price_response): - """Test getting multiple commodity prices.""" - # Mock responses for each commodity - responses = [ - Mock( - status_code=200, - json=lambda: { - "status": "success", - "data": { - "code": "BRENT_CRUDE_USD", - "price": 75.50, - "currency": "USD", - "created_at": "2024-01-15T10:00:00Z", - "type": "spot_price", - }, - }, - ), - Mock( - status_code=200, - json=lambda: { - "status": "success", - "data": { - "code": "WTI_USD", - "price": 70.25, - "currency": "USD", - "created_at": "2024-01-15T10:00:00Z", - "type": "spot_price", - }, + """Two codes are ONE batched request (api#7240). + + This test previously asserted call_count == 2, encoding the defect: + get_multiple looped over get() and spent quota per code. The API + accepts 20 codes in one request that counts once. + """ + mock_request.return_value = Mock( + status_code=200, + json=lambda: { + "status": "success", + "data": { + "prices": [ + { + "code": "BRENT_CRUDE_USD", + "price": 75.50, + "currency": "USD", + "created_at": "2024-01-15T10:00:00Z", + "type": "spot_price", + }, + { + "code": "WTI_USD", + "price": 70.25, + "currency": "USD", + "created_at": "2024-01-15T10:00:00Z", + "type": "spot_price", + }, + ] }, - ), - ] - mock_request.side_effect = responses + }, + ) client = OilPriceAPI(api_key=api_key) prices = client.prices.get_multiple(["BRENT_CRUDE_USD", "WTI_USD"]) @@ -77,7 +76,7 @@ def test_get_multiple_prices(self, mock_request, api_key, mock_price_response): assert prices[0].value == 75.50 assert prices[1].commodity == "WTI_USD" assert prices[1].value == 70.25 - assert mock_request.call_count == 2 + assert mock_request.call_count == 1, "two codes must cost ONE request" @patch("httpx.Client.request") def test_get_multiple_prices_with_failures(self, mock_request, api_key): @@ -112,12 +111,18 @@ def test_get_multiple_prices_with_failures(self, mock_request, api_key): }, ), ] - mock_request.side_effect = responses + # api#7240: the batch is tried first. The API rejects the WHOLE request + # when any code is unknown, so the chunk is retried per code — which is + # what the three per-code responses above now serve. + mock_request.side_effect = [ + Mock(status_code=400, json=lambda: {"status": "fail", "data": {"error": "invalid_code"}}), + *responses, + ] client = OilPriceAPI(api_key=api_key) prices = client.prices.get_multiple(["BRENT_CRUDE_USD", "INVALID_CODE", "NATURAL_GAS_USD"]) - # Should only return 2 prices (skips the failed one) + # Still returns the good ones and skips the failure — contract preserved assert len(prices) == 2 assert prices[0].commodity == "BRENT_CRUDE_USD" assert prices[1].commodity == "NATURAL_GAS_USD" @@ -414,3 +419,120 @@ def test_get_price_does_not_invent_missing_currency(self, mock_request, api_key) assert price.value == 100.0 assert price.currency is None assert price.unit == "index_points" + + +class TestGetMultipleBatching: + """api#7240 — get_multiple must batch, not loop. + + The REST API accepts up to 20 codes in ONE request that counts ONCE against + quota (verified against production 2026-08-23: 20 codes -> 200, 21 -> 400 + "Too many commodity codes requested (max: 20, requested: 21)"). + + Looping made the SDK cost up to 20x more quota than writing the call by + hand, which is why these tests assert REQUEST COUNT, not just results. + """ + + @staticmethod + def _batch_response(codes): + return Mock( + status_code=200, + json=lambda: { + "status": "success", + "data": { + "prices": [ + { + "code": c, + "price": 10.0 + i, + "currency": "USD", + "created_at": "2026-08-23T10:00:00Z", + "type": "spot_price", + } + for i, c in enumerate(codes) + ] + }, + }, + ) + + @patch("httpx.Client.request") + def test_three_codes_cost_one_request(self, mock_request, api_key): + codes = ["BRENT_CRUDE_USD", "WTI_USD", "NATURAL_GAS_USD"] + mock_request.return_value = self._batch_response(codes) + + client = OilPriceAPI(api_key=api_key) + prices = client.prices.get_multiple(codes) + + assert mock_request.call_count == 1, "3 codes must be ONE request, not three" + assert [p.commodity for p in prices] == codes + sent = mock_request.call_args.kwargs["params"]["by_code"] + assert sent == "BRENT_CRUDE_USD,WTI_USD,NATURAL_GAS_USD" + + @patch("httpx.Client.request") + def test_twenty_five_codes_cost_two_requests(self, mock_request, api_key): + codes = [f"CODE_{i}_USD" for i in range(25)] + mock_request.side_effect = [ + self._batch_response(codes[:20]), + self._batch_response(codes[20:]), + ] + + client = OilPriceAPI(api_key=api_key) + prices = client.prices.get_multiple(codes) + + assert mock_request.call_count == 2, "25 codes must be ceil(25/20) = 2 requests" + assert len(prices) == 25 + + @patch("httpx.Client.request") + def test_single_code_still_works(self, mock_request, api_key): + """One code returns a FLAT data object, not data.prices[].""" + mock_request.return_value = Mock( + status_code=200, + json=lambda: { + "status": "success", + "data": { + "code": "BRENT_CRUDE_USD", + "price": 75.5, + "currency": "USD", + "created_at": "2026-08-23T10:00:00Z", + "type": "spot_price", + }, + }, + ) + client = OilPriceAPI(api_key=api_key) + prices = client.prices.get_multiple(["BRENT_CRUDE_USD"]) + + assert mock_request.call_count == 1 + assert len(prices) == 1 + assert prices[0].value == 75.5 + + @patch("httpx.Client.request") + def test_bad_code_in_batch_falls_back_and_reports_per_code(self, mock_request, api_key): + """A 400 on the batch must not lose the good codes. + + The API rejects the whole request when any code is unknown, so the + batch is retried per code — for that chunk only — to preserve the + per-code failure contract. + """ + good = { + "status": "success", + "data": { + "code": "BRENT_CRUDE_USD", + "price": 75.5, + "currency": "USD", + "created_at": "2026-08-23T10:00:00Z", + "type": "spot_price", + }, + } + mock_request.side_effect = [ + Mock(status_code=400, json=lambda: {"status": "fail", "data": {"error": "invalid_code"}}), + Mock(status_code=200, json=lambda: good), + Mock(status_code=400, json=lambda: {"status": "fail", "data": {"error": "invalid_code"}}), + ] + + client = OilPriceAPI(api_key=api_key) + prices, failures = client.prices.get_multiple( + ["BRENT_CRUDE_USD", "NOT_A_REAL_CODE"], return_failures=True + ) + + assert len(prices) == 1 + assert prices[0].commodity == "BRENT_CRUDE_USD" + assert len(failures) == 1 + assert failures[0][0] == "NOT_A_REAL_CODE"