From fcea7ff3a8352bd966aec6b881c53044e478c863 Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 17:25:38 +0200 Subject: [PATCH] Let the catalogue ask Stripe what products it already has The price half of this question was answered; the product half was left open and named as a known gap. It is the more consequential one: a second product makes every activePricesFor() ask about the wrong one, so price recognition goes blind for that whole family and every orphan on the first product is unreachable. The whole active list comes back with no metadata filter, because Stripe cannot search metadata and an account holds a handful of products. Deciding which are ours belongs to the next commit. Co-Authored-By: Claude Opus 5 --- app/Services/Stripe/FakeStripeClient.php | 36 +++++++++++++- app/Services/Stripe/HttpStripeClient.php | 45 ++++++++++++++++++ app/Services/Stripe/StripeClient.php | 17 +++++++ .../Billing/StripeIdempotencyKeyTest.php | 47 +++++++++++++++++++ 4 files changed, 143 insertions(+), 2 deletions(-) diff --git a/app/Services/Stripe/FakeStripeClient.php b/app/Services/Stripe/FakeStripeClient.php index 1db81fa..8de2c57 100644 --- a/app/Services/Stripe/FakeStripeClient.php +++ b/app/Services/Stripe/FakeStripeClient.php @@ -16,7 +16,7 @@ class FakeStripeClient implements StripeClient { public bool $configured = true; - /** @var array */ + /** @var array */ public array $products = []; /** @var array */ @@ -184,7 +184,14 @@ class FakeStripeClient implements StripeClient } $id = 'prod_'.substr(sha1($name.count($this->products)), 0, 12); - $this->products[$id] = ['name' => $name, 'metadata' => $metadata]; + $this->products[$id] = [ + 'name' => $name, + 'metadata' => $metadata, + // Stripe stamps a creation time and AdoptStripeProduct takes the + // OLDEST of several. A counter cannot drift the way a clock in a + // test can. + 'created' => count($this->products) + 1, + ]; $this->rememberKey($key, $id, $parameters); @@ -294,6 +301,31 @@ class FakeStripeClient implements StripeClient return $found; } + public function activeProducts(): array + { + $found = []; + + foreach ($this->products as $id => $product) { + $found[] = [ + 'id' => $id, + 'name' => $product['name'], + 'created' => $product['created'], + 'metadata' => array_map(fn ($value) => (string) $value, $product['metadata']), + ]; + } + + return $found; + } + + /** + * A Product that exists at Stripe and in no row of ours — the orphan a run + * leaves when it dies between Stripe's create and our storing its id. + */ + public function plantProduct(string $id, string $name, array $metadata = [], int $created = 1): void + { + $this->products[$id] = ['name' => $name, 'metadata' => $metadata, 'created' => $created]; + } + public function updatePriceMetadata(string $priceId, array $metadata): void { if (isset($this->prices[$priceId])) { diff --git a/app/Services/Stripe/HttpStripeClient.php b/app/Services/Stripe/HttpStripeClient.php index dc08835..b562a7a 100644 --- a/app/Services/Stripe/HttpStripeClient.php +++ b/app/Services/Stripe/HttpStripeClient.php @@ -212,6 +212,51 @@ class HttpStripeClient implements StripeClient return $prices; } + public function activeProducts(): array + { + $products = []; + $after = null; + + do { + $page = $this->request() + ->get($this->url('products'), array_filter([ + 'active' => 'true', + 'limit' => 100, + 'starting_after' => $after, + ], fn ($value) => $value !== null)) + ->throw() + ->json(); + + $data = (array) ($page['data'] ?? []); + + foreach ($data as $product) { + $products[] = [ + 'id' => (string) ($product['id'] ?? ''), + 'name' => (string) ($product['name'] ?? ''), + 'created' => (int) ($product['created'] ?? 0), + 'metadata' => array_map( + fn ($value) => (string) $value, + (array) ($product['metadata'] ?? []), + ), + ]; + } + + $after = $data === [] ? null : ($data[array_key_last($data)]['id'] ?? null); + + // Same reason as activePricesFor(): a half-read list here means a + // Product we own goes unrecognised, and the next run mints a second + // one — which blinds the Price recognition for the whole family. + if (($page['has_more'] ?? false) === true && $after === null) { + throw new RuntimeException( + 'Stripe reported another page of products and the last item of this one carried no id; ' + .'refusing to stop half-read.' + ); + } + } while (($page['has_more'] ?? false) === true); + + return $products; + } + public function updatePriceMetadata(string $priceId, array $metadata): void { $this->request() diff --git a/app/Services/Stripe/StripeClient.php b/app/Services/Stripe/StripeClient.php index ab7908d..57195cb 100644 --- a/app/Services/Stripe/StripeClient.php +++ b/app/Services/Stripe/StripeClient.php @@ -152,6 +152,23 @@ interface StripeClient */ public function activePricesFor(string $productId): array; + /** + * Every active Product on the account, with its metadata. + * + * The other half of the orphan question, and the more consequential one. A + * run that created a Product and died before storing its id leaves an orphan + * exactly as a Price does — but a second Product is worse than a duplicate + * Price, because activePricesFor() is then asked about the wrong one and the + * Price recognition goes blind for that whole family. + * + * The WHOLE active list, with no metadata filter: Stripe cannot search + * metadata, and an account holds a handful of Products. Which of them is + * ours is App\Services\Billing\AdoptStripeProduct's decision. + * + * @return array}> + */ + public function activeProducts(): array; + /** * Write metadata onto a Price that already exists. * diff --git a/tests/Feature/Billing/StripeIdempotencyKeyTest.php b/tests/Feature/Billing/StripeIdempotencyKeyTest.php index 1f8a9df..db5d53d 100644 --- a/tests/Feature/Billing/StripeIdempotencyKeyTest.php +++ b/tests/Feature/Billing/StripeIdempotencyKeyTest.php @@ -280,3 +280,50 @@ it('refuses to stop half-read when Stripe says there is another page', function expect(fn () => (new HttpStripeClient)->activePricesFor('prod_1')) ->toThrow(RuntimeException::class, 'half-read'); }); + +it('pages through every active product of the account', function () { + Http::fake([ + 'api.stripe.com/*' => Http::sequence() + ->push([ + 'data' => [ + ['id' => 'prod_a', 'name' => 'Team', 'created' => 100, + 'metadata' => ['plan_family' => 'team', 'plan_family_id' => '3']], + ['id' => 'prod_b', 'name' => 'Priority Support', 'created' => 101, + 'metadata' => ['addon' => 'priority_support']], + ], + 'has_more' => true, + ]) + ->push([ + 'data' => [['id' => 'prod_c', 'name' => 'Etwas anderes', 'created' => 102, 'metadata' => []]], + 'has_more' => false, + ]), + ]); + + $products = (new HttpStripeClient)->activeProducts(); + + expect($products)->toHaveCount(3) + ->and($products[0])->toBe([ + 'id' => 'prod_a', + 'name' => 'Team', + 'created' => 100, + 'metadata' => ['plan_family' => 'team', 'plan_family_id' => '3'], + ]) + ->and($products[2]['id'])->toBe('prod_c'); + + // No metadata filter is sent: Stripe cannot search by it, so the whole + // active list comes back and the matching happens here. An account holds a + // handful of products, not thousands. + Http::assertSent(fn ($request) => str_contains($request->url(), 'active=true') + && ! str_contains($request->url(), 'metadata')); + Http::assertSent(fn ($request) => str_contains($request->url(), 'starting_after=prod_b')); +}); + +it('lets the fake answer with the products it holds', function () { + $fake = new FakeStripeClient; + + $minted = $fake->createProduct('Team', ['plan_family_id' => '3']); + $fake->plantProduct('prod_orphan', 'Team', ['plan_family_id' => '3'], created: 0); + + expect(collect($fake->activeProducts())->pluck('id')->all()) + ->toContain($minted, 'prod_orphan'); +});