gnodet-bot commented on code in PR #28589:
URL: https://github.com/apache/camel/pull/28589#discussion_r4395656913
##########
components/camel-ftp-common/src/main/java/org/apache/camel/component/file/remote/SftpDefaultMoveExistingFileStrategy.java:
##########
@@ -48,7 +48,11 @@ public boolean moveExistingFile(GenericFileEndpoint<?>
endpoint, GenericFileOper
//any leading separator
String parent =
FileUtil.stripLeadingSeparator(FileUtil.onlyPath(fileName));
String onlyName = FileUtil.stripPath(fileName);
- dummy.getIn().setHeader(FtpConstants.FILE_NAME, fileName);
+ // the file name is the name in the directory of the file, as when the
producer has changed into that
+ // directory (stepwise=true); a relative result is completed with that
directory below. Without stepwise
+ // (and in mina-sftp) the given name has the path from the login
directory, and with it ${file:name} would
Review Comment:
💡 The comment says _"Without stepwise (and in mina-sftp)"_ but omits the
third trigger: `tempFileName` with `stepwise=true`, where `GenericFileProducer`
passes the full target path. The upgrade guide lists all three scenarios
correctly — the code comment should match:
```suggestion
// the file name is the name in the directory of the file, as when
the producer has changed into that
// directory (stepwise=true); a relative result is completed with
that directory below. Without stepwise,
// with a tempFileName, and in mina-sftp, the given name has the
path from the login directory, and with
// it ${file:name} would put that path again below the directory of
the file
```
##########
components/camel-ftp/src/main/java/org/apache/camel/component/file/remote/FtpDefaultMoveExistingFileStrategy.java:
##########
@@ -51,7 +51,11 @@ public boolean moveExistingFile(GenericFileEndpoint<?>
endpoint, GenericFileOper
//any leading separator
String parent =
FileUtil.stripLeadingSeparator(FileUtil.onlyPath(fileName));
String onlyName = FileUtil.stripPath(fileName);
- dummy.getIn().setHeader(FtpConstants.FILE_NAME, fileName);
+ // the file name is the name in the directory of the file, as when the
producer has changed into that
+ // directory (stepwise=true); a relative result is completed with that
directory below. Without stepwise
+ // (and in mina-sftp) the given name has the path from the login
directory, and with it ${file:name} would
Review Comment:
💡 Same comment omission as in `SftpDefaultMoveExistingFileStrategy` —
`tempFileName` (third trigger) is missing. Should mirror the upgrade guide
wording:
```suggestion
// the file name is the name in the directory of the file, as when
the producer has changed into that
// directory (stepwise=true); a relative result is completed with
that directory below. Without stepwise,
// with a tempFileName, and in mina-sftp, the given name has the
path from the login directory, and with
// it ${file:name} would put that path again below the directory of
the file
```
--
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]
