Skip to content

Commit 34f9380

Browse files
authored
devtools_shared: Use the file package for a file system abstraction for testing (#9872)
1 parent 048218d commit 34f9380

11 files changed

Lines changed: 212 additions & 100 deletions

File tree

packages/devtools_app_shared/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ resolution: workspace
1515
dependencies:
1616
collection: ^1.15.0
1717
dds_service_extensions: ^2.0.0
18-
devtools_shared: ^13.0.0
18+
devtools_shared: ^14.0.0-wip
1919
dtd: ^4.0.0
2020
flutter:
2121
sdk: flutter

packages/devtools_extensions/pubspec.yaml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ executables:
1818

1919
dependencies:
2020
args: ^2.4.2
21-
devtools_shared: ^13.0.0
21+
devtools_shared: ^14.0.0-wip
2222
devtools_app_shared: ^0.5.1
2323
flutter:
2424
sdk: flutter

packages/devtools_shared/CHANGELOG.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,8 +3,33 @@ Copyright 2025 The Flutter Authors
33
Use of this source code is governed by a BSD-style license that can be
44
found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
55
-->
6+
# 14.0.0-wip
7+
8+
* **Breaking changes**: `LocalFileSystem`, an extension which provided some handy
9+
helpers, has been refactored into an extension on the `file` package's
10+
`FileSystem` abstraction. In detail:
11+
* `fileSystem` is a new top-level constant which represents the local (real)
12+
file system.
13+
* `LocalFileSystem.devToolsDir()` is now a static getter,
14+
`FileSystemExtension.devToolsDir`.
15+
* `LocalFileSystem.maybeMoveLegacyDevToolsStore()` is now an instance method,
16+
`FileSystemExtension.maybeMoveLegacyDevToolsStore()`.
17+
* `LocalFileSystem.devToolsStoreLocation()` is now a static getter,
18+
`FileSystemExtension.devToolsStoreLocation`.
19+
* `LocalFileSystem.ensureDevToolsDirectory()` is now private.
20+
* `LocalFileSystem.devToolsFileFromPath()` is now an instance method,
21+
`FileSystemExtension.devToolsFileFromPath()`.
22+
* `LocalFileSystem.devToolsFileAsJson()` is now an instance method,
23+
`FileSystemExtension.devToolsFileAsJson()`.
24+
* `LocalFileSystem.flutterStoreExists()` is now an instance getter,
25+
`FileSystemExtension.flutterStoreExists`.
26+
* `IOPersistentProperties.new` accepts a new optional `FileSystem fs`
27+
argument.
28+
* Update `LocalFileSystem` and `IOPersistentProperties` to use `package:file`
29+
instead of `dart:io` to allow mocking the file system.
630

731
# 13.0.2
32+
833
* Validate the `devtoolsOptionsUri` query parameter in the extension enabled
934
state handler so it must be a `file:` URI named `devtools_options.yaml`,
1035
preventing arbitrary file writes by the DevTools server process.

packages/devtools_shared/lib/src/server/devtools_store.dart

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,11 @@ enum DevToolsStoreKeys {
2121
/// Provides access to the local DevTools store (~/.flutter-devtools/.devtools).
2222
class DevToolsUsage {
2323
DevToolsUsage() {
24-
LocalFileSystem.maybeMoveLegacyDevToolsStore();
24+
// TODO(srawlins): Accept a FileSystem parameter during tests.
25+
fileSystem.maybeMoveLegacyDevToolsStore();
2526
properties = IOPersistentProperties(
2627
storeName,
27-
documentDirPath: LocalFileSystem.devToolsDir(),
28+
documentDirPath: FileSystemExtension.devToolsDir,
2829
);
2930
_removeLegacyKeys();
3031
}

packages/devtools_shared/lib/src/server/file_system.dart

Lines changed: 66 additions & 55 deletions
Original file line numberDiff line numberDiff line change
@@ -3,108 +3,117 @@
33
// found in the LICENSE file or at https://developers.google.com/open-source/licenses/bsd.
44

55
import 'dart:convert';
6-
import 'dart:io';
6+
import 'dart:io' as io show Platform, File;
77

8+
import 'package:file/file.dart';
9+
import 'package:file/local.dart';
810
import 'package:path/path.dart' as path;
911

1012
import 'devtools_store.dart';
1113

12-
/// A namespace local file system utlities.
13-
extension LocalFileSystem on Never {
14-
static String _userHomeDir() {
15-
final envKey = Platform.operatingSystem == 'windows' ? 'APPDATA' : 'HOME';
16-
return Platform.environment[envKey] ?? '.';
14+
/// The real, local file system, which can be avoided in tests.
15+
const FileSystem fileSystem = LocalFileSystem();
16+
17+
extension FileSystemExtension on FileSystem {
18+
static String get _userHomeDir {
19+
final envKey = io.Platform.operatingSystem == 'windows'
20+
? 'APPDATA'
21+
: 'HOME';
22+
return io.Platform.environment[envKey] ?? '.';
1723
}
1824

19-
/// Returns the path to the DevTools storage directory.
20-
static String devToolsDir() {
21-
return path.join(_userHomeDir(), '.flutter-devtools');
25+
/// The path to the DevTools storage directory.
26+
static String get devToolsDir {
27+
return path.join(_userHomeDir, '.flutter-devtools');
2228
}
2329

24-
/// Moves the .devtools file to ~/.flutter-devtools/.devtools if the .devtools
25-
/// file exists in the user's home directory.
26-
static void maybeMoveLegacyDevToolsStore() {
27-
final file = File(path.join(_userHomeDir(), DevToolsUsage.storeName));
28-
if (file.existsSync()) {
29-
ensureDevToolsDirectory();
30-
file.copySync(devToolsStoreLocation());
31-
file.deleteSync();
30+
/// Moves the `.devtools` file to `~/.flutter-devtools/.devtools` if the
31+
/// `.devtools` file exists in the user's home directory.
32+
void maybeMoveLegacyDevToolsStore() {
33+
final storeFile = file(path.join(_userHomeDir, DevToolsUsage.storeName));
34+
if (storeFile.existsSync()) {
35+
_ensureDevToolsDirectory();
36+
storeFile.copySync(devToolsStoreLocation);
37+
storeFile.deleteSync();
3238
}
3339
}
3440

35-
static String devToolsStoreLocation() {
36-
return path.join(devToolsDir(), DevToolsUsage.storeName);
41+
static String get devToolsStoreLocation {
42+
return path.join(devToolsDir, DevToolsUsage.storeName);
3743
}
3844

39-
/// Creates the ~/.flutter-devtools directory if it does not already exist.
40-
static void ensureDevToolsDirectory() {
41-
Directory(devToolsDir()).createSync();
45+
/// Creates the `~/.flutter-devtools` directory if it does not already exist.
46+
void _ensureDevToolsDirectory() {
47+
directory(devToolsDir).createSync();
4248
}
4349

4450
/// Returns a DevTools file from the given path.
4551
///
4652
/// Only files within ~/.flutter-devtools/ can be accessed.
47-
static File? devToolsFileFromPath(String pathFromDevToolsDir) {
48-
if (pathFromDevToolsDir.contains('..') ||
49-
path.isAbsolute(pathFromDevToolsDir)) {
53+
File? devToolsFileFromPath(String relativePath) {
54+
if (relativePath.contains('..') || path.isAbsolute(relativePath)) {
5055
// The passed in path should not be able to walk up the directory tree
51-
// outside of the ~/.flutter-devtools/ directory. It must also not be an
52-
// absolute path: path.join() discards the base directory when its second
53-
// argument is absolute, which would otherwise allow reading an arbitrary
54-
// file on disk (e.g. an absolute path to a credentials .json file).
56+
// outside of the `~/.flutter-devtools/` directory. It must also not be an
57+
// absolute path: `path.join()` discards the base directory when its
58+
// second argument is absolute, which would otherwise allow reading an
59+
// arbitrary file on disk (e.g. an absolute path to a credentials `.json`
60+
// file).
5561
return null;
5662
}
5763

58-
ensureDevToolsDirectory();
59-
final devToolsDirPath = devToolsDir();
60-
final file = File(path.join(devToolsDirPath, pathFromDevToolsDir));
64+
_ensureDevToolsDirectory();
65+
final targetFile = file(path.join(devToolsDir, relativePath));
6166
// Defense in depth: ensure the resolved path is actually contained within
6267
// the DevTools directory.
63-
if (!path.isWithin(devToolsDirPath, file.path)) {
64-
return null;
65-
}
66-
if (!file.existsSync()) {
67-
return null;
68-
}
69-
return file;
68+
if (!path.isWithin(devToolsDir, targetFile.path)) return null;
69+
if (!targetFile.existsSync()) return null;
70+
return targetFile;
7071
}
7172

72-
/// Returns a DevTools file from the given path as encoded json.
73+
/// Returns a DevTools file from the given path as encoded JSON.
7374
///
74-
/// Only files within ~/.flutter-devtools/ can be accessed.
75-
static String? devToolsFileAsJson(String pathFromDevToolsDir) {
76-
final file = devToolsFileFromPath(pathFromDevToolsDir);
77-
if (file == null) return null;
75+
/// Only files within `~/.flutter-devtools/` can be accessed.
76+
String? devToolsFileAsJson(String relativePath) {
77+
final targetFile = devToolsFileFromPath(relativePath);
78+
if (targetFile == null) return null;
7879

79-
final fileName = path.basename(file.path);
80+
final fileName = path.basename(targetFile.path);
8081
if (!fileName.endsWith('.json')) return null;
8182

82-
final content = file.readAsStringSync();
83+
final content = targetFile.readAsStringSync();
8384
final json = jsonDecode(content) as Map;
84-
json['lastModifiedTime'] = file.lastModifiedSync().toString();
85+
json['lastModifiedTime'] = targetFile.lastModifiedSync().toString();
8586
return jsonEncode(json);
8687
}
8788

8889
/// Whether the flutter store file exists.
89-
static bool flutterStoreExists() {
90-
final flutterStore = File(path.join(_userHomeDir(), '.flutter'));
90+
bool get flutterStoreExists {
91+
final flutterStore = file(path.join(_userHomeDir, '.flutter'));
9192
return flutterStore.existsSync();
9293
}
9394
}
9495

9596
class IOPersistentProperties {
96-
IOPersistentProperties(this.name, {String? documentDirPath}) {
97+
IOPersistentProperties(
98+
this.name, {
99+
String? documentDirPath,
100+
FileSystem fs = fileSystem,
101+
}) : _fs = fs {
97102
final fileName = name.replaceAll(' ', '_');
98-
documentDirPath ??= LocalFileSystem._userHomeDir();
99-
_file = File(path.join(documentDirPath, fileName));
103+
documentDirPath ??= FileSystemExtension._userHomeDir;
104+
_file = _fs.file(path.join(documentDirPath, fileName));
100105
if (!_file.existsSync()) {
101106
_file.createSync(recursive: true);
102107
}
103108
syncSettings();
104109
}
105110

106-
IOPersistentProperties.fromFile(File file) : name = path.basename(file.path) {
107-
_file = file;
111+
// TODO(srawlins): This is unused in any devtools code. Even in tests. Either
112+
// test it, if it is needed by users, or remove it.
113+
IOPersistentProperties.fromFile(io.File file)
114+
: name = path.basename(file.path),
115+
_fs = file is File ? file.fileSystem : fileSystem {
116+
_file = file is File ? file : _fs.file(file.path);
108117
if (!_file.existsSync()) {
109118
_file.createSync(recursive: true);
110119
}
@@ -113,7 +122,9 @@ class IOPersistentProperties {
113122

114123
final String name;
115124

116-
late File _file;
125+
final FileSystem _fs;
126+
127+
late final File _file;
117128

118129
late Map<String, Object?> _map;
119130

packages/devtools_shared/lib/src/server/handlers/_app_size.dart

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ extension _AppSizeHandler on Never {
1919
if (missingRequiredParams != null) return missingRequiredParams;
2020

2121
final filePath = queryParams[AppSizeApi.baseAppSizeFilePropertyName]!;
22-
final fileJson = LocalFileSystem.devToolsFileAsJson(filePath);
22+
final fileJson = fileSystem.devToolsFileAsJson(filePath);
2323
if (fileJson == null) {
2424
return api.badRequest('No JSON file available at $filePath.');
2525
}
@@ -39,7 +39,7 @@ extension _AppSizeHandler on Never {
3939
if (missingRequiredParams != null) return missingRequiredParams;
4040

4141
final filePath = queryParams[AppSizeApi.testAppSizeFilePropertyName]!;
42-
final fileJson = LocalFileSystem.devToolsFileAsJson(filePath);
42+
final fileJson = fileSystem.devToolsFileAsJson(filePath);
4343
if (fileJson == null) {
4444
return api.badRequest('No JSON file available at $filePath.');
4545
}

packages/devtools_shared/lib/src/server/server_api.dart

Lines changed: 4 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,9 +72,7 @@ class ServerApi {
7272
// ----- Flutter Tool GA store. -----
7373
case apiGetFlutterGAClientId:
7474
return _encodeResponse(
75-
LocalFileSystem.flutterStoreExists()
76-
? _flutterStore.flutterClientId
77-
: '',
75+
fileSystem.flutterStoreExists ? _flutterStore.flutterClientId : '',
7876
api: api,
7977
);
8078

@@ -223,16 +221,16 @@ class ServerApi {
223221
: null;
224222
}
225223

226-
/// Accessing DevTools store file e.g., ~/.flutter-devtools/.devtools
224+
/// Accessing DevTools store file e.g., `~/.flutter-devtools/.devtools`.
227225
static final _devToolsStore = DevToolsUsage();
228226

229-
/// Accessing Flutter store file e.g., ~/.flutter
227+
/// Accessing Flutter store file e.g., `~/.flutter`.
230228
static final _flutterStore = FlutterStore();
231229

232230
static DevToolsUsage get devToolsPreferences => _devToolsStore;
233231

234232
/// Provides read and write access to DevTools options files
235-
/// (e.g. path/to/app/root/devtools_options.yaml).
233+
/// (e.g. `path/to/app/root/devtools_options.yaml`).
236234
static final _devToolsOptions = DevToolsOptions();
237235

238236
/// Logs a page view in the DevTools server.

packages/devtools_shared/pubspec.yaml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@
44
name: devtools_shared
55
description: Package of shared Dart structures between devtools_app, dds, and other tools.
66

7-
version: 13.0.2
7+
version: 14.0.0-wip
88

99
repository: https://github.com/flutter/devtools/tree/master/packages/devtools_shared
1010

@@ -18,6 +18,7 @@ dependencies:
1818
collection: ^1.15.0
1919
dtd: ^4.0.0
2020
extension_discovery: ^2.1.0
21+
file: ^7.0.0
2122
meta: ^1.9.1
2223
path: ^1.8.0
2324
shelf: ^1.1.0

0 commit comments

Comments
 (0)