Refactor temporary file handling for efficiency (#1865)
diff --git a/bin/dartdoc.dart b/bin/dartdoc.dart
index 903b331..2b70ffa 100644
--- a/bin/dartdoc.dart
+++ b/bin/dartdoc.dart
@@ -11,6 +11,7 @@
import 'package:dartdoc/dartdoc.dart';
import 'package:dartdoc/src/html/html_generator.dart';
import 'package:dartdoc/src/logging.dart';
+import 'package:dartdoc/src/tool_runner.dart';
import 'package:stack_trace/stack_trace.dart';
class DartdocProgramOptionContext extends DartdocGeneratorOptionContext
@@ -94,8 +95,9 @@
}
}, when: config.asyncStackTraces);
} finally {
- // Clear out any cached tool snapshots.
+ // Clear out any cached tool snapshots and temporary directories.
SnapshotCache.instance.dispose();
+ ToolTempFileTracker.instance.dispose();
}
}
diff --git a/lib/src/dartdoc_options.dart b/lib/src/dartdoc_options.dart
index 2f409fb..15d7a47 100644
--- a/lib/src/dartdoc_options.dart
+++ b/lib/src/dartdoc_options.dart
@@ -21,6 +21,7 @@
import 'package:args/args.dart';
import 'package:dartdoc/dartdoc.dart';
import 'package:dartdoc/src/io_utils.dart';
+import 'package:dartdoc/src/tool_runner.dart';
import 'package:dartdoc/src/tuple.dart';
import 'package:path/path.dart' as pathLib;
import 'package:yaml/yaml.dart';
@@ -243,11 +244,12 @@
return snapshots[toolPath];
}
- void dispose() {
- if (snapshotCache != null && snapshotCache.existsSync()) {
- snapshotCache.deleteSync(recursive: true);
- }
+ Future<void> dispose() {
_instance = null;
+ if (snapshotCache != null && snapshotCache.existsSync()) {
+ return snapshotCache.delete(recursive: true);
+ }
+ return null;
}
}
@@ -288,12 +290,16 @@
class ToolConfiguration {
final Map<String, ToolDefinition> tools;
+ ToolRunner _runner;
+ ToolRunner get runner => _runner ??= ToolRunner(this);
+
ToolConfiguration._(this.tools);
static ToolConfiguration get empty {
return new ToolConfiguration._({});
}
+ // TODO(jcollins-g): consider caching these.
static ToolConfiguration fromYamlMap(
YamlMap yamlMap, pathLib.Context pathContext) {
var newToolDefinitions = <String, ToolDefinition>{};
diff --git a/lib/src/markdown_processor.dart b/lib/src/markdown_processor.dart
index 56b4116..9e00368 100644
--- a/lib/src/markdown_processor.dart
+++ b/lib/src/markdown_processor.dart
@@ -15,7 +15,6 @@
import 'package:dartdoc/src/warnings.dart';
import 'package:html/parser.dart' show parse;
import 'package:markdown/markdown.dart' as md;
-import 'package:quiver/iterables.dart' as quiverIterables;
const validHtmlTags = const [
"a",
diff --git a/lib/src/model.dart b/lib/src/model.dart
index 55262c1..323e58a 100644
--- a/lib/src/model.dart
+++ b/lib/src/model.dart
@@ -57,7 +57,6 @@
import 'package:dartdoc/src/model_utils.dart';
import 'package:dartdoc/src/package_meta.dart' show PackageMeta, FileContents;
import 'package:dartdoc/src/special_elements.dart';
-import 'package:dartdoc/src/tool_runner.dart';
import 'package:dartdoc/src/tuple.dart';
import 'package:dartdoc/src/utils.dart';
import 'package:dartdoc/src/warnings.dart';
@@ -4121,9 +4120,6 @@
/// 2018-09-18T21:15+00:00
Future<String> _evaluateTools(String rawDocs) async {
if (config.allowTools) {
- var runner = new ToolRunner(config.tools, (String message) async {
- warn(PackageWarning.toolError, message: message);
- });
int invocationIndex = 0;
return await _replaceAllMappedAsync(rawDocs, basicToolRegExp,
(basicMatch) async {
@@ -4138,7 +4134,8 @@
// Count the number of invocations of tools in this dartdoc block,
// so that tools can differentiate different blocks from each other.
invocationIndex++;
- return await runner.run(args,
+ return await config.tools.runner.run(args,
+ (String message) async => warn(PackageWarning.toolError, message: message),
content: basicMatch[2],
environment: {
'SOURCE_LINE': lineAndColumn?.item1?.toString(),
@@ -4153,7 +4150,7 @@
'ELEMENT_NAME': fullyQualifiedNameWithoutLibrary,
'INVOCATION_INDEX': invocationIndex.toString(),
}..removeWhere((key, value) => value == null));
- }).whenComplete(runner.dispose);
+ });
} else {
return rawDocs;
}
@@ -4783,7 +4780,7 @@
// specialClasses handler so when we find them, they get added.
specialClasses = new SpecialClasses();
// Go through docs of every ModelElement in package to pre-build the macros
- // index.
+ // index. Uses toList() in order to get all the precaching on the stack.
List<Future> precacheFutures = precacheLocalDocs().toList();
for (Future f in precacheFutures) await f;
_localDocumentationBuilt = true;
diff --git a/lib/src/tool_runner.dart b/lib/src/tool_runner.dart
index c31a7ae..58f63c6 100644
--- a/lib/src/tool_runner.dart
+++ b/lib/src/tool_runner.dart
@@ -19,6 +19,35 @@
/// limiting both parallelization and the number of open temporary files.
final MultiFutureTracker _toolTracker = new MultiFutureTracker(4);
+/// Can be called when the ToolRunner is no longer needed.
+///
+/// This will remove any temporary files created by the tool runner.
+class ToolTempFileTracker {
+ final Directory temporaryDirectory;
+
+ ToolTempFileTracker._() : temporaryDirectory = Directory.systemTemp.createTempSync('dartdoc_tools_');
+
+ static ToolTempFileTracker _instance;
+ static ToolTempFileTracker get instance => _instance ??= ToolTempFileTracker._();
+
+ int _temporaryFileCount = 0;
+ Future<File> createTemporaryFile() async {
+ _temporaryFileCount++;
+ File tempFile = new File(pathLib.join(
+ temporaryDirectory.absolute.path,
+ 'input_$_temporaryFileCount'));
+ await tempFile.create(recursive: true);
+ return tempFile;
+ }
+
+ /// Call once no more files are to be created.
+ Future<void> dispose() async {
+ if (temporaryDirectory.existsSync()) {
+ return temporaryDirectory.delete(recursive: true);
+ }
+ }
+}
+
/// A helper class for running external tools.
class ToolRunner {
/// Creates a new ToolRunner.
@@ -26,53 +55,13 @@
/// Takes a [toolConfiguration] that describes all of the available tools.
/// An optional `errorCallback` will be called for each error message
/// generated by the tool.
- ToolRunner(this.toolConfiguration, [this._errorCallback]);
+ ToolRunner(this.toolConfiguration);
final ToolConfiguration toolConfiguration;
- final ToolErrorCallback _errorCallback;
- int _temporaryFileCount = 0;
-
- Future<Directory> _temporaryDirectory;
- Future<Directory> get temporaryDirectory {
- if (_temporaryDirectory == null) {
- _temporaryDirectory = Directory.systemTemp.createTemp('dartdoc_tools_');
- }
- return _temporaryDirectory;
- }
-
- void _error(String message) {
- if (_errorCallback != null) {
- _errorCallback(message);
- }
- }
-
- Future<File> _createTemporaryFile() async {
- _temporaryFileCount++;
- File tempFile = new File(pathLib.join(
- (await temporaryDirectory).absolute.path,
- 'input_$_temporaryFileCount'));
- await tempFile.create(recursive: true);
- return tempFile;
- }
-
- /// Must be called when the ToolRunner is no longer needed. Ideally, this is
- /// called in the finally section of a try/finally.
- ///
- /// This will remove any temporary files created by the tool runner.
- void dispose() {
- if (_temporaryDirectory != null) disposeAsync(_temporaryDirectory);
- }
-
- /// Avoid blocking on I/O for cleanups.
- static Future<void> disposeAsync(Future<Directory> temporaryDirectory) async {
- Directory tempDir = await temporaryDirectory;
- if (await tempDir.exists()) {
- return tempDir.delete(recursive: true);
- }
- }
void _runSetup(
- String name, ToolDefinition tool, Map<String, String> environment) async {
+ String name, ToolDefinition tool, Map<String, String> environment,
+ ToolErrorCallback toolErrorCallback) async {
bool isDartSetup = ToolDefinition.isDartExecutable(tool.setupCommand[0]);
var args = tool.setupCommand.toList();
String commandPath;
@@ -82,18 +71,19 @@
} else {
commandPath = args.removeAt(0);
}
- await _runProcess(name, '', commandPath, args, environment);
+ await _runProcess(name, '', commandPath, args, environment, toolErrorCallback);
tool.setupComplete = true;
}
Future<String> _runProcess(String name, String content, String commandPath,
- List<String> args, Map<String, String> environment) async {
+ List<String> args, Map<String, String> environment,
+ ToolErrorCallback toolErrorCallback) async {
String commandString() => ([commandPath] + args).join(' ');
try {
ProcessResult result =
await Process.run(commandPath, args, environment: environment);
if (result.exitCode != 0) {
- _error('Tool "$name" returned non-zero exit code '
+ toolErrorCallback('Tool "$name" returned non-zero exit code '
'(${result.exitCode}) when run as '
'"${commandString()}" from ${Directory.current}\n'
'Input to $name was:\n'
@@ -104,7 +94,7 @@
return result.stdout;
}
} on ProcessException catch (exception) {
- _error('Failed to run tool "$name" as '
+ toolErrorCallback('Failed to run tool "$name" as '
'"${commandString()}": $exception\n'
'Input to $name was:\n'
'$content');
@@ -118,18 +108,18 @@
///
/// The [args] must not be null, and it must have at least one member (the name
/// of the tool).
- Future<String> run(List<String> args,
+ Future<String> run(List<String> args, ToolErrorCallback toolErrorCallback,
{String content, Map<String, String> environment}) async {
Future runner;
// Prevent too many tools from running simultaneously.
await _toolTracker.addFutureFromClosure(() {
- runner = _run(args, content: content, environment: environment);
+ runner = _run(args, toolErrorCallback, content: content, environment: environment);
return runner;
});
return runner;
}
- Future<String> _run(List<String> args,
+ Future<String> _run(List<String> args, ToolErrorCallback toolErrorCallback,
{String content, Map<String, String> environment}) async {
assert(args != null);
assert(args.isNotEmpty);
@@ -137,7 +127,7 @@
environment ??= <String, String>{};
var tool = args.removeAt(0);
if (!toolConfiguration.tools.containsKey(tool)) {
- _error('Unable to find definition for tool "$tool" in tool map. '
+ toolErrorCallback('Unable to find definition for tool "$tool" in tool map. '
'Did you add it to dartdoc_options.yaml?');
return '';
}
@@ -152,7 +142,7 @@
// file before running the tool synchronously.
// Write the content to a temp file.
- var tmpFile = await _createTemporaryFile();
+ var tmpFile = await ToolTempFileTracker.instance.createTemporaryFile();
await tmpFile.writeAsString(content);
// Substitute the temp filename for the "$INPUT" token, and all of the other
@@ -188,7 +178,7 @@
}
if (toolDefinition.setupCommand != null && !toolDefinition.setupComplete)
- await _runSetup(tool, toolDefinition, envWithInput);
+ await _runSetup(tool, toolDefinition, envWithInput, toolErrorCallback);
argsWithInput = toolArgs + argsWithInput;
var commandPath;
@@ -201,13 +191,14 @@
} else {
commandPath = argsWithInput.removeAt(0);
}
+
if (callCompleter != null) {
return _runProcess(
- tool, content, commandPath, argsWithInput, envWithInput)
+ tool, content, commandPath, argsWithInput, envWithInput, toolErrorCallback)
.whenComplete(callCompleter);
} else {
return _runProcess(
- tool, content, commandPath, argsWithInput, envWithInput);
+ tool, content, commandPath, argsWithInput, envWithInput, toolErrorCallback);
}
}
}
diff --git a/test/tool_runner_test.dart b/test/tool_runner_test.dart
index 361d3e2..5c9600d 100644
--- a/test/tool_runner_test.dart
+++ b/test/tool_runner_test.dart
@@ -20,6 +20,8 @@
File setupFile;
ToolRunner runner;
+ ToolTempFileTracker tracker;
+ ToolErrorCallback errorCallback;
final errors = <String>[];
setUpAll(() async {
@@ -81,11 +83,12 @@
'missing': new ToolDefinition(['/a/missing/executable'], null, "missing"),
});
- runner = new ToolRunner(toolMap, (String message) => errors.add(message));
+ runner = new ToolRunner(toolMap);
+ errorCallback = (String message) => errors.add(message);
});
tearDownAll(() {
tempDir?.deleteSync(recursive: true);
- runner?.dispose();
+ tracker?.dispose();
SnapshotCache.instance.dispose();
setupFile = null;
tempDir = null;
@@ -100,6 +103,7 @@
test('can invoke a Dart tool, and second run is a snapshot.', () async {
var result = await runner.run(
['drill', r'--file=$INPUT'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isEmpty);
@@ -109,6 +113,7 @@
expect(setupFile.existsSync(), isFalse);
result = await runner.run(
['drill', r'--file=$INPUT'],
+ errorCallback,
content: 'TEST INPUT 2',
);
expect(errors, isEmpty);
@@ -120,6 +125,7 @@
test('can invoke a Dart tool', () async {
var result = await runner.run(
['drill', r'--file=$INPUT'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isEmpty);
@@ -131,6 +137,7 @@
test('can invoke a non-Dart tool', () async {
String result = await runner.run(
['non_dart', '--version'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isEmpty);
@@ -139,6 +146,7 @@
test('can invoke a pre-snapshotted tool', () async {
var result = await runner.run(
['snapshot_drill', r'--file=$INPUT'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isEmpty);
@@ -148,6 +156,7 @@
test('can invoke a tool with a setup action', () async {
var result = await runner.run(
['setup_drill', r'--file=$INPUT'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isEmpty);
@@ -158,6 +167,7 @@
test('fails if tool not in tool map', () async {
String result = await runner.run(
['hammer', r'--file=$INPUT'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isNotEmpty);
@@ -168,6 +178,7 @@
test('fails if tool returns non-zero status', () async {
String result = await runner.run(
['drill', r'--file=/a/missing/file'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isNotEmpty);
@@ -177,6 +188,7 @@
test("fails if tool in tool map doesn't exist", () async {
String result = await runner.run(
['missing'],
+ errorCallback,
content: 'TEST INPUT',
);
expect(errors, isNotEmpty);
