Fix/local history bugs

lib/src/local_history/local_history_comparison_view.dart

@@ -98,8 +98,14 @@ class _LocalHistoryComparisonViewState
       currentSourceVersion: matchingBuffer?.revision ?? cachedNote?.revision,
       currentSource: matchingBuffer?.text ?? cachedNote?.content,
       currentPath: document.currentPath,
+      sourceGeneration: history.comparisonRevision,
+      deleted: document.deleted,
     );
     if (_request != request) {
+      if (_request?.documentId != request.documentId ||
+          _request?.revisionId != request.revisionId) {
+        _recoveryError = null;
+      }
       _request = request;
       _future = _buildSnapshot(document, revision, request);
     }
@@ -134,10 +140,11 @@ class _LocalHistoryComparisonViewState
           onClose: () => ref
               .read(localHistoryControllerProvider.notifier)
               .clearComparison(),
-          onRestoreAll: actionsAreCurrent && data.canRestore
-              ? () => _restore(document, revision)
+          onRestoreAll: !_recovering && actionsAreCurrent && data.canRestore
+              ? () => _restore(document, revision, comparison: data.comparison)
               : null,
-          onRestoreChange: actionsAreCurrent && data.canRestoreChange
+          onRestoreChange:
+              !_recovering && actionsAreCurrent && data.canRestoreChange
               ? (change) => _restore(
                   document,
                   revision,
@@ -146,9 +153,13 @@ class _LocalHistoryComparisonViewState
                 )
               : null,
           onRestoreOriginal:
-              document.remoteNote != null && actionsAreCurrent && data.missing
+              !_recovering &&
+                  document.remoteNote != null &&
+                  actionsAreCurrent &&
+                  data.missing
               ? () => unawaited(_recoverRemote(document, revision))
-              : actionsAreCurrent &&
+              : !_recovering &&
+                    actionsAreCurrent &&
                     data.missing &&
                     (document.currentPath ?? revision.summary.historicalPath) !=
                         null
@@ -159,7 +170,10 @@ class _LocalHistoryComparisonViewState
                 )
               : null,
           onRestoreNewLocation:
-              document.remoteNote == null && actionsAreCurrent && data.missing
+              !_recovering &&
+                  document.remoteNote == null &&
+                  actionsAreCurrent &&
+                  data.missing
               ? () => _chooseRestoreLocation(document, revision)
               : null,
         );
@@ -217,15 +231,53 @@ class _LocalHistoryComparisonViewState
     LocalHistoryRevision revision, {
     SourceComparison? comparison,
     SourceComparisonChange? change,
-  }) async {
-    await ref
-        .read(workspaceControllerProvider.notifier)
-        .restoreLocalHistoryRevision(
-          document: document,
-          revision: revision,
-          comparison: comparison,
-          change: change,
+  }) => _runRestore(() async {
+    final controller = ref.read(workspaceControllerProvider.notifier);
+    final workspace = ref.read(workspaceControllerProvider).workspace;
+    final path = document.currentPath;
+    final needsWorkspaceNavigation =
+        controller.localHistoryBufferForDocument(document, revision) == null &&
+        (document.remoteNote != null
+            ? workspace?.nextcloudAccountId != document.remoteNote!.accountId
+            : path != null &&
+                  (workspace == null ||
+                      workspace.isRemote ||
+                      !(p.equals(workspace.rootPath, path) ||
+                          p.isWithin(workspace.rootPath, path))));
+    if (needsWorkspaceNavigation &&
+        (!await confirmSafeToContinue(context, ref) || !mounted)) {
+      return null;
+    }
+    return controller.restoreLocalHistoryRevision(
+      document: document,
+      revision: revision,
+      comparison: comparison,
+      change: change,
+    );
+  });
+
+  Future<void> _runRestore(Future<bool?> Function() operation) async {
+    if (_recovering) return;
+    setState(() {
+      _recovering = true;
+      _recoveryError = null;
+    });
+    try {
+      final restored = await operation();
+      if (mounted && restored == false) {
+        setState(
+          () => _recoveryError = context.l10n.localHistoryRestoreUnavailable,
         );
+      }
+    } on Object {
+      if (mounted) {
+        setState(
+          () => _recoveryError = context.l10n.localHistoryRestoreUnavailable,
+        );
+      }
+    } finally {
+      if (mounted) setState(() => _recovering = false);
+    }
   }
 
   Future<void> _chooseRestoreLocation(
@@ -298,11 +350,11 @@ class _LocalHistoryComparisonViewState
     LocalHistoryDocument document,
     LocalHistoryRevision revision,
     String path,
-  ) async {
-    if (!await confirmSafeToContinue(context, ref) || !mounted) return;
+  ) => _runRestore(() async {
+    if (!await confirmSafeToContinue(context, ref) || !mounted) return null;
     final controller = ref.read(workspaceControllerProvider.notifier);
     final exists = await controller.localHistoryRestorePathExists(path);
-    if (!mounted) return;
+    if (!mounted) return null;
     var overwrite = false;
     if (exists) {
       overwrite =
@@ -326,15 +378,15 @@ class _LocalHistoryComparisonViewState
             ),
           ) ??
           false;
-      if (!overwrite || !mounted) return;
+      if (!overwrite || !mounted) return null;
     }
-    await controller.restoreMissingLocalHistoryRevision(
+    return controller.restoreMissingLocalHistoryRevision(
       document: document,
       revision: revision,
       destinationPath: path,
       overwriteExisting: overwrite,
     );
-  }
+  });
 }
 
 class _ComparisonRequest {
@@ -346,6 +398,8 @@ class _ComparisonRequest {
     required this.currentSourceVersion,
     required this.currentSource,
     required this.currentPath,
+    required this.sourceGeneration,
+    required this.deleted,
   });
 
   final String documentId;
@@ -355,6 +409,8 @@ class _ComparisonRequest {
   final int? currentSourceVersion;
   final String? currentSource;
   final String? currentPath;
+  final int sourceGeneration;
+  final bool deleted;
 
   @override
   bool operator ==(Object other) =>
@@ -365,7 +421,9 @@ class _ComparisonRequest {
       other.currentBufferId == currentBufferId &&
       other.currentSourceVersion == currentSourceVersion &&
       other.currentSource == currentSource &&
-      other.currentPath == currentPath;
+      other.currentPath == currentPath &&
+      other.sourceGeneration == sourceGeneration &&
+      other.deleted == deleted;
 
   @override
   int get hashCode => Object.hash(
@@ -376,6 +434,8 @@ class _ComparisonRequest {
     currentSourceVersion,
     currentSource,
     currentPath,
+    sourceGeneration,
+    deleted,
   );
 }
 

lib/src/local_history/local_history_controller.dart

@@ -64,6 +64,7 @@ class LocalHistoryState {
   const LocalHistoryState({
     this.snapshot = const LocalHistorySnapshot(),
     this.searchRevision = 0,
+    this.comparisonRevision = 0,
     this.loading = false,
     this.selectedDocumentId,
     this.selectedRevisionId,
@@ -75,6 +76,7 @@ class LocalHistoryState {
 
   final LocalHistorySnapshot snapshot;
   final int searchRevision;
+  final int comparisonRevision;
   final bool loading;
   final String? selectedDocumentId;
   final String? selectedRevisionId;
@@ -95,6 +97,7 @@ class LocalHistoryState {
   LocalHistoryState copyWith({
     LocalHistorySnapshot? snapshot,
     int? searchRevision,
+    int? comparisonRevision,
     bool? loading,
     Object? selectedDocumentId = _unset,
     Object? selectedRevisionId = _unset,
@@ -105,6 +108,7 @@ class LocalHistoryState {
   }) => LocalHistoryState(
     snapshot: snapshot ?? this.snapshot,
     searchRevision: searchRevision ?? this.searchRevision,
+    comparisonRevision: comparisonRevision ?? this.comparisonRevision,
     loading: loading ?? this.loading,
     selectedDocumentId: identical(selectedDocumentId, _unset)
         ? this.selectedDocumentId
@@ -583,7 +587,12 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
 
   Future<bool> refresh() async {
     final generation = ++_loadGeneration;
-    if (ref.mounted) state = state.copyWith(loading: true);
+    if (ref.mounted) {
+      state = state.copyWith(
+        loading: true,
+        comparisonRevision: state.comparisonRevision + 1,
+      );
+    }
     try {
       // Defaults are presentation state while settings hydrate. In particular,
       // the default retention age must not delete history before a longer saved
@@ -706,6 +715,27 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
   void observeHistoryTraversal(
     DocumentBuffer previous,
     DocumentBuffer current,
+  ) => _observeSourceReplacement(
+    previous,
+    current,
+    LocalHistoryCaptureReason.beforeHistoryTraversal,
+  );
+
+  /// Retains each displaced remote version independently of the coalesced
+  /// checkpoint for the incoming source, including rapid successive refreshes.
+  void observeAuthoritativeReplacement(
+    DocumentBuffer previous,
+    DocumentBuffer current,
+  ) => _observeSourceReplacement(
+    previous,
+    current,
+    LocalHistoryCaptureReason.externalChange,
+  );
+
+  void _observeSourceReplacement(
+    DocumentBuffer previous,
+    DocumentBuffer current,
+    LocalHistoryCaptureReason reason,
   ) {
     if (previous.text == current.text ||
         !policy.recordingEnabled ||
@@ -720,8 +750,7 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
     // normal retention may already have evicted their durable revision.
     final existingOwner = _protections.entries.where((entry) {
       final retained = entry.value.snapshot;
-      return entry.value.reason ==
-              LocalHistoryCaptureReason.beforeHistoryTraversal &&
+      return entry.value.reason == reason &&
           _detachedClosedSourceId(entry.key) == previous.id &&
           retained.acceptedClearEpoch == previousSnapshot.acceptedClearEpoch &&
           _sameOptionalPath(retained.path, previousSnapshot.path) &&
@@ -755,7 +784,7 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
       }
     } else {
       final owner = _newDetachedOwner(
-        'history-capture:traversal:${previous.id}',
+        'history-capture:replacement:${previous.id}',
       );
       final snapshot = previousSnapshot
           .forBuffer(owner)
@@ -766,7 +795,7 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
       if (documentId != null) _documentIdsByBuffer[owner] = documentId;
       _protections[owner] = (
         snapshot: snapshot,
-        reason: LocalHistoryCaptureReason.beforeHistoryTraversal,
+        reason: reason,
         force: false,
         ignoreBinding: false,
         allowPathChange: false,
@@ -4164,6 +4193,11 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
   String? documentIdForBuffer(String bufferId) =>
       _documentIdsByBuffer[bufferId];
 
+  /// Invalidates closed-file comparisons after filesystem notifications.
+  void invalidateCurrentSourceComparisons() {
+    state = state.copyWith(comparisonRevision: state.comparisonRevision + 1);
+  }
+
   /// Restores the stable history identity recorded with an editor session.
   ///
   /// The store snapshot is authoritative: stale session metadata cannot bind
@@ -5998,6 +6032,7 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
     final prefix = [
       'history-capture:closed:',
       'history-capture:traversal:',
+      'history-capture:replacement:',
     ].where(owner.startsWith).firstOrNull;
     if (prefix == null) return null;
     var sourceAndSuffix = owner.substring(prefix.length);
@@ -6236,20 +6271,49 @@ class LocalHistoryController extends Notifier<LocalHistoryState> {
             .where((r) => documents.containsKey(r.documentId))
             .toList()
           ..sort((a, b) => b.capturedAt.compareTo(a.capturedAt));
-    for (final summary in summaries) {
+    const batchSize = 32;
+    const maximumBatchBytes = 8 * 1024 * 1024;
+    for (var offset = 0; offset < summaries.length;) {
       if (!current()) return null;
-      // Empty scoped queries can use summaries without reading any bodies.
-      final revision = needle.isEmpty
-          ? null
-          : await _store.readRevision(summary.id);
+      var end = offset + 1;
+      var bytes = summaries[offset].storageBytes;
+      while (end < summaries.length && end - offset < batchSize) {
+        final nextBytes = summaries[end].storageBytes;
+        if (bytes + nextBytes > maximumBatchBytes) break;
+        bytes += nextBytes;
+        end++;
+      }
+      final batch = summaries.sublist(offset, end);
+      offset = end;
+      final store = _store;
+      final batchReader = store is LocalHistoryBatchReader
+          ? store as LocalHistoryBatchReader
+          : null;
+      final revisions = needle.isEmpty || batchReader == null
+          ? <String, LocalHistoryRevision>{}
+          : {
+              for (final revision in await batchReader.readRevisions(
+                batch.map((r) => r.id),
+              ))
+                revision.summary.id: revision,
+            };
       if (!current()) return null;
-      if (needle.isEmpty ||
-          revision?.source.toLowerCase().contains(needle) == true) {
-        matches.add(
-          LocalHistoryRevisionMatch(documents[summary.documentId]!, summary),
-        );
-        if (matches.length > maximumResults) return page(truncated: true);
-        onProgress?.call(page());
+      for (final summary in batch) {
+        // Empty scoped queries can use summaries without reading any bodies.
+        final revision = needle.isEmpty
+            ? null
+            : batchReader != null
+            ? revisions[summary.id]
+            : await store.readRevision(summary.id);
+        if (!current()) return null;
+        if (needle.isEmpty ||
+            revision?.source.toLowerCase().contains(needle) == true) {
+          matches.add(
+            LocalHistoryRevisionMatch(documents[summary.documentId]!, summary),
+          );
+          if (matches.length > maximumResults) return page(truncated: true);
+          onProgress?.call(page());
+        }
       }
     }
     return current() ? page() : null;