Simplify storage API by consolidating discovery into loader module#46
Merged
cfuehrmann merged 15 commits intomainfrom Oct 6, 2025
Merged
Simplify storage API by consolidating discovery into loader module#46cfuehrmann merged 15 commits intomainfrom
cfuehrmann merged 15 commits intomainfrom
Conversation
- Create catalog/discovery.rs with domain-level directory finding logic - Move discovery functions from storage layer to catalog domain layer - Rename storage/ to jsonc/ to accurately reflect JSONC file format implementation - Update all module references and function calls to use new structure - Improve architectural consistency with proper separation of concerns
…ure/format separation - Add JsoncError type for JSONC format-specific parsing and validation errors - Enhance AppError with proper domain variants (UnknownIngredientError, DuplicateKeyError) - Add infrastructure error handling (FileError) to AppError - Refactor JSONC loader to use new error architecture with proper conversions - Remove legacy LoadError dependencies from active code paths - Achieve clean architectural boundaries for future format extensibility
Remove '_app' suffixes from function names now that legacy LoadError functions have been removed, improving code readability without changing functionality.
…LoadError - Move JsoncError internal to catalog/jsonc module with proper encapsulation - Update JsoncError variants to action-based names (Parsing, Deserializing, SchemaValidation) - Eliminate LoadError entirely and consolidate domain errors into AppError - Create clean boundary conversion from JsoncError to AppError at catalog layer - Remove all format-specific error variants from public API - Maintain DuplicateGroup as part of AppError module Results in clean two-tier error architecture: - JsoncError (internal): Pure JSONC format errors, never escapes catalog/jsonc - AppError (public): Domain + infrastructure errors, clean external API All 34 tests pass with correct error messages and clippy compliance achieved.
- Remove RecipeNotFound (unused domain variant) - Remove FileError (redundant with Io for infrastructure errors) - Add clarifying comment about storage format error encapsulation - Streamline error types to essential variants only Results in cleaner, more focused error architecture with no functional changes.
- Change Other(String) to Other { message: String } for consistency
- Update all Other variant creation sites to use structured format
- Consolidate message handling in Display implementation
- Maintain same functionality with cleaner, more consistent API
All error variants now follow the same structured pattern for better
consistency and future extensibility.
- Add end-to-end snapshot test for JsoncError::Parsing format errors - Remove unused Display implementation details from JsoncError - Replace with minimal Display required by std::error::Error trait - Test verifies actual error conversion behavior through convert_jsonc_error() The detailed Display implementation was dead code since error formatting happens in the conversion function, not the Display trait. Test count: 37.
- Delete test_jsonc_parsing_syntax_error() and its snapshot - Test served its purpose to demonstrate JsoncError Display was dead code - Return to 36 tests, maintaining clean test suite The test proved that error formatting happens in convert_jsonc_error() function rather than JsoncError Display implementation.
- Delete unused Display implementation for JsoncError - Remove std::error::Error implementation - JsoncError is now a pure data structure with no trait implementations - All error formatting happens in convert_jsonc_error() function This completes the dead code removal - JsoncError exists solely as an internal data structure for the catalog/jsonc module.
…creation This removes the entire JsoncError infrastructure in favor of creating AppError::Other directly in the JSONC loader. The intermediate error type provided no functional value beyond message formatting, which is now handled inline where errors occur. Changes: - Delete src/catalog/jsonc/error.rs entirely - Replace all JsoncError usage with AppError::Other in loader.rs - Remove JsoncError module from mod.rs - Eliminate convert_jsonc_error() conversion function All functionality and error messages remain identical.
…ization This improves error handling by creating specific variants for common error types: - FileUnreadable: File system access issues (permissions, missing files) - ParsingError: Structured data parsing issues (syntax errors, empty files) Benefits: - Format-agnostic design ready for YAML, CSV, and other data formats - Cleaner error messages (removed format-specific advice from empty file errors) - Better separation of concerns between file access vs parsing vs validation Changes: - Add FileUnreadable and ParsingError variants to AppError - Update JSONC loader to use appropriate error types - Simplify empty file error message to just state the filename
…d semantic error types This represents a comprehensive redesign of the error system to achieve perfect semantic coverage with no catch-all categories: ## Major Improvements: ### Eliminated Generic Catch-All - Remove AppError::Other entirely - no more ambiguous error category - Every error now has specific semantic meaning ### Added Semantic Error Variants - SchemaComplianceError: Data doesn't comply with schema requirements - InvalidSchema: Schema compilation failures - TypeMappingError: Converting parsed data to domain types fails ### Cleaned Variant Names - UnknownIngredientError → UnknownIngredient - DuplicateKeyError → DuplicateKey ### Improved Message Origin Architecture - FileUnreadable: Now uses structured data (path, io_error) with domain-level formatting - Format-agnostic errors: Messages generated in AppError Display - Format-specific errors: Messages generated in format modules (JSONC, YAML, etc.) ### Enhanced Schema Handling - Proper error handling for embedded schema parsing (removed expect calls) - Simplified schema file creation (direct string copying vs complex serialization) ## Final Error Architecture: 1. CatalogNotFound - Catalog directory search failures 2. DirectoryNotEmpty - Directory not empty during initialization 3. UnknownIngredient - Domain-specific ingredient reference errors 4. DuplicateKey - Domain-specific uniqueness violations 5. FileUnreadable - File system access issues (structured data) 6. ParsingError - Syntax parsing failures (format-specific messages) 7. SchemaComplianceError - Data doesn't comply with schema requirements 8. InvalidSchema - Schema compilation failures 9. TypeMappingError - Converting Value to domain structs fails 10. Io(std::io::Error) - Automatic io::Error conversion for ? operator ## Benefits: - Complete semantic coverage - every error type represents specific failure mode - No ambiguous catch-all category - precise error categorization - Format-agnostic design - ready for YAML, CSV, XML support - Proper message origin - domain vs format-specific separation - Better user experience - clear, specific error messages - Maintainable architecture - easy to extend for new formats
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Major refactoring that renames storage module to jsonc and consolidates error handling by eliminating LoadError in favor of semantic AppError variants.
Key Changes
Module Restructuring:
storage/tojsonc/to better reflect JSON-with-comments format handlingdiscovery.rsfromstorage/tocatalog/level for better organizationError Architecture Consolidation:
LoadErrorentirely - replaced with semanticAppErrorvariantsFileUnreadableandParsingErrorvariants for better error categorizationAppErrorenumAPI Improvements:
Benefits
AppError) instead of multiple error enumsAll 36 tests pass with updated snapshots reflecting cleaner error messages.