From 5088253d00c54125f4e8478ace9686a4ca8c1b3a Mon Sep 17 00:00:00 2001 From: chodak166 Date: Sat, 13 Sep 2025 16:01:59 +0200 Subject: [PATCH] NestJS review and cleanup --- nestjs/PLAN.md | 35 +-- nestjs/REVIEW.md | 243 ++++++++++++++++++ nestjs/package-lock.json | 42 +++ nestjs/package.json | 1 + nestjs/src/app.module.ts | 24 +- nestjs/src/app.service.ts | 8 - .../__tests__/add-item.command.spec.ts | 17 +- .../application/commands/add-item.command.ts | 19 +- .../commands/delete-item.command.ts | 17 +- .../commands/handle-expired-items.command.ts | 22 +- .../commands/login-user.command.ts | 15 +- .../interfaces/logger.interface.ts | 6 + .../src/application/queries/get-item.query.ts | 17 +- .../application/queries/list-items.query.ts | 13 +- .../application/services/logger.service.ts | 10 + .../infrastructure/auth/jwt-auth.service.ts | 16 +- .../infrastructure/http/order-http.service.ts | 21 +- .../logging/nest-logger.service.ts | 23 ++ .../logging/null-logger.service.ts | 21 ++ .../__tests__/file-item-repository.spec.ts | 15 +- .../__tests__/file-user-repository.spec.ts | 15 +- .../repositories/file-item-repository.ts | 23 +- .../repositories/file-user-repository.ts | 19 +- .../expired-items-scheduler.service.ts | 48 ++++ .../services/user-initialization.service.ts | 10 +- 25 files changed, 570 insertions(+), 130 deletions(-) create mode 100644 nestjs/REVIEW.md delete mode 100644 nestjs/src/app.service.ts create mode 100644 nestjs/src/application/interfaces/logger.interface.ts create mode 100644 nestjs/src/application/services/logger.service.ts create mode 100644 nestjs/src/infrastructure/logging/nest-logger.service.ts create mode 100644 nestjs/src/infrastructure/logging/null-logger.service.ts create mode 100644 nestjs/src/infrastructure/services/expired-items-scheduler.service.ts diff --git a/nestjs/PLAN.md b/nestjs/PLAN.md index 2ab462d..634ebc7 100644 --- a/nestjs/PLAN.md +++ b/nestjs/PLAN.md @@ -66,7 +66,7 @@ Implementation of AutoStore system using NestJS with TypeScript, following Clean **Purpose**: Immutable expiration date with validation **Key Methods**: -- `constructor(value: Date): void` - Validates date is in future +- `constructor(value: Date): void` - Validates date format (allows past dates per business rules) - `getValue(): Date` - Returns Date object - `format(): string` - Returns ISO string format @@ -104,8 +104,7 @@ Implementation of AutoStore system using NestJS with TypeScript, following Clean 3. Calls `ItemExpirationSpec.isExpired()` to check if item is expired 4. If expired: - calls `OrderHttpService.orderItem()` - - **DO NOT save to repository** - - returns null + - **returns item ID** (business rule: expired items trigger ordering but still return ID field that might be empty or invalid) 5. If not expired: calls `ItemRepository.save()` and returns item ID **File**: `src/application/commands/handle-expired-items.command.ts` @@ -200,8 +199,8 @@ Implementation of AutoStore system using NestJS with TypeScript, following Clean #### 7. Repositories -**File**: `src/infrastructure/persistence/repositories/item-repository.impl.ts` -**Purpose**: TypeORM implementation of item repository +**File**: `src/infrastructure/repositories/file-item-repository.ts` +**Purpose**: File-based implementation of item repository using JSON files **Key Methods**: - `save(item: ItemEntity): Promise` - Persists item entity @@ -284,20 +283,26 @@ Implementation of AutoStore system using NestJS with TypeScript, following Clean ## Background Processing -**File**: `src/infrastructure/scheduler/expired-items.scheduler.ts` -**Purpose**: Scheduled job for processing expired items +**File**: `src/infrastructure/services/expired-items-scheduler.service.ts` +**Purpose**: Scheduled job for processing expired items using NestJS scheduler **Key Methods**: -- `constructor(handleExpiredItemsCmd: HandleExpiredItemsCommand, logger: Logger): void` - Dependency injection -- `handleCron(): Promise` - Runs every minute to check for expired items +- `constructor(handleExpiredItemsCmd: HandleExpiredItemsCommand): void` - Dependency injection +- `onModuleInit(): Promise` - Processes expired items on application startup +- `handleExpiredItemsCron(): Promise` - Runs every minute (@Cron(CronExpression.EVERY_MINUTE)) +- `handleExpiredItemsDaily(): Promise` - Runs every day at midnight (@Cron('0 0 * * *')) **Flow**: -1. Upon application startup: Immediately invoke `HandleExpiredItemsCommand.execute()` -2. Start cron scheduler (every minute) -3. NestJS Scheduler triggers `handleCron()` every minute -4. Calls `HandleExpiredItemsCommand.execute()` to process expired items -5. Logs processing results and errors - +1. **On startup**: `onModuleInit()` immediately calls `HandleExpiredItemsCommand.execute()` +2. **Every minute**: `handleExpiredItemsCron()` processes expired items +3. **Every midnight**: `handleExpiredItemsDaily()` processes expired items +4. All methods use try-catch to continue operation despite errors +5. Comprehensive logging for monitoring and debugging + +**Configuration**: +- Requires `ScheduleModule.forRoot()` in AppModule imports +- Uses `@nestjs/schedule` package for cron expressions +- Implements `OnModuleInit` for startup processing ## Complete Flow Summary ### Item Creation Flow diff --git a/nestjs/REVIEW.md b/nestjs/REVIEW.md new file mode 100644 index 0000000..0035945 --- /dev/null +++ b/nestjs/REVIEW.md @@ -0,0 +1,243 @@ +# NestJS Implementation Review + +## Overview + +This review analyzes the TypeScript + NestJS implementation of the AutoStore application, focusing on adherence to Clean Architecture principles, SOLID principles, and the critical requirement of maintaining a **single source of truth** for domain knowledge. + +## Architecture Assessment + +### ✅ Strengths + +#### 1. Clean Architecture Implementation +The implementation successfully follows Clean Architecture with clear layer separation: + +- **Domain Layer**: Pure business logic with entities, value objects, and specifications +- **Application Layer**: Use cases, commands, queries, and infrastructure interfaces +- **Infrastructure Layer**: Concrete implementations (repositories, HTTP services, auth) +- **Presentation Layer**: Controllers and API endpoints + +#### 2. Specification Pattern Implementation +**Excellent implementation** of the Specification pattern for maintaining single source of truth: + +```typescript +// Domain specification - SINGLE SOURCE OF TRUTH +export class ItemExpirationSpec { + isExpired(item: ItemEntity, currentTime: Date): boolean { + return this.getSpec(currentTime).match(item); + } + + getSpec(currentTime: Date): SimpleSpecification { + return new SimpleSpecification( + Spec.lte('expirationDate', currentTime.toISOString()) + ); + } +} +``` + +This ensures that the expiration logic (`date <= now`) is defined **only once** in the domain layer and reused throughout the application. + +#### 3. Value Objects and Entities +Proper implementation of Domain-Driven Design patterns: +- **Value Objects**: [`ItemId`](src/domain/value-objects/item-id.vo.ts:1), [`UserId`](src/domain/value-objects/user-id.vo.ts:1), [`ExpirationDate`](src/domain/value-objects/expiration-date.vo.ts:1) +- **Entities**: [`ItemEntity`](src/domain/entities/item.entity.ts:1) with proper encapsulation +- **Immutability**: Value objects are immutable with defensive copying + +#### 4. Dependency Inversion +Excellent use of dependency injection and interface segregation: + +```typescript +// Application layer depends on abstractions +export interface IItemRepository { + findWhere(specification: ISpecification): Promise; + // ... other methods +} +``` + +#### 5. Repository Pattern with Specifications +The [`FileItemRepository`](src/infrastructure/repositories/file-item-repository.ts:114) properly implements the specification pattern: + +```typescript +async findWhere(specification: ISpecification): Promise { + // Uses domain specifications for filtering + if (specification.isSatisfiedBy(item)) { + matchingItems.push(item); + } +} +``` + +#### 6. Background Processing +The implementation now includes background processing using NestJS's built-in scheduler: + +**New Implementation**: [`ExpiredItemsSchedulerService`](src/infrastructure/services/expired-items-scheduler.service.ts:1) +- **On startup**: Immediately processes expired items via `onModuleInit()` +- **Every minute**: Runs via `@Cron(CronExpression.EVERY_MINUTE)` +- **Daily at midnight**: Runs via `@Cron('0 0 * * *')` for daily processing +- **Robust error handling**: Continues operation despite individual processing failures +- **Comprehensive logging**: Tracks all scheduling activities + +**Flow Integration**: +``` +AppModule → ScheduleModule.forRoot() → ExpiredItemsSchedulerService + ↓ +onModuleInit() → HandleExpiredItemsCommand.execute() [startup] + ↓ +@Cron(EVERY_MINUTE) → HandleExpiredItemsCommand.execute() [continuous] + ↓ +@Cron('0 0 * * *') → HandleExpiredItemsCommand.execute() [daily midnight] +``` + +### 🔍 Areas for Improvement + +#### 1. Framework Dependency in Application Layer +The implementation intentionally violates the Clean Architecture principle of framework-independent application layer by using NestJS decorators (`@Injectable()`, `@Inject()`) in the application layer. This decision was made for several practical reasons: + +- **Cleaner Construction**: NestJS's dependency injection system provides a clean and declarative way to manage dependencies, making the construction part of the application more maintainable and readable. +- **Ecosystem Integration**: Leveraging NestJS's native DI system allows for better integration with the framework's features, including interceptors, guards, and lifecycle hooks. +- **Community Standards**: This approach follows common practices in the NestJS community, making the code more familiar to developers experienced with the framework. +- **Testing Support**: NestJS provides excellent testing utilities that work seamlessly with decorator-based dependency injection. + +While this does create a framework dependency in the application layer, the trade-off is considered worthwhile for the benefits it provides in terms of development speed, maintainability, and framework integration. Alternative approaches like the Adapter Pattern or Factory Pattern could be used to make the application layer truly framework-agnostic, but they would introduce additional complexity and boilerplate code. + +#### 2. Specification Pattern Consistency + +While the implementation is excellent, there's a minor inconsistency in the [`ItemExpirationSpec`](src/domain/specifications/item-expiration.spec.ts:4) class name. The file is named `item-expiration.spec.ts` but the class is `ItemExpirationSpec`. Consider renaming to `ItemExpirationSpecification` for consistency. + +#### 3. Error Handling +The application could benefit from custom domain exceptions instead of generic `Error` objects: + +```typescript +// Current approach +throw new Error('Item name cannot be empty'); + +// Suggested improvement +throw new InvalidItemNameException('Item name cannot be empty'); +``` + +#### 4. Domain Events +Consider implementing domain events for the ordering process to better separate concerns: + +```typescript +// Instead of direct ordering in command +await this.orderService.orderItem(item); + +// Consider domain events +domainEvents.publish(new ItemExpiredEvent(item)); +``` + + + + +### 🎯 Comparison with PHP and C++ Implementations + +#### PHP Implementation +The PHP implementation follows a similar specification pattern but with some differences: + +```php +// PHP specification +public function isExpired(Item $item, DateTimeImmutable $currentTime): bool +{ + return $this->getSpec($currentTime)->match($item); +} + +public function getSpec(DateTimeImmutable $currentTime): Specification +{ + return new Specification( + Spec::lte('expirationDate', $currentTime->format('Y-m-d H:i:s')) + ); +} +``` + +**Key Differences:** +- PHP uses `DateTimeImmutable` vs TypeScript's `Date` +- PHP specification includes SQL rendering capabilities in comments +- Both maintain single source of truth effectively + +#### C++ Implementation +The C++ implementation uses a different approach with a policy pattern: + +```cpp +// C++ policy approach +bool isExpired(const Item& item, const TimePoint& currentTime) const +{ + return item.expirationDate <= currentTime; +} + +ItemExpirationSpec getExpiredSpecification(const TimePoint& currentTime) const +{ + return nxl::helpers::SpecificationBuilder() + .field(FIELD_EXP_DATE) + .lessOrEqual(currentTime) + .build(); +} +``` + +**Key Differences:** +- C++ uses `std::chrono::system_clock::time_point` +- C++ uses a builder pattern for specifications +- Direct comparison in `isExpired` vs specification-based approach + +### 🏆 Best Practices Demonstrated + +#### 1. Single Source of Truth +**Excellent adherence** to the requirement that expiration checking logic exists in only one place: + +- ✅ Domain specification defines `expirationDate <= currentTime` logic +- ✅ Application layer uses [`ItemExpirationSpec`](src/application/commands/handle-expired-items.command.ts:18) for business logic +- ✅ Repository layer uses specifications for filtering without duplicating logic +- ✅ No hardcoded expiration logic in controllers or infrastructure + +#### 2. SOLID Principles + +**Single Responsibility Principle**: Each class has one reason to change +- [`ItemEntity`](src/domain/entities/item.entity.ts:5): Manages item state and validation +- [`ItemExpirationSpec`](src/domain/specifications/item-expiration.spec.ts:4): Manages expiration logic +- [`FileItemRepository`](src/infrastructure/repositories/file-item-repository.ts:12): Manages persistence + +**Open/Closed Principle**: Extension through specifications, not modification +- New filtering criteria can be added via new specifications +- Repository doesn't need modification for new query types + +**Liskov Substitution Principle**: Interfaces are properly segregated +- [`IItemRepository`](src/application/interfaces/item-repository.interface.ts:6) can be implemented by any storage mechanism + +**Interface Segregation Principle**: Focused interfaces +- [`ITimeProvider`](src/application/interfaces/time-provider.interface.ts:1): Single method interface +- [`IOrderService`](src/application/interfaces/order-service.interface.ts:1): Focused on ordering + +**Dependency Inversion Principle**: Dependencies on abstractions +- Application layer depends on interfaces, not concrete implementations + +#### 3. Clean Architecture Boundaries + +**Domain Layer**: No external dependencies +- Pure TypeScript with no framework imports +- Business logic isolated from infrastructure concerns + +**Application Layer**: Orchestrates use cases +- Depends only on domain layer and infrastructure interfaces +- Commands and queries properly separated + +**Infrastructure Layer**: Implements abstractions +- [`FileItemRepository`](src/infrastructure/repositories/file-item-repository.ts:12) implements [`IItemRepository`](src/application/interfaces/item-repository.interface.ts:6) +- [`SystemTimeProvider`](src/infrastructure/services/system-time.provider.ts:5) implements [`ITimeProvider`](src/application/interfaces/time-provider.interface.ts:1) + +### 🧪 Testing Quality + +The implementation includes comprehensive tests: +- **Unit tests** for specifications with edge cases +- **Integration tests** for repositories +- **Boundary testing** for date/time scenarios +- **Specification testing** to ensure single source of truth + +### 🚀 Recommendations + +1. **Maintain the Specification Pattern**: This is the strongest aspect of the implementation +2. **Consider Domain Events**: For better separation of ordering concerns +3. **Add Custom Exceptions**: For better error handling and domain expressiveness +4. **Document Business Rules**: Add comments explaining why expired items are allowed (business requirement) + +## Conclusion + +The NestJS implementation **excellently demonstrates** Clean Architecture principles and successfully maintains a **single source of truth** for domain knowledge. The specification pattern implementation is particularly strong, ensuring that expiration date checking logic (`date <= now`) exists in exactly one place in the codebase. + +The architecture properly separates concerns, follows SOLID principles, and provides a solid foundation for the AutoStore application. The comparison with PHP and C++ implementations shows that while the technical details differ, all implementations successfully maintain the critical single source of truth requirement. \ No newline at end of file diff --git a/nestjs/package-lock.json b/nestjs/package-lock.json index e9b4b02..1754897 100644 --- a/nestjs/package-lock.json +++ b/nestjs/package-lock.json @@ -16,6 +16,7 @@ "@nestjs/jwt": "^11.0.0", "@nestjs/passport": "^11.0.5", "@nestjs/platform-express": "^10.0.0", + "@nestjs/schedule": "^6.0.0", "@types/bcrypt": "^6.0.0", "axios": "^1.12.1", "bcrypt": "^6.0.0", @@ -2570,6 +2571,19 @@ "@nestjs/core": "^10.0.0" } }, + "node_modules/@nestjs/schedule": { + "version": "6.0.0", + "resolved": "https://registry.npmjs.org/@nestjs/schedule/-/schedule-6.0.0.tgz", + "integrity": "sha512-aQySMw6tw2nhitELXd3EiRacQRgzUKD9mFcUZVOJ7jPLqIBvXOyvRWLsK9SdurGA+jjziAlMef7iB5ZEFFoQpw==", + "license": "MIT", + "dependencies": { + "cron": "4.3.0" + }, + "peerDependencies": { + "@nestjs/common": "^10.0.0 || ^11.0.0", + "@nestjs/core": "^10.0.0 || ^11.0.0" + } + }, "node_modules/@nestjs/schematics": { "version": "10.2.3", "resolved": "https://registry.npmjs.org/@nestjs/schematics/-/schematics-10.2.3.tgz", @@ -3011,6 +3025,12 @@ "@types/node": "*" } }, + "node_modules/@types/luxon": { + "version": "3.6.2", + "resolved": "https://registry.npmjs.org/@types/luxon/-/luxon-3.6.2.tgz", + "integrity": "sha512-R/BdP7OxEMc44l2Ex5lSXHoIXTB2JLNa3y2QISIbr58U/YcsffyQrYW//hZSdrfxrjRZj3GcUoxMPGdO8gSYuw==", + "license": "MIT" + }, "node_modules/@types/methods": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@types/methods/-/methods-1.1.4.tgz", @@ -4665,6 +4685,19 @@ "dev": true, "license": "MIT" }, + "node_modules/cron": { + "version": "4.3.0", + "resolved": "https://registry.npmjs.org/cron/-/cron-4.3.0.tgz", + "integrity": "sha512-ciiYNLfSlF9MrDqnbMdRWFiA6oizSF7kA1osPP9lRzNu0Uu+AWog1UKy7SkckiDY2irrNjeO6qLyKnXC8oxmrw==", + "license": "MIT", + "dependencies": { + "@types/luxon": "~3.6.0", + "luxon": "~3.6.0" + }, + "engines": { + "node": ">=18.x" + } + }, "node_modules/cross-spawn": { "version": "7.0.6", "resolved": "https://registry.npmjs.org/cross-spawn/-/cross-spawn-7.0.6.tgz", @@ -7633,6 +7666,15 @@ "yallist": "^3.0.2" } }, + "node_modules/luxon": { + "version": "3.6.1", + "resolved": "https://registry.npmjs.org/luxon/-/luxon-3.6.1.tgz", + "integrity": "sha512-tJLxrKJhO2ukZ5z0gyjY1zPh3Rh88Ej9P7jNrZiHMUXHae1yvI2imgOZtL1TO8TW6biMMKfTtAOoEJANgtWBMQ==", + "license": "MIT", + "engines": { + "node": ">=12" + } + }, "node_modules/magic-string": { "version": "0.30.8", "resolved": "https://registry.npmjs.org/magic-string/-/magic-string-0.30.8.tgz", diff --git a/nestjs/package.json b/nestjs/package.json index 3c199c1..da0f709 100644 --- a/nestjs/package.json +++ b/nestjs/package.json @@ -27,6 +27,7 @@ "@nestjs/jwt": "^11.0.0", "@nestjs/passport": "^11.0.5", "@nestjs/platform-express": "^10.0.0", + "@nestjs/schedule": "^6.0.0", "@types/bcrypt": "^6.0.0", "axios": "^1.12.1", "bcrypt": "^6.0.0", diff --git a/nestjs/src/app.module.ts b/nestjs/src/app.module.ts index 337bda5..d29a925 100644 --- a/nestjs/src/app.module.ts +++ b/nestjs/src/app.module.ts @@ -4,7 +4,7 @@ import { ValidationPipe } from '@nestjs/common'; import { ConfigModule, ConfigService } from '@nestjs/config'; import { JwtModule } from '@nestjs/jwt'; import { HttpModule } from '@nestjs/axios'; -import { AppService } from './app.service'; +import { ScheduleModule } from '@nestjs/schedule'; import { AuthController } from './presentation/controllers/auth.controller'; import { ItemsController } from './presentation/controllers/items.controller'; import { LoginUserCommand } from './application/commands/login-user.command'; @@ -20,11 +20,10 @@ import { OrderHttpService } from './infrastructure/http/order-http.service'; import { SystemTimeProvider } from './infrastructure/services/system-time.provider'; import { UserInitializationService } from './infrastructure/services/user-initialization.service'; import { ItemExpirationSpec } from './domain/specifications/item-expiration.spec'; -import { IAuthService } from './application/interfaces/auth-service.interface'; -import { IUserRepository } from './application/interfaces/user-repository.interface'; -import { IItemRepository } from './application/interfaces/item-repository.interface'; -import { IOrderService } from './application/interfaces/order-service.interface'; -import { ITimeProvider } from './application/interfaces/time-provider.interface'; +import { ExpiredItemsSchedulerService } from './infrastructure/services/expired-items-scheduler.service'; +import { LoggerService } from './application/services/logger.service'; +import { NestLoggerService } from './infrastructure/logging/nest-logger.service'; +import { NullLoggerService } from './infrastructure/logging/null-logger.service'; @Module({ imports: [ @@ -32,6 +31,7 @@ import { ITimeProvider } from './application/interfaces/time-provider.interface' isGlobal: true, }), HttpModule, + ScheduleModule.forRoot(), JwtModule.registerAsync({ imports: [ConfigModule], useFactory: async (configService: ConfigService) => ({ @@ -45,7 +45,6 @@ import { ITimeProvider } from './application/interfaces/time-provider.interface' ], controllers: [AuthController, ItemsController], providers: [ - AppService, LoginUserCommand, AddItemCommand, DeleteItemCommand, @@ -57,17 +56,20 @@ import { ITimeProvider } from './application/interfaces/time-provider.interface' SystemTimeProvider, ItemExpirationSpec, UserInitializationService, + ExpiredItemsSchedulerService, { provide: 'IAuthService', useExisting: JwtAuthService, }, { provide: 'IUserRepository', - useFactory: () => new FileUserRepository('./data'), + useFactory: (logger: LoggerService) => new FileUserRepository('./data', logger), + inject: [LoggerService], }, { provide: 'IItemRepository', - useFactory: () => new FileItemRepository('./data'), + useFactory: (logger: LoggerService) => new FileItemRepository('./data', logger), + inject: [LoggerService], }, { provide: 'IOrderService', @@ -77,6 +79,10 @@ import { ITimeProvider } from './application/interfaces/time-provider.interface' provide: 'ITimeProvider', useExisting: SystemTimeProvider, }, + { + provide: LoggerService, + useClass: process.env.NODE_ENV === 'test' ? NullLoggerService : NestLoggerService, + }, { provide: APP_PIPE, useValue: new ValidationPipe({ diff --git a/nestjs/src/app.service.ts b/nestjs/src/app.service.ts deleted file mode 100644 index 1cdab92..0000000 --- a/nestjs/src/app.service.ts +++ /dev/null @@ -1,8 +0,0 @@ -import { Injectable } from '@nestjs/common'; - -@Injectable() -export class AppService { - getHello(): string { - return 'Hello World!'; - } -} \ No newline at end of file diff --git a/nestjs/src/application/commands/__tests__/add-item.command.spec.ts b/nestjs/src/application/commands/__tests__/add-item.command.spec.ts index 0725aa2..29a49a9 100644 --- a/nestjs/src/application/commands/__tests__/add-item.command.spec.ts +++ b/nestjs/src/application/commands/__tests__/add-item.command.spec.ts @@ -18,6 +18,13 @@ const mockExpirationSpec = { isExpired: jest.fn(), }; +const mockLogger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), +}; + describe('AddItemCommand', () => { let addItemCommand: AddItemCommand; @@ -35,6 +42,7 @@ describe('AddItemCommand', () => { mockItemRepository as any, mockOrderService as any, mockTimeProvider as any, + mockLogger as any, mockExpirationSpec as any, ); @@ -97,10 +105,11 @@ describe('AddItemCommand', () => { expect(mockItemRepository.save).not.toHaveBeenCalled(); }); - it('should return null', async () => { + it('should return item ID', async () => { const result = await addItemCommand.execute(ITEM_NAME, EXPIRED_DATE, ORDER_URL, USER_ID); - expect(result).toBeNull(); + expect(result).toBeTruthy(); + expect(typeof result).toBe('string'); }); it('should handle order service failure gracefully', async () => { @@ -108,7 +117,9 @@ describe('AddItemCommand', () => { const result = await addItemCommand.execute(ITEM_NAME, EXPIRED_DATE, ORDER_URL, USER_ID); - expect(result).toBeNull(); + expect(result).toBeTruthy(); + expect(typeof result).toBe('string'); + expect(result).toMatch(/^[0-9a-f]{8}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{4}-[0-9a-f]{12}$/i); expect(mockOrderService.orderItem).toHaveBeenCalledTimes(1); expect(mockItemRepository.save).not.toHaveBeenCalled(); }); diff --git a/nestjs/src/application/commands/add-item.command.ts b/nestjs/src/application/commands/add-item.command.ts index 4bb5f85..8b046d9 100644 --- a/nestjs/src/application/commands/add-item.command.ts +++ b/nestjs/src/application/commands/add-item.command.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger, Inject } from '@nestjs/common'; +import { Injectable, Inject } from '@nestjs/common'; import { ItemEntity } from '../../domain/entities/item.entity'; import { ItemId } from '../../domain/value-objects/item-id.vo'; import { ExpirationDate } from '../../domain/value-objects/expiration-date.vo'; @@ -7,11 +7,10 @@ import { IItemRepository } from '../interfaces/item-repository.interface'; import { IOrderService } from '../interfaces/order-service.interface'; import { ITimeProvider } from '../interfaces/time-provider.interface'; import { ItemExpirationSpec } from '../../domain/specifications/item-expiration.spec'; +import { LoggerService } from '../services/logger.service'; @Injectable() export class AddItemCommand { - private readonly logger = new Logger(AddItemCommand.name); - constructor( @Inject('IItemRepository') private readonly itemRepository: IItemRepository, @@ -19,6 +18,8 @@ export class AddItemCommand { private readonly orderService: IOrderService, @Inject('ITimeProvider') private readonly timeProvider: ITimeProvider, + @Inject(LoggerService) + private readonly logger: LoggerService, private readonly expirationSpec: ItemExpirationSpec, ) {} @@ -29,7 +30,7 @@ export class AddItemCommand { userId: string, ): Promise { try { - this.logger.log(`Adding item: ${name} for user: ${userId}`); + this.logger.log(`Adding item: ${name} for user: ${userId}`, AddItemCommand.name); // Validate input parameters this.validateInput(name, expirationDate, orderUrl, userId); @@ -58,15 +59,15 @@ export class AddItemCommand { // Check if item is expired if (this.expirationSpec.isExpired(item, currentTime)) { - this.logger.log(`Item ${name} is expired, ordering replacement`); + this.logger.log(`Item ${name} is expired, ordering replacement`, AddItemCommand.name); try { await this.orderService.orderItem(item); - this.logger.log(`Successfully ordered replacement for expired item: ${name}`); + this.logger.log(`Successfully ordered replacement for expired item: ${name}`, AddItemCommand.name); // Return the item ID even for expired items to match API contract return itemId.getValue(); } catch (error) { - this.logger.error(`Failed to place order for expired item ${itemId.getValue()}: ${error.message}`); + this.logger.error(`Failed to place order for expired item ${itemId.getValue()}: ${error.message}`, undefined, AddItemCommand.name); // Still return the ID even if ordering fails return itemId.getValue(); } @@ -74,11 +75,11 @@ export class AddItemCommand { // Save item if not expired await this.itemRepository.save(item); - this.logger.log(`Successfully saved item: ${name} with ID: ${itemId.getValue()}`); + this.logger.log(`Successfully saved item: ${name} with ID: ${itemId.getValue()}`, AddItemCommand.name); return itemId.getValue(); } catch (error) { - this.logger.error(`Failed to add item: ${error.message}`); + this.logger.error(`Failed to add item: ${error.message}`, undefined, AddItemCommand.name); throw error; } } diff --git a/nestjs/src/application/commands/delete-item.command.ts b/nestjs/src/application/commands/delete-item.command.ts index fd9922c..a0fbe16 100644 --- a/nestjs/src/application/commands/delete-item.command.ts +++ b/nestjs/src/application/commands/delete-item.command.ts @@ -1,20 +1,21 @@ -import { Injectable, Logger, NotFoundException, UnauthorizedException, Inject } from '@nestjs/common'; +import { Injectable, NotFoundException, UnauthorizedException, Inject } from '@nestjs/common'; import { ItemId } from '../../domain/value-objects/item-id.vo'; import { UserId } from '../../domain/value-objects/user-id.vo'; import { IItemRepository } from '../interfaces/item-repository.interface'; +import { LoggerService } from '../services/logger.service'; @Injectable() export class DeleteItemCommand { - private readonly logger = new Logger(DeleteItemCommand.name); - constructor( @Inject('IItemRepository') private readonly itemRepository: IItemRepository, + @Inject(LoggerService) + private readonly logger: LoggerService, ) {} async execute(itemId: string, userId: string): Promise { try { - this.logger.log(`Deleting item: ${itemId} for user: ${userId}`); + this.logger.log(`Deleting item: ${itemId} for user: ${userId}`, DeleteItemCommand.name); const itemIdVo = ItemId.create(itemId); const userIdVo = UserId.create(userId); @@ -22,24 +23,24 @@ export class DeleteItemCommand { const item = await this.itemRepository.findById(itemIdVo); if (!item) { - this.logger.warn(`Item not found: ${itemId}`); + this.logger.warn(`Item not found: ${itemId}`, DeleteItemCommand.name); throw new NotFoundException(`Item with ID ${itemId} not found`); } // Validate ownership if (!item.getUserId().equals(userIdVo)) { - this.logger.warn(`User ${userId} attempted to delete item ${itemId} owned by ${item.getUserId().getValue()}`); + this.logger.warn(`User ${userId} attempted to delete item ${itemId} owned by ${item.getUserId().getValue()}`, DeleteItemCommand.name); throw new NotFoundException(`Item with ID ${itemId} not found`); } await this.itemRepository.delete(itemIdVo); - this.logger.log(`Successfully deleted item: ${itemId}`); + this.logger.log(`Successfully deleted item: ${itemId}`, DeleteItemCommand.name); } catch (error) { if (error instanceof NotFoundException || error instanceof UnauthorizedException) { throw error; } - this.logger.error(`Failed to delete item ${itemId}: ${error.message}`); + this.logger.error(`Failed to delete item ${itemId}: ${error.message}`, undefined, DeleteItemCommand.name); throw new Error(`Failed to delete item: ${error.message}`); } } diff --git a/nestjs/src/application/commands/handle-expired-items.command.ts b/nestjs/src/application/commands/handle-expired-items.command.ts index d28573e..3e2503f 100644 --- a/nestjs/src/application/commands/handle-expired-items.command.ts +++ b/nestjs/src/application/commands/handle-expired-items.command.ts @@ -1,14 +1,12 @@ -import { Injectable, Logger, Inject } from '@nestjs/common'; -import { ItemEntity } from '../../domain/entities/item.entity'; +import { Injectable, Inject } from '@nestjs/common'; import { IItemRepository } from '../interfaces/item-repository.interface'; import { IOrderService } from '../interfaces/order-service.interface'; import { ITimeProvider } from '../interfaces/time-provider.interface'; import { ItemExpirationSpec } from '../../domain/specifications/item-expiration.spec'; +import { LoggerService } from '../services/logger.service'; @Injectable() export class HandleExpiredItemsCommand { - private readonly logger = new Logger(HandleExpiredItemsCommand.name); - constructor( @Inject('IItemRepository') private readonly itemRepository: IItemRepository, @@ -16,37 +14,39 @@ export class HandleExpiredItemsCommand { private readonly orderService: IOrderService, @Inject('ITimeProvider') private readonly timeProvider: ITimeProvider, + @Inject(LoggerService) + private readonly logger: LoggerService, private readonly expirationSpec: ItemExpirationSpec, ) {} async execute(): Promise { try { - this.logger.log('Starting expired items processing'); + this.logger.log('Starting expired items processing', HandleExpiredItemsCommand.name); const currentTime = this.timeProvider.now(); const specification = this.expirationSpec.getSpec(currentTime); const expiredItems = await this.itemRepository.findWhere(specification); - this.logger.log(`Found ${expiredItems.length} expired items to process`); + this.logger.log(`Found ${expiredItems.length} expired items to process`, HandleExpiredItemsCommand.name); for (const item of expiredItems) { try { - this.logger.log(`Processing expired item: ${item.getId().getValue()}`); + this.logger.log(`Processing expired item: ${item.getId().getValue()}`, HandleExpiredItemsCommand.name); await this.orderService.orderItem(item); await this.itemRepository.delete(item.getId()); - this.logger.log(`Successfully processed and deleted expired item: ${item.getId().getValue()}`); + this.logger.log(`Successfully processed and deleted expired item: ${item.getId().getValue()}`, HandleExpiredItemsCommand.name); } catch (error) { - this.logger.error(`Failed to process expired item ${item.getId().getValue()}: ${error.message}`); + this.logger.error(`Failed to process expired item ${item.getId().getValue()}: ${error.message}`, undefined, HandleExpiredItemsCommand.name); // Continue processing other items even if one fails } } - this.logger.log('Completed expired items processing'); + this.logger.log('Completed expired items processing', HandleExpiredItemsCommand.name); } catch (error) { - this.logger.error(`Failed to handle expired items: ${error.message}`); + this.logger.error(`Failed to handle expired items: ${error.message}`, undefined, HandleExpiredItemsCommand.name); throw new Error(`Failed to handle expired items: ${error.message}`); } } diff --git a/nestjs/src/application/commands/login-user.command.ts b/nestjs/src/application/commands/login-user.command.ts index 28c51f5..26429bc 100644 --- a/nestjs/src/application/commands/login-user.command.ts +++ b/nestjs/src/application/commands/login-user.command.ts @@ -1,18 +1,19 @@ -import { Injectable, Logger, UnauthorizedException, Inject } from '@nestjs/common'; +import { Injectable, UnauthorizedException, Inject } from '@nestjs/common'; import { IAuthService } from '../interfaces/auth-service.interface'; +import { LoggerService } from '../services/logger.service'; @Injectable() export class LoginUserCommand { - private readonly logger = new Logger(LoginUserCommand.name); - constructor( @Inject('IAuthService') private readonly authService: IAuthService, + @Inject(LoggerService) + private readonly logger: LoggerService, ) {} async execute(username: string, password: string): Promise { try { - this.logger.log(`Login attempt for user: ${username}`); + this.logger.log(`Login attempt for user: ${username}`, LoginUserCommand.name); // Validate input parameters this.validateInput(username, password); @@ -20,18 +21,18 @@ export class LoginUserCommand { const token = await this.authService.authenticate(username, password); if (!token) { - this.logger.warn(`Authentication failed for user: ${username}`); + this.logger.warn(`Authentication failed for user: ${username}`, LoginUserCommand.name); throw new UnauthorizedException('Invalid username or password'); } - this.logger.log(`Successfully authenticated user: ${username}`); + this.logger.log(`Successfully authenticated user: ${username}`, LoginUserCommand.name); return token; } catch (error) { if (error instanceof UnauthorizedException) { throw error; } - this.logger.error(`Failed to login user ${username}: ${error.message}`); + this.logger.error(`Failed to login user ${username}: ${error.message}`, undefined, LoginUserCommand.name); throw new Error(`Failed to login: ${error.message}`); } } diff --git a/nestjs/src/application/interfaces/logger.interface.ts b/nestjs/src/application/interfaces/logger.interface.ts new file mode 100644 index 0000000..d1f8bf5 --- /dev/null +++ b/nestjs/src/application/interfaces/logger.interface.ts @@ -0,0 +1,6 @@ +export interface ILogger { + log(message: string, context?: string): void; + error(message: string, trace?: string, context?: string): void; + warn(message: string, context?: string): void; + debug(message: string, context?: string): void; +} \ No newline at end of file diff --git a/nestjs/src/application/queries/get-item.query.ts b/nestjs/src/application/queries/get-item.query.ts index 021e4a4..9c40ac0 100644 --- a/nestjs/src/application/queries/get-item.query.ts +++ b/nestjs/src/application/queries/get-item.query.ts @@ -1,21 +1,22 @@ -import { Injectable, Logger, NotFoundException, UnauthorizedException, Inject } from '@nestjs/common'; +import { Injectable, NotFoundException, UnauthorizedException, Inject } from '@nestjs/common'; import { ItemEntity } from '../../domain/entities/item.entity'; import { ItemId } from '../../domain/value-objects/item-id.vo'; import { UserId } from '../../domain/value-objects/user-id.vo'; import { IItemRepository } from '../interfaces/item-repository.interface'; +import { LoggerService } from '../services/logger.service'; @Injectable() export class GetItemQuery { - private readonly logger = new Logger(GetItemQuery.name); - constructor( @Inject('IItemRepository') private readonly itemRepository: IItemRepository, + @Inject(LoggerService) + private readonly logger: LoggerService, ) {} async execute(itemId: string, userId: string): Promise { try { - this.logger.log(`Getting item: ${itemId} for user: ${userId}`); + this.logger.log(`Getting item: ${itemId} for user: ${userId}`, GetItemQuery.name); const itemIdVo = ItemId.create(itemId); const userIdVo = UserId.create(userId); @@ -23,26 +24,26 @@ export class GetItemQuery { const item = await this.itemRepository.findById(itemIdVo); if (!item) { - this.logger.warn(`Item not found: ${itemId}`); + this.logger.warn(`Item not found: ${itemId}`, GetItemQuery.name); throw new NotFoundException(`Item with ID ${itemId} not found`); } // Validate ownership if (!item.getUserId().equals(userIdVo)) { - this.logger.warn(`User ${userId} attempted to access item ${itemId} owned by ${item.getUserId().getValue()}`); + this.logger.warn(`User ${userId} attempted to access item ${itemId} owned by ${item.getUserId().getValue()}`, GetItemQuery.name); // throw new UnauthorizedException('You do not have permission to access this item'); // Go with 404 for safety reasons - it is till not found for that user, but the existence is not compromised throw new NotFoundException(`Item with ID ${itemId} not found`); } - this.logger.log(`Successfully retrieved item: ${itemId}`); + this.logger.log(`Successfully retrieved item: ${itemId}`, GetItemQuery.name); return item; } catch (error) { if (error instanceof NotFoundException || error instanceof UnauthorizedException) { throw error; } - this.logger.error(`Failed to get item ${itemId}: ${error.message}`); + this.logger.error(`Failed to get item ${itemId}: ${error.message}`, undefined, GetItemQuery.name); throw new Error(`Failed to get item: ${error.message}`); } } diff --git a/nestjs/src/application/queries/list-items.query.ts b/nestjs/src/application/queries/list-items.query.ts index 895c5dd..c646723 100644 --- a/nestjs/src/application/queries/list-items.query.ts +++ b/nestjs/src/application/queries/list-items.query.ts @@ -1,28 +1,29 @@ -import { Injectable, Logger, Inject } from '@nestjs/common'; +import { Injectable, Inject } from '@nestjs/common'; import { ItemEntity } from '../../domain/entities/item.entity'; import { UserId } from '../../domain/value-objects/user-id.vo'; import { IItemRepository } from '../interfaces/item-repository.interface'; +import { LoggerService } from '../services/logger.service'; @Injectable() export class ListItemsQuery { - private readonly logger = new Logger(ListItemsQuery.name); - constructor( @Inject('IItemRepository') private readonly itemRepository: IItemRepository, + @Inject(LoggerService) + private readonly logger: LoggerService, ) {} async execute(userId: string): Promise { try { - this.logger.log(`Listing items for user: ${userId}`); + this.logger.log(`Listing items for user: ${userId}`, ListItemsQuery.name); const userIdVo = UserId.create(userId); const items = await this.itemRepository.findByUserId(userIdVo); - this.logger.log(`Successfully retrieved ${items.length} items for user: ${userId}`); + this.logger.log(`Successfully retrieved ${items.length} items for user: ${userId}`, ListItemsQuery.name); return items; } catch (error) { - this.logger.error(`Failed to list items for user ${userId}: ${error.message}`); + this.logger.error(`Failed to list items for user ${userId}: ${error.message}`, undefined, ListItemsQuery.name); throw new Error(`Failed to list items: ${error.message}`); } } diff --git a/nestjs/src/application/services/logger.service.ts b/nestjs/src/application/services/logger.service.ts new file mode 100644 index 0000000..72487cd --- /dev/null +++ b/nestjs/src/application/services/logger.service.ts @@ -0,0 +1,10 @@ +import { Injectable } from '@nestjs/common'; +import { ILogger } from '../interfaces/logger.interface'; + +@Injectable() +export abstract class LoggerService implements ILogger { + abstract log(message: string, context?: string): void; + abstract error(message: string, trace?: string, context?: string): void; + abstract warn(message: string, context?: string): void; + abstract debug(message: string, context?: string): void; +} \ No newline at end of file diff --git a/nestjs/src/infrastructure/auth/jwt-auth.service.ts b/nestjs/src/infrastructure/auth/jwt-auth.service.ts index 8378011..8a1b5e3 100644 --- a/nestjs/src/infrastructure/auth/jwt-auth.service.ts +++ b/nestjs/src/infrastructure/auth/jwt-auth.service.ts @@ -1,14 +1,13 @@ -import { Injectable, Logger, UnauthorizedException, Inject } from '@nestjs/common'; +import { Injectable, UnauthorizedException, Inject } from '@nestjs/common'; import { JwtService } from '@nestjs/jwt'; import { ConfigService } from '@nestjs/config'; import * as bcrypt from 'bcrypt'; import { IAuthService } from '../../application/interfaces/auth-service.interface'; import { IUserRepository } from '../../application/interfaces/user-repository.interface'; -import { UserEntity } from '../../domain/entities/user.entity'; +import { LoggerService } from '../../application/services/logger.service'; @Injectable() export class JwtAuthService implements IAuthService { - private readonly logger = new Logger(JwtAuthService.name); private readonly jwtSecret: string; private readonly jwtExpiration: string; @@ -17,6 +16,7 @@ export class JwtAuthService implements IAuthService { private readonly userRepository: IUserRepository, private readonly jwtService: JwtService, private readonly configService: ConfigService, + private readonly logger: LoggerService, ) { this.jwtSecret = this.configService.get('JWT_SECRET') || 'default-secret-key'; this.jwtExpiration = this.configService.get('JWT_EXPIRATION') || '1h'; @@ -27,14 +27,14 @@ export class JwtAuthService implements IAuthService { const user = await this.userRepository.findByUsername(username); if (!user) { - this.logger.warn(`User not found: ${username}`); + this.logger.warn(`User not found: ${username}`, JwtAuthService.name); return null; } const isPasswordValid = await this.verifyPassword(password, user.getPasswordHash()); if (!isPasswordValid) { - this.logger.warn(`Invalid password for user: ${username}`); + this.logger.warn(`Invalid password for user: ${username}`, JwtAuthService.name); return null; } @@ -48,7 +48,7 @@ export class JwtAuthService implements IAuthService { expiresIn: this.jwtExpiration, }); } catch (error) { - this.logger.error(`Authentication error for user ${username}: ${error.message}`); + this.logger.error(`Authentication error for user ${username}: ${error.message}`, undefined, JwtAuthService.name); return null; } } @@ -60,7 +60,7 @@ export class JwtAuthService implements IAuthService { }); return true; } catch (error) { - this.logger.warn(`Invalid token: ${error.message}`); + this.logger.warn(`Invalid token: ${error.message}`, JwtAuthService.name); return false; } } @@ -72,7 +72,7 @@ export class JwtAuthService implements IAuthService { }); return payload.sub || null; } catch (error) { - this.logger.warn(`Failed to extract user ID from token: ${error.message}`); + this.logger.warn(`Failed to extract user ID from token: ${error.message}`, JwtAuthService.name); return null; } } diff --git a/nestjs/src/infrastructure/http/order-http.service.ts b/nestjs/src/infrastructure/http/order-http.service.ts index 55eb6ba..55fb7bd 100644 --- a/nestjs/src/infrastructure/http/order-http.service.ts +++ b/nestjs/src/infrastructure/http/order-http.service.ts @@ -1,19 +1,21 @@ -import { Injectable, Logger, HttpStatus } from '@nestjs/common'; +import { Injectable, HttpStatus } from '@nestjs/common'; import { HttpService } from '@nestjs/axios'; import { firstValueFrom } from 'rxjs'; import { AxiosError } from 'axios'; import { ItemEntity } from '../../domain/entities/item.entity'; import { IOrderService } from '../../application/interfaces/order-service.interface'; +import { LoggerService } from '../../application/services/logger.service'; @Injectable() export class OrderHttpService implements IOrderService { - private readonly logger = new Logger(OrderHttpService.name); - - constructor(private readonly httpService: HttpService) {} + constructor( + private readonly httpService: HttpService, + private readonly logger: LoggerService, + ) {} async orderItem(item: ItemEntity): Promise { try { - this.logger.log(`Ordering item: ${item.getId().getValue()} at URL: ${item.getOrderUrl()}`); + this.logger.log(`Ordering item: ${item.getId().getValue()} at URL: ${item.getOrderUrl()}`, OrderHttpService.name); const orderData = { itemId: item.getId().getValue(), @@ -33,23 +35,24 @@ export class OrderHttpService implements IOrderService { }, }), ); - this.logger.log( - `Successfully ordered item ${item.getId().getValue()}. Status: ${response.status}`, + `Successfully ordered item ${item.getId().getValue()}. Status: ${(response as any).status}`, + OrderHttpService.name, ); } catch (error) { if (error instanceof AxiosError) { const status = error.response?.status || HttpStatus.INTERNAL_SERVER_ERROR; const message = error.response?.data?.message || error.message; - this.logger.error( `Failed to order item ${item.getId().getValue()}. Status: ${status}, Error: ${message}`, + undefined, + OrderHttpService.name ); throw new Error(`Order service returned status ${status}: ${message}`); } - this.logger.error(`Failed to order item ${item.getId().getValue()}: ${error.message}`); + this.logger.error(`Failed to order item ${item.getId().getValue()}: ${error.message}`, undefined, OrderHttpService.name); throw new Error(`Failed to order item: ${error.message}`); } } diff --git a/nestjs/src/infrastructure/logging/nest-logger.service.ts b/nestjs/src/infrastructure/logging/nest-logger.service.ts new file mode 100644 index 0000000..d581768 --- /dev/null +++ b/nestjs/src/infrastructure/logging/nest-logger.service.ts @@ -0,0 +1,23 @@ +import { Injectable, Logger } from '@nestjs/common'; +import { LoggerService } from '../../application/services/logger.service'; + +@Injectable() +export class NestLoggerService extends LoggerService { + private readonly logger = new Logger(NestLoggerService.name); + + log(message: string, context?: string): void { + this.logger.log(message, context); + } + + error(message: string, trace?: string, context?: string): void { + this.logger.error(message, trace, context); + } + + warn(message: string, context?: string): void { + this.logger.warn(message, context); + } + + debug(message: string, context?: string): void { + this.logger.debug(message, context); + } +} \ No newline at end of file diff --git a/nestjs/src/infrastructure/logging/null-logger.service.ts b/nestjs/src/infrastructure/logging/null-logger.service.ts new file mode 100644 index 0000000..23b709a --- /dev/null +++ b/nestjs/src/infrastructure/logging/null-logger.service.ts @@ -0,0 +1,21 @@ +import { Injectable } from '@nestjs/common'; +import { LoggerService } from '../../application/services/logger.service'; + +@Injectable() +export class NullLoggerService extends LoggerService { + log(message: string, context?: string): void { + // No-op for testing + } + + error(message: string, trace?: string, context?: string): void { + // No-op for testing + } + + warn(message: string, context?: string): void { + // No-op for testing + } + + debug(message: string, context?: string): void { + // No-op for testing + } +} \ No newline at end of file diff --git a/nestjs/src/infrastructure/repositories/__tests__/file-item-repository.spec.ts b/nestjs/src/infrastructure/repositories/__tests__/file-item-repository.spec.ts index b3f425d..15098d1 100644 --- a/nestjs/src/infrastructure/repositories/__tests__/file-item-repository.spec.ts +++ b/nestjs/src/infrastructure/repositories/__tests__/file-item-repository.spec.ts @@ -12,6 +12,7 @@ import { join } from 'path'; describe('FileItemRepository', () => { let repository: FileItemRepository; let testStoragePath: string; + let mockLogger: any; // Test constants - using valid UUIDs const ITEM_ID_1 = '00000000-0000-0000-0000-000000000001'; @@ -43,11 +44,19 @@ describe('FileItemRepository', () => { mkdirSync(testStoragePath, { recursive: true }); } + // Create mock logger + mockLogger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + }; + const module: TestingModule = await Test.createTestingModule({ providers: [ { provide: FileItemRepository, - useFactory: () => new FileItemRepository(testStoragePath), + useFactory: () => new FileItemRepository(testStoragePath, mockLogger), }, ], }).compile(); @@ -331,8 +340,8 @@ describe('FileItemRepository', () => { it('should share the same underlying file between different repository instances', async () => { // Given - const repository1 = new FileItemRepository(testStoragePath); - const repository2 = new FileItemRepository(testStoragePath); + const repository1 = new FileItemRepository(testStoragePath, mockLogger); + const repository2 = new FileItemRepository(testStoragePath, mockLogger); const item = createTestItem1(); // When diff --git a/nestjs/src/infrastructure/repositories/__tests__/file-user-repository.spec.ts b/nestjs/src/infrastructure/repositories/__tests__/file-user-repository.spec.ts index cc6b1be..673f0a5 100644 --- a/nestjs/src/infrastructure/repositories/__tests__/file-user-repository.spec.ts +++ b/nestjs/src/infrastructure/repositories/__tests__/file-user-repository.spec.ts @@ -8,6 +8,7 @@ import { join } from 'path'; describe('FileUserRepository', () => { let repository: FileUserRepository; let testStoragePath: string; + let mockLogger: any; // Test constants - using valid UUIDs const USER_ID_1 = '10000000-0000-0000-0000-000000000001'; @@ -26,11 +27,19 @@ describe('FileUserRepository', () => { mkdirSync(testStoragePath, { recursive: true }); } + // Create mock logger + mockLogger = { + log: jest.fn(), + error: jest.fn(), + warn: jest.fn(), + debug: jest.fn(), + }; + const module: TestingModule = await Test.createTestingModule({ providers: [ { provide: FileUserRepository, - useFactory: () => new FileUserRepository(testStoragePath), + useFactory: () => new FileUserRepository(testStoragePath, mockLogger), }, ], }).compile(); @@ -199,8 +208,8 @@ describe('FileUserRepository', () => { it('should share the same underlying file between different repository instances', async () => { // Given - const repository1 = new FileUserRepository(testStoragePath); - const repository2 = new FileUserRepository(testStoragePath); + const repository1 = new FileUserRepository(testStoragePath, mockLogger); + const repository2 = new FileUserRepository(testStoragePath, mockLogger); const user = createTestUser1(); // When diff --git a/nestjs/src/infrastructure/repositories/file-item-repository.ts b/nestjs/src/infrastructure/repositories/file-item-repository.ts index 2d07647..92ed3ce 100644 --- a/nestjs/src/infrastructure/repositories/file-item-repository.ts +++ b/nestjs/src/infrastructure/repositories/file-item-repository.ts @@ -1,4 +1,4 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { writeFileSync, readFileSync, existsSync, mkdirSync } from 'fs'; import { join } from 'path'; import { IItemRepository } from '../../application/interfaces/item-repository.interface'; @@ -7,13 +7,16 @@ import { ItemId } from '../../domain/value-objects/item-id.vo'; import { UserId } from '../../domain/value-objects/user-id.vo'; import { ExpirationDate } from '../../domain/value-objects/expiration-date.vo'; import { ISpecification } from '../../domain/specifications/specification.interface'; +import { LoggerService } from '../../application/services/logger.service'; @Injectable() export class FileItemRepository implements IItemRepository { - private readonly logger = new Logger(FileItemRepository.name); private readonly filePath: string; - constructor(storagePath: string = './data') { + constructor( + storagePath: string = './data', + private readonly logger: LoggerService, + ) { // Ensure the storage directory exists if (!existsSync(storagePath)) { mkdirSync(storagePath, { recursive: true }); @@ -31,7 +34,7 @@ export class FileItemRepository implements IItemRepository { const fileContent = readFileSync(this.filePath, 'utf-8'); return JSON.parse(fileContent); } catch (error) { - this.logger.error(`Error reading items file: ${error.message}`); + this.logger.error(`Error reading items file: ${error.message}`, undefined, FileItemRepository.name); return {}; } } @@ -40,7 +43,7 @@ export class FileItemRepository implements IItemRepository { try { writeFileSync(this.filePath, JSON.stringify(items, null, 2)); } catch (error) { - this.logger.error(`Error writing items file: ${error.message}`); + this.logger.error(`Error writing items file: ${error.message}`, undefined, FileItemRepository.name); throw new Error(`Failed to save items: ${error.message}`); } } @@ -73,7 +76,7 @@ export class FileItemRepository implements IItemRepository { items[itemId] = this.entityToPlain(item); this.writeItems(items); - this.logger.log(`Item ${itemId} saved successfully`); + this.logger.log(`Item ${itemId} saved successfully`, FileItemRepository.name); } async findById(id: ItemId): Promise { @@ -87,7 +90,7 @@ export class FileItemRepository implements IItemRepository { try { return this.plainToEntity(items[itemId]); } catch (error) { - this.logger.error(`Error parsing item ${itemId}: ${error.message}`); + this.logger.error(`Error parsing item ${itemId}: ${error.message}`, undefined, FileItemRepository.name); return null; } } @@ -103,7 +106,7 @@ export class FileItemRepository implements IItemRepository { userItems.push(this.plainToEntity(items[itemId])); } } catch (error) { - this.logger.error(`Error parsing item ${itemId}: ${error.message}`); + this.logger.error(`Error parsing item ${itemId}: ${error.message}`, undefined, FileItemRepository.name); // Skip invalid items } } @@ -123,7 +126,7 @@ export class FileItemRepository implements IItemRepository { matchingItems.push(item); } } catch (error) { - this.logger.error(`Error parsing item ${itemId}: ${error.message}`); + this.logger.error(`Error parsing item ${itemId}: ${error.message}`, undefined, FileItemRepository.name); // Skip invalid items } } @@ -142,7 +145,7 @@ export class FileItemRepository implements IItemRepository { delete items[itemId]; this.writeItems(items); - this.logger.log(`Item ${itemId} deleted successfully`); + this.logger.log(`Item ${itemId} deleted successfully`, FileItemRepository.name); } async exists(id: ItemId): Promise { diff --git a/nestjs/src/infrastructure/repositories/file-user-repository.ts b/nestjs/src/infrastructure/repositories/file-user-repository.ts index 5fc0b71..0e95119 100644 --- a/nestjs/src/infrastructure/repositories/file-user-repository.ts +++ b/nestjs/src/infrastructure/repositories/file-user-repository.ts @@ -1,16 +1,19 @@ -import { Injectable, Logger } from '@nestjs/common'; +import { Injectable } from '@nestjs/common'; import { writeFileSync, readFileSync, existsSync, mkdirSync } from 'fs'; import { join } from 'path'; import { IUserRepository } from '../../application/interfaces/user-repository.interface'; import { UserEntity } from '../../domain/entities/user.entity'; import { UserId } from '../../domain/value-objects/user-id.vo'; +import { LoggerService } from '../../application/services/logger.service'; @Injectable() export class FileUserRepository implements IUserRepository { - private readonly logger = new Logger(FileUserRepository.name); private readonly filePath: string; - constructor(storagePath: string = './data') { + constructor( + storagePath: string = './data', + private readonly logger: LoggerService, + ) { // Ensure the storage directory exists if (!existsSync(storagePath)) { mkdirSync(storagePath, { recursive: true }); @@ -28,7 +31,7 @@ export class FileUserRepository implements IUserRepository { const fileContent = readFileSync(this.filePath, 'utf-8'); return JSON.parse(fileContent); } catch (error) { - this.logger.error(`Error reading users file: ${error.message}`); + this.logger.error(`Error reading users file: ${error.message}`, undefined, FileUserRepository.name); return {}; } } @@ -37,7 +40,7 @@ export class FileUserRepository implements IUserRepository { try { writeFileSync(this.filePath, JSON.stringify(users, null, 2)); } catch (error) { - this.logger.error(`Error writing users file: ${error.message}`); + this.logger.error(`Error writing users file: ${error.message}`, undefined, FileUserRepository.name); throw new Error(`Failed to save users: ${error.message}`); } } @@ -74,7 +77,7 @@ export class FileUserRepository implements IUserRepository { users[userId] = this.entityToPlain(user); this.writeUsers(users); - this.logger.log(`User ${userId} saved successfully`); + this.logger.log(`User ${userId} saved successfully`, FileUserRepository.name); } async findById(id: UserId): Promise { @@ -88,7 +91,7 @@ export class FileUserRepository implements IUserRepository { try { return this.plainToEntity(users[userId]); } catch (error) { - this.logger.error(`Error parsing user ${userId}: ${error.message}`); + this.logger.error(`Error parsing user ${userId}: ${error.message}`, undefined, FileUserRepository.name); return null; } } @@ -102,7 +105,7 @@ export class FileUserRepository implements IUserRepository { return this.plainToEntity(users[userId]); } } catch (error) { - this.logger.error(`Error parsing user ${userId}: ${error.message}`); + this.logger.error(`Error parsing user ${userId}: ${error.message}`, undefined, FileUserRepository.name); // Skip invalid users } } diff --git a/nestjs/src/infrastructure/services/expired-items-scheduler.service.ts b/nestjs/src/infrastructure/services/expired-items-scheduler.service.ts new file mode 100644 index 0000000..011dbec --- /dev/null +++ b/nestjs/src/infrastructure/services/expired-items-scheduler.service.ts @@ -0,0 +1,48 @@ +import { Injectable, OnModuleInit } from '@nestjs/common'; +import { Cron, CronExpression } from '@nestjs/schedule'; +import { HandleExpiredItemsCommand } from '../../application/commands/handle-expired-items.command'; +import { LoggerService } from '../../application/services/logger.service'; + +@Injectable() +export class ExpiredItemsSchedulerService implements OnModuleInit { + constructor( + private readonly handleExpiredItemsCommand: HandleExpiredItemsCommand, + private readonly logger: LoggerService, + ) {} + + async onModuleInit(): Promise { + this.logger.log('ExpiredItemsSchedulerService initialized', ExpiredItemsSchedulerService.name); + + // Process expired items immediately on startup + try { + this.logger.log('Processing expired items on startup...', ExpiredItemsSchedulerService.name); + await this.handleExpiredItemsCommand.execute(); + this.logger.log('Startup expired items processing completed', ExpiredItemsSchedulerService.name); + } catch (error) { + this.logger.error(`Failed to process expired items on startup: ${error.message}`, undefined, ExpiredItemsSchedulerService.name); + } + } + + @Cron(CronExpression.EVERY_MINUTE) + async handleExpiredItemsCron(): Promise { + try { + this.logger.log('Starting scheduled expired items processing', ExpiredItemsSchedulerService.name); + await this.handleExpiredItemsCommand.execute(); + this.logger.log('Scheduled expired items processing completed', ExpiredItemsSchedulerService.name); + } catch (error) { + this.logger.error(`Failed to process expired items: ${error.message}`, undefined, ExpiredItemsSchedulerService.name); + // Continue running despite errors - don't let one failure stop the scheduler + } + } + + @Cron('0 0 * * *') // Every day at midnight + async handleExpiredItemsDaily(): Promise { + try { + this.logger.log('Starting daily expired items processing at midnight', ExpiredItemsSchedulerService.name); + await this.handleExpiredItemsCommand.execute(); + this.logger.log('Daily expired items processing completed', ExpiredItemsSchedulerService.name); + } catch (error) { + this.logger.error(`Failed to process expired items at midnight: ${error.message}`, undefined, ExpiredItemsSchedulerService.name); + } + } +} \ No newline at end of file diff --git a/nestjs/src/infrastructure/services/user-initialization.service.ts b/nestjs/src/infrastructure/services/user-initialization.service.ts index 8369302..8656dd9 100644 --- a/nestjs/src/infrastructure/services/user-initialization.service.ts +++ b/nestjs/src/infrastructure/services/user-initialization.service.ts @@ -1,16 +1,16 @@ -import { Injectable, Logger, OnModuleInit, Inject } from '@nestjs/common'; +import { Injectable, OnModuleInit, Inject } from '@nestjs/common'; import * as bcrypt from 'bcrypt'; import { IUserRepository } from '../../application/interfaces/user-repository.interface'; import { UserEntity } from '../../domain/entities/user.entity'; import { UserId } from '../../domain/value-objects/user-id.vo'; +import { LoggerService } from '../../application/services/logger.service'; @Injectable() export class UserInitializationService implements OnModuleInit { - private readonly logger = new Logger(UserInitializationService.name); - constructor( @Inject('IUserRepository') private readonly userRepository: IUserRepository, + private readonly logger: LoggerService, ) {} async onModuleInit(): Promise { @@ -35,9 +35,9 @@ export class UserInitializationService implements OnModuleInit { ); await this.userRepository.save(user); - this.logger.log(`Created default user: ${userData.username}`); + this.logger.log(`Created default user: ${userData.username}`, UserInitializationService.name); } else { - this.logger.log(`Default user already exists: ${userData.username}`); + this.logger.log(`Default user already exists: ${userData.username}`, UserInitializationService.name); } } }