Skip to content

Apply NullAbility to sftp module #10292

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 5 commits into from
Aug 8, 2025
Merged

Apply NullAbility to sftp module #10292

merged 5 commits into from
Aug 8, 2025

Conversation

@ngocnhan-tran1996
Copy link
Contributor

Related to: #10083

@artembilan artembilan added this to the 7.0.0-M2 milestone Aug 8, 2025
@ngocnhan-tran1996
Copy link
Contributor Author

@artembilan

Has anything been updated regarding the PR build?
Because the log shows the following error:

* Where:
Build file '/home/runner/work/spring-integration/spring-integration/build.gradle' line: 532

* What went wrong:
A problem occurred evaluating root project 'spring-integration'.
> org/gradle/util/ConfigureUtil

@artembilan
Copy link
Member

Has anything been updated regarding the PR build?

Yeah... Looks like gradle-setup GH action no install Gradle 9.0.0 which we are not ready yet.
We have to use a wrapper in CI/CD instead.
Give me a couple mins to review your PR and I'll push the fix for GHA workflows I have tested in other projects.

artembilan
artembilan requested changes Aug 8, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Great work!
Let's see if my review valid!
Thank you!

@artembilan
Copy link
Member

The latest main is green again: https://github.com/spring-projects/spring-integration/actions/runs/16838982796.

Please, consider to rebase your branch respectively.

@ngocnhan-tran1996
spring-projectsGH-10083: Apply NullAbility to sftp module
b92963d 
Related to: spring-projects#10083

Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
@ngocnhan-tran1996
Adjust @nullable annotations
61cce7a 
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
@ngocnhan-tran1996
Convert to dot if path is null
7a9c4a4 
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
artembilan
artembilan approved these changes Aug 8, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

LGTM.
as long as PR build is green, I'll merge it.
Thank you!
Was nice way to finish the week 😄

artembilan
artembilan requested changes Aug 8, 2025
View reviewed changes
Copy link
Member

@artembilan artembilan left a comment

Choose a reason for hiding this comment

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

Looks like SMB module has been affected by your change as well:

 SmbTests > testSmbInboundStreamFlow() FAILED
    java.lang.AssertionError at SmbTests.java:148

@ngocnhan-tran1996
Copy link
Contributor Author

The test is green on my local machine

image

However, I will revert RemoteFileTemplate to avoid affecting other modules.

@artembilan
Copy link
Member

However, I will revert RemoteFileTemplate to avoid affecting other modules.

Well, I think it is probably better to fix those modules as well.
That is a natural evolution of the change we are doing so far.

@ngocnhan-tran1996
Overridden method does not define nullability
a0fb561 
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
artembilan
artembilan requested changes Aug 8, 2025
View reviewed changes
...-sftp/src/main/java/org/springframework/integration/sftp/session/SftpRemoteFileTemplate.java Outdated
}

@SuppressWarnings("unchecked")
@SuppressWarnings({"unchecked", "NullAway"}) // Overridden method does not define nullability
Copy link
Member

Choose a reason for hiding this comment

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

No. Bring back, please, the fix we have just had in super class.
And fix SMB one respectively.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I haven't done anything and the test's green 👀

@ngocnhan-tran1996
Revert SuppressWarnings NullAway
7a84842 
Signed-off-by: Tran Ngoc Nhan <ngocnhan.tran1996@gmail.com>
@artembilan artembilan merged commit a325883 into spring-projects:main Aug 8, 2025
3 checks passed
@ngocnhan-tran1996 ngocnhan-tran1996 deleted the gh-10083-sftp-module branch August 9, 2025 05:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@artembilan artembilan artembilan requested changes

Assignees

No one assigned

Labels

in: sftp type: enhancement

Projects

None yet

Milestone

7.0.0-M2

Development

Successfully merging this pull request may close these issues.

2 participants

@ngocnhan-tran1996 @artembilan