Files
jdescopingtool/PLANS/architecture-review.md
T
Joseph Doherty 26ff8d9b4f Initial commit: JDE Scoping Tool migration project
Set up repository with legacy .NET Framework 4.8 source (OLD/),
new .NET 10 Blazor solution (NEW/), OpenSpec specifications,
documentation, and project configuration.
2026-01-02 07:43:29 -05:00

285 lines
12 KiB
Markdown

# JdeScopingTool Architecture Review
**Date:** January 1, 2026
**Reviewer:** Claude + Codex MCP
**Scope:** Clean Architecture compliance of NEW/ .NET 10 solution
---
## Executive Summary
The current architecture has several **critical** and **high-severity** Clean Architecture violations that need to be addressed. The most pressing issues are:
1. **Duplicate interfaces in different namespaces** - causes DI resolution failures
2. **Infrastructure code in Core layer** - violates the dependency rule
3. **DI composition leaking into Core** - composition should be in Host only
---
## Current Project Structure
```
NEW/src/
├── JdeScoping.Core # Domain layer (BUT contains infrastructure)
├── JdeScoping.DataAccess # Data access layer
├── JdeScoping.DataSync # Data synchronization service
├── JdeScoping.SearchProcessing # Search query processing
├── JdeScoping.ExcelExport # Excel file generation
├── JdeScoping.Api # Web API controllers
├── JdeScoping.Host # Composition root (ASP.NET host)
├── JdeScoping.Client # Blazor WebAssembly UI
└── JdeScoping.Database # Database migrations (DbUp)
```
### Dependency Graph (Current)
```
Host → Api, Core, Database, DataAccess, DataSync, SearchProcessing
Api → Core
DataAccess → Core
DataSync → Core, DataAccess
SearchProcessing → Core, DataAccess
ExcelExport → Core
Database → (standalone)
Client → (standalone WASM)
```
---
## Critical Issues
### 1. Duplicate `ILotFinderRepository` Interfaces (CRITICAL)
**Problem:** Two separate `ILotFinderRepository` interfaces exist in different namespaces:
| Location | Namespace | Partials |
|----------|-----------|----------|
| `Core/Interfaces/` | `JdeScoping.Core.Interfaces` | `ILotFinderRepository.cs`, `.SearchOperations.cs`, `.Lookups.cs` |
| `DataAccess/Interfaces/` | `JdeScoping.DataAccess.Interfaces` | `ILotFinderRepository.cs`, `.DataSync.cs`, `.Lookups.cs`, `.SearchManagement.cs` |
**Impact:**
- `SearchController` (`Api/Controllers/SearchController.cs:23`) depends on `JdeScoping.Core.Interfaces.ILotFinderRepository`
- DI registration in DataAccess registers `JdeScoping.DataAccess.Interfaces.ILotFinderRepository`
- These are **completely different interfaces** - partial interfaces only merge within the same namespace/assembly
- **Runtime DI will fail** because the Core interface is never registered
**Fix:**
1. Consolidate ALL `ILotFinderRepository` partials into `JdeScoping.Core.Interfaces`
2. Have `LotFinderRepository` implementation in DataAccess implement the Core interface
3. Remove duplicate interface files from DataAccess
---
### 2. Infrastructure Code in Core Layer (HIGH)
**Problem:** Core contains concrete implementations that depend on infrastructure packages.
**Files in Core that violate this:**
- `Core/Repositories/Jde/JdeOracleDataSource.cs` - uses `Oracle.ManagedDataAccess.Client`, `Dapper`
- `Core/Repositories/Jde/JdeFileDataSource.cs` - file system access
- `Core/Repositories/Cms/CmsOracleDataSource.cs` - uses `Oracle.ManagedDataAccess.Client`, `Dapper`
- `Core/Repositories/Cms/CmsFileDataSource.cs` - file system access
- `Core/Auth/LdapAuthService.cs` - uses `System.DirectoryServices.Protocols`
**Package references in Core.csproj that should not be there:**
```xml
<PackageReference Include="Dapper" Version="2.1.66" />
<PackageReference Include="Microsoft.Data.SqlClient" Version="6.1.3" />
<PackageReference Include="Oracle.ManagedDataAccess.Core" Version="23.26.0" />
<PackageReference Include="ClosedXML" Version="0.105.0" />
<PackageReference Include="System.DirectoryServices.Protocols" Version="10.0.1" />
```
**Fix:**
1. Move `JdeOracleDataSource`, `CmsOracleDataSource` to `JdeScoping.DataAccess/Sources/` or new `JdeScoping.Infrastructure`
2. Move `JdeFileDataSource`, `CmsFileDataSource` alongside the Oracle implementations
3. Move `LdapAuthService` to `JdeScoping.Api/Auth/` or new `JdeScoping.Infrastructure`
4. Remove infrastructure package references from Core.csproj
5. Keep only interfaces (`IJdeDataSource`, `ICmsDataSource`, `IAuthService`) in Core
---
### 3. DI Registration Extensions in Core (HIGH)
**Problem:** Core contains service registration extensions, which is composition logic that belongs in Host.
**Files:**
- `Core/Extensions/DataSourceServiceExtensions.cs`
- `Core/Extensions/AuthServiceExtensions.cs`
- `Core/Extensions/DataSyncServiceExtensions.cs`
- `Core/Extensions/ExcelExportServiceExtensions.cs`
- `Core/Extensions/SearchProcessingServiceExtensions.cs`
**Fix:**
1. Move these to `JdeScoping.Host/Extensions/` or to each respective project's own extension class
2. Each infrastructure project should expose an `AddXxx(IServiceCollection)` method
3. Host calls these methods in composition root
---
## Medium Issues
### 4. ViewModels in Core Layer (MEDIUM)
**Problem:** Core contains ViewModels that are UI-shaped DTOs.
**Files:**
- `Core/ViewModels/WorkOrderViewModel.cs`
- `Core/ViewModels/LotViewModel.cs`
- `Core/ViewModels/ItemViewModel.cs`
- `Core/ViewModels/WorkCenterViewModel.cs`
- `Core/ViewModels/ProfitCenterViewModel.cs`
- `Core/ViewModels/PartOperationViewModel.cs`
**Impact:** Core should contain domain models, not presentation layer DTOs.
**Fix Options:**
1. **If ViewModels are shared between API and Client:** Move to a shared `JdeScoping.Contracts` or `JdeScoping.Shared` project
2. **If ViewModels are API-only:** Move to `JdeScoping.Api/Models/`
3. **Alternative:** Keep in Core but rename to `*Dto` to clarify they are data transfer objects, not UI-specific
---
### 5. SearchProcessing Uses Dapper/SqlClient Directly (MEDIUM)
**Problem:** `SearchProcessing/Services/SearchProcessor.cs` uses Dapper and SqlClient directly.
**Impact:** This puts data access logic inside what should be a use-case/application layer.
**Fix Options:**
1. Move SearchProcessing data access into DataAccess (define ports like `ISearchQueryExecutor` in Core, implement in DataAccess)
2. Or accept that SearchProcessing is infrastructure and treat it as such
---
### 6. Duplicate/Unused Excel Export Abstractions (LOW)
**Problem:** Excel export has split abstractions:
- `Core/Interfaces/IExcelWriter.cs` - Core defines interface
- `Core/Extensions/ExcelExportServiceExtensions.cs` - Core has registration
- `ExcelExport/Interfaces/IExcelExportService.cs` - ExcelExport defines different interface
- `ExcelExport/ServiceCollectionExtensions.cs` - ExcelExport has registration
**Fix:**
1. Decide on one interface (keep `IExcelWriter` in Core)
2. Have ExcelExport implement the Core interface
3. Remove duplicate interface/registration from ExcelExport
---
## Recommended Target Architecture
### Clean Architecture Layers
```
┌─────────────────────────────────────────────────────────────┐
│ HOST │
│ JdeScoping.Host (Composition Root, Program.cs) │
└─────────────────────────────────────────────────────────────┘
┌──────────────────────┼──────────────────────┐
▼ ▼ ▼
┌─────────────────┐ ┌─────────────────┐ ┌─────────────────┐
│ PRESENTATION │ │ APPLICATION │ │ INFRASTRUCTURE │
│ JdeScoping.Api │ │ (Use Cases) │ │ │
│ JdeScoping. │ │ │ │ DataAccess │
│ Client │ │ │ │ DataSync │
└─────────────────┘ └─────────────────┘ │ ExcelExport │
│ │ │ Infrastructure* │
└──────────────────────┼───────────┴─────────────────┘
┌─────────────────┐
│ DOMAIN │
│ JdeScoping.Core │
│ (Pure, no deps) │
└─────────────────┘
```
*New project for Oracle/LDAP implementations
### Dependency Direction (Target)
All dependencies point inward toward Core:
- Host → All projects
- Api → Core only
- DataAccess → Core only
- DataSync → Core, DataAccess
- SearchProcessing → Core, DataAccess
- ExcelExport → Core only
- Infrastructure → Core only
- Client → (standalone, calls Api)
---
## Refactoring Plan
### Phase 1: Fix Critical Issues
1. **Consolidate ILotFinderRepository** (Day 1)
- Move all partial interface files to `Core/Interfaces/`
- Update DataAccess implementation to use Core interface
- Update DI registration
- Verify all consumers use `JdeScoping.Core.Interfaces`
2. **Move Infrastructure from Core** (Day 1-2)
- Create `JdeScoping.Infrastructure` project (or use DataAccess)
- Move `JdeOracleDataSource`, `CmsOracleDataSource` implementations
- Move `JdeFileDataSource`, `CmsFileDataSource` implementations
- Move `LdapAuthService`
- Remove infrastructure packages from Core.csproj
### Phase 2: Fix High Issues
3. **Move DI Extensions** (Day 2)
- Move service registration extensions to respective infrastructure projects
- Update Host to call each project's `AddXxx()` method
### Phase 3: Fix Medium Issues
4. **Organize ViewModels** (Day 3)
- Decide on placement (Shared, Api, or keep in Core)
- Move and update references
5. **Clean up SearchProcessing** (Day 3)
- Define ports in Core if needed
- Ensure data access goes through DataAccess layer
6. **Unify Excel Export** (Day 3)
- Single interface in Core
- Single implementation/registration in ExcelExport
---
## Verification Checklist
After refactoring, verify:
- [ ] `dotnet build` succeeds
- [ ] All tests pass
- [ ] Core.csproj has NO infrastructure package references (only abstractions)
- [ ] Core contains ONLY: Models, Interfaces, Options, Helpers, pure domain logic
- [ ] No circular dependencies
- [ ] DI resolves all interfaces correctly at runtime
- [ ] API endpoints work end-to-end
---
## Files to Move/Change
| Current Location | Target Location | Action |
|-----------------|-----------------|--------|
| `Core/Repositories/Jde/*` | `Infrastructure/Sources/Jde/` | Move |
| `Core/Repositories/Cms/*` | `Infrastructure/Sources/Cms/` | Move |
| `Core/Auth/LdapAuthService.cs` | `Infrastructure/Auth/` | Move |
| `DataAccess/Interfaces/ILotFinderRepository*.cs` | `Core/Interfaces/` | Consolidate |
| `Core/Extensions/*ServiceExtensions.cs` | `Host/Extensions/` or respective projects | Move |
| `Core/ViewModels/*` | Decision needed | TBD |
---
## Questions to Resolve
1. Should ViewModels be shared with Blazor Client? If yes, create `JdeScoping.Shared` project.
2. Should SearchProcessing be treated as application layer or infrastructure?
3. Prefer single `Infrastructure` project or keep separate (`DataAccess`, `DataSync`, etc.)?