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* diff --git a/Assets/Scripts/Services/IImportFolder.cs b/Assets/Scripts/Services/IImportFolder.cs index a6e34ed..84f8d3c 100644 --- a/Assets/Scripts/Services/IImportFolder.cs +++ b/Assets/Scripts/Services/IImportFolder.cs @@ -1,6 +1,12 @@ -namespace StlVault.Services +using System.Threading.Tasks; + +namespace StlVault.Services { internal interface IImportFolder : IFileSource { + /// + /// Rescans the import folder to detect added, changed, or deleted files. + /// + Task RescanAsync(); } } \ No newline at end of file diff --git a/Assets/Scripts/Services/ImportFolder.cs b/Assets/Scripts/Services/ImportFolder.cs index 755e2b4..8791c40 100644 --- a/Assets/Scripts/Services/ImportFolder.cs +++ b/Assets/Scripts/Services/ImportFolder.cs @@ -172,6 +172,8 @@ await Task.Run(() => public override void Dispose() => _tokenSource?.Cancel(); + public Task RescanAsync() => RescanItemsAsync(); + public async Task OnDeletedAsync() { State.Value = Deleting; diff --git a/Assets/Scripts/ViewModels/FileSourceModel.cs b/Assets/Scripts/ViewModels/FileSourceModel.cs index d6bd329..42ec73c 100644 --- a/Assets/Scripts/ViewModels/FileSourceModel.cs +++ b/Assets/Scripts/ViewModels/FileSourceModel.cs @@ -7,7 +7,7 @@ namespace StlVault.ViewModels { - internal class FileSourceModel + internal class FileSourceModel { public BindableProperty Path { get; } = new BindableProperty(); @@ -17,6 +17,7 @@ internal class FileSourceModel public ICommand SelectCommand { get; } public ICommand EditCommand { get; } public ICommand DeleteCommand { get; } + [CanBeNull] public ICommand RescanCommand { get; } public FileSourceModel( [NotNull] IFileSource source, @@ -33,6 +34,11 @@ public FileSourceModel( EditCommand = new DelegateCommand(() => onEdit(this)); DeleteCommand = new DelegateCommand(() => onDelete(this)); + if (FileSource is IImportFolder importFolder) + { + RescanCommand = new DelegateCommand(async () => await importFolder.RescanAsync()); + } + Path.Value = FileSource.DisplayName; } } diff --git a/Assets/Scripts/Views/ImportFolderView.cs b/Assets/Scripts/Views/ImportFolderView.cs index 6487ef8..964f6a9 100644 --- a/Assets/Scripts/Views/ImportFolderView.cs +++ b/Assets/Scripts/Views/ImportFolderView.cs @@ -33,10 +33,18 @@ protected override void OnViewModelBound() ViewModel.State.OnMainThread().ValueChanged += UpdateStateIcon; UpdateStateIcon(ViewModel.State); - + _text.BindTo(ViewModel.Path); ViewModel.Path.ValueChanged += OnPathOnValueChanged; + // Make icon clickable to trigger rescan + if (ViewModel.RescanCommand != null && _icon != null) + { + var iconButton = _icon.GetComponent