From 3c273922379f0ceeb49e8bc725bdc3604d186ac9 Mon Sep 17 00:00:00 2001 From: fboucher-os Date: Sun, 13 Sep 2026 08:35:47 -0400 Subject: [PATCH] Fix document sync stuck and improve sync progress and offline fallback (fixes #196) --- .../Tests/PostsTests.cs | 30 +++++ .../SyncProgressEventArgs.cs | 4 +- .../SyncServiceTests.cs | 110 +++++++++++++++ .../Data/OfflineDataService.cs | 47 +++++-- src/NoteBookmark.MauiApp/Data/SyncService.cs | 125 ++++++++++++++++-- .../Components/Pages/Posts.razor | 30 +++-- src/NoteBookmark.SharedUI/IDataService.cs | 1 + src/NoteBookmark.SharedUI/PostNoteClient.cs | 1 + 8 files changed, 315 insertions(+), 33 deletions(-) diff --git a/src/NoteBookmark.BlazorApp.Tests/Tests/PostsTests.cs b/src/NoteBookmark.BlazorApp.Tests/Tests/PostsTests.cs index 6c3c6db..c4e5ec9 100644 --- a/src/NoteBookmark.BlazorApp.Tests/Tests/PostsTests.cs +++ b/src/NoteBookmark.BlazorApp.Tests/Tests/PostsTests.cs @@ -162,5 +162,35 @@ public void Posts_DisplaysCleaningStatus_WhenSyncProgressChangedFired() cut.Markup.Should().Contain("Cleaning..."); } + + [Fact] + public void Posts_SyncProgressChanged_WhenIsComplete_ReloadsPosts() + { + var cut = Render(); + + _dataServiceMock.Invocations.Clear(); + + cut.InvokeAsync(() => + { + _dataServiceMock.Raise(s => s.SyncProgressChanged += null, new SyncProgressEventArgs(0, 0, "Synchronization complete!", isComplete: true)); + }); + + _dataServiceMock.Verify(s => s.GetUnreadPosts(), Times.AtLeastOnce); + } + + [Fact] + public void Posts_SyncButton_DisabledAndLoadingReflectsIsSyncing() + { + _dataServiceMock.SetupGet(s => s.CanSync).Returns(true); + _dataServiceMock.SetupGet(s => s.IsSyncing).Returns(true); + + var cut = Render(); + + var buttons = cut.FindComponents(); + var syncButton = buttons.FirstOrDefault(b => b.Instance.Title == "Sync posts and comments"); + syncButton.Should().NotBeNull(); + syncButton!.Instance.Disabled.Should().BeTrue(); + syncButton.Instance.Loading.Should().BeTrue(); + } } diff --git a/src/NoteBookmark.Domain/SyncProgressEventArgs.cs b/src/NoteBookmark.Domain/SyncProgressEventArgs.cs index fc7399a..ee7be29 100644 --- a/src/NoteBookmark.Domain/SyncProgressEventArgs.cs +++ b/src/NoteBookmark.Domain/SyncProgressEventArgs.cs @@ -7,12 +7,14 @@ public class SyncProgressEventArgs : EventArgs public int Current { get; } public int Total { get; } public string Status { get; } + public bool IsComplete { get; } public double Percentage => Total > 0 ? (double)Current / Total * 100 : 0; - public SyncProgressEventArgs(int current, int total, string status) + public SyncProgressEventArgs(int current, int total, string status, bool isComplete = false) { Current = current; Total = total; Status = status; + IsComplete = isComplete; } } diff --git a/src/NoteBookmark.MauiApp.Tests/SyncServiceTests.cs b/src/NoteBookmark.MauiApp.Tests/SyncServiceTests.cs index 1ad1706..4204ab5 100644 --- a/src/NoteBookmark.MauiApp.Tests/SyncServiceTests.cs +++ b/src/NoteBookmark.MauiApp.Tests/SyncServiceTests.cs @@ -470,6 +470,116 @@ public async Task SyncAsync_ShouldRaiseSyncProgressChanged_WhenDownloadingPostHt progressEvents.Should().Contain(e => e.Status == "Downloading 1 of 2 posts..." && e.Current == 1 && e.Total == 2); progressEvents.Should().Contain(e => e.Status == "Downloading 2 of 2 posts..." && e.Current == 2 && e.Total == 2); progressEvents.Last().Status.Should().Be("Synchronization complete!"); + progressEvents.Last().IsComplete.Should().BeTrue(); + } + + [Fact] + public async Task PullPhase_ReadPosts_ShouldNotCallGetPost_AndShouldSaveDirectly() + { + var readPostL = new PostL + { + Id = "read1", + RowKey = "read1", + PartitionKey = "pk", + Title = "Read Post", + is_read = true, + DateModified = DateTime.UtcNow + }; + + _localDataServiceMock.Setup(c => c.GetPendingSyncNotesAsync()).ReturnsAsync(new List()); + _localDataServiceMock.Setup(c => c.GetPostsAsync()).ReturnsAsync(new List()); + _apiClientMock.Setup(c => c.GetPostsModifiedAfter(DateTime.MinValue)).ReturnsAsync(new List { readPostL }); + _apiClientMock.Setup(c => c.GetNotesModifiedAfter(It.IsAny())).ReturnsAsync(new List()); + + await _sut.SyncAsync(); + + // GetPost should NOT be called for read posts + _apiClientMock.Verify(c => c.GetPost("read1"), Times.Never); + _localDataServiceMock.Verify(c => c.SavePostAsync(It.Is(p => p.Id == "read1" && p.is_read == true), false), Times.Once); + } + + [Fact] + public async Task PullPhase_UnreadPost_WhenGetPostFails_ShouldFallbackToBasicPost() + { + var unreadPostL = new PostL + { + Id = "unread1", + RowKey = "unread1", + PartitionKey = "pk", + Title = "Unread Post", + is_read = false, + DateModified = DateTime.UtcNow + }; + + _localDataServiceMock.Setup(c => c.GetPendingSyncNotesAsync()).ReturnsAsync(new List()); + _localDataServiceMock.Setup(c => c.GetPostsAsync()).ReturnsAsync(new List()); + _apiClientMock.Setup(c => c.GetPostsModifiedAfter(DateTime.MinValue)).ReturnsAsync(new List { unreadPostL }); + _apiClientMock.Setup(c => c.GetPost("unread1")).ThrowsAsync(new System.Net.Http.HttpRequestException("404 Not Found")); + _apiClientMock.Setup(c => c.GetNotesModifiedAfter(It.IsAny())).ReturnsAsync(new List()); + + await _sut.SyncAsync(); + + // Should fall back and save basic post without throwing + _localDataServiceMock.Verify(c => c.SavePostAsync(It.Is(p => p.Id == "unread1" && p.Title == "Unread Post"), false), Times.Once); + } + + [Fact] + public async Task PullPhase_ShouldReportProgress_WhenPullingPosts() + { + var postL1 = new PostL { Id = "p1", RowKey = "p1", PartitionKey = "pk", Title = "Post 1", is_read = true, DateModified = DateTime.UtcNow }; + var postL2 = new PostL { Id = "p2", RowKey = "p2", PartitionKey = "pk", Title = "Post 2", is_read = true, DateModified = DateTime.UtcNow }; + + _localDataServiceMock.Setup(c => c.GetPendingSyncNotesAsync()).ReturnsAsync(new List()); + _localDataServiceMock.Setup(c => c.GetPostsAsync()).ReturnsAsync(new List()); + _apiClientMock.Setup(c => c.GetPostsModifiedAfter(DateTime.MinValue)).ReturnsAsync(new List { postL1, postL2 }); + _apiClientMock.Setup(c => c.GetNotesModifiedAfter(It.IsAny())).ReturnsAsync(new List()); + + var progressEvents = new List(); + _sut.SyncProgressChanged += (sender, args) => progressEvents.Add(args); + + await _sut.SyncAsync(); + + progressEvents.Should().Contain(e => e.Status == "Pulling 0 of 2 posts..." && e.Current == 0 && e.Total == 2); + progressEvents.Should().Contain(e => e.Status == "Pulling 1 of 2 posts..." && e.Current == 1 && e.Total == 2); + progressEvents.Should().Contain(e => e.Status == "Pulling 2 of 2 posts..." && e.Current == 2 && e.Total == 2); + } + + [Fact] + public async Task SyncAsync_WhenFails_ShouldRaiseSyncProgressChangedWithIsCompleteAndFailureStatus() + { + _localDataServiceMock.Setup(c => c.GetPendingSyncNotesAsync()).ThrowsAsync(new InvalidOperationException("DB error")); + + var progressEvents = new List(); + _sut.SyncProgressChanged += (sender, args) => progressEvents.Add(args); + + Func act = async () => await _sut.SyncAsync(); + await act.Should().ThrowAsync(); + + progressEvents.Should().NotBeEmpty(); + var lastEvent = progressEvents.Last(); + lastEvent.IsComplete.Should().BeTrue(); + lastEvent.Status.Should().Contain("Sync failed: DB error"); + } + + [Fact] + public async Task IsSyncing_ShouldReflectActiveSyncTask() + { + var tcs = new TaskCompletionSource>(); + _localDataServiceMock.Setup(c => c.GetPendingSyncNotesAsync()).Returns(tcs.Task); + + _sut.IsSyncing.Should().BeFalse(); + + var syncTask = _sut.SyncAsync(); + + _sut.IsSyncing.Should().BeTrue(); + + tcs.SetResult(new List()); + _apiClientMock.Setup(c => c.GetPostsModifiedAfter(It.IsAny())).ReturnsAsync(new List()); + _apiClientMock.Setup(c => c.GetNotesModifiedAfter(It.IsAny())).ReturnsAsync(new List()); + + await syncTask; + + _sut.IsSyncing.Should().BeFalse(); } } diff --git a/src/NoteBookmark.MauiApp/Data/OfflineDataService.cs b/src/NoteBookmark.MauiApp/Data/OfflineDataService.cs index 1136a47..7e38d03 100644 --- a/src/NoteBookmark.MauiApp/Data/OfflineDataService.cs +++ b/src/NoteBookmark.MauiApp/Data/OfflineDataService.cs @@ -178,9 +178,16 @@ public async Task DeleteNote(string noteId) { if (IsOnline) { - var post = await apiClient.GetPost(id); - if (post != null) await localDataService.SavePostAsync(post); - return post; + try + { + var post = await apiClient.GetPost(id); + if (post != null) await localDataService.SavePostAsync(post); + return post; + } + catch + { + return await localDataService.GetPostAsync(id); + } } else { @@ -243,6 +250,34 @@ public async Task ExtractPostDetailsAndSave(string url) return false; // Can't extract offline } + public async Task GetPostHtmlAsync(string postId) + { + var localHtml = await localHtmlStorageService.GetPostHtmlAsync(postId); + if (!string.IsNullOrEmpty(localHtml)) + { + return localHtml; + } + + if (IsOnline) + { + try + { + var remoteHtml = await apiClient.GetPostHtmlAsync(postId); + if (!string.IsNullOrEmpty(remoteHtml)) + { + await localHtmlStorageService.SavePostHtmlAsync(postId, remoteHtml); + return remoteHtml; + } + } + catch + { + // Fall back to null if remote fetch fails + } + } + + return null; + } + public async Task DeletePost(string id) { if (IsOnline) @@ -273,17 +308,13 @@ public async Task DeletePost(string id) } } - public Task SaveReadingNotesMarkdown(string markdown, string number) => apiClient.SaveReadingNotesMarkdown(markdown, number); - - public Task GetPostHtmlAsync(string postId) - => localHtmlStorageService.GetPostHtmlAsync(postId); - public Task SyncAsync() => syncService.SyncAsync(); public event EventHandler? SyncProgressChanged { add => syncService.SyncProgressChanged += value; remove => syncService.SyncProgressChanged -= value; } + public bool IsSyncing => syncService.IsSyncing; public bool IsOffline => connectivity.NetworkAccess != NetworkAccess.Internet; public bool CanSync => true; diff --git a/src/NoteBookmark.MauiApp/Data/SyncService.cs b/src/NoteBookmark.MauiApp/Data/SyncService.cs index 495b749..a82698b 100644 --- a/src/NoteBookmark.MauiApp/Data/SyncService.cs +++ b/src/NoteBookmark.MauiApp/Data/SyncService.cs @@ -27,17 +27,39 @@ public class SyncService( ILocalHtmlStorageService localHtmlStorageService) : ISyncService { private const string LastSyncTimestampKey = "LastSyncTimestamp"; - private bool _isSyncing; + private readonly object _syncLock = new(); + private Task? _currentSyncTask; + + public bool IsSyncing + { + get + { + lock (_syncLock) + { + return _currentSyncTask != null && !_currentSyncTask.IsCompleted; + } + } + } - public bool IsSyncing => _isSyncing; public event EventHandler? ConflictDetected; public event EventHandler? SyncProgressChanged; - public async Task SyncAsync() + public Task SyncAsync() { - if (_isSyncing) return; + lock (_syncLock) + { + if (_currentSyncTask != null && !_currentSyncTask.IsCompleted) + { + return _currentSyncTask; + } - _isSyncing = true; + _currentSyncTask = DoSyncAsync(); + return _currentSyncTask; + } + } + + private async Task DoSyncAsync() + { try { SyncProgressChanged?.Invoke(this, new SyncProgressEventArgs(0, 0, "Starting synchronization...")); @@ -57,11 +79,13 @@ public async Task SyncAsync() await SyncHtmlAsync(); await SetPreferenceAsync(LastSyncTimestampKey, DateTime.UtcNow.ToString("O")); - SyncProgressChanged?.Invoke(this, new SyncProgressEventArgs(0, 0, "Synchronization complete!")); + SyncProgressChanged?.Invoke(this, new SyncProgressEventArgs(0, 0, "Synchronization complete!", isComplete: true)); } - finally + catch (Exception ex) { - _isSyncing = false; + logger.LogError(ex, "Synchronization failed."); + SyncProgressChanged?.Invoke(this, new SyncProgressEventArgs(0, 0, $"Sync failed: {ex.Message}", isComplete: true)); + throw; } } @@ -191,6 +215,8 @@ private async Task PullAsync(DateTime? lastSync) // 2. Any post that was deleted on the online database while offline should be deleted locally. var localPosts = await localDataService.GetPostsAsync() ?? new List(); + var localPostMap = localPosts.ToDictionary(p => p.Id ?? p.RowKey); + foreach (var localPost in localPosts) { var id = localPost.Id ?? localPost.RowKey; @@ -198,26 +224,97 @@ private async Task PullAsync(DateTime? lastSync) { await localDataService.DeletePostAsync(id, isPendingSync: false); await localDataService.MarkSyncedAsync(id, isPost: true); + localPostMap.Remove(id); } } // 3. Pull new/modified posts + var postsToPull = new List(); foreach (var remotePostL in allRemotePosts) { var id = remotePostL.Id ?? remotePostL.RowKey; - var localPost = await localDataService.GetPostAsync(id); - if (localPost is null || remotePostL.DateModified > localPost.DateModified) + if (!localPostMap.TryGetValue(id, out var lp)) + { + lp = await localDataService.GetPostAsync(id); + } + + if (lp is null || remotePostL.DateModified > lp.DateModified) + { + postsToPull.Add(remotePostL); + } + } + + if (postsToPull.Count > 0) + { + SyncProgressChanged?.Invoke(this, new SyncProgressEventArgs(0, postsToPull.Count, $"Pulling 0 of {postsToPull.Count} posts...")); + + for (int i = 0; i < postsToPull.Count; i++) { - var fullPost = await apiClient.GetPost(id); - if (fullPost is not null) + var remotePostL = postsToPull[i]; + var id = remotePostL.Id ?? remotePostL.RowKey; + + Post postToSave; + if (remotePostL.is_read == true) { - await localDataService.SavePostAsync(fullPost, isPendingSync: false); + postToSave = new Post + { + Id = id, + RowKey = remotePostL.RowKey, + PartitionKey = remotePostL.PartitionKey, + Title = remotePostL.Title, + Url = remotePostL.Url, + Date_published = remotePostL.Date_published, + Excerpt = remotePostL.Excerpt, + is_read = remotePostL.is_read, + DateModified = remotePostL.DateModified + }; } + else + { + try + { + var fullPost = await apiClient.GetPost(id); + postToSave = fullPost ?? new Post + { + Id = id, + RowKey = remotePostL.RowKey, + PartitionKey = remotePostL.PartitionKey, + Title = remotePostL.Title, + Url = remotePostL.Url, + Date_published = remotePostL.Date_published, + Excerpt = remotePostL.Excerpt, + is_read = remotePostL.is_read, + DateModified = remotePostL.DateModified + }; + } + catch (Exception ex) + { + logger.LogWarning(ex, "Failed to retrieve full post for {PostId}, saving summary metadata", id); + postToSave = new Post + { + Id = id, + RowKey = remotePostL.RowKey, + PartitionKey = remotePostL.PartitionKey, + Title = remotePostL.Title, + Url = remotePostL.Url, + Date_published = remotePostL.Date_published, + Excerpt = remotePostL.Excerpt, + is_read = remotePostL.is_read, + DateModified = remotePostL.DateModified + }; + } + } + + await localDataService.SavePostAsync(postToSave, isPendingSync: false); + localPostMap[id] = postToSave; + + int current = i + 1; + SyncProgressChanged?.Invoke(this, new SyncProgressEventArgs(current, postsToPull.Count, $"Pulling {current} of {postsToPull.Count} posts...")); } } // 4. Pull notes modified since lastSync - var remoteNotes = await apiClient.GetNotesModifiedAfter(lastSync ?? DateTime.MinValue); + var remoteNotes = await apiClient.GetNotesModifiedAfter(lastSync ?? DateTime.MinValue) ?? new List(); if (remoteNotes.Any()) { var pendingNotes = await localDataService.GetPendingSyncNotesAsync(); diff --git a/src/NoteBookmark.SharedUI/Components/Pages/Posts.razor b/src/NoteBookmark.SharedUI/Components/Pages/Posts.razor index 0dbebea..bcf310b 100644 --- a/src/NoteBookmark.SharedUI/Components/Pages/Posts.razor +++ b/src/NoteBookmark.SharedUI/Components/Pages/Posts.razor @@ -21,10 +21,10 @@ @if (client.CanSync) { - Sync + Sync } - @if (isSyncing || !string.IsNullOrEmpty(syncProgressStatus)) + @if (isSyncing || client.IsSyncing || !string.IsNullOrEmpty(syncProgressStatus)) { @@ -126,9 +126,6 @@ finally { isSyncing = false; - syncProgressStatus = string.Empty; - syncProgressCurrent = 0; - syncProgressTotal = 0; StateHasChanged(); } } @@ -279,7 +276,7 @@ private async Task SyncNow() { - if (isSyncing) return; + if (isSyncing || client.IsSyncing) return; isSyncing = true; try { @@ -295,9 +292,6 @@ finally { isSyncing = false; - syncProgressStatus = string.Empty; - syncProgressCurrent = 0; - syncProgressTotal = 0; StateHasChanged(); } } @@ -311,12 +305,28 @@ private void OnSyncProgressChanged(object? sender, SyncProgressEventArgs e) { - InvokeAsync(() => + InvokeAsync(async () => { syncProgressCurrent = e.Current; syncProgressTotal = e.Total; syncProgressStatus = e.Status; StateHasChanged(); + + if (e.IsComplete) + { + isSyncing = false; + await LoadPosts(); + StateHasChanged(); + + await Task.Delay(2500); + if (!client.IsSyncing && syncProgressStatus == e.Status) + { + syncProgressStatus = string.Empty; + syncProgressCurrent = 0; + syncProgressTotal = 0; + StateHasChanged(); + } + } }); } diff --git a/src/NoteBookmark.SharedUI/IDataService.cs b/src/NoteBookmark.SharedUI/IDataService.cs index 0de89de..b5a0887 100644 --- a/src/NoteBookmark.SharedUI/IDataService.cs +++ b/src/NoteBookmark.SharedUI/IDataService.cs @@ -26,6 +26,7 @@ public interface IDataService Task GetPostHtmlAsync(string postId); Task SyncAsync(); event System.EventHandler? SyncProgressChanged; + bool IsSyncing { get; } bool IsOffline { get; } bool CanSync { get; } } diff --git a/src/NoteBookmark.SharedUI/PostNoteClient.cs b/src/NoteBookmark.SharedUI/PostNoteClient.cs index 4e3a493..b0245a8 100644 --- a/src/NoteBookmark.SharedUI/PostNoteClient.cs +++ b/src/NoteBookmark.SharedUI/PostNoteClient.cs @@ -210,6 +210,7 @@ public async Task> GetNotesModifiedAfter(DateTime modifiedAfter) public Task SyncAsync() => Task.CompletedTask; public event EventHandler? SyncProgressChanged { add { } remove { } } + public bool IsSyncing => false; public bool IsOffline => false; public bool CanSync => false; }