Refactor SSH commands with SshCommand base class
The SshCommand base class extends BaseCommand and provides the common pattern of calling startThread with a CommandRunnable that parses argments and then invokes the real logic. The new annotation @RequiresCapability can be declared on any concrete implementation of SshCommand to name a capability the caller must have before they can run the contained command. This is enforced inside of the DispatchCommand, which is what handles creating and delegating to any paticular command implementation. @AdminCommand is replaced by the new @RequriesCapability annotation, which is more explicitly declaring the administrate server dependency. Most existing commands have been ported to this SshCommand base class, cleaning up a lot of the code. A few special cases still exist such as StreamEvents or ScpCommand that are not trivial to port, as their start implementation is well outside of the common pattern. Plugin authors should be encouraged to extend from SshCommand for the foreseeable future. I eventually would like to get away from needing to extend this class, and instead use a simpler interface declaration, but that is a much bigger change to make with how DispatchCommand and BaseCommand are connected together. Change-Id: I4f1de60c6fdeb207197dfccc135b4d532443d5b2
This commit is contained in:
		@@ -14,21 +14,22 @@
 | 
			
		||||
 | 
			
		||||
package com.google.gerrit.sshd.commands;
 | 
			
		||||
 | 
			
		||||
import com.google.gerrit.common.data.GlobalCapability;
 | 
			
		||||
import com.google.gerrit.server.IdentifiedUser;
 | 
			
		||||
import com.google.gerrit.sshd.BaseCommand;
 | 
			
		||||
import com.google.gerrit.sshd.RequiresCapability;
 | 
			
		||||
import com.google.inject.Inject;
 | 
			
		||||
 | 
			
		||||
import net.sf.ehcache.Ehcache;
 | 
			
		||||
 | 
			
		||||
import org.apache.sshd.server.Environment;
 | 
			
		||||
import org.kohsuke.args4j.Option;
 | 
			
		||||
 | 
			
		||||
import java.io.PrintWriter;
 | 
			
		||||
import java.util.ArrayList;
 | 
			
		||||
import java.util.List;
 | 
			
		||||
import java.util.SortedSet;
 | 
			
		||||
 | 
			
		||||
/** Causes the caches to purge all entries and reload. */
 | 
			
		||||
@RequiresCapability(GlobalCapability.FLUSH_CACHES)
 | 
			
		||||
final class FlushCaches extends CacheCommand {
 | 
			
		||||
  private static final String WEB_SESSIONS = "web_sessions";
 | 
			
		||||
 | 
			
		||||
@@ -44,27 +45,8 @@ final class FlushCaches extends CacheCommand {
 | 
			
		||||
  @Inject
 | 
			
		||||
  IdentifiedUser currentUser;
 | 
			
		||||
 | 
			
		||||
  private PrintWriter p;
 | 
			
		||||
 | 
			
		||||
  @Override
 | 
			
		||||
  public void start(final Environment env) {
 | 
			
		||||
    startThread(new CommandRunnable() {
 | 
			
		||||
      @Override
 | 
			
		||||
      public void run() throws Exception {
 | 
			
		||||
        if (!currentUser.getCapabilities().canFlushCaches()) {
 | 
			
		||||
          String msg = String.format(
 | 
			
		||||
            "fatal: %s does not have \"Flush Caches\" capability.",
 | 
			
		||||
            currentUser.getUserName());
 | 
			
		||||
          throw new UnloggedFailure(BaseCommand.STATUS_NOT_ADMIN, msg);
 | 
			
		||||
        }
 | 
			
		||||
 | 
			
		||||
        parseCommandLine();
 | 
			
		||||
        flush();
 | 
			
		||||
      }
 | 
			
		||||
    });
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private void flush() throws Failure {
 | 
			
		||||
  protected void run() throws Failure {
 | 
			
		||||
    if (caches.contains(WEB_SESSIONS)
 | 
			
		||||
        && !currentUser.getCapabilities().canAdministrateServer()) {
 | 
			
		||||
      String msg = String.format(
 | 
			
		||||
@@ -73,7 +55,6 @@ final class FlushCaches extends CacheCommand {
 | 
			
		||||
      throw new UnloggedFailure(BaseCommand.STATUS_NOT_ADMIN, msg);
 | 
			
		||||
    }
 | 
			
		||||
 | 
			
		||||
    p = toPrintWriter(err);
 | 
			
		||||
    if (list) {
 | 
			
		||||
      if (all || caches.size() > 0) {
 | 
			
		||||
        throw error("error: cannot use --list with --all or --cache");
 | 
			
		||||
@@ -106,10 +87,10 @@ final class FlushCaches extends CacheCommand {
 | 
			
		||||
 | 
			
		||||
  private void doList() {
 | 
			
		||||
    for (final String name : cacheNames()) {
 | 
			
		||||
      p.print(name);
 | 
			
		||||
      p.print('\n');
 | 
			
		||||
      stderr.print(name);
 | 
			
		||||
      stderr.print('\n');
 | 
			
		||||
    }
 | 
			
		||||
    p.flush();
 | 
			
		||||
    stderr.flush();
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
  private void doBulkFlush() {
 | 
			
		||||
@@ -120,12 +101,12 @@ final class FlushCaches extends CacheCommand {
 | 
			
		||||
          try {
 | 
			
		||||
            c.removeAll();
 | 
			
		||||
          } catch (Throwable e) {
 | 
			
		||||
            p.println("error: cannot flush cache \"" + name + "\": " + e);
 | 
			
		||||
            stderr.println("error: cannot flush cache \"" + name + "\": " + e);
 | 
			
		||||
          }
 | 
			
		||||
        }
 | 
			
		||||
      }
 | 
			
		||||
    } finally {
 | 
			
		||||
      p.flush();
 | 
			
		||||
      stderr.flush();
 | 
			
		||||
    }
 | 
			
		||||
  }
 | 
			
		||||
 | 
			
		||||
 
 | 
			
		||||
		Reference in New Issue
	
	Block a user