gnodet commented on PR #22158:
URL: https://github.com/apache/camel/pull/22158#issuecomment-5305902659
Thanks for the thorough review @davsclaus! All 12 findings have been
addressed in the latest push. Here's a summary:
### Critical
1. **MojoHelper backend module registrations** — ✅ Fixed. Added `case
"camel-state-store":` in `MojoHelper.getComponentPath()` registering all 4
sub-modules (camel-state-store, camel-state-store-caffeine,
camel-state-store-redis, camel-state-store-infinispan).
### Medium
2. **`StateStoreComponent.doStop()` resource leak** — ✅ Fixed. Each
`backend.stop()` is now wrapped in try-catch. Exceptions are logged at WARN,
the first is captured and re-thrown after all backends have been stopped and
the map cleared.
3. **Case-sensitive operation lookup** — ✅ Fixed. `determineOperation()`
first tries exact `valueOf()`, then falls back to case-insensitive matching. On
failure, throws a clear error listing all valid operations.
4. **Infinispan `Thread.sleep()` in `start()`** — ✅ Fixed. Removed the retry
loop entirely. `start()` now creates the `RemoteCacheManager` and calls
`getOrCreateCache()` directly — fails fast if the server is unavailable.
5. **Infinispan IT `RemoteCacheManager` leak** — ✅ Fixed. Added `@AfterEach
cleanUp()` that stops and nulls the `testCacheManager`.
6. **Adoc include directives misplaced** — ✅ Fixed. The includes now match
the standard Camel pattern. CI generated-files check passes.
### Low
7. **Redis/Infinispan `managed*` flag not reset in `stop()`** — ✅ Fixed.
Both `RedisStateStoreBackend.stop()` and `InfinispanStateStoreBackend.stop()`
now reset `managedRedisson`/`managedCacheManager` to `false`.
8. **Silent backend discard** — ✅ Fixed. A WARN log is now emitted when a
`storeName` already has a backend and an explicit `backend=#ref` is ignored.
9. **No fallback log** — ✅ Fixed. Three cases covered: INFO when falling
back to `InMemoryStateStoreBackend`, DEBUG when auto-discovering a single
backend, WARN when multiple backends are found and cannot auto-select.
10. **`InMemoryStateStoreBackend.keys()` eviction** — ✅ Fixed. Both `keys()`
and `size()` now call `evictExpired()` before returning results, using
`removeIf(e -> e.getValue().isExpired())`.
11. **Redundant caffeine version** — ✅ Fixed. Removed the explicit
`${caffeine-version}` from `camel-state-store-caffeine/pom.xml`; it inherits
from parent BOM.
12. **Test gaps** — ✅ Fixed. Added:
- Redis IT `testDelete()` now verifies key is gone after deletion
- Infinispan IT `testClear()` and negative `contains("missing")` case
- `testInvalidOperationThrows()` for bogus operation name
- `testDeleteNonExistentKey()` across in-memory, Redis, and Infinispan
- `testCaseInsensitiveOperation()` for mixed case operations
_Claude Code on behalf of Guillaume Nodet_
--
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]
