From cdea9e923f6e2bc65371e09a476efcc609c3bfe0 Mon Sep 17 00:00:00 2001 From: nexxo Date: Thu, 30 Jul 2026 10:46:46 +0200 Subject: [PATCH] One source for what counts as the same call MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit IdempotencyKey::forPrice()/forProduct() built the parameter array they fingerprint, and FakeStripeClient::createPrice()/createProduct() rebuilt the identical array literal by hand purely to feed replay()/rememberKey(). Nothing tied those two literals together — a later change to what a Price call fingerprints could drift from what the fake checks for a collision, and no test would catch it, because the HttpStripeClient tests exercise forPrice() alone and the FakeStripeClient tests exercise its own local array alone. priceParameters()/productParameters() are now the one answer to "what counts as the same call"; forPrice()/forProduct() and the fake both ask it instead of each keeping their own copy. Fingerprint and key format are unchanged — this is the duplication coming out, not new behaviour. refund(), cancelSubscription(), addSubscriptionItem() and createCheckoutSession() are untouched: their local parameter arrays are the fake's own record of the call, never fingerprinted into a key, so there was nothing to duplicate there. Co-Authored-By: Claude Opus 5 --- app/Services/Stripe/FakeStripeClient.php | 19 ++++++----- app/Services/Stripe/IdempotencyKey.php | 41 +++++++++++++++++++----- 2 files changed, 44 insertions(+), 16 deletions(-) diff --git a/app/Services/Stripe/FakeStripeClient.php b/app/Services/Stripe/FakeStripeClient.php index c6bb114..34b9f81 100644 --- a/app/Services/Stripe/FakeStripeClient.php +++ b/app/Services/Stripe/FakeStripeClient.php @@ -161,7 +161,12 @@ class FakeStripeClient implements StripeClient public function createProduct(string $name, array $metadata = [], ?string $idempotencyKey = null): string { $key = IdempotencyKey::forProduct($idempotencyKey, $name, $metadata); - $parameters = ['name' => $name, 'metadata' => $metadata]; + + // The same parameters forProduct() just fingerprinted, asked for + // through the one method that knows what they are — a local literal + // here could drift from what the key actually covers without a test + // ever seeing it. + $parameters = IdempotencyKey::productParameters($name, $metadata); // Replays the first answer for a repeated key, as Stripe does. $replayed = $this->replay($key, $parameters); @@ -187,13 +192,11 @@ class FakeStripeClient implements StripeClient ?string $idempotencyKey = null, ): string { $key = IdempotencyKey::forPrice($idempotencyKey, $productId, $amountCents, $currency, $interval, $metadata); - $parameters = [ - 'product' => $productId, - 'unit_amount' => $amountCents, - 'currency' => strtolower($currency), - 'interval' => $interval, - 'metadata' => $metadata, - ]; + + // Same reasoning as createProduct(): the parameters forPrice() just + // fingerprinted, not a second literal that has to be kept in step by + // hand. + $parameters = IdempotencyKey::priceParameters($productId, $amountCents, $currency, $interval, $metadata); $replayed = $this->replay($key, $parameters); diff --git a/app/Services/Stripe/IdempotencyKey.php b/app/Services/Stripe/IdempotencyKey.php index f42b79d..d71f8ea 100644 --- a/app/Services/Stripe/IdempotencyKey.php +++ b/app/Services/Stripe/IdempotencyKey.php @@ -38,19 +38,44 @@ final class IdempotencyKey string $interval, array $metadata, ): ?string { - return self::with($key, [ - 'product' => $productId, - 'unit_amount' => $amountCents, - 'currency' => strtolower($currency), - 'interval' => $interval, - 'metadata' => $metadata, - ]); + return self::with($key, self::priceParameters($productId, $amountCents, $currency, $interval, $metadata)); } /** The key for a Product, for the same reason and against the same trap. */ public static function forProduct(?string $key, string $name, array $metadata): ?string { - return self::with($key, ['name' => $name, 'metadata' => $metadata]); + return self::with($key, self::productParameters($name, $metadata)); + } + + /** + * What counts as "the same call" for a Price — public because + * FakeStripeClient has to ask this exact question too, to decide whether a + * repeated key is a retry or a changed call. Built inline a second time in + * the fake, it would only ever match this by coincidence, and a later + * change to what a Price call fingerprints could drift from what the fake + * detects without a single test noticing — which is exactly the gap this + * task exists to close. + */ + public static function priceParameters( + string $productId, + int $amountCents, + string $currency, + string $interval, + array $metadata, + ): array { + return [ + 'product' => $productId, + 'unit_amount' => $amountCents, + 'currency' => strtolower($currency), + 'interval' => $interval, + 'metadata' => $metadata, + ]; + } + + /** What counts as "the same call" for a Product — same reason as priceParameters(). */ + public static function productParameters(string $name, array $metadata): array + { + return ['name' => $name, 'metadata' => $metadata]; } /**