GH-3247: Fix SftpSession.exists for error code (#3248)

* GH-3247: Fix `SftpSession.exists` for error code

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

When there is no path on the SFTP server, a `ChannelSftp.SSH_FX_NO_SUCH_FILE`
error is returned in the thrown `SftpException`.

* Fix `SftpSession.exists()` to check for the `SSH_FX_NO_SUCH_FILE` to
return `false` and re-throw an exception otherwise
* Add mock test for `SftpSession.exists()`
* Add `org.mockito.AdditionalMatchers` to `checkstyle.xml` exclusions

**Cherry-pick to 5.2.x & 5.1.x**

* * Add exists tests against Mina embedded server
This commit is contained in:
Artem Bilan
2020-04-14 15:26:14 -04:00
committed by GitHub
parent 97702ae712
commit b75f1bd0da
4 changed files with 85 additions and 21 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.
@@ -127,16 +127,15 @@ public class SftpSession implements Session<LsEntry> {
@Override
public String[] listNames(String path) throws IOException {
LsEntry[] entries = this.list(path);
List<String> names = new ArrayList<String>();
for (int i = 0; i < entries.length; i++) {
String fileName = entries[i].getFilename();
SftpATTRS attrs = entries[i].getAttrs();
List<String> names = new ArrayList<>();
for (LsEntry entry : entries) {
String fileName = entry.getFilename();
SftpATTRS attrs = entry.getAttrs();
if (!attrs.isDir() && !attrs.isLink()) {
names.add(fileName);
}
}
String[] fileNames = new String[names.size()];
return names.toArray(fileNames);
return names.toArray(new String[0]);
}
@@ -270,15 +269,19 @@ public class SftpSession implements Session<LsEntry> {
}
@Override
public boolean exists(String path) {
public boolean exists(String path) throws IOException {
try {
this.channel.lstat(path);
return true;
}
catch (@SuppressWarnings("unused") SftpException e) {
// ignore
catch (SftpException ex) {
if (ex.id == ChannelSftp.SSH_FX_NO_SUCH_FILE) {
return false;
}
else {
throw new NestedIOException("Cannot check 'lstat' for path " + path, ex);
}
}
return false;
}
void connect() {

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.
@@ -17,8 +17,14 @@
package org.springframework.integration.sftp.outbound;
import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.mockito.AdditionalMatchers.and;
import static org.mockito.AdditionalMatchers.not;
import static org.mockito.ArgumentMatchers.anyString;
import static org.mockito.ArgumentMatchers.eq;
import static org.mockito.BDDMockito.willAnswer;
import static org.mockito.BDDMockito.willReturn;
import static org.mockito.BDDMockito.willThrow;
import static org.mockito.Mockito.doAnswer;
import static org.mockito.Mockito.doNothing;
import static org.mockito.Mockito.doReturn;
@@ -30,6 +36,7 @@ import static org.mockito.Mockito.when;
import java.io.File;
import java.io.FileOutputStream;
import java.io.IOException;
import java.io.InputStream;
import java.lang.reflect.Constructor;
import java.util.ArrayList;
@@ -38,12 +45,13 @@ import java.util.List;
import java.util.Vector;
import java.util.concurrent.atomic.AtomicInteger;
import org.junit.Test;
import org.junit.jupiter.api.Test;
import org.mockito.Mockito;
import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.BeanFactory;
import org.springframework.context.support.ClassPathXmlApplicationContext;
import org.springframework.core.NestedIOException;
import org.springframework.expression.common.LiteralExpression;
import org.springframework.integration.file.DefaultFileNameGenerator;
import org.springframework.integration.file.remote.FileInfo;
@@ -67,6 +75,7 @@ import com.jcraft.jsch.ChannelSftp.LsEntry;
import com.jcraft.jsch.JSch;
import com.jcraft.jsch.JSchException;
import com.jcraft.jsch.SftpATTRS;
import com.jcraft.jsch.SftpException;
/**
* @author Oleg Zhurakousky
@@ -76,7 +85,7 @@ import com.jcraft.jsch.SftpATTRS;
*/
public class SftpOutboundTests {
private static com.jcraft.jsch.Session jschSession = mock(com.jcraft.jsch.Session.class);
private static final com.jcraft.jsch.Session jschSession = mock(com.jcraft.jsch.Session.class);
@Test
public void testHandleFileMessage() throws Exception {
@@ -84,7 +93,7 @@ public class SftpOutboundTests {
assertThat(targetDir.exists()).as("target directory does not exist: " + targetDir.getName()).isTrue();
SessionFactory<LsEntry> sessionFactory = new TestSftpSessionFactory();
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
handler.setRemoteDirectoryExpression(new LiteralExpression(targetDir.getName()));
DefaultFileNameGenerator fGenerator = new DefaultFileNameGenerator();
fGenerator.setBeanFactory(mock(BeanFactory.class));
@@ -133,7 +142,7 @@ public class SftpOutboundTests {
file.delete();
}
SessionFactory<LsEntry> sessionFactory = new TestSftpSessionFactory();
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
DefaultFileNameGenerator fGenerator = new DefaultFileNameGenerator();
fGenerator.setBeanFactory(mock(BeanFactory.class));
fGenerator.setExpression("'foo.txt'");
@@ -142,7 +151,7 @@ public class SftpOutboundTests {
handler.setBeanFactory(mock(BeanFactory.class));
handler.afterPropertiesSet();
handler.handleMessage(new GenericMessage<byte[]>("byte[] data".getBytes()));
handler.handleMessage(new GenericMessage<>("byte[] data".getBytes()));
assertThat(new File("remote-target-dir", "foo.txt").exists()).isTrue();
byte[] inFile = FileCopyUtils.copyToByteArray(file);
assertThat(new String(inFile)).isEqualTo("byte[] data");
@@ -171,7 +180,7 @@ public class SftpOutboundTests {
}
@Test //INT-2275
public void testFtpOutboundGatewayInsideChain() throws Exception {
public void testFtpOutboundGatewayInsideChain() {
ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext(
"SftpOutboundInsideChainTests-context.xml", getClass());
@@ -202,17 +211,17 @@ public class SftpOutboundTests {
@SuppressWarnings("unchecked")
SessionFactory<LsEntry> sessionFactory = mock(SessionFactory.class);
when(sessionFactory.getSession()).thenReturn(session);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
handler.setAutoCreateDirectory(true);
handler.setRemoteDirectoryExpression(new LiteralExpression("/foo/bar/baz"));
handler.setBeanFactory(mock(BeanFactory.class));
handler.afterPropertiesSet();
final List<String> madeDirs = new ArrayList<String>();
final List<String> madeDirs = new ArrayList<>();
doAnswer(invocation -> {
madeDirs.add(invocation.getArgument(0));
return null;
}).when(session).mkdir(anyString());
handler.handleMessage(new GenericMessage<String>("qux"));
handler.handleMessage(new GenericMessage<>("qux"));
assertThat(madeDirs.size()).isEqualTo(3);
assertThat(madeDirs.get(0)).isEqualTo("/foo");
assertThat(madeDirs.get(1)).isEqualTo("/foo/bar");
@@ -380,6 +389,34 @@ public class SftpOutboundTests {
verify(jschSession2).disconnect();
}
@Test
public void testExists() throws SftpException, IOException {
ChannelSftp channelSftp = mock(ChannelSftp.class);
willReturn(mock(SftpATTRS.class))
.given(channelSftp)
.lstat(eq("exist"));
willThrow(new SftpException(ChannelSftp.SSH_FX_NO_SUCH_FILE, "Path does not exist."))
.given(channelSftp)
.lstat(eq("notExist"));
willThrow(new SftpException(ChannelSftp.SSH_FX_CONNECTION_LOST, "Connection lost."))
.given(channelSftp)
.lstat(and(not(eq("exist")), not(eq("notExist"))));
SftpSession sftpSession = new SftpSession(mock(com.jcraft.jsch.Session.class));
DirectFieldAccessor fieldAccessor = new DirectFieldAccessor(sftpSession);
fieldAccessor.setPropertyValue("channel", channelSftp);
assertThat(sftpSession.exists("exist")).isTrue();
assertThat(sftpSession.exists("notExist")).isFalse();
assertThatExceptionOfType(NestedIOException.class).
isThrownBy(() -> sftpSession.exists("foo"));
}
private void noopConnect(ChannelSftp channel1) throws JSchException {
doNothing().when(channel1).connect(5000);
}

View File

@@ -44,6 +44,7 @@ import org.springframework.beans.DirectFieldAccessor;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.context.ApplicationContext;
import org.springframework.context.annotation.Bean;
import org.springframework.core.NestedIOException;
import org.springframework.integration.channel.DirectChannel;
import org.springframework.integration.event.inbound.ApplicationEventListeningMessageProducer;
import org.springframework.integration.file.FileHeaders;
@@ -59,6 +60,7 @@ import org.springframework.integration.sftp.server.PathMovedEvent;
import org.springframework.integration.sftp.server.PathRemovedEvent;
import org.springframework.integration.sftp.server.SessionClosedEvent;
import org.springframework.integration.sftp.server.SessionOpenedEvent;
import org.springframework.integration.sftp.session.DefaultSftpSessionFactory;
import org.springframework.integration.sftp.session.SftpRemoteFileTemplate;
import org.springframework.integration.support.MessageBuilder;
import org.springframework.integration.test.util.TestUtils;
@@ -561,6 +563,27 @@ public class SftpServerOutboundTests extends SftpTestSupport {
assertThat(files[0].getAttrs().getSize()).isEqualTo(6);
}
@Test
public void testSessionExists() throws IOException {
DefaultSftpSessionFactory sessionFactory = new DefaultSftpSessionFactory();
sessionFactory.setHost("localhost");
sessionFactory.setPort(port);
sessionFactory.setUser("foo");
sessionFactory.setPassword("foo");
sessionFactory.setAllowUnknownKeys(true);
Session<LsEntry> session = sessionFactory.getSession();
assertThat(session.exists("sftpSource")).isTrue();
assertThat(session.exists("notExist")).isFalse();
session.close();
assertThatExceptionOfType(NestedIOException.class)
.isThrownBy(() -> session.exists("any"))
.withRootCauseInstanceOf(IOException.class)
.withStackTraceContaining("Pipe closed");
}
@SuppressWarnings("unused")
private static final class TestMessageSessionCallback
implements MessageSessionCallback<LsEntry, Object> {

View File

@@ -84,6 +84,7 @@
org.mockito.BDDMockito.*,
org.mockito.AdditionalAnswers.*,
org.mockito.ArgumentMatchers.*,
org.mockito.AdditionalMatchers.*,
org.springframework.integration.gemfire.config.xml.ParserTestUtil.*,
org.springframework.integration.test.mock.MockIntegration.*,
org.springframework.integration.test.util.TestUtils.*,