From 9f2ffdb8a909333117616fd6925d457abbced154 Mon Sep 17 00:00:00 2001 From: Krzysztof kuhy Rudnicki Date: Thu, 1 Oct 2026 12:17:06 +0200 Subject: [PATCH] Attach any file to a note, of any size, not only images MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit πŸ“Ž now picks any file (Android: Camera / Gallery / File; desktop: an unfiltered dialog) and the desktop page accepts any dropped file. Photo and screenshot formats up to 64 MB are still re-encoded and shown inline; everything else is stored byte for byte on dufs beside the images and linked as [name](url), shown as a row with its size that opens in the platform's app (Android) or a new browser tab (desktop). BACKLOG.md rewrites file links to ~/data/cloud/todo-images/ like image links. There is no size cap, so no layer holds a file in memory: uploads and downloads stream through DufsImageClient, writes land in a .tmp renamed when whole (an interrupted download no longer leaves a truncated file that looks done), the desktop page POSTs the browser's own Blob with fetch, and Android's "File" uses file_picker, because file_selector's Android side reads the whole file into memory on the UI thread. Verified with a 1 GB file end to end on desktop (peak RSS +36 MB) and on an API 36 emulator. The wrapper serves attachments from the origin that holds the GitHub token and the notes, so only script-free types open inline (nosniff), the rest download; writes carrying another site's Origin are refused (this also closes cross-site overwrites of the backup files); and attachment routes run concurrently so a long upload or a playing video cannot stall the page. Also: cupertino_icons 2.0.0 (unused, but the freshness gate blocks the pubspec otherwise), open_file 4.0.0 and file_picker 13.1.0 pinned. Co-Authored-By: Claude Opus 5.5 --- AGENTS.md | 42 ++-- lib/attachments/file_post.dart | 6 + lib/attachments/file_post_io.dart | 9 + lib/attachments/file_post_web.dart | 23 ++ lib/attachments/image_store_api.dart | 42 +++- lib/attachments/image_store_io.dart | 51 ++++- lib/attachments/image_store_web.dart | 65 +++++- lib/data/note_template_parser.dart | 24 +- lib/desktop/static_content_type.dart | 42 ++++ lib/desktop/wrapper_attachment_serving.dart | 127 +++++++++++ lib/desktop/wrapper_image_routes.dart | 89 +++++--- lib/desktop/wrapper_server.dart | 82 +++---- lib/images/attachment_ingest.dart | 112 +++++++++ lib/images/attachment_links.dart | 65 ++++++ lib/images/attachment_ref.dart | 160 +++++++++---- lib/images/dufs_image_client.dart | 62 +++-- lib/images/image_cache_store.dart | 142 ++++++------ lib/images/image_processing.dart | 34 +++ lib/images/reconcile_report.dart | 40 ++++ lib/sync/notes_markdown.dart | 6 +- lib/ui/attachment_picking.dart | 113 +++++++++ lib/ui/capture_app_bar.dart | 16 +- lib/ui/capture_screen.dart | 7 +- lib/ui/capture_screen_images.dart | 9 + lib/ui/capture_screen_sync.dart | 4 +- lib/ui/capture_screen_widget.dart | 4 +- lib/ui/cloud_images_settings.dart | 5 +- lib/ui/image_attach.dart | 35 ++- lib/ui/image_input_io.dart | 8 + lib/ui/image_input_web.dart | 31 ++- lib/ui/image_picking.dart | 104 --------- ...image_strip.dart => attachment_strip.dart} | 30 +-- lib/ui/images/file_embed.dart | 179 +++++++++++++++ lib/ui/images/file_opening.dart | 34 +++ lib/ui/markdown_view.dart | 19 +- lib/ui/note_detail_screen.dart | 20 +- lib/ui/note_form.dart | 4 +- pubspec.lock | 148 +++++++++++- pubspec.yaml | 8 +- test/attachments/image_store_api_test.dart | 16 +- test/attachments/image_store_io_test.dart | 73 +++++- test/attachments/image_store_web_test.dart | 99 +++++++- test/capture_screen_harness.dart | 6 +- test/desktop/wrapper_attach_route_test.dart | 215 ++++++++++++++++++ .../wrapper_attachment_serving_test.dart | 78 +++++++ test/desktop/wrapper_image_routes_test.dart | 13 +- test/fake_image_store.dart | 27 +++ test/images/attachment_ingest_test.dart | 145 ++++++++++++ test/images/attachment_links_test.dart | 61 +++++ test/images/attachment_ref_test.dart | 90 +++++--- test/images/dufs_image_client_test.dart | 57 +++-- test/images/fake_dufs.dart | 29 ++- test/images/image_cache_store_test.dart | 66 +++++- test/images/image_processing_test.dart | 38 ++++ test/ui_images/attach_files_test.dart | 97 ++++++++ ...test.dart => attachment_picking_test.dart} | 68 +++--- test/ui_images/capture_images_test.dart | 50 ++-- .../detail_and_misc_images_test.dart | 83 ++++--- test/ui_images/file_embed_test.dart | 127 +++++++++++ test/ui_images/file_opening_test.dart | 88 +++++++ test/ui_images/image_attach_test.dart | 2 +- test/ui_images/image_embed_test.dart | 6 +- 62 files changed, 2968 insertions(+), 567 deletions(-) create mode 100644 lib/attachments/file_post.dart create mode 100644 lib/attachments/file_post_io.dart create mode 100644 lib/attachments/file_post_web.dart create mode 100644 lib/desktop/static_content_type.dart create mode 100644 lib/desktop/wrapper_attachment_serving.dart create mode 100644 lib/images/attachment_ingest.dart create mode 100644 lib/images/attachment_links.dart create mode 100644 lib/images/reconcile_report.dart create mode 100644 lib/ui/attachment_picking.dart delete mode 100644 lib/ui/image_picking.dart rename lib/ui/images/{image_strip.dart => attachment_strip.dart} (52%) create mode 100644 lib/ui/images/file_embed.dart create mode 100644 lib/ui/images/file_opening.dart create mode 100644 test/desktop/wrapper_attach_route_test.dart create mode 100644 test/desktop/wrapper_attachment_serving_test.dart create mode 100644 test/images/attachment_ingest_test.dart create mode 100644 test/images/attachment_links_test.dart create mode 100644 test/ui_images/attach_files_test.dart rename test/ui_images/{image_picking_test.dart => attachment_picking_test.dart} (52%) create mode 100644 test/ui_images/file_embed_test.dart create mode 100644 test/ui_images/file_opening_test.dart diff --git a/AGENTS.md b/AGENTS.md index 3c500e6..129f4c0 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -68,7 +68,7 @@ Three layers under `lib/`, each single-purpose: token. `sync_settings.dart`: persisted owner/repo/token/clientId. - `notes_markdown.dart`: round-trippable single-file export/import format (HTML-comment `` markers). -- `images/` + `attachments/` β€” attached images (see "Images" below). +- `images/` + `attachments/` β€” attached images and files (see "Images and other attachments" below). - `ui/` β€” screens, all take an injected `NoteRepository`. - `capture_screen.dart`: landing screen; always-focused text box pre-filled with the structured template; lazy note creation on first keystroke. @@ -99,26 +99,38 @@ that, the stepper would fold a retired section into its neighbour on the next save. `tool/migrate_backlog.dart` converts an old export (Settings β†’ Export β†’ migrate β†’ Settings β†’ Import). -### Images +### Images and other attachments -A note never holds image bytes, only a line -`![](https://kuhy-cloud.duckdns.org/todo-images//.jpg)` -(`lib/images/attachment_ref.dart` owns that shape), so sync and older builds are -untouched. Every image is re-encoded before it leaves the device: ≀2560 px, -JPEG q85, EXIF stripped after applying its rotation (`image_processing.dart`). -`ImageCacheStore` keeps every linked image on-device (offline) plus a +A note never holds attachment bytes, only a line pointing at +`https://kuhy-cloud.duckdns.org/todo-images//.`: an image +as `![](…)`, any other file as `[name](…)` (`lib/images/attachment_ref.dart` +owns the path, `attachment_links.dart` the links), so sync and older builds are +untouched. Anything picked or dropped goes through `AttachmentIngest`: a +JPEG/PNG/GIF/WebP/BMP/TIFF of at most 64 MB is re-encoded (≀2560 px, JPEG q85, +EXIF stripped after applying its rotation β€” `image_processing.dart`); every +other file, of any size, is kept byte-for-byte. There is deliberately **no +size cap**, so nothing may hold a file in memory: uploads and downloads stream +(`DufsImageClient`), the desktop page POSTs the browser's own Blob with +`fetch` (`file_post_web.dart`), and writes land in a `.tmp` renamed when whole. +`ImageCacheStore` keeps every linked attachment on-device (offline) plus a `.pending` marker per queued upload, and `reconcile` uploads, prefetches and -evicts (never an empty ref set, never within 10 min). Android talks to dufs -directly with the `todo` login's password (Settings β†’ Cloud images); the -desktop page never does β€” the wrapper's `/images/*` routes do, with +evicts (never an empty ref set, never within 10 min; stale `.tmp`s too). +Android talks to dufs directly with the `todo` login's password (Settings β†’ +Cloud images) and opens files with `open_file`; the desktop page never talks +to dufs β€” the wrapper's `/images/*` routes do, with `~/.config/dufs/logins/todo.env`, created by -`~/src/dufs-cloud/scripts/add_dufs_login.sh todo /todo-images rw`. BACKLOG.md -rewrites links to `~/data/cloud/todo-images/…` so an agent can open them. +`~/src/dufs-cloud/scripts/add_dufs_login.sh todo /todo-images rw`. The wrapper +serves attachments from the same origin as the notes and the GitHub token, so +only script-free types (raster images, PDF, plain text, audio, video) are shown +inline, always `nosniff`; everything else is a download +(`wrapper_attachment_serving.dart`). Attachment routes run concurrently, the +rest of the wrapper one request at a time. BACKLOG.md rewrites links to +`~/data/cloud/todo-images/…` so an agent can open them. ## Testing -- **The suite must stay quick and fully green.** It is currently **427 tests at - 100% line coverage** (2512 lines), ~30s on this machine with +- **The suite must stay quick and fully green.** It is currently **494 tests at + 100% line coverage** (2870 lines), ~40s on this machine with `--concurrency=3` (unbounded concurrency under `capped.sh`'s 4 GiB gets its test processes SIGTERMed while the wrapper still exits 0 β€” check the last line says "All tests passed"). Don't regress either. diff --git a/lib/attachments/file_post.dart b/lib/attachments/file_post.dart new file mode 100644 index 0000000..686d5d3 --- /dev/null +++ b/lib/attachments/file_post.dart @@ -0,0 +1,6 @@ +/// `postFile` for the current platform: the browser's own `fetch` on the +/// desktop web build, so a picked file streams from disk to the wrapper +/// without ever becoming a Dart byte list. +library; + +export 'file_post_io.dart' if (dart.library.js_interop) 'file_post_web.dart'; diff --git a/lib/attachments/file_post_io.dart b/lib/attachments/file_post_io.dart new file mode 100644 index 0000000..370ac0e --- /dev/null +++ b/lib/attachments/file_post_io.dart @@ -0,0 +1,9 @@ +/// Off the web nothing posts files to the wrapper: Android stores them +/// itself. +library; + +import 'package:todo/attachments/image_store_api.dart'; + +/// Always throws; only the desktop page posts files. +Future<({int status, String body})> postFile(Uri url, XFile file) => + throw UnsupportedError('Only the desktop page posts files'); diff --git a/lib/attachments/file_post_web.dart b/lib/attachments/file_post_web.dart new file mode 100644 index 0000000..13e72e8 --- /dev/null +++ b/lib/attachments/file_post_web.dart @@ -0,0 +1,23 @@ +/// POSTs a picked or dropped file to the desktop wrapper with `fetch`. +/// +/// `package:http`'s browser client buffers a request body whole, and +/// `XFile.readAsBytes` copies the file into Dart: either would put a +/// multi-gigabyte attachment in the page's memory. Handing the file's own +/// Blob to `fetch` lets the browser stream it from disk instead. +library; + +import 'dart:js_interop'; + +import 'package:todo/attachments/image_store_api.dart'; +import 'package:web/web.dart' as web; + +/// POSTs [file] to [url]. [file]'s path is the object URL that file_selector +/// or the drop listener created for it. +Future<({int status, String body})> postFile(Uri url, XFile file) async { + final source = await web.window.fetch(file.path.toJS).toDart; + final blob = await source.blob().toDart; + final response = await web.window + .fetch(url.toString().toJS, web.RequestInit(method: 'POST', body: blob)) + .toDart; + return (status: response.status, body: (await response.text().toDart).toDart); +} diff --git a/lib/attachments/image_store_api.dart b/lib/attachments/image_store_api.dart index a6a83a0..a20c17e 100644 --- a/lib/attachments/image_store_api.dart +++ b/lib/attachments/image_store_api.dart @@ -1,15 +1,20 @@ -/// What the UI asks of attached images, independent of platform. +/// What the UI asks of attachments (images and any other file), +/// independent of platform. /// -/// Android keeps images itself (an `ImageCacheStore` in app storage, talking -/// to dufs directly); the desktop web page delegates everything to the local -/// wrapper's `/images/*` routes. Both implement [ImageStore], picked by the -/// conditional export in `image_store.dart`. +/// Android keeps attachments itself (an `ImageCacheStore` in app storage, +/// talking to dufs directly); the desktop web page delegates everything to +/// the local wrapper's `/images/*` routes. Both implement [ImageStore], +/// picked by the conditional export in `image_store.dart`. library; +import 'package:file_selector/file_selector.dart' show XFile; import 'package:flutter/foundation.dart'; +import 'package:todo/images/attachment_links.dart'; import 'package:todo/images/attachment_ref.dart'; +export 'package:file_selector/file_selector.dart' show XFile; + /// Where to draw an image from right now. sealed class ImageSource { const ImageSource(); @@ -63,7 +68,7 @@ class ImageStoreException implements Exception { String toString() => message; } -/// Stores, shows and syncs the images notes link to. Notifies when an image +/// Stores, shows and syncs the attachments notes link to. Notifies when one /// becomes available or changes state, so embeds can redraw. abstract class ImageStore extends ChangeNotifier { /// Stores [bytes] (any image format) for [noteId] and queues the upload. @@ -72,8 +77,13 @@ abstract class ImageStore extends ChangeNotifier { /// [ImageStoreException] when it cannot be stored. Future attach(String noteId, Uint8List bytes); - /// Uploads the queue, fetches every image [noteTexts] link to that this - /// device lacks, and evicts cached images no note links to. + /// Stores the picked or dropped [file] for [noteId] β€” re-encoded when it is + /// an image, byte-for-byte otherwise, whatever its size β€” and queues the + /// upload. Throws [ImageStoreException] when it cannot be stored. + Future attachFile(String noteId, XFile file); + + /// Uploads the queue, fetches every attachment [noteTexts] link to that + /// this device lacks, and evicts cached ones no note links to. Future reconcile(Iterable noteTexts); /// Where to draw [ref] from. @@ -82,13 +92,16 @@ abstract class ImageStore extends ChangeNotifier { /// Whether [ref] is still waiting to upload (always false when unknown). bool isPending(AttachmentRef ref); + /// Size in bytes of [ref] on this device, or null when it is not here yet. + Future sizeOf(AttachmentRef ref); + /// Login and queue summary. Future status(); } -/// Every image ref across [noteTexts]. -Set imageRefsAcross(Iterable noteTexts) => { - for (final text in noteTexts) ...imageRefsIn(text), +/// Every attachment ref across [noteTexts]. +Set attachmentRefsAcross(Iterable noteTexts) => { + for (final text in noteTexts) ...attachmentRefsIn(text), }; /// The store used where none was provided (tests, and any screen built @@ -98,6 +111,10 @@ class NoImageStore extends ImageStore { Future attach(String noteId, Uint8List bytes) async => throw const ImageStoreException('Images are not available here'); + @override + Future attachFile(String noteId, XFile file) async => + throw const ImageStoreException('Attachments are not available here'); + @override Future reconcile(Iterable noteTexts) async {} @@ -107,6 +124,9 @@ class NoImageStore extends ImageStore { @override bool isPending(AttachmentRef ref) => false; + @override + Future sizeOf(AttachmentRef ref) async => null; + @override Future status() async => const ImageStoreStatus(configured: false, pending: 0); diff --git a/lib/attachments/image_store_io.dart b/lib/attachments/image_store_io.dart index df52f73..59e1e3e 100644 --- a/lib/attachments/image_store_io.dart +++ b/lib/attachments/image_store_io.dart @@ -1,4 +1,4 @@ -/// Android: images live in app storage and sync with dufs directly. +/// Android: attachments live in app storage and sync with dufs directly. library; import 'dart:async'; @@ -10,6 +10,7 @@ import 'package:path/path.dart' as p; import 'package:path_provider/path_provider.dart'; import 'package:todo/attachments/image_password_store.dart'; import 'package:todo/attachments/image_store_api.dart'; +import 'package:todo/images/attachment_ingest.dart'; import 'package:todo/images/attachment_ref.dart'; import 'package:todo/images/dufs_image_client.dart'; import 'package:todo/images/image_cache_store.dart'; @@ -22,28 +23,41 @@ import 'package:todo/images/image_processing.dart'; Future openImageStore() async { final support = await getApplicationSupportDirectory(); final root = Directory(p.join(support.path, 'images')); - return IoImageStore(ImageCacheStore(root)); + // Both pickers copy what they return into the cache directory. + final pickerCopies = (await getTemporaryDirectory()).path; + return IoImageStore(ImageCacheStore(root), pickerCopies: pickerCopies); } // coverage:ignore-end /// [ImageStore] over a local [ImageCacheStore], uploading with the login /// from [ImagePasswordStore]. class IoImageStore extends ImageStore { - /// Creates a store over [cache]; the rest is injectable for tests. + /// Creates a store over [cache]. Picked files under [pickerCopies] are the + /// pickers' private copies and are moved rather than copied. The rest is + /// injectable for tests. IoImageStore( this.cache, { + this.pickerCopies, this._passwords = const ImagePasswordStore(), http.Client Function()? httpClient, Future Function(Uint8List)? prepare, }) : _httpClient = httpClient ?? http.Client.new, - _prepare = prepare ?? prepareImageInBackground; + _prepare = prepare ?? prepareImageInBackground, + _ingest = AttachmentIngest( + cache, + prepare: prepare ?? prepareImageInBackground, + ); - /// The on-device images and upload queue. + /// The on-device attachments and upload queue. final ImageCacheStore cache; + /// Directory the pickers copy their results into, if known. + final String? pickerCopies; + final ImagePasswordStore _passwords; final http.Client Function() _httpClient; final Future Function(Uint8List) _prepare; + final AttachmentIngest _ingest; @override Future attach(String noteId, Uint8List bytes) async { @@ -62,9 +76,28 @@ class IoImageStore extends ImageStore { return ref; } + @override + Future attachFile(String noteId, XFile file) async { + final copies = pickerCopies; + final Attachment attachment; + try { + attachment = await _ingest.fromFile( + noteId, + file.name, + File(file.path), + move: copies != null && p.isWithin(copies, file.path), + ); + } on FileSystemException catch (e) { + throw ImageStoreException('Could not save ${file.name}: ${e.message}'); + } + notifyListeners(); + unawaited(_sync(const {})); + return attachment; + } + @override Future reconcile(Iterable noteTexts) => - _sync(imageRefsAcross(noteTexts)); + _sync(attachmentRefsAcross(noteTexts)); Future _sync(Set refs) async { final login = await _passwords.login(); @@ -92,6 +125,12 @@ class IoImageStore extends ImageStore { @override bool isPending(AttachmentRef ref) => cache.isPending(ref); + @override + Future sizeOf(AttachmentRef ref) async { + final file = cache.fileFor(ref); + return file.existsSync() ? await file.length() : null; + } + @override Future status() async => ImageStoreStatus( configured: await _passwords.login() != null, diff --git a/lib/attachments/image_store_web.dart b/lib/attachments/image_store_web.dart index 8b51291..b983872 100644 --- a/lib/attachments/image_store_web.dart +++ b/lib/attachments/image_store_web.dart @@ -1,4 +1,4 @@ -/// Desktop (web build): every image operation goes through the local +/// Desktop (web build): every attachment operation goes through the local /// wrapper's `/images/*` routes; the page never sees the dufs password. library; @@ -8,6 +8,7 @@ import 'dart:typed_data'; import 'package:http/http.dart' as http; +import 'package:todo/attachments/file_post.dart' as file_post; import 'package:todo/attachments/image_store_api.dart'; import 'package:todo/images/attachment_ref.dart'; import 'package:todo/sync/desktop_wrapper.dart'; @@ -16,16 +17,30 @@ import 'package:todo/sync/desktop_wrapper.dart'; Future openImageStore() async => WebImageStore(origin: servingWrapperOrigin()); +/// Sends a file to the wrapper and returns its answer; injectable for tests. +typedef FilePost = + Future<({int status, String body})> Function(Uri url, XFile file); + /// [ImageStore] backed by the desktop wrapper at [origin]. class WebImageStore extends ImageStore { - /// Creates a store talking to [origin]; [httpClient] is injectable. - WebImageStore({required this.origin, http.Client? httpClient}) - : _http = httpClient ?? http.Client(); + /// Creates a store talking to [origin]; [httpClient] and [postFile] are + /// injectable. + WebImageStore({ + required this.origin, + http.Client? httpClient, + FilePost? postFile, + }) : _http = httpClient ?? http.Client(), + _postFile = postFile ?? file_post.postFile; /// The wrapper's origin, e.g. `http://localhost:8730`. final String origin; final http.Client _http; + final FilePost _postFile; + + // Sizes the wrapper reported. A ref's bytes never change (a fresh uuid per + // attachment), so a known size never goes stale. + final Map _sizes = {}; // Bumped whenever the wrapper may hold new bytes, so an image that failed // to load (not downloaded yet) is requested again under a fresh URL. @@ -56,9 +71,34 @@ class WebImageStore extends ImageStore { return ref; } + @override + Future attachFile(String noteId, XFile file) async { + final url = _images( + 'attach', + ).replace(queryParameters: {'note': noteId, 'name': file.name}); + final ({int status, String body}) response; + try { + response = await _postFile(url, file); + } on Object { + throw ImageStoreException( + 'The desktop helper is not running; ${file.name} was not saved', + ); + } + if (response.status != 200) { + throw ImageStoreException( + 'The desktop helper refused ${file.name} (${response.status})', + ); + } + final attachment = Attachment.fromJson(jsonDecode(response.body)); + _revision++; + notifyListeners(); + unawaited(_reconcile(const {})); + return attachment; + } + @override Future reconcile(Iterable noteTexts) => - _reconcile(imageRefsAcross(noteTexts)); + _reconcile(attachmentRefsAcross(noteTexts)); Future _reconcile(Set refs) async { try { @@ -87,6 +127,21 @@ class WebImageStore extends ImageStore { @override bool isPending(AttachmentRef ref) => false; + @override + Future sizeOf(AttachmentRef ref) async { + if (_sizes[ref] case final known?) return known; + try { + final response = await _http.head(_images(ref.relativePath)); + final length = response.statusCode == 200 + ? int.tryParse(response.headers['content-length'] ?? '') + : null; + if (length != null) _sizes[ref] = length; + return length; + } on Exception { + return null; + } + } + @override Future status() async { try { diff --git a/lib/data/note_template_parser.dart b/lib/data/note_template_parser.dart index 3c62c69..8647d7c 100644 --- a/lib/data/note_template_parser.dart +++ b/lib/data/note_template_parser.dart @@ -17,6 +17,7 @@ library; import 'package:todo/data/note_template.dart'; +import 'package:todo/images/attachment_links.dart'; import 'package:todo/images/attachment_ref.dart'; /// Matches a single `http(s)://…` token. @@ -141,24 +142,27 @@ ParsedNote parse(NoteTemplate template, String text) { } /// The note's display title: the `#` heading text without its `#` marker, or -/// the first non-empty line for freeform notes. Attached-image lines are -/// skipped (a 120-char dufs URL is no title); a note of nothing but images -/// is titled by their count. +/// the first non-empty line for freeform notes. Attachment lines are skipped +/// (a 120-char dufs URL is no title); a note of nothing but attachments is +/// titled by their count, or by the file's name when it holds just one. String noteTitle(String text) { - var images = 0; + final attachments = []; for (final line in text.split('\n')) { final trimmed = line.trim(); if (trimmed.isEmpty) continue; - if (imageRefOfLine(trimmed) != null) { - images++; + if (attachmentOfLine(trimmed) case final attachment?) { + attachments.add(attachment); continue; } return trimmed.replaceFirst(RegExp(r'^#+\s*'), ''); } - return switch (images) { - 0 => '', - 1 => 'Image', - _ => '$images images', + final images = attachments.where((a) => a.isImage).length; + return switch ((attachments.length, images)) { + (0, _) => '', + (1, 1) => 'Image', + (1, _) => attachments.single.name, + (final n, final i) when i == n => '$n images', + (final n, _) => '$n attachments', }; } diff --git a/lib/desktop/static_content_type.dart b/lib/desktop/static_content_type.dart new file mode 100644 index 0000000..4aa982b --- /dev/null +++ b/lib/desktop/static_content_type.dart @@ -0,0 +1,42 @@ +/// Content types for the web build's static assets, served by the wrapper. +library; + +import 'dart:io'; + +import 'package:path/path.dart' as p; + +/// Content type for [filePath]. +/// +/// Flutter web is strict here: CanvasKit refuses to instantiate a `.wasm` +/// served as anything but `application/wasm`, and the app silently fails to +/// render if the bootstrap `.js` is mislabelled. +ContentType staticContentTypeFor(String filePath) { + switch (p.extension(filePath).toLowerCase()) { + case '.html': + return ContentType.html; + case '.js' || '.mjs': + return ContentType('text', 'javascript', charset: 'utf-8'); + case '.json': + return ContentType.json; + case '.wasm': + return ContentType('application', 'wasm'); + case '.css': + return ContentType('text', 'css', charset: 'utf-8'); + case '.png': + return ContentType('image', 'png'); + case '.jpg' || '.jpeg': + return ContentType('image', 'jpeg'); + case '.svg': + return ContentType('image', 'svg+xml'); + case '.ttf': + return ContentType('font', 'ttf'); + case '.otf': + return ContentType('font', 'otf'); + case '.woff2': + return ContentType('font', 'woff2'); + case '.bin' || '.symbols': + return ContentType.binary; + default: + return ContentType.binary; + } +} diff --git a/lib/desktop/wrapper_attachment_serving.dart b/lib/desktop/wrapper_attachment_serving.dart new file mode 100644 index 0000000..66bbf72 --- /dev/null +++ b/lib/desktop/wrapper_attachment_serving.dart @@ -0,0 +1,127 @@ +/// Serving one cached attachment from the desktop wrapper, for an `` in +/// the page or a file opened in a new tab. +/// +/// The wrapper's origin also holds the GitHub token (localStorage) and the +/// notes (IndexedDB), so an attached `.html` or `.svg` rendered here would +/// run script with access to both. Only types that cannot script are shown +/// inline β€” raster images, PDF, plain text, audio and video β€” always with +/// `nosniff` so the browser cannot second-guess the type; everything else is +/// sent as a download. +/// +/// Single byte ranges are honoured so a video can seek. +library; + +import 'dart:io'; + +import 'package:todo/images/attachment_ref.dart'; + +/// The content type for an attachment with [extension], and whether it may +/// be shown inline (see the library comment). +({ContentType type, bool inline}) attachmentContentType(String extension) { + final type = switch (extension) { + 'jpg' || 'jpeg' => ContentType('image', 'jpeg'), + 'png' || 'gif' || 'webp' || 'bmp' => ContentType('image', extension), + 'pdf' => ContentType('application', 'pdf'), + 'txt' || + 'md' || + 'log' || + 'csv' => ContentType('text', 'plain', charset: 'utf-8'), + 'mp4' || 'm4v' => ContentType('video', 'mp4'), + 'webm' => ContentType('video', 'webm'), + 'ogv' => ContentType('video', 'ogg'), + 'mov' => ContentType('video', 'quicktime'), + 'mp3' => ContentType('audio', 'mpeg'), + 'm4a' => ContentType('audio', 'mp4'), + 'ogg' || 'oga' || 'opus' => ContentType('audio', 'ogg'), + 'wav' => ContentType('audio', 'wav'), + 'flac' => ContentType('audio', 'flac'), + _ => null, + }; + return type == null + ? (type: ContentType.binary, inline: false) + : (type: type, inline: true); +} + +/// `Content-Disposition` for [fileName]: RFC 6266 with an RFC 5987 +/// `filename*`, so any name (spaces, Polish letters, quotes) survives. +String contentDisposition(String fileName, {required bool inline}) { + final encoded = Uri.encodeComponent(fileName).replaceAllMapped( + RegExp("[!'()*]"), + (m) => '%${m[0]!.codeUnitAt(0).toRadixString(16).toUpperCase()}', + ); + return "${inline ? 'inline' : 'attachment'}; filename*=UTF-8''$encoded"; +} + +/// Answers a GET or HEAD for [ref] from [file], named [fileName] when the +/// page passed one (else the stored name). 404 when it is not cached. +Future serveAttachment( + HttpRequest request, + AttachmentRef ref, + File file, { + String? fileName, +}) async { + final response = request.response; + if (!file.existsSync()) { + response.statusCode = HttpStatus.notFound; + return; + } + final length = await file.length(); + final kind = attachmentContentType(ref.extension); + response.headers + ..contentType = kind.type + ..set('x-content-type-options', 'nosniff') + ..set(HttpHeaders.acceptRangesHeader, 'bytes') + ..set( + 'content-disposition', + contentDisposition(fileName ?? ref.file, inline: kind.inline), + ) + // The name is a fresh uuid per attachment, so the bytes never change. + ..set(HttpHeaders.cacheControlHeader, 'private, max-age=31536000'); + var start = 0; + var end = length - 1; + final range = request.headers.value(HttpHeaders.rangeHeader); + if (range != null) { + final parsed = parseByteRange(range, length); + if (parsed == null) { + response + ..statusCode = HttpStatus.requestedRangeNotSatisfiable + ..headers.set(HttpHeaders.contentRangeHeader, 'bytes */$length'); + return; + } + (start, end) = parsed; + response + ..statusCode = HttpStatus.partialContent + ..headers.set( + HttpHeaders.contentRangeHeader, + 'bytes $start-$end/$length', + ); + } + response.contentLength = length == 0 ? 0 : end - start + 1; + if (request.method == 'HEAD' || length == 0) return; + // A client leaving mid-body (a video seek abandons its old range) does not + // throw here: dart:io absorbs it and the stream just stops being read. + await response.addStream(file.openRead(start, end + 1)); +} + +/// The inclusive byte range a single-range `Range` header asks for within +/// [length] bytes, or null when it is malformed, multi-range or outside the +/// file. +(int, int)? parseByteRange(String header, int length) { + final match = RegExp(r'^bytes=(\d*)-(\d*)$').firstMatch(header.trim()); + if (match == null) return null; + final first = int.tryParse(match[1]!); + final last = int.tryParse(match[2]!); + final int start; + final int end; + if (first == null) { + // `bytes=-N`: the final N bytes. + if (last == null || last == 0) return null; + start = last >= length ? 0 : length - last; + end = length - 1; + } else { + start = first; + end = last == null || last >= length ? length - 1 : last; + } + if (start >= length || start > end) return null; + return (start, end); +} diff --git a/lib/desktop/wrapper_image_routes.dart b/lib/desktop/wrapper_image_routes.dart index c12ee64..87b38c1 100644 --- a/lib/desktop/wrapper_image_routes.dart +++ b/lib/desktop/wrapper_image_routes.dart @@ -1,18 +1,22 @@ /// The desktop wrapper's `/images/*` routes: the web page's only way to -/// store, show and sync attached images. +/// store, show and sync attachments (images and any other file). /// /// The page never talks to dufs itself. That keeps the dufs password out of /// the browser, needs no CORS on dufs, and gives the desktop the same offline /// store the phone has (an [ImageCacheStore] on disk). Same-origin with the /// page, so an `` just works. /// -/// * `POST /images//.jpg` β€” raw bytes of any image; re-encoded -/// here (the page must not decode on its UI thread), stored, queued. -/// 415 when the bytes are not an image. -/// * `GET /images//.jpg` β€” the cached JPEG, 404 when absent. -/// * `POST /images/reconcile` β€” `{"refs": ["/.jpg", …]}`: every -/// image the notes link to. Uploads the queue, fetches what is missing, -/// evicts the rest; answers with the [ReconcileReport]. +/// * `POST /images/attach?note=&name=` β€” a picked or +/// dropped file of any size, streamed to disk; stored as a re-encoded image +/// when it is one, byte-for-byte otherwise. Answers with the [Attachment]. +/// * `POST /images//.jpg` β€” raw bytes of a pasted image; +/// re-encoded here (the page must not decode on its UI thread), stored, +/// queued. 415 when the bytes are not an image. +/// * `GET|HEAD /images//.[?name=]` β€” the cached +/// file (see `wrapper_attachment_serving.dart`), 404 when absent. +/// * `POST /images/reconcile` β€” `{"refs": ["/.", …]}`: +/// every attachment the notes link to. Uploads the queue, fetches what is +/// missing, evicts the rest; answers with the [ReconcileReport]. /// * `GET /images/status` β€” `{"configured": bool, "pending": n}`. library; @@ -22,16 +26,14 @@ import 'dart:typed_data'; import 'package:http/http.dart' as http; import 'package:todo/desktop/dufs_login_env.dart'; +import 'package:todo/desktop/wrapper_attachment_serving.dart'; import 'package:todo/desktop/wrapper_paths.dart'; +import 'package:todo/images/attachment_ingest.dart'; import 'package:todo/images/attachment_ref.dart'; import 'package:todo/images/dufs_image_client.dart'; import 'package:todo/images/image_cache_store.dart'; import 'package:todo/images/image_processing.dart'; -/// Largest upload accepted from the page, before re-encoding (a raw 50 MP -/// phone photo is ~25 MB). -const int kMaxImageUploadBytes = 64 * 1024 * 1024; - /// The routes the real wrapper uses: cache under /// `~/.local/share/todo-desktop/images`, login from `todo.env`. WrapperImageRoutes defaultWrapperImageRoutes(String home) => WrapperImageRoutes( @@ -49,9 +51,13 @@ class WrapperImageRoutes { http.Client Function()? httpClient, Future Function(Uint8List)? prepare, }) : _httpClient = httpClient ?? http.Client.new, - _prepare = prepare ?? prepareImageInBackground; + _prepare = prepare ?? prepareImageInBackground, + _ingest = AttachmentIngest( + store, + prepare: prepare ?? prepareImageInBackground, + ); - /// The on-disk image cache and upload queue. + /// The on-disk attachment cache and upload queue. final ImageCacheStore store; /// `todo.env` written by `add_dufs_login.sh`. @@ -59,17 +65,21 @@ class WrapperImageRoutes { final http.Client Function() _httpClient; final Future Function(Uint8List) _prepare; + final AttachmentIngest _ingest; - static const _prefix = '/images/'; + /// Every path these routes answer starts with this. + static const prefix = '/images/'; /// Answers [request] and returns true when it is an `/images/` route. Future handle(HttpRequest request) async { final path = request.uri.path; - if (!path.startsWith(_prefix)) return false; - final rest = path.substring(_prefix.length); + if (!path.startsWith(prefix)) return false; + final rest = path.substring(prefix.length); final response = request.response; final method = request.method; - if (rest == 'reconcile' && method == 'POST') { + if (rest == 'attach' && method == 'POST') { + await _attach(request); + } else if (rest == 'reconcile' && method == 'POST') { await _reconcile(request); } else if (rest == 'status' && method == 'GET') { _json(response, { @@ -77,9 +87,14 @@ class WrapperImageRoutes { 'pending': store.pending().length, }); } else if (AttachmentRef.tryParse(rest) case final ref?) { - if (method == 'GET') { - await _serve(response, ref); - } else if (method == 'POST') { + if (method == 'GET' || method == 'HEAD') { + await serveAttachment( + request, + ref, + store.fileFor(ref), + fileName: request.uri.queryParameters['name'], + ); + } else if (method == 'POST' && ref.extension == 'jpg') { await _put(request, ref); } else { response.statusCode = HttpStatus.methodNotAllowed; @@ -92,17 +107,29 @@ class WrapperImageRoutes { DufsLogin? _login() => readDufsLoginEnv(File(loginEnvPath)); - Future _serve(HttpResponse response, AttachmentRef ref) async { - final file = store.fileFor(ref); - if (!file.existsSync()) { - response.statusCode = HttpStatus.notFound; + Future _attach(HttpRequest request) async { + final query = request.uri.queryParameters; + final noteId = query['note'] ?? ''; + final response = request.response; + if (!isSafeNoteId(noteId)) { + // Read the body anyway: answering mid-upload looks like a dropped + // connection to the browser, not a 400. + await request.drain(); + response.statusCode = HttpStatus.badRequest; return; } - response.headers - ..contentType = ContentType('image', 'jpeg') - // The name is a fresh uuid per image, so the bytes never change. - ..set(HttpHeaders.cacheControlHeader, 'private, max-age=31536000'); - await response.addStream(file.openRead()); + try { + final attachment = await _ingest.fromStream( + noteId, + query['name'] ?? '', + request, + ); + _json(response, attachment.toJson()); + } on FileSystemException catch (e) { + response + ..statusCode = HttpStatus.internalServerError + ..write(e.message); + } } Future _put(HttpRequest request, AttachmentRef ref) async { @@ -112,7 +139,7 @@ class WrapperImageRoutes { // is still sending makes it see a dropped connection, not the 413. await for (final chunk in request) { tooLarge = - tooLarge || builder.length + chunk.length > kMaxImageUploadBytes; + tooLarge || builder.length + chunk.length > kMaxImageDecodeBytes; if (!tooLarge) builder.add(chunk); } if (tooLarge) { diff --git a/lib/desktop/wrapper_server.dart b/lib/desktop/wrapper_server.dart index f0ab5b2..6e64c4d 100644 --- a/lib/desktop/wrapper_server.dart +++ b/lib/desktop/wrapper_server.dart @@ -5,6 +5,7 @@ import 'dart:io'; import 'package:crdt_sync/crdt_sync.dart'; import 'package:path/path.dart' as p; +import 'package:todo/desktop/static_content_type.dart'; import 'package:todo/desktop/wrapper_image_routes.dart'; export 'wrapper_paths.dart'; @@ -127,15 +128,48 @@ class WrapperServer { Future _serve(HttpServer server) async { await for (final request in server) { - try { - await _handle(request); - } on Exception { - request.response.statusCode = HttpStatus.internalServerError; + // Attachment routes can run for minutes (a multi-gigabyte upload, a + // video being watched, a reconcile fetching a large file) and are safe + // side by side, so they must not hold up the page's own requests. + // Everything else stays one at a time: the backup files are + // overwritten whole and must not interleave. + if (request.uri.path.startsWith(WrapperImageRoutes.prefix)) { + unawaited(_respond(request)); + } else { + await _respond(request); } - await request.response.close(); } } + Future _respond(HttpRequest request) async { + try { + if (_isCrossSiteWrite(request)) { + // Read the body anyway: answering mid-upload looks like a dropped + // connection, not a refusal. + await request.drain(); + request.response.statusCode = HttpStatus.forbidden; + } else { + await _handle(request); + } + } on Exception { + request.response.statusCode = HttpStatus.internalServerError; + } + await request.response.close(); + } + + /// A write sent by some other site's page. Any website open in the same + /// browser can POST here without a preflight (a CORS "simple request"), + /// which would let it overwrite the backups or push unbounded files into + /// the attachment store and on to dufs. The app's own page always sends its + /// own origin; local tools (curl, tests) send none. + bool _isCrossSiteWrite(HttpRequest request) { + if (request.method == 'GET' || request.method == 'HEAD') return false; + final origin = request.headers.value('origin'); + return origin != null && + origin != 'http://localhost:$port' && + origin != 'http://127.0.0.1:$port'; + } + Future _handle(HttpRequest request) async { final path = request.uri.path; if (path == '/backup/backlog') { @@ -199,39 +233,7 @@ class WrapperServer { await request.response.addStream(file.openRead()); } - /// Content type for [filePath]. - /// - /// Flutter web is strict here: CanvasKit refuses to instantiate a `.wasm` - /// served as anything but `application/wasm`, and the app silently fails to - /// render if the bootstrap `.js` is mislabelled. - static ContentType contentTypeFor(String filePath) { - switch (p.extension(filePath).toLowerCase()) { - case '.html': - return ContentType.html; - case '.js' || '.mjs': - return ContentType('text', 'javascript', charset: 'utf-8'); - case '.json': - return ContentType.json; - case '.wasm': - return ContentType('application', 'wasm'); - case '.css': - return ContentType('text', 'css', charset: 'utf-8'); - case '.png': - return ContentType('image', 'png'); - case '.jpg' || '.jpeg': - return ContentType('image', 'jpeg'); - case '.svg': - return ContentType('image', 'svg+xml'); - case '.ttf': - return ContentType('font', 'ttf'); - case '.otf': - return ContentType('font', 'otf'); - case '.woff2': - return ContentType('font', 'woff2'); - case '.bin' || '.symbols': - return ContentType.binary; - default: - return ContentType.binary; - } - } + /// Content type for [filePath]; see [staticContentTypeFor]. + static ContentType contentTypeFor(String filePath) => + staticContentTypeFor(filePath); } diff --git a/lib/images/attachment_ingest.dart b/lib/images/attachment_ingest.dart new file mode 100644 index 0000000..f1f6c14 --- /dev/null +++ b/lib/images/attachment_ingest.dart @@ -0,0 +1,112 @@ +/// Turning an incoming file into a stored attachment: re-encoded as an image +/// when it is one, kept byte-for-byte otherwise. +/// +/// The one place that decides, shared by the Android store and the desktop +/// wrapper. The file lands on disk first and is classified from there, so its +/// size never reaches memory: only something that sniffs as a photo format +/// (see `looksLikeImage`) and is at most [kMaxImageDecodeBytes] is read whole, +/// to be decoded. An image that fails to decode is kept as a file rather than +/// refused. +/// +/// Pure Dart + `dart:io`. +library; + +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/image_cache_store.dart'; +import 'package:todo/images/image_processing.dart'; + +/// Stores incoming files in [store], queued for upload. +class AttachmentIngest { + /// Creates an ingest over [store]; [prepare] is injectable for tests. + AttachmentIngest( + this.store, { + Future Function(Uint8List)? prepare, + }) : _prepare = prepare ?? prepareImageInBackground; + + /// Where attachments are kept. + final ImageCacheStore store; + + final Future Function(Uint8List) _prepare; + + /// Stores [data], the contents of a file called [name], for [noteId]. + Future fromStream( + String noteId, + String name, + Stream> data, + ) async { + final ref = AttachmentRef.forFile(noteId, name); + final tmp = store.tempFor(ref); + await tmp.parent.create(recursive: true); + return await _guarded(tmp, () async { + final sink = tmp.openWrite(); + try { + await sink.addStream(data); + } finally { + await sink.close(); + } + return await _settle(ref, name, tmp); + }); + } + + /// Stores the local file [source], called [name], for [noteId]. + /// + /// With [move] (a picker's private copy, nobody else's file) it is renamed + /// into place, so a multi-gigabyte pick is never on disk twice; otherwise, + /// or across filesystems, it is copied. + Future fromFile( + String noteId, + String name, + File source, { + required bool move, + }) async { + if (!move) return await fromStream(noteId, name, source.openRead()); + final ref = AttachmentRef.forFile(noteId, name); + final tmp = store.tempFor(ref); + await tmp.parent.create(recursive: true); + try { + await source.rename(tmp.path); + } on FileSystemException { + final attachment = await fromStream(noteId, name, source.openRead()); + await source.delete(); + return attachment; + } + return await _guarded(tmp, () => _settle(ref, name, tmp)); + } + + /// Runs [body], removing [tmp] if it fails so a failed attach leaves + /// nothing behind. + Future _guarded( + File tmp, + Future Function() body, + ) async { + try { + return await body(); + } on Object { + if (tmp.existsSync()) await tmp.delete(); + rethrow; + } + } + + Future _settle(AttachmentRef ref, String name, File tmp) async { + if (await tmp.length() <= kMaxImageDecodeBytes && await _sniff(tmp)) { + final jpeg = await _prepare(await tmp.readAsBytes()); + if (jpeg != null) { + final image = AttachmentRef.create(ref.noteId); + await store.put(image, jpeg); + await tmp.delete(); + return Attachment(ref: image, name: name, isImage: true); + } + } + await store.adopt(ref, tmp); + return Attachment(ref: ref, name: name, isImage: false); + } + + Future _sniff(File file) async { + final head = []; + await file.openRead(0, kImageSniffBytes).forEach(head.addAll); + return looksLikeImage(head); + } +} diff --git a/lib/images/attachment_links.dart b/lib/images/attachment_links.dart new file mode 100644 index 0000000..6490485 --- /dev/null +++ b/lib/images/attachment_links.dart @@ -0,0 +1,65 @@ +/// Finding attachment links in a note's text, and rewriting them for +/// `BACKLOG.md`. +/// +/// One regex reads both shapes: `![](url)` is an image, `[name](url)` a file. +/// Matching them together (rather than a file regex beside an image regex) +/// is what keeps an image link, which contains `[](url)`, from also counting +/// as a file. +/// +/// Pure Dart: the desktop wrapper imports it. +library; + +import 'package:todo/images/attachment_ref.dart'; + +final String _base = RegExp.escape('$kDufsBaseUrl/$kImagesDir/'); +const _label = r'\[[^\]\n]*\]\('; +const _link = r'(!?)\[([^\]\n]*)\]\('; +final _anyLink = RegExp('$_link$_base($kAttachmentPathPattern)\\)'); +final _wholeLine = RegExp('^\\s*${_anyLink.pattern}\\s*\$'); +final _urlLink = RegExp('($_label)$_base($kAttachmentPathPattern\\))'); +final _backlogLink = RegExp( + '($_label)${RegExp.escape('$kBacklogImagesRoot/')}' + '($kAttachmentPathPattern\\))', +); + +Attachment _attachment(RegExpMatch match) => Attachment( + ref: AttachmentRef.tryParse(match.group(3)!)!, + name: match.group(2)!, + isImage: match.group(1)!.isNotEmpty, +); + +/// Every attachment [text] links to, in order, each ref once. +List attachmentsIn(String text) { + final seen = {}; + final found = []; + for (final match in _anyLink.allMatches(text)) { + final attachment = _attachment(match); + if (seen.add(attachment.ref)) found.add(attachment); + } + return found; +} + +/// Every ref [text] links to, images and files alike. +Set attachmentRefsIn(String text) => { + for (final attachment in attachmentsIn(text)) attachment.ref, +}; + +/// The attachment when [line] is nothing but one attachment link, else null. +Attachment? attachmentOfLine(String line) { + final match = _wholeLine.firstMatch(line); + return match == null ? null : _attachment(match); +} + +/// Rewrites attachment links to their on-disk path under +/// [kBacklogImagesRoot]. +String toBacklogAttachmentPaths(String text) => text.replaceAllMapped( + _urlLink, + (m) => '${m[1]}$kBacklogImagesRoot/${m[2]}', +); + +/// Exact inverse of [toBacklogAttachmentPaths], so an exported backlog +/// imports back with its links intact. +String fromBacklogAttachmentPaths(String text) => text.replaceAllMapped( + _backlogLink, + (m) => '${m[1]}$kDufsBaseUrl/$kImagesDir/${m[2]}', +); diff --git a/lib/images/attachment_ref.dart b/lib/images/attachment_ref.dart index 8f5cbcf..e40cb3b 100644 --- a/lib/images/attachment_ref.dart +++ b/lib/images/attachment_ref.dart @@ -1,10 +1,11 @@ -/// Where a note's image lives, and how a note's text points at it. +/// Where a note's attachment lives on dufs. /// -/// A note never stores image bytes: it carries a plain markdown link -/// `![](https://kuhy-cloud.duckdns.org/todo-images//.jpg)`, so the -/// CRDT log, the sync format and every older build stay untouched (an old -/// build just shows the link as text). This file is the single source of truth -/// for that link's shape; everything else goes through it. +/// A note never stores attachment bytes: it carries a plain markdown link to +/// `https://kuhy-cloud.duckdns.org/todo-images//.` β€” an +/// image as `![](…)`, any other file as `[name](…)` β€” so the CRDT log, the +/// sync format and every older build stay untouched (an old build just shows +/// the link as text). This file owns the stored path's shape; +/// `attachment_links.dart` owns the links that point at it. /// /// Pure Dart: the desktop wrapper (`bin/todo_desktop.dart`) imports it. library; @@ -14,78 +15,139 @@ import 'package:uuid/uuid.dart'; /// Public origin of kuhy's dufs server (Caddy β†’ dufs on 127.0.0.1:5000). const kDufsBaseUrl = 'https://kuhy-cloud.duckdns.org'; -/// The dufs folder the `todo` login is scoped to. +/// The dufs folder the `todo` login is scoped to. Named for images, which +/// came first; it holds every kind of attachment. const kImagesDir = 'todo-images'; /// Where the same folder sits on kuhy's PC, as written into `BACKLOG.md` so an -/// agent reading the backlog can open the image straight from disk. +/// agent reading the backlog can open the attachment straight from disk. const kBacklogImagesRoot = '~/data/cloud/$kImagesDir'; +/// Extension given to a file whose name has none usable. +const kUnknownExtension = 'bin'; + // Segments are restricted to URL- and filesystem-safe characters, which is // what makes a ref safe to join onto a cache directory without a traversal -// check of its own. +// check of its own. The single, dot-free extension also keeps the cache's +// `.pending` and `.tmp` companions from ever parsing as refs. const _segment = '[A-Za-z0-9_-]+'; -const _path = '$_segment/$_segment\\.jpg'; -const _label = r'!\[[^\]\n]*\]\('; -final _refPath = RegExp('^($_segment)/($_segment\\.jpg)\$'); -final _urlLink = RegExp( - '$_label${RegExp.escape('$kDufsBaseUrl/$kImagesDir/')}($_path)\\)', -); -final _backlogLink = RegExp( - '($_label)${RegExp.escape('$kBacklogImagesRoot/')}($_path\\))', -); -final _wholeLine = RegExp('^\\s*${_urlLink.pattern}\\s*\$'); +const _extension = '[a-z0-9]{1,10}'; -/// One image, addressed as `/` under [kImagesDir]. +/// The pattern of a stored path, `/.`, for link regexes. +const kAttachmentPathPattern = '$_segment/$_segment\\.$_extension'; + +final _refPath = RegExp('^$kAttachmentPathPattern\$'); +final _usableExtension = RegExp('^$_extension\$'); +final _noteId = RegExp('^$_segment\$'); + +/// Whether [noteId] can name an attachment folder. +bool isSafeNoteId(String noteId) => _noteId.hasMatch(noteId); + +/// One stored attachment, addressed as `/` under [kImagesDir]. /// /// An extension type over that relative path: equality and hashing are the /// string's, so refs drop straight into sets. extension type const AttachmentRef._(String relativePath) { - /// A new, unique ref for an image being attached to [noteId]. - factory AttachmentRef.create(String noteId) { - final ref = AttachmentRef.tryParse('$noteId/${const Uuid().v4()}.jpg'); + /// A new, unique ref for an image being attached to [noteId]. Images are + /// always stored re-encoded, hence always `.jpg`. + factory AttachmentRef.create(String noteId) => _fresh(noteId, 'jpg'); + + /// A new, unique ref for the file [fileName] being attached to [noteId], + /// keeping its extension when that is short and plain, else + /// [kUnknownExtension]. + factory AttachmentRef.forFile(String noteId, String fileName) => + _fresh(noteId, extensionOf(fileName)); + + static AttachmentRef _fresh(String noteId, String extension) { + final ref = tryParse('$noteId/${const Uuid().v4()}.$extension'); if (ref == null) throw ArgumentError.value(noteId, 'noteId'); return ref; } - /// Parses `/.jpg`, or null when it is not a safe image path. + /// Parses `/.`, or null when it is not a safe path. static AttachmentRef? tryParse(String relativePath) => _refPath.hasMatch(relativePath) ? AttachmentRef._(relativePath) : null; - /// The note the image was attached to (it names the folder only). + /// The note the attachment was added to (it names the folder only). String get noteId => relativePath.substring(0, relativePath.indexOf('/')); - /// `.jpg`. + /// `.`. String get file => relativePath.substring(relativePath.indexOf('/') + 1); + /// The lower-case extension, without its dot. + String get extension => + relativePath.substring(relativePath.lastIndexOf('.') + 1); + /// The public dufs URL stored in the note. String get url => '$kDufsBaseUrl/$kImagesDir/$relativePath'; - /// The markdown line inserted into a note. - String get markdown => '![]($url)'; + /// The markdown line that shows this as an inline image. + String get imageMarkdown => '![]($url)'; + + /// The markdown line that links this as the file [name]. + String fileMarkdown(String name) => '[${linkLabel(name)}]($url)'; } -/// Every image [text] links to, in order, without duplicates. -Set imageRefsIn(String text) => { - for (final match in _urlLink.allMatches(text)) - AttachmentRef.tryParse(match.group(1)!)!, -}; - -/// The image when [line] is nothing but one image link, else null. -AttachmentRef? imageRefOfLine(String line) { - final match = _wholeLine.firstMatch(line); - return match == null ? null : AttachmentRef.tryParse(match.group(1)!); +/// [fileName]'s extension, lower-cased, when it is 1–10 letters or digits; +/// otherwise [kUnknownExtension]. +String extensionOf(String fileName) { + final dot = fileName.lastIndexOf('.'); + if (dot <= 0) return kUnknownExtension; + final extension = fileName.substring(dot + 1).toLowerCase(); + return _usableExtension.hasMatch(extension) ? extension : kUnknownExtension; } -/// Rewrites image links to their on-disk path under [kBacklogImagesRoot]. -String toBacklogImagePaths(String text) => text.replaceAllMapped( - _urlLink, - (m) => m[0]!.replaceFirst('$kDufsBaseUrl/$kImagesDir', kBacklogImagesRoot), -); +/// [name] made safe as a markdown link label: brackets would end the label +/// early and a line break would split the link, so both are replaced. +String linkLabel(String name) { + final label = name + .replaceAll('[', '(') + .replaceAll(']', ')') + .replaceAll(RegExp(r'[\r\n]+'), ' ') + .trim(); + return label.isEmpty ? 'file' : label; +} -/// Exact inverse of [toBacklogImagePaths], so an exported backlog imports -/// back with its links intact. -String fromBacklogImagePaths(String text) => text.replaceAllMapped( - _backlogLink, - (m) => '${m[1]}$kDufsBaseUrl/$kImagesDir/${m[2]}', -); +/// An attachment as inserted into a note: the stored ref, the name it was +/// picked under, and whether it is shown as an image. +class Attachment { + /// Creates an attachment. + const Attachment({ + required this.ref, + required this.name, + required this.isImage, + }); + + /// Where it is stored. + final AttachmentRef ref; + + /// The file name it was picked or dropped under. + final String name; + + /// True for a re-encoded image (`![](…)`), false for a plain file. + final bool isImage; + + /// The line inserted into the note. + String get markdown => isImage ? ref.imageMarkdown : ref.fileMarkdown(name); + + /// JSON form, the desktop wrapper's answer to an attach. + Map toJson() => { + 'ref': ref.relativePath, + 'name': name, + 'image': isImage, + }; + + /// Parses [toJson]'s output; throws [FormatException] on anything else. + static Attachment fromJson(Object? json) { + if (json case { + 'ref': final String path, + 'name': final String name, + 'image': final bool isImage, + }) { + if (AttachmentRef.tryParse(path) case final ref?) { + return Attachment(ref: ref, name: name, isImage: isImage); + } + } + throw FormatException('Not an attachment: $json'); + } +} diff --git a/lib/images/dufs_image_client.dart b/lib/images/dufs_image_client.dart index d03cde7..c51d1fe 100644 --- a/lib/images/dufs_image_client.dart +++ b/lib/images/dufs_image_client.dart @@ -1,15 +1,17 @@ -/// The three WebDAV calls image sync needs, against kuhy's dufs. +/// The three WebDAV calls attachment sync needs, against kuhy's dufs. /// /// Deliberately not dufs-cloud's full `DufsClient` (~700 lines with listing, /// moves, streaming progress and an xml parser): todo only ever puts, gets and /// checks one folder. dufs creates missing parent folders on PUT (verified /// against dufs 0.46.0), so there is no MKCOL step. /// +/// Bodies stream both ways: an attachment can be gigabytes, and neither the +/// phone nor the wrapper may hold one in memory. +/// /// Pure Dart: the desktop wrapper imports it. library; import 'dart:convert'; -import 'dart:typed_data'; import 'package:http/http.dart' as http; @@ -46,7 +48,7 @@ enum DufsCheck { unreachable, } -/// Upload / download / check for images under [kImagesDir]. +/// Upload / download / check for attachments under [kImagesDir]. class DufsImageClient { /// Creates a client for [login]; [httpClient] is injectable for tests. DufsImageClient(this.login, {http.Client? httpClient}) @@ -65,25 +67,42 @@ class DufsImageClient { Uri _uri(String relative) => Uri.parse('${login.baseUrl}/$kImagesDir/$relative'); - /// PUTs [bytes] as [ref]; true on a 2xx. - Future upload(AttachmentRef ref, Uint8List bytes) async { + /// PUTs the [length] bytes of [body] as [ref]; true on a 2xx. + /// + /// [body] is only listened to once the connection is up, so a file stream + /// passed here is never read when dufs is unreachable. + Future upload( + AttachmentRef ref, + Stream> body, + int length, + ) async { try { - final response = await _http.put( - _uri(ref.relativePath), - headers: {..._auth, 'content-type': 'image/jpeg'}, - body: bytes, - ); + final request = _StreamBodyRequest('PUT', _uri(ref.relativePath), body) + ..contentLength = length + ..headers.addAll({ + ..._auth, + 'content-type': ref.extension == 'jpg' + ? 'image/jpeg' + : 'application/octet-stream', + }); + final response = await _http.send(request); + await response.stream.drain(); return response.statusCode >= 200 && response.statusCode < 300; } on Exception { return false; } } - /// GETs [ref]'s bytes, or null when it is missing or unreachable. - Future download(AttachmentRef ref) async { + /// GETs [ref], returning its body as a stream, or null when it is missing + /// or unreachable. The caller must consume the stream. + Future>?> openDownload(AttachmentRef ref) async { try { - final response = await _http.get(_uri(ref.relativePath), headers: _auth); - return response.statusCode == 200 ? response.bodyBytes : null; + final request = http.Request('GET', _uri(ref.relativePath)) + ..headers.addAll(_auth); + final response = await _http.send(request); + if (response.statusCode == 200) return response.stream; + await response.stream.drain(); + return null; } on Exception { return null; } @@ -109,3 +128,18 @@ class DufsImageClient { /// Releases the underlying HTTP client. void close() => _http.close(); } + +/// A request whose body is an existing stream, pulled by the client as it +/// sends. Unlike `http.StreamedRequest`, nothing is buffered when the +/// connection fails before the body is wanted. +class _StreamBodyRequest extends http.BaseRequest { + _StreamBodyRequest(super.method, super.url, this._body); + + final Stream> _body; + + @override + http.ByteStream finalize() { + super.finalize(); + return http.ByteStream(_body); + } +} diff --git a/lib/images/image_cache_store.dart b/lib/images/image_cache_store.dart index 74cf86c..5abf4d9 100644 --- a/lib/images/image_cache_store.dart +++ b/lib/images/image_cache_store.dart @@ -1,9 +1,15 @@ -/// The on-device copy of every image a note links to, plus the upload queue. +/// The on-device copy of every attachment a note links to, plus the upload +/// queue. /// -/// One directory, `//.jpg`, mirrors dufs' `todo-images/`. -/// An image attached while offline gets a `.jpg.pending` marker next to -/// it; the marker is the queue, so it survives restarts with no separate -/// bookkeeping, and it is only removed after dufs has accepted the upload. +/// One directory, `//.`, mirrors dufs' +/// `todo-images/`. An attachment added while offline gets a +/// `..pending` marker next to it; the marker is the queue, so it +/// survives restarts with no separate bookkeeping, and it is only removed +/// after dufs has accepted the upload. +/// +/// Files are never held in memory whole: uploads stream from disk and +/// downloads stream to a `.tmp` beside the target, renamed only once complete, +/// so an interrupted download cannot leave a truncated file that looks done. /// /// Used as-is by the Android app and by the desktop wrapper (the web page /// never talks to dufs). Pure Dart + `dart:io`. @@ -16,57 +22,25 @@ import 'dart:typed_data'; import 'package:path/path.dart' as p; import 'package:todo/images/attachment_ref.dart'; import 'package:todo/images/dufs_image_client.dart'; +import 'package:todo/images/reconcile_report.dart'; -/// How long a cached, unreferenced image is kept before eviction. +export 'package:todo/images/reconcile_report.dart'; + +/// How long a cached, unreferenced attachment is kept before eviction, and +/// how old an abandoned `.tmp` must be before it is swept. /// -/// Covers the gap between writing an image and its link reaching the note -/// store (and, on desktop, the page's next reconcile): without it a -/// reconcile landing in that gap would delete a fresh image. +/// Covers the gap between writing an attachment and its link reaching the +/// note store (and, on desktop, the page's next reconcile): without it a +/// reconcile landing in that gap would delete a fresh attachment. const kEvictionGrace = Duration(minutes: 10); -/// Counts from one [ImageCacheStore.reconcile] pass. -class ReconcileReport { - /// Creates a report. - const ReconcileReport({ - this.uploaded = 0, - this.downloaded = 0, - this.evicted = 0, - this.pending = 0, - this.missing = 0, - }); - - /// Queued images dufs accepted this pass. - final int uploaded; - - /// Linked images fetched from dufs this pass. - final int downloaded; - - /// Unlinked cached images deleted. - final int evicted; - - /// Images still waiting to upload after the pass. - final int pending; - - /// Linked images still not on this device after the pass. - final int missing; - - /// JSON form, for the wrapper's `/images/reconcile` response. - Map toJson() => { - 'uploaded': uploaded, - 'downloaded': downloaded, - 'evicted': evicted, - 'pending': pending, - 'missing': missing, - }; -} - -/// Local image files and their upload queue under [root]. +/// Local attachment files and their upload queue under [root]. class ImageCacheStore { /// Creates a store rooted at [root]; [now] is injectable for tests. ImageCacheStore(this.root, {DateTime Function()? now}) : _now = now ?? DateTime.now; - /// Directory holding `/.jpg`. + /// Directory holding `/.`. final Directory root; final DateTime Function() _now; @@ -81,6 +55,9 @@ class ImageCacheStore { File fileFor(AttachmentRef ref) => File(p.join(root.path, ref.noteId, ref.file)); + /// Scratch file next to [ref]'s, for writes that must land whole. + File tempFor(AttachmentRef ref) => File('${fileFor(ref).path}.tmp'); + File _marker(AttachmentRef ref) => File('${fileFor(ref).path}.pending'); /// Whether [ref] is waiting to upload. @@ -88,29 +65,35 @@ class ImageCacheStore { /// Stores [bytes] as [ref] and queues it for upload. Future put(AttachmentRef ref, Uint8List bytes) async { - final file = fileFor(ref); - await file.parent.create(recursive: true); - // Marker first: a crash between the two writes then leaves a marker with - // no file (skipped by reconcile), never a file that silently never uploads. - await _marker(ref).writeAsString(''); - final tmp = File('${file.path}.tmp'); + final tmp = tempFor(ref); + await tmp.parent.create(recursive: true); await tmp.writeAsBytes(bytes, flush: true); - await tmp.rename(file.path); + await adopt(ref, tmp); } - /// Every image waiting to upload. + /// Moves the complete file [tmp] into place as [ref] and queues it for + /// upload. [tmp] must be on the same filesystem (see [tempFor]). + Future adopt(AttachmentRef ref, File tmp) async { + await fileFor(ref).parent.create(recursive: true); + // Marker first: a crash between the two then leaves a marker with no + // file (skipped by reconcile), never a file that silently never uploads. + await _marker(ref).writeAsString(''); + await tmp.rename(fileFor(ref).path); + } + + /// Every attachment waiting to upload. List pending() => [ for (final marker in _files('.pending')) ?AttachmentRef.tryParse(_relative(marker).replaceFirst('.pending', '')), ]; - /// Uploads the queue, fetches linked images this device lacks, and evicts - /// cached images nothing links to any more. + /// Uploads the queue, fetches linked attachments this device lacks, and + /// evicts cached ones nothing links to any more. /// /// Without a [client] (no password set yet) only eviction runs. Eviction is /// skipped entirely when [referenced] is empty β€” an empty set far more - /// likely means "notes not loaded yet" than "every image unlinked" β€” and it - /// never touches a queued image or one younger than [kEvictionGrace]. + /// likely means "notes not loaded yet" than "everything unlinked" β€” and it + /// never touches a queued attachment or one younger than [kEvictionGrace]. /// /// Overlapping calls do not run in parallel: calls made before the next /// pass starts share it, with their refs unioned and the newest client. @@ -149,19 +132,15 @@ class ImageCacheStore { for (final ref in pending()) { final file = fileFor(ref); if (!file.existsSync()) continue; - if (await client.upload(ref, await file.readAsBytes())) { + final length = await file.length(); + if (await client.upload(ref, file.openRead(), length)) { await _marker(ref).delete(); uploaded++; } } for (final ref in pass.referenced) { if (fileFor(ref).existsSync()) continue; - final bytes = await client.download(ref); - if (bytes == null) continue; - final file = fileFor(ref); - await file.parent.create(recursive: true); - await file.writeAsBytes(bytes, flush: true); - downloaded++; + if (await _download(ref, client)) downloaded++; } } final evicted = pass.referenced.isEmpty ? 0 : _evict(pass.referenced); @@ -174,13 +153,40 @@ class ImageCacheStore { ); } + Future _download(AttachmentRef ref, DufsImageClient client) async { + final body = await client.openDownload(ref); + if (body == null) return false; + final tmp = tempFor(ref); + try { + await tmp.parent.create(recursive: true); + final sink = tmp.openWrite(); + try { + await sink.addStream(body); + } finally { + await sink.close(); + } + await tmp.rename(fileFor(ref).path); + return true; + } on Exception { + // A dropped connection mid-body: retry from scratch next pass. + if (tmp.existsSync()) await tmp.delete(); + return false; + } + } + int _evict(Set referenced) { final cutoff = _now().subtract(kEvictionGrace); var evicted = 0; - for (final file in _files('.jpg')) { + for (final file in _files('')) { + if (file.lastModifiedSync().isAfter(cutoff)) continue; + // An abandoned write (killed mid-download or mid-attach): unreachable + // by any ref, and possibly gigabytes. + if (file.path.endsWith('.tmp')) { + file.deleteSync(); + continue; + } final ref = AttachmentRef.tryParse(_relative(file)); if (ref == null || referenced.contains(ref) || isPending(ref)) continue; - if (file.lastModifiedSync().isAfter(cutoff)) continue; file.deleteSync(); evicted++; } diff --git a/lib/images/image_processing.dart b/lib/images/image_processing.dart index 625ca08..41491df 100644 --- a/lib/images/image_processing.dart +++ b/lib/images/image_processing.dart @@ -24,6 +24,40 @@ const kMaxImageSide = 2560; /// JPEG quality of a stored image. const kJpegQuality = 85; +/// Largest attachment still tried as an image (a raw 50 MP phone photo is +/// ~25 MB). Anything bigger is kept as a file: decoding it would need several +/// times its size in memory. +const int kMaxImageDecodeBytes = 64 * 1024 * 1024; + +/// How many leading bytes [looksLikeImage] needs. +const kImageSniffBytes = 12; + +/// Whether [head] starts like a photo or screenshot format: JPEG, PNG, GIF, +/// WebP, BMP or TIFF. +/// +/// Only these are re-encoded. `package:image` also decodes PSD, TGA, EXR and +/// ICO, but flattening those to a JPEG would destroy what they were attached +/// for, so they stay files. +bool looksLikeImage(List head) { + bool starts(List magic, [int at = 0]) { + if (head.length < at + magic.length) return false; + for (var i = 0; i < magic.length; i++) { + if (head[at + i] != magic[i]) return false; + } + return true; + } + + const riff = [0x52, 0x49, 0x46, 0x46]; + const webp = [0x57, 0x45, 0x42, 0x50]; + return starts(const [0xFF, 0xD8, 0xFF]) || + starts(const [0x89, 0x50, 0x4E, 0x47]) || + starts(const [0x47, 0x49, 0x46, 0x38]) || + (starts(riff) && starts(webp, 8)) || + starts(const [0x42, 0x4D]) || + starts(const [0x49, 0x49, 0x2A, 0x00]) || + starts(const [0x4D, 0x4D, 0x00, 0x2A]); +} + /// Re-encodes [bytes] as a metadata-free JPEG no larger than [kMaxImageSide] /// on its long side, or returns null when [bytes] is not a decodable image. Uint8List? prepareImage(Uint8List bytes) { diff --git a/lib/images/reconcile_report.dart b/lib/images/reconcile_report.dart new file mode 100644 index 0000000..296b426 --- /dev/null +++ b/lib/images/reconcile_report.dart @@ -0,0 +1,40 @@ +/// Counts from one `ImageCacheStore.reconcile` pass. +/// +/// Pure Dart: the desktop wrapper returns it as JSON to the page. +library; + +/// Counts from one reconcile pass. +class ReconcileReport { + /// Creates a report. + const ReconcileReport({ + this.uploaded = 0, + this.downloaded = 0, + this.evicted = 0, + this.pending = 0, + this.missing = 0, + }); + + /// Queued attachments dufs accepted this pass. + final int uploaded; + + /// Linked attachments fetched from dufs this pass. + final int downloaded; + + /// Unlinked cached attachments deleted. + final int evicted; + + /// Attachments still waiting to upload after the pass. + final int pending; + + /// Linked attachments still not on this device after the pass. + final int missing; + + /// JSON form, for the wrapper's `/images/reconcile` response. + Map toJson() => { + 'uploaded': uploaded, + 'downloaded': downloaded, + 'evicted': evicted, + 'pending': pending, + 'missing': missing, + }; +} diff --git a/lib/sync/notes_markdown.dart b/lib/sync/notes_markdown.dart index 7e420df..04c09ba 100644 --- a/lib/sync/notes_markdown.dart +++ b/lib/sync/notes_markdown.dart @@ -1,5 +1,5 @@ import 'package:todo/data/note.dart'; -import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/attachment_links.dart'; import 'package:uuid/uuid.dart'; /// Serialises notes to (and parses them back from) a single Markdown file. @@ -47,7 +47,7 @@ class NotesMarkdown { 'created="${note.createdAt.toIso8601String()}" ' 'updated="${note.updatedAt.toIso8601String()}" -->', ) - ..writeln(toBacklogImagePaths(note.text)) + ..writeln(toBacklogAttachmentPaths(note.text)) ..writeln(); } return buffer.toString(); @@ -68,7 +68,7 @@ class NotesMarkdown { final bodyEnd = i + 1 < markers.length ? markers[i + 1].start : content.length; - final body = fromBacklogImagePaths( + final body = fromBacklogAttachmentPaths( content.substring(bodyStart, bodyEnd).trim(), ); diff --git a/lib/ui/attachment_picking.dart b/lib/ui/attachment_picking.dart new file mode 100644 index 0000000..0ac2ea0 --- /dev/null +++ b/lib/ui/attachment_picking.dart @@ -0,0 +1,113 @@ +/// The πŸ“Ž button: pick photos from the camera or gallery, or any file +/// (Android), or any file from disk (desktop), and hand them on. +library; + +import 'package:file_picker/file_picker.dart' show FilePicker; +import 'package:file_selector/file_selector.dart'; +import 'package:flutter/foundation.dart'; +import 'package:flutter/material.dart'; +import 'package:image_picker/image_picker.dart'; + +export 'package:file_selector/file_selector.dart' show XFile; + +/// Where the phone takes an attachment from. +enum AttachmentPickSource { + /// A new photo. + camera, + + /// Existing photos, several at once. + gallery, + + /// Any files, several at once, from the system file picker. + file, +} + +/// Picks files and returns them; empty when the user cancels. +/// +/// Injectable so widget tests never reach a platform picker. +typedef AttachmentPicking = Future> Function(BuildContext context); + +/// The platform picker: a Camera/Gallery/File sheet on Android, an +/// unfiltered multi-select file dialog on the desktop (web) build. +/// +/// [fromPhone] is the platform call behind the sheet, injectable for tests. +Future> pickAttachments( + BuildContext context, { + Future> Function(AttachmentPickSource) fromPhone = _pickFromPhone, +}) async { + // The desktop build has no sheet: straight to the file dialog. + if (kIsWeb) return await openFiles(); // coverage:ignore-line + final source = await showModalBottomSheet( + context: context, + builder: (context) => SafeArea( + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + ListTile( + leading: const Icon(Icons.photo_camera_outlined), + title: const Text('Camera'), + onTap: () => Navigator.pop(context, AttachmentPickSource.camera), + ), + ListTile( + leading: const Icon(Icons.photo_library_outlined), + title: const Text('Gallery'), + onTap: () => Navigator.pop(context, AttachmentPickSource.gallery), + ), + ListTile( + leading: const Icon(Icons.insert_drive_file_outlined), + title: const Text('File'), + onTap: () => Navigator.pop(context, AttachmentPickSource.file), + ), + ], + ), + ), + ); + if (source == null) return const []; + return await fromPhone(source); +} + +// coverage:ignore-start +// Thin calls into platform pickers with no test binding; the sheet above and +// everything downstream of the returned files are covered. +// +// "File" uses file_picker, not file_selector: file_selector's Android side +// reads the whole file into memory on the UI thread (an `int`-sized byte +// array, then across the platform channel), so a large pick crashes the app. +// file_picker streams it into the cache directory on a background thread. +Future> _pickFromPhone(AttachmentPickSource source) async { + final picker = ImagePicker(); + return switch (source) { + AttachmentPickSource.camera => [ + ?await picker.pickImage(source: ImageSource.camera), + ], + AttachmentPickSource.gallery => await picker.pickMultiImage(), + AttachmentPickSource.file => [ + for (final file in await FilePicker.pickFiles()) file.xFile, + ], + }; +} +// coverage:ignore-end + +/// App-bar action that picks attachments and passes them to [onFiles]. +class AttachButton extends StatelessWidget { + /// Creates the button; [picking] defaults to [pickAttachments]. + const AttachButton({required this.onFiles, this.picking, super.key}); + + /// Receives the picked files (never called for a cancelled pick). + final ValueChanged> onFiles; + + /// The picker, injectable for tests. + final AttachmentPicking? picking; + + @override + Widget build(BuildContext context) { + return IconButton( + tooltip: 'Attach', + icon: const Icon(Icons.attach_file), + onPressed: () async { + final files = await (picking ?? pickAttachments)(context); + if (files.isNotEmpty) onFiles(files); + }, + ); + } +} diff --git a/lib/ui/capture_app_bar.dart b/lib/ui/capture_app_bar.dart index 53604bb..44f8cb9 100644 --- a/lib/ui/capture_app_bar.dart +++ b/lib/ui/capture_app_bar.dart @@ -4,13 +4,11 @@ /// screen still owns every action; this just lays the row out. library; -import 'dart:typed_data'; - import 'package:flutter/material.dart'; import 'package:todo/data/note_repository.dart'; import 'package:todo/frame_stats.dart'; -import 'package:todo/ui/image_picking.dart'; +import 'package:todo/ui/attachment_picking.dart'; /// App bar for the capture screen. class CaptureAppBar extends StatelessWidget implements PreferredSizeWidget { @@ -23,8 +21,8 @@ class CaptureAppBar extends StatelessWidget implements PreferredSizeWidget { required this.onSync, required this.onOpenList, required this.onOpenSettings, - required this.onAttachImages, - this.imagePicking, + required this.onAttachFiles, + this.attachmentPicking, super.key, }); @@ -49,11 +47,11 @@ class CaptureAppBar extends StatelessWidget implements PreferredSizeWidget { /// Opens the settings screen. final VoidCallback onOpenSettings; - /// Receives images picked with the πŸ“Ž action. - final ValueChanged> onAttachImages; + /// Receives files picked with the πŸ“Ž action. + final ValueChanged> onAttachFiles; /// The picker behind πŸ“Ž; null uses the platform one. - final ImagePicking? imagePicking; + final AttachmentPicking? attachmentPicking; @override Size get preferredSize => const Size.fromHeight(kToolbarHeight); @@ -80,7 +78,7 @@ class CaptureAppBar extends StatelessWidget implements PreferredSizeWidget { ), // coverage:ignore-end // Not gated on advanced mode: attaching is basic capture. - AttachImageButton(onImages: onAttachImages, picking: imagePicking), + AttachButton(onFiles: onAttachFiles, picking: attachmentPicking), IconButton( tooltip: 'New note', onPressed: onNewNote, diff --git a/lib/ui/capture_screen.dart b/lib/ui/capture_screen.dart index b5b115b..da034b5 100644 --- a/lib/ui/capture_screen.dart +++ b/lib/ui/capture_screen.dart @@ -1,5 +1,6 @@ import 'dart:async'; import 'dart:typed_data'; + import 'package:crdt_sync/crdt_sync.dart'; import 'package:design_system/design_system.dart'; import 'package:flutter/material.dart'; @@ -14,6 +15,7 @@ import 'package:todo/data/note_template.dart'; import 'package:todo/sync/firebase_backend.dart'; import 'package:todo/sync/local_backup.dart'; import 'package:todo/sync/sync_settings.dart'; +import 'package:todo/ui/attachment_picking.dart'; import 'package:todo/ui/capture_app_bar.dart'; import 'package:todo/ui/capture_draft.dart'; import 'package:todo/ui/capture_status.dart'; @@ -21,7 +23,6 @@ import 'package:todo/ui/capture_status_footer.dart'; import 'package:todo/ui/capture_sync_runner.dart'; import 'package:todo/ui/image_attach.dart'; import 'package:todo/ui/image_input.dart'; -import 'package:todo/ui/image_picking.dart'; import 'package:todo/ui/note_form.dart'; import 'package:todo/ui/notes_list_screen.dart'; import 'package:todo/ui/settings_screen.dart'; @@ -147,8 +148,8 @@ class _CaptureScreenState extends State onSync: _sync, onOpenList: _openList, onOpenSettings: _openSettings, - onAttachImages: _attachImages, - imagePicking: widget.imagePicking, + onAttachFiles: _attachFiles, + attachmentPicking: widget.attachmentPicking, ), body: _captureBody(advanced: advanced), ); diff --git a/lib/ui/capture_screen_images.dart b/lib/ui/capture_screen_images.dart index d7bf78b..39a5e53 100644 --- a/lib/ui/capture_screen_images.dart +++ b/lib/ui/capture_screen_images.dart @@ -17,6 +17,15 @@ extension _CaptureImages on _CaptureScreenState { rejected: rejected, ); + /// Stores picked or dropped [files] under the draft's note (reserving its + /// id) and inserts their links at the cursor. + Future _attachFiles(List files) => attachFiles( + context, + noteId: _draft.reserveId, + target: _attach, + files: files, + ); + /// Uploads queued images, prefetches every image any note links to, and /// evicts what no note links to. Runs after every sync attempt, on resume /// and at launch (via the launch sync). diff --git a/lib/ui/capture_screen_sync.dart b/lib/ui/capture_screen_sync.dart index d6ab9f9..34ada3f 100644 --- a/lib/ui/capture_screen_sync.dart +++ b/lib/ui/capture_screen_sync.dart @@ -128,11 +128,13 @@ extension _CaptureSyncBehaviour on _CaptureScreenState { } /// The composer column: metadata row, editor, and status footer. Images - /// pasted or dropped anywhere on the (desktop) page attach to the draft. + /// pasted and files dropped anywhere on the (desktop) page attach to the + /// draft. Widget _captureBody({required bool advanced}) { return ImageInputListener( onImages: (images, rejected) => unawaited(_attachImages(images, rejected: rejected)), + onFiles: (files) => unawaited(_attachFiles(files)), child: _composer(advanced: advanced), ); } diff --git a/lib/ui/capture_screen_widget.dart b/lib/ui/capture_screen_widget.dart index 688b4c5..3fd68f5 100644 --- a/lib/ui/capture_screen_widget.dart +++ b/lib/ui/capture_screen_widget.dart @@ -22,7 +22,7 @@ class CaptureScreen extends StatefulWidget { this.localBackup, this.firebaseFactory, this.stateStore, - this.imagePicking, + this.attachmentPicking, super.key, }); @@ -57,7 +57,7 @@ class CaptureScreen extends StatefulWidget { final LocalBackup? localBackup; /// The πŸ“Ž picker; null uses the platform one. Injected by tests. - final ImagePicking? imagePicking; + final AttachmentPicking? attachmentPicking; /// How long focus must stay lost before a background sync fires. Public so /// tests can pump exactly past it. diff --git a/lib/ui/cloud_images_settings.dart b/lib/ui/cloud_images_settings.dart index f9a1007..f76bfc7 100644 --- a/lib/ui/cloud_images_settings.dart +++ b/lib/ui/cloud_images_settings.dart @@ -34,7 +34,7 @@ class CloudImagesTile extends StatelessWidget { Widget build(BuildContext context) => ListTile( contentPadding: EdgeInsets.zero, title: const Text('Cloud images'), - subtitle: const Text('Where attached images upload (dufs)'), + subtitle: const Text('Where attached images and files upload (dufs)'), trailing: const Icon(Icons.chevron_right), onTap: () => Navigator.of(context).push( MaterialPageRoute( @@ -166,7 +166,8 @@ class _CloudImagesScreenState extends State { const SizedBox(height: 16), Text( 'Images are shrunk to 2560px JPEGs without location data before ' - 'they leave this device.', + 'they leave this device. Other files upload unchanged, at any ' + 'size.', style: Theme.of(context).textTheme.bodySmall, ), ], diff --git a/lib/ui/image_attach.dart b/lib/ui/image_attach.dart index b495981..dcfdf18 100644 --- a/lib/ui/image_attach.dart +++ b/lib/ui/image_attach.dart @@ -1,8 +1,10 @@ -/// Turning picked, pasted or dropped image bytes into links in the open note. +/// Turning pasted image bytes and picked or dropped files into links in the +/// open note. /// /// Shared by the capture and detail screens: each owns one /// [ImageAttachController], hands it to its [NoteEditor] (which binds the -/// insert-at-cursor), and routes every image source through [attachImages]. +/// insert-at-cursor), and routes pastes through [attachImages] and picks and +/// drops through [attachFiles]. library; import 'dart:typed_data'; @@ -43,7 +45,7 @@ class ImageAttachController { /// once there is something to save; asking for it is what reserves one. /// Reserving writes nothing, so a rejected paste still never creates an /// empty note. [rejected] counts inputs already known -/// not to be images (e.g. a dropped PDF), so one message covers both. +/// not to be images (e.g. a pasted PDF), so one message covers both. Future attachImages( BuildContext context, { required String Function() noteId, @@ -68,7 +70,7 @@ Future attachImages( break; } } - target.insert([for (final ref in refs) ref.markdown]); + target.insert([for (final ref in refs) ref.imageMarkdown]); if (!context.mounted) return; if (failure != null) { showError(context, failure); @@ -79,3 +81,28 @@ Future attachImages( ); } } + +/// Stores each of [files] (picked or dropped, any type and size) for the note +/// [noteId] names and inserts their links, in order, through [target]: +/// images inline, anything else as a file link. Stops at the first file that +/// cannot be stored, keeping the ones before it. +Future attachFiles( + BuildContext context, { + required String Function() noteId, + required ImageAttachController target, + required List files, +}) async { + final store = ImageStoreScope.read(context); + final attached = []; + String? failure; + for (final file in files) { + try { + attached.add(await store.attachFile(noteId(), file)); + } on ImageStoreException catch (e) { + failure = e.message; + break; + } + } + target.insert([for (final attachment in attached) attachment.markdown]); + if (failure != null && context.mounted) showError(context, failure); +} diff --git a/lib/ui/image_input_io.dart b/lib/ui/image_input_io.dart index f5bb2d2..5b41870 100644 --- a/lib/ui/image_input_io.dart +++ b/lib/ui/image_input_io.dart @@ -1,17 +1,22 @@ import 'dart:typed_data'; +import 'package:file_selector/file_selector.dart' show XFile; import 'package:flutter/widgets.dart'; /// Receives pasted or dropped images and the count of non-image files. typedef ImageInputCallback = void Function(List images, int rejected); +/// Receives dropped files of any type. +typedef FileInputCallback = void Function(List files); + /// Off the web there is no document to paste or drop onto (Android attaches /// through the πŸ“Ž button), so this is just [child]. class ImageInputListener extends StatelessWidget { /// Wraps [child]; [onImages] is never called on this platform. const ImageInputListener({ required this.onImages, + required this.onFiles, required this.child, super.key, }); @@ -19,6 +24,9 @@ class ImageInputListener extends StatelessWidget { /// Unused off the web. final ImageInputCallback onImages; + /// Unused off the web. + final FileInputCallback onFiles; + /// The screen body. final Widget child; diff --git a/lib/ui/image_input_web.dart b/lib/ui/image_input_web.dart index 5a7ada4..7d314aa 100644 --- a/lib/ui/image_input_web.dart +++ b/lib/ui/image_input_web.dart @@ -1,16 +1,21 @@ -/// Ctrl+V and drag-and-drop of images onto the desktop (web) page. +/// Ctrl+V of images and drag-and-drop of any file onto the desktop (web) +/// page. /// /// Flutter web has no drop target of its own, so this listens on the DOM /// `document` directly (`package:web`, no native plugin): `dragover` must be /// cancelled for the page to accept a drop at all, and `drop` must be, or /// Chrome navigates away to the dropped file. A paste is only taken over when /// the clipboard carries files; plain-text pastes stay the text field's. +/// +/// A dropped file is handed on as an [XFile] over an object URL, never read +/// here, so its size does not matter (see `file_post_web.dart`). library; import 'dart:async'; import 'dart:js_interop'; import 'dart:typed_data'; +import 'package:file_selector/file_selector.dart' show XFile; import 'package:flutter/widgets.dart'; import 'package:todo/ui/image_input_files.dart'; import 'package:web/web.dart' as web; @@ -19,19 +24,26 @@ import 'package:web/web.dart' as web; typedef ImageInputCallback = void Function(List images, int rejected); -/// Calls [onImages] for images pasted or dropped anywhere on the page while -/// this widget's route is the visible one. +/// Receives dropped files of any type. +typedef FileInputCallback = void Function(List files); + +/// Calls [onImages] for images pasted and [onFiles] for files dropped +/// anywhere on the page while this widget's route is the visible one. class ImageInputListener extends StatefulWidget { /// Wraps [child], listening while mounted. const ImageInputListener({ required this.onImages, + required this.onFiles, required this.child, super.key, }); - /// Receives the image bytes in order, plus the non-image count. + /// Receives pasted image bytes in order, plus the non-image count. final ImageInputCallback onImages; + /// Receives dropped files in order. + final FileInputCallback onFiles; + /// The screen body. final Widget child; @@ -78,9 +90,18 @@ class _ImageInputListenerState extends State { final files = event.dataTransfer?.files; if (!_active || files == null || files.length == 0) return; event.preventDefault(); - unawaited(_take(files)); + widget.onFiles([ + for (var i = 0; i < files.length; i++) _xFile(files.item(i)!), + ]); } + XFile _xFile(web.File file) => XFile( + web.URL.createObjectURL(file), + name: file.name, + length: file.size, + mimeType: file.type, + ); + void _paste(web.ClipboardEvent event) { final files = event.clipboardData?.files; if (!_active || files == null || files.length == 0) return; diff --git a/lib/ui/image_picking.dart b/lib/ui/image_picking.dart deleted file mode 100644 index ed05680..0000000 --- a/lib/ui/image_picking.dart +++ /dev/null @@ -1,104 +0,0 @@ -/// The πŸ“Ž button: pick images from the camera or gallery (Android) or from -/// disk (desktop), and hand their bytes on. -library; - -import 'package:file_selector/file_selector.dart'; -import 'package:flutter/foundation.dart'; -import 'package:flutter/material.dart'; -import 'package:image_picker/image_picker.dart'; - -/// Where the phone takes an image from. -enum ImagePickSource { - /// A new photo. - camera, - - /// Existing photos, several at once. - gallery, -} - -/// Picks images and returns their bytes; empty when the user cancels. -/// -/// Injectable so widget tests never reach a platform picker. -typedef ImagePicking = Future> Function(BuildContext context); - -/// The platform picker: a Camera/Gallery sheet on Android, a multi-select -/// file dialog filtered to images on the desktop (web) build. -/// -/// [fromPhone] is the platform call behind the sheet, injectable for tests. -Future> pickImages( - BuildContext context, { - Future> Function(ImagePickSource) fromPhone = _pickFromPhone, -}) async { - // The desktop build has no sheet: straight to the file dialog. - if (kIsWeb) return await _pickFiles(); // coverage:ignore-line - final source = await showModalBottomSheet( - context: context, - builder: (context) => SafeArea( - child: Column( - mainAxisSize: MainAxisSize.min, - children: [ - ListTile( - leading: const Icon(Icons.photo_camera_outlined), - title: const Text('Camera'), - onTap: () => Navigator.pop(context, ImagePickSource.camera), - ), - ListTile( - leading: const Icon(Icons.photo_library_outlined), - title: const Text('Gallery'), - onTap: () => Navigator.pop(context, ImagePickSource.gallery), - ), - ], - ), - ), - ); - if (source == null) return const []; - return await fromPhone(source); -} - -// coverage:ignore-start -// Thin calls into platform pickers with no test binding; the sheet above and -// everything downstream of the returned bytes are covered. -Future> _pickFromPhone(ImagePickSource source) async { - final picker = ImagePicker(); - final files = switch (source) { - ImagePickSource.camera => [ - ?await picker.pickImage(source: ImageSource.camera), - ], - ImagePickSource.gallery => await picker.pickMultiImage(), - }; - return [for (final file in files) await file.readAsBytes()]; -} - -Future> _pickFiles() async { - final files = await openFiles( - acceptedTypeGroups: const [ - XTypeGroup(label: 'Images', mimeTypes: ['image/*']), - ], - ); - return [for (final file in files) await file.readAsBytes()]; -} -// coverage:ignore-end - -/// App-bar action that picks images and passes them to [onImages]. -class AttachImageButton extends StatelessWidget { - /// Creates the button; [picking] defaults to [pickImages]. - const AttachImageButton({required this.onImages, this.picking, super.key}); - - /// Receives the picked images (never called for a cancelled pick). - final ValueChanged> onImages; - - /// The picker, injectable for tests. - final ImagePicking? picking; - - @override - Widget build(BuildContext context) { - return IconButton( - tooltip: 'Attach image', - icon: const Icon(Icons.attach_file), - onPressed: () async { - final images = await (picking ?? pickImages)(context); - if (images.isNotEmpty) onImages(images); - }, - ); - } -} diff --git a/lib/ui/images/image_strip.dart b/lib/ui/images/attachment_strip.dart similarity index 52% rename from lib/ui/images/image_strip.dart rename to lib/ui/images/attachment_strip.dart index 0d8b003..7617ff7 100644 --- a/lib/ui/images/image_strip.dart +++ b/lib/ui/images/attachment_strip.dart @@ -1,27 +1,29 @@ import 'package:design_system/design_system.dart'; import 'package:flutter/material.dart'; -import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/attachment_links.dart'; +import 'package:todo/ui/images/file_embed.dart'; import 'package:todo/ui/images/image_embed.dart'; /// Height of the thumbnail row under the editor. -const kImageStripHeight = 88.0; +const kAttachmentStripHeight = 88.0; -/// A row of thumbnails for every image [text] links to, shown under the -/// editing field so a note's pictures are visible while writing it. The +/// A row of tiles for every attachment [text] links to β€” thumbnails for +/// images, icon-and-name tiles for files β€” shown under the editing field so a +/// note's attachments are visible (and files openable) while writing it. The /// editing modes show the links as text; without this the default (Raw) -/// mode would never show an image at all. Empty when there are none. -class ImageStrip extends StatelessWidget { +/// mode would never show one at all. Empty when there are none. +class AttachmentStrip extends StatelessWidget { /// Creates the strip for [text]. - const ImageStrip({required this.text, super.key}); + const AttachmentStrip({required this.text, super.key}); /// The note text being edited. final String text; @override Widget build(BuildContext context) { - final refs = imageRefsIn(text).toList(); - if (refs.isEmpty) return const SizedBox.shrink(); + final attachments = attachmentsIn(text); + if (attachments.isEmpty) return const SizedBox.shrink(); // Same centred column as the editor above it (rule 21), so on the wide // desktop window the thumbnails sit under the text, not at the far edge. return Padding( @@ -30,15 +32,17 @@ class ImageStrip extends StatelessWidget { child: ConstrainedBox( constraints: const BoxConstraints( maxWidth: kProseMaxWidth, - maxHeight: kImageStripHeight, + maxHeight: kAttachmentStripHeight, ), child: ListView.separated( scrollDirection: Axis.horizontal, - itemCount: refs.length, + itemCount: attachments.length, separatorBuilder: (_, _) => const SizedBox(width: 8), itemBuilder: (context, i) => SizedBox( - width: kImageStripHeight, - child: ImageEmbed(ref: refs[i]), + width: kAttachmentStripHeight, + child: attachments[i].isImage + ? ImageEmbed(ref: attachments[i].ref) + : FileEmbed(attachment: attachments[i], compact: true), ), ), ), diff --git a/lib/ui/images/file_embed.dart b/lib/ui/images/file_embed.dart new file mode 100644 index 0000000..339d8ba --- /dev/null +++ b/lib/ui/images/file_embed.dart @@ -0,0 +1,179 @@ +/// One attached non-image file as it appears in a note: an icon for its +/// type, its name and size. Tapping opens it (see `file_opening.dart`). +library; + +import 'package:design_system/design_system.dart'; +import 'package:flutter/material.dart'; + +import 'package:todo/attachments/image_store_api.dart'; +import 'package:todo/attachments/image_store_scope.dart'; +import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/ui/images/file_opening.dart'; +import 'package:todo/ui/images/image_embed.dart'; + +/// Opens an attached file; returns why it could not, or null. +typedef FileOpening = Future Function(ImageSource source, String name); + +/// Renders the file [attachment] from the nearest [ImageStoreScope]. +class FileEmbed extends StatefulWidget { + /// Creates an embed: a full-width row inline, or a square tile when + /// [compact] (the strip under the editor). [opening] is injectable. + const FileEmbed({ + required this.attachment, + this.compact = false, + this.opening = openAttachedFile, + super.key, + }); + + /// The file to show. + final Attachment attachment; + + /// Square tile instead of a row. + final bool compact; + + /// What a tap does. + final FileOpening opening; + + @override + State createState() => _FileEmbedState(); +} + +class _FileEmbedState extends State { + late Future _size; + + // Re-asked whenever the store notifies (a download may have landed), not + // on every rebuild: on desktop each ask is a request to the wrapper. + @override + void didChangeDependencies() { + super.didChangeDependencies(); + _size = ImageStoreScope.of(context).sizeOf(widget.attachment.ref); + } + + Future _open(ImageSource source) async { + final failure = await widget.opening(source, widget.attachment.name); + if (failure != null && mounted) showError(context, failure); + } + + @override + Widget build(BuildContext context) { + final store = ImageStoreScope.of(context); + final attachment = widget.attachment; + final theme = Theme.of(context); + return FutureBuilder( + future: _size, + builder: (context, snapshot) { + final size = snapshot.data; + final detail = size == null ? kImageNotDownloaded : formatBytes(size); + final icon = Icon(fileIconFor(attachment.ref.extension)); + final name = Text( + attachment.name, + maxLines: widget.compact ? 2 : 1, + overflow: TextOverflow.ellipsis, + textAlign: widget.compact ? TextAlign.center : TextAlign.start, + style: widget.compact ? theme.textTheme.bodySmall : null, + ); + final pending = store.isPending(attachment.ref) + ? const Tooltip( + message: kImageWaitingToUpload, + child: Icon(Icons.cloud_upload_outlined, size: 18), + ) + : null; + final content = widget.compact + ? Stack( + children: [ + Center( + child: Column( + mainAxisSize: MainAxisSize.min, + children: [ + icon, + const SizedBox(height: AppSpacing.xs), + name, + ], + ), + ), + if (pending != null) + Positioned(right: 0, top: 0, child: pending), + ], + ) + : Row( + children: [ + icon, + const SizedBox(width: AppSpacing.sm), + Expanded( + child: Column( + crossAxisAlignment: CrossAxisAlignment.start, + children: [ + name, + Text(detail, style: theme.textTheme.bodySmall), + ], + ), + ), + ?pending, + ], + ); + return Semantics( + label: 'Attached file ${attachment.name}, $detail', + button: size != null, + child: Material( + color: theme.colorScheme.surfaceContainerHighest, + borderRadius: BorderRadius.circular(AppRadius.sm), + child: InkWell( + borderRadius: BorderRadius.circular(AppRadius.sm), + onTap: size == null + ? null + : () => _open(store.sourceFor(attachment.ref)), + child: Padding( + padding: const EdgeInsets.all(AppSpacing.sm), + child: content, + ), + ), + ), + ); + }, + ); + } +} + +/// [bytes] as a short human size: `512 B`, `1.2 KB`, `34 MB`, `2.1 GB`. +String formatBytes(int bytes) { + const units = ['B', 'KB', 'MB', 'GB', 'TB']; + var value = bytes.toDouble(); + var unit = 0; + while (value >= 1024 && unit < units.length - 1) { + value /= 1024; + unit++; + } + final digits = unit == 0 || value >= 10 ? 0 : 1; + return '${value.toStringAsFixed(digits)} ${units[unit]}'; +} + +/// The icon for a file with [extension]. +IconData fileIconFor(String extension) => switch (extension) { + 'pdf' => Icons.picture_as_pdf_outlined, + 'mp4' || + 'm4v' || + 'webm' || + 'mov' || + 'mkv' || + 'avi' || + 'ogv' => Icons.movie_outlined, + 'mp3' || + 'm4a' || + 'ogg' || + 'oga' || + 'opus' || + 'wav' || + 'flac' || + 'aac' => Icons.audiotrack_outlined, + 'txt' || 'md' || 'log' || 'csv' || 'json' => Icons.description_outlined, + 'zip' || 'tar' || 'gz' || '7z' || 'rar' || 'xz' => Icons.folder_zip_outlined, + 'png' || + 'gif' || + 'webp' || + 'bmp' || + 'svg' || + 'psd' || + 'heic' || + 'tif' => Icons.image_outlined, + _ => Icons.insert_drive_file_outlined, +}; diff --git a/lib/ui/images/file_opening.dart b/lib/ui/images/file_opening.dart new file mode 100644 index 0000000..556f3db --- /dev/null +++ b/lib/ui/images/file_opening.dart @@ -0,0 +1,34 @@ +/// Opening an attached file outside the app. +/// +/// Android hands the cached file to whichever app handles its type (the +/// system chooser when there is no default), through `open_file`'s +/// FileProvider. The desktop page opens the wrapper's copy in a new browser +/// tab, which shows PDFs, text and media and downloads anything else (see +/// `wrapper_attachment_serving.dart`). +library; + +import 'package:open_file/open_file.dart'; +import 'package:todo/attachments/image_store_api.dart'; +import 'package:url_launcher/url_launcher.dart'; + +/// Opens [source], the file called [name]; returns why it could not, or null. +Future openAttachedFile(ImageSource source, String name) async { + switch (source) { + case LocalImageSource(:final path): + final result = await OpenFile.open(path); + return switch (result.type) { + ResultType.done => null, + ResultType.noAppToOpen => 'No app on this phone opens $name', + _ => 'Could not open $name: ${result.message}', + }; + case NetworkImageSource(:final url): + final uri = Uri.parse(url); + final named = uri.replace( + queryParameters: {...uri.queryParameters, 'name': name}, + ); + final opened = await launchUrl(named, webOnlyWindowName: '_blank'); + return opened ? null : 'Could not open $name'; + case MissingImageSource(): + return '$name is not on this device yet'; + } +} diff --git a/lib/ui/markdown_view.dart b/lib/ui/markdown_view.dart index f85a542..ed4eab4 100644 --- a/lib/ui/markdown_view.dart +++ b/lib/ui/markdown_view.dart @@ -1,7 +1,8 @@ import 'package:design_system/design_system.dart'; import 'package:flutter/material.dart'; -import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/attachment_links.dart'; +import 'package:todo/ui/images/file_embed.dart'; import 'package:todo/ui/images/image_embed.dart'; /// Tallest an image is drawn inside a rendered note. @@ -11,7 +12,7 @@ const kInlineImageHeight = 320.0; /// /// The app's notes use a small, known Markdown subset β€” an `#` title, `##` /// section headings, italic `_guidance_` lines, `-` bullet lists and plain -/// paragraphs, plus one attached image per line (see `attachment_ref.dart`) +/// paragraphs, plus one attachment per line (see `attachment_links.dart`) /// β€” so a tailored line-based renderer covers everything we emit without /// pulling in a full Markdown engine. It keeps the quick-capture app lean and /// is trivial to test. @@ -37,14 +38,18 @@ class MarkdownView extends StatelessWidget { widgets.add(const SizedBox(height: 8)); continue; } - if (imageRefOfLine(trimmed) case final image?) { + if (attachmentOfLine(trimmed) case final attachment?) { widgets.add( Padding( padding: const EdgeInsets.symmetric(vertical: 4), - child: ConstrainedBox( - constraints: const BoxConstraints(maxHeight: kInlineImageHeight), - child: ImageEmbed(ref: image, fit: BoxFit.contain), - ), + child: attachment.isImage + ? ConstrainedBox( + constraints: const BoxConstraints( + maxHeight: kInlineImageHeight, + ), + child: ImageEmbed(ref: attachment.ref, fit: BoxFit.contain), + ) + : FileEmbed(attachment: attachment), ), ); continue; diff --git a/lib/ui/note_detail_screen.dart b/lib/ui/note_detail_screen.dart index c5de417..ad18a54 100644 --- a/lib/ui/note_detail_screen.dart +++ b/lib/ui/note_detail_screen.dart @@ -7,9 +7,9 @@ import 'package:todo/data/app_settings.dart'; import 'package:todo/data/note.dart'; import 'package:todo/data/note_repository.dart'; import 'package:todo/data/note_template.dart'; +import 'package:todo/ui/attachment_picking.dart'; import 'package:todo/ui/image_attach.dart'; import 'package:todo/ui/image_input.dart'; -import 'package:todo/ui/image_picking.dart'; import 'package:todo/ui/note_form.dart'; /// Full-screen view of a single note: read it in full, edit its body through @@ -24,7 +24,7 @@ class NoteDetailScreen extends StatefulWidget { required this.note, required this.repository, required this.appSettings, - this.imagePicking, + this.attachmentPicking, super.key, }); @@ -40,7 +40,7 @@ class NoteDetailScreen extends StatefulWidget { final ValueNotifier appSettings; /// The πŸ“Ž picker; null uses the platform one. Injected by tests. - final ImagePicking? imagePicking; + final AttachmentPicking? attachmentPicking; @override State createState() => _NoteDetailScreenState(); @@ -61,6 +61,13 @@ class _NoteDetailScreenState extends State { rejected: rejected, ); + Future _attachFiles(List files) => attachFiles( + context, + noteId: () => _note.id, + target: _attach, + files: files, + ); + Future _persist(Note next) async { setState(() => _note = next); await widget.repository.upsert(next); @@ -87,9 +94,9 @@ class _NoteDetailScreenState extends State { appBar: AppBar( title: Text(title.isEmpty ? '(empty)' : title), actions: [ - AttachImageButton( - onImages: _attachImages, - picking: widget.imagePicking, + AttachButton( + onFiles: _attachFiles, + picking: widget.attachmentPicking, ), IconButton( tooltip: 'Delete note', @@ -101,6 +108,7 @@ class _NoteDetailScreenState extends State { body: ImageInputListener( onImages: (images, rejected) => unawaited(_attachImages(images, rejected: rejected)), + onFiles: (files) => unawaited(_attachFiles(files)), child: Padding( padding: const EdgeInsets.all(16), // The same form capture renders, gated on the same setting, so an diff --git a/lib/ui/note_form.dart b/lib/ui/note_form.dart index 65b27d2..ba71a71 100644 --- a/lib/ui/note_form.dart +++ b/lib/ui/note_form.dart @@ -13,7 +13,7 @@ import 'package:flutter/material.dart'; import 'package:todo/data/note.dart'; import 'package:todo/data/note_template.dart'; import 'package:todo/ui/image_attach.dart'; -import 'package:todo/ui/images/image_strip.dart'; +import 'package:todo/ui/images/attachment_strip.dart'; import 'package:todo/ui/note_editor.dart'; import 'package:todo/ui/note_form_chrome_gate.dart'; import 'package:todo/ui/note_meta_row.dart'; @@ -129,7 +129,7 @@ class _NoteFormState extends State { if (!_previewing) ValueListenableBuilder( valueListenable: _text, - builder: (context, text, _) => ImageStrip(text: text), + builder: (context, text, _) => AttachmentStrip(text: text), ), ], ); diff --git a/pubspec.lock b/pubspec.lock index 3cf487f..85547c2 100644 --- a/pubspec.lock +++ b/pubspec.lock @@ -1,6 +1,14 @@ # Generated by pub # See https://dart.dev/tools/pub/glossary#lockfile packages: + android_file_picker: + dependency: transitive + description: + name: android_file_picker + sha256: "14ab27769b54c48d5a8a71aa9858372b7a3ae77572ea0a8aee744643c5d8c53d" + url: "https://pub.dev" + source: hosted + version: "2.0.0" archive: dependency: transitive description: @@ -126,10 +134,18 @@ packages: dependency: "direct main" description: name: cupertino_icons - sha256: "41e005c33bd814be4d3096aff55b1908d419fde52ca656c8c47719ec745873cd" + sha256: b7cc2390aac296c083bea762859413eb1cf5b630c70e0c7e4d831018f762a8b2 url: "https://pub.dev" source: hosted - version: "1.0.9" + version: "2.0.0" + dbus: + dependency: transitive + description: + name: dbus + sha256: f7db8504e715b835f5ebe5fe74604ccd961d504c73389a5d8e81bafc2e2f3612 + url: "https://pub.dev" + source: hosted + version: "0.8.0" design_system: dependency: "direct main" description: @@ -179,6 +195,46 @@ packages: url: "https://pub.dev" source: hosted version: "7.0.1" + file_picker: + dependency: "direct main" + description: + name: file_picker + sha256: "98c0b156b6380ba55bc767a14e2e6b3feb246e713ad40092424596fa79005bea" + url: "https://pub.dev" + source: hosted + version: "13.1.0" + file_picker_darwin: + dependency: transitive + description: + name: file_picker_darwin + sha256: "51aef9f4c80449c736e7cc02198675fda73689f0bd45a84da642cfeab0250d46" + url: "https://pub.dev" + source: hosted + version: "2.1.2" + file_picker_linux: + dependency: transitive + description: + name: file_picker_linux + sha256: "9c870771500c758eb46506c98532c3ee60e6e51d0a62dcde24a849ef213ea5ce" + url: "https://pub.dev" + source: hosted + version: "2.0.1" + file_picker_platform_interface: + dependency: transitive + description: + name: file_picker_platform_interface + sha256: bbdc085a6f168e63f147e9ce8988f79fa0800a58e7f25a48f762c60837107ba5 + url: "https://pub.dev" + source: hosted + version: "4.0.0" + file_picker_web: + dependency: transitive + description: + name: file_picker_web + sha256: b3004268da0c1b52baa18df430e7ec4438194de855a807d26b447cbf07ef3496 + url: "https://pub.dev" + source: hosted + version: "4.0.0" file_selector: dependency: "direct main" description: @@ -603,6 +659,70 @@ packages: url: "https://pub.dev" source: hosted version: "9.4.1" + open_file: + dependency: "direct main" + description: + name: open_file + sha256: "4f9b00d42565249ed4346976bc0d57f91303bec391f6b0b217921277cf8e6e4f" + url: "https://pub.dev" + source: hosted + version: "4.0.0" + open_file_android: + dependency: transitive + description: + name: open_file_android + sha256: f48a342b0347bf82bebf68c752f5cfce162965ce8449c5ecdafc406d294aa63d + url: "https://pub.dev" + source: hosted + version: "1.1.0" + open_file_ios: + dependency: transitive + description: + name: open_file_ios + sha256: "0e3e67dcd12f69888128f533cacc092039627dd4f1d7a95a2826b814ae4ea8aa" + url: "https://pub.dev" + source: hosted + version: "1.1.0" + open_file_linux: + dependency: transitive + description: + name: open_file_linux + sha256: efd9c1f9679745cf35a54d21d809a1fe5a1196d4a1353fe2d205ba298ae86dc9 + url: "https://pub.dev" + source: hosted + version: "1.1.0" + open_file_mac: + dependency: transitive + description: + name: open_file_mac + sha256: "1abe00e65f792b9baa1e15fdf4d44af5e813b1696ac307d71535543122a01e30" + url: "https://pub.dev" + source: hosted + version: "1.1.0" + open_file_platform_interface: + dependency: transitive + description: + name: open_file_platform_interface + sha256: dc302e4bd5c1d77acec4240bd0dbc1d9f2ee6882b1ce52bca5298369da7acf12 + url: "https://pub.dev" + source: hosted + version: "1.1.0" + open_file_web: + dependency: transitive + description: + name: open_file_web + sha256: "4945221c9ee98a5a6cdf26730fe0fabb3a36c1e423148bff4c32c283079ee532" + url: "https://pub.dev" + source: hosted + version: "1.1.0" + open_file_windows: + dependency: transitive + description: + name: open_file_windows + sha256: "6a97d8f2a5966fb0a7f613a901f1d502f93456f64e8a7519d3204184413aab79" + url: "https://pub.dev" + source: hosted + version: "1.1.0" package_config: dependency: transitive description: @@ -667,6 +787,14 @@ packages: url: "https://pub.dev" source: hosted version: "2.3.0" + petitparser: + dependency: transitive + description: + name: petitparser + sha256: "2f1564aadb14215e478563b5406cb80729d022158c01dc50c96013c2f7eb4790" + url: "https://pub.dev" + source: hosted + version: "7.1.0" platform: dependency: transitive description: @@ -1049,6 +1177,14 @@ packages: url: "https://pub.dev" source: hosted version: "6.3.0" + windows_file_picker: + dependency: transitive + description: + name: windows_file_picker + sha256: "9f3aa833068b09e380fdc59560ad260a4a14e9397173abb533699f967443602e" + url: "https://pub.dev" + source: hosted + version: "2.0.0" xdg_directories: dependency: transitive description: @@ -1057,6 +1193,14 @@ packages: url: "https://pub.dev" source: hosted version: "1.1.0" + xml: + dependency: transitive + description: + name: xml + sha256: "0fe8cc98946def31cbcbc8734aa4067031e6f65ea3a78a508060bfed2200bfeb" + url: "https://pub.dev" + source: hosted + version: "7.1.0" yaml: dependency: transitive description: diff --git a/pubspec.yaml b/pubspec.yaml index 93b5c2a..7e4c549 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -16,7 +16,7 @@ publish_to: 'none' # Remove this line if you wish to publish to pub.dev # https://developer.apple.com/library/archive/documentation/General/Reference/InfoPlistKeyReference/Articles/CoreFoundationKeys.html # In Windows, build-name is used as the major, minor, and patch parts # of the product and file versions while build-number is used as the build suffix. -version: 1.0.40+1 +version: 1.0.41+1 environment: sdk: ^3.12.2 @@ -38,7 +38,7 @@ dependencies: # The following adds the Cupertino Icons font to your application. # Use with the CupertinoIcons class for iOS style icons. - cupertino_icons: 1.0.9 + cupertino_icons: 2.0.0 # The shared palette, scales and components. Owns the tokens: there is # deliberately no local theme.dart to drift from the frozen table. @@ -47,6 +47,7 @@ dependencies: url: https://github.com/kuhyx/utils ref: design_system-v0.2.1 path: design_system + file_picker: 13.1.0 file_selector: 1.1.0 flutter: sdk: flutter @@ -71,6 +72,9 @@ dependencies: image: 4.10.1 # Camera + gallery picks for image attachments on Android. image_picker: 1.2.3 + # Hands an attached non-image file to whichever Android app opens its + # type (FileProvider intent). Adds no permissions to the manifest. + open_file: 4.0.0 path: 1.9.1 path_provider: 2.1.6 share_plus: 13.3.0 diff --git a/test/attachments/image_store_api_test.dart b/test/attachments/image_store_api_test.dart index fd4482a..3b613ce 100644 --- a/test/attachments/image_store_api_test.dart +++ b/test/attachments/image_store_api_test.dart @@ -15,9 +15,14 @@ void main() { store.attach('n1', Uint8List(1)), throwsA(isA()), ); + await expectLater( + store.attachFile('n1', XFile('/nonexistent')), + throwsA(isA()), + ); await store.reconcile(['x']); expect(store.sourceFor(ref), isA()); expect(store.isPending(ref), isFalse); + expect(await store.sizeOf(ref), isNull); final status = await store.status(); expect((status.configured, status.pending), (false, 0)); }); @@ -26,11 +31,16 @@ void main() { expect(const ImageStoreException('boom').toString(), 'boom'); }); - test('imageRefsAcross unions every note', () { + test('attachmentRefsAcross unions every note, images and files', () { final other = AttachmentRef.create('n2'); + final pdf = AttachmentRef.forFile('n3', 'a.pdf'); expect( - imageRefsAcross([ref.markdown, '${other.markdown}\n${ref.markdown}']), - {ref, other}, + attachmentRefsAcross([ + ref.imageMarkdown, + '${other.imageMarkdown}\n${ref.imageMarkdown}', + pdf.fileMarkdown('a.pdf'), + ]), + {ref, other, pdf}, ); }); diff --git a/test/attachments/image_store_io_test.dart b/test/attachments/image_store_io_test.dart index c4dba21..72ee4a8 100644 --- a/test/attachments/image_store_io_test.dart +++ b/test/attachments/image_store_io_test.dart @@ -5,6 +5,7 @@ import 'package:flutter_test/flutter_test.dart'; import 'package:todo/attachments/image_password_store.dart'; import 'package:todo/attachments/image_store_api.dart'; import 'package:todo/attachments/image_store_io.dart'; +import 'package:path/path.dart' as p; import 'package:todo/images/image_cache_store.dart'; import '../fake_secure_storage.dart'; @@ -20,6 +21,7 @@ void main() { IoImageStore make({Future Function(Uint8List)? prepare}) { final s = IoImageStore( ImageCacheStore(root), + pickerCopies: p.join(root.parent.path, '${p.basename(root.path)}-picks'), httpClient: dufs.httpClient, prepare: prepare ?? (b) async => b.first == 0xFF ? b : null, ); @@ -88,8 +90,77 @@ void main() { expect(notified, 0); }); + group('attachFile', () { + late Directory picks; + late Directory elsewhere; + + setUp(() { + picks = Directory(store.pickerCopies!)..createSync(); + elsewhere = Directory.systemTemp.createTempSync('io_store_src'); + }); + tearDown(() { + picks.deleteSync(recursive: true); + elsewhere.deleteSync(recursive: true); + }); + + XFile file(Directory dir, String name, List bytes) { + final path = p.join(dir.path, name); + File(path).writeAsBytesSync(bytes); + return XFile(path, name: name); + } + + test('stores a file, notifies and uploads it', () async { + final a = await store.attachFile('n1', file(elsewhere, 'r.pdf', [1, 2])); + expect(a.isImage, isFalse); + expect(notified, greaterThanOrEqualTo(1)); + expect(await store.sizeOf(a.ref), 2); + await pumpEventQueue(); + expect(dufs.files[a.ref.relativePath], [1, 2]); + expect(File(p.join(elsewhere.path, 'r.pdf')).existsSync(), isTrue); + }); + + test("moves a picker's own copy instead of copying it", () async { + final picked = file(picks, 'big.bin', [3]); + final a = await store.attachFile('n1', picked); + expect(File(picked.path).existsSync(), isFalse); + expect(await store.sizeOf(a.ref), 1); + }); + + test('routes an image through the preparer', () async { + final a = await store.attachFile( + 'n1', + file(elsewhere, 'p.jpg', [0xFF, 0xD8, 0xFF, 9]), + ); + expect(a.isImage, isTrue); + }); + + test('fails loudly when the file cannot be stored', () async { + await expectLater( + store.attachFile('n1', XFile(p.join(elsewhere.path, 'gone.pdf'))), + throwsA(isA()), + ); + }); + + test('without picker copies everything is copied', () async { + final plain = IoImageStore( + ImageCacheStore(root), + httpClient: dufs.httpClient, + ); + final picked = file(picks, 'kept.txt', [1]); + await plain.attachFile('n1', picked); + expect(File(picked.path).existsSync(), isTrue); + }); + }); + + test('sizeOf is null for an attachment not on this device', () async { + final ref = attachmentRefsAcross([ + '[a.pdf](https://kuhy-cloud.duckdns.org/todo-images/n9/z.pdf)', + ]).single; + expect(await store.sizeOf(ref), isNull); + }); + test('a missing image has a missing source', () { - final ref = imageRefsAcross([ + final ref = attachmentRefsAcross([ '![](https://kuhy-cloud.duckdns.org/todo-images/n9/z.jpg)', ]).single; expect(store.sourceFor(ref), isA()); diff --git a/test/attachments/image_store_web_test.dart b/test/attachments/image_store_web_test.dart index 1a96575..3486788 100644 --- a/test/attachments/image_store_web_test.dart +++ b/test/attachments/image_store_web_test.dart @@ -5,6 +5,7 @@ import 'dart:typed_data'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; +import 'package:todo/attachments/file_post_io.dart' as file_post_io; import 'package:todo/attachments/image_store_api.dart'; import 'package:todo/attachments/image_store_web.dart'; import 'package:todo/images/attachment_ref.dart'; @@ -13,14 +14,18 @@ void main() { final requests = []; late int notified; - WebImageStore make(MockClientHandler handler) { + final posted = <(Uri, XFile)>[]; + + WebImageStore make(MockClientHandler handler, {FilePost? postFile}) { requests.clear(); + posted.clear(); final store = WebImageStore( origin: 'http://localhost:8731', httpClient: MockClient((r) { requests.add(r); return handler(r); }), + postFile: postFile, ); notified = 0; store.addListener(() => notified++); @@ -86,7 +91,7 @@ void main() { test('reconcile sends every linked ref; a download bumps the url', () async { final ref = AttachmentRef.create('n1'); final store = make((_) async => report(downloaded: 1)); - await store.reconcile(['a ${ref.markdown}', 'none']); + await store.reconcile(['a ${ref.imageMarkdown}', 'none']); expect(jsonDecode(requests.single.body), { 'refs': [ref.relativePath], }); @@ -116,4 +121,94 @@ void main() { test('the default http client is created', () { expect(WebImageStore(origin: 'http://x'), isA()); }); + + group('attachFile', () { + // On the VM an XFile's name is its path's basename. + final file = XFile('/picked/Raport.pdf'); + final answer = jsonEncode( + Attachment( + ref: AttachmentRef.forFile('n1', 'Raport.pdf'), + name: 'Raport.pdf', + isImage: false, + ).toJson(), + ); + + FilePost replying(int status, String body) => (url, file) async { + posted.add((url, file)); + return (status: status, body: body); + }; + + test('posts the file itself and adopts the answer', () async { + final store = make( + (_) async => report(), + postFile: replying(200, answer), + ); + final attached = await store.attachFile('n1', file); + await pumpEventQueue(); + final (url, sent) = posted.single; + expect(url.path, '/images/attach'); + expect(url.queryParameters, {'note': 'n1', 'name': 'Raport.pdf'}); + expect(sent, same(file)); + expect(attached.name, 'Raport.pdf'); + expect(attached.isImage, isFalse); + expect(notified, 1); + expect(requests.single.url.path, '/images/reconcile'); + }); + + test('a refusal or no wrapper throws a readable error', () async { + await expectLater( + make( + (_) async => report(), + postFile: replying(500, ''), + ).attachFile('n1', file), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('refused Raport.pdf (500)'), + ), + ), + ); + await expectLater( + make( + (_) async => report(), + postFile: (_, _) => throw StateError('fetch failed'), + ).attachFile('n1', file), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('not running'), + ), + ), + ); + }); + + test('off the web the default poster refuses', () { + expect( + () => file_post_io.postFile(Uri.parse('http://x'), file), + throwsUnsupportedError, + ); + }); + }); + + test('sizeOf asks the wrapper once, then remembers', () async { + final ref = AttachmentRef.forFile('n1', 'a.pdf'); + final store = make( + (r) async => http.Response('', 200, headers: {'content-length': '42'}), + ); + expect(await store.sizeOf(ref), 42); + expect(await store.sizeOf(ref), 42); + expect(requests.single.method, 'HEAD'); + expect(requests.single.url.path, '/images/${ref.relativePath}'); + }); + + test('sizeOf is null until the wrapper has the file', () async { + final ref = AttachmentRef.forFile('n1', 'a.pdf'); + expect(await make((_) async => http.Response('', 404)).sizeOf(ref), isNull); + expect( + await make((_) => throw const SocketException('x')).sizeOf(ref), + isNull, + ); + }); } diff --git a/test/capture_screen_harness.dart b/test/capture_screen_harness.dart index 1646fb2..74faf71 100644 --- a/test/capture_screen_harness.dart +++ b/test/capture_screen_harness.dart @@ -24,9 +24,9 @@ import 'package:todo/data/note.dart'; import 'package:todo/data/note_template.dart'; import 'package:todo/sync/local_backup.dart'; import 'package:todo/sync/notes_markdown.dart'; +import 'package:todo/ui/attachment_picking.dart'; import 'package:todo/ui/capture_screen.dart'; import 'package:todo/ui/github_mirror_screen.dart'; -import 'package:todo/ui/image_picking.dart'; import 'package:todo/ui/settings_screen.dart'; import 'fake_note_repository.dart'; @@ -50,7 +50,7 @@ Future pumpCapture( ValueNotifier? appSettings, AnalyticsService? analytics, ImageStore? imageStore, - ImagePicking? imagePicking, + AttachmentPicking? attachmentPicking, }) async { SharedPreferences.setMockInitialValues(prefs); installFakeSecureStorage(); @@ -85,7 +85,7 @@ Future pumpCapture( firebaseFactory: firebaseFactory ?? () async => null, stateStore: InMemorySyncStateStore(), localBackup: backup, - imagePicking: imagePicking, + attachmentPicking: attachmentPicking, ), ); await tester.pumpWidget( diff --git a/test/desktop/wrapper_attach_route_test.dart b/test/desktop/wrapper_attach_route_test.dart new file mode 100644 index 0000000..4f5b533 --- /dev/null +++ b/test/desktop/wrapper_attach_route_test.dart @@ -0,0 +1,215 @@ +import 'dart:async'; +import 'dart:convert'; +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:http/http.dart' as http; +import 'package:path/path.dart' as p; +import 'package:todo/desktop/wrapper_image_routes.dart'; +import 'package:todo/desktop/wrapper_server.dart'; +import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/image_cache_store.dart'; + +void main() { + late Directory root; + late WrapperServer server; + late String base; + late ImageCacheStore store; + + setUp(() async { + root = Directory.systemTemp.createTempSync('wrapper_attach'); + File(p.join(root.path, 'index.html')).writeAsStringSync('

todo

'); + store = ImageCacheStore(Directory(p.join(root.path, 'images'))); + server = WrapperServer( + webRoot: root.path, + backlogPath: p.join(root.path, 'BACKLOG.md'), + logPath: p.join(root.path, 'log.json'), + todoCredentialsPath: p.join(root.path, 'none.json'), + images: WrapperImageRoutes( + store: store, + loginEnvPath: p.join(root.path, 'none.env'), + // Identity "re-encode": decodes anything starting like a JPEG. + prepare: (bytes) async => bytes.first == 0xFF ? bytes : null, + ), + ); + await server.start(0); + base = 'http://localhost:${server.port}'; + }); + tearDown(() async { + await server.stop(); + root.deleteSync(recursive: true); + }); + + Future attach( + List body, { + String note = 'n1', + String name = 'Raport.pdf', + Map headers = const {}, + }) => http.post( + Uri.parse('$base/images/attach?note=$note&name=$name'), + body: body, + headers: headers, + ); + + Attachment attached(http.Response response) => + Attachment.fromJson(jsonDecode(response.body)); + + test('attach stores any file byte for byte and answers with it', () async { + final response = await attach('%PDF-1.7 x'.codeUnits); + expect(response.statusCode, 200); + final a = attached(response); + expect((a.name, a.isImage, a.ref.extension), ('Raport.pdf', false, 'pdf')); + expect(store.fileFor(a.ref).readAsStringSync(), '%PDF-1.7 x'); + expect(store.isPending(a.ref), isTrue); + }); + + test('attach turns an image into a re-encoded jpg', () async { + final a = attached(await attach([0xFF, 0xD8, 0xFF, 1], name: 'p.png')); + expect(a.isImage, isTrue); + expect(a.ref.extension, 'jpg'); + }); + + test('attach refuses an unsafe note and reports a disk failure', () async { + expect((await attach([1], note: '..')).statusCode, 400); + expect((await attach([1], note: '')).statusCode, 400); + File(store.root.path).writeAsStringSync('in the way'); + expect((await attach([1])).statusCode, 500); + }); + + group('serving', () { + late AttachmentRef pdf; + + setUp(() async { + pdf = attached(await attach(List.filled(100, 7))).ref; + }); + + test('GET names the file and keeps the browser from sniffing', () async { + final response = await http.get( + Uri.parse('$base/images/${pdf.relativePath}?name=Raport%20v2.pdf'), + ); + expect(response.statusCode, 200); + expect(response.headers['content-type'], 'application/pdf'); + expect(response.headers['x-content-type-options'], 'nosniff'); + expect(response.headers['accept-ranges'], 'bytes'); + expect( + response.headers['content-disposition'], + "inline; filename*=UTF-8''Raport%20v2.pdf", + ); + expect(response.bodyBytes, List.filled(100, 7)); + }); + + test('a scriptable type is only ever a download', () async { + final html = attached(await attach([1], name: 'evil.html')).ref; + final response = await http.get( + Uri.parse('$base/images/${html.relativePath}'), + ); + expect(response.headers['content-type'], 'application/octet-stream'); + expect( + response.headers['content-disposition'], + startsWith('attachment;'), + ); + }); + + test('HEAD reports the size without a body', () async { + final response = await http.head( + Uri.parse('$base/images/${pdf.relativePath}'), + ); + expect(response.statusCode, 200); + expect(response.headers['content-length'], '100'); + expect(response.bodyBytes, isEmpty); + }); + + test('a range is served partially, a bad one refused', () async { + final url = Uri.parse('$base/images/${pdf.relativePath}'); + final part = await http.get(url, headers: {'range': 'bytes=10-19'}); + expect(part.statusCode, 206); + expect(part.headers['content-range'], 'bytes 10-19/100'); + expect(part.bodyBytes, hasLength(10)); + final bad = await http.get(url, headers: {'range': 'bytes=500-'}); + expect(bad.statusCode, 416); + expect(bad.headers['content-range'], 'bytes */100'); + }); + + test('an empty file is served empty', () async { + final empty = attached(await attach([], name: 'empty.txt')).ref; + final response = await http.get( + Uri.parse('$base/images/${empty.relativePath}'), + ); + expect(response.statusCode, 200); + expect(response.bodyBytes, isEmpty); + }); + + test('a client leaving mid-download does not stop the server', () async { + final big = attached( + await attach(List.filled(16 * 1024 * 1024, 1), name: 'big.bin'), + ).ref; + final client = HttpClient(); + final request = await client.getUrl( + Uri.parse('$base/images/${big.relativePath}'), + ); + final response = await request.close(); + await response.first; + client.close(force: true); + await Future.delayed(const Duration(milliseconds: 50)); + expect((await http.get(Uri.parse('$base/'))).statusCode, 200); + }); + }); + + test('writes from another site are refused, the app own allowed', () async { + final foreign = await attach( + [1], + headers: {'origin': 'https://evil.example'}, + ); + expect(foreign.statusCode, 403); + expect(store.pending(), isEmpty); + final backup = await http.post( + Uri.parse('$base/backup/backlog'), + headers: {'origin': 'https://evil.example'}, + body: 'pwned', + ); + expect(backup.statusCode, 403); + expect(File(p.join(root.path, 'BACKLOG.md')).existsSync(), isFalse); + for (final own in [ + 'http://localhost:${server.port}', + 'http://127.0.0.1:${server.port}', + ]) { + expect((await attach([1], headers: {'origin': own})).statusCode, 200); + } + final read = await http.get( + Uri.parse('$base/'), + headers: {'origin': 'https://evil.example'}, + ); + expect(read.statusCode, 200, reason: 'reads stay CORS-protected'); + }); + + test('an upload abandoned halfway leaves nothing behind', () async { + final socket = await Socket.connect('localhost', server.port); + socket.write( + 'POST /images/attach?note=n1&name=a.bin HTTP/1.1\r\n' + 'Host: localhost\r\nContent-Length: 1000000\r\n\r\nabc', + ); + await socket.flush(); + await Future.delayed(const Duration(milliseconds: 100)); + socket.destroy(); + await Future.delayed(const Duration(milliseconds: 100)); + expect(store.root.listSync(recursive: true).whereType(), isEmpty); + expect((await http.get(Uri.parse('$base/'))).statusCode, 200); + }); + + test('a slow upload does not hold up the page', () async { + final slow = http.StreamedRequest( + 'POST', + Uri.parse('$base/images/attach?note=n1&name=slow.bin'), + )..contentLength = 4; + final sent = http.Client().send(slow); + slow.sink.add([1, 2]); + await Future.delayed(const Duration(milliseconds: 50)); + final page = await http + .get(Uri.parse('$base/')) + .timeout(const Duration(seconds: 5)); + expect(page.statusCode, 200); + slow.sink.add([3, 4]); + unawaited(slow.sink.close()); + expect((await sent).statusCode, 200); + }); +} diff --git a/test/desktop/wrapper_attachment_serving_test.dart b/test/desktop/wrapper_attachment_serving_test.dart new file mode 100644 index 0000000..61a3023 --- /dev/null +++ b/test/desktop/wrapper_attachment_serving_test.dart @@ -0,0 +1,78 @@ +import 'dart:io'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:todo/desktop/wrapper_attachment_serving.dart'; + +void main() { + test('script-free types are inline, everything else downloads', () { + const inline = { + 'jpg': 'image/jpeg', + 'jpeg': 'image/jpeg', + 'png': 'image/png', + 'gif': 'image/gif', + 'webp': 'image/webp', + 'bmp': 'image/bmp', + 'pdf': 'application/pdf', + 'txt': 'text/plain', + 'md': 'text/plain', + 'log': 'text/plain', + 'csv': 'text/plain', + 'mp4': 'video/mp4', + 'm4v': 'video/mp4', + 'webm': 'video/webm', + 'ogv': 'video/ogg', + 'mov': 'video/quicktime', + 'mp3': 'audio/mpeg', + 'm4a': 'audio/mp4', + 'ogg': 'audio/ogg', + 'oga': 'audio/ogg', + 'opus': 'audio/ogg', + 'wav': 'audio/wav', + 'flac': 'audio/flac', + }; + inline.forEach((extension, mime) { + final kind = attachmentContentType(extension); + expect(kind.type.mimeType, mime, reason: extension); + expect(kind.inline, isTrue, reason: extension); + }); + expect(attachmentContentType('txt').type.charset, 'utf-8'); + for (final risky in ['html', 'svg', 'xhtml', 'js', 'bin', 'exe']) { + final kind = attachmentContentType(risky); + expect(kind.type, ContentType.binary, reason: risky); + expect(kind.inline, isFalse, reason: risky); + } + }); + + test('the disposition carries any name, RFC 5987 encoded', () { + expect( + contentDisposition('Raport (v2) zaΕΌΓ³Ε‚Δ‡*.pdf', inline: true), + "inline; filename*=UTF-8''Raport%20%28v2%29%20za%C5%BC%C3%B3%C5%82%C4%87%2A.pdf", + ); + expect( + contentDisposition("it's.html", inline: false), + "attachment; filename*=UTF-8''it%27s.html", + ); + }); + + test('parseByteRange reads one range within the file', () { + expect(parseByteRange('bytes=0-9', 100), (0, 9)); + expect(parseByteRange('bytes=5-', 10), (5, 9)); + expect(parseByteRange('bytes=5-100', 10), (5, 9)); + expect(parseByteRange('bytes=-3', 10), (7, 9)); + expect(parseByteRange('bytes=-100', 10), (0, 9)); + expect(parseByteRange(' bytes=1-1 ', 10), (1, 1)); + }); + + test('parseByteRange refuses what it cannot serve', () { + for (final header in [ + 'bytes=10-', + 'bytes=5-2', + 'bytes=-0', + 'bytes=-', + 'bytes=0-1,4-5', + 'items=0-1', + ]) { + expect(parseByteRange(header, 10), isNull, reason: header); + } + }); +} diff --git a/test/desktop/wrapper_image_routes_test.dart b/test/desktop/wrapper_image_routes_test.dart index 38f61d8..dc1bbc1 100644 --- a/test/desktop/wrapper_image_routes_test.dart +++ b/test/desktop/wrapper_image_routes_test.dart @@ -7,8 +7,9 @@ import 'package:http/http.dart' as http; import 'package:path/path.dart' as p; import 'package:todo/desktop/wrapper_image_routes.dart'; import 'package:todo/desktop/wrapper_server.dart'; -import 'package:todo/images/image_cache_store.dart'; import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/image_cache_store.dart'; +import 'package:todo/images/image_processing.dart'; import '../images/fake_dufs.dart'; @@ -89,7 +90,7 @@ void main() { }); test('oversized uploads are refused with 413', () async { - final big = Uint8List(kMaxImageUploadBytes + 1)..[0] = 0xFF; + final big = Uint8List(kMaxImageDecodeBytes + 1)..[0] = 0xFF; final post = await http.post(Uri.parse('$base/images/$rel'), body: big); expect(post.statusCode, 413); }); @@ -101,6 +102,14 @@ void main() { 404, ); expect((await http.delete(Uri.parse('$base/images/$rel'))).statusCode, 405); + // Only a pasted image may be POSTed to its own path. + expect( + (await http.post( + Uri.parse('$base/images/n1/x.pdf'), + body: [1], + )).statusCode, + 405, + ); expect( (await http.get(Uri.parse('$base/images/reconcile'))).statusCode, 404, diff --git a/test/fake_image_store.dart b/test/fake_image_store.dart index 8f56a45..4fefec5 100644 --- a/test/fake_image_store.dart +++ b/test/fake_image_store.dart @@ -15,6 +15,12 @@ class FakeImageStore extends ImageStore { /// Bytes accepted per ref, in attach order. final Map attached = {}; + /// Files accepted through [attachFile], in order. + final List attachedFiles = []; + + /// What [sizeOf] reports for an attachment on this device. + int size = 2048; + /// Note texts passed to each reconcile. final List> reconciles = []; @@ -50,6 +56,27 @@ class FakeImageStore extends ImageStore { return ref; } + /// Files whose name ends in `.png` or `.jpg` come back as images. + @override + Future attachFile(String noteId, XFile file) async { + await gate; + final failure = this.failure; + if (failure != null) throw failure; + attachedFiles.add(file); + final isImage = file.name.endsWith('.png') || file.name.endsWith('.jpg'); + return Attachment( + ref: isImage + ? AttachmentRef.create(noteId) + : AttachmentRef.forFile(noteId, file.name), + name: file.name, + isImage: isImage, + ); + } + + @override + Future sizeOf(AttachmentRef ref) async => + missing.contains(ref) ? null : size; + @override Future reconcile(Iterable noteTexts) async => reconciles.add(noteTexts.toList()); diff --git a/test/images/attachment_ingest_test.dart b/test/images/attachment_ingest_test.dart new file mode 100644 index 0000000..1e028fc --- /dev/null +++ b/test/images/attachment_ingest_test.dart @@ -0,0 +1,145 @@ +import 'dart:io'; +import 'dart:typed_data'; + +import 'package:flutter_test/flutter_test.dart'; +import 'package:image/image.dart' as img; +import 'package:path/path.dart' as p; +import 'package:todo/images/attachment_ingest.dart'; +import 'package:todo/images/image_cache_store.dart'; +import 'package:todo/images/image_processing.dart'; + +void main() { + late Directory root; + late ImageCacheStore store; + late List prepared; + + final png = img.encodePng(img.Image(width: 4, height: 3)); + final jpeg = Uint8List.fromList([0xFF, 0xD8, 0xFF, 1]); + + AttachmentIngest ingest({Uint8List? Function(Uint8List)? prepare}) => + AttachmentIngest( + store, + prepare: (bytes) async { + prepared.add(bytes.length); + return (prepare ?? (_) => jpeg)(bytes); + }, + ); + + setUp(() { + root = Directory.systemTemp.createTempSync('ingest'); + store = ImageCacheStore(Directory(p.join(root.path, 'store'))); + prepared = []; + }); + tearDown(() => root.deleteSync(recursive: true)); + + void expectNoTemps() => expect( + store.root.listSync(recursive: true).where((e) => e.path.endsWith('.tmp')), + isEmpty, + ); + + test('a non-image is stored byte for byte as a queued file', () async { + final pdf = '%PDF-1.7 hello'.codeUnits; + final a = await ingest().fromStream('n1', 'Raport.pdf', Stream.value(pdf)); + expect(a.isImage, isFalse); + expect(a.name, 'Raport.pdf'); + expect(a.ref.extension, 'pdf'); + expect(store.fileFor(a.ref).readAsBytesSync(), pdf); + expect(store.isPending(a.ref), isTrue); + expect(prepared, isEmpty, reason: 'never decoded'); + expectNoTemps(); + }); + + test('an image is re-encoded and stored as a jpg', () async { + final a = await ingest().fromStream('n1', 'shot.png', Stream.value(png)); + expect(a.isImage, isTrue); + expect(a.ref.extension, 'jpg'); + expect(store.fileFor(a.ref).readAsBytesSync(), jpeg); + expect(store.isPending(a.ref), isTrue); + expect(prepared, [png.length]); + expectNoTemps(); + }); + + test('the real preparer is the default', () async { + final a = await AttachmentIngest( + store, + ).fromStream('n1', 'shot.png', Stream.value(png)); + expect(img.decodeJpg(store.fileFor(a.ref).readAsBytesSync()), isNotNull); + }); + + test('an image that will not decode is kept as a file', () async { + final a = await ingest( + prepare: (_) => null, + ).fromStream('n1', 'broken.png', Stream.value(png)); + expect(a.isImage, isFalse); + expect(a.ref.extension, 'png'); + expect(store.fileFor(a.ref).readAsBytesSync(), png); + }); + + test('an image too big to decode is kept as a file, unread', () async { + final chunk = Uint8List(1024 * 1024); + Stream> huge() async* { + yield png.sublist(0, kImageSniffBytes); + for (var i = 0; i < kMaxImageDecodeBytes ~/ chunk.length; i++) { + yield chunk; + } + } + + final a = await ingest().fromStream('n1', 'huge.png', huge()); + expect(a.isImage, isFalse); + expect(prepared, isEmpty); + expect( + store.fileFor(a.ref).lengthSync(), + kMaxImageDecodeBytes + kImageSniffBytes, + ); + }); + + test('a failed read leaves nothing behind', () async { + Stream> broken() async* { + yield [1, 2]; + throw const FileSystemException('gone'); + } + + await expectLater( + ingest().fromStream('n1', 'a.bin', broken()), + throwsA(isA()), + ); + expectNoTemps(); + expect(store.pending(), isEmpty); + }); + + test('fromFile copies a file it does not own', () async { + final source = File(p.join(root.path, 'mine.txt'))..writeAsStringSync('x'); + final a = await ingest().fromFile('n1', 'mine.txt', source, move: false); + expect(store.fileFor(a.ref).readAsStringSync(), 'x'); + expect(source.existsSync(), isTrue); + }); + + test('fromFile moves a picker copy instead of duplicating it', () async { + final source = File(p.join(root.path, 'copy.txt'))..writeAsStringSync('x'); + final a = await ingest().fromFile('n1', 'copy.txt', source, move: true); + expect(store.fileFor(a.ref).readAsStringSync(), 'x'); + expect(source.existsSync(), isFalse); + expectNoTemps(); + }); + + test('a failure after the move cleans up the moved file', () async { + final source = File(p.join(root.path, 'copy.png'))..writeAsBytesSync(png); + await expectLater( + AttachmentIngest( + store, + prepare: (_) async => throw const FileSystemException('disk full'), + ).fromFile('n1', 'copy.png', source, move: true), + throwsA(isA()), + ); + expectNoTemps(); + }); + + test('fromFile falls back to copy-and-delete across filesystems', () async { + final shm = Directory('/dev/shm').createTempSync('ingest'); + addTearDown(() => shm.deleteSync(recursive: true)); + final source = File(p.join(shm.path, 'far.txt'))..writeAsStringSync('y'); + final a = await ingest().fromFile('n1', 'far.txt', source, move: true); + expect(store.fileFor(a.ref).readAsStringSync(), 'y'); + expect(source.existsSync(), isFalse); + }, skip: Directory('/dev/shm').existsSync() ? false : 'needs /dev/shm'); +} diff --git a/test/images/attachment_links_test.dart b/test/images/attachment_links_test.dart new file mode 100644 index 0000000..6243b5a --- /dev/null +++ b/test/images/attachment_links_test.dart @@ -0,0 +1,61 @@ +import 'package:flutter_test/flutter_test.dart'; +import 'package:todo/images/attachment_links.dart'; +import 'package:todo/images/attachment_ref.dart'; + +void main() { + final image = AttachmentRef.tryParse('n1/a.jpg')!; + final pdf = AttachmentRef.tryParse('n1/b.pdf')!; + final other = AttachmentRef.create('n2'); + + test('attachmentsIn finds images and files, in order, each once', () { + final text = + 'hi ${image.imageMarkdown} and\n${pdf.fileMarkdown('b.pdf')}\n' + '${other.imageMarkdown}\n${image.imageMarkdown}\n' + '![x](https://example.com/a.jpg) [y]($kDufsBaseUrl/Keepass/a.pdf)'; + final found = attachmentsIn(text); + expect([for (final a in found) a.ref], [image, pdf, other]); + expect([for (final a in found) a.isImage], [true, false, true]); + expect(found[1].name, 'b.pdf'); + expect(attachmentsIn('no attachments'), isEmpty); + }); + + test('an image link is never also read as a file link', () { + final found = attachmentsIn(image.imageMarkdown); + expect(found, hasLength(1)); + expect(found.single.isImage, isTrue); + }); + + test('attachmentRefsIn covers both kinds', () { + expect( + attachmentRefsIn('${image.imageMarkdown}\n${pdf.fileMarkdown('b')}'), + {image, pdf}, + ); + }); + + test('attachmentOfLine only matches a line that is just one link', () { + expect(attachmentOfLine(' ${image.imageMarkdown} ')!.ref, image); + expect(attachmentOfLine('![alt text](${image.url})')!.name, 'alt text'); + final file = attachmentOfLine(pdf.fileMarkdown('Raport.pdf'))!; + expect(file.ref, pdf); + expect(file.isImage, isFalse); + expect(file.name, 'Raport.pdf'); + expect(attachmentOfLine('see ${image.imageMarkdown}'), isNull); + expect(attachmentOfLine('plain'), isNull); + }); + + test('backlog paths round-trip exactly, images and files alike', () { + final text = + 'a\n${image.imageMarkdown}\n${pdf.fileMarkdown('b.pdf')}\n' + '![](https://example.com/x.jpg)'; + final disk = toBacklogAttachmentPaths(text); + expect(disk, contains('![]($kBacklogImagesRoot/n1/a.jpg)')); + expect(disk, contains('[b.pdf]($kBacklogImagesRoot/n1/b.pdf)')); + expect(disk, isNot(contains('$kDufsBaseUrl/$kImagesDir'))); + expect(fromBacklogAttachmentPaths(disk), text); + }); + + test('a bare path outside a link is left alone', () { + const text = 'see $kBacklogImagesRoot/n1/b.pdf'; + expect(fromBacklogAttachmentPaths(text), text); + }); +} diff --git a/test/images/attachment_ref_test.dart b/test/images/attachment_ref_test.dart index 9edc71a..bf78550 100644 --- a/test/images/attachment_ref_test.dart +++ b/test/images/attachment_ref_test.dart @@ -8,9 +8,11 @@ void main() { test('a ref exposes its parts, url and markdown', () { expect(ref.noteId, 'note-1'); expect(ref.file, '0815ff8a-203e-4f54-9cc5-8a58b27982ec.jpg'); + expect(ref.extension, 'jpg'); expect(ref.relativePath, rel); expect(ref.url, '$kDufsBaseUrl/$kImagesDir/$rel'); - expect(ref.markdown, '![](${ref.url})'); + expect(ref.imageMarkdown, '![](${ref.url})'); + expect(ref.fileMarkdown('a.pdf'), '[a.pdf](${ref.url})'); }); test('create files a fresh uuid jpg under the note', () { @@ -21,19 +23,40 @@ void main() { expect(a, isNot(b)); }); - test('create refuses an unsafe note id', () { - expect(() => AttachmentRef.create('../x'), throwsArgumentError); + test('forFile keeps a plain extension, else falls back to bin', () { + expect(AttachmentRef.forFile('n1', 'Report.PDF').extension, 'pdf'); + expect(AttachmentRef.forFile('n1', 'a.tar.gz').extension, 'gz'); + for (final name in ['README', '.bashrc', 'x.', 'a.verylongextension']) { + expect(AttachmentRef.forFile('n1', name).extension, 'bin', reason: name); + } + expect( + AttachmentRef.forFile('n1', 'a.pdf'), + isNot(AttachmentRef.forFile('n1', 'a.pdf')), + ); }); - test('tryParse rejects traversal, nesting and other extensions', () { + test('create and forFile refuse an unsafe note id', () { + expect(() => AttachmentRef.create('../x'), throwsArgumentError); + expect(() => AttachmentRef.forFile('a/b', 'x.pdf'), throwsArgumentError); + expect(isSafeNoteId('note-1_A'), isTrue); + expect(isSafeNoteId('../x'), isFalse); + expect(isSafeNoteId(''), isFalse); + }); + + test('tryParse takes any short extension but rejects unsafe paths', () { + expect(AttachmentRef.tryParse('a/b.png'), isNotNull); + expect(AttachmentRef.tryParse('a/b.mp4'), isNotNull); for (final bad in [ '../x/y.jpg', 'a/../b.jpg', 'a/b/c.jpg', - 'a/b.png', 'a/.jpg', '/a/b.jpg', 'a b/c.jpg', + 'a/b.jpg.pending', + 'a/b.pdf.tmp', + 'a/b.PDF', + 'a/b', ]) { expect(AttachmentRef.tryParse(bad), isNull, reason: bad); } @@ -43,32 +66,43 @@ void main() { expect({ref, AttachmentRef.tryParse(rel)!}, hasLength(1)); }); - test('imageRefsIn finds every dufs image link, in order, once', () { - final other = AttachmentRef.create('n2'); - final text = - 'hi ${ref.markdown} and\n${other.markdown}\n${ref.markdown}\n' - '![x](https://example.com/a.jpg) ![]($kDufsBaseUrl/Keepass/a.jpg)'; - expect(imageRefsIn(text).toList(), [ref, other]); - expect(imageRefsIn('no images'), isEmpty); + test('linkLabel keeps a name readable but unable to break the link', () { + expect(linkLabel('Raport [final].pdf'), 'Raport (final).pdf'); + expect(linkLabel('two\r\nlines.txt'), 'two lines.txt'); + expect(linkLabel(' '), 'file'); }); - test('imageRefOfLine only matches a line that is just one image', () { - expect(imageRefOfLine(' ${ref.markdown} '), ref); - expect(imageRefOfLine('see ${ref.markdown}'), isNull); - expect(imageRefOfLine('![alt text](${ref.url})'), ref); - expect(imageRefOfLine('plain'), isNull); - }); + group('Attachment', () { + final file = AttachmentRef.tryParse('n1/b.pdf')!; - test('backlog paths round-trip exactly', () { - final text = 'a\n${ref.markdown}\n![](https://example.com/x.jpg)'; - final disk = toBacklogImagePaths(text); - expect(disk, contains('![]($kBacklogImagesRoot/$rel)')); - expect(disk, isNot(contains(kDufsBaseUrl))); - expect(fromBacklogImagePaths(disk), text); - }); + test('markdown is an image line or a named file link', () { + expect( + Attachment(ref: ref, name: 'x.png', isImage: true).markdown, + ref.imageMarkdown, + ); + expect( + Attachment(ref: file, name: 'a].pdf', isImage: false).markdown, + '[a).pdf](${file.url})', + ); + }); - test('a bare path outside an image link is left alone', () { - const text = 'see $kBacklogImagesRoot/$rel'; - expect(fromBacklogImagePaths(text), text); + test('round-trips through JSON', () { + final attachment = Attachment(ref: file, name: 'a.pdf', isImage: false); + final back = Attachment.fromJson(attachment.toJson()); + expect(back.ref, file); + expect(back.name, 'a.pdf'); + expect(back.isImage, isFalse); + }); + + test('fromJson rejects anything else', () { + for (final bad in [ + null, + 'x', + {'ref': '../x.pdf', 'name': 'a', 'image': false}, + {'ref': 'n1/b.pdf', 'name': 'a'}, + ]) { + expect(() => Attachment.fromJson(bad), throwsFormatException); + } + }); }); } diff --git a/test/images/dufs_image_client_test.dart b/test/images/dufs_image_client_test.dart index e98bcd4..6e06833 100644 --- a/test/images/dufs_image_client_test.dart +++ b/test/images/dufs_image_client_test.dart @@ -1,12 +1,11 @@ import 'dart:convert'; import 'dart:io'; -import 'dart:typed_data'; import 'package:flutter_test/flutter_test.dart'; import 'package:http/http.dart' as http; import 'package:http/testing.dart'; -import 'package:todo/images/dufs_image_client.dart'; import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/dufs_image_client.dart'; void main() { final ref = AttachmentRef.tryParse('n1/a.jpg')!; @@ -20,12 +19,14 @@ void main() { expect(const DufsLogin(user: 'u', password: 'p').baseUrl, kDufsBaseUrl); }); - test('upload PUTs the bytes with basic auth; true on 2xx', () async { + Stream> body(List bytes) => Stream.value(bytes); + + test('upload streams the body with basic auth and its length', () async { late http.Request seen; final ok = await client((r) async { seen = r; return http.Response('', 201); - }).upload(ref, Uint8List.fromList([1, 2])); + }).upload(ref, body([1, 2]), 2); expect(ok, isTrue); expect(seen.method, 'PUT'); expect(seen.url.toString(), 'https://d/todo-images/n1/a.jpg'); @@ -34,35 +35,47 @@ void main() { expect(seen.bodyBytes, [1, 2]); }); + test('upload declares the length up front, so nothing is buffered', () async { + final capture = _CapturingClient(); + final file = AttachmentRef.tryParse('n1/b.pdf')!; + await DufsImageClient( + login, + httpClient: capture, + ).upload(file, body([1, 2, 3]), 3); + expect(capture.seen.contentLength, 3); + expect(capture.seen.headers['content-type'], 'application/octet-stream'); + }); + test('upload is false on a refusal or a network error', () async { expect( await client( (_) async => http.Response('', 403), - ).upload(ref, Uint8List(1)), + ).upload(ref, body([1]), 1), isFalse, ); expect( await client( (_) => throw const SocketException('down'), - ).upload(ref, Uint8List(1)), + ).upload(ref, body([1]), 1), isFalse, ); }); - test('download returns bytes on 200, null otherwise', () async { + test('openDownload streams the body on 200, null otherwise', () async { + final stream = await client((r) async { + expect(r.method, 'GET'); + expect(r.headers['authorization'], auth); + return http.Response.bytes([7, 8], 200); + }).openDownload(ref); + expect(await stream!.expand((chunk) => chunk).toList(), [7, 8]); expect( - await client((r) async { - expect(r.headers['authorization'], auth); - return http.Response.bytes([7, 8], 200); - }).download(ref), - [7, 8], - ); - expect( - await client((_) async => http.Response('', 404)).download(ref), + await client((_) async => http.Response('', 404)).openDownload(ref), isNull, ); expect( - await client((_) => throw const SocketException('down')).download(ref), + await client( + (_) => throw const SocketException('down'), + ).openDownload(ref), isNull, ); }); @@ -108,3 +121,15 @@ class _ClosingClient extends http.BaseClient { @override void close() => onClose(); } + +/// Records the request as sent, before any mock flattens it. +class _CapturingClient extends http.BaseClient { + late http.BaseRequest seen; + + @override + Future send(http.BaseRequest request) async { + seen = request; + await request.finalize().drain(); + return http.StreamedResponse(const Stream.empty(), 201); + } +} diff --git a/test/images/fake_dufs.dart b/test/images/fake_dufs.dart index 56862a5..013b862 100644 --- a/test/images/fake_dufs.dart +++ b/test/images/fake_dufs.dart @@ -21,25 +21,38 @@ class FakeDufs { /// Every request's `METHOD path`, in order. final List requests = []; - /// The HTTP client the image client talks through. - http.Client httpClient() => MockClient((request) async { + /// Paths whose GET drops the connection after the first chunk. + final Set dropMidBody = {}; + + /// The HTTP client the image client talks through. Streaming, like the + /// real one: bodies arrive and leave as streams. + http.Client httpClient() => MockClient.streaming((request, body) async { requests.add('${request.method} ${request.url.path}'); if (!online) throw const SocketException('offline'); final key = request.url.path.replaceFirst('/todo-images/', ''); switch (request.method) { case 'PUT': - files[key] = request.bodyBytes; - return http.Response('', 201); + files[key] = await body.toBytes(); + return http.StreamedResponse(const Stream.empty(), 201); case 'GET': final bytes = files[key]; - return bytes == null - ? http.Response('', 404) - : http.Response.bytes(bytes, 200); + if (bytes == null) { + return http.StreamedResponse(const Stream.empty(), 404); + } + return http.StreamedResponse( + dropMidBody.contains(key) ? _dropped(bytes) : Stream.value(bytes), + 200, + ); default: - return http.Response('', 207); + return http.StreamedResponse(const Stream.empty(), 207); } }); + Stream> _dropped(List bytes) async* { + yield bytes; + throw const SocketException('connection dropped'); + } + /// A client over this fake. DufsImageClient client() => DufsImageClient( const DufsLogin(user: 'todo', password: 'pw', baseUrl: 'https://d'), diff --git a/test/images/image_cache_store_test.dart b/test/images/image_cache_store_test.dart index 29f1fd9..22d7d56 100644 --- a/test/images/image_cache_store_test.dart +++ b/test/images/image_cache_store_test.dart @@ -4,8 +4,8 @@ import 'dart:typed_data'; import 'package:flutter_test/flutter_test.dart'; import 'package:path/path.dart' as p; -import 'package:todo/images/image_cache_store.dart'; import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/images/image_cache_store.dart'; import 'fake_dufs.dart'; @@ -38,6 +38,19 @@ void main() { expect(File('${store.fileFor(a).path}.tmp').existsSync(), isFalse); }); + test('adopt moves a finished file into place and queues it', () async { + final pdf = AttachmentRef.tryParse('n1/c.pdf')!; + final tmp = store.tempFor(pdf); + await tmp.parent.create(recursive: true); + tmp.writeAsBytesSync([4, 5]); + await store.adopt(pdf, tmp); + expect(store.fileFor(pdf).readAsBytesSync(), [4, 5]); + expect(tmp.existsSync(), isFalse); + expect(store.pending(), [pdf]); + await store.reconcile({}, dufs.client()); + expect(dufs.files['n1/c.pdf'], [4, 5]); + }); + test('an empty or missing root has nothing pending', () { root.deleteSync(recursive: true); expect(store.pending(), isEmpty); @@ -92,6 +105,18 @@ void main() { expect(report.missing, 0); }); + test('a download dropped mid-body leaves no file behind', () async { + dufs.files['n2/b.jpg'] = [9, 9]; + dufs.dropMidBody.add('n2/b.jpg'); + final report = await store.reconcile({b}, dufs.client()); + expect(report.downloaded, 0); + expect(report.missing, 1); + expect(store.fileFor(b).existsSync(), isFalse); + expect(store.tempFor(b).existsSync(), isFalse); + dufs.dropMidBody.clear(); + expect((await store.reconcile({b}, dufs.client())).downloaded, 1); + }); + test('an image missing on dufs stays missing', () async { final report = await store.reconcile({b}, dufs.client()); expect(report.downloaded, 0); @@ -128,6 +153,29 @@ void main() { expect(File(p.join(root.path, 'n1', 'odd name.jpg')).existsSync(), isTrue); }); + test('evicts any kind of attachment, never a marker', () async { + final pdf = AttachmentRef.tryParse('n1/c.pdf')!; + dufs.files['n1/c.pdf'] = [1]; + await store.reconcile({pdf}, dufs.client()); + age(pdf, const Duration(hours: 1)); + expect((await store.reconcile({b}, dufs.client())).evicted, 1); + expect(store.fileFor(pdf).existsSync(), isFalse); + }); + + test('sweeps abandoned .tmp files once they are old', () async { + final stale = store.tempFor(a); + final fresh = store.tempFor(b); + for (final tmp in [stale, fresh]) { + await tmp.parent.create(recursive: true); + tmp.writeAsBytesSync([1]); + } + stale.setLastModifiedSync(now.subtract(const Duration(hours: 1))); + final report = await store.reconcile({b}, null); + expect(report.evicted, 0, reason: 'a sweep is not an eviction'); + expect(stale.existsSync(), isFalse); + expect(fresh.existsSync(), isTrue); + }); + test('a young unlinked image survives the grace period', () async { dufs.files['n1/a.jpg'] = [1]; await store.reconcile({a}, dufs.client()); @@ -168,18 +216,16 @@ void main() { }); test('a failing pass does not wedge the queue', () async { - final boom = ImageCacheStore( - Directory(p.join(root.path, 'file-not-dir')), - now: () => now, - ); - File(boom.root.path).writeAsStringSync('x'); - dufs.files['n1/a.jpg'] = [1]; // downloading it must create a dir: fails + // An unreadable folder makes listing the queue throw. + final locked = Directory(p.join(root.path, 'locked'))..createSync(); + Process.runSync('chmod', ['000', locked.path]); + addTearDown(() => Process.runSync('chmod', ['755', locked.path])); await expectLater( - boom.reconcile({a}, dufs.client()), + store.reconcile({a}, dufs.client()), throwsA(isA()), ); - File(boom.root.path).deleteSync(); - final report = await boom.reconcile({}, null); + Process.runSync('chmod', ['755', locked.path]); + final report = await store.reconcile({}, null); expect(report.pending, 0); }); } diff --git a/test/images/image_processing_test.dart b/test/images/image_processing_test.dart index c11b8ad..aafcd09 100644 --- a/test/images/image_processing_test.dart +++ b/test/images/image_processing_test.dart @@ -54,4 +54,42 @@ void main() { expect(prepareImage(Uint8List.fromList('hello world'.codeUnits)), isNull); expect(prepareImage(Uint8List(0)), isNull); }); + + test('looksLikeImage knows the photo and screenshot formats', () { + final formats = { + 'jpeg': img.encodeJpg(img.Image(width: 2, height: 2)), + 'png': img.encodePng(img.Image(width: 2, height: 2)), + 'gif': img.encodeGif(img.Image(width: 2, height: 2)), + 'bmp': img.encodeBmp(img.Image(width: 2, height: 2)), + 'tiff': img.encodeTiff(img.Image(width: 2, height: 2)), + 'webp': Uint8List.fromList([ + ...'RIFF'.codeUnits, + 0, + 0, + 0, + 0, + ...'WEBP'.codeUnits, + ]), + 'tiff big-endian': Uint8List.fromList([0x4D, 0x4D, 0x00, 0x2A]), + }; + formats.forEach((name, bytes) { + expect( + looksLikeImage(bytes.take(kImageSniffBytes).toList()), + isTrue, + reason: name, + ); + }); + }); + + test('looksLikeImage refuses everything else', () { + for (final head in [ + '%PDF-1.7'.codeUnits, + 'hello world'.codeUnits, + [...'RIFF'.codeUnits, 0, 0, 0, 0, ...'AVI '.codeUnits], + [0xFF, 0xD8], + [], + ]) { + expect(looksLikeImage(head), isFalse, reason: '$head'); + } + }); } diff --git a/test/ui_images/attach_files_test.dart b/test/ui_images/attach_files_test.dart new file mode 100644 index 0000000..bddbab9 --- /dev/null +++ b/test/ui_images/attach_files_test.dart @@ -0,0 +1,97 @@ +import 'dart:async'; + +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:todo/attachments/image_store_api.dart'; +import 'package:todo/attachments/image_store_scope.dart'; +import 'package:todo/ui/image_attach.dart'; + +import '../fake_image_store.dart'; + +void main() { + late FakeImageStore store; + late ImageAttachController target; + late List> inserted; + + setUp(() { + store = FakeImageStore(); + target = ImageAttachController(); + inserted = []; + target.bind(inserted.add); + }); + + Future pumpScope(WidgetTester tester) async { + late BuildContext context; + await tester.pumpWidget( + ImageStoreScope( + store: store, + child: MaterialApp( + home: Scaffold( + body: Builder( + builder: (c) { + context = c; + return const SizedBox(); + }, + ), + ), + ), + ), + ); + return context; + } + + testWidgets('files and images attach in order, each as its own line', ( + tester, + ) async { + final context = await pumpScope(tester); + await attachFiles( + context, + noteId: () => 'n1', + target: target, + files: [XFile('/p/a.pdf'), XFile('/p/shot.png')], + ); + final lines = inserted.single; + expect(lines.first, startsWith('[a.pdf](')); + expect(lines.last, startsWith('![](')); + }); + + testWidgets('a store failure is shown and keeps what came before', ( + tester, + ) async { + final context = await pumpScope(tester); + var calls = 0; + await attachFiles( + context, + noteId: () { + if (++calls == 2) store.failure = const ImageStoreException('full'); + return 'n1'; + }, + target: target, + files: [XFile('/p/a.pdf'), XFile('/p/b.pdf'), XFile('/p/c.pdf')], + ); + await tester.pump(); + expect(inserted.single, hasLength(1)); + expect(find.text('full'), findsOneWidget); + }); + + testWidgets('nothing is shown once the screen closed mid-attach', ( + tester, + ) async { + final context = await pumpScope(tester); + final gate = Completer(); + store + ..gate = gate.future + ..failure = const ImageStoreException('full'); + final done = attachFiles( + context, + noteId: () => 'n1', + target: target, + files: [XFile('/p/a.pdf')], + ); + await tester.pumpWidget(const SizedBox()); + gate.complete(); + await done; + await tester.pump(); + expect(find.text('full'), findsNothing); + }); +} diff --git a/test/ui_images/image_picking_test.dart b/test/ui_images/attachment_picking_test.dart similarity index 52% rename from test/ui_images/image_picking_test.dart rename to test/ui_images/attachment_picking_test.dart index b0c1dcf..a30e5b3 100644 --- a/test/ui_images/image_picking_test.dart +++ b/test/ui_images/attachment_picking_test.dart @@ -1,53 +1,44 @@ -import 'dart:typed_data'; - import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; -import 'package:todo/ui/image_picking.dart'; +import 'package:todo/ui/attachment_picking.dart'; void main() { - testWidgets('the button passes picked images on, ignores a cancel', ( + testWidgets('the button passes picked files on, ignores a cancel', ( tester, ) async { - final got = >[]; - var result = []; + final got = >[]; + var result = []; await tester.pumpWidget( MaterialApp( home: Scaffold( - body: AttachImageButton( - onImages: got.add, - picking: (_) async => result, - ), + body: AttachButton(onFiles: got.add, picking: (_) async => result), ), ), ); - await tester.tap(find.byTooltip('Attach image')); + await tester.tap(find.byTooltip('Attach')); await tester.pump(); expect(got, isEmpty); - result = [ - Uint8List.fromList([1]), - ]; - await tester.tap(find.byTooltip('Attach image')); + result = [XFile('/picked/a.pdf')]; + await tester.tap(find.byTooltip('Attach')); await tester.pump(); - expect(got.single.single, [1]); + expect(got.single.single.name, 'a.pdf'); }); - Future?> openSheet( + Future?> openSheet( WidgetTester tester, - List asked, + List asked, ) async { - List? result; + List? result; await tester.pumpWidget( MaterialApp( home: Scaffold( body: Builder( builder: (context) => TextButton( - onPressed: () async => result = await pickImages( + onPressed: () async => result = await pickAttachments( context, fromPhone: (source) async { asked.add(source); - return [ - Uint8List.fromList([source.index]), - ]; + return [XFile('/picked/${source.name}')]; }, ), child: const Text('pick'), @@ -61,22 +52,33 @@ void main() { return result; } - testWidgets('the phone sheet offers Camera and Gallery', (tester) async { - final asked = []; - await openSheet(tester, asked); - expect(find.text('Camera'), findsOneWidget); - await tester.tap(find.text('Gallery')); - await tester.pumpAndSettle(); - expect(asked, [ImagePickSource.gallery]); + Future choose(WidgetTester tester, String entry) async { await tester.tap(find.text('pick')); await tester.pumpAndSettle(); - await tester.tap(find.text('Camera')); + await tester.tap(find.text(entry)); await tester.pumpAndSettle(); - expect(asked, [ImagePickSource.gallery, ImagePickSource.camera]); + } + + testWidgets('the phone sheet offers Camera, Gallery and File', ( + tester, + ) async { + final asked = []; + await openSheet(tester, asked); + expect(find.text('Camera'), findsOneWidget); + expect(find.text('File'), findsOneWidget); + await tester.tap(find.text('Gallery')); + await tester.pumpAndSettle(); + await choose(tester, 'Camera'); + await choose(tester, 'File'); + expect(asked, [ + AttachmentPickSource.gallery, + AttachmentPickSource.camera, + AttachmentPickSource.file, + ]); }); testWidgets('dismissing the sheet picks nothing', (tester) async { - final asked = []; + final asked = []; await openSheet(tester, asked); await tester.tapAt(const Offset(10, 10)); await tester.pumpAndSettle(); diff --git a/test/ui_images/capture_images_test.dart b/test/ui_images/capture_images_test.dart index 7cfc480..1f6a25b 100644 --- a/test/ui_images/capture_images_test.dart +++ b/test/ui_images/capture_images_test.dart @@ -4,33 +4,36 @@ import 'package:flutter/material.dart'; import 'package:flutter_test/flutter_test.dart'; import 'package:todo/data/app_settings.dart'; import 'package:todo/data/note.dart'; +import 'package:todo/ui/attachment_picking.dart'; import 'package:todo/ui/image_input.dart'; -import 'package:todo/ui/images/image_strip.dart'; +import 'package:todo/ui/images/attachment_strip.dart'; import '../capture_screen_harness.dart'; import '../fake_image_store.dart'; void main() { - testWidgets('πŸ“Ž on an empty draft creates the note under the image id', ( + testWidgets('πŸ“Ž on an empty draft creates the note under the file id', ( tester, ) async { final store = FakeImageStore(); final repo = await pumpCapture( tester, imageStore: store, - imagePicking: (_) async => [ - Uint8List.fromList([1]), + attachmentPicking: (_) async => [ + XFile('/picked/shot.png'), + XFile('/picked/Raport.pdf'), ], appSettings: ValueNotifier(const AppSettings(advancedMode: false)), ); - await tester.tap(find.byTooltip('Attach image')); + await tester.tap(find.byTooltip('Attach')); await tester.pump(); await tester.pump(); - final ref = store.attached.keys.single; + expect(store.attachedFiles, hasLength(2)); final notes = await repo.listNotes(); - expect(notes.single.id, ref.noteId); - expect(notes.single.text, contains(ref.markdown)); - expect(find.byType(ImageStrip), findsOneWidget); + expect(notes.single.text, contains('![](')); + expect(notes.single.text, contains('[Raport.pdf](')); + expect(notes.single.text, contains('/${notes.single.id}/')); + expect(find.byType(AttachmentStrip), findsOneWidget); }); testWidgets('πŸ“Ž on a started draft files under that note', (tester) async { @@ -38,19 +41,18 @@ void main() { final repo = await pumpCapture( tester, imageStore: store, - imagePicking: (_) async => [ - Uint8List.fromList([1]), - ], + attachmentPicking: (_) async => [XFile('/picked/a.pdf')], appSettings: ValueNotifier(const AppSettings(advancedMode: false)), ); await tester.enterText(find.byType(TextField), 'idea'); await tester.pump(); final id = (await repo.listNotes()).single.id; - await tester.tap(find.byTooltip('Attach image')); + await tester.tap(find.byTooltip('Attach')); await tester.pump(); await tester.pump(); - expect(store.attached.keys.single.noteId, id); - expect((await repo.listNotes()).single.text, startsWith('idea\n![](')); + final text = (await repo.listNotes()).single.text; + expect(text, startsWith('idea\n[a.pdf](')); + expect(text, contains('/$id/')); }); testWidgets('launch and resume reconcile with every note', (tester) async { @@ -94,7 +96,23 @@ void main() { expect(store.reconciles.length, greaterThan(before)); }); - testWidgets('a paste or drop attaches to the draft', (tester) async { + testWidgets('a drop attaches any file to the draft', (tester) async { + final store = FakeImageStore(); + final repo = await pumpCapture( + tester, + imageStore: store, + appSettings: ValueNotifier(const AppSettings(advancedMode: false)), + ); + tester.widget(find.byType(ImageInputListener)).onFiles([ + XFile('/dropped/clip.mp4'), + ]); + await tester.pump(); + await tester.pump(); + expect(store.attachedFiles.single.name, 'clip.mp4'); + expect((await repo.listNotes()).single.text, contains('[clip.mp4](')); + }); + + testWidgets('a paste attaches to the draft', (tester) async { final store = FakeImageStore(); final repo = await pumpCapture( tester, diff --git a/test/ui_images/detail_and_misc_images_test.dart b/test/ui_images/detail_and_misc_images_test.dart index f2373c5..4294f02 100644 --- a/test/ui_images/detail_and_misc_images_test.dart +++ b/test/ui_images/detail_and_misc_images_test.dart @@ -9,11 +9,13 @@ import 'package:todo/data/note_template_parser.dart'; import 'package:todo/images/attachment_ref.dart'; import 'package:todo/sync/desktop_wrapper.dart'; import 'package:todo/sync/notes_markdown.dart'; +import 'package:todo/ui/attachment_picking.dart'; import 'package:todo/ui/capture_draft.dart'; import 'package:todo/ui/image_input.dart'; import 'package:todo/ui/image_input_files.dart'; +import 'package:todo/ui/images/attachment_strip.dart'; +import 'package:todo/ui/images/file_embed.dart'; import 'package:todo/ui/images/image_embed.dart'; -import 'package:todo/ui/images/image_strip.dart'; import 'package:todo/ui/markdown_view.dart'; import 'package:todo/ui/note_detail_screen.dart'; @@ -48,9 +50,7 @@ void main() { note: note('first\nsecond'), repository: repo, appSettings: ValueNotifier(const AppSettings(advancedMode: true)), - imagePicking: (_) async => [ - Uint8List.fromList([1]), - ], + attachmentPicking: (_) async => [XFile('/picked/shot.png')], ), ), ), @@ -58,41 +58,52 @@ void main() { await tester.pump(); tester.widget(find.byType(TextField)).controller!.selection = const TextSelection.collapsed(offset: 5); - await tester.tap(find.byTooltip('Attach image')); + await tester.tap(find.byTooltip('Attach')); await tester.pump(); await tester.pump(); - final added = store.attached.keys.single; - expect(added.noteId, 'n1'); expect( (await repo.listNotes()).single.text, - 'first\n${added.markdown}\nsecond', + matches(RegExp(r'^first\n!\[\]\(\S+/n1/\S+\.jpg\)\nsecond$')), ); - expect(find.byType(ImageStrip), findsOneWidget); - // A paste or drop lands in the same note. - tester.widget(find.byType(ImageInputListener)).onImages( - [ - Uint8List.fromList([2]), - ], - 0, + expect(find.byType(AttachmentStrip), findsOneWidget); + // A paste and a drop land in the same note. + final input = tester.widget( + find.byType(ImageInputListener), ); + input.onImages([ + Uint8List.fromList([2]), + ], 0); await tester.pump(); await tester.pump(); - expect(store.attached, hasLength(2)); - expect(store.attached.keys.last.noteId, 'n1'); + expect(store.attached.keys.single.noteId, 'n1'); + input.onFiles([XFile('/dropped/Raport.pdf')]); + await tester.pump(); + await tester.pump(); + expect((await repo.listNotes()).single.text, contains('[Raport.pdf](')); // View mode renders inline and hides the strip. await tester.tap(find.text('View')); await tester.pump(); - expect(find.byType(ImageStrip), findsNothing); + expect(find.byType(AttachmentStrip), findsNothing); expect(find.byType(ImageEmbed), findsNWidgets(2)); + expect(find.byType(FileEmbed), findsOneWidget); }); - testWidgets('markdown view draws an image line as an embed', (tester) async { + testWidgets('markdown view draws image and file lines as embeds', ( + tester, + ) async { + final pdf = AttachmentRef.forFile('n1', 'a.pdf'); await tester.pumpWidget( MaterialApp( - home: Scaffold(body: MarkdownView(text: 'text\n${ref.markdown}\n')), + home: Scaffold( + body: MarkdownView( + text: 'text\n${ref.imageMarkdown}\n${pdf.fileMarkdown('a.pdf')}\n', + ), + ), ), ); + await tester.pump(); expect(find.byType(ImageEmbed), findsOneWidget); + expect(find.byType(FileEmbed), findsOneWidget); expect(find.text('text'), findsOneWidget); }); @@ -100,7 +111,11 @@ void main() { await tester.pumpWidget( Directionality( textDirection: TextDirection.ltr, - child: ImageInputListener(onImages: (_, _) {}, child: const Text('x')), + child: ImageInputListener( + onImages: (_, _) {}, + onFiles: (_) {}, + child: const Text('x'), + ), ), ); expect(find.text('x'), findsOneWidget); @@ -117,17 +132,31 @@ void main() { expect(split.rejected, 2); }); - test('noteTitle skips image lines', () { - expect(noteTitle('${ref.markdown}\n# Real title'), 'Real title'); - expect(noteTitle(ref.markdown), 'Image'); - expect(noteTitle('${ref.markdown}\n\n${ref.markdown}'), '2 images'); + test('noteTitle skips attachment lines', () { + final pdf = AttachmentRef.forFile('n1', 'a.pdf'); + final file = pdf.fileMarkdown('Raport.pdf'); + expect(noteTitle('${ref.imageMarkdown}\n# Real title'), 'Real title'); + expect(noteTitle('$file\n# Real title'), 'Real title'); + expect(noteTitle(ref.imageMarkdown), 'Image'); + expect( + noteTitle('${ref.imageMarkdown}\n\n${ref.imageMarkdown}'), + '2 images', + ); + expect(noteTitle(file), 'Raport.pdf'); + expect(noteTitle('$file\n${ref.imageMarkdown}'), '2 attachments'); expect(noteTitle(''), ''); }); test('the backlog export uses disk paths and parses back', () { - final exported = NotesMarkdown.export([note('t\n${ref.markdown}')]); + final pdf = AttachmentRef.forFile('n1', 'a.pdf'); + final text = 't\n${ref.imageMarkdown}\n${pdf.fileMarkdown('a.pdf')}'; + final exported = NotesMarkdown.export([note(text)]); expect(exported, contains('![]($kBacklogImagesRoot/${ref.relativePath})')); - expect(NotesMarkdown.parse(exported).single.text, 't\n${ref.markdown}'); + expect( + exported, + contains('[a.pdf]($kBacklogImagesRoot/${pdf.relativePath})'), + ); + expect(NotesMarkdown.parse(exported).single.text, text); }); test('the page talks to the wrapper that served it', () { diff --git a/test/ui_images/file_embed_test.dart b/test/ui_images/file_embed_test.dart new file mode 100644 index 0000000..f2e8b10 --- /dev/null +++ b/test/ui_images/file_embed_test.dart @@ -0,0 +1,127 @@ +import 'package:flutter/material.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:todo/attachments/image_store_api.dart'; +import 'package:todo/attachments/image_store_scope.dart'; +import 'package:todo/images/attachment_ref.dart'; +import 'package:todo/ui/images/attachment_strip.dart'; +import 'package:todo/ui/images/file_embed.dart'; +import 'package:todo/ui/images/image_embed.dart'; + +import '../fake_image_store.dart'; + +void main() { + final pdf = AttachmentRef.forFile('n1', 'a.pdf'); + final attachment = Attachment(ref: pdf, name: 'Raport.pdf', isImage: false); + late FakeImageStore store; + late List<(ImageSource, String)> opened; + String? failure; + + setUp(() { + store = FakeImageStore(); + opened = []; + failure = null; + }); + + Future pump(WidgetTester tester, Widget child) async { + await tester.pumpWidget( + ImageStoreScope( + store: store, + child: MaterialApp(home: Scaffold(body: child)), + ), + ); + await tester.pump(); + } + + FileEmbed embed({bool compact = false}) => FileEmbed( + attachment: attachment, + compact: compact, + opening: (source, name) async { + opened.add((source, name)); + return failure; + }, + ); + + testWidgets('a row shows the name and size and opens on tap', (tester) async { + await pump(tester, embed()); + expect(find.text('Raport.pdf'), findsOneWidget); + expect(find.text('2.0 KB'), findsOneWidget); + expect(find.byIcon(Icons.picture_as_pdf_outlined), findsOneWidget); + await tester.tap(find.text('Raport.pdf')); + await tester.pump(); + expect(opened.single.$2, 'Raport.pdf'); + expect(opened.single.$1, isA()); + }); + + testWidgets('a failed open is reported', (tester) async { + failure = 'No app on this phone opens Raport.pdf'; + await pump(tester, embed()); + await tester.tap(find.text('Raport.pdf')); + await tester.pump(); + expect(find.text(failure!), findsOneWidget); + }); + + testWidgets('a file not here yet says so and does not open', (tester) async { + store.missing.add(pdf); + await pump(tester, embed()); + expect(find.text(kImageNotDownloaded), findsOneWidget); + await tester.tap(find.text('Raport.pdf')); + await tester.pump(); + expect(opened, isEmpty); + }); + + testWidgets('a queued file carries the upload badge, in both shapes', ( + tester, + ) async { + store.pending.add(pdf); + await pump(tester, embed()); + expect(find.byTooltip(kImageWaitingToUpload), findsOneWidget); + await pump(tester, embed(compact: true)); + expect(find.byTooltip(kImageWaitingToUpload), findsOneWidget); + }); + + testWidgets('the size is asked again when the store changes', (tester) async { + store.missing.add(pdf); + await pump(tester, embed()); + expect(find.text(kImageNotDownloaded), findsOneWidget); + store + ..missing.clear() + ..changed(); + await tester.pump(); + await tester.pump(); + expect(find.text('2.0 KB'), findsOneWidget); + }); + + testWidgets('the strip shows a compact tile for a file', (tester) async { + final image = AttachmentRef.create('n1'); + await pump( + tester, + AttachmentStrip( + text: '${image.imageMarkdown}\n${pdf.fileMarkdown('Raport.pdf')}', + ), + ); + expect(find.byType(ImageEmbed), findsOneWidget); + final tile = tester.widget(find.byType(FileEmbed)); + expect(tile.compact, isTrue); + expect(find.text('2.0 KB'), findsNothing, reason: 'no room for a size'); + }); + + test('formatBytes picks a readable unit', () { + expect(formatBytes(0), '0 B'); + expect(formatBytes(512), '512 B'); + expect(formatBytes(1536), '1.5 KB'); + expect(formatBytes(10 * 1024), '10 KB'); + expect(formatBytes(34 * 1024 * 1024), '34 MB'); + expect(formatBytes(2254857830), '2.1 GB'); + expect(formatBytes(5000 * 1024 * 1024 * 1024 * 1024), '5000 TB'); + }); + + test('fileIconFor groups types by kind', () { + expect(fileIconFor('pdf'), Icons.picture_as_pdf_outlined); + expect(fileIconFor('mkv'), Icons.movie_outlined); + expect(fileIconFor('flac'), Icons.audiotrack_outlined); + expect(fileIconFor('md'), Icons.description_outlined); + expect(fileIconFor('7z'), Icons.folder_zip_outlined); + expect(fileIconFor('psd'), Icons.image_outlined); + expect(fileIconFor('bin'), Icons.insert_drive_file_outlined); + }); +} diff --git a/test/ui_images/file_opening_test.dart b/test/ui_images/file_opening_test.dart new file mode 100644 index 0000000..9c3b9b2 --- /dev/null +++ b/test/ui_images/file_opening_test.dart @@ -0,0 +1,88 @@ +import 'dart:convert'; + +import 'package:flutter/services.dart'; +import 'package:flutter_test/flutter_test.dart'; +import 'package:plugin_platform_interface/plugin_platform_interface.dart'; +import 'package:todo/attachments/image_store_api.dart'; +import 'package:todo/ui/images/file_opening.dart'; +import 'package:url_launcher_platform_interface/link.dart'; +import 'package:url_launcher_platform_interface/url_launcher_platform_interface.dart'; + +/// Records the URL instead of opening a browser. +class _FakeUrlLauncher extends UrlLauncherPlatform + with MockPlatformInterfaceMixin { + final List<(String, String?)> launched = []; + bool succeed = true; + + @override + final LinkDelegate? linkDelegate = null; + + @override + Future launchUrl(String url, LaunchOptions options) async { + launched.add((url, options.webOnlyWindowName)); + return succeed; + } +} + +void main() { + TestWidgetsFlutterBinding.ensureInitialized(); + const channel = MethodChannel('open_file'); + final calls = >[]; + + void answer(int type, String message) { + TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, (call) async { + calls.add(call.arguments as Map); + return jsonEncode({'type': type, 'message': message}); + }); + } + + setUp(calls.clear); + tearDown( + () => TestDefaultBinaryMessengerBinding.instance.defaultBinaryMessenger + .setMockMethodCallHandler(channel, null), + ); + + test('a local file goes to the app that handles its type', () async { + answer(0, 'done'); + expect( + await openAttachedFile(const LocalImageSource('/c/a.pdf'), 'a.pdf'), + isNull, + ); + expect(calls.single['file_path'], '/c/a.pdf'); + }); + + test('no handler or a failure says so', () async { + answer(-1, 'No APP found'); + expect( + await openAttachedFile(const LocalImageSource('/c/a.xyz'), 'a.xyz'), + 'No app on this phone opens a.xyz', + ); + answer(-4, 'File opened incorrectly'); + expect( + await openAttachedFile(const LocalImageSource('/c/a.pdf'), 'a.pdf'), + 'Could not open a.pdf: File opened incorrectly', + ); + }); + + test('the desktop opens the wrapper copy, named, in a new tab', () async { + final launcher = _FakeUrlLauncher(); + UrlLauncherPlatform.instance = launcher; + const source = NetworkImageSource( + 'http://localhost:8730/images/n/u.pdf?r=2', + ); + expect(await openAttachedFile(source, 'Raport v2.pdf'), isNull); + final (url, window) = launcher.launched.single; + expect(Uri.parse(url).queryParameters, {'r': '2', 'name': 'Raport v2.pdf'}); + expect(window, '_blank'); + launcher.succeed = false; + expect(await openAttachedFile(source, 'a.pdf'), 'Could not open a.pdf'); + }); + + test('a file not on this device cannot be opened', () async { + expect( + await openAttachedFile(const MissingImageSource(), 'a.pdf'), + 'a.pdf is not on this device yet', + ); + }); +} diff --git a/test/ui_images/image_attach_test.dart b/test/ui_images/image_attach_test.dart index 7a003b4..c625c79 100644 --- a/test/ui_images/image_attach_test.dart +++ b/test/ui_images/image_attach_test.dart @@ -71,7 +71,7 @@ void main() { expect(asked, 2); expect(store.attached.values.map((b) => b.first), [1, 2]); expect(inserted.single, [ - for (final ref in store.attached.keys) ref.markdown, + for (final ref in store.attached.keys) ref.imageMarkdown, ]); await tester.pump(); expect(find.byType(SnackBar), findsNothing); diff --git a/test/ui_images/image_embed_test.dart b/test/ui_images/image_embed_test.dart index c2688ed..e8fbec2 100644 --- a/test/ui_images/image_embed_test.dart +++ b/test/ui_images/image_embed_test.dart @@ -6,7 +6,7 @@ import 'package:image/image.dart' as img; import 'package:todo/attachments/image_store_scope.dart'; import 'package:todo/images/attachment_ref.dart'; import 'package:todo/ui/images/image_embed.dart'; -import 'package:todo/ui/images/image_strip.dart'; +import 'package:todo/ui/images/attachment_strip.dart'; import 'package:todo/ui/images/image_viewer_screen.dart'; import '../fake_image_store.dart'; @@ -85,13 +85,13 @@ void main() { final other = AttachmentRef.create('n2'); await pump( tester, - ImageStrip(text: 'x\n${ref.markdown}\n${other.markdown}'), + AttachmentStrip(text: 'x\n${ref.imageMarkdown}\n${other.imageMarkdown}'), ); expect(find.byType(ImageEmbed), findsNWidgets(2)); }); testWidgets('the strip is empty without images', (tester) async { - await pump(tester, const ImageStrip(text: 'no images')); + await pump(tester, const AttachmentStrip(text: 'no images')); expect(find.byType(ImageEmbed), findsNothing); }); }