Small refactoring of the command implementations to extract common code
Project: http://git-wip-us.apache.org/repos/asf/mina-sshd/repo Commit: http://git-wip-us.apache.org/repos/asf/mina-sshd/commit/a04f585d Tree: http://git-wip-us.apache.org/repos/asf/mina-sshd/tree/a04f585d Diff: http://git-wip-us.apache.org/repos/asf/mina-sshd/diff/a04f585d Branch: refs/heads/master Commit: a04f585d390a3b49c1a5ad86f74d132189065dea Parents: d1c4f60 Author: Guillaume Nodet <gno...@apache.org> Authored: Mon May 28 09:30:59 2018 +0200 Committer: Guillaume Nodet <gno...@apache.org> Committed: Mon May 28 10:06:47 2018 +0200 ---------------------------------------------------------------------- .../server/command/AbstractCommandSupport.java | 78 ++++++++-- .../AbstractDelegatingCommandFactory.java | 12 +- .../command/AbstractFileSystemCommand.java | 69 +++++++++ .../command/DelegatingCommandFactory.java | 43 ------ .../sshd/client/session/ClientSessionTest.java | 10 +- .../org/apache/sshd/git/AbstractGitCommand.java | 54 +------ .../org/apache/sshd/server/scp/ScpCommand.java | 146 ++----------------- .../org/apache/sshd/client/scp/ScpTest.java | 23 +-- 8 files changed, 164 insertions(+), 271 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java index 58cf034..e797456 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractCommandSupport.java @@ -26,12 +26,17 @@ import java.util.Collection; import java.util.concurrent.ExecutorService; import java.util.concurrent.Future; +import org.apache.sshd.common.session.Session; +import org.apache.sshd.common.session.SessionHolder; import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.logging.AbstractLoggingBean; import org.apache.sshd.common.util.threads.ExecutorServiceCarrier; import org.apache.sshd.common.util.threads.ThreadUtils; import org.apache.sshd.server.Environment; import org.apache.sshd.server.ExitCallback; +import org.apache.sshd.server.SessionAware; +import org.apache.sshd.server.session.ServerSession; +import org.apache.sshd.server.session.ServerSessionHolder; /** * Provides a basic useful skeleton for {@link Command} executions @@ -40,17 +45,20 @@ import org.apache.sshd.server.ExitCallback; */ public abstract class AbstractCommandSupport extends AbstractLoggingBean - implements Command, Runnable, ExitCallback, ExecutorServiceCarrier { - private final String command; - private InputStream in; - private OutputStream out; - private OutputStream err; - private ExitCallback callback; - private Environment environment; - private Future<?> cmdFuture; - private ExecutorService executorService; - private boolean shutdownOnExit; - private boolean cbCalled; + implements Command, Runnable, ExecutorServiceCarrier, SessionAware, + SessionHolder<Session>, ServerSessionHolder { + protected final String command; + protected InputStream in; + protected OutputStream out; + protected OutputStream err; + protected ExitCallback callback; + protected Environment environment; + protected Future<?> cmdFuture; + protected Thread cmdRunner; + protected ExecutorService executorService; + protected boolean shutdownOnExit; + protected boolean cbCalled; + protected ServerSession serverSession; protected AbstractCommandSupport(String command, ExecutorService executorService, boolean shutdownOnExit) { this.command = command; @@ -70,6 +78,21 @@ public abstract class AbstractCommandSupport } @Override + public Session getSession() { + return getServerSession(); + } + + @Override + public ServerSession getServerSession() { + return serverSession; + } + + @Override + public void setSession(ServerSession session) { + serverSession = session; + } + + @Override public ExecutorService getExecutorService() { return executorService; } @@ -126,24 +149,47 @@ public abstract class AbstractCommandSupport @Override public void start(Environment env) throws IOException { environment = env; - ExecutorService executors = getExecutorService(); - cmdFuture = executors.submit(this); + try { + ExecutorService executors = getExecutorService(); + cmdFuture = executors.submit(() -> { + cmdRunner = Thread.currentThread(); + this.run(); + }); + } catch (RuntimeException e) { // e.g., RejectedExecutionException + log.error("Failed (" + e.getClass().getSimpleName() + ") to start command=" + command + ": " + e.getMessage(), e); + throw new IOException(e); + } } @Override public void destroy() { + // if thread has not completed, cancel it + boolean debugEnabled = log.isDebugEnabled(); + if ((cmdFuture != null) && (!cmdFuture.isDone()) && (cmdRunner != Thread.currentThread())) { + boolean result = cmdFuture.cancel(true); + // TODO consider waiting some reasonable (?) amount of time for cancellation + if (debugEnabled) { + log.debug("destroy() - cancel pending future=" + result); + } + } + + cmdFuture = null; + ExecutorService executors = getExecutorService(); if ((executors != null) && (!executors.isShutdown()) && isShutdownOnExit()) { Collection<Runnable> runners = executors.shutdownNow(); - if (log.isDebugEnabled()) { + if (debugEnabled) { log.debug("destroy() - shutdown executor service - runners count=" + runners.size()); } } this.executorService = null; } - @Override - public void onExit(int exitValue, String exitMessage) { + protected void onExit(int exitValue) { + onExit(exitValue, ""); + } + + protected void onExit(int exitValue, String exitMessage) { if (cbCalled) { if (log.isTraceEnabled()) { log.trace("onExit({}) ignore exitValue={}, message={} - already called", http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractDelegatingCommandFactory.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractDelegatingCommandFactory.java b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractDelegatingCommandFactory.java index e780c52..6005038 100644 --- a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractDelegatingCommandFactory.java +++ b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractDelegatingCommandFactory.java @@ -27,7 +27,7 @@ import org.apache.sshd.common.util.logging.AbstractLoggingBean; * * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> */ -public abstract class AbstractDelegatingCommandFactory extends AbstractLoggingBean implements DelegatingCommandFactory { +public abstract class AbstractDelegatingCommandFactory extends AbstractLoggingBean implements CommandFactory { private final String name; /* * NOTE: we expose setters since there is no problem to change these settings between @@ -44,12 +44,10 @@ public abstract class AbstractDelegatingCommandFactory extends AbstractLoggingBe return name; } - @Override public CommandFactory getDelegateCommandFactory() { return delegate; } - @Override public void setDelegateCommandFactory(CommandFactory factory) { delegate = factory; } @@ -68,6 +66,14 @@ public abstract class AbstractDelegatingCommandFactory extends AbstractLoggingBe return createUnsupportedCommand(command); } + /** + * @param command The command about to be executed + * @return {@code true} if this command is supported by the command + * factory, {@code false} if it will be passed on to the + * {@link #getDelegateCommandFactory() delegate} factory + */ + public abstract boolean isSupportedCommand(String command); + protected abstract Command executeSupportedCommand(String command); protected Command createUnsupportedCommand(String command) { http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractFileSystemCommand.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractFileSystemCommand.java b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractFileSystemCommand.java new file mode 100644 index 0000000..e4d0f58 --- /dev/null +++ b/sshd-core/src/main/java/org/apache/sshd/server/command/AbstractFileSystemCommand.java @@ -0,0 +1,69 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one + * or more contributor license agreements. See the NOTICE file + * distributed with this work for additional information + * regarding copyright ownership. The ASF licenses this file + * to you under the Apache License, Version 2.0 (the + * "License"); you may not use this file except in compliance + * with the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, + * software distributed under the License is distributed on an + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY + * KIND, either express or implied. See the License for the + * specific language governing permissions and limitations + * under the License. + */ + +package org.apache.sshd.server.command; + +import java.io.IOException; +import java.nio.file.FileSystem; +import java.util.concurrent.ExecutorService; + +import org.apache.sshd.common.file.FileSystemAware; + +/** + * Provides a basic useful skeleton for {@link Command} executions that require file system access + * + * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> + */ +public abstract class AbstractFileSystemCommand extends AbstractCommandSupport implements FileSystemAware { + + protected FileSystem fileSystem; + + public AbstractFileSystemCommand(String command, ExecutorService executorService, boolean shutdownOnExit) { + super(command, executorService, shutdownOnExit); + } + + public FileSystem getFileSystem() { + return fileSystem; + } + + @Override + public void setFileSystem(FileSystem fileSystem) { + this.fileSystem = fileSystem; + } + + @Override + public void destroy() { + try { + super.destroy(); + } finally { + if (fileSystem != null) { + try { + fileSystem.close(); + } catch (UnsupportedOperationException | IOException e) { + if (log.isDebugEnabled()) { + log.debug("destroy({}) - failed ({}) to close file system={}: {}", + this, e.getClass().getSimpleName(), fileSystem, e.getMessage()); + } + } finally { + fileSystem = null; + } + } + } + } +} http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-core/src/main/java/org/apache/sshd/server/command/DelegatingCommandFactory.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/main/java/org/apache/sshd/server/command/DelegatingCommandFactory.java b/sshd-core/src/main/java/org/apache/sshd/server/command/DelegatingCommandFactory.java deleted file mode 100644 index e1ce89c..0000000 --- a/sshd-core/src/main/java/org/apache/sshd/server/command/DelegatingCommandFactory.java +++ /dev/null @@ -1,43 +0,0 @@ -/* - * Licensed to the Apache Software Foundation (ASF) under one - * or more contributor license agreements. See the NOTICE file - * distributed with this work for additional information - * regarding copyright ownership. The ASF licenses this file - * to you under the Apache License, Version 2.0 (the - * "License"); you may not use this file except in compliance - * with the License. You may obtain a copy of the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, - * software distributed under the License is distributed on an - * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY - * KIND, either express or implied. See the License for the - * specific language governing permissions and limitations - * under the License. - */ - -package org.apache.sshd.server.command; - -/** - * Represents a {@link CommandFactory} that filters the commands it recognizes - * and delegates the ones it doesn't to another delegate factory. The behavior - * of such a delegating factory is undefined if it receives a command it does - * not recognize and not delegate has been set. The recommended behavior in this - * case is to throw some exception - though this is not mandatory - * - * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> - */ -public interface DelegatingCommandFactory extends CommandFactory { - CommandFactory getDelegateCommandFactory(); - - void setDelegateCommandFactory(CommandFactory factory); - - /** - * @param command The command about to be executed - * @return {@code true} if this command is supported by the command - * factory, {@code false} if it will be passed on to the - * {@link #getDelegateCommandFactory() delegate} factory - */ - boolean isSupportedCommand(String command); -} http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java ---------------------------------------------------------------------- diff --git a/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java b/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java index 03acaa4..87dc5ff 100644 --- a/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java +++ b/sshd-core/src/test/java/org/apache/sshd/client/session/ClientSessionTest.java @@ -153,13 +153,13 @@ public class ClientSessionTest extends BaseTestSupport { @Test public void testExceptionThrownIfNonZeroExitStatus() throws Exception { final String expectedCommand = getCurrentTestName() + "-CMD"; - final int exepectedErrorCode = 7365; - sshd.setCommandFactory(command -> new CommandExecutionHelper() { + final int expectedErrorCode = 7365; + sshd.setCommandFactory(command -> new CommandExecutionHelper(command) { private boolean cmdProcessed; @Override - public void onExit(int exitValue, String exitMessage) { - super.onExit((exitValue == 0) ? exepectedErrorCode : exitValue, exitMessage); + protected void onExit(int exitValue, String exitMessage) { + super.onExit((exitValue == 0) ? expectedErrorCode : exitValue, exitMessage); } @Override @@ -195,6 +195,6 @@ public class ClientSessionTest extends BaseTestSupport { actualErrorMessage = cause.getMessage(); } - assertEquals("Mismatched captured error code", Integer.toString(exepectedErrorCode), actualErrorMessage); + assertEquals("Mismatched captured error code", Integer.toString(expectedErrorCode), actualErrorMessage); } } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java ---------------------------------------------------------------------- diff --git a/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java b/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java index 7315610..1542e75 100644 --- a/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java +++ b/sshd-git/src/main/java/org/apache/sshd/git/AbstractGitCommand.java @@ -19,7 +19,6 @@ package org.apache.sshd.git; -import java.io.IOException; import java.io.OutputStream; import java.nio.file.FileSystem; import java.util.ArrayList; @@ -28,11 +27,7 @@ import java.util.Objects; import java.util.concurrent.ExecutorService; import org.apache.sshd.common.channel.ChannelOutputStream; -import org.apache.sshd.common.file.FileSystemAware; -import org.apache.sshd.server.SessionAware; -import org.apache.sshd.server.command.AbstractCommandSupport; -import org.apache.sshd.server.session.ServerSession; -import org.apache.sshd.server.session.ServerSessionHolder; +import org.apache.sshd.server.command.AbstractFileSystemCommand; /** * Provides basic support for GIT command implementations @@ -40,16 +35,15 @@ import org.apache.sshd.server.session.ServerSessionHolder; * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> */ public abstract class AbstractGitCommand - extends AbstractCommandSupport - implements SessionAware, FileSystemAware, ServerSessionHolder, GitLocationResolverCarrier { + extends AbstractFileSystemCommand + implements GitLocationResolverCarrier { public static final int CHAR = 0x001; public static final int DELIMITER = 0x002; public static final int STARTQUOTE = 0x004; public static final int ENDQUOTE = 0x008; - private final GitLocationResolver rootDirResolver; - private FileSystem fileSystem; - private ServerSession session; + protected final GitLocationResolver rootDirResolver; + protected FileSystem fileSystem; protected AbstractGitCommand(GitLocationResolver rootDirResolver, String command, ExecutorService executorService, boolean shutdownOnExit) { super(command, executorService, shutdownOnExit); @@ -61,25 +55,6 @@ public abstract class AbstractGitCommand return rootDirResolver; } - public FileSystem getFileSystem() { - return fileSystem; - } - - @Override - public void setFileSystem(FileSystem fileSystem) { - this.fileSystem = fileSystem; - } - - @Override - public ServerSession getServerSession() { - return session; - } - - @Override - public void setSession(ServerSession session) { - this.session = session; - } - @Override public void setOutputStream(OutputStream out) { super.setOutputStream(out); @@ -97,25 +72,6 @@ public abstract class AbstractGitCommand } @Override - public void destroy() { - try { - super.destroy(); - } finally { - FileSystem fs = getFileSystem(); - if (fs != null) { - try { - fs.close(); - } catch (UnsupportedOperationException | IOException e) { - if (log.isDebugEnabled()) { - log.debug("destroy({}) - failed ({}) to close file system={}: {}", - this, e.getClass().getSimpleName(), fs, e.getMessage()); - } - } - } - } - } - - @Override public String toString() { return super.toString() + "[session=" + getServerSession() + "]"; } http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommand.java ---------------------------------------------------------------------- diff --git a/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommand.java b/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommand.java index 7729859..3bb4453 100644 --- a/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommand.java +++ b/sshd-scp/src/main/java/org/apache/sshd/server/scp/ScpCommand.java @@ -19,32 +19,19 @@ package org.apache.sshd.server.scp; import java.io.IOException; -import java.io.InputStream; -import java.io.OutputStream; -import java.nio.file.FileSystem; -import java.util.Collection; import java.util.Collections; import java.util.concurrent.ExecutorService; -import java.util.concurrent.Future; -import org.apache.sshd.common.file.FileSystemAware; import org.apache.sshd.common.scp.ScpException; import org.apache.sshd.common.scp.ScpFileOpener; import org.apache.sshd.common.scp.ScpHelper; import org.apache.sshd.common.scp.ScpTransferEventListener; import org.apache.sshd.common.scp.helpers.DefaultScpFileOpener; -import org.apache.sshd.common.session.Session; -import org.apache.sshd.common.session.SessionHolder; import org.apache.sshd.common.util.GenericUtils; -import org.apache.sshd.common.util.logging.AbstractLoggingBean; -import org.apache.sshd.common.util.threads.ExecutorServiceCarrier; import org.apache.sshd.common.util.threads.ThreadUtils; import org.apache.sshd.server.Environment; -import org.apache.sshd.server.ExitCallback; -import org.apache.sshd.server.SessionAware; -import org.apache.sshd.server.command.Command; +import org.apache.sshd.server.command.AbstractFileSystemCommand; import org.apache.sshd.server.session.ServerSession; -import org.apache.sshd.server.session.ServerSessionHolder; /** * This commands provide SCP support on both server and client side. @@ -54,11 +41,8 @@ import org.apache.sshd.server.session.ServerSessionHolder; * @author <a href="mailto:d...@mina.apache.org">Apache MINA SSHD Project</a> */ public class ScpCommand - extends AbstractLoggingBean - implements Command, Runnable, FileSystemAware, SessionAware, - SessionHolder<Session>, ServerSessionHolder, ExecutorServiceCarrier { + extends AbstractFileSystemCommand { - protected final String name; protected final int sendBufferSize; protected final int receiveBufferSize; protected final ScpFileOpener opener; @@ -67,19 +51,9 @@ public class ScpCommand protected boolean optF; protected boolean optD; protected boolean optP; // TODO: handle modification times - protected FileSystem fileSystem; protected String path; - protected InputStream in; - protected OutputStream out; - protected OutputStream err; - protected ExitCallback callback; protected IOException error; - protected Future<?> pendingFuture; protected ScpTransferEventListener listener; - protected ServerSession serverSession; - - private ExecutorService executorService; - private boolean shutdownOnExit; /** * @param command The command to be executed @@ -101,16 +75,7 @@ public class ScpCommand ExecutorService executorService, boolean shutdownOnExit, int sendSize, int receiveSize, ScpFileOpener fileOpener, ScpTransferEventListener eventListener) { - name = command; - - if (executorService == null) { - String poolName = command.replace(' ', '_').replace('/', ':'); - this.executorService = ThreadUtils.newSingleThreadExecutor(poolName); - this.shutdownOnExit = true; // we always close the ad-hoc executor service - } else { - this.executorService = executorService; - this.shutdownOnExit = shutdownOnExit; - } + super(command, executorService, shutdownOnExit); if (sendSize < ScpHelper.MIN_SEND_BUFFER_SIZE) { throw new IllegalArgumentException("<ScpCommmand>(" + command + ") send buffer size " @@ -184,100 +149,11 @@ public class ScpCommand } @Override - public ExecutorService getExecutorService() { - return executorService; - } - - @Override - public boolean isShutdownOnExit() { - return shutdownOnExit; - } - - @Override - public Session getSession() { - return getServerSession(); - } - - @Override - public ServerSession getServerSession() { - return serverSession; - } - - @Override - public void setSession(ServerSession session) { - serverSession = session; - } - - @Override - public void setInputStream(InputStream in) { - this.in = in; - } - - @Override - public void setOutputStream(OutputStream out) { - this.out = out; - } - - @Override - public void setErrorStream(OutputStream err) { - this.err = err; - } - - @Override - public void setExitCallback(ExitCallback callback) { - this.callback = callback; - } - - @Override - public void setFileSystem(FileSystem fs) { - this.fileSystem = fs; - } - - @Override public void start(Environment env) throws IOException { if (error != null) { throw error; } - - try { - ExecutorService executors = getExecutorService(); - pendingFuture = executors.submit(this); - } catch (RuntimeException e) { // e.g., RejectedExecutionException - log.error("Failed (" + e.getClass().getSimpleName() + ") to start command=" + name + ": " + e.getMessage(), e); - throw new IOException(e); - } - } - - @Override - public void destroy() { - // if thread has not completed, cancel it - boolean debugEnabled = log.isDebugEnabled(); - if ((pendingFuture != null) && (!pendingFuture.isDone())) { - boolean result = pendingFuture.cancel(true); - // TODO consider waiting some reasonable (?) amount of time for cancellation - if (debugEnabled) { - log.debug("destroy() - cancel pending future=" + result); - } - } - - pendingFuture = null; - - ExecutorService executors = getExecutorService(); - if ((executors != null) && (!executors.isShutdown()) && isShutdownOnExit()) { - Collection<Runnable> runners = executors.shutdownNow(); - if (debugEnabled) { - log.debug("destroy() - shutdown executor service - runners count=" + runners.size()); - } - } - this.executorService = null; - - try { - fileSystem.close(); - } catch (UnsupportedOperationException e) { - // Ignore - } catch (IOException e) { - log.debug("Error closing FileSystem", e); - } + super.start(env); } @Override @@ -305,28 +181,28 @@ public class ScpCommand // this is an exception so status cannot be OK/WARNING if ((exitValue == ScpHelper.OK) || (exitValue == ScpHelper.WARNING)) { if (debugEnabled) { - log.debug("run({})[{}] normalize status code={}", session, name, exitValue); + log.debug("run({})[{}] normalize status code={}", session, command, exitValue); } exitValue = ScpHelper.ERROR; } exitMessage = GenericUtils.trimToEmpty(e.getMessage()); - writeCommandResponseMessage(name, exitValue, exitMessage); + writeCommandResponseMessage(command, exitValue, exitMessage); } catch (IOException e2) { if (debugEnabled) { log.debug("run({})[{}] Failed ({}) to send error response: {}", - session, name, e.getClass().getSimpleName(), e.getMessage()); + session, command, e.getClass().getSimpleName(), e.getMessage()); } if (log.isTraceEnabled()) { - log.trace("run(" + session + ")[" + name + "] error response failure details", e2); + log.trace("run(" + session + ")[" + command + "] error response failure details", e2); } } if (debugEnabled) { log.debug("run({})[{}] Failed ({}) to run command: {}", - session, name, e.getClass().getSimpleName(), e.getMessage()); + session, command, e.getClass().getSimpleName(), e.getMessage()); } if (log.isTraceEnabled()) { - log.trace("run(" + session + ")[" + name + "] command execution failure details", e); + log.trace("run(" + session + ")[" + command + "] command execution failure details", e); } } finally { if (callback != null) { @@ -345,6 +221,6 @@ public class ScpCommand @Override public String toString() { - return getClass().getSimpleName() + "(" + getSession() + ") " + name; + return getClass().getSimpleName() + "(" + getSession() + ") " + command; } } \ No newline at end of file http://git-wip-us.apache.org/repos/asf/mina-sshd/blob/a04f585d/sshd-scp/src/test/java/org/apache/sshd/client/scp/ScpTest.java ---------------------------------------------------------------------- diff --git a/sshd-scp/src/test/java/org/apache/sshd/client/scp/ScpTest.java b/sshd-scp/src/test/java/org/apache/sshd/client/scp/ScpTest.java index 9db3587..3cc15b7 100644 --- a/sshd-scp/src/test/java/org/apache/sshd/client/scp/ScpTest.java +++ b/sshd-scp/src/test/java/org/apache/sshd/client/scp/ScpTest.java @@ -60,7 +60,6 @@ import org.apache.sshd.common.util.GenericUtils; import org.apache.sshd.common.util.OsUtils; import org.apache.sshd.common.util.ValidateUtils; import org.apache.sshd.common.util.io.IoUtils; -import org.apache.sshd.server.ExitCallback; import org.apache.sshd.server.SshServer; import org.apache.sshd.server.command.Command; import org.apache.sshd.server.scp.ScpCommand; @@ -767,8 +766,7 @@ public class ScpTest extends BaseTestSupport { @Test // see SSHD-628 public void testScpExitStatusPropagation() throws Exception { final int testExitValue = 7365; - class InternalScpCommand extends ScpCommand implements ExitCallback { - private ExitCallback delegate; + class InternalScpCommand extends ScpCommand { InternalScpCommand(String command, ExecutorService executorService, boolean shutdownOnExit, int sendSize, int receiveSize, ScpFileOpener opener, ScpTransferEventListener eventListener) { @@ -782,24 +780,9 @@ public class ScpTest extends BaseTestSupport { } @Override - public void setExitCallback(ExitCallback callback) { - delegate = callback; - super.setExitCallback(this); - } - - @Override - public void onExit(int exitValue) { - onExit(exitValue, Integer.toString(exitValue)); - } - - @Override - public void onExit(int exitValue, String exitMessage) { + protected void onExit(int exitValue, String exitMessage) { outputDebugMessage("onExit(%s) status=%d", this, exitValue); - if (exitValue == ScpHelper.OK) { - delegate.onExit(testExitValue, exitMessage); - } else { - delegate.onExit(exitValue, exitMessage); - } + super.onExit((exitValue == ScpHelper.OK) ? testExitValue : exitValue, exitMessage); } }