GH-3370: Remove synchronized from RemoteFileUtils (#3380)

* GH-3370: Remove synchronized from RemoteFileUtils

Fixes https://github.com/spring-projects/spring-integration/issues/3370

The `synchronized` on the `RemoteFileUtils.makeDirectories()` makes an application too
 slow, especially when we deal with different paths in different sessions

* Remove the `synchronized` from that method and rework `SftpSession.mkdir()`
to return `false` when "A file cannot be created if it already exists" exception
is thrown from the server.
Essentially make an `exists()` call to be sure that an exception is really related
to "file-already-exists" answer from the server

**Cherry-pick to 5.3.x, 5.2.x & 4.3.x**

* * Re-throw an exception in the `SftpSession.mkdir()`
when error code is not `4` or remote dir does not exist

* * Check `session.mkdir()` result in the
`RemoteFileUtils` to throw an `IOException` when `false`

* * Fix mock test to return `true` for `mkdir` instead of `null`
This commit is contained in:
Artem Bilan
2020-09-10 13:46:57 -04:00
committed by GitHub
parent 6c2a4c97c5
commit 47cae4670f
5 changed files with 23 additions and 12 deletions

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -28,6 +28,7 @@ import org.springframework.integration.file.remote.session.Session;
* Utility methods for supporting remote file operations.
*
* @author Gary Russell
* @author Artem Bilan
*
* @since 3.0
*
@@ -46,13 +47,11 @@ public final class RemoteFileUtils {
* @param logger The logger.
* @throws IOException Any IOException.
*/
public static synchronized <F> void makeDirectories(String path, Session<F> session, String remoteFileSeparator,
public static <F> void makeDirectories(String path, Session<F> session, String remoteFileSeparator,
Log logger) throws IOException {
if (!session.exists(path)) {
int nextSeparatorIndex = path.lastIndexOf(remoteFileSeparator);
if (nextSeparatorIndex > -1) {
List<String> pathsToCreate = new LinkedList<>();
while (nextSeparatorIndex > -1) {
@@ -71,13 +70,19 @@ public final class RemoteFileUtils {
if (logger.isDebugEnabled()) {
logger.debug("Creating '" + pathToCreate + "'");
}
session.mkdir(pathToCreate);
tryCreateRemoteDirectory(session, pathToCreate);
}
}
else {
session.mkdir(path);
tryCreateRemoteDirectory(session, path);
}
}
}
private static void tryCreateRemoteDirectory(Session<?> session, String path) throws IOException {
if (!session.mkdir(path)) {
throw new IOException("Could not create a remote directory: " + path);
}
}
}

View File

@@ -1,5 +1,5 @@
/*
* Copyright 2002-2019 the original author or authors.
* Copyright 2002-2020 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.
@@ -121,6 +121,7 @@ public abstract class AbstractRemoteFileOutboundGateway<F> extends AbstractReply
MessageSessionCallback<F, ?> messageSessionCallback) {
this(new RemoteFileTemplate<>(sessionFactory), messageSessionCallback);
remoteFileTemplateExplicitlySet(false);
}
/**
@@ -165,6 +166,7 @@ public abstract class AbstractRemoteFileOutboundGateway<F> extends AbstractReply
@Nullable String expression) {
this(new RemoteFileTemplate<>(sessionFactory), command, expression);
remoteFileTemplateExplicitlySet(false);
}
/**
@@ -516,6 +518,9 @@ public abstract class AbstractRemoteFileOutboundGateway<F> extends AbstractReply
}
populateBeanFactoryIntoComponentsIfAny();
if (!this.remoteFileTemplateExplicitlySet) {
this.remoteFileTemplate.afterPropertiesSet();
}
}
private void populateBeanFactoryIntoComponentsIfAny() {

View File

@@ -295,7 +295,7 @@ public class RemoteFileOutboundGatewayTests {
final List<String> madeDirs = new ArrayList<>();
doAnswer(invocation -> {
madeDirs.add(invocation.getArgument(0));
return null;
return true;
}).when(session).mkdir(anyString());
when(sessionFactory.getSession()).thenReturn(session);
Message<String> requestMessage = MessageBuilder.withPayload("foo")
@@ -939,7 +939,6 @@ public class RemoteFileOutboundGatewayTests {
Session<TestLsEntry> session = mock(Session.class);
TestRemoteFileOutboundGateway gw = new TestRemoteFileOutboundGateway(sessionFactory, "mput", "payload");
gw.setRemoteDirectoryExpression(new LiteralExpression("foo/"));
gw.setBeanFactory(mock(BeanFactory.class));
gw.afterPropertiesSet();
when(sessionFactory.getSession()).thenReturn(session);
final AtomicReference<String> written = new AtomicReference<>();

View File

@@ -252,8 +252,10 @@ public class SftpSession implements Session<LsEntry> {
try {
this.channel.mkdir(remoteDirectory);
}
catch (SftpException e) {
throw new NestedIOException("failed to create remote directory '" + remoteDirectory + "'.", e);
catch (SftpException ex) {
if (ex.id != ChannelSftp.SSH_FX_FAILURE || !exists(remoteDirectory)) {
throw new NestedIOException("failed to create remote directory '" + remoteDirectory + "'.", ex);
}
}
return true;
}

View File

@@ -219,7 +219,7 @@ public class SftpOutboundTests {
final List<String> madeDirs = new ArrayList<>();
doAnswer(invocation -> {
madeDirs.add(invocation.getArgument(0));
return null;
return true;
}).when(session).mkdir(anyString());
handler.handleMessage(new GenericMessage<>("qux"));
assertThat(madeDirs.size()).isEqualTo(3);