Skip to content

Commit b75f1bd

Browse files
authored
spring-projectsGH-3247: Fix SftpSession.exists for error code (spring-projects#3248)
* spring-projectsGH-3247: Fix `SftpSession.exists` for error code Fixes spring-projects#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
1 parent 97702ae commit b75f1bd

File tree

4 files changed

+85
-21
lines changed

4 files changed

+85
-21
lines changed

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

+14-11
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -127,16 +127,15 @@ public LsEntry[] list(String path) throws IOException {
127127
@Override
128128
public String[] listNames(String path) throws IOException {
129129
LsEntry[] entries = this.list(path);
130-
List<String> names = new ArrayList<String>();
131-
for (int i = 0; i < entries.length; i++) {
132-
String fileName = entries[i].getFilename();
133-
SftpATTRS attrs = entries[i].getAttrs();
130+
List<String> names = new ArrayList<>();
131+
for (LsEntry entry : entries) {
132+
String fileName = entry.getFilename();
133+
SftpATTRS attrs = entry.getAttrs();
134134
if (!attrs.isDir() && !attrs.isLink()) {
135135
names.add(fileName);
136136
}
137137
}
138-
String[] fileNames = new String[names.size()];
139-
return names.toArray(fileNames);
138+
return names.toArray(new String[0]);
140139
}
141140

142141

@@ -270,15 +269,19 @@ public boolean rmdir(String remoteDirectory) throws IOException {
270269
}
271270

272271
@Override
273-
public boolean exists(String path) {
272+
public boolean exists(String path) throws IOException {
274273
try {
275274
this.channel.lstat(path);
276275
return true;
277276
}
278-
catch (@SuppressWarnings("unused") SftpException e) {
279-
// ignore
277+
catch (SftpException ex) {
278+
if (ex.id == ChannelSftp.SSH_FX_NO_SUCH_FILE) {
279+
return false;
280+
}
281+
else {
282+
throw new NestedIOException("Cannot check 'lstat' for path " + path, ex);
283+
}
280284
}
281-
return false;
282285
}
283286

284287
void connect() {

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

+47-10
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2019 the original author or authors.
2+
* Copyright 2002-2020 the original author or authors.
33
*
44
* Licensed under the Apache License, Version 2.0 (the "License");
55
* you may not use this file except in compliance with the License.
@@ -17,8 +17,14 @@
1717
package org.springframework.integration.sftp.outbound;
1818

1919
import static org.assertj.core.api.Assertions.assertThat;
20+
import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
21+
import static org.mockito.AdditionalMatchers.and;
22+
import static org.mockito.AdditionalMatchers.not;
2023
import static org.mockito.ArgumentMatchers.anyString;
24+
import static org.mockito.ArgumentMatchers.eq;
2125
import static org.mockito.BDDMockito.willAnswer;
26+
import static org.mockito.BDDMockito.willReturn;
27+
import static org.mockito.BDDMockito.willThrow;
2228
import static org.mockito.Mockito.doAnswer;
2329
import static org.mockito.Mockito.doNothing;
2430
import static org.mockito.Mockito.doReturn;
@@ -30,6 +36,7 @@
3036

3137
import java.io.File;
3238
import java.io.FileOutputStream;
39+
import java.io.IOException;
3340
import java.io.InputStream;
3441
import java.lang.reflect.Constructor;
3542
import java.util.ArrayList;
@@ -38,12 +45,13 @@
3845
import java.util.Vector;
3946
import java.util.concurrent.atomic.AtomicInteger;
4047

41-
import org.junit.Test;
48+
import org.junit.jupiter.api.Test;
4249
import org.mockito.Mockito;
4350

4451
import org.springframework.beans.DirectFieldAccessor;
4552
import org.springframework.beans.factory.BeanFactory;
4653
import org.springframework.context.support.ClassPathXmlApplicationContext;
54+
import org.springframework.core.NestedIOException;
4755
import org.springframework.expression.common.LiteralExpression;
4856
import org.springframework.integration.file.DefaultFileNameGenerator;
4957
import org.springframework.integration.file.remote.FileInfo;
@@ -67,6 +75,7 @@
6775
import com.jcraft.jsch.JSch;
6876
import com.jcraft.jsch.JSchException;
6977
import com.jcraft.jsch.SftpATTRS;
78+
import com.jcraft.jsch.SftpException;
7079

7180
/**
7281
* @author Oleg Zhurakousky
@@ -76,15 +85,15 @@
7685
*/
7786
public class SftpOutboundTests {
7887

79-
private static com.jcraft.jsch.Session jschSession = mock(com.jcraft.jsch.Session.class);
88+
private static final com.jcraft.jsch.Session jschSession = mock(com.jcraft.jsch.Session.class);
8089

8190
@Test
8291
public void testHandleFileMessage() throws Exception {
8392
File targetDir = new File("remote-target-dir");
8493
assertThat(targetDir.exists()).as("target directory does not exist: " + targetDir.getName()).isTrue();
8594

8695
SessionFactory<LsEntry> sessionFactory = new TestSftpSessionFactory();
87-
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
96+
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
8897
handler.setRemoteDirectoryExpression(new LiteralExpression(targetDir.getName()));
8998
DefaultFileNameGenerator fGenerator = new DefaultFileNameGenerator();
9099
fGenerator.setBeanFactory(mock(BeanFactory.class));
@@ -133,7 +142,7 @@ public void testHandleBytesMessage() throws Exception {
133142
file.delete();
134143
}
135144
SessionFactory<LsEntry> sessionFactory = new TestSftpSessionFactory();
136-
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
145+
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
137146
DefaultFileNameGenerator fGenerator = new DefaultFileNameGenerator();
138147
fGenerator.setBeanFactory(mock(BeanFactory.class));
139148
fGenerator.setExpression("'foo.txt'");
@@ -142,7 +151,7 @@ public void testHandleBytesMessage() throws Exception {
142151
handler.setBeanFactory(mock(BeanFactory.class));
143152
handler.afterPropertiesSet();
144153

145-
handler.handleMessage(new GenericMessage<byte[]>("byte[] data".getBytes()));
154+
handler.handleMessage(new GenericMessage<>("byte[] data".getBytes()));
146155
assertThat(new File("remote-target-dir", "foo.txt").exists()).isTrue();
147156
byte[] inFile = FileCopyUtils.copyToByteArray(file);
148157
assertThat(new String(inFile)).isEqualTo("byte[] data");
@@ -171,7 +180,7 @@ public void testSftpOutboundChannelAdapterInsideChain() throws Exception {
171180
}
172181

173182
@Test //INT-2275
174-
public void testFtpOutboundGatewayInsideChain() throws Exception {
183+
public void testFtpOutboundGatewayInsideChain() {
175184
ClassPathXmlApplicationContext context = new ClassPathXmlApplicationContext(
176185
"SftpOutboundInsideChainTests-context.xml", getClass());
177186

@@ -202,17 +211,17 @@ public void testMkDir() throws Exception {
202211
@SuppressWarnings("unchecked")
203212
SessionFactory<LsEntry> sessionFactory = mock(SessionFactory.class);
204213
when(sessionFactory.getSession()).thenReturn(session);
205-
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<LsEntry>(sessionFactory);
214+
FileTransferringMessageHandler<LsEntry> handler = new FileTransferringMessageHandler<>(sessionFactory);
206215
handler.setAutoCreateDirectory(true);
207216
handler.setRemoteDirectoryExpression(new LiteralExpression("/foo/bar/baz"));
208217
handler.setBeanFactory(mock(BeanFactory.class));
209218
handler.afterPropertiesSet();
210-
final List<String> madeDirs = new ArrayList<String>();
219+
final List<String> madeDirs = new ArrayList<>();
211220
doAnswer(invocation -> {
212221
madeDirs.add(invocation.getArgument(0));
213222
return null;
214223
}).when(session).mkdir(anyString());
215-
handler.handleMessage(new GenericMessage<String>("qux"));
224+
handler.handleMessage(new GenericMessage<>("qux"));
216225
assertThat(madeDirs.size()).isEqualTo(3);
217226
assertThat(madeDirs.get(0)).isEqualTo("/foo");
218227
assertThat(madeDirs.get(1)).isEqualTo("/foo/bar");
@@ -380,6 +389,34 @@ public void testSharedSessionCachedReset() throws Exception {
380389
verify(jschSession2).disconnect();
381390
}
382391

392+
@Test
393+
public void testExists() throws SftpException, IOException {
394+
ChannelSftp channelSftp = mock(ChannelSftp.class);
395+
396+
willReturn(mock(SftpATTRS.class))
397+
.given(channelSftp)
398+
.lstat(eq("exist"));
399+
400+
willThrow(new SftpException(ChannelSftp.SSH_FX_NO_SUCH_FILE, "Path does not exist."))
401+
.given(channelSftp)
402+
.lstat(eq("notExist"));
403+
404+
willThrow(new SftpException(ChannelSftp.SSH_FX_CONNECTION_LOST, "Connection lost."))
405+
.given(channelSftp)
406+
.lstat(and(not(eq("exist")), not(eq("notExist"))));
407+
408+
SftpSession sftpSession = new SftpSession(mock(com.jcraft.jsch.Session.class));
409+
DirectFieldAccessor fieldAccessor = new DirectFieldAccessor(sftpSession);
410+
fieldAccessor.setPropertyValue("channel", channelSftp);
411+
412+
assertThat(sftpSession.exists("exist")).isTrue();
413+
414+
assertThat(sftpSession.exists("notExist")).isFalse();
415+
416+
assertThatExceptionOfType(NestedIOException.class).
417+
isThrownBy(() -> sftpSession.exists("foo"));
418+
}
419+
383420
private void noopConnect(ChannelSftp channel1) throws JSchException {
384421
doNothing().when(channel1).connect(5000);
385422
}

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

+23
Original file line numberDiff line numberDiff line change
@@ -44,6 +44,7 @@
4444
import org.springframework.beans.factory.annotation.Autowired;
4545
import org.springframework.context.ApplicationContext;
4646
import org.springframework.context.annotation.Bean;
47+
import org.springframework.core.NestedIOException;
4748
import org.springframework.integration.channel.DirectChannel;
4849
import org.springframework.integration.event.inbound.ApplicationEventListeningMessageProducer;
4950
import org.springframework.integration.file.FileHeaders;
@@ -59,6 +60,7 @@
5960
import org.springframework.integration.sftp.server.PathRemovedEvent;
6061
import org.springframework.integration.sftp.server.SessionClosedEvent;
6162
import org.springframework.integration.sftp.server.SessionOpenedEvent;
63+
import org.springframework.integration.sftp.session.DefaultSftpSessionFactory;
6264
import org.springframework.integration.sftp.session.SftpRemoteFileTemplate;
6365
import org.springframework.integration.support.MessageBuilder;
6466
import org.springframework.integration.test.util.TestUtils;
@@ -561,6 +563,27 @@ private void assertLength6(SftpRemoteFileTemplate template) {
561563
assertThat(files[0].getAttrs().getSize()).isEqualTo(6);
562564
}
563565

566+
@Test
567+
public void testSessionExists() throws IOException {
568+
DefaultSftpSessionFactory sessionFactory = new DefaultSftpSessionFactory();
569+
sessionFactory.setHost("localhost");
570+
sessionFactory.setPort(port);
571+
sessionFactory.setUser("foo");
572+
sessionFactory.setPassword("foo");
573+
sessionFactory.setAllowUnknownKeys(true);
574+
Session<LsEntry> session = sessionFactory.getSession();
575+
576+
assertThat(session.exists("sftpSource")).isTrue();
577+
assertThat(session.exists("notExist")).isFalse();
578+
579+
session.close();
580+
581+
assertThatExceptionOfType(NestedIOException.class)
582+
.isThrownBy(() -> session.exists("any"))
583+
.withRootCauseInstanceOf(IOException.class)
584+
.withStackTraceContaining("Pipe closed");
585+
}
586+
564587
@SuppressWarnings("unused")
565588
private static final class TestMessageSessionCallback
566589
implements MessageSessionCallback<LsEntry, Object> {

src/checkstyle/checkstyle.xml

+1
Original file line numberDiff line numberDiff line change
@@ -84,6 +84,7 @@
8484
org.mockito.BDDMockito.*,
8585
org.mockito.AdditionalAnswers.*,
8686
org.mockito.ArgumentMatchers.*,
87+
org.mockito.AdditionalMatchers.*,
8788
org.springframework.integration.gemfire.config.xml.ParserTestUtil.*,
8889
org.springframework.integration.test.mock.MockIntegration.*,
8990
org.springframework.integration.test.util.TestUtils.*,

0 commit comments

Comments
 (0)