From 8951cb4fcafa383e4bc9ef28aa925950557c8f7b Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 17 Nov 2022 17:22:44 +0100 Subject: [PATCH 1/7] Fix ensure all options are processed before integrations are loaded --- .../core/AndroidOptionsInitializer.java | 96 ++++++------------- .../io/sentry/android/core/SentryAndroid.java | 26 +++-- .../core/AndroidOptionsInitializerTest.kt | 39 ++++++-- .../core/AndroidTransactionProfilerTest.kt | 16 +++- .../android/core/SentryInitProviderTest.kt | 3 +- 5 files changed, 96 insertions(+), 84 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 0ecbbb9067d..b817908cfd8 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -7,6 +7,7 @@ import android.content.pm.PackageInfo; import android.content.res.AssetManager; import android.os.Build; +import androidx.annotation.NonNull; import io.sentry.ILogger; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; @@ -24,6 +25,7 @@ import java.util.Properties; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; +import org.jetbrains.annotations.TestOnly; /** * Android Options initializer, it reads configurations from AndroidManifest and sets to the @@ -41,35 +43,10 @@ private AndroidOptionsInitializer() {} * @param options the SentryAndroidOptions * @param context the Application context */ - static void init(final @NotNull SentryAndroidOptions options, final @NotNull Context context) { - Objects.requireNonNull(context, "The application context is required."); - Objects.requireNonNull(options, "The options object is required."); - - init(options, context, new AndroidLogger(), false, false); - } - - /** - * Init method of the Android Options initializer - * - * @param options the SentryAndroidOptions - * @param context the Application context - * @param logger the ILogger interface - * @param isFragmentAvailable whether the Fragment integration is available on the classpath - * @param isTimberAvailable whether the Timber integration is available on the classpath - */ - static void init( - final @NotNull SentryAndroidOptions options, - @NotNull Context context, - final @NotNull ILogger logger, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { - init( - options, - context, - logger, - new BuildInfoProvider(logger), - isFragmentAvailable, - isTimberAvailable); + static void loadDefaultAndMetadataOptions( + final @NotNull SentryAndroidOptions options, @NotNull Context context) { + final ILogger logger = new AndroidLogger(); + loadDefaultAndMetadataOptions(options, context, logger, new BuildInfoProvider(logger)); } /** @@ -79,45 +56,12 @@ static void init( * @param context the Application context * @param logger the ILogger interface * @param buildInfoProvider the BuildInfoProvider interface - * @param isFragmentAvailable whether the Fragment integration is available on the classpath - * @param isTimberAvailable whether the Timber integration is available on the classpath */ - static void init( + static void loadDefaultAndMetadataOptions( final @NotNull SentryAndroidOptions options, @NotNull Context context, final @NotNull ILogger logger, - final @NotNull BuildInfoProvider buildInfoProvider, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { - init( - options, - context, - logger, - buildInfoProvider, - new LoadClass(), - isFragmentAvailable, - isTimberAvailable); - } - - /** - * Init method of the Android Options initializer - * - * @param options the SentryAndroidOptions - * @param context the Application context - * @param logger the ILogger interface - * @param buildInfoProvider the BuildInfoProvider interface - * @param loadClass the LoadClass wrapper - * @param isFragmentAvailable whether the Fragment integration is available on the classpath - * @param isTimberAvailable whether the Timber integration is available on the classpath - */ - static void init( - final @NotNull SentryAndroidOptions options, - @NotNull Context context, - final @NotNull ILogger logger, - final @NotNull BuildInfoProvider buildInfoProvider, - final @NotNull LoadClass loadClass, - final boolean isFragmentAvailable, - final boolean isTimberAvailable) { + final @NotNull BuildInfoProvider buildInfoProvider) { Objects.requireNonNull(context, "The context is required."); // it returns null if ContextImpl, so let's check for nullability @@ -135,6 +79,28 @@ static void init( initializeCacheDirs(context, options); options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); + readDefaultOptionValues(options, context, buildInfoProvider); + } + + @TestOnly + static void initializeIntegrationsAndProcessors( + @NonNull SentryAndroidOptions options, @NonNull Context context) { + initializeIntegrationsAndProcessors( + options, + context, + new BuildInfoProvider(new AndroidLogger()), + new LoadClass(), + false, + false); + } + + static void initializeIntegrationsAndProcessors( + @NonNull SentryAndroidOptions options, + @NonNull Context context, + @NonNull BuildInfoProvider buildInfoProvider, + @NonNull LoadClass loadClass, + boolean isFragmentAvailable, + boolean isTimberAvailable) { final ActivityFramesTracker activityFramesTracker = new ActivityFramesTracker(loadClass, options); @@ -147,8 +113,6 @@ static void init( isFragmentAvailable, isTimberAvailable); - readDefaultOptionValues(options, context, buildInfoProvider); - options.addEventProcessor( new DefaultAndroidEventProcessor(context, buildInfoProvider, options)); options.addEventProcessor(new PerformanceAndroidEventProcessor(options, activityFramesTracker)); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index 9bcc107d21a..a3b5b6fc090 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -102,11 +102,24 @@ public static synchronized void init( (isTimberUpstreamAvailable && classLoader.isClassAvailable(SENTRY_TIMBER_INTEGRATION_CLASS_NAME, options)); - AndroidOptionsInitializer.init( - options, context, logger, isFragmentAvailable, isTimberAvailable); + final BuildInfoProvider buildInfoProvider = new BuildInfoProvider(logger); + final LoadClass loadClass = new LoadClass(); + + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( + options, context, logger, buildInfoProvider); + configuration.configure(options); - deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable); resetEnvelopeCacheIfNeeded(options); + + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + options, + context, + buildInfoProvider, + loadClass, + isFragmentAvailable, + isTimberAvailable); + + deduplicateIntegrations(options, isFragmentAvailable, isTimberAvailable); }, true); } catch (IllegalAccessException e) { @@ -132,7 +145,8 @@ public static synchronized void init( /** * Deduplicate potentially duplicated Fragment and Timber integrations, which can be added - * automatically by our SDK as well as by the user. The user's ones win over ours. + * automatically by our SDK as well as by the user. The user's ones (provided first in the + * options.integrations list) win over ours. * * @param options SentryOptions to retrieve integrations from */ @@ -158,14 +172,14 @@ private static void deduplicateIntegrations( } if (fragmentIntegrations.size() > 1) { - for (int i = 0; i < fragmentIntegrations.size() - 1; i++) { + for (int i = 1; i < fragmentIntegrations.size(); i++) { final Integration integration = fragmentIntegrations.get(i); options.getIntegrations().remove(integration); } } if (timberIntegrations.size() > 1) { - for (int i = 0; i < timberIntegrations.size() - 1; i++) { + for (int i = 1; i < timberIntegrations.size(); i++) { final Integration integration = timberIntegrations.get(i); options.getIntegrations().remove(integration); } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index e760dadee80..5d5e4820a0c 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -52,8 +52,12 @@ class AndroidOptionsInitializerTest { whenever(mockContext.applicationContext.cacheDir).thenReturn(file) } mockContext.configureContext() + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( + sentryOptions, + if (useRealContext) context else mockContext + ) sentryOptions.configureOptions() - AndroidOptionsInitializer.init( + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( sentryOptions, if (useRealContext) context else mockContext ) @@ -71,12 +75,19 @@ class AndroidOptionsInitializerTest { putString(ManifestMetadataReader.DSN, "https://key@sentry.io/123") } ) - sentryOptions.setDebug(true) - AndroidOptionsInitializer.init( + sentryOptions.isDebug = true + val buildInfo = createBuildInfo(minApi) + + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( sentryOptions, mockContext, logger, - createBuildInfo(minApi), + buildInfo + ) + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + sentryOptions, + mockContext, + buildInfo, createClassMock(classToLoad), isFragmentAvailable, isTimberAvailable @@ -381,9 +392,13 @@ class AndroidOptionsInitializerTest { @Test fun `When Activity Frames Tracking is enabled, the Activity Frames Tracker should be available`() { - fixture.initSut(hasAppContext = true, configureOptions = { - isEnableFramesTracking = true - }) + fixture.initSut( + hasAppContext = true, + useRealContext = true, + configureOptions = { + isEnableFramesTracking = true + } + ) val activityLifeCycleIntegration = fixture.sentryOptions.integrations .first { it is ActivityLifecycleIntegration } @@ -410,9 +425,13 @@ class AndroidOptionsInitializerTest { @Test fun `When Frames Tracking is initially disabled, but enabled via configureOptions it should be available`() { fixture.sentryOptions.isEnableFramesTracking = false - fixture.initSut(hasAppContext = true, configureOptions = { - isEnableFramesTracking = true - }) + fixture.initSut( + hasAppContext = true, + useRealContext = true, + configureOptions = { + isEnableFramesTracking = true + } + ) val activityLifeCycleIntegration = fixture.sentryOptions.integrations .first { it is ActivityLifecycleIntegration } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt index bbdc8dc5257..b7b5f64e2e4 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidTransactionProfilerTest.kt @@ -71,7 +71,21 @@ class AndroidTransactionProfilerTest { @BeforeTest fun `set up`() { context = ApplicationProvider.getApplicationContext() - AndroidOptionsInitializer.init(fixture.options, context, fixture.mockLogger, false, false) + val buildInfoProvider = BuildInfoProvider(fixture.mockLogger) + AndroidOptionsInitializer.loadDefaultAndMetadataOptions( + fixture.options, + context, + fixture.mockLogger, + buildInfoProvider + ) + AndroidOptionsInitializer.initializeIntegrationsAndProcessors( + fixture.options, + context, + buildInfoProvider, + LoadClass(), + false, + false + ) // Profiler doesn't start if the folder doesn't exists. // Usually it's generated when calling Sentry.init, but for tests we can create it manually. File(fixture.options.profilingTracesDirPath!!).mkdirs() diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt index a54f39e5684..990d83b142f 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryInitProviderTest.kt @@ -132,7 +132,8 @@ class SentryInitProviderTest { val mockContext = ContextUtilsTest.mockMetaData(metaData = metaData) metaData.putBoolean(ManifestMetadataReader.NDK_ENABLE, false) - AndroidOptionsInitializer.init(sentryOptions, mockContext) + AndroidOptionsInitializer.loadDefaultAndMetadataOptions(sentryOptions, mockContext) + AndroidOptionsInitializer.initializeIntegrationsAndProcessors(sentryOptions, mockContext) assertFalse(sentryOptions.isEnableNdk) } From d3d54d7b8e063143272c918bf6067ad60a6405dd Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 17 Nov 2022 17:42:01 +0100 Subject: [PATCH 2/7] Fix tests relying on a real context --- .../io/sentry/android/core/AndroidOptionsInitializerTest.kt | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt index 5d5e4820a0c..f3bc66b7f16 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/AndroidOptionsInitializerTest.kt @@ -80,13 +80,13 @@ class AndroidOptionsInitializerTest { AndroidOptionsInitializer.loadDefaultAndMetadataOptions( sentryOptions, - mockContext, + context, logger, buildInfo ) AndroidOptionsInitializer.initializeIntegrationsAndProcessors( sentryOptions, - mockContext, + context, buildInfo, createClassMock(classToLoad), isFragmentAvailable, @@ -410,7 +410,7 @@ class AndroidOptionsInitializerTest { @Test fun `When Frames Tracking is disabled, the Activity Frames Tracker should not be available`() { - fixture.initSut(hasAppContext = true, configureOptions = { + fixture.initSut(hasAppContext = true, useRealContext = true, configureOptions = { isEnableFramesTracking = false }) From 79826748166b7eab356247bdf0f0942db93b5935 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Thu, 17 Nov 2022 17:53:32 +0100 Subject: [PATCH 3/7] Update Changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5588cdda145..b59a7f4a9c8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,11 @@ # Changelog +## Unreleased + +### Fixes + +- Fix ensure all options are processed before integrations are loaded ([#2377](https://github.com/getsentry/sentry-java/pull/2377)) + ## 6.7.1 ### Fixes From c1c47a2c537d21cc4dc00a3b5f22b0c93dfea7bd Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 18 Nov 2022 07:26:47 +0100 Subject: [PATCH 4/7] Add missing TestOnly annotation --- .../java/io/sentry/android/core/AndroidOptionsInitializer.java | 1 + 1 file changed, 1 insertion(+) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index b817908cfd8..42eb3893cd5 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -43,6 +43,7 @@ private AndroidOptionsInitializer() {} * @param options the SentryAndroidOptions * @param context the Application context */ + @TestOnly static void loadDefaultAndMetadataOptions( final @NotNull SentryAndroidOptions options, @NotNull Context context) { final ILogger logger = new AndroidLogger(); From 857ba73a24f4e2686e26be68f367de973882a850 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 18 Nov 2022 10:23:07 +0100 Subject: [PATCH 5/7] Fix mark parameters as final and use correct NotNull annotation --- .../android/core/AndroidOptionsInitializer.java | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index 42eb3893cd5..fa9425fcf6e 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -7,7 +7,6 @@ import android.content.pm.PackageInfo; import android.content.res.AssetManager; import android.os.Build; -import androidx.annotation.NonNull; import io.sentry.ILogger; import io.sentry.SendFireAndForgetEnvelopeSender; import io.sentry.SendFireAndForgetOutboxSender; @@ -45,7 +44,7 @@ private AndroidOptionsInitializer() {} */ @TestOnly static void loadDefaultAndMetadataOptions( - final @NotNull SentryAndroidOptions options, @NotNull Context context) { + final @NotNull SentryAndroidOptions options, final @NotNull Context context) { final ILogger logger = new AndroidLogger(); loadDefaultAndMetadataOptions(options, context, logger, new BuildInfoProvider(logger)); } @@ -85,7 +84,7 @@ static void loadDefaultAndMetadataOptions( @TestOnly static void initializeIntegrationsAndProcessors( - @NonNull SentryAndroidOptions options, @NonNull Context context) { + final @NotNull SentryAndroidOptions options, final @NotNull Context context) { initializeIntegrationsAndProcessors( options, context, @@ -96,12 +95,12 @@ static void initializeIntegrationsAndProcessors( } static void initializeIntegrationsAndProcessors( - @NonNull SentryAndroidOptions options, - @NonNull Context context, - @NonNull BuildInfoProvider buildInfoProvider, - @NonNull LoadClass loadClass, - boolean isFragmentAvailable, - boolean isTimberAvailable) { + final @NotNull SentryAndroidOptions options, + final @NotNull Context context, + final @NotNull BuildInfoProvider buildInfoProvider, + final @NotNull LoadClass loadClass, + final boolean isFragmentAvailable, + final boolean isTimberAvailable) { final ActivityFramesTracker activityFramesTracker = new ActivityFramesTracker(loadClass, options); From 4800ae9efde9cbaf5f033fc80dad2b9c96f8f263 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 18 Nov 2022 11:45:12 +0100 Subject: [PATCH 6/7] Fix envelopeDiskCache not being properly updated --- .../api/sentry-android-core.api | 1 + .../core/AndroidOptionsInitializer.java | 8 ++++++- .../core/cache/AndroidEnvelopeCache.java | 6 +++++ .../sentry/android/core/SentryAndroidTest.kt | 23 +++++++++++++++++++ 4 files changed, 37 insertions(+), 1 deletion(-) diff --git a/sentry-android-core/api/sentry-android-core.api b/sentry-android-core/api/sentry-android-core.api index 4bdcfecd270..2767a3738d6 100644 --- a/sentry-android-core/api/sentry-android-core.api +++ b/sentry-android-core/api/sentry-android-core.api @@ -224,6 +224,7 @@ public final class io/sentry/android/core/UserInteractionIntegration : android/a public final class io/sentry/android/core/cache/AndroidEnvelopeCache : io/sentry/cache/EnvelopeCache { public fun (Lio/sentry/android/core/SentryAndroidOptions;)V + public fun getDirectory ()Ljava/io/File; public static fun hasStartupCrashMarker (Lio/sentry/SentryOptions;)Z public fun store (Lio/sentry/SentryEnvelope;Lio/sentry/Hint;)V } diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java index fa9425fcf6e..069f657fbd3 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/AndroidOptionsInitializer.java @@ -15,6 +15,7 @@ import io.sentry.android.core.internal.modules.AssetsModulesLoader; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; +import io.sentry.transport.NoOpEnvelopeCache; import io.sentry.util.Objects; import java.io.BufferedInputStream; import java.io.File; @@ -77,7 +78,6 @@ static void loadDefaultAndMetadataOptions( ManifestMetadataReader.applyMetadata(context, options, buildInfoProvider); initializeCacheDirs(context, options); - options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); readDefaultOptionValues(options, context, buildInfoProvider); } @@ -101,6 +101,12 @@ static void initializeIntegrationsAndProcessors( final @NotNull LoadClass loadClass, final boolean isFragmentAvailable, final boolean isTimberAvailable) { + + if (options.getCacheDirPath() != null + && options.getEnvelopeDiskCache() instanceof NoOpEnvelopeCache) { + options.setEnvelopeDiskCache(new AndroidEnvelopeCache(options)); + } + final ActivityFramesTracker activityFramesTracker = new ActivityFramesTracker(loadClass, options); diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java index e08ef8b1281..1104adadd47 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java @@ -17,6 +17,7 @@ import java.io.File; import org.jetbrains.annotations.ApiStatus; import org.jetbrains.annotations.NotNull; +import org.jetbrains.annotations.TestOnly; @ApiStatus.Internal public final class AndroidEnvelopeCache extends EnvelopeCache { @@ -58,6 +59,11 @@ public void store(@NotNull SentryEnvelope envelope, @NotNull Hint hint) { } } + @TestOnly + public @NotNull File getDirectory() { + return directory; + } + private void writeStartupCrashMarkerFile() { // we use outbox path always, as it's the one that will also contain markers if hybrid sdks // decide to write it, which will trigger the blocking init diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt index a6d4f6cddfe..ebbfd13b1f3 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/SentryAndroidTest.kt @@ -10,16 +10,21 @@ import io.sentry.SentryEnvelope import io.sentry.SentryLevel import io.sentry.SentryLevel.DEBUG import io.sentry.SentryLevel.FATAL +import io.sentry.SentryOptions +import io.sentry.android.core.cache.AndroidEnvelopeCache import io.sentry.android.fragment.FragmentLifecycleIntegration import io.sentry.android.timber.SentryTimberIntegration import io.sentry.cache.IEnvelopeCache import io.sentry.transport.NoOpEnvelopeCache +import io.sentry.util.StringUtils import org.junit.runner.RunWith import org.mockito.kotlin.any import org.mockito.kotlin.eq import org.mockito.kotlin.mock import org.mockito.kotlin.never import org.mockito.kotlin.verify +import java.nio.file.Files +import kotlin.io.path.absolutePathString import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals @@ -163,6 +168,24 @@ class SentryAndroidTest { assertTrue { refOptions!!.envelopeDiskCache is CustomEnvelopCache } } + @Test + fun `When initializing Sentry manually and changing both cache dir and dsn, the corresponding options should reflect that change`() { + var options: SentryOptions? = null + + val mockContext = ContextUtilsTest.createMockContext(true) + val cacheDirPath = Files.createTempDirectory("new_cache").absolutePathString() + SentryAndroid.init(mockContext) { + it.dsn = "https://key@sentry.io/123" + it.cacheDirPath = cacheDirPath + options = it + } + + val dsnHash = StringUtils.calculateStringHash(options!!.dsn, options!!.logger) + val expectedCacheDir = "$cacheDirPath/$dsnHash" + assertEquals(expectedCacheDir, options!!.cacheDirPath) + assertEquals(expectedCacheDir, (options!!.envelopeDiskCache as AndroidEnvelopeCache).directory.absolutePath) + } + private class CustomEnvelopCache : IEnvelopeCache { override fun iterator(): MutableIterator = TODO() override fun store(envelope: SentryEnvelope, hint: Hint) = Unit From 484701face90e55dad1ff52104d8cb3880a12079 Mon Sep 17 00:00:00 2001 From: Markus Hintersteiner Date: Fri, 18 Nov 2022 13:42:54 +0100 Subject: [PATCH 7/7] Remove obsolete resetEnvelopeCacheIfNeeded --- .../io/sentry/android/core/SentryAndroid.java | 17 ----------------- 1 file changed, 17 deletions(-) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java index a3b5b6fc090..e9e8a75c1fb 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/SentryAndroid.java @@ -9,10 +9,8 @@ import io.sentry.Sentry; import io.sentry.SentryLevel; import io.sentry.SentryOptions; -import io.sentry.android.core.cache.AndroidEnvelopeCache; import io.sentry.android.fragment.FragmentLifecycleIntegration; import io.sentry.android.timber.SentryTimberIntegration; -import io.sentry.transport.NoOpEnvelopeCache; import java.lang.reflect.InvocationTargetException; import java.util.ArrayList; import java.util.Date; @@ -109,7 +107,6 @@ public static synchronized void init( options, context, logger, buildInfoProvider); configuration.configure(options); - resetEnvelopeCacheIfNeeded(options); AndroidOptionsInitializer.initializeIntegrationsAndProcessors( options, @@ -185,18 +182,4 @@ private static void deduplicateIntegrations( } } } - - /** - * Resets envelope cache if {@link SentryOptions#getCacheDirPath()} was set to null by the user - * and the IEnvelopCache implementation remained ours (AndroidEnvelopeCache), which relies on - * cacheDirPath set. - * - * @param options SentryOptions to retrieve cacheDirPath from - */ - private static void resetEnvelopeCacheIfNeeded(final @NotNull SentryAndroidOptions options) { - if (options.getCacheDirPath() == null - && options.getEnvelopeDiskCache() instanceof AndroidEnvelopeCache) { - options.setEnvelopeDiskCache(NoOpEnvelopeCache.getInstance()); - } - } }