Skip to content

Commit 85667b4

Browse files
AdobelsAdobels
committed
Address review comments
- Remove boolDecoder init customization - Revert changes to the default set of values in tests - Create a dedicated test to cover various string boolean variants
1 parent 9b6bc7b commit 85667b4

File tree

5 files changed

+86
-150
lines changed

5 files changed

+86
-150
lines changed

‎Sources/Configuration/Providers/EnvironmentVariables/EnvironmentVariablesProvider.swift‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -173,7 +173,7 @@ public struct EnvironmentVariablesProvider: Sendable {
173173
public init(
174174
secretsSpecifier: SecretsSpecifier<String, String> = .none,
175175
bytesDecoder: some ConfigBytesFromStringDecoder = .base64,
176-
boolDecoder: BoolDecoder = .allBooleanStrings,
176+
boolDecoder: BoolDecoder = .init(),
177177
arraySeparator: Character = ","
178178
) {
179179
self.init(
@@ -212,7 +212,7 @@ public struct EnvironmentVariablesProvider: Sendable {
212212
environmentVariables: [String: String],
213213
secretsSpecifier: SecretsSpecifier<String, String> = .none,
214214
bytesDecoder: some ConfigBytesFromStringDecoder = .base64,
215-
boolDecoder: BoolDecoder = .allBooleanStrings,
215+
boolDecoder: BoolDecoder = .init(),
216216
arraySeparator: Character = ","
217217
) {
218218
let tuples: [(String, EnvironmentValue)] = environmentVariables.map { key, value in
@@ -262,7 +262,7 @@ public struct EnvironmentVariablesProvider: Sendable {
262262
allowMissing: Bool = false,
263263
secretsSpecifier: SecretsSpecifier<String, String> = .none,
264264
bytesDecoder: some ConfigBytesFromStringDecoder = .base64,
265-
boolDecoder: BoolDecoder = .allBooleanStrings,
265+
boolDecoder: BoolDecoder = .init(),
266266
arraySeparator: Character = ","
267267
) async throws {
268268
try await self.init(
@@ -294,7 +294,7 @@ public struct EnvironmentVariablesProvider: Sendable {
294294
fileSystem: some CommonProviderFileSystem,
295295
secretsSpecifier: SecretsSpecifier<String, String> = .none,
296296
bytesDecoder: some ConfigBytesFromStringDecoder = .base64,
297-
boolDecoder: BoolDecoder = .allBooleanStrings,
297+
boolDecoder: BoolDecoder = .init(),
298298
arraySeparator: Character = ","
299299
) async throws {
300300
let loadedData = try await fileSystem.fileContents(atPath: environmentFilePath)

‎Sources/Configuration/ValueCoders/ConfigBoolsFromStringDecoder.swift‎

Lines changed: 13 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -21,44 +21,27 @@ import Foundation
2121
/// A decoder that converts a boolean string into a Bool, taking into account different boolean string pairs.
2222
///
2323
/// This decoder is able to convert a string to Bool values when the string’s Boolean value format is 0 or 1, true or false, or yes or no.
24-
/// Boolean strings taken into account when decoding are configurable at init time only.
2524
///
2625
/// ## Boolean values
2726
///
28-
/// By default, following boolean string pairs are decoded to a Bool value: trueFalse (true, false), oneZero (1, 0), yesNo (yes, no).
27+
/// Following boolean string pairs are decoded to a Bool value: trueFalse (true, false), oneZero (1, 0), yesNo (yes, no).
2928
/// Decoding is case-insensitive.
3029
@available(Configuration 1.0, *)
3130
public struct BoolDecoder: Sendable {
3231

33-
public enum BooleanString: Sendable { case trueFalse, oneZero, yesNo }
32+
/// Creates a new bool decoder.
33+
public init() {}
3434

35-
public static var allBooleanStrings: Self { .init(booleanStrings: [.trueFalse, .oneZero, .yesNo]) }
36-
37-
private let booleanStrings: [BooleanString]
38-
39-
public init(booleanStrings: [BooleanString]) {
40-
self.booleanStrings = booleanStrings
41-
}
42-
43-
func decodeBool(from string: String) -> Bool? {
44-
for semantic in self.booleanStrings {
45-
switch semantic {
46-
case .trueFalse:
47-
let stringLowercased = string.lowercased()
48-
if ["true", "false"].contains(stringLowercased) {
49-
return stringLowercased == "true"
50-
}
51-
case .oneZero:
52-
if ["1", "0"].contains(string) {
53-
return string == "1"
54-
}
55-
case .yesNo:
56-
let stringLowercased = string.lowercased()
57-
if ["yes", "no"].contains(stringLowercased) {
58-
return stringLowercased == "yes"
59-
}
60-
}
35+
public func decodeBool(from string: String) -> Bool? {
36+
let stringLowercased = string.lowercased()
37+
return if ["true", "false"].contains(stringLowercased) {
38+
stringLowercased == "true"
39+
} else if ["yes", "no"].contains(stringLowercased) {
40+
stringLowercased == "yes"
41+
} else if ["1", "0"].contains(stringLowercased) {
42+
stringLowercased == "1"
43+
} else {
44+
nil
6145
}
62-
return nil
6346
}
6447
}

‎Tests/ConfigurationTests/ConfigBoolsFromStringDecoderTests.swift‎

Lines changed: 17 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -18,28 +18,24 @@ import Foundation
1818

1919
struct ConfigBoolsFromStringDecoderTests {
2020

21+
@Test()
2122
@available(Configuration 1.0, *)
22-
@Test("boolDecoder, all boolean strings enabled")
23-
func boolDecoderAllBooleanStringsEnabled() throws {
24-
let bd = BoolDecoder(booleanStrings: [.oneZero, .trueFalse, .yesNo])
25-
#expect(bd.decodeBool(from: "1") == true)
26-
#expect(bd.decodeBool(from: "0") == false)
27-
#expect(["Yes", "yes", "YES", "yES"].allSatisfy { bd.decodeBool(from: $0) == true })
28-
#expect(["No", "no", "NO", "nO"].allSatisfy { bd.decodeBool(from: $0) == false })
29-
#expect(["true", "TRUE", "trUe"].allSatisfy { bd.decodeBool(from: $0) == true })
30-
#expect(["false", "FALSE", "faLse"].allSatisfy { bd.decodeBool(from: $0) == false })
31-
#expect(["_true_", "_false_", "11", "00"].allSatisfy { bd.decodeBool(from: $0) == nil })
32-
}
23+
func stringToBool() throws {
24+
let bd = BoolDecoder()
25+
let cases: [(expected: Bool?, input: [String])] = [
26+
(true, ["1"]),
27+
(false, ["0"]),
28+
(true, ["Yes", "yes", "YES", "yES"]),
29+
(false, ["No", "no", "NO", "nO"]),
30+
(true, ["true", "TRUE", "trUe"]),
31+
(false, ["false", "FALSE", "faLse"]),
32+
(nil, ["", "_true_", "_false_", "_yes_", "_no_", "_1_", "_0_", "11", "00"])
33+
]
3334

34-
@available(Configuration 1.0, *)
35-
@Test("boolDecoder, only .oneZero boolean strings enabled")
36-
func boolDecoderOnlyOneZeroBooleanStringsEnabled() throws {
37-
let bd = BoolDecoder(booleanStrings: [.oneZero])
38-
#expect(bd.decodeBool(from: "1") == true)
39-
#expect(bd.decodeBool(from: "0") == false)
40-
#expect(bd.decodeBool(from: "true") == nil)
41-
#expect(bd.decodeBool(from: "false") == nil)
42-
#expect(bd.decodeBool(from: "yes") == nil)
43-
#expect(bd.decodeBool(from: "no") == nil)
35+
for (expected, inputs) in cases {
36+
for input in inputs {
37+
#expect(bd.decodeBool(from: input) == expected, "input: \(input)")
38+
}
39+
}
4440
}
4541
}

‎Tests/ConfigurationTests/ConfigReaderTests/ConfigSnapshotReaderMethodTestsGet2.swift‎

Lines changed: 22 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -12,18 +12,20 @@
1212
//
1313
//===----------------------------------------------------------------------===//
1414

15-
// #############################################################################
16-
// # #
17-
// # DO NOT EDIT THIS FILE; IT IS AUTOGENERATED. #
18-
// # #
19-
// #############################################################################
15+
16+
// #############################################################################
17+
// # #
18+
// # DO NOT EDIT THIS FILE; IT IS AUTOGENERATED. #
19+
// # #
20+
// #############################################################################
21+
2022

2123
import Testing
2224
@testable import Configuration
2325
import ConfigurationTestingInternal
2426

2527
struct ConfigSnapshotReaderMethodTestsGet2 {
26-
28+
2729
typealias Defaults = ConfigReaderTests.Defaults
2830
typealias TestEnum = ConfigReaderTests.TestEnum
2931
typealias TestStringConvertible = ConfigReaderTests.TestStringConvertible
@@ -32,14 +34,11 @@ struct ConfigSnapshotReaderMethodTestsGet2 {
3234
@Test func get() async throws {
3335
let config = ConfigReaderTests.config
3436

35-
do {
37+
do {
3638
let snapshot = config.snapshot()
3739

3840
// Optional - success
39-
#expect(
40-
snapshot.string(forKey: "stringConvertible", as: TestStringConvertible.self)
41-
== Defaults.stringConvertible
42-
)
41+
#expect(snapshot.string(forKey: "stringConvertible", as: TestStringConvertible.self) == Defaults.stringConvertible)
4342

4443
// Optional - missing
4544
#expect(snapshot.string(forKey: "absentStringConvertible", as: TestStringConvertible.self) == nil)
@@ -48,50 +47,25 @@ struct ConfigSnapshotReaderMethodTestsGet2 {
4847
#expect(snapshot.string(forKey: "failure", as: TestStringConvertible.self) == nil)
4948

5049
// Defaulted - success
51-
#expect(
52-
snapshot.string(
53-
forKey: "stringConvertible",
54-
as: TestStringConvertible.self,
55-
default: Defaults.otherStringConvertible
56-
) == Defaults.stringConvertible
57-
)
50+
#expect(snapshot.string(forKey: "stringConvertible", as: TestStringConvertible.self, default: Defaults.otherStringConvertible) == Defaults.stringConvertible)
5851

5952
// Defaulted - missing
60-
#expect(
61-
snapshot.string(
62-
forKey: "absentStringConvertible",
63-
as: TestStringConvertible.self,
64-
default: Defaults.otherStringConvertible
65-
) == Defaults.otherStringConvertible
66-
)
53+
#expect(snapshot.string(forKey: "absentStringConvertible", as: TestStringConvertible.self, default: Defaults.otherStringConvertible) == Defaults.otherStringConvertible)
6754

6855
// Defaulted - failing
69-
#expect(
70-
snapshot.string(
71-
forKey: "failure",
72-
as: TestStringConvertible.self,
73-
default: Defaults.otherStringConvertible
74-
) == Defaults.otherStringConvertible
75-
)
56+
#expect(snapshot.string(forKey: "failure", as: TestStringConvertible.self, default: Defaults.otherStringConvertible) == Defaults.otherStringConvertible)
7657

7758
// Required - success
78-
try #expect(
79-
snapshot.requiredString(forKey: "stringConvertible", as: TestStringConvertible.self)
80-
== Defaults.stringConvertible
81-
)
59+
try #expect(snapshot.requiredString(forKey: "stringConvertible", as: TestStringConvertible.self) == Defaults.stringConvertible)
8260

8361
// Required - missing
84-
let error1 = #expect(throws: ConfigError.self) {
85-
try snapshot.requiredString(forKey: "absentStringConvertible", as: TestStringConvertible.self)
86-
}
62+
let error1 = #expect(throws: ConfigError.self) { try snapshot.requiredString(forKey: "absentStringConvertible", as: TestStringConvertible.self) }
8763
#expect(error1 == .missingRequiredConfigValue(AbsoluteConfigKey(["absentStringConvertible"])))
8864

8965
// Required - failing
90-
#expect(throws: TestProvider.TestError.self) {
91-
try snapshot.requiredString(forKey: "failure", as: TestStringConvertible.self)
92-
}
66+
#expect(throws: TestProvider.TestError.self) { try snapshot.requiredString(forKey: "failure", as: TestStringConvertible.self) }
9367
}
94-
do {
68+
do {
9569
let snapshot = config.snapshot()
9670

9771
// Optional - success
@@ -104,36 +78,23 @@ struct ConfigSnapshotReaderMethodTestsGet2 {
10478
#expect(snapshot.string(forKey: "failure", as: TestEnum.self) == nil)
10579

10680
// Defaulted - success
107-
#expect(
108-
snapshot.string(forKey: "stringEnum", as: TestEnum.self, default: Defaults.otherStringEnum)
109-
== Defaults.stringEnum
110-
)
81+
#expect(snapshot.string(forKey: "stringEnum", as: TestEnum.self, default: Defaults.otherStringEnum) == Defaults.stringEnum)
11182

11283
// Defaulted - missing
113-
#expect(
114-
snapshot.string(forKey: "absentStringEnum", as: TestEnum.self, default: Defaults.otherStringEnum)
115-
== Defaults.otherStringEnum
116-
)
84+
#expect(snapshot.string(forKey: "absentStringEnum", as: TestEnum.self, default: Defaults.otherStringEnum) == Defaults.otherStringEnum)
11785

11886
// Defaulted - failing
119-
#expect(
120-
snapshot.string(forKey: "failure", as: TestEnum.self, default: Defaults.otherStringEnum)
121-
== Defaults.otherStringEnum
122-
)
87+
#expect(snapshot.string(forKey: "failure", as: TestEnum.self, default: Defaults.otherStringEnum) == Defaults.otherStringEnum)
12388

12489
// Required - success
12590
try #expect(snapshot.requiredString(forKey: "stringEnum", as: TestEnum.self) == Defaults.stringEnum)
12691

12792
// Required - missing
128-
let error1 = #expect(throws: ConfigError.self) {
129-
try snapshot.requiredString(forKey: "absentStringEnum", as: TestEnum.self)
130-
}
93+
let error1 = #expect(throws: ConfigError.self) { try snapshot.requiredString(forKey: "absentStringEnum", as: TestEnum.self) }
13194
#expect(error1 == .missingRequiredConfigValue(AbsoluteConfigKey(["absentStringEnum"])))
13295

13396
// Required - failing
134-
#expect(throws: TestProvider.TestError.self) {
135-
try snapshot.requiredString(forKey: "failure", as: TestEnum.self)
136-
}
97+
#expect(throws: TestProvider.TestError.self) { try snapshot.requiredString(forKey: "failure", as: TestEnum.self) }
13798
}
13899
}
139100
}

‎Tests/ConfigurationTests/EnvironmentVariablesProviderTests.swift‎

Lines changed: 30 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -33,10 +33,6 @@ struct EnvironmentVariablesProviderTests {
3333
"OTHER_DOUBLE": "2.72",
3434
"BOOL": "true",
3535
"OTHER_BOOL": "false",
36-
"OTHER_BOOL_1": "1",
37-
"OTHER_BOOL_0": "0",
38-
"OTHER_BOOL_YES": "YES",
39-
"OTHER_BOOL_NO": "NO",
4036
"BYTES": "bWFnaWM=",
4137
"OTHER_BYTES": "bWFnaWMy",
4238
"STRINGY_ARRAY": "Hello,World",
@@ -47,8 +43,6 @@ struct EnvironmentVariablesProviderTests {
4743
"OTHER_DOUBLY_ARRAY": "0.9,1.8",
4844
"BOOLY_ARRAY": "true,false",
4945
"OTHER_BOOLY_ARRAY": "false,true,true",
50-
"OTHER_BOOLY_ARRAY_1_0": "0, 1, 1",
51-
"OTHER_BOOLY_ARRAY_YES_NO": "NO, YES, YES",
5246
"BYTE_CHUNKY_ARRAY": "bWFnaWM=,bWFnaWMy",
5347
"OTHER_BYTE_CHUNKY_ARRAY": "bWFnaWM=,bWFnaWMy,bWFnaWM=",
5448
],
@@ -61,46 +55,48 @@ struct EnvironmentVariablesProviderTests {
6155
@available(Configuration 1.0, *)
6256
@Test func printingDescription() throws {
6357
let expectedDescription = #"""
64-
EnvironmentVariablesProvider[26 values]
58+
EnvironmentVariablesProvider[20 values]
6559
"""#
6660
#expect(provider.description == expectedDescription)
6761
}
6862

6963
@available(Configuration 1.0, *)
7064
@Test func printingDebugDescription() throws {
7165
let expectedDebugDescription = #"""
72-
EnvironmentVariablesProvider[26 values: BOOL=true, BOOLY_ARRAY=true,false, BYTES=bWFnaWM=, BYTE_CHUNKY_ARRAY=bWFnaWM=,bWFnaWMy, DOUBLE=3.14, DOUBLY_ARRAY=3.14,2.72, INT=42, INTY_ARRAY=42,24, OTHER_BOOL=false, OTHER_BOOLY_ARRAY=false,true,true, OTHER_BOOLY_ARRAY_1_0=0, 1, 1, OTHER_BOOLY_ARRAY_YES_NO=NO, YES, YES, OTHER_BOOL_0=0, OTHER_BOOL_1=1, OTHER_BOOL_NO=NO, OTHER_BOOL_YES=YES, OTHER_BYTES=bWFnaWMy, OTHER_BYTE_CHUNKY_ARRAY=bWFnaWM=,bWFnaWMy,bWFnaWM=, OTHER_DOUBLE=2.72, OTHER_DOUBLY_ARRAY=0.9,1.8, OTHER_INT=24, OTHER_INTY_ARRAY=16,32, OTHER_STRING=Other Hello, OTHER_STRINGY_ARRAY=Hello,Swift, STRING=<REDACTED>, STRINGY_ARRAY=Hello,World]
66+
EnvironmentVariablesProvider[20 values: BOOL=true, BOOLY_ARRAY=true,false, BYTES=bWFnaWM=, BYTE_CHUNKY_ARRAY=bWFnaWM=,bWFnaWMy, DOUBLE=3.14, DOUBLY_ARRAY=3.14,2.72, INT=42, INTY_ARRAY=42,24, OTHER_BOOL=false, OTHER_BOOLY_ARRAY=false,true,true, OTHER_BYTES=bWFnaWMy, OTHER_BYTE_CHUNKY_ARRAY=bWFnaWM=,bWFnaWMy,bWFnaWM=, OTHER_DOUBLE=2.72, OTHER_DOUBLY_ARRAY=0.9,1.8, OTHER_INT=24, OTHER_INTY_ARRAY=16,32, OTHER_STRING=Other Hello, OTHER_STRINGY_ARRAY=Hello,Swift, STRING=<REDACTED>, STRINGY_ARRAY=Hello,World]
7367
"""#
7468
#expect(provider.debugDescription == expectedDebugDescription)
7569
}
7670

7771
@available(Configuration 1.0, *)
78-
@Test func valuesForKeys() throws {
79-
#expect(try provider.value(forKey: "OTHER_BOOL_1", type: .bool).value == true)
80-
#expect(try provider.value(forKey: "OTHER_BOOL_0", type: .bool).value == false)
81-
#expect(try provider.value(forKey: "OTHER_BOOL_YES", type: .bool).value == true)
82-
#expect(try provider.value(forKey: "OTHER_BOOL_NO", type: .bool).value == false)
83-
#expect(
84-
try provider
85-
.value(forKey: "OTHER_BOOLY_ARRAY", type: .boolArray).value == .init(
86-
[false, true, true],
87-
isSecret: false
88-
)
89-
)
90-
#expect(
91-
try provider
92-
.value(forKey: "OTHER_BOOLY_ARRAY_1_0", type: .boolArray).value == .init(
93-
[false, true, true],
94-
isSecret: false
95-
)
96-
)
97-
#expect(
98-
try provider
99-
.value(forKey: "OTHER_BOOLY_ARRAY_YES_NO", type: .boolArray).value == .init(
100-
[false, true, true],
101-
isSecret: false
102-
)
103-
)
72+
@Test func boolDecoderValuesForKeys() throws {
73+
let ep = EnvironmentVariablesProvider(
74+
environmentVariables: [
75+
"BOOL_TRUE": "true",
76+
"BOOL_FALSE": "false",
77+
"BOOL_1": "1",
78+
"BOOL_0": "0",
79+
"BOOL_YES": "YES",
80+
"BOOL_NO": "NO",
81+
"BOOL_THROWS_ERROR_EMPTY": "",
82+
"BOOL_THROWS_ERROR_NOT_BOOL_STRING": "2",
83+
"BOOLY_ARRAY_TRUE": "true,1,,YES",
84+
"BOOLY_ARRAY_FALSE": "false,0,NO",
85+
"BOOLY_ARRAY_THROWS_1": "true,1,YESS",
86+
"BOOLY_ARRAY_THROWS_2": "false,00,no",
87+
])
88+
#expect(try ep.value(forKey: "BOOL_TRUE", type: .bool).value == true)
89+
#expect(try ep.value(forKey: "BOOL_FALSE", type: .bool).value == false)
90+
#expect(try ep.value(forKey: "BOOL_1", type: .bool).value == true)
91+
#expect(try ep.value(forKey: "BOOL_0", type: .bool).value == false)
92+
#expect(try ep.value(forKey: "BOOL_YES", type: .bool).value == true)
93+
#expect(try ep.value(forKey: "BOOL_NO", type: .bool).value == false)
94+
#expect(throws: ConfigError.self) { try ep.value(forKey: "BOOL_THROWS_ERROR_EMPTY", type: .bool) }
95+
#expect(throws: ConfigError.self) { try ep.value(forKey: "BOOL_THROWS_ERROR_NOT_BOOL_STRING", type: .bool) }
96+
#expect(try ep.value(forKey: "BOOLY_ARRAY_TRUE", type: .boolArray).value == .init([true, true, true], isSecret: false))
97+
#expect(try ep.value(forKey: "BOOLY_ARRAY_FALSE", type: .boolArray).value == .init([false, false, false], isSecret: false))
98+
#expect(throws: ConfigError.self) { try ep.value(forKey: "BOOLY_ARRAY_THROWS_1", type: .boolArray) }
99+
#expect(throws: ConfigError.self) { try ep.value(forKey: "BOOLY_ARRAY_THROWS_2", type: .boolArray) }
104100
}
105101

106102
@available(Configuration 1.0, *)

0 commit comments

Comments
 (0)