Croway commented on PR #29273:
URL: https://github.com/apache/camel/pull/29273#issuecomment-6682961245
@gnodet-bot @davsclaus thanks for the reviews. I checked each point against
f8955b30bfd8 and added three commits.
1. **`SftpATTRS` in a public method:** changed in 126790db7161.
`SftpOperations` already has JSch types in its public API (the
`SftpOperations(Proxy)` constructor, `ExtendedUserInfo extends UserInfo`, the
protected `createSession` returning a `Session`), and the move to mwiede/jsch
kept the `com.jcraft.jsch` package, so it did not break them. But this method
is new in this PR, and the read lock only needs two values. So
`getFileAttributes` now returns a small `SftpOperations.FileAttributes(long
fileLength, long lastModified)` record, with the modification time in millis,
as `SftpRemoteFile.getLastModified()`. It cannot be package-private, because
`SftpExclusiveReadLockCheck` is in the `strategy` package. No existing public
method changes.
2. **Pinning the JSch behaviour that `isLstatUnanswered` relies on:** a unit
test with hand-made `SftpException`s would only test the one-line helper. It
builds the exceptions itself, so it cannot see JSch change how it builds them.
What pins the behaviour is the real JSch (2.28.8) against a server:
- A status sent by the server has no cause:
`listsDirectoryWhenServerDoesNotAnswerLstat` (5 statuses, both
`fastExistsCheck` values) and
`listsDirectoryWhenServerAnswersNeitherLstatNorStatOfFile` cover it against the
embedded server, and they run in the PR build. camel-ftp runs failsafe in its
default `full` profile, and the JDK 21 build of f8955b30bfd8 ran
`SftpExistsFileIT`: 22 tests, 0 failures. If JSch gave a status a cause, these
12 tests would fail. I checked that by disabling the fallback: 12 errors.
- An I/O failure has a cause: this side had no test, so I added
`failsWhenConnectionIsLostDuringLstat` in 01b0467b6dcd. The embedded server
drops the connection when it receives the LSTAT. The check must then fail, with
an `SftpException` that has a cause (here `4: java.io.IOException: inputstream
is closed`), so the test fails if JSch stops giving that cause. If the check
answered "missing" for that failure, the test would fail too (2 failures). 30
repeated runs passed. On this side, a missing cause would not change the
outcome: the fallback listing goes through the same lost connection, and it
fails as well. The cause only saves a listing that cannot succeed.
3. **Long line in the upgrade guide:** rewrapped at 120 columns in
96f98e5e9c14.
Whole `camel-ftp` module (`mvn verify`): 143 unit tests, 475 + 1 ITs, 0
failures.
_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]
