Set parent logger reference upon initialization (scala-js#10)

Remove the `findParent` method, which happens to create multiple
root loggers with the empty name "".

Instead add `prepareParentLoggerOf(name)` to ensure creating and
registering the parent loggers in advance. This also reduces the
overhead of resolving parent loggers.

Author: xerial

src/main/scala/java/util/logging/Logger.scala

@@ -5,7 +5,8 @@ import scala.collection.mutable
 
 object Logger {
 
-  val GLOBAL_LOGGER_NAME: String =  "global"
+  final val GLOBAL_LOGGER_NAME = "global"
+  private final val ROOT_LOGGER_NAME = ""
 
   // Not implemented, deprecated on JDK 1.8
   //val global: Logger
@@ -14,30 +15,57 @@ object Logger {
 
   private val loggers: mutable.Map[String, Logger] = mutable.Map.empty
 
+  /** Find parent logger */
+  @tailrec
+  private def findParentLoggerOf(name: String): Logger = {
+    name match {
+      case null => // Anonymous logger
+        rootLogger
+      case ROOT_LOGGER_NAME =>
+        null
+      case _ =>
+        val parentName = name.substring(0, name.lastIndexOf('.').max(0))
+        loggers.get(parentName) match {
+          case Some(l) => l
+          case None    => findParentLoggerOf(parentName)
+        }
+    }
+  }
+
   private def newLogger(name: String): Logger = {
     val logger = new Logger(name, null)
     logger.setLevel(null)
     logger.setUseParentHandlers(true)
-    logger.setParent(rootLogger)
+    logger.setParent(findParentLoggerOf(name))
+    if (name != null) {
+      updateChildLoggerParent(logger)
+    }
     logger
   }
 
+  private def updateChildLoggerParent(newParent: Logger): Unit ={
+    val prefix = s"${newParent.getName}."
+    // Traverse all child loggers of the new parent
+    for ((name, childLogger) <- loggers if name.startsWith(prefix)) {
+      val currentParent = childLogger.getParent
+      // For example, when a new parent a.b is added:
+      // - if child is a.b.c.d (parent = null) => needs to be a.b.c.d (parent = a.b)
+      // - if child is a.b.c.e (parent = a.b.c) => no update is required.
+      if (currentParent == null || !currentParent.getName.startsWith(prefix)) {
+        childLogger.setParent(newParent)
+      }
+    }
+  }
+
   // Root is not visible to the outside but gives defaults
   private[this] val rootLogger: Logger = {
-    val l = new Logger("", null)
-    l.setLevel(defaultLogLevel)
-    l.setUseParentHandlers(false)
-    l.setParent(null)
-    l
+    val root = getLogger(ROOT_LOGGER_NAME)
+    root.setLevel(defaultLogLevel)
+    root.setUseParentHandlers(false)
+    root
   }
 
-  private[this] val globalLogger: Logger = {
-    val l = new Logger(GLOBAL_LOGGER_NAME, null)
-    l.setLevel(defaultLogLevel)
-    l.setUseParentHandlers(true)
-    l.setParent(rootLogger)
-    l
-  }
+  private[this] val globalLogger: Logger = getLogger(GLOBAL_LOGGER_NAME)
 
   def getGlobal(): Logger = globalLogger
 
@@ -51,20 +79,6 @@ object Logger {
   // Not implemented, no resource bundle in scala.js
   //def getLogger(name: String, resourceBundle: String): Logger
 
-  private[logging] def findParent(logger: Logger): Option[Logger] = {
-    @tailrec
-    def go(s: List[String]): Option[Logger] = s match {
-      case Nil => None
-
-      case b if loggers.contains(b.mkString(".")) =>
-        loggers.get(b.mkString("."))
-
-      case b => go(b.dropRight(1))
-    }
-
-    go(Option(logger.getName).getOrElse("").split("\\.").toList.dropRight(1))
-  }
-
   def getAnonymousLogger(): Logger = {
     // No references to anonymous loggers are kept
     newLogger(null)
@@ -315,7 +329,7 @@ class Logger(name: String, resourceBundle: String) {
 
   def getUseParentHandlers(): Boolean = useParentsHandlers
 
-  def getParent(): Logger = Logger.findParent(this).getOrElse(parent)
+  def getParent(): Logger = parent
 
   def setParent(parent: Logger): Unit = this.parent = parent
 }

testSuite/shared/src/test/scala/org/scalajs/testsuite/javalib/util/logging/LoggerTest.scala

@@ -5,6 +5,8 @@ import java.util.logging._
 import org.junit.{Ignore, Test}
 import org.junit.Assert._
 
+import scala.annotation.tailrec
+
 class LoggerTest {
   // We add a prefix to all loggers. This avoids some errors when running tests
   // more than once as the loggers are global
@@ -54,6 +56,10 @@ class LoggerTest {
     assertNull(Logger.getAnonymousLogger().getLevel)
     assertTrue(Logger.getAnonymousLogger().getUseParentHandlers)
     assertNotNull(Logger.getAnonymousLogger().getParent)
+
+    // Sanity test of getAnonymousLogger after adding a new logger
+    val logger = Logger.getLogger(s"$prefix.a")
+    Logger.getAnonymousLogger()
   }
 
   @Test def test_properties(): Unit = {
@@ -337,10 +343,38 @@ class LoggerTest {
   }
 
   @Test def test_logger_parents_by_name(): Unit = {
-    val l1 = Logger.getLogger(s"$prefix.a.b.c.d")
-    assertEquals("", l1.getParent.getName)
+    val loggerName = s"${prefix}.a.b.c.d"
+    var l1 = Logger.getLogger(loggerName)
+    val p1 = l1.getParent
+
+    // l1's parent should be the root logger
+    val r = Logger.getLogger("")
+    assertEquals(p1.getName, r.getName)
+    assertSame(p1, r)
+
+    // Add a logger in between root and l1
+    val l2 = Logger.getLogger(s"${prefix}.a.b")
+    // l1's parent should be l2
+    val p2 = l1.getParent
+    assertEquals(l2.getName, p2.getName)
+    assertSame(l2, p2)
+  }
 
-    val l2 = Logger.getLogger(s"$prefix.a.b")
-    assertEquals(l2.getName, l1.getParent.getName)
+  @Test def test_single_root(): Unit = {
+    val l1 = Logger.getLogger(s"${prefix}.test1")
+    val l2 = Logger.getLogger(s"${prefix}.test2.a")
+
+    @tailrec
+    def findRoot(l: Logger): Logger = l.getParent match {
+      case null  => l
+      case other => findRoot(l.getParent)
+    }
+
+    val r1 = findRoot(l1)
+    val r2 = findRoot(l2)
+    val r = Logger.getLogger("")
+    assertSame(r1, r2)
+    assertSame(r1, r)
+    assertSame(r2, r)
   }
 }