search

spring-projects / spring-integration-extensions

The Spring Integration Extensions project provides extension components for Spring Integration
http://www.springintegration.org/
280 stars 266 forks source link

Deprecate fileReplace option from SmbSessionFactory and remove it in future #246

Closed abdurrehmansyeds closed 2 years ago

abdurrehmansyeds commented 2 years ago

Currently the file replace option is provided via SmbSessionFactory, I feel that the session object should not have the logic to replace files it should be limited to just connection. FileTransferringMessageHandler has FileExistsMode.REPLACE as default file exists mode but still it fails to replace. Can we please have the file replace control in FileTransferringMessageHandler and deprecate the one from SmbSessionFactory?

The same has been discussed here in SO thread.

artembilan commented 2 years ago

This is good candidate for end-user contribution. Feel free to raise a PR and we will review it ASAP: https://github.com/spring-projects/spring-integration/blob/main/CONTRIBUTING.adoc

Thank you!

GregBragg commented 2 years ago

This will probably require a refactoring of the code to create an outbound channel adapter specific to SMB to move this existing functionality out of SmbSessionFactory.

artembilan commented 2 years ago

@GregBragg ,

That's exactly the point that FileTransferringMessageHandler already has that functionality with delegation to respective RemoteFileTemplate implementation.

GregBragg commented 2 years ago

Yes exactly what I was thinking... I'm looking into this now.

GregBragg commented 2 years ago

We can't fully deprecate the replaceFile option as it will break current implementations for 1.2.x versions, however I've created an outbound channel adapter, SmbMessageHandler to mitigate this issue.

To fully remove replaceFile will require refactoring a lot of code that from my perspective is not worth the effort. Using an existing configuration of replaceFile=true when creating SmBSessionFactory for outbound will still be supported.

GregBragg commented 2 years ago

We can't fully deprecate the replaceFile option as it will break current implementations for 1.2.x versions, however I've created an outbound channel adapter, SmbMessageHandler to mitigate this issue.

To fully remove replaceFile will require refactoring a lot of code that from my perspective is not worth the effort. Using an existing configuration of replaceFile=true when creating SmBSessionFactory for outbound will still be supported.

I retract my previous statement... to fix this properly we need to rip out the legacy implementations of values replaceFile and useTempFile flags in SmbConfig, the base classes of Spring Integration already handle this functionality gracefully.

As a result we need to build a new major version to clean up this code.

artembilan commented 2 years ago

@abdurrehmansyeds , @GregBragg , spring-integration-smb-1.2.2.RELEASE is on its way to Maven Central. However it is already available in Spring Repo: https://repo.spring.io/ui/native/libs-release/org/springframework/integration/spring-integration-smb/1.2.2.RELEASE/