fix(billing): address review feedback on BillingUsage model

- Validate organizationId in increment(), sanitize counter key against injection
- Handle E11000 duplicate key race on concurrent upserts (retry once)
- Add runValidators to reset() for consistency with increment()
- Tighten month regex to reject invalid months (00, 13)
- Use UTC for currentMonth() to avoid timezone-dependent boundaries
- Capture month once in get() to prevent cross-boundary inconsistency
- Remove redundant individual index on organizationId (compound covers it)
- Add missing Jest globals imports in test file
- Add tests for semantically invalid month values

Author: PierreBrisorgueil

modules/billing/models/billing.usage.model.mongoose.js

@@ -14,7 +14,6 @@ const UsageMongoose = new Schema(
       type: Schema.ObjectId,
       ref: 'Organization',
       required: true,
-      index: true,
     },
     month: {
       type: String,

modules/billing/models/billing.usage.schema.js

@@ -10,7 +10,7 @@ const objectIdRegex = /^[a-f\d]{24}$/i;
 
 const BillingUsage = z.object({
   organizationId: z.string().trim().regex(objectIdRegex, 'organizationId must be a valid ObjectId'),
-  month: z.string().trim().regex(/^\d{4}-\d{2}$/, 'month must be in YYYY-MM format'),
+  month: z.string().trim().regex(/^\d{4}-(0[1-9]|1[0-2])$/, 'month must be in YYYY-MM format'),
   counters: z.record(z.string(), z.number()).default(() => ({})),
 });
 

modules/billing/repositories/billing.usage.repository.js

@@ -5,6 +5,8 @@ import mongoose from 'mongoose';
 
 const BillingUsage = mongoose.model('BillingUsage');
 
+const SAFE_KEY_RE = /^[a-zA-Z0-9_-]+$/;
+
 /**
  * @function get
  * @description Fetch a single usage document by organizationId and month.
@@ -26,11 +28,26 @@ const get = (organizationId, month) => {
  * @param {Number} amount - The amount to increment by.
  * @returns {Promise<Object>} The updated usage document.
  */
-const increment = (organizationId, month, key, amount) => BillingUsage.findOneAndUpdate(
-  { organizationId, month },
-  { $inc: { [`counters.${key}`]: amount } },
-  { upsert: true, returnDocument: 'after', runValidators: true },
-).exec();
+const increment = async (organizationId, month, key, amount) => {
+  if (!mongoose.Types.ObjectId.isValid(organizationId)) return null;
+  if (!SAFE_KEY_RE.test(key)) throw new Error(`Invalid counter key: ${key}`);
+  try {
+    return await BillingUsage.findOneAndUpdate(
+      { organizationId, month },
+      { $inc: { [`counters.${key}`]: amount } },
+      { upsert: true, returnDocument: 'after', runValidators: true },
+    ).exec();
+  } catch (err) {
+    if (err.code === 11000) {
+      return BillingUsage.findOneAndUpdate(
+        { organizationId, month },
+        { $inc: { [`counters.${key}`]: amount } },
+        { returnDocument: 'after', runValidators: true },
+      ).exec();
+    }
+    throw err;
+  }
+};
 
 /**
  * @function reset
@@ -44,7 +61,7 @@ const reset = (organizationId, month) => {
   return BillingUsage.findOneAndUpdate(
     { organizationId, month },
     { $set: { counters: {} } },
-    { returnDocument: 'after' },
+    { returnDocument: 'after', runValidators: true },
   ).exec();
 };
 

modules/billing/services/billing.usage.service.js

@@ -9,8 +9,8 @@ import UsageRepository from '../repositories/billing.usage.repository.js';
  */
 const currentMonth = () => {
   const now = new Date();
-  const year = now.getFullYear();
-  const month = String(now.getMonth() + 1).padStart(2, '0');
+  const year = now.getUTCFullYear();
+  const month = String(now.getUTCMonth() + 1).padStart(2, '0');
   return `${year}-${month}`;
 };
 
@@ -29,8 +29,9 @@ const increment = (organizationId, key, amount) => UsageRepository.increment(org
  * @returns {Promise<Object>} The usage document or an object with empty counters.
  */
 const get = async (organizationId) => {
-  const usage = await UsageRepository.get(organizationId, currentMonth());
-  return usage || { organizationId, month: currentMonth(), counters: {} };
+  const month = currentMonth();
+  const usage = await UsageRepository.get(organizationId, month);
+  return usage || { organizationId, month, counters: {} };
 };
 
 /**

modules/billing/tests/billing.usage.unit.tests.js

@@ -1,7 +1,7 @@
 /**
  * Module dependencies.
  */
-import { jest, beforeEach, afterEach } from '@jest/globals';
+import { jest, describe, test, beforeEach, afterEach, expect } from '@jest/globals';
 import schema from '../models/billing.usage.schema.js';
 
 /**
@@ -51,6 +51,18 @@ describe('BillingUsage unit tests:', () => {
       expect(result.error).toBeDefined();
     });
 
+    test('should reject semantically invalid month 2026-00', () => {
+      usage.month = '2026-00';
+      const result = schema.BillingUsage.safeParse(usage);
+      expect(result.error).toBeDefined();
+    });
+
+    test('should reject semantically invalid month 2026-13', () => {
+      usage.month = '2026-13';
+      const result = schema.BillingUsage.safeParse(usage);
+      expect(result.error).toBeDefined();
+    });
+
     test('should default counters to empty object', () => {
       delete usage.counters;
       const result = schema.BillingUsage.safeParse(usage);