Fix Config.logLevel precedence and javadoc

library/src/main/kotlin/com/gabrielfeo/develocity/api/Config.kt

@@ -14,12 +14,15 @@ import kotlin.time.Duration.Companion.days
 data class Config(
 
     /**
-     * Changes the default log level for library classes, such as the HTTP client. By default,
-     * log level is the value of
-     * [org.slf4j.simpleLogger.defaultLogLevel](https://www.slf4j.org/api/org/slf4j/simple/SimpleLogger.html)
-     * system property or `"off"`.
+     * Changes the default log level for library classes, such as the HTTP client. Default value, by order of
+     * precedence:
+     *
+     * - `DEVELOCITY_API_LOG_LEVEL` environment variable
+     * - `org.slf4j.simpleLogger.defaultLogLevel` system property
+     * - `"off"`
      *
      * Possible values:
+     *
      * - "off" (default)
      * - "error"
      * - "warn"
@@ -30,6 +33,7 @@ data class Config(
      */
     val logLevel: String =
         env["DEVELOCITY_API_LOG_LEVEL"]
+            ?: systemProperties.logLevel
             ?: "off",
 
     /**
@@ -38,15 +42,15 @@ data class Config(
      */
     val apiUrl: String =
         env["DEVELOCITY_API_URL"]
-            ?: error("DEVELOCITY_API_URL is required"),
+            ?: error(ERROR_NULL_API_URL),
 
     /**
      * Provides the access token for a Develocity API instance. By default, uses environment
      * variable `DEVELOCITY_API_TOKEN`.
      */
     val apiToken: () -> String = {
         env["DEVELOCITY_API_TOKEN"]
-            ?: error("DEVELOCITY_API_TOKEN is required")
+            ?: error(ERROR_NULL_API_TOKEN)
     },
 
     /**
@@ -143,11 +147,14 @@ data class Config(
 
         /**
          * HTTP cache location. By default, uses environment variable `DEVELOCITY_API_CACHE_DIR`
-         * or the system temporary folder (`java.io.tmpdir` / develocity-api-kotlin-cache).
+         * or creates a `~/.develocity-api-kotlin-cache` directory.
          */
         val cacheDir: File =
             env["DEVELOCITY_API_CACHE_DIR"]?.let(::File)
-                ?: File(systemProperties["user.home"], ".develocity-api-kotlin-cache"),
+                ?: run {
+                    val userHome = checkNotNull(systemProperties.userHome) { ERROR_NULL_USER_HOME }
+                    File(userHome, ".develocity-api-kotlin-cache")
+                },
 
         /**
          * Max size of the HTTP cache. By default, uses environment variable
@@ -204,3 +211,7 @@ data class Config(
                 ?: 1.days.inWholeSeconds,
     )
 }
+
+private const val ERROR_NULL_API_URL = "DEVELOCITY_API_URL is required"
+private const val ERROR_NULL_API_TOKEN = "DEVELOCITY_API_TOKEN is required"
+private const val ERROR_NULL_USER_HOME = "'user.home' system property must not be null"

library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/LoggerFactory.kt

@@ -18,11 +18,10 @@ class RealLoggerFactory(
     }
 
     private fun setLogLevel() {
-        if (System.getProperty(SIMPLE_LOGGER_LOG_LEVEL) != null) {
-            return
-        }
-        System.setProperty(SIMPLE_LOGGER_LOG_LEVEL, config.logLevel)
+        System.setProperty(LOG_LEVEL_SYSTEM_PROPERTY, config.logLevel)
     }
-}
 
-internal const val SIMPLE_LOGGER_LOG_LEVEL = "org.slf4j.simpleLogger.defaultLogLevel"
+    companion object {
+        const val LOG_LEVEL_SYSTEM_PROPERTY = "org.slf4j.simpleLogger.defaultLogLevel"
+    }
+}

library/src/main/kotlin/com/gabrielfeo/develocity/api/internal/SystemProperties.kt

@@ -3,9 +3,11 @@ package com.gabrielfeo.develocity.api.internal
 internal var systemProperties: SystemProperties = RealSystemProperties
 
 internal interface SystemProperties {
-    operator fun get(name: String): String?
+    val userHome: String?
+    val logLevel: String?
 }
 
 internal object RealSystemProperties : SystemProperties {
-    override fun get(name: String): String? = System.getProperty(name)
+    override val userHome: String? = System.getProperty("user.home")
+    override val logLevel: String? = System.getProperty(RealLoggerFactory.LOG_LEVEL_SYSTEM_PROPERTY)
 }

library/src/test/kotlin/com/gabrielfeo/develocity/api/ConfigTest.kt

@@ -9,6 +9,7 @@ class ConfigTest {
     @BeforeTest
     fun before() {
         env = FakeEnv("DEVELOCITY_API_URL" to "https://example.com/api/")
+        systemProperties = FakeSystemProperties()
     }
 
     @Test
@@ -51,4 +52,24 @@ class ConfigTest {
         (env as FakeEnv)["DEVELOCITY_API_READ_TIMEOUT_MILLIS"] = "100000"
         assertEquals(100_000L, Config().readTimeoutMillis)
     }
+
+    @Test
+    fun `Given logLevel in env, logLevel is env value`() {
+        (env as FakeEnv)["DEVELOCITY_API_LOG_LEVEL"] = "trace"
+        assertEquals("trace", Config().logLevel)
+    }
+
+    @Test
+    fun `Given logLevel in System props and not in env, logLevel is prop value`() {
+        (env as FakeEnv)["DEVELOCITY_API_LOG_LEVEL"] = null
+        (systemProperties as FakeSystemProperties).logLevel = "info"
+        assertEquals("info", Config().logLevel)
+    }
+
+    @Test
+    fun `Given no logLevel set, logLevel is off`() {
+        (env as FakeEnv)["DEVELOCITY_API_LOG_LEVEL"] = null
+        (systemProperties as FakeSystemProperties).logLevel = null
+        assertEquals("off", Config().logLevel)
+    }
 }

library/src/test/kotlin/com/gabrielfeo/develocity/api/internal/LoggerFactoryTest.kt

@@ -8,32 +8,19 @@ import kotlin.test.assertEquals
 
 class LoggerFactoryTest {
 
+    private val logLevelProperty = RealLoggerFactory.LOG_LEVEL_SYSTEM_PROPERTY
+
     @BeforeTest
     @AfterTest
     fun cleanup() {
-        System.clearProperty(SIMPLE_LOGGER_LOG_LEVEL)
+        System.clearProperty(logLevelProperty)
         env = FakeEnv("DEVELOCITY_API_URL" to "https://example.com/")
     }
 
     @Test
-    fun `Logging off by default`() {
-        val loggerFactory = RealLoggerFactory(Config())
-        loggerFactory.newLogger(LoggerFactoryTest::class)
-        assertEquals("off", System.getProperty(SIMPLE_LOGGER_LOG_LEVEL))
-    }
-
-    @Test
-    fun `Pre-existing defaultLogLevel is honored`() {
-        val loggerFactory = RealLoggerFactory(Config(logLevel = "bar"))
-        System.setProperty(SIMPLE_LOGGER_LOG_LEVEL, "foo")
-        loggerFactory.newLogger(LoggerFactoryTest::class)
-        assertEquals("foo", System.getProperty(SIMPLE_LOGGER_LOG_LEVEL))
-    }
-
-    @Test
-    fun `Logging can be set from config`() {
+    fun `Level always copied from`() {
         val loggerFactory = RealLoggerFactory(Config(logLevel = "foo"))
         loggerFactory.newLogger(LoggerFactoryTest::class)
-        assertEquals("foo", System.getProperty(SIMPLE_LOGGER_LOG_LEVEL))
+        assertEquals("foo", System.getProperty(logLevelProperty))
     }
 }

library/src/testFixtures/kotlin/com/gabrielfeo/develocity/api/internal/FakeSystemProperties.kt

@@ -0,0 +1,6 @@
+package com.gabrielfeo.develocity.api.internal
+
+data class FakeSystemProperties(
+    override var userHome: String? = System.getProperty("java.io.tmpdir"),
+    override var logLevel: String? = null,
+) : SystemProperties