diff --git a/CHANGELOG.md b/CHANGELOG.md index 69b6c4b..a7da1d8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -13,6 +13,15 @@ origin is compared against the configured base URL before the credential is attached. Explicit custom `$baseUrl` values are unaffected. +### Fixed + +- Reject malformed price rows instead of reporting them as `$0.00`. A row + without a usable code or a numeric price, an unparseable timestamp, and a + missing or non-array `prices` field now raise `ApiException` across + `latest()`, the historical period methods and `demoPrices()`. Legitimate + zero and negative prices are preserved, and a genuinely empty `prices` list + still returns an empty array. + ## 2.1.2 (2026-08-11) ### Fixed diff --git a/src/Client.php b/src/Client.php index ff0785e..265095e 100644 --- a/src/Client.php +++ b/src/Client.php @@ -105,12 +105,12 @@ public function latest(?string $byCode = null): Price|array } return array_map( - fn (mixed $price): Price => $this->latestPriceOrFail($price, $body), + fn (mixed $price): Price => $this->priceOrFail($price, $body, '/v1/prices/latest'), array_values($data['prices']), ); } - return $this->latestPriceOrFail($data, $body); + return $this->priceOrFail($data, $body, '/v1/prices/latest'); } /** @@ -162,9 +162,8 @@ public function demoPrices(): array { $body = $this->request('/v1/demo/prices', []); $data = $this->dataOrFail($body, '/v1/demo/prices'); - $prices = is_array($data['prices'] ?? null) ? $data['prices'] : []; - return array_map(Price::fromArray(...), array_values($prices)); + return $this->priceListOrFail($data, $body, '/v1/demo/prices'); } /** @@ -183,11 +182,11 @@ public function raw(): RawClient private function historical(string $period, ?string $byCode): array { $params = $byCode !== null ? ['by_code' => $byCode] : []; - $body = $this->request('/v1/prices/' . $period, $params); - $data = $this->dataOrFail($body, '/v1/prices/' . $period); - $prices = is_array($data['prices'] ?? null) ? $data['prices'] : []; + $path = '/v1/prices/' . $period; + $body = $this->request($path, $params); + $data = $this->dataOrFail($body, $path); - return array_map(Price::fromArray(...), array_values($prices)); + return $this->priceListOrFail($data, $body, $path); } /** @@ -209,23 +208,58 @@ private function dataOrFail(array $body, string $path): array } /** - * @param mixed $data + * Decode a list-returning endpoint. + * + * A missing or non-array `prices` field is a malformed envelope, not an + * empty result: returning [] for it reports "no data" for a response the + * SDK simply failed to understand. An actual empty list stays empty. + * + * @param array $data * @param array $body + * + * @return list */ - private function latestPriceOrFail(mixed $data, array $body): Price + private function priceListOrFail(array $data, array $body, string $path): array + { + if (!array_key_exists('prices', $data) || !is_array($data['prices'])) { + throw new ApiException( + sprintf('Unexpected response shape from %s: no price list in the envelope.', $path), + 200, + $body, + ); + } + + return array_map( + fn (mixed $row): Price => $this->priceOrFail($row, $body, $path), + array_values($data['prices']), + ); + } + + /** + * The single validated price-row boundary shared by latest, history and demo. + * + * @param array $body + */ + private function priceOrFail(mixed $row, array $body, string $path): Price + { + if (is_array($row)) { + try { + return Price::fromArray($row); + } catch (ApiException $e) { + throw new ApiException($this->malformedRowMessage($path, $e->getMessage()), 200, $body); + } + } + + throw new ApiException($this->malformedRowMessage($path, 'Price row is not an object.'), 200, $body); + } + + private function malformedRowMessage(string $path, string $detail): string { - if ( - !is_array($data) - || !isset($data['code']) - || !is_string($data['code']) - || trim($data['code']) === '' - || !array_key_exists('price', $data) - || !is_numeric($data['price']) - ) { - throw new ApiException('Unexpected latest price shape from /v1/prices/latest.', 200, $body); + if ($path === '/v1/prices/latest') { + return 'Unexpected latest price shape from /v1/prices/latest. ' . $detail; } - return Price::fromArray($data); + return sprintf('Unexpected price row in the response from %s. %s', $path, $detail); } /** diff --git a/src/Price.php b/src/Price.php index 8134e6c..b1b1e79 100644 --- a/src/Price.php +++ b/src/Price.php @@ -6,6 +6,7 @@ use DateTimeImmutable; use DateTimeInterface; +use OilPriceAPI\Exception\ApiException; /** * Immutable price data transfer object. @@ -41,10 +42,32 @@ public function __construct( * `created_at`/`updated_at` for the timestamp and `change_24h`/ * `change_percent_24h` for the 24h change. * + * A row that does not carry a usable code and a numeric price is rejected + * rather than defaulted: a manufactured $0.00 is indistinguishable from a + * real quote once it leaves the SDK. Legitimate zero and negative prices + * are preserved. + * * @param array $data + * + * @throws ApiException when a required field is missing or unparseable */ public static function fromArray(array $data): self { + if ( + !isset($data['code']) + || !is_string($data['code']) + || trim($data['code']) === '' + ) { + throw new ApiException('Price row is missing a usable commodity code.'); + } + + if (!array_key_exists('price', $data) || !is_numeric($data['price'])) { + throw new ApiException(sprintf( + 'Price row for %s is missing a numeric price; refusing to report it as 0.', + $data['code'], + )); + } + $timestamp = $data['created_at'] ?? $data['updated_at'] ?? null; $updatedAt = null; if (is_string($timestamp) && $timestamp !== '') { @@ -56,14 +79,20 @@ public static function fromArray(array $data): self $parsed = null; } } - $updatedAt = $parsed ?: null; + if (!$parsed instanceof DateTimeImmutable) { + throw new ApiException(sprintf( + 'Price row for %s carries an unparseable timestamp.', + $data['code'], + )); + } + $updatedAt = $parsed; } $change = $data['change_24h'] ?? $data['change_percent_24h'] ?? null; return new self( - code: (string) ($data['code'] ?? ''), - price: (float) ($data['price'] ?? 0.0), + code: $data['code'], + price: (float) $data['price'], currency: (string) ($data['currency'] ?? 'USD'), updatedAt: $updatedAt, change24h: is_numeric($change) ? (float) $change : null, diff --git a/tests/MalformedPriceRowTest.php b/tests/MalformedPriceRowTest.php new file mode 100644 index 0000000..9082474 --- /dev/null +++ b/tests/MalformedPriceRowTest.php @@ -0,0 +1,168 @@ +transport = new MockTransport(); + } + + private function client(?string $key = 'fixture_key_NOT_REAL'): Client + { + return new Client($key, 'https://api.oilpriceapi.com', 10.0, 0, $this->transport); + } + + /** + * @return array}> + */ + public static function malformedRows(): array + { + return [ + 'empty row' => [[]], + 'missing price' => [['code' => 'BRENT_CRUDE_USD']], + 'null price' => [['code' => 'BRENT_CRUDE_USD', 'price' => null]], + 'non-numeric price' => [['code' => 'BRENT_CRUDE_USD', 'price' => 'not-a-number']], + 'empty string price' => [['code' => 'BRENT_CRUDE_USD', 'price' => '']], + 'array price' => [['code' => 'BRENT_CRUDE_USD', 'price' => ['value' => 70.0]]], + 'missing code' => [['price' => 70.0]], + 'empty code' => [['code' => ' ', 'price' => 70.0]], + 'non-string code' => [['code' => 123, 'price' => 70.0]], + 'malformed timestamp' => [['code' => 'BRENT_CRUDE_USD', 'price' => 70.0, 'created_at' => 'not-a-date']], + ]; + } + + /** + * @param array $row + */ + #[DataProvider('malformedRows')] + public function testHistoricalRejectsMalformedRow(array $row): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['prices' => [$row]]]); + + $this->expectException(ApiException::class); + $this->client()->pastDay('BRENT_CRUDE_USD'); + } + + /** + * @param array $row + */ + #[DataProvider('malformedRows')] + public function testDemoRejectsMalformedRow(array $row): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['prices' => [$row]]]); + + $this->expectException(ApiException::class); + $this->client(null)->demoPrices(); + } + + /** + * @param array $row + */ + #[DataProvider('malformedRows')] + public function testLatestRejectsMalformedRow(array $row): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => $row]); + + $this->expectException(ApiException::class); + $this->client()->latest('BRENT_CRUDE_USD'); + } + + public function testHistoricalNeverManufacturesAZeroPrice(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['prices' => [new \stdClass()]]]); + + try { + $prices = $this->client()->pastDay('BRENT_CRUDE_USD'); + $this->fail(sprintf( + 'An unparseable price row was returned as data: %s', + json_encode(array_map(static fn (Price $p): array => $p->toArray(), $prices)), + )); + } catch (ApiException $e) { + $this->assertStringContainsString('/v1/prices/past_day', $e->getMessage()); + } + } + + public function testNonArrayPricesFieldIsNotAnEmptySuccessfulList(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['prices' => 'garbage']]); + + $this->expectException(ApiException::class); + $this->client()->pastWeek('BRENT_CRUDE_USD'); + } + + public function testMissingPricesFieldIsNotAnEmptySuccessfulList(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['note' => 'no prices key']]); + + $this->expectException(ApiException::class); + $this->client()->pastMonth('BRENT_CRUDE_USD'); + } + + public function testMissingPricesFieldOnDemoIsNotAnEmptySuccessfulList(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['note' => 'no prices key']]); + + $this->expectException(ApiException::class); + $this->client(null)->demoPrices(); + } + + public function testLegitimatelyEmptyListIsStillAnEmptyList(): void + { + $this->transport->queue(200, ['status' => 'success', 'data' => ['prices' => []]]); + + $this->assertSame([], $this->client()->pastYear('BRENT_CRUDE_USD')); + } + + /** + * @return array + */ + public static function legitimateNumericPrices(): array + { + return [ + 'zero' => [0], + 'zero float' => [0.0], + 'zero string' => ['0.00'], + 'negative' => [-37.63], + 'numeric string' => ['71.80'], + ]; + } + + #[DataProvider('legitimateNumericPrices')] + public function testLegitimateNumericPricesArePreserved(float|int|string $price): void + { + $this->transport->queue(200, [ + 'status' => 'success', + 'data' => ['prices' => [['code' => 'WTI_USD', 'price' => $price, 'created_at' => '2026-04-20T00:00:00Z']]], + ]); + + $prices = $this->client()->pastDay('WTI_USD'); + + $this->assertCount(1, $prices); + $this->assertSame((float) $price, $prices[0]->price); + } + + public function testPriceFromArrayDoesNotInventAZeroPrice(): void + { + $this->expectException(ApiException::class); + Price::fromArray(['code' => 'BRENT_CRUDE_USD']); + } +}