From 680661c70460b17cdb0fc34b42c1f510bd105656 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Wed, 7 Jan 2026 17:08:55 -0500 Subject: [PATCH 01/14] feat(listing): implement expired listing deletion timer (#200) ## Summary Implements automatic deletion of expired listings after 6 months archival period, following Azure Functions timer trigger pattern. ## Changes ### Timer Handler - Added `expired-listing-deletion-handler.ts` - Azure Functions timer that runs daily at 2 AM - Integrated OpenTelemetry tracing for observability ### Application Services - Added `processExpiredDeletions` service to batch delete expired/cancelled listings older than 6 months - Added `deleteByListing` service to clean up related conversations - Added `forSystemTask()` factory method for system-level operations without user context ### Domain Layer - Added `requestDelete()` method to Conversation aggregate (following ItemListing pattern) - Added `deleteBlob()` method to BlobStorage service interface ### Persistence Layer - Added `getExpiredForDeletion()` to ItemListingReadRepository - Added `getByListingId()` to ConversationReadRepository ### Infrastructure - Added `@azure/storage-blob` to service-blob-storage for blob cleanup - Added `@sthrift/domain` dependency to context-spec ## Testing - Comprehensive unit tests for all new services - Feature files for BDD testing - Timer handler tests with OpenTelemetry mocks Closes #200 --- apps/api/src/cellix.ts | 57 ++- apps/api/src/index.ts | 11 + .../expired-listing-deletion-handler.test.ts | 122 ++++++ .../expired-listing-deletion-handler.ts | 47 +++ .../conversation/delete-by-listing.test.ts | 203 ++++++++++ .../conversation/delete-by-listing.ts | 50 +++ .../features/delete-by-listing.feature | 30 ++ .../conversation/conversation/index.ts | 6 + .../src/contexts/listing/index.ts | 19 +- .../process-expired-deletions.feature | 51 +++ .../src/contexts/listing/item/index.ts | 18 +- .../item/process-expired-deletions.test.ts | 367 ++++++++++++++++++ .../listing/item/process-expired-deletions.ts | 113 ++++++ .../sthrift/application-services/src/index.ts | 18 +- packages/sthrift/context-spec/package.json | 1 + packages/sthrift/context-spec/src/index.ts | 2 + .../conversation/conversation/conversation.ts | 15 + .../src/domain/services/blob-storage.ts | 1 + packages/sthrift/graphql/src/init/handler.ts | 6 +- .../conversation.read-repository.test.ts | 91 ++++- .../conversation.read-repository.ts | 29 ++ .../conversation.read-repository.feature | 19 + .../item-listing.read-repository.feature | 14 + .../item/item-listing.read-repository.test.ts | 106 +++++ .../item/item-listing.read-repository.ts | 26 ++ packages/sthrift/rest/src/index.ts | 6 +- .../sthrift/service-blob-storage/package.json | 1 + .../sthrift/service-blob-storage/src/index.ts | 14 + pnpm-lock.yaml | 95 ++++- 29 files changed, 1512 insertions(+), 26 deletions(-) create mode 100644 apps/api/src/timers/expired-listing-deletion-handler.test.ts create mode 100644 apps/api/src/timers/expired-listing-deletion-handler.ts create mode 100644 packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts create mode 100644 packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts create mode 100644 packages/sthrift/application-services/src/contexts/conversation/conversation/features/delete-by-listing.feature create mode 100644 packages/sthrift/application-services/src/contexts/listing/item/features/process-expired-deletions.feature create mode 100644 packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts create mode 100644 packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index 0d1db292f..9390183ad 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -1,4 +1,4 @@ -import { app, type HttpFunctionOptions, type HttpHandler } from '@azure/functions'; +import { app, type HttpFunctionOptions, type HttpHandler, type TimerHandler } from '@azure/functions'; import type { ServiceBase } from '@cellix/api-services-spec'; import api, { SpanStatusCode, type Tracer, trace } from '@opentelemetry/api'; @@ -103,6 +103,28 @@ interface AzureFunctionHandlerRegistry, ) => HttpHandler, ): AzureFunctionHandlerRegistry; + /** + * Registers an Azure Function Timer endpoint. + * + * @remarks + * The `handlerCreator` is invoked when the timer fires and receives the application services host. + * Use it to create a handler for scheduled tasks like cleanup, maintenance, etc. + * Registration is allowed in phases `'app-services'` and `'handlers'`. + * + * @param name - Function name to bind in Azure Functions. + * @param schedule - NCRONTAB expression for the timer schedule. + * @param handlerCreator - Factory that, given the app services host, returns a `TimerHandler`. + * @returns The registry (for chaining). + * + * @throws Error - If called before application services are initialized. + */ + registerAzureFunctionTimerHandler( + name: string, + schedule: string, + handlerCreator: ( + applicationServicesHost: AppHost, + ) => TimerHandler, + ): AzureFunctionHandlerRegistry; /** * Finalizes configuration and starts the application. * @@ -167,6 +189,12 @@ interface PendingHandler { handlerCreator: (applicationServicesHost: AppHost) => HttpHandler; } +interface PendingTimerHandler { + name: string; + schedule: string; + handlerCreator: (applicationServicesHost: AppHost) => TimerHandler; +} + type Phase = 'infrastructure' | 'context' | 'app-services' | 'handlers' | 'started'; /** @@ -192,6 +220,7 @@ export class Cellix private readonly tracer: Tracer; private readonly servicesInternal: Map, ServiceBase> = new Map(); private readonly pendingHandlers: Array> = []; + private readonly pendingTimerHandlers: Array> = []; private serviceInitializedInternal = false; private phase: Phase = 'infrastructure'; @@ -283,6 +312,19 @@ export class Cellix return this; } + public registerAzureFunctionTimerHandler( + name: string, + schedule: string, + handlerCreator: ( + applicationServicesHost: RequestScopedHost, + ) => TimerHandler, + ): AzureFunctionHandlerRegistry { + this.ensurePhase('app-services', 'handlers'); + this.pendingTimerHandlers.push({ name, schedule, handlerCreator }); + this.phase = 'handlers'; + return this; + } + public startUp(): Promise> { this.ensurePhase('handlers', 'app-services'); if (!this.contextCreatorInternal) { @@ -307,6 +349,19 @@ export class Cellix }); } + // Register timer handlers + for (const t of this.pendingTimerHandlers) { + app.timer(t.name, { + schedule: t.schedule, + handler: (timer, context) => { + if (!this.appServicesHostInternal) { + throw new Error('Application not started yet'); + } + return t.handlerCreator(this.appServicesHostInternal)(timer, context); + }, + }); + } + // appStart hook app.hook.appStart(async () => { const root = api.context.active(); diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 5fca8822f..719e714a7 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -23,6 +23,7 @@ import { ServiceMessagingMock } from '@sthrift/messaging-service-mock'; import { graphHandlerCreator } from '@sthrift/graphql'; import { restHandlerCreator } from '@sthrift/rest'; +import { expiredListingDeletionHandlerCreator } from './timers/expired-listing-deletion-handler.ts'; import type {PaymentService} from '@cellix/payment-service'; import { PaymentServiceMock } from '@sthrift/payment-service-mock'; @@ -69,6 +70,10 @@ Cellix.initializeInfrastructureServices( ? serviceRegistry.getInfrastructureService(PaymentServiceMock) : serviceRegistry.getInfrastructureService(PaymentServiceCybersource); + const blobStorageService = serviceRegistry.getInfrastructureService( + ServiceBlobStorage, + ); + const { domainDataSource } = dataSourcesFactory.withSystemPassport(); RegisterEventHandlers(domainDataSource); @@ -80,6 +85,7 @@ Cellix.initializeInfrastructureServices( ), paymentService, messagingService, + blobStorageService, }; }) .initializeApplicationServices((context) => @@ -98,4 +104,9 @@ Cellix.initializeInfrastructureServices( { route: '{communityId}/{role}/{memberId}/{*rest}' }, restHandlerCreator, ) + .registerAzureFunctionTimerHandler( + 'processExpiredListingDeletions', + '0 0 2 * * *', + expiredListingDeletionHandlerCreator, + ) .startUp(); diff --git a/apps/api/src/timers/expired-listing-deletion-handler.test.ts b/apps/api/src/timers/expired-listing-deletion-handler.test.ts new file mode 100644 index 000000000..5adc76510 --- /dev/null +++ b/apps/api/src/timers/expired-listing-deletion-handler.test.ts @@ -0,0 +1,122 @@ +import { describe, it, expect, vi, beforeEach } from 'vitest'; +import type { Timer, InvocationContext } from '@azure/functions'; +import type { ApplicationServicesFactory } from '@sthrift/application-services'; +import { expiredListingDeletionHandlerCreator } from './expired-listing-deletion-handler.ts'; + +// Mock OpenTelemetry +vi.mock('@opentelemetry/api', () => ({ + trace: { + getTracer: () => ({ + startActiveSpan: vi.fn(async (_name, callback) => { + const mockSpan = { + setAttribute: vi.fn(), + setStatus: vi.fn(), + recordException: vi.fn(), + end: vi.fn(), + }; + return await callback(mockSpan); + }), + }), + }, + SpanStatusCode: { + OK: 1, + ERROR: 2, + }, +})); + +describe('expiredListingDeletionHandler', () => { + let mockContext: InvocationContext; + let mockTimer: Timer; + let mockFactory: ApplicationServicesFactory; + let mockProcessExpiredDeletions: ReturnType; + + beforeEach(() => { + mockContext = { + log: vi.fn(), + error: vi.fn(), + } as unknown as InvocationContext; + + mockTimer = { + isPastDue: false, + schedule: { + adjustForDST: false, + }, + scheduleStatus: { + last: new Date().toISOString(), + next: new Date().toISOString(), + lastUpdated: new Date().toISOString(), + }, + }; + + mockProcessExpiredDeletions = vi.fn().mockResolvedValue({ + deletedCount: 5, + deletedListingIds: ['id1', 'id2', 'id3', 'id4', 'id5'], + deletedConversationsCount: 10, + deletedImagesCount: 15, + errors: [], + }); + + mockFactory = { + forSystemTask: vi.fn().mockReturnValue({ + Listing: { + ItemListing: { + processExpiredDeletions: mockProcessExpiredDeletions, + }, + }, + }), + } as unknown as ApplicationServicesFactory; + }); + + it('should call processExpiredDeletions and log success', async () => { + const handler = expiredListingDeletionHandlerCreator(mockFactory); + await handler(mockTimer, mockContext); + + expect(mockFactory.forSystemTask).toHaveBeenCalledOnce(); + expect(mockProcessExpiredDeletions).toHaveBeenCalledOnce(); + expect(mockContext.log).toHaveBeenCalledWith('ExpiredListingDeletion: Timer triggered'); + expect(mockContext.log).toHaveBeenCalledWith( + 'ExpiredListingDeletion: Completed - 5 deleted, 0 errors', + ); + }); + + it('should log past due message when timer is past due', async () => { + mockTimer.isPastDue = true; + const handler = expiredListingDeletionHandlerCreator(mockFactory); + await handler(mockTimer, mockContext); + + expect(mockContext.log).toHaveBeenCalledWith('ExpiredListingDeletion: Timer is past due'); + }); + + it('should log errors when processExpiredDeletions returns errors', async () => { + const errors = [{ listingId: 'failed-1', error: 'Delete failed' }]; + mockProcessExpiredDeletions.mockResolvedValue({ + deletedCount: 2, + deletedListingIds: ['id1', 'id2'], + deletedConversationsCount: 4, + deletedImagesCount: 6, + errors, + }); + + const handler = expiredListingDeletionHandlerCreator(mockFactory); + await handler(mockTimer, mockContext); + + expect(mockContext.log).toHaveBeenCalledWith( + 'ExpiredListingDeletion: Completed - 2 deleted, 1 errors', + ); + expect(mockContext.log).toHaveBeenCalledWith( + `ExpiredListingDeletion: Errors: ${JSON.stringify(errors)}`, + ); + }); + + it('should throw and log error when processExpiredDeletions fails', async () => { + const testError = new Error('Database connection failed'); + mockProcessExpiredDeletions.mockRejectedValue(testError); + + const handler = expiredListingDeletionHandlerCreator(mockFactory); + + await expect(handler(mockTimer, mockContext)).rejects.toThrow('Database connection failed'); + expect(mockContext.error).toHaveBeenCalledWith( + 'ExpiredListingDeletion: Failed - Database connection failed', + ); + }); +}); diff --git a/apps/api/src/timers/expired-listing-deletion-handler.ts b/apps/api/src/timers/expired-listing-deletion-handler.ts new file mode 100644 index 000000000..7ee52e86d --- /dev/null +++ b/apps/api/src/timers/expired-listing-deletion-handler.ts @@ -0,0 +1,47 @@ +import type { TimerHandler } from '@azure/functions'; +import type { ApplicationServicesFactory, AppServicesHost, ApplicationServices } from '@sthrift/application-services'; +import { trace, SpanStatusCode } from '@opentelemetry/api'; + +const tracer = trace.getTracer('timer:expired-listing-deletion'); + +export const expiredListingDeletionHandlerCreator = ( + applicationServicesHost: AppServicesHost, +): TimerHandler => { + return async (timer, context) => { + await tracer.startActiveSpan('processExpiredListingDeletions', async (span) => { + try { + context.log('ExpiredListingDeletion: Timer triggered'); + + if (timer.isPastDue) { + context.log('ExpiredListingDeletion: Timer is past due'); + } + + const factory = applicationServicesHost as ApplicationServicesFactory; + const systemServices = factory.forSystemTask(); + const result = await systemServices.Listing.ItemListing.processExpiredDeletions(); + + span.setAttribute('deletedCount', result.deletedCount); + span.setAttribute('errorCount', result.errors.length); + + context.log( + `ExpiredListingDeletion: Completed - ${result.deletedCount} deleted, ${result.errors.length} errors`, + ); + + if (result.errors.length > 0) { + context.log(`ExpiredListingDeletion: Errors: ${JSON.stringify(result.errors)}`); + } + + span.setStatus({ code: SpanStatusCode.OK }); + } catch (error) { + span.setStatus({ code: SpanStatusCode.ERROR }); + if (error instanceof Error) { + span.recordException(error); + context.error(`ExpiredListingDeletion: Failed - ${error.message}`); + } + throw error; + } finally { + span.end(); + } + }); + }; +}; diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts new file mode 100644 index 000000000..d3b7666db --- /dev/null +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts @@ -0,0 +1,203 @@ +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describeFeature, loadFeature } from '@amiceli/vitest-cucumber'; +import { expect, vi } from 'vitest'; +import type { DataSources } from '@sthrift/persistence'; +import { deleteByListing } from './delete-by-listing.ts'; + +const test = { for: describeFeature }; +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const feature = await loadFeature( + path.resolve(__dirname, 'features/delete-by-listing.feature'), +); + +test.for(feature, ({ Background, Scenario }) => { + let mockDataSources: DataSources; + let mockConversations: Array<{ id: string }>; + let mockConversationRepo: { + get: ReturnType; + save: ReturnType; + }; + let mockConversationUow: { + withScopedTransaction: ReturnType; + }; + let shouldFailFirstConversation: boolean; + let deleteFunction: (listingId: string) => Promise<{ + deletedCount: number; + deletedConversationIds: string[]; + errors: Array<{ conversationId: string; error: string }>; + }>; + let result: { + deletedCount: number; + deletedConversationIds: string[]; + errors: Array<{ conversationId: string; error: string }>; + }; + + Background(({ Given, And }) => { + Given('the data sources are available', () => { + shouldFailFirstConversation = false; + mockConversations = []; + }); + + And('the conversation repository is available', () => { + let callCount = 0; + + mockConversationRepo = { + get: vi.fn().mockImplementation(() => ({ + requestDelete: vi.fn(), + })), + save: vi.fn().mockResolvedValue(undefined), + }; + + mockConversationUow = { + withScopedTransaction: vi.fn((callback) => { + callCount++; + if (shouldFailFirstConversation && callCount === 1) { + return Promise.reject(new Error('Failed to delete conversation')); + } + return callback(mockConversationRepo); + }), + }; + + mockDataSources = { + domainDataSource: { + Conversation: { + Conversation: { + ConversationUnitOfWork: mockConversationUow, + }, + }, + }, + readonlyDataSource: { + Conversation: { + Conversation: { + ConversationReadRepo: { + getByListingId: vi.fn().mockImplementation(() => { + return Promise.resolve(mockConversations); + }), + }, + }, + }, + }, + } as unknown as DataSources; + + deleteFunction = deleteByListing(mockDataSources); + }); + }); + + Scenario( + 'Successfully deleting conversations when no conversations exist for listing', + ({ Given, When, Then, And }) => { + Given( + 'the listing with id {string} has no conversations', + () => { + mockConversations = []; + }, + ); + + When( + 'conversations are deleted for listing id {string}', + async () => { + result = await deleteFunction('listing-no-conversations'); + }, + ); + + Then('the result should show 0 deleted conversations', () => { + expect(result.deletedCount).toBe(0); + }); + + And('the result should have no errors', () => { + expect(result.errors).toHaveLength(0); + }); + }, + ); + + Scenario( + 'Successfully deleting a single conversation for a listing', + ({ Given, When, Then, And }) => { + Given( + 'the listing with id {string} has 1 conversation', + () => { + mockConversations = [{ id: 'conv-1' }]; + }, + ); + + When( + 'conversations are deleted for listing id {string}', + async () => { + result = await deleteFunction('listing-single-conv'); + }, + ); + + Then('the result should show 1 deleted conversations', () => { + expect(result.deletedCount).toBe(1); + }); + + And('the deleted conversation ids should be returned', () => { + expect(result.deletedConversationIds).toContain('conv-1'); + }); + }, + ); + + Scenario( + 'Successfully deleting multiple conversations for a listing', + ({ Given, When, Then, And }) => { + Given( + 'the listing with id {string} has 3 conversations', + () => { + mockConversations = [ + { id: 'conv-1' }, + { id: 'conv-2' }, + { id: 'conv-3' }, + ]; + }, + ); + + When( + 'conversations are deleted for listing id {string}', + async () => { + result = await deleteFunction('listing-multi-conv'); + }, + ); + + Then('the result should show 3 deleted conversations', () => { + expect(result.deletedCount).toBe(3); + }); + + And('all 3 conversation ids should be included in the result', () => { + expect(result.deletedConversationIds).toHaveLength(3); + expect(result.deletedConversationIds).toContain('conv-1'); + expect(result.deletedConversationIds).toContain('conv-2'); + expect(result.deletedConversationIds).toContain('conv-3'); + }); + }, + ); + + Scenario('Handling conversation deletion failure', ({ Given, When, Then, And }) => { + Given( + 'the listing with id {string} has 2 conversations', + () => { + mockConversations = [{ id: 'conv-1' }, { id: 'conv-2' }]; + }, + ); + + And('the first conversation deletion throws an error', () => { + shouldFailFirstConversation = true; + }); + + When( + 'conversations are deleted for listing id {string}', + async () => { + result = await deleteFunction('listing-error'); + }, + ); + + Then('the result should show 1 deleted conversations', () => { + expect(result.deletedCount).toBe(1); + }); + + And('the result errors should include 1 error', () => { + expect(result.errors).toHaveLength(1); + expect(result.errors[0].conversationId).toBe('conv-1'); + }); + }); +}); diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts new file mode 100644 index 000000000..ad25efa4d --- /dev/null +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts @@ -0,0 +1,50 @@ +import type { DataSources } from '@sthrift/persistence'; + +export interface DeleteByListingResult { + deletedCount: number; + deletedConversationIds: string[]; + errors: Array<{ conversationId: string; error: string }>; +} + +export const deleteByListing = (dataSources: DataSources) => { + return async (listingId: string): Promise => { + const result: DeleteByListingResult = { + deletedCount: 0, + deletedConversationIds: [], + errors: [], + }; + + const conversations = + await dataSources.readonlyDataSource.Conversation.Conversation.ConversationReadRepo.getByListingId( + listingId, + ); + + if (conversations.length === 0) { + return result; + } + + const uow = + dataSources.domainDataSource.Conversation.Conversation + .ConversationUnitOfWork; + + for (const conversation of conversations) { + const conversationId = conversation.id; + + try { + await uow.withScopedTransaction(async (repo) => { + const domainConversation = await repo.get(conversationId); + domainConversation.requestDelete(); + await repo.save(domainConversation); + }); + result.deletedCount++; + result.deletedConversationIds.push(conversationId); + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); + result.errors.push({ conversationId, error: errorMessage }); + } + } + + return result; + }; +}; diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/features/delete-by-listing.feature b/packages/sthrift/application-services/src/contexts/conversation/conversation/features/delete-by-listing.feature new file mode 100644 index 000000000..900dda228 --- /dev/null +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/features/delete-by-listing.feature @@ -0,0 +1,30 @@ +Feature: Delete Conversations By Listing + + Background: + Given the data sources are available + And the conversation repository is available + + Scenario: Successfully deleting conversations when no conversations exist for listing + Given the listing with id "listing-no-conversations" has no conversations + When conversations are deleted for listing id "listing-no-conversations" + Then the result should show 0 deleted conversations + And the result should have no errors + + Scenario: Successfully deleting a single conversation for a listing + Given the listing with id "listing-single-conv" has 1 conversation + When conversations are deleted for listing id "listing-single-conv" + Then the result should show 1 deleted conversations + And the deleted conversation ids should be returned + + Scenario: Successfully deleting multiple conversations for a listing + Given the listing with id "listing-multi-conv" has 3 conversations + When conversations are deleted for listing id "listing-multi-conv" + Then the result should show 3 deleted conversations + And all 3 conversation ids should be included in the result + + Scenario: Handling conversation deletion failure + Given the listing with id "listing-error" has 2 conversations + And the first conversation deletion throws an error + When conversations are deleted for listing id "listing-error" + Then the result should show 1 deleted conversations + And the result errors should include 1 error diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/index.ts b/packages/sthrift/application-services/src/contexts/conversation/conversation/index.ts index bfa308c36..88756c738 100644 --- a/packages/sthrift/application-services/src/contexts/conversation/conversation/index.ts +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/index.ts @@ -10,6 +10,10 @@ import { type ConversationSendMessageCommand, sendMessage, } from './send-message.ts'; +import { + type DeleteByListingResult, + deleteByListing, +} from './delete-by-listing.ts'; export interface ConversationApplicationService { create: ( @@ -26,6 +30,7 @@ export interface ConversationApplicationService { sendMessage: ( command: ConversationSendMessageCommand, ) => Promise; + deleteByListing: (listingId: string) => Promise; } export const Conversation = ( @@ -36,5 +41,6 @@ export const Conversation = ( queryById: queryById(dataSources), queryByUser: queryByUser(dataSources), sendMessage: sendMessage(dataSources), + deleteByListing: deleteByListing(dataSources), }; }; diff --git a/packages/sthrift/application-services/src/contexts/listing/index.ts b/packages/sthrift/application-services/src/contexts/listing/index.ts index 24fb9e58b..7b1ea28fd 100644 --- a/packages/sthrift/application-services/src/contexts/listing/index.ts +++ b/packages/sthrift/application-services/src/contexts/listing/index.ts @@ -1,17 +1,32 @@ +import type { Domain } from '@sthrift/domain'; import type { DataSources } from '@sthrift/persistence'; import { ItemListing as ItemListingApi, type ItemListingApplicationService, + type ItemListingDependencies, } from './item/index.ts'; export interface ListingContextApplicationService { ItemListing: ItemListingApplicationService; } +export interface ListingDependencies { + dataSources: DataSources; + blobStorage?: Domain.Services['BlobStorage']; +} + export const Listing = ( - dataSources: DataSources, + deps: DataSources | ListingDependencies, ): ListingContextApplicationService => { + const dataSources = 'dataSources' in deps ? deps.dataSources : deps; + const blobStorage = 'blobStorage' in deps ? deps.blobStorage : undefined; + + const itemListingDeps: ItemListingDependencies = { dataSources }; + if (blobStorage) { + itemListingDeps.blobStorage = blobStorage; + } + return { - ItemListing: ItemListingApi(dataSources), + ItemListing: ItemListingApi(itemListingDeps), }; }; diff --git a/packages/sthrift/application-services/src/contexts/listing/item/features/process-expired-deletions.feature b/packages/sthrift/application-services/src/contexts/listing/item/features/process-expired-deletions.feature new file mode 100644 index 000000000..d44a3e62e --- /dev/null +++ b/packages/sthrift/application-services/src/contexts/listing/item/features/process-expired-deletions.feature @@ -0,0 +1,51 @@ +Feature: Process Expired Listing Deletions + + Background: + Given the data sources are available + And the blob storage service is available + + Scenario: Successfully processing expired listings with no expired listings found + Given there are no expired listings + When the expired deletion process runs + Then the result should show 0 deleted listings + And the result should show 0 deleted conversations + And the result should show 0 deleted images + And the result should have no errors + + Scenario: Successfully deleting a single expired listing with images and conversations + Given there is 1 expired listing with id "listing-123" + And the listing has 2 images + And the listing has 3 conversations + When the expired deletion process runs + Then the result should show 1 deleted listings + And the result should show 3 deleted conversations + And the result should show 2 deleted images + And the deleted listing ids should include "listing-123" + + Scenario: Successfully deleting multiple expired listings + Given there are 3 expired listings + When the expired deletion process runs + Then the result should show 3 deleted listings + + Scenario: Handling image deletion failure gracefully + Given there is 1 expired listing with id "listing-456" + And the listing has 2 images + And the blob storage fails to delete the first image + When the expired deletion process runs + Then the result should show 1 deleted listings + And the result should show 1 deleted images + + Scenario: Handling listing deletion failure + Given there is 1 expired listing with id "listing-789" + And the listing deletion throws an error + When the expired deletion process runs + Then the result should show 0 deleted listings + And the result errors should include listing id "listing-789" + + Scenario: Processing expired listings without blob storage service + Given there is 1 expired listing with id "listing-no-blob" + And the listing has 2 images + And no blob storage service is provided + When the expired deletion process runs + Then the result should show 1 deleted listings + And the result should show 0 deleted images diff --git a/packages/sthrift/application-services/src/contexts/listing/item/index.ts b/packages/sthrift/application-services/src/contexts/listing/item/index.ts index 96bf35318..8801a2ed8 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/index.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/index.ts @@ -12,6 +12,10 @@ import { type ItemListingDeleteCommand, deleteListings } from './delete.ts'; import { type ItemListingUpdateCommand, update } from './update.ts'; import { type ItemListingUnblockCommand, unblock } from './unblock.ts'; import { queryPaged } from './query-paged.ts'; +import { + type ProcessExpiredDeletionsResult, + processExpiredDeletions, +} from './process-expired-deletions.ts'; export interface ItemListingApplicationService { create: ( @@ -51,11 +55,20 @@ export interface ItemListingApplicationService { page: number; pageSize: number; }>; + processExpiredDeletions: () => Promise; +} + +export interface ItemListingDependencies { + dataSources: DataSources; + blobStorage?: Domain.Services['BlobStorage']; } export const ItemListing = ( - dataSources: DataSources, + deps: DataSources | ItemListingDependencies, ): ItemListingApplicationService => { + const dataSources = 'dataSources' in deps ? deps.dataSources : deps; + const blobStorage = 'blobStorage' in deps ? deps.blobStorage : undefined; + return { create: create(dataSources), queryById: queryById(dataSources), @@ -63,8 +76,9 @@ export const ItemListing = ( queryAll: queryAll(dataSources), cancel: cancel(dataSources), update: update(dataSources), - deleteListings: deleteListings(dataSources), + deleteListings: deleteListings(dataSources), unblock: unblock(dataSources), queryPaged: queryPaged(dataSources), + processExpiredDeletions: processExpiredDeletions(dataSources, blobStorage), }; }; diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts new file mode 100644 index 000000000..4c2ed5b59 --- /dev/null +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts @@ -0,0 +1,367 @@ +import path from 'node:path'; +import { fileURLToPath } from 'node:url'; +import { describeFeature, loadFeature } from '@amiceli/vitest-cucumber'; +import { expect, vi } from 'vitest'; +import type { DataSources } from '@sthrift/persistence'; +import type { Domain } from '@sthrift/domain'; +import { processExpiredDeletions } from './process-expired-deletions.ts'; + +const test = { for: describeFeature }; +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const feature = await loadFeature( + path.resolve(__dirname, 'features/process-expired-deletions.feature'), +); + +test.for(feature, ({ Background, Scenario }) => { + let mockDataSources: DataSources; + let mockBlobStorage: Domain.Services['BlobStorage'] | undefined; + let mockExpiredListings: Array<{ + id: string; + images: string[]; + }>; + let mockConversations: Array<{ id: string }>; + let mockListingRepo: { + get: ReturnType; + save: ReturnType; + }; + let mockConversationRepo: { + get: ReturnType; + save: ReturnType; + }; + let mockListingUow: { + withScopedTransaction: ReturnType; + }; + let mockConversationUow: { + withScopedTransaction: ReturnType; + }; + let deleteBlobSpy: ReturnType; + let shouldFailListingDeletion: boolean; + let shouldFailBlobDeletion: boolean; + let result: { + deletedCount: number; + deletedListingIds: string[]; + deletedConversationsCount: number; + deletedImagesCount: number; + errors: Array<{ listingId: string; error: string }>; + }; + + Background(({ Given, And }) => { + Given('the data sources are available', () => { + shouldFailListingDeletion = false; + shouldFailBlobDeletion = false; + mockExpiredListings = []; + mockConversations = []; + + mockListingRepo = { + get: vi.fn().mockImplementation(() => ({ + requestDelete: vi.fn(), + })), + save: vi.fn().mockResolvedValue(undefined), + }; + + mockConversationRepo = { + get: vi.fn().mockImplementation(() => ({ + requestDelete: vi.fn(), + })), + save: vi.fn().mockResolvedValue(undefined), + }; + + mockListingUow = { + withScopedTransaction: vi.fn((callback) => { + if (shouldFailListingDeletion) { + return Promise.reject(new Error('Failed to delete listing')); + } + return callback(mockListingRepo); + }), + }; + + mockConversationUow = { + withScopedTransaction: vi.fn((callback) => { + return callback(mockConversationRepo); + }), + }; + + mockDataSources = { + domainDataSource: { + Listing: { + ItemListing: { + ItemListingUnitOfWork: mockListingUow, + }, + }, + Conversation: { + Conversation: { + ConversationUnitOfWork: mockConversationUow, + }, + }, + }, + readonlyDataSource: { + Listing: { + ItemListing: { + ItemListingReadRepo: { + getExpiredForDeletion: vi.fn().mockImplementation(() => { + return Promise.resolve(mockExpiredListings); + }), + }, + }, + }, + Conversation: { + Conversation: { + ConversationReadRepo: { + getByListingId: vi.fn().mockImplementation(() => { + return Promise.resolve(mockConversations); + }), + }, + }, + }, + }, + } as unknown as DataSources; + }); + + And('the blob storage service is available', () => { + let callCount = 0; + deleteBlobSpy = vi.fn().mockImplementation(() => { + callCount++; + if (shouldFailBlobDeletion && callCount === 1) { + return Promise.reject(new Error('Blob delete failed')); + } + return Promise.resolve(); + }); + mockBlobStorage = { + deleteBlob: deleteBlobSpy, + } as unknown as Domain.Services['BlobStorage']; + }); + }); + + Scenario( + 'Successfully processing expired listings with no expired listings found', + ({ Given, When, Then, And }) => { + Given('there are no expired listings', () => { + mockExpiredListings = []; + }); + + When('the expired deletion process runs', async () => { + const processFunction = processExpiredDeletions( + mockDataSources, + mockBlobStorage, + ); + result = await processFunction(); + }); + + Then('the result should show 0 deleted listings', () => { + expect(result.deletedCount).toBe(0); + }); + + And('the result should show 0 deleted conversations', () => { + expect(result.deletedConversationsCount).toBe(0); + }); + + And('the result should show 0 deleted images', () => { + expect(result.deletedImagesCount).toBe(0); + }); + + And('the result should have no errors', () => { + expect(result.errors).toHaveLength(0); + }); + }, + ); + + Scenario( + 'Successfully deleting a single expired listing with images and conversations', + ({ Given, When, Then, And }) => { + Given( + 'there is 1 expired listing with id {string}', + (_, listingId: string) => { + mockExpiredListings = [ + { + id: listingId, + images: [], + }, + ]; + }, + ); + + And('the listing has 2 images', () => { + mockExpiredListings[0].images = ['image1.jpg', 'image2.jpg']; + }); + + And('the listing has 3 conversations', () => { + mockConversations = [ + { id: 'conv-1' }, + { id: 'conv-2' }, + { id: 'conv-3' }, + ]; + }); + + When('the expired deletion process runs', async () => { + const processFunction = processExpiredDeletions( + mockDataSources, + mockBlobStorage, + ); + result = await processFunction(); + }); + + Then('the result should show 1 deleted listings', () => { + expect(result.deletedCount).toBe(1); + }); + + And('the result should show 3 deleted conversations', () => { + expect(result.deletedConversationsCount).toBe(3); + }); + + And('the result should show 2 deleted images', () => { + expect(result.deletedImagesCount).toBe(2); + }); + + And( + 'the deleted listing ids should include {string}', + (_, listingId: string) => { + expect(result.deletedListingIds).toContain(listingId); + }, + ); + }, + ); + + Scenario( + 'Successfully deleting multiple expired listings', + ({ Given, When, Then }) => { + Given('there are 3 expired listings', () => { + mockExpiredListings = [ + { id: 'listing-1', images: [] }, + { id: 'listing-2', images: [] }, + { id: 'listing-3', images: [] }, + ]; + }); + + When('the expired deletion process runs', async () => { + const processFunction = processExpiredDeletions( + mockDataSources, + mockBlobStorage, + ); + result = await processFunction(); + }); + + Then('the result should show 3 deleted listings', () => { + expect(result.deletedCount).toBe(3); + }); + }, + ); + + Scenario( + 'Handling image deletion failure gracefully', + ({ Given, When, Then, And }) => { + Given( + 'there is 1 expired listing with id {string}', + (_, listingId: string) => { + mockExpiredListings = [ + { + id: listingId, + images: [], + }, + ]; + }, + ); + + And('the listing has 2 images', () => { + mockExpiredListings[0].images = ['image1.jpg', 'image2.jpg']; + }); + + And('the blob storage fails to delete the first image', () => { + shouldFailBlobDeletion = true; + }); + + When('the expired deletion process runs', async () => { + const processFunction = processExpiredDeletions( + mockDataSources, + mockBlobStorage, + ); + result = await processFunction(); + }); + + Then('the result should show 1 deleted listings', () => { + expect(result.deletedCount).toBe(1); + }); + + And('the result should show 1 deleted images', () => { + expect(result.deletedImagesCount).toBe(1); + }); + }, + ); + + Scenario('Handling listing deletion failure', ({ Given, When, Then, And }) => { + Given( + 'there is 1 expired listing with id {string}', + (_, listingId: string) => { + mockExpiredListings = [ + { + id: listingId, + images: [], + }, + ]; + }, + ); + + And('the listing deletion throws an error', () => { + shouldFailListingDeletion = true; + }); + + When('the expired deletion process runs', async () => { + const processFunction = processExpiredDeletions( + mockDataSources, + mockBlobStorage, + ); + result = await processFunction(); + }); + + Then('the result should show 0 deleted listings', () => { + expect(result.deletedCount).toBe(0); + }); + + And( + 'the result errors should include listing id {string}', + (_, listingId: string) => { + expect(result.errors.some((e) => e.listingId === listingId)).toBe(true); + }, + ); + }); + + Scenario( + 'Processing expired listings without blob storage service', + ({ Given, When, Then, And }) => { + Given( + 'there is 1 expired listing with id {string}', + (_, listingId: string) => { + mockExpiredListings = [ + { + id: listingId, + images: [], + }, + ]; + }, + ); + + And('the listing has 2 images', () => { + mockExpiredListings[0].images = ['image1.jpg', 'image2.jpg']; + }); + + And('no blob storage service is provided', () => { + mockBlobStorage = undefined; + }); + + When('the expired deletion process runs', async () => { + const processFunction = processExpiredDeletions( + mockDataSources, + mockBlobStorage, + ); + result = await processFunction(); + }); + + Then('the result should show 1 deleted listings', () => { + expect(result.deletedCount).toBe(1); + }); + + And('the result should show 0 deleted images', () => { + expect(result.deletedImagesCount).toBe(0); + }); + }, + ); +}); diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts new file mode 100644 index 000000000..50cc6e833 --- /dev/null +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts @@ -0,0 +1,113 @@ +import type { Domain } from '@sthrift/domain'; +import type { DataSources } from '@sthrift/persistence'; +import { deleteByListing as deleteConversationsByListing } from '../../conversation/conversation/delete-by-listing.ts'; + +const ARCHIVAL_MONTHS = 6; +const BATCH_SIZE = 100; +const BLOB_CONTAINER_NAME = 'listing-images'; + +export interface ProcessExpiredDeletionsResult { + deletedCount: number; + deletedListingIds: string[]; + deletedConversationsCount: number; + deletedImagesCount: number; + errors: Array<{ listingId: string; error: string }>; +} + +async function deleteListingImages( + blobStorage: Domain.Services['BlobStorage'], + images: string[], +): Promise { + let deletedCount = 0; + for (const imagePath of images) { + try { + await blobStorage.deleteBlob(BLOB_CONTAINER_NAME, imagePath); + deletedCount++; + } catch (error) { + console.warn(`[ExpiredDeletion] Failed to delete image ${imagePath}: ${error instanceof Error ? error.message : String(error)}`); + } + } + return deletedCount; +} + +async function deleteListingById( + uow: DataSources['domainDataSource']['Listing']['ItemListing']['ItemListingUnitOfWork'], + listingId: string, +): Promise { + await uow.withScopedTransaction(async (repo) => { + const domainListing = await repo.get(listingId); + domainListing.requestDelete(); + await repo.save(domainListing); + }); +} + +export const processExpiredDeletions = ( + dataSources: DataSources, + blobStorage?: Domain.Services['BlobStorage'], +): (() => Promise) => { + return async (): Promise => { + const result: ProcessExpiredDeletionsResult = { + deletedCount: 0, + deletedListingIds: [], + deletedConversationsCount: 0, + deletedImagesCount: 0, + errors: [], + }; + + const expiredListings = + await dataSources.readonlyDataSource.Listing.ItemListing.ItemListingReadRepo.getExpiredForDeletion( + ARCHIVAL_MONTHS, + BATCH_SIZE, + ); + + console.log( + `[ExpiredDeletion] Found ${expiredListings.length} listings eligible for deletion`, + ); + + if (expiredListings.length === 0) { + return result; + } + + const uow = + dataSources.domainDataSource.Listing.ItemListing.ItemListingUnitOfWork; + const deleteConversations = deleteConversationsByListing(dataSources); + + for (const listing of expiredListings) { + const listingId = listing.id; + + try { + if (blobStorage && listing.images && listing.images.length > 0) { + const imagesDeleted = await deleteListingImages(blobStorage, listing.images); + result.deletedImagesCount += imagesDeleted; + console.log(`[ExpiredDeletion] Deleted ${imagesDeleted} images for listing ${listingId}`); + } + + const conversationResult = await deleteConversations(listingId); + result.deletedConversationsCount += conversationResult.deletedCount; + if (conversationResult.errors.length > 0) { + console.warn( + `[ExpiredDeletion] Errors deleting conversations for listing ${listingId}: ${JSON.stringify(conversationResult.errors)}`, + ); + } + + await deleteListingById(uow, listingId); + result.deletedCount++; + result.deletedListingIds.push(listingId); + console.log(`[ExpiredDeletion] Deleted listing: ${listingId}`); + } catch (error) { + const errorMessage = + error instanceof Error ? error.message : String(error); + result.errors.push({ listingId, error: errorMessage }); + console.error( + `[ExpiredDeletion] Failed to delete listing ${listingId}: ${errorMessage}`, + ); + } + } + + console.log( + `[ExpiredDeletion] Completed: ${result.deletedCount} listings deleted, ${result.deletedConversationsCount} conversations deleted, ${result.deletedImagesCount} images deleted, ${result.errors.length} errors`, + ); + + return result; + }; +}; \ No newline at end of file diff --git a/packages/sthrift/application-services/src/index.ts b/packages/sthrift/application-services/src/index.ts index c935a792a..2398cf2f9 100644 --- a/packages/sthrift/application-services/src/index.ts +++ b/packages/sthrift/application-services/src/index.ts @@ -58,11 +58,18 @@ export interface VerifiedUser { export type PrincipalHints = Record; +export interface SystemTaskServices { + Listing: ListingContextApplicationService; + Conversation: ConversationContextApplicationService; +} + export interface AppServicesHost { forRequest(rawAuthHeader?: string, hints?: PrincipalHints): Promise; } -export type ApplicationServicesFactory = AppServicesHost; +export interface ApplicationServicesFactory extends AppServicesHost { + forSystemTask(): SystemTaskServices; +} export const buildApplicationServicesFactory = ( infrastructureServicesRegistry: ApiContextSpec, @@ -126,6 +133,15 @@ export const buildApplicationServicesFactory = ( return { forRequest, + forSystemTask: (): SystemTaskServices => { + const dataSources = + infrastructureServicesRegistry.dataSourcesFactory.withSystemPassport(); + const blobStorage = infrastructureServicesRegistry.blobStorageService; + return { + Listing: Listing({ dataSources, blobStorage }), + Conversation: Conversation(dataSources), + }; + }, }; }; diff --git a/packages/sthrift/context-spec/package.json b/packages/sthrift/context-spec/package.json index 429e01a86..e1bf3633f 100644 --- a/packages/sthrift/context-spec/package.json +++ b/packages/sthrift/context-spec/package.json @@ -21,6 +21,7 @@ }, "dependencies": { "@sthrift/persistence": "workspace:*", + "@sthrift/domain": "workspace:*", "@cellix/payment-service": "workspace:*", "@sthrift/service-token-validation": "workspace:*", "@cellix/messaging-service": "workspace:*" diff --git a/packages/sthrift/context-spec/src/index.ts b/packages/sthrift/context-spec/src/index.ts index 8e11070ac..2154e35c3 100644 --- a/packages/sthrift/context-spec/src/index.ts +++ b/packages/sthrift/context-spec/src/index.ts @@ -2,6 +2,7 @@ import type { DataSourcesFactory } from '@sthrift/persistence'; import type { TokenValidation } from '@sthrift/service-token-validation'; import type { PaymentService } from '@cellix/payment-service'; import type { MessagingService } from '@cellix/messaging-service'; +import type { Domain } from '@sthrift/domain'; export interface ApiContextSpec { //mongooseService:Exclude; @@ -9,4 +10,5 @@ export interface ApiContextSpec { tokenValidationService: TokenValidation; paymentService: PaymentService; messagingService: MessagingService; + blobStorageService: Domain.Services['BlobStorage']; } diff --git a/packages/sthrift/domain/src/domain/contexts/conversation/conversation/conversation.ts b/packages/sthrift/domain/src/domain/contexts/conversation/conversation/conversation.ts index 00937c369..3550ca841 100644 --- a/packages/sthrift/domain/src/domain/contexts/conversation/conversation/conversation.ts +++ b/packages/sthrift/domain/src/domain/contexts/conversation/conversation/conversation.ts @@ -192,4 +192,19 @@ export class Conversation get schemaVersion(): string { return this.props.schemaVersion; } + + requestDelete(): void { + if ( + !this.visa.determineIf( + (domainPermissions) => domainPermissions.canManageConversation, + ) + ) { + throw new DomainSeedwork.PermissionError( + 'You do not have permission to delete this conversation', + ); + } + if (!this.isDeleted) { + super.isDeleted = true; + } + } } diff --git a/packages/sthrift/domain/src/domain/services/blob-storage.ts b/packages/sthrift/domain/src/domain/services/blob-storage.ts index 2af557dd8..61169e67a 100644 --- a/packages/sthrift/domain/src/domain/services/blob-storage.ts +++ b/packages/sthrift/domain/src/domain/services/blob-storage.ts @@ -1,4 +1,5 @@ export interface BlobStorage { createValetKey(storageAccount: string, path: string, expiration: Date): Promise; + deleteBlob(containerName: string, blobPath: string): Promise; } \ No newline at end of file diff --git a/packages/sthrift/graphql/src/init/handler.ts b/packages/sthrift/graphql/src/init/handler.ts index 67c7660a1..9c1016cac 100644 --- a/packages/sthrift/graphql/src/init/handler.ts +++ b/packages/sthrift/graphql/src/init/handler.ts @@ -1,6 +1,6 @@ import { ApolloServer } from '@apollo/server'; import type { HttpHandler } from '@azure/functions'; -import type { ApplicationServicesFactory, PrincipalHints } from '@sthrift/application-services'; +import type { AppServicesHost, ApplicationServices, PrincipalHints } from '@sthrift/application-services'; import { type AzureFunctionsMiddlewareOptions, startServerAndCreateHandler, @@ -16,7 +16,7 @@ const isProduction = process.env['NODE_ENV'] === 'production'; const MAX_QUERY_DEPTH = 10; export const graphHandlerCreator = ( - applicationServicesFactory: ApplicationServicesFactory, + applicationServicesHost: AppServicesHost, ): HttpHandler => { // Set up Apollo Server with security configurations // Note: Apollo Server v4 removed direct CORS support - CORS must be handled at the web framework level @@ -39,7 +39,7 @@ export const graphHandlerCreator = ( communityId: req.headers.get('x-community-id') ?? undefined, }; return { - applicationServices: await applicationServicesFactory.forRequest(authHeader, hints), + applicationServices: await applicationServicesHost.forRequest(authHeader, hints), }; }, }; diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts index fe77579dd..30b984a72 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts @@ -401,7 +401,7 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { Scenario('Getting conversation with invalid ObjectId format', ({ Given, When, Then }) => { Given('an invalid ObjectId will cause an error', () => { - // Mock MongooseSeedwork.ObjectId constructor to throw + // Mock MongooseSeedwork.ObjectId constructor to throw once, then restore vi.spyOn(MongooseSeedwork, 'ObjectId').mockImplementationOnce(() => { throw new Error('Invalid ObjectId'); }); @@ -413,10 +413,99 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { createValidObjectId('reserver'), createValidObjectId('listing') ); + // Restore all mocks after test to prevent interference with subsequent tests + vi.restoreAllMocks(); }); Then('it should return null due to ObjectId error', () => { expect(result).toBeNull(); }); }); + + Scenario('Getting conversations by listing ID', ({ Given, When, Then, And }) => { + Given('a Conversation document with listing "listing-1"', () => { + mockConversations = [ + makeMockConversation({ + listing: makeMockListing('listing-1'), + }), + ]; + // Update mock model to return new conversations + const createMockQuery = (queryResult: unknown) => { + const mockQuery = { + lean: vi.fn(), + populate: vi.fn(), + exec: vi.fn().mockResolvedValue(queryResult), + catch: vi.fn((onReject) => Promise.resolve(queryResult).catch(onReject)), + }; + mockQuery.lean.mockReturnValue(mockQuery); + mockQuery.populate.mockReturnValue(mockQuery); + // biome-ignore lint/suspicious/noThenProperty: Intentional thenable mock for Mongoose queries + Object.defineProperty(mockQuery, 'then', { + value: vi.fn((onResolve) => Promise.resolve(queryResult).then(onResolve)), + enumerable: false, + configurable: true, + }); + return mockQuery; + }; + mockModel.find = vi.fn(() => createMockQuery(mockConversations)) as unknown as typeof mockModel.find; + }); + + When('I call getByListingId with "listing-1"', async () => { + result = await repository.getByListingId(createValidObjectId('listing-1')); + }); + + Then('I should receive an array of Conversation entities', () => { + expect(Array.isArray(result)).toBe(true); + }); + + And('the array should contain conversations for that listing', () => { + const conversations = + result as Domain.Contexts.Conversation.Conversation.ConversationEntityReference[]; + expect(conversations.length).toBeGreaterThan(0); + }); + }); + + Scenario('Getting conversations by listing ID with no conversations', ({ When, Then }) => { + When('I call getByListingId with "listing-without-conversations"', async () => { + mockModel.find = vi.fn(() => createNullPopulateChain([])) as unknown as typeof mockModel.find; + + result = await repository.getByListingId(createValidObjectId('listing-without-conversations')); + }); + + Then('I should receive an empty array for listing', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }); + + Scenario('Getting conversations by listing ID with empty string', ({ When, Then }) => { + When('I call getByListingId with empty string', async () => { + result = await repository.getByListingId(''); + }); + + Then('I should receive an empty array for listing', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }); + + Scenario('Getting conversations by listing ID with invalid ObjectId', ({ Given, When, Then }) => { + Given('an invalid ObjectId will cause an error', () => { + // Mock MongooseSeedwork.ObjectId constructor to throw + vi.spyOn(MongooseSeedwork, 'ObjectId').mockImplementationOnce(() => { + throw new Error('Invalid ObjectId'); + }); + }); + + When('I call getByListingId with invalid ID', async () => { + result = await repository.getByListingId('invalid-id'); + // Restore all mocks after test to prevent interference with subsequent tests + vi.restoreAllMocks(); + }); + + Then('I should receive an empty array for listing', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }); }); diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts index ac862981f..1ede7389b 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts @@ -34,6 +34,13 @@ export interface ConversationReadRepository { listingId: string, options?: FindOneOptions, ) => Promise; + + getByListingId: ( + listingId: string, + options?: FindOptions, + ) => Promise< + Domain.Contexts.Conversation.Conversation.ConversationEntityReference[] + >; } export class ConversationReadRepositoryImpl @@ -138,6 +145,28 @@ export class ConversationReadRepositoryImpl return null; } } + + async getByListingId( + listingId: string, + options?: FindOptions, + ): Promise< + Domain.Contexts.Conversation.Conversation.ConversationEntityReference[] + > { + if (!listingId || listingId.trim() === '') { + return []; + } + + try { + const result = await this.mongoDataSource.find( + { listing: new MongooseSeedwork.ObjectId(listingId) }, + { ...options, populateFields: populateFields }, + ); + return result.map((doc) => this.converter.toDomain(doc, this.passport)); + } catch (error) { + console.warn('Error with ObjectId in getByListingId:', error); + return []; + } + } } export const getConversationReadRepository = ( diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/features/conversation.read-repository.feature b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/features/conversation.read-repository.feature index e026a9dd1..a0581f8ca 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/features/conversation.read-repository.feature +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/features/conversation.read-repository.feature @@ -69,3 +69,22 @@ And valid Conversation documents exist in the database Given an invalid ObjectId will cause an error When I call getBySharerReserverListing with invalid ID Then it should return null due to ObjectId error + + Scenario: Getting conversations by listing ID + Given a Conversation document with listing "listing-1" + When I call getByListingId with "listing-1" + Then I should receive an array of Conversation entities + And the array should contain conversations for that listing + + Scenario: Getting conversations by listing ID with no conversations + When I call getByListingId with "listing-without-conversations" + Then I should receive an empty array for listing + + Scenario: Getting conversations by listing ID with empty string + When I call getByListingId with empty string + Then I should receive an empty array for listing + + Scenario: Getting conversations by listing ID with invalid ObjectId + Given an invalid ObjectId will cause an error + When I call getByListingId with invalid ID + Then I should receive an empty array for listing diff --git a/packages/sthrift/persistence/src/datasources/readonly/listing/item/features/item-listing.read-repository.feature b/packages/sthrift/persistence/src/datasources/readonly/listing/item/features/item-listing.read-repository.feature index abe25422f..cd0e73fbe 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/listing/item/features/item-listing.read-repository.feature +++ b/packages/sthrift/persistence/src/datasources/readonly/listing/item/features/item-listing.read-repository.feature @@ -76,3 +76,17 @@ Given an ItemListingReadRepository instance with models and passport Scenario: Getting paged listings when count query returns null When I call getPaged and count query returns null Then I should receive result with total 0 + + Scenario: Getting expired listings for deletion + Given expired listings exist past the archival period + When I call getExpiredForDeletion with 6 months archival period + Then I should receive expired listings + + Scenario: Getting expired listings for deletion when none exist + When I call getExpiredForDeletion and no expired listings exist + Then I should receive empty array for expired + + Scenario: Getting expired listings for deletion with custom limit + Given more expired listings exist than the limit + When I call getExpiredForDeletion with limit 5 + Then I should receive at most 5 listings diff --git a/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.test.ts b/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.test.ts index 4b7bc04e2..7c688ade4 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.test.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.test.ts @@ -537,4 +537,110 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { }); }, ); + + Scenario('Getting expired listings for deletion', ({ Given, When, Then }) => { + let result: unknown; + let mockExpiredListings: unknown[]; + + Given('expired listings exist past the archival period', () => { + const oldDate = new Date(); + oldDate.setMonth(oldDate.getMonth() - 8); + + mockExpiredListings = [ + { + id: 'expired-1', + title: 'Expired Item 1', + state: 'Expired', + updatedAt: oldDate, + }, + { + id: 'expired-2', + title: 'Cancelled Item', + state: 'Cancelled', + updatedAt: oldDate, + }, + ]; + }); + + When('I call getExpiredForDeletion with 6 months archival period', async () => { + const mockModel = { + find: vi.fn(() => createQueryChain(mockExpiredListings)), + }; + + const mockModels = createMockModelsContext(mockModel); + const mockPassport = createMockPassport(); + + repository = getItemListingReadRepository(mockModels, mockPassport); + result = await repository.getExpiredForDeletion(6); + }); + + Then('I should receive expired listings', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(2); + }); + }); + + Scenario( + 'Getting expired listings for deletion when none exist', + ({ When, Then }) => { + let result: unknown; + + When( + 'I call getExpiredForDeletion and no expired listings exist', + async () => { + const mockModel = { + find: vi.fn(() => createQueryChain([])), + }; + + const mockModels = createMockModelsContext(mockModel); + const mockPassport = createMockPassport(); + + repository = getItemListingReadRepository(mockModels, mockPassport); + result = await repository.getExpiredForDeletion(6); + }, + ); + + Then('I should receive empty array for expired', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }, + ); + + Scenario( + 'Getting expired listings for deletion with custom limit', + ({ Given, When, Then }) => { + let result: unknown; + let mockListings: unknown[]; + + Given('more expired listings exist than the limit', () => { + const oldDate = new Date(); + oldDate.setMonth(oldDate.getMonth() - 8); + + mockListings = Array.from({ length: 5 }, (_, i) => ({ + id: `expired-${i}`, + title: `Expired Item ${i}`, + state: 'Expired', + updatedAt: oldDate, + })); + }); + + When('I call getExpiredForDeletion with limit 5', async () => { + const mockModel = { + find: vi.fn(() => createQueryChain(mockListings)), + }; + + const mockModels = createMockModelsContext(mockModel); + const mockPassport = createMockPassport(); + + repository = getItemListingReadRepository(mockModels, mockPassport); + result = await repository.getExpiredForDeletion(6, 5); + }); + + Then('I should receive at most 5 listings', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBeLessThanOrEqual(5); + }); + }, + ); }); diff --git a/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts b/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts index 0633d697e..239f296b0 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts @@ -40,6 +40,13 @@ export interface ItemListingReadRepository { ) => Promise< Domain.Contexts.Listing.ItemListing.ItemListingEntityReference[] >; + + getExpiredForDeletion: ( + archivalMonths: number, + limit?: number, + ) => Promise< + Domain.Contexts.Listing.ItemListing.ItemListingEntityReference[] + >; } class ItemListingReadRepositoryImpl @@ -189,6 +196,25 @@ class ItemListingReadRepositoryImpl return []; } } + + async getExpiredForDeletion( + archivalMonths: number, + limit = 100, + ): Promise { + const cutoffDate = new Date(); + cutoffDate.setMonth(cutoffDate.getMonth() - archivalMonths); + + const result = await this.mongoDataSource.find( + { + state: { $in: ['Expired', 'Cancelled'] }, + updatedAt: { $lt: cutoffDate }, + }, + { limit }, + ); + + if (!result || result.length === 0) return []; + return result.map((doc) => this.converter.toDomain(doc, this.passport)); + } } export function getItemListingReadRepository( diff --git a/packages/sthrift/rest/src/index.ts b/packages/sthrift/rest/src/index.ts index 563096640..569f67551 100644 --- a/packages/sthrift/rest/src/index.ts +++ b/packages/sthrift/rest/src/index.ts @@ -3,14 +3,14 @@ import type { HttpResponseInit, InvocationContext, } from '@azure/functions'; -import type { ApplicationServicesFactory, PrincipalHints } from '@sthrift/application-services'; +import type { AppServicesHost, ApplicationServices, PrincipalHints } from '@sthrift/application-services'; export type HttpHandler = ( request: HttpRequest, context: InvocationContext, ) => Promise; -export const restHandlerCreator = (applicationServicesFactory: ApplicationServicesFactory): HttpHandler => { +export const restHandlerCreator = (applicationServicesHost: AppServicesHost): HttpHandler => { return async (request: HttpRequest, _context: InvocationContext) => { const rawAuthHeader = request.headers.get('Authorization') ?? undefined; const hints: PrincipalHints = { @@ -19,7 +19,7 @@ export const restHandlerCreator = (applicationServicesFactory: ApplicationServic // biome-ignore lint:useLiteralKeys communityId: request.params['communityId'] ?? undefined, }; - const applicationServices = await applicationServicesFactory.forRequest(rawAuthHeader, hints); + const applicationServices = await applicationServicesHost.forRequest(rawAuthHeader, hints); return { status: 200, diff --git a/packages/sthrift/service-blob-storage/package.json b/packages/sthrift/service-blob-storage/package.json index 2d6005ae2..f30edca31 100644 --- a/packages/sthrift/service-blob-storage/package.json +++ b/packages/sthrift/service-blob-storage/package.json @@ -20,6 +20,7 @@ "clean": "rimraf dist" }, "dependencies": { + "@azure/storage-blob": "^12.29.1", "@cellix/api-services-spec": "workspace:*", "@sthrift/domain": "workspace:*" }, diff --git a/packages/sthrift/service-blob-storage/src/index.ts b/packages/sthrift/service-blob-storage/src/index.ts index 9bd7981b0..34a5d2e51 100644 --- a/packages/sthrift/service-blob-storage/src/index.ts +++ b/packages/sthrift/service-blob-storage/src/index.ts @@ -1,7 +1,9 @@ import type { ServiceBase } from '@cellix/api-services-spec'; import type { Domain } from '@sthrift/domain'; +import { BlobServiceClient } from '@azure/storage-blob'; export class ServiceBlobStorage implements ServiceBase { + private blobServiceClient: BlobServiceClient | undefined; async startUp(): Promise { @@ -12,6 +14,8 @@ export class ServiceBlobStorage implements ServiceBase { return await Promise.resolve(`Valet key for ${storageAccount}/${path} valid until ${expiration.toISOString()}`); } + + async deleteBlob(containerName: string, blobPath: string): Promise { + if (!this.blobServiceClient) { + throw new Error('BlobServiceClient is not initialized'); + } + const containerClient = this.blobServiceClient.getContainerClient(containerName); + const blobClient = containerClient.getBlobClient(blobPath); + await blobClient.deleteIfExists(); + } + shutDown(): Promise { console.log('ServiceBlobStorage stopped'); return Promise.resolve(); diff --git a/pnpm-lock.yaml b/pnpm-lock.yaml index 57c3fea48..78faf00f8 100644 --- a/pnpm-lock.yaml +++ b/pnpm-lock.yaml @@ -688,6 +688,9 @@ importers: '@cellix/payment-service': specifier: workspace:* version: link:../../cellix/payment-service + '@sthrift/domain': + specifier: workspace:* + version: link:../domain '@sthrift/persistence': specifier: workspace:* version: link:../persistence @@ -1103,6 +1106,9 @@ importers: packages/sthrift/service-blob-storage: dependencies: + '@azure/storage-blob': + specifier: ^12.29.1 + version: 12.29.1 '@cellix/api-services-spec': specifier: workspace:* version: link:../../cellix/api-services-spec @@ -1657,6 +1663,10 @@ packages: resolution: {integrity: sha512-XPArKLzsvl0Hf0CaGyKHUyVgF7oDnhKoP85Xv6M4StF/1AhfORhZudHtOyf2s+FcbuQ9dPRAjB8J2KvRRMUK2A==} engines: {node: '>=20.0.0'} + '@azure/core-xml@1.5.0': + resolution: {integrity: sha512-D/sdlJBMJfx7gqoj66PKVmhDDaU6TKA49ptcolxdas29X7AfvLTmfAGLjAcIMBK7UZ2o4lygHIqVckOlQU3xWw==} + engines: {node: '>=20.0.0'} + '@azure/functions-opentelemetry-instrumentation@0.1.0': resolution: {integrity: sha512-eRitTbOUDhlzc4o2Q9rjbXiMYa/ep06m2jIkN7HOuLP0aHnjPh3zHXtqji/NyeqT/GfHjCgJr+r8+49s7KER7w==} engines: {node: '>=18.0'} @@ -1709,6 +1719,14 @@ packages: resolution: {integrity: sha512-gNCFokEoQQEkhu2T8i1i+1iW2o9wODn2slu5tpqJmjV1W7qf9dxVv6GNXW1P1WC8wMga8BCc2t/oMhOK3iwRQg==} engines: {node: '>=18.0.0'} + '@azure/storage-blob@12.29.1': + resolution: {integrity: sha512-7ktyY0rfTM0vo7HvtK6E3UvYnI9qfd6Oz6z/+92VhGRveWng3kJwMKeUpqmW/NmwcDNbxHpSlldG+vsUnRFnBg==} + engines: {node: '>=20.0.0'} + + '@azure/storage-common@12.1.1': + resolution: {integrity: sha512-eIOH1pqFwI6UmVNnDQvmFeSg0XppuzDLFeUNO/Xht7ODAzRLgGDh7h550pSxoA+lPDxBl1+D2m/KG3jWzCUjTg==} + engines: {node: '>=20.0.0'} + '@babel/code-frame@7.27.1': resolution: {integrity: sha512-cjQ7ZlQ0Mv3b47hABuTevyTuYN4i+loJKGeV9flcCgIK37cCXRh+L1bd3iBHlynerhQ7BhCkn2BPbQUL+rGqFg==} engines: {node: '>=6.9.0'} @@ -6978,6 +6996,10 @@ packages: fast-uri@3.1.0: resolution: {integrity: sha512-iPeeDKJSWf4IEOasVVrknXpaBV0IApz/gp7S2bb7Z4Lljbl2MGJRqInZiUrQwV16cpzw/D3S5j5Julj/gT52AA==} + fast-xml-parser@5.3.3: + resolution: {integrity: sha512-2O3dkPAAC6JavuMm8+4+pgTk+5hoAs+CjZ+sWcQLkX9+/tHRuTkQh/Oaifr8qDmZ8iEHb771Ea6G8CdwkrgvYA==} + hasBin: true + fastq@1.19.1: resolution: {integrity: sha512-GwLTyxkCXjXbxqIhTsMI2Nui8huMPtnxg7krajPJAjnEG/iiOS7i+zCtWGZR9G0NBKbXKh6X9m9UIsYX/N6vvQ==} @@ -10953,6 +10975,9 @@ packages: resolution: {integrity: sha512-k55yxKHwaXnpYGsOzg4Vl8+tDrWylxDEpknGjhTiZB8dFRU5rTo9CAzeycivxV3s+zlTKwrs6WxMxR95n26kwg==} engines: {node: '>=0.10.0'} + strnum@2.1.2: + resolution: {integrity: sha512-l63NF9y/cLROq/yqKXSLtcMeeyOfnSQlfMSlzFt/K73oIaD8DGaQWd7Z34X9GPiKqP5rbSh84Hl4bOlLcjiSrQ==} + style-to-js@1.1.18: resolution: {integrity: sha512-JFPn62D4kJaPTnhFUI244MThx+FEGbi+9dw1b9yBBQ+1CZpV7QAT8kUtJ7b7EUNdHajjF/0x8fT+16oLJoojLg==} @@ -12447,7 +12472,7 @@ snapshots: '@azure/core-rest-pipeline@1.16.3': dependencies: '@azure/abort-controller': 2.1.2 - '@azure/core-auth': 1.7.2 + '@azure/core-auth': 1.10.1 '@azure/core-tracing': 1.3.1 '@azure/core-util': 1.13.1 '@azure/logger': 1.3.0 @@ -12481,6 +12506,11 @@ snapshots: transitivePeerDependencies: - supports-color + '@azure/core-xml@1.5.0': + dependencies: + fast-xml-parser: 5.3.3 + tslib: 2.8.1 + '@azure/functions-opentelemetry-instrumentation@0.1.0(@opentelemetry/api@1.9.0)': dependencies: '@opentelemetry/api': 1.9.0 @@ -12611,6 +12641,39 @@ snapshots: transitivePeerDependencies: - supports-color + '@azure/storage-blob@12.29.1': + dependencies: + '@azure/abort-controller': 2.1.2 + '@azure/core-auth': 1.10.1 + '@azure/core-client': 1.10.1 + '@azure/core-http-compat': 2.3.1 + '@azure/core-lro': 2.7.2 + '@azure/core-paging': 1.6.2 + '@azure/core-rest-pipeline': 1.22.1 + '@azure/core-tracing': 1.3.1 + '@azure/core-util': 1.13.1 + '@azure/core-xml': 1.5.0 + '@azure/logger': 1.3.0 + '@azure/storage-common': 12.1.1 + events: 3.3.0 + tslib: 2.8.1 + transitivePeerDependencies: + - supports-color + + '@azure/storage-common@12.1.1': + dependencies: + '@azure/abort-controller': 2.1.2 + '@azure/core-auth': 1.10.1 + '@azure/core-http-compat': 2.3.1 + '@azure/core-rest-pipeline': 1.22.1 + '@azure/core-tracing': 1.3.1 + '@azure/core-util': 1.13.1 + '@azure/logger': 1.3.0 + events: 3.3.0 + tslib: 2.8.1 + transitivePeerDependencies: + - supports-color + '@babel/code-frame@7.27.1': dependencies: '@babel/helper-validator-identifier': 7.28.5 @@ -15381,7 +15444,7 @@ snapshots: '@graphql-tools/optimize@2.0.0(graphql@16.11.0)': dependencies: graphql: 16.11.0 - tslib: 2.6.3 + tslib: 2.8.1 '@graphql-tools/prisma-loader@8.0.17(@types/node@24.9.2)(graphql@16.11.0)': dependencies: @@ -15417,7 +15480,7 @@ snapshots: '@ardatan/relay-compiler': 12.0.3(graphql@16.11.0) '@graphql-tools/utils': 10.9.1(graphql@16.11.0) graphql: 16.11.0 - tslib: 2.6.3 + tslib: 2.8.1 transitivePeerDependencies: - encoding @@ -18291,7 +18354,7 @@ snapshots: path-case: 3.0.4 sentence-case: 3.0.4 snake-case: 3.0.4 - tslib: 2.6.3 + tslib: 2.8.1 char-regex@1.0.2: {} @@ -19563,6 +19626,10 @@ snapshots: fast-uri@3.1.0: {} + fast-xml-parser@5.3.3: + dependencies: + strnum: 2.1.2 + fastq@1.19.1: dependencies: reusify: 1.1.0 @@ -20567,7 +20634,7 @@ snapshots: is-lower-case@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 is-map@2.0.3: {} @@ -20650,7 +20717,7 @@ snapshots: is-upper-case@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 is-weakmap@2.0.2: {} @@ -21113,11 +21180,11 @@ snapshots: lower-case-first@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 lower-case@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 lowercase-keys@3.0.0: {} @@ -24088,7 +24155,7 @@ snapshots: sponge-case@1.0.1: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 sprintf-js@1.0.3: {} @@ -24278,6 +24345,8 @@ snapshots: dependencies: escape-string-regexp: 1.0.5 + strnum@2.1.2: {} + style-to-js@1.1.18: dependencies: style-to-object: 1.0.11 @@ -24347,7 +24416,7 @@ snapshots: swap-case@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 symbol-tree@3.2.4: {} @@ -24459,7 +24528,7 @@ snapshots: title-case@3.0.3: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 tldts-core@6.1.86: {} @@ -24852,11 +24921,11 @@ snapshots: upper-case-first@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 upper-case@2.0.2: dependencies: - tslib: 2.6.3 + tslib: 2.8.1 uri-js@4.4.1: dependencies: From 24db601df63f14833158f952cf9a9c1bd3c7dca2 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 10:49:19 -0500 Subject: [PATCH 02/14] fix: resolve knip warning and Sourcery type safety suggestion - Remove export from ListingDependencies interface (only used internally) - Fix type safety in expired-listing-deletion-handler by using ApplicationServicesFactory directly instead of casting from AppServicesHost --- apps/api/src/timers/expired-listing-deletion-handler.ts | 7 +++---- .../application-services/src/contexts/listing/index.ts | 2 +- 2 files changed, 4 insertions(+), 5 deletions(-) diff --git a/apps/api/src/timers/expired-listing-deletion-handler.ts b/apps/api/src/timers/expired-listing-deletion-handler.ts index 7ee52e86d..574c70481 100644 --- a/apps/api/src/timers/expired-listing-deletion-handler.ts +++ b/apps/api/src/timers/expired-listing-deletion-handler.ts @@ -1,11 +1,11 @@ import type { TimerHandler } from '@azure/functions'; -import type { ApplicationServicesFactory, AppServicesHost, ApplicationServices } from '@sthrift/application-services'; +import type { ApplicationServicesFactory } from '@sthrift/application-services'; import { trace, SpanStatusCode } from '@opentelemetry/api'; const tracer = trace.getTracer('timer:expired-listing-deletion'); export const expiredListingDeletionHandlerCreator = ( - applicationServicesHost: AppServicesHost, + applicationServicesFactory: ApplicationServicesFactory, ): TimerHandler => { return async (timer, context) => { await tracer.startActiveSpan('processExpiredListingDeletions', async (span) => { @@ -16,8 +16,7 @@ export const expiredListingDeletionHandlerCreator = ( context.log('ExpiredListingDeletion: Timer is past due'); } - const factory = applicationServicesHost as ApplicationServicesFactory; - const systemServices = factory.forSystemTask(); + const systemServices = applicationServicesFactory.forSystemTask(); const result = await systemServices.Listing.ItemListing.processExpiredDeletions(); span.setAttribute('deletedCount', result.deletedCount); diff --git a/packages/sthrift/application-services/src/contexts/listing/index.ts b/packages/sthrift/application-services/src/contexts/listing/index.ts index 7b1ea28fd..a550b1e12 100644 --- a/packages/sthrift/application-services/src/contexts/listing/index.ts +++ b/packages/sthrift/application-services/src/contexts/listing/index.ts @@ -10,7 +10,7 @@ export interface ListingContextApplicationService { ItemListing: ItemListingApplicationService; } -export interface ListingDependencies { +interface ListingDependencies { dataSources: DataSources; blobStorage?: Domain.Services['BlobStorage']; } From a90d8530b25ce339b6a9813f7ae5bc3f0d642125 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 10:54:40 -0500 Subject: [PATCH 03/14] fix: align handler implementation signatures with interface declarations Use AppHost type alias in both registerAzureFunctionHttpHandler and registerAzureFunctionTimerHandler implementations instead of expanding to RequestScopedHost for consistency with interface declarations --- apps/api/src/cellix.ts | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index 9390183ad..fa086f1c6 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -303,7 +303,7 @@ export class Cellix name: string, options: Omit, handlerCreator: ( - applicationServicesHost: RequestScopedHost, + applicationServicesHost: AppHost, ) => HttpHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); @@ -316,7 +316,7 @@ export class Cellix name: string, schedule: string, handlerCreator: ( - applicationServicesHost: RequestScopedHost, + applicationServicesHost: AppHost, ) => TimerHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); From 8aed03ca0d06d8826f5c96c65171bb5ec301e9f5 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 11:12:09 -0500 Subject: [PATCH 04/14] refactor: simplify test mocks following Sourcery suggestions - Use mockResolvedValue instead of mockImplementation where appropriate - Ensure Promise.resolve wraps callback returns for consistency - Keep existing test patterns while improving readability --- .../conversation/delete-by-listing.test.ts | 9 +++------ .../item/process-expired-deletions.test.ts | 16 ++++++---------- 2 files changed, 9 insertions(+), 16 deletions(-) diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts index d3b7666db..2fdcc83d1 100644 --- a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts @@ -40,8 +40,6 @@ test.for(feature, ({ Background, Scenario }) => { }); And('the conversation repository is available', () => { - let callCount = 0; - mockConversationRepo = { get: vi.fn().mockImplementation(() => ({ requestDelete: vi.fn(), @@ -49,13 +47,14 @@ test.for(feature, ({ Background, Scenario }) => { save: vi.fn().mockResolvedValue(undefined), }; + let callCount = 0; mockConversationUow = { withScopedTransaction: vi.fn((callback) => { callCount++; if (shouldFailFirstConversation && callCount === 1) { return Promise.reject(new Error('Failed to delete conversation')); } - return callback(mockConversationRepo); + return Promise.resolve(callback(mockConversationRepo)); }), }; @@ -71,9 +70,7 @@ test.for(feature, ({ Background, Scenario }) => { Conversation: { Conversation: { ConversationReadRepo: { - getByListingId: vi.fn().mockImplementation(() => { - return Promise.resolve(mockConversations); - }), + getByListingId: vi.fn().mockResolvedValue(mockConversations), }, }, }, diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts index 4c2ed5b59..e168db63e 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts @@ -71,14 +71,14 @@ test.for(feature, ({ Background, Scenario }) => { if (shouldFailListingDeletion) { return Promise.reject(new Error('Failed to delete listing')); } - return callback(mockListingRepo); + return Promise.resolve(callback(mockListingRepo)); }), }; mockConversationUow = { - withScopedTransaction: vi.fn((callback) => { - return callback(mockConversationRepo); - }), + withScopedTransaction: vi.fn((callback) => + Promise.resolve(callback(mockConversationRepo)), + ), }; mockDataSources = { @@ -98,18 +98,14 @@ test.for(feature, ({ Background, Scenario }) => { Listing: { ItemListing: { ItemListingReadRepo: { - getExpiredForDeletion: vi.fn().mockImplementation(() => { - return Promise.resolve(mockExpiredListings); - }), + getExpiredForDeletion: vi.fn().mockResolvedValue(mockExpiredListings), }, }, }, Conversation: { Conversation: { ConversationReadRepo: { - getByListingId: vi.fn().mockImplementation(() => { - return Promise.resolve(mockConversations); - }), + getByListingId: vi.fn().mockResolvedValue(mockConversations), }, }, }, From 6341a662d3750a40f6177997d83b9a7a354333e9 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 12:06:56 -0500 Subject: [PATCH 05/14] refactor: move hard-coded deletion config to environment-based configuration - Add ListingDeletionConfig interface to @sthrift/context-spec for archivalMonths, batchSize, and blobContainerName - Thread configuration from environment variables through ApiContextSpec to application services - Update process-expired-deletions.ts to accept config parameter instead of hard-coded constants - Improve error handling in ConversationReadRepository: * Narrow try/catch scope to ObjectId construction only * Add structured error logging with context (listingId, userId, etc.) * Replace console.warn with console.error for better visibility - Update all tests to pass mock configuration - Add generic type parameter to registerAzureFunctionTimerHandler for flexible factory typing Resolves Sourcery feedback: archival period, batch size, and blob container now configurable per environment --- apps/api/src/cellix.ts | 6 +- apps/api/src/index.ts | 5 + .../src/contexts/listing/index.ts | 6 ++ .../src/contexts/listing/item/index.ts | 12 ++- .../item/process-expired-deletions.test.ts | 14 +++ .../listing/item/process-expired-deletions.ts | 19 ++-- .../sthrift/application-services/src/index.ts | 8 +- packages/sthrift/context-spec/src/index.ts | 24 +++++ .../conversation.read-repository.ts | 97 ++++++++++++------- 9 files changed, 139 insertions(+), 52 deletions(-) diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index fa086f1c6..bb1615b42 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -312,15 +312,15 @@ export class Cellix return this; } - public registerAzureFunctionTimerHandler( + public registerAzureFunctionTimerHandler = AppHost>( name: string, schedule: string, handlerCreator: ( - applicationServicesHost: AppHost, + applicationServicesFactory: TFactory, ) => TimerHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); - this.pendingTimerHandlers.push({ name, schedule, handlerCreator }); + this.pendingTimerHandlers.push({ name, schedule, handlerCreator: handlerCreator as (host: AppHost) => TimerHandler }); this.phase = 'handlers'; return this; } diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 719e714a7..7f91c48ee 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -86,6 +86,11 @@ Cellix.initializeInfrastructureServices( paymentService, messagingService, blobStorageService, + listingDeletionConfig: { + archivalMonths: Number(process.env['LISTING_ARCHIVAL_MONTHS']) || 6, + batchSize: Number(process.env['LISTING_DELETION_BATCH_SIZE']) || 100, + blobContainerName: process.env['LISTING_IMAGES_CONTAINER'] || 'listing-images', + }, }; }) .initializeApplicationServices((context) => diff --git a/packages/sthrift/application-services/src/contexts/listing/index.ts b/packages/sthrift/application-services/src/contexts/listing/index.ts index a550b1e12..6d5e4c04b 100644 --- a/packages/sthrift/application-services/src/contexts/listing/index.ts +++ b/packages/sthrift/application-services/src/contexts/listing/index.ts @@ -1,5 +1,6 @@ import type { Domain } from '@sthrift/domain'; import type { DataSources } from '@sthrift/persistence'; +import type { ListingDeletionConfig } from '@sthrift/context-spec'; import { ItemListing as ItemListingApi, type ItemListingApplicationService, @@ -13,6 +14,7 @@ export interface ListingContextApplicationService { interface ListingDependencies { dataSources: DataSources; blobStorage?: Domain.Services['BlobStorage']; + listingDeletionConfig?: ListingDeletionConfig; } export const Listing = ( @@ -20,11 +22,15 @@ export const Listing = ( ): ListingContextApplicationService => { const dataSources = 'dataSources' in deps ? deps.dataSources : deps; const blobStorage = 'blobStorage' in deps ? deps.blobStorage : undefined; + const listingDeletionConfig = 'listingDeletionConfig' in deps ? deps.listingDeletionConfig : undefined; const itemListingDeps: ItemListingDependencies = { dataSources }; if (blobStorage) { itemListingDeps.blobStorage = blobStorage; } + if (listingDeletionConfig) { + itemListingDeps.listingDeletionConfig = listingDeletionConfig; + } return { ItemListing: ItemListingApi(itemListingDeps), diff --git a/packages/sthrift/application-services/src/contexts/listing/item/index.ts b/packages/sthrift/application-services/src/contexts/listing/item/index.ts index 8801a2ed8..fa5ad9285 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/index.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/index.ts @@ -1,5 +1,6 @@ import type { Domain } from '@sthrift/domain'; import type { DataSources } from '@sthrift/persistence'; +import type { ListingDeletionConfig } from '@sthrift/context-spec'; import { type ItemListingCreateCommand, create } from './create.ts'; import { type ItemListingQueryByIdCommand, queryById } from './query-by-id.ts'; import { @@ -61,6 +62,7 @@ export interface ItemListingApplicationService { export interface ItemListingDependencies { dataSources: DataSources; blobStorage?: Domain.Services['BlobStorage']; + listingDeletionConfig?: ListingDeletionConfig; } export const ItemListing = ( @@ -68,6 +70,14 @@ export const ItemListing = ( ): ItemListingApplicationService => { const dataSources = 'dataSources' in deps ? deps.dataSources : deps; const blobStorage = 'blobStorage' in deps ? deps.blobStorage : undefined; + const config = 'listingDeletionConfig' in deps ? deps.listingDeletionConfig : undefined; + + // Use default config if not provided (for backward compatibility and testing) + const deletionConfig: ListingDeletionConfig = config ?? { + archivalMonths: 6, + batchSize: 100, + blobContainerName: 'listing-images', + }; return { create: create(dataSources), @@ -79,6 +89,6 @@ export const ItemListing = ( deleteListings: deleteListings(dataSources), unblock: unblock(dataSources), queryPaged: queryPaged(dataSources), - processExpiredDeletions: processExpiredDeletions(dataSources, blobStorage), + processExpiredDeletions: processExpiredDeletions(dataSources, deletionConfig, blobStorage), }; }; diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts index e168db63e..dc6a8bbc2 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts @@ -4,6 +4,7 @@ import { describeFeature, loadFeature } from '@amiceli/vitest-cucumber'; import { expect, vi } from 'vitest'; import type { DataSources } from '@sthrift/persistence'; import type { Domain } from '@sthrift/domain'; +import type { ListingDeletionConfig } from '@sthrift/context-spec'; import { processExpiredDeletions } from './process-expired-deletions.ts'; const test = { for: describeFeature }; @@ -15,6 +16,7 @@ const feature = await loadFeature( test.for(feature, ({ Background, Scenario }) => { let mockDataSources: DataSources; let mockBlobStorage: Domain.Services['BlobStorage'] | undefined; + let mockConfig: ListingDeletionConfig; let mockExpiredListings: Array<{ id: string; images: string[]; @@ -52,6 +54,12 @@ test.for(feature, ({ Background, Scenario }) => { mockExpiredListings = []; mockConversations = []; + mockConfig = { + archivalMonths: 6, + batchSize: 100, + blobContainerName: 'listing-images', + }; + mockListingRepo = { get: vi.fn().mockImplementation(() => ({ requestDelete: vi.fn(), @@ -138,6 +146,7 @@ test.for(feature, ({ Background, Scenario }) => { When('the expired deletion process runs', async () => { const processFunction = processExpiredDeletions( mockDataSources, + mockConfig, mockBlobStorage, ); result = await processFunction(); @@ -191,6 +200,7 @@ test.for(feature, ({ Background, Scenario }) => { When('the expired deletion process runs', async () => { const processFunction = processExpiredDeletions( mockDataSources, + mockConfig, mockBlobStorage, ); result = await processFunction(); @@ -231,6 +241,7 @@ test.for(feature, ({ Background, Scenario }) => { When('the expired deletion process runs', async () => { const processFunction = processExpiredDeletions( mockDataSources, + mockConfig, mockBlobStorage, ); result = await processFunction(); @@ -268,6 +279,7 @@ test.for(feature, ({ Background, Scenario }) => { When('the expired deletion process runs', async () => { const processFunction = processExpiredDeletions( mockDataSources, + mockConfig, mockBlobStorage, ); result = await processFunction(); @@ -303,6 +315,7 @@ test.for(feature, ({ Background, Scenario }) => { When('the expired deletion process runs', async () => { const processFunction = processExpiredDeletions( mockDataSources, + mockConfig, mockBlobStorage, ); result = await processFunction(); @@ -346,6 +359,7 @@ test.for(feature, ({ Background, Scenario }) => { When('the expired deletion process runs', async () => { const processFunction = processExpiredDeletions( mockDataSources, + mockConfig, mockBlobStorage, ); result = await processFunction(); diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts index 50cc6e833..792d80001 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts @@ -1,11 +1,8 @@ import type { Domain } from '@sthrift/domain'; import type { DataSources } from '@sthrift/persistence'; +import type { ListingDeletionConfig } from '@sthrift/context-spec'; import { deleteByListing as deleteConversationsByListing } from '../../conversation/conversation/delete-by-listing.ts'; -const ARCHIVAL_MONTHS = 6; -const BATCH_SIZE = 100; -const BLOB_CONTAINER_NAME = 'listing-images'; - export interface ProcessExpiredDeletionsResult { deletedCount: number; deletedListingIds: string[]; @@ -17,11 +14,12 @@ export interface ProcessExpiredDeletionsResult { async function deleteListingImages( blobStorage: Domain.Services['BlobStorage'], images: string[], + containerName: string, ): Promise { let deletedCount = 0; for (const imagePath of images) { try { - await blobStorage.deleteBlob(BLOB_CONTAINER_NAME, imagePath); + await blobStorage.deleteBlob(containerName, imagePath); deletedCount++; } catch (error) { console.warn(`[ExpiredDeletion] Failed to delete image ${imagePath}: ${error instanceof Error ? error.message : String(error)}`); @@ -43,6 +41,7 @@ async function deleteListingById( export const processExpiredDeletions = ( dataSources: DataSources, + config: ListingDeletionConfig, blobStorage?: Domain.Services['BlobStorage'], ): (() => Promise) => { return async (): Promise => { @@ -56,8 +55,8 @@ export const processExpiredDeletions = ( const expiredListings = await dataSources.readonlyDataSource.Listing.ItemListing.ItemListingReadRepo.getExpiredForDeletion( - ARCHIVAL_MONTHS, - BATCH_SIZE, + config.archivalMonths, + config.batchSize, ); console.log( @@ -77,7 +76,11 @@ export const processExpiredDeletions = ( try { if (blobStorage && listing.images && listing.images.length > 0) { - const imagesDeleted = await deleteListingImages(blobStorage, listing.images); + const imagesDeleted = await deleteListingImages( + blobStorage, + listing.images, + config.blobContainerName, + ); result.deletedImagesCount += imagesDeleted; console.log(`[ExpiredDeletion] Deleted ${imagesDeleted} images for listing ${listingId}`); } diff --git a/packages/sthrift/application-services/src/index.ts b/packages/sthrift/application-services/src/index.ts index 2398cf2f9..2aec56a85 100644 --- a/packages/sthrift/application-services/src/index.ts +++ b/packages/sthrift/application-services/src/index.ts @@ -134,11 +134,11 @@ export const buildApplicationServicesFactory = ( return { forRequest, forSystemTask: (): SystemTaskServices => { - const dataSources = - infrastructureServicesRegistry.dataSourcesFactory.withSystemPassport(); - const blobStorage = infrastructureServicesRegistry.blobStorageService; + const { dataSourcesFactory, blobStorageService, listingDeletionConfig } = + infrastructureServicesRegistry; + const dataSources = dataSourcesFactory.withSystemPassport(); return { - Listing: Listing({ dataSources, blobStorage }), + Listing: Listing({ dataSources, blobStorage: blobStorageService, listingDeletionConfig }), Conversation: Conversation(dataSources), }; }, diff --git a/packages/sthrift/context-spec/src/index.ts b/packages/sthrift/context-spec/src/index.ts index 2154e35c3..7d8095c33 100644 --- a/packages/sthrift/context-spec/src/index.ts +++ b/packages/sthrift/context-spec/src/index.ts @@ -4,6 +4,29 @@ import type { PaymentService } from '@cellix/payment-service'; import type { MessagingService } from '@cellix/messaging-service'; import type { Domain } from '@sthrift/domain'; +/** + * Configuration for expired listing deletion processing + */ +export interface ListingDeletionConfig { + /** + * Number of months after which archived listings become eligible for permanent deletion + * @default 6 + */ + archivalMonths: number; + + /** + * Maximum number of listings to process per batch execution + * @default 100 + */ + batchSize: number; + + /** + * Name of the blob container where listing images are stored + * @default 'listing-images' + */ + blobContainerName: string; +} + export interface ApiContextSpec { //mongooseService:Exclude; dataSourcesFactory: DataSourcesFactory; // NOT an infrastructure service @@ -11,4 +34,5 @@ export interface ApiContextSpec { paymentService: PaymentService; messagingService: MessagingService; blobStorageService: Domain.Services['BlobStorage']; + listingDeletionConfig: ListingDeletionConfig; } diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts index 1ede7389b..7b83806e1 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts @@ -94,24 +94,30 @@ export class ConversationReadRepositoryImpl return []; } + let userObjectId: MongooseSeedwork.ObjectId; try { - const result = await this.mongoDataSource.find( - { - $or: [ - { sharer: new MongooseSeedwork.ObjectId(userId) }, - { reserver: new MongooseSeedwork.ObjectId(userId) }, - ], - }, - { - ...options, - populateFields: populateFields, - }, - ); - return result.map((doc) => this.converter.toDomain(doc, this.passport)); + userObjectId = new MongooseSeedwork.ObjectId(userId); } catch (error) { - console.warn('Error with ObjectId:', error); + console.error('[ConversationReadRepository] Invalid ObjectId format for userId:', { + userId, + error: error instanceof Error ? error.message : String(error), + }); return []; } + + const result = await this.mongoDataSource.find( + { + $or: [ + { sharer: userObjectId }, + { reserver: userObjectId }, + ], + }, + { + ...options, + populateFields: populateFields, + }, + ); + return result.map((doc) => this.converter.toDomain(doc, this.passport)); } async getBySharerReserverListing( @@ -124,26 +130,39 @@ export class ConversationReadRepositoryImpl return null; } + let sharerObjectId: MongooseSeedwork.ObjectId; + let reserverObjectId: MongooseSeedwork.ObjectId; + let listingObjectId: MongooseSeedwork.ObjectId; + try { - const result = await this.mongoDataSource.findOne( - { - sharer: new MongooseSeedwork.ObjectId(sharerId), - reserver: new MongooseSeedwork.ObjectId(reserverId), - listing: new MongooseSeedwork.ObjectId(listingId), - }, - { - ...options, - populateFields: populateFields, - }, - ); - if (!result) { - return null; - } - return this.converter.toDomain(result, this.passport); + sharerObjectId = new MongooseSeedwork.ObjectId(sharerId); + reserverObjectId = new MongooseSeedwork.ObjectId(reserverId); + listingObjectId = new MongooseSeedwork.ObjectId(listingId); } catch (error) { - console.warn('Error with ObjectId in getBySharerReserverListing:', error); + console.error('[ConversationReadRepository] Invalid ObjectId format in getBySharerReserverListing:', { + sharerId, + reserverId, + listingId, + error: error instanceof Error ? error.message : String(error), + }); return null; } + + const result = await this.mongoDataSource.findOne( + { + sharer: sharerObjectId, + reserver: reserverObjectId, + listing: listingObjectId, + }, + { + ...options, + populateFields: populateFields, + }, + ); + if (!result) { + return null; + } + return this.converter.toDomain(result, this.passport); } async getByListingId( @@ -156,16 +175,22 @@ export class ConversationReadRepositoryImpl return []; } + let objectId: MongooseSeedwork.ObjectId; try { - const result = await this.mongoDataSource.find( - { listing: new MongooseSeedwork.ObjectId(listingId) }, - { ...options, populateFields: populateFields }, - ); - return result.map((doc) => this.converter.toDomain(doc, this.passport)); + objectId = new MongooseSeedwork.ObjectId(listingId); } catch (error) { - console.warn('Error with ObjectId in getByListingId:', error); + console.error('[ConversationReadRepository] Invalid ObjectId format for listingId:', { + listingId, + error: error instanceof Error ? error.message : String(error), + }); return []; } + + const result = await this.mongoDataSource.find( + { listing: objectId }, + { ...options, populateFields: populateFields }, + ); + return result.map((doc) => this.converter.toDomain(doc, this.passport)); } } From 85e0296b100b73b578be4789af02cfe6c6c92965 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 13:46:12 -0500 Subject: [PATCH 06/14] fix: update handler signatures and improve type safety for Azure function timer registration --- apps/api/src/cellix.ts | 16 +++++++++++----- apps/api/src/index.ts | 3 +++ 2 files changed, 14 insertions(+), 5 deletions(-) diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index bb1615b42..1e02e869a 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -118,11 +118,11 @@ interface AzureFunctionHandlerRegistry = AppHost>( name: string, schedule: string, handlerCreator: ( - applicationServicesHost: AppHost, + applicationServicesFactory: TFactory, ) => TimerHandler, ): AzureFunctionHandlerRegistry; /** @@ -192,7 +192,7 @@ interface PendingHandler { interface PendingTimerHandler { name: string; schedule: string; - handlerCreator: (applicationServicesHost: AppHost) => TimerHandler; + handlerCreator: (applicationServicesHost: RequestScopedHost) => TimerHandler; } type Phase = 'infrastructure' | 'context' | 'app-services' | 'handlers' | 'started'; @@ -312,7 +312,7 @@ export class Cellix return this; } - public registerAzureFunctionTimerHandler = AppHost>( + public registerAzureFunctionTimerHandler = AppHost>( name: string, schedule: string, handlerCreator: ( @@ -320,7 +320,13 @@ export class Cellix ) => TimerHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); - this.pendingTimerHandlers.push({ name, schedule, handlerCreator: handlerCreator as (host: AppHost) => TimerHandler }); + // Type assertion is safe here because TFactory extends RequestScopedHost + // and the actual runtime value passed will be compatible with whatever TFactory the caller specified + this.pendingTimerHandlers.push({ + name, + schedule, + handlerCreator: handlerCreator as (host: RequestScopedHost) => TimerHandler + }); this.phase = 'handlers'; return this; } diff --git a/apps/api/src/index.ts b/apps/api/src/index.ts index 7f91c48ee..4b73c99a2 100644 --- a/apps/api/src/index.ts +++ b/apps/api/src/index.ts @@ -87,8 +87,11 @@ Cellix.initializeInfrastructureServices( messagingService, blobStorageService, listingDeletionConfig: { + // biome-ignore lint/complexity/useLiteralKeys: TypeScript requires bracket notation for process.env archivalMonths: Number(process.env['LISTING_ARCHIVAL_MONTHS']) || 6, + // biome-ignore lint/complexity/useLiteralKeys: TypeScript requires bracket notation for process.env batchSize: Number(process.env['LISTING_DELETION_BATCH_SIZE']) || 100, + // biome-ignore lint/complexity/useLiteralKeys: TypeScript requires bracket notation for process.env blobContainerName: process.env['LISTING_IMAGES_CONTAINER'] || 'listing-images', }, }; From d481322aa877de9cdb328514023fd801a25547d3 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 14:01:57 -0500 Subject: [PATCH 07/14] fix coverage error --- .../conversation/conversation/delete-by-listing.test.ts | 2 +- .../contexts/listing/item/process-expired-deletions.test.ts | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts index 2fdcc83d1..7fbeea6b9 100644 --- a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.test.ts @@ -70,7 +70,7 @@ test.for(feature, ({ Background, Scenario }) => { Conversation: { Conversation: { ConversationReadRepo: { - getByListingId: vi.fn().mockResolvedValue(mockConversations), + getByListingId: vi.fn().mockImplementation(() => Promise.resolve(mockConversations)), }, }, }, diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts index dc6a8bbc2..9c617c3c6 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.test.ts @@ -106,14 +106,14 @@ test.for(feature, ({ Background, Scenario }) => { Listing: { ItemListing: { ItemListingReadRepo: { - getExpiredForDeletion: vi.fn().mockResolvedValue(mockExpiredListings), + getExpiredForDeletion: vi.fn().mockImplementation(() => Promise.resolve(mockExpiredListings)), }, }, }, Conversation: { Conversation: { ConversationReadRepo: { - getByListingId: vi.fn().mockResolvedValue(mockConversations), + getByListingId: vi.fn().mockImplementation(() => Promise.resolve(mockConversations)), }, }, }, From d5ceacc072c027f960b6c10f54a06958c897dfc1 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 15:07:00 -0500 Subject: [PATCH 08/14] fix: enhance error handling and logging in getBySharerReserverListing method --- .../conversation.read-repository.test.ts | 24 +++++++++---- .../conversation.read-repository.ts | 36 ++++++++++++------- 2 files changed, 40 insertions(+), 20 deletions(-) diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts index 30b984a72..01335f7ee 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts @@ -48,10 +48,21 @@ function makePassport(): Domain.Passport { } function createNullPopulateChain(result: T) { - const innerLean = { lean: vi.fn(async () => result) }; - const innerPopulate = { populate: vi.fn(() => innerLean) }; - const outerPopulate = { populate: vi.fn(() => innerPopulate) }; - return { populate: vi.fn(() => outerPopulate) }; + const mockQuery = { + lean: vi.fn(), + populate: vi.fn(), + exec: vi.fn().mockResolvedValue(result), + catch: vi.fn((onReject) => Promise.resolve(result).catch(onReject)), + }; + mockQuery.lean.mockReturnValue(mockQuery); + mockQuery.populate.mockReturnValue(mockQuery); + // biome-ignore lint/suspicious/noThenProperty: Intentional thenable mock for Mongoose queries + Object.defineProperty(mockQuery, 'then', { + value: vi.fn((onResolve) => Promise.resolve(result).then(onResolve)), + enumerable: false, + configurable: true, + }); + return mockQuery; } function makeMockUser(id: string): Models.User.PersonalUser { @@ -323,9 +334,8 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { reserverId = createValidObjectId('reserver-1'); listingId = createValidObjectId('listing-1'); - mockModel.findOne = vi.fn().mockReturnValue({ - lean: vi.fn().mockResolvedValue(makeMockConversation()), - }) as never; + const mockConversation = makeMockConversation(); + mockModel.findOne = vi.fn(() => createNullPopulateChain(mockConversation)) as unknown as typeof mockModel.findOne; }); When('I call getBySharerReserverListing', async () => { diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts index 7b83806e1..11042a0f2 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.ts @@ -148,21 +148,31 @@ export class ConversationReadRepositoryImpl return null; } - const result = await this.mongoDataSource.findOne( - { - sharer: sharerObjectId, - reserver: reserverObjectId, - listing: listingObjectId, - }, - { - ...options, - populateFields: populateFields, - }, - ); - if (!result) { + try { + const result = await this.mongoDataSource.findOne( + { + sharer: sharerObjectId, + reserver: reserverObjectId, + listing: listingObjectId, + }, + { + ...options, + populateFields: populateFields, + }, + ); + if (!result) { + return null; + } + return this.converter.toDomain(result, this.passport); + } catch (error) { + console.error('[ConversationReadRepository] Error in getBySharerReserverListing:', { + sharerId, + reserverId, + listingId, + error: error instanceof Error ? error.message : String(error), + }); return null; } - return this.converter.toDomain(result, this.passport); } async getByListingId( From c9e4176d225f99162069024c1944c9aa23530f11 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Thu, 8 Jan 2026 15:16:26 -0500 Subject: [PATCH 09/14] fix: add validation for archivalMonths in getExpiredForDeletion method --- .../listing/item/item-listing.read-repository.ts | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts b/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts index 239f296b0..43cb2027d 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/listing/item/item-listing.read-repository.ts @@ -201,6 +201,18 @@ class ItemListingReadRepositoryImpl archivalMonths: number, limit = 100, ): Promise { + // Validate archivalMonths to avoid unexpected date calculation when misconfigured + if (!Number.isFinite(archivalMonths) || archivalMonths <= 0) { + console.error( + '[ItemListingReadRepository] Invalid archivalMonths value:', + { + archivalMonths, + isFinite: Number.isFinite(archivalMonths), + }, + ); + return []; + } + const cutoffDate = new Date(); cutoffDate.setMonth(cutoffDate.getMonth() - archivalMonths); From 8f22120a3b1b76af731e975d3895f01ffbc30543 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Fri, 9 Jan 2026 10:32:15 -0500 Subject: [PATCH 10/14] fix snyk vulnerability --- .snyk | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/.snyk b/.snyk index 11a711c6d..2e4e6271c 100644 --- a/.snyk +++ b/.snyk @@ -33,3 +33,13 @@ ignore: reason: 'Transitive dependency in express, @docusaurus/core, @apollo/server, apollo-link-rest; not exploitable in current usage.' expires: '2026-01-19T00:00:00.000Z' created: '2026-01-05T09:39:00.000Z' + 'SNYK-JS-PNPMNPMCONF-14897556': + - '@docusaurus/core@3.9.2 > update-notifier@6.0.2 > latest-version@7.0.0 > package-json@8.1.1 > registry-auth-token@5.1.0 > @pnpm/npm-conf@2.3.1': + reason: 'Deep transitive dependency in Docusaurus; command injection vulnerability not exploitable in docs build context (static site generator, no untrusted input). Upgrade path blocked until Docusaurus updates update-notifier. Will reassess when Docusaurus 3.10+ is available.' + expires: '2026-07-08T00:00:00.000Z' + created: '2026-01-09T00:00:00.000Z' + - '@docusaurus/preset-classic@3.9.2 > @docusaurus/core@3.9.2 > update-notifier@6.0.2 > latest-version@7.0.0 > package-json@8.1.1 > registry-auth-token@5.1.0 > @pnpm/npm-conf@2.3.1': + reason: 'Deep transitive dependency in Docusaurus; command injection vulnerability not exploitable in docs build context (static site generator, no untrusted input). Upgrade path blocked until Docusaurus updates update-notifier. Will reassess when Docusaurus 3.10+ is available.' + expires: '2026-07-08T00:00:00.000Z' + created: '2026-01-09T00:00:00.000Z' + From f02f121af3c04d8ed5d6325198641d477cc7d78f Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Fri, 9 Jan 2026 11:48:05 -0500 Subject: [PATCH 11/14] fix: update handler signatures to improve type safety in Azure function timer registration --- apps/api/src/cellix.ts | 18 ++++++++---------- .../timers/expired-listing-deletion-handler.ts | 9 ++++++--- 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index 1e02e869a..05f2ed327 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -117,15 +117,15 @@ interface AzureFunctionHandlerRegistry = AppHost>( + */ + registerAzureFunctionTimerHandler( name: string, schedule: string, handlerCreator: ( - applicationServicesFactory: TFactory, + applicationServicesHost: RequestScopedHost, ) => TimerHandler, ): AzureFunctionHandlerRegistry; - /** + /** * Finalizes configuration and starts the application. * * @remarks @@ -177,7 +177,7 @@ interface InitializedServiceRegistry { type UninitializedServiceRegistry = InfrastructureServiceRegistry; -type RequestScopedHost = { +export type RequestScopedHost = { forRequest(rawAuthHeader?: string, hints?: H): Promise; }; @@ -312,20 +312,18 @@ export class Cellix return this; } - public registerAzureFunctionTimerHandler = AppHost>( + public registerAzureFunctionTimerHandler( name: string, schedule: string, handlerCreator: ( - applicationServicesFactory: TFactory, + applicationServicesHost: RequestScopedHost, ) => TimerHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); - // Type assertion is safe here because TFactory extends RequestScopedHost - // and the actual runtime value passed will be compatible with whatever TFactory the caller specified this.pendingTimerHandlers.push({ name, schedule, - handlerCreator: handlerCreator as (host: RequestScopedHost) => TimerHandler + handlerCreator, }); this.phase = 'handlers'; return this; diff --git a/apps/api/src/timers/expired-listing-deletion-handler.ts b/apps/api/src/timers/expired-listing-deletion-handler.ts index 574c70481..e7ff6a560 100644 --- a/apps/api/src/timers/expired-listing-deletion-handler.ts +++ b/apps/api/src/timers/expired-listing-deletion-handler.ts @@ -1,11 +1,12 @@ import type { TimerHandler } from '@azure/functions'; -import type { ApplicationServicesFactory } from '@sthrift/application-services'; +import type { RequestScopedHost } from '../cellix.ts'; +import type { ApplicationServices, ApplicationServicesFactory } from '@sthrift/application-services'; import { trace, SpanStatusCode } from '@opentelemetry/api'; const tracer = trace.getTracer('timer:expired-listing-deletion'); export const expiredListingDeletionHandlerCreator = ( - applicationServicesFactory: ApplicationServicesFactory, + applicationServicesHost: RequestScopedHost, ): TimerHandler => { return async (timer, context) => { await tracer.startActiveSpan('processExpiredListingDeletions', async (span) => { @@ -16,7 +17,9 @@ export const expiredListingDeletionHandlerCreator = ( context.log('ExpiredListingDeletion: Timer is past due'); } - const systemServices = applicationServicesFactory.forSystemTask(); + // At runtime, this will be an ApplicationServicesFactory which has forSystemTask() + const factory = applicationServicesHost as ApplicationServicesFactory; + const systemServices = factory.forSystemTask(); const result = await systemServices.Listing.ItemListing.processExpiredDeletions(); span.setAttribute('deletedCount', result.deletedCount); From 2386054a0d42019645928f40bbd1039fcb3f2ca2 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Fri, 9 Jan 2026 14:36:37 -0500 Subject: [PATCH 12/14] fix: enhance type safety and improve query chaining in various handlers and tests --- apps/api/src/cellix.ts | 10 +- .../expired-listing-deletion-handler.ts | 11 +-- .../conversation/delete-by-listing.ts | 4 + .../listing/item/process-expired-deletions.ts | 35 +++++-- .../conversation.read-repository.test.ts | 96 ++++++------------- ...eservation-request.read-repository.test.ts | 64 ++++++------- 6 files changed, 97 insertions(+), 123 deletions(-) diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index 05f2ed327..e90a21e9f 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -118,11 +118,11 @@ interface AzureFunctionHandlerRegistry = AppHost>( name: string, schedule: string, handlerCreator: ( - applicationServicesHost: RequestScopedHost, + applicationServicesHost: TFactory, ) => TimerHandler, ): AzureFunctionHandlerRegistry; /** @@ -312,18 +312,18 @@ export class Cellix return this; } - public registerAzureFunctionTimerHandler( + public registerAzureFunctionTimerHandler = AppHost>( name: string, schedule: string, handlerCreator: ( - applicationServicesHost: RequestScopedHost, + applicationServicesHost: TFactory, ) => TimerHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); this.pendingTimerHandlers.push({ name, schedule, - handlerCreator, + handlerCreator: handlerCreator as (host: RequestScopedHost) => TimerHandler, }); this.phase = 'handlers'; return this; diff --git a/apps/api/src/timers/expired-listing-deletion-handler.ts b/apps/api/src/timers/expired-listing-deletion-handler.ts index e7ff6a560..02360a4bc 100644 --- a/apps/api/src/timers/expired-listing-deletion-handler.ts +++ b/apps/api/src/timers/expired-listing-deletion-handler.ts @@ -1,15 +1,14 @@ import type { TimerHandler } from '@azure/functions'; -import type { RequestScopedHost } from '../cellix.ts'; -import type { ApplicationServices, ApplicationServicesFactory } from '@sthrift/application-services'; +import type { ApplicationServicesFactory } from '@sthrift/application-services'; import { trace, SpanStatusCode } from '@opentelemetry/api'; const tracer = trace.getTracer('timer:expired-listing-deletion'); export const expiredListingDeletionHandlerCreator = ( - applicationServicesHost: RequestScopedHost, + applicationServicesHost: ApplicationServicesFactory, ): TimerHandler => { return async (timer, context) => { - await tracer.startActiveSpan('processExpiredListingDeletions', async (span) => { + await tracer.startActiveSpan('processExpiredDeletions', async (span) => { try { context.log('ExpiredListingDeletion: Timer triggered'); @@ -17,9 +16,7 @@ export const expiredListingDeletionHandlerCreator = ( context.log('ExpiredListingDeletion: Timer is past due'); } - // At runtime, this will be an ApplicationServicesFactory which has forSystemTask() - const factory = applicationServicesHost as ApplicationServicesFactory; - const systemServices = factory.forSystemTask(); + const systemServices = applicationServicesHost.forSystemTask(); const result = await systemServices.Listing.ItemListing.processExpiredDeletions(); span.setAttribute('deletedCount', result.deletedCount); diff --git a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts index ad25efa4d..34a489337 100644 --- a/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts +++ b/packages/sthrift/application-services/src/contexts/conversation/conversation/delete-by-listing.ts @@ -27,6 +27,10 @@ export const deleteByListing = (dataSources: DataSources) => { dataSources.domainDataSource.Conversation.Conversation .ConversationUnitOfWork; + // Note: Using per-conversation transactions to maintain error isolation. + // While a single batched transaction would be more performant, it would + // cause all deletions to fail if any one conversation has an issue. + // This approach ensures maximum resilience and partial success. for (const conversation of conversations) { const conversationId = conversation.id; diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts index 792d80001..e9a8777c7 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts @@ -3,6 +3,8 @@ import type { DataSources } from '@sthrift/persistence'; import type { ListingDeletionConfig } from '@sthrift/context-spec'; import { deleteByListing as deleteConversationsByListing } from '../../conversation/conversation/delete-by-listing.ts'; +type BlobStorageService = Domain.Services['BlobStorage']; + export interface ProcessExpiredDeletionsResult { deletedCount: number; deletedListingIds: string[]; @@ -12,17 +14,34 @@ export interface ProcessExpiredDeletionsResult { } async function deleteListingImages( - blobStorage: Domain.Services['BlobStorage'], + blobStorage: BlobStorageService, images: string[], containerName: string, ): Promise { let deletedCount = 0; - for (const imagePath of images) { - try { - await blobStorage.deleteBlob(containerName, imagePath); - deletedCount++; - } catch (error) { - console.warn(`[ExpiredDeletion] Failed to delete image ${imagePath}: ${error instanceof Error ? error.message : String(error)}`); + // Process images concurrently with bounded concurrency + const imagePromises = images.map((imagePath) => + blobStorage + .deleteBlob(containerName, imagePath) + .then(() => ({ success: true as const, imagePath })) + .catch((error) => ({ + success: false as const, + imagePath, + error: + error instanceof Error ? error.message : String(error), + })), + ); + + const results = await Promise.allSettled(imagePromises); + for (const result of results) { + if (result.status === 'fulfilled') { + if (result.value.success) { + deletedCount++; + } else { + console.warn( + `[ExpiredDeletion] Failed to delete image ${result.value.imagePath}: ${result.value.error}`, + ); + } } } return deletedCount; @@ -42,7 +61,7 @@ async function deleteListingById( export const processExpiredDeletions = ( dataSources: DataSources, config: ListingDeletionConfig, - blobStorage?: Domain.Services['BlobStorage'], + blobStorage?: BlobStorageService, ): (() => Promise) => { return async (): Promise => { const result: ProcessExpiredDeletionsResult = { diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts index 01335f7ee..bc35611df 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts @@ -47,21 +47,26 @@ function makePassport(): Domain.Passport { } as unknown as Domain.Passport); } -function createNullPopulateChain(result: T) { +function createQueryChain(result: T) { const mockQuery = { lean: vi.fn(), populate: vi.fn(), exec: vi.fn().mockResolvedValue(result), catch: vi.fn((onReject) => Promise.resolve(result).catch(onReject)), }; + mockQuery.lean.mockReturnValue(mockQuery); mockQuery.populate.mockReturnValue(mockQuery); + // biome-ignore lint/suspicious/noThenProperty: Intentional thenable mock for Mongoose queries Object.defineProperty(mockQuery, 'then', { - value: vi.fn((onResolve) => Promise.resolve(result).then(onResolve)), + value: vi.fn((onResolve, onReject) => + Promise.resolve(result).then(onResolve, onReject), + ), enumerable: false, configurable: true, }); + return mockQuery; } @@ -144,34 +149,10 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { passport = makePassport(); mockConversations = [makeMockConversation()]; - // Create mock query that supports chaining and is thenable - const createMockQuery = (result: unknown) => { - const mockQuery = { - lean: vi.fn(), - populate: vi.fn(), - exec: vi.fn().mockResolvedValue(result), - catch: vi.fn((onReject) => Promise.resolve(result).catch(onReject)), - }; - // Configure methods to return the query object for chaining - mockQuery.lean.mockReturnValue(mockQuery); - mockQuery.populate.mockReturnValue(mockQuery); - - // SONARQUBE SUPPRESSION: S7739 - Intentional thenable mock - // This object intentionally implements the 'then' property to mock Mongoose - // query behavior. Mongoose queries are thenable and can be awaited. - // biome-ignore lint/suspicious/noThenProperty: Intentional thenable mock for Mongoose queries - Object.defineProperty(mockQuery, 'then', { - value: vi.fn((onResolve) => Promise.resolve(result).then(onResolve)), - enumerable: false, - configurable: true, - }); - return mockQuery; - }; - mockModel = { - find: vi.fn(() => createMockQuery(mockConversations)), - findById: vi.fn(() => createMockQuery(mockConversations[0])), - findOne: vi.fn(() => createMockQuery(mockConversations[0] || null)), + find: vi.fn(() => createQueryChain(mockConversations)), + findById: vi.fn(() => createQueryChain(mockConversations[0])), + findOne: vi.fn(() => createQueryChain(mockConversations[0] || null)), } as unknown as Models.Conversation.ConversationModelType; const modelsContext = { @@ -239,7 +220,7 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { Scenario('Getting a conversation by nonexistent ID', ({ When, Then }) => { When('I call getById with "nonexistent-id"', async () => { - mockModel.findById = vi.fn(() => createNullPopulateChain(null)) as unknown as typeof mockModel.findById; + mockModel.findById = vi.fn(() => createQueryChain(null)) as unknown as typeof mockModel.findById; result = await repository.getById('nonexistent-id'); }); @@ -300,7 +281,7 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { 'Getting conversations by user ID with no conversations', ({ When, Then }) => { When('I call getByUser with "user-without-conversations"', async () => { - mockModel.find = vi.fn(() => createNullPopulateChain([])) as unknown as typeof mockModel.find; + mockModel.find = vi.fn(() => createQueryChain([])) as unknown as typeof mockModel.find; result = await repository.getByUser(createValidObjectId('user-without-conversations')); }); @@ -330,15 +311,13 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { let listingId: string; Given('valid sharer, reserver, and listing IDs', () => { - sharerId = createValidObjectId('sharer-1'); - reserverId = createValidObjectId('reserver-1'); - listingId = createValidObjectId('listing-1'); + sharerId = createValidObjectId('sharer-1'); + reserverId = createValidObjectId('reserver-1'); + listingId = createValidObjectId('listing-1'); - const mockConversation = makeMockConversation(); - mockModel.findOne = vi.fn(() => createNullPopulateChain(mockConversation)) as unknown as typeof mockModel.findOne; - }); - - When('I call getBySharerReserverListing', async () => { + const mockConversation = makeMockConversation(); + mockModel.findOne = vi.fn(() => createQueryChain(mockConversation)) as unknown as typeof mockModel.findOne; + }); When('I call getBySharerReserverListing', async () => { result = await repository.getBySharerReserverListing(sharerId, reserverId, listingId); }); @@ -433,34 +412,15 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { }); Scenario('Getting conversations by listing ID', ({ Given, When, Then, And }) => { - Given('a Conversation document with listing "listing-1"', () => { - mockConversations = [ - makeMockConversation({ - listing: makeMockListing('listing-1'), - }), - ]; - // Update mock model to return new conversations - const createMockQuery = (queryResult: unknown) => { - const mockQuery = { - lean: vi.fn(), - populate: vi.fn(), - exec: vi.fn().mockResolvedValue(queryResult), - catch: vi.fn((onReject) => Promise.resolve(queryResult).catch(onReject)), - }; - mockQuery.lean.mockReturnValue(mockQuery); - mockQuery.populate.mockReturnValue(mockQuery); - // biome-ignore lint/suspicious/noThenProperty: Intentional thenable mock for Mongoose queries - Object.defineProperty(mockQuery, 'then', { - value: vi.fn((onResolve) => Promise.resolve(queryResult).then(onResolve)), - enumerable: false, - configurable: true, - }); - return mockQuery; - }; - mockModel.find = vi.fn(() => createMockQuery(mockConversations)) as unknown as typeof mockModel.find; - }); - - When('I call getByListingId with "listing-1"', async () => { + Given('a Conversation document with listing "listing-1"', () => { + mockConversations = [ + makeMockConversation({ + listing: makeMockListing('listing-1'), + }), + ]; + // Update mock model to return new conversations + mockModel.find = vi.fn(() => createQueryChain(mockConversations)) as unknown as typeof mockModel.find; + }); When('I call getByListingId with "listing-1"', async () => { result = await repository.getByListingId(createValidObjectId('listing-1')); }); @@ -477,7 +437,7 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { Scenario('Getting conversations by listing ID with no conversations', ({ When, Then }) => { When('I call getByListingId with "listing-without-conversations"', async () => { - mockModel.find = vi.fn(() => createNullPopulateChain([])) as unknown as typeof mockModel.find; + mockModel.find = vi.fn(() => createQueryChain([])) as unknown as typeof mockModel.find; result = await repository.getByListingId(createValidObjectId('listing-without-conversations')); }); diff --git a/packages/sthrift/persistence/src/datasources/readonly/reservation-request/reservation-request/reservation-request.read-repository.test.ts b/packages/sthrift/persistence/src/datasources/readonly/reservation-request/reservation-request/reservation-request.read-repository.test.ts index 9b334ca78..01b2c7fc5 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/reservation-request/reservation-request/reservation-request.read-repository.test.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/reservation-request/reservation-request/reservation-request.read-repository.test.ts @@ -1,12 +1,12 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { describeFeature, loadFeature } from '@amiceli/vitest-cucumber'; -import { expect, vi } from 'vitest'; +import { MongooseSeedwork } from '@cellix/mongoose-seedwork'; import type { Models } from '@sthrift/data-sources-mongoose-models'; -import type { ModelsContext } from '../../../../models-context.ts'; import type { Domain } from '@sthrift/domain'; +import { expect, vi } from 'vitest'; +import type { ModelsContext } from '../../../../models-context.ts'; import { ReservationRequestReadRepositoryImpl } from './reservation-request.read-repository.ts'; -import { MongooseSeedwork } from '@cellix/mongoose-seedwork'; // Helper to create a valid 24-character hex string from a simple ID function createValidObjectId(id: string): string { @@ -48,10 +48,26 @@ function makePassport(): Domain.Passport { } as unknown as Domain.Passport); } -function createNullPopulateChain(result: T) { - const innerLean = { lean: vi.fn(async () => result) }; - const innerPopulate = { populate: vi.fn(() => innerLean) }; - return { populate: vi.fn(() => innerPopulate) }; +function createQueryChain(result: T) { + const mockQuery = { + lean: vi.fn(), + populate: vi.fn(), + exec: vi.fn().mockResolvedValue(result), + catch: vi.fn((onReject) => Promise.resolve(result).catch(onReject)), + }; + + mockQuery.lean.mockReturnValue(mockQuery); + mockQuery.populate.mockReturnValue(mockQuery); + + Object.defineProperty(mockQuery, 'then', { + value: vi.fn((onResolve, onReject) => + Promise.resolve(result).then(onResolve, onReject), + ), + enumerable: false, + configurable: true, + }); + + return mockQuery; } function makeMockUser(id: string): Models.User.PersonalUser { @@ -143,34 +159,12 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { passport = makePassport(); mockReservationRequests = [makeMockReservationRequest()]; - // Create mock query that supports chaining and is thenable - const createMockQuery = (result: unknown) => { - const mockQuery = { - lean: vi.fn(), - populate: vi.fn(), - sort: vi.fn(), - limit: vi.fn(), - exec: vi.fn().mockResolvedValue(result), - catch: vi.fn((onReject) => Promise.resolve(result).catch(onReject)), - }; - // Configure methods to return the query object for chaining - mockQuery.lean.mockReturnValue(mockQuery); - mockQuery.populate.mockReturnValue(mockQuery); - mockQuery.sort.mockReturnValue(mockQuery); - mockQuery.limit.mockReturnValue(mockQuery); - - // Make the query thenable (like Mongoose queries are) by adding then as property - Object.defineProperty(mockQuery, 'then', { - value: vi.fn((onResolve) => Promise.resolve(result).then(onResolve)), - enumerable: false, - }); - return mockQuery; - }; - mockModel = { - find: vi.fn(() => createMockQuery(mockReservationRequests)), - findById: vi.fn(() => createMockQuery(mockReservationRequests[0])), - findOne: vi.fn(() => createMockQuery(mockReservationRequests[0] || null)), + find: vi.fn(() => createQueryChain(mockReservationRequests)), + findById: vi.fn(() => createQueryChain(mockReservationRequests[0])), + findOne: vi.fn(() => + createQueryChain(mockReservationRequests[0] || null), + ), aggregate: vi.fn(() => ({ exec: vi.fn().mockResolvedValue(mockReservationRequests), })), @@ -260,7 +254,7 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { ({ When, Then }) => { When('I call getById with "nonexistent-id"', async () => { mockModel.findById = vi.fn(() => - createNullPopulateChain(null), + createQueryChain(null), ) as unknown as typeof mockModel.findById; result = await repository.getById('nonexistent-id'); From 07e8f435ec80c2f69cfb1944b788482330ff93b7 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Fri, 9 Jan 2026 16:13:40 -0500 Subject: [PATCH 13/14] refactor: improve code readability and consistency in process-expired-deletions and conversation read repository tests - Updated comments for clarity in process-expired-deletions.ts - Adjusted formatting for better readability in conversation.read-repository.test.ts - Consolidated import statements in conversation.read-repository.test.ts - Enhanced error handling and test scenarios for missing IDs and invalid ObjectId formats --- apps/api/src/cellix.ts | 559 ++++++++++-------- .../listing/item/process-expired-deletions.ts | 13 +- .../conversation.read-repository.test.ts | 391 +++++++----- 3 files changed, 565 insertions(+), 398 deletions(-) diff --git a/apps/api/src/cellix.ts b/apps/api/src/cellix.ts index e90a21e9f..2ffc99a83 100644 --- a/apps/api/src/cellix.ts +++ b/apps/api/src/cellix.ts @@ -1,101 +1,116 @@ -import { app, type HttpFunctionOptions, type HttpHandler, type TimerHandler } from '@azure/functions'; +import { + app, + type HttpFunctionOptions, + type HttpHandler, + type TimerHandler, +} from '@azure/functions'; import type { ServiceBase } from '@cellix/api-services-spec'; import api, { SpanStatusCode, type Tracer, trace } from '@opentelemetry/api'; -interface InfrastructureServiceRegistry { - /** - * Registers an infrastructure service with the application. - * - * @remarks - * Must be called during the {@link Phase | 'infrastructure'} phase. Each - * constructor key can be registered at most once. - * - * @typeParam T - The concrete service type. - * @param service - The service instance to register. - * @returns The registry (for chaining). - * - * @throws Error - If called outside the infrastructure phase or the service key is already registered. - */ - registerInfrastructureService(service: T): InfrastructureServiceRegistry; +interface InfrastructureServiceRegistry< + ContextType = unknown, + AppServices = unknown, +> { + /** + * Registers an infrastructure service with the application. + * + * @remarks + * Must be called during the {@link Phase | 'infrastructure'} phase. Each + * constructor key can be registered at most once. + * + * @typeParam T - The concrete service type. + * @param service - The service instance to register. + * @returns The registry (for chaining). + * + * @throws Error - If called outside the infrastructure phase or the service key is already registered. + */ + registerInfrastructureService( + service: T, + ): InfrastructureServiceRegistry; } interface ContextBuilder { - /** - * Defines the infrastructure context available for the application. - * - * @remarks - * Must be called during the {@link Phase | 'infrastructure'} phase. Stores the `contextCreator` - * and transitions the application to the {@link Phase | 'context'} phase. The provided function - * will be invoked during {@link startUp} (inside the Azure Functions `appStart` hook) after all - * infrastructure services have successfully started. Note that `ContextType` is defined in the - * `api-context-spec` package. - * - * @param contextCreator - Function that builds the infrastructure context from the initialized service registry. - * @returns An {@link ApplicationServicesInitializer} for configuring application services. - * - * @throws Error - If called outside the 'infrastructure' phase. - */ + /** + * Defines the infrastructure context available for the application. + * + * @remarks + * Must be called during the {@link Phase | 'infrastructure'} phase. Stores the `contextCreator` + * and transitions the application to the {@link Phase | 'context'} phase. The provided function + * will be invoked during {@link startUp} (inside the Azure Functions `appStart` hook) after all + * infrastructure services have successfully started. Note that `ContextType` is defined in the + * `api-context-spec` package. + * + * @param contextCreator - Function that builds the infrastructure context from the initialized service registry. + * @returns An {@link ApplicationServicesInitializer} for configuring application services. + * + * @throws Error - If called outside the 'infrastructure' phase. + */ setContext( - contextCreator: (serviceRegistry: InitializedServiceRegistry) => ContextType, + contextCreator: ( + serviceRegistry: InitializedServiceRegistry, + ) => ContextType, ): ApplicationServicesInitializer; } interface ApplicationServicesInitializer { - /** - * Registers the factory that creates the request-scoped application services host. - * - * @remarks - * Must be called during the {@link Phase | 'context'} phase, after {@link setContext}. Stores the - * factory and transitions the application to the {@link Phase | 'app-services'} phase. The factory - * will be invoked during {@link startUp} to produce an {@link AppHost} that can build - * request-scoped services via {@link AppHost.forRequest}. Note that `AppServices` is defined in the - * `api-application-services` package. - * - * @param factory - Function that produces the application services host from the infrastructure context. - * @returns An {@link AzureFunctionHandlerRegistry} for registering HTTP handlers or starting the app. - * - * @throws Error - If the context creator has not been set via {@link setContext}, or if called outside the 'context' phase. - * - * @example - * ```ts - * initializeApplicationServices((infraCtx) => createAppHost(infraCtx)) - * .registerAzureFunctionHttpHandler('health', { authLevel: 'anonymous' }, (host) => async (req, fnCtx) => { - * const app = await host.forRequest(); - * return app.Health.handle(req, fnCtx); - * }); - * ``` - */ + /** + * Registers the factory that creates the request-scoped application services host. + * + * @remarks + * Must be called during the {@link Phase | 'context'} phase, after {@link setContext}. Stores the + * factory and transitions the application to the {@link Phase | 'app-services'} phase. The factory + * will be invoked during {@link startUp} to produce an {@link AppHost} that can build + * request-scoped services via {@link AppHost.forRequest}. Note that `AppServices` is defined in the + * `api-application-services` package. + * + * @param factory - Function that produces the application services host from the infrastructure context. + * @returns An {@link AzureFunctionHandlerRegistry} for registering HTTP handlers or starting the app. + * + * @throws Error - If the context creator has not been set via {@link setContext}, or if called outside the 'context' phase. + * + * @example + * ```ts + * initializeApplicationServices((infraCtx) => createAppHost(infraCtx)) + * .registerAzureFunctionHttpHandler('health', { authLevel: 'anonymous' }, (host) => async (req, fnCtx) => { + * const app = await host.forRequest(); + * return app.Health.handle(req, fnCtx); + * }); + * ``` + */ initializeApplicationServices( - factory: (infrastructureContext: ContextType) => AppHost + factory: (infrastructureContext: ContextType) => AppHost, ): AzureFunctionHandlerRegistry; } -interface AzureFunctionHandlerRegistry { - /** - * Registers an Azure Function HTTP endpoint. - * - * @remarks - * The `handlerCreator` is invoked per request and receives the application services host. - * Use it to create a request-scoped handler (e.g., to build per-request context). - * Registration is allowed in phases `'app-services'` and `'handlers'`. - * - * @param name - Function name to bind in Azure Functions. - * @param options - Azure Functions HTTP options (excluding the handler). - * @param handlerCreator - Factory that, given the app services host, returns an `HttpHandler`. - * @returns The registry (for chaining). - * - * @throws Error - If called before application services are initialized. - * - * @example - * ```ts - * registerAzureFunctionHttpHandler('graphql', { authLevel: 'anonymous' }, (host) => { - * return async (req, ctx) => { - * const app = await host.forRequest(req.headers.get('authorization') ?? undefined); - * return app.GraphQL.handle(req, ctx); - * }; - * }); - * ``` - */ +interface AzureFunctionHandlerRegistry< + ContextType = unknown, + AppServices = unknown, +> { + /** + * Registers an Azure Function HTTP endpoint. + * + * @remarks + * The `handlerCreator` is invoked per request and receives the application services host. + * Use it to create a request-scoped handler (e.g., to build per-request context). + * Registration is allowed in phases `'app-services'` and `'handlers'`. + * + * @param name - Function name to bind in Azure Functions. + * @param options - Azure Functions HTTP options (excluding the handler). + * @param handlerCreator - Factory that, given the app services host, returns an `HttpHandler`. + * @returns The registry (for chaining). + * + * @throws Error - If called before application services are initialized. + * + * @example + * ```ts + * registerAzureFunctionHttpHandler('graphql', { authLevel: 'anonymous' }, (host) => { + * return async (req, ctx) => { + * const app = await host.forRequest(req.headers.get('authorization') ?? undefined); + * return app.GraphQL.handle(req, ctx); + * }; + * }); + * ``` + */ registerAzureFunctionHttpHandler( name: string, options: Omit, @@ -103,40 +118,43 @@ interface AzureFunctionHandlerRegistry, ) => HttpHandler, ): AzureFunctionHandlerRegistry; - /** - * Registers an Azure Function Timer endpoint. - * - * @remarks - * The `handlerCreator` is invoked when the timer fires and receives the application services host. - * Use it to create a handler for scheduled tasks like cleanup, maintenance, etc. - * Registration is allowed in phases `'app-services'` and `'handlers'`. - * - * @param name - Function name to bind in Azure Functions. - * @param schedule - NCRONTAB expression for the timer schedule. - * @param handlerCreator - Factory that, given the app services host, returns a `TimerHandler`. - * @returns The registry (for chaining). - * - * @throws Error - If called before application services are initialized. + /** + * Registers an Azure Function Timer endpoint. + * + * @remarks + * The `handlerCreator` is invoked when the timer fires and receives the application services host. + * Use it to create a handler for scheduled tasks like cleanup, maintenance, etc. + * Registration is allowed in phases `'app-services'` and `'handlers'`. + * + * @param name - Function name to bind in Azure Functions. + * @param schedule - NCRONTAB expression for the timer schedule. + * @param handlerCreator - Factory that, given the app services host, returns a `TimerHandler`. + * @returns The registry (for chaining). + * + * @throws Error - If called before application services are initialized. */ - registerAzureFunctionTimerHandler = AppHost>( + registerAzureFunctionTimerHandler< + TFactory extends RequestScopedHost< + AppServices, + unknown + > = AppHost, + >( name: string, schedule: string, - handlerCreator: ( - applicationServicesHost: TFactory, - ) => TimerHandler, + handlerCreator: (applicationServicesHost: TFactory) => TimerHandler, ): AzureFunctionHandlerRegistry; /** - * Finalizes configuration and starts the application. - * - * @remarks - * This registers function handlers with Azure Functions, starts all infrastructure - * services (in parallel), builds the infrastructure context, and initializes - * application services. After this resolves, the application is in the `'started'` phase. - * - * @returns A promise that resolves to the started application facade. - * - * @throws Error - If the context builder or application services factory have not been configured. - */ + * Finalizes configuration and starts the application. + * + * @remarks + * This registers function handlers with Azure Functions, starts all infrastructure + * services (in parallel), builds the infrastructure context, and initializes + * application services. After this resolves, the application is in the `'started'` phase. + * + * @returns A promise that resolves to the started application facade. + * + * @throws Error - If the context builder or application services factory have not been configured. + */ startUp(): Promise>; } @@ -146,39 +164,41 @@ interface StartedApplication } interface InitializedServiceRegistry { - /** - * Retrieves a registered infrastructure service by its constructor key. - * - * @remarks - * Services are keyed by their constructor identity (not by name), which is - * minification-safe. You must pass the same class you used when registering - * the service; base classes or interfaces will not match. - * - * @typeParam T - The concrete service type. - * @param serviceKey - The service class (constructor) used at registration time. - * @returns The registered service instance. - * - * @throws Error - If no service is registered for the provided key. - * - * @example - * ```ts - * // registration - * registry.registerInfrastructureService(new BlobStorageService(...)); - * - * // lookup - * const blob = app.getInfrastructureService(BlobStorageService); - * await blob.startUp(); - * ``` - */ + /** + * Retrieves a registered infrastructure service by its constructor key. + * + * @remarks + * Services are keyed by their constructor identity (not by name), which is + * minification-safe. You must pass the same class you used when registering + * the service; base classes or interfaces will not match. + * + * @typeParam T - The concrete service type. + * @param serviceKey - The service class (constructor) used at registration time. + * @returns The registered service instance. + * + * @throws Error - If no service is registered for the provided key. + * + * @example + * ```ts + * // registration + * registry.registerInfrastructureService(new BlobStorageService(...)); + * + * // lookup + * const blob = app.getInfrastructureService(BlobStorageService); + * await blob.startUp(); + * ``` + */ getInfrastructureService(serviceKey: ServiceKey): T; get servicesInitialized(): boolean; } -type UninitializedServiceRegistry = InfrastructureServiceRegistry; - +type UninitializedServiceRegistry< + ContextType = unknown, + AppServices = unknown, +> = InfrastructureServiceRegistry; export type RequestScopedHost = { - forRequest(rawAuthHeader?: string, hints?: H): Promise; + forRequest(rawAuthHeader?: string, hints?: H): Promise; }; type AppHost = RequestScopedHost; @@ -186,16 +206,25 @@ type AppHost = RequestScopedHost; interface PendingHandler { name: string; options: Omit; - handlerCreator: (applicationServicesHost: AppHost) => HttpHandler; + handlerCreator: ( + applicationServicesHost: AppHost, + ) => HttpHandler; } interface PendingTimerHandler { name: string; schedule: string; - handlerCreator: (applicationServicesHost: RequestScopedHost) => TimerHandler; + handlerCreator: ( + applicationServicesHost: RequestScopedHost, + ) => TimerHandler; } -type Phase = 'infrastructure' | 'context' | 'app-services' | 'handlers' | 'started'; +type Phase = + | 'infrastructure' + | 'context' + | 'app-services' + | 'handlers' + | 'started'; /** * Minification-safe key for service lookup: the service class (constructor). @@ -214,13 +243,24 @@ export class Cellix StartedApplication { private contextInternal: ContextType | undefined; - private appServicesHostInternal: RequestScopedHost | undefined; - private contextCreatorInternal: ((serviceRegistry: InitializedServiceRegistry) => ContextType) | undefined; - private appServicesHostBuilder: ((infrastructureContext: ContextType) => RequestScopedHost) | undefined; + private appServicesHostInternal: + | RequestScopedHost + | undefined; + private contextCreatorInternal: + | ((serviceRegistry: InitializedServiceRegistry) => ContextType) + | undefined; + private appServicesHostBuilder: + | (( + infrastructureContext: ContextType, + ) => RequestScopedHost) + | undefined; private readonly tracer: Tracer; - private readonly servicesInternal: Map, ServiceBase> = new Map(); + private readonly servicesInternal: Map, ServiceBase> = + new Map(); private readonly pendingHandlers: Array> = []; - private readonly pendingTimerHandlers: Array> = []; + private readonly pendingTimerHandlers: Array< + PendingTimerHandler + > = []; private serviceInitializedInternal = false; private phase: Phase = 'infrastructure'; @@ -228,39 +268,42 @@ export class Cellix this.tracer = trace.getTracer('cellix:bootstrap'); } - /** - * Begins configuring a Cellix application by registering infrastructure services. - * - * @remarks - * This is the first step in the bootstrap sequence. It constructs a new Cellix instance in the - * {@link Phase | 'infrastructure'} phase, invokes your `registerServices` callback to register - * infrastructure services, and returns a {@link ContextBuilder} to define the infrastructure context. - * - * The typical flow is: {@link initializeInfrastructureServices} → {@link setContext} → - * {@link initializeApplicationServices} → {@link registerAzureFunctionHttpHandler} → {@link startUp}. - * - * @typeParam ContextType - The shape of your infrastructure context that will be created in {@link setContext}. - * @typeParam AppServices - The application services host type produced by {@link initializeApplicationServices}. - * - * @param registerServices - Callback invoked once to register infrastructure services. - * @returns A {@link ContextBuilder} for defining the infrastructure context. - * - * @example - * ```ts - * Cellix.initializeInfrastructureServices((r) => { - * r.registerInfrastructureService(new BlobStorageService(...)); - * r.registerInfrastructureService(new TokenValidationService(...)); - * }) - * .setContext((registry) => buildInfraContext(registry)) - * .initializeApplicationServices((ctx) => createAppHost(ctx)) - * .registerAzureFunctionHttpHandler('graphql', { authLevel: 'anonymous' }, (host) => async (req, fnCtx) => { - * const app = await host.forRequest(req.headers.get('authorization') ?? undefined); - * return app.GraphQL.handle(req, fnCtx); - * }) - * .startUp(); - * ``` - */ - public static initializeInfrastructureServices( + /** + * Begins configuring a Cellix application by registering infrastructure services. + * + * @remarks + * This is the first step in the bootstrap sequence. It constructs a new Cellix instance in the + * {@link Phase | 'infrastructure'} phase, invokes your `registerServices` callback to register + * infrastructure services, and returns a {@link ContextBuilder} to define the infrastructure context. + * + * The typical flow is: {@link initializeInfrastructureServices} → {@link setContext} → + * {@link initializeApplicationServices} → {@link registerAzureFunctionHttpHandler} → {@link startUp}. + * + * @typeParam ContextType - The shape of your infrastructure context that will be created in {@link setContext}. + * @typeParam AppServices - The application services host type produced by {@link initializeApplicationServices}. + * + * @param registerServices - Callback invoked once to register infrastructure services. + * @returns A {@link ContextBuilder} for defining the infrastructure context. + * + * @example + * ```ts + * Cellix.initializeInfrastructureServices((r) => { + * r.registerInfrastructureService(new BlobStorageService(...)); + * r.registerInfrastructureService(new TokenValidationService(...)); + * }) + * .setContext((registry) => buildInfraContext(registry)) + * .initializeApplicationServices((ctx) => createAppHost(ctx)) + * .registerAzureFunctionHttpHandler('graphql', { authLevel: 'anonymous' }, (host) => async (req, fnCtx) => { + * const app = await host.forRequest(req.headers.get('authorization') ?? undefined); + * return app.GraphQL.handle(req, fnCtx); + * }) + * .startUp(); + * ``` + */ + public static initializeInfrastructureServices< + ContextType, + AppServices = unknown, + >( registerServices: ( registry: UninitializedServiceRegistry, ) => void, @@ -270,17 +313,25 @@ export class Cellix return instance; } - public registerInfrastructureService(service: T): InfrastructureServiceRegistry { + public registerInfrastructureService( + service: T, + ): InfrastructureServiceRegistry { this.ensurePhase('infrastructure'); - const key = service.constructor as ServiceKey; + const key = service.constructor as ServiceKey; if (this.servicesInternal.has(key)) { - throw new Error(`Service already registered for constructor: ${service.constructor.name}`); + throw new Error( + `Service already registered for constructor: ${service.constructor.name}`, + ); } this.servicesInternal.set(key, service); return this; } - public setContext(contextCreator: (serviceRegistry: InitializedServiceRegistry) => ContextType): ApplicationServicesInitializer { + public setContext( + contextCreator: ( + serviceRegistry: InitializedServiceRegistry, + ) => ContextType, + ): ApplicationServicesInitializer { this.ensurePhase('infrastructure'); this.contextCreatorInternal = contextCreator; this.phase = 'context'; @@ -288,11 +339,15 @@ export class Cellix } public initializeApplicationServices( - factory: (infrastructureContext: ContextType) => RequestScopedHost, + factory: ( + infrastructureContext: ContextType, + ) => RequestScopedHost, ): AzureFunctionHandlerRegistry { this.ensurePhase('context'); if (!this.contextCreatorInternal) { - throw new Error('Context creator must be set before initializing application services'); + throw new Error( + 'Context creator must be set before initializing application services', + ); } this.appServicesHostBuilder = factory; this.phase = 'app-services'; @@ -312,18 +367,24 @@ export class Cellix return this; } - public registerAzureFunctionTimerHandler = AppHost>( + public registerAzureFunctionTimerHandler< + TFactory extends RequestScopedHost< + AppServices, + unknown + > = AppHost, + >( name: string, schedule: string, - handlerCreator: ( - applicationServicesHost: TFactory, - ) => TimerHandler, + handlerCreator: (applicationServicesHost: TFactory) => TimerHandler, ): AzureFunctionHandlerRegistry { this.ensurePhase('app-services', 'handlers'); - this.pendingTimerHandlers.push({ - name, - schedule, - handlerCreator: handlerCreator as (host: RequestScopedHost) => TimerHandler, + // Type erasure: TFactory is compatible with RequestScopedHost by constraint + // This widening is safe because the handler will receive the actual appServicesHostInternal at runtime + this.pendingTimerHandlers.push({ + name, + schedule, + handlerCreator: + handlerCreator as PendingTimerHandler['handlerCreator'], }); this.phase = 'handlers'; return this; @@ -348,7 +409,10 @@ export class Cellix if (!this.appServicesHostInternal) { throw new Error('Application not started yet'); } - return h.handlerCreator(this.appServicesHostInternal)(request, context); + return h.handlerCreator(this.appServicesHostInternal)( + request, + context, + ); }, }); } @@ -379,9 +443,13 @@ export class Cellix } this.contextInternal = this.contextCreatorInternal(this); if (!this.appServicesHostBuilder) { - throw new Error('Application services factory not provided. Call initializeApplicationServices().'); + throw new Error( + 'Application services factory not provided. Call initializeApplicationServices().', + ); } - this.appServicesHostInternal = this.appServicesHostBuilder(this.contextInternal); + this.appServicesHostInternal = this.appServicesHostBuilder( + this.contextInternal, + ); span.setStatus({ code: SpanStatusCode.OK }); console.log('Cellix started'); } catch (err) { @@ -401,33 +469,42 @@ export class Cellix app.hook.appTerminate(async () => { const root = api.context.active(); await api.context.with(root, async () => { - await this.tracer.startActiveSpan('cellix.appTerminate', async (span) => { - try { - await this.stopAllServicesWithTracing(); - span.setStatus({ code: SpanStatusCode.OK }); - console.log('Cellix stopped'); - } catch (err) { - span.setStatus({ code: SpanStatusCode.ERROR }); - if (err instanceof Error) { - span.recordException(err); + await this.tracer.startActiveSpan( + 'cellix.appTerminate', + async (span) => { + try { + await this.stopAllServicesWithTracing(); + span.setStatus({ code: SpanStatusCode.OK }); + console.log('Cellix stopped'); + } catch (err) { + span.setStatus({ code: SpanStatusCode.ERROR }); + if (err instanceof Error) { + span.recordException(err); + } + throw err; + } finally { + span.end(); } - throw err; - } finally { - span.end(); - } - }); + }, + ); }); }); } private ensurePhase(...allowed: Phase[]): void { if (!allowed.includes(this.phase)) { - throw new Error(`Invalid operation in phase '${this.phase}'. Allowed phases: ${allowed.join(', ')}`); + throw new Error( + `Invalid operation in phase '${this.phase}'. Allowed phases: ${allowed.join(', ')}`, + ); } } - public getInfrastructureService(serviceKey: ServiceKey): T { - const service = this.servicesInternal.get(serviceKey as ServiceKey); + public getInfrastructureService( + serviceKey: ServiceKey, + ): T { + const service = this.servicesInternal.get( + serviceKey as ServiceKey, + ); if (!service) { const name = (serviceKey as { name?: string }).name ?? 'UnknownService'; throw new Error(`Service not found: ${name}`); @@ -460,29 +537,45 @@ export class Cellix private async stopAllServicesWithTracing(): Promise { await this.iterateServicesWithTracing('stop', 'shutDown'); } - private async iterateServicesWithTracing(operationName: 'start' | 'stop', serviceMethod: 'startUp' | 'shutDown'): Promise { + private async iterateServicesWithTracing( + operationName: 'start' | 'stop', + serviceMethod: 'startUp' | 'shutDown', + ): Promise { const operationFullName = `${operationName.charAt(0).toUpperCase() + operationName.slice(1)}Service`; - const operationActionPending = operationName === 'start' ? 'starting' : 'stopping'; - const operationActionCompleted = operationName === 'start' ? 'started' : 'stopped'; + const operationActionPending = + operationName === 'start' ? 'starting' : 'stopping'; + const operationActionCompleted = + operationName === 'start' ? 'started' : 'stopped'; await Promise.all( Array.from(this.servicesInternal.entries()).map(([ctor, service]) => - this.tracer.startActiveSpan(`Service ${(ctor as unknown as { name?: string }).name ?? 'Service'} ${operationName}`, async (span) => { - try { - const ctorName = (ctor as unknown as { name?: string }).name ?? 'Service'; - console.log(`${operationFullName}: Service ${ctorName} ${operationActionPending}`); - await service[serviceMethod](); - span.setStatus({ code: SpanStatusCode.OK, message: `Service ${ctorName} ${operationActionCompleted}` }); - console.log(`${operationFullName}: Service ${ctorName} ${operationActionCompleted}`); - } catch (err) { - span.setStatus({ code: SpanStatusCode.ERROR }); - if (err instanceof Error) { - span.recordException(err); + this.tracer.startActiveSpan( + `Service ${(ctor as unknown as { name?: string }).name ?? 'Service'} ${operationName}`, + async (span) => { + try { + const ctorName = + (ctor as unknown as { name?: string }).name ?? 'Service'; + console.log( + `${operationFullName}: Service ${ctorName} ${operationActionPending}`, + ); + await service[serviceMethod](); + span.setStatus({ + code: SpanStatusCode.OK, + message: `Service ${ctorName} ${operationActionCompleted}`, + }); + console.log( + `${operationFullName}: Service ${ctorName} ${operationActionCompleted}`, + ); + } catch (err) { + span.setStatus({ code: SpanStatusCode.ERROR }); + if (err instanceof Error) { + span.recordException(err); + } + throw err; + } finally { + span.end(); } - throw err; - } finally { - span.end(); - } - }), + }, + ), ), ); } diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts index e9a8777c7..5edb7559f 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts @@ -1,6 +1,6 @@ +import type { ListingDeletionConfig } from '@sthrift/context-spec'; import type { Domain } from '@sthrift/domain'; import type { DataSources } from '@sthrift/persistence'; -import type { ListingDeletionConfig } from '@sthrift/context-spec'; import { deleteByListing as deleteConversationsByListing } from '../../conversation/conversation/delete-by-listing.ts'; type BlobStorageService = Domain.Services['BlobStorage']; @@ -19,7 +19,7 @@ async function deleteListingImages( containerName: string, ): Promise { let deletedCount = 0; - // Process images concurrently with bounded concurrency + // Process all images concurrently (unbounded parallelism) for maximum throughput const imagePromises = images.map((imagePath) => blobStorage .deleteBlob(containerName, imagePath) @@ -27,8 +27,7 @@ async function deleteListingImages( .catch((error) => ({ success: false as const, imagePath, - error: - error instanceof Error ? error.message : String(error), + error: error instanceof Error ? error.message : String(error), })), ); @@ -101,7 +100,9 @@ export const processExpiredDeletions = ( config.blobContainerName, ); result.deletedImagesCount += imagesDeleted; - console.log(`[ExpiredDeletion] Deleted ${imagesDeleted} images for listing ${listingId}`); + console.log( + `[ExpiredDeletion] Deleted ${imagesDeleted} images for listing ${listingId}`, + ); } const conversationResult = await deleteConversations(listingId); @@ -132,4 +133,4 @@ export const processExpiredDeletions = ( return result; }; -}; \ No newline at end of file +}; diff --git a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts index bc35611df..5df75460b 100644 --- a/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts +++ b/packages/sthrift/persistence/src/datasources/readonly/conversation/conversation/conversation.read-repository.test.ts @@ -1,12 +1,12 @@ import path from 'node:path'; import { fileURLToPath } from 'node:url'; import { describeFeature, loadFeature } from '@amiceli/vitest-cucumber'; -import { expect, vi } from 'vitest'; +import { MongooseSeedwork } from '@cellix/mongoose-seedwork'; import type { Models } from '@sthrift/data-sources-mongoose-models'; -import type { ModelsContext } from '../../../../models-context.ts'; import type { Domain } from '@sthrift/domain'; +import { expect, vi } from 'vitest'; +import type { ModelsContext } from '../../../../models-context.ts'; import { ConversationReadRepositoryImpl } from './conversation.read-repository.ts'; -import { MongooseSeedwork } from '@cellix/mongoose-seedwork'; // Helper to create a valid 24-character hex string from a simple ID function createValidObjectId(id: string): string { @@ -122,7 +122,9 @@ function makeMockConversation( ): Models.Conversation.Conversation { const conversationId = overrides.id || 'conv-1'; const defaultConv = { - _id: new MongooseSeedwork.ObjectId(createValidObjectId(conversationId as string)), + _id: new MongooseSeedwork.ObjectId( + createValidObjectId(conversationId as string), + ), id: conversationId, sharer: makeMockUser('user-1'), reserver: makeMockUser('user-2'), @@ -179,10 +181,7 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { Scenario('Getting all conversations', ({ Given, When, Then, And }) => { Given('multiple Conversation documents in the database', () => { - mockConversations = [ - makeMockConversation(), - makeMockConversation(), - ]; + mockConversations = [makeMockConversation(), makeMockConversation()]; }); When('I call getAll', async () => { result = await repository.getAll(); @@ -220,7 +219,9 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { Scenario('Getting a conversation by nonexistent ID', ({ When, Then }) => { When('I call getById with "nonexistent-id"', async () => { - mockModel.findById = vi.fn(() => createQueryChain(null)) as unknown as typeof mockModel.findById; + mockModel.findById = vi.fn(() => + createQueryChain(null), + ) as unknown as typeof mockModel.findById; result = await repository.getById('nonexistent-id'); }); @@ -269,11 +270,14 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { Then('I should receive an array of Conversation entities', () => { expect(Array.isArray(result)).toBe(true); }); - And('the array should contain conversations where user is reserver', () => { - const conversations = - result as Domain.Contexts.Conversation.Conversation.ConversationEntityReference[]; - expect(conversations.length).toBeGreaterThan(0); - }); + And( + 'the array should contain conversations where user is reserver', + () => { + const conversations = + result as Domain.Contexts.Conversation.Conversation.ConversationEntityReference[]; + expect(conversations.length).toBeGreaterThan(0); + }, + ); }, ); @@ -281,9 +285,13 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { 'Getting conversations by user ID with no conversations', ({ When, Then }) => { When('I call getByUser with "user-without-conversations"', async () => { - mockModel.find = vi.fn(() => createQueryChain([])) as unknown as typeof mockModel.find; + mockModel.find = vi.fn(() => + createQueryChain([]), + ) as unknown as typeof mockModel.find; - result = await repository.getByUser(createValidObjectId('user-without-conversations')); + result = await repository.getByUser( + createValidObjectId('user-without-conversations'), + ); }); Then('I should receive an empty array', () => { expect(Array.isArray(result)).toBe(true); @@ -305,177 +313,242 @@ test.for(feature, ({ Scenario, Background, BeforeEachScenario }) => { }, ); - Scenario('Getting conversation by sharer, reserver, and listing', ({ Given, When, Then }) => { - let sharerId: string; - let reserverId: string; - let listingId: string; - - Given('valid sharer, reserver, and listing IDs', () => { - sharerId = createValidObjectId('sharer-1'); - reserverId = createValidObjectId('reserver-1'); - listingId = createValidObjectId('listing-1'); - - const mockConversation = makeMockConversation(); - mockModel.findOne = vi.fn(() => createQueryChain(mockConversation)) as unknown as typeof mockModel.findOne; - }); When('I call getBySharerReserverListing', async () => { - result = await repository.getBySharerReserverListing(sharerId, reserverId, listingId); - }); + Scenario( + 'Getting conversation by sharer, reserver, and listing', + ({ Given, When, Then }) => { + let sharerId: string; + let reserverId: string; + let listingId: string; + + Given('valid sharer, reserver, and listing IDs', () => { + sharerId = createValidObjectId('sharer-1'); + reserverId = createValidObjectId('reserver-1'); + listingId = createValidObjectId('listing-1'); + + const mockConversation = makeMockConversation(); + mockModel.findOne = vi.fn(() => + createQueryChain(mockConversation), + ) as unknown as typeof mockModel.findOne; + }); - Then('I should receive a Conversation entity or null', () => { - expect(result).toBeDefined(); - }); - }); + When('I call getBySharerReserverListing', async () => { + result = await repository.getBySharerReserverListing( + sharerId, + reserverId, + listingId, + ); + }); - Scenario('Getting conversation with missing sharer ID', ({ Given, When, Then }) => { - Given('empty sharer ID', () => { - // Empty string setup - }); + Then('I should receive a Conversation entity or null', () => { + expect(result).toBeDefined(); + }); + }, + ); - When('I call getBySharerReserverListing with empty sharer', async () => { - result = await repository.getBySharerReserverListing('', createValidObjectId('reserver'), createValidObjectId('listing')); - }); + Scenario( + 'Getting conversation with missing sharer ID', + ({ Given, When, Then }) => { + Given('empty sharer ID', () => { + // Empty string setup + }); - Then('it should return null', () => { - expect(result).toBeNull(); - }); - }); + When('I call getBySharerReserverListing with empty sharer', async () => { + result = await repository.getBySharerReserverListing( + '', + createValidObjectId('reserver'), + createValidObjectId('listing'), + ); + }); - Scenario('Getting conversation with missing reserver ID', ({ Given, When, Then }) => { - Given('empty reserver ID', () => { - // Empty string setup - }); + Then('it should return null', () => { + expect(result).toBeNull(); + }); + }, + ); - When('I call getBySharerReserverListing with empty reserver', async () => { - result = await repository.getBySharerReserverListing(createValidObjectId('sharer'), '', createValidObjectId('listing')); - }); + Scenario( + 'Getting conversation with missing reserver ID', + ({ Given, When, Then }) => { + Given('empty reserver ID', () => { + // Empty string setup + }); - Then('it should return null', () => { - expect(result).toBeNull(); - }); - }); + When( + 'I call getBySharerReserverListing with empty reserver', + async () => { + result = await repository.getBySharerReserverListing( + createValidObjectId('sharer'), + '', + createValidObjectId('listing'), + ); + }, + ); - Scenario('Getting conversation with missing listing ID', ({ Given, When, Then }) => { - Given('empty listing ID', () => { - // Empty string setup - }); + Then('it should return null', () => { + expect(result).toBeNull(); + }); + }, + ); - When('I call getBySharerReserverListing with empty listing', async () => { - result = await repository.getBySharerReserverListing(createValidObjectId('sharer'), createValidObjectId('reserver'), ''); - }); + Scenario( + 'Getting conversation with missing listing ID', + ({ Given, When, Then }) => { + Given('empty listing ID', () => { + // Empty string setup + }); - Then('it should return null', () => { - expect(result).toBeNull(); - }); - }); + When('I call getBySharerReserverListing with empty listing', async () => { + result = await repository.getBySharerReserverListing( + createValidObjectId('sharer'), + createValidObjectId('reserver'), + '', + ); + }); - Scenario('Getting conversation with error in database query', ({ Given, When, Then }) => { - Given('an error will occur during the query', () => { - mockModel.findOne = vi.fn().mockImplementation(() => { - throw new Error('Database error'); + Then('it should return null', () => { + expect(result).toBeNull(); }); - }); + }, + ); - When('I call getBySharerReserverListing', async () => { - result = await repository.getBySharerReserverListing( - createValidObjectId('sharer'), - createValidObjectId('reserver'), - createValidObjectId('listing') - ); - }); + Scenario( + 'Getting conversation with error in database query', + ({ Given, When, Then }) => { + Given('an error will occur during the query', () => { + mockModel.findOne = vi.fn().mockImplementation(() => { + throw new Error('Database error'); + }); + }); - Then('it should return null due to error', () => { - expect(result).toBeNull(); - }); - }); + When('I call getBySharerReserverListing', async () => { + result = await repository.getBySharerReserverListing( + createValidObjectId('sharer'), + createValidObjectId('reserver'), + createValidObjectId('listing'), + ); + }); - Scenario('Getting conversation with invalid ObjectId format', ({ Given, When, Then }) => { - Given('an invalid ObjectId will cause an error', () => { - // Mock MongooseSeedwork.ObjectId constructor to throw once, then restore - vi.spyOn(MongooseSeedwork, 'ObjectId').mockImplementationOnce(() => { - throw new Error('Invalid ObjectId'); + Then('it should return null due to error', () => { + expect(result).toBeNull(); }); - }); + }, + ); - When('I call getBySharerReserverListing with invalid ID', async () => { - result = await repository.getBySharerReserverListing( - 'invalid-id', - createValidObjectId('reserver'), - createValidObjectId('listing') - ); - // Restore all mocks after test to prevent interference with subsequent tests - vi.restoreAllMocks(); - }); + Scenario( + 'Getting conversation with invalid ObjectId format', + ({ Given, When, Then }) => { + Given('an invalid ObjectId will cause an error', () => { + // Mock MongooseSeedwork.ObjectId constructor to throw once, then restore + vi.spyOn(MongooseSeedwork, 'ObjectId').mockImplementationOnce(() => { + throw new Error('Invalid ObjectId'); + }); + }); - Then('it should return null due to ObjectId error', () => { - expect(result).toBeNull(); - }); - }); + When('I call getBySharerReserverListing with invalid ID', async () => { + result = await repository.getBySharerReserverListing( + 'invalid-id', + createValidObjectId('reserver'), + createValidObjectId('listing'), + ); + // Restore all mocks after test to prevent interference with subsequent tests + vi.restoreAllMocks(); + }); - Scenario('Getting conversations by listing ID', ({ Given, When, Then, And }) => { - Given('a Conversation document with listing "listing-1"', () => { - mockConversations = [ - makeMockConversation({ - listing: makeMockListing('listing-1'), - }), - ]; - // Update mock model to return new conversations - mockModel.find = vi.fn(() => createQueryChain(mockConversations)) as unknown as typeof mockModel.find; - }); When('I call getByListingId with "listing-1"', async () => { - result = await repository.getByListingId(createValidObjectId('listing-1')); - }); + Then('it should return null due to ObjectId error', () => { + expect(result).toBeNull(); + }); + }, + ); - Then('I should receive an array of Conversation entities', () => { - expect(Array.isArray(result)).toBe(true); - }); + Scenario( + 'Getting conversations by listing ID', + ({ Given, When, Then, And }) => { + Given('a Conversation document with listing "listing-1"', () => { + mockConversations = [ + makeMockConversation({ + listing: makeMockListing('listing-1'), + }), + ]; + // Update mock model to return new conversations + mockModel.find = vi.fn(() => + createQueryChain(mockConversations), + ) as unknown as typeof mockModel.find; + }); + When('I call getByListingId with "listing-1"', async () => { + result = await repository.getByListingId( + createValidObjectId('listing-1'), + ); + }); - And('the array should contain conversations for that listing', () => { - const conversations = - result as Domain.Contexts.Conversation.Conversation.ConversationEntityReference[]; - expect(conversations.length).toBeGreaterThan(0); - }); - }); + Then('I should receive an array of Conversation entities', () => { + expect(Array.isArray(result)).toBe(true); + }); - Scenario('Getting conversations by listing ID with no conversations', ({ When, Then }) => { - When('I call getByListingId with "listing-without-conversations"', async () => { - mockModel.find = vi.fn(() => createQueryChain([])) as unknown as typeof mockModel.find; + And('the array should contain conversations for that listing', () => { + const conversations = + result as Domain.Contexts.Conversation.Conversation.ConversationEntityReference[]; + expect(conversations.length).toBeGreaterThan(0); + }); + }, + ); - result = await repository.getByListingId(createValidObjectId('listing-without-conversations')); - }); + Scenario( + 'Getting conversations by listing ID with no conversations', + ({ When, Then }) => { + When( + 'I call getByListingId with "listing-without-conversations"', + async () => { + mockModel.find = vi.fn(() => + createQueryChain([]), + ) as unknown as typeof mockModel.find; + + result = await repository.getByListingId( + createValidObjectId('listing-without-conversations'), + ); + }, + ); - Then('I should receive an empty array for listing', () => { - expect(Array.isArray(result)).toBe(true); - expect((result as unknown[]).length).toBe(0); - }); - }); + Then('I should receive an empty array for listing', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }, + ); - Scenario('Getting conversations by listing ID with empty string', ({ When, Then }) => { - When('I call getByListingId with empty string', async () => { - result = await repository.getByListingId(''); - }); + Scenario( + 'Getting conversations by listing ID with empty string', + ({ When, Then }) => { + When('I call getByListingId with empty string', async () => { + result = await repository.getByListingId(''); + }); - Then('I should receive an empty array for listing', () => { - expect(Array.isArray(result)).toBe(true); - expect((result as unknown[]).length).toBe(0); - }); - }); + Then('I should receive an empty array for listing', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }, + ); - Scenario('Getting conversations by listing ID with invalid ObjectId', ({ Given, When, Then }) => { - Given('an invalid ObjectId will cause an error', () => { - // Mock MongooseSeedwork.ObjectId constructor to throw - vi.spyOn(MongooseSeedwork, 'ObjectId').mockImplementationOnce(() => { - throw new Error('Invalid ObjectId'); + Scenario( + 'Getting conversations by listing ID with invalid ObjectId', + ({ Given, When, Then }) => { + Given('an invalid ObjectId will cause an error', () => { + // Mock MongooseSeedwork.ObjectId constructor to throw + vi.spyOn(MongooseSeedwork, 'ObjectId').mockImplementationOnce(() => { + throw new Error('Invalid ObjectId'); + }); }); - }); - When('I call getByListingId with invalid ID', async () => { - result = await repository.getByListingId('invalid-id'); - // Restore all mocks after test to prevent interference with subsequent tests - vi.restoreAllMocks(); - }); + When('I call getByListingId with invalid ID', async () => { + result = await repository.getByListingId('invalid-id'); + // Restore all mocks after test to prevent interference with subsequent tests + vi.restoreAllMocks(); + }); - Then('I should receive an empty array for listing', () => { - expect(Array.isArray(result)).toBe(true); - expect((result as unknown[]).length).toBe(0); - }); - }); + Then('I should receive an empty array for listing', () => { + expect(Array.isArray(result)).toBe(true); + expect((result as unknown[]).length).toBe(0); + }); + }, + ); }); From 4929f084095870fb7e0c33d62a94d718e3deb749 Mon Sep 17 00:00:00 2001 From: Kerry Lin Date: Tue, 13 Jan 2026 21:35:46 -0500 Subject: [PATCH 14/14] fix: optimize image deletion process by implementing chunked processing to avoid throttling --- .../listing/item/process-expired-deletions.ts | 46 ++++++++++--------- 1 file changed, 25 insertions(+), 21 deletions(-) diff --git a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts index 5edb7559f..079120c9b 100644 --- a/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts +++ b/packages/sthrift/application-services/src/contexts/listing/item/process-expired-deletions.ts @@ -18,28 +18,32 @@ async function deleteListingImages( images: string[], containerName: string, ): Promise { + const CHUNK_SIZE = 5; // Process 5 images at a time to avoid throttling let deletedCount = 0; - // Process all images concurrently (unbounded parallelism) for maximum throughput - const imagePromises = images.map((imagePath) => - blobStorage - .deleteBlob(containerName, imagePath) - .then(() => ({ success: true as const, imagePath })) - .catch((error) => ({ - success: false as const, - imagePath, - error: error instanceof Error ? error.message : String(error), - })), - ); - - const results = await Promise.allSettled(imagePromises); - for (const result of results) { - if (result.status === 'fulfilled') { - if (result.value.success) { - deletedCount++; - } else { - console.warn( - `[ExpiredDeletion] Failed to delete image ${result.value.imagePath}: ${result.value.error}`, - ); + + for (let i = 0; i < images.length; i += CHUNK_SIZE) { + const chunk = images.slice(i, i + CHUNK_SIZE); + const chunkPromises = chunk.map((imagePath) => + blobStorage + .deleteBlob(containerName, imagePath) + .then(() => ({ success: true as const, imagePath })) + .catch((error) => ({ + success: false as const, + imagePath, + error: error instanceof Error ? error.message : String(error), + })), + ); + + const results = await Promise.allSettled(chunkPromises); + for (const result of results) { + if (result.status === 'fulfilled') { + if (result.value.success) { + deletedCount++; + } else { + console.warn( + `[ExpiredDeletion] Failed to delete image ${result.value.imagePath}: ${result.value.error}`, + ); + } } } }