[flutter tools] Add a DelegatingLogger class (#67581)

[flutter tools] Add a DelegatingLogger class

Move most of `DelegateLogger` `from test/src/testbed.dart` to
`lib/src/base/logger.dart` to better formalize the common practice of
chaining `Logger`s together.  I renamed the class since it isn't
itself the delegate and to better match the `Delegating...` classes
from `package:collection`.

Additionally, add a freestanding `asLogger<T>` function to "cast" a
`Logger` into a matching delegate if possible.  This will allow
`Logger` chains to be ordered a *bit* more freely (e.g.
`NotifyingLogger` and `AppRunLogger` will no longer required to be
at the end of the chain, an unwritten rule that has led to breakage in
google3).  Chain order still matters since lack of virtual dispatch
means that parent `Logger`s can never invoke child methods, however.

I made `asLogger<T>` a freestanding function because I didn't want to
make it part of the `Logger` interface (and I thought that making it
an extension method might be weird).

Bonus cleanup:
There no longer appears to be a way to construct an `AppRunLogger`
with a null parent, so remove all of code paths for that case and
make the `parent` construction parameter required.
This commit is contained in:
James D. Lin 2020-10-09 15:55:24 -07:00 committed by GitHub
parent bdb830a833
commit e4206ac5dd
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
6 changed files with 299 additions and 186 deletions

View file

@ -173,6 +173,107 @@ abstract class Logger {
void clear();
}
/// A [Logger] that forwards all methods to another one.
///
/// Classes can derive from this to add functionality to an existing [Logger].
class DelegatingLogger implements Logger {
@visibleForTesting
@protected
DelegatingLogger(this._delegate);
final Logger _delegate;
@override
bool get quiet => _delegate.quiet;
@override
set quiet(bool value) => _delegate.quiet = value;
@override
bool get hasTerminal => _delegate.hasTerminal;
@override
Terminal get _terminal => _delegate._terminal;
@override
OutputPreferences get _outputPreferences => _delegate._outputPreferences;
@override
TimeoutConfiguration get _timeoutConfiguration => _delegate._timeoutConfiguration;
@override
bool get isVerbose => _delegate.isVerbose;
@override
void printError(String message, {StackTrace stackTrace, bool emphasis, TerminalColor color, int indent, int hangingIndent, bool wrap}) {
_delegate.printError(
message,
stackTrace: stackTrace,
emphasis: emphasis,
color: color,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
}
@override
void printStatus(String message, {bool emphasis, TerminalColor color, bool newline, int indent, int hangingIndent, bool wrap}) {
_delegate.printStatus(message,
emphasis: emphasis,
color: color,
newline: newline,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
}
@override
void printTrace(String message) {
_delegate.printTrace(message);
}
@override
void sendEvent(String name, [Map<String, dynamic> args]) {
_delegate.sendEvent(name, args);
}
@override
Status startProgress(String message, {Duration timeout, String progressId, bool multilineOutput = false, int progressIndicatorPadding = kDefaultStatusPadding}) {
return _delegate.startProgress(message,
timeout: timeout,
progressId: progressId,
multilineOutput: multilineOutput,
progressIndicatorPadding: progressIndicatorPadding,
);
}
@override
bool get supportsColor => _delegate.supportsColor;
@override
void clear() => _delegate.clear();
}
/// If [logger] is a [DelegatingLogger], walks the delegate chain and returns
/// the first delegate with the matching type.
///
/// Throws a [StateError] if no matching delegate is found.
@override
T asLogger<T extends Logger>(Logger logger) {
final Logger original = logger;
while (true) {
if (logger is T) {
return logger;
} else if (logger is DelegatingLogger) {
logger = (logger as DelegatingLogger)._delegate;
} else {
throw StateError('$original has no ancestor delegate of type $T');
}
}
}
class StdoutLogger extends Logger {
StdoutLogger({
@required Terminal terminal,
@ -508,27 +609,17 @@ class BufferLogger extends Logger {
}
}
class VerboseLogger extends Logger {
VerboseLogger(this.parent, {
class VerboseLogger extends DelegatingLogger {
VerboseLogger(Logger parent, {
StopwatchFactory stopwatchFactory = const StopwatchFactory()
}) : _stopwatch = stopwatchFactory.createStopwatch(),
_stopwatchFactory = stopwatchFactory {
_stopwatchFactory = stopwatchFactory,
super(parent) {
_stopwatch.start();
}
final Logger parent;
final Stopwatch _stopwatch;
@override
Terminal get _terminal => parent._terminal;
@override
OutputPreferences get _outputPreferences => parent._outputPreferences;
@override
TimeoutConfiguration get _timeoutConfiguration => parent._timeoutConfiguration;
final StopwatchFactory _stopwatchFactory;
@override
@ -635,28 +726,19 @@ class VerboseLogger extends Logger {
final String indentMessage = message.replaceAll('\n', '\n$indent');
if (type == _LogType.error) {
parent.printError(prefix + _terminal.bolden(indentMessage));
super.printError(prefix + _terminal.bolden(indentMessage));
if (stackTrace != null) {
parent.printError(indent + stackTrace.toString().replaceAll('\n', '\n$indent'));
super.printError(indent + stackTrace.toString().replaceAll('\n', '\n$indent'));
}
} else if (type == _LogType.status) {
parent.printStatus(prefix + _terminal.bolden(indentMessage));
super.printStatus(prefix + _terminal.bolden(indentMessage));
} else {
parent.printStatus(prefix + indentMessage);
super.printStatus(prefix + indentMessage);
}
}
@override
void sendEvent(String name, [Map<String, dynamic> args]) { }
@override
bool get supportsColor => parent.supportsColor;
@override
bool get hasTerminal => parent.hasTerminal;
@override
void clear() => parent.clear();
}
enum _LogType { error, status, trace }

View file

@ -54,7 +54,7 @@ class DaemonCommand extends FlutterCommand {
final Daemon daemon = Daemon(
stdinCommandStream, stdoutCommandResponse,
notifyingLogger: globals.logger as NotifyingLogger,
notifyingLogger: asLogger<NotifyingLogger>(globals.logger),
);
final int code = await daemon.onExit;
if (code != 0) {
@ -525,7 +525,7 @@ class AppDomain extends Domain {
enableHotReload,
cwd,
LaunchMode.run,
globals.logger as AppRunLogger,
asLogger<AppRunLogger>(globals.logger),
);
}
@ -960,15 +960,14 @@ dynamic _toJsonable(dynamic obj) {
return '$obj';
}
class NotifyingLogger extends Logger {
NotifyingLogger({ @required this.verbose, this.parent }) {
class NotifyingLogger extends DelegatingLogger {
NotifyingLogger({ @required this.verbose, @required Logger parent }) : super(parent) {
_messageController = StreamController<LogMessage>.broadcast(
onListen: _onListen,
);
}
final bool verbose;
final Logger parent;
final List<LogMessage> messageBuffer = <LogMessage>[];
StreamController<LogMessage> _messageController;
@ -1012,7 +1011,7 @@ class NotifyingLogger extends Logger {
if (!verbose) {
return;
}
parent?.printError(message);
super.printError(message);
}
@override
@ -1136,83 +1135,13 @@ class EmulatorDomain extends Domain {
//
// TODO(devoncarew): To simplify this code a bit, we could choose to specialize
// this class into two, one for each of the above use cases.
class AppRunLogger extends Logger {
AppRunLogger({ this.parent });
class AppRunLogger extends DelegatingLogger {
AppRunLogger({ @required Logger parent }) : super(parent);
AppDomain domain;
AppInstance app;
final Logger parent;
int _nextProgressId = 0;
@override
void printError(
String message, {
StackTrace stackTrace,
bool emphasis,
TerminalColor color,
int indent,
int hangingIndent,
bool wrap,
}) {
if (parent != null) {
parent.printError(
message,
stackTrace: stackTrace,
emphasis: emphasis,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
} else {
if (stackTrace != null) {
_sendLogEvent(<String, dynamic>{
'log': message,
'stackTrace': stackTrace.toString(),
'error': true,
});
} else {
_sendLogEvent(<String, dynamic>{
'log': message,
'error': true,
});
}
}
}
@override
void printStatus(
String message, {
bool emphasis = false,
TerminalColor color,
bool newline = true,
int indent,
int hangingIndent,
bool wrap,
}) {
if (parent != null) {
parent.printStatus(
message,
emphasis: emphasis,
color: color,
newline: newline,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
} else {
_sendLogEvent(<String, dynamic>{'log': message});
}
}
@override
void printTrace(String message) {
if (parent != null) {
parent.printTrace(message);
} else {
_sendLogEvent(<String, dynamic>{'log': message, 'trace': true});
}
}
Status _status;
@override
@ -1249,14 +1178,6 @@ class AppRunLogger extends Logger {
domain = null;
}
void _sendLogEvent(Map<String, dynamic> event) {
if (domain == null) {
printStatus('event sent after app closed: $event');
} else {
domain._sendAppEvent(app, 'log', event);
}
}
void _sendProgressEvent({
@required String eventId,
@required String eventType,

View file

@ -11,6 +11,7 @@ import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
import 'package:flutter_tools/src/base/terminal.dart';
import 'package:flutter_tools/src/commands/daemon.dart';
import 'package:matcher/matcher.dart';
import 'package:mockito/mockito.dart';
import 'package:fake_async/fake_async.dart';
@ -73,6 +74,133 @@ void main() {
), isA<AppRunLogger>());
});
testWithoutContext('DelegatingLogger delegates', () {
final FakeLogger fakeLogger = FakeLogger();
final DelegatingLogger delegatingLogger = DelegatingLogger(fakeLogger);
expect(
() => delegatingLogger.quiet,
_throwsInvocationFor(() => fakeLogger.quiet),
);
expect(
() => delegatingLogger.quiet = true,
_throwsInvocationFor(() => fakeLogger.quiet = true),
);
expect(
() => delegatingLogger.hasTerminal,
_throwsInvocationFor(() => fakeLogger.hasTerminal),
);
expect(
() => delegatingLogger.isVerbose,
_throwsInvocationFor(() => fakeLogger.isVerbose),
);
const String message = 'message';
final StackTrace stackTrace = StackTrace.current;
const bool emphasis = true;
const TerminalColor color = TerminalColor.cyan;
const int indent = 88;
const int hangingIndent = 52;
const bool wrap = true;
const bool newline = true;
expect(
() => delegatingLogger.printError(message,
stackTrace: stackTrace,
emphasis: emphasis,
color: color,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
),
_throwsInvocationFor(() => fakeLogger.printError(message,
stackTrace: stackTrace,
emphasis: emphasis,
color: color,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
)),
);
expect(
() => delegatingLogger.printStatus(message,
emphasis: emphasis,
color: color,
newline: newline,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
),
_throwsInvocationFor(() => fakeLogger.printStatus(message,
emphasis: emphasis,
color: color,
newline: newline,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
)),
);
expect(
() => delegatingLogger.printTrace(message),
_throwsInvocationFor(() => fakeLogger.printTrace(message)),
);
final Map<String, dynamic> eventArgs = <String, dynamic>{};
expect(
() => delegatingLogger.sendEvent(message, eventArgs),
_throwsInvocationFor(() => fakeLogger.sendEvent(message, eventArgs)),
);
const Duration timeout = Duration(seconds: 12);
const String progressId = 'progressId';
const bool multilineOutput = true;
const int progressIndicatorPadding = kDefaultStatusPadding * 2;
expect(
() => delegatingLogger.startProgress(message,
timeout: timeout,
progressId: progressId,
multilineOutput: multilineOutput,
progressIndicatorPadding: progressIndicatorPadding,
),
_throwsInvocationFor(() => fakeLogger.startProgress(message,
timeout: timeout,
progressId: progressId,
multilineOutput: multilineOutput,
progressIndicatorPadding: progressIndicatorPadding,
)),
);
expect(
() => delegatingLogger.supportsColor,
_throwsInvocationFor(() => fakeLogger.supportsColor),
);
expect(
() => delegatingLogger.clear(),
_throwsInvocationFor(() => fakeLogger.clear()),
);
});
testWithoutContext('asLogger finds the correct delegate', () async {
final FakeLogger fakeLogger = FakeLogger();
final VerboseLogger verboseLogger = VerboseLogger(fakeLogger);
final NotifyingLogger notifyingLogger =
NotifyingLogger(verbose: true, parent: verboseLogger);
expect(asLogger<Logger>(notifyingLogger), notifyingLogger);
expect(asLogger<NotifyingLogger>(notifyingLogger), notifyingLogger);
expect(asLogger<VerboseLogger>(notifyingLogger), verboseLogger);
expect(asLogger<FakeLogger>(notifyingLogger), fakeLogger);
expect(
() => asLogger<AppRunLogger>(notifyingLogger),
throwsA(isA<StateError>()),
);
});
group('AppContext', () {
FakeStopwatch fakeStopWatch;
@ -1079,3 +1207,38 @@ class FakeStopwatchFactory implements StopwatchFactory {
return stopwatch ?? FakeStopwatch();
}
}
/// A fake [Logger] that throws the [Invocation] for any method call.
class FakeLogger implements Logger {
@override
dynamic noSuchMethod(Invocation invocation) => throw invocation;
}
/// Returns the [Invocation] thrown from a call to [FakeLogger].
Invocation _invocationFor(dynamic Function() fakeCall) {
try {
fakeCall();
} on Invocation catch (invocation) {
return invocation;
}
throw UnsupportedError('_invocationFor can be used only with Fake objects '
'that throw Invocations');
}
/// Returns a [Matcher] that matches against an expected [Invocation].
Matcher _matchesInvocation(Invocation expected) {
return const TypeMatcher<Invocation>()
// Compare Symbol strings instead of comparing Symbols directly for a nicer failure message.
.having((Invocation actual) => actual.memberName.toString(), 'memberName', expected.memberName.toString())
.having((Invocation actual) => actual.isGetter, 'isGetter', expected.isGetter)
.having((Invocation actual) => actual.isSetter, 'isSetter', expected.isSetter)
.having((Invocation actual) => actual.isMethod, 'isMethod', expected.isMethod)
.having((Invocation actual) => actual.typeArguments, 'typeArguments', expected.typeArguments)
.having((Invocation actual) => actual.positionalArguments, 'positionalArguments', expected.positionalArguments)
.having((Invocation actual) => actual.namedArguments, 'namedArguments', expected.namedArguments);
}
/// Returns a [Matcher] that matches against an [Invocation] thrown from a call
/// to [FakeLogger].
Matcher _throwsInvocationFor(dynamic Function() fakeCall) =>
throwsA(_matchesInvocation(_invocationFor(fakeCall)));

View file

@ -70,9 +70,9 @@ void main() {
test('Can successfully run and connect without vmservice', () => testbed.run(() async {
_setupMocks();
final DelegateLogger delegateLogger = globals.logger as DelegateLogger;
final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger;
final MockStatus mockStatus = MockStatus();
delegateLogger.status = mockStatus;
fakeStatusLogger.status = mockStatus;
final Completer<DebugConnectionInfo> connectionInfoCompleter = Completer<DebugConnectionInfo>();
unawaited(residentWebRunner.run(
connectionInfoCompleter: connectionInfoCompleter,
@ -83,7 +83,7 @@ void main() {
verify(mockStatus.stop()).called(1);
}, overrides: <Type, Generator>{
BuildSystem: () => mockBuildSystem,
Logger: () => DelegateLogger(BufferLogger(
Logger: () => FakeStatusLogger(BufferLogger(
terminal: AnsiTerminal(
stdio: null,
platform: const LocalPlatform(),

View file

@ -282,10 +282,10 @@ void main() {
final ResidentRunner residentWebRunner = setUpResidentRunner(mockFlutterDevice);
fakeVmServiceHost = FakeVmServiceHost(requests: kAttachExpectations.toList());
_setupMocks();
final DelegateLogger delegateLogger = globals.logger as DelegateLogger;
final BufferLogger bufferLogger = delegateLogger.delegate as BufferLogger;
final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger;
final BufferLogger bufferLogger = asLogger<BufferLogger>(fakeStatusLogger);
final MockStatus status = MockStatus();
delegateLogger.status = status;
fakeStatusLogger.status = status;
final Completer<DebugConnectionInfo> connectionInfoCompleter = Completer<DebugConnectionInfo>();
unawaited(residentWebRunner.run(
connectionInfoCompleter: connectionInfoCompleter,
@ -297,7 +297,7 @@ void main() {
expect(bufferLogger.statusText, contains('Debug service listening on ws://127.0.0.1/abcd/'));
expect(debugConnectionInfo.wsUri.toString(), 'ws://127.0.0.1/abcd/');
}, overrides: <Type, Generator>{
Logger: () => DelegateLogger(BufferLogger.test()),
Logger: () => FakeStatusLogger(BufferLogger.test()),
FileSystem: () => fileSystem,
ProcessManager: () => processManager,
Pub: () => MockPub(),
@ -1500,9 +1500,9 @@ void main() {
when(chrome.chromeConnection).thenReturn(mockChromeConnection);
chromiumLauncher.testLaunchChromium(chrome);
final DelegateLogger delegateLogger = globals.logger as DelegateLogger;
final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger;
final MockStatus mockStatus = MockStatus();
delegateLogger.status = mockStatus;
fakeStatusLogger.status = mockStatus;
final ResidentWebRunner runner = DwdsWebRunnerFactory().createWebRunner(
mockFlutterDevice,
flutterProject: FlutterProject.current(),
@ -1519,7 +1519,7 @@ void main() {
await connectionInfoCompleter.future;
// Ensure we got the URL and that it was already launched.
expect((delegateLogger.delegate as BufferLogger).eventText,
expect(asLogger<BufferLogger>(fakeStatusLogger).eventText,
contains(json.encode(<String, Object>{
'name': 'app.webLaunchUrl',
'args': <String, Object>{
@ -1530,7 +1530,7 @@ void main() {
)));
expect(fakeVmServiceHost.hasRemainingExpectations, false);
}, overrides: <Type, Generator>{
Logger: () => DelegateLogger(BufferLogger.test()),
Logger: () => FakeStatusLogger(BufferLogger.test()),
FileSystem: () => fileSystem,
ProcessManager: () => processManager,
Pub: () => MockPub(),
@ -1547,9 +1547,9 @@ void main() {
return Uri.parse('http://localhost:8765/app/');
});
final DelegateLogger delegateLogger = globals.logger as DelegateLogger;
final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger;
final MockStatus mockStatus = MockStatus();
delegateLogger.status = mockStatus;
fakeStatusLogger.status = mockStatus;
final ResidentWebRunner runner = DwdsWebRunnerFactory().createWebRunner(
mockFlutterDevice,
flutterProject: FlutterProject.current(),
@ -1566,7 +1566,7 @@ void main() {
await connectionInfoCompleter.future;
// Ensure we got the URL and that it was not already launched.
expect((delegateLogger.delegate as BufferLogger).eventText,
expect(asLogger<BufferLogger>(fakeStatusLogger).eventText,
contains(json.encode(<String, Object>{
'name': 'app.webLaunchUrl',
'args': <String, Object>{
@ -1577,7 +1577,7 @@ void main() {
)));
expect(fakeVmServiceHost.hasRemainingExpectations, false);
}, overrides: <Type, Generator>{
Logger: () => DelegateLogger(BufferLogger.test()),
Logger: () => FakeStatusLogger(BufferLogger.test()),
FileSystem: () => fileSystem,
ProcessManager: () => processManager,
Pub: () => MockPub(),
@ -1654,9 +1654,9 @@ void main() {
final ResidentRunner residentWebRunner = setUpResidentRunner(mockFlutterDevice);
fakeVmServiceHost = FakeVmServiceHost(requests: <VmServiceExpectation>[]);
_setupMocks();
final DelegateLogger delegateLogger = globals.logger as DelegateLogger;
final FakeStatusLogger fakeStatusLogger = globals.logger as FakeStatusLogger;
final MockStatus mockStatus = MockStatus();
delegateLogger.status = mockStatus;
fakeStatusLogger.status = mockStatus;
when(mockWebDevFS.connect(any)).thenThrow(StateError(''));
@ -1664,7 +1664,7 @@ void main() {
verify(mockStatus.stop()).called(1);
expect(fakeVmServiceHost.hasRemainingExpectations, false);
}, overrides: <Type, Generator>{
Logger: () => DelegateLogger(BufferLogger(
Logger: () => FakeStatusLogger(BufferLogger(
terminal: AnsiTerminal(
stdio: null,
platform: const LocalPlatform(),

View file

@ -783,68 +783,15 @@ class TestFeatureFlags implements FeatureFlags {
}
}
class DelegateLogger implements Logger {
DelegateLogger(this.delegate);
class FakeStatusLogger extends DelegatingLogger {
FakeStatusLogger(Logger delegate) : super(delegate);
final Logger delegate;
Status status;
@override
bool get quiet => delegate.quiet;
@override
set quiet(bool value) => delegate.quiet;
@override
bool get hasTerminal => delegate.hasTerminal;
@override
bool get isVerbose => delegate.isVerbose;
@override
void printError(String message, {StackTrace stackTrace, bool emphasis, TerminalColor color, int indent, int hangingIndent, bool wrap}) {
delegate.printError(
message,
stackTrace: stackTrace,
emphasis: emphasis,
color: color,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
}
@override
void printStatus(String message, {bool emphasis, TerminalColor color, bool newline, int indent, int hangingIndent, bool wrap}) {
delegate.printStatus(message,
emphasis: emphasis,
color: color,
indent: indent,
hangingIndent: hangingIndent,
wrap: wrap,
);
}
@override
void printTrace(String message) {
delegate.printTrace(message);
}
@override
void sendEvent(String name, [Map<String, dynamic> args]) {
delegate.sendEvent(name, args);
}
@override
Status startProgress(String message, {Duration timeout, String progressId, bool multilineOutput = false, int progressIndicatorPadding = kDefaultStatusPadding}) {
return status;
}
@override
bool get supportsColor => delegate.supportsColor;
@override
void clear() => delegate.clear();
}
/// An implementation of the Cache which does not download or require locking.