fix(spring): Use ValueWrapper to determine cache hit in typed get

adinauer · claude · adinauer · commit 282054940e69 · 2026-03-18T05:35:50.000+01:00
The get(key, type) method incorrectly used result != null to detect
cache hits, failing to distinguish a miss from a cached null value.
Now uses delegate.get(key) to check the ValueWrapper first.

Co-Authored-By: Claude Opus 4.6 (1M context) &lt;noreply@anthropic.com&gt;
diff --git a/sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java b/sentry-spring-7/src/main/java/io/sentry/spring7/cache/SentryCacheWrapper.java
@@ -67,8 +67,9 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes
       return delegate.get(key, type);
     }
     try {
+      final ValueWrapper wrapper = delegate.get(key);
+      span.setData(SpanDataConvention.CACHE_HIT_KEY, wrapper != null);
       final T result = delegate.get(key, type);
-      span.setData(SpanDataConvention.CACHE_HIT_KEY, result != null);
       span.setStatus(SpanStatus.OK);
       return result;
     } catch (Throwable e) {
diff --git a/sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt b/sentry-spring-7/src/test/kotlin/io/sentry/spring7/cache/SentryCacheWrapperTest.kt
@@ -85,6 +85,8 @@ class SentryCacheWrapperTest {
   fun `get with type creates span with cache hit true on hit`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
+    val valueWrapper = mock<Cache.ValueWrapper>()
+    whenever(delegate.get("myKey")).thenReturn(valueWrapper)
     whenever(delegate.get("myKey", String::class.java)).thenReturn("value")
 
     val result = wrapper.get("myKey", String::class.java)
@@ -94,10 +96,26 @@ class SentryCacheWrapperTest {
     assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with type creates span with cache hit true when cached value is null`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    val valueWrapper = mock<Cache.ValueWrapper>()
+    whenever(delegate.get("myKey")).thenReturn(valueWrapper)
+    whenever(delegate.get("myKey", String::class.java)).thenReturn(null)
+
+    val result = wrapper.get("myKey", String::class.java)
+
+    assertNull(result)
+    assertEquals(1, tx.spans.size)
+    assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
+  }
+
   @Test
   fun `get with type creates span with cache hit false on miss`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
+    whenever(delegate.get("myKey")).thenReturn(null)
     whenever(delegate.get("myKey", String::class.java)).thenReturn(null)
 
     val result = wrapper.get("myKey", String::class.java)
@@ -107,6 +125,22 @@ class SentryCacheWrapperTest {
     assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with type sets error status and throwable on exception`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    val exception = RuntimeException("cache error")
+    whenever(delegate.get("myKey")).thenReturn(mock<Cache.ValueWrapper>())
+    whenever(delegate.get("myKey", String::class.java)).thenThrow(exception)
+
+    assertFailsWith<RuntimeException> { wrapper.get("myKey", String::class.java) }
+
+    assertEquals(1, tx.spans.size)
+    val span = tx.spans.first()
+    assertEquals(SpanStatus.INTERNAL_ERROR, span.status)
+    assertEquals(exception, span.throwable)
+  }
+
   // -- get(Object key, Callable<T>) --
 
   @Test
diff --git a/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java b/sentry-spring-jakarta/src/main/java/io/sentry/spring/jakarta/cache/SentryCacheWrapper.java
@@ -67,8 +67,9 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes
       return delegate.get(key, type);
     }
     try {
+      final ValueWrapper wrapper = delegate.get(key);
+      span.setData(SpanDataConvention.CACHE_HIT_KEY, wrapper != null);
       final T result = delegate.get(key, type);
-      span.setData(SpanDataConvention.CACHE_HIT_KEY, result != null);
       span.setStatus(SpanStatus.OK);
       return result;
     } catch (Throwable e) {
diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/cache/SentryCacheWrapperTest.kt
@@ -85,6 +85,8 @@ class SentryCacheWrapperTest {
   fun `get with type creates span with cache hit true on hit`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
+    val valueWrapper = mock<Cache.ValueWrapper>()
+    whenever(delegate.get("myKey")).thenReturn(valueWrapper)
     whenever(delegate.get("myKey", String::class.java)).thenReturn("value")
 
     val result = wrapper.get("myKey", String::class.java)
@@ -94,10 +96,26 @@ class SentryCacheWrapperTest {
     assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with type creates span with cache hit true when cached value is null`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    val valueWrapper = mock<Cache.ValueWrapper>()
+    whenever(delegate.get("myKey")).thenReturn(valueWrapper)
+    whenever(delegate.get("myKey", String::class.java)).thenReturn(null)
+
+    val result = wrapper.get("myKey", String::class.java)
+
+    assertNull(result)
+    assertEquals(1, tx.spans.size)
+    assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
+  }
+
   @Test
   fun `get with type creates span with cache hit false on miss`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
+    whenever(delegate.get("myKey")).thenReturn(null)
     whenever(delegate.get("myKey", String::class.java)).thenReturn(null)
 
     val result = wrapper.get("myKey", String::class.java)
@@ -107,6 +125,22 @@ class SentryCacheWrapperTest {
     assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with type sets error status and throwable on exception`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    val exception = RuntimeException("cache error")
+    whenever(delegate.get("myKey")).thenReturn(mock<Cache.ValueWrapper>())
+    whenever(delegate.get("myKey", String::class.java)).thenThrow(exception)
+
+    assertFailsWith<RuntimeException> { wrapper.get("myKey", String::class.java) }
+
+    assertEquals(1, tx.spans.size)
+    val span = tx.spans.first()
+    assertEquals(SpanStatus.INTERNAL_ERROR, span.status)
+    assertEquals(exception, span.throwable)
+  }
+
   // -- get(Object key, Callable<T>) --
 
   @Test
diff --git a/sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java b/sentry-spring/src/main/java/io/sentry/spring/cache/SentryCacheWrapper.java
@@ -65,8 +65,9 @@ public SentryCacheWrapper(final @NotNull Cache delegate, final @NotNull IScopes
       return delegate.get(key, type);
     }
     try {
+      final ValueWrapper wrapper = delegate.get(key);
+      span.setData(SpanDataConvention.CACHE_HIT_KEY, wrapper != null);
       final T result = delegate.get(key, type);
-      span.setData(SpanDataConvention.CACHE_HIT_KEY, result != null);
       span.setStatus(SpanStatus.OK);
       return result;
     } catch (Throwable e) {
diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/cache/SentryCacheWrapperTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/cache/SentryCacheWrapperTest.kt
@@ -83,6 +83,8 @@ class SentryCacheWrapperTest {
   fun `get with type creates span with cache hit true on hit`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
+    val valueWrapper = mock<Cache.ValueWrapper>()
+    whenever(delegate.get("myKey")).thenReturn(valueWrapper)
     whenever(delegate.get("myKey", String::class.java)).thenReturn("value")
 
     val result = wrapper.get("myKey", String::class.java)
@@ -92,10 +94,26 @@ class SentryCacheWrapperTest {
     assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with type creates span with cache hit true when cached value is null`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    val valueWrapper = mock<Cache.ValueWrapper>()
+    whenever(delegate.get("myKey")).thenReturn(valueWrapper)
+    whenever(delegate.get("myKey", String::class.java)).thenReturn(null)
+
+    val result = wrapper.get("myKey", String::class.java)
+
+    assertNull(result)
+    assertEquals(1, tx.spans.size)
+    assertEquals(true, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
+  }
+
   @Test
   fun `get with type creates span with cache hit false on miss`() {
     val tx = createTransaction()
     val wrapper = SentryCacheWrapper(delegate, scopes)
+    whenever(delegate.get("myKey")).thenReturn(null)
     whenever(delegate.get("myKey", String::class.java)).thenReturn(null)
 
     val result = wrapper.get("myKey", String::class.java)
@@ -105,6 +123,22 @@ class SentryCacheWrapperTest {
     assertEquals(false, tx.spans.first().getData(SpanDataConvention.CACHE_HIT_KEY))
   }
 
+  @Test
+  fun `get with type sets error status and throwable on exception`() {
+    val tx = createTransaction()
+    val wrapper = SentryCacheWrapper(delegate, scopes)
+    val exception = RuntimeException("cache error")
+    whenever(delegate.get("myKey")).thenReturn(mock<Cache.ValueWrapper>())
+    whenever(delegate.get("myKey", String::class.java)).thenThrow(exception)
+
+    assertFailsWith<RuntimeException> { wrapper.get("myKey", String::class.java) }
+
+    assertEquals(1, tx.spans.size)
+    val span = tx.spans.first()
+    assertEquals(SpanStatus.INTERNAL_ERROR, span.status)
+    assertEquals(exception, span.throwable)
+  }
+
   // -- get(Object key, Callable<T>) --
 
   @Test
