Skip to content

Commit 6e41c15

Browse files
OmarAlJarrahOmarAlJarrah
committed
refactor: rename HttpLogLevel.fromEnv to fromConfiguration and test the non-env layers
The factory resolves through the full Configuration layering (override -> env -> system property), not just the environment, so fromEnv was a misnomer that the KDoc had to walk back. Rename it to fromConfiguration to match the existing ProxyOptions.fromConfiguration(Configuration) factory and tighten the doc accordingly. Add tests covering resolution from the system-property layer and an explicit override winning over a conflicting env value, which the prior env-only suite never exercised despite the documented layering.
1 parent 64e5a05 commit 6e41c15

3 files changed

Lines changed: 54 additions & 23 deletions

File tree

‎sdk-core/api/sdk-core.api‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -832,17 +832,17 @@ public final class org/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel : java/
832832
public static final field Companion Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel$Companion;
833833
public static final field HEADERS Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
834834
public static final field NONE Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
835-
public static final fun fromEnv (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
836-
public static final fun fromEnv (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
835+
public static final fun fromConfiguration (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
836+
public static final fun fromConfiguration (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
837837
public static fun getEntries ()Lkotlin/enums/EnumEntries;
838838
public static fun valueOf (Ljava/lang/String;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
839839
public static fun values ()[Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
840840
}
841841

842842
public final class org/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel$Companion {
843-
public final fun fromEnv (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
844-
public final fun fromEnv (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
845-
public static synthetic fun fromEnv$default (Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel$Companion;Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
843+
public final fun fromConfiguration (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
844+
public final fun fromConfiguration (Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
845+
public static synthetic fun fromConfiguration$default (Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel$Companion;Ljava/lang/String;Lorg/dexpace/sdk/core/config/Configuration;Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;ILjava/lang/Object;)Lorg/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel;
846846
}
847847

848848
public final class org/dexpace/sdk/core/http/pipeline/steps/HttpRedirectCondition {

‎sdk-core/src/main/kotlin/org/dexpace/sdk/core/http/pipeline/steps/HttpLogLevel.kt‎

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -37,14 +37,13 @@ public enum class HttpLogLevel {
3737

3838
public companion object {
3939
/**
40-
* Resolves a log level from the configuration [key], reading through the testable
41-
* config seam of [source].
40+
* Resolves a log level by looking [key] up in [source].
4241
*
43-
* Despite the name, resolution is not env-only: [source] is consulted via
44-
* [Configuration.get], which applies the full layering — explicit override -> environment
45-
* variable -> normalized system property -> default. So the value may legitimately come
46-
* from an override or from a system property (e.g. the key `MY_PRODUCT_LOG_LEVEL` also
47-
* matches the `my.product.log.level` system property), not strictly the environment.
42+
* [source] is consulted via [Configuration.get], so the value goes through the full
43+
* layering — explicit override -> environment variable -> normalized system property ->
44+
* default. The value may come from any of those layers: the key `MY_PRODUCT_LOG_LEVEL`
45+
* also matches the `my.product.log.level` system property, and an explicit override wins
46+
* over both.
4847
*
4948
* The SDK is a toolkit, not a product, so it deliberately bakes in **no** default key —
5049
* the caller (e.g. a generated client) supplies its own product's variable name, and
@@ -61,7 +60,7 @@ public enum class HttpLogLevel {
6160
*/
6261
@JvmStatic
6362
@JvmOverloads
64-
public fun fromEnv(
63+
public fun fromConfiguration(
6564
key: String,
6665
source: Configuration,
6766
default: HttpLogLevel = NONE,

‎sdk-core/src/test/kotlin/org/dexpace/sdk/core/http/pipeline/steps/HttpLogLevelTest.kt‎

Lines changed: 42 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,7 @@ class HttpLogLevelTest {
2626
val cfg = configWithEnv("MY_PRODUCT_LOG_LEVEL" to "HEADERS")
2727
assertEquals(
2828
HttpLogLevel.HEADERS,
29-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
29+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
3030
)
3131
}
3232

@@ -35,7 +35,7 @@ class HttpLogLevelTest {
3535
val cfg = configWithEnv("MY_PRODUCT_LOG_LEVEL" to "body_and_headers")
3636
assertEquals(
3737
HttpLogLevel.BODY_AND_HEADERS,
38-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
38+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
3939
)
4040
}
4141

@@ -44,26 +44,58 @@ class HttpLogLevelTest {
4444
val cfg = configWithEnv("MY_PRODUCT_LOG_LEVEL" to " Headers ")
4545
assertEquals(
4646
HttpLogLevel.HEADERS,
47-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
47+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
4848
)
4949
}
5050

5151
@Test
5252
fun `each level name round-trips`() {
5353
HttpLogLevel.entries.forEach { level ->
5454
val cfg = configWithEnv("LL" to level.name)
55-
assertEquals(level, HttpLogLevel.fromEnv("LL", cfg, HttpLogLevel.NONE))
55+
assertEquals(level, HttpLogLevel.fromConfiguration("LL", cfg, HttpLogLevel.NONE))
5656
}
5757
}
5858

59+
// ----- Resolution honors the full Configuration layering, not just env -----
60+
61+
@Test
62+
fun `value resolves from the system-property layer when the env is unset`() {
63+
// Env unset; the property is looked up under the normalized name (MY_PRODUCT_LOG_LEVEL
64+
// -> my.product.log.level). The resolved value must still feed the level parsing.
65+
val cfg =
66+
ConfigurationBuilder()
67+
.envSource { null }
68+
.propsSource { name -> if (name == "my.product.log.level") "HEADERS" else null }
69+
.build()
70+
assertEquals(
71+
HttpLogLevel.HEADERS,
72+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
73+
)
74+
}
75+
76+
@Test
77+
fun `explicit override wins over a conflicting env value`() {
78+
// Override is the top layer; it must take precedence over the env entry for the same key.
79+
val cfg =
80+
ConfigurationBuilder()
81+
.put("MY_PRODUCT_LOG_LEVEL", "BODY_AND_HEADERS")
82+
.envSource { name -> if (name == "MY_PRODUCT_LOG_LEVEL") "HEADERS" else null }
83+
.propsSource { null }
84+
.build()
85+
assertEquals(
86+
HttpLogLevel.BODY_AND_HEADERS,
87+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
88+
)
89+
}
90+
5991
// ----- Unset key falls back to the supplied default -----
6092

6193
@Test
6294
fun `unset key returns the supplied default`() {
6395
val cfg = configWithEnv() // nothing set
6496
assertEquals(
6597
HttpLogLevel.HEADERS,
66-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.HEADERS),
98+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.HEADERS),
6799
)
68100
}
69101

@@ -73,18 +105,18 @@ class HttpLogLevelTest {
73105
val cfg = configWithEnv("MY_PRODUCT_LOG_LEVEL" to "")
74106
assertEquals(
75107
HttpLogLevel.BODY_AND_HEADERS,
76-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.BODY_AND_HEADERS),
108+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.BODY_AND_HEADERS),
77109
)
78110
}
79111

80112
@Test
81113
fun `whitespace-only value returns the supplied default`() {
82114
// Configuration only treats an exactly-empty env string as absent, so a whitespace-only
83-
// value is returned as-is; fromEnv's own trim collapses it to "" and falls to the default.
115+
// value is returned as-is; fromConfiguration's own trim collapses it to "" and falls to the default.
84116
val cfg = configWithEnv("MY_PRODUCT_LOG_LEVEL" to " ")
85117
assertEquals(
86118
HttpLogLevel.HEADERS,
87-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.HEADERS),
119+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.HEADERS),
88120
)
89121
}
90122

@@ -95,7 +127,7 @@ class HttpLogLevelTest {
95127
val cfg = configWithEnv("MY_PRODUCT_LOG_LEVEL" to "VERBOSE")
96128
assertEquals(
97129
HttpLogLevel.NONE,
98-
HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
130+
HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg, HttpLogLevel.NONE),
99131
)
100132
}
101133

@@ -104,6 +136,6 @@ class HttpLogLevelTest {
104136
@Test
105137
fun `default parameter is NONE when omitted`() {
106138
val cfg = configWithEnv() // nothing set
107-
assertEquals(HttpLogLevel.NONE, HttpLogLevel.fromEnv("MY_PRODUCT_LOG_LEVEL", cfg))
139+
assertEquals(HttpLogLevel.NONE, HttpLogLevel.fromConfiguration("MY_PRODUCT_LOG_LEVEL", cfg))
108140
}
109141
}

0 commit comments

Comments
 (0)