GH-8581: Do not overwrite configuration of external provided SshClient

[azaaiman]

Fixes #8581

Moved configuring an internal provided SshClient to a separate method, which only will execute if isInnerClient is true.
Added a test proving an external configured SshClient can connect to a SshServer.

[artembilan]

Error: eckstyle] [ERROR] /home/runner/work/spring-integration/spring-integration/spring-integration-sftp/src/test/java/org/springframework/integration/sftp/session/SftpSessionFactoryTests.java:41:47: Using a static member import should be avoided - org.junit.jupiter.api.Assertions.assertDoesNotThrow. [AvoidStaticImport]

We prefer to use assertion from AsssertJ library. See assertThatNoException() instead of JUnit's one.

[azaaiman]

Sure, I replaced it.

[artembilan]

I'm not sure that this test really reflects what we would expect from its name.
I don't even think that we need to test against the real server and make connections.
There is probably just enough to verify against some mock that its method calls don't happen or so.

[azaaiman]

I have thought about using some mock.
But the init code does start a connection and tries to authenticate.

What you would want is something like this:

@Test
void externallyProvidedSshClientShouldNotHaveItsConfigurationOverwritten() throws IOException {
	SshClient externalClient = spy(SshClient.setUpDefaultClient());

	DefaultSftpSessionFactory sftpSessionFactory = new DefaultSftpSessionFactory(externalClient, false);
	sftpSessionFactory.setHost("localhost");
	sftpSessionFactory.setUser("user");

	assertThatNoException().isThrownBy(() -> sftpSessionFactory.getSession());

	verify(externalClient, never()).setServerKeyVerifier(any());
	verify(externalClient, never()).setPasswordIdentityProvider(any());
	verify(externalClient, never()).setKeyIdentityProvider(any());
	verify(externalClient, never()).setUserInteraction(any());
}

But that would raise an exception:

java.lang.IllegalStateException: failed to create SFTP Session
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.getSession(DefaultSftpSessionFactory.java:292)
...
Caused by: org.apache.sshd.common.SshException: No more authentication methods available
...
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.initClientSession(DefaultSftpSessionFactory.java:319)
	at org.springframework.integration.sftp.session.DefaultSftpSessionFactory.getSession(DefaultSftpSessionFactory.java:282)
	... 89 more
Caused by: org.apache.sshd.common.SshException: No more authentication methods available
	at org.apache.sshd.client.session.ClientUserAuthService.tryNext(ClientUserAuthService.java:379)

I doubt it would be wise to mock the better part of the calls which happen in the init towards the SshClient.

Any suggestions?

[artembilan]

Yeah... I see: the integration with that SshClient API is very heavy, therefore it is probably better to exchange a performance of the test in favor of clear code and direct calls instead of our error-prone mocks.
So, never mind then and thank you for your thoughts!
Please, consider to fix an assert in favor of assertThatNoException() - and we are good to merge this.

[artembilan]

Merged as 775e6fd and cherry-picked to 6.0.x.

@azaaiman ,

Thank you for contribution; looking forward for more!