30th-dguv commented on code in PR #20100:
URL: https://github.com/apache/camel/pull/20100#discussion_r2577831805
##########
components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbOperations.java:
##########
@@ -192,26 +193,56 @@ public boolean existsFile(String name) throws
GenericFileOperationFailedExceptio
@Override
public boolean renameFile(String from, String to) throws
GenericFileOperationFailedException {
+ boolean renamed;
connectIfNecessary();
try (File src
= share.openFile(from, EnumSet.of(AccessMask.GENERIC_READ,
AccessMask.DELETE), null,
SMB2ShareAccess.ALL, SMB2CreateDisposition.FILE_OPEN,
null)) {
-
- try (File dst
- = share.openFile(to, EnumSet.of(AccessMask.GENERIC_WRITE),
EnumSet.of(FileAttributes.FILE_ATTRIBUTE_NORMAL),
- SMB2ShareAccess.ALL,
SMB2CreateDisposition.FILE_CREATE,
-
EnumSet.of(SMB2CreateOptions.FILE_DIRECTORY_FILE))) {
-
- src.remoteCopyTo(dst);
- } catch (Exception e) {
- throw new GenericFileOperationFailedException(e.getMessage(),
e);
+ if (configuration.isRenameUsingCopy()) {
+ renamed = copyAndDeleteRenameStrategy(src, to);
+ } else {
+ renamed = atomicRenameFile(src, to);
+ if (!renamed && configuration.isCopyAndDeleteOnRenameFail()) {
+ // we could not rename using atomic rename strategy, so
lets fallback and do a copy/delete approach.
+ LOG.debug("Cannot rename file from: {} to: {}, will now
use a copy/delete approach instead",
+ src.getUncPath(), to);
+ renamed = copyAndDeleteRenameStrategy(src, to);
+ }
}
-
- src.deleteOnClose();
} catch (SMBRuntimeException smbre) {
safeDisconnect(smbre);
throw smbre;
}
+ return renamed;
+ }
+
+ public boolean atomicRenameFile(File src, String to)
+ throws GenericFileOperationFailedException {
+ boolean renamed = false;
+ try {
+ src.rename(to);
+ renamed = true;
+ } catch (SMBRuntimeException smbre) {
+ renamed = false;
Review Comment:
We are swallowing an exception here. If `isCopyAndDeleteOnRenameFail`
fallback is enabled we must log a WARNING here. If the fallback is disabled, we
must throw `GenericFileOperationFailedException` here providing this exception
as a cause.
##########
components/camel-smb/src/main/java/org/apache/camel/component/smb/SmbConfiguration.java:
##########
@@ -89,6 +89,17 @@ public class SmbConfiguration extends
GenericFileConfiguration {
+ " configurations, like timeouts")
private SmbConfig smbConfig;
+ @UriParam(label = "producer,advanced",
+ description = "Perform rename operations using a copy and delete
strategy. This option takes "
+ + "precedence over the copyAndDeleteOnRenameFail
parameter that will automatically fall back to the copy and delete "
+ + "strategy, but only after additional delays.",
+ defaultValue = "true")
+ private boolean renameUsingCopy = true;
Review Comment:
Why it is `true` by default? Atomic rename of a file is a natural and
durable solution. Rename using copy must be the a fallback by default, note the
first choice.
--
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]
