GH-3168: Fix FtpSession warning on logout

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

* Call `this.client.noop()` instead of `this.client.isConnected()` to really
check that client has a live connection with the server before calling a `this.client.logout()`
* Wrap `this.client.logout()` into a `try..catch` to be sure that we wil call a
`this.client.disconnect()` even if `logout()` fails for some reason.
* Change `WARN` logs about `Session.close()` into a `DEBUG` level -
when we have a problem with closing session because of server disconnect reason
we have no any control to do with a situation

**Cherry-pick to 5.2.x**
This commit is contained in:
Artem Bilan
2020-02-10 15:04:14 -05:00
committed by Gary Russell
parent 1bd9954a76
commit 23b1a9fb7e
2 changed files with 23 additions and 9 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.
@@ -152,16 +152,21 @@ public class FtpSession implements Session<FTPFile> {
@Override
public void close() {
try {
if (this.readingRaw.get() && !finalizeRaw() && LOGGER.isWarnEnabled()) {
LOGGER.warn("Finalize on readRaw() returned false for " + this);
if (this.readingRaw.get() && !finalizeRaw() && LOGGER.isDebugEnabled()) {
LOGGER.debug("Finalize on readRaw() returned false for " + this);
}
if (this.client.isConnected()) {
this.client.logout();
if (isOpen()) {
try {
this.client.logout();
}
catch (IOException ex) {
LOGGER.debug("failed to logout FTPClient", ex);
}
}
this.client.disconnect();
}
catch (Exception e) {
LOGGER.warn("failed to disconnect FTPClient", e);
catch (Exception ex) {
LOGGER.debug("failed to disconnect FTPClient", ex);
}
}
@@ -170,7 +175,8 @@ public class FtpSession implements Session<FTPFile> {
try {
this.client.noop();
}
catch (Exception e) {
catch (Exception ex) {
LOGGER.debug("failed to noop FTPClient", ex);
return false;
}
return true;

View File

@@ -21,7 +21,9 @@ import static org.assertj.core.api.Assertions.assertThatExceptionOfType;
import static org.assertj.core.api.Assertions.assertThatIllegalArgumentException;
import static org.assertj.core.api.Assertions.fail;
import static org.mockito.Mockito.doReturn;
import static org.mockito.Mockito.doThrow;
import static org.mockito.Mockito.mock;
import static org.mockito.Mockito.times;
import static org.mockito.Mockito.verify;
import java.lang.reflect.Field;
@@ -69,7 +71,6 @@ class SessionFactoryTests {
sessionFactory.setDataTimeout(789);
doReturn(200).when(client).getReplyCode();
doReturn(true).when(client).login("foo", null);
doReturn(true).when(client).isConnected();
FtpSession session = sessionFactory.getSession();
verify(client).setConnectTimeout(123);
verify(client).setDefaultTimeout(456);
@@ -79,6 +80,13 @@ class SessionFactoryTests {
verify(client).logout();
verify(client).disconnect();
doThrow(RuntimeException.class).when(client).noop();
session.close();
verify(client).logout();
verify(client, times(2)).disconnect();
}
@Test