From 303e7a011eee130ff7a0c4724ddcb13713a88c73 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 23 Jan 2026 18:39:23 -0500 Subject: [PATCH 1/2] docs: map existing codebase - STACK.md - Technologies and dependencies - ARCHITECTURE.md - System design and patterns - STRUCTURE.md - Directory layout - CONVENTIONS.md - Code style and patterns - TESTING.md - Test structure - INTEGRATIONS.md - External services - CONCERNS.md - Technical debt and issues --- .planning/codebase/ARCHITECTURE.md | 192 +++++++++++++++++ .planning/codebase/CONCERNS.md | 229 ++++++++++++++++++++ .planning/codebase/CONVENTIONS.md | 200 ++++++++++++++++++ .planning/codebase/INTEGRATIONS.md | 154 ++++++++++++++ .planning/codebase/STACK.md | 133 ++++++++++++ .planning/codebase/STRUCTURE.md | 239 +++++++++++++++++++++ .planning/codebase/TESTING.md | 328 +++++++++++++++++++++++++++++ 7 files changed, 1475 insertions(+) create mode 100644 .planning/codebase/ARCHITECTURE.md create mode 100644 .planning/codebase/CONCERNS.md create mode 100644 .planning/codebase/CONVENTIONS.md create mode 100644 .planning/codebase/INTEGRATIONS.md create mode 100644 .planning/codebase/STACK.md create mode 100644 .planning/codebase/STRUCTURE.md create mode 100644 .planning/codebase/TESTING.md diff --git a/.planning/codebase/ARCHITECTURE.md b/.planning/codebase/ARCHITECTURE.md new file mode 100644 index 0000000..4b51e5d --- /dev/null +++ b/.planning/codebase/ARCHITECTURE.md @@ -0,0 +1,192 @@ +# Architecture + +**Analysis Date:** 2026-01-23 + +## Pattern Overview + +**Overall:** MVVM (Model-View-ViewModel) with Message-Based Communication + +**Key Characteristics:** +- Strict separation between Views (UI), ViewModels (presentation logic), and Services (business logic) +- Decoupled components communicate via a message aggregator (publish-subscribe pattern) +- Observable collections and bindable properties for reactive data flow +- Async/await patterns for long-running operations (file scanning, preview generation) + +## Layers + +**Presentation Layer (Views):** +- Purpose: Render UI and handle user input through Unity UI components +- Location: `Assets/Scripts/Views/` +- Contains: View classes inheriting from `ViewBase`, dialog implementations, UI component bindings +- Depends on: ViewModels, Unity UI framework +- Used by: User interaction triggers, MonoBehaviour lifecycle + +**ViewModel Layer:** +- Purpose: Manage presentation state, commands, and observable properties; translate user actions to service calls +- Location: `Assets/Scripts/ViewModels/` +- Contains: Model classes for each screen/dialog (e.g., `SearchModel`, `ItemsModel`, `CollectionsModel`), command definitions, selection tracking +- Depends on: Services, Messages, Utilities (BindableProperty, ObservableList, Commands) +- Used by: Views (bound via `BindTo()` method), Message receivers + +**Service Layer (Business Logic):** +- Purpose: Core application logic, data management, and file operations +- Location: `Assets/Scripts/Services/` +- Contains: `Library` (main data service), `ConfigStore` (persistence), `ImportFolder` (file source abstraction), preview builders, metadata stores +- Depends on: Config, Utilities, FileSystem, Logging, Messaging +- Used by: ViewModels, other services + +**Utility Layer:** +- Purpose: Cross-cutting concerns and infrastructure +- Location: `Assets/Scripts/Util/` +- Contains: Messaging system, commands, collections, file system abstractions, tag filtering, logging, STL file parsing +- Depends on: Minimal dependencies; Some core .NET/Unity only +- Used by: All layers + +**Configuration Layer:** +- Purpose: Define data structures for application and user settings +- Location: `Assets/Scripts/Config/` +- Contains: Config classes (`CollectionConfig`, `ImportFolderConfig`, `SavedSearchConfig`), metadata structures +- Depends on: No dependencies (pure data) +- Used by: Services and ViewModels for persistent storage + +**Messages:** +- Purpose: Define loosely-coupled communication contracts +- Location: `Assets/Scripts/Messages/` +- Contains: Message classes for cross-component communication (e.g., `SearchChangedMessage`, `SelectionChangedMessage`) +- Depends on: No dependencies +- Used by: ViewModels to publish events, message receivers to subscribe + +## Data Flow + +**Search and Filter Flow:** + +1. User enters search tags in `SearchModel` (ViewModel) +2. `SearchModel` sends `SearchChangedMessage` via message aggregator +3. `ItemsModel` receives the message, calls `Library.GetItemPreviewMetadata(filters)` +4. `Library` queries `TagManager` to filter items by tags +5. Filtered items returned as observable list (`IPreviewList`) +6. `ItemsModel` updates its observable collection, triggering View updates +7. Views display updated item list via data binding + +**Item Selection Flow:** + +1. User clicks item in View (e.g., `ItemView`) +2. View sends `SelectionChangedMessage` via message aggregator +3. `ItemsModel` receives and processes selection, managing range selection state +4. Selection state reflected in `ItemPreviewModel.Selected` (BindableProperty) +5. Views observe property changes and update UI accordingly +6. Mass selection operations broadcast `MassSelectionStartingMessage` and `MassSelectionFinishedMessage` to suppress intermediate updates + +**File Import Flow:** + +1. User configures import folder via `AddImportFolderModel` +2. Folder path saved to config via `ConfigStore` +3. `ImportFolder` (file source) scans directory for STL files using `IFileSystem` +4. Files indexed and metadata extracted via `StlImporter` and geometry parsing +5. Preview images generated by `PreviewCam` (Unity camera rendering) +6. Metadata and previews cached in `PreviewImageStore` +7. `Library` notifies subscribers (ViewModels) of new items via observable collections + +**State Synchronization Flow:** + +1. User makes changes (tags, selection, settings) +2. ViewModels update via `BindableProperty` value changes +3. Changes trigger `ValueChanged` events +4. Views observe events and re-render +5. Services persist changes asynchronously to disk via `ConfigStore` +6. Layout state captured during application shutdown, restored on startup + +## Key Abstractions + +**ILibrary:** +- Purpose: Central interface for accessing and manipulating file metadata and preview data +- Examples: `Assets/Scripts/Services/Library.cs` +- Pattern: Facade pattern providing unified access to complex subsystems (tags, previews, file sources) + +**IFileSource:** +- Purpose: Abstract different sources of 3D model files (folders, containers, cloud storage) +- Examples: `Assets/Scripts/Services/ImportFolder.cs`, `FileSourceBase.cs` +- Pattern: Strategy pattern allowing pluggable file source implementations + +**IMessageRelay:** +- Purpose: Decoupled pub-sub messaging between components +- Examples: `Assets/Scripts/Util/Messaging/MessageAggregator.cs` +- Pattern: Observer/Mediator pattern using WeakReferences to avoid memory leaks + +**IBindableProperty:** +- Purpose: Observable property with change notification for reactive UI binding +- Examples: `Assets/Scripts/Util/BindableProperty.cs` +- Pattern: Observable pattern with value transformation and update suppression support + +**ICommand/IAsyncCommand:** +- Purpose: Encapsulate user actions and async operations with execution state tracking +- Examples: `Assets/Scripts/Util/Commands/DelegateCommand.cs`, `AsyncCommand.cs` +- Pattern: Command pattern with ICommand interface for UI binding + +**IReadOnlyObservableCollection:** +- Purpose: Provide change notifications for collection modifications +- Examples: `Assets/Scripts/Util/Collections/ObservableList.cs` +- Pattern: Observer pattern on collections with add/remove/clear events + +**IConfigStore:** +- Purpose: Abstract persistence layer for application configuration and metadata +- Examples: `Assets/Scripts/Services/ConfigStore.cs` +- Pattern: Repository pattern with async JSON serialization and ZIP compression + +## Entry Points + +**ViewInitializer:** +- Location: `Assets/Scripts/Views/ViewInitializer.cs` +- Triggers: Unity `Start()` lifecycle method on application launch +- Responsibilities: + - Creates and wires all services (Library, ConfigStore, MessageAggregator, UpdateChecker) + - Instantiates all ViewModels + - Binds ViewModels to Views + - Subscribes ViewModels to message aggregator + - Initializes application state (settings, layout, file sources) + - Handles application shutdown (saves state, disposes resources) + +**ApplicationView:** +- Location: `Assets/Scripts/Views/ApplicationView.cs` +- Triggers: View binding from ViewInitializer +- Responsibilities: Displays top-level commands (settings, feedback, help) + +**LibraryView:** +- Location: `Assets/Scripts/Views/LibraryView.cs` (via `Util/Unity/LibraryView.cs`) +- Triggers: View binding from ViewInitializer +- Responsibilities: Renders grid of 3D file preview items + +## Error Handling + +**Strategy:** Exception catching with logging, graceful degradation + +**Patterns:** +- Try-catch blocks in async initialization methods (e.g., `ViewInitializer.Start()`) +- Logging via `ILogger` (UnityLogger) for diagnostics +- User feedback via dialogs for critical errors (update checks, missing dependencies) +- File system errors handled in `ImportFolder.RescanItemsAsync()` with exception logging +- STL parsing errors caught and logged without halting import process +- Config file not found handled gracefully with default values + +## Cross-Cutting Concerns + +**Logging:** +- Approach: Dependency injection of `ILogger` interface; UnityLogger implementation +- Entry point: `Assets/Scripts/Util/Logging/UnityLogger.cs` +- Configured by: `ApplicationSettings.LogLevel` property bound in ViewInitializer + +**Validation:** +- Approach: Input validation in ViewModels before service calls; null checking with NotNull attributes +- Examples: `SearchModel` validates search tags before calling Library; dialog models validate user input + +**Authentication:** +- Approach: Not applicable; desktop application without user accounts +- File access: Controlled by OS file permissions on import folders + +**Async State Management:** +- Approach: `NotifyTask` and `NotifyingTaskWrapper` wrap async operations for UI binding +- Progress tracking: `ProgressMessage` and `MassSelectionStartingMessage` / `MassSelectionFinishedMessage` suppress UI updates during batch operations + +--- + +*Architecture analysis: 2026-01-23* diff --git a/.planning/codebase/CONCERNS.md b/.planning/codebase/CONCERNS.md new file mode 100644 index 0000000..9e2bae4 --- /dev/null +++ b/.planning/codebase/CONCERNS.md @@ -0,0 +1,229 @@ +# Codebase Concerns + +**Analysis Date:** 2026-01-23 + +## Tech Debt + +**Single Monolithic Scene:** +- Issue: All application content is part of a single `MainScene.unity` with all views, dialogs, and UI elements loaded at startup +- Files: `Assets/Scenes/MainScene.unity`, `Assets/Scripts/Views/ViewInitializer.cs` (284 lines) +- Impact: Scene becomes increasingly difficult to manage as features are added; long scene load times; memory usage cannot be optimized per feature; difficulty isolating components for testing +- Fix approach: Implement scene loading strategy with separate scenes for dialogs and major views; lazy-load non-essential UI elements; consider using Addressables for dynamic scene management + +**Synchronous Asset Blocking on Startup:** +- Issue: `LoadAsyncOrDefault().Result` in `ViewInitializer.Awake()` blocks main thread during scene initialization +- Files: `Assets/Scripts/Views/ViewInitializer.cs` (line 70) +- Impact: Increases initial load time; may cause app freezing on slower devices; IL2CPP builds particularly sensitive to main thread blocking +- Fix approach: Implement async initialization pattern; move layout restoration to after scene load completion; add progress indicator during startup + +**Unchecked Null Returns from Preview Loading:** +- Issue: `PreviewImageStore.LoadPreviewAsync()` returns `null` on error without logging or notifying caller; code consuming preview bytes doesn't validate nulls +- Files: `Assets/Scripts/Services/PreviewImageStore.cs` (lines 46-49); `Assets/Scripts/Views/ItemViewBase.cs` (lines 53-54) +- Impact: Silent failures when preview images are missing or corrupted; UI may display broken images or allocate textures incorrectly; difficult to diagnose preview loading issues +- Fix approach: Add explicit error logging in `LoadPreviewAsync`; return `Array.Empty()` instead of null for consistency; add null-coalescing validation in consumers + +**Bare Catch Blocks Swallowing Exceptions:** +- Issue: Multiple catch blocks with no exception handling or logging: `AddImportFolderModel.cs` line 44, `NotifyTask.cs` line 36, `PreviewImageStore.cs` line 46, `Library.cs` line 132 +- Files: `Assets/Scripts/ViewModels/AddImportFolderModel.cs`, `Assets/Scripts/Util/Commands/NotifyTask.cs`, `Assets/Scripts/Services/PreviewImageStore.cs`, `Assets/Scripts/Services/Library.cs` +- Impact: Errors are silently suppressed; difficult to diagnose failures; users have no feedback when operations fail; logging systems can't track error patterns +- Fix approach: Implement proper exception logging in all catch blocks; use empty catch only for `OperationCanceledException`; add specific exception types where appropriate + +**Unsafe STL Parsing with Minimal Bounds Checking:** +- Issue: `TextualStl.cs` uses unsafe pointer arithmetic (`goto` statements, manual pointer advancement) with limited validation; potential for buffer overruns on malformed files +- Files: `Assets/Scripts/Util/Stl/TextualStl.cs` (lines 27-196, especially 172-181 whitespace seeking, 123-127 exponent parsing) +- Impact: Malformed ASCII STL files could crash application or exhibit undefined behavior; security concern if processing untrusted files; IL2CPP AOT compilation may have different unsafe behavior than editor +- Fix approach: Add explicit length checks before pointer advancement; implement state machine parser instead of goto-based approach; validate file structure before parsing; add try-catch around pointer operations + +**Binary STL Facet Count Validation Gap:** +- Issue: `BinaryStl.FromBytes()` validates facet count against calculated count but uses `Math.Min()` to proceed regardless; silently truncates if counts mismatch +- Files: `Assets/Scripts/Util/Stl/BinaryStl.cs` (lines 60-66) +- Impact: Incomplete models imported without user awareness; data loss occurs silently; only warning logged; users may export or print incomplete geometry +- Fix approach: Throw exception when facet counts don't match; log full error details; provide user option to skip corrupted files during import; validate before mesh generation + +## Known Bugs + +**FileSystemWatcher Crash on Shutdown:** +- Symptoms: Application crashes when exiting while import folders are being watched +- Files: `Assets/Scripts/Services/ImportFolder.cs` (line 48 shows commented-out watcher code) +- Trigger: Add import folder, configure to watch subdirectories, exit application; file watcher events fire during shutdown +- Workaround: Watcher implementation disabled; folder watching must be manually triggered via "Rescan" button +- Status: Related to issue #4 (commit 58453be); watcher completely disabled in current implementation + +**Malformed ASCII STL with Whitespace in Solid Name:** +- Symptoms: Import fails for ASCII STL files with multiple whitespace characters in solid name declaration +- Files: `Assets/Scripts/Util/Stl/TextualStl.cs` (line 174 reads solid name until line feed) +- Trigger: ASCII STL with `solid My Model Name` (multiple spaces) +- Workaround: Rename file to have single spaces in solid name +- Status: Fixed in commit 8d75295; now properly handles whitespace + +**Empty STL Files Cause Crashes:** +- Symptoms: Application crashes when importing empty or minimal STL files +- Files: `Assets/Scripts/Services/ImportFolder.cs` (line 83 - filters by size > 112 bytes) +- Trigger: Try to import STL file smaller than 112 bytes +- Workaround: Filter prevents import at filesystem level +- Status: Fixed in commit f633551; minimum file size check implemented + +## Security Considerations + +**Untrusted File Input - STL Parsing:** +- Risk: Malicious or malformed STL files could trigger undefined behavior in unsafe code paths or denial of service +- Files: `Assets/Scripts/Util/Stl/TextualStl.cs`, `Assets/Scripts/Util/Stl/BinaryStl.cs`, `Assets/Scripts/Util/Stl/StlImporter.cs` +- Current mitigation: Basic file size checks (112 bytes minimum); binary format validation checks; textual format relies on robust pointer arithmetic +- Recommendations: Add comprehensive input validation; implement file size limits per import folder config; add timeout for parsing operations; consider sandboxing STL import to separate process; log suspicious file characteristics + +**Path Traversal in Import Folders:** +- Risk: Recursive subdirectory scanning could follow symlinks; malicious folder structures could cause excessive traversal +- Files: `Assets/Scripts/Services/ImportFolder.cs` (line 82 uses `Directory.GetFiles` with recursive option); `Assets/Scripts/Util/FileSystem/FolderFileSystem.cs` (line 32) +- Current mitigation: User must explicitly grant folder access; no symlink handling specified +- Recommendations: Add symlink detection and rejection; implement maximum recursion depth; add traversal timeout; log path traversal attempts + +**Unencrypted Config Storage:** +- Risk: Configuration files stored as uncompressed JSON in plaintext; API keys or sensitive paths could be exposed +- Files: `Assets/Scripts/Services/ConfigStore.cs` (lines 31-100) +- Current mitigation: Files stored in application data directory with OS permissions; old format supports zip compression +- Recommendations: Implement encryption for sensitive config values; use encrypted zip for all config storage; document which config fields contain sensitive data; add migration for existing unencrypted configs + +**Exception Details in Logs:** +- Risk: Full exception stack traces logged to files could expose internal implementation details +- Files: `Assets/Scripts/Util/Logging/` (all logging calls) +- Current mitigation: Debug logging level filters applied in some contexts +- Recommendations: Sanitize exception messages before logging user data; implement log file rotation and cleanup; restrict log access to user accounts + +## Performance Bottlenecks + +**Synchronous File I/O in Preview Generation:** +- Problem: `FolderFileSystem.GetFiles()` blocks thread during recursive directory traversal for large import folders +- Files: `Assets/Scripts/Util/FileSystem/FolderFileSystem.cs` (lines 27-52); `Assets/Scripts/Services/ImportFolder.cs` (lines 77-107) +- Cause: `Directory.GetFiles()` is synchronous; runs in `Task.Run()` but still blocks; recursive option traverses entire tree before returning +- Improvement path: Implement incremental directory traversal; return results as they're discovered; add cancellation support for long scans; consider using FindFirstFile/FindNextFile on Windows for better performance on large directories + +**Preview Image Texture Allocation Serialization:** +- Problem: Queue-based slot allocation for texture loading (`ItemViewUpdateQueue.ConsumeSlot()`) serializes all texture uploads +- Files: `Assets/Scripts/Views/ItemViewBase.cs` (line 78); likely `Assets/Scripts/Views/ItemViewUpdateQueue.cs` +- Cause: Unity texture operations must happen on main thread; simple slot-based throttling prevents parallel loads +- Improvement path: Batch texture uploads; use texture streaming API; implement priority queue for visible items; profile main thread impact of texture operations + +**Library Lock Contention on Massive Collections:** +- Problem: Single `ReaderWriterLockSlim` protects all library data; tag filtering, metadata updates, and preview streams compete for write lock +- Files: `Assets/Scripts/Services/Library.cs` (line 27) +- Cause: All read/write operations serialize through single lock; filtering (`GetItemPreviewMetadata()` line 61) requires read lock during initialization +- Improvement path: Use lock-free data structures for tag manager; implement fine-grained locking per preview stream; profile lock contention with large libraries (10k+ items) + +**Mesh Import Redundancy:** +- Problem: Mesh imported twice per item: once for preview generation with `centerVertices: true` and once for 3D view with `centerVertices: false` +- Files: `Assets/Scripts/Services/Library.cs` (lines 79-81, 114-116); `Assets/Scripts/Util/Stl/StlImporter.cs` +- Cause: Preview and 3D view have different vertex centering requirements; no mesh caching between operations +- Improvement path: Cache transformed meshes; implement lazy 3D mesh generation; defer centering until render time; combine into single import pass + +## Fragile Areas + +**Reflection-Based JSON Serialization with IL2CPP:** +- Files: `Assets/Scripts/Services/ConfigStore.cs`, `Assets/Scripts/Config/` (all config classes) +- Why fragile: Newtonsoft.Json uses reflection to serialize/deserialize config objects; IL2CPP AOT compilation may strip types not explicitly referenced in code +- Safe modification: Add `[Preserve]` attributes to all config classes; explicit serialization hints in JSON settings; test all config paths in IL2CPP build (commit e28f448 references previous IL2CPP stripping fix) +- Test coverage: `Assets/Tests/` has no config serialization tests + +**Message-Driven UI Updates:** +- Files: `Assets/Scripts/Util/Messaging/MessageAggregator.cs`, `Assets/Scripts/Views/ViewInitializer.cs` (180+ message subscriptions) +- Why fragile: ViewInitializer wires 30+ ViewModels and Views with message routing; no centralized registry; difficult to track message flow; message handler errors could cascade +- Safe modification: Add message tracing/logging; implement circuit breaker for handler failures; validate all message subscribers are properly disposed on shutdown +- Test coverage: No integration tests for message routing paths + +**PreviewList Filtered View Management:** +- Files: `Assets/Scripts/Services/PreviewList.cs`, `Assets/Scripts/Services/Library.cs` (lines 57-65) +- Why fragile: `PreviewList` holds weak reference through closure (`list => _lock.Write(() => _previewStreams.Remove(list))`); GC behavior affects view validity +- Safe modification: Implement explicit disposal pattern; validate list still exists before cleanup; add reference counting for filtered views +- Test coverage: No tests for filtered view lifecycle; garbage collection timing assumptions untested + +**Cross-Platform File Path Handling:** +- Files: `Assets/Scripts/Services/ImportFolder.cs`, `Assets/Scripts/Util/FileSystem/FolderFileSystem.cs` +- Why fragile: Path.Combine and Path.DirectorySeparatorChar used inconsistently; hardcoded forward slashes in some config paths; symlink behavior platform-specific +- Safe modification: Audit all path operations; use Path.GetFullPath for normalization; test on Windows/Mac/Linux with relative paths, symlinks, and unicode filenames +- Test coverage: No platform-specific path tests + +## Scaling Limits + +**Library Memory Usage with Large Collections:** +- Current capacity: ~10,000 items tested (10k preview models with metadata) +- Limit: Each `ItemPreviewModel` stores: texture data (cached as Texture2D), geometry info, all source file references, and tag strings; multiply by large libraries and memory grows rapidly +- Scaling path: Implement streaming library view (load metadata only, lazy texture loading); use object pooling for frequently created models; implement cache eviction for previews outside viewport + +**Single-Threaded Preview Image Loading:** +- Current capacity: ~100 previews visible in viewport; texture uploads serialized +- Limit: Main thread blocked by texture operations during rapid scrolling; frame rate degrades; users see white/placeholder images +- Scaling path: Implement multi-threaded texture streaming; batch upload operations; prioritize visible items; implement LOD texture versions for background loads + +**Filter Operation Complexity:** +- Current capacity: ~5,000 items with ~20 filters applied +- Limit: `TagManager.Filter()` called per filtered view; linear scan through all items for each filter; no index caching +- Scaling path: Implement inverted tag index; cache filter results; implement incremental filtering + +## Dependencies at Risk + +**Newtonsoft.Json (JSON.NET):** +- Risk: Major version dependency in Unity (not official json library); requires IL2CPP metadata preservation; serialization behavior differs between Mono and IL2CPP +- Impact: Config loading fails silently; user settings lost on IL2CPP build; version updates may break serialization +- Migration plan: Evaluate JsonUtility (built-in) or Unity.Serialization; implement compatibility layer for config migration; extensive IL2CPP testing before dependency updates + +**DOTween Animation Library:** +- Risk: Tween library requires UI updates on main thread; unfinished tweens on shutdown could cause memory leaks +- Impact: Memory leak if tweens not properly disposed; animation glitches with rapid state changes +- Migration plan: Profile tween completion on shutdown; implement cleanup in ViewInitializer.OnDestroy(); consider built-in Coroutine animations for critical paths + +**UnityEngine Reflection API:** +- Risk: Reliance on reflection for dynamic type loading in generic config system; IL2CPP may strip types +- Impact: Config loading fails for stripped types; metadata preservation required +- Migration plan: Explicit type registration; pre-generate type tables; test IL2CPP build thoroughly + +## Missing Critical Features + +**File Watching/Auto-Update Disabled:** +- Problem: FileSystemWatcher implementation commented out; import folders don't auto-update when files added/removed externally +- Blocks: Can't support live library updates; users must manually rescan; external file managers don't integrate +- Impact: Major usability gap for workflow with external file management + +**Export Functionality:** +- Problem: Roadmap lists export for printing as v1.0 feature; no export implementation present +- Blocks: Can't generate print-ready files; users must export manually via 3D view (slow); no batch export +- Impact: Primary use case (3D printing) incomplete + +**Undo/Redo System:** +- Problem: Non-destructive editing (rotation) supported but no undo; users can't correct mistakes +- Blocks: Risk of accidental data modifications; edits are permanent +- Impact: Users discouraged from using rotation feature + +## Test Coverage Gaps + +**STL File Format Parsing:** +- What's not tested: Malformed files, edge cases in binary format detection, scientific notation in ASCII STL, exponent parsing in `TextualStl.cs` (lines 118-127) +- Files: `Assets/Scripts/Util/Stl/TextualStl.cs`, `Assets/Scripts/Util/Stl/BinaryStl.cs`, `Assets/Scripts/Tests/` (no STL format tests) +- Risk: Malformed files cause silent failure or crash; format detection may misclassify files; parsing errors undetected +- Priority: High - core functionality + +**Library Synchronization with Threading:** +- What's not tested: Lock contention under concurrent access, deadlock scenarios, cancel token handling during lock operations +- Files: `Assets/Scripts/Services/Library.cs` (multiple async operations with locks) +- Risk: Race conditions in multi-threaded import; potential deadlocks; cancellation doesn't propagate properly +- Priority: High - affects data integrity + +**Configuration Persistence Edge Cases:** +- What's not tested: Config loading failure scenarios, missing config files, corrupted zip files, permission errors +- Files: `Assets/Scripts/Services/ConfigStore.cs` (simple try-catch returning new T()) +- Risk: User settings lost silently; config errors not surfaced to user +- Priority: Medium - affects user experience + +**View Lifecycle and Memory Leaks:** +- What's not tested: Dialog open/close sequences, texture cleanup on view destruction, event subscription cleanup +- Files: `Assets/Scripts/Views/` (many ViewBase implementations) +- Risk: Memory leaks from ununsubscribed events; textures not released; long-running sessions degrade +- Priority: Medium - affects stability + +**Platform-Specific File System Behavior:** +- What's not tested: Symlink handling, case sensitivity on Linux, unicode filenames, path length limits, network drives +- Files: `Assets/Scripts/Util/FileSystem/FolderFileSystem.cs`, `Assets/Scripts/Services/ImportFolder.cs` +- Risk: Platform-specific crashes; features work on dev machine but fail on user systems +- Priority: Medium - affects cross-platform reliability + +--- + +*Concerns audit: 2026-01-23* diff --git a/.planning/codebase/CONVENTIONS.md b/.planning/codebase/CONVENTIONS.md new file mode 100644 index 0000000..4e162eb --- /dev/null +++ b/.planning/codebase/CONVENTIONS.md @@ -0,0 +1,200 @@ +# Coding Conventions + +**Analysis Date:** 2026-01-23 + +## Naming Patterns + +**Files:** +- PascalCase for all C# files: `ConfigStore.cs`, `ItemsModel.cs`, `ILibrary.cs` +- Test files use `*Tests.cs` suffix: `TagManagerTests.cs`, `SavedSearchesTests.cs` +- Interface files use `I` prefix: `IConfigStore.cs`, `ILibrary.cs`, `IMessageRelay.cs` + +**Classes:** +- PascalCase for class names: `ApplicationModel`, `Library`, `DelegateCommand` +- Internal keyword used for non-public classes: `internal class ApplicationModel` +- Nested private classes use PascalCase: `ActionDisposable`, `Node` + +**Interfaces:** +- PascalCase with `I` prefix: `IConfigStore`, `ILibrary`, `IMessageRelay` +- Generic parameters in interface names: `ICommand`, `IBindableProperty` + +**Properties:** +- PascalCase for public properties: `Value`, `CanExecute`, `Items`, `Tags` +- Auto-properties commonly used: `public ICommand ShowAppSettingsCommand { get; }` +- Read-only properties use `{ get; }` or `{ get; private set; }` + +**Methods:** +- PascalCase for all methods: `GetRecommendations()`, `ImportBatched()`, `TryGetLocalPath()` +- Async methods use `Async` suffix: `InitializeAsync()`, `LoadAsyncOrDefault()`, `GetMeshAsync()` +- Private methods prefixed with lowercase: `private void UpdateTags()`, `private async Task ImportFile()` +- Boolean methods prefix with `Is`, `Can`, `Try`, `Should`: `CanExecute()`, `TryGetLocalPath()`, `TryGetFileSource()` + +**Local Variables:** +- camelCase for all local variables: `var trie = new ArrayTrie()`, `var previewList = new PreviewList()` +- Underscore prefix for private fields: `private readonly ReaderWriterLockSlim _lock` +- Single letter variables for loops: `for (var i = 0; i < items.Count; i++)` + +**Constants/Static Fields:** +- UPPER_SNAKE_CASE for constants: seen in enum values (`Add`, `Remove`) +- Logger instance follows pattern: `private static readonly ILogger Logger = UnityLogger.Instance` + +**Type Parameters:** +- Single letter capitalized: ``, ``, `` +- Fully spelled out when more complex: `where T : class, new()` + +**Enums:** +- PascalCase for enum names: `TagAction`, `RecommendationMode` +- PascalCase for enum values: `Add`, `Remove`, `Search`, `Tagging` + +## Code Style + +**Formatting:** +- No explicit formatter configuration detected (likely using Visual Studio defaults) +- Consistent 4-space indentation +- Brace style: opening braces on same line (Allman-adjacent) + ```csharp + public void ToggleAll() + { + using (NotifyMassSelection()) + { + var allSelected = Items.All(item => item.Selected); + foreach (var previewModel in Items) + { + previewModel.Selected.Value = !allSelected; + } + } + } + ``` +- Single-line conditionals allowed when simple: `if (TransformValue != null) { value = TransformValue(value); }` + +**Linting:** +- ReSharper suppression attributes used: `[SuppressMessage("ReSharper", "InconsistentlySynchronizedField")]` +- ReSharper directives for suppressing warnings: `// ReSharper disable once UnusedTypeParameter` +- JetBrains.Annotations used for null safety: `[NotNull]`, `[StringFormatMethod("message")]` + +**Null Checking:** +- Null-coalescing operators: `_canExecuteFunc?.Invoke() ?? true` +- Null-conditional operators: `ValueChanging?.Invoke(_value)` +- Explicit null checks with throw: `_library = library ?? throw new ArgumentNullException(nameof(library))` +- Method guards at entry point: `if(relay is null) throw new ArgumentNullException(nameof(relay))` + +## Import Organization + +**Order:** +1. System namespaces: `using System;`, `using System.Collections.Generic;`, `using System.Threading.Tasks;` +2. Third-party/external: `using JetBrains.Annotations;`, `using Moq;`, `using NUnit.Framework;` +3. Unity engine: `using UnityEngine;` +4. Project namespaces: `using StlVault.Services;`, `using StlVault.Util.Messaging;` + +**Aliasing:** +- Rare, but seen for disambiguation: `using NotifyCollectionChangedAction = System.Collections.Specialized.NotifyCollectionChangedAction;` +- No widespread path aliases detected + +**Namespace Nesting:** +- Follows folder structure: `Assets/Scripts/ViewModels/` → `namespace StlVault.ViewModels` +- Deep nesting used: `StlVault.Util.Collections`, `StlVault.Util.Messaging`, `StlVault.Util.Logging` + +## Error Handling + +**Exception Patterns:** +- Try-catch in service methods: `catch (Exception ex) { Logger.Error(ex, "Error while {0}", context); }` +- Try-catch with rethrow: seen in `ConfigStore.LoadAsyncOrDefault()` with file operations +- Generic catch for file operations, then null return: `catch { localPath = null; return false; }` +- Null checks as guards before processing: `if (addedFiles?.Any() != true) return;` + +**Validation:** +- Argument null checks at method entry: `_relay = relay ?? throw new ArgumentNullException(nameof(relay))` +- Precondition assertions: `Debug.Assert(CanExecute(parameter));` +- Early returns for invalid states: `if (message.SearchTags.SequenceEqual(_currentSearchTags)) return;` + +**Logging Errors:** +- `Logger.Error(ex, "Error while rotating {0}", previewModel.Name);` +- Format string with context information +- Trace/Info for non-critical: `Logger.Trace("Reading file {0} - Took {0}ms.", fileName);` + +## Comments + +**When to Comment:** +- Minimal inline comments observed +- Summary comments used for public APIs and method signatures +- No TODO/FIXME/HACK comments found in source code (indicates clean codebase) + +**XML Documentation:** +- `/// ` tags used for method documentation: seen in `Library.InitializeAsync()` +- Example: + ```csharp + /// + /// Called during initial creation of scene graph. + /// Must finish before others can access it. + /// + ``` + +**Suppression Comments:** +- ReSharper directives: `// ReSharper disable once UnusedTypeParameter` +- SuppressMessage attributes: `[SuppressMessage("ReSharper", "InconsistentlySynchronizedField")]` + +## Function Design + +**Size Guideline:** +- Methods range from 5-40 lines typically +- Longer methods (50+ lines) handle batching/iteration logic: `ImportBatched()`, `Library.RebuildPreview()` +- Private helper methods kept small and focused + +**Parameters:** +- Decorated with `[NotNull]` attribute for reference types +- Generic constraints used: `where T : class, new()` +- Default parameters rare, but used in async: `LoadAsyncOrDefault()` +- Out parameters for multiple returns: `TryGetLocalPath(string filePath, out string localPath)` + +**Return Values:** +- Boolean returns with out parameter for result: `TryGetFileSource(out IFileSource source, out string filePath)` +- Task-based async returns: `Task`, `Task` +- Nullable returns: `return null;` when operation fails +- Tuples for multiple return values: `return (hash, geoInfo, resolution);` + +**Async/Await:** +- Task-based async throughout: `async Task`, `async Task` +- Result unwrapping: `action().GetAwaiter().GetResult()` in test utilities +- Proper await usage: `await task;` not `task.Wait()` +- CancellationToken support: `CancellationToken token` as final parameter + +## Module Design + +**Exports:** +- Internal types not exposed: `internal class Library`, `internal interface ILibrary` +- Public utility classes for general use: `public static class ChunkedOperations`, `public class Disposable` +- Public base classes: `public class ObservableCollection` + +**Namespace Structure:** +- Clear separation by concern: ViewModels, Services, Util, Config, Messages +- Util subnamespaces: Collections, Messaging, Logging, Commands, Tags, FileSystem, Stl, Unity +- Test namespace separate: `StlVault.Tests` + +**Barrel Files:** +- Not extensively used; direct imports from specific files preferred +- No index.ts / wrapper pattern observed + +**Dependency Injection:** +- Constructor injection pattern used throughout +- `[NotNull]` attributes enforce non-null dependencies +- Factory pattern for complex object creation: `IImportFolderFactory` + +**Observable Pattern:** +- Custom observable collections: `ObservableList`, `ObservableSet` +- Event-based notification: `PropertyChanged`, `CollectionChanged` +- Bindable properties with change events: `IBindableProperty` + +## Testing Conventions + +**Test Method Naming:** +- Descriptive names: `ShouldFindStoredWord()`, `ShouldIncrementOccurrencesOnMultipleInserts()` +- Format: `Should[Expected]` or `Should[Action]When[Condition]` + +**Setup Patterns:** +- Initialize method creates test fixtures: `private async Task InitializeModel()` +- Mock creation in setup: `_store = new Mock();` +- Test data building: `new SavedSearchConfig {Alias = "Test", Tags = {"Test1", "Test2"}}` + +--- + +*Convention analysis: 2026-01-23* diff --git a/.planning/codebase/INTEGRATIONS.md b/.planning/codebase/INTEGRATIONS.md new file mode 100644 index 0000000..958dc7c --- /dev/null +++ b/.planning/codebase/INTEGRATIONS.md @@ -0,0 +1,154 @@ +# External Integrations + +**Analysis Date:** 2026-01-23 + +## APIs & External Services + +**Update Checking:** +- stlvault.com - Version release information endpoint + - Client: HttpClient (System.Net.Http) + - Endpoint: `http://stlvault.com/release.json` + - Response Format: JSON containing version info and release notes + - Implementation: `Assets/Scripts/Services/UpdateChecker.cs` + - Authentication: None (public endpoint) + +## Data Storage + +**Local File System Only:** +- Configuration stored as JSON files on disk +- Location: Application data directory with `Config/` subdirectory +- Compression: Optional ZIP compression for large metadata files +- Implementation: `Assets/Scripts/Services/ConfigStore.cs` + +**Database:** +- Not applicable - uses local JSON-based storage +- No remote database integration +- No SQL dependencies + +**File Storage:** +- Local filesystem only for 3D model files (.stl) +- No cloud storage integration +- No network file system support + +**Caching:** +- In-memory caching only +- Preview images stored locally as texture files +- No distributed cache (Redis, Memcached, etc.) + +## Authentication & Identity + +**Auth Provider:** +- Not applicable - no user authentication system +- Application is single-user, file-based +- No remote authentication services +- No cloud account integration + +## Monitoring & Observability + +**Error Tracking:** +- Not applicable - no remote error tracking (no Sentry, Rollbar, etc.) +- Local logging only + +**Logs:** +- Unity's built-in logging system (Debug.Log, UnityLogger) +- Implementation: `Assets/Scripts/Util/Logging/` +- Log output: Unity console and application logs +- No centralized log aggregation + +**Performance Monitoring:** +- Built-in profiling: Unity's Profiler window +- No remote performance monitoring +- Manual timing instrumentation via extension methods + +## CI/CD & Deployment + +**Hosting:** +- Desktop application - Windows, macOS, Linux +- Standalone executable distribution +- GitHub Releases for distribution +- No cloud deployment + +**CI Pipeline:** +- GitHub Actions for automated testing +- Configuration: `.github/` directory +- Test runner: Unity Test Framework via GitHub Actions +- Build targets: Windows, macOS, Linux +- No containerization (Docker) + +**Build Process:** +- IL2CPP for release builds (native compilation) +- Mono for editor/development builds +- Platform-specific: + - Windows: BuildTarget.StandaloneWindows + - macOS: BuildTarget.StandaloneOSX (requires code signing) + - Linux: BuildTarget.StandaloneLinux64 + +## Environment Configuration + +**Required Environment Variables:** +- None detected - application uses hardcoded URLs and local filesystem paths +- Version info from `Application.version` (Unity player settings) + +**Data Paths:** +- Application data: Platform-dependent (typically `%APPDATA%\StlVault` on Windows, `~/Library/Application Support/StlVault` on macOS) +- Working directory: User-selected import folders for 3D model files +- Configuration persisted to local disk + +**Secrets Location:** +- Not applicable - no API keys, tokens, or secrets in use +- All URLs are hardcoded as non-sensitive public endpoints + +## File Dialogs & System Integration + +**Native File Browser:** +- StandaloneFileBrowser plugin for cross-platform file selection +- Implementation: `Assets/Plugins/StandaloneFileBrowser/` +- Platform implementations: + - Windows: `StandaloneFileBrowserWindows.cs` (native Windows API) + - macOS: `StandaloneFileBrowserMac.cs` (native macOS API) + - Linux: `StandaloneFileBrowserLinux.cs` (Zenity integration) +- Use case: Selecting import folders and save locations + +**Process Management:** +- Windows native process creation via `kernel32.dll` DllImport +- Location: `Assets/Scripts/ViewModels/NativeMethods.cs` +- Used for: Opening external applications or URLs + +## Webhooks & Callbacks + +**Incoming:** +- None - application has no server component +- No webhook endpoints + +**Outgoing:** +- Update check sends GET request to stlvault.com +- No callback registration with external services + +## Data Formats + +**Input Formats Supported:** +- STL (Stereolithography) - Binary and ASCII variants +- Implementations: `Assets/Scripts/Util/Stl/BinaryStl.cs`, `Assets/Scripts/Util/Stl/TextualStl.cs` +- File detection: Format detection via file header analysis + +**Output Formats:** +- JSON - Configuration and metadata serialization +- PNG - Preview image thumbnails (generated via Unity rendering) +- ZIP - Optional compression for configuration archives + +**Serialization:** +- JSON serialization via Newtonsoft.Json +- Default behavior: Ignores null/default values +- Schema: Custom C# classes with [JsonProperty] attributes + +## Version Management + +**Current Version:** +- Retrieved from: `Application.version` (Unity player settings) +- Format: Semantic versioning (e.g., "1.0.0") +- Update Channel File: `http://stlvault.com/release.json` +- Version Tracking: Local file in application data directory + +--- + +*Integration audit: 2026-01-23* diff --git a/.planning/codebase/STACK.md b/.planning/codebase/STACK.md new file mode 100644 index 0000000..24e1e54 --- /dev/null +++ b/.planning/codebase/STACK.md @@ -0,0 +1,133 @@ +# Technology Stack + +**Analysis Date:** 2026-01-23 + +## Languages + +**Primary:** +- C# - Unity scripting language for all application logic + +**Markup/Config:** +- YAML - Unity configuration files +- JSON - Data serialization for configuration and metadata + +## Runtime + +**Environment:** +- Unity 2019.3.2f1 - Game engine and runtime +- .NET Framework / Mono - C# runtime (editor); IL2CPP for release builds + +**Platform Targets:** +- Windows (standalone) +- macOS (standalone) +- Linux (standalone) + +## Frameworks + +**Core Framework:** +- Unity Engine 2019.3.2f1 - 3D graphics engine and application framework +- Universal Render Pipeline (URP) 7.1.8 - Graphics rendering + +**UI Framework:** +- Unity UI (UGUI) 1.0.0 - Canvas-based UI system +- TextMesh Pro 2.0.1 - Advanced text and font rendering +- Timeline 1.2.10 - Animation sequencing + +**Testing:** +- Unity Test Framework (UTF) 1.1.11 - Unit testing +- Moq - Mocking library for unit tests + +**Animation:** +- DOTween - Tweening and animation library (tween-based property animation) + +**IDE Support:** +- Rider 1.1.4 - JetBrains IDE integration + +## Key Dependencies + +**Critical:** +- Newtonsoft.Json (Json.NET) - JSON serialization and deserialization + - Location: `Assets/Plugins/JsonDotNet/` + - Used in: Configuration loading/saving, Update checker +- Castle.Core - DI/AOP framework support + - Location: `Assets/Plugins/Castle/` + - Version: Included as runtime dependency + +**Infrastructure:** +- System.IO.Compression - ZIP file support for compressed metadata storage +- System.Net.Http - HTTP client for version checking +- System.Security.Cryptography - SHA hash computation for file integrity +- System.Threading (ReaderWriterLockSlim) - Thread synchronization +- System.Collections.Generic.IEnumerable - Collection handling + +**File Operations:** +- StandaloneFileBrowser - Cross-platform native file dialog + - Location: `Assets/Plugins/StandaloneFileBrowser/` + - Supports: Windows, macOS, Linux + +**3D Visualization:** +- RuntimeSceneGizmo - Scene manipulation gizmos + - Location: `Assets/Plugins/RuntimeSceneGizmo/` + +**Utilities:** +- JetBrains.Annotations - Code analysis attributes +- System.Buffers, System.Memory - Memory optimization + +## Configuration + +**Project Configuration:** +- Located: `ProjectSettings/ProjectSettings.asset` +- Company: StlVault +- Product: StlVault +- Default Resolution: 1500x900 (windowed) +- Default Orientation: Landscape + +**Build Configuration:** +- IL2CPP scripting backend for release builds +- Mono for editor development +- AoT compilation constraints (no Reflection.Emit) +- IL-Stripper enabled for optimization + +**Serialization Settings:** +- JSON serialization uses Newtonsoft.Json defaults with `DefaultValueHandling.Ignore` +- Configuration files location: `Config/` subdirectory in application data +- Compressed storage available for large metadata files (ZIP format) + +## Package Manager + +**Unity Package Manager:** +- Manifest location: `Packages/manifest.json` +- Dependencies specified in UPM format +- Installed packages include Unity standard modules and third-party assets + +**Dependencies tracked:** +- com.unity.2d.sprite 1.0.0 +- com.unity.render-pipelines.universal 7.1.8 +- com.unity.test-framework 1.1.11 +- com.unity.textmeshpro 2.0.1 +- com.unity.timeline 1.2.10 +- Core Unity modules (UI, physics, animation, etc.) + +## Platform Requirements + +**Development:** +- Unity Editor 2019.3.2f1 +- IL2CPP Support for target platform +- Supported IDEs: Rider, Visual Studio +- Requires Git LFS for asset management +- C# 7.3+ language features + +**Production:** +- Windows (standalone executable) +- macOS (app bundle, requires code signing) +- Linux (standalone executable) +- Native file dialog support via system APIs + +**Compilation Constraints:** +- No runtime code generation (IL2CPP incompatible) +- Generic type specialization at compile time +- Reflection-based invocation restricted (IL-Stripper may remove unreferenced methods) + +--- + +*Stack analysis: 2026-01-23* diff --git a/.planning/codebase/STRUCTURE.md b/.planning/codebase/STRUCTURE.md new file mode 100644 index 0000000..00ed2a0 --- /dev/null +++ b/.planning/codebase/STRUCTURE.md @@ -0,0 +1,239 @@ +# Codebase Structure + +**Analysis Date:** 2026-01-23 + +## Directory Layout + +``` +StlVault/ +├── Assets/ +│ ├── Scripts/ # All C# source code (207 files) +│ │ ├── Config/ # Configuration data structures +│ │ ├── Editor/ # Editor-only utilities +│ │ ├── Messages/ # Pub-sub message definitions +│ │ ├── Services/ # Business logic and data management +│ │ ├── Util/ # Cross-cutting concerns and utilities +│ │ │ ├── Collections/ # Observable collection implementations +│ │ │ ├── Commands/ # Command pattern implementations +│ │ │ ├── FileSystem/ # File system abstraction +│ │ │ ├── Logging/ # Logging framework +│ │ │ ├── Messaging/ # Message aggregator +│ │ │ ├── Stl/ # STL file parsing and export +│ │ │ ├── Tags/ # Tag filtering and search +│ │ │ └── Unity/ # Unity-specific utilities +│ │ ├── ViewModels/ # Presentation logic (40+ models) +│ │ ├── Views/ # UI views and dialogs (50+ views) +│ │ └── Tests/ # Unit and integration tests +│ ├── Plugins/ # Third-party plugins +│ │ ├── StandaloneFileBrowser/ # Cross-platform file picker +│ │ └── RuntimeSceneGizmo/ # 3D manipulation gizmos +│ ├── Demigiant/ # DOTween animation library +│ └── Resources/ # Assets (textures, prefabs) +├── ProjectSettings/ # Unity project configuration +├── Packages/ # Package dependencies +├── README.md # Project overview +├── DEVELOPMENT.md # Development guide +└── .planning/ # Planning documents + └── codebase/ # Codebase analysis docs +``` + +## Directory Purposes + +**Assets/Scripts/Config:** +- Purpose: Define data structures for application settings, user preferences, and file metadata +- Contains: Configuration classes that are serialized to JSON +- Key files: `ApplicationSettings.cs`, `CollectionConfig.cs`, `ImportFolderConfig.cs`, `SavedSearchConfig.cs` + +**Assets/Scripts/Editor:** +- Purpose: Editor-only utilities that run in Unity Editor, not in built applications +- Contains: Development shortcuts and profiling tools +- Key files: `SwitchShortcutProfileOnPlay.cs` + +**Assets/Scripts/Messages:** +- Purpose: Define strongly-typed messages for loosely-coupled communication +- Contains: Empty message classes used as publish-subscribe contracts +- Key files: `SearchChangedMessage.cs`, `SelectionChangedMessage.cs`, `MassSelectionStartingMessage.cs` + +**Assets/Scripts/Services:** +- Purpose: Core business logic, data persistence, and external system integration +- Contains: Central service classes and interfaces +- Key files: + - `Library.cs` - Main service facade for all data operations + - `ConfigStore.cs` - JSON persistence with compression + - `ImportFolder.cs` - File source implementation + - `PreviewImageStore.cs` - Preview image caching + +**Assets/Scripts/Util:** +- Purpose: Reusable utilities and cross-cutting concerns +- Contains: + - Collections: `ObservableList`, `ObservableSet`, change tracking wrappers + - Commands: ICommand implementation with async support + - FileSystem: IFileSystem abstraction over OS file operations + - Logging: ILogger interface with UnityLogger implementation + - Messaging: MessageAggregator pub-sub implementation + - Stl: STL file format parsing and export + - Tags: Tag-based filtering and search with wildcards + - Unity: UI component extensions, custom layout groups, camera utilities + +**Assets/Scripts/ViewModels:** +- Purpose: Presentation logic binding user input to business logic +- Contains: 40+ model classes for different screens and dialogs +- Naming pattern: `{ScreenName}Model.cs` (e.g., `SearchModel`, `ItemsModel`) +- Key models: + - `ApplicationModel` - Top-level app commands + - `SearchModel` - Search/filter logic + - `ItemsModel` - Item collection and selection + - `ItemPreviewModel` - Individual item data + - `ImportFoldersModel` - Import folder management + - `CollectionsModel` - Collection management + - Dialog models: `AddCollectionModel`, `AddImportFolderModel`, etc. + +**Assets/Scripts/Views:** +- Purpose: UI rendering and event handling +- Contains: 50+ view classes inheriting from `ViewBase` +- Naming pattern: `{ScreenName}View.cs` or `{ScreenName}Dialog.cs` +- Key views: + - `ApplicationView` - Top-level commands + - `LibraryView` - Item grid rendering (via `Util/Unity/LibraryView.cs`) + - `ItemView` - Individual item preview card + - Dialog views: `AddCollectionDialog`, `ApplicationSettingsDialog`, etc. + +**Assets/Scripts/Tests:** +- Purpose: Unit and integration tests +- Contains: Test fixtures and test implementations +- Key files: `LibraryTests.cs`, `TagManagerTests.cs`, `SavedSearchesTests.cs`, `ArrayTrieTests.cs` + +**Assets/Plugins:** +- Purpose: Third-party plugin integrations +- Contains: + - StandaloneFileBrowser: Cross-platform file picker (Windows, Mac, Linux) + - RuntimeSceneGizmo: 3D manipulation tools + +**Assets/Demigiant:** +- Purpose: DOTween animation library (external dependency) +- Contains: Animation framework for UI and object tweening + +## Key File Locations + +**Entry Points:** +- `Assets/Scripts/Views/ViewInitializer.cs`: Application startup and wiring (main entry point) +- `Assets/Scenes/Main.unity`: Unity scene containing initial ViewInitializer GameObject + +**Configuration:** +- `Assets/Scripts/Config/ApplicationSettings.cs`: Runtime application settings +- `Assets/Scripts/Config/CollectionsConfigFile.cs`: User-defined collections (file list) +- `Assets/Scripts/Config/ImportFoldersConfigFile.cs`: Import folder configurations +- `Assets/Scripts/Config/SavedSearchesConfigFile.cs`: Saved search definitions + +**Core Logic:** +- `Assets/Scripts/Services/Library.cs`: Central facade for all data operations +- `Assets/Scripts/Services/ConfigStore.cs`: JSON persistence layer +- `Assets/Scripts/Util/Messaging/MessageAggregator.cs`: Pub-sub message broker +- `Assets/Scripts/Util/Tags/TagManager.cs`: Tag-based filtering engine + +**Testing:** +- `Assets/Tests/`: All test files +- Test runners configured in ProjectSettings + +## Naming Conventions + +**Files:** +- PascalCase for class files: `ApplicationModel.cs`, `ImportFolder.cs` +- Matching class name to filename: `class Library` in `Library.cs` +- Interface prefix `I`: `ILibrary.cs`, `IFileSource.cs` +- Test suffix: `LibraryTests.cs`, `TagManagerTests.cs` + +**Directories:** +- PascalCase: `Config`, `Services`, `Views`, `ViewModels` +- Feature grouping: `Util/Collections/`, `Util/Commands/`, `Util/Tags/` +- Lowercase for Unity assets: `Plugins`, `Resources`, `Scenes` + +**Classes:** +- PascalCase: `ApplicationModel`, `SearchModel`, `ItemPreviewModel` +- Interface prefix `I`: `ILibrary`, `ICommand`, `IBindableProperty` +- Abstract base suffix `Base`: `ViewBase`, `FileSourceBase`, `DialogModelBase` +- Model suffix for ViewModels: `SearchModel`, `ItemsModel` +- View suffix for Views: `ApplicationView`, `ItemView` +- Factory suffix: `ImportFolderFactory`, `FilterFactory` + +**Methods:** +- PascalCase (C# convention): `GetItemPreviewMetadata()`, `AddTag()`, `RotateAsync()` +- Async suffix: `InitializeAsync()`, `GetFileBytesAsync()`, `RescanItemsAsync()` +- Event handler prefix: `OnQuitRequested()`, `TimerOnElapsed()` +- Property prefix for getters: `Get*()`, rarely `Is*()` + +**Properties:** +- PascalCase: `Library.Parallelism`, `ItemPreviewModel.Tags` +- Private fields prefixed with underscore: `_library`, `_relay`, `_configStore` +- Public properties without prefix: `public string Name { get; }` + +## Where to Add New Code + +**New Feature (e.g., Add Tag Recommendations):** +- Primary code: Create service in `Assets/Scripts/Services/` (e.g., `TagRecommender.cs`) +- ViewModel logic: Add model in `Assets/Scripts/ViewModels/` (e.g., `TagRecommendationModel.cs`) +- UI: Add view in `Assets/Scripts/Views/` (e.g., `TagRecommendationView.cs`) +- Messages: Add message type in `Assets/Scripts/Messages/` if cross-component communication needed +- Tests: Add tests in `Assets/Tests/` with `*Tests.cs` suffix + +**New Component/Module (e.g., Plugin for Custom File Format):** +- Implementation: Create in `Assets/Scripts/Services/` with `IFileSource` interface +- Config: Add config class in `Assets/Scripts/Config/` +- Factory: Extend `IImportFolderFactory` or create new factory +- Integration: Wire in `ViewInitializer.cs` Start() method + +**New Dialog:** +- ViewModel: Create `{DialogName}Model.cs` in `Assets/Scripts/ViewModels/` +- View: Create `{DialogName}Dialog.cs` in `Assets/Scripts/Views/` +- Message: Add `RequestShowDialogMessage.{DialogName}` in `Assets/Messages/` if using dialog request pattern +- Binding: Add binding call in `ViewInitializer.cs` BindViewModels() + +**Utilities:** +- Shared helpers: `Assets/Scripts/Util/{Category}/{UtilityName}.cs` +- Examples: `Assets/Scripts/Util/Tags/FilterFactory.cs`, `Assets/Scripts/Util/Commands/CommandExtensions.cs` +- Extensions: `Assets/Scripts/Util/{Category}/{TypeName}Extensions.cs` + +**New Filter/Search Feature:** +- Filter implementation: `Assets/Scripts/Util/Tags/` (e.g., `StartsWithWildcardFilter.cs`) +- Register in: `Assets/Scripts/Util/Tags/FilterFactory.cs` +- Use in: `TagManager.cs` for filtering logic + +## Special Directories + +**Assets/Scripts/Util/Unity:** +- Purpose: UI component utilities and extensions specific to Unity +- Generated: No +- Committed: Yes +- Key files: + - `LibraryView.cs`: Custom grid layout for item previews + - `VirtualGridLayoutGroup.cs`: Virtualized grid rendering for performance + - `WrapGroup.cs`: Horizontal layout wrapper + - `GuiCallbackQueue.cs`: Thread-safe UI update queueing from background threads + - `GuiThreadQueuedProperty.cs`: Observable property with UI thread marshaling + +**Assets/Tests:** +- Purpose: Unit and integration test execution +- Generated: No +- Committed: Yes +- Run via: Unity Test Framework in Editor or CI/CD pipeline +- Key test patterns: See TESTING.md + +**Assets/Resources:** +- Purpose: Pre-loaded assets needed at runtime +- Generated: No +- Committed: Yes (selective) +- Contents: Prefabs, icons, layouts + +**ProjectSettings:** +- Purpose: Unity engine configuration (editor settings, build settings, quality) +- Generated: Partially (Unity auto-updates some files) +- Committed: Yes (standard practice for shared projects) + +**Packages:** +- Purpose: Package manifest and dependencies +- Generated: Partially (lock files auto-generated) +- Committed: Yes (manifest.json for reproducible builds) + +--- + +*Structure analysis: 2026-01-23* diff --git a/.planning/codebase/TESTING.md b/.planning/codebase/TESTING.md new file mode 100644 index 0000000..84c0935 --- /dev/null +++ b/.planning/codebase/TESTING.md @@ -0,0 +1,328 @@ +# Testing Patterns + +**Analysis Date:** 2026-01-23 + +## Test Framework + +**Runner:** +- NUnit 3 (via `nunit.framework.dll`) +- Unity Test Framework integration +- Config: `Assets/Tests/Tests.asmdef` + +**Mocking Framework:** +- Moq for mock object creation +- Castle.Core for dynamic proxy generation (Moq dependency) + +**Assertion Library:** +- NUnit.Framework assertions: `Assert.AreEqual()`, `Assert.Contains()`, `Assert.True()`, `Assert.IsTrue()` +- Custom assertion helper: `Expect` class in `Assets/Tests/Expect.cs` + +**Run Commands:** +```bash +# Tests run through Unity Test Framework in Editor +# Automated via GitHub Actions (from git history) +``` + +## Test File Organization + +**Location:** +- Dedicated test directory: `Assets/Tests/` +- Tests are separate from source code (not co-located) +- Test assembly definition: `Tests.asmdef` references `Scripts` assembly + +**Naming:** +- `*Tests` suffix: `ArrayTrieTests.cs`, `TagManagerTests.cs`, `LibraryTests.cs`, `SavedSearchesTests.cs` +- Namespace matches pattern: `namespace StlVault.Tests` +- Internal test utilities: `TestUtils.cs`, `Expect.cs` + +**Structure:** +``` +Assets/ +├── Tests/ +│ ├── Tests.asmdef # Test assembly definition +│ ├── ArrayTrieTests.cs # Unit tests for ArrayTrie +│ ├── TagManagerTests.cs # Unit tests for TagManager +│ ├── SavedSearchesTests.cs # ViewModel tests +│ ├── LibraryTests.cs # Service tests (stubs only) +│ ├── TestUtils.cs # Shared test utilities +│ └── Expect.cs # Custom assertions +└── Scripts/ + └── [Source code] +``` + +## Test Structure + +**Test Method Anatomy:** + +```csharp +[Test] +public void ShouldFindStoredWord() +{ + // Arrange + var trie = new ArrayTrie(); + + // Act + trie.Insert("test"); + var results = trie.Find("te").Select(t => t.word).ToList(); + + // Assert + Assert.AreEqual(1, results.Count); + Assert.Contains("test", results); +} +``` + +**Patterns:** + +**Setup/Teardown:** +- Per-test setup via method initialization (no `[SetUp]` attribute used) +- Example from `SavedSearchesTests.cs`: + ```csharp + private async Task InitializeModel() + { + _store = new Mock(); + _relay = new Mock(); + var config = new SavedSearchesConfigFile + { + new SavedSearchConfig {Alias = "Test", Tags = {"Test1", "Test2"}} + }; + _store.Setup(s => s.LoadAsyncOrDefault()) + .ReturnsAsync(config); + + var model = new SavedSearchesModel(_store.Object, _relay.Object); + await model.InitializeAsync(); + return model; + } + ``` + +**Assertion Patterns:** +- Direct assertions: `Assert.AreEqual(1, results.Count);` +- Moq verifications: `_relay.Verify(r => r.Send(model, It.Is(...)));` +- Sequence equality: `Assert.IsTrue(expected.SequenceEqual(results.Select(r => r.SearchTag)));` +- Boolean assertions: `Assert.True(results.Count == 2);`, `Assert.False(results.Contains(...))` + +## Mocking + +**Framework:** Moq with Castle.Core + +**Patterns:** + +```csharp +// Create mock +var store = new Mock(); + +// Setup return value +store.Setup(s => s.LoadAsyncOrDefault()) + .ReturnsAsync(config); + +// Use mock in test +var model = new SavedSearchesModel(_store.Object, _relay.Object); + +// Verify calls +_relay.Verify(r => r.Send(model, It.Is(...))); +``` + +**Custom Test Helper:** +```csharp +internal static class TestUtils +{ + public static void Run(Func action) + { + action().GetAwaiter().GetResult(); + } + + public static Mock CreateStore(T config) where T : class, new() + { + var store = new Mock(); + store.Setup(s => s.LoadAsyncOrDefault()).ReturnsAsync(config); + return store; + } +} +``` + +**What to Mock:** +- Service dependencies: `IConfigStore`, `IMessageRelay` +- External integrations: anything with I* interface +- Stateful components: configuration, messaging systems + +**What NOT to Mock:** +- Core domain objects: `TagManager`, `ArrayTrie` +- Data structures: `ObservableSet`, collections +- Test scenarios often create real instances to test behavior + +## Fixtures and Factories + +**Test Data:** +Example from `TagManagerTests.cs`: +```csharp +private class Tagged : ITagged +{ + public ObservableSet Tags { get; } = new ObservableSet(); +} + +[Test] +public void ShouldFindMatchingRecommendations() +{ + var search = "test"; + var previous = new string[0]; + var expected = new[] {"test1", "test2"}; + + var tagged = new[] + { + new Tagged {Tags = {"test1", "test2"}} + }; + + var tagManager = new TagManager(); + tagManager.AddFrom(tagged); + // ... assertions +} +``` + +**Custom Assertions Helper:** +```csharp +public static class Expect +{ + public static void Contains(T expected, IEnumerable items, string message = null) + { + Assert.Contains(expected, items.ToList(), message); + } + + public static bool Contains(this IEnumerable items, params T[] expected) + { + return expected.All(items.Contains); + } +} +``` + +**Location:** +- Test data created inline in test methods +- Shared test utilities in `Assets/Tests/TestUtils.cs` +- Custom assertions in `Assets/Tests/Expect.cs` + +## Coverage + +**Requirements:** No enforced coverage targets detected + +**Current State:** +- Unit tests exist for: `ArrayTrie`, `TagManager`, `SavedSearchesModel` +- Integration test stubs exist for: `LibraryTests.cs` (methods defined but empty) +- Coverage appears incomplete but tests focus on critical logic + +## Test Types + +**Unit Tests:** +- Scope: Individual class/method behavior +- Example: `ArrayTrieTests.cs` - tests data structure insertion and search +- Approach: Direct instantiation, minimal mocking +- Files: `ArrayTrieTests.cs`, `TagManagerTests.cs`, `Expect.cs` + +```csharp +[Test] +public void ShouldIncrementOccurrencesOnMultipleInserts() +{ + var trie = new ArrayTrie(); + + trie.Insert("test"); + trie.Insert("test"); + var results = trie.Find("te").ToList(); + + Assert.AreEqual(1, results.Count); + Assert.AreEqual(2, results[0].occurrences); +} +``` + +**Integration Tests:** +- Scope: Multiple components working together +- Example: `SavedSearchesTests.cs` - ViewModel with mocked services +- Approach: Real components + mocked dependencies +- Files: `SavedSearchesTests.cs` + +```csharp +[Test] +public void SelectingSearchShouldSendChangeMessage() +{ + TestUtils.Run(async () => + { + var model = await InitializeModel(); + + model.SavedSearches.First().SelectCommand.Execute(); + _relay.Verify(r => r.Send(model, + It.Is(msg => msg.SearchTags.Contains("Test1", "Test2")))); + }); +} +``` + +**E2E Tests:** +- Framework: Not detected +- Status: Not used in current codebase + +## Common Patterns + +**Async Testing:** +```csharp +[Test] +public void ShouldBeRestoredFromConfig() +{ + TestUtils.Run(async () => + { + var model = await InitializeModel(); + + Assert.IsTrue(model.SavedSearches.Any(search => search.Alias == "Test")); + }); +} +``` + +Pattern: Wrap async code with `TestUtils.Run()` which calls `GetAwaiter().GetResult()` to run async operations synchronously. + +**Error Scenario Testing:** +- Not extensively demonstrated in current tests +- When testing errors, NUnit attributes `[Test]` and assertions would handle expected exceptions +- Current approach: focus on happy path and behavioral assertions + +**Data-Driven Testing:** +- Not used; individual test methods for each scenario +- Example: `ShouldFindMatchingRecommendations()`, `ShouldOrderRecommendationsBasedOnOccurence()` as separate tests +- Could use `[TestCase]` attributes for parameterized tests (not currently done) + +## Test Dependencies + +**Assembly Definition (`Tests.asmdef`):** +```json +{ + "name": "Tests", + "references": [ + "UnityEngine.TestRunner", + "UnityEditor.TestRunner", + "Scripts" + ], + "allowUnsafeCode": true, + "overrideReferences": true, + "precompiledReferences": [ + "nunit.framework.dll", + "Castle.Core.dll", + "Moq.dll", + "System.Buffers.dll", + "System.Memory.dll" + ], + "defineConstraints": ["UNITY_INCLUDE_TESTS"] +} +``` + +**Packages Required:** +- NUnit 3.x +- Moq 4.x +- Castle.Core (for Moq) +- System.Buffers, System.Memory (runtime dependencies) + +## Test Execution + +**Via Unity Editor:** +- Window → General → Test Runner +- Tests.asmdef marked for Editor platform only + +**Via GitHub Actions:** +- CI pipeline runs tests (from commit history reference) +- File: `.github/workflows/` (not examined but referenced in recent commits) + +--- + +*Testing analysis: 2026-01-23* From b60bb82e7f5b2f4815919b63e2600f0d3d272542 Mon Sep 17 00:00:00 2001 From: Tom Boucher Date: Fri, 23 Jan 2026 18:44:39 -0500 Subject: [PATCH 2/2] Fix deep subfolder scanning failing silently Fixes #52, #73 Directory.GetFiles with SearchOption.AllDirectories throws and returns nothing when ANY subdirectory is inaccessible (permissions, symlinks, long paths). This broke recursive scanning for users with large nested folder structures. Replace with GetFilesResilient() that: - Handles exceptions per-directory instead of failing the entire scan - Catches UnauthorizedAccessException and PathTooLongException specifically - Logs warnings for inaccessible paths but continues scanning - Continues to discover files in accessible directories Co-Authored-By: Claude Opus 4.5 --- .../Util/FileSystem/FolderFileSystem.cs | 68 ++++++++++++++++++- 1 file changed, 65 insertions(+), 3 deletions(-) diff --git a/Assets/Scripts/Util/FileSystem/FolderFileSystem.cs b/Assets/Scripts/Util/FileSystem/FolderFileSystem.cs index 3a441af..9d9352a 100644 --- a/Assets/Scripts/Util/FileSystem/FolderFileSystem.cs +++ b/Assets/Scripts/Util/FileSystem/FolderFileSystem.cs @@ -26,10 +26,9 @@ public IFolderWatcher CreateWatcher(string filter, bool scanSubDirectories) public IReadOnlyList GetFiles(string pattern, bool recursive) { - var options = recursive ? SearchOption.AllDirectories : SearchOption.TopDirectoryOnly; var files = new List(); - - foreach (var file in Directory.GetFiles(_rootPath, pattern, options)) + + foreach (var file in GetFilesResilient(_rootPath, pattern, recursive)) { try { @@ -51,6 +50,69 @@ public IReadOnlyList GetFiles(string pattern, bool recursive) return files; } + /// + /// Enumerates files with resilient error handling per-directory. + /// Unlike Directory.GetFiles with SearchOption.AllDirectories, this method + /// continues scanning when individual directories are inaccessible. + /// + private IEnumerable GetFilesResilient(string path, string pattern, bool recursive) + { + // Get files in current directory + string[] files = null; + try + { + files = Directory.GetFiles(path, pattern, SearchOption.TopDirectoryOnly); + } + catch (UnauthorizedAccessException) + { + Logger.Warn("Access denied to directory: {0}", path); + } + catch (PathTooLongException) + { + Logger.Warn("Path too long: {0}", path); + } + catch (Exception ex) + { + Logger.Warn(ex, "Error accessing directory: {0}", path); + } + + if (files != null) + { + foreach (var file in files) + yield return file; + } + + // Recurse into subdirectories if requested + if (!recursive) yield break; + + string[] directories = null; + try + { + directories = Directory.GetDirectories(path); + } + catch (UnauthorizedAccessException) + { + Logger.Warn("Access denied to subdirectories of: {0}", path); + } + catch (PathTooLongException) + { + Logger.Warn("Path too long for subdirectories: {0}", path); + } + catch (Exception ex) + { + Logger.Warn(ex, "Error accessing subdirectories of: {0}", path); + } + + if (directories != null) + { + foreach (var directory in directories) + { + foreach (var file in GetFilesResilient(directory, pattern, recursive)) + yield return file; + } + } + } + public Task ReadAllBytesAsync(string filePath) { var path = Path.Combine(_rootPath, filePath);