Add back write caching. (#4001)

* Add back write caching.

* Address review comments.

* Address review comments.

Author: davidmorgan

_test_common/lib/runner_asset_writer_spy.dart

@@ -23,7 +23,4 @@ class RunnerAssetWriterSpy extends AssetWriterSpy implements RunnerAssetWriter {
 
   @override
   Future<void> deleteDirectory(AssetId id) => _delegate.deleteDirectory(id);
-
-  @override
-  Future<void> completeBuild() async {}
 }

_test_common/lib/test_environment.dart

@@ -70,8 +70,12 @@ class TestBuildEnvironment implements BuildEnvironment {
   BuildEnvironment copyWith({
     void Function(LogRecord)? onLogOverride,
     RunnerAssetWriter? writer,
+    AssetReader? reader,
   }) => TestBuildEnvironment(
-    readerWriter: (writer as TestReaderWriter?) ?? _readerWriter,
+    readerWriter:
+        (writer as TestReaderWriter?) ??
+        (reader as TestReaderWriter?) ??
+        _readerWriter,
     throwOnPrompt: throwOnPrompt,
   );
 

build/lib/src/state/filesystem_cache.dart

@@ -6,6 +6,7 @@ import 'dart:async';
 import 'dart:convert';
 import 'dart:typed_data';
 
+import '../../build.dart';
 import '../asset/id.dart';
 import 'lru_cache.dart';
 
@@ -16,6 +17,9 @@ abstract interface class FilesystemCache {
   /// Clears all [ids] from all caches.
   void invalidate(Iterable<AssetId> ids);
 
+  /// Flushes pending writes and deletes.
+  void flush();
+
   /// Whether [id] exists.
   ///
   /// Returns a cached result if available, or caches and returns `ifAbsent()`.
@@ -26,6 +30,17 @@ abstract interface class FilesystemCache {
   /// Returns a cached result if available, or caches and returns `ifAbsent()`.
   Uint8List readAsBytes(AssetId id, {required Uint8List Function() ifAbsent});
 
+  /// Writes [contents] to [id].
+  ///
+  /// [writer] is a function that does the actual write. If this cache does
+  /// write caching, it is not called until [flush], and might not be called at
+  /// all if another write to the same asset happens first.
+  void writeAsBytes(
+    AssetId id,
+    List<int> contents, {
+    required void Function() writer,
+  });
+
   /// Reads [id] as a `String`.
   ///
   /// Returns a cached result if available, or caches and returns `ifAbsent()`.
@@ -34,6 +49,25 @@ abstract interface class FilesystemCache {
     Encoding encoding = utf8,
     required Uint8List Function() ifAbsent,
   });
+
+  /// Writes [contents] to [id].
+  ///
+  /// [writer] is a function that does the actual write. If this cache does
+  /// write caching, it is not called until [flush], and might not be called at
+  /// all if another write to the same asset happens first.
+  void writeAsString(
+    AssetId id,
+    String contents, {
+    Encoding encoding = utf8,
+    required void Function() writer,
+  });
+
+  /// Deletes [id].
+  ///
+  /// [deleter] is a function that does the actual delete. If this cache does
+  /// write caching, it is not called until [flush], and might not be called at
+  /// all if another write to the same asset happens first.
+  void delete(AssetId id, {required void Function() deleter});
 }
 
 /// [FilesystemCache] that always reads from the underlying source.
@@ -43,19 +77,40 @@ class PassthroughFilesystemCache implements FilesystemCache {
   @override
   Future<void> invalidate(Iterable<AssetId> ids) async {}
 
+  @override
+  void flush() {}
+
   @override
   bool exists(AssetId id, {required bool Function() ifAbsent}) => ifAbsent();
 
   @override
   Uint8List readAsBytes(AssetId id, {required Uint8List Function() ifAbsent}) =>
       ifAbsent();
 
+  @override
+  void writeAsBytes(
+    AssetId id,
+    List<int> contents, {
+    required void Function() writer,
+  }) => writer();
+
   @override
   String readAsString(
     AssetId id, {
     Encoding encoding = utf8,
     required Uint8List Function() ifAbsent,
   }) => encoding.decode(ifAbsent());
+
+  @override
+  void writeAsString(
+    AssetId id,
+    String contents, {
+    Encoding encoding = utf8,
+    required void Function() writer,
+  }) => writer();
+
+  @override
+  void delete(AssetId id, {required void Function() deleter}) => deleter();
 }
 
 /// [FilesystemCache] that stores data in memory.
@@ -83,8 +138,13 @@ class InMemoryFilesystemCache implements FilesystemCache {
     (value) => value.length,
   );
 
+  final _pendingWrites = <AssetId, _PendingWrite>{};
+
   @override
   Future<void> invalidate(Iterable<AssetId> ids) async {
+    if (_pendingWrites.isNotEmpty) {
+      throw StateError("Can't invalidate while there are pending writes.");
+    }
     for (var id in ids) {
       _existsCache.remove(id);
       _bytesContentCache.remove(id);
@@ -93,11 +153,30 @@ class InMemoryFilesystemCache implements FilesystemCache {
   }
 
   @override
-  bool exists(AssetId id, {required bool Function() ifAbsent}) =>
-      _existsCache.putIfAbsent(id, ifAbsent);
+  void flush() {
+    for (final write in _pendingWrites.values) {
+      write.writer();
+    }
+    _pendingWrites.clear();
+  }
+
+  @override
+  bool exists(AssetId id, {required bool Function() ifAbsent}) {
+    final maybePendingWrite = _pendingWrites[id];
+    if (maybePendingWrite != null) {
+      return !maybePendingWrite.isDelete;
+    }
+    return _existsCache.putIfAbsent(id, ifAbsent);
+  }
 
   @override
   Uint8List readAsBytes(AssetId id, {required Uint8List Function() ifAbsent}) {
+    final maybePendingWrite = _pendingWrites[id];
+    if (maybePendingWrite != null) {
+      // Throws if it's a delete; callers should check [exists] before reading.
+      return maybePendingWrite.bytes!;
+    }
+
     final maybeResult = _bytesContentCache[id];
     if (maybeResult != null) return maybeResult;
 
@@ -106,27 +185,87 @@ class InMemoryFilesystemCache implements FilesystemCache {
     return result;
   }
 
+  @override
+  void writeAsBytes(
+    AssetId id,
+    List<int> contents, {
+    required void Function() writer,
+  }) {
+    _stringContentCache.remove(id);
+    _writeBytes(id, contents, writer: writer);
+  }
+
+  void _writeBytes(
+    AssetId id,
+    List<int> contents, {
+    required void Function() writer,
+  }) {
+    final uint8ListContents =
+        contents is Uint8List ? contents : Uint8List.fromList(contents);
+    _bytesContentCache[id] = uint8ListContents;
+    _existsCache[id] = true;
+    _pendingWrites[id] = _PendingWrite(
+      writer: writer,
+      bytes: uint8ListContents,
+    );
+  }
+
   @override
   String readAsString(
     AssetId id, {
     Encoding encoding = utf8,
     required Uint8List Function() ifAbsent,
   }) {
+    // Encodings other than utf8 do not use `_stringContentCache`. Read as
+    // bytes then convert, instead.
     if (encoding != utf8) {
       final bytes = readAsBytes(id, ifAbsent: ifAbsent);
       return encoding.decode(bytes);
     }
 
+    // Check `_stringContentCache` first to use it as a cache for conversion of
+    // bytes from _pendingWrites.
     final maybeResult = _stringContentCache[id];
     if (maybeResult != null) return maybeResult;
 
-    var bytes = _bytesContentCache[id];
-    if (bytes == null) {
-      bytes = ifAbsent();
-      _bytesContentCache[id] = bytes;
-    }
+    final bytes = readAsBytes(id, ifAbsent: ifAbsent);
     final result = utf8.decode(bytes);
     _stringContentCache[id] = result;
     return result;
   }
+
+  @override
+  void writeAsString(
+    AssetId id,
+    String contents, {
+    Encoding encoding = utf8,
+    required void Function() writer,
+  }) {
+    // Encodings other than utf8 do not use `_stringContentCache`.
+    if (encoding == utf8) {
+      _stringContentCache[id] = contents;
+    } else {
+      _stringContentCache.remove(id);
+    }
+    final bytes = encoding.encode(contents);
+    _writeBytes(id, bytes, writer: writer);
+  }
+
+  @override
+  void delete(AssetId id, {required void Function() deleter}) {
+    _stringContentCache.remove(id);
+    _bytesContentCache.remove(id);
+    _existsCache[id] = false;
+    _pendingWrites[id] = _PendingWrite(writer: deleter);
+  }
+}
+
+/// The data that will be written on flush; used for reads before flush.
+class _PendingWrite {
+  final void Function() writer;
+  final Uint8List? bytes;
+
+  _PendingWrite({required this.writer, this.bytes});
+
+  bool get isDelete => bytes == null;
 }