gnodet-bot commented on code in PR #29505:
URL: https://github.com/apache/camel/pull/29505#discussion_r4559770374
##########
components/camel-redis/src/test/java/org/apache/camel/component/redis/processor/aggregate/integration/RedisAggregationRepositoryOperationsIT.java:
##########
@@ -151,6 +156,131 @@ public void testOptimisticRemoveWithRecovery() {
}
}
+ @Test
+ public void testOptimisticRemoveWhenRecoveryWriteFails() {
+ String mapName = "optimisticRemoveRecoveryFailure";
+ RedisAggregationRepository repo = createRepo(mapName, true);
+
+ try {
+ // 1. Add an exchange to the active map
+ Exchange exchange = createExchange("recoverable");
+ repo.add(context, "key1", null, exchange);
+
+ // 2. Create a Redis String at the recovery map's key
+ String recoveryMapName = mapName + "-completed";
+ repo.getRedisson()
+ .getBucket(recoveryMapName)
+ .set("wrong-type");
+
+ // 3. Attempt removal; recovery write should fail
+ assertThrows(RedisException.class,
+ () -> repo.remove(context, "key1", exchange));
+
+ // 4. Verify the exchange still exists in the active map
+ Exchange result = repo.get(context, "key1");
+ assertNotNull(result);
+ assertEquals("recoverable", result.getIn().getBody(String.class));
+
+ } finally {
+ repo.stop();
+ }
+ }
+
+ @Test
+ public void testOptimisticRemoveDoesNotPopulateRecoveryOnLockingFailure() {
+ String mapName = "optimisticRemoveLockingFailure";
+ RedisAggregationRepository repo = createRepo(mapName, true);
+
+ try {
+ // 1. Add the original Exchange
+ Exchange original = createExchange("original");
+ repo.add(context, "key1", null, original);
+
+ // 2. Retrieve the original Exchange
+ Exchange stale = repo.get(context, "key1");
+
+ // 3. Simulate another update to the active Exchange
+ Exchange updated = createExchange("updated");
+ repo.add(context, "key1", stale, updated);
+
+ // 4. Attempt to remove the stale Exchange
+ assertThrows(OptimisticLockingException.class,
+ () -> repo.remove(context, "key1", stale));
+
+ // 5. Verify the updated Exchange remains active
+ Exchange result = repo.get(context, "key1");
+ assertNotNull(result);
+ assertEquals("updated", result.getIn().getBody(String.class));
+
+ // 6. Verify the stale Exchange was removed from recovery storage
Review Comment:
💡 **Misleading comment** — the stale exchange was never *written* to
recovery storage, so nothing was "removed." The Lua script returns 0 when the
CAS check fails, leaving the recovery map untouched. The comment should say
`was never written`:
```suggestion
// 6. Verify the stale Exchange was never written to recovery
storage
```
--
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]
