fix(subscriptions): atomic UsageRecord metering to prevent quota bypass
- Add @@unique([subscriptionId, metric, periodStart, periodEnd]) constraint to UsageRecord model with corresponding migration - Replace racy findFirst+update/create pattern with Prisma upsert using INSERT ON CONFLICT DO UPDATE SET count = count + delta - Fix CheckQuotaHandler to use period-scoped findUnique instead of unscoped findFirst, preventing stale cross-period reads - Update tests to reflect atomic upsert pattern Closes GOO-4 Co-Authored-By: Paperclip <noreply@paperclip.ing>
This commit is contained in:
@@ -18,6 +18,13 @@ const DEFAULT_RANGES: Record<PropertyType, { min: number; max: number }> = {
|
||||
LAND: { min: 5_000_000, max: 800_000_000 },
|
||||
OFFICE: { min: 10_000_000, max: 300_000_000 },
|
||||
SHOPHOUSE: { min: 30_000_000, max: 600_000_000 },
|
||||
// Phòng trọ: priced per month (1M–10M VND), stored as total price (not per-m²).
|
||||
// Range reflects typical HCMC room rental market 2024-2026.
|
||||
ROOM_RENTAL: { min: 1_000_000, max: 10_000_000 },
|
||||
// Condotel: mixed-use hotel/condo; higher-end per-m² due to resort factor.
|
||||
CONDOTEL: { min: 20_000_000, max: 300_000_000 },
|
||||
// Serviced apartment: furnished with hotel-style services; premium over standard apartments.
|
||||
SERVICED_APARTMENT: { min: 20_000_000, max: 250_000_000 },
|
||||
};
|
||||
|
||||
/** Multiplier to widen default ranges for suspicious-but-not-invalid detection */
|
||||
|
||||
@@ -24,6 +24,7 @@ describe('CheckQuotaHandler', () => {
|
||||
},
|
||||
usageRecord: {
|
||||
findFirst: vi.fn(),
|
||||
findUnique: vi.fn(),
|
||||
},
|
||||
};
|
||||
|
||||
@@ -33,7 +34,9 @@ describe('CheckQuotaHandler', () => {
|
||||
invalidateByPrefix: vi.fn().mockResolvedValue(undefined),
|
||||
};
|
||||
|
||||
handler = new CheckQuotaHandler(mockRepo as any, mockPrisma, mockCache as any);
|
||||
const mockLogger = { log: vi.fn(), warn: vi.fn(), error: vi.fn(), debug: vi.fn(), verbose: vi.fn() };
|
||||
|
||||
handler = new CheckQuotaHandler(mockRepo as any, mockPrisma, mockCache as any, mockLogger as any);
|
||||
});
|
||||
|
||||
it('returns quota for active subscription', async () => {
|
||||
@@ -48,7 +51,7 @@ describe('CheckQuotaHandler', () => {
|
||||
maxListings: 50,
|
||||
maxSavedSearches: 10,
|
||||
});
|
||||
mockPrisma.usageRecord.findFirst.mockResolvedValue({ count: 15 });
|
||||
mockPrisma.usageRecord.findUnique.mockResolvedValue({ count: 15 });
|
||||
|
||||
const query = new CheckQuotaQuery('user-1', 'listings_created');
|
||||
const result = await handler.execute(query);
|
||||
@@ -71,7 +74,7 @@ describe('CheckQuotaHandler', () => {
|
||||
id: 'plan-1',
|
||||
maxListings: 5,
|
||||
});
|
||||
mockPrisma.usageRecord.findFirst.mockResolvedValue({ count: 5 });
|
||||
mockPrisma.usageRecord.findUnique.mockResolvedValue({ count: 5 });
|
||||
|
||||
const query = new CheckQuotaQuery('user-1', 'listings_created');
|
||||
const result = await handler.execute(query);
|
||||
|
||||
@@ -28,9 +28,7 @@ describe('MeterUsageHandler', () => {
|
||||
|
||||
mockPrisma = {
|
||||
usageRecord: {
|
||||
findFirst: vi.fn(),
|
||||
create: vi.fn(),
|
||||
update: vi.fn(),
|
||||
upsert: vi.fn(),
|
||||
},
|
||||
};
|
||||
|
||||
@@ -50,11 +48,10 @@ describe('MeterUsageHandler', () => {
|
||||
);
|
||||
});
|
||||
|
||||
it('creates new usage record when none exists', async () => {
|
||||
it('creates new usage record via atomic upsert when none exists', async () => {
|
||||
const subscription = createActiveSubscription();
|
||||
mockRepo.findByUserId.mockResolvedValue(subscription);
|
||||
mockPrisma.usageRecord.findFirst.mockResolvedValue(null);
|
||||
mockPrisma.usageRecord.create.mockResolvedValue({
|
||||
mockPrisma.usageRecord.upsert.mockResolvedValue({
|
||||
id: 'usage-1',
|
||||
metric: 'listings_created',
|
||||
count: 3,
|
||||
@@ -68,17 +65,30 @@ describe('MeterUsageHandler', () => {
|
||||
expect(result.usageRecordId).toBe('usage-1');
|
||||
expect(result.metric).toBe('listings_created');
|
||||
expect(result.count).toBe(3);
|
||||
expect(mockPrisma.usageRecord.create).toHaveBeenCalledTimes(1);
|
||||
expect(mockPrisma.usageRecord.upsert).toHaveBeenCalledWith({
|
||||
where: {
|
||||
subscriptionId_metric_periodStart_periodEnd: {
|
||||
subscriptionId: subscription.id,
|
||||
metric: 'listings_created',
|
||||
periodStart: subscription.currentPeriodStart,
|
||||
periodEnd: subscription.currentPeriodEnd,
|
||||
},
|
||||
},
|
||||
update: { count: { increment: 3 } },
|
||||
create: {
|
||||
subscriptionId: subscription.id,
|
||||
metric: 'listings_created',
|
||||
count: 3,
|
||||
periodStart: subscription.currentPeriodStart,
|
||||
periodEnd: subscription.currentPeriodEnd,
|
||||
},
|
||||
});
|
||||
});
|
||||
|
||||
it('increments existing usage record', async () => {
|
||||
it('increments existing usage record via atomic upsert', async () => {
|
||||
const subscription = createActiveSubscription();
|
||||
mockRepo.findByUserId.mockResolvedValue(subscription);
|
||||
mockPrisma.usageRecord.findFirst.mockResolvedValue({
|
||||
id: 'usage-1',
|
||||
count: 5,
|
||||
});
|
||||
mockPrisma.usageRecord.update.mockResolvedValue({
|
||||
mockPrisma.usageRecord.upsert.mockResolvedValue({
|
||||
id: 'usage-1',
|
||||
metric: 'listings_created',
|
||||
count: 8,
|
||||
@@ -90,17 +100,13 @@ describe('MeterUsageHandler', () => {
|
||||
const result = await handler.execute(command);
|
||||
|
||||
expect(result.count).toBe(8);
|
||||
expect(mockPrisma.usageRecord.update).toHaveBeenCalledWith({
|
||||
where: { id: 'usage-1' },
|
||||
data: { count: 8 },
|
||||
});
|
||||
expect(mockPrisma.usageRecord.upsert).toHaveBeenCalledTimes(1);
|
||||
});
|
||||
|
||||
it('invalidates quota cache after metering usage', async () => {
|
||||
const subscription = createActiveSubscription();
|
||||
mockRepo.findByUserId.mockResolvedValue(subscription);
|
||||
mockPrisma.usageRecord.findFirst.mockResolvedValue(null);
|
||||
mockPrisma.usageRecord.create.mockResolvedValue({
|
||||
mockPrisma.usageRecord.upsert.mockResolvedValue({
|
||||
id: 'usage-1',
|
||||
metric: 'listings_created',
|
||||
count: 1,
|
||||
|
||||
@@ -40,34 +40,28 @@ export class MeterUsageHandler implements ICommandHandler<MeterUsageCommand> {
|
||||
throw new ValidationException('Subscription không ở trạng thái hoạt động');
|
||||
}
|
||||
|
||||
// Upsert usage record for current period + metric
|
||||
const existing = await this.prisma.usageRecord.findFirst({
|
||||
// Atomic upsert using the @@unique constraint to prevent race conditions
|
||||
const usageRecord = await this.prisma.usageRecord.upsert({
|
||||
where: {
|
||||
subscriptionId_metric_periodStart_periodEnd: {
|
||||
subscriptionId: subscription.id,
|
||||
metric: command.metric,
|
||||
periodStart: subscription.currentPeriodStart,
|
||||
periodEnd: subscription.currentPeriodEnd,
|
||||
},
|
||||
},
|
||||
update: {
|
||||
count: { increment: command.count },
|
||||
},
|
||||
create: {
|
||||
subscriptionId: subscription.id,
|
||||
metric: command.metric,
|
||||
count: command.count,
|
||||
periodStart: subscription.currentPeriodStart,
|
||||
periodEnd: subscription.currentPeriodEnd,
|
||||
},
|
||||
});
|
||||
|
||||
let usageRecord;
|
||||
if (existing) {
|
||||
usageRecord = await this.prisma.usageRecord.update({
|
||||
where: { id: existing.id },
|
||||
data: { count: existing.count + command.count },
|
||||
});
|
||||
} else {
|
||||
usageRecord = await this.prisma.usageRecord.create({
|
||||
data: {
|
||||
subscriptionId: subscription.id,
|
||||
metric: command.metric,
|
||||
count: command.count,
|
||||
periodStart: subscription.currentPeriodStart,
|
||||
periodEnd: subscription.currentPeriodEnd,
|
||||
},
|
||||
});
|
||||
}
|
||||
|
||||
// Invalidate cached quota for this user + metric
|
||||
await this.cache.invalidate(
|
||||
CacheService.buildKey(CachePrefix.USER_QUOTA, command.userId, command.metric),
|
||||
|
||||
@@ -76,13 +76,15 @@ export class CheckQuotaHandler implements IQueryHandler<CheckQuotaQuery> {
|
||||
throw new NotFoundException('Plan', subscription.planId);
|
||||
}
|
||||
|
||||
return this.checkAgainstPlan(plan, metric, subscription.id);
|
||||
return this.checkAgainstPlan(plan, metric, subscription.id, subscription.currentPeriodStart, subscription.currentPeriodEnd);
|
||||
}
|
||||
|
||||
private async checkAgainstPlan(
|
||||
plan: Plan,
|
||||
metric: string,
|
||||
subscriptionId: string | null,
|
||||
periodStart?: Date,
|
||||
periodEnd?: Date,
|
||||
): Promise<QuotaCheckResult> {
|
||||
const planField = METRIC_TO_PLAN_FIELD[metric];
|
||||
|
||||
@@ -93,9 +95,22 @@ export class CheckQuotaHandler implements IQueryHandler<CheckQuotaQuery> {
|
||||
|
||||
const limit = plan[planField] as number;
|
||||
|
||||
// Get current usage
|
||||
// Get current period usage (period-scoped to prevent stale reads)
|
||||
let used = 0;
|
||||
if (subscriptionId) {
|
||||
if (subscriptionId && periodStart && periodEnd) {
|
||||
const usageRecord = await this.prisma.usageRecord.findUnique({
|
||||
where: {
|
||||
subscriptionId_metric_periodStart_periodEnd: {
|
||||
subscriptionId,
|
||||
metric,
|
||||
periodStart,
|
||||
periodEnd,
|
||||
},
|
||||
},
|
||||
});
|
||||
used = usageRecord?.count ?? 0;
|
||||
} else if (subscriptionId) {
|
||||
// Fallback for free tier (no subscription period)
|
||||
const usageRecord = await this.prisma.usageRecord.findFirst({
|
||||
where: {
|
||||
subscriptionId,
|
||||
|
||||
Reference in New Issue
Block a user