From 94e667acb90b989a5da6ee4a87a070b7bd6f16fe Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Thu, 27 Aug 2026 13:10:09 +0530 Subject: [PATCH 1/7] =?UTF-8?q?feat(attachment=5Fengine):=20AttachmentMana?= =?UTF-8?q?ger.prefetch(url)=20=E2=80=94=20cache-warm=20from=20a=20bare=20?= =?UTF-8?q?URL?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a background cache-warming entry point where the caller supplies only a URL (and optionally id/name) — no pre-built Attachment, no render/open needed. Reuses the existing resolve() pipeline end to end (cache-hit check, in-flight dedup, download, cache write), so it automatically inherits every fix from the prior audit pass: 64 KB chunked reads, debounced+durable metadata writes, serialized cache mutations, dispose()/teardown. Best-effort by design: a failed prefetch (network error, 404, etc.) does not throw, but is still reported through the configured AttachmentDiagnosticsSink, same as a failed open(). Caller must pass a stable id when url is a short-lived signed URL — using the URL itself as the cache identity (the default when id is omitted) only works for URLs that don't rotate; a signed URL's cache identity needs to come from somewhere stable, same constraint Attachment.stableIdentity already documents for remoteUrl. Documented in the dartdoc and CHANGELOG. New tests: prefetch() downloads+caches a bare URL; a second prefetch() for an already-cached id is a no-op (no re-download); a failed prefetch() completes without throwing. 184/184 tests pass. flutter analyze clean. license-check and dart pub publish --dry-run clean. Co-Authored-By: Claude Sonnet 5 --- .../attachment_engine/CHANGELOG.md | 9 ++ .../lib/src/manager/attachment_manager.dart | 34 +++++ .../test/attachment_manager_test.dart | 135 ++++++++++++++++++ 3 files changed, 178 insertions(+) diff --git a/packages/attachment_engine/attachment_engine/CHANGELOG.md b/packages/attachment_engine/attachment_engine/CHANGELOG.md index bf013b8..a7b9b00 100644 --- a/packages/attachment_engine/attachment_engine/CHANGELOG.md +++ b/packages/attachment_engine/attachment_engine/CHANGELOG.md @@ -1,5 +1,14 @@ ## 0.0.1-dev.2 +* New `AttachmentManager.prefetch(url, {id, name})` — warms the cache for a + URL in the background without a caller having to build a full + `Attachment` or open/render anything. Best-effort: a failed prefetch + doesn't throw, but is still reported through the configured + `AttachmentDiagnosticsSink`. Reuses the existing resolve pipeline (cache + check → dedup via in-flight registry → download → cache write), so it + gets every fix above for free. **Caller must pass a stable `id` when + `url` is a short-lived signed URL** — otherwise each rotated URL is + treated as new content and never dedupes against a prior prefetch/open. * **Perf**: downloads, previews and offline Office viewers now read files in 64 KB chunks (`RandomAccessFile`) instead of one large synchronous `readAsBytesSync()`/`readAsBytes()` call, keeping peak memory bounded diff --git a/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart b/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart index 774fcbe..ca7561e 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart @@ -10,6 +10,7 @@ import '../download/download_manager.dart'; import '../models/attachment.dart'; import '../models/attachment_capabilities.dart'; import '../models/attachment_failure.dart'; +import '../models/attachment_source.dart'; import '../models/resolved_attachment.dart'; import '../native/native_open_channel.dart'; import '../native/native_share_channel.dart'; @@ -147,6 +148,39 @@ class AttachmentManager { Future retry(Attachment attachment) => _resolveWithDiagnostics(attachment); + /// Warms the cache for [url] in the background — the caller supplies + /// only a URL (and optionally [id]/[name]); no pre-built [Attachment], + /// no UI needs to open it. Already-cached content is a fast no-op (the + /// normal resolve() cache-hit check). Concurrent calls for the same + /// [id] are deduplicated, same as [open]. + /// + /// [id] should be a stable identifier for the underlying content, + /// distinct from [url] itself whenever [url] is a short-lived signed + /// URL — otherwise a later call (whether [prefetch] or [open]) for the + /// same logical file but a freshly-rotated URL won't be recognized as + /// the same cache entry, and this download happens again for nothing. + /// When omitted, [url] itself is used as the identity, which is only + /// correct for URLs that don't rotate. + /// + /// This is genuinely best-effort: a failed prefetch (network error, 404, + /// etc.) does not throw. It's still reported through the configured + /// [AttachmentDiagnosticsSink], same as a failed [open], so failures + /// remain observable without forcing every caller to handle them. + Future prefetch(String url, {String? id, String? name}) async { + final attachment = Attachment( + id: id ?? url, + name: name ?? url, + source: AttachmentSource.url(url), + remoteUrl: url, + ); + try { + await _resolveWithDiagnostics(attachment); + } catch (_) { + // Swallowed deliberately — see dartdoc. _resolveWithDiagnostics + // already reported the failure to diagnostics before rethrowing. + } + } + Future _resolveWithDiagnostics( Attachment attachment, ) async { diff --git a/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart b/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart index 0e466fa..e15e691 100644 --- a/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart +++ b/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart @@ -97,6 +97,100 @@ void main() { ); }); + group('AttachmentManager.prefetch', () { + test('downloads and caches content for a bare URL, with no pre-built ' + 'Attachment and no render/open', () async { + final cacheManager = AttachmentCacheManager( + metadataStore: FileBasedMetadataStore( + directoryProvider: () async => tempDir, + ), + directoryProvider: () async => tempDir, + ); + await cacheManager.init(); + final client = _FakePrefetchDownloadClient(); + final resolver = AttachmentResolver( + cacheManager: cacheManager, + downloadManager: DownloadManager(client: client), + connectivityChecker: _AlwaysOnline(), + ); + final manager = AttachmentManager( + resolver: resolver, + cacheManager: cacheManager, + ); + + await manager.prefetch('https://example.com/report.pdf'); + + expect(client.callCount, 1); + expect( + await cacheManager.lookup( + Attachment( + // Same id the URL was prefetched under (url itself, since no + // explicit id was given) — lookup() keys purely on + // stableIdentity, name/source here are irrelevant. + id: 'https://example.com/report.pdf', + name: 'x', + source: const AttachmentSource.url('https://example.com/x'), + ), + ), + isNotNull, + reason: + 'the URL itself is used as the cache identity when no id ' + 'is supplied', + ); + }); + + test('a second prefetch() for an already-cached URL does not ' + 're-download', () async { + final cacheManager = AttachmentCacheManager( + metadataStore: FileBasedMetadataStore( + directoryProvider: () async => tempDir, + ), + directoryProvider: () async => tempDir, + ); + await cacheManager.init(); + final client = _FakePrefetchDownloadClient(); + final resolver = AttachmentResolver( + cacheManager: cacheManager, + downloadManager: DownloadManager(client: client), + connectivityChecker: _AlwaysOnline(), + ); + final manager = AttachmentManager( + resolver: resolver, + cacheManager: cacheManager, + ); + + await manager.prefetch('https://example.com/report.pdf', id: 'r1'); + await manager.prefetch('https://example.com/report.pdf', id: 'r1'); + + expect(client.callCount, 1); + }); + + test('a failed prefetch() does not throw (best-effort)', () async { + final cacheManager = AttachmentCacheManager( + metadataStore: _NoopStore(), + directoryProvider: () async => tempDir, + ); + await cacheManager.init(); + final resolver = AttachmentResolver( + cacheManager: cacheManager, + // maxRetries: 1 (fail fast) — this test only cares that prefetch() + // doesn't throw, not about retry timing/backoff. + downloadManager: DownloadManager( + client: _AlwaysFailingClient(), + maxRetries: 1, + ), + connectivityChecker: _AlwaysOnline(), + ); + final manager = AttachmentManager( + resolver: resolver, + cacheManager: cacheManager, + ); + + // Must complete without throwing. + await manager.prefetch('https://example.com/broken.bin'); + }); + }); + group('AttachmentManager.initializeDefault re-initialization', () { test('disposes the previous singleton instance before replacing it ' '(no leaked download-progress resources across re-init)', () async { @@ -148,3 +242,44 @@ class _UnusedDownloadClient implements DownloadClient { @override void cancel(Object cancelToken) {} } + +class _AlwaysOnline implements ConnectivityChecker { + @override + Future hasConnection() async => true; +} + +class _FakePrefetchDownloadClient implements DownloadClient { + int callCount = 0; + + @override + Future download( + String url, { + void Function(DownloadProgress progress)? onProgress, + Object? cancelToken, + String? destinationHint, + bool resume = false, + }) async { + callCount++; + return Uint8List.fromList([1, 2, 3, 4]); + } + + @override + Object createCancelToken() => Object(); + @override + void cancel(Object cancelToken) {} +} + +class _AlwaysFailingClient implements DownloadClient { + @override + Future download( + String url, { + void Function(DownloadProgress progress)? onProgress, + Object? cancelToken, + String? destinationHint, + bool resume = false, + }) => throw Exception('simulated network failure'); + @override + Object createCancelToken() => Object(); + @override + void cancel(Object cancelToken) {} +} From 52c0d167c9178bd688a0c684daf4f6c3f58ff506 Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Thu, 27 Aug 2026 13:20:06 +0530 Subject: [PATCH 2/7] fix(attachment_engine): prefetch() returns ResolvedAttachment, not void MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Callers need to know whether a prefetch actually downloaded something or the content was already cached (fromCache), and need the resolved attachment/localPath to show or use the now-cached file elsewhere in the app (e.g. a cached indicator, opening it from a different screen) without triggering a second download. void gave them neither. prefetch() now returns Future — the resolved result on success, null on failure (still best-effort: no throw, still reported to AttachmentDiagnosticsSink). Updated tests assert fromCache is false on first prefetch, true on a second prefetch of the same id, and that the returned localPath actually exists on disk. 184/184 tests pass. flutter analyze clean. license-check and dart pub publish --dry-run clean. Co-Authored-By: Claude Sonnet 5 --- .../attachment_engine/CHANGELOG.md | 19 +++++---- .../lib/src/manager/attachment_manager.dart | 24 ++++++++--- .../test/attachment_manager_test.dart | 41 ++++++++++++++++--- 3 files changed, 67 insertions(+), 17 deletions(-) diff --git a/packages/attachment_engine/attachment_engine/CHANGELOG.md b/packages/attachment_engine/attachment_engine/CHANGELOG.md index a7b9b00..1f13dfd 100644 --- a/packages/attachment_engine/attachment_engine/CHANGELOG.md +++ b/packages/attachment_engine/attachment_engine/CHANGELOG.md @@ -2,13 +2,18 @@ * New `AttachmentManager.prefetch(url, {id, name})` — warms the cache for a URL in the background without a caller having to build a full - `Attachment` or open/render anything. Best-effort: a failed prefetch - doesn't throw, but is still reported through the configured - `AttachmentDiagnosticsSink`. Reuses the existing resolve pipeline (cache - check → dedup via in-flight registry → download → cache write), so it - gets every fix above for free. **Caller must pass a stable `id` when - `url` is a short-lived signed URL** — otherwise each rotated URL is - treated as new content and never dedupes against a prior prefetch/open. + `Attachment` or open/render anything. Returns the `ResolvedAttachment` + on success (`null` on failure) — check `.fromCache` to know whether this + call actually downloaded anything or the content was already cached, + and use `.attachment`/`.localPath` to show, share, or open the + now-cached file anywhere else in the app without a second download. + Best-effort: a failed prefetch doesn't throw, but is still reported + through the configured `AttachmentDiagnosticsSink`. Reuses the existing + resolve pipeline (cache check → dedup via in-flight registry → download + → cache write), so it gets every fix above for free. **Caller must pass + a stable `id` when `url` is a short-lived signed URL** — otherwise each + rotated URL is treated as new content and never dedupes against a prior + prefetch/open. * **Perf**: downloads, previews and offline Office viewers now read files in 64 KB chunks (`RandomAccessFile`) instead of one large synchronous `readAsBytesSync()`/`readAsBytes()` call, keeping peak memory bounded diff --git a/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart b/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart index ca7561e..0a94630 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart @@ -154,6 +154,14 @@ class AttachmentManager { /// normal resolve() cache-hit check). Concurrent calls for the same /// [id] are deduplicated, same as [open]. /// + /// Returns the [ResolvedAttachment] on success — inspect + /// [ResolvedAttachment.fromCache] to know whether this call actually + /// downloaded anything or the content was already cached, and + /// [ResolvedAttachment.localPath]/`.attachment` to display, share, or + /// hand off the now-cached file anywhere else in the app (e.g. a + /// "download complete" indicator, or opening it in a different screen) + /// without triggering a second download. Returns `null` on failure. + /// /// [id] should be a stable identifier for the underlying content, /// distinct from [url] itself whenever [url] is a short-lived signed /// URL — otherwise a later call (whether [prefetch] or [open]) for the @@ -163,10 +171,15 @@ class AttachmentManager { /// correct for URLs that don't rotate. /// /// This is genuinely best-effort: a failed prefetch (network error, 404, - /// etc.) does not throw. It's still reported through the configured - /// [AttachmentDiagnosticsSink], same as a failed [open], so failures - /// remain observable without forcing every caller to handle them. - Future prefetch(String url, {String? id, String? name}) async { + /// etc.) does not throw, it returns `null`. It's still reported through + /// the configured [AttachmentDiagnosticsSink], same as a failed [open], + /// so failures remain observable without forcing every caller to + /// handle them. + Future prefetch( + String url, { + String? id, + String? name, + }) async { final attachment = Attachment( id: id ?? url, name: name ?? url, @@ -174,10 +187,11 @@ class AttachmentManager { remoteUrl: url, ); try { - await _resolveWithDiagnostics(attachment); + return await _resolveWithDiagnostics(attachment); } catch (_) { // Swallowed deliberately — see dartdoc. _resolveWithDiagnostics // already reported the failure to diagnostics before rethrowing. + return null; } } diff --git a/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart b/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart index e15e691..6d92fc3 100644 --- a/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart +++ b/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart @@ -118,9 +118,23 @@ void main() { cacheManager: cacheManager, ); - await manager.prefetch('https://example.com/report.pdf'); + final result = await manager.prefetch('https://example.com/report.pdf'); expect(client.callCount, 1); + expect( + result, + isNotNull, + reason: + 'a successful prefetch must hand back a ResolvedAttachment ' + '(localPath + fromCache) so callers can show/use the now-cached ' + 'file elsewhere without a second download', + ); + expect( + result!.fromCache, + isFalse, + reason: 'this was a fresh download, not a cache hit', + ); + expect(File(result.localPath).existsSync(), isTrue); expect( await cacheManager.lookup( Attachment( @@ -159,10 +173,25 @@ void main() { cacheManager: cacheManager, ); - await manager.prefetch('https://example.com/report.pdf', id: 'r1'); - await manager.prefetch('https://example.com/report.pdf', id: 'r1'); + final first = await manager.prefetch( + 'https://example.com/report.pdf', + id: 'r1', + ); + final second = await manager.prefetch( + 'https://example.com/report.pdf', + id: 'r1', + ); expect(client.callCount, 1); + expect(first!.fromCache, isFalse); + expect( + second!.fromCache, + isTrue, + reason: + 'the second call must be told it was served from cache, so ' + 'a caller can distinguish "just downloaded" from "already had ' + 'it" — e.g. to skip showing a download progress indicator', + ); }); test('a failed prefetch() does not throw (best-effort)', () async { @@ -186,8 +215,10 @@ void main() { cacheManager: cacheManager, ); - // Must complete without throwing. - await manager.prefetch('https://example.com/broken.bin'); + // Must complete without throwing, and signal failure via a null + // result rather than an exception the caller has to catch. + final result = await manager.prefetch('https://example.com/broken.bin'); + expect(result, isNull); }); }); From 8a669f3aaa32c8c17fb058d36ad8e0718860bece Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Thu, 27 Aug 2026 13:49:19 +0530 Subject: [PATCH 3/7] feat(attachment_engine): pin attachments to guarantee offline availability MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit AttachmentManager.pinForOffline(attachment) caches (if needed) and exempts an attachment from every automatic cache-cleanup path — size-cap LRU eviction, clearUnused(), clearExpired() — so a file the user explicitly marked 'save for offline'/'keep downloaded' survives storage pressure instead of silently disappearing under cache limits. unpinFromOffline()/isPinnedForOffline() reverse/query it. Pinning is deliberately scoped: it only exempts a file from *automatic* cleanup, not from an explicit, targeted removal. deleteCache() (and AttachmentCacheManager.clearAttachment()/clearAll()) still remove a pinned file on request — a host app always has a way to actually delete one when it needs to, pinned or not. lookup() now also serves a pinned entry past its remote expiresAt/retention window: those describe when the *remote* source is considered stale, not whether the already-downloaded bytes are usable — and the entire point of pinning is guaranteed offline availability regardless of the remote/network state. New CacheEntry.pinned field (default false; absent in metadata written before this change, read back as false — no migration needed). New tests: CachePolicy skips pinned entries for both size-cap eviction and expiry selection (and documents that enough pinned content can genuinely leave the cache over its cap — deliberate); pin() throws for an uncached attachment; unpin() restores normal eviction eligibility; isPinned() is false for uncached content; clearAttachment() still removes a pinned entry; clearUnused() skips one. Plus AttachmentManager-level pinForOffline()/unpinFromOffline() integration tests through the full write -> pin -> eviction-pressure -> lookup path. 195/195 tests pass. flutter analyze clean. license-check and dart pub publish --dry-run clean. Co-Authored-By: Claude Sonnet 5 --- .../attachment_engine/CHANGELOG.md | 15 +++ .../src/cache/attachment_cache_manager.dart | 78 +++++++++++- .../lib/src/cache/cache_metadata_store.dart | 19 ++- .../lib/src/cache/cache_policy.dart | 13 +- .../lib/src/manager/attachment_manager.dart | 26 +++- .../test/attachment_cache_manager_test.dart | 111 ++++++++++++++++++ .../test/attachment_manager_test.dart | 77 ++++++++++++ .../test/cache_policy_test.dart | 57 +++++++++ 8 files changed, 390 insertions(+), 6 deletions(-) diff --git a/packages/attachment_engine/attachment_engine/CHANGELOG.md b/packages/attachment_engine/attachment_engine/CHANGELOG.md index 1f13dfd..384d3c9 100644 --- a/packages/attachment_engine/attachment_engine/CHANGELOG.md +++ b/packages/attachment_engine/attachment_engine/CHANGELOG.md @@ -1,5 +1,20 @@ ## 0.0.1-dev.2 +* New "keep available offline" pinning: `AttachmentManager.pinForOffline(attachment)` + caches (if needed) and exempts an attachment from every *automatic* + cache-cleanup path — size-cap LRU eviction, `AttachmentCacheManager.clearUnused`, + and `.clearExpired` — so a file the user explicitly marked "save for + offline"/"keep downloaded" survives storage pressure and won't quietly + disappear. `unpinFromOffline`/`isPinnedForOffline` reverse/query it. + Pinning only exempts a file from automatic cleanup, not from an + explicit, targeted removal: `AttachmentManager.deleteCache` (and + `AttachmentCacheManager.clearAttachment`/`.clearAll`) still remove a + pinned file when the host app genuinely needs to. A pinned entry also + stays servable from `lookup()` (cache hit) even past its remote + `expiresAt`/retention window, since a signed URL's expiry describes the + *remote* source, not whether the already-downloaded bytes are usable + offline. New `CacheEntry.pinned` field (defaults to `false`; absent in + metadata written before this release, read back as `false`). * New `AttachmentManager.prefetch(url, {id, name})` — warms the cache for a URL in the background without a caller having to build a full `Attachment` or open/render anything. Returns the `ResolvedAttachment` diff --git a/packages/attachment_engine/attachment_engine/lib/src/cache/attachment_cache_manager.dart b/packages/attachment_engine/attachment_engine/lib/src/cache/attachment_cache_manager.dart index 23716fd..e1a5690 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/cache/attachment_cache_manager.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/cache/attachment_cache_manager.dart @@ -115,7 +115,14 @@ class AttachmentCacheManager { if (!_config.enabled) return null; final entry = await _store.get(attachment.stableIdentity); if (entry == null) return null; - if (entry.isExpired || _isPastRetention(entry)) return null; + // A pinned entry stays servable from cache even past its remote + // expiresAt/retention window: those describe when the *remote* source + // is considered stale, not whether the already-downloaded bytes are + // usable — and "pinned" means the host app deliberately wants this + // file available offline regardless of network/remote-source state. + if (!entry.pinned && (entry.isExpired || _isPastRetention(entry))) { + return null; + } final file = File(entry.localPath); if (!await file.exists()) return null; // Runs on every cache hit (e.g. every tile in a grid resolving its @@ -281,6 +288,10 @@ class AttachmentCacheManager { }); } + /// Removes [attachment]'s cache entry, even if it's [CacheEntry.pinned] — + /// an explicit, targeted delete like this always succeeds; pinning only + /// exempts a file from *automatic* cleanup (see [pin]'s dartdoc), not + /// from a deliberate removal request. Future clearAttachment(Attachment attachment) async { if (!_config.enabled) return; await _serialized(() async { @@ -290,7 +301,9 @@ class AttachmentCacheManager { } /// Clears cache entries that have not been accessed within [unusedFor] - /// (default 30 days). + /// (default 30 days). [CacheEntry.pinned] entries are skipped — see + /// [pin]'s dartdoc; use [clearAttachment] to remove a specific pinned + /// entry regardless, or [unpin] it first. Future clearUnused({ Duration unusedFor = const Duration(days: 30), }) async { @@ -299,13 +312,17 @@ class AttachmentCacheManager { final cutoff = DateTime.now().subtract(unusedFor); final entries = await _store.getAll(); for (final entry in entries.where( - (e) => e.lastAccessedAt.isBefore(cutoff), + (e) => e.lastAccessedAt.isBefore(cutoff) && !e.pinned, )) { await _deleteEntry(entry); } }); } + /// Removes every cache entry, including [CacheEntry.pinned] ones — this + /// is an explicit, whole-cache wipe (e.g. a host app's "Clear cache" + /// settings action or logout), not automatic cleanup, so pinning does + /// not exempt anything from it. Future clearAll() async { if (!_config.enabled) return; await _serialized(() async { @@ -316,6 +333,61 @@ class AttachmentCacheManager { }); } + /// Marks [attachment]'s already-cached content as exempt from automatic + /// cleanup — size-cap LRU eviction, [clearUnused], and [clearExpired] — + /// so the host app can guarantee a file stays available offline (e.g. a + /// user explicitly chose "make available offline"/"keep downloaded"). + /// + /// [attachment] must already be cached — resolve/cache it first (e.g. + /// via `AttachmentManager.open`/`.prefetch`) — otherwise this throws + /// [StateError]. Pinning is only about protecting existing content; + /// it does not itself trigger a download. + /// + /// Pinning does NOT protect against [clearAttachment]/[clearAll]: those + /// remain a deliberate, explicit way to remove (or free up space from) a + /// pinned file when the host app genuinely needs to. + Future pin(Attachment attachment) async { + if (!_config.enabled) return; + await _serialized(() async { + final entry = await _store.get(attachment.stableIdentity); + if (entry == null) { + throw StateError( + 'Cannot pin "${attachment.stableIdentity}": it is not currently ' + 'cached. Resolve/cache it first (e.g. AttachmentManager.open or ' + '.prefetch) before pinning.', + ); + } + if (entry.pinned) return; + await _store.put(entry.copyWith(pinned: true)); + if (_store case final FileBasedMetadataStore fileStore) { + await fileStore.flushPending(); + } + }); + } + + /// Reverses [pin]: [attachment]'s cache entry (if any) becomes eligible + /// for automatic cleanup again. A no-op if [attachment] isn't cached, or + /// isn't currently pinned. + Future unpin(Attachment attachment) async { + if (!_config.enabled) return; + await _serialized(() async { + final entry = await _store.get(attachment.stableIdentity); + if (entry == null || !entry.pinned) return; + await _store.put(entry.copyWith(pinned: false)); + if (_store case final FileBasedMetadataStore fileStore) { + await fileStore.flushPending(); + } + }); + } + + /// Whether [attachment] is currently both cached and [pin]ned. False for + /// content that isn't cached at all. + Future isPinned(Attachment attachment) async { + if (!_config.enabled) return false; + final entry = await _store.get(attachment.stableIdentity); + return entry?.pinned ?? false; + } + /// Releases resources and, for the default [FileBasedMetadataStore], /// forces any debounced-but-not-yet-written metadata update /// ([FileBasedMetadataStore.put]/`delete` both debounce their actual diff --git a/packages/attachment_engine/attachment_engine/lib/src/cache/cache_metadata_store.dart b/packages/attachment_engine/attachment_engine/lib/src/cache/cache_metadata_store.dart index 3c2deea..6da6637 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/cache/cache_metadata_store.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/cache/cache_metadata_store.dart @@ -24,6 +24,7 @@ class CacheEntry { this.attachmentType, this.category = CacheEntryCategory.original, this.checksum, + this.pinned = false, }); final String key; @@ -36,10 +37,21 @@ class CacheEntry { final CacheEntryCategory category; final String? checksum; + /// When true, this entry is exempt from every *automatic* removal path — + /// size-cap LRU eviction ([CachePolicy.selectEntriesToEvict]), + /// [AttachmentCacheManager.clearUnused], and + /// [AttachmentCacheManager.clearExpired] — so a file the host app has + /// deliberately marked "keep available offline" survives cache pressure. + /// It does NOT protect against an explicit, targeted removal: + /// [AttachmentCacheManager.clearAttachment]/[AttachmentCacheManager.clearAll] + /// still remove it, same as any other entry — pinning only opts a file + /// out of automatic cleanup, not out of a deliberate delete. + final bool pinned; + bool get isExpired => expiresAt != null && expiresAt!.isBefore(DateTime.now()); - CacheEntry copyWith({DateTime? lastAccessedAt}) { + CacheEntry copyWith({DateTime? lastAccessedAt, bool? pinned}) { return CacheEntry( key: key, localPath: localPath, @@ -50,6 +62,7 @@ class CacheEntry { attachmentType: attachmentType, category: category, checksum: checksum, + pinned: pinned ?? this.pinned, ); } @@ -63,6 +76,7 @@ class CacheEntry { 'attachmentType': attachmentType, 'category': category.name, 'checksum': checksum, + 'pinned': pinned, }; static CacheEntry fromMap(Map map) { @@ -81,6 +95,9 @@ class CacheEntry { orElse: () => CacheEntryCategory.original, ), checksum: map['checksum'] as String?, + // Absent in metadata written before this field existed — defaults to + // false (not pinned), the pre-existing behavior for every entry. + pinned: map['pinned'] as bool? ?? false, ); } } diff --git a/packages/attachment_engine/attachment_engine/lib/src/cache/cache_policy.dart b/packages/attachment_engine/attachment_engine/lib/src/cache/cache_policy.dart index 58b89ae..4bcdebb 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/cache/cache_policy.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/cache/cache_policy.dart @@ -16,6 +16,13 @@ class CachePolicy { /// evicted (oldest-accessed first) to bring total size at or under /// [maxTotalSizeBytes], optionally after also making room for /// [incomingBytes] of new content. + /// + /// [CacheEntry.pinned] entries are never selected — a file the host app + /// marked "keep available offline" survives cache pressure. Their bytes + /// still count toward the current total, so if enough content is + /// pinned, the cache can end up genuinely over [maxTotalSizeBytes] with + /// nothing left evictable; that's the deliberate tradeoff of pinning, + /// not a bug. List selectEntriesToEvict( List entries, { int incomingBytes = 0, @@ -27,13 +34,17 @@ class CachePolicy { final toEvict = []; for (final entry in sorted) { if (total <= maxTotalSizeBytes) break; + if (entry.pinned) continue; toEvict.add(entry); total -= entry.sizeBytes; } return toEvict; } + /// Entries whose [CacheEntry.expiresAt] has passed — excluding + /// [CacheEntry.pinned] ones, for the same "automatic cleanup never + /// removes a pinned file" reason as [selectEntriesToEvict]. List selectExpired(List entries) { - return entries.where((e) => e.isExpired).toList(); + return entries.where((e) => e.isExpired && !e.pinned).toList(); } } diff --git a/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart b/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart index 0a94630..1d9e1a5 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/manager/attachment_manager.dart @@ -250,10 +250,34 @@ class AttachmentManager { } } - /// Removes any cached copy of [attachment]. + /// Removes any cached copy of [attachment] — including a [pinForOffline]d + /// one; this is a deliberate, explicit delete, not automatic cleanup. Future deleteCache(Attachment attachment) => _cacheManager.clearAttachment(attachment); + /// Ensures [attachment] is cached (resolving/downloading it first if + /// needed, same as [open]) and marks it exempt from automatic cache + /// cleanup — size-cap eviction won't touch it even under storage + /// pressure, so it stays available offline until explicitly removed via + /// [unpinFromOffline] + cleanup, or [deleteCache]. Use this for a + /// user-facing "keep available offline"/"save for offline" action. + Future pinForOffline(Attachment attachment) async { + final resolved = await _resolveWithDiagnostics(attachment); + await _cacheManager.pin(resolved.attachment); + return resolved; + } + + /// Reverses [pinForOffline]: [attachment]'s cached content (if any) + /// becomes eligible for automatic cleanup again. Does not itself delete + /// anything — the file is removed later by ordinary cache pressure, or + /// immediately via [deleteCache]. + Future unpinFromOffline(Attachment attachment) => + _cacheManager.unpin(attachment); + + /// Whether [attachment] is currently cached and [pinForOffline]d. + Future isPinnedForOffline(Attachment attachment) => + _cacheManager.isPinned(attachment); + /// Recomputes capabilities for [attachment] in its current state. AttachmentCapabilities capabilitiesFor(Attachment attachment) { return CapabilityEngine( diff --git a/packages/attachment_engine/attachment_engine/test/attachment_cache_manager_test.dart b/packages/attachment_engine/attachment_engine/test/attachment_cache_manager_test.dart index e38971b..7da7af0 100644 --- a/packages/attachment_engine/attachment_engine/test/attachment_cache_manager_test.dart +++ b/packages/attachment_engine/attachment_engine/test/attachment_cache_manager_test.dart @@ -177,6 +177,117 @@ void main() { expect(await manager.totalSizeBytes(), 3000); // 30 * 100 }); }); + + group('AttachmentCacheManager pinning (offline availability)', () { + test('a pinned attachment survives eviction that would otherwise remove ' + 'it as the least-recently-used entry', () async { + final store = _CountingStore(); + final manager = AttachmentCacheManager( + metadataStore: store, + policy: const CachePolicy(maxTotalSizeBytes: 250), + directoryProvider: () async => tempDir, + ); + await manager.init(); + + final keep = attachment('keep'); + await manager.write(keep, Uint8List(100)); + await manager.pin(keep); + + // Two more writes that, combined with 'keep', would exceed the + // 250-byte cap and normally evict 'keep' first (oldest). + await manager.write(attachment('b'), Uint8List(100)); + await manager.write(attachment('c'), Uint8List(100)); + + expect( + await manager.lookup(keep), + isNotNull, + reason: 'pinned entry must not be evicted despite being oldest', + ); + }); + + test('pin() throws for an attachment that is not cached', () async { + final manager = AttachmentCacheManager( + metadataStore: _CountingStore(), + directoryProvider: () async => tempDir, + ); + await manager.init(); + + expect( + () => manager.pin(attachment('never-cached')), + throwsA(isA()), + ); + }); + + test('unpin() makes a previously-pinned entry evictable again', () async { + final manager = AttachmentCacheManager( + metadataStore: _CountingStore(), + policy: const CachePolicy(maxTotalSizeBytes: 250), + directoryProvider: () async => tempDir, + ); + await manager.init(); + + final a = attachment('a'); + await manager.write(a, Uint8List(100)); + await manager.pin(a); + expect(await manager.isPinned(a), isTrue); + + await manager.unpin(a); + expect(await manager.isPinned(a), isFalse); + + await manager.write(attachment('b'), Uint8List(100)); + await manager.write(attachment('c'), Uint8List(100)); + + expect( + await manager.lookup(a), + isNull, + reason: 'no longer pinned, so ordinary LRU eviction applies to it', + ); + }); + + test('isPinned() is false for content that is not cached at all', () async { + final manager = AttachmentCacheManager( + metadataStore: _CountingStore(), + directoryProvider: () async => tempDir, + ); + await manager.init(); + + expect(await manager.isPinned(attachment('unknown')), isFalse); + }); + + test('clearAttachment() still removes a pinned entry — explicit delete ' + 'always works', () async { + final manager = AttachmentCacheManager( + metadataStore: _CountingStore(), + directoryProvider: () async => tempDir, + ); + await manager.init(); + + final a = attachment('a'); + await manager.write(a, Uint8List(100)); + await manager.pin(a); + + await manager.clearAttachment(a); + + expect(await manager.lookup(a), isNull); + }); + + test('clearUnused() skips a pinned entry even when it is well past the ' + 'unused-for cutoff', () async { + final manager = AttachmentCacheManager( + metadataStore: _CountingStore(), + directoryProvider: () async => tempDir, + ); + await manager.init(); + + final a = attachment('a'); + await manager.write(a, Uint8List(100)); + await manager.pin(a); + + await manager.clearUnused(unusedFor: Duration.zero); + + expect(await manager.lookup(a), isNotNull); + }); + }); } /// Wraps a real in-memory-ish store, counting getAll() calls so tests can diff --git a/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart b/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart index 6d92fc3..41b946e 100644 --- a/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart +++ b/packages/attachment_engine/attachment_engine/test/attachment_manager_test.dart @@ -97,6 +97,83 @@ void main() { ); }); + group('AttachmentManager.pinForOffline', () { + Attachment urlAttachment(String id) => Attachment( + id: id, + name: '$id.bin', + source: AttachmentSource.url('https://example.com/$id'), + ); + + test('caches and pins an attachment so it survives eviction under ' + 'storage pressure', () async { + final cacheManager = AttachmentCacheManager( + metadataStore: FileBasedMetadataStore( + directoryProvider: () async => tempDir, + ), + // The fake download client below returns a fixed 4-byte payload + // per attachment — an 8-byte cap means the 3rd attachment's + // download genuinely forces eviction of an older, non-pinned + // entry. + policy: const CachePolicy(maxTotalSizeBytes: 8), + directoryProvider: () async => tempDir, + ); + await cacheManager.init(); + final resolver = AttachmentResolver( + cacheManager: cacheManager, + downloadManager: DownloadManager(client: _FakePrefetchDownloadClient()), + connectivityChecker: _AlwaysOnline(), + ); + final manager = AttachmentManager( + resolver: resolver, + cacheManager: cacheManager, + ); + + final keep = urlAttachment('keep'); + await manager.pinForOffline(keep); + expect(await manager.isPinnedForOffline(keep), isTrue); + + // Two more downloads that, combined with 'keep', exceed the + // 8-byte cap and would normally evict 'keep' first (oldest). + await manager.open(urlAttachment('b')); + await manager.open(urlAttachment('c')); + + final reopened = await manager.open(keep); + expect( + reopened.fromCache, + isTrue, + reason: + 'pinned entry must still be served from cache, not ' + 're-downloaded because it was evicted', + ); + }); + + test('unpinFromOffline() reverses pinForOffline()', () async { + final cacheManager = AttachmentCacheManager( + metadataStore: FileBasedMetadataStore( + directoryProvider: () async => tempDir, + ), + directoryProvider: () async => tempDir, + ); + await cacheManager.init(); + final resolver = AttachmentResolver( + cacheManager: cacheManager, + downloadManager: DownloadManager(client: _FakePrefetchDownloadClient()), + connectivityChecker: _AlwaysOnline(), + ); + final manager = AttachmentManager( + resolver: resolver, + cacheManager: cacheManager, + ); + + final a = urlAttachment('a'); + await manager.pinForOffline(a); + expect(await manager.isPinnedForOffline(a), isTrue); + + await manager.unpinFromOffline(a); + expect(await manager.isPinnedForOffline(a), isFalse); + }); + }); + group('AttachmentManager.prefetch', () { test('downloads and caches content for a bare URL, with no pre-built ' 'Attachment and no render/open', () async { diff --git a/packages/attachment_engine/attachment_engine/test/cache_policy_test.dart b/packages/attachment_engine/attachment_engine/test/cache_policy_test.dart index 579728d..07f2aab 100644 --- a/packages/attachment_engine/attachment_engine/test/cache_policy_test.dart +++ b/packages/attachment_engine/attachment_engine/test/cache_policy_test.dart @@ -10,6 +10,7 @@ CacheEntry _entry( int size, DateTime lastAccessed, { DateTime? expiresAt, + bool pinned = false, }) { return CacheEntry( key: key, @@ -18,6 +19,7 @@ CacheEntry _entry( createdAt: lastAccessed, lastAccessedAt: lastAccessed, expiresAt: expiresAt, + pinned: pinned, ); } @@ -80,5 +82,60 @@ void main() { expect(expired.map((e) => e.key), ['expired']); }); + + test('never selects a pinned entry for eviction, even as the oldest ' + 'and over the size cap', () { + const policy = CachePolicy(maxTotalSizeBytes: 100); + final now = DateTime(2024, 1, 10); + final entries = [ + _entry( + 'oldest-pinned', + 40, + now.subtract(const Duration(days: 3)), + pinned: true, + ), + _entry('middle', 40, now.subtract(const Duration(days: 2))), + _entry('newest', 40, now.subtract(const Duration(days: 1))), + ]; + + final toEvict = policy.selectEntriesToEvict(entries); + + // 'oldest-pinned' would normally be evicted first (LRU) but is + // exempt; 'middle' is the next-oldest non-pinned entry instead. + expect(toEvict.map((e) => e.key), ['middle']); + }); + + test('a pinned entry can leave the cache genuinely over the cap when ' + 'nothing evictable remains — deliberate, not a bug', () { + const policy = CachePolicy(maxTotalSizeBytes: 10); + final now = DateTime(2024, 1, 10); + final entries = [_entry('only-entry-pinned', 40, now, pinned: true)]; + + expect(policy.selectEntriesToEvict(entries), isEmpty); + }); + + test('selectExpired excludes pinned entries even past expiresAt', () { + const policy = CachePolicy(maxTotalSizeBytes: 1000000); + final now = DateTime.now(); + final entries = [ + _entry( + 'expired-pinned', + 1, + now, + expiresAt: now.subtract(const Duration(days: 1)), + pinned: true, + ), + _entry( + 'expired-unpinned', + 1, + now, + expiresAt: now.subtract(const Duration(days: 1)), + ), + ]; + + final expired = policy.selectExpired(entries); + + expect(expired.map((e) => e.key), ['expired-unpinned']); + }); }); } From 6cb8a1bb781dd6a5acabaf7ff92664fa116a932e Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Mon, 31 Aug 2026 17:22:00 +0530 Subject: [PATCH 4/7] fix(attachment_engine): resolve openOfficePreview on QuickLook dismiss, not presentation - attachment_engine_ios: OfficePreviewChannel now implements QLPreviewControllerDelegate; the openOfficePreview future completes only on previewControllerDidDismiss instead of right after presenting the modal, so callers finally learn when the preview actually closes. - attachment_engine: OfficeAttachmentRenderer gains an onDismissed callback fired once that future resolves, so a host app can react to the QuickLook close (e.g. pop the screen behind it) instead of being left showing a bare placeholder. - Version bumps: attachment_engine 0.0.1-dev.3, attachment_engine_ios 0.0.1-dev.2, with CHANGELOG entries for both. --- .../attachment_engine/CHANGELOG.md | 9 +++++++ .../lib/src/renderers/office_renderer.dart | 18 ++++++++++++- .../attachment_engine/pubspec.yaml | 4 +-- .../test/office_renderer_test.dart | 24 +++++++++++++++++ .../attachment_engine_ios/CHANGELOG.md | 9 +++++++ .../OfficePreviewChannel.swift | 26 ++++++++++++++++++- .../attachment_engine_ios/pubspec.yaml | 2 +- 7 files changed, 87 insertions(+), 5 deletions(-) diff --git a/packages/attachment_engine/attachment_engine/CHANGELOG.md b/packages/attachment_engine/attachment_engine/CHANGELOG.md index 384d3c9..3480ca3 100644 --- a/packages/attachment_engine/attachment_engine/CHANGELOG.md +++ b/packages/attachment_engine/attachment_engine/CHANGELOG.md @@ -1,3 +1,12 @@ +## 0.0.1-dev.3 + +* New `OfficeAttachmentRenderer.onDismissed` callback, invoked once the + user dismisses the iOS QuickLook office-document preview. Requires + `attachment_engine_ios` 0.0.1-dev.2+, whose `openOfficePreview` now + resolves only on the modal's actual dismissal rather than as soon as it + is presented — previously there was no signal at all for a host app to + react to the close (e.g. to pop the screen left behind it). + ## 0.0.1-dev.2 * New "keep available offline" pinning: `AttachmentManager.pinForOffline(attachment)` diff --git a/packages/attachment_engine/attachment_engine/lib/src/renderers/office_renderer.dart b/packages/attachment_engine/attachment_engine/lib/src/renderers/office_renderer.dart index 7ab3158..16e6490 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/renderers/office_renderer.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/renderers/office_renderer.dart @@ -77,6 +77,7 @@ class OfficeAttachmentRenderer extends AttachmentRenderer { this.externalOpenConfig = const ExternalOpenConfig(), this.connectivityChecker = const DefaultConnectivityChecker(), this.isUrlSafeForOfficeOnline, + this.onDismissed, }); final OfficeConversionStrategy? conversionStrategy; @@ -112,6 +113,15 @@ class OfficeAttachmentRenderer extends AttachmentRenderer { /// — e.g. `(attachment) => attachment.remoteUrl?.startsWith(myPublicCdnPrefix) ?? false`. final bool Function(Attachment attachment)? isUrlSafeForOfficeOnline; + /// Called once the user dismisses the in-app iOS QuickLook preview (i.e. + /// after [NativeOfficeChannel.openOfficePreview] resolves). QuickLook is + /// presented as a full modal over whatever is behind this renderer, so + /// there's typically nothing meaningful left to show once it closes — + /// hosts that push this renderer onto its own route commonly want to pop + /// that route here instead of leaving the user on a bare screen. Not + /// called on Android, where no such modal exists. + final VoidCallback? onDismissed; + @override AttachmentType get type => .office; @@ -124,6 +134,7 @@ class OfficeAttachmentRenderer extends AttachmentRenderer { externalOpenConfig: externalOpenConfig, connectivityChecker: connectivityChecker, isUrlSafeForOfficeOnline: isUrlSafeForOfficeOnline, + onDismissed: onDismissed, ); } } @@ -136,6 +147,7 @@ class _OfficeView extends StatefulWidget { this.externalOpenConfig = const ExternalOpenConfig(), this.connectivityChecker = const DefaultConnectivityChecker(), this.isUrlSafeForOfficeOnline, + this.onDismissed, }); final Attachment attachment; final OfficeConversionStrategy? conversionStrategy; @@ -143,6 +155,7 @@ class _OfficeView extends StatefulWidget { final ExternalOpenConfig externalOpenConfig; final ConnectivityChecker connectivityChecker; final bool Function(Attachment attachment)? isUrlSafeForOfficeOnline; + final VoidCallback? onDismissed; @override State<_OfficeView> createState() => _OfficeViewState(); @@ -235,10 +248,13 @@ class _OfficeViewState extends State<_OfficeView> { } // Genuine in-app preview via QuickLook — requires a local file URL, // which the resolver guarantees by the time this renderer runs. - // Always wins on iOS; conversion isn't needed here. + // Always wins on iOS; conversion isn't needed here. This resolves + // only once the user actually dismisses the QuickLook modal (see + // OfficePreviewChannel), not merely once it's presented. await NativeOfficeChannel.openOfficePreview(path); if (mounted && generation == _openGeneration) { setState(() => _previewedInApp = true); + widget.onDismissed?.call(); } return; } diff --git a/packages/attachment_engine/attachment_engine/pubspec.yaml b/packages/attachment_engine/attachment_engine/pubspec.yaml index d6c1d5c..c492032 100644 --- a/packages/attachment_engine/attachment_engine/pubspec.yaml +++ b/packages/attachment_engine/attachment_engine/pubspec.yaml @@ -1,6 +1,6 @@ name: attachment_engine description: "Universal attachment engine for Flutter to resolve, cache, download, preview, and render documents, media, and web content using native platform APIs." -version: 0.0.1-dev.2 +version: 0.0.1-dev.3 homepage: "https://github.com/dhc-tech/flutter-packages/tree/main/packages/attachment_engine/attachment_engine" repository: "https://github.com/dhc-tech/flutter-packages" issue_tracker: "https://github.com/dhc-tech/flutter-packages/issues" @@ -23,7 +23,7 @@ dependencies: sdk: flutter attachment_engine_platform_interface: ^0.0.1-dev.0 attachment_engine_android: ^0.0.1-dev.0 - attachment_engine_ios: ^0.0.1-dev.0 + attachment_engine_ios: ">=0.0.1-dev.2 <0.0.2" attachment_engine_windows: ^0.0.1-dev.0 attachment_engine_linux: ^0.0.1-dev.0 attachment_engine_macos: ^0.0.1-dev.0 diff --git a/packages/attachment_engine/attachment_engine/test/office_renderer_test.dart b/packages/attachment_engine/attachment_engine/test/office_renderer_test.dart index 6110ac2..e3a1d1e 100644 --- a/packages/attachment_engine/attachment_engine/test/office_renderer_test.dart +++ b/packages/attachment_engine/attachment_engine/test/office_renderer_test.dart @@ -168,6 +168,30 @@ void main() { ); }); + testWidgets( + 'iOS calls onDismissed once the QuickLook preview future resolves ' + '(i.e. once the user has dismissed it)', + (tester) async { + var dismissed = 0; + final renderer = OfficeAttachmentRenderer( + platformInfo: const _FakePlatformInfo(isIOS: true), + onDismissed: () => dismissed++, + ); + await tester.pumpWidget( + Directionality( + textDirection: TextDirection.ltr, + child: Builder( + builder: (context) => + renderer.build(context, officeAttachment('/tmp/f.docx')), + ), + ), + ); + await tester.pumpAndSettle(); + + expect(dismissed, 1); + }, + ); + testWidgets('Android with no connection and no public URL falls back to ' 'external-open', (tester) async { final renderer = OfficeAttachmentRenderer( diff --git a/packages/attachment_engine/attachment_engine_ios/CHANGELOG.md b/packages/attachment_engine/attachment_engine_ios/CHANGELOG.md index f7911ef..ab84e58 100644 --- a/packages/attachment_engine/attachment_engine_ios/CHANGELOG.md +++ b/packages/attachment_engine/attachment_engine_ios/CHANGELOG.md @@ -1,3 +1,12 @@ +## 0.0.1-dev.2 + +* Fixes `openOfficePreview`'s Office/QuickLook `QLPreviewController` modal + resolving its Dart future as soon as the preview was *presented* instead + of when the user actually *dismissed* it, leaving callers with no signal + to react to the close (e.g. to pop the screen behind the now-closed + modal). It now implements `QLPreviewControllerDelegate` and only + completes on `previewControllerDidDismiss`. + ## 0.0.1-dev.1 * Initial release: the iOS implementation extracted from `attachment_engine` diff --git a/packages/attachment_engine/attachment_engine_ios/ios/attachment_engine_ios/Sources/attachment_engine_ios/OfficePreviewChannel.swift b/packages/attachment_engine/attachment_engine_ios/ios/attachment_engine_ios/Sources/attachment_engine_ios/OfficePreviewChannel.swift index b371b10..51d0b04 100644 --- a/packages/attachment_engine/attachment_engine_ios/ios/attachment_engine_ios/Sources/attachment_engine_ios/OfficePreviewChannel.swift +++ b/packages/attachment_engine/attachment_engine_ios/ios/attachment_engine_ios/Sources/attachment_engine_ios/OfficePreviewChannel.swift @@ -21,9 +21,19 @@ import UIKit /// local path (as `AttachmentResolver` already guarantees) before invoking /// this channel. /// +/// `openOfficePreview`'s async completion is deferred until the user +/// actually dismisses the QuickLook modal (via +/// `QLPreviewControllerDelegate.previewControllerDidDismiss`), not until +/// it's merely presented — the Dart side uses this to know when to react +/// (e.g. pop back to the previous screen) instead of leaving a bare Flutter +/// view showing behind the now-dismissed preview. +/// /// Implements the Pigeon-generated `OfficeHostApi`. -class OfficePreviewChannel: NSObject, OfficeHostApi, QLPreviewControllerDataSource { +class OfficePreviewChannel: NSObject, OfficeHostApi, QLPreviewControllerDataSource, + QLPreviewControllerDelegate +{ private var previewItemURL: URL? + private var dismissContinuation: CheckedContinuation? func register(with messenger: FlutterBinaryMessenger) { OfficeHostApiSetup.setUp(binaryMessenger: messenger, api: self) @@ -45,7 +55,14 @@ class OfficePreviewChannel: NSObject, OfficeHostApi, QLPreviewControllerDataSour } let preview = QLPreviewController() preview.dataSource = self + preview.delegate = self presenter.present(preview, animated: true, completion: nil) + + // Suspend until previewControllerDidDismiss fires, so the caller learns + // when the modal actually closes rather than just when it was shown. + await withCheckedContinuation { continuation in + self.dismissContinuation = continuation + } } // MARK: QLPreviewControllerDataSource @@ -57,4 +74,11 @@ class OfficePreviewChannel: NSObject, OfficeHostApi, QLPreviewControllerDataSour func previewController(_ controller: QLPreviewController, previewItemAt index: Int) -> QLPreviewItem { previewItemURL! as QLPreviewItem } + + // MARK: QLPreviewControllerDelegate + + func previewControllerDidDismiss(_ controller: QLPreviewController) { + dismissContinuation?.resume() + dismissContinuation = nil + } } diff --git a/packages/attachment_engine/attachment_engine_ios/pubspec.yaml b/packages/attachment_engine/attachment_engine_ios/pubspec.yaml index a3a777c..2fdfe5e 100644 --- a/packages/attachment_engine/attachment_engine_ios/pubspec.yaml +++ b/packages/attachment_engine/attachment_engine_ios/pubspec.yaml @@ -1,6 +1,6 @@ name: attachment_engine_ios description: "iOS implementation of attachment_engine providing native PDFKit, AVFoundation playback, QuickLook preview, download, and share." -version: 0.0.1-dev.1 +version: 0.0.1-dev.2 repository: "https://github.com/dhc-tech/flutter-packages/tree/main/packages/attachment_engine/attachment_engine_ios" issue_tracker: "https://github.com/dhc-tech/flutter-packages/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+attachment_engine_ios%22" From 5eff1e775120ad9c4d727b5ecc52d42e2c105188 Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Mon, 31 Aug 2026 17:39:10 +0530 Subject: [PATCH 5/7] fix(attachment_engine): revert premature attachment_engine_ios constraint tightening Live pub.dev only has attachment_engine_ios up to 0.0.1-dev.1; dev.2 exists locally but isn't published yet. Tightening the constraint to >=0.0.1-dev.2 broke pub resolution (confirmed via pana, which resolves against the real published registry, not local pubspec_overrides.yaml). Reverted to ^0.0.1-dev.0 until dev.2 is actually published. --- packages/attachment_engine/attachment_engine/pubspec.yaml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/attachment_engine/attachment_engine/pubspec.yaml b/packages/attachment_engine/attachment_engine/pubspec.yaml index c492032..34758f9 100644 --- a/packages/attachment_engine/attachment_engine/pubspec.yaml +++ b/packages/attachment_engine/attachment_engine/pubspec.yaml @@ -23,7 +23,7 @@ dependencies: sdk: flutter attachment_engine_platform_interface: ^0.0.1-dev.0 attachment_engine_android: ^0.0.1-dev.0 - attachment_engine_ios: ">=0.0.1-dev.2 <0.0.2" + attachment_engine_ios: ^0.0.1-dev.0 attachment_engine_windows: ^0.0.1-dev.0 attachment_engine_linux: ^0.0.1-dev.0 attachment_engine_macos: ^0.0.1-dev.0 From 0870ff548b7873b267c6f22543d989dac8a7d185 Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Tue, 1 Sep 2026 10:11:45 +0530 Subject: [PATCH 6/7] fix(attachment_engine): default TextAttachmentRenderer.showSearch to false The in-file search bar was on by default for every plain-text preview. Consuming apps that don't want it had to override the renderer at the call site. Flip the default off; opt in explicitly via showSearch: true. --- .../attachment_engine/attachment_engine/CHANGELOG.md | 6 ++++++ .../lib/src/renderers/text_renderer.dart | 12 ++++++------ .../attachment_engine/attachment_engine/pubspec.yaml | 2 +- 3 files changed, 13 insertions(+), 7 deletions(-) diff --git a/packages/attachment_engine/attachment_engine/CHANGELOG.md b/packages/attachment_engine/attachment_engine/CHANGELOG.md index 3480ca3..5b4baf0 100644 --- a/packages/attachment_engine/attachment_engine/CHANGELOG.md +++ b/packages/attachment_engine/attachment_engine/CHANGELOG.md @@ -1,3 +1,9 @@ +## 0.0.1-dev.4 + +* `TextAttachmentRenderer.showSearch` now defaults to `false` (was `true`). + The in-file search bar remains available by opting in explicitly; the + plain scrollable text view is now the out-of-the-box behavior. + ## 0.0.1-dev.3 * New `OfficeAttachmentRenderer.onDismissed` callback, invoked once the diff --git a/packages/attachment_engine/attachment_engine/lib/src/renderers/text_renderer.dart b/packages/attachment_engine/attachment_engine/lib/src/renderers/text_renderer.dart index 7b14cd9..88fd09c 100644 --- a/packages/attachment_engine/attachment_engine/lib/src/renderers/text_renderer.dart +++ b/packages/attachment_engine/attachment_engine/lib/src/renderers/text_renderer.dart @@ -14,15 +14,15 @@ import 'renderer.dart'; /// Plain text viewer. Set [snippetMode] to true (via [TextAttachmentRenderer.preview]) /// for a short, non-scrolling preview rather than the full document. /// -/// Full (non-snippet) mode includes a search bar (case-insensitive, with -/// match count and next/previous navigation that scrolls to and highlights -/// each match) — set [showSearch] to false to opt out and get the plain -/// scrollable text view instead. +/// Full (non-snippet) mode can optionally show a search bar (case-insensitive, +/// with match count and next/previous navigation that scrolls to and +/// highlights each match) — set [showSearch] to true to opt in; it defaults +/// to off, giving the plain scrollable text view. class TextAttachmentRenderer extends AttachmentRenderer { const TextAttachmentRenderer({ this.snippetMode = false, this.snippetLength = 280, - this.showSearch = true, + this.showSearch = false, }); final bool snippetMode; @@ -30,7 +30,7 @@ class TextAttachmentRenderer extends AttachmentRenderer { /// Whether the full (non-snippet) view shows an in-file search bar. /// Ignored when [snippetMode] is true (a 3-line preview has nothing - /// meaningful to search). + /// meaningful to search). Off by default. final bool showSearch; @override diff --git a/packages/attachment_engine/attachment_engine/pubspec.yaml b/packages/attachment_engine/attachment_engine/pubspec.yaml index 34758f9..6d42b73 100644 --- a/packages/attachment_engine/attachment_engine/pubspec.yaml +++ b/packages/attachment_engine/attachment_engine/pubspec.yaml @@ -1,6 +1,6 @@ name: attachment_engine description: "Universal attachment engine for Flutter to resolve, cache, download, preview, and render documents, media, and web content using native platform APIs." -version: 0.0.1-dev.3 +version: 0.0.1-dev.4 homepage: "https://github.com/dhc-tech/flutter-packages/tree/main/packages/attachment_engine/attachment_engine" repository: "https://github.com/dhc-tech/flutter-packages" issue_tracker: "https://github.com/dhc-tech/flutter-packages/issues" From bad9b306299267d6e71109c989e0142d99ce1071 Mon Sep 17 00:00:00 2001 From: Digvijaysinh Chauhan Date: Tue, 1 Sep 2026 10:23:20 +0530 Subject: [PATCH 7/7] test(attachment_engine): pin showSearch: true where tests exercise the search feature Fixes 8 test failures introduced by the showSearch default flipping to false: these tests exercise the in-file search UI and previously relied on the (now-changed) default rather than opting in explicitly. --- .../attachment_engine/test/text_renderer_test.dart | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/attachment_engine/attachment_engine/test/text_renderer_test.dart b/packages/attachment_engine/attachment_engine/test/text_renderer_test.dart index 511880d..e835404 100644 --- a/packages/attachment_engine/attachment_engine/test/text_renderer_test.dart +++ b/packages/attachment_engine/attachment_engine/test/text_renderer_test.dart @@ -32,7 +32,9 @@ void main() { Future pumpTextView( WidgetTester tester, String content, { - TextAttachmentRenderer renderer = const TextAttachmentRenderer(), + TextAttachmentRenderer renderer = const TextAttachmentRenderer( + showSearch: true, + ), }) async { final file = File('${tempDir.path}/sample.txt'); file.writeAsStringSync(content); @@ -195,7 +197,7 @@ void main() { 'reusing the same widget for different text reloads its lines and ' 'search results instead of keeping the previous document\'s', (tester) async { - const renderer = TextAttachmentRenderer(); + const renderer = TextAttachmentRenderer(showSearch: true); final fileA = File('${tempDir.path}/a.txt'); fileA.writeAsStringSync('apple pie\nbanana split\n'); final fileB = File('${tempDir.path}/b.txt');