diff --git a/pkgs/watcher/lib/src/directory_watcher/linux.dart b/pkgs/watcher/lib/src/directory_watcher/linux.dart index 0f587cec3..5fdda77a8 100644 --- a/pkgs/watcher/lib/src/directory_watcher/linux.dart +++ b/pkgs/watcher/lib/src/directory_watcher/linux.dart @@ -82,7 +82,7 @@ class _LinuxDirectoryWatcher }))); // Batch the inotify changes together so that we can dedup events. - var innerStream = _nativeEvents.stream.map(Event.new).batchEvents(); + var innerStream = _nativeEvents.stream.batchAndConvertEvents(); _listen(innerStream, _onBatch, onError: (Object error, StackTrace stackTrace) { // Guarantee that ready always completes. diff --git a/pkgs/watcher/lib/src/directory_watcher/mac_os.dart b/pkgs/watcher/lib/src/directory_watcher/mac_os.dart index 509cf6fe6..1d0db4419 100644 --- a/pkgs/watcher/lib/src/directory_watcher/mac_os.dart +++ b/pkgs/watcher/lib/src/directory_watcher/mac_os.dart @@ -8,6 +8,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; import '../directory_watcher.dart'; +import '../event.dart'; import '../path_set.dart'; import '../resubscribable.dart'; import '../utils.dart'; @@ -63,7 +64,7 @@ class _MacOSDirectoryWatcher /// /// This is separate from [_listSubscriptions] because this stream /// occasionally needs to be resubscribed in order to work around issue 14849. - StreamSubscription>? _watchSubscription; + StreamSubscription>? _watchSubscription; /// The subscription to the [Directory.list] call for the initial listing of /// the directory to determine its initial state. @@ -109,7 +110,7 @@ class _MacOSDirectoryWatcher } /// The callback that's run when [Directory.watch] emits a batch of events. - void _onBatch(List batch) { + void _onBatch(List batch) { // If we get a batch of events before we're ready to begin emitting events, // it's probable that it's a batch of pre-watcher events (see issue 14373). // Ignore those events and re-list the directory. @@ -132,8 +133,8 @@ class _MacOSDirectoryWatcher : [canonicalEvent]; for (var event in events) { - if (event is FileSystemCreateEvent) { - if (!event.isDirectory) { + switch (event.type) { + case EventType.createFile: // If we already know about the file, treat it like a modification. // This can happen if a file is copied on top of an existing one. // We'll see an ADD event for the latter file when from the user's @@ -143,34 +144,38 @@ class _MacOSDirectoryWatcher _emitEvent(type, path); _files.add(path); - continue; - } - - if (_files.containsDir(path)) continue; - - var stream = Directory(path) - .list(recursive: true) - .ignoring(); - var subscription = stream.listen((entity) { - if (entity is Directory) return; - if (_files.contains(path)) return; - - _emitEvent(ChangeType.ADD, entity.path); - _files.add(entity.path); - }, cancelOnError: true); - subscription.onDone(() { - _listSubscriptions.remove(subscription); - }); - subscription.onError(_emitError); - _listSubscriptions.add(subscription); - } else if (event is FileSystemModifyEvent) { - assert(!event.isDirectory); - _emitEvent(ChangeType.MODIFY, path); - } else { - assert(event is FileSystemDeleteEvent); - for (var removedPath in _files.remove(path)) { - _emitEvent(ChangeType.REMOVE, removedPath); - } + + case EventType.createDirectory: + if (_files.containsDir(path)) continue; + + var stream = Directory(path) + .list(recursive: true) + .ignoring(); + var subscription = stream.listen((entity) { + if (entity is Directory) return; + if (_files.contains(path)) return; + + _emitEvent(ChangeType.ADD, entity.path); + _files.add(entity.path); + }, cancelOnError: true); + subscription.onDone(() { + _listSubscriptions.remove(subscription); + }); + subscription.onError(_emitError); + _listSubscriptions.add(subscription); + + case EventType.modifyFile: + _emitEvent(ChangeType.MODIFY, path); + + case EventType.delete: + for (var removedPath in _files.remove(path)) { + _emitEvent(ChangeType.REMOVE, removedPath); + } + + case EventType.moveFile: + case EventType.moveDirectory: + case EventType.modifyDirectory: + assert(event.type.isNeverReceivedOnMacOS); } } }); @@ -178,122 +183,78 @@ class _MacOSDirectoryWatcher /// Sort all the events in a batch into sets based on their path. /// - /// A single input event may result in multiple events in the returned map; - /// for example, a MOVE event becomes a DELETE event for the source and a - /// CREATE event for the destination. + /// Events for `path` are discarded. /// - /// The returned events won't contain any [FileSystemMoveEvent]s, nor will it - /// contain any events relating to [path]. - Map> _sortEvents(List batch) { - var eventsForPaths = >{}; + /// Events under directories that are created are discarded. + Map> _sortEvents(List batch) { + var eventsForPaths = >{}; // FSEvents can report past events, including events on the root directory // such as it being created. We want to ignore these. If the directory is // really deleted, that's handled by [_onDone]. batch = batch.where((event) => event.path != path).toList(); - // Events within directories that already have events are superfluous; the - // directory's full contents will be examined anyway, so we ignore such - // events. Emitting them could cause useless or out-of-order events. - var directories = unionAll(batch.map((event) { - if (!event.isDirectory) return {}; - if (event is FileSystemMoveEvent) { - var destination = event.destination; - if (destination != null) { - return {event.path, destination}; - } - } - return {event.path}; + // Events within directories that already have create events are not needed + // as the directory's full content will be listed. + var createdDirectories = unionAll(batch.map((event) { + return event.type == EventType.createDirectory + ? {event.path} + : const {}; })); - bool isInModifiedDirectory(String path) => - directories.any((dir) => path != dir && p.isWithin(dir, path)); + bool isInCreatedDirectory(String path) => + createdDirectories.any((dir) => path != dir && p.isWithin(dir, path)); - void addEvent(String path, FileSystemEvent event) { - if (isInModifiedDirectory(path)) return; - eventsForPaths.putIfAbsent(path, () => {}).add(event); + void addEvent(String path, Event event) { + if (isInCreatedDirectory(path)) return; + eventsForPaths.putIfAbsent(path, () => {}).add(event); } for (var event in batch) { - // The Mac OS watcher doesn't emit move events. See issue 14806. - assert(event is! FileSystemMoveEvent); addEvent(event.path, event); } return eventsForPaths; } - /// Returns the canonical event from a batch of events on the same path, if - /// one exists. - /// - /// If [batch] doesn't contain any contradictory events (e.g. DELETE and - /// CREATE, or events with different values for `isDirectory`), this returns a - /// single event that describes what happened to the path in question. - /// - /// If [batch] does contain contradictory events, this returns `null` to - /// indicate that the state of the path on the filesystem should be checked to - /// determine what occurred. - FileSystemEvent? _canonicalEvent(Set batch) { - // An empty batch indicates that we've learned earlier that the batch is - // contradictory (e.g. because of a move). + /// Returns the canonical event from a batch of events on the same path, or + /// `null` to indicate that the filesystem should be checked. + Event? _canonicalEvent(Set batch) { + // If the batch is empty, return `null`. if (batch.isEmpty) return null; - var type = batch.first.type; - var isDir = batch.first.isDirectory; - var hadModifyEvent = false; - - for (var event in batch.skip(1)) { - // If one event reports that the file is a directory and another event - // doesn't, that's a contradiction. - if (isDir != event.isDirectory) return null; - - // Modify events don't contradict either CREATE or REMOVE events. We can - // safely assume the file was modified after a CREATE or before the - // REMOVE; otherwise there will also be a REMOVE or CREATE event - // (respectively) that will be contradictory. - if (event is FileSystemModifyEvent) { - hadModifyEvent = true; - continue; - } - assert(event is FileSystemCreateEvent || event is FileSystemDeleteEvent); - - // If we previously thought this was a MODIFY, we now consider it to be a - // CREATE or REMOVE event. This is safe for the same reason as above. - if (type == FileSystemEvent.modify) { - type = event.type; - continue; + // Resolve the event type for the batch. + var types = batch.map((e) => e.type).toSet(); + EventType type; + if (types.length == 1) { + // There's only one event. + type = types.single; + } else if (types.length == 2 && + types.contains(EventType.modifyFile) && + types.contains(EventType.createFile)) { + // Combine events of type [EventType.modifyFile] and + // [EventType.createFile] to one event. + if (_files.contains(batch.first.path)) { + // The file already existed: this can happen due to a create from + // before the watcher started being reported. + type = EventType.modifyFile; + } else { + type = EventType.createFile; } - - // A CREATE event contradicts a REMOVE event and vice versa. - assert(type == FileSystemEvent.create || type == FileSystemEvent.delete); - if (type != event.type) return null; + } else { + // There are incompatible event types, check the filesystem. + return null; } - // If we got a CREATE event for a file we already knew about, that comes - // from FSEvents reporting an add that happened prior to the watch - // beginning. If we also received a MODIFY event, we want to report that, - // but not the CREATE. - if (type == FileSystemEvent.create && - hadModifyEvent && - _files.contains(batch.first.path)) { - type = FileSystemEvent.modify; + // Issue 16003 means that a CREATE event for a directory can indicate + // that the directory was moved and then re-created. + // [_eventsBasedOnFileSystem] will handle this correctly by producing a + // DELETE event followed by a CREATE event if the directory exists. + if (type == EventType.createDirectory) { + return null; } - switch (type) { - case FileSystemEvent.create: - // Issue 16003 means that a CREATE event for a directory can indicate - // that the directory was moved and then re-created. - // [_eventsBasedOnFileSystem] will handle this correctly by producing a - // DELETE event followed by a CREATE event if the directory exists. - if (isDir) return null; - return FileSystemCreateEvent(batch.first.path, false); - case FileSystemEvent.delete: - return FileSystemDeleteEvent(batch.first.path, isDir); - case FileSystemEvent.modify: - return FileSystemModifyEvent(batch.first.path, isDir, false); - default: - throw StateError('unreachable'); - } + return batch.firstWhere((e) => e.type == type); } /// Returns one or more events that describe the change between the last known @@ -303,35 +264,35 @@ class _MacOSDirectoryWatcher /// to the user, unlike the batched events from [Directory.watch]. The /// returned list may be empty, indicating that no changes occurred to [path] /// (probably indicating that it was created and then immediately deleted). - List _eventsBasedOnFileSystem(String path) { + List _eventsBasedOnFileSystem(String path) { var fileExisted = _files.contains(path); var dirExisted = _files.containsDir(path); var fileExists = File(path).existsSync(); var dirExists = Directory(path).existsSync(); - var events = []; + var events = []; if (fileExisted) { if (fileExists) { - events.add(FileSystemModifyEvent(path, false, false)); + events.add(Event.modifyFile(path)); } else { - events.add(FileSystemDeleteEvent(path, false)); + events.add(Event.delete(path)); } } else if (dirExisted) { if (dirExists) { // If we got contradictory events for a directory that used to exist and // still exists, we need to rescan the whole thing in case it was // replaced with a different directory. - events.add(FileSystemDeleteEvent(path, true)); - events.add(FileSystemCreateEvent(path, true)); + events.add(Event.delete(path)); + events.add(Event.createDirectory(path)); } else { - events.add(FileSystemDeleteEvent(path, true)); + events.add(Event.delete(path)); } } if (!fileExisted && fileExists) { - events.add(FileSystemCreateEvent(path, false)); + events.add(Event.createFile(path)); } else if (!dirExisted && dirExists) { - events.add(FileSystemCreateEvent(path, true)); + events.add(Event.createDirectory(path)); } return events; @@ -362,7 +323,8 @@ class _MacOSDirectoryWatcher /// Start or restart the underlying [Directory.watch] stream. void _startWatch() { // Batch the FSEvent changes together so that we can dedup events. - var innerStream = Directory(path).watch(recursive: true).batchEvents(); + var innerStream = + Directory(path).watch(recursive: true).batchAndConvertEvents(); _watchSubscription = innerStream.listen(_onBatch, onError: _eventsController.addError, onDone: _onDone); } diff --git a/pkgs/watcher/lib/src/event.dart b/pkgs/watcher/lib/src/event.dart index 159106d64..1a7f92b57 100644 --- a/pkgs/watcher/lib/src/event.dart +++ b/pkgs/watcher/lib/src/event.dart @@ -13,7 +13,47 @@ import 'dart:io'; /// /// So, this extension type hides `isDirectory` and instead provides an /// [EventType] enum with the seven types of event actually used. -extension type Event(FileSystemEvent event) { +extension type Event._(FileSystemEvent event) { + /// Converts [event] to an [Event]. + /// + /// Returns `null` and asserts `false` if [event] is unexpected on this + /// platform. So, it will cause tests to fail but real code can continue + /// ignoring the event. + static Event? checkAndConvert(FileSystemEvent event) { + var result = Event._(event); + if (Platform.isMacOS) { + if (result.type.isNeverReceivedOnMacOS) { + assert(false); + return null; + } + } + return result; + } + + /// A create event for a file at [path]. + static Event createFile(String path) => + Event._(FileSystemCreateEvent(path, false)); + + /// A create event for a directory at [path]. + static Event createDirectory(String path) => + Event._(FileSystemCreateEvent(path, true)); + + /// A delete event for [path]. + /// + /// Delete events do not specify whether they are for files or directories. + static Event delete(String path) => Event._(FileSystemDeleteEvent( + path, + // `FileSystemDeleteEvent` just discards `isDirectory`. + false /* isDirectory */)); + + /// A modify event for the file at [path]. + static Event modifyFile(String path) => Event._(FileSystemModifyEvent( + path, + false /* isDirectory */, + // Don't set `contentChanged`, even pass through from the OS, as + // `package:watcher` never reads it. + false /* contentChanged */)); + /// See [FileSystemEvent.path]. String get path => event.path; @@ -56,4 +96,13 @@ enum EventType { modifyDirectory, moveFile, moveDirectory; + + bool get isNeverReceivedOnMacOS { + // See https://github.com/dart-lang/sdk/issues/14806. + if (this == moveFile || this == moveDirectory) { + return true; + } + if (this == modifyDirectory) return true; + return false; + } } diff --git a/pkgs/watcher/lib/src/file_watcher/native.dart b/pkgs/watcher/lib/src/file_watcher/native.dart index 2fcf45b34..ae961fe18 100644 --- a/pkgs/watcher/lib/src/file_watcher/native.dart +++ b/pkgs/watcher/lib/src/file_watcher/native.dart @@ -51,7 +51,7 @@ class _NativeFileWatcher implements FileWatcher, ManuallyClosedWatcher { var file = File(path); // Batch the events together so that we can dedupe them. - var stream = file.watch().map(Event.new).batchEvents(); + var stream = file.watch().batchAndConvertEvents(); if (Platform.isMacOS) { var existedAtStartupFuture = file.exists(); diff --git a/pkgs/watcher/lib/src/utils.dart b/pkgs/watcher/lib/src/utils.dart index e5ef54c66..b63a93bb6 100644 --- a/pkgs/watcher/lib/src/utils.dart +++ b/pkgs/watcher/lib/src/utils.dart @@ -6,6 +6,8 @@ import 'dart:async'; import 'dart:collection'; import 'dart:io'; +import 'event.dart'; + /// Returns `true` if [error] is a [FileSystemException] for a missing /// directory. bool isDirectoryNotFoundException(Object error) { @@ -20,7 +22,7 @@ bool isDirectoryNotFoundException(Object error) { Set unionAll(Iterable> sets) => sets.fold({}, (union, set) => union.union(set)); -extension BatchEvents on Stream { +extension BatchEvents on Stream { /// Batches all events that are sent at the same time. /// /// When multiple events are synchronously added to a stream controller, the @@ -28,11 +30,16 @@ extension BatchEvents on Stream { /// asynchronous firing of each event. In order to recreate the synchronous /// batches, this collates all the events that are received in "nearby" /// microtasks. - Stream> batchEvents() { - var batch = Queue(); - return StreamTransformer>.fromHandlers( + /// + /// Converts to [Event] using [Event.checkAndConvert], discarding events for + /// which it returns `null`. + Stream> batchAndConvertEvents() { + var batch = Queue(); + return StreamTransformer>.fromHandlers( handleData: (event, sink) { - batch.add(event); + var convertedEvent = Event.checkAndConvert(event); + if (convertedEvent == null) return; + batch.add(convertedEvent); // [Timer.run] schedules an event that runs after any microtasks that have // been scheduled.