Skip to content

GH-3247: Fix SftpSession.exists for error code #3248

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Apr 14, 2020

Conversation

artembilan
Copy link
Member

Fixes #3247

When there is no path on the SFTP server, a ChannelSftp.SSH_FX_NO_SUCH_FILE
error is returned in the thrown SftpException.

  • Fix SftpSession.exists() to check for the SSH_FX_NO_SUCH_FILE to
    return false and re-throw an exception otherwise
  • Add mock test for SftpSession.exists()
  • Add org.mockito.AdditionalMatchers to checkstyle.xml exclusions

Cherry-pick to 5.2.x & 5.1.x

Fixes spring-projects#3247

When there is no path on the SFTP server, a `ChannelSftp.SSH_FX_NO_SUCH_FILE`
error is returned in the thrown `SftpException`.

* Fix `SftpSession.exists()` to check for the `SSH_FX_NO_SUCH_FILE` to
return `false` and re-throw an exception otherwise
* Add mock test for `SftpSession.exists()`
* Add `org.mockito.AdditionalMatchers` to `checkstyle.xml` exclusions

**Cherry-pick to 5.2.x & 5.1.x**
Copy link
Contributor

@garyrussell garyrussell left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, but I think we need a test against the mina server, e.g. in SftpServerOutboundTests to verify we actually get that result from JSch.

@artembilan artembilan requested a review from garyrussell April 14, 2020 15:28
@garyrussell
Copy link
Contributor

Probably not related but

> Task :spring-integration-sftp:test
SftpStreamingMessageSourceTests > testMaxFetchLambdaFilter FAILED
    org.springframework.messaging.MessagingException at SftpStreamingMessageSourceTests.java:160
        Caused by: org.springframework.integration.util.PoolItemNotAvailableException at SftpStreamingMessageSourceTests.java:160
            Caused by: java.lang.IllegalStateException at SftpStreamingMessageSourceTests.java:160
                Caused by: java.lang.IllegalStateException at SftpStreamingMessageSourceTests.java:160
                    Caused by: com.jcraft.jsch.JSchException at SftpStreamingMessageSourceTests.java:160

@artembilan
Copy link
Member Author

@garyrussell ,

It isn't related, indeed, but I can't figure out such a sporadic failure. Something is broken sometimes on Mina.
So, I have restarted the build and now it is green.

Feel free to merge!

Thanks

@garyrussell garyrussell merged commit b75f1bd into spring-projects:master Apr 14, 2020
garyrussell pushed a commit that referenced this pull request Apr 14, 2020
* GH-3247: Fix `SftpSession.exists` for error code

Fixes #3247

When there is no path on the SFTP server, a `ChannelSftp.SSH_FX_NO_SUCH_FILE`
error is returned in the thrown `SftpException`.

* Fix `SftpSession.exists()` to check for the `SSH_FX_NO_SUCH_FILE` to
return `false` and re-throw an exception otherwise
* Add mock test for `SftpSession.exists()`
* Add `org.mockito.AdditionalMatchers` to `checkstyle.xml` exclusions

**Cherry-pick to 5.2.x & 5.1.x**

* * Add exists tests against Mina embedded server
@garyrussell
Copy link
Contributor

Hmmm - I just cherry-picked this to 5.2 but while resolving conflicts in 5.1, I changed the exception to UncheckedIOException to avoid a breaking change. I will push that to the other branches as well.

garyrussell pushed a commit that referenced this pull request Apr 14, 2020
* GH-3247: Fix `SftpSession.exists` for error code

Fixes #3247

When there is no path on the SFTP server, a `ChannelSftp.SSH_FX_NO_SUCH_FILE`
error is returned in the thrown `SftpException`.

* Fix `SftpSession.exists()` to check for the `SSH_FX_NO_SUCH_FILE` to
return `false` and re-throw an exception otherwise
* Add mock test for `SftpSession.exists()`
* Add `org.mockito.AdditionalMatchers` to `checkstyle.xml` exclusions

**Cherry-pick to 5.2.x & 5.1.x**

* * Add exists tests against Mina embedded server
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SftpSession.exists() does not throw exception
2 participants