This is an automated email from the ASF dual-hosted git repository.
lgoldstein pushed a commit to branch master
in repository https://gitbox.apache.org/repos/asf/mina-sshd.git
commit 23a6e7bd4c1f60ae7bb086254ef1394f357403c1
Author: Lyor Goldstein <lgoldst...@apache.org>
AuthorDate: Tue Nov 5 19:16:18 2019 +0200
[SSHD-931] Using an executor supplier instead of a specific instance in
'AbstractGitCommandFactory'
---
CHANGES.md | 7 ++---
.../util/threads/ExecutorServiceCarrier.java | 2 +-
...ceCarrier.java => ExecutorServiceProvider.java} | 15 ++++++++---
...er.java => ManagedExecutorServiceSupplier.java} | 11 +++++---
.../apache/sshd/git/AbstractGitCommandFactory.java | 31 +++++++++++++++-------
.../sshd/git/pack/GitPackCommandFactory.java | 9 ++++---
.../apache/sshd/git/pgm/GitPgmCommandFactory.java | 11 +++++---
.../apache/sshd/server/scp/ScpCommandFactory.java | 18 ++++++-------
.../subsystem/sftp/SftpSubsystemFactory.java | 19 +++++--------
9 files changed, 75 insertions(+), 48 deletions(-)
diff --git a/CHANGES.md b/CHANGES.md
index d9ba927..983f320 100644
--- a/CHANGES.md
+++ b/CHANGES.md
@@ -6,9 +6,10 @@
## Major code re-factoring
-* `SftpSubSystemFactory,ScpCommandFactory` and their respective `Builder`(s)
use a `Supplier<CloseableExecutorService>`
-instead of an executor instance in order to allow users to provide a "fresh"
instance every time an SFTP
-session is initiated and protect their instance from shutdown when session is
destroyed:
+* `SftpSubSystemFactory,ScpCommandFactory` and their respective `Builder`(s)
as well as the
+`AbstractGitCommandFactory` use a `Supplier<CloseableExecutorService>` instead
of an executor instance
+in order to allow users to provide a "fresh" instance every time a new command
instance
+is initiated and protect their instance from shutdown when session is
destroyed:
```java
CloseableExecutorService mySpecialExecutor = ...;
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
index b44bd46..72603a1 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
@@ -22,10 +22,10 @@ package org.apache.sshd.common.util.threads;
/**
* @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
*/
+@FunctionalInterface
public interface ExecutorServiceCarrier {
/**
* @return The {@link CloseableExecutorService} to use
*/
CloseableExecutorService getExecutorService();
-
}
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceProvider.java
similarity index 61%
copy from
sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
copy to
sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceProvider.java
index b44bd46..afd5eba 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceProvider.java
@@ -19,13 +19,22 @@
package org.apache.sshd.common.util.threads;
+import java.util.function.Supplier;
+
/**
* @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
*/
-public interface ExecutorServiceCarrier {
+@FunctionalInterface
+public interface ExecutorServiceProvider {
/**
- * @return The {@link CloseableExecutorService} to use
+ * @return A {@link Supplier} of {@link CloseableExecutorService} to be
used when
+ * asynchronous execution required. If {@code null} then a single-threaded
+ * ad-hoc service is used.
*/
- CloseableExecutorService getExecutorService();
+ Supplier<? extends CloseableExecutorService> getExecutorServiceProvider();
+ default CloseableExecutorService resolveExecutorService() {
+ Supplier<? extends CloseableExecutorService> provider =
getExecutorServiceProvider();
+ return (provider == null) ? null : provider.get();
+ }
}
diff --git
a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ManagedExecutorServiceSupplier.java
similarity index 69%
copy from
sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
copy to
sshd-common/src/main/java/org/apache/sshd/common/util/threads/ManagedExecutorServiceSupplier.java
index b44bd46..56b96d491 100644
---
a/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ExecutorServiceCarrier.java
+++
b/sshd-common/src/main/java/org/apache/sshd/common/util/threads/ManagedExecutorServiceSupplier.java
@@ -19,13 +19,16 @@
package org.apache.sshd.common.util.threads;
+import java.util.function.Supplier;
+
/**
* @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
*/
-public interface ExecutorServiceCarrier {
+public interface ManagedExecutorServiceSupplier extends
ExecutorServiceProvider {
/**
- * @return The {@link CloseableExecutorService} to use
+ * @param provider The {@link Supplier} of {@link
CloseableExecutorService}-s to be used
+ * when asynchronous execution is required. If {@code null} then a
single-threaded
+ * ad-hoc service is used.
*/
- CloseableExecutorService getExecutorService();
-
+ void setExecutorServiceProvider(Supplier<? extends
CloseableExecutorService> provider);
}
diff --git
a/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommandFactory.java
b/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommandFactory.java
index dd0cb3c..2588083 100644
--- a/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommandFactory.java
+++ b/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommandFactory.java
@@ -19,26 +19,28 @@
package org.apache.sshd.git;
+import java.util.function.Supplier;
+
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.ValidateUtils;
import org.apache.sshd.common.util.threads.CloseableExecutorService;
-import org.apache.sshd.common.util.threads.ExecutorServiceCarrier;
+import org.apache.sshd.common.util.threads.ExecutorServiceProvider;
import org.apache.sshd.server.command.AbstractDelegatingCommandFactory;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.command.CommandFactory;
import org.apache.sshd.server.shell.UnknownCommand;
/**
- * TODO Add javadoc
+ * Helper class for various Git command factories
*
* @author <a href="mailto:d...@mina.apache.org";>Apache MINA SSHD Project</a>
*/
public abstract class AbstractGitCommandFactory
extends AbstractDelegatingCommandFactory
- implements ExecutorServiceCarrier, GitLocationResolverCarrier {
+ implements ExecutorServiceProvider, GitLocationResolverCarrier {
private final String cmdPrefix;
private GitLocationResolver rootDirResolver;
- private CloseableExecutorService executorService;
+ private Supplier<? extends CloseableExecutorService> executorsProvider;
/**
* @param name Command factory logical name
@@ -48,7 +50,8 @@ public abstract class AbstractGitCommandFactory
protected AbstractGitCommandFactory(String name, String cmdPrefix) {
super(name);
- this.cmdPrefix = ValidateUtils.checkNotNullAndNotEmpty(cmdPrefix, "No
command prefix provided");
+ this.cmdPrefix = ValidateUtils.checkNotNullAndNotEmpty(
+ cmdPrefix, "No command prefix provided");
}
public String getCommandPrefix() {
@@ -56,12 +59,18 @@ public abstract class AbstractGitCommandFactory
}
@Override
- public CloseableExecutorService getExecutorService() {
- return executorService;
+ public Supplier<? extends CloseableExecutorService>
getExecutorServiceProvider() {
+ return executorsProvider;
}
- public AbstractGitCommandFactory
withExecutorService(CloseableExecutorService executorService) {
- this.executorService = executorService;
+ /**
+ * @param provider A {@link Supplier} of {@link CloseableExecutorService}
to be used when
+ * starting a Git command execution. If {@code null} then a
single-threaded ad-hoc service is used.
+ * @return Self instance
+ */
+ public AbstractGitCommandFactory withExecutorServiceProvider(
+ Supplier<? extends CloseableExecutorService> provider) {
+ this.executorsProvider = provider;
return this;
}
@@ -100,5 +109,9 @@ public abstract class AbstractGitCommandFactory
return new UnknownCommand(command);
}
+ protected CloseableExecutorService resolveExecutorService(String command) {
+ return resolveExecutorService();
+ }
+
protected abstract AbstractGitCommand createGitCommand(String command);
}
diff --git
a/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommandFactory.java
b/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommandFactory.java
index 7c12459..a6b6e0d 100644
--- a/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommandFactory.java
+++ b/sshd-git/src/main/java/org/apache/sshd/git/pack/GitPackCommandFactory.java
@@ -18,6 +18,8 @@
*/
package org.apache.sshd.git.pack;
+import java.util.function.Supplier;
+
import org.apache.sshd.common.util.threads.CloseableExecutorService;
import org.apache.sshd.git.AbstractGitCommandFactory;
import org.apache.sshd.git.GitLocationResolver;
@@ -52,12 +54,13 @@ public class GitPackCommandFactory extends
AbstractGitCommandFactory {
}
@Override
- public GitPackCommandFactory withExecutorService(CloseableExecutorService
executorService) {
- return (GitPackCommandFactory)
super.withExecutorService(executorService);
+ public GitPackCommandFactory withExecutorServiceProvider(
+ Supplier<? extends CloseableExecutorService> provider) {
+ return (GitPackCommandFactory)
super.withExecutorServiceProvider(provider);
}
@Override
public GitPackCommand createGitCommand(String command) {
- return new GitPackCommand(getGitLocationResolver(), command,
getExecutorService());
+ return new GitPackCommand(getGitLocationResolver(), command,
resolveExecutorService(command));
}
}
diff --git
a/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommandFactory.java
b/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommandFactory.java
index ad805f1..85b8d7f 100644
--- a/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommandFactory.java
+++ b/sshd-git/src/main/java/org/apache/sshd/git/pgm/GitPgmCommandFactory.java
@@ -18,9 +18,12 @@
*/
package org.apache.sshd.git.pgm;
+import java.util.function.Supplier;
+
import org.apache.sshd.common.util.threads.CloseableExecutorService;
import org.apache.sshd.git.AbstractGitCommandFactory;
import org.apache.sshd.git.GitLocationResolver;
+import org.apache.sshd.git.pack.GitPackCommandFactory;
import org.apache.sshd.server.command.CommandFactory;
/**
@@ -52,12 +55,14 @@ public class GitPgmCommandFactory extends
AbstractGitCommandFactory {
}
@Override
- public GitPgmCommandFactory withExecutorService(CloseableExecutorService
executorService) {
- return (GitPgmCommandFactory)
super.withExecutorService(executorService);
+ public GitPackCommandFactory withExecutorServiceProvider(
+ Supplier<? extends CloseableExecutorService> provider) {
+ return (GitPackCommandFactory)
super.withExecutorServiceProvider(provider);
}
@Override
public GitPgmCommand createGitCommand(String command) {
- return new GitPgmCommand(getGitLocationResolver(),
command.substring(GIT_COMMAND_PREFIX.length()), getExecutorService());
+ String rawCommand = command.substring(GIT_COMMAND_PREFIX.length());
+ return new GitPgmCommand(getGitLocationResolver(), rawCommand,
resolveExecutorService(command));
}
}
diff --git
a/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommandFactory.java
b/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommandFactory.java
index 0e051ad..6a18102 100644
--- a/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommandFactory.java
+++ b/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommandFactory.java
@@ -30,6 +30,7 @@ import org.apache.sshd.common.util.EventListenerUtils;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.ObjectBuilder;
import org.apache.sshd.common.util.threads.CloseableExecutorService;
+import org.apache.sshd.common.util.threads.ManagedExecutorServiceSupplier;
import org.apache.sshd.server.command.AbstractDelegatingCommandFactory;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.command.CommandFactory;
@@ -44,7 +45,7 @@ import org.apache.sshd.server.command.CommandFactory;
*/
public class ScpCommandFactory
extends AbstractDelegatingCommandFactory
- implements ScpFileOpenerHolder, Cloneable {
+ implements ManagedExecutorServiceSupplier, ScpFileOpenerHolder,
Cloneable {
public static final String SCP_FACTORY_NAME = "scp";
@@ -123,16 +124,14 @@ public class ScpCommandFactory
this.fileOpener = fileOpener;
}
+ @Override
public Supplier<? extends CloseableExecutorService>
getExecutorServiceProvider() {
return executorsProvider;
}
- /**
- * @param provider A {@link Supplier} of {@link CloseableExecutorService}
to be used when
- * starting {@link ScpCommand} execution. If {@code null} then a
single-threaded
- * ad-hoc service is used.
- */
- public void setExecutorServiceProvider(Supplier<? extends
CloseableExecutorService> provider) {
+ @Override
+ public void setExecutorServiceProvider(
+ Supplier<? extends CloseableExecutorService> provider) {
executorsProvider = provider;
}
@@ -147,7 +146,7 @@ public class ScpCommandFactory
public void setSendBufferSize(int sendSize) {
if (sendSize < ScpHelper.MIN_SEND_BUFFER_SIZE) {
throw new IllegalArgumentException("<ScpCommandFactory>() send
buffer size "
- + "(" + sendSize + ") below minimum required (" +
ScpHelper.MIN_SEND_BUFFER_SIZE + ")");
+ + "(" + sendSize + ") below minimum required (" +
ScpHelper.MIN_SEND_BUFFER_SIZE + ")");
}
sendBufferSize = sendSize;
}
@@ -214,8 +213,7 @@ public class ScpCommandFactory
}
protected CloseableExecutorService resolveExecutorService(String command) {
- Supplier<? extends CloseableExecutorService> provider =
getExecutorServiceProvider();
- return (provider == null) ? null : provider.get();
+ return resolveExecutorService();
}
@Override
diff --git
a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
index 9b71f61..2154d4c 100644
---
a/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
+++
b/sshd-sftp/src/main/java/org/apache/sshd/server/subsystem/sftp/SftpSubsystemFactory.java
@@ -27,6 +27,7 @@ import org.apache.sshd.common.subsystem.sftp.SftpConstants;
import org.apache.sshd.common.util.GenericUtils;
import org.apache.sshd.common.util.ObjectBuilder;
import org.apache.sshd.common.util.threads.CloseableExecutorService;
+import org.apache.sshd.common.util.threads.ManagedExecutorServiceSupplier;
import org.apache.sshd.server.channel.ChannelSession;
import org.apache.sshd.server.command.Command;
import org.apache.sshd.server.subsystem.SubsystemFactory;
@@ -36,7 +37,8 @@ import org.apache.sshd.server.subsystem.SubsystemFactory;
*/
public class SftpSubsystemFactory
extends AbstractSftpEventListenerManager
- implements SubsystemFactory, SftpEventListenerManager,
SftpFileSystemAccessorManager {
+ implements ManagedExecutorServiceSupplier, SubsystemFactory,
+ SftpEventListenerManager, SftpFileSystemAccessorManager {
public static final String NAME = SftpConstants.SFTP_SUBSYSTEM_NAME;
public static final UnsupportedAttributePolicy DEFAULT_POLICY =
UnsupportedAttributePolicy.Warn;
@@ -97,16 +99,14 @@ public class SftpSubsystemFactory
return NAME;
}
+ @Override
public Supplier<? extends CloseableExecutorService>
getExecutorServiceProvider() {
return executorsProvider;
}
- /**
- * @param provider The {@link Supplier} of {@link
CloseableExecutorService}-s to be used by the
- * {@link SftpSubsystem} command when starting execution. If {@code null}
then a single-threaded
- * ad-hoc service is used.
- */
- public void setExecutorServiceProvider(Supplier<? extends
CloseableExecutorService> provider) {
+ @Override
+ public void setExecutorServiceProvider(
+ Supplier<? extends CloseableExecutorService> provider) {
executorsProvider = provider;
}
@@ -140,11 +140,6 @@ public class SftpSubsystemFactory
errorStatusDataHandler = Objects.requireNonNull(handler, "No error
status data handler provided");
}
- protected CloseableExecutorService resolveExecutorService() {
- Supplier<? extends CloseableExecutorService> provider =
getExecutorServiceProvider();
- return (provider == null) ? null : provider.get();
- }
-
@Override
public Command createSubsystem(ChannelSession channel) throws IOException {
SftpSubsystem subsystem =
