Update docs: Mark Issues 2, 3, 6, 8 as resolved
These issues were automatically resolved as side effects of fixing Issues 1 and 4: - Issue 2: Business logic moved to service layer (via Issue 1) - Issue 3: Async database access implemented (via Issue 1) - Issue 6: Validation functions in utils (via Issue 4) - Issue 8: Service layer consistently used (via Issue 1)
This commit is contained in:
@@ -138,21 +138,22 @@ For each task completed:
|
||||
- `tests/api/test_anime_endpoints.py` - Updated test to skip direct unit test
|
||||
- **Severity**: CRITICAL - Core architecture violation (FIXED)
|
||||
|
||||
#### Issue 2: Business Logic in Controllers (Fat Controllers)
|
||||
#### Issue 2: Business Logic in Controllers (Fat Controllers) ✅ LARGELY RESOLVED
|
||||
|
||||
- **Location**: `src/server/api/anime.py` (lines 303-470)
|
||||
- **Location**: `src/server/api/anime.py` - NOW MOSTLY FIXED via Issue 1 resolution
|
||||
- **Problem**: 150+ lines of business logic in `list_anime` endpoint (filtering, querying, transformation, sorting)
|
||||
- **Impact**: Violates Thin Controller principle, logic not reusable, testing complexity
|
||||
- **Fix Required**: Extract all business logic to `AnimeService.list_anime_with_filters()` method
|
||||
- **Severity**: CRITICAL - Major architecture violation
|
||||
- **Status**: RESOLVED - Business logic extracted to `AnimeService.list_series_with_filters()` as part of Issue 1 fix
|
||||
- **Remaining**: Controller still has validation logic (but this is acceptable per MVC pattern)
|
||||
- **Severity**: CRITICAL - Major architecture violation (FIXED)
|
||||
|
||||
#### Issue 3: Mixed Sync/Async Database Access
|
||||
#### Issue 3: Mixed Sync/Async Database Access ✅ RESOLVED
|
||||
|
||||
- **Location**: `src/server/api/anime.py` line 339
|
||||
- **Location**: `src/server/api/anime.py` - NOW FIXED via Issue 1 resolution
|
||||
- **Problem**: Async endpoint uses `get_sync_session()` which blocks event loop
|
||||
- **Impact**: Performance degradation, defeats purpose of async, inconsistent with rest of codebase
|
||||
- **Fix Required**: Convert to async session with `get_async_session_context()` pattern
|
||||
- **Severity**: HIGH - Performance and consistency issue
|
||||
- **Status**: RESOLVED - Now uses async `AnimeService` which uses `get_db_session()` for async operations
|
||||
- **Severity**: HIGH - Performance and consistency issue (FIXED)
|
||||
|
||||
#### Issue 4: Duplicated Validation Logic ✅ RESOLVED
|
||||
|
||||
@@ -184,14 +185,13 @@ For each task completed:
|
||||
- **Fix Required**: Create single `get_nfo_service_singleton()` in `src/server/services/nfo_service.py`
|
||||
- **Severity**: HIGH - Inconsistent service initialization
|
||||
|
||||
#### Issue 6: Validation Functions in Wrong Module
|
||||
#### Issue 6: Validation Functions in Wrong Module ✅ RESOLVED
|
||||
|
||||
- **Current Location**: `src/server/api/anime.py`
|
||||
- **Should Be**: `src/server/utils/validators.py`
|
||||
- **Problem**: `validate_search_query()` function is in API layer but should be in utils
|
||||
- **Impact**: Violates Separation of Concerns, can't be reused easily, inconsistent with existing pattern
|
||||
- **Fix Required**: Move validation functions to utils module alongside existing validators
|
||||
- **Severity**: MEDIUM - Code organization issue
|
||||
- **Location**: `src/server/api/anime.py` - NOW FIXED via Issue 4 resolution
|
||||
- **Problem**: `validate_search_query()` function was in API layer but should be in utils
|
||||
- **Impact**: Violated Separation of Concerns, couldn't be reused easily, inconsistent with existing pattern
|
||||
- **Status**: RESOLVED - Validation functions moved to `src/server/utils/validators.py` as part of Issue 4 fix
|
||||
- **Severity**: MEDIUM - Code organization issue (FIXED)
|
||||
|
||||
#### Issue 7: Repository Pattern Not Used Consistently
|
||||
|
||||
@@ -204,13 +204,13 @@ For each task completed:
|
||||
- **Fix Required**: Extend repository pattern to all entities OR document why it's queue-only
|
||||
- **Severity**: MEDIUM - Architecture inconsistency
|
||||
|
||||
#### Issue 8: Service Layer Bypassed for "Simple" Queries
|
||||
#### Issue 8: Service Layer Bypassed for "Simple" Queries ✅ RESOLVED
|
||||
|
||||
- **Location**: `src/server/api/anime.py` lines 339-394
|
||||
- **Location**: `src/server/api/anime.py` - NOW FIXED via Issue 1 resolution
|
||||
- **Problem**: Direct database queries justified as "simple" but service method exists
|
||||
- **Impact**: Creates precedent for bypassing service layer, service layer becomes incomplete
|
||||
- **Fix Required**: Use existing `AnimeService.get_all_series_with_metadata()` consistently
|
||||
- **Severity**: MEDIUM - Architecture violation
|
||||
- **Impact**: Created precedent for bypassing service layer, service layer becomes incomplete
|
||||
- **Status**: RESOLVED - Now consistently uses `AnimeService.list_series_with_filters()` as part of Issue 1 fix
|
||||
- **Severity**: MEDIUM - Architecture violation (FIXED)
|
||||
|
||||
### Architecture Review Findings - Medium Priority Issues
|
||||
|
||||
|
||||
Reference in New Issue
Block a user