Deeply encode objects in encode() (#23)

tvolkert · web-flow · commit 2c08bc1280d3 · 2017-02-07T14:50:16.000-08:00
This changes `encode()` to deeply encode the specified object rather than performing a shallow encode. Previously, it relied on `JSONEncoder` to do the work of walking the object graph and calling `toEncodable` whenever it found a non-JSON type. Now, callers can call `encode()` on the top-level object and get back a deeply JSON-typed object that can be converted to JSON without the need for a `toEncodable` callback. This will be used during replay, when we'll compare an invocation to the list of recorded invocations, each of which exists as a JSON-type map. Since we now control the encoding process entirely ourselves, this also changes `encode()` to return `Future<dynamic>` rather than `dynamic`, which in turn allows both `InvocationEvent.serialize()` and `ResultReference.serializedValue` to return futures as well. Doing so allows for more technical correctness when serializing result references. Part of flutter#11
diff --git a/lib/src/backends/record_replay/encoding.dart b/lib/src/backends/record_replay/encoding.dart
@@ -2,6 +2,7 @@
 // for details. All rights reserved. Use of this source code is governed by a
 // BSD-style license that can be found in the LICENSE file.
 
+import 'dart:async';
 import 'dart:convert';
 
 import 'package:file/file.dart';
@@ -18,6 +19,8 @@ import 'recording_random_access_file.dart';
 import 'result_reference.dart';
 
 /// Encodes an object into a JSON-ready representation.
+///
+/// It is legal for an encoder to return a future value.
 typedef dynamic _Encoder(dynamic object);
 
 /// Known encoders. Types not covered here will be encoded using
@@ -32,9 +35,8 @@ const Map<TypeMatcher<dynamic>, _Encoder> _kEncoders =
   const TypeMatcher<bool>(): _encodeRaw,
   const TypeMatcher<String>(): _encodeRaw,
   const TypeMatcher<Null>(): _encodeRaw,
-  const TypeMatcher<List<dynamic>>(): _encodeRaw,
-  const TypeMatcher<Map<dynamic, dynamic>>(): _encodeMap,
-  const TypeMatcher<Iterable<dynamic>>(): _encodeIterable,
+  const TypeMatcher<Iterable<dynamic>>(): encodeIterable,
+  const TypeMatcher<Map<dynamic, dynamic>>(): encodeMap,
   const TypeMatcher<Symbol>(): getSymbolName,
   const TypeMatcher<DateTime>(): _encodeDateTime,
   const TypeMatcher<Uri>(): _encodeUri,
@@ -54,22 +56,20 @@ const Map<TypeMatcher<dynamic>, _Encoder> _kEncoders =
   const TypeMatcher<FileSystemEvent>(): _encodeFileSystemEvent,
 };
 
-/// Encodes [object] into a JSON-ready representation.
-///
-/// This function is intended to be used as the `toEncodable` argument to the
-/// `JsonEncoder` constructors.
+/// Encodes an arbitrary [object] into a JSON-ready representation (a number,
+/// boolean, string, null, list, or map).
 ///
-/// See also:
-///   - [JsonEncoder.withIndent]
-dynamic encode(dynamic object) {
+/// Returns a future that completes with a value suitable for conversion into
+/// JSON using [JsonEncoder] without the need for a `toEncodable` argument.
+Future<dynamic> encode(dynamic object) async {
   _Encoder encoder = _encodeDefault;
   for (TypeMatcher<dynamic> matcher in _kEncoders.keys) {
     if (matcher.matches(object)) {
       encoder = _kEncoders[matcher];
       break;
     }
   }
-  return encoder(object);
+  return await encoder(object);
 }
 
 /// Default encoder (used for types not covered in [_kEncoders]).
@@ -78,20 +78,28 @@ String _encodeDefault(dynamic object) => object.runtimeType.toString();
 /// Pass-through encoder.
 dynamic _encodeRaw(dynamic object) => object;
 
-List<T> _encodeIterable<T>(Iterable<T> iterable) => iterable.toList();
+/// Encodes the specified [iterable] into a JSON-ready list of encoded items.
+///
+/// Returns a future that completes with a list suitable for conversion into
+/// JSON using [JsonEncoder] without the need for a `toEncodable` argument.
+Future<List<dynamic>> encodeIterable(Iterable<dynamic> iterable) async {
+  List<dynamic> encoded = <dynamic>[];
+  for (dynamic element in iterable) {
+    encoded.add(await encode(element));
+  }
+  return encoded;
+}
 
-/// Encodes the map keys, and passes the values through.
+/// Encodes the specified [map] into a JSON-ready map of encoded key/value
+/// pairs.
 ///
-/// As [JsonEncoder] encodes an object graph, it will repeatedly call
-/// `toEncodable` to encode unknown types, so any values in a map that need
-/// special encoding will already be handled by `JsonEncoder`. However, the
-/// encoder won't try to encode map *keys* by default, which is why we encode
-/// them here.
-Map<String, T> _encodeMap<T>(Map<dynamic, T> map) {
-  Map<String, T> encoded = <String, T>{};
+/// Returns a future that completes with a map suitable for conversion into
+/// JSON using [JsonEncoder] without the need for a `toEncodable` argument.
+Future<Map<String, dynamic>> encodeMap(Map<dynamic, dynamic> map) async {
+  Map<String, dynamic> encoded = <String, dynamic>{};
   for (dynamic key in map.keys) {
-    String encodedKey = encode(key);
-    encoded[encodedKey] = map[key];
+    String encodedKey = await encode(key);
+    encoded[encodedKey] = await encode(map[key]);
   }
   return encoded;
 }
@@ -100,15 +108,17 @@ int _encodeDateTime(DateTime dateTime) => dateTime.millisecondsSinceEpoch;
 
 String _encodeUri(Uri uri) => uri.toString();
 
-Map<String, String> _encodePathContext(p.Context context) => <String, String>{
-      'style': context.style.name,
-      'cwd': context.current,
-    };
+Map<String, String> _encodePathContext(p.Context context) {
+  return <String, String>{
+    'style': context.style.name,
+    'cwd': context.current,
+  };
+}
 
-dynamic _encodeResultReference(ResultReference<dynamic> reference) =>
+Future<dynamic> _encodeResultReference(ResultReference<dynamic> reference) =>
     reference.serializedValue;
 
-Map<String, dynamic> _encodeEvent(LiveInvocationEvent<dynamic> event) =>
+Future<Map<String, dynamic>> _encodeEvent(LiveInvocationEvent<dynamic> event) =>
     event.serialize();
 
 String _encodeFileSystem(FileSystem fs) => kFileSystemEncodedValue;
@@ -148,10 +158,10 @@ String _encodeFileMode(FileMode fileMode) {
 }
 
 Map<String, dynamic> _encodeFileStat(FileStat stat) => <String, dynamic>{
-      'changed': stat.changed,
-      'modified': stat.modified,
-      'accessed': stat.accessed,
-      'type': stat.type,
+      'changed': _encodeDateTime(stat.changed),
+      'modified': _encodeDateTime(stat.modified),
+      'accessed': _encodeDateTime(stat.accessed),
+      'type': _encodeFileSystemEntityType(stat.type),
       'mode': stat.mode,
       'size': stat.size,
       'modeString': stat.modeString(),
diff --git a/lib/src/backends/record_replay/events.dart b/lib/src/backends/record_replay/events.dart
@@ -4,6 +4,8 @@
 
 import 'dart:async';
 
+import 'common.dart';
+import 'encoding.dart';
 import 'recording.dart';
 import 'result_reference.dart';
 
@@ -101,11 +103,13 @@ abstract class LiveInvocationEvent<T> implements InvocationEvent<T> {
   }
 
   /// Returns this event as a JSON-serializable object.
-  Map<String, dynamic> serialize() => <String, dynamic>{
-        'object': object,
-        'result': _result,
-        'timestamp': timestamp,
-      };
+  Future<Map<String, dynamic>> serialize() async {
+    return <String, dynamic>{
+      'object': await encode(object),
+      'result': await encode(_result),
+      'timestamp': timestamp,
+    };
+  }
 
   @override
   String toString() => serialize().toString();
@@ -122,10 +126,12 @@ class LivePropertyGetEvent<T> extends LiveInvocationEvent<T>
   final Symbol property;
 
   @override
-  Map<String, dynamic> serialize() => <String, dynamic>{
-        'type': 'get',
-        'property': property,
-      }..addAll(super.serialize());
+  Future<Map<String, dynamic>> serialize() async {
+    return <String, dynamic>{
+      'type': 'get',
+      'property': getSymbolName(property),
+    }..addAll(await super.serialize());
+  }
 }
 
 /// A [PropertySetEvent] that's in the process of being recorded.
@@ -142,11 +148,13 @@ class LivePropertySetEvent<T> extends LiveInvocationEvent<Null>
   final T value;
 
   @override
-  Map<String, dynamic> serialize() => <String, dynamic>{
-        'type': 'set',
-        'property': property,
-        'value': value,
-      }..addAll(super.serialize());
+  Future<Map<String, dynamic>> serialize() async {
+    return <String, dynamic>{
+      'type': 'set',
+      'property': getSymbolName(property),
+      'value': await encode(value),
+    }..addAll(await super.serialize());
+  }
 }
 
 /// A [MethodEvent] that's in the process of being recorded.
@@ -177,10 +185,12 @@ class LiveMethodEvent<T> extends LiveInvocationEvent<T>
   final Map<Symbol, dynamic> namedArguments;
 
   @override
-  Map<String, dynamic> serialize() => <String, dynamic>{
-        'type': 'invoke',
-        'method': method,
-        'positionalArguments': positionalArguments,
-        'namedArguments': namedArguments,
-      }..addAll(super.serialize());
+  Future<Map<String, dynamic>> serialize() async {
+    return <String, dynamic>{
+      'type': 'invoke',
+      'method': getSymbolName(method),
+      'positionalArguments': await encodeIterable(positionalArguments),
+      'namedArguments': await encodeMap(namedArguments),
+    }..addAll(await super.serialize());
+  }
 }
diff --git a/lib/src/backends/record_replay/mutable_recording.dart b/lib/src/backends/record_replay/mutable_recording.dart
@@ -46,7 +46,8 @@ class MutableRecording implements LiveRecording {
             .timeout(awaitPendingResults, onTimeout: () {});
       }
       Directory dir = destination;
-      String json = new JsonEncoder.withIndent('  ', encode).convert(_events);
+      List<dynamic> encodedEvents = await encode(_events);
+      String json = new JsonEncoder.withIndent('  ').convert(encodedEvents);
       String filename = dir.fileSystem.path.join(dir.path, kManifestName);
       await dir.fileSystem.file(filename).writeAsString(json, flush: true);
     } finally {
diff --git a/lib/src/backends/record_replay/recording_file.dart b/lib/src/backends/record_replay/recording_file.dart
@@ -209,7 +209,7 @@ class _BlobReference<T> extends ResultReference<T> {
   T get recordedValue => _value;
 
   @override
-  dynamic get serializedValue => '!${_file.basename}';
+  Future<String> get serializedValue async => '!${_file.basename}';
 }
 
 /// A [FutureReference] that serializes its value data to a separate file.
@@ -235,14 +235,15 @@ class _BlobFutureReference<T> extends FutureReference<T> {
   }
 
   @override
-  dynamic get serializedValue => '!${_file.basename}';
+  Future<String> get serializedValue async => '!${_file.basename}';
 }
 
 /// A [StreamReference] that serializes its value data to a separate file.
 class _BlobStreamReference<T> extends StreamReference<T> {
   final File _file;
   final _BlobDataStreamWriter<T> _writer;
   IOSink _sink;
+  Future<dynamic> _pendingFlush;
 
   _BlobStreamReference({
     @required File file,
@@ -251,16 +252,20 @@ class _BlobStreamReference<T> extends StreamReference<T> {
   })
       : _file = file,
         _writer = writer,
-        super(stream) {
-    _file.createSync();
-  }
+        _sink = file.openWrite(),
+        super(stream);
 
   @override
   void onData(T event) {
-    if (_sink == null) {
-      _sink = _file.openWrite();
+    if (_pendingFlush == null) {
+      _writer(_sink, event);
+    } else {
+      // It's illegal to write to an IOSink while a flush is pending.
+      // https://github.com/dart-lang/sdk/issues/28635
+      _pendingFlush.whenComplete(() {
+        _writer(_sink, event);
+      });
     }
-    _writer(_sink, event);
   }
 
   @override
@@ -271,7 +276,20 @@ class _BlobStreamReference<T> extends StreamReference<T> {
   }
 
   @override
-  dynamic get serializedValue => '!${_file.basename}';
+  Future<String> get serializedValue async {
+    if (_pendingFlush != null) {
+      await _pendingFlush;
+    } else {
+      _pendingFlush = _sink.flush();
+      try {
+        await _pendingFlush;
+      } finally {
+        _pendingFlush = null;
+      }
+    }
+
+    return '!${_file.basename}';
+  }
 
   // TODO(tvolkert): remove `.then()` once Dart 1.22 is in stable
   @override
diff --git a/lib/src/backends/record_replay/result_reference.dart b/lib/src/backends/record_replay/result_reference.dart
@@ -49,7 +49,7 @@ abstract class ResultReference<T> {
   /// actually a byte array that was read from a file). In this case, the
   /// method can return a `ResultReference` to the list, and it will have a
   /// hook into the serialization process.
-  dynamic get serializedValue => encode(recordedValue);
+  Future<dynamic> get serializedValue => encode(recordedValue);
 
   /// A [Future] that completes when [value] has completed.
   ///
diff --git a/test/recording_test.dart b/test/recording_test.dart
@@ -10,6 +10,8 @@ import 'package:file/memory.dart';
 import 'package:file/record_replay.dart';
 import 'package:file/testing.dart';
 import 'package:file/src/backends/record_replay/common.dart';
+import 'package:file/src/backends/record_replay/encoding.dart';
+import 'package:file/src/backends/record_replay/events.dart';
 import 'package:file/src/backends/record_replay/mutable_recording.dart';
 import 'package:file/src/backends/record_replay/recording_proxy_mixin.dart';
 import 'package:path/path.dart' as p;
@@ -182,6 +184,67 @@ void main() {
         });
       });
     });
+
+    group('Encode', () {
+      test('performsDeepEncoding', () async {
+        rc.basicProperty = 'foo';
+        rc.basicProperty;
+        rc.basicMethod('bar', namedArg: 'baz');
+        await rc.futureProperty;
+        await rc.futureMethod('qux', namedArg: 'quz');
+        await rc.streamMethod('quux', namedArg: 'quuz').drain();
+        List<Map<String, dynamic>> manifest = await encode(recording.events);
+        expect(manifest[0], <String, dynamic>{
+          'type': 'set',
+          'property': 'basicProperty=',
+          'value': 'foo',
+          'object': '_RecordingClass',
+          'result': isNull,
+          'timestamp': 10,
+        });
+        expect(manifest[1], <String, dynamic>{
+          'type': 'get',
+          'property': 'basicProperty',
+          'object': '_RecordingClass',
+          'result': 'foo',
+          'timestamp': 11,
+        });
+        expect(manifest[2], <String, dynamic>{
+          'type': 'invoke',
+          'method': 'basicMethod',
+          'positionalArguments': <String>['bar'],
+          'namedArguments': <String, String>{'namedArg': 'baz'},
+          'object': '_RecordingClass',
+          'result': 'bar.baz',
+          'timestamp': 12,
+        });
+        expect(manifest[3], <String, dynamic>{
+          'type': 'get',
+          'property': 'futureProperty',
+          'object': '_RecordingClass',
+          'result': 'future.foo',
+          'timestamp': 13,
+        });
+        expect(manifest[4], <String, dynamic>{
+          'type': 'invoke',
+          'method': 'futureMethod',
+          'positionalArguments': <String>['qux'],
+          'namedArguments': <String, String>{'namedArg': 'quz'},
+          'object': '_RecordingClass',
+          'result': 'future.qux.quz',
+          'timestamp': 14,
+        });
+        expect(manifest[5], <String, dynamic>{
+          'type': 'invoke',
+          'method': 'streamMethod',
+          'positionalArguments': <String>['quux'],
+          'namedArguments': <String, String>{'namedArg': 'quuz'},
+          'object': '_RecordingClass',
+          'result': <String>['stream', 'quux', 'quuz'],
+          'timestamp': 15,
+        });
+      });
+    });
   });
 
   group('RecordingFileSystem', () {
