Skip to content

Support fileExistModeExpression() in Ftp.outboundGateway #9988

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

Closed
anthologia opened this issue Apr 23, 2025 · 7 comments · Fixed by #10019
Closed

Support fileExistModeExpression() in Ftp.outboundGateway #9988

anthologia opened this issue Apr 23, 2025 · 7 comments · Fixed by #10019

Comments

@anthologia
Copy link
Contributor

Expected Behavior
The Ftp.outboundGateway should support a fileExistModeExpression() method that accepts a SpEL expression to dynamically determine the FileExistsMode at runtime. This would allow users to conditionally choose between REPLACE, APPEND, FAIL, or other file exist modes based on message content or context variables.

Current Behavior
Currently, the Ftp.outboundGateway only supports fileExistMode() which accepts a static FileExistsMode value. This mode is fixed at configuration time and cannot be changed dynamically during message processing.

Context
I'd be willing to implement this feature if the team agrees it would be valuable to the Spring Integration community.

@artembilan
Copy link
Member

Sounds reasonable.
Feel free to contribute!
Thanks

@artembilan artembilan added in: sftp in: ftp in: smb and removed status: waiting-for-triage The issue need to be evaluated and its future decided labels Apr 23, 2025
@artembilan artembilan added this to the Backlog milestone Apr 23, 2025
@anthologia
Copy link
Contributor Author

While implementing this feature, I noticed a logic inconsistency between RemoteFileTemplate and AbstractRemoteFileOutboundGateway.

RemoteFileTemplate.send() performs this validation:

private String send(Message<?> message, String subDirectory, FileExistsMode mode) {
	Assert.notNull(this.directoryExpressionProcessor, "'remoteDirectoryExpression' is required");
	Assert.isTrue(!FileExistsMode.APPEND.equals(mode) || !this.useTemporaryFileName,
			"Cannot append when using a temporary file name");

However, AbstractRemoteFileOutboundGateway.setFileExistsMode() automatically changes settings:

public void setFileExistsMode(FileExistsMode fileExistsMode) {
	this.fileExistsMode = fileExistsMode;
	if (FileExistsMode.APPEND.equals(fileExistsMode)) {
		this.remoteFileTemplate.setUseTemporaryFileName(false);
	}
}

This seems problematic as it creates a structure where proper assertion validation becomes impossible. I think it would be more appropriate to remove this automatic configuration from setFileExistsMode() and instead clearly document the relationship between these two settings in the API documentation.

However, I'm not sure about the best approach given backward compatibility concerns. What do you think?

@artembilan
Copy link
Member

where proper assertion validation becomes impossible

I'm not sure what mean.
The RemoteFileTemplate cannot make an assumption about it API usage.
However, the AbstractRemoteFileOutboundGateway is a user of that RemoteFileTemplate and really can make some.
And we have it with that explicit setUseTemporaryFileName(false). when we provide setFileExistsMode.

I might be OK to remove that assumption, but that indeed would be a breaking change for the next 7.0.
We cannot apply it for the current last 6.5 GA milestone.

@anthologia
Copy link
Contributor Author

anthologia commented May 9, 2025

"where proper assertion validation becomes impossible"

I meant that the assertion in RemoteFileTemplate always passes because setFileExistsMode() automatically sets setUseTemporaryFileName(false).

The reason I'm asking is that I've implemented setFileExistsModeExpression() to resolve the issue. This method needs to parse the existsMode value at runtime for each request. However, if I follow the current pattern of setting setUseTemporaryFileName(false) as done in setFileExistsMode(), it might cause concurrency issues when handling multiple requests with different modes simultaneously.

I thought it might be worth considering removing the automatic setUseTemporaryFileName(false) from setFileExistsMode() to ensure consistent behavior between setFileExistsMode() and setFileExistsModeExpression(). This would also allow RemoteFileTemplate's assertion check to work meaningfully.

If this change is too significant for the current release cycle, are there better alternatives to maintain consistency while adding this feature? I'm looking for ways to safely handle dynamically changing modes at runtime while maintaining API consistency..

I'll go ahead and create a PR with my current implementation so you can review the approach and we can discuss further options!

@artembilan
Copy link
Member

Right, I see your point now.
While the static setFileExistsMode() is done only once on the configuration phase, so there is no harm to reset the setUseTemporaryFileName as well, the runtime one resolved by the setFileExistsModeExpression is more problematic.
We really cannot mutate the configuration state of the RemoteFileTemplate because of thread safety.

I wonder if something like totally ignore useTemporaryFileName option when FileExistsMode.APPEND would be much robust solution.
This way, even at runtime resolution, we would be safe because we would not look into that option in case of FileExistsMode.APPEND.

@artembilan
Copy link
Member

@anthologia ,

Let me know what do you think about my last suggestion!
The point is that we are having release next week, so I'd like to have this in.
Thanks

@anthologia
Copy link
Contributor Author

@artembilan,

I agree that ignoring useTemporaryFileName when in APPEND mode is a robust solution for thread safety! I didn't know you had a release schedule in mind! I'll submit the PR today. Thank you.

anthologia added a commit to anthologia/spring-integration that referenced this issue May 13, 2025
Fixes: spring-projects#9988
Issue link: spring-projects#9988

This change allows dynamic determination of FileExistsMode using SpEL expressions,
making the component more flexible when handling file existence conflicts.

* Add fileExistsModeExpression field and setter methods
* Use resolveFileExistsMode in put and get operations
* Add changes to the docs

Signed-off-by: Jooyoung Pyoung <[email protected]>
anthologia added a commit to anthologia/spring-integration that referenced this issue May 13, 2025
Fixes: spring-projects#9988
Issue link: spring-projects#9988

This change allows dynamic determination of FileExistsMode using SpEL expressions,
making the component more flexible when handling file existence conflicts.

* Add fileExistsModeExpression field and setter methods
* Use resolveFileExistsMode in put and get operations
* Add changes to the docs

Signed-off-by: Jooyoung Pyoung <[email protected]>
@artembilan artembilan modified the milestones: Backlog, 6.5.0 May 15, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants