allthingssecurity opened a new pull request, #29438:
URL: https://github.com/apache/camel/pull/29438

   # Description
   
   [CAMEL-26737](https://issues.apache.org/jira/browse/CAMEL-26737)
   
   Follow-up of #29405 (CAMEL-26708), as asked in its review.
   
   `JdbcKeyValueRepository.replace(key, expected, newValue, ttl)` and 
`delete(key, expected)` did not look at `EXPIRES_AT`. On an entry whose TTL had 
passed they matched, returned `true`, and `replace` gave the entry a new TTL, 
so a value that had expired came back. For the same key `get` returns `null` 
and `putIfAbsent` treats it as absent.
   
   Change:
   - The default `updateIfValueString` and `deleteIfValueString` end with `AND 
(EXPIRES_AT = 0 OR EXPIRES_AT > ?)`, and `replace` / `delete(key, expected)` 
pass the current time as the last parameter. This is the condition of 
`selectKeysString`. On an expired entry both return `false` and leave the row 
alone, as `MemoryKeyValueRepository` does. Lazy expiry deletes it later, as 
before.
   - The statements are still derived from the defaults when `tableName` is 
set, as in `doInit` today.
   - The descriptions of the two options say that they match only an entry that 
has not expired. The bean metadata is regenerated and its catalog copy updated.
   - The Javadoc of both methods, a note in the JDBC section of 
`keyValueRepository.adoc` (the catalog copy is byte-identical), and a 4.24 
upgrade guide entry are updated. The upgrade guide entry gives both new default 
statements and says that users who set their own `updateIfValueString` / 
`deleteIfValueString` must add the condition with its parameter. Otherwise the 
repository passes one more parameter than the statement has placeholders: most 
drivers fail with an SQL error, while MariaDB Connector/J 3.x ignores the extra 
parameter (read from the driver code, not run) and the old statement keeps 
matching an expired entry. No YAML sample changes, so `eip-samples.json` is not 
touched.
   
   No Camel component calls these two methods; they are part of the 
`KeyValueRepository` API.
   
   Not in this PR: `toExpiresAt` adds the TTL to the current time without an 
overflow check, so a TTL near `Long.MAX_VALUE` milliseconds stores a negative 
`EXPIRES_AT`. `get` treats such an entry as never expiring, while `keys()` (and 
now `replace` / `delete(key, expected)`) treat it as expired. 
`MemoryKeyValueRepository.toExpiresAt` has the same unchecked addition (there 
the entry expires at once). That is older than this change; a JIRA will be 
filed for it.
   
   For `main` only (4.24): the parameters of two customizable statements change.
   
   Tests (`JdbcKeyValueRepositoryTest`, embedded H2, no Docker; the expired row 
is written with `EXPIRES_AT = 1` through the repository's own `insertString`, 
so no sleeps):
   - `testReplaceExpiredEntry`: `replace` on an expired entry returns `false`; 
the row keeps its value and `EXPIRES_AT`, and `get` returns `null`.
   - `testDeleteWithExpectedValueExpiredEntry`: the same for `delete(key, 
expected)`.
   - `testReplaceAndDeleteWithExpectedValueEntryWithTtl` (control, passes on 
main too): on an entry with a TTL that has not passed, both still succeed (the 
existing tests cover entries without a TTL).
   - `testReplaceAndDeleteWithExpectedValueCustomTableName`: with `tableName`, 
the derived statements skip an expired entry and work on a live one.
   
   On main the first, second and fourth fail (`expected: <false> but was: 
<true>`). With the fix all pass, and the two key-value test classes passed 20 
of 20 runs (38 tests per run, 33 + 5, one JVM per run). camel-sql module tests: 
346 run (offline), 0 failures. `SqlFunctionDataSourceTest` was left out: it 
installs an embedded MariaDB and cannot run offline here (unrelated to this 
change).
   
   # 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): 
[CAMEL-26737](https://issues.apache.org/jira/browse/CAMEL-26737).
   
   # 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 the core modules and camel-sql with the formatter and import-sort 
plugins and ran the camel-sql tests. The build regenerated the bean metadata of 
`JdbcKeyValueRepository` with the committed content, and I copied it to the 
catalog. The catalog copy of the doc page was updated by hand to match the 
source page. 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]

Reply via email to