NIFI-15683 Fix SFTP Move strategy failure when destination file alrea…

[rakesh-rsky]

NIFI-15683 Fix SFTP Move strategy failure when destination file already exists

When FetchSFTP uses completion strategy 'Move', some SFTP servers (notably OpenSSH) return SSH_FX_FAILURE on rename if the destination file already exists, rather than atomically overwriting it. This causes a warning and leaves the source file in place.

Add a delete-then-rename fallback: if the initial rename fails, attempt to delete the destination and retry the rename. If the destination does not exist (FileNotFoundException), the original rename exception is re-thrown to preserve the error signal for unrelated failures.

Summary

NIFI-15683

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• Apache NiFi Jira issue created

Pull Request Tracking

• Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000
• Pull request contains commits signed with a registered key indicating Verified status

Pull Request Formatting

• Pull Request based on current revision of the main branch
• Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• Build completed using ./mvnw clean install -P contrib-check
  • JDK 21
  • JDK 25

Licensing

• New dependencies are compatible with the Apache License 2.0 according to the License Policy
• New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• Documentation formatting appears as expected in rendered files

[exceptionfactory]

Thanks for proposing this change @rakesh-rsky. A similar effort recently stalled, attempting to handle failures related to moving files. There are several problems with potential solutions, including existing expectations, and lack of clear failure paths for post-fetch operations. For these reasons, falling back to deleting a file may not be appropriate. In addition, catching the general IOException is too broad to be a safe approach. This implementation needs further consideration of potential options, before moving forward.

[rakesh-rsky]

Thank you for the review feedback @exceptionfactory.

You're right on both points — the automatic delete-then-rename fallback was too aggressive, and catching the general IOException was too broad and could trigger a remote file deletion for unrelated failures like network errors or permission issues.

I've reworked the approach:

• Added a Move Conflict Resolution property with two values: Fail (default, preserves existing behaviour) and Replace (explicit opt-in).
• When Replace is selected, the processor proactively checks if the destination exists using getRemoteFileInfo() before attempting the rename. If it exists, it deletes it and renames. If not, it simply renames as normal.
• This removes the broad IOException catch entirely — no exception-driven control flow, no silent destructive fallback.
• The default is Fail, so existing deployments are unaffected.

This gives users explicit control over the conflict resolution behaviour while keeping the failure paths clean and predictable.

[exceptionfactory]

Thanks for the work on this @rakesh-rsky. The revised approach looks like a good way forward, the new property and opt-in behavior provide a good path for optional handling.

From an implementation perspective, it would be helpful to define the new allowable values in an enum that implements DescribedValue, following the pattern elsewhere in the project.

Any description strings that go over one line can also be defined as multiline strings, instead of using concatenation.

The new test class looks good, and although there are divergent naming conventions, I recommend that new tests be named with Test as the suffix, so FetchSFTPTest in this case.

If you can address these items, that would help move this forward.