Skip to content

Commit 4366cab

Browse files
committed
Apply PR suggestions round 3
1 parent ea8d9f7 commit 4366cab

File tree

2 files changed

+78
-12
lines changed

2 files changed

+78
-12
lines changed

internal-api/src/main/java/datadog/trace/bootstrap/config/provider/StableConfigParser.java

Lines changed: 10 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -31,37 +31,35 @@ public static StableConfigSource.StableConfig parse(String filePath) throws IOEx
3131
Map<String, String> configMap = new HashMap<>();
3232
String[] configId = new String[1];
3333
try (Stream<String> lines = Files.lines(Paths.get(filePath))) {
34-
// Use AtomicBoolean because it's mutable within a lambda expression to
35-
boolean[] apmConfigFound =
36-
new boolean[1]; // Track if we've found 'apm_configuration_default:'
37-
boolean[] configIdFound = new boolean[1]; // Track if we've found 'config_id:'
34+
int apmConfigNotFound = -1, apmConfigStarted = 0, apmConfigComplete = 1;
35+
int[] apmConfigFound = {apmConfigNotFound};
3836
lines.forEach(
3937
line -> {
4038
Matcher matcher = idPattern.matcher(line);
4139
if (matcher.find()) {
4240
// Do not allow duplicate config_id keys
43-
if (configIdFound[0]) {
41+
if (configId[0] != null) {
4442
throw new RuntimeException("Duplicate config_id keys found; file may be malformed");
4543
}
4644
configId[0] = trimQuotes(matcher.group(1).trim());
47-
configIdFound[0] = true;
4845
return; // go to next line
4946
}
50-
// TODO: Do not allow duplicate apm_configuration_default keys? Or just return early.
51-
if (!apmConfigFound[0] && apmConfigPattern.matcher(line).matches()) {
52-
apmConfigFound[0] = true;
47+
// TODO: Do not allow duplicate apm_configuration_default keys; and/or return early once
48+
// apmConfigFound[0] == apmConfigComplete
49+
if (apmConfigFound[0] == apmConfigNotFound
50+
&& apmConfigPattern.matcher(line).matches()) {
51+
apmConfigFound[0] = apmConfigStarted;
5352
return; // go to next line
5453
}
55-
56-
if (apmConfigFound[0]) {
54+
if (apmConfigFound[0] == apmConfigStarted) {
5755
Matcher keyValueMatcher = keyValPattern.matcher(line);
5856
if (keyValueMatcher.matches()) {
5957
configMap.put(
6058
keyValueMatcher.group(1).trim(),
6159
trimQuotes(keyValueMatcher.group(2).trim())); // Store key-value pair in map
6260
} else {
6361
// If we encounter a non-indented or non-key-value line, stop processing
64-
apmConfigFound[0] = false;
62+
apmConfigFound[0] = apmConfigComplete;
6563
}
6664
}
6765
});

internal-api/src/test/groovy/datadog/trace/bootstrap/config/provider/StableConfigParserTest.groovy

Lines changed: 68 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,4 +56,72 @@ something-else-irrelevant: value-irrelevant
5656
cfg.get("KEY_FIVE") == "[a,b,c,d]"
5757
Files.delete(filePath)
5858
}
59+
60+
def "test duplicate config_id"() {
61+
when:
62+
Path filePath = StableConfigSourceTest.tempFile()
63+
if (filePath == null) {
64+
throw new AssertionError("Failed to create test file")
65+
}
66+
String yaml = """
67+
config_id: 12345
68+
something-irrelevant: ""
69+
apm_configuration_default:
70+
DD_KEY: value
71+
config_id: 67890
72+
"""
73+
74+
try {
75+
StableConfigSourceTest.writeFileRaw(filePath, yaml)
76+
} catch (IOException e) {
77+
throw new AssertionError("Failed to write to file: ${e.message}")
78+
}
79+
80+
Exception exception
81+
StableConfigSource.StableConfig cfg
82+
try {
83+
cfg = StableConfigParser.parse(filePath.toString())
84+
} catch (Exception e) {
85+
exception = e
86+
}
87+
88+
then:
89+
cfg == null
90+
exception != null
91+
exception.getMessage() == "Duplicate config_id keys found; file may be malformed"
92+
}
93+
94+
def "test duplicate apm_configuration_default"() {
95+
// Assert that only the first entry is used
96+
when:
97+
Path filePath = StableConfigSourceTest.tempFile()
98+
if (filePath == null) {
99+
throw new AssertionError("Failed to create test file")
100+
}
101+
String yaml = """
102+
apm_configuration_default:
103+
KEY_1: value_1
104+
something-else-irrelevant: value-irrelevant
105+
apm_configuration_default:
106+
KEY_2: value_2
107+
"""
108+
try {
109+
StableConfigSourceTest.writeFileRaw(filePath, yaml)
110+
} catch (IOException e) {
111+
throw new AssertionError("Failed to write to file: ${e.message}")
112+
}
113+
114+
StableConfigSource.StableConfig cfg
115+
try {
116+
cfg = StableConfigParser.parse(filePath.toString())
117+
} catch (Exception e) {
118+
throw new AssertionError("Failed to parse the file: ${e.message}")
119+
}
120+
121+
then:
122+
def keys = cfg.getKeys()
123+
keys.size() == 1
124+
!keys.contains("KEY_2")
125+
cfg.get("KEY_1") == "value_1"
126+
}
59127
}

0 commit comments

Comments
 (0)