GH-8745: Add RFT.shouldMarkSessionAsDirty() (#8759)
* GH-8745: Add RFT.shouldMarkSessionAsDirty() Fixes https://github.com/spring-projects/spring-integration/issues/8745 Not all errors caught in the `RemoteFileTemplate.execute()` are fatal to mark session as dirty and physically close the target session in the cache * Introduce a `RemoteFileTemplate.shouldMarkSessionAsDirty()` to consult with an exception if it is really a fatal error to close the session in the end. * Override `shouldMarkSessionAsDirty()` in the `RemoteFileTemplate` implementations to check statuses of respective protocol errors **Cherry-pick to `6.1.x` & `6.0.x`** * * Fix tests for pool interaction * * Fix language in Javadocs * Add more `not dirty` statuses to `SftpRemoteFileTemplate` & `SmbRemoteFileTemplate`
This commit is contained in:
committed by
Gary Russell
parent
b456fe63cb
commit
274a3e0028
@@ -20,10 +20,12 @@ import java.io.IOException;
|
||||
|
||||
import org.apache.commons.net.ftp.FTPClient;
|
||||
import org.apache.commons.net.ftp.FTPFile;
|
||||
import org.apache.commons.net.ftp.FTPReply;
|
||||
|
||||
import org.springframework.integration.file.remote.ClientCallback;
|
||||
import org.springframework.integration.file.remote.RemoteFileTemplate;
|
||||
import org.springframework.integration.file.remote.session.SessionFactory;
|
||||
import org.springframework.lang.Nullable;
|
||||
import org.springframework.messaging.MessagingException;
|
||||
import org.springframework.util.Assert;
|
||||
import org.springframework.util.ObjectUtils;
|
||||
@@ -34,6 +36,7 @@ import org.springframework.util.ObjectUtils;
|
||||
*
|
||||
* @author Gary Russell
|
||||
* @author Artem Bilan
|
||||
*
|
||||
* @since 4.1
|
||||
*
|
||||
*/
|
||||
@@ -82,22 +85,11 @@ public class FtpRemoteFileTemplate extends RemoteFileTemplate<FTPFile> {
|
||||
public boolean exists(final String path) {
|
||||
return doExecuteWithClient(client -> {
|
||||
try {
|
||||
switch (FtpRemoteFileTemplate.this.existsMode) {
|
||||
|
||||
case STAT:
|
||||
return client.getStatus(path) != null;
|
||||
|
||||
case NLST:
|
||||
String[] names = client.listNames(path);
|
||||
return !ObjectUtils.isEmpty(names);
|
||||
|
||||
case NLST_AND_DIRS:
|
||||
return FtpRemoteFileTemplate.super.exists(path);
|
||||
|
||||
default:
|
||||
throw new IllegalStateException("Unsupported 'existsMode': " +
|
||||
FtpRemoteFileTemplate.this.existsMode);
|
||||
}
|
||||
return switch (FtpRemoteFileTemplate.this.existsMode) {
|
||||
case STAT -> client.getStatus(path) != null;
|
||||
case NLST -> !ObjectUtils.isEmpty(client.listNames(path));
|
||||
case NLST_AND_DIRS -> FtpRemoteFileTemplate.super.exists(path);
|
||||
};
|
||||
}
|
||||
catch (IOException e) {
|
||||
throw new MessagingException("Failed to check the remote path for " + path, e);
|
||||
@@ -105,6 +97,38 @@ public class FtpRemoteFileTemplate extends RemoteFileTemplate<FTPFile> {
|
||||
});
|
||||
}
|
||||
|
||||
@Override
|
||||
protected boolean shouldMarkSessionAsDirty(Exception ex) {
|
||||
IOException ftpException = findIoException(ex);
|
||||
if (ftpException != null) {
|
||||
return isStatusDirty(ftpException.getMessage());
|
||||
}
|
||||
else {
|
||||
return super.shouldMarkSessionAsDirty(ex);
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if {@link IOException#getMessage()} is treated as fatal.
|
||||
* @param ftpErrorMessage the value from {@link IOException#getMessage()}.
|
||||
* @return true if {@link IOException#getMessage()} is treated as fatal.
|
||||
* @since 6.0.8
|
||||
*/
|
||||
protected boolean isStatusDirty(String ftpErrorMessage) {
|
||||
return !ftpErrorMessage.contains("" + FTPReply.FILE_UNAVAILABLE)
|
||||
&& !ftpErrorMessage.contains("" + FTPReply.FILE_NAME_NOT_ALLOWED);
|
||||
}
|
||||
|
||||
@Nullable
|
||||
private static IOException findIoException(Throwable ex) {
|
||||
if (ex == null || ex instanceof IOException) {
|
||||
return (IOException) ex;
|
||||
}
|
||||
else {
|
||||
return findIoException(ex.getCause());
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* The {@link #exists(String)} operation mode.
|
||||
* @since 4.1.9
|
||||
|
||||
@@ -1,5 +1,5 @@
|
||||
/*
|
||||
* Copyright 2014-2022 the original author or authors.
|
||||
* Copyright 2014-2023 the original author or authors.
|
||||
*
|
||||
* Licensed under the Apache License, Version 2.0 (the "License");
|
||||
* you may not use this file except in compliance with the License.
|
||||
@@ -34,6 +34,7 @@ import org.springframework.expression.common.LiteralExpression;
|
||||
import org.springframework.integration.file.DefaultFileNameGenerator;
|
||||
import org.springframework.integration.file.remote.ClientCallbackWithoutResult;
|
||||
import org.springframework.integration.file.remote.SessionCallbackWithoutResult;
|
||||
import org.springframework.integration.file.remote.session.Session;
|
||||
import org.springframework.integration.file.remote.session.SessionFactory;
|
||||
import org.springframework.integration.file.support.FileExistsMode;
|
||||
import org.springframework.integration.ftp.FtpTestSupport;
|
||||
@@ -53,9 +54,7 @@ import static org.mockito.Mockito.when;
|
||||
/**
|
||||
* @author Gary Russell
|
||||
* @author Artem Bilan
|
||||
*
|
||||
* @since 4.1
|
||||
*
|
||||
*/
|
||||
@SpringJUnitConfig
|
||||
@DirtiesContext
|
||||
@@ -142,6 +141,25 @@ public class FtpRemoteFileTemplateTests extends FtpTestSupport {
|
||||
assertThat(pool.getActiveCount()).isEqualTo(0);
|
||||
}
|
||||
|
||||
@Test
|
||||
public void sessionIsNotDirtyOnNoSuchFileError() {
|
||||
Session<FTPFile> session = this.sessionFactory.getSession();
|
||||
session.close();
|
||||
|
||||
FtpRemoteFileTemplate template = new FtpRemoteFileTemplate(this.sessionFactory);
|
||||
|
||||
assertThatExceptionOfType(MessagingException.class)
|
||||
.isThrownBy(() -> template.rename("No_such_file1", "No_such_file2"))
|
||||
.withRootCauseInstanceOf(IOException.class)
|
||||
.withStackTraceContaining("553 : No such file or directory");
|
||||
|
||||
Session<FTPFile> newSession = this.sessionFactory.getSession();
|
||||
assertThat(TestUtils.getPropertyValue(newSession, "targetSession"))
|
||||
.isSameAs(TestUtils.getPropertyValue(session, "targetSession"));
|
||||
|
||||
newSession.close();
|
||||
}
|
||||
|
||||
@Configuration
|
||||
public static class Config {
|
||||
|
||||
|
||||
Reference in New Issue
Block a user