Skip to content

Commit

Permalink
move valid input/output checks to the BuildStepImpl
Browse files Browse the repository at this point in the history
  • Loading branch information
@jakemac53
jakemac53 committed Feb 8, 2016
1 parent f3ad9a9 commit adebd06
Show file tree
Hide file tree
Showing 8 changed files with 104 additions and 77 deletions.
15 changes: 0 additions & 15 deletions lib/src/asset/file_based.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,29 +26,18 @@ class FileBasedAssetReader implements AssetReader {

@override
Future<bool> hasInput(AssetId id) async {
_checkInput(id);
return _fileFor(id, packageGraph).exists();
}

@override
Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) async {
_checkInput(id);

var file = await _fileFor(id, packageGraph);
if (!await file.exists()) {
throw new AssetNotFoundException(id);
}
return file.readAsString(encoding: encoding);
}

/// Checks that [id] is a valid input, and throws an [InvalidInputException]
/// if its not.
void _checkInput(AssetId id) {
if (id.package != packageGraph.root.name && !id.path.startsWith('lib/')) {
throw new InvalidInputException(id);
}
}

/// Searches for all [AssetId]s matching [inputSet]s.
Stream<AssetId> listAssetIds(Iterable<InputSet> inputSets) async* {
var seenAssets = new Set<AssetId>();
Expand Down Expand Up @@ -88,10 +77,6 @@ class FileBasedAssetWriter implements AssetWriter {

@override
Future writeAsString(Asset asset, {Encoding encoding: UTF8}) async {
if (asset.id.package != packageGraph.root.name) {
throw new InvalidOutputException(asset);
}

var file = _fileFor(asset.id, packageGraph);
await file.create(recursive: true);
await file.writeAsString(asset.stringContents, encoding: encoding);
Expand Down
35 changes: 30 additions & 5 deletions lib/src/builder/build_step_impl.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import 'package:logging/logging.dart';

import '../analyzer/resolver.dart';
import '../asset/asset.dart';
import '../asset/exceptions.dart';
import '../asset/id.dart';
import '../asset/reader.dart';
import '../asset/writer.dart';
Expand All @@ -20,6 +21,7 @@ import 'exceptions.dart';
/// A single step in the build processes. This represents a single input and
/// its expected and real outputs. It also handles tracking of dependencies.
class BuildStepImpl implements BuildStep {
/// Single `_resolvers` instance for all [BuildStepImpl]s
static code_transformers.Resolvers _resolvers =
new code_transformers.Resolvers(code_transformers.dartSdkDirectory);

Expand All @@ -36,6 +38,7 @@ class BuildStepImpl implements BuildStep {
_logger ??= new Logger(input.id.toString());
return _logger;
}

Logger _logger;

/// The actual outputs of this build step.
Expand All @@ -56,22 +59,27 @@ class BuildStepImpl implements BuildStep {
/// Used internally for writing files.
final AssetWriter _writer;

BuildStepImpl(
this.input, Iterable<AssetId> expectedOutputs, this._reader, this._writer)
/// The current root package, used for input/output validation.
final String _rootPackage;

BuildStepImpl(this.input, Iterable<AssetId> expectedOutputs, this._reader,
this._writer, this._rootPackage)
: expectedOutputs = new List.unmodifiable(expectedOutputs) {
/// The [input] is always a dependency.
_dependencies.add(input.id);
}

/// Checks if an [Asset] by [id] exists as an input for this [BuildStep].
Future<bool> hasInput(AssetId id) {
_checkInput(id);
_dependencies.add(id);
return _reader.hasInput(id);
}

/// Reads an [Asset] by [id] as a [String] using [encoding].
@override
Future<String> readAsString(AssetId id, {Encoding encoding: UTF8}) {
_checkInput(id);
_dependencies.add(id);
return _reader.readAsString(id, encoding: encoding);
}
Expand All @@ -83,9 +91,7 @@ class BuildStepImpl implements BuildStep {
/// [expectedOutputs].
@override
void writeAsString(Asset asset, {Encoding encoding: UTF8}) {
if (!expectedOutputs.any((id) => id == asset.id)) {
throw new UnexpectedOutputException(asset);
}
_checkOutput(asset);
_outputs.add(asset);
var done = _writer.writeAsString(asset, encoding: encoding);
_outputsCompleted = _outputsCompleted.then((_) => done);
Expand All @@ -101,4 +107,23 @@ class BuildStepImpl implements BuildStep {
await _outputsCompleted;
await _logger?.clearListeners();
}

/// Checks that [id] is a valid input, and throws an [InvalidInputException]
/// if its not.
void _checkInput(AssetId id) {
if (id.package != _rootPackage && !id.path.startsWith('lib/')) {
throw new InvalidInputException(id);
}
}

/// Checks that [asset] is a valid output, and throws an
/// [InvalidOutputException] or [UnexcpectedOutputException] if it's not.
void _checkOutput(Asset asset) {
if (asset.id.package != _rootPackage) {
throw new InvalidOutputException(asset);
}
if (!expectedOutputs.any((id) => id == asset.id)) {
throw new UnexpectedOutputException(asset);
}
}
}
27 changes: 20 additions & 7 deletions lib/src/generate/build.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import 'package:logging/logging.dart';
import 'package:path/path.dart' as path;

import '../asset/asset.dart';
import '../asset/exceptions.dart';
import '../asset/cache.dart';
import '../asset/file_based.dart';
import '../asset/id.dart';
Expand Down Expand Up @@ -131,6 +132,7 @@ Future _deletePreviousOutputs(List<List<Phase>> phaseGroups) async {
}
}
}

/// Once the group is done, add all outputs so they can be used in the next
/// phase.
for (var outputId in groupOutputIds) {
Expand All @@ -146,10 +148,10 @@ Future _deletePreviousOutputs(List<List<Phase>> phaseGroups) async {
'which already exist on disk. This is likely because the `.build` '
'folder was deleted.');
var done = false;
while(!done) {
while (!done) {
stdout.write('Delete these files (y/n) (or list them (l))?: ');
var input = stdin.readLineSync();
switch(input) {
switch (input) {
case 'y':
stdout.writeln('Deleting files...');
await Future.wait(conflictingOutputs.map((output) {
Expand Down Expand Up @@ -184,7 +186,7 @@ Future<BuildResult> _runPhases(List<List<Phase>> phaseGroups) async {
for (var builder in phase.builders) {
// TODO(jakemac): Optimize, we can run all the builders in a phase
// at the same time instead of sequentially.
await for (var output in _runBuilder(builder, inputs)) {
await for (var output in _runBuilder(builder, inputs, allInputs)) {
groupOutputs.add(output);
outputs.add(output);
}
Expand Down Expand Up @@ -251,12 +253,23 @@ bool _isValidInput(AssetId input) {
}

/// Runs [builder] with [inputs] as inputs.
Stream<Asset> _runBuilder(Builder builder, Iterable<AssetId> inputs) async* {
for (var input in inputs) {
Stream<Asset> _runBuilder(Builder builder, Iterable<AssetId> primaryInputs,
Map<String, Set<AssetId>> allInputs) async* {
for (var input in primaryInputs) {
var expectedOutputs = builder.declareOutputs(input);
/// Validate [expectedOutputs].
for (var output in expectedOutputs) {
if (output.package != _packageGraph.root.name) {
throw new InvalidOutputException(new Asset(output, ''));
}
if (allInputs[output.package]?.contains(output) == true) {
throw new InvalidOutputException(new Asset(output, ''));
}
}

var inputAsset = new Asset(input, await _reader.readAsString(input));
var buildStep =
new BuildStepImpl(inputAsset, expectedOutputs, _reader, _writer);
var buildStep = new BuildStepImpl(
inputAsset, expectedOutputs, _reader, _writer, _packageGraph.root.name);
await builder.build(buildStep);
await buildStep.complete();
for (var output in buildStep.outputs) {
Expand Down
3 changes: 2 additions & 1 deletion lib/src/transformer/transformer.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ abstract class BuilderTransformer implements Transformer, DeclaringTransformer {
}

// Run the build step.
var buildStep = new BuildStepImpl(input, expected, reader, writer);
var buildStep =
new BuildStepImpl(input, expected, reader, writer, input.id.package);
await builder.build(buildStep);
await buildStep.complete();
}));
Expand Down
10 changes: 6 additions & 4 deletions test/analyzer/resolver_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,14 +17,17 @@ import '../common/common.dart';
main() {
var entryPoint = makeAssetId('a|web/main.dart');
Future validateResolver(
{Map<String, String> inputs, validator(Resolver), List messages: const []}) async {
{Map<String, String> inputs,
validator(Resolver),
List messages: const []}) async {
var writer = new InMemoryAssetWriter();
var reader = new InMemoryAssetReader(writer.assets);
var assets = makeAssets(inputs);
addAssets(assets.values, writer);

var builder = new TestBuilder(validator);
var buildStep = new BuildStepImpl(assets[entryPoint], [], reader, writer);
var buildStep =
new BuildStepImpl(assets[entryPoint], [], reader, writer, 'a');
var logs = [];
if (messages != null) {
buildStep.logger.onRecord.listen(logs.add);
Expand Down Expand Up @@ -175,8 +178,7 @@ main() {
// First from the AST walker
'[WARNING] a|web/main.dart: $warningMessage "/b.dart"',
'[WARNING] a|web/main.dart: $warningMessage "/b.dart"',
],
validator: (resolver) {
], validator: (resolver) {
var lib = resolver.getLibrary(entryPoint);
expect(lib.importedLibraries.length, 1);
});
Expand Down
34 changes: 2 additions & 32 deletions test/asset/file_based_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -26,12 +26,8 @@ main() {
'world\n');
});

test('can only read package dependency files in the lib dir', () async {
test('can read package dependency files in the lib dir', () async {
expect(await reader.readAsString(makeAssetId('a|lib/a.txt')), 'A\n');
expect(reader.readAsString(makeAssetId('a|web/a.txt')),
throwsA(invalidInputException));
expect(reader.readAsString(makeAssetId('a|a.txt')),
throwsA(invalidInputException));
});

test('can check for existence of any application package files', () async {
Expand All @@ -46,16 +42,10 @@ main() {
await reader.hasInput(makeAssetId('basic_pkg|lib/a.txt')), isFalse);
});

test('can only check for existence of package dependency files in lib',
test('can check for existence of package dependency files in lib',
() async {
expect(await reader.hasInput(makeAssetId('a|lib/a.txt')), isTrue);
expect(await reader.hasInput(makeAssetId('a|lib/b.txt')), isFalse);
expect(reader.hasInput(makeAssetId('a|web/a.txt')),
throwsA(invalidInputException));
expect(reader.hasInput(makeAssetId('a|a.txt')),
throwsA(invalidInputException));
expect(reader.hasInput(makeAssetId('foo|bar.txt')),
throwsA(invalidInputException));
});

test('throws when attempting to read a non-existent file', () async {
Expand Down Expand Up @@ -105,25 +95,5 @@ main() {
await writer.delete(asset.id);
expect(await file.exists(), isFalse);
});

test('can\'t output files in package dependencies', () async {
var asset = makeAsset('a|test.txt');
expect(writer.writeAsString(asset), throwsA(invalidOutputException));
});

test('can\'t output files in arbitrary packages', () async {
var asset = makeAsset('foo|bar.txt');
expect(writer.writeAsString(asset), throwsA(invalidOutputException));
});

test('can\'t delete files in package dependencies', () async {
var id = makeAssetId('a|test.txt');
expect(writer.delete(id), throws);
});

test('can\'t delete files in arbitrary dependencies', () async {
var id = makeAssetId('foo|test.txt');
expect(writer.delete(id), throws);
});
});
}
41 changes: 35 additions & 6 deletions test/builder/build_step_impl_test.dart
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@ main() {
reader = new StubAssetReader();
writer = new StubAssetWriter();
primary = makeAsset();
buildStep = new BuildStepImpl(primary, [], reader, writer);
buildStep =
new BuildStepImpl(primary, [], reader, writer, primary.id.package);
});

test('tracks dependencies on read', () async {
Expand Down Expand Up @@ -53,7 +54,8 @@ main() {
test('tracks outputs', () async {
var a1 = makeAsset();
var a2 = makeAsset();
buildStep = new BuildStepImpl(primary, [a1.id, a2.id], reader, writer);
buildStep = new BuildStepImpl(
primary, [a1.id, a2.id], reader, writer, primary.id.package);

buildStep.writeAsString(a1);
expect(buildStep.outputs, [a1]);
Expand Down Expand Up @@ -86,6 +88,33 @@ main() {
'[SEVERE] ${primary.id}: goodbye',
]);
});

test('hasInput throws invalidInputExceptions', () async {
expect(() => buildStep.hasInput(makeAssetId('b|web/a.txt')),
throwsA(invalidInputException));
expect(() => buildStep.hasInput(makeAssetId('b|a.txt')),
throwsA(invalidInputException));
expect(() => buildStep.hasInput(makeAssetId('foo|bar.txt')),
throwsA(invalidInputException));
});

test('readAsString throws InvalidInputExceptions', () async {
expect(() => buildStep.readAsString(makeAssetId('b|web/a.txt')),
throwsA(invalidInputException));
expect(() => buildStep.readAsString(makeAssetId('b|a.txt')),
throwsA(invalidInputException));
expect(() => buildStep.readAsString(makeAssetId('foo|bar.txt')),
throwsA(invalidInputException));
});

test('writeAsString throws InvalidOutputExceptions', () async {
var a1 = makeAsset('b|test.txt');
expect(
() => buildStep.writeAsString(a1), throwsA(invalidOutputException));
var a2 = makeAsset('foo|bar.txt');
expect(
() => buildStep.writeAsString(a2), throwsA(invalidOutputException));
});
});

group('with in memory file system', () {
Expand All @@ -109,8 +138,8 @@ main() {
});
addAssets(inputs.values, writer);
var outputId = new AssetId.parse('$primary.combined');
var buildStep = new BuildStepImpl(
inputs[new AssetId.parse(primary)], [outputId], reader, writer);
var buildStep = new BuildStepImpl(inputs[new AssetId.parse(primary)],
[outputId], reader, writer, 'a');

await fileCombiner.build(buildStep);
await buildStep.complete();
Expand Down Expand Up @@ -140,8 +169,8 @@ main() {
addAssets(inputs.values, writer);

var primary = makeAssetId('a|web/a.dart');
var buildStep =
new BuildStepImpl(inputs[primary], [], reader, writer);
var buildStep = new BuildStepImpl(
inputs[primary], [], reader, writer, primary.package);
var resolver = await buildStep.resolve(primary);

var aLib = resolver.getLibrary(primary);
Expand Down
Loading

0 comments on commit adebd06

Please sign in to comment.