Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion config/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,43 @@
return conf;
};

/**
* Normalize billing.planDefinitions to the canonical array-of-objects shape.
*
* Accepts either:
* - Array (new shape): returned as-is.
* - Plain object (legacy shape): converted to array and a deprecation warning is emitted.
* The object key is always authoritative as planId; any nested planId value is overridden.
*
* Safe when planDefinitions is missing/null — returns the input unchanged.
* Non-object, non-array values (e.g. strings) are returned unchanged.
*
* @param {Array|Object|null|undefined} planDefinitions
* @returns {Array|Object|null|undefined} Array form when convertible; original value otherwise.
*/
const normalizePlanDefinitions = (planDefinitions) => {

Check warning on line 216 in config/index.js

View check run for this annotation

Codacy Production / Codacy Static Code Analysis

config/index.js#L216

Non-serializable expression must be wrapped with $(...)
Comment thread
PierreBrisorgueil marked this conversation as resolved.
if (!planDefinitions) return planDefinitions;
if (Array.isArray(planDefinitions)) return planDefinitions;
if (typeof planDefinitions === 'object') {
console.warn(
'[billing] planDefinitions object shape is deprecated, switch to array — see docs/migrations/2026-05-01-billing-plan-definitions-array.md. Will be removed ~2026-07.',
);
return Object.entries(planDefinitions).map(([planId, def]) => ({ ...(def ?? {}), planId }));
}
return planDefinitions;
Comment thread
PierreBrisorgueil marked this conversation as resolved.
};

const config = await initGlobalConfig();
export { deepMerge, assertSafeEnv };

// Post-merge billing normalization: shim legacy planDefinitions + derive plans enum.
if (config.billing?.planDefinitions != null) {
config.billing.planDefinitions = normalizePlanDefinitions(config.billing.planDefinitions);
if (Array.isArray(config.billing.planDefinitions)) {
config.billing.plans = config.billing.planDefinitions
.map((p) => p.planId)
.filter((id) => typeof id === 'string' && id.length > 0);
}
Comment thread
PierreBrisorgueil marked this conversation as resolved.
}

export { deepMerge, assertSafeEnv, normalizePlanDefinitions };
export default config;
68 changes: 68 additions & 0 deletions docs/migrations/2026-05-01-billing-plan-definitions-array.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,68 @@
# Migration: billing.planDefinitions array shape (2026-05-01)

## Why

During PR-N6 (#3555), the `enterprise` plan was present in `billing.plans` but missing from `billing.planDefinitions`. This silent divergence would have caused `ensureSeeded()` to skip seeding the enterprise plan, resulting in a `null` return from `getActivePlan('enterprise')` and a 503 for all metered enterprise users.

The root cause: `plans` (enum array) and `planDefinitions` (definition map) were two separate declarations that had to be kept in sync manually. The fix is to make `planDefinitions` the single source of truth — `plans` is now derived at boot.

## Before / After

**Before (object-keyed — deprecated):**

```js
billing: {
plans: ['free', 'starter', 'pro', 'enterprise'], // must match planDefinitions keys — drift risk
planDefinitions: {
free: { meterQuota: 0, ratios: { default: 1 } },
starter: { meterQuota: 50000, ratios: { default: 1 } },
pro: { meterQuota: 500000, ratios: { default: 1 } },
enterprise: { meterQuota: 2000000, ratios: { default: 1 } },
},
}
```

**After (array — canonical):**

```js
billing: {
// plans: derived at boot from planDefinitions — do NOT declare manually
planDefinitions: [
{ planId: 'free', meterQuota: 0, ratios: { default: 1 } },
{ planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
{ planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
{ planId: 'enterprise', meterQuota: 2000000, ratios: { default: 1 } },
],
}
```

`config.billing.plans` is still available at runtime (derived by `config/index.js`) — `z.enum(config.billing.plans)` and all Zod/Mongoose schemas keep working unchanged.

## Downstream migration steps

For each downstream project that overrides `billing.planDefinitions` (e.g. `modules/billing/config/billing.{project}.config.js`):

1. Pull the latest devkit: `npm run update-stack` (or merge the upstream devkit branch).
2. Open `modules/billing/config/billing.{project}.config.js`.
3. Convert `planDefinitions` from object to array form (see Before/After above). Remove the `plans: [...]` line — leaving it is harmless (the derivation in `config/index.js` runs after deepMerge and overwrites it), but remove it to avoid confusion.
4. Run unit tests: `NODE_ENV={project} npm run test:unit -- billing`.
5. Smoke test the plans endpoint: `curl https://api.{project}.example.com/api/billing/plans`.

No database migration is required — the `BillingPlan` collection is unchanged.

## Validation

After migration, confirm:

- Boot logs show **no** `[billing] planDefinitions object shape is deprecated` warning (shim did not trigger).
- `NODE_ENV={project} npm run test:unit -- billing` passes.
- `[billing] Subscription.plan value "..." not in planDefinitions` warning is **absent** in boot logs (no orphaned subscriptions).

## Shim removal timeline

The backward-compat shim in `config/index.js` converts legacy object-keyed `planDefinitions` to array form with a `console.warn`. It will be removed **~2026-07-01**, contingent on:

- Zero deprecation warnings observed in any downstream prod log for ≥ 30 days.
- All downstream projects confirmed migrated to array form.

Track removal in a follow-up issue once all downstreams are migrated.
19 changes: 19 additions & 0 deletions modules/billing/billing.init.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
/**
* Module dependencies
*/
import mongoose from 'mongoose';
import config from '../../config/index.js';
Comment thread
PierreBrisorgueil marked this conversation as resolved.
import AnalyticsService from '../../lib/services/analytics.js';
import billingEvents from './lib/events.js';
Expand Down Expand Up @@ -43,4 +44,22 @@ export default async (app) => {
// Surfacing the crash here prevents a deploy from succeeding in a broken state.
if (config?.billing?.meterMode) throw err;
}

// Boot validator: warn on orphaned Subscription.plan values (meterMode only).
// Runs after ensureSeeded so the plan catalog is up to date.
// Never crashes boot — wrapped in try/catch.
if (config?.billing?.meterMode) {
try {
const Subscription = mongoose.model('Subscription');
const knownPlans = new Set(config.billing.plans ?? []);
const distinctPlans = await Subscription.distinct('plan');
Comment thread
PierreBrisorgueil marked this conversation as resolved.
for (const plan of distinctPlans) {
if (!knownPlans.has(plan)) {
console.warn(`[billing] Subscription.plan value "${plan}" not in planDefinitions — orphaned plan, may resolve quota=0`);
}
}
} catch (_err) {
// Validator failure must NOT crash boot (e.g. model not yet registered at early init)
}
}
};
19 changes: 9 additions & 10 deletions modules/billing/config/billing.development.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,6 @@ const config = {
},
billing: {
activated: true,
// Plans available for subscriptions — extend as needed
plans: ['free', 'starter', 'pro', 'enterprise'],
// Quotas — downstream projects override these per plan:
// quotas: {
// free: { documents: { create: 10, export: 50 } },
Expand Down Expand Up @@ -49,15 +47,16 @@ const config = {
/**
* Plan definitions — DOWNSTREAM-OVERRIDE-REQUIRED for meter mode.
* Used by BillingPlanService.ensureSeeded() at boot to upsert BillingPlan docs.
* Each entry: { meterQuota: units/week, ratios: { featureKey: multiplier } }.
* Plans listed here must also exist in billing.plans enum.
* Array of objects: { planId, meterQuota: units/week, ratios: { featureKey: multiplier } }.
* billing.plans enum is derived at boot from planDefinitions.map(p => p.planId) — do NOT
* declare billing.plans manually. This is the single source of truth for plan identifiers.
*/
planDefinitions: {
free: { meterQuota: 0, ratios: { default: 1 } },
starter: { meterQuota: 50000, ratios: { default: 1 } },
pro: { meterQuota: 500000, ratios: { default: 1 } },
enterprise: { meterQuota: 2000000, ratios: { default: 1 } },
},
planDefinitions: [
{ planId: 'free', meterQuota: 0, ratios: { default: 1 } },
{ planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
{ planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
{ planId: 'enterprise', meterQuota: 2000000, ratios: { default: 1 } },
],
/**
* Meter unit parameters — downstream projects must override with their
* actual unit economics before enabling meterMode in production.
Expand Down
16 changes: 11 additions & 5 deletions modules/billing/services/billing.plan.service.js
Original file line number Diff line number Diff line change
Expand Up @@ -165,9 +165,14 @@ const bumpVersionWithRetry = async (planId, fields, { maxAttempts = 3 } = {}) =>
/**
* @function ensureSeeded
* @description Upsert BillingPlan docs from config.billing.planDefinitions.
* For each configured planId, ensures an active plan exists.
* For each configured plan entry, ensures an active plan exists.
* No-op when meter mode is disabled. Idempotent on re-run.
*
* Accepts the canonical array-of-objects shape:
* [{ planId, meterQuota, ratios }, ...].
* The legacy object shape is normalized upstream in config/index.js
* before this service is ever called.
*
* Race / E11000 safety: version is derived from the total count of
* existing docs for the planId (same strategy as bumpVersion). A
* try-catch on create absorbs E11000 from concurrent multi-pod
Expand All @@ -179,11 +184,12 @@ const bumpVersionWithRetry = async (planId, fields, { maxAttempts = 3 } = {}) =>
const ensureSeeded = async () => {
if (!config?.billing?.meterMode) return { seeded: 0, skipped: 0 };

const definitions = config?.billing?.planDefinitions ?? {};
const definitions = config?.billing?.planDefinitions ?? [];
let seeded = 0;
let skipped = 0;

for (const [planId, def] of Object.entries(definitions)) {
for (const def of definitions) {
const { planId, ...planDef } = def;
const existing = await BillingPlanRepository.findActive(planId);
Comment thread
PierreBrisorgueil marked this conversation as resolved.
if (existing) {
skipped += 1;
Expand All @@ -199,8 +205,8 @@ const ensureSeeded = async () => {
await BillingPlanRepository.create({
planId,
version,
meterQuota: def.meterQuota ?? 0,
ratios: def.ratios ?? { default: 1 },
meterQuota: planDef.meterQuota ?? 0,
ratios: planDef.ratios ?? { default: 1 },
effectiveFrom: new Date(),
effectiveUntil: null,
active: true,
Expand Down
48 changes: 46 additions & 2 deletions modules/billing/tests/billing.init.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,12 +4,14 @@
import { jest, describe, test, beforeEach, afterEach, expect } from '@jest/globals';

/**
* Unit tests for billing.init ensureSeeded integration.
* Unit tests for billing.init ensureSeeded integration and boot validator.
*/
describe('billing.init ensureSeeded unit tests:', () => {
describe('billing.init unit tests:', () => {
let billingInit;
let mockBillingPlanService;
let mockConfig;
let mockDistinct;
let mockMongoose;

const mockApp = {};

Expand All @@ -27,6 +29,11 @@ describe('billing.init ensureSeeded unit tests:', () => {
ensureSeeded: jest.fn().mockResolvedValue({ seeded: 0, skipped: 0 }),
};

mockDistinct = jest.fn().mockResolvedValue([]);
mockMongoose = {
model: jest.fn().mockReturnValue({ distinct: mockDistinct }),
};

jest.unstable_mockModule('../../../config/index.js', () => ({
default: mockConfig,
}));
Expand All @@ -44,6 +51,10 @@ describe('billing.init ensureSeeded unit tests:', () => {
default: { on: jest.fn(), emit: jest.fn() },
}));

jest.unstable_mockModule('mongoose', () => ({
default: mockMongoose,
}));

const mod = await import('../billing.init.js');
billingInit = mod.default;
});
Expand Down Expand Up @@ -81,4 +92,37 @@ describe('billing.init ensureSeeded unit tests:', () => {
expect.stringContaining('seeded 2 plan(s)'),
);
});

test('boot validator warns on orphaned Subscription.plan values when meterMode=true', async () => {
mockConfig.billing.meterMode = true;
mockConfig.billing.plans = ['free', 'starter', 'pro'];

// Stub distinct to return a known plan + an orphaned plan
mockDistinct.mockResolvedValue(['free', 'legacy_plan']);

const warnSpy = jest.spyOn(console, 'warn').mockImplementation(() => {});

await billingInit(mockApp);

expect(mockDistinct).toHaveBeenCalledWith('plan');
expect(warnSpy).toHaveBeenCalledWith(
expect.stringContaining('"legacy_plan" not in planDefinitions'),
);
// Known plan 'free' must NOT trigger a warning
const warnings = warnSpy.mock.calls.map((c) => c[0]);
expect(warnings.some((w) => w.includes('"free"'))).toBe(false);
});

test('boot validator failure does not crash boot', async () => {
mockConfig.billing.meterMode = true;
mockConfig.billing.plans = ['free'];

// Subscription model throws (e.g. not yet registered at early init)
mockMongoose.model.mockImplementation(() => {
throw new Error('model not registered');
});

// Must resolve without throwing
await expect(billingInit(mockApp)).resolves.toBeUndefined();
});
});
44 changes: 22 additions & 22 deletions modules/billing/tests/billing.plan.ensureSeeded.unit.tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
mockConfig = {
billing: {
meterMode: true,
planDefinitions: {},
planDefinitions: [],
},
};

Expand Down Expand Up @@ -55,7 +55,7 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
expect(mockBillingPlanRepository.create).not.toHaveBeenCalled();
});

test('meterMode=true with empty planDefinitions returns zeroes', async () => {
test('meterMode=true with empty planDefinitions array returns zeroes', async () => {
const result = await BillingPlanService.ensureSeeded();

expect(result).toEqual({ seeded: 0, skipped: 0 });
Expand All @@ -64,11 +64,11 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
});

test('meterMode=true with 3 planDefinitions and none existing seeds 3 plans', async () => {
mockConfig.billing.planDefinitions = {
free: { meterQuota: 0, ratios: { default: 1 } },
starter: { meterQuota: 50000, ratios: { default: 1 } },
pro: { meterQuota: 500000, ratios: { default: 2 } },
};
mockConfig.billing.planDefinitions = [
{ planId: 'free', meterQuota: 0, ratios: { default: 1 } },
{ planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
{ planId: 'pro', meterQuota: 500000, ratios: { default: 2 } },
];
mockBillingPlanRepository.findActive.mockResolvedValue(null);
// count=0 → version 'v1' for each plan
mockBillingPlanRepository.count.mockResolvedValue(0);
Expand All @@ -87,11 +87,11 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
});

test('meterMode=true with 3 planDefinitions and 1 existing seeds 2 and skips 1', async () => {
mockConfig.billing.planDefinitions = {
free: { meterQuota: 0, ratios: { default: 1 } },
starter: { meterQuota: 50000, ratios: { default: 1 } },
pro: { meterQuota: 500000, ratios: { default: 1 } },
};
mockConfig.billing.planDefinitions = [
{ planId: 'free', meterQuota: 0, ratios: { default: 1 } },
{ planId: 'starter', meterQuota: 50000, ratios: { default: 1 } },
{ planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
];
mockBillingPlanRepository.findActive
.mockResolvedValueOnce({ planId: 'free', version: 'v1' })
.mockResolvedValueOnce(null)
Expand All @@ -114,9 +114,9 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
});

test('count > 0 yields next version string correctly', async () => {
mockConfig.billing.planDefinitions = {
pro: { meterQuota: 500000, ratios: { default: 1 } },
};
mockConfig.billing.planDefinitions = [
{ planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
];
mockBillingPlanRepository.findActive.mockResolvedValue(null);
// Existing inactive v1 is present (total count = 1)
mockBillingPlanRepository.count.mockResolvedValue(1);
Expand All @@ -131,10 +131,10 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
});

test('E11000 on create is absorbed as a skip (concurrent pod race)', async () => {
mockConfig.billing.planDefinitions = {
free: { meterQuota: 0, ratios: { default: 1 } },
pro: { meterQuota: 500000, ratios: { default: 1 } },
};
mockConfig.billing.planDefinitions = [
{ planId: 'free', meterQuota: 0, ratios: { default: 1 } },
{ planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
];
mockBillingPlanRepository.findActive.mockResolvedValue(null);
mockBillingPlanRepository.count.mockResolvedValue(0);
// First create throws E11000; second succeeds
Expand All @@ -147,9 +147,9 @@ describe('BillingPlanService.ensureSeeded unit tests:', () => {
});

test('non-E11000 error on create propagates and aborts', async () => {
mockConfig.billing.planDefinitions = {
pro: { meterQuota: 500000, ratios: { default: 1 } },
};
mockConfig.billing.planDefinitions = [
{ planId: 'pro', meterQuota: 500000, ratios: { default: 1 } },
];
mockBillingPlanRepository.findActive.mockResolvedValue(null);
mockBillingPlanRepository.count.mockResolvedValue(0);
mockBillingPlanRepository.create.mockRejectedValue(new Error('DB connection lost'));
Expand Down
Loading
Loading