Skip to content

Commit bd7a3bc

Browse files
garyrussellartembilan
authored andcommitted
GH-2605: (S)FTP test cached sessions
Resolves #2605 * Suppress unused field warning. * Fix typos. * Use lstat() - don't follow symbolic lincs
1 parent eed5fe7 commit bd7a3bc

File tree

8 files changed

+88
-10
lines changed

8 files changed

+88
-10
lines changed

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

Lines changed: 13 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -48,6 +48,8 @@ public class CachingSessionFactory<F> implements SessionFactory<F>, DisposableBe
4848

4949
private final boolean isSharedSessionCapable;
5050

51+
private boolean testSession;
52+
5153
private volatile long sharedSessionEpoch;
5254

5355
/**
@@ -83,7 +85,7 @@ public Session<F> createForPool() {
8385

8486
@Override
8587
public boolean isStale(Session<F> session) {
86-
return !session.isOpen();
88+
return CachingSessionFactory.this.testSession ? !session.test() : !session.isOpen();
8789
}
8890

8991
@Override
@@ -115,6 +117,15 @@ public void setPoolSize(int poolSize) {
115117
this.pool.setPoolSize(poolSize);
116118
}
117119

120+
/**
121+
* Set to true to test the session when checking one out from the cache.
122+
* @param testSession true to test.
123+
* @since 5.1
124+
*/
125+
public void setTestSession(boolean testSession) {
126+
this.testSession = testSession;
127+
}
128+
118129
/**
119130
* Get a session from the pool (or block if none available).
120131
*/

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

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -107,4 +107,14 @@ public interface Session<F> extends Closeable {
107107
*/
108108
Object getClientInstance();
109109

110+
/**
111+
* Test the session is still alive, e.g. when checking out from a pool.
112+
* The default implementation simply delegates to {@link #isOpen()}.
113+
* @return true if the test is successful.
114+
* @since 5.1
115+
*/
116+
default boolean test() {
117+
return this.isOpen();
118+
}
119+
110120
}

spring-integration-file/src/test/java/org/springframework/integration/file/remote/session/CachingSessionFactoryTests.java

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2013-2016 the original author or authors.
2+
* Copyright 2013-2018 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.
@@ -52,6 +52,7 @@ public class CachingSessionFactoryTests {
5252
public void testCacheAndReset() {
5353
TestSessionFactory factory = new TestSessionFactory();
5454
CachingSessionFactory<String> cache = new CachingSessionFactory<String>(factory);
55+
cache.setTestSession(true);
5556
Session<String> sess1 = cache.getSession();
5657
assertEquals("session:1", TestUtils.getPropertyValue(sess1, "targetSession.id"));
5758
Session<String> sess2 = cache.getSession();
@@ -61,6 +62,7 @@ public void testCacheAndReset() {
6162
assertTrue(sess1.isOpen());
6263
sess1 = cache.getSession();
6364
assertEquals("session:1", TestUtils.getPropertyValue(sess1, "targetSession.id"));
65+
assertTrue((TestUtils.getPropertyValue(sess1, "targetSession.testCalled", Boolean.class)));
6466
sess1.close();
6567
assertTrue(sess1.isOpen());
6668
// reset the cache; should close idle (sess1); sess2 should closed later
@@ -122,6 +124,9 @@ private class TestSession implements Session<String> {
122124

123125
private volatile boolean open = true;
124126

127+
@SuppressWarnings("unused")
128+
private boolean testCalled;
129+
125130
private TestSession(String id) {
126131
this.id = id;
127132
}
@@ -197,6 +202,12 @@ public Object getClientInstance() {
197202
return null;
198203
}
199204

205+
@Override
206+
public boolean test() {
207+
this.testCalled = true;
208+
return true;
209+
}
210+
200211
}
201212

202213
}

spring-integration-ftp/src/main/java/org/springframework/integration/ftp/session/FtpSession.java

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -226,4 +226,20 @@ public FTPClient getClientInstance() {
226226
return this.client;
227227
}
228228

229+
230+
@Override
231+
public boolean test() {
232+
return isOpen() && doTest();
233+
}
234+
235+
private boolean doTest() {
236+
try {
237+
this.client.noop();
238+
return true;
239+
}
240+
catch (IOException e) {
241+
return false;
242+
}
243+
}
244+
229245
}

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

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
/*
2-
* Copyright 2002-2016 the original author or authors.
2+
* Copyright 2002-2018 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.
@@ -278,4 +278,20 @@ public ChannelSftp getClientInstance() {
278278
return this.channel;
279279
}
280280

281+
@Override
282+
public boolean test() {
283+
return isOpen() && doTest();
284+
}
285+
286+
private boolean doTest() {
287+
try {
288+
this.channel.lstat(this.channel.getHome());
289+
return true;
290+
}
291+
catch (Exception e) {
292+
return false;
293+
}
294+
}
295+
296+
281297
}

src/reference/asciidoc/ftp.adoc

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -85,10 +85,6 @@ The following example shows a complete configuration:
8585
----
8686
====
8787

88-
Every time an adapter requests a session object from its `SessionFactory`, the session is returned from a session pool maintained by a caching wrapper around the factory.
89-
A session in the session pool might go stale (if it has been disconnected by the server due to inactivity), so the `SessionFactory` performs validation to make sure that it never returns a stale session to the adapter.
90-
If a stale session was encountered, it is removed from the pool, and a new one is created.
91-
9288
NOTE: If you experience connectivity problems and would like to trace session creation as well as see which sessions are polled, you can enable session tracing by setting the logger to the `TRACE` level (for example, `log4j.category.org.springframework.integration.file=TRACE`).
9389

9490
Now you need only inject these session factories into your adapters.
@@ -482,6 +478,7 @@ public class FtpJavaApplication {
482478
sf.setPort(port);
483479
sf.setUsername("foo");
484480
sf.setPassword("foo");
481+
sf.setTestSession(true);
485482
return new CachingSessionFactory<FTPFile>(sf);
486483
}
487484
@@ -884,6 +881,7 @@ public class FtpJavaApplication {
884881
sf.setPort(port);
885882
sf.setUsername("foo");
886883
sf.setPassword("foo");
884+
sf.setTestSession(true);
887885
return new CachingSessionFactory<FTPFile>(sf);
888886
}
889887
@@ -941,6 +939,7 @@ public class FtpJavaApplication {
941939
sf.setPort(port);
942940
sf.setUsername("foo");
943941
sf.setPassword("foo");
942+
sf.setTestSession(true);
944943
return new CachingSessionFactory<FTPFile>(sf);
945944
}
946945
@@ -1274,6 +1273,7 @@ public class FtpJavaApplication {
12741273
sf.setPort(port);
12751274
sf.setUsername("foo");
12761275
sf.setPassword("foo");
1276+
sf.setTestSession(true);
12771277
return new CachingSessionFactory<FTPFile>(sf);
12781278
}
12791279
@@ -1313,6 +1313,7 @@ public class FtpJavaApplication {
13131313
sf.setPort(port);
13141314
sf.setUsername("foo");
13151315
sf.setPassword("foo");
1316+
sf.setTestSession(true);
13161317
return new CachingSessionFactory<FTPFile>(sf);
13171318
}
13181319
@@ -1412,6 +1413,9 @@ Starting with Spring Integration 3.0, the `CachingConnectionFactory` provides a
14121413
When invoked, all idle sessions are immediately closed and in-use sessions are closed when they are returned to the cache.
14131414
New requests for sessions establish new sessions as necessary.
14141415

1416+
Starting with version 5.1, the `CachingSessionFactory` has a new property `testSession`.
1417+
When true, the session will be tested by sending a NOOP command to ensure it is still active; if not, it will be removed from the cache; a new session is created if no active sessions are in the cache.
1418+
14151419
[[ftp-rft]]
14161420
=== Using `RemoteFileTemplate`
14171421

src/reference/asciidoc/sftp.adoc

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -253,6 +253,9 @@ When invoked, all idle sessions are immediately closed and in-use sessions are c
253253
When using `isSharedSession=true`, the channel is closed and the shared session is closed only when the last channel is closed.
254254
New requests for sessions establish new sessions as necessary.
255255

256+
Starting with version 5.1, the `CachingSessionFactory` has a new property `testSession`.
257+
When true, the session will be tested by performing a `stat(getHome())` command to ensure it is still active; if not, it will be removed from the cache; a new session is created if no active sessions are in the cache.
258+
256259
[[sftp-rft]]
257260
=== Using `RemoteFileTemplate`
258261

@@ -476,6 +479,7 @@ public class SftpJavaApplication {
476479
factory.setUser("foo");
477480
factory.setPassword("foo");
478481
factory.setAllowUnknownKeys(true);
482+
factory.setTestSession(true);
479483
return new CachingSessionFactory<LsEntry>(factory);
480484
}
481485
@@ -873,6 +877,7 @@ public class SftpJavaApplication {
873877
factory.setUser("foo");
874878
factory.setPassword("foo");
875879
factory.setAllowUnknownKeys(true);
880+
factory.setTestSession(true);
876881
return new CachingSessionFactory<LsEntry>(factory);
877882
}
878883
@@ -1257,6 +1262,7 @@ public class SftpJavaApplication {
12571262
sf.setPort(port);
12581263
sf.setUsername("foo");
12591264
sf.setPassword("foo");
1265+
factory.setTestSession(true);
12601266
return new CachingSessionFactory<LsEntry>(sf);
12611267
}
12621268

src/reference/asciidoc/whats-new.adoc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -162,6 +162,10 @@ See <<ftp-streaming>> and <<sftp-streaming>> for more information.
162162
In addition, the synchronizers for inbound channel adapters can now be provided with a `Comparator`.
163163
This is useful when using `maxFetchSize` to limit the files retrieved.
164164

165+
The `CachingSessionFactory` has a new property `testSession` which, when true, causes the factory to perform a `test()` operation on the `Session` when checking out an existing session from the cache.
166+
167+
See <<sftp-session-caching>> and <<ftp-session-caching>> for more information.
168+
165169
[[x51.-tcp]]
166170
=== TCP Support
167171

0 commit comments

Comments
 (0)