From bd7a3bc4edb32501c13b09fd6208b38188f1246c Mon Sep 17 00:00:00 2001 From: Gary Russell Date: Tue, 23 Oct 2018 13:59:00 -0400 Subject: [PATCH] GH-2605: (S)FTP test cached sessions Resolves https://github.com/spring-projects/spring-integration/issues/2605 * Suppress unused field warning. * Fix typos. * Use lstat() - don't follow symbolic lincs --- .../remote/session/CachingSessionFactory.java | 15 +++++++++++++-- .../file/remote/session/Session.java | 12 +++++++++++- .../session/CachingSessionFactoryTests.java | 13 ++++++++++++- .../integration/ftp/session/FtpSession.java | 18 +++++++++++++++++- .../integration/sftp/session/SftpSession.java | 18 +++++++++++++++++- src/reference/asciidoc/ftp.adoc | 12 ++++++++---- src/reference/asciidoc/sftp.adoc | 6 ++++++ src/reference/asciidoc/whats-new.adoc | 4 ++++ 8 files changed, 88 insertions(+), 10 deletions(-) diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java index fe4597bf3c..20445a7723 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/CachingSessionFactory.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -48,6 +48,8 @@ public class CachingSessionFactory implements SessionFactory, DisposableBe private final boolean isSharedSessionCapable; + private boolean testSession; + private volatile long sharedSessionEpoch; /** @@ -83,7 +85,7 @@ public class CachingSessionFactory implements SessionFactory, DisposableBe @Override public boolean isStale(Session session) { - return !session.isOpen(); + return CachingSessionFactory.this.testSession ? !session.test() : !session.isOpen(); } @Override @@ -115,6 +117,15 @@ public class CachingSessionFactory implements SessionFactory, DisposableBe this.pool.setPoolSize(poolSize); } + /** + * Set to true to test the session when checking one out from the cache. + * @param testSession true to test. + * @since 5.1 + */ + public void setTestSession(boolean testSession) { + this.testSession = testSession; + } + /** * Get a session from the pool (or block if none available). */ diff --git a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/Session.java b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/Session.java index 76c0789299..7c4bcfd450 100644 --- a/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/Session.java +++ b/spring-integration-file/src/main/java/org/springframework/integration/file/remote/session/Session.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -107,4 +107,14 @@ public interface Session extends Closeable { */ Object getClientInstance(); + /** + * Test the session is still alive, e.g. when checking out from a pool. + * The default implementation simply delegates to {@link #isOpen()}. + * @return true if the test is successful. + * @since 5.1 + */ + default boolean test() { + return this.isOpen(); + } + } diff --git a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/session/CachingSessionFactoryTests.java b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/session/CachingSessionFactoryTests.java index 2168828dd6..3cce48210d 100644 --- a/spring-integration-file/src/test/java/org/springframework/integration/file/remote/session/CachingSessionFactoryTests.java +++ b/spring-integration-file/src/test/java/org/springframework/integration/file/remote/session/CachingSessionFactoryTests.java @@ -1,5 +1,5 @@ /* - * Copyright 2013-2016 the original author or authors. + * Copyright 2013-2018 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. @@ -52,6 +52,7 @@ public class CachingSessionFactoryTests { public void testCacheAndReset() { TestSessionFactory factory = new TestSessionFactory(); CachingSessionFactory cache = new CachingSessionFactory(factory); + cache.setTestSession(true); Session sess1 = cache.getSession(); assertEquals("session:1", TestUtils.getPropertyValue(sess1, "targetSession.id")); Session sess2 = cache.getSession(); @@ -61,6 +62,7 @@ public class CachingSessionFactoryTests { assertTrue(sess1.isOpen()); sess1 = cache.getSession(); assertEquals("session:1", TestUtils.getPropertyValue(sess1, "targetSession.id")); + assertTrue((TestUtils.getPropertyValue(sess1, "targetSession.testCalled", Boolean.class))); sess1.close(); assertTrue(sess1.isOpen()); // reset the cache; should close idle (sess1); sess2 should closed later @@ -122,6 +124,9 @@ public class CachingSessionFactoryTests { private volatile boolean open = true; + @SuppressWarnings("unused") + private boolean testCalled; + private TestSession(String id) { this.id = id; } @@ -197,6 +202,12 @@ public class CachingSessionFactoryTests { return null; } + @Override + public boolean test() { + this.testCalled = true; + return true; + } + } } diff --git a/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/session/FtpSession.java b/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/session/FtpSession.java index a7391ee696..294899aa36 100644 --- a/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/session/FtpSession.java +++ b/spring-integration-ftp/src/main/java/org/springframework/integration/ftp/session/FtpSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -226,4 +226,20 @@ public class FtpSession implements Session { return this.client; } + + @Override + public boolean test() { + return isOpen() && doTest(); + } + + private boolean doTest() { + try { + this.client.noop(); + return true; + } + catch (IOException e) { + return false; + } + } + } diff --git a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java index 160c396ba3..ef7a651fd5 100644 --- a/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java +++ b/spring-integration-sftp/src/main/java/org/springframework/integration/sftp/session/SftpSession.java @@ -1,5 +1,5 @@ /* - * Copyright 2002-2016 the original author or authors. + * Copyright 2002-2018 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. @@ -278,4 +278,20 @@ public class SftpSession implements Session { return this.channel; } + @Override + public boolean test() { + return isOpen() && doTest(); + } + + private boolean doTest() { + try { + this.channel.lstat(this.channel.getHome()); + return true; + } + catch (Exception e) { + return false; + } + } + + } diff --git a/src/reference/asciidoc/ftp.adoc b/src/reference/asciidoc/ftp.adoc index f83ad8a8e6..2fd2c19f2c 100644 --- a/src/reference/asciidoc/ftp.adoc +++ b/src/reference/asciidoc/ftp.adoc @@ -85,10 +85,6 @@ The following example shows a complete configuration: ---- ==== -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. -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. -If a stale session was encountered, it is removed from the pool, and a new one is created. - 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`). Now you need only inject these session factories into your adapters. @@ -482,6 +478,7 @@ public class FtpJavaApplication { sf.setPort(port); sf.setUsername("foo"); sf.setPassword("foo"); + sf.setTestSession(true); return new CachingSessionFactory(sf); } @@ -884,6 +881,7 @@ public class FtpJavaApplication { sf.setPort(port); sf.setUsername("foo"); sf.setPassword("foo"); + sf.setTestSession(true); return new CachingSessionFactory(sf); } @@ -941,6 +939,7 @@ public class FtpJavaApplication { sf.setPort(port); sf.setUsername("foo"); sf.setPassword("foo"); + sf.setTestSession(true); return new CachingSessionFactory(sf); } @@ -1274,6 +1273,7 @@ public class FtpJavaApplication { sf.setPort(port); sf.setUsername("foo"); sf.setPassword("foo"); + sf.setTestSession(true); return new CachingSessionFactory(sf); } @@ -1313,6 +1313,7 @@ public class FtpJavaApplication { sf.setPort(port); sf.setUsername("foo"); sf.setPassword("foo"); + sf.setTestSession(true); return new CachingSessionFactory(sf); } @@ -1412,6 +1413,9 @@ Starting with Spring Integration 3.0, the `CachingConnectionFactory` provides a When invoked, all idle sessions are immediately closed and in-use sessions are closed when they are returned to the cache. New requests for sessions establish new sessions as necessary. +Starting with version 5.1, the `CachingSessionFactory` has a new property `testSession`. +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. + [[ftp-rft]] === Using `RemoteFileTemplate` diff --git a/src/reference/asciidoc/sftp.adoc b/src/reference/asciidoc/sftp.adoc index e75b28d7d0..66b645ddfd 100644 --- a/src/reference/asciidoc/sftp.adoc +++ b/src/reference/asciidoc/sftp.adoc @@ -253,6 +253,9 @@ When invoked, all idle sessions are immediately closed and in-use sessions are c When using `isSharedSession=true`, the channel is closed and the shared session is closed only when the last channel is closed. New requests for sessions establish new sessions as necessary. +Starting with version 5.1, the `CachingSessionFactory` has a new property `testSession`. +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. + [[sftp-rft]] === Using `RemoteFileTemplate` @@ -476,6 +479,7 @@ public class SftpJavaApplication { factory.setUser("foo"); factory.setPassword("foo"); factory.setAllowUnknownKeys(true); + factory.setTestSession(true); return new CachingSessionFactory(factory); } @@ -873,6 +877,7 @@ public class SftpJavaApplication { factory.setUser("foo"); factory.setPassword("foo"); factory.setAllowUnknownKeys(true); + factory.setTestSession(true); return new CachingSessionFactory(factory); } @@ -1257,6 +1262,7 @@ public class SftpJavaApplication { sf.setPort(port); sf.setUsername("foo"); sf.setPassword("foo"); + factory.setTestSession(true); return new CachingSessionFactory(sf); } diff --git a/src/reference/asciidoc/whats-new.adoc b/src/reference/asciidoc/whats-new.adoc index b5664ae25a..db7f79b778 100644 --- a/src/reference/asciidoc/whats-new.adoc +++ b/src/reference/asciidoc/whats-new.adoc @@ -162,6 +162,10 @@ See <> and <> for more information. In addition, the synchronizers for inbound channel adapters can now be provided with a `Comparator`. This is useful when using `maxFetchSize` to limit the files retrieved. +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. + +See <> and <> for more information. + [[x51.-tcp]] === TCP Support