From 13ade364432510fc8d87f6d64c92ce4d78b6b373 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 11:57:56 -0400 Subject: [PATCH] fix(diesel): unwrap the API envelope before building DieselPrice (#110) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit client.diesel.get_price() raised pydantic ValidationError with 7 errors on every call. GET /v1/diesel-prices?state=CA returns {"status": "success", "data": {"regional_average": {...}, "sources": {...}, "upgrade": {...}, "location": {"type": "state", "state_code": "CA"}}} The resource looked for `regional_average` at the TOP level, missed it because it is nested under `data`, fell through to the `"data" in response` branch and handed the whole `data` object to DieselPrice(**...). Two fixes: * unwrap `data` first, then take `regional_average` — for both get_price() and get_stations() * `regional_average` carries `region` ("california"), not the `state` the model requires, so fill `state` from the envelope's location.state_code, falling back to the state the caller asked for Verified live against production on 2026-09-13 with the smoke-test key: client.diesel.get_price("CA") now returns state='CA' price=8.136 currency='USD' unit='gallon' granularity='state' source='aaa' cached=True, matching the raw client.request() form exactly. get_stations() is a POST, so it is verified by signature and fixtures only — not called against production. The older top-level and flat shapes still parse, so the existing test_diesel_resource.py fixtures are untouched and still green. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- oilpriceapi/resources/diesel.py | 61 ++++++--- tests/unit/test_diesel_envelope.py | 196 +++++++++++++++++++++++++++++ 2 files changed, 239 insertions(+), 18 deletions(-) create mode 100644 tests/unit/test_diesel_envelope.py diff --git a/oilpriceapi/resources/diesel.py b/oilpriceapi/resources/diesel.py index db0108d..dbef03a 100644 --- a/oilpriceapi/resources/diesel.py +++ b/oilpriceapi/resources/diesel.py @@ -10,17 +10,29 @@ from ..models import DieselPrice, DieselStationsResponse +def _payload(response): + """Step inside the API's ``{"status": ..., "data": {...}}`` envelope. + + ``/v1/diesel-prices`` nests everything under ``data``. Responses that are + already unwrapped (older shapes, hand-built fixtures) pass straight through, + so both forms keep working. + """ + if isinstance(response, dict) and isinstance(response.get("data"), dict): + return response["data"] + return response + + class DieselResource: """Resource for diesel price operations. Provides access to state-level diesel price averages and station-level pricing. Example: - >>> # Get the available state average + >>> # Get the available state average >>> price = client.diesel.get_price("CA") >>> print(f"California diesel: ${price.price:.2f}/gallon") - >>> # Get nearby stations when enabled for the current account + >>> # Get nearby stations when enabled for the current account >>> result = client.diesel.get_stations(lat=37.7749, lng=-122.4194) >>> print(f"Found {len(result.stations)} stations") """ @@ -36,8 +48,8 @@ def __init__(self, client): def get_price(self, state: str) -> DieselPrice: """Get average diesel price for a US state. - Returns the available EIA state-level average diesel price. Access and - request limits follow the account's current entitlement and API metadata. + Returns the available EIA state-level average diesel price. Access and + request limits follow the account's current entitlement and API metadata. Args: state: Two-letter US state code (e.g., "CA", "TX", "NY") @@ -85,13 +97,25 @@ def get_price(self, state: str) -> DieselPrice: params={"state": state.upper()} ) - # Parse response - API returns { regional_average: {...} } - if "regional_average" in response: - price_data = response["regional_average"] - elif "data" in response: - price_data = response["data"] + # Production returns {"status": ..., "data": {"regional_average": {...}, + # "location": {...}, ...}}. The record is one level deeper than the old + # code looked, so it fell through and handed the whole envelope to the + # model (#110). Unwrap `data` first, then take `regional_average`. + payload = _payload(response) + + if isinstance(payload, dict) and isinstance(payload.get("regional_average"), dict): + price_data = dict(payload["regional_average"]) + location = payload.get("location") else: - price_data = response + price_data = dict(payload) if isinstance(payload, dict) else payload + location = None + + # `regional_average` carries `region` ("california"), not `state`, which + # the model requires. The envelope's location block has the code; fall + # back to what the caller asked for. + if isinstance(price_data, dict) and not price_data.get("state"): + state_code = location.get("state_code") if isinstance(location, dict) else None + price_data["state"] = state_code or state.upper() return DieselPrice(**price_data) @@ -105,11 +129,11 @@ def get_stations( Returns station-level diesel prices within specified radius using Google Maps data. - Station-level access and allowances depend on the account's current - entitlement. Review https://www.oilpriceapi.com/pricing and the API's - response metadata instead of relying on SDK-bundled limits. - - Use the returned source timestamp to apply the application's freshness policy. + Station-level access and allowances depend on the account's current + entitlement. Review https://www.oilpriceapi.com/pricing and the API's + response metadata instead of relying on SDK-bundled limits. + + Use the returned source timestamp to apply the application's freshness policy. Args: lat: Latitude (-90 to 90) @@ -122,8 +146,8 @@ def get_stations( Raises: ValidationError: If coordinates or radius are invalid AuthenticationError: If API key is invalid - RateLimitError: If the API reports the request limit exceeded (429) - OilPriceAPIError: If the account cannot access station queries (403) + RateLimitError: If the API reports the request limit exceeded (429) + OilPriceAPIError: If the account cannot access station queries (403) Example: >>> # Get stations near San Francisco @@ -202,7 +226,8 @@ def get_stations( } ) - return DieselStationsResponse(**response) + # Same envelope as get_price: the stations block lives under `data`. + return DieselStationsResponse(**_payload(response)) def to_dataframe( self, diff --git a/tests/unit/test_diesel_envelope.py b/tests/unit/test_diesel_envelope.py new file mode 100644 index 0000000..1392f32 --- /dev/null +++ b/tests/unit/test_diesel_envelope.py @@ -0,0 +1,196 @@ +"""client.diesel.* must parse the envelope production actually returns (#110). + +The fixtures below are the real response bodies. The state-average one was +captured live from GET /v1/diesel-prices?state=CA on 2026-09-13; the station one +mirrors the documented POST /v1/diesel-prices/stations envelope, verified by +signature only because it is a POST. + +The existing tests in test_diesel_resource.py mock a *top-level* +`regional_average`, a shape the API does not return — which is exactly how this +shipped. Those shapes stay supported; these add the shape customers hit. +""" + +from unittest.mock import Mock, patch + +import pytest + +from oilpriceapi import OilPriceAPI +from oilpriceapi.models import DieselPrice, DieselStationsResponse + +# Not a credential: a fixture string, every request here is mocked. +FIXTURE_KEY = "-".join(["fixture", "not", "a", "real", "key"]) + +# Captured live from production on 2026-09-13. +LIVE_STATE_ENVELOPE = { + "status": "success", + "data": { + "regional_average": { + "price": 8.136, + "currency": "USD", + "unit": "gallon", + "region": "california", + "granularity": "state", + "source": "aaa", + "updated_at": "2026-09-13T14:57:19Z", + "cached": True, + }, + "sources": {"primary": "aaa"}, + "upgrade": {"message": "Station-level pricing available on higher plans"}, + "location": {"type": "state", "state_code": "CA"}, + }, +} + +LIVE_STATIONS_ENVELOPE = { + "status": "success", + "data": { + "regional_average": { + "price": 8.136, + "currency": "USD", + "unit": "gallon", + "region": "california", + "granularity": "state", + "source": "aaa", + "updated_at": "2026-09-13T14:57:19Z", + }, + "stations": [ + { + "name": "Fixture Truck Stop", + "address": "1 Fixture Way, San Francisco, CA", + "location": {"lat": 37.7749, "lng": -122.4194}, + "diesel_price": 7.99, + "formatted_price": "$7.99", + "currency": "USD", + "unit": "gallon", + "price_delta": -0.146, + "price_vs_average": "$0.15 below average", + } + ], + "search_area": { + "center": {"lat": 37.7749, "lng": -122.4194}, + "radius_meters": 8047, + "radius_miles": 5.0, + }, + "metadata": { + "total_stations": 1, + "source": "google_maps", + "cached": False, + "api_cost": 0.024, + "timestamp": "2026-09-13T14:57:19Z", + }, + }, +} + + +def _mock_client(mock_request, payload): + response = Mock() + response.status_code = 200 + response.json.return_value = payload + mock_request.return_value = response + return OilPriceAPI(api_key=FIXTURE_KEY) + + +@patch("httpx.Client.request") +def test_get_price_parses_the_live_production_envelope(mock_request): + client = _mock_client(mock_request, LIVE_STATE_ENVELOPE) + + price = client.diesel.get_price("CA") + + assert isinstance(price, DieselPrice) + assert price.price == 8.136 + assert price.currency == "USD" + assert price.unit == "gallon" + assert price.granularity == "state" + assert price.source == "aaa" + assert price.cached is True + assert price.updated_at.year == 2026 + + +@patch("httpx.Client.request") +def test_get_price_fills_state_from_the_envelope_location(mock_request): + """`regional_average` carries `region`, not `state` — the code must not drop it.""" + client = _mock_client(mock_request, LIVE_STATE_ENVELOPE) + + assert client.diesel.get_price("ca").state == "CA" + + +@patch("httpx.Client.request") +def test_get_price_falls_back_to_the_requested_state_when_location_is_absent(mock_request): + payload = { + "status": "success", + "data": { + "regional_average": dict(LIVE_STATE_ENVELOPE["data"]["regional_average"]), + }, + } + client = _mock_client(mock_request, payload) + + assert client.diesel.get_price("tx").state == "TX" + + +@patch("httpx.Client.request") +def test_get_price_still_accepts_the_legacy_top_level_shape(mock_request): + payload = { + "regional_average": { + "state": "CA", + "price": 3.89, + "currency": "USD", + "unit": "gallon", + "granularity": "state", + "source": "EIA", + "updated_at": "2025-12-15T10:00:00Z", + } + } + client = _mock_client(mock_request, payload) + + price = client.diesel.get_price("CA") + assert price.price == 3.89 + assert price.state == "CA" + + +@patch("httpx.Client.request") +def test_get_price_still_accepts_a_flat_record(mock_request): + payload = { + "state": "NY", + "price": 4.21, + "currency": "USD", + "unit": "gallon", + "granularity": "state", + "source": "EIA", + "updated_at": "2025-12-15T10:00:00Z", + } + client = _mock_client(mock_request, payload) + + assert client.diesel.get_price("NY").price == 4.21 + + +@patch("httpx.Client.request") +def test_get_stations_parses_the_data_envelope(mock_request): + client = _mock_client(mock_request, LIVE_STATIONS_ENVELOPE) + + result = client.diesel.get_stations(lat=37.7749, lng=-122.4194, radius=8047) + + assert isinstance(result, DieselStationsResponse) + assert result.regional_average.price == 8.136 + assert len(result.stations) == 1 + assert result.stations[0].diesel_price == 7.99 + assert result.search_area.radius_miles == 5.0 + assert result.metadata.total_stations == 1 + + +@patch("httpx.Client.request") +def test_get_stations_still_accepts_the_legacy_top_level_shape(mock_request): + payload = dict(LIVE_STATIONS_ENVELOPE["data"]) + client = _mock_client(mock_request, payload) + + result = client.diesel.get_stations(lat=37.7749, lng=-122.4194) + assert len(result.stations) == 1 + + +@patch("httpx.Client.request") +def test_to_dataframe_state_works_on_the_live_envelope(mock_request): + pytest.importorskip("pandas") + client = _mock_client(mock_request, LIVE_STATE_ENVELOPE) + + df = client.diesel.to_dataframe(state="CA") + + assert df["price"].iloc[0] == 8.136 + assert df["state"].iloc[0] == "CA"