feat(sync): sync pipeline implementation (#27)#53
Merged
Conversation
Tests reference SyncError, SyncRuleEvaluator, FileClassifier, SyncJob, SyncConflict, ISyncService, ISyncScheduler — none of which exist yet. All tests fail to compile. Compile error count: 5 new errors (CS0234 — namespace AStar.Dev.CloudSyncFunctional.Sync does not exist) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
) - Fix FileClassifier to use exact token equality (Equals) not substring matching (Contains); add false-positive test for keyword-in-token case - Fix SyncScheduler.TriggerAccountAsync to handle SyncAccountAsync Result: SyncCompleted fires on success only; failures are logged via LogSyncFailed - Register IHttpClientFactory via AddHttpClient() in App DI composition root; GraphService and HttpDownloader both depend on it - Thread accountId and driveId through ISyncWorker.ExecuteAsync and ISyncPipeline.RunAsync so uploads resolve the correct drive context - Replace GetClientForToken throw-in-Match with TryGetClientForToken returning Result; callers use BindAsync - Replace System.IO.* in GraphService.UploadFileAsync with IFileSystem - Replace raw new HttpClient() in UploadChunksAsync with IHttpClientFactory - Add full chunk retry loop to UploadChunksAsync (429, 404, backoff) - Resolve GetDownloadUrlAsync before passing URL to IHttpDownloader - Fix HttpDownloader.WriteFileAsync to use FileMode.Create (truncate) - Fix HttpDownloader 429 double-sleep: single delay then continue - Fix SyncPipeline to use Interlocked.Increment for global progress counter - Fix SyncedItemRegistrar to match UpsertAsync result and log on failure - Fix SyncService.ResolveConflictAsync to handle result in Skip branch - Fix RemoteFolderEnumerator and LocalDeletionDetector intermediate unwrapping: chain MatchAsync directly on Task - Introduce SyncRule domain record + SyncRuleFactory; remove SyncRuleEntity dependency from SyncRuleEvaluator and RemoteFolderEnumerator - Add SyncConflictFactory; typed AccountId/OneDriveItemId on SyncConflict - Remove primitive string? DriveId from OneDriveAccount; DriveIdValue only - Extract MapToDomain helper in SyncScheduler to eliminate duplication - Add interfaces for all six pipeline steps; SyncService injects by interface - Add LocalChangeDetector: UploadJob carries ParentFolderPath (resolved at execution time by SyncWorker via GetFolderIdByPathAsync) - Update GivenASyncConflict and GivenASyncRuleEvaluator to use factories Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Test results0 tests 0 ✅ 0s ⏱️ Results for commit 1e72313. ♻️ This comment has been updated with latest results. |
Tests cover: - AccountViewModel.AccountId property (missing) - WorkspaceViewModel.OnWizardCompleted populates FolderNode from SelectedFolders (missing) - WorkspaceViewModel.LoadPersistedAccounts loads sync rules as FolderNode (missing) - WorkspaceViewModel.TriggerSync command calls ISyncScheduler (missing) Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- SyncRuleEntity no longer leaks above the repository layer; ISyncRuleRepository.GetByAccountAsync now returns IReadOnlyList<SyncRule> with entity-to-domain mapping in the repository - Add GetAllByAccountIdsAsync batch method to eliminate N+1 query in WorkspaceViewModel.LoadPersistedAccountsAsync - WorkspaceViewModel exposes HasError/ErrorMessage reactive properties; TriggerSync.ThrownExceptions subscribed and logs via optional ILogger - canSync predicate now gates on _syncScheduler is not null, preventing silent no-op when scheduler absent - Extract InitializeCommands() to eliminate duplicated canSync/TriggerSync wiring across constructors - MapToViewModel single-pass: build includedFolders list once, derive FolderCount from .Count - ExecuteTriggerSyncAsync simplified to one-liner; canSync guards make the null checks redundant - WorkspaceViewModel implements IDisposable; CompositeDisposable cleans up ThrownExceptions subscription - SyncService removes redundant entity→domain mapping step now that the repository returns domain records Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
- Fix ODataError exceptions logging full error code/message instead of generic ex.Message - Fix Linux token cache registration by adding LinuxKeyring attributes, with unprotected-file fallback - Add GetDriveIdAsync to IGraphService/GraphService; wizard now fetches and persists DriveId on account creation - Fix cross-thread InvalidOperationException from ReactiveCommandBase.OnCanExecuteChanged by adding outputScheduler: AvaloniaScheduler.Instance to all commands - Replace Dispatcher.UIThread.Post with RxSchedulers.MainThreadScheduler.Schedule so property mutations are testable (ImmediateScheduler in tests, AvaloniaScheduler in production) - Fix test fixture to set RxSchedulers.MainThreadScheduler = ImmediateScheduler.Instance; add GetDriveIdAsync mock to AddAccount tests; add test for GetDriveIdAsync failure path Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jbarden
approved these changes
May 29, 2026
jbarden
left a comment
There was a problem hiding this comment.
I know this went through several rounds of review and fix before it got to me...there are a few methods that are still too long etc but I know there's a plan to test out matt pocock's "improve-codebase-architecture" skill so will accept them to see if it flags
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
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
RemoteFolderEnumerator→RemoteDeletionDetector→LocalDeletionDetector→DownloadJobBuilder→LocalChangeDetector→JobExecutor(parallel bounded-channel execution viaSyncPipeline)SyncRuledomain record,SyncConflictwith typed value-object IDs,SyncErrordiscriminated union, and all supporting interfacesKey fixes applied during review
FileClassifier— token matching now uses exactEquals(not substringContains); false-positive test addedSyncScheduler—SyncAccountAsyncresult handled viaMatchAsync;SyncCompletedfires on success only; failures loggedApp.axaml.cs—services.AddHttpClient()added;IHttpClientFactorynow registered forGraphServiceandHttpDownloaderGraphService—System.IO.*replaced withIFileSystem; rawnew HttpClient()replaced withIHttpClientFactory;GetClientForTokenno longer throws insideMatch;UploadChunksAsynchas full retry loop (429 / 404 / backoff)SyncWorker— callsGetDownloadUrlAsyncbefore download;accountId/driveIdthreaded through from callerHttpDownloader—FileMode.Createtruncates on re-download; 429 double-sleep eliminatedSyncPipeline— globalInterlocked.Incrementcounter replaces per-worker counterIRemoteFolderEnumerator,IRemoteDeletionDetector,ILocalDeletionDetector,IDownloadJobBuilder,ILocalChangeDetector,IJobExecutoradded;SyncServiceinjects all by interfaceTest plan
dotnet build— 0 errors, 0 warningsdotnet run --project test/AStar.Dev.CloudSyncFunctional.Tests.Unit— 230 passed, 0 failedSyncCompletedevent is not raised when sync fails (scheduler fix)/photographs/img.jpgis not classified under a"photo"keyword rule (classifier fix)🤖 Generated with Claude Code