From 5f54eb7cc2d585e0aaa5d6b655a155f8dc85b295 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 13:05:16 -0400 Subject: [PATCH 1/2] chore: release as 3.0.0 and write down the BC break #18 shipped `Client::VERSION` still read '2.1.2' and the changelog entry carried no version heading at all, while `Price::fromArray()` - public, documented, and callable directly - went from total to partial in #18. An integrator reading either signal would conclude nothing in their code could break. - `Client::VERSION` is 3.0.0. The change shipped to main labelled as a patch; this corrects the label rather than re-releasing the code. - CHANGELOG gets a version heading and a `### Breaking changes` section: a table of exactly what throws now that did not before, what is still preserved (legitimate zero and negative prices, an empty `prices` list, a row with no timestamp), and the code a caller writes to adapt. - README gets an "Upgrading to 3.0" section making the same point where a caller will actually see it. - `VersioningTest` fences all of it: the version is semver, its major is past the last BC-compatible line, the topmost changelog entry names a version and matches `Client::VERSION`, that entry documents the break and what it throws, and the User-Agent carries the same version it claims. PHPStan is deliberately NOT wired in here. Level 9 reports 17 errors on src/, and six of them are the `(string)` casts being removed in the #21 PR while an eleventh is in the transport being rewritten in the #22 PR. Adding it now means either a baseline that conflicts with both, or fixing Client.php in a PR about version numbers. Inventory posted to #24 instead, to land after those two. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- CHANGELOG.md | 45 +++++++++++- README.md | 27 +++++++ src/Client.php | 2 +- tests/PublicClaimsTest.php | 2 +- tests/VersioningTest.php | 142 +++++++++++++++++++++++++++++++++++++ 5 files changed, 215 insertions(+), 3 deletions(-) create mode 100644 tests/VersioningTest.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 56147ab..1d782e6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,6 +1,49 @@ # Changelog -## Unreleased +## 3.0.0 (unreleased) + +### Breaking changes + +- `Price::fromArray()` is no longer total. It is public, documented, and + callable directly, and it now throws `ApiException` for payloads the 2.x + line accepted silently. + + **What throws now that did not before:** + + | Row | 2.x behaviour | 3.0 behaviour | + | --- | --- | --- | + | no `code`, or a blank or non-string `code` | `code` became `''` | `ApiException` | + | no `price`, or a non-numeric `price` | `price` became `0.0` | `ApiException` | + | an unparseable `created_at`/`updated_at` | a date was invented | `ApiException` | + + A legitimate zero or negative price is still preserved; a genuinely empty + `prices` list still returns an empty array; a row with no timestamp field at + all is still valid and leaves `updatedAt` null. + + **What callers should do:** the methods on `Client` (`latest()`, the + historical period methods, `demoPrices()`) already surfaced malformed data + as `ApiException`, so a caller that catches `ApiException` around API calls + needs no change. A caller that invokes `Price::fromArray()` on its own + payloads must now wrap it: + + ```php + try { + $price = \OilPriceAPI\Price::fromArray($row); + } catch (\OilPriceAPI\Exception\ApiException $e) { + // The row was malformed. Previously you received a Price carrying + // manufactured values - $0.00, an empty code, or an invented date. + // Handle or skip the row; do not retry, the payload will not change. + } + ``` + + A caller that relied on the manufactured values - reading `$price->price` + as `0.0` to mean "no data", say - must switch to catching the exception. + There is no opt-out, by design: the manufactured values were + indistinguishable from real quotes once they left the SDK. + +- The version jumps 2.1.2 to 3.0.0 with no 2.2.0 in between. The change above + shipped to `main` labelled as a patch; this corrects the label rather than + re-releasing the code. ### Security diff --git a/README.md b/README.md index d78fd13..2ad2344 100644 --- a/README.md +++ b/README.md @@ -193,6 +193,33 @@ Availability varies by dataset, plan, source, and account entitlement. Review [current access](https://www.oilpriceapi.com/pricing) rather than relying on a plan claim copied into package metadata. +## Upgrading to 3.0 + +`Price::fromArray()` is public and documented, and in 3.0 it throws +`ApiException` for payloads 2.x accepted silently: a missing, blank or +non-string `code`, a missing or non-numeric `price`, and an unparseable +timestamp. 2.x manufactured a value in each case — `$0.00`, an empty code, an +invented date — and a manufactured value is indistinguishable from a real +quote once it leaves the SDK. + +If you call `Client` methods (`latest()`, the historical period methods, +`demoPrices()`) and already catch `ApiException`, nothing changes: those +methods surfaced malformed data as `ApiException` before. If you call +`Price::fromArray()` on your own payloads, wrap it: + +```php +try { + $price = \OilPriceAPI\Price::fromArray($row); +} catch (\OilPriceAPI\Exception\ApiException $e) { + // Malformed row. Previously you got a Price carrying manufactured values. + // Handle or skip it; retrying will not change the payload. +} +``` + +A legitimate zero or negative price is still preserved, an empty `prices` list +still returns an empty array, and a row with no timestamp is still valid. See +the CHANGELOG for the full table. + ## Reviewed Product Facts The versioned, reviewed contract is diff --git a/src/Client.php b/src/Client.php index dff4edc..f81be4a 100644 --- a/src/Client.php +++ b/src/Client.php @@ -28,7 +28,7 @@ */ final class Client { - public const VERSION = '2.1.2'; + public const VERSION = '3.0.0'; public const DEFAULT_BASE_URL = 'https://api.oilpriceapi.com'; public const DEFAULT_TIMEOUT = 10.0; public const DEFAULT_MAX_RETRIES = 3; diff --git a/tests/PublicClaimsTest.php b/tests/PublicClaimsTest.php index dcb117a..fe943b7 100644 --- a/tests/PublicClaimsTest.php +++ b/tests/PublicClaimsTest.php @@ -312,7 +312,7 @@ public function testCanonicalDeveloperContractIsDiscoverable(): void ); self::assertSame('oilpriceapi/oilpriceapi', $composer['name']); self::assertSame('>=8.1', $composer['require']['php']); - self::assertSame('2.1.2', Client::VERSION); + self::assertSame('3.0.0', Client::VERSION); self::assertSame('https://api.oilpriceapi.com', Client::DEFAULT_BASE_URL); $readme = (string) file_get_contents($root . '/README.md'); diff --git a/tests/VersioningTest.php b/tests/VersioningTest.php new file mode 100644 index 0000000..07f3abd --- /dev/null +++ b/tests/VersioningTest.php @@ -0,0 +1,142 @@ +assertMatchesRegularExpression( + '/^\d+\.\d+\.\d+$/', + Client::VERSION, + 'Client::VERSION must be a plain MAJOR.MINOR.PATCH string.', + ); + } + + /** + * `Price::fromArray()` throws for input the 2.x line accepted. That is not + * a patch. + */ + public function testVersionRecordsTheBreakingChangeAsAMajor(): void + { + [$major] = array_map('intval', explode('.', Client::VERSION)); + + $this->assertGreaterThan( + self::LAST_BC_COMPATIBLE_MAJOR, + $major, + sprintf( + 'Price::fromArray() went from total to partial, so %s is not a valid version ' + . 'for this line: a caller reading it would expect no breakage.', + Client::VERSION, + ), + ); + } + + /** + * The topmost changelog entry must name a version. "## Unreleased" tells a + * reader nothing about whether upgrading is safe. + */ + public function testTopmostChangelogEntryCarriesAVersionHeading(): void + { + $heading = self::firstChangelogHeading(); + + $this->assertMatchesRegularExpression( + '/^\d+\.\d+\.\d+/', + $heading, + sprintf('The topmost CHANGELOG entry is "## %s" and names no version.', $heading), + ); + } + + public function testTopmostChangelogEntryMatchesTheShippedVersion(): void + { + $this->assertStringStartsWith( + Client::VERSION, + self::firstChangelogHeading(), + 'The CHANGELOG and Client::VERSION disagree about what is shipping.', + ); + } + + /** + * The break has to be written down in words a caller can act on: what + * throws now that did not before. + */ + public function testChangelogDocumentsTheBreakingChange(): void + { + $entry = self::firstChangelogEntry(); + + $this->assertMatchesRegularExpression( + '/###\s+Breaking changes/i', + $entry, + 'The major entry has no "### Breaking changes" section.', + ); + $this->assertStringContainsString( + 'Price::fromArray', + $entry, + 'The breaking-change note does not name the method that broke.', + ); + $this->assertMatchesRegularExpression( + '/ApiException/', + $entry, + 'The breaking-change note does not say what is thrown.', + ); + } + + /** + * The version a customer sees in our access logs must be the version we + * think we shipped. + */ + public function testUserAgentCarriesTheSameVersion(): void + { + $transport = new MockTransport(); + $transport->queue(200, [ + 'status' => 'success', + 'data' => ['code' => 'BRENT_CRUDE_USD', 'price' => 71.8, 'currency' => 'USD'], + ]); + $client = new Client('fixture_key_NOT_REAL_0123456789', Client::DEFAULT_BASE_URL, 10.0, 0, $transport); + + $client->latest('BRENT_CRUDE_USD'); + + $this->assertSame( + 'oilpriceapi-php/' . Client::VERSION, + $transport->requests[0]['headers']['User-Agent'], + ); + } + + private static function changelog(): string + { + return (string) file_get_contents(dirname(__DIR__) . '/CHANGELOG.md'); + } + + private static function firstChangelogHeading(): string + { + preg_match('/^## (.+)$/m', self::changelog(), $matches); + self::assertNotEmpty($matches, 'CHANGELOG.md has no "## " heading.'); + + return trim($matches[1]); + } + + private static function firstChangelogEntry(): string + { + $parts = preg_split('/^## /m', self::changelog()); + self::assertIsArray($parts); + self::assertArrayHasKey(1, $parts, 'CHANGELOG.md has no entries.'); + + return $parts[1]; + } +} From 1575edafb74a18a226fc95adc977e5ad41881d04 Mon Sep 17 00:00:00 2001 From: Karl Waldman Date: Sun, 13 Sep 2026 13:15:36 -0400 Subject: [PATCH 2/2] docs: list every 3.0.0 break under Breaking changes, not just the first The section documented only the #18 break. A reader deciding whether it is safe to upgrade reads "### Breaking changes" and nothing else, so a break recorded only under "Fixed" or "Security" is one they meet in production. Added to the table: `currency` is now required, a non-string `currency`, and a non-string `unit`/`name`/`source`/`type`/`formatted` (#21). Split the single "unparseable timestamp" row into the two distinct cases #20 introduced - a value PHP repaired into a plausible date, and a relative expression such as `now` - because #18 already rejected the genuinely unparseable and conflating them understates what changed. Added as their own entries: redirects are no longer followed (#22), and the new `lib-curl >= 7.58.0` platform requirement, which is an install-time break on an old host and appears nowhere else a reader would look. Called out the two rejections a reader would not predict: leap seconds, and naive timestamps read as UTC rather than host-local. Extended the "what still works" paragraph to cover absent and null label fields, so a reader can tell the optional fields stayed optional. VersioningTest now fences all of it: eight required breaks, two surprising rejections, and the still-works guarantees. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_015ao5paex73xXvuM424Libo --- CHANGELOG.md | 30 +++++++++++++-- tests/VersioningTest.php | 81 ++++++++++++++++++++++++++++++++++++++++ 2 files changed, 107 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1d782e6..56c8bd8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -14,11 +14,22 @@ | --- | --- | --- | | no `code`, or a blank or non-string `code` | `code` became `''` | `ApiException` | | no `price`, or a non-numeric `price` | `price` became `0.0` | `ApiException` | - | an unparseable `created_at`/`updated_at` | a date was invented | `ApiException` | + | no `currency` | `currency` became `'USD'` | `ApiException` | + | a non-string `currency` | `['EUR']` became `'Array'`, `978` became `'978'` | `ApiException` | + | a non-string `unit`, `name`, `source`, `type` or `formatted` | cast, so `true` became `'1'` | `ApiException` | + | a `created_at`/`updated_at` PHP had to repair | `2026-13-45T99:99:99Z` became `2027-02-18T04:40:39Z` | `ApiException` | + | a relative `created_at`/`updated_at` | `now`, `next friday`, `+1 week` all became dates | `ApiException` | A legitimate zero or negative price is still preserved; a genuinely empty `prices` list still returns an empty array; a row with no timestamp field at - all is still valid and leaves `updatedAt` null. + all is still valid and leaves `updatedAt` null; and an absent or explicitly + null `unit`, `name`, `source`, `type` or `formatted` still means "not + provided" rather than an error. + + Two deliberate calls inside the timestamp rule, called out because they + reject input a reader might expect to pass: a leap second (`23:59:60`) is + rejected rather than rolled silently into the next minute, and a naive + timestamp is read as UTC rather than as the host's local timezone. **What callers should do:** the methods on `Client` (`latest()`, the historical period methods, `demoPrices()`) already surfaced malformed data @@ -41,8 +52,19 @@ There is no opt-out, by design: the manufactured values were indistinguishable from real quotes once they left the SDK. -- The version jumps 2.1.2 to 3.0.0 with no 2.2.0 in between. The change above - shipped to `main` labelled as a patch; this corrects the label rather than +- **Redirects are no longer followed.** A `$baseUrl` whose host answers with a + 3xx now raises `TransportException` instead of being followed silently. + `https://api.oilpriceapi.com` does not redirect, so this affects only a + custom base URL; point it at the final URL. Details under Security below. + +- **New platform requirement: `lib-curl >= 7.58.0`.** `composer.json` accepted + any libcurl. 7.58.0 is where libcurl began stripping `Authorization` across + an origin change, which the SDK depended on without saying so. Composer will + now refuse to install on an older host rather than leaving the credential + exposed. + +- The version jumps 2.1.2 to 3.0.0 with no 2.2.0 in between. The changes above + shipped to `main` labelled as patches; this corrects the label rather than re-releasing the code. ### Security diff --git a/tests/VersioningTest.php b/tests/VersioningTest.php index 07f3abd..463dc18 100644 --- a/tests/VersioningTest.php +++ b/tests/VersioningTest.php @@ -5,6 +5,7 @@ namespace OilPriceAPI\Tests; use OilPriceAPI\Client; +use PHPUnit\Framework\Attributes\DataProvider; use PHPUnit\Framework\TestCase; /** @@ -97,6 +98,71 @@ public function testChangelogDocumentsTheBreakingChange(): void ); } + /** + * The breaking-change section must enumerate EVERY break in the release, + * not just the first one found. A reader checking whether it is safe to + * upgrade reads this section and nothing else; a break documented only + * under "Fixed" or "Security" is a break they will meet in production. + * + * @return array + */ + public static function breaksThatMustBeListed(): array + { + return [ + 'fromArray is partial' => ['Price::fromArray'], + 'currency is required' => ['no `currency`'], + 'non-string currency' => ['a non-string `currency`'], + 'non-string label fields' => ['non-string `unit`'], + 'repaired timestamps' => ['PHP had to repair'], + 'relative timestamps' => ['a relative `created_at`'], + 'redirects not followed' => ['Redirects are no longer followed'], + 'libcurl floor' => ['lib-curl >= 7.58.0'], + ]; + } + + #[DataProvider('breaksThatMustBeListed')] + public function testEveryBreakIsListedUnderBreakingChanges(string $needle): void + { + $this->assertStringContainsString( + $needle, + self::breakingChangesSection(), + sprintf('"%s" is not documented under "### Breaking changes".', $needle), + ); + } + + /** + * The rejections a reader would not predict have to be named, or they + * arrive as a surprise on a row that looks fine. + * + * @return array + */ + public static function surprisingRejections(): array + { + return [ + 'leap second' => ['leap second'], + 'naive timestamp timezone' => ['read as UTC'], + ]; + } + + #[DataProvider('surprisingRejections')] + public function testSurprisingRejectionsAreCalledOut(string $needle): void + { + $this->assertStringContainsString($needle, self::breakingChangesSection()); + } + + /** + * What stays working matters as much as what breaks: without it a reader + * cannot tell whether a legitimate zero price still survives. + */ + public function testBreakingChangesSectionSaysWhatStillWorks(): void + { + $section = self::breakingChangesSection(); + + $this->assertMatchesRegularExpression('/zero or negative price is still preserved/', $section); + $this->assertMatchesRegularExpression('/empty\s+`prices`\s+list still returns an empty array/', $section); + $this->assertStringContainsString('no timestamp field', $section); + } + /** * The version a customer sees in our access logs must be the version we * think we shipped. @@ -118,6 +184,21 @@ public function testUserAgentCarriesTheSameVersion(): void ); } + /** + * The text between "### Breaking changes" and the next "###" heading. + */ + private static function breakingChangesSection(): string + { + $entry = self::firstChangelogEntry(); + $start = strpos($entry, '### Breaking changes'); + self::assertIsInt($start, 'The topmost entry has no "### Breaking changes" section.'); + + $rest = substr($entry, $start + strlen('### Breaking changes')); + $next = strpos($rest, '###'); + + return $next === false ? $rest : substr($rest, 0, $next); + } + private static function changelog(): string { return (string) file_get_contents(dirname(__DIR__) . '/CHANGELOG.md');