davsclaus commented on PR #28861:
URL: https://github.com/apache/camel/pull/28861#issuecomment-6611043513
Follow-up to the review of 2026-11-02: I checked its points against current
`main`, and they still hold. Some notes and suggested next steps:
**Notes**
- The race only affects `idempotent=false`, which is the default with `move`
or `delete`. With `idempotent=true`, `processStrategyCommit` adds the
idempotent key before the in-progress key is removed, so the second poll is
rejected by `notUnique()`.
- For the local file consumer there is no "residual window" after the check.
The commit moves the file before it removes the in-progress key. So if `add()`
succeeds and the file still exists, the file is either unmoved or a new file
with the same name.
**Suggested next steps**
1. **Scope the check to `FileConsumer`.** Add a protected hook in
`GenericFileConsumer` that defaults to `true`, and override it in
`FileConsumer` with a local `File.exists()`. In the shared `isValidFile()`, the
check breaks FTP and JSch SFTP with relative directories: with `stepwise=true`
the consumer has already changed into the directory, so
`existsFile("inbox/a.txt")` resolves to `inbox/inbox/a.txt` and every file is
skipped. Without `fastExistsCheck` it also lists the parent directory once per
file.
2. **On the skip path, also remove the eager idempotent key** when
`idempotent` and `idempotentEager` are on, as `removeExcessiveIdempotentFile()`
does. Otherwise, after a rollback with `moveFailed`, or when the file is
deleted from outside, every later file with that name is treated as a duplicate.
3. **Make the test deterministic.** Use a custom `inProgressRepository`
whose `add()` first moves the target file to `.camel`, which simulates a commit
landing between `contains()` and `add()`. Assert that `mock:done` receives
nothing and that the key is not kept. Add an
`idempotent=true&idempotentEager=true` variant that checks the idempotent
repository does not keep the key either.
4. **Run the camel-ftp tests.** CI only built `camel-file` and `camel-core`,
so the relative-directory regression was not caught.
5. **File a follow-up JIRA for the remote consumers** (FTP, SFTP, SMB,
azure-files). They have the same race, but a check there must resolve against
the current directory, must be cheap, and must not let unchecked exceptions
(`SMBRuntimeException`, `ShareStorageException`) leak the in-progress key.
6. **Backport.** The PR has the `lts` label, but CAMEL-25778 has no
fixVersion yet. Let's decide that once the scoped fix is in.
_Claude Code on behalf of Claus Ibsen_
--
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]
