allthingssecurity opened a new pull request, #27164: URL: https://github.com/apache/camel/pull/27164
# Description [CAMEL-25215](https://issues.apache.org/jira/browse/CAMEL-25215) `KeyValueRepository` has default `putIfAbsent`, `replace` and `delete(key, expectedValue)` implementations that are documented as not atomic, and repositories backed by a store with atomic operations should override them. The Caffeine, Hazelcast, Infinispan, Cassandra, JDBC, JPA, Kafka and Redis repositories override `putIfAbsent` (the operation the idempotent adapter uses) with an atomic operation of their store; `EhcacheKeyValueRepository` and `JCacheKeyValueRepository` did not override any of the three. `KeyValueIdempotentRepository.add` is a `putIfAbsent`, and the Idempotent Consumer calls `add` without a lock. With these two repositories, two exchanges with the same message id could both be added and both processed. This is the defect of CAMEL-25156 (Caffeine) and CAMEL-25155 (Spring Redis) in the key-value adapter. The state-store `putIfAbsent` operation has the same race. This change, in both repositories: - `putIfAbsent` uses the cache's `putIfAbsent`. An expired entry (these repositories expire entries lazily) counts as absent and is replaced with `replace(key, expired, new)`, retrying if the entry changed in the meantime. - `replace` and `delete(key, expectedValue)` compare the value and then use `replace(key, current, new)` / `remove(key, current)`, and retry if the entry changed in the meantime. - The lazy removal of an expired entry in `get`, `contains` and `keys` removes only that entry, so it cannot delete a new entry stored for the key in the meantime. - `KeyValueTtlValue` gets `equals` and `hashCode` based on a random token set per write and the expiry time, not on the wrapped value. A cache that stores copies of its values (the JCache default `storeByValue=true`, Ehcache with a value copier or an off-heap tier) compares the stored copy with the expected entry, and the wrapped value may have no value equality (`byte[]`, a POJO without `equals`). The token survives the copy, so a compare-and-swap fails only when another thread changed the entry, and the retry loops end. (An equality on the wrapped value would make `putIfAbsent` spin forever on an expired `byte[]` entry in such a cache.) The key-value repositories are new in 4.23, so no release is affected. Tests: - New `EhcacheKeyValueRepositoryConcurrentPutIfAbsentTest` and `JCacheKeyValueRepositoryConcurrentPutIfAbsentTest`. Two threads call `KeyValueIdempotentRepository.add` for the same key, through a cache whose `get` returns only when both threads have read the key. Exactly one `add` must return true. - Without the change both tests fail (`[true, true]`). With it, they pass. - New `putIfAbsent` tests for a new, an existing and an expired key in both repository tests. - New `EhcacheKeyValueRepositoryByValueTest` (a serializing value copier) and `JCacheKeyValueRepositoryByValueTest` (a JCache whose `replace` and `remove(k, v)` compare the stored copy with `equals`; the Hazelcast provider used by the module tests compares serialized bytes). With an expired `byte[]` entry, `putIfAbsent` must return within 5 seconds and `get` must remove the entry. With an equality on the wrapped value both tests fail in both modules (`execution timed out after 5000 ms`, and the expired entry left in the cache). - The JCache concurrency test takes about 2 seconds with the change: only the thread that lost the race reads the key, so it waits for the barrier timeout. - With the change, camel-ehcache passes 69 tests, camel-jcache 91, and the key-value tests of camel-support (69) and camel-core (20) pass. # Target - [x] I checked that the commit is targeting the correct branch (Camel 4 uses the `main` branch) # Tracking - [x] If this is a large change, bug fix, or code improvement, I checked there is a [JIRA issue](https://issues.apache.org/jira/browse/CAMEL) filed for the change (usually before you start working on it). # Apache Camel coding standards and style - [x] I checked that each commit in the pull request has a meaningful subject line and body. - [ ] I have run `mvn clean install -DskipTests` locally from root folder and I have committed all auto-generated changes. (I built and tested the affected module, including the formatter and import-sort plugins. I did not run the full root build.) # AI-assisted contributions - [x] If this PR includes AI-generated code, commits have proper co-authorship attribution (e.g., `Co-authored-by` trailers) and the PR description identifies the AI tool used. This PR was prepared with Claude Code (Claude Opus 5.5). The commit carries a `Co-Authored-By` trailer. _Claude Code on behalf of allthingssecurity_ 🤖 Generated with [Claude Code](https://claude.com/claude-code) -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
