Skip to content

Commit fdd896b

Browse files
runningcodeclaude
andcommitted
ref(core): Collapse the HostnameCache constructor chain (JAVA-579)
The four constructors were a telescoping chain of test seams: one to shorten the 5-hour TTL, one to stub the localhost lookup, one to fake the ticker. With a fake ticker a test can advance past the real TTL instantly, so the duration parameter has no remaining purpose and the chain collapses to a single seam. HostnameCache(long) had no callers at all — both apparent call sites were Kotlin trailing-lambda calls that SAM-convert to HostnameCache(long, Callable). MainEventProcessorTest's cache-expiry test now advances the ticker instead of polling with awaitility, making it deterministic rather than just fast. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 378d9be commit fdd896b

3 files changed

Lines changed: 18 additions & 32 deletions

File tree

‎sentry/src/main/java/io/sentry/HostnameCache.java‎

Lines changed: 7 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
* Time sensitive cache in charge of keeping track of the hostname. The {@code
2525
* InetAddress.getLocalHost().getCanonicalHostName()} call can be quite expensive and could be
2626
* called for the creation of each {@link SentryEvent}. This system will prevent unnecessary costs
27-
* by keeping track of the hostname for a period defined during the construction. For performance
28-
* purposes, the operation of retrieving the hostname will automatically fail after a period of time
29-
* defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
27+
* by keeping track of the hostname for a period defined by {@link #HOSTNAME_CACHE_DURATION}. For
28+
* performance purposes, the operation of retrieving the hostname will automatically fail after a
29+
* period of time defined by {@link #GET_HOSTNAME_TIMEOUT} without result.
3030
*
3131
* <p>HostnameCache is a singleton and its instance should be obtained through {@link
3232
* HostnameCache#getInstance()}.
@@ -45,9 +45,6 @@ public final class HostnameCache {
4545
private static final @NotNull AutoClosableReentrantLock staticLock =
4646
new AutoClosableReentrantLock();
4747

48-
/** Time for which the cache is kept, in milliseconds. */
49-
private final long cacheDurationMillis;
50-
5148
private final @NotNull MonotonicTicker ticker;
5249

5350
/** Current value for hostname (might change over time). */
@@ -76,32 +73,20 @@ public final class HostnameCache {
7673
}
7774

7875
private HostnameCache() {
79-
this(HOSTNAME_CACHE_DURATION);
80-
}
81-
82-
HostnameCache(long cacheDurationMillis) {
8376
// avoid method refs on Android due to some issues with older AGP setups
8477
// noinspection Convert2MethodRef
85-
this(cacheDurationMillis, () -> InetAddress.getLocalHost());
86-
}
87-
88-
HostnameCache(long cacheDurationMillis, final @NotNull Callable<InetAddress> getLocalhost) {
89-
this(cacheDurationMillis, getLocalhost, JavaMonotonicTicker.getInstance());
78+
this(() -> InetAddress.getLocalHost(), JavaMonotonicTicker.getInstance());
9079
}
9180

9281
/**
9382
* Sets up a cache for the hostname.
9483
*
95-
* @param cacheDurationMillis cache duration in milliseconds.
9684
* @param getLocalhost a callback to obtain the localhost address - this is mostly here because of
9785
* testability
9886
* @param ticker the ticker the cache lifetime is measured on
9987
*/
10088
HostnameCache(
101-
long cacheDurationMillis,
102-
final @NotNull Callable<InetAddress> getLocalhost,
103-
final @NotNull MonotonicTicker ticker) {
104-
this.cacheDurationMillis = cacheDurationMillis;
89+
final @NotNull Callable<InetAddress> getLocalhost, final @NotNull MonotonicTicker ticker) {
10590
this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
10691
this.ticker = Objects.requireNonNull(ticker, "ticker is required");
10792
// Nothing resolved yet, so the cache is stale rather than fresh until updateCache says
@@ -152,7 +137,8 @@ private void updateCache() {
152137
() -> {
153138
try {
154139
hostname = getLocalhost.call().getCanonicalHostName();
155-
cacheFreshUntil = Deadline.after(ticker, cacheDurationMillis, TimeUnit.MILLISECONDS);
140+
cacheFreshUntil =
141+
Deadline.after(ticker, HOSTNAME_CACHE_DURATION, TimeUnit.MILLISECONDS);
156142
} finally {
157143
updateRunning.set(false);
158144
}

‎sentry/src/test/java/io/sentry/HostnameCacheTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@ class HostnameCacheTest {
1515
private fun getSut(): HostnameCache {
1616
val address = mock<InetAddress>()
1717
whenever(address.canonicalHostName).thenReturn("myhost")
18-
return HostnameCache(TimeUnit.HOURS.toMillis(1)) { address }
18+
return HostnameCache({ address }, TestMonotonicTicker())
1919
}
2020

2121
@Test
@@ -29,7 +29,7 @@ class HostnameCacheTest {
2929
val ticker = TestMonotonicTicker()
3030
val address = mock<InetAddress>()
3131
whenever(address.canonicalHostName).thenReturn("first", "second")
32-
val cache = HostnameCache(TimeUnit.HOURS.toMillis(5), { address }, ticker)
32+
val cache = HostnameCache({ address }, ticker)
3333

3434
assertThat(cache.hostname).isEqualTo("first")
3535

‎sentry/src/test/java/io/sentry/MainEventProcessorTest.kt‎

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -6,9 +6,11 @@ import io.sentry.protocol.DebugMeta
66
import io.sentry.protocol.SdkVersion
77
import io.sentry.protocol.SentryTransaction
88
import io.sentry.protocol.User
9+
import io.sentry.time.TestMonotonicTicker
910
import io.sentry.util.HintUtils
1011
import java.lang.RuntimeException
1112
import java.net.InetAddress
13+
import java.util.concurrent.TimeUnit
1214
import kotlin.test.AfterTest
1315
import kotlin.test.Test
1416
import kotlin.test.assertEquals
@@ -17,7 +19,6 @@ import kotlin.test.assertNotNull
1719
import kotlin.test.assertNull
1820
import kotlin.test.assertSame
1921
import kotlin.test.assertTrue
20-
import org.awaitility.kotlin.await
2122
import org.mockito.Mockito
2223
import org.mockito.kotlin.mock
2324
import org.mockito.kotlin.reset
@@ -36,6 +37,7 @@ class MainEventProcessorTest {
3637
}
3738
val scopes = mock<IScopes>()
3839
val getLocalhost = mock<InetAddress>()
40+
val hostnameCacheTicker = TestMonotonicTicker()
3941
lateinit var sentryTracer: SentryTracer
4042
private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java)
4143

@@ -48,7 +50,6 @@ class MainEventProcessorTest {
4850
serverName: String? = "server",
4951
host: String? = null,
5052
resolveHostDelay: Long? = null,
51-
hostnameCacheDuration: Long = 10,
5253
proguardUuid: String? = null,
5354
bundleIds: List<String>? = null,
5455
modules: Map<String, String>? = null,
@@ -76,7 +77,7 @@ class MainEventProcessorTest {
7677
whenever(scopes.options).thenReturn(sentryOptions)
7778
sentryTracer = SentryTracer(TransactionContext("", ""), scopes)
7879

79-
val hostnameCache = HostnameCache(hostnameCacheDuration) { getLocalhost }
80+
val hostnameCache = HostnameCache({ getLocalhost }, hostnameCacheTicker)
8081
hostnameCacheMock.`when`<Any> { HostnameCache.getInstance() }.thenReturn(hostnameCache)
8182

8283
return MainEventProcessor(sentryOptions)
@@ -401,7 +402,7 @@ class MainEventProcessorTest {
401402

402403
@Test
403404
fun `uses cache to retrieve servername for subsequent events`() {
404-
val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000)
405+
val processor = fixture.getSut(serverName = null, host = "aHost")
405406
val firstEvent = SentryEvent()
406407
processor.process(firstEvent, Hint())
407408
assertEquals("aHost", firstEvent.serverName)
@@ -420,12 +421,11 @@ class MainEventProcessorTest {
420421

421422
reset(fixture.getLocalhost)
422423
whenever(fixture.getLocalhost.canonicalHostName).thenReturn("newHost")
424+
fixture.hostnameCacheTicker.advance(6, TimeUnit.HOURS)
423425

424-
await.untilAsserted {
425-
val secondEvent = SentryEvent()
426-
processor.process(secondEvent, Hint())
427-
assertEquals("newHost", secondEvent.serverName)
428-
}
426+
val secondEvent = SentryEvent()
427+
processor.process(secondEvent, Hint())
428+
assertEquals("newHost", secondEvent.serverName)
429429
}
430430

431431
@Test

0 commit comments

Comments
 (0)