diff --git a/CHANGELOG.md b/CHANGELOG.md index 8cb55637577..c11ea0449f0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,12 @@ ## Unreleased +### Behavioral Changes + +- The outbox and cache directories are no longer created by `Sentry.init` ([#5792](https://github.com/getsentry/sentry-java/pull/5792)) + - They are now created lazily by whichever component first writes into them, off the init thread. As a result, the directories at `SentryOptions.getOutboxPath()` and `SentryOptions.getCacheDirPath()` are not guaranteed to exist once `Sentry.init` returns. + - If you write envelopes into the outbox path yourself instead of going through the SDK — as hybrid SDKs do for `captureEnvelope` — create the directory first, e.g. `new File(outboxPath).mkdirs()`. + ### Improvements - Skip building Android manifest metadata debug log messages when debug logging is disabled, reducing allocations during SDK init ([#5790](https://github.com/getsentry/sentry-java/pull/5790)) @@ -13,6 +19,7 @@ ### Performance +- Create the outbox and cache directories lazily in their consumers instead of during SDK init, moving the `mkdirs()` calls off the init (main) thread ([#5792](https://github.com/getsentry/sentry-java/pull/5792)) - Reduce the number of SDK threads: `LifecycleWatcher` now schedules the session-end task on the shared timer executor instead of creating a dedicated `java.util.Timer` thread ([#5819](https://github.com/getsentry/sentry-java/pull/5819)) - Speed up deserialization of arbitrary JSON objects by typing numbers without throwing exceptions ([#5783](https://github.com/getsentry/sentry-java/pull/5783)) diff --git a/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java b/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java index 482d90c6e6c..ab95ae32daa 100644 --- a/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java +++ b/sentry-android-core/src/main/java/io/sentry/android/core/EnvelopeFileObserverIntegration.java @@ -10,8 +10,10 @@ import io.sentry.SentryLevel; import io.sentry.SentryOptions; import io.sentry.util.AutoClosableReentrantLock; +import io.sentry.util.FileUtils; import io.sentry.util.Objects; import java.io.Closeable; +import java.io.File; import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; import org.jetbrains.annotations.TestOnly; @@ -67,6 +69,12 @@ private void startOutboxSender( final @NotNull IScopes scopes, final @NotNull SentryOptions options, final @NotNull String path) { + // Create the outbox dir here (on the executor) so the observer can watch it for envelopes + // written by hybrid SDKs, instead of blocking Sentry.init on the mkdirs. + if (!FileUtils.createDirectory(new File(path))) { + options.getLogger().log(SentryLevel.ERROR, "Failed to create outbox dir %s", path); + } + final OutboxSender outboxSender = new OutboxSender( scopes, 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 e1590e47943..1ef02dfdd2c 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 @@ -93,7 +93,7 @@ private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull @TestOnly public @NotNull File getDirectory() { - return directory; + return directory.getFile(); } private void writeStartupCrashMarkerFile() { @@ -106,7 +106,14 @@ private void writeStartupCrashMarkerFile() { .log(DEBUG, "Outbox path is null, the startup crash marker file will not be written"); return; } - final File crashMarkerFile = new File(outboxPath, STARTUP_CRASH_MARKER_FILE); + // The outbox dir is no longer created during Sentry.init, so create it here in case the native + // SDK (which normally creates it) is disabled. + final File outboxDir = new File(outboxPath); + if (!FileUtils.createDirectory(outboxDir)) { + options.getLogger().log(ERROR, "Failed to create outbox dir %s", outboxPath); + return; + } + final File crashMarkerFile = new File(outboxDir, STARTUP_CRASH_MARKER_FILE); try { crashMarkerFile.createNewFile(); } catch (Throwable e) { diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt index 97276d67566..0b13f4ca4d8 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/EnvelopeFileObserverIntegrationTest.kt @@ -14,6 +14,8 @@ import kotlin.test.AfterTest import kotlin.test.BeforeTest import kotlin.test.Test import kotlin.test.assertEquals +import kotlin.test.assertFalse +import kotlin.test.assertTrue import org.junit.runner.RunWith import org.mockito.kotlin.eq import org.mockito.kotlin.mock @@ -122,4 +124,19 @@ class EnvelopeFileObserverIntegrationTest { verify(fixture.logger) .log(eq(SentryLevel.DEBUG), eq("EnvelopeFileObserverIntegration installed.")) } + + @Test + fun `register creates the outbox dir when it does not exist yet`() { + val outboxDir = File(file, "outbox") + assertFalse(outboxDir.exists()) + + fixture.getSut { it.executorService = ImmediateExecutorService() } + val integration = + object : EnvelopeFileObserverIntegration() { + override fun getPath(options: SentryOptions): String = outboxDir.absolutePath + } + integration.register(fixture.scopes, fixture.scopes.options) + + assertTrue(outboxDir.isDirectory) + } } diff --git a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt index 09d3a779df0..a4063ccb148 100644 --- a/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt +++ b/sentry-android-core/src/test/java/io/sentry/android/core/cache/AndroidEnvelopeCacheTest.kt @@ -125,6 +125,20 @@ class AndroidEnvelopeCacheTest { assertTrue(fixture.startupCrashMarkerFile.exists()) } + @Test + fun `creates outbox dir when writing startup crash file and dir does not exist yet`() { + val cache = fixture.getSut(dir = tmpDir, appStartMillis = 1000L, currentTimeMillis = 2000L) + + val outboxDir = File(fixture.options.outboxPath!!) + assertTrue(outboxDir.deleteRecursively()) + assertFalse(outboxDir.exists()) + + val hints = HintUtils.createWithTypeCheckHint(UncaughtHint()) + cache.storeEnvelope(fixture.envelope, hints) + + assertTrue(fixture.startupCrashMarkerFile.exists()) + } + @Test fun `when no AnrV2 hint exists, does not write last anr report file`() { val cache = fixture.getSut(tmpDir) diff --git a/sentry-android-integration-tests/sentry-uitest-android-critical/src/main/java/io/sentry/uitest/android/critical/MainActivity.kt b/sentry-android-integration-tests/sentry-uitest-android-critical/src/main/java/io/sentry/uitest/android/critical/MainActivity.kt index 46bfe7e44b7..7e0ff9d61c3 100644 --- a/sentry-android-integration-tests/sentry-uitest-android-critical/src/main/java/io/sentry/uitest/android/critical/MainActivity.kt +++ b/sentry-android-integration-tests/sentry-uitest-android-critical/src/main/java/io/sentry/uitest/android/critical/MainActivity.kt @@ -66,6 +66,9 @@ class MainActivity : ComponentActivity() { Button(onClick = { Sentry.close() }) { Text("Close SDK") } Button( onClick = { + // The SDK creates the outbox dir lazily on its executor, so an external + // writer has to create it itself. + File(outboxPath).mkdirs() val file = File(outboxPath, "corrupted.envelope") val corruptedEnvelopeContent = """ diff --git a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt index ade47363296..3fa2c904873 100644 --- a/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt +++ b/sentry-android-integration-tests/sentry-uitest-android/src/androidTest/java/io/sentry/uitest/android/EnvelopeTests.kt @@ -262,9 +262,14 @@ class EnvelopeTests : BaseUiTest() { optionsRef = options } + // The SDK creates the outbox dir lazily on its executor, so an external writer racing + // Sentry.init has to create it itself. + val outboxDir = File(optionsRef!!.outboxPath!!) + outboxDir.mkdirs() + // based on // https://github.com/getsentry/sentry-native/blob/20d5d5f75f1f48228f2f47e2bb99b17f9996ebbf/ndk/lib/src/androidTest/java/io/sentry/ndk/SentryNdkTest.java#L131 - File(optionsRef!!.outboxPath, "14779dbf-b2f0-4c00-f4e5-4a287abc4267") + File(outboxDir, "14779dbf-b2f0-4c00-f4e5-4a287abc4267") .writeText( """ {"dsn":"https://key@sentry.io/proj","event_id":"729ff878-5539-458d-f657-a1acf423a127","sent_at":"2025-04-02T10:02:04.732577Z"} diff --git a/sentry/api/sentry.api b/sentry/api/sentry.api index c623e71d08f..14780d1a4b2 100644 --- a/sentry/api/sentry.api +++ b/sentry/api/sentry.api @@ -7692,6 +7692,7 @@ public final class io/sentry/util/ExceptionUtils { public final class io/sentry/util/FileUtils { public fun ()V + public static fun createDirectory (Ljava/io/File;)Z public static fun deleteRecursively (Ljava/io/File;)Z public static fun readBytesFromFile (Ljava/lang/String;J)[B public static fun readText (Ljava/io/File;)Ljava/lang/String; @@ -7757,6 +7758,12 @@ public final class io/sentry/util/JsonSerializationUtils { public static fun calendarToMap (Ljava/util/Calendar;)Ljava/util/Map; } +public final class io/sentry/util/LazyDirectory { + public fun (Ljava/lang/String;)V + public fun getFile ()Ljava/io/File; + public fun getOrCreate ()Ljava/io/File; +} + public final class io/sentry/util/LazyEvaluator { public fun (Lio/sentry/util/LazyEvaluator$Evaluator;)V public fun getValue ()Ljava/lang/Object; diff --git a/sentry/src/main/java/io/sentry/Sentry.java b/sentry/src/main/java/io/sentry/Sentry.java index 8bba9d92e4f..266aa39e793 100644 --- a/sentry/src/main/java/io/sentry/Sentry.java +++ b/sentry/src/main/java/io/sentry/Sentry.java @@ -463,8 +463,9 @@ private static void handleAppStartProfilingConfig( () -> { final String cacheDirPath = options.getCacheDirPathWithoutDsn(); if (cacheDirPath != null) { + final @NotNull File cacheDir = new File(cacheDirPath); final @NotNull File appStartProfilingConfigFile = - new File(cacheDirPath, APP_START_PROFILING_CONFIG_FILE_NAME); + new File(cacheDir, APP_START_PROFILING_CONFIG_FILE_NAME); try { // We always delete the config file for app start profiling FileUtils.deleteRecursively(appStartProfilingConfigFile); @@ -481,6 +482,14 @@ private static void handleAppStartProfilingConfig( "Tracing is disabled and app start profiling will not start."); return; } + // The cache dir is no longer created during init, so create it here before writing: + // createNewFile() fails if the parent is missing. + if (!FileUtils.createDirectory(cacheDir)) { + options + .getLogger() + .log(SentryLevel.ERROR, "Failed to create cache dir %s", cacheDirPath); + return; + } if (appStartProfilingConfigFile.createNewFile()) { // If old app start profiling is false, it means the transaction will not be // sampled, but we create the file anyway to allow continuous profiling on app @@ -616,19 +625,14 @@ private static void initConfigurations(final @NotNull SentryOptions options) { // TODO: read values from conf file, Build conf or system envs // eg release, distinctId, sentryClientName - // this should be after setting serializers - final String outboxPath = options.getOutboxPath(); - if (outboxPath != null) { - final File outboxDir = new File(outboxPath); - outboxDir.mkdirs(); - } else { + // The outbox and cache dirs are created lazily by their consumers (envelope cache, outbox file + // observer, native SDK) off the init thread, so we don't stat/mkdir them here. + if (options.getOutboxPath() == null) { logger.log(SentryLevel.INFO, "No outbox dir path is defined in options."); } final String cacheDirPath = options.getCacheDirPath(); if (cacheDirPath != null) { - final File cacheDir = new File(cacheDirPath); - cacheDir.mkdirs(); final IEnvelopeCache envelopeCache = options.getEnvelopeDiskCache(); // only overwrite the cache impl if it's not already set if (envelopeCache instanceof NoOpEnvelopeCache) { diff --git a/sentry/src/main/java/io/sentry/SentryOptions.java b/sentry/src/main/java/io/sentry/SentryOptions.java index f10f2aede05..93104101f81 100644 --- a/sentry/src/main/java/io/sentry/SentryOptions.java +++ b/sentry/src/main/java/io/sentry/SentryOptions.java @@ -1104,7 +1104,11 @@ String getCacheDirPathWithoutDsn() { } /** - * Returns the outbox path if cacheDirPath is set + * Returns the outbox path if cacheDirPath is set. + * + *

The directory is created lazily by the SDK on a background thread, so it is not guaranteed + * to exist when {@code Sentry.init} returns. Callers writing envelopes here directly (for example + * hybrid SDKs) must create it themselves, e.g. {@code new File(outboxPath).mkdirs()}. * * @return the outbox path or null if not set */ diff --git a/sentry/src/main/java/io/sentry/cache/CacheStrategy.java b/sentry/src/main/java/io/sentry/cache/CacheStrategy.java index 479c0e42eaf..5f3e605f2a1 100644 --- a/sentry/src/main/java/io/sentry/cache/CacheStrategy.java +++ b/sentry/src/main/java/io/sentry/cache/CacheStrategy.java @@ -10,6 +10,7 @@ import io.sentry.SentryOptions; import io.sentry.Session; import io.sentry.clientreport.DiscardReason; +import io.sentry.util.LazyDirectory; import io.sentry.util.LazyEvaluator; import io.sentry.util.Objects; import java.io.BufferedInputStream; @@ -39,7 +40,7 @@ abstract class CacheStrategy { protected @NotNull SentryOptions options; protected final @NotNull LazyEvaluator serializer = new LazyEvaluator<>(() -> options.getSerializer()); - protected final @NotNull File directory; + protected final @NotNull LazyDirectory directory; private final int maxSize; CacheStrategy( @@ -48,9 +49,7 @@ abstract class CacheStrategy { final int maxSize) { Objects.requireNonNull(directoryPath, "Directory is required."); this.options = Objects.requireNonNull(options, "SentryOptions is required."); - - this.directory = new File(directoryPath); - + this.directory = new LazyDirectory(directoryPath); this.maxSize = maxSize; } @@ -60,13 +59,12 @@ abstract class CacheStrategy { * @return true if valid and has permissions or false otherwise */ protected boolean isDirectoryValid() { - if (!directory.isDirectory() || !directory.canWrite() || !directory.canRead()) { + final File dir = directory.getFile(); + if (!dir.isDirectory() || !dir.canWrite() || !dir.canRead()) { options .getLogger() .log( - ERROR, - "The directory for caching files is inaccessible.: %s", - directory.getAbsolutePath()); + ERROR, "The directory for caching files is inaccessible.: %s", dir.getAbsolutePath()); return false; } return true; diff --git a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java index 475993bbc1d..618de655478 100644 --- a/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java +++ b/sentry/src/main/java/io/sentry/cache/EnvelopeCache.java @@ -109,10 +109,13 @@ public boolean storeEnvelope(final @NotNull SentryEnvelope envelope, final @NotN private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @NotNull Hint hint) { Objects.requireNonNull(envelope, "Envelope is required."); + // Create the cache dir lazily on the first write so Sentry.init doesn't block on the mkdirs. + final String directoryPath = directory.getOrCreate().getAbsolutePath(); + rotateCacheIfNeeded(allEnvelopeFiles()); - final File currentSessionFile = getCurrentSessionFile(directory.getAbsolutePath()); - final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); + final File currentSessionFile = getCurrentSessionFile(directoryPath); + final File previousSessionFile = getPreviousSessionFile(directoryPath); if (HintUtils.hasType(hint, SessionEnd.class)) { if (!currentSessionFile.delete()) { @@ -199,7 +202,7 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not @SuppressWarnings("JavaUtilDate") private void tryEndPreviousSession(final @NotNull Hint hint) { final Object sdkHint = HintUtils.getSentrySdkHint(hint); - final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath()); + final File previousSessionFile = getPreviousSessionFile(directory.getFile().getAbsolutePath()); if (previousSessionFile.exists()) { options.getLogger().log(WARNING, "Previous session is not ended, we'd need to end it."); @@ -372,6 +375,10 @@ public void discard(final @NotNull SentryEnvelope envelope) { * Returns the envelope's file path. If the envelope wasn't added to the cache beforehand, a * random file name is assigned. * + *

This only computes a path and never creates the directory, so that {@link + * #discard(SentryEnvelope)} doesn't resurrect a cache dir it is only deleting from. Writers go + * through {@link #storeInternal}, which creates the directory up front. + * * @param envelope the SentryEnvelope object * @return the file */ @@ -385,7 +392,7 @@ public void discard(final @NotNull SentryEnvelope envelope) { fileNameMap.put(envelope, fileName); } - return new File(directory.getAbsolutePath(), fileName); + return new File(directory.getFile(), fileName); } } @@ -431,7 +438,7 @@ public void discard(final @NotNull SentryEnvelope envelope) { if (isDirectoryValid()) { // lets filter the session.json here final File[] files = - directory.listFiles((__, fileName) -> fileName.endsWith(SUFFIX_ENVELOPE_FILE)); + directory.getFile().listFiles((__, fileName) -> fileName.endsWith(SUFFIX_ENVELOPE_FILE)); if (files != null) { return files; } diff --git a/sentry/src/main/java/io/sentry/util/FileUtils.java b/sentry/src/main/java/io/sentry/util/FileUtils.java index 73b83713c77..337a9a8863d 100644 --- a/sentry/src/main/java/io/sentry/util/FileUtils.java +++ b/sentry/src/main/java/io/sentry/util/FileUtils.java @@ -8,6 +8,7 @@ import java.io.FileReader; import java.io.IOException; import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; import org.jetbrains.annotations.Nullable; @ApiStatus.Internal @@ -36,6 +37,21 @@ public static boolean deleteRecursively(@Nullable File file) { return file.delete(); } + /** + * Creates the directory and any missing parents, if it does not exist yet. + * + *

Callers are expected to log a failure: a missing directory otherwise surfaces later as an + * unrelated-looking write error. + * + * @param directory the directory to create + * @return true if the directory exists once this returns, false if it could not be created + */ + public static boolean createDirectory(final @NotNull File directory) { + // mkdirs() also returns false when another thread created the directory first, so re-check + // instead of reporting a failure the caller would act on by skipping its write. + return directory.isDirectory() || directory.mkdirs() || directory.isDirectory(); + } + /** * Reads the content of a File into a String. If the file does not exist or is not a file, null is * returned. Do not use with large files, as the String is kept in memory! diff --git a/sentry/src/main/java/io/sentry/util/LazyDirectory.java b/sentry/src/main/java/io/sentry/util/LazyDirectory.java new file mode 100644 index 00000000000..62334c7665d --- /dev/null +++ b/sentry/src/main/java/io/sentry/util/LazyDirectory.java @@ -0,0 +1,38 @@ +package io.sentry.util; + +import java.io.File; +import org.jetbrains.annotations.ApiStatus; +import org.jetbrains.annotations.NotNull; + +/** + * A filesystem directory that is created on demand rather than up front, so the (potentially + * blocking) {@code mkdirs()} runs on the thread that first writes into it instead of on the SDK + * init thread. + * + *

Read paths should use {@link #getFile()}, which never touches the filesystem. Write paths + * should call {@link #getOrCreate()} once before writing. Creation is not cached: on Android the + * cache dir lives under {@code Context.getCacheDir()}, which the system may wipe at any time, so + * each write re-checks. + */ +@ApiStatus.Internal +public final class LazyDirectory { + + private final @NotNull File file; + + public LazyDirectory(final @NotNull String path) { + this.file = new File(path); + } + + /** Returns the directory without touching the filesystem. */ + public @NotNull File getFile() { + return file; + } + + /** Returns the directory, creating it and any missing parents if it does not exist yet. */ + public @NotNull File getOrCreate() { + // A failed mkdirs is not reported here: callers are write paths, so the failure surfaces as the + // write error they already log and report. + FileUtils.createDirectory(file); + return file; + } +} diff --git a/sentry/src/test/java/io/sentry/SentryTest.kt b/sentry/src/test/java/io/sentry/SentryTest.kt index 8d05697fda4..98cda8e9d82 100644 --- a/sentry/src/test/java/io/sentry/SentryTest.kt +++ b/sentry/src/test/java/io/sentry/SentryTest.kt @@ -183,7 +183,7 @@ class SentryTest { } @Test - fun `outboxPath should be created at initialization`() { + fun `outboxPath is not created during initialization`() { var sentryOptions: SentryOptions? = null initForTest { it.dsn = dsn @@ -191,13 +191,13 @@ class SentryTest { sentryOptions = it } + // The outbox dir is created lazily by its consumers (file observer, native SDK), not at init. val file = File(sentryOptions!!.outboxPath!!) - assertTrue(file.exists()) - file.deleteOnExit() + assertFalse(file.exists()) } @Test - fun `cacheDirPath should be created at initialization`() { + fun `cacheDirPath is not created during initialization`() { var sentryOptions: SentryOptions? = null initForTest { it.dsn = dsn @@ -205,13 +205,13 @@ class SentryTest { sentryOptions = it } + // The cache dir is created lazily on the first envelope store, not at init. val file = File(sentryOptions!!.cacheDirPath!!) - assertTrue(file.exists()) - file.deleteOnExit() + assertFalse(file.exists()) } @Test - fun `getCacheDirPathWithoutDsn should be created at initialization`() { + fun `cacheDirPathWithoutDsn is not created during initialization`() { var sentryOptions: SentryOptions? = null initForTest { it.dsn = dsn @@ -221,8 +221,7 @@ class SentryTest { val cacheDirPathWithoutDsn = sentryOptions!!.cacheDirPathWithoutDsn!! val file = File(cacheDirPathWithoutDsn) - assertTrue(file.exists()) - file.deleteOnExit() + assertFalse(file.exists()) } @Test @@ -1317,6 +1316,23 @@ class SentryTest { assertTrue(appStartProfilingConfigFile.exists()) } + @Test + fun `init creates app start profiling config when the cache dir does not exist yet`() { + val path = getTempPath() + // Profiling is left disabled on purpose: it is the only other init-time consumer that creates + // the cache dir, so with it off nothing materializes the dir before the config is written. + initForTest { + it.dsn = dsn + it.cacheDirPath = path + it.isEnableAppStartProfiling = false + it.isStartProfilerOnAppStart = true + it.tracesSampleRate = 0.0 + it.profilesSampleRate = null + it.executorService = ImmediateExecutorService() + } + assertTrue(File(path, "app_start_profiling_config").exists()) + } + @Test fun `init saves SentryAppStartProfilingOptions to disk`() { var options = SentryOptions() diff --git a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt index 973070e0afe..dda06ee7e63 100644 --- a/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt +++ b/sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt @@ -79,6 +79,22 @@ class EnvelopeCacheTest { file.deleteRecursively() } + @Test + fun `creates cache dir on store when it does not exist yet`() { + val cache = fixture.getSUT() + + val file = File(fixture.options.cacheDirPath!!) + assertTrue(file.deleteRecursively()) + assertFalse(file.exists()) + + cache.store(SentryEnvelope.from(fixture.options.serializer, createSession(), null)) + + assertTrue(file.exists()) + assertEquals(1, file.list()?.size) + + file.deleteRecursively() + } + @Test fun `tolerates discarding unknown envelope`() { val cache = fixture.getSUT() @@ -88,6 +104,19 @@ class EnvelopeCacheTest { // no exception thrown } + @Test + fun `does not create cache dir on discard`() { + val cache = fixture.getSUT() + + val file = File(fixture.options.cacheDirPath!!) + assertTrue(file.deleteRecursively()) + assertFalse(file.exists()) + + cache.discard(SentryEnvelope.from(fixture.options.serializer, createSession(), null)) + + assertFalse(file.exists()) + } + @Test fun `creates current file on session start`() { val cache = fixture.getSUT() @@ -450,7 +479,7 @@ class EnvelopeCacheTest { cache.store(envelopeA, Hint()) cache.store(envelopeB, Hint()) - assertEquals(2, cache.directory.list()?.size) + assertEquals(2, cache.directory.file.list()?.size) } @Test diff --git a/sentry/src/test/java/io/sentry/util/FileUtilsTest.kt b/sentry/src/test/java/io/sentry/util/FileUtilsTest.kt index 65745c0d3d2..60eb092e125 100644 --- a/sentry/src/test/java/io/sentry/util/FileUtilsTest.kt +++ b/sentry/src/test/java/io/sentry/util/FileUtilsTest.kt @@ -1,7 +1,10 @@ package io.sentry.util +import com.google.common.truth.Truth.assertThat import java.io.File import java.nio.file.Files +import java.util.concurrent.ConcurrentLinkedQueue +import java.util.concurrent.CyclicBarrier import kotlin.test.Test import kotlin.test.assertEquals import kotlin.test.assertFalse @@ -73,4 +76,51 @@ class FileUtilsTest { f.writeText(text) assertEquals(text, FileUtils.readText(f)) } + + @Test + fun `createDirectory creates the directory and any missing parents`() { + val dir = File(Files.createTempDirectory("create-dir-test").toFile(), "nested/outbox") + + assertThat(FileUtils.createDirectory(dir)).isTrue() + assertThat(dir.isDirectory).isTrue() + } + + @Test + fun `createDirectory returns true when the directory already exists`() { + val dir = Files.createTempDirectory("create-dir-test").toFile() + + assertThat(FileUtils.createDirectory(dir)).isTrue() + } + + @Test + fun `createDirectory returns false when the directory cannot be created`() { + val file = Files.createTempFile("create-dir-test", "test").toFile() + + // a regular file already occupies the path, so it can never become a directory + assertThat(FileUtils.createDirectory(file)).isFalse() + } + + @Test + fun `createDirectory returns true for every caller when threads race to create it`() { + val threadCount = 8 + // mkdirs() returns false for the losers of the race, so every caller must still see success + repeat(50) { iteration -> + val dir = File(Files.createTempDirectory("create-dir-race").toFile(), "run$iteration/outbox") + val barrier = CyclicBarrier(threadCount) + val results = ConcurrentLinkedQueue() + + val threads = + (1..threadCount).map { + Thread { + barrier.await() + results.add(FileUtils.createDirectory(dir)) + } + } + threads.forEach { it.start() } + threads.forEach { it.join() } + + assertThat(results).hasSize(threadCount) + assertThat(results).doesNotContain(false) + } + } } diff --git a/sentry/src/test/java/io/sentry/util/LazyDirectoryTest.kt b/sentry/src/test/java/io/sentry/util/LazyDirectoryTest.kt new file mode 100644 index 00000000000..13e4df001e4 --- /dev/null +++ b/sentry/src/test/java/io/sentry/util/LazyDirectoryTest.kt @@ -0,0 +1,45 @@ +package io.sentry.util + +import com.google.common.truth.Truth.assertThat +import java.nio.file.Files +import kotlin.test.Test + +class LazyDirectoryTest { + @Test + fun `getFile does not create the directory`() { + val path = Files.createTempDirectory("lazy-dir-test").resolve("outbox") + val lazyDirectory = LazyDirectory(path.toString()) + + assertThat(lazyDirectory.file.exists()).isFalse() + } + + @Test + fun `getOrCreate creates the directory and any missing parents`() { + val path = Files.createTempDirectory("lazy-dir-test").resolve("nested").resolve("outbox") + val lazyDirectory = LazyDirectory(path.toString()) + + val created = lazyDirectory.getOrCreate() + + assertThat(created.isDirectory).isTrue() + assertThat(created.absolutePath).isEqualTo(path.toFile().absolutePath) + } + + @Test + fun `getOrCreate is idempotent when the directory already exists`() { + val path = Files.createTempDirectory("lazy-dir-test").resolve("outbox") + val lazyDirectory = LazyDirectory(path.toString()) + + assertThat(lazyDirectory.getOrCreate().isDirectory).isTrue() + assertThat(lazyDirectory.getOrCreate().isDirectory).isTrue() + } + + @Test + fun `getOrCreate recreates the directory after it is deleted`() { + val path = Files.createTempDirectory("lazy-dir-test").resolve("outbox") + val lazyDirectory = LazyDirectory(path.toString()) + + assertThat(lazyDirectory.getOrCreate().delete()).isTrue() + + assertThat(lazyDirectory.getOrCreate().isDirectory).isTrue() + } +}