Merge "Allow to hide download schemes in the UI"
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java b/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java
index c21e31c..76d4166 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHook.java
@@ -14,110 +14,99 @@
 
 package com.googlesource.gerrit.plugins.download.command;
 
+import com.google.common.annotations.VisibleForTesting;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.config.DownloadScheme;
-import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.config.CanonicalWebUrl;
 import com.google.gerrit.server.config.GerritServerConfig;
 import com.google.inject.Inject;
 import com.google.inject.Provider;
 import com.googlesource.gerrit.plugins.download.scheme.AnonymousHttpScheme;
 import com.googlesource.gerrit.plugins.download.scheme.HttpScheme;
 import com.googlesource.gerrit.plugins.download.scheme.SshScheme;
-import java.util.Optional;
 import org.eclipse.jgit.lib.Config;
 
 public class CloneWithCommitMsgHook extends CloneCommand {
-  private static final String HOOK = "hooks/commit-msg";
-  private static final String TARGET = " `git rev-parse --git-dir`/";
+  @VisibleForTesting protected static final String HOOK_COMMAND_KEY = "installCommitMsgHookCommand";
+  @VisibleForTesting protected static final String HOOKS_DIR = "`git rev-parse --git-dir`/hooks/";
+  @VisibleForTesting protected static final String EXTRA_COMMAND_KEY = "installCommitExtraCommand";
+
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+  private static final String HOOK = "commit-msg";
 
   private final String configCommand;
   private final String extraCommand;
-  private final SshScheme sshScheme;
-  private final Provider<CurrentUser> userProvider;
+  private final String canonicalWebUrl;
 
   @Inject
   CloneWithCommitMsgHook(
-      @GerritServerConfig Config config, SshScheme sshScheme, Provider<CurrentUser> userProvider) {
+      @GerritServerConfig Config config, @CanonicalWebUrl @Nullable Provider<String> urlProvider) {
     this.configCommand = config.getString("gerrit", null, "installCommitMsgHookCommand");
     this.extraCommand = config.getString("gerrit", null, "installCommitExtraCommand");
-    this.sshScheme = sshScheme;
-    this.userProvider = userProvider;
+    this.canonicalWebUrl = urlProvider != null ? urlProvider.get() : null;
   }
 
+  @Nullable
   @Override
   public String getCommand(DownloadScheme scheme, String project) {
-    Optional<String> username = userProvider.get().getUserName();
-    if (!username.isPresent()) {
-      return null;
-    }
     String projectName = getBaseName(project);
+    StringBuilder command = null;
 
     if (configCommand != null) {
-      return new StringBuilder()
-          .append(super.getCommand(scheme, project))
+      command =
+          new StringBuilder()
+              .append(super.getCommand(scheme, project))
+              .append(" && (cd ")
+              .append(QuoteUtil.quote(projectName))
+              .append(" && ")
+              .append(configCommand)
+              .append(")");
+    } else if (scheme instanceof HttpScheme
+        || scheme instanceof AnonymousHttpScheme
+        || scheme instanceof SshScheme) {
+      command =
+          new StringBuilder()
+              .append(super.getCommand(scheme, project))
+              .append(" && (cd ")
+              .append(QuoteUtil.quote(projectName))
+              .append(" && mkdir -p ")
+              .append(HOOKS_DIR)
+              .append(" && curl -Lo ")
+              .append(HOOKS_DIR)
+              .append(HOOK)
+              .append(" ")
+              .append(getHookUrl())
+              .append("; chmod +x ")
+              .append(HOOKS_DIR)
+              .append(HOOK)
+              .append(")");
+    }
+
+    if (extraCommand != null && command != null) {
+      command
           .append(" && (cd ")
           .append(QuoteUtil.quote(projectName))
           .append(" && ")
-          .append(configCommand)
-          .append(")")
-          .toString();
+          .append(extraCommand)
+          .append(")");
     }
-
-    if (scheme instanceof SshScheme) {
-      StringBuilder b =
-          new StringBuilder().append(super.getCommand(scheme, project)).append(" && scp -p");
-
-      if (sshScheme.getSshdPort() != 22) {
-        b.append(" -P ").append(sshScheme.getSshdPort());
-      }
-
-      b.append(" ")
-          .append(username.get())
-          .append("@")
-          .append(sshScheme.getSshdHost())
-          .append(":")
-          .append(HOOK)
-          .append(" ")
-          .append(QuoteUtil.quote(projectName + "/.git/hooks/"));
-      if (extraCommand != null) {
-        b.append(" && (cd ")
-            .append(QuoteUtil.quote(projectName))
-            .append(" && ")
-            .append(extraCommand)
-            .append(")");
-      }
-      return b.toString();
-    }
-
-    if (scheme instanceof HttpScheme || scheme instanceof AnonymousHttpScheme) {
-      return new StringBuilder()
-          .append(super.getCommand(scheme, project))
-          .append(" && (cd ")
-          .append(QuoteUtil.quote(projectName))
-          .append(" && mkdir -p .git/hooks")
-          .append(" && curl -Lo")
-          .append(TARGET)
-          .append(HOOK)
-          .append(" ")
-          .append(getHttpHost(scheme, project))
-          .append("tools/")
-          .append(HOOK)
-          .append("; chmod +x")
-          .append(TARGET)
-          .append(HOOK)
-          .append(")")
-          .toString();
-    }
-    return null;
+    return command != null ? command.toString() : null;
   }
 
-  private String getHttpHost(DownloadScheme scheme, String project) {
-    String host = scheme.getUrl(project);
-    host = host.substring(0, host.lastIndexOf(project));
-    int auth = host.lastIndexOf("/a/");
-    if (auth > -1) {
-      host = host.substring(0, auth + 1);
+  private StringBuilder getHookUrl() {
+    StringBuilder hookUrl = new StringBuilder();
+    if (canonicalWebUrl != null) {
+      hookUrl.append(canonicalWebUrl);
+      if (!canonicalWebUrl.endsWith("/")) {
+        hookUrl.append("/");
+      }
+      hookUrl.append("tools/hooks/").append(HOOK);
+    } else {
+      logger.atWarning().log(
+          "Cannot add commit-msg hook URL since gerrit.canonicalWebUrl isn't configured.");
     }
-    return host;
+    return hookUrl;
   }
 
   private static String getBaseName(String project) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/command/GitDownloadCommand.java b/src/main/java/com/googlesource/gerrit/plugins/download/command/GitDownloadCommand.java
index e106510..d258ec2 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/command/GitDownloadCommand.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/command/GitDownloadCommand.java
@@ -15,6 +15,7 @@
 package com.googlesource.gerrit.plugins.download.command;
 
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Change;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
@@ -59,6 +60,7 @@
         cfg.getBoolean(DOWNLOAD, KEY_CHECK_FOR_HIDDEN_CHANGE_REFS, false);
   }
 
+  @Nullable
   @Override
   public final String getCommand(DownloadScheme scheme, String project, String ref) {
     if (commandAllowed) {
@@ -97,6 +99,7 @@
     }
   }
 
+  @Nullable
   private String resolveRef(String project, String ref) {
     if (project.startsWith("$") || ref.startsWith("$")) {
       // No real value but placeholders are being used.
@@ -139,6 +142,7 @@
    * @param url The project URL this change is for.
    * @param id The change/PS numbers.
    */
+  @Nullable
   String getRepoCommand(String url, String id) {
     // Most commands don't support this, so default it to nothing.
     return null;
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/AnonymousHttpScheme.java b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/AnonymousHttpScheme.java
index 061103d..f18998a 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/AnonymousHttpScheme.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/AnonymousHttpScheme.java
@@ -43,6 +43,7 @@
     this.schemeHidden = downloadConfig.getHiddenSchemes().contains(ANON_HTTP);
   }
 
+  @Nullable
   @Override
   public String getUrl(String project) {
     if (!isEnabled()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java
index 1ef9d4d..923470d 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/HttpScheme.java
@@ -50,6 +50,7 @@
     this.schemeHidden = downloadConfig.getHiddenSchemes().contains(HTTP);
   }
 
+  @Nullable
   @Override
   public String getUrl(String project) {
     if (!isEnabled() || !userProvider.get().isIdentifiedUser()) {
diff --git a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java
index ba48109..4628f73 100644
--- a/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java
+++ b/src/main/java/com/googlesource/gerrit/plugins/download/scheme/SshScheme.java
@@ -16,6 +16,7 @@
 
 import static com.google.gerrit.entities.CoreDownloadSchemes.SSH;
 
+import com.google.common.annotations.VisibleForTesting;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.extensions.config.DownloadScheme;
 import com.google.gerrit.server.CurrentUser;
@@ -41,7 +42,8 @@
   private final boolean schemeHidden;
 
   @Inject
-  SshScheme(
+  @VisibleForTesting
+  public SshScheme(
       @SshAdvertisedAddresses List<String> sshAddresses,
       @CanonicalWebUrl @Nullable Provider<String> urlProvider,
       Provider<CurrentUser> userProvider,
@@ -84,6 +86,7 @@
     this.schemeHidden = downloadConfig.getHiddenSchemes().contains(SSH);
   }
 
+  @Nullable
   @Override
   public String getUrl(String project) {
     if (!isEnabled() || !userProvider.get().isIdentifiedUser()) {
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
new file mode 100644
index 0000000..bf7a49d
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/DownloadCommandTest.java
@@ -0,0 +1,105 @@
+// Copyright (C) 2023 The Android Open Source Project
+//
+// Licensed 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 com.googlesource.gerrit.plugins.download;
+
+import com.google.common.collect.ImmutableList;
+import com.google.gerrit.entities.Account;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.account.GroupMembership;
+import com.google.gerrit.server.config.DownloadConfig;
+import com.google.inject.Provider;
+import com.google.inject.util.Providers;
+import com.googlesource.gerrit.plugins.download.scheme.HttpScheme;
+import com.googlesource.gerrit.plugins.download.scheme.SshScheme;
+import java.io.UnsupportedEncodingException;
+import java.net.URLEncoder;
+import java.nio.charset.StandardCharsets;
+import java.util.Optional;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Before;
+import org.junit.Ignore;
+
+@Ignore
+public class DownloadCommandTest {
+
+  static CurrentUser fakeUser() {
+    return new CurrentUser() {
+      @Override
+      public Optional<String> getUserName() {
+        return Optional.of(ENV.userName);
+      }
+
+      @Override
+      public GroupMembership getEffectiveGroups() {
+        throw new UnsupportedOperationException("not implemented");
+      }
+
+      @Override
+      public Object getCacheKey() {
+        return new Object();
+      }
+
+      @Override
+      public boolean isIdentifiedUser() {
+        return true;
+      }
+
+      @Override
+      public Account.Id getAccountId() {
+        return Account.id(1);
+      }
+    };
+  }
+
+  public static class TestEnvironment {
+    public final String fqdn = "gerrit.company.com";
+    public final String projectName = "my/project";
+    public final String userName = "john-doe@company.com";
+    public final int sshPort = 29418;
+
+    public String urlEncodedUserName() throws UnsupportedEncodingException {
+      return URLEncoder.encode(userName, StandardCharsets.UTF_8.name());
+    }
+
+    public String canonicalUrl() {
+      return "https://" + fqdn + "/";
+    }
+  }
+
+  protected static TestEnvironment ENV = new TestEnvironment();
+
+  protected HttpScheme httpScheme;
+  protected SshScheme sshScheme;
+  protected Provider<String> urlProvider;
+  protected Provider<CurrentUser> userProvider;
+
+  public DownloadCommandTest() {
+    super();
+  }
+
+  @Before
+  public void setUp() {
+    Config cfg = new Config();
+    urlProvider = Providers.of(ENV.canonicalUrl());
+    DownloadConfig downloadConfig = new DownloadConfig(cfg);
+    userProvider = Providers.of(fakeUser());
+    httpScheme = new HttpScheme(cfg, urlProvider, userProvider, downloadConfig);
+    sshScheme =
+        new SshScheme(
+            ImmutableList.of(String.format("%s:%d", ENV.fqdn, ENV.sshPort)),
+            urlProvider,
+            userProvider,
+            downloadConfig);
+  }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
new file mode 100644
index 0000000..d4ca37c
--- /dev/null
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/command/CloneWithCommitMsgHookTest.java
@@ -0,0 +1,130 @@
+// Copyright (C) 2023 The Android Open Source Project
+//
+// Licensed 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 com.googlesource.gerrit.plugins.download.command;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.googlesource.gerrit.plugins.download.command.CloneWithCommitMsgHook.EXTRA_COMMAND_KEY;
+import static com.googlesource.gerrit.plugins.download.command.CloneWithCommitMsgHook.HOOKS_DIR;
+import static com.googlesource.gerrit.plugins.download.command.CloneWithCommitMsgHook.HOOK_COMMAND_KEY;
+
+import com.googlesource.gerrit.plugins.download.DownloadCommandTest;
+import org.eclipse.jgit.lib.Config;
+import org.junit.Test;
+
+public class CloneWithCommitMsgHookTest extends DownloadCommandTest {
+
+  @Test
+  public void testSshNoConfiguredCommands() throws Exception {
+    String command = getCloneCommand(null, null).getCommand(sshScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && %s",
+                sshScheme.getUrl(ENV.projectName), getDefaultHookCommand()));
+  }
+
+  @Test
+  public void testSshConfiguredHookCommand() throws Exception {
+    String hookCommand = "my hook command";
+    String command = getCloneCommand(hookCommand, null).getCommand(sshScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && (cd %s && %s)",
+                sshScheme.getUrl(ENV.projectName), baseName(ENV.projectName), hookCommand));
+  }
+
+  @Test
+  public void testSshConfiguredExtraCommand() throws Exception {
+    String extraCommand = "my extra command";
+    String command = getCloneCommand(extraCommand, null).getCommand(sshScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && (cd %s && %s)",
+                sshScheme.getUrl(ENV.projectName), baseName(ENV.projectName), extraCommand));
+  }
+
+  @Test
+  public void testSshConfiguredHookAndExtraCommand() throws Exception {
+    String hookCommand = "my hook command";
+    String extraCommand = "my extra command";
+    String command =
+        getCloneCommand(hookCommand, extraCommand).getCommand(sshScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && (cd %s && %s) && (cd %s && %s)",
+                sshScheme.getUrl(ENV.projectName),
+                baseName(ENV.projectName),
+                hookCommand,
+                baseName(ENV.projectName),
+                extraCommand));
+  }
+
+  @Test
+  public void testHttpNoConfiguredCommands() throws Exception {
+    String command = getCloneCommand(null, null).getCommand(httpScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && %s",
+                httpScheme.getUrl(ENV.projectName), getDefaultHookCommand()));
+  }
+
+  @Test
+  public void testHttpConfiguredExtraCommand() throws Exception {
+    String extraCommand = "my extra command";
+    String command = getCloneCommand(extraCommand, null).getCommand(httpScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && (cd %s && %s)",
+                httpScheme.getUrl(ENV.projectName), baseName(ENV.projectName), extraCommand));
+  }
+
+  @Test
+  public void testHttpConfiguredHookAndExtraCommand() throws Exception {
+    String hookCommand = "my hook command";
+    String extraCommand = "my extra command";
+    String command =
+        getCloneCommand(hookCommand, extraCommand).getCommand(httpScheme, ENV.projectName);
+    assertThat(command)
+        .isEqualTo(
+            String.format(
+                "git clone \"%s\" && (cd %s && %s) && (cd %s && %s)",
+                httpScheme.getUrl(ENV.projectName),
+                baseName(ENV.projectName),
+                hookCommand,
+                baseName(ENV.projectName),
+                extraCommand));
+  }
+
+  private String baseName(String projectName) {
+    return projectName.substring(projectName.lastIndexOf('/') + 1);
+  }
+
+  private String getDefaultHookCommand() {
+    return String.format(
+        "(cd %s && mkdir -p %s && curl -Lo %scommit-msg https://%s/tools/hooks/commit-msg; chmod +x %scommit-msg)",
+        baseName(ENV.projectName), HOOKS_DIR, HOOKS_DIR, ENV.fqdn, HOOKS_DIR);
+  }
+
+  private CloneCommand getCloneCommand(String hookCommand, String extraCommand) {
+    Config cfg = new Config();
+    cfg.setString("gerrit", null, HOOK_COMMAND_KEY, hookCommand);
+    cfg.setString("gerrit", null, EXTRA_COMMAND_KEY, extraCommand);
+    return new CloneWithCommitMsgHook(cfg, urlProvider);
+  }
+}
diff --git a/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java b/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java
index ad38898..eaae634 100644
--- a/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java
+++ b/src/test/java/com/googlesource/gerrit/plugins/download/scheme/SchemeTest.java
@@ -16,72 +16,24 @@
 
 import static com.google.common.truth.Truth.assertThat;
 
-import com.google.gerrit.entities.Account;
-import com.google.gerrit.server.CurrentUser;
-import com.google.gerrit.server.account.GroupMembership;
-import com.google.gerrit.server.config.DownloadConfig;
-import com.google.inject.Provider;
-import com.google.inject.util.Providers;
-import java.util.List;
-import java.util.Optional;
-import org.eclipse.jgit.lib.Config;
-import org.junit.Before;
+import com.googlesource.gerrit.plugins.download.DownloadCommandTest;
 import org.junit.Test;
 
-public class SchemeTest {
-  private HttpScheme httpScheme;
-  private SshScheme sshScheme;
-
-  @Before
-  public void setUp() {
-    Config cfg = new Config();
-    Provider<String> urlProvider = Providers.of("https://gerrit.company.com/");
-    Provider<CurrentUser> userProvider = Providers.of(fakeUser());
-    DownloadConfig downloadConfig = new DownloadConfig(cfg);
-    httpScheme = new HttpScheme(cfg, urlProvider, userProvider, downloadConfig);
-    sshScheme =
-        new SshScheme(
-            List.of("gerrit.company.com:29418"), urlProvider, userProvider, downloadConfig);
+public class SchemeTest extends DownloadCommandTest {
+  @Test
+  public void ensureHttpSchemeEncodedInUrl() throws Exception {
+    assertThat(httpScheme.getUrl(ENV.projectName))
+        .isEqualTo(
+            String.format(
+                "https://%s@%s/a/%s", ENV.urlEncodedUserName(), ENV.fqdn, ENV.projectName));
   }
 
   @Test
-  public void ensureHttpSchemeEncodedInUrl() {
-    assertThat(httpScheme.getUrl("foo"))
-        .isEqualTo("https://john-doe%40company.com@gerrit.company.com/a/foo");
-  }
-
-  @Test
-  public void ensureSshSchemeEncodedInUrl() {
-    assertThat(sshScheme.getUrl("foo"))
-        .isEqualTo("ssh://john-doe%40company.com@gerrit.company.com:29418/foo");
-  }
-
-  private static CurrentUser fakeUser() {
-    return new CurrentUser() {
-      @Override
-      public Optional<String> getUserName() {
-        return Optional.of("john-doe@company.com");
-      }
-
-      @Override
-      public GroupMembership getEffectiveGroups() {
-        throw new UnsupportedOperationException("not implemented");
-      }
-
-      @Override
-      public Object getCacheKey() {
-        return new Object();
-      }
-
-      @Override
-      public boolean isIdentifiedUser() {
-        return true;
-      }
-
-      @Override
-      public Account.Id getAccountId() {
-        return Account.id(1);
-      }
-    };
+  public void ensureSshSchemeEncodedInUrl() throws Exception {
+    assertThat(sshScheme.getUrl(ENV.projectName))
+        .isEqualTo(
+            String.format(
+                "ssh://%s@%s:%d/%s",
+                ENV.urlEncodedUserName(), ENV.fqdn, ENV.sshPort, ENV.projectName));
   }
 }