diff --git a/.github/workflows/release.yml b/.github/workflows/release.yml index 3ef1ab6c..c7d8356e 100644 --- a/.github/workflows/release.yml +++ b/.github/workflows/release.yml @@ -359,6 +359,7 @@ jobs: class ${FORMULA_CLASS} < Formula desc "Clipboard history manager" homepage "https://github.com/${{ github.repository }}" + license "GPL-3.0-only" version "${VERSION}" on_linux do diff --git a/app/lib/shell/focus_manager.dart b/app/lib/shell/focus_manager.dart index a1624016..4b04d435 100644 --- a/app/lib/shell/focus_manager.dart +++ b/app/lib/shell/focus_manager.dart @@ -42,6 +42,14 @@ typedef _AttachThreadInputNative = typedef _AttachThreadInputDart = int Function(int idAttach, int idAttachTo, int fAttach); +typedef _GetGUIThreadInfoNative = + Int32 Function(Uint32 idThread, Pointer lpgui); +typedef _GetGUIThreadInfoDart = + int Function(int idThread, Pointer lpgui); + +typedef _GetAncestorNative = IntPtr Function(IntPtr hWnd, Uint32 gaFlags); +typedef _GetAncestorDart = int Function(int hWnd, int gaFlags); + class _Win32 { _Win32._() { assert(Platform.isWindows, '_Win32 requires Windows'); @@ -52,6 +60,13 @@ class _Win32 { static const int swRestore = 9; static const int gwlStyle = -16; static const int wsMinimize = 0x20000000; + static const int gaRoot = 2; + + // GUITHREADINFO on 64-bit: cbSize+flags (8 bytes) then six HWNDs and a RECT. + // hwndFocus is the second handle. Flutter dropped 32-bit Windows, so the + // pointer width these offsets assume cannot change under us. + static const int guiThreadInfoSize = 72; + static const int guiThreadInfoFocusOffset = 16; late final _user32 = DynamicLibrary.open('user32.dll'); late final _kernel32 = DynamicLibrary.open('kernel32.dll'); @@ -94,6 +109,12 @@ class _Win32 { .lookupFunction<_AttachThreadInputNative, _AttachThreadInputDart>( 'AttachThreadInput', ); + late final getGUIThreadInfo = _user32 + .lookupFunction<_GetGUIThreadInfoNative, _GetGUIThreadInfoDart>( + 'GetGUIThreadInfo', + ); + late final getAncestor = _user32 + .lookupFunction<_GetAncestorNative, _GetAncestorDart>('GetAncestor'); } class WindowFocusManager { @@ -166,6 +187,13 @@ class WindowFocusManager { } await Future.delayed(Duration(milliseconds: delayBeforePasteMs)); + final focusRoot = _keyboardFocusRoot(); + if (focusRoot != _previousWindow) { + AppLogger.warn( + 'Paste target is active but lacks keyboard focus: ' + 'expected=$_previousWindow, focused=$focusRoot', + ); + } final inputResponse = await _simulatePasteWindows(); if (!inputResponse.success) return inputResponse; AppLogger.info( @@ -222,6 +250,19 @@ class WindowFocusManager { return false; } + // Hiding the panel already hands the foreground back in the common case. + // Attaching input queues when the destination owns it anyway is not free: + // detaching resets the keyboard focus Windows had just restored, so the + // window stays active but the synthetic Ctrl+V lands nowhere. AppWindow's + // own activation path skips the juggling for the same reason. + if (w.getForegroundWindow() == _previousWindow) { + AppLogger.info( + 'Focus restore: destination already in foreground ' + '(hwnd=$_previousWindow)', + ); + return true; + } + final currentThreadId = w.getCurrentThreadId(); var attached = false; @@ -249,12 +290,42 @@ class WindowFocusManager { Future _waitForFocusWindows(int maxAttempts) async { final w = _Win32.instance; for (var i = 0; i < maxAttempts; i++) { - if (w.getForegroundWindow() == _previousWindow) return true; + if (w.getForegroundWindow() == _previousWindow) { + if (i > 0) { + AppLogger.info('Focus verify: destination active after $i retries'); + } + return true; + } await Future.delayed(const Duration(milliseconds: 10)); } return false; } + /// Root window currently owning keyboard focus, or 0 when it cannot be read. + /// + /// [_waitForFocusWindows] only proves the destination is the active + /// top-level window. Chromium-based apps activate long before their render + /// process takes keyboard focus, and a stale input-queue attachment can + /// leave a window active with no focus at all — both swallow the Ctrl+V + /// while every call in the paste path still reports success. + int _keyboardFocusRoot() { + final w = _Win32.instance; + final info = calloc(_Win32.guiThreadInfoSize); + try { + info.cast().value = _Win32.guiThreadInfoSize; + if (w.getGUIThreadInfo(0, info) == 0) return 0; + final focused = (info + _Win32.guiThreadInfoFocusOffset) + .cast() + .value; + return focused == 0 ? 0 : w.getAncestor(focused, _Win32.gaRoot); + } catch (e) { + AppLogger.warn('Keyboard focus probe failed: $e'); + return 0; + } finally { + calloc.free(info); + } + } + Future _simulatePasteWindows() async { try { final response = await WindowsHotkeyChannel.sendPaste(); diff --git a/app/lib/shell/single_instance.dart b/app/lib/shell/single_instance.dart index 4a023f6e..2094e90c 100644 --- a/app/lib/shell/single_instance.dart +++ b/app/lib/shell/single_instance.dart @@ -178,11 +178,23 @@ const int _genericWrite = 0x40000000; const int _openExisting = 3; const int _invalidHandleValue = -1; -const String _pipeName = r'\\.\pipe\CopyPasteSingleInstance'; +const String _pipeNameBase = r'\\.\pipe\CopyPasteSingleInstance'; class SingleInstance { - static const String _mutexName = r'Local\CopyPaste_SingleInstance_Mutex'; - static const String _wakeupFileName = 'copypaste.wakeup'; + static const String _mutexNameBase = r'Local\CopyPaste_SingleInstance_Mutex'; + static const String _wakeupFileNameBase = 'copypaste.wakeup'; + + /// Suffix for every OS-global name this class owns. + /// + /// Tests must set it. The production names are shared with any CopyPaste + /// already running on the machine, which holds the mutex and drains the + /// wakeup signals the suite asserts on — so leaving it empty makes the + /// tests fail on exactly the developer machines that use the app. + static String namespace = ''; + + static String get _mutexName => '$_mutexNameBase$namespace'; + static String get _wakeupFileName => '$_wakeupFileNameBase$namespace'; + static String get _pipeName => '$_pipeNameBase$namespace'; static int _mutexHandle = 0; static RandomAccessFile? _lockFile; @@ -308,7 +320,7 @@ class SingleInstance { // Also keep file-based polling as safety net _listenForWakeupFile(onWakeup); - Isolate.spawn(_pipeServerLoop, _pipeReceivePort!.sendPort) + Isolate.spawn(_pipeServerLoop, (_pipeReceivePort!.sendPort, _pipeName)) .then((isolate) { _pipeIsolate = isolate; }) @@ -319,7 +331,8 @@ class SingleInstance { /// Runs in a dedicated isolate. Blocks on ConnectNamedPipe waiting for /// second-instance clients, then reads their message and forwards it. - static void _pipeServerLoop(SendPort sendPort) { + static void _pipeServerLoop((SendPort, String) args) { + final (sendPort, pipeName) = args; final kernel32 = DynamicLibrary.open('kernel32.dll'); final createNamedPipe = kernel32 .lookupFunction<_CreateNamedPipeWNative, _CreateNamedPipeWDart>( @@ -342,7 +355,9 @@ class SingleInstance { .lookupFunction<_GetLastErrorNative, _GetLastErrorDart>('GetLastError'); while (true) { - final name = _pipeName.toNativeUtf16(); + // Statics do not cross isolate boundaries, so the name travels as an + // argument instead of being read from `namespace` again. + final name = pipeName.toNativeUtf16(); final hPipe = createNamedPipe( name, _pipeAccessInbound, diff --git a/app/test/services/release_manifest_service_test.dart b/app/test/services/release_manifest_service_test.dart index ab22019d..f846d4fd 100644 --- a/app/test/services/release_manifest_service_test.dart +++ b/app/test/services/release_manifest_service_test.dart @@ -446,10 +446,15 @@ void main() { 'https://example.com/fail.sig'; final emitted = []; - final sub = ReleaseManifestService.stream.listen(emitted.add); + // Waiting a fixed slice raced the failing fetch under a loaded suite. + final firstEmit = Completer(); + final sub = ReleaseManifestService.stream.listen((state) { + emitted.add(state); + if (!firstEmit.isCompleted) firstEmit.complete(); + }); await ReleaseManifestService.initialize(storageConfigDir: tmpDir.path); - await Future.delayed(const Duration(milliseconds: 100)); + await firstEmit.future.timeout(const Duration(seconds: 10)); unawaited(sub.cancel()); diff --git a/app/test/shell/single_instance_test.dart b/app/test/shell/single_instance_test.dart index cbcf3310..3cffbf36 100644 --- a/app/test/shell/single_instance_test.dart +++ b/app/test/shell/single_instance_test.dart @@ -5,7 +5,13 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:copypaste/shell/single_instance.dart'; -String get _wakeupFilePath => '${Directory.systemTemp.path}/copypaste.wakeup'; +// Keeps the mutex, pipe and wakeup file off the names a CopyPaste running on +// this machine already owns; without it that instance holds the mutex and +// drains the wakeup signals these tests assert on. +final String _namespace = '_test_$pid'; + +String get _wakeupFilePath => + '${Directory.systemTemp.path}/copypaste.wakeup$_namespace'; void _cleanupWakeupFile() { try { @@ -14,6 +20,8 @@ void _cleanupWakeupFile() { } void main() { + setUpAll(() => SingleInstance.namespace = _namespace); + group('SingleInstance – Windows', () { setUp(() { if (!Platform.isWindows) return; diff --git a/core/lib/config/app_config.dart b/core/lib/config/app_config.dart index cc06e637..77d9345d 100644 --- a/core/lib/config/app_config.dart +++ b/core/lib/config/app_config.dart @@ -59,8 +59,13 @@ class AppConfig { this.lastWindowY, }); - factory AppConfig.fromJson(Map json) { - final defaults = defaultForCurrentPlatform(); + /// [platform] overrides the host OS so migrations can be exercised off the + /// platform they target; coverage runs on Linux, where the Windows branches + /// would otherwise never execute. + factory AppConfig.fromJson(Map json, {String? platform}) { + final os = platform ?? Platform.operatingSystem; + final isWindows = os == 'windows'; + final defaults = defaultForPlatform(os); final hotkeyUseCtrl = json['hotkeyUseCtrl'] as bool? ?? defaults.hotkeyUseCtrl; final hotkeyUseWin = json['hotkeyUseWin'] as bool? ?? defaults.hotkeyUseWin; @@ -128,7 +133,7 @@ class AppConfig { // to Ctrl+Alt+V. Revert only that exact automatic binding; version 1 // custom bindings and every other versioned combination remain untouched. final versionTwoWindowsOpen = - Platform.isWindows && + isWindows && shortcutDefaultsVersion == 2 && hotkeyUseCtrl && !hotkeyUseWin && @@ -147,7 +152,7 @@ class AppConfig { // those two exact automatic Windows bindings to Ctrl+Alt+V; bindings from // versions where they could have been user-defined remain untouched. final legacyWindowsPlainPaste = - Platform.isWindows && + isWindows && plainPasteHotkeyUseCtrl && !plainPasteHotkeyUseWin && plainPasteHotkeyVirtualKey == 0x56 && @@ -176,23 +181,24 @@ class AppConfig { final storedPasteDefaultsVersion = json['pasteDefaultsVersion'] as int? ?? 1; - // The previous Windows default always waited 100 ms before restoring - // focus and another 180 ms before sending Ctrl+V. Native focus - // verification now makes that fixed delay unnecessary. Migrate only the - // exact former default tuple so independently tuned values are preserved. - final legacyWindowsSafePaste = - Platform.isWindows && + // v2 moved Windows onto the Instant preset, assuming native focus + // verification made fixed delays unnecessary. It does not: the check only + // proves the destination is the active top-level window, so the paste can + // still outrun apps that route keyboard focus internally. Undo it for + // anyone left on those exact values; tuned tuples are preserved. + final untouchedInstantPaste = + isWindows && storedPasteDefaultsVersion < pasteDefaultsVersion && - duplicateIgnoreWindowMs == 450 && - delayBeforeFocusMs == 100 && - delayBeforePasteMs == 180 && + duplicateIgnoreWindowMs == 300 && + delayBeforeFocusMs == 0 && + delayBeforePasteMs == 20 && maxFocusVerifyAttempts == 15; - if (legacyWindowsSafePaste) { - duplicateIgnoreWindowMs = 300; - delayBeforeFocusMs = 0; - delayBeforePasteMs = 20; - maxFocusVerifyAttempts = 15; - AppLogger.info('Updated Windows paste timing to the Instant preset'); + if (untouchedInstantPaste) { + duplicateIgnoreWindowMs = 350; + delayBeforeFocusMs = 80; + delayBeforePasteMs = 120; + maxFocusVerifyAttempts = 12; + AppLogger.info('Updated Windows paste timing to the Normal preset'); } return AppConfig( @@ -289,7 +295,7 @@ class AppConfig { } static const int shortcutDefaultsVersion = 5; - static const int pasteDefaultsVersion = 2; + static const int pasteDefaultsVersion = 3; static AppConfig defaultForCurrentPlatform() => defaultForPlatform(Platform.operatingSystem); @@ -309,10 +315,10 @@ class AppConfig { plainPasteHotkeyUseCtrl: true, plainPasteHotkeyUseAlt: true, plainPasteHotkeyUseShift: false, - duplicateIgnoreWindowMs: 300, - delayBeforeFocusMs: 0, - delayBeforePasteMs: 20, - maxFocusVerifyAttempts: 15, + duplicateIgnoreWindowMs: 350, + delayBeforeFocusMs: 80, + delayBeforePasteMs: 120, + maxFocusVerifyAttempts: 12, ), // Control+Shift+V opens the panel. The optional global plain-paste binding // includes every modifier and stays disabled until explicitly enabled. diff --git a/core/lib/services/cleanup_service.dart b/core/lib/services/cleanup_service.dart index a5934d0e..570a3159 100644 --- a/core/lib/services/cleanup_service.dart +++ b/core/lib/services/cleanup_service.dart @@ -15,9 +15,11 @@ class CleanupService { StorageConfig? storage, int Function()? getKeepBrokenDays, int Function()? getImagesQuotaMB, + bool Function(String)? probePath, }) : _storage = storage, _getKeepBrokenDays = getKeepBrokenDays ?? (() => 30), - _getImagesQuotaMB = getImagesQuotaMB ?? (() => 0); + _getImagesQuotaMB = getImagesQuotaMB ?? (() => 0), + _probePath = probePath ?? _probePathOnDisk; static const Duration _checkInterval = Duration(hours: 18); static const String _lastCleanupFileName = 'last_cleanup.txt'; @@ -29,6 +31,7 @@ class CleanupService { int Function() _getKeepBrokenDays; int Function() _getImagesQuotaMB; final StorageConfig? _storage; + final bool Function(String) _probePath; Timer? _timer; bool _disposed = false; @@ -129,9 +132,16 @@ class CleanupService { Future _cleanOrphanImages() async { final storage = _storage; if (storage == null) return; + // Kept apart from the sweep below: tracking walks user-supplied paths that + // can live on flaky volumes, and its failure must not strand orphan files + // on disk forever. try { await _trackBrokenExternalRefs(); + } catch (e) { + AppLogger.error('Broken reference tracking failed: $e'); + } + try { final allImageItems = await _repository.getImagePaths(); final allThumbPaths = await _repository.getThumbPaths(); final canonicalImagesDir = p.canonicalize(storage.imagesPath); @@ -189,7 +199,10 @@ class CleanupService { continue; } - final exists = File(path).existsSync() || Directory(path).existsSync(); + final exists = _pathExists(path); + // Probe failed rather than reported absence: same meaning as an offline + // volume, so leave brokenSince untouched instead of starting the clock. + if (exists == null) continue; if (exists) { if (item.brokenSince != null) { await _repository.update(item.copyWith(brokenSince: null)); @@ -374,6 +387,23 @@ class CleanupService { /// present. When the volume is offline (drive not mounted, NAS down, /// removable disk unplugged), callers should skip purge logic so the user /// does not lose history entries on a temporary disconnection. + static bool _probePathOnDisk(String path) => + File(path).existsSync() || Directory(path).existsSync(); + + /// Whether [path] is on disk, or null when the probe itself failed. + /// + /// An unreachable network share throws instead of reporting absence, and + /// [isVolumePresent] cannot tell the two apart either: its own catch assumes + /// the volume is present, which lands here. + bool? _pathExists(String path) { + try { + return _probePath(path); + } catch (e) { + AppLogger.warn('[CleanupService] path probe failed for "$path": $e'); + return null; + } + } + static bool isVolumePresent(String path) { try { if (Platform.isWindows) { diff --git a/core/lib/services/image_processing_queue.dart b/core/lib/services/image_processing_queue.dart index f030095d..65f537f7 100644 --- a/core/lib/services/image_processing_queue.dart +++ b/core/lib/services/image_processing_queue.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'dart:isolate'; import 'dart:typed_data'; +import 'package:flutter/foundation.dart' show visibleForTesting; import 'package:path/path.dart' as p; import '../models/clipboard_item.dart'; @@ -139,7 +140,7 @@ class ImageProcessingQueue { // Remove BMP fallback now that the final PNG exists. final bmpPath = p.join(job.imagesPath, '${job.item.id}.bmp'); - _deleteOwned(bmpPath, job.imagesPath); + await deleteOwned(bmpPath, job.imagesPath); if (_disposed) return; @@ -172,16 +173,36 @@ class ImageProcessingQueue { } /// Deletes a file only if it is canonically inside [imagesDir]. - static void _deleteOwned(String path, String imagesDir) { + /// + /// [delete] replaces the filesystem call so the retry path can be exercised + /// without depending on OS-specific ways of locking a file. + @visibleForTesting + static Future deleteOwned( + String path, + String imagesDir, { + void Function(File)? delete, + }) async { try { final base = p.canonicalize(imagesDir); final target = p.canonicalize(path); final sep = base.endsWith(p.separator) ? base : '$base${p.separator}'; if (!target.startsWith(sep)) return; final f = File(target); - if (f.existsSync()) f.deleteSync(); + final remove = delete ?? (File file) => file.deleteSync(); + // Windows denies the delete while a scanner or the clipboard watcher + // still holds the handle it just opened. One-shot deletion leaked the + // fallback BMP permanently, so give the handle time to close. + for (var attempt = 0; ; attempt++) { + try { + if (f.existsSync()) remove(f); + return; + } on FileSystemException { + if (attempt == 2) rethrow; + await Future.delayed(const Duration(milliseconds: 150)); + } + } } catch (e) { - AppLogger.warn('[ImageQueue] _deleteOwned failed for "$path": $e'); + AppLogger.warn('[ImageQueue] deleteOwned failed for "$path": $e'); } } } diff --git a/core/test/app_config_test.dart b/core/test/app_config_test.dart index 523c2c79..88e2d8e0 100644 --- a/core/test/app_config_test.dart +++ b/core/test/app_config_test.dart @@ -126,10 +126,10 @@ void main() { expect(windows.plainPasteHotkeyUseCtrl, isTrue); expect(windows.plainPasteHotkeyUseAlt, isTrue); expect(windows.plainPasteHotkeyUseShift, isFalse); - expect(windows.duplicateIgnoreWindowMs, 300); - expect(windows.delayBeforeFocusMs, 0); - expect(windows.delayBeforePasteMs, 20); - expect(windows.maxFocusVerifyAttempts, 15); + expect(windows.duplicateIgnoreWindowMs, 350); + expect(windows.delayBeforeFocusMs, 80); + expect(windows.delayBeforePasteMs, 120); + expect(windows.maxFocusVerifyAttempts, 12); final macos = AppConfig.defaultForPlatform('macos'); expect(macos.hotkeyUseCtrl, isTrue); @@ -289,31 +289,44 @@ void main() { } }); - test('legacy Windows Safe timing migrates to Instant', () { - if (!Platform.isWindows) return; + test('untouched Instant timing migrates to Normal', () { + final restored = AppConfig.fromJson({ + 'pasteDefaultsVersion': 2, + 'duplicateIgnoreWindowMs': 300, + 'delayBeforeFocusMs': 0, + 'delayBeforePasteMs': 20, + 'maxFocusVerifyAttempts': 15, + }, platform: 'windows'); + + expect(restored.duplicateIgnoreWindowMs, 350); + expect(restored.delayBeforeFocusMs, 80); + expect(restored.delayBeforePasteMs, 120); + expect(restored.maxFocusVerifyAttempts, 12); + }); + + test('legacy Safe timing is no longer forced onto Instant', () { final restored = AppConfig.fromJson({ 'pasteDefaultsVersion': 1, 'duplicateIgnoreWindowMs': 450, 'delayBeforeFocusMs': 100, 'delayBeforePasteMs': 180, 'maxFocusVerifyAttempts': 15, - }); + }, platform: 'windows'); - expect(restored.duplicateIgnoreWindowMs, 300); - expect(restored.delayBeforeFocusMs, 0); - expect(restored.delayBeforePasteMs, 20); + expect(restored.duplicateIgnoreWindowMs, 450); + expect(restored.delayBeforeFocusMs, 100); + expect(restored.delayBeforePasteMs, 180); expect(restored.maxFocusVerifyAttempts, 15); }); test('legacy custom Windows timing is preserved', () { - if (!Platform.isWindows) return; final restored = AppConfig.fromJson({ 'pasteDefaultsVersion': 1, 'duplicateIgnoreWindowMs': 451, 'delayBeforeFocusMs': 100, 'delayBeforePasteMs': 180, 'maxFocusVerifyAttempts': 15, - }); + }, platform: 'windows'); expect(restored.duplicateIgnoreWindowMs, 451); expect(restored.delayBeforeFocusMs, 100); @@ -329,10 +342,10 @@ void main() { File(path).writeAsStringSync( jsonEncode({ 'shortcutDefaultsVersion': AppConfig.shortcutDefaultsVersion, - 'pasteDefaultsVersion': 1, - 'duplicateIgnoreWindowMs': 450, - 'delayBeforeFocusMs': 100, - 'delayBeforePasteMs': 180, + 'pasteDefaultsVersion': 2, + 'duplicateIgnoreWindowMs': 300, + 'delayBeforeFocusMs': 0, + 'delayBeforePasteMs': 20, 'maxFocusVerifyAttempts': 15, }), ); @@ -341,14 +354,14 @@ void main() { final persisted = jsonDecode(File(path).readAsStringSync()) as Map; - expect(restored.delayBeforeFocusMs, 0); - expect(restored.delayBeforePasteMs, 20); + expect(restored.delayBeforeFocusMs, 80); + expect(restored.delayBeforePasteMs, 120); expect( persisted['pasteDefaultsVersion'], AppConfig.pasteDefaultsVersion, ); - expect(persisted['delayBeforeFocusMs'], 0); - expect(persisted['delayBeforePasteMs'], 20); + expect(persisted['delayBeforeFocusMs'], 80); + expect(persisted['delayBeforePasteMs'], 120); } finally { dir.deleteSync(recursive: true); } diff --git a/core/test/cleanup_service_orphan_test.dart b/core/test/cleanup_service_orphan_test.dart index 5a650409..53d9ddf2 100644 --- a/core/test/cleanup_service_orphan_test.dart +++ b/core/test/cleanup_service_orphan_test.dart @@ -116,6 +116,35 @@ void main() { expect(orphan.existsSync(), isFalse); }); + test('unreachable external path never starts the purge clock', () async { + // An offline network share throws instead of reporting absence. Treating + // that as "file missing" would mark a healthy item broken, and letting + // the throw escape used to abort the orphan sweep entirely. + await repo.save( + ClipboardItem( + content: p.join(tempDir.path, 'offline_share', 'shot.png'), + type: ClipboardContentType.image, + contentHash: 'hash-external', + ), + ); + final orphan = File(p.join(storage.imagesPath, 'orphan_probe.png')) + ..writeAsBytesSync([1, 2, 3]); + + final service = CleanupService( + repo, + () => 30, + storage: storage, + probePath: (_) => throw const FileSystemException('Exists failed'), + ); + service.start(tempDir.path); + await Future.delayed(const Duration(milliseconds: 100)); + service.dispose(); + + final items = await repo.getAll(); + expect(items.single.brokenSince, isNull); + expect(orphan.existsSync(), isFalse); + }); + test('does not crash when images directory is missing', () async { // Remove images directory to simulate missing dir Directory(storage.imagesPath).deleteSync(recursive: true); diff --git a/core/test/cleanup_service_test.dart b/core/test/cleanup_service_test.dart index feeb3b85..200434cd 100644 --- a/core/test/cleanup_service_test.dart +++ b/core/test/cleanup_service_test.dart @@ -288,16 +288,46 @@ void main() { await repoWithPassingClear.close(); }, ); + + test('orphan sweep still runs when broken-ref tracking throws', () async { + final storage = await StorageConfig.create(baseDir: tempDir.path); + await storage.ensureDirectories(); + final orphan = File('${storage.imagesPath}/stranded.png') + ..writeAsBytesSync([1, 2, 3]); + + final inner = SqliteRepository.inMemory(); + final repo = _HybridRepo( + inner, + failGetImagePaths: false, + failGetAll: true, + ); + final service = CleanupService(repo, () => 30, storage: storage); + + service.start(tempDir.path); + await Future.delayed(const Duration(milliseconds: 100)); + service.dispose(); + await inner.close(); + + // Tracking blew up, but the sweep it used to abort completed anyway. + expect(orphan.existsSync(), isFalse); + }); }); } class _HybridRepo implements IClipboardRepository { - _HybridRepo(this._inner); + _HybridRepo( + this._inner, { + this.failGetImagePaths = true, + this.failGetAll = false, + }); final IClipboardRepository _inner; + final bool failGetImagePaths; + final bool failGetAll; @override - Future> getImagePaths() => - Future.error(Exception('forced getImagePaths error')); + Future> getImagePaths() => failGetImagePaths + ? Future.error(Exception('forced getImagePaths error')) + : _inner.getImagePaths(); @override Future> getThumbPaths() => _inner.getThumbPaths(); @@ -322,7 +352,9 @@ class _HybridRepo implements IClipboardRepository { Future findByContentHash(String hash) => _inner.findByContentHash(hash); @override - Future> getAll() => _inner.getAll(); + Future> getAll() => failGetAll + ? Future.error(Exception('forced getAll error')) + : _inner.getAll(); @override Future delete(String id) => _inner.delete(id); @override diff --git a/core/test/image_processing_queue_test.dart b/core/test/image_processing_queue_test.dart index a6913c11..4fddd250 100644 --- a/core/test/image_processing_queue_test.dart +++ b/core/test/image_processing_queue_test.dart @@ -190,4 +190,61 @@ void main() { expect(slowRepo.updates.where((u) => u.id == 'slow'), isEmpty); }); }); + + group('ImageProcessingQueue deleteOwned', () { + late Directory dir; + + setUp(() => dir = Directory.systemTemp.createTempSync('img_delete_')); + tearDown(() => dir.deleteSync(recursive: true)); + + test('retries a locked file and gives up without throwing', () async { + final target = File(p.join(dir.path, 'locked.bmp')) + ..writeAsBytesSync([1]); + var attempts = 0; + + await ImageProcessingQueue.deleteOwned( + target.path, + dir.path, + delete: (_) { + attempts++; + throw const FileSystemException('held by another process'); + }, + ); + + expect(attempts, 3); + expect(target.existsSync(), isTrue); + }); + + test('succeeds when a later attempt gets the handle', () async { + final target = File(p.join(dir.path, 'transient.bmp')) + ..writeAsBytesSync([1]); + var attempts = 0; + + await ImageProcessingQueue.deleteOwned( + target.path, + dir.path, + delete: (file) { + attempts++; + if (attempts == 1) { + throw const FileSystemException('held by another process'); + } + file.deleteSync(); + }, + ); + + expect(attempts, 2); + expect(target.existsSync(), isFalse); + }); + + test('refuses paths outside the images directory', () async { + var called = false; + await ImageProcessingQueue.deleteOwned( + p.join(dir.parent.path, 'outside.bmp'), + dir.path, + delete: (_) => called = true, + ); + + expect(called, isFalse); + }); + }); }