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

* GH-3370: Remove synchronized from RemoteFileUtils

Fixes #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`
# Conflicts:
#	spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileUtils.java
#	spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java
#	spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java
#	spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpOutboundTests.java

Author: artembilan

spring-integration-file/src/main/java/org/springframework/integration/file/remote/RemoteFileUtils.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2016 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.
@@ -27,6 +27,8 @@
 /**
  * Utility methods for supporting remote file operations.
  * @author Gary Russell
+ * @author Artem Bilan
+ *
  * @since 3.0
  *
  */
@@ -45,13 +47,11 @@ private 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<String>();
 				while (nextSeparatorIndex > -1) {
@@ -70,13 +70,19 @@ public static synchronized <F> void makeDirectories(String path, Session<F> sess
 					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);
+		}
+	}
+
 }

spring-integration-file/src/main/java/org/springframework/integration/file/remote/gateway/AbstractRemoteFileOutboundGateway.java

@@ -1,5 +1,5 @@
 /*
- * Copyright 2002-2017 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.
@@ -510,14 +510,13 @@ protected void doInit() {
 		if (Command.MGET.equals(this.command)) {
 			Assert.isTrue(!(this.options.contains(Option.SUBDIRS)),
 					"Cannot use " + Option.SUBDIRS.toString() + " when using 'mget' use "
-							+ Option.RECURSIVE.toString() +	" to obtain files in subdirectories");
+							+ Option.RECURSIVE.toString() + " to obtain files in subdirectories");
 		}
 
 		if (getBeanFactory() != null) {
 			if (this.fileNameProcessor != null) {
 				this.fileNameProcessor.setBeanFactory(getBeanFactory());
 			}
-
 			this.renameProcessor.setBeanFactory(getBeanFactory());
 			this.remoteFileTemplate.setBeanFactory(getBeanFactory());
 		}
@@ -527,20 +526,20 @@ protected void doInit() {
 	protected Object handleRequestMessage(final Message<?> requestMessage) {
 		if (this.command != null) {
 			switch (this.command) {
-			case LS:
-				return doLs(requestMessage);
-			case GET:
-				return doGet(requestMessage);
-			case MGET:
-				return doMget(requestMessage);
-			case RM:
-				return doRm(requestMessage);
-			case MV:
-				return doMv(requestMessage);
-			case PUT:
-				return doPut(requestMessage);
-			case MPUT:
-				return doMput(requestMessage);
+				case LS:
+					return doLs(requestMessage);
+				case GET:
+					return doGet(requestMessage);
+				case MGET:
+					return doMget(requestMessage);
+				case RM:
+					return doRm(requestMessage);
+				case MV:
+					return doMv(requestMessage);
+				case PUT:
+					return doPut(requestMessage);
+				case MPUT:
+					return doMput(requestMessage);
 			}
 		}
 		return this.remoteFileTemplate.execute(new SessionCallback<F, Object>() {
@@ -570,8 +569,8 @@ public List<?> doInSession(Session<F> session) throws IOException {
 			}
 		});
 		return this.getMessageBuilderFactory().withPayload(payload)
-			.setHeader(FileHeaders.REMOTE_DIRECTORY, dir)
-			.build();
+				.setHeader(FileHeaders.REMOTE_DIRECTORY, dir)
+				.build();
 	}
 
 	private Object doGet(final Message<?> requestMessage) {
@@ -621,9 +620,9 @@ public List<File> doInSession(Session<F> session) throws IOException {
 			}
 		});
 		return this.getMessageBuilderFactory().withPayload(payload)
-			.setHeader(FileHeaders.REMOTE_DIRECTORY, remoteDir)
-			.setHeader(FileHeaders.REMOTE_FILE, remoteFilename)
-			.build();
+				.setHeader(FileHeaders.REMOTE_DIRECTORY, remoteDir)
+				.setHeader(FileHeaders.REMOTE_FILE, remoteFilename)
+				.build();
 	}
 
 	private Object doRm(Message<?> requestMessage) {
@@ -634,24 +633,24 @@ private Object doRm(Message<?> requestMessage) {
 		boolean payload = this.remoteFileTemplate.remove(remoteFilePath);
 
 		return this.getMessageBuilderFactory().withPayload(payload)
-			.setHeader(FileHeaders.REMOTE_DIRECTORY, remoteDir)
-			.setHeader(FileHeaders.REMOTE_FILE, remoteFilename)
-			.build();
+				.setHeader(FileHeaders.REMOTE_DIRECTORY, remoteDir)
+				.setHeader(FileHeaders.REMOTE_FILE, remoteFilename)
+				.build();
 	}
 
 	private Object doMv(Message<?> requestMessage) {
-		String remoteFilePath =  this.fileNameProcessor.processMessage(requestMessage);
+		String remoteFilePath = this.fileNameProcessor.processMessage(requestMessage);
 		String remoteFilename = getRemoteFilename(remoteFilePath);
 		String remoteDir = getRemoteDirectory(remoteFilePath, remoteFilename);
 		String remoteFileNewPath = this.renameProcessor.processMessage(requestMessage);
 		Assert.hasLength(remoteFileNewPath, "New filename cannot be empty");
 
 		this.remoteFileTemplate.rename(remoteFilePath, remoteFileNewPath);
 		return this.getMessageBuilderFactory().withPayload(Boolean.TRUE)
-			.setHeader(FileHeaders.REMOTE_DIRECTORY, remoteDir)
-			.setHeader(FileHeaders.REMOTE_FILE, remoteFilename)
-			.setHeader(FileHeaders.RENAME_TO, remoteFileNewPath)
-			.build();
+				.setHeader(FileHeaders.REMOTE_DIRECTORY, remoteDir)
+				.setHeader(FileHeaders.REMOTE_FILE, remoteFilename)
+				.setHeader(FileHeaders.RENAME_TO, remoteFileNewPath)
+				.build();
 	}
 
 	private String doPut(Message<?> requestMessage) {
@@ -722,7 +721,7 @@ private List<String> putLocalDirectory(Message<?> requestMessage, File file, Str
 				else if (this.options.contains(Option.RECURSIVE)) {
 					String newSubDirectory = (StringUtils.hasText(subDirectory) ?
 							subDirectory + this.remoteFileTemplate.getRemoteFileSeparator() : "")
-						+ filteredFile.getName();
+							+ filteredFile.getName();
 					replies.addAll(this.putLocalDirectory(requestMessage, filteredFile, newSubDirectory));
 				}
 			}
@@ -795,7 +794,7 @@ private List<F> listFilesInRemoteDir(Session<F> session, String directory, Strin
 						}
 					}
 					if (recursion && this.isDirectory(file) && !(".".equals(fileName)) && !("..".equals(fileName))) {
-						lsFiles.addAll(listFilesInRemoteDir(session, directory,  subDirectory + fileName
+						lsFiles.addAll(listFilesInRemoteDir(session, directory, subDirectory + fileName
 								+ this.remoteFileTemplate.getRemoteFileSeparator()));
 					}
 				}
@@ -858,7 +857,7 @@ protected void purgeDots(List<F> lsFiles) {
 	 * @throws IOException Any IOException.
 	 */
 	protected File get(Message<?> message, Session<F> session, String remoteDir, String remoteFilePath,
-	                   String remoteFilename, boolean lsFirst) throws IOException {
+			String remoteFilename, boolean lsFirst) throws IOException {
 		F[] files = null;
 		if (lsFirst) {
 			files = session.list(remoteFilePath);
@@ -931,7 +930,7 @@ else if (FileExistsMode.IGNORE != fileExistsMode) {
 	}
 
 	protected List<File> mGet(Message<?> message, Session<F> session, String remoteDirectory,
-							  String remoteFilename) throws IOException {
+			String remoteFilename) throws IOException {
 		if (this.options.contains(Option.RECURSIVE)) {
 			if (logger.isWarnEnabled() && !("*".equals(remoteFilename))) {
 				logger.warn("File name pattern must be '*' when using recursion");
@@ -1018,7 +1017,7 @@ private List<File> mGetWithRecursion(Message<?> message, Session<F> session, Str
 				String fileName = this.getRemoteFilename(fullFileName);
 				String actualRemoteDirectory = this.getRemoteDirectory(fullFileName, fileName);
 				File file = get(message, session, actualRemoteDirectory,
-							fullFileName, fileName, false);
+						fullFileName, fileName, false);
 				if (this.options.contains(Option.PRESERVE_TIMESTAMP)) {
 					file.setLastModified(getModified(lsEntry.getFileInfo()));
 				}

spring-integration-file/src/test/java/org/springframework/integration/file/remote/gateway/RemoteFileOutboundGatewayTests.java

@@ -323,7 +323,7 @@ public Object answer(InvocationOnMock invocation) throws Throwable {
 			@Override
 			public Object answer(InvocationOnMock invocation) throws Throwable {
 				madeDirs.add((String) invocation.getArguments()[0]);
-				return null;
+				return true;
 			}
 		}).when(session).mkdir(anyString());
 		when(sessionFactory.getSession()).thenReturn(session);

spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java

@@ -229,8 +229,10 @@ public boolean mkdir(String remoteDirectory) throws IOException {
 		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;
 	}

spring-integration-sftp/src/test/java/org/springframework/integration/sftp/outbound/SftpOutboundTests.java

@@ -216,7 +216,7 @@ public void testMkDir() throws Exception {
 			@Override
 			public Object answer(InvocationOnMock invocation) throws Throwable {
 				madeDirs.add((String) invocation.getArguments()[0]);
-				return null;
+				return true;
 			}
 		}).when(session).mkdir(anyString());
 		handler.handleMessage(new GenericMessage<String>("qux"));