Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 84687fe

Browse files
authored
Replace json_utils with a modern extension type, add tests. (#52769)
I started on a refactoring of flutter/flutter#147666, and was frustrated with the JSON decoding of `gn desc --format=json`, so I used a similar pattern from my micro-json library (https://pub.dev/packages/jsonut) and encapsulated it as an extension type. My favorite is the `JsonObject.map` function which replaces: ```dart /// Construct a RunTarget from a JSON map. factory RunTarget.fromJson(Map<String, Object> map) { final List<String> errors = <String>[]; final String name = stringOfJson(map, _nameKey, errors)!; final String id = stringOfJson(map, _idKey, errors)!; final String targetPlatform = stringOfJson(map, _targetPlatformKey, errors)!; if (errors.isNotEmpty) { throw FormatException('Failed to parse RunTarget: ${errors.join('\n')}'); } return RunTarget._(name, id, targetPlatform); } ``` ... with: ```dart /// Construct a RunTarget from a JSON map. factory RunTarget.fromJson(Map<String, Object> map) { return JsonObject(map).map((JsonObject json) => RunTarget._( json.string(_nameKey), json.string(_idKey), json.string(_targetPlatformKey), )); } ```
1 parent f9e33d9 commit 84687fe

File tree

7 files changed

+663
-160
lines changed

7 files changed

+663
-160
lines changed

tools/engine_tool/lib/src/gn_utils.dart

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -11,8 +11,8 @@ import 'package:process_runner/process_runner.dart';
1111

1212
import 'build_utils.dart';
1313
import 'environment.dart';
14-
import 'json_utils.dart';
1514
import 'proc_utils.dart';
15+
import 'typed_json.dart';
1616

1717
/// Canonicalized build targets start with this prefix.
1818
const String buildTargetPrefix = '//';
@@ -86,9 +86,8 @@ Future<Map<String, BuildTarget>> findTargets(
8686
environment.logger
8787
.fatal('gn desc output is malformed $label has no value.');
8888
}
89-
final Map<String, Object?> properties =
90-
targetEntry.value! as Map<String, Object?>;
91-
final String? typeString = getString(properties, 'type');
89+
final JsonObject properties = JsonObject(targetEntry.value! as Map<String, Object?>);
90+
final String? typeString = properties.stringOrNull('type');
9291
if (typeString == null) {
9392
environment.logger.fatal('gn desc is missing target type: $properties');
9493
}
@@ -97,7 +96,7 @@ Future<Map<String, BuildTarget>> findTargets(
9796
// Target is a type that we don't support.
9897
continue;
9998
}
100-
final bool testOnly = getBool(properties, 'testonly');
99+
final bool testOnly = properties.boolean('testonly');
101100
final List<String> outputs =
102101
await _runGnOutputs(buildDir.path, label, environment);
103102
File? executable;

tools/engine_tool/lib/src/json_utils.dart

Lines changed: 0 additions & 119 deletions
This file was deleted.

tools/engine_tool/lib/src/proc_utils.dart

Lines changed: 21 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -3,16 +3,15 @@
33
// found in the LICENSE file.
44

55
import 'dart:async';
6-
import 'dart:convert';
76
import 'dart:io';
87
import 'dart:math';
98

109
import 'package:path/path.dart' as p;
1110
import 'package:process_runner/process_runner.dart';
1211

1312
import 'environment.dart';
14-
import 'json_utils.dart';
1513
import 'logger.dart';
14+
import 'typed_json.dart';
1615
import 'worker_pool.dart';
1716

1817
/// Artifacts from an exited sub-process.
@@ -33,18 +32,18 @@ final class ProcessArtifacts {
3332

3433
/// Constructs an instance of ProcessArtifacts from serialized JSON text.
3534
factory ProcessArtifacts.fromJson(String serialized) {
36-
final Map<String, dynamic> artifact =
37-
jsonDecode(serialized) as Map<String, dynamic>;
38-
final List<String> errors = <String>[];
39-
final Directory cwd = Directory(stringOfJson(artifact, 'cwd', errors)!);
40-
final List<String> commandLine =
41-
stringListOfJson(artifact, 'commandLine', errors)!;
42-
final int exitCode = intOfJson(artifact, 'exitCode', errors)!;
43-
final String stdout = stringOfJson(artifact, 'stdout', errors)!;
44-
final String stderr = stringOfJson(artifact, 'stderr', errors)!;
45-
final int? pid = intOfJson(artifact, 'pid', errors);
46-
return ProcessArtifacts(cwd, commandLine, exitCode, stdout, stderr,
47-
pid: pid);
35+
final JsonObject artifact = JsonObject.parse(serialized);
36+
return artifact.map((JsonObject json) => ProcessArtifacts(
37+
Directory(json.string('cwd')),
38+
json.stringList('commandLine'),
39+
json.integer('exitCode'),
40+
json.string('stdout'),
41+
json.string('stderr'),
42+
pid: json.integer('pid'),
43+
), onError: (JsonObject source, JsonMapException e) {
44+
throw FormatException('Failed to parse ProcessArtifacts: $e', source.toPrettyString());
45+
},
46+
);
4847
}
4948

5049
/// Constructs an instance of ProcessArtifacts from a file containing JSON.
@@ -54,16 +53,14 @@ final class ProcessArtifacts {
5453

5554
/// Saves ProcessArtifacts into file.
5655
void save(File file) {
57-
final Map<String, Object> data = <String, Object>{};
58-
if (pid != null) {
59-
data['pid'] = pid!;
60-
}
61-
data['exitCode'] = exitCode;
62-
data['stdout'] = stdout;
63-
data['stderr'] = stderr;
64-
data['cwd'] = cwd.absolute.path;
65-
data['commandLine'] = commandLine;
66-
file.writeAsStringSync(jsonEncodePretty(data));
56+
file.writeAsStringSync(JsonObject(<String, Object?>{
57+
if (pid != null) 'pid': pid,
58+
'exitCode': exitCode,
59+
'stdout': stdout,
60+
'stderr': stderr,
61+
'cwd': cwd.absolute.path,
62+
'commandLine': commandLine,
63+
}).toPrettyString());
6764
}
6865

6966
/// Creates a temporary file and saves the artifacts into it.

tools/engine_tool/lib/src/run_utils.dart

Lines changed: 8 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ import 'dart:convert';
77
import 'package:process_runner/process_runner.dart';
88

99
import 'environment.dart';
10-
import 'json_utils.dart';
10+
import 'typed_json.dart';
1111

1212
const String _targetPlatformKey = 'targetPlatform';
1313
const String _nameKey = 'name';
@@ -17,16 +17,13 @@ const String _idKey = 'id';
1717
class RunTarget {
1818
/// Construct a RunTarget from a JSON map.
1919
factory RunTarget.fromJson(Map<String, Object> map) {
20-
final List<String> errors = <String>[];
21-
final String name = stringOfJson(map, _nameKey, errors)!;
22-
final String id = stringOfJson(map, _idKey, errors)!;
23-
final String targetPlatform =
24-
stringOfJson(map, _targetPlatformKey, errors)!;
25-
26-
if (errors.isNotEmpty) {
27-
throw FormatException('Failed to parse RunTarget: ${errors.join('\n')}');
28-
}
29-
return RunTarget._(name, id, targetPlatform);
20+
return JsonObject(map).map((JsonObject json) => RunTarget._(
21+
json.string(_nameKey),
22+
json.string(_idKey),
23+
json.string(_targetPlatformKey),
24+
), onError: (JsonObject source, JsonMapException e) {
25+
throw FormatException('Failed to parse RunTarget: $e', source.toPrettyString());
26+
});
3027
}
3128

3229
RunTarget._(this.name, this.id, this.targetPlatform);

0 commit comments

Comments
 (0)