Skip to content

Commit 2cd4aaf

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 7a5b20d commit 2cd4aaf

3 files changed

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

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

5451
/** Current value for hostname (might change over time). */
@@ -77,32 +74,20 @@ public final class HostnameCache {
7774
}
7875

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

9382
/**
9483
* Sets up a cache for the hostname.
9584
*
96-
* @param cacheDurationMillis cache duration in milliseconds.
9785
* @param getLocalhost a callback to obtain the localhost address - this is mostly here because of
9886
* testability
9987
* @param ticker the ticker the cache lifetime is measured on
10088
*/
10189
HostnameCache(
102-
long cacheDurationMillis,
103-
final @NotNull Callable<InetAddress> getLocalhost,
104-
final @NotNull MonotonicTicker ticker) {
105-
this.cacheDurationMillis = cacheDurationMillis;
90+
final @NotNull Callable<InetAddress> getLocalhost, final @NotNull MonotonicTicker ticker) {
10691
this.getLocalhost = Objects.requireNonNull(getLocalhost, "getLocalhost is required");
10792
this.ticker = Objects.requireNonNull(ticker, "ticker is required");
10893
// Nothing resolved yet, so the cache is stale rather than fresh until updateCache says
@@ -145,7 +130,8 @@ private void updateCache() {
145130
() -> {
146131
try {
147132
hostname = getLocalhost.call().getCanonicalHostName();
148-
cacheFreshUntil = Deadline.after(ticker, cacheDurationMillis, TimeUnit.MILLISECONDS);
133+
cacheFreshUntil =
134+
Deadline.after(ticker, HOSTNAME_CACHE_DURATION, TimeUnit.MILLISECONDS);
149135
} finally {
150136
updateRunning.set(false);
151137
}

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

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

2222
@Test
@@ -30,7 +30,7 @@ class HostnameCacheTest {
3030
val ticker = TestMonotonicTicker()
3131
val address = mock<InetAddress>()
3232
whenever(address.canonicalHostName).thenReturn("first", "second")
33-
val cache = HostnameCache(TimeUnit.HOURS.toMillis(5), { address }, ticker)
33+
val cache = HostnameCache({ address }, ticker)
3434

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

@@ -46,11 +46,11 @@ class HostnameCacheTest {
4646
val ticker = TestMonotonicTicker()
4747
val address = mock<InetAddress>()
4848
whenever(address.canonicalHostName).thenReturn("myhost")
49-
val cache = HostnameCache(TimeUnit.HOURS.toMillis(1), { address }, ticker)
49+
val cache = HostnameCache({ address }, ticker)
5050
// Reject the next submit the way an executor that could not start a thread would, and let the
5151
// cache go stale so that reading the hostname attempts a refresh.
5252
cache.getProperty<ThreadPoolExecutor>("executorService").shutdown()
53-
ticker.advance(1, TimeUnit.HOURS)
53+
ticker.advance(5, TimeUnit.HOURS)
5454

5555
assertThat(cache.hostname).isEqualTo("myhost")
5656

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

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,9 +7,11 @@ import io.sentry.protocol.DebugMeta
77
import io.sentry.protocol.SdkVersion
88
import io.sentry.protocol.SentryTransaction
99
import io.sentry.protocol.User
10+
import io.sentry.time.TestMonotonicTicker
1011
import io.sentry.util.HintUtils
1112
import java.lang.RuntimeException
1213
import java.net.InetAddress
14+
import java.util.concurrent.TimeUnit
1315
import kotlin.test.AfterTest
1416
import kotlin.test.Test
1517
import kotlin.test.assertEquals
@@ -18,7 +20,6 @@ import kotlin.test.assertNotNull
1820
import kotlin.test.assertNull
1921
import kotlin.test.assertSame
2022
import kotlin.test.assertTrue
21-
import org.awaitility.kotlin.await
2223
import org.mockito.Mockito
2324
import org.mockito.kotlin.mock
2425
import org.mockito.kotlin.reset
@@ -37,6 +38,7 @@ class MainEventProcessorTest {
3738
}
3839
val scopes = mock<IScopes>()
3940
val getLocalhost = mock<InetAddress>()
41+
val hostnameCacheTicker = TestMonotonicTicker()
4042
lateinit var sentryTracer: SentryTracer
4143
private val hostnameCacheMock = Mockito.mockStatic(HostnameCache::class.java)
4244

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

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

8384
return MainEventProcessor(sentryOptions)
@@ -435,7 +436,7 @@ class MainEventProcessorTest {
435436

436437
@Test
437438
fun `uses cache to retrieve servername for subsequent events`() {
438-
val processor = fixture.getSut(serverName = null, host = "aHost", hostnameCacheDuration = 1000)
439+
val processor = fixture.getSut(serverName = null, host = "aHost")
439440
val firstEvent = SentryEvent()
440441
processor.process(firstEvent, Hint())
441442
assertEquals("aHost", firstEvent.serverName)
@@ -454,12 +455,11 @@ class MainEventProcessorTest {
454455

455456
reset(fixture.getLocalhost)
456457
whenever(fixture.getLocalhost.canonicalHostName).thenReturn("newHost")
458+
fixture.hostnameCacheTicker.advance(6, TimeUnit.HOURS)
457459

458-
await.untilAsserted {
459-
val secondEvent = SentryEvent()
460-
processor.process(secondEvent, Hint())
461-
assertEquals("newHost", secondEvent.serverName)
462-
}
460+
val secondEvent = SentryEvent()
461+
processor.process(secondEvent, Hint())
462+
assertEquals("newHost", secondEvent.serverName)
463463
}
464464

465465
@Test

0 commit comments

Comments
 (0)