Skip to content

Commit 1c7d553

Browse files
runningcodeclaude
andcommitted
Address PR review comments
- Include buildConfiguration in initial property check to fix bug where buildConfiguration-only properties would be skipped - Add isEmpty() checks for all property values before applying - Add comment explaining break statement (only process first properties file) - Add test for buildConfiguration-only scenario - Add test for empty string values 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
1 parent 47bad3a commit 1c7d553

File tree

2 files changed

+47
-5
lines changed

2 files changed

+47
-5
lines changed

sentry/src/main/java/io/sentry/util/DebugMetaPropertiesApplier.java

Lines changed: 16 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -99,28 +99,37 @@ private static void applyDistributionOptions(
9999
final @Nullable String orgAuthToken = getDistributionOrgAuthToken(properties);
100100
final @Nullable String buildConfiguration = getDistributionBuildConfiguration(properties);
101101

102-
if (orgSlug != null || projectSlug != null || orgAuthToken != null) {
102+
if (orgSlug != null
103+
|| projectSlug != null
104+
|| orgAuthToken != null
105+
|| buildConfiguration != null) {
103106
final @NotNull SentryOptions.DistributionOptions distributionOptions =
104107
options.getDistribution();
105108

106-
if (orgSlug != null && distributionOptions.orgSlug.isEmpty()) {
109+
if (orgSlug != null && !orgSlug.isEmpty() && distributionOptions.orgSlug.isEmpty()) {
107110
options.getLogger().log(SentryLevel.DEBUG, "Distribution org slug found: %s", orgSlug);
108111
distributionOptions.orgSlug = orgSlug;
109112
}
110113

111-
if (projectSlug != null && distributionOptions.projectSlug.isEmpty()) {
114+
if (projectSlug != null
115+
&& !projectSlug.isEmpty()
116+
&& distributionOptions.projectSlug.isEmpty()) {
112117
options
113118
.getLogger()
114119
.log(SentryLevel.DEBUG, "Distribution project slug found: %s", projectSlug);
115120
distributionOptions.projectSlug = projectSlug;
116121
}
117122

118-
if (orgAuthToken != null && distributionOptions.orgAuthToken.isEmpty()) {
123+
if (orgAuthToken != null
124+
&& !orgAuthToken.isEmpty()
125+
&& distributionOptions.orgAuthToken.isEmpty()) {
119126
options.getLogger().log(SentryLevel.DEBUG, "Distribution org auth token found");
120127
distributionOptions.orgAuthToken = orgAuthToken;
121128
}
122129

123-
if (buildConfiguration != null && distributionOptions.buildConfiguration == null) {
130+
if (buildConfiguration != null
131+
&& !buildConfiguration.isEmpty()
132+
&& distributionOptions.buildConfiguration == null) {
124133
options
125134
.getLogger()
126135
.log(
@@ -130,6 +139,8 @@ private static void applyDistributionOptions(
130139
distributionOptions.buildConfiguration = buildConfiguration;
131140
}
132141

142+
// We only process the first properties file that contains distribution options
143+
// to maintain consistency with other properties like proguardUuid
133144
break;
134145
}
135146
}

sentry/src/test/java/io/sentry/util/DebugMetaPropertiesApplierTest.kt

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -99,4 +99,35 @@ class DebugMetaPropertiesApplierTest {
9999
assertEquals(originalOrgSlug, options.distribution.orgSlug)
100100
assertEquals(originalProjectSlug, options.distribution.projectSlug)
101101
}
102+
103+
@Test
104+
fun `applies buildConfiguration only when it is the only property set`() {
105+
val properties = Properties()
106+
properties.setProperty("io.sentry.distribution.build-configuration", "debug")
107+
108+
val options = SentryOptions()
109+
DebugMetaPropertiesApplier.apply(options, listOf(properties))
110+
111+
assertEquals("debug", options.distribution.buildConfiguration)
112+
assertEquals("", options.distribution.orgSlug)
113+
assertEquals("", options.distribution.projectSlug)
114+
assertEquals("", options.distribution.orgAuthToken)
115+
}
116+
117+
@Test
118+
fun `does not apply empty string values`() {
119+
val properties = Properties()
120+
properties.setProperty("io.sentry.distribution.org-slug", "")
121+
properties.setProperty("io.sentry.distribution.project-slug", "")
122+
properties.setProperty("io.sentry.distribution.org-auth-token", "")
123+
properties.setProperty("io.sentry.distribution.build-configuration", "")
124+
125+
val options = SentryOptions()
126+
DebugMetaPropertiesApplier.apply(options, listOf(properties))
127+
128+
assertEquals("", options.distribution.orgSlug)
129+
assertEquals("", options.distribution.projectSlug)
130+
assertEquals("", options.distribution.orgAuthToken)
131+
assertNull(options.distribution.buildConfiguration)
132+
}
102133
}

0 commit comments

Comments
 (0)