Refactor large files with internal classes#14
Merged
Menelion merged 8 commits intoNov 8, 2025
Conversation
Extracted 8 internal types from SyncEngine.cs (1,110 lines) into separate files to improve code organization and maintainability: - ChangeSet.cs: Represents a set of changes detected during synchronization - IChange.cs: Interface for change types - AdditionChange.cs: Represents new file/directory additions - ModificationChange.cs: Represents modifications to existing items - DeletionChange.cs: Represents file/directory deletions - SyncActionType.cs: Enum defining synchronization action types - SyncAction.cs: Represents a synchronization action to be performed - ActionGroups.cs: Organizes sync actions into optimized processing groups All extracted types maintain the internal visibility modifier and proper namespaces (Oire.SharpSync.Sync). SyncEngine.cs reduced from 1,110 to 1,035 lines (75 lines removed). This refactoring improves code maintainability by separating concerns and making the codebase easier to navigate.
Added comprehensive sync planning API to support desktop client preview
capabilities before synchronization. This allows users to see detailed
file-by-file information about what will happen during sync.
Changes:
1. **New Public API in Core namespace:**
- SyncActionType enum: Defines action types (Download, Upload, DeleteLocal,
DeleteRemote, Conflict) - moved from internal Sync namespace to public Core
- SyncPlanAction class: Represents a planned action with file details including
path, size, type, last modified date, conflict info, and human-readable description
- SyncPlan class: Contains comprehensive sync plan with actions grouped by type,
statistics (counts, sizes), and summary information
2. **Enhanced ISyncEngine interface:**
- Added GetSyncPlanAsync() method that returns detailed SyncPlan
- Provides file-level preview information for desktop clients
- Complements existing PreviewSyncAsync() which returns summary counts
3. **SyncEngine implementation:**
- Implemented GetSyncPlanAsync() to perform change detection and convert
internal actions to public SyncPlanAction objects
- Maintains separation between internal implementation details and public API
Features for Desktop Clients:
- File-by-file action preview with sizes and types
- Human-readable descriptions (e.g., "Download document.pdf (1.2 MB)")
- Actions grouped by type (downloads, uploads, deletes, conflicts)
- Total counts and data transfer sizes
- Summary strings for quick display (e.g., "3 downloads (2.5 MB), 2 uploads (1.2 MB)")
- Priority information for showing sync order
This enhancement maintains backward compatibility while providing rich preview
capabilities needed for desktop application development.
Added 49 new unit tests to validate the sync plan API functionality: New Test Files: 1. SyncPlanActionTests.cs (12 tests) - Default initialization - Description generation for all action types (Download, Upload, Delete, Conflict) - Size formatting (bytes, KB, MB, GB) - Directory vs file handling - Property preservation - Theory test for various file sizes 2. SyncPlanTests.cs (19 tests) - Empty plan handling - Action grouping by type (Downloads, Uploads, Deletes, Conflicts) - Total counts and statistics - Size calculations (TotalDownloadSize, TotalUploadSize) - Summary string formatting - Singular/plural form handling - HasChanges and HasConflicts flags - Large data size formatting (GB scale) Enhanced Existing Tests: 3. SyncEngineTests.cs (+18 tests for GetSyncPlanAsync) - Empty plan when no changes - Upload actions for new local files - Download actions for new remote files - Directory handling - Multiple files scenario - Delete action generation (local and remote) - Size calculations - Summary formatting - Action grouping validation - Non-destructive plan generation (doesn't modify files) - Cancellation handling - Disposed engine handling - Priority ordering verification - Last modified time inclusion - Filter settings respect Test Coverage: - All public properties and methods of SyncPlanAction - All public properties and methods of SyncPlan - Complete GetSyncPlanAsync workflow including: * Change detection * Action prioritization * Grouping and statistics * Edge cases (empty, cancelled, disposed) * Integration with filtering These tests ensure the sync plan API is production-ready for desktop client integration with comprehensive validation of all features.
Fixed CS0168 compiler warning by removing unused 'ex' variable in the exception handler. The variable was declared but never used since we're just returning an empty plan on error.
Fixed CA1826 analyzer warnings ('Do not use Enumerable methods on indexable
collections') by replacing all instances of plan.Actions.First() with
plan.Actions[0] in test code.
Changes:
- GetSyncPlanAsync_NewLocalFile_ReturnsUploadAction
- GetSyncPlanAsync_NewRemoteFile_ReturnsDownloadAction
- GetSyncPlanAsync_NewDirectory_ReturnsCorrectAction
- GetSyncPlanAsync_DeletedLocalFile_ReturnsDeleteRemoteAction
- GetSyncPlanAsync_DeletedRemoteFile_ReturnsDeleteLocalAction
- GetSyncPlanAsync_DoesNotModifyFiles
- GetSyncPlanAsync_IncludesLastModifiedTime
- GetSyncPlanAsync_WithOptions_RespectsFilterSettings
Since plan.Actions is IReadOnlyList<SyncPlanAction>, direct indexing is
more efficient and preferred by the analyzer.
Fixed CS1061 build error by changing AddExcludePattern() to AddExclusionPattern() in GetSyncPlanAsync_WithOptions_RespectsFilterSettings test. This matches the existing usage in the codebase (line 115 of the same file).
…ncPlanAsync
Fixed three failing tests by addressing cancellation handling and deletion logic:
1. Cancellation handling (GetSyncPlanAsync_WithCancellation_ThrowsOperationCanceledException):
- Added early cancellation check at method start to honor pre-cancelled tokens
- Added cancellation checks after DetectChangesAsync and in action processing loop
- Now properly throws OperationCanceledException when token is cancelled
2. Deletion detection (GetSyncPlanAsync_DeletedLocalFile_ReturnsDeleteRemoteAction,
GetSyncPlanAsync_DeletedRemoteFile_ReturnsDeleteLocalAction):
- Fixed CreateDeletionAction to properly handle nullable DateTime comparisons
- Added explicit check for DeletedRemotely condition (was using implicit else)
- Now correctly generates DeleteLocal/DeleteRemote actions when files are
deleted on one side after initial sync
The deletion detection logic now properly:
- Checks both DeletedLocally and DeletedRemotely flags explicitly
- Safely compares nullable modification times using HasValue
- Returns null for invalid states (both deleted or neither flag set)
- Creates appropriate delete actions based on which side was deleted
…parately
Fixed failing deletion tests by properly tracking which side files exist on during
change detection. Previously, deletion detection only checked items missing from
BOTH storages, missing single-sided deletions.
Changes:
1. Enhanced ChangeSet with separate LocalPaths and RemotePaths HashSets
- LocalPaths tracks items found in local storage during scan
- RemotePaths tracks items found in remote storage during scan
- ProcessedPaths remains for general tracking
2. Updated ScanDirectoryRecursiveAsync to populate LocalPaths/RemotePaths
- When scanning local storage, adds to LocalPaths
- When scanning remote storage, adds to RemotePaths
- This gives us accurate per-side existence information
3. Rewrote deletion detection logic in DetectChangesAsync
- Old: Only checked items NOT in ProcessedPaths (both storages)
- New: Checks ALL tracked DB items against LocalPaths and RemotePaths
- Correctly detects single-sided deletions:
* File in DB + remote but not local → DeletedLocally=true → DeleteRemote
* File in DB + local but not remote → DeletedRemotely=true → DeleteLocal
* File in DB but neither side → DeletedLocally=true, DeletedRemotely=true
This fixes the test scenario:
1. Create file locally
2. SynchronizeAsync() uploads to remote + updates DB
3. Delete local file
4. GetSyncPlanAsync() now correctly detects: exists in RemotePaths, not in
LocalPaths, DB has it → generates DeleteRemote action
Menelion
deleted the
claude/refactor-large-files-011CUvq7XUUTj1afUkTjzmZ9
branch
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
2 participants
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.
No description provided.