Skip to content

Commit 6b8adb7

Browse files
gnodetclaude
authored andcommitted
CAMEL-23239: Fix review issues in camel-state-store
- Fix Infinispan backend lifecycle: nullify cache in stop() to allow restart - Fix InMemory putIfAbsent race condition: use compute() for atomicity - Fix Caffeine put()/delete() atomicity: use asMap() for atomic previous-value return - Fix Redis stop(): nullify mapCache for lifecycle hygiene - Increase TTL test sleep margins to 5x to prevent CI flakiness - Document first-one-wins semantics on getOrCreateBackend() Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
1 parent de505ad commit 6b8adb7

10 files changed

Lines changed: 25 additions & 21 deletions

File tree

components/camel-state-store/camel-state-store-caffeine/src/main/java/org/apache/camel/component/statestore/caffeine/CaffeineStateStoreBackend.java

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -35,8 +35,7 @@ public class CaffeineStateStoreBackend implements StateStoreBackend {
3535

3636
@Override
3737
public Object put(String key, Object value, long ttlMillis) {
38-
TimedValue previous = cache.getIfPresent(key);
39-
cache.put(key, new TimedValue(value, ttlMillis));
38+
TimedValue previous = cache.asMap().put(key, new TimedValue(value, ttlMillis));
4039
return previous != null ? previous.value() : null;
4140
}
4241

@@ -48,8 +47,7 @@ public Object get(String key) {
4847

4948
@Override
5049
public Object delete(String key) {
51-
TimedValue previous = cache.getIfPresent(key);
52-
cache.invalidate(key);
50+
TimedValue previous = cache.asMap().remove(key);
5351
return previous != null ? previous.value() : null;
5452
}
5553

components/camel-state-store/camel-state-store-caffeine/src/test/java/org/apache/camel/component/statestore/caffeine/CaffeineStateStoreBackendTest.java

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -151,7 +151,7 @@ void testTtlExpiry() throws Exception {
151151
Map.of(StateStoreConstants.KEY, "ttlKey"));
152152
assertEquals("expiring", result);
153153

154-
Thread.sleep(400);
154+
Thread.sleep(1000);
155155
backend.keys(); // trigger Caffeine cleanup
156156

157157
result = template.requestBodyAndHeaders(
@@ -172,7 +172,7 @@ void testPerMessageTtlHeader() throws Exception {
172172
Map.of(StateStoreConstants.KEY, "ttlKey"));
173173
assertEquals("expiring", result);
174174

175-
Thread.sleep(400);
175+
Thread.sleep(1000);
176176
backend.keys();
177177

178178
result = template.requestBodyAndHeaders(

components/camel-state-store/camel-state-store-infinispan/src/main/java/org/apache/camel/component/statestore/infinispan/InfinispanStateStoreBackend.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,7 @@ public void start() {
120120

121121
@Override
122122
public void stop() {
123+
cache = null;
123124
if (managedCacheManager && cacheManager != null) {
124125
cacheManager.stop();
125126
cacheManager = null;

components/camel-state-store/camel-state-store-infinispan/src/test/java/org/apache/camel/component/statestore/infinispan/InfinispanStateStoreBackendIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -176,7 +176,7 @@ void testTtlExpiry() throws Exception {
176176
Map.of(StateStoreConstants.KEY, "ttlKey"));
177177
assertEquals("expiring", result);
178178

179-
Thread.sleep(700);
179+
Thread.sleep(2500);
180180

181181
result = template.requestBodyAndHeaders(
182182
"direct:get", null,

components/camel-state-store/camel-state-store-redis/src/main/java/org/apache/camel/component/statestore/redis/RedisStateStoreBackend.java

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -96,6 +96,7 @@ public void start() {
9696

9797
@Override
9898
public void stop() {
99+
mapCache = null;
99100
if (managedRedisson && redisson != null) {
100101
redisson.shutdown();
101102
redisson = null;

components/camel-state-store/camel-state-store-redis/src/test/java/org/apache/camel/component/statestore/redis/RedisStateStoreBackendIT.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ void testTtlExpiry() throws Exception {
166166
Map.of(StateStoreConstants.KEY, "ttlKey"));
167167
assertEquals("expiring", result);
168168

169-
Thread.sleep(700);
169+
Thread.sleep(2500);
170170

171171
result = template.requestBodyAndHeaders(
172172
"direct:get", null,

components/camel-state-store/camel-state-store/src/main/java/org/apache/camel/component/statestore/InMemoryStateStoreBackend.java

Lines changed: 8 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -76,16 +76,15 @@ public boolean contains(String key) {
7676
@Override
7777
public Object putIfAbsent(String key, Object value, long ttlMillis) {
7878
long expiresAt = ttlMillis > 0 ? System.currentTimeMillis() + ttlMillis : 0;
79-
Entry existing = store.putIfAbsent(key, new Entry(value, expiresAt));
80-
if (existing != null) {
81-
if (existing.isExpired()) {
82-
// expired, replace it
83-
store.replace(key, existing, new Entry(value, expiresAt));
84-
return null;
79+
Object[] result = new Object[1];
80+
store.compute(key, (k, current) -> {
81+
if (current == null || current.isExpired()) {
82+
return new Entry(value, expiresAt);
8583
}
86-
return existing.value();
87-
}
88-
return null;
84+
result[0] = current.value();
85+
return current;
86+
});
87+
return result[0];
8988
}
9089

9190
@Override

components/camel-state-store/camel-state-store/src/main/java/org/apache/camel/component/statestore/StateStoreComponent.java

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,11 @@ protected Endpoint createEndpoint(String uri, String remaining, Map<String, Obje
4343
return endpoint;
4444
}
4545

46+
/**
47+
* Returns the backend for the given store name, creating one if needed. Uses first-one-wins semantics: if a backend
48+
* already exists for this store name, subsequent calls return the existing one regardless of the explicit backend
49+
* passed.
50+
*/
4651
StateStoreBackend getOrCreateBackend(String storeName, StateStoreBackend explicitBackend) {
4752
if (explicitBackend != null) {
4853
return backends.computeIfAbsent(storeName, k -> {

components/camel-state-store/camel-state-store/src/test/java/org/apache/camel/component/statestore/StateStoreTest.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -191,7 +191,7 @@ void testPerMessageTtlHeader() throws Exception {
191191
Map.of(StateStoreConstants.KEY, "ttlKey"));
192192
assertEquals("expiring", result);
193193

194-
Thread.sleep(300);
194+
Thread.sleep(1000);
195195

196196
// should be expired
197197
result = template.requestBodyAndHeaders(

components/camel-state-store/camel-state-store/src/test/java/org/apache/camel/component/statestore/StateStoreTtlTest.java

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -38,8 +38,8 @@ void testEntryExpiresAfterTtl() throws Exception {
3838
java.util.Map.of(StateStoreConstants.KEY, "ttlKey"));
3939
assertEquals("expiring", result);
4040

41-
// wait for TTL to expire
42-
Thread.sleep(300);
41+
// wait for TTL to expire (5x margin over 200ms TTL)
42+
Thread.sleep(1000);
4343

4444
// should be expired now
4545
Object expired = template.requestBodyAndHeaders(
@@ -54,7 +54,7 @@ void testContainsReturnsFalseAfterTtl() throws Exception {
5454
"direct:put", "expiring",
5555
java.util.Map.of(StateStoreConstants.KEY, "ttlKey"));
5656

57-
Thread.sleep(300);
57+
Thread.sleep(1000);
5858

5959
Object exists = template.requestBodyAndHeaders(
6060
"direct:contains", null,

0 commit comments

Comments
 (0)