Recognise the product Stripe already has
The gap beside the guard. A run that creates a product and dies before storing its id leaves an orphan exactly as a price does, and past the key's twenty-four hours the next run makes a second one — after which activePricesFor() is asked about the wrong product, price recognition goes blind for the whole family, and every orphaned price on the first product is unreachable. Two things are unlike the price side, deliberately. No money gate: a product carries no amount, so its metadata is the whole of the proof. And a duplicate is reported, never deactivated — archiving a price is provably harmless because Stripe keeps billing subscriptions already on it, but deactivating a product makes its prices unsellable and contracts can be running on those. Silent about strangers, too. This asks for every active product on the account, so an operator selling something else through it is an ordinary state, not a finding worth a line on every run. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>feature/host-bootstrap
nexxo
parent
fcea7ff3a8
commit
d6ec09a9b4
|
|
@ -8,6 +8,7 @@ use App\Models\PlanVersion;
|
|||
use App\Models\Subscription;
|
||||
use App\Services\Billing\AddonCatalogue;
|
||||
use App\Services\Billing\AddonPrices;
|
||||
use App\Services\Billing\AdoptStripeProduct;
|
||||
use App\Services\Billing\PlanPrices;
|
||||
use App\Services\Billing\TaxTreatment;
|
||||
use App\Services\Stripe\StripeClient;
|
||||
|
|
@ -101,32 +102,25 @@ class SyncStripeCatalogue extends Command
|
|||
$created++;
|
||||
|
||||
if (! $dryRun) {
|
||||
$productId = $stripe->createProduct(
|
||||
$metadata = ['plan_family' => $family->key, 'plan_family_id' => (string) $family->id];
|
||||
|
||||
// Asked BEFORE minting, for the same reason the Price side
|
||||
// asks: a run that died between Stripe's create and our
|
||||
// update left a Product this row does not know, and the key
|
||||
// below stops protecting it after twenty-four hours.
|
||||
// plan_family_id is what proves such a Product is this
|
||||
// family's.
|
||||
$productId = app(AdoptStripeProduct::class)($metadata, ['plan_family_id']);
|
||||
|
||||
$productId ??= $stripe->createProduct(
|
||||
$family->name,
|
||||
['plan_family' => $family->key, 'plan_family_id' => (string) $family->id],
|
||||
$metadata,
|
||||
// Covers a RETRY, for twenty-four hours, and nothing
|
||||
// after that: Stripe forgets a key at the end of them.
|
||||
// A crash between Stripe creating the Product and us
|
||||
// storing its id therefore leaves an orphan, and the
|
||||
// next run past the expiry makes a second Product for
|
||||
// this family. There is no recognition step for Products
|
||||
// — App\Services\Billing\AdoptStripePrice covers Prices
|
||||
// only — so nothing here ever asks Stripe what Products
|
||||
// it already has. A KNOWN GAP, not something this key
|
||||
// closes, and a worse one than a duplicate Price:
|
||||
// activePricesFor() would then be asked about the new
|
||||
// Product, and the Price recognition goes blind for the
|
||||
// whole family.
|
||||
//
|
||||
// IdempotencyKey::forProduct() folds the name and the
|
||||
// metadata into what goes on the wire, so a renamed
|
||||
// family under an unstored id now mints a second Product
|
||||
// where it used to answer HTTP 400 for a day. That is
|
||||
// the trade this branch chose deliberately: a blockade
|
||||
// reaches a paying customer, a duplicate Product does
|
||||
// not.
|
||||
// beyond them. What stops a second Product for one
|
||||
// family is the adoption step above.
|
||||
idempotencyKey: "clupilot-product-{$family->id}",
|
||||
);
|
||||
|
||||
$family->update(['stripe_product_id' => $productId]);
|
||||
}
|
||||
}
|
||||
|
|
|
|||
|
|
@ -13,6 +13,7 @@ use App\Mail\Transport\MailboxTransport;
|
|||
use App\Models\Customer;
|
||||
use App\Models\MaintenanceNotification;
|
||||
use App\Provisioning\PipelineRegistry;
|
||||
use App\Services\Billing\AdoptStripeProduct;
|
||||
use App\Services\Billing\CustomDomainAccess;
|
||||
use App\Services\Dns\FileHostDnsDirectory;
|
||||
use App\Services\Dns\HetznerDnsClient;
|
||||
|
|
@ -57,6 +58,13 @@ class AppServiceProvider extends ServiceProvider
|
|||
config('provisioning.pipelines', []),
|
||||
));
|
||||
|
||||
// Singletons because both carry a record across one run that the caller
|
||||
// reads afterwards: which products were duplicates, and how many prices
|
||||
// were adopted rather than created. Resolved per call they would each
|
||||
// start empty — SyncStripeCatalogue asks the container for PlanPrices
|
||||
// once per catalogue row.
|
||||
$this->app->singleton(AdoptStripeProduct::class);
|
||||
|
||||
// Real I/O implementations; tests swap in fakes via app()->instance().
|
||||
$this->app->bind(RemoteShell::class, PhpseclibRemoteShell::class);
|
||||
$this->app->bind(WireguardHub::class, LocalWireguardHub::class);
|
||||
|
|
|
|||
|
|
@ -284,14 +284,19 @@ final class AddonPrices
|
|||
return $existing;
|
||||
}
|
||||
|
||||
return $this->stripe->createProduct(
|
||||
$metadata = ['addon' => $addonKey];
|
||||
|
||||
// Asked before minting. A run that created this Product and died before
|
||||
// any row carried its id leaves an orphan — and a second Product would
|
||||
// make activePricesFor() ask about the wrong one, blinding the Price
|
||||
// recognition for this module entirely.
|
||||
$adopted = app(AdoptStripeProduct::class)($metadata, ['addon']);
|
||||
|
||||
return $adopted ?? $this->stripe->createProduct(
|
||||
app(AddonCatalogue::class)->name($addonKey),
|
||||
['addon' => $addonKey],
|
||||
// A retry's guard for twenty-four hours and nothing beyond them, the
|
||||
// same as the plan side's in SyncStripeCatalogue — and the same known
|
||||
// gap: there is no recognition step for Products, so a run that
|
||||
// created this one and died before any row carried its id leaves an
|
||||
// orphan nothing here will ever ask Stripe about.
|
||||
$metadata,
|
||||
// A retry's guard for twenty-four hours and nothing beyond them;
|
||||
// what stops a second Product is the adoption step above.
|
||||
idempotencyKey: "clupilot-addon-product-{$addonKey}",
|
||||
);
|
||||
}
|
||||
|
|
|
|||
|
|
@ -0,0 +1,137 @@
|
|||
<?php
|
||||
|
||||
namespace App\Services\Billing;
|
||||
|
||||
use App\Services\Stripe\StripeClient;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
/**
|
||||
* The Stripe Product we were about to create and Stripe already has.
|
||||
*
|
||||
* The sibling of AdoptStripePrice, one level up, and the gap that made that one
|
||||
* fragile. A run that creates a Product and dies before storing its id leaves an
|
||||
* orphan; the next run past the idempotency key's twenty-four hours makes a
|
||||
* second Product — and from then on activePricesFor() is asked about the new
|
||||
* one, so the Price recognition goes blind for the whole family and every
|
||||
* orphaned Price on the first Product is unreachable.
|
||||
*
|
||||
* **No money gate.** A Product carries no amount, no interval and no recurrence,
|
||||
* so there is nothing here that could move money and nothing to compare but the
|
||||
* metadata. That is the whole of the proof: a Product must CONFIRM an
|
||||
* identifying key of ours and contradict none it carries.
|
||||
*
|
||||
* **A duplicate is reported, never deactivated.** This is the one place the
|
||||
* Price side's reasoning does not carry over. Archiving a Price is provably
|
||||
* harmless — Stripe goes on billing every subscription already on it, which is
|
||||
* what the grandfathering rule has always rested on. Deactivating a Product
|
||||
* makes its PRICES unsellable, and contracts can be running on those. What was
|
||||
* proven for one is not proven for the other, so it is not done.
|
||||
*
|
||||
* **Silent about strangers.** Unlike the Price side, which asks only about our
|
||||
* own Product and warns when something unexplained sits there, this asks for
|
||||
* every active Product on the account. An operator selling something else
|
||||
* through the same account is an ordinary state, not a finding, and warning
|
||||
* about it would fire on every run for ever.
|
||||
*/
|
||||
final class AdoptStripeProduct
|
||||
{
|
||||
/**
|
||||
* Products that matched but were not taken, oldest-first order having
|
||||
* decided against them. Read by stripe:sync-catalogue for its report — a
|
||||
* log line alone is not seen by the operator running the sweep.
|
||||
*
|
||||
* @var array<int, string>
|
||||
*/
|
||||
public array $duplicates = [];
|
||||
|
||||
public function __construct(private readonly StripeClient $stripe) {}
|
||||
|
||||
/**
|
||||
* The id of an existing Stripe Product to use instead of creating one.
|
||||
*
|
||||
* @param array<string, string> $metadata what the create call would send
|
||||
* @param array<int, string> $identifying metadata keys that mark a Product as ours
|
||||
*/
|
||||
public function __invoke(array $metadata, array $identifying): ?string
|
||||
{
|
||||
$metadata = array_map(fn ($value) => (string) $value, $metadata);
|
||||
|
||||
$candidates = [];
|
||||
|
||||
foreach ($this->stripe->activeProducts() as $product) {
|
||||
if ($this->contradicts($product['metadata'], $metadata)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
if (! $this->confirms($product['metadata'], $metadata, $identifying)) {
|
||||
continue;
|
||||
}
|
||||
|
||||
$candidates[] = $product;
|
||||
}
|
||||
|
||||
if ($candidates === []) {
|
||||
return null;
|
||||
}
|
||||
|
||||
// Oldest first, and on a tie the lower id, so two runs agree. The tie is
|
||||
// not merely theoretical: createProduct()'s counter and plantProduct()'s
|
||||
// default both start at 1, so a minted Product and a planted orphan can
|
||||
// share a `created` — the same collision AdoptStripePrice's usort breaks
|
||||
// the same way, documented at FakeStripeClient::createPrice().
|
||||
usort($candidates, fn (array $a, array $b) => [$a['created'], $a['id']] <=> [$b['created'], $b['id']]);
|
||||
|
||||
$adopted = array_shift($candidates);
|
||||
|
||||
foreach ($candidates as $duplicate) {
|
||||
$this->duplicates[] = $duplicate['id'];
|
||||
|
||||
Log::warning('stripe: a second product claims to be the same thing — left alone, not deactivated', [
|
||||
'product' => $duplicate['id'],
|
||||
'adopted' => $adopted['id'],
|
||||
'metadata' => $metadata,
|
||||
]);
|
||||
}
|
||||
|
||||
Log::info('stripe: adopted an existing product instead of creating a second one', [
|
||||
'product' => $adopted['id'],
|
||||
'metadata' => $metadata,
|
||||
]);
|
||||
|
||||
return $adopted['id'];
|
||||
}
|
||||
|
||||
/**
|
||||
* @param array<string, string> $found
|
||||
* @param array<string, string> $expected
|
||||
*/
|
||||
private function contradicts(array $found, array $expected): bool
|
||||
{
|
||||
foreach ($expected as $key => $value) {
|
||||
if (array_key_exists($key, $found) && $found[$key] !== $value) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
|
||||
/**
|
||||
* Failing to contradict is not enough — an empty metadata bag contradicts
|
||||
* nothing, and this list holds every Product on the account.
|
||||
*
|
||||
* @param array<string, string> $found
|
||||
* @param array<string, string> $expected
|
||||
* @param array<int, string> $identifying
|
||||
*/
|
||||
private function confirms(array $found, array $expected, array $identifying): bool
|
||||
{
|
||||
foreach ($identifying as $key) {
|
||||
if (isset($found[$key]) && $found[$key] === ($expected[$key] ?? null)) {
|
||||
return true;
|
||||
}
|
||||
}
|
||||
|
||||
return false;
|
||||
}
|
||||
}
|
||||
|
|
@ -0,0 +1,113 @@
|
|||
<?php
|
||||
|
||||
use App\Models\PlanFamily;
|
||||
use App\Models\StripeAddonPrice;
|
||||
use App\Models\Subscription;
|
||||
use App\Services\Billing\AddonPrices;
|
||||
use App\Services\Billing\AdoptStripeProduct;
|
||||
use App\Services\Billing\TaxTreatment;
|
||||
use App\Services\Stripe\FakeStripeClient;
|
||||
use App\Services\Stripe\StripeClient;
|
||||
use Illuminate\Contracts\Console\Kernel;
|
||||
use Illuminate\Support\Facades\Log;
|
||||
|
||||
/**
|
||||
* A run that dies between Stripe creating a PRODUCT and us storing its id leaves
|
||||
* an orphan too — and that one is worse than an orphaned Price.
|
||||
*
|
||||
* A second Product does not merely duplicate something. Every activePricesFor()
|
||||
* afterwards is asked about the new Product, so the Price recognition built for
|
||||
* exactly this failure goes blind for the whole family, and every orphaned Price
|
||||
* on the first Product becomes unreachable. The guard disables itself through
|
||||
* the gap beside it.
|
||||
*
|
||||
* Two things are deliberately unlike the Price side. There is no money gate: a
|
||||
* Product carries no amount, so the metadata is the whole of the proof. And a
|
||||
* duplicate is REPORTED, never deactivated — an archived Price is provably
|
||||
* harmless because Stripe goes on billing subscriptions already on it, but
|
||||
* deactivating a Product makes its Prices unsellable, and contracts can be
|
||||
* running on those.
|
||||
*/
|
||||
beforeEach(function () {
|
||||
$this->stripe = new FakeStripeClient;
|
||||
app()->instance(StripeClient::class, $this->stripe);
|
||||
});
|
||||
|
||||
it('adopts an orphaned family product instead of minting a second one', function () {
|
||||
// A run that got as far as creating the Product and died before storing its
|
||||
// id: Stripe has it, plan_families does not.
|
||||
$family = PlanFamily::query()->orderBy('tier')->firstOrFail();
|
||||
$family->update(['stripe_product_id' => null]);
|
||||
|
||||
$this->stripe->plantProduct('prod_orphan_family', $family->name, [
|
||||
'plan_family' => $family->key,
|
||||
'plan_family_id' => (string) $family->id,
|
||||
]);
|
||||
|
||||
app(Kernel::class)->call('stripe:sync-catalogue');
|
||||
|
||||
// Counting every product would prove nothing — the same run mints one for
|
||||
// each of the OTHER families. What must hold is that this family's id is
|
||||
// carried by exactly one product, and that it is the planted one.
|
||||
expect($family->refresh()->stripe_product_id)->toBe('prod_orphan_family')
|
||||
->and(collect($this->stripe->products)
|
||||
->filter(fn (array $p) => ($p['metadata']['plan_family_id'] ?? null) === (string) $family->id)
|
||||
->keys()->all())->toBe(['prod_orphan_family']);
|
||||
});
|
||||
|
||||
it('adopts an orphaned module product instead of minting a second one', function () {
|
||||
$this->stripe->plantProduct('prod_orphan_module', 'Priority Support', [
|
||||
'addon' => 'priority_support',
|
||||
]);
|
||||
|
||||
$before = count($this->stripe->products);
|
||||
|
||||
app(AddonPrices::class)->ensure(
|
||||
'priority_support', 2900, 'EUR', Subscription::TERM_MONTHLY, TaxTreatment::domestic(),
|
||||
);
|
||||
|
||||
expect(StripeAddonPrice::query()
|
||||
->where('addon_key', 'priority_support')
|
||||
->value('stripe_product_id'))->toBe('prod_orphan_module')
|
||||
->and(count($this->stripe->products))->toBe($before);
|
||||
});
|
||||
|
||||
it('adopts nothing a product does not prove about itself', function () {
|
||||
// What an operator selling something else through the same Stripe account
|
||||
// leaves lying around. Silent, unlike the Price side: we list every product
|
||||
// on the account, so warning about each foreign one would fire on every run
|
||||
// for as long as it exists.
|
||||
Log::spy();
|
||||
|
||||
$this->stripe->plantProduct('prod_stranger', 'Etwas ganz anderes', []);
|
||||
|
||||
expect(app(AdoptStripeProduct::class)(
|
||||
['addon' => 'priority_support'], ['addon'],
|
||||
))->toBeNull();
|
||||
|
||||
Log::shouldNotHaveReceived('warning');
|
||||
});
|
||||
|
||||
it('adopts nothing that contradicts what we would have written', function () {
|
||||
$this->stripe->plantProduct('prod_other_module', 'Collabora Pro', ['addon' => 'collabora_pro']);
|
||||
|
||||
expect(app(AdoptStripeProduct::class)(
|
||||
['addon' => 'priority_support'], ['addon'],
|
||||
))->toBeNull();
|
||||
});
|
||||
|
||||
it('takes the oldest of two matching products and leaves the other alone', function () {
|
||||
Log::spy();
|
||||
|
||||
$this->stripe->plantProduct('prod_second', 'Priority Support', ['addon' => 'priority_support'], created: 200);
|
||||
$this->stripe->plantProduct('prod_first', 'Priority Support', ['addon' => 'priority_support'], created: 100);
|
||||
|
||||
$adopt = app(AdoptStripeProduct::class);
|
||||
|
||||
expect($adopt(['addon' => 'priority_support'], ['addon']))->toBe('prod_first')
|
||||
// NOT deactivated. Its prices would become unsellable, and a contract
|
||||
// can be running on one of them — the asymmetry with archivePrice().
|
||||
->and($adopt->duplicates)->toBe(['prod_second']);
|
||||
|
||||
Log::shouldHaveReceived('warning')->once();
|
||||
});
|
||||
Loading…
Reference in New Issue