Skip to content
Open
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
6 changes: 4 additions & 2 deletions src/services/ProductStripeService.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,9 +152,10 @@ export class ProductStripeService {
if (Array.isArray(updates.variants)) {
const incomingVariants = updates.variants
const existingVariants = Array.isArray(existingProduct.variants) ? existingProduct.variants : []
const existingVariantsMap = new Map(existingVariants.filter(ev => ev.id).map(ev => [ev.id, ev]))
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Guard variant map build against null entries

existingVariantsMap is now built eagerly with existingVariants.filter(ev => ev.id), which throws if stored variant arrays contain null/undefined elements; this can happen because the admin update route accepts raw JSON and does not validate variant item shape before persistence (src/routes/admin/products.js). Before this commit, the find path only ran when an incoming variant had an id, so updates containing only new variants could still succeed despite malformed historical entries. With the eager map build, those same updates now fail immediately with a 500, so the map construction should be null-safe (and mirrored for variants2).

Useful? React with 👍 / 👎.


const results = await Promise.all(incomingVariants.map(async (v) => {
const prior = v.id ? existingVariants.find(ev => ev.id === v.id) : undefined
const prior = v.id ? existingVariantsMap.get(v.id) : undefined
const wantsCustom = !!v.hasCustomPrice && typeof v.price === 'number' && v.price > 0

if (wantsCustom) {
Expand Down Expand Up @@ -200,9 +201,10 @@ export class ProductStripeService {
if (Array.isArray(updates.variants2)) {
const incomingVariants = updates.variants2
const existingVariants = Array.isArray(existingProduct.variants2) ? existingProduct.variants2 : []
const existingVariantsMap = new Map(existingVariants.filter(ev => ev.id).map(ev => [ev.id, ev]))

const results = await Promise.all(incomingVariants.map(async (v) => {
const prior = v.id ? existingVariants.find(ev => ev.id === v.id) : undefined
const prior = v.id ? existingVariantsMap.get(v.id) : undefined
const wantsCustom = !!v.hasCustomPrice && typeof v.price === 'number' && v.price > 0

if (wantsCustom) {
Expand Down
61 changes: 61 additions & 0 deletions tests/services/ProductStripeService.perf.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,61 @@
import { describe, it, expect, vi, beforeEach } from 'vitest'
import { ProductStripeService } from '../../src/services/ProductStripeService.js'

class MockStripeService {
async createProduct() { return { id: 'prod_123' } }
async createPrice(data) { return { id: `price_${Math.random()}` } }
async archivePrice() { return true }
}

describe('ProductStripeService Performance', () => {
let stripeService
let productStripeService

beforeEach(() => {
vi.clearAllMocks()
stripeService = new MockStripeService()
productStripeService = new ProductStripeService(stripeService)
})

it('should verify variant update performance', async () => {
// Generate a large number of existing variants
const NUM_VARIANTS = 10000;
const existingVariants = Array.from({ length: NUM_VARIANTS }).map((_, i) => ({
id: `var_${i}`,
name: `Variant ${i}`,
price: 10 + i,
hasCustomPrice: true,
stripePriceId: `price_${i}`
}));

const existingVariants2 = Array.from({ length: NUM_VARIANTS }).map((_, i) => ({
id: `var2_${i}`,
name: `Variant2 ${i}`,
price: 100 + i,
hasCustomPrice: true,
stripePriceId: `price2_${i}`
}));

const existingProduct = {
stripeProductId: 'prod_123',
stripePriceId: 'base_price_123',
variants: existingVariants,
variants2: existingVariants2
};

// Updates with the same variants, to trigger the O(N^2) behavior
const updates = {
variants: existingVariants.map(v => ({ ...v, price: v.price + 1 })),
variants2: existingVariants2.map(v => ({ ...v, price: v.price + 1 }))
};

console.log(`--- Starting Performance Test with ${NUM_VARIANTS} variants (primary and secondary) ---`);
const start = performance.now();
await productStripeService.updateProductVariants(existingProduct, updates, stripeService);
const end = performance.now();

console.log(`Update time: ${(end - start).toFixed(2)}ms`);

expect(true).toBe(true);
})
})