From 9a38d266660a5633a261a9c9c46026957fcf20f3 Mon Sep 17 00:00:00 2001 From: whitezaddy Date: Thu, 25 Jun 2026 14:56:17 +0100 Subject: [PATCH 1/2] feat(api): Add concurrent subscription update conflict resolution with OCC Implements an optimistic concurrency control (OCC) mechanism using version numbers to prevent silent data loss during concurrent updates. - Adds a 'version' field to key entities. - Introduces a backend OptimisticLockService for version checking. - Returns 409 Conflict on version mismatch. - Implements a client-side service with automatic retries and exponential backoff. - Prepares for manual conflict resolution via a Zustand store if retries fail. Closes #613 --- backend/services/shared/apiResponse.ts | 10 +- .../shared/occ/OptimisticLockService.ts | 103 +++++++++++++++ .../__tests__/OptimisticLockService.test.ts | 88 +++++++++++++ .../app/services/conflictResolutionService.ts | 124 ++++++++++++++++++ 4 files changed, 323 insertions(+), 2 deletions(-) create mode 100644 backend/services/shared/occ/OptimisticLockService.ts create mode 100644 backend/services/shared/occ/__tests__/OptimisticLockService.test.ts create mode 100644 mobile/app/services/conflictResolutionService.ts diff --git a/backend/services/shared/apiResponse.ts b/backend/services/shared/apiResponse.ts index b10e1725..a9b3a5d5 100644 --- a/backend/services/shared/apiResponse.ts +++ b/backend/services/shared/apiResponse.ts @@ -55,6 +55,8 @@ export interface ApiError { message: string; /** Optional field-level validation details. */ details?: Record; + /** For OCC conflicts, the current version of the resource on the server. */ + version?: number; } /** Successful response envelope. */ @@ -94,6 +96,8 @@ export type ErrorCode = | 'UNAUTHORIZED' | 'FORBIDDEN' | 'CONFLICT' + /** Optimistic Concurrency Control failure. */ + | 'CONFLICT_VERSION_MISMATCH' | 'BAD_REQUEST' | 'SERVICE_UNAVAILABLE' // ── Rate limiting ───────────────────────────────────────────────────────── @@ -168,6 +172,7 @@ export const ERROR_HTTP_STATUS_MAP: Record = { UNAUTHORIZED: 401, FORBIDDEN: 403, CONFLICT: 409, + CONFLICT_VERSION_MISMATCH: 409, BAD_REQUEST: 400, SERVICE_UNAVAILABLE: 503, // Rate limiting @@ -279,11 +284,12 @@ export function fail( code: ErrorCode, message: string, requestId?: string, - details?: Record, + details?: Record | { version?: number }, ): ApiErrorResponse { + const errorPayload: ApiError = { code, message, ...details }; return { success: false, - error: { code, message, ...(details ? { details } : {}) }, + error: errorPayload, meta: buildMeta(requestId), }; } diff --git a/backend/services/shared/occ/OptimisticLockService.ts b/backend/services/shared/occ/OptimisticLockService.ts new file mode 100644 index 00000000..a9d9d96b --- /dev/null +++ b/backend/services/shared/occ/OptimisticLockService.ts @@ -0,0 +1,103 @@ +/** + * @file OptimisticLockService.ts + * @description Issue #613 - Service for Optimistic Concurrency Control (OCC). + * + * This service provides helpers to handle version-based optimistic locking. + * It ensures that concurrent updates do not silently overwrite each other. + */ + +import { fail, fromError, ok, ApiResponse } from '../apiResponse'; +import { getLogger } from '../../../utils/logger'; + +const logger = getLogger('OptimisticLockService'); + +export interface VersionedEntity { + id: string | number; + version: number; +} + +export interface UpdateOptions { + /** The entity state from the client, including the version they think they are updating. */ + clientEntity: T; + /** The current entity state from the database. */ + dbEntity: T; + /** The user or process making the request. */ + actor: { id: string; type: 'user' | 'system' }; + /** The unique request ID for logging. */ + requestId?: string; + /** If true, bypasses the version check (for admin overrides). */ + force?: boolean; +} + +/** + * Checks if an update operation can proceed by comparing client and database entity versions. + * + * @returns A successful ApiResponse if the update is allowed, or a 409 Conflict error response if not. + */ +export function checkVersion( + options: UpdateOptions, +): ApiResponse { + const { clientEntity, dbEntity, actor, requestId, force = false } = options; + + if (force) { + logger.warn( + { + actor, + entityId: dbEntity.id, + clientVersion: clientEntity.version, + dbVersion: dbEntity.version, + requestId, + }, + 'OCC check bypassed with force=true', + ); + return ok(undefined, requestId); + } + + if (clientEntity.version !== dbEntity.version) { + logger.warn( + { + actor, + entityId: dbEntity.id, + clientVersion: clientEntity.version, + dbVersion: dbEntity.version, + requestId, + }, + 'OCC conflict detected: version mismatch', + ); + return fail( + 'CONFLICT_VERSION_MISMATCH', + `The resource was updated by another process. Please refresh and try again.`, + requestId, + { version: dbEntity.version }, + ); + } + + return ok(undefined, requestId); +} + +/** + * Executes a version-checked update. + * + * @param updateFn A function that performs the database update. It receives the new version number. + * It should return the updated entity or null/undefined if the update fails. + * @returns The result of the update function, or a conflict error. + */ +export async function withOptimisticLock( + options: UpdateOptions, + updateFn: (newVersion: number) => Promise, +): Promise> { + const versionCheckResult = checkVersion(options); + if (!versionCheckResult.success) { + return versionCheckResult; + } + + const newVersion = options.dbEntity.version + 1; + + try { + const result = await updateFn(newVersion); + // Assuming the update function returns null if the DB update fails (e.g., row count 0) + return result ? ok(result, options.requestId) : fromError(new Error('Update failed'), options.requestId); + } catch (err) { + return fromError(err, options.requestId); + } +} \ No newline at end of file diff --git a/backend/services/shared/occ/__tests__/OptimisticLockService.test.ts b/backend/services/shared/occ/__tests__/OptimisticLockService.test.ts new file mode 100644 index 00000000..c2864ce3 --- /dev/null +++ b/backend/services/shared/occ/__tests__/OptimisticLockService.test.ts @@ -0,0 +1,88 @@ +import { checkVersion, VersionedEntity } from '../OptimisticLockService'; + +describe('OptimisticLockService', () => { + const actor = { id: 'user-123', type: 'user' as const }; + + describe('checkVersion', () => { + it('should succeed if versions match', () => { + const clientEntity: VersionedEntity = { id: 'sub-1', version: 2 }; + const dbEntity: VersionedEntity = { id: 'sub-1', version: 2 }; + + const result = checkVersion({ clientEntity, dbEntity, actor }); + + expect(result.success).toBe(true); + }); + + it('should fail with 409 conflict if versions mismatch', () => { + const clientEntity: VersionedEntity = { id: 'sub-1', version: 1 }; + const dbEntity: VersionedEntity = { id: 'sub-1', version: 2 }; + + const result = checkVersion({ clientEntity, dbEntity, actor }); + + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.code).toBe('CONFLICT_VERSION_MISMATCH'); + expect(result.error.message).toContain('The resource was updated by another process.'); + expect(result.error.version).toBe(2); + expect(result.meta.apiVersion).toBe(1); + } + }); + + it('should succeed if force=true is used, even with version mismatch', () => { + const clientEntity: VersionedEntity = { id: 'sub-1', version: 1 }; + const dbEntity: VersionedEntity = { id: 'sub-1', version: 2 }; + + const result = checkVersion({ clientEntity, dbEntity, actor, force: true }); + + expect(result.success).toBe(true); + }); + + it('should include requestId in meta for both success and failure', () => { + const requestId = 'test-request-id'; + + // Success case + const successResult = checkVersion({ + clientEntity: { id: 'sub-1', version: 1 }, + dbEntity: { id: 'sub-1', version: 1 }, + actor, + requestId, + }); + expect(successResult.meta.requestId).toBe(requestId); + + // Failure case + const failureResult = checkVersion({ + clientEntity: { id: 'sub-1', version: 1 }, + dbEntity: { id: 'sub-1', version: 2 }, + actor, + requestId, + }); + expect(failureResult.meta.requestId).toBe(requestId); + }); + + it('should handle a complex entity type', () => { + interface Subscription extends VersionedEntity { + name: string; + status: 'active' | 'paused'; + } + + const clientEntity: Subscription = { + id: 'sub-1', + version: 3, + name: 'New Name', + status: 'paused', + }; + const dbEntity: Subscription = { + id: 'sub-1', + version: 4, + name: 'Old Name', + status: 'active', + }; + + const result = checkVersion({ clientEntity, dbEntity, actor }); + expect(result.success).toBe(false); + if (!result.success) { + expect(result.error.version).toBe(4); + } + }); + }); +}); \ No newline at end of file diff --git a/mobile/app/services/conflictResolutionService.ts b/mobile/app/services/conflictResolutionService.ts new file mode 100644 index 00000000..c57f6c37 --- /dev/null +++ b/mobile/app/services/conflictResolutionService.ts @@ -0,0 +1,124 @@ +/** + * @file conflictResolutionService.ts + * @description Issue #613 - Client-side service for handling OCC conflicts. + * + * This service provides a wrapper for API mutation functions to automatically + * handle 409 version conflicts with a retry mechanism. + */ + +import { create } from 'zustand'; +import { ApiErrorResponse } from '../../../backend/services/shared/apiResponse'; + + +export interface ConflictState { + entityId: string | number; + /** The user's attempted changes that were rejected. */ + localState: T; + /** The state of the entity on the server that caused the conflict. */ + remoteState: T; + /** The error response from the server. */ + error: ApiErrorResponse; +} + +interface ConflictStore { + conflict: ConflictState | null; + resolve: (conflict: ConflictState | null) => void; +} + +// A generic Zustand store for managing a single, active conflict. +// In a real app, you might want a map of conflicts by entityId. +export const useConflictStore = create>((set) => ({ + conflict: null, + resolve: (conflict) => set({ conflict }), +})); + +export interface RetryOptions { + /** The mutation function to wrap. It must accept the entity to save. */ + mutationFn: (entity: T) => Promise; + /** A function to fetch the latest version of the entity from the server. */ + fetchLatestFn: (id: string | number) => Promise; + /** The initial entity state being submitted by the user. */ + entity: T & { id: string | number; version: number }; + /** Maximum number of retry attempts. Defaults to 3. */ + maxRetries?: number; + /** Initial backoff delay in ms. Defaults to 100. */ + initialBackoffMs?: number; + /** Optional callback for when retries are exhausted and manual resolution is required. */ + onConflictResolved?: (conflict: ConflictState) => void; +} + +/** + * Wraps a mutation function with automatic retry logic for OCC conflicts. + * If all retries fail, it populates the conflict store for manual resolution. + */ +export async function withConflictResolution( + options: RetryOptions, +): Promise { + const { + mutationFn, + fetchLatestFn, + entity, + maxRetries = 3, + initialBackoffMs = 100, + onConflictResolved, + } = options; + + let lastError: ApiErrorResponse | null = null; + let currentEntity = entity; + + for (let attempt = 0; attempt < maxRetries; attempt++) { + const response = await mutationFn(currentEntity); + + if (response.success) { + return response; + } + + lastError = response; + + // Check if it's a version conflict error + if (response.error.code === 'CONFLICT_VERSION_MISMATCH' && response.error.version !== undefined) { + // It's a conflict, try to fetch the latest version and retry + console.log(`Attempt ${attempt + 1}: Conflict detected. Retrying...`); + + // Exponential backoff + if (attempt > 0) { + const backoff = initialBackoffMs * Math.pow(2, attempt); + await new Promise((resolve) => setTimeout(resolve, backoff)); + } + + try { + const latestEntity = await fetchLatestFn(entity.id); + // Merge user's changes onto the new base version + currentEntity = { ...latestEntity, ...entity, version: latestEntity.version }; + continue; // Retry the loop + } catch (fetchError) { + console.error('Failed to fetch latest entity for conflict resolution:', fetchError); + // If fetching the latest fails, we can't proceed automatically. + break; + } + } else { + // Not a conflict error, so fail immediately + return response; + } + } + + // If all retries are exhausted, set the conflict state for the UI to handle + if (lastError && lastError.error.code === 'CONFLICT_VERSION_MISMATCH') { + try { + const remoteState = await fetchLatestFn(entity.id); + const conflict: ConflictState = { + entityId: entity.id, + localState: entity, + remoteState: remoteState, + error: lastError, + }; + // Use the callback if provided, otherwise fall back to the global store + onConflictResolved ? onConflictResolved(conflict) : useConflictStore.getState().resolve(conflict); + } catch (fetchError) { + console.error('Failed to fetch latest entity for manual conflict resolution:', fetchError); + // Return the original error as we cannot construct the full conflict state + } + } + + return lastError!; +} \ No newline at end of file From 8c2cd804ad4c19a7766b6080102af00543ddd8f3 Mon Sep 17 00:00:00 2001 From: whitezaddy Date: Thu, 25 Jun 2026 23:24:10 +0100 Subject: [PATCH 2/2] fix(lint): resolve prettier, unused var, and path errors Clears out multiple workspace errors: - Fixes Prettier layout/syntax issues in crdt.ts and .storybook/main.js. - Resolves import/export name collisions in the design-system barrel file. - Corrects broken relative import paths in test files. - Removes numerous unused variables and imports flagged by the linter. --- .storybook/main.js | 9 +-- sandbox/services/usageTrackingService.ts | 2 +- shared/types/crdt.ts | 23 ++++---- .../__tests__/Button.accessibility.test.tsx | 31 +++++------ .../__tests__/visualRegression.e2e.ts | 9 +-- src/design-system/components/Modal.tsx | 12 ++-- src/design-system/index.ts | 55 ++----------------- src/design-system/utils/fontScaling.ts | 2 - src/hooks/useFocusManagement.ts | 1 - src/services/auth/deviceAttestationService.ts | 34 ++++++------ src/store/invoiceStore.ts | 2 - src/store/subscriptionStore.ts | 3 +- src/store/transactionStore.ts | 8 +-- src/utils/__tests__/accessibility.test.ts | 19 +++---- 14 files changed, 65 insertions(+), 145 deletions(-) diff --git a/.storybook/main.js b/.storybook/main.js index d3e1a940..7f97869d 100644 --- a/.storybook/main.js +++ b/.storybook/main.js @@ -1,15 +1,12 @@ /** * Storybook Configuration for SubTrackr Design System - * + * * Location: .storybook/main.js * Run: npm run storybook */ module.exports = { - stories: [ - '../src/design-system/stories/**/*.stories.{ts,tsx}', - '../src/**/*.stories.{ts,tsx}', - ], + stories: ['../src/design-system/stories/**/*.stories.{ts,tsx}', '../src/**/*.stories.{ts,tsx}'], addons: [ '@storybook/addon-essentials', '@storybook/addon-ondevice-actions', @@ -30,7 +27,7 @@ module.exports = { reactDocgenTypescriptOptions: { shouldExtractLiteralValuesAsTypes: true, shouldRemoveUndefinedFromOptional: true, - propFilter: (prop: any) => { + propFilter: (prop) => { if (prop.parent) { return !prop.parent.fileName.includes('node_modules'); } diff --git a/sandbox/services/usageTrackingService.ts b/sandbox/services/usageTrackingService.ts index ee16da62..dc9cbaa0 100644 --- a/sandbox/services/usageTrackingService.ts +++ b/sandbox/services/usageTrackingService.ts @@ -1,4 +1,4 @@ -import { UsageMetrics, HourlyUsage, DailyUsage } from '../types/sandbox'; +import { UsageMetrics } from '../types/sandbox'; export class UsageTrackingService { private usageData: Map = new Map(); diff --git a/shared/types/crdt.ts b/shared/types/crdt.ts index 2943cefe..bdb7f5ea 100644 --- a/shared/types/crdt.ts +++ b/shared/types/crdt.ts @@ -79,10 +79,7 @@ export interface LWWRegister { } /** Merge two LWW-Registers: the one with the later timestamp wins. */ -export function mergeLWWRegister( - local: LWWRegister, - remote: LWWRegister, -): LWWRegister { +export function mergeLWWRegister(local: LWWRegister, remote: LWWRegister): LWWRegister { if (remote.timestamp > local.timestamp) return remote; if (remote.timestamp < local.timestamp) return local; // Tie-break by nodeId for determinism @@ -96,7 +93,7 @@ export function mergeLWWRegister( * Each element has a unique tag (UUID) to distinguish concurrent add/remove. * An element is in the set if any of its tags are in `added` but not in `removed`. */ -export interface ORSet { +export interface ORSet { /** Map from element key (serialised) to a set of unique add-tags. */ added: Record; /** Set of removed tags. */ @@ -104,7 +101,7 @@ export interface ORSet { } /** Add an element to an OR-Set, generating a unique tag. */ -export function orSetAdd(set: ORSet, key: string, tag: string): ORSet { +export function orSetAdd(set: ORSet, key: string, tag: string): ORSet { const existing = set.added[key] ?? []; return { ...set, @@ -113,7 +110,7 @@ export function orSetAdd(set: ORSet, key: string, tag: string): ORSet { } /** Remove an element from an OR-Set by marking all its current tags as removed. */ -export function orSetRemove(set: ORSet, key: string): ORSet { +export function orSetRemove(set: ORSet, key: string): ORSet { const tags = set.added[key] ?? []; return { ...set, @@ -122,18 +119,18 @@ export function orSetRemove(set: ORSet, key: string): ORSet { } /** Check whether an element is in the OR-Set. */ -export function orSetContains(set: ORSet, key: string): boolean { +export function orSetContains(set: ORSet, key: string): boolean { const tags = set.added[key] ?? []; return tags.some((tag) => !set.removed.includes(tag)); } /** Return all keys currently in the OR-Set. */ -export function orSetValues(set: ORSet): string[] { - return Object.keys(set.added).filter((key) => orSetContains(set, key)); +export function orSetValues(set: ORSet): string[] { + return Object.keys(set.added).filter((key) => orSetContains(set, key)); } /** Merge two OR-Sets: union of added tags, union of removed tags. */ -export function mergeORSet(local: ORSet, remote: ORSet): ORSet { +export function mergeORSet(local: ORSet, remote: ORSet): ORSet { const mergedAdded: Record = { ...local.added }; for (const [key, tags] of Object.entries(remote.added)) { const localTags = mergedAdded[key] ?? []; @@ -207,7 +204,7 @@ export function lwwMapSet( key: string, value: T, nodeId: string, - clock: VectorClock, + clock: VectorClock ): LWWMap { return { ...map, @@ -266,7 +263,7 @@ export interface EntityCRDTState { /** LWW scalar fields. */ lwwFields: LWWMap; /** OR-Set collections (keyed by field name). */ - orSets: Record>; + orSets: Record; /** PN-Counters (keyed by field name). */ counters: Record; /** Merged vector clock across all fields. */ diff --git a/src/components/common/__tests__/Button.accessibility.test.tsx b/src/components/common/__tests__/Button.accessibility.test.tsx index d1c97c6b..140d2ac3 100644 --- a/src/components/common/__tests__/Button.accessibility.test.tsx +++ b/src/components/common/__tests__/Button.accessibility.test.tsx @@ -5,42 +5,37 @@ import React from 'react'; import { render } from '@testing-library/react-native'; import { Button } from '../Button'; -import { runAccessibilityChecks, expectNoAccessibilityViolations } from '../../utils/__tests__/accessibility.test'; +import { + runAccessibilityChecks, + expectNoAccessibilityViolations, +} from '../../../utils/__tests__/accessibility.test'; describe('Button Accessibility', () => { it('should have no accessibility violations with minimal props', () => { const component =