Make detached Loggers work regardless of hierarchicalLoggingEnabled (flutter#71)

Previously detached Loggers could log messages only if
hierarchicalLoggingEnabled was true.  This makes no sense to me since
detached Loggers aren't part of a Logger hierarchy.

Fixes https://github.com/dart-lang/logging/issues/34.

Author: jamesderlin

CHANGELOG.md

@@ -2,6 +2,7 @@
 
 * Add top level `defaultLevel`.
 * Require Dart `>=2.0.0`.
+* Make detached loggers work regardless of `hierarchicalLoggingEnabled`.
 
 ## 0.11.3+2
 
@@ -21,7 +22,7 @@
 
 ## 0.11.2
 
-* Added Logger.detached - a convenience factory to obtain a logger that is not
+* Added `Logger.detached` - a convenience factory to obtain a logger that is not
   attached to this library's logger hierarchy.
 
 ## 0.11.1+1

lib/src/logger.dart

@@ -80,33 +80,40 @@ class Logger {
   Logger._internal(this.name, this.parent, Map<String, Logger> children)
       : _children = children,
         children = UnmodifiableMapView(children) {
-    if (parent != null) {
+    if (parent == null) {
+      _level = defaultLevel;
+    } else {
       parent._children[name] = this;
     }
   }
 
   /// Effective level considering the levels established in this logger's
   /// parents (when [hierarchicalLoggingEnabled] is true).
   Level get level {
-    if (hierarchicalLoggingEnabled) {
-      if (_level != null) return _level;
-      if (parent != null) return parent.level;
+    Level effectiveLevel;
+
+    if (parent == null) {
+      // We're either the root logger or a detached logger.  Return our own
+      // level.
+      effectiveLevel = _level;
+    } else if (!hierarchicalLoggingEnabled) {
+      effectiveLevel = root._level;
+    } else {
+      effectiveLevel = _level ?? parent.level;
     }
-    return root._level;
+
+    assert(effectiveLevel != null);
+    return effectiveLevel;
   }
 
   /// Override the level for this particular [Logger] and its children.
   set level(Level value) {
-    if (hierarchicalLoggingEnabled && parent != null) {
-      _level = value;
-    } else {
-      if (parent != null) {
-        throw UnsupportedError(
-            'Please set "hierarchicalLoggingEnabled" to true if you want to '
-            'change the level on a non-root logger.');
-      }
-      root._level = value;
+    if (!hierarchicalLoggingEnabled && parent != null) {
+      throw UnsupportedError(
+          'Please set "hierarchicalLoggingEnabled" to true if you want to '
+          'change the level on a non-root logger.');
     }
+    _level = value;
   }
 
   /// Returns a stream of messages added to this [Logger].
@@ -174,14 +181,16 @@ class Logger {
       var record =
           LogRecord(logLevel, msg, fullName, error, stackTrace, zone, object);
 
-      if (hierarchicalLoggingEnabled) {
+      if (parent == null) {
+        _publish(record);
+      } else if (!hierarchicalLoggingEnabled) {
+        root._publish(record);
+      } else {
         var target = this;
         while (target != null) {
           target._publish(record);
           target = target.parent;
         }
-      } else {
-        root._publish(record);
       }
     }
   }
@@ -234,7 +243,7 @@ class Logger {
   }
 
   /// Top-level root [Logger].
-  static final Logger root = Logger('').._level = defaultLevel;
+  static final Logger root = Logger('');
 
   /// All [Logger]s in the system.
   static final Map<String, Logger> _loggers = <String, Logger>{};

test/logging_test.dart

@@ -10,6 +10,8 @@ import 'package:logging/logging.dart';
 import 'package:test/test.dart';
 
 void main() {
+  final hierarchicalLoggingEnabledDefault = hierarchicalLoggingEnabled;
+
   test('level comparison is a valid comparator', () {
     var level1 = const Level('NOT_REAL1', 253);
     expect(level1 == level1, isTrue);
@@ -213,6 +215,11 @@ void main() {
   });
 
   group('detached loggers', () {
+    tearDown(() {
+      hierarchicalLoggingEnabled = hierarchicalLoggingEnabledDefault;
+      Logger.root.level = defaultLevel;
+    });
+
     test('create new instances of Logger', () {
       var a1 = Logger.detached('a');
       var a2 = Logger.detached('a');
@@ -232,6 +239,51 @@ void main() {
       var a = Logger.detached('a');
       expect(a.children, {});
     });
+
+    test('have levels independent of the root level', () {
+      void testDetachedLoggerLevel(bool withHierarchy) {
+        hierarchicalLoggingEnabled = withHierarchy;
+
+        const newRootLevel = Level.ALL;
+        const newDetachedLevel = Level.OFF;
+
+        Logger.root.level = newRootLevel;
+
+        final detached = Logger.detached('a');
+        expect(detached.level, defaultLevel);
+        expect(Logger.root.level, newRootLevel);
+
+        detached.level = newDetachedLevel;
+        expect(detached.level, newDetachedLevel);
+        expect(Logger.root.level, newRootLevel);
+      }
+
+      testDetachedLoggerLevel(false);
+      testDetachedLoggerLevel(true);
+    });
+
+    test('log messages regardless of hierarchy', () {
+      void testDetachedLoggerOnRecord(bool withHierarchy) {
+        var calls = 0;
+        void handler(_) => calls += 1;
+
+        hierarchicalLoggingEnabled = withHierarchy;
+
+        final detached = Logger.detached('a');
+        detached.level = Level.ALL;
+        detached.onRecord.listen(handler);
+
+        Logger.root.info('foo');
+        expect(calls, 0);
+
+        detached.info('foo');
+        detached.info('foo');
+        expect(calls, 2);
+      }
+
+      testDetachedLoggerOnRecord(false);
+      testDetachedLoggerOnRecord(true);
+    });
   });
 
   group('mutating levels', () {
@@ -294,6 +346,16 @@ void main() {
       expect(c.level, equals(Level.FINE));
     });
 
+    test('loggers effective level - with changing hierarchy', () {
+      hierarchicalLoggingEnabled = true;
+      d.level = Level.SHOUT;
+      hierarchicalLoggingEnabled = false;
+
+      expect(root.level, Level.INFO);
+      expect(d.level, root.level);
+      expect(e.level, root.level);
+    });
+
     test('isLoggable is appropriate', () {
       hierarchicalLoggingEnabled = true;
       root.level = Level.SEVERE;