Skip to content

Commit 1fdc454

Browse files
runningcodeclaude
andcommitted
ref(core): Encapsulate lazy dir creation in a LazyDirectory value object (JAVA-613)
Replace the duplicated "create the dir if it does not exist" idiom in the envelope cache, outbox file observer, and startup-crash-marker paths with a single LazyDirectory type that materializes the directory on first access. CacheStrategy now owns its directory as a LazyDirectory: write paths call getOrCreate(), while path-building and validity checks use getFile() so they do not create the directory as a side effect. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
1 parent 46681de commit 1fdc454

8 files changed

Lines changed: 97 additions & 30 deletions

File tree

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

Lines changed: 4 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,9 @@
1010
import io.sentry.SentryLevel;
1111
import io.sentry.SentryOptions;
1212
import io.sentry.util.AutoClosableReentrantLock;
13+
import io.sentry.util.LazyDirectory;
1314
import io.sentry.util.Objects;
1415
import java.io.Closeable;
15-
import java.io.File;
1616
import org.jetbrains.annotations.NotNull;
1717
import org.jetbrains.annotations.Nullable;
1818
import org.jetbrains.annotations.TestOnly;
@@ -68,12 +68,9 @@ private void startOutboxSender(
6868
final @NotNull IScopes scopes,
6969
final @NotNull SentryOptions options,
7070
final @NotNull String path) {
71-
// Create the outbox dir lazily here (on the executor) so the observer can watch it for
72-
// envelopes written by hybrid SDKs, instead of blocking Sentry.init on the mkdirs.
73-
final File outboxDir = new File(path);
74-
if (!outboxDir.isDirectory()) {
75-
outboxDir.mkdirs();
76-
}
71+
// Materialize the outbox dir here (on the executor) so the observer can watch it for envelopes
72+
// written by hybrid SDKs, instead of blocking Sentry.init on the mkdirs.
73+
new LazyDirectory(path).getOrCreate();
7774

7875
final OutboxSender outboxSender =
7976
new OutboxSender(

sentry-android-core/src/main/java/io/sentry/android/core/cache/AndroidEnvelopeCache.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
import io.sentry.transport.ICurrentDateProvider;
1919
import io.sentry.util.FileUtils;
2020
import io.sentry.util.HintUtils;
21+
import io.sentry.util.LazyDirectory;
2122
import io.sentry.util.Objects;
2223
import java.io.File;
2324
import java.io.FileNotFoundException;
@@ -93,7 +94,7 @@ private boolean storeInternalAndroid(@NotNull SentryEnvelope envelope, @NotNull
9394

9495
@TestOnly
9596
public @NotNull File getDirectory() {
96-
return directory;
97+
return directory.getFile();
9798
}
9899

99100
private void writeStartupCrashMarkerFile() {
@@ -106,14 +107,11 @@ private void writeStartupCrashMarkerFile() {
106107
.log(DEBUG, "Outbox path is null, the startup crash marker file will not be written");
107108
return;
108109
}
109-
final File crashMarkerFile = new File(outboxPath, STARTUP_CRASH_MARKER_FILE);
110+
// The outbox dir is no longer created during Sentry.init, so materialize it here in case the
111+
// native SDK (which normally creates it) is disabled.
112+
final File outboxDir = new LazyDirectory(outboxPath).getOrCreate();
113+
final File crashMarkerFile = new File(outboxDir, STARTUP_CRASH_MARKER_FILE);
110114
try {
111-
// The outbox dir is no longer created during Sentry.init, so ensure it exists here in case
112-
// the native SDK (which normally creates it) is disabled.
113-
final File outboxDir = crashMarkerFile.getParentFile();
114-
if (outboxDir != null && !outboxDir.isDirectory()) {
115-
outboxDir.mkdirs();
116-
}
117115
crashMarkerFile.createNewFile();
118116
} catch (Throwable e) {
119117
options.getLogger().log(ERROR, "Error writing the startup crash marker file to the disk", e);

sentry/api/sentry.api

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7757,6 +7757,12 @@ public final class io/sentry/util/JsonSerializationUtils {
77577757
public static fun calendarToMap (Ljava/util/Calendar;)Ljava/util/Map;
77587758
}
77597759

7760+
public final class io/sentry/util/LazyDirectory {
7761+
public fun <init> (Ljava/lang/String;)V
7762+
public fun getFile ()Ljava/io/File;
7763+
public fun getOrCreate ()Ljava/io/File;
7764+
}
7765+
77607766
public final class io/sentry/util/LazyEvaluator {
77617767
public fun <init> (Lio/sentry/util/LazyEvaluator$Evaluator;)V
77627768
public fun getValue ()Ljava/lang/Object;

sentry/src/main/java/io/sentry/cache/CacheStrategy.java

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@
1010
import io.sentry.SentryOptions;
1111
import io.sentry.Session;
1212
import io.sentry.clientreport.DiscardReason;
13+
import io.sentry.util.LazyDirectory;
1314
import io.sentry.util.LazyEvaluator;
1415
import io.sentry.util.Objects;
1516
import java.io.BufferedInputStream;
@@ -39,7 +40,7 @@ abstract class CacheStrategy {
3940
protected @NotNull SentryOptions options;
4041
protected final @NotNull LazyEvaluator<ISerializer> serializer =
4142
new LazyEvaluator<>(() -> options.getSerializer());
42-
protected final @NotNull File directory;
43+
protected final @NotNull LazyDirectory directory;
4344
private final int maxSize;
4445

4546
CacheStrategy(
@@ -49,7 +50,7 @@ abstract class CacheStrategy {
4950
Objects.requireNonNull(directoryPath, "Directory is required.");
5051
this.options = Objects.requireNonNull(options, "SentryOptions is required.");
5152

52-
this.directory = new File(directoryPath);
53+
this.directory = new LazyDirectory(directoryPath);
5354

5455
this.maxSize = maxSize;
5556
}
@@ -60,13 +61,12 @@ abstract class CacheStrategy {
6061
* @return true if valid and has permissions or false otherwise
6162
*/
6263
protected boolean isDirectoryValid() {
63-
if (!directory.isDirectory() || !directory.canWrite() || !directory.canRead()) {
64+
final File dir = directory.getFile();
65+
if (!dir.isDirectory() || !dir.canWrite() || !dir.canRead()) {
6466
options
6567
.getLogger()
6668
.log(
67-
ERROR,
68-
"The directory for caching files is inaccessible.: %s",
69-
directory.getAbsolutePath());
69+
ERROR, "The directory for caching files is inaccessible.: %s", dir.getAbsolutePath());
7070
return false;
7171
}
7272
return true;

sentry/src/main/java/io/sentry/cache/EnvelopeCache.java

Lines changed: 6 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -110,14 +110,12 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
110110
Objects.requireNonNull(envelope, "Envelope is required.");
111111

112112
// Create the cache dir lazily on the first write so Sentry.init doesn't block on the mkdirs.
113-
if (!directory.isDirectory()) {
114-
directory.mkdirs();
115-
}
113+
final String directoryPath = directory.getOrCreate().getAbsolutePath();
116114

117115
rotateCacheIfNeeded(allEnvelopeFiles());
118116

119-
final File currentSessionFile = getCurrentSessionFile(directory.getAbsolutePath());
120-
final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath());
117+
final File currentSessionFile = getCurrentSessionFile(directoryPath);
118+
final File previousSessionFile = getPreviousSessionFile(directoryPath);
121119

122120
if (HintUtils.hasType(hint, SessionEnd.class)) {
123121
if (!currentSessionFile.delete()) {
@@ -204,7 +202,7 @@ private boolean storeInternal(final @NotNull SentryEnvelope envelope, final @Not
204202
@SuppressWarnings("JavaUtilDate")
205203
private void tryEndPreviousSession(final @NotNull Hint hint) {
206204
final Object sdkHint = HintUtils.getSentrySdkHint(hint);
207-
final File previousSessionFile = getPreviousSessionFile(directory.getAbsolutePath());
205+
final File previousSessionFile = getPreviousSessionFile(directory.getFile().getAbsolutePath());
208206

209207
if (previousSessionFile.exists()) {
210208
options.getLogger().log(WARNING, "Previous session is not ended, we'd need to end it.");
@@ -390,7 +388,7 @@ public void discard(final @NotNull SentryEnvelope envelope) {
390388
fileNameMap.put(envelope, fileName);
391389
}
392390

393-
return new File(directory.getAbsolutePath(), fileName);
391+
return new File(directory.getFile().getAbsolutePath(), fileName);
394392
}
395393
}
396394

@@ -436,7 +434,7 @@ public void discard(final @NotNull SentryEnvelope envelope) {
436434
if (isDirectoryValid()) {
437435
// lets filter the session.json here
438436
final File[] files =
439-
directory.listFiles((__, fileName) -> fileName.endsWith(SUFFIX_ENVELOPE_FILE));
437+
directory.getFile().listFiles((__, fileName) -> fileName.endsWith(SUFFIX_ENVELOPE_FILE));
440438
if (files != null) {
441439
return files;
442440
}
Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,33 @@
1+
package io.sentry.util;
2+
3+
import java.io.File;
4+
import org.jetbrains.annotations.ApiStatus;
5+
import org.jetbrains.annotations.NotNull;
6+
7+
/**
8+
* A filesystem directory that is created on demand rather than up front, so the (potentially
9+
* blocking) {@code mkdirs()} runs on the thread that first writes into it instead of on the SDK
10+
* init thread.
11+
*/
12+
@ApiStatus.Internal
13+
public final class LazyDirectory {
14+
15+
private final @NotNull File file;
16+
17+
public LazyDirectory(final @NotNull String path) {
18+
this.file = new File(path);
19+
}
20+
21+
/** Returns the directory without touching the filesystem. */
22+
public @NotNull File getFile() {
23+
return file;
24+
}
25+
26+
/** Returns the directory, creating it and any missing parents if it does not exist yet. */
27+
public @NotNull File getOrCreate() {
28+
if (!file.isDirectory()) {
29+
file.mkdirs();
30+
}
31+
return file;
32+
}
33+
}

sentry/src/test/java/io/sentry/cache/EnvelopeCacheTest.kt

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -466,7 +466,7 @@ class EnvelopeCacheTest {
466466
cache.store(envelopeA, Hint())
467467
cache.store(envelopeB, Hint())
468468

469-
assertEquals(2, cache.directory.list()?.size)
469+
assertEquals(2, cache.directory.file.list()?.size)
470470
}
471471

472472
@Test
Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
package io.sentry.util
2+
3+
import com.google.common.truth.Truth.assertThat
4+
import java.nio.file.Files
5+
import kotlin.test.Test
6+
7+
class LazyDirectoryTest {
8+
@Test
9+
fun `getFile does not create the directory`() {
10+
val path = Files.createTempDirectory("lazy-dir-test").resolve("outbox")
11+
val lazyDirectory = LazyDirectory(path.toString())
12+
13+
assertThat(lazyDirectory.file.exists()).isFalse()
14+
}
15+
16+
@Test
17+
fun `getOrCreate creates the directory and any missing parents`() {
18+
val path = Files.createTempDirectory("lazy-dir-test").resolve("nested").resolve("outbox")
19+
val lazyDirectory = LazyDirectory(path.toString())
20+
21+
val created = lazyDirectory.getOrCreate()
22+
23+
assertThat(created.isDirectory).isTrue()
24+
assertThat(created.absolutePath).isEqualTo(path.toFile().absolutePath)
25+
}
26+
27+
@Test
28+
fun `getOrCreate is idempotent when the directory already exists`() {
29+
val path = Files.createTempDirectory("lazy-dir-test").resolve("outbox")
30+
val lazyDirectory = LazyDirectory(path.toString())
31+
32+
assertThat(lazyDirectory.getOrCreate().isDirectory).isTrue()
33+
assertThat(lazyDirectory.getOrCreate().isDirectory).isTrue()
34+
}
35+
}

0 commit comments

Comments
 (0)