Croway opened a new pull request, #29273: URL: https://github.com/apache/camel/pull/29273
JIRA: [CAMEL-26656](https://issues.apache.org/jira/browse/CAMEL-26656) ### What was wrong The JSch-based `sftp` component listed a whole directory to check one file: - `SftpOperations.existsFile` listed the parent directory and compared the names. - `fastExistsFile` (`fastExistsCheck=true`) calls `channel.ls(name)`. In JSch 2.28.8, for an existing file, that is STAT, then OPENDIR of the parent and READDIR to the end. Only a missing file was cheap. - `readLock=changed` (`SftpExclusiveReadLockCheck`) listed the parent directory on every check, with or without `fastExistsCheck`. So `tempFileName`/`tempPrefix` (two checks per message), `fileExist=Ignore|Fail|Move`, a producer `doneFileName` and `readLock=changed` (two checks per file with the default `readLockMinAge=0`) cost one listing of the whole directory per check. ### What changed - `existsFile` and `fastExistsFile` send a single `channel.lstat(path)`, where `SSH_FX_NO_SUCH_FILE` means false. The path is quoted with `quotePathForJsch`, as `isSymbolicLink` does. The directory is split from the name with `FileUtil.onlyPath`/`stripPath`, as before. - If the server answers the LSTAT with `PERMISSION_DENIED`, `OP_UNSUPPORTED` or the generic `FAILURE` (a status from the server, not an I/O failure), the check falls back to the listing it used before: the parent listing for `existsFile`, `ls(name)` for `fastExistsFile`. - `readLock=changed` reads the size and modification time with one LSTAT per check, through the new public `SftpOperations.getFileAttributes(String)`, with the same fallback. Operations that are not `SftpOperations` keep the old listing. - **Symbolic links:** I chose `lstat`, not `stat`, on purpose. The directory listing (OpenSSH and MINA SSHD) reports the link's own attributes. So with `lstat` the read lock keeps comparing the link's own size and modification time, as before, and the fallback listing gives the same answer. With `stat`, the read lock would have started following links, which is a separate behaviour change. The choice is documented in the code and in the docs. - No option or default changes. FTP/FTPS, `camel-mina-sftp`, and how `put`/`get`/`rm`/`rename` pass paths are untouched. ### Tests New `SftpExistsFileIT`, against the embedded MINA SFTP server, counts requests and directory entries with an `SftpEventListener`: - `existsFile`, with `fastExistsCheck` true and false: existing file, missing file, directory path, missing directory, in a 10-file and a 1000-file directory. Every check is exactly 1 LSTAT, 0 entries listed. - The fallback: when the server answers LSTAT with PERMISSION_DENIED, OP_UNSUPPORTED or FAILURE, the answers are still correct, through the listing. - A dangling symbolic link exists, with `fastExistsCheck` true and false. - Producer: `fileExist=Fail` (both `fastExistsCheck` values) and `tempFileName` into a 1000-file directory: 2 LSTAT, 0 OPENDIR, 0 entries. - `readLock=changed` in a 1000-file directory: the only OPENDIR is the poll, and fewer than 1000 entries are listed. - `readLock=changed` still waits for a changing file. The file grows before each of the first three checks; this is driven from the server listener, with no sleeps. The consumed body is the complete file. Evidence: - origin/main: `Tests run: 16, Failures: 16`. With the fix: `Tests run: 16, Failures: 0`. - The earlier reproducer (camel-sftp, 2000-file directory, per message): `tempPrefix` 56 -> 18 requests, 4,184 -> 129 entries; `fileExist=Ignore` 35 -> 16 requests, 2,157 -> 129 entries; `readLock=changed` +24 requests/+2,004 entries -> +2 requests/+0 entries per file. The 129 entries left are the `autoCreate` directory check, which is a separate issue. - Whole `camel-ftp` module (`mvn verify`: unit tests and all embedded-server ITs): BUILD SUCCESS: 143 unit tests, 467 + 1 ITs, 0 failures, after rebase on the latest main. The skipped tests are existing manual, IPv6 and FTPS-implicit ones. ### Docs - `sftp-component.adoc`: a new section "Checking whether a file exists". It covers one request per check, `fastExistsCheck` no longer making a difference for these checks with sftp, symbolic links (not followed, so a dangling link exists and the read lock compares the link's own attributes), case-insensitive servers, and the fallback. The catalog copy is refreshed. - 4.24 upgrade guide: dangling links now exist with `fastExistsCheck=true`. On case-insensitive servers (Windows- or macOS-backed), a case-variant name now exists. For example, `fileExist=Fail` sending `REPORT.TXT` next to `report.txt` now fails, where before it replaced `report.txt`. I checked this on APFS with the embedded server. ### Backport Not proposed for camel-4.18.x / camel-4.22.x. The change alters existence answers on case-insensitive servers, and for dangling links with `fastExistsCheck=true`, which needs the upgrade-guide note. The same code exists on both branches, so a backport is mechanical if wanted. _Claude Code on behalf of Federico Mariani_ -- 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]
