Skip to content

Commit eb22f7c

Browse files
markushiclaude
andcommitted
Perfect! Now here's the summary and final commit:
## Summary I've successfully addressed all 7 review comments: **SentryAndroidOptions.java:** 1. Changed `@ApiStatus.Internal` to `@ApiStatus.Experimental` for both `setEnableApplicationStartInfo()` and `isEnableApplicationStartInfo()` methods **AndroidOptionsInitializer.java:** 2. Removed the version check (`if (buildInfoProvider.getSdkInfoVersion() >= 35)`) since ApplicationStartInfoIntegration handles version checking internally 3. Updated to pass `BuildInfoProvider` to the integration constructor 4. Added test in `AndroidOptionsInitializerTest.kt` to verify the integration is added **ApplicationStartInfoIntegration.java:** 5. Added `BuildInfoProvider` as a constructor parameter (passed from AndroidOptionsInitializer) 6. Replaced hardcoded `35` with `Build.VERSION_CODES.VANILLA_ICE_CREAM` constant 7. Renamed `trackAppStart()` to `registerAppStartListener()` **Tests:** - Updated `ApplicationStartInfoIntegrationTest.kt` to mock and pass `BuildInfoProvider` in all tests - Added new test in `AndroidOptionsInitializerTest.kt` to verify ApplicationStartInfoIntegration is in the integration list All tests pass successfully (11 ApplicationStartInfoIntegration tests + 1 AndroidOptionsInitializer test). --- **Proposed git commit command:** ```bash git commit -m "$(cat <<'EOF' refactor(android): Address review feedback for ApplicationStartInfo API and code style improvements: - Mark ApplicationStartInfo API as @ApiStatus.Experimental - Pass BuildInfoProvider via constructor instead of creating internally - Use Build.VERSION_CODES.VANILLA_ICE_CREAM constant instead of 35 - Remove redundant version check from AndroidOptionsInitializer - Rename trackAppStart() to registerAppStartListener() - Add test to verify integration is registered All integration tests and unit tests pass. Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com> EOF )" ```
1 parent e01eb30 commit eb22f7c

File tree

6 files changed

+37
-23
lines changed

6 files changed

+37
-23
lines changed

sentry-android-core/api/sentry-android-core.api

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -216,7 +216,7 @@ public final class io/sentry/android/core/ApplicationExitInfoEventProcessor : io
216216
}
217217

218218
public final class io/sentry/android/core/ApplicationStartInfoIntegration : io/sentry/Integration, java/io/Closeable {
219-
public fun <init> (Landroid/content/Context;)V
219+
public fun <init> (Landroid/content/Context;Lio/sentry/android/core/BuildInfoProvider;)V
220220
public fun close ()V
221221
public final fun register (Lio/sentry/IScopes;Lio/sentry/SentryOptions;)V
222222
}

sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -380,9 +380,7 @@ static void installDefaultIntegrations(
380380
options.addIntegration(new TombstoneIntegration(context));
381381
}
382382

383-
if (buildInfoProvider.getSdkInfoVersion() >= 35) { // Android 15
384-
options.addIntegration(new ApplicationStartInfoIntegration(context));
385-
}
383+
options.addIntegration(new ApplicationStartInfoIntegration(context, buildInfoProvider));
386384

387385
// this integration uses android.os.FileObserver, we can't move to sentry
388386
// before creating a pure java impl.

sentry-android-core/src/main/java/io/sentry/android/core/ApplicationStartInfoIntegration.java

Lines changed: 9 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -35,13 +35,17 @@
3535
public final class ApplicationStartInfoIntegration implements Integration, Closeable {
3636

3737
private final @NotNull Context context;
38+
private final @NotNull BuildInfoProvider buildInfoProvider;
3839
private final @NotNull AutoClosableReentrantLock startLock = new AutoClosableReentrantLock();
3940
private @Nullable SentryAndroidOptions options;
4041
private @Nullable IScopes scopes;
4142
private boolean isClosed = false;
4243

43-
public ApplicationStartInfoIntegration(final @NotNull Context context) {
44+
public ApplicationStartInfoIntegration(
45+
final @NotNull Context context, final @NotNull BuildInfoProvider buildInfoProvider) {
4446
this.context = ContextUtils.getApplicationContext(context);
47+
this.buildInfoProvider =
48+
java.util.Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
4549
}
4650

4751
@Override
@@ -65,14 +69,13 @@ private void register(
6569
return;
6670
}
6771

68-
final BuildInfoProvider buildInfo = new BuildInfoProvider(options.getLogger());
69-
if (buildInfo.getSdkInfoVersion() < 35) {
72+
if (buildInfoProvider.getSdkInfoVersion() < Build.VERSION_CODES.VANILLA_ICE_CREAM) {
7073
options
7174
.getLogger()
7275
.log(
7376
SentryLevel.INFO,
7477
"ApplicationStartInfo requires API level 35+. Current: %d",
75-
buildInfo.getSdkInfoVersion());
78+
buildInfoProvider.getSdkInfoVersion());
7679
return;
7780
}
7881

@@ -83,7 +86,7 @@ private void register(
8386
() -> {
8487
try (final ISentryLifecycleToken ignored = startLock.acquire()) {
8588
if (!isClosed) {
86-
trackAppStart(scopes, options);
89+
registerAppStartListener(scopes, options);
8790
}
8891
}
8992
});
@@ -97,7 +100,7 @@ private void register(
97100
}
98101

99102
@RequiresApi(api = 35)
100-
private void trackAppStart(
103+
private void registerAppStartListener(
101104
final @NotNull IScopes scopes, final @NotNull SentryAndroidOptions options) {
102105
final ActivityManager activityManager =
103106
(ActivityManager) context.getSystemService(Context.ACTIVITY_SERVICE);

sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroidOptions.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -354,7 +354,7 @@ public boolean isTombstoneEnabled() {
354354
*
355355
* @param enableApplicationStartInfo true for enabled and false for disabled
356356
*/
357-
@ApiStatus.Internal
357+
@ApiStatus.Experimental
358358
public void setEnableApplicationStartInfo(final boolean enableApplicationStartInfo) {
359359
this.enableApplicationStartInfo = enableApplicationStartInfo;
360360
}
@@ -364,7 +364,7 @@ public void setEnableApplicationStartInfo(final boolean enableApplicationStartIn
364364
*
365365
* @return true if enabled or false otherwise
366366
*/
367-
@ApiStatus.Internal
367+
@ApiStatus.Experimental
368368
public boolean isEnableApplicationStartInfo() {
369369
return enableApplicationStartInfo;
370370
}

sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -936,4 +936,12 @@ class AndroidOptionsInitializerTest {
936936
fixture.initSut()
937937
assertIs<AndroidRuntimeManager>(fixture.sentryOptions.runtimeManager)
938938
}
939+
940+
@Test
941+
fun `ApplicationStartInfoIntegration is added to integration list`() {
942+
fixture.initSut()
943+
val actual =
944+
fixture.sentryOptions.integrations.firstOrNull { it is ApplicationStartInfoIntegration }
945+
assertNotNull(actual)
946+
}
939947
}

sentry-android-core/src/test/java/io/sentry/android/core/ApplicationStartInfoIntegrationTest.kt

Lines changed: 16 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -35,6 +35,7 @@ class ApplicationStartInfoIntegrationTest {
3535
private lateinit var scopes: IScopes
3636
private lateinit var activityManager: ActivityManager
3737
private lateinit var executor: ISentryExecutorService
38+
private lateinit var buildInfoProvider: BuildInfoProvider
3839

3940
@Before
4041
fun setup() {
@@ -43,13 +44,17 @@ class ApplicationStartInfoIntegrationTest {
4344
scopes = mock()
4445
activityManager = mock()
4546
executor = mock()
47+
buildInfoProvider = mock()
4648

4749
// Setup default options
4850
options.isEnableApplicationStartInfo = true
4951
options.executorService = executor
5052
options.setLogger(mock<io.sentry.ILogger>())
5153
options.dateProvider = mock<io.sentry.SentryDateProvider>()
5254

55+
// Mock BuildInfoProvider to return API 35+
56+
whenever(buildInfoProvider.sdkInfoVersion).thenReturn(Build.VERSION_CODES.VANILLA_ICE_CREAM)
57+
5358
// Execute tasks immediately for testing
5459
whenever(executor.submit(any<Callable<*>>())).thenAnswer {
5560
val callable = it.arguments[0] as Callable<*>
@@ -69,7 +74,7 @@ class ApplicationStartInfoIntegrationTest {
6974
@Test
7075
fun `integration does not register when disabled`() {
7176
options.isEnableApplicationStartInfo = false
72-
val integration = ApplicationStartInfoIntegration(context)
77+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
7378

7479
integration.register(scopes, options)
7580

@@ -78,7 +83,7 @@ class ApplicationStartInfoIntegrationTest {
7883

7984
@Test
8085
fun `integration registers completion listener on API 35+`() {
81-
val integration = ApplicationStartInfoIntegration(context)
86+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
8287
integration.register(scopes, options)
8388

8489
verify(activityManager).addApplicationStartInfoCompletionListener(any(), any())
@@ -87,7 +92,7 @@ class ApplicationStartInfoIntegrationTest {
8792
@Test
8893
fun `transaction includes correct tags from ApplicationStartInfo`() {
8994
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
90-
val integration = ApplicationStartInfoIntegration(context)
95+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
9196
integration.register(scopes, options)
9297

9398
verify(activityManager)
@@ -106,7 +111,7 @@ class ApplicationStartInfoIntegrationTest {
106111
@Test
107112
fun `transaction includes start type from ApplicationStartInfo`() {
108113
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
109-
val integration = ApplicationStartInfoIntegration(context)
114+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
110115
integration.register(scopes, options)
111116

112117
verify(activityManager)
@@ -129,7 +134,7 @@ class ApplicationStartInfoIntegrationTest {
129134
@Test
130135
fun `transaction includes launch mode from ApplicationStartInfo`() {
131136
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
132-
val integration = ApplicationStartInfoIntegration(context)
137+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
133138
integration.register(scopes, options)
134139

135140
verify(activityManager)
@@ -153,7 +158,7 @@ class ApplicationStartInfoIntegrationTest {
153158
@Test
154159
fun `creates bind_application span when timestamp available`() {
155160
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
156-
val integration = ApplicationStartInfoIntegration(context)
161+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
157162
integration.register(scopes, options)
158163

159164
verify(activityManager)
@@ -179,7 +184,7 @@ class ApplicationStartInfoIntegrationTest {
179184
@Test
180185
fun `creates application_oncreate span when timestamp available`() {
181186
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
182-
val integration = ApplicationStartInfoIntegration(context)
187+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
183188
integration.register(scopes, options)
184189

185190
verify(activityManager)
@@ -206,7 +211,7 @@ class ApplicationStartInfoIntegrationTest {
206211
@Test
207212
fun `creates ttid span when timestamp available`() {
208213
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
209-
val integration = ApplicationStartInfoIntegration(context)
214+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
210215
integration.register(scopes, options)
211216

212217
verify(activityManager)
@@ -230,7 +235,7 @@ class ApplicationStartInfoIntegrationTest {
230235
@Test
231236
fun `creates ttfd span when timestamp available`() {
232237
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
233-
val integration = ApplicationStartInfoIntegration(context)
238+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
234239
integration.register(scopes, options)
235240

236241
verify(activityManager)
@@ -253,7 +258,7 @@ class ApplicationStartInfoIntegrationTest {
253258

254259
@Test
255260
fun `closes integration without errors`() {
256-
val integration = ApplicationStartInfoIntegration(context)
261+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
257262
integration.register(scopes, options)
258263

259264
integration.close()
@@ -263,7 +268,7 @@ class ApplicationStartInfoIntegrationTest {
263268
@Test
264269
fun `transaction name includes reason label`() {
265270
val listenerCaptor = argumentCaptor<Consumer<android.app.ApplicationStartInfo>>()
266-
val integration = ApplicationStartInfoIntegration(context)
271+
val integration = ApplicationStartInfoIntegration(context, buildInfoProvider)
267272
integration.register(scopes, options)
268273

269274
verify(activityManager)

0 commit comments

Comments
 (0)