Problem
MigrationScriptExecutor has become a God class again with too many responsibilities:
- Migration orchestration - coordinating the overall migration process
- Rollback logic - implementing 4 different rollback strategies (BACKUP, DOWN, BOTH, NONE)
- Hook execution - calling lifecycle hooks (onBeforeMigrate, onAfterMigrate, onMigrationError)
- Script execution - running individual migration scripts
- State management - tracking executed and pending migrations
- Rendering - coordinating with ConsoleRenderer for output
This violates the Single Responsibility Principle and makes the class difficult to maintain and test.
Proposed Solution
Extract responsibilities into focused services:
1. RollbackService
- Encapsulates all rollback strategies (BACKUP, DOWN, BOTH, NONE)
- Methods:
rollback(strategy: RollbackStrategy, attemptedScripts: MigrationScript[]): Promise<void>rollbackWithBackup(): Promise<void>rollbackWithDown(scripts: MigrationScript[]): Promise<void>rollbackWithBoth(scripts: MigrationScript[]): Promise<void>
- Dependencies: BackupService, IDatabaseMigrationHandler, Config, ILogger
2. MigrationHookExecutor
- Manages lifecycle hook execution
- Methods:
executeBeforeMigrate(script: MigrationScript): Promise<void>executeAfterMigrate(result: MigrationScript, message: string): Promise<void>executeMigrationError(script: MigrationScript, error: Error): Promise<void>
- Dependencies: IMigrationLifecycleHooks
3. MigrationScriptExecutor (Simplified)
- Focused only on orchestrating the overall migration flow
- Delegates to specialized services
- Methods remain the same but implementation is simplified:
migrate(): Promise<IMigrationResult>list(limit?: number): Promise<void>
- Dependencies:
- RollbackService
- MigrationHookExecutor
- MigrationScriptRunner
- BackupService
- MigrationScanner
- ConsoleRenderer
- Config
- ILogger
Benefits
- ✅ Single Responsibility Principle - each class has one clear purpose
- ✅ Easier Testing - smaller, focused unit tests
- ✅ Better Maintainability - changes to rollback logic don't affect hook execution
- ✅ Improved Readability - clear separation of concerns
- ✅ Reusability - RollbackService and MigrationHookExecutor can be used independently
Implementation Notes
- Keep backward compatibility - public API remains unchanged
- Move private methods to appropriate services
- Update tests to reflect new structure
- Update documentation with new architecture
Acceptance Criteria
Problem
MigrationScriptExecutorhas become a God class again with too many responsibilities:This violates the Single Responsibility Principle and makes the class difficult to maintain and test.
Proposed Solution
Extract responsibilities into focused services:
1. RollbackService
rollback(strategy: RollbackStrategy, attemptedScripts: MigrationScript[]): Promise<void>rollbackWithBackup(): Promise<void>rollbackWithDown(scripts: MigrationScript[]): Promise<void>rollbackWithBoth(scripts: MigrationScript[]): Promise<void>2. MigrationHookExecutor
executeBeforeMigrate(script: MigrationScript): Promise<void>executeAfterMigrate(result: MigrationScript, message: string): Promise<void>executeMigrationError(script: MigrationScript, error: Error): Promise<void>3. MigrationScriptExecutor (Simplified)
migrate(): Promise<IMigrationResult>list(limit?: number): Promise<void>Benefits
Implementation Notes
Acceptance Criteria