Add PlantGuide iOS app with plant identification and care management
- Implement camera capture and plant identification workflow - Add Core Data persistence for plants, care schedules, and cached API data - Create collection view with grid/list layouts and filtering - Build plant detail views with care information display - Integrate Trefle botanical API for plant care data - Add local image storage for captured plant photos - Implement dependency injection container for testability - Include accessibility support throughout the app Bug fixes in this commit: - Fix Trefle API decoding by removing duplicate CodingKeys - Fix LocalCachedImage to load from correct PlantImages directory - Set dateAdded when saving plants for proper collection sorting Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
This commit is contained in:
Trey t
289
Docs/ARCHITECTURE_REMEDIATION_PLAN.md
Normal file
289
Docs/ARCHITECTURE_REMEDIATION_PLAN.md
Normal file
@@ -0,0 +1,289 @@
|
||||
# Architecture Remediation Plan
|
||||
|
||||
**Project:** PlantGuide iOS App
|
||||
**Date:** 2026-01-23
|
||||
**Priority:** Pre-Production Cleanup
|
||||
**Estimated Scope:** Medium (1-2 sprint cycles)
|
||||
|
||||
---
|
||||
|
||||
## Overview
|
||||
|
||||
This plan addresses architectural concerns identified during code review. Items are prioritized by risk and impact to production stability.
|
||||
|
||||
---
|
||||
|
||||
## Phase 1: Critical Fixes (Do First)
|
||||
|
||||
These items could cause runtime crashes or data inconsistency if not addressed.
|
||||
|
||||
### Task 1.1: Fix Repository Protocol Conformance
|
||||
|
||||
**File:** `PlantGuide/Data/Repositories/InMemoryPlantRepository.swift`
|
||||
|
||||
**Problem:** `InMemoryPlantRepository` is returned as `PlantRepositoryProtocol` in DIContainer but doesn't actually conform to that protocol.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Add `PlantRepositoryProtocol` conformance to `InMemoryPlantRepository`
|
||||
- [ ] Implement any missing required methods from the protocol
|
||||
- [ ] Verify compilation succeeds
|
||||
- [ ] Add unit test to verify conformance
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Code compiles without warnings
|
||||
- `DIContainer.plantRepository` returns a valid `PlantRepositoryProtocol` instance
|
||||
|
||||
---
|
||||
|
||||
### Task 1.2: Fix Core Data Stack Thread Safety
|
||||
|
||||
**File:** `PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift`
|
||||
|
||||
**Problem:** `lazy var persistentContainer` initialization isn't thread-safe despite class being marked `@unchecked Sendable`.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Replace `lazy var` with a thread-safe initialization pattern
|
||||
- [ ] Option A: Use `DispatchQueue.sync` for synchronized access
|
||||
- [ ] Option B: Use an actor for the Core Data stack
|
||||
- [ ] Option C: Initialize in `init()` instead of lazily
|
||||
- [ ] Add concurrency tests to verify thread safety
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- No race conditions when accessing `persistentContainer` from multiple threads
|
||||
- Existing Core Data tests still pass
|
||||
|
||||
---
|
||||
|
||||
### Task 1.3: Replace Unowned with Weak References in DIContainer
|
||||
|
||||
**File:** `PlantGuide/Core/DI/DIContainer.swift`
|
||||
|
||||
**Problem:** `[unowned self]` in lazy closures could crash if accessed after deallocation.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Find all `[unowned self]` captures in DIContainer (lines ~121, 150, 196, 204, 213)
|
||||
- [ ] Replace with `[weak self]` and add guard statements
|
||||
- [ ] Example pattern:
|
||||
```swift
|
||||
LazyService { [weak self] in
|
||||
guard let self else { fatalError("DIContainer deallocated unexpectedly") }
|
||||
return PlantNetAPIService.configured(...)
|
||||
}
|
||||
```
|
||||
- [ ] Consider if fatalError is appropriate or if optional return is better
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- No `unowned` references in DIContainer
|
||||
- App behaves correctly during normal lifecycle
|
||||
|
||||
---
|
||||
|
||||
## Phase 2: Architectural Consistency (High Priority)
|
||||
|
||||
These items address design inconsistencies that affect maintainability.
|
||||
|
||||
### Task 2.1: Resolve SwiftData vs Core Data Conflict
|
||||
|
||||
**Files:**
|
||||
- `PlantGuide/PlantGuideApp.swift`
|
||||
- `PlantGuide/Item.swift`
|
||||
|
||||
**Problem:** App initializes SwiftData ModelContainer but uses Core Data for persistence. `Item.swift` appears to be unused template code.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] **Decision Required:** Confirm with team - Are we using SwiftData or Core Data?
|
||||
- [ ] If Core Data (current approach):
|
||||
- [ ] Remove `sharedModelContainer` from `PlantGuideApp.swift`
|
||||
- [ ] Remove `.modelContainer(sharedModelContainer)` modifier
|
||||
- [ ] Delete `Item.swift`
|
||||
- [ ] Remove `import SwiftData` if no longer needed
|
||||
- [ ] If migrating to SwiftData (future):
|
||||
- [ ] Document migration plan
|
||||
- [ ] Keep current Core Data as transitional
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Only one persistence technology in active use
|
||||
- No dead code related to unused persistence layer
|
||||
|
||||
---
|
||||
|
||||
### Task 2.2: Unify Repository Implementation
|
||||
|
||||
**Files:**
|
||||
- `PlantGuide/Core/DI/DIContainer.swift`
|
||||
- `PlantGuide/Data/Repositories/InMemoryPlantRepository.swift`
|
||||
|
||||
**Problem:** `plantRepository` returns in-memory storage while `plantCollectionRepository` returns Core Data. This creates potential data sync issues.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] **Decision Required:** Should all plant data use Core Data in production?
|
||||
- [ ] If yes (recommended):
|
||||
- [ ] Update `DIContainer.plantRepository` to return `_coreDataPlantStorage.value`
|
||||
- [ ] Ensure `CoreDataPlantStorage` conforms to `PlantRepositoryProtocol`
|
||||
- [ ] Keep `InMemoryPlantRepository` for testing only
|
||||
- [ ] Add `#if DEBUG` around sample data in `InMemoryPlantRepository`
|
||||
- [ ] Update any code that assumes in-memory behavior
|
||||
- [ ] Run full test suite to verify no regressions
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Single source of truth for plant data in production
|
||||
- Tests can still use in-memory repository via DI
|
||||
|
||||
---
|
||||
|
||||
### Task 2.3: Guard Sample Data with DEBUG Flag
|
||||
|
||||
**File:** `PlantGuide/Data/Repositories/InMemoryPlantRepository.swift`
|
||||
|
||||
**Problem:** `seedWithSampleData()` runs in production builds.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Wrap `seedWithSampleData()` call in `init()` with `#if DEBUG`:
|
||||
```swift
|
||||
private init() {
|
||||
#if DEBUG
|
||||
seedWithSampleData()
|
||||
#endif
|
||||
}
|
||||
```
|
||||
- [ ] Consider making sample data opt-in via a parameter
|
||||
- [ ] Verify production builds don't include sample plants
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Release builds start with empty repository
|
||||
- Debug builds can optionally include sample data
|
||||
|
||||
---
|
||||
|
||||
## Phase 3: Clean Architecture Compliance (Medium Priority)
|
||||
|
||||
These items improve separation of concerns and testability.
|
||||
|
||||
### Task 3.1: Extract UI Extensions from Domain Enums
|
||||
|
||||
**Files:**
|
||||
- `PlantGuide/Domain/Entities/Enums.swift` (source)
|
||||
- Create: `PlantGuide/Presentation/Extensions/CareTaskType+UI.swift`
|
||||
- Create: `PlantGuide/Presentation/Extensions/Enums+UI.swift`
|
||||
|
||||
**Problem:** Domain enums import SwiftUI and contain UI-specific code (colors, icons), violating Clean Architecture.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Create new file `Presentation/Extensions/Enums+UI.swift`
|
||||
- [ ] Move these extensions from `Enums.swift`:
|
||||
- `CareTaskType.iconName`
|
||||
- `CareTaskType.iconColor`
|
||||
- `CareTaskType.description`
|
||||
- `LightRequirement.description`
|
||||
- `WateringFrequency.description`
|
||||
- `FertilizerFrequency.description`
|
||||
- `HumidityLevel.description`
|
||||
- [ ] Remove `import SwiftUI` from `Enums.swift`
|
||||
- [ ] Verify all views still compile and display correctly
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- `Domain/Entities/Enums.swift` has no SwiftUI import
|
||||
- All UI functionality preserved in Presentation layer
|
||||
- Domain layer has zero UI framework dependencies
|
||||
|
||||
---
|
||||
|
||||
### Task 3.2: Reduce Singleton Usage (Optional Refactor)
|
||||
|
||||
**Files:** Multiple
|
||||
|
||||
**Problem:** Heavy singleton usage reduces testability and flexibility.
|
||||
|
||||
**Action Items:**
|
||||
- [ ] **Low Priority** - Document current singletons:
|
||||
- `DIContainer.shared`
|
||||
- `CoreDataStack.shared`
|
||||
- `InMemoryPlantRepository.shared`
|
||||
- `FilterPreferencesStorage.shared`
|
||||
- [ ] For new code, prefer dependency injection over `.shared` access
|
||||
- [ ] Consider refactoring `FilterPreferencesStorage` to be injected
|
||||
- [ ] Keep `DIContainer.shared` as acceptable app-level singleton
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- New code uses DI patterns
|
||||
- Existing singletons documented
|
||||
- No new singletons added without justification
|
||||
|
||||
---
|
||||
|
||||
## Phase 4: Code Cleanup (Low Priority)
|
||||
|
||||
Minor improvements for code hygiene.
|
||||
|
||||
### Task 4.1: Remove Unused Template Files
|
||||
|
||||
**File:** `PlantGuide/Item.swift`
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Verify `Item` is not referenced anywhere (search project)
|
||||
- [ ] Delete `Item.swift`
|
||||
- [ ] Remove from Xcode project if needed
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- No unused files in project
|
||||
|
||||
---
|
||||
|
||||
### Task 4.2: Review @unchecked Sendable Usage
|
||||
|
||||
**Files:**
|
||||
- `PlantGuide/Data/DataSources/Remote/NetworkService/NetworkService.swift`
|
||||
- `PlantGuide/Data/DataSources/Local/CoreData/CoreDataStack.swift`
|
||||
|
||||
**Action Items:**
|
||||
- [ ] Audit each `@unchecked Sendable` usage
|
||||
- [ ] Document why each is safe OR fix the underlying issue
|
||||
- [ ] Add code comments explaining thread-safety guarantees
|
||||
|
||||
**Acceptance Criteria:**
|
||||
- Each `@unchecked Sendable` has documented justification
|
||||
- No hidden thread-safety bugs
|
||||
|
||||
---
|
||||
|
||||
## Task Checklist Summary
|
||||
|
||||
### Must Complete Before Release
|
||||
- [ ] 1.1 - Fix Repository Protocol Conformance
|
||||
- [ ] 1.2 - Fix Core Data Stack Thread Safety
|
||||
- [ ] 1.3 - Replace Unowned References
|
||||
- [ ] 2.1 - Resolve SwiftData vs Core Data
|
||||
- [ ] 2.2 - Unify Repository Implementation
|
||||
- [ ] 2.3 - Guard Sample Data
|
||||
|
||||
### Should Complete
|
||||
- [ ] 3.1 - Extract UI Extensions from Domain
|
||||
- [ ] 4.1 - Remove Unused Template Files
|
||||
|
||||
### Nice to Have
|
||||
- [ ] 3.2 - Reduce Singleton Usage
|
||||
- [ ] 4.2 - Review @unchecked Sendable
|
||||
|
||||
---
|
||||
|
||||
## Decision Log
|
||||
|
||||
| Decision | Options | Chosen | Date | Decided By |
|
||||
|----------|---------|--------|------|------------|
|
||||
| Persistence technology | SwiftData / Core Data | TBD | | |
|
||||
| Production repository | Core Data / In-Memory | TBD | | |
|
||||
|
||||
---
|
||||
|
||||
## Notes for Developer
|
||||
|
||||
1. **Start with Phase 1** - These are potential crash sources
|
||||
2. **Run tests after each task** - Ensure no regressions
|
||||
3. **Create separate commits** - One commit per task for easy review/revert
|
||||
4. **Update this doc** - Check off items as completed
|
||||
5. **Ask questions** - Flag any blockers in standup
|
||||
|
||||
---
|
||||
|
||||
*Document created by: Project Manager*
|
||||
*Last updated: 2026-01-23*
|
||||
Reference in New Issue
Block a user