-
-
Notifications
You must be signed in to change notification settings - Fork 459
Add support for reading distribution options from properties #4784
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -17,6 +17,7 @@ public static void apply( | |
if (debugMetaProperties != null) { | ||
applyToOptions(options, debugMetaProperties); | ||
applyBuildTool(options, debugMetaProperties); | ||
applyDistributionOptions(options, debugMetaProperties); | ||
} | ||
} | ||
|
||
|
@@ -89,4 +90,79 @@ private static void applyBuildTool( | |
final @NotNull Properties debugMetaProperties) { | ||
return debugMetaProperties.getProperty("io.sentry.build-tool-version"); | ||
} | ||
|
||
private static void applyDistributionOptions( | ||
final @NotNull SentryOptions options, final @NotNull List<Properties> debugMetaProperties) { | ||
for (Properties properties : debugMetaProperties) { | ||
final @Nullable String orgSlug = getDistributionOrgSlug(properties); | ||
final @Nullable String projectSlug = getDistributionProjectSlug(properties); | ||
final @Nullable String orgAuthToken = getDistributionOrgAuthToken(properties); | ||
final @Nullable String buildConfiguration = getDistributionBuildConfiguration(properties); | ||
|
||
if (orgSlug != null | ||
|| projectSlug != null | ||
|| orgAuthToken != null | ||
|| buildConfiguration != null) { | ||
final @NotNull SentryOptions.DistributionOptions distributionOptions = | ||
options.getDistribution(); | ||
|
||
if (orgSlug != null && !orgSlug.isEmpty() && distributionOptions.orgSlug.isEmpty()) { | ||
options.getLogger().log(SentryLevel.DEBUG, "Distribution org slug found: %s", orgSlug); | ||
distributionOptions.orgSlug = orgSlug; | ||
} | ||
|
||
if (projectSlug != null | ||
&& !projectSlug.isEmpty() | ||
&& distributionOptions.projectSlug.isEmpty()) { | ||
options | ||
.getLogger() | ||
.log(SentryLevel.DEBUG, "Distribution project slug found: %s", projectSlug); | ||
distributionOptions.projectSlug = projectSlug; | ||
} | ||
|
||
if (orgAuthToken != null | ||
&& !orgAuthToken.isEmpty() | ||
&& distributionOptions.orgAuthToken.isEmpty()) { | ||
options.getLogger().log(SentryLevel.DEBUG, "Distribution org auth token found"); | ||
distributionOptions.orgAuthToken = orgAuthToken; | ||
} | ||
|
||
if (buildConfiguration != null | ||
&& !buildConfiguration.isEmpty() | ||
&& distributionOptions.buildConfiguration == null) { | ||
options | ||
.getLogger() | ||
.log( | ||
SentryLevel.DEBUG, | ||
"Distribution build configuration found: %s", | ||
buildConfiguration); | ||
distributionOptions.buildConfiguration = buildConfiguration; | ||
} | ||
|
||
// We only process the first properties file that contains distribution options | ||
// to maintain consistency with other properties like proguardUuid | ||
break; | ||
} | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Loop Exits Early on Distribution PropertyThe |
||
} | ||
|
||
private static @Nullable String getDistributionOrgSlug( | ||
final @NotNull Properties debugMetaProperties) { | ||
return debugMetaProperties.getProperty("io.sentry.distribution.org-slug"); | ||
} | ||
|
||
private static @Nullable String getDistributionProjectSlug( | ||
final @NotNull Properties debugMetaProperties) { | ||
return debugMetaProperties.getProperty("io.sentry.distribution.project-slug"); | ||
} | ||
|
||
private static @Nullable String getDistributionOrgAuthToken( | ||
final @NotNull Properties debugMetaProperties) { | ||
return debugMetaProperties.getProperty("io.sentry.distribution.org-auth-token"); | ||
} | ||
|
||
private static @Nullable String getDistributionBuildConfiguration( | ||
final @NotNull Properties debugMetaProperties) { | ||
return debugMetaProperties.getProperty("io.sentry.distribution.build-configuration"); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,133 @@ | ||
package io.sentry.util | ||
|
||
import io.sentry.SentryOptions | ||
import java.util.Properties | ||
import kotlin.test.Test | ||
import kotlin.test.assertEquals | ||
import kotlin.test.assertNull | ||
|
||
class DebugMetaPropertiesApplierTest { | ||
|
||
@Test | ||
fun `applies distribution options from properties`() { | ||
val properties = Properties() | ||
properties.setProperty("io.sentry.distribution.org-slug", "test-org") | ||
properties.setProperty("io.sentry.distribution.project-slug", "test-project") | ||
properties.setProperty("io.sentry.distribution.org-auth-token", "test-token") | ||
properties.setProperty("io.sentry.distribution.build-configuration", "debug") | ||
|
||
val options = SentryOptions() | ||
DebugMetaPropertiesApplier.apply(options, listOf(properties)) | ||
|
||
assertEquals("test-org", options.distribution.orgSlug) | ||
assertEquals("test-project", options.distribution.projectSlug) | ||
assertEquals("test-token", options.distribution.orgAuthToken) | ||
assertEquals("debug", options.distribution.buildConfiguration) | ||
} | ||
|
||
@Test | ||
fun `applies partial distribution options from properties`() { | ||
val properties = Properties() | ||
properties.setProperty("io.sentry.distribution.org-slug", "test-org") | ||
properties.setProperty("io.sentry.distribution.project-slug", "test-project") | ||
|
||
val options = SentryOptions() | ||
DebugMetaPropertiesApplier.apply(options, listOf(properties)) | ||
|
||
assertEquals("test-org", options.distribution.orgSlug) | ||
assertEquals("test-project", options.distribution.projectSlug) | ||
assertEquals("", options.distribution.orgAuthToken) | ||
assertNull(options.distribution.buildConfiguration) | ||
} | ||
|
||
@Test | ||
fun `does not override existing distribution options`() { | ||
val properties = Properties() | ||
properties.setProperty("io.sentry.distribution.org-slug", "properties-org") | ||
properties.setProperty("io.sentry.distribution.project-slug", "properties-project") | ||
properties.setProperty("io.sentry.distribution.org-auth-token", "properties-token") | ||
properties.setProperty("io.sentry.distribution.build-configuration", "properties-config") | ||
|
||
val options = SentryOptions() | ||
options.distribution.orgSlug = "existing-org" | ||
options.distribution.projectSlug = "existing-project" | ||
options.distribution.orgAuthToken = "existing-token" | ||
options.distribution.buildConfiguration = "existing-config" | ||
|
||
DebugMetaPropertiesApplier.apply(options, listOf(properties)) | ||
|
||
assertEquals("existing-org", options.distribution.orgSlug) | ||
assertEquals("existing-project", options.distribution.projectSlug) | ||
assertEquals("existing-token", options.distribution.orgAuthToken) | ||
assertEquals("existing-config", options.distribution.buildConfiguration) | ||
} | ||
|
||
@Test | ||
fun `applies distribution options from first properties file with values`() { | ||
val properties1 = Properties() | ||
val properties2 = Properties() | ||
properties2.setProperty("io.sentry.distribution.org-slug", "org-from-second") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You have There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Good point. Actually this is intentional, we only want to apply properties from the first file with values (which is why we have the break). We don't want to merge files. Although I'm happy to debate this behavior! |
||
properties2.setProperty("io.sentry.distribution.project-slug", "project-from-second") | ||
|
||
val options = SentryOptions() | ||
DebugMetaPropertiesApplier.apply(options, listOf(properties1, properties2)) | ||
|
||
assertEquals("org-from-second", options.distribution.orgSlug) | ||
assertEquals("project-from-second", options.distribution.projectSlug) | ||
} | ||
|
||
@Test | ||
fun `does nothing when properties list is empty`() { | ||
val options = SentryOptions() | ||
val originalOrgSlug = options.distribution.orgSlug | ||
val originalProjectSlug = options.distribution.projectSlug | ||
|
||
DebugMetaPropertiesApplier.apply(options, emptyList()) | ||
|
||
assertEquals(originalOrgSlug, options.distribution.orgSlug) | ||
assertEquals(originalProjectSlug, options.distribution.projectSlug) | ||
} | ||
|
||
@Test | ||
fun `does nothing when properties list is null`() { | ||
val options = SentryOptions() | ||
val originalOrgSlug = options.distribution.orgSlug | ||
val originalProjectSlug = options.distribution.projectSlug | ||
|
||
DebugMetaPropertiesApplier.apply(options, null) | ||
|
||
assertEquals(originalOrgSlug, options.distribution.orgSlug) | ||
assertEquals(originalProjectSlug, options.distribution.projectSlug) | ||
} | ||
|
||
@Test | ||
fun `applies buildConfiguration only when it is the only property set`() { | ||
val properties = Properties() | ||
properties.setProperty("io.sentry.distribution.build-configuration", "debug") | ||
|
||
val options = SentryOptions() | ||
DebugMetaPropertiesApplier.apply(options, listOf(properties)) | ||
|
||
assertEquals("debug", options.distribution.buildConfiguration) | ||
assertEquals("", options.distribution.orgSlug) | ||
assertEquals("", options.distribution.projectSlug) | ||
assertEquals("", options.distribution.orgAuthToken) | ||
} | ||
|
||
@Test | ||
fun `does not apply empty string values`() { | ||
val properties = Properties() | ||
properties.setProperty("io.sentry.distribution.org-slug", "") | ||
properties.setProperty("io.sentry.distribution.project-slug", "") | ||
properties.setProperty("io.sentry.distribution.org-auth-token", "") | ||
properties.setProperty("io.sentry.distribution.build-configuration", "") | ||
|
||
val options = SentryOptions() | ||
DebugMetaPropertiesApplier.apply(options, listOf(properties)) | ||
|
||
assertEquals("", options.distribution.orgSlug) | ||
assertEquals("", options.distribution.projectSlug) | ||
assertEquals("", options.distribution.orgAuthToken) | ||
assertNull(options.distribution.buildConfiguration) | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm confused about this
break;
do we not need to go though all properties?Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is a good question. I did this this to keep it consistent with the other functions in this file which also have a break. Example
This does the following:
- Apply those properties (only the non-empty ones that aren't already set)
- Then break out of the loop immediately
I'll add a comment here that explains this behavior. I'm happy to debate this behavior though! I think it would be confusing if the properties were overridden from different files.