Merge "RetryableAction.ActionType: Add javadoc"
diff --git a/java/com/google/gerrit/common/data/GarbageCollectionResult.java b/java/com/google/gerrit/common/data/GarbageCollectionResult.java
index 5ed0158..5e3601e 100644
--- a/java/com/google/gerrit/common/data/GarbageCollectionResult.java
+++ b/java/com/google/gerrit/common/data/GarbageCollectionResult.java
@@ -18,6 +18,7 @@
 import java.util.ArrayList;
 import java.util.List;
 
+/** A list of errors occurred during GC. */
 public class GarbageCollectionResult {
   protected List<Error> errors;
 
diff --git a/java/com/google/gerrit/extensions/api/projects/BanCommitInput.java b/java/com/google/gerrit/extensions/api/projects/BanCommitInput.java
index b0f674f..b24eca0 100644
--- a/java/com/google/gerrit/extensions/api/projects/BanCommitInput.java
+++ b/java/com/google/gerrit/extensions/api/projects/BanCommitInput.java
@@ -17,6 +17,7 @@
 import com.google.common.collect.Lists;
 import java.util.List;
 
+/** Commits that will forbidden to be uploaded. */
 public class BanCommitInput {
   public List<String> commits;
   public String reason;
diff --git a/java/com/google/gerrit/pgm/init/InitSshd.java b/java/com/google/gerrit/pgm/init/InitSshd.java
index 68bdefc..c0adbe7 100644
--- a/java/com/google/gerrit/pgm/init/InitSshd.java
+++ b/java/com/google/gerrit/pgm/init/InitSshd.java
@@ -124,7 +124,7 @@
                   "-q" /* quiet */,
                   "-t",
                   "ed25519",
-                  "-P",
+                  "-N",
                   emptyPassphraseArg,
                   "-C",
                   comment,
@@ -152,7 +152,7 @@
                   "ecdsa",
                   "-b",
                   "256",
-                  "-P",
+                  "-N",
                   emptyPassphraseArg,
                   "-C",
                   comment,
@@ -180,7 +180,7 @@
                   "ecdsa",
                   "-b",
                   "384",
-                  "-P",
+                  "-N",
                   emptyPassphraseArg,
                   "-C",
                   comment,
@@ -208,7 +208,7 @@
                   "ecdsa",
                   "-b",
                   "521",
-                  "-P",
+                  "-N",
                   emptyPassphraseArg,
                   "-C",
                   comment,
diff --git a/java/com/google/gerrit/server/git/BanCommit.java b/java/com/google/gerrit/server/git/BanCommit.java
index 4473ab7..242c11b 100644
--- a/java/com/google/gerrit/server/git/BanCommit.java
+++ b/java/com/google/gerrit/server/git/BanCommit.java
@@ -20,7 +20,6 @@
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.entities.RefNames;
 import com.google.gerrit.extensions.restapi.AuthException;
-import com.google.gerrit.git.LockFailureException;
 import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.GerritPersonIdent;
 import com.google.gerrit.server.IdentifiedUser;
@@ -47,6 +46,12 @@
 import org.eclipse.jgit.revwalk.RevCommit;
 import org.eclipse.jgit.revwalk.RevWalk;
 
+/**
+ * Logic for banning commits from being uploaded.
+ *
+ * <p>Gerrit has a per-project list of commits that are forbidden to be pushed. This class reads and
+ * writes the banned commits list in {@code refs/meta/reject-commits}.
+ */
 @Singleton
 public class BanCommit {
   /**
@@ -91,9 +96,14 @@
     this.tz = gerritIdent.getTimeZone();
   }
 
+  /**
+   * Bans a list of commits from the given project.
+   *
+   * <p>The user must be specified, so it can be checked for the {@code BAN_COMMIT} permission.
+   */
   public BanCommitResult ban(
       Project.NameKey project, CurrentUser user, List<ObjectId> commitsToBan, String reason)
-      throws AuthException, LockFailureException, IOException, PermissionBackendException {
+      throws AuthException, IOException, PermissionBackendException {
     permissionBackend.user(user).project(project).check(ProjectPermission.BAN_COMMIT);
 
     final BanCommitResult result = new BanCommitResult();
diff --git a/java/com/google/gerrit/server/git/BanCommitResult.java b/java/com/google/gerrit/server/git/BanCommitResult.java
index 9fadae2..c78123e 100644
--- a/java/com/google/gerrit/server/git/BanCommitResult.java
+++ b/java/com/google/gerrit/server/git/BanCommitResult.java
@@ -18,6 +18,7 @@
 import java.util.List;
 import org.eclipse.jgit.lib.ObjectId;
 
+/** The outcome of the {@link com.google.gerrit.server.git.BanCommit} operation. */
 public class BanCommitResult {
   private final List<ObjectId> newlyBannedCommits = new ArrayList<>(4);
   private final List<ObjectId> alreadyBannedCommits = new ArrayList<>(4);
diff --git a/java/com/google/gerrit/server/git/ChangeReportFormatter.java b/java/com/google/gerrit/server/git/ChangeReportFormatter.java
index f897a1d..e0efaef 100644
--- a/java/com/google/gerrit/server/git/ChangeReportFormatter.java
+++ b/java/com/google/gerrit/server/git/ChangeReportFormatter.java
@@ -18,6 +18,7 @@
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.entities.Change;
 
+/** Formatter for git command-line progress messages. */
 public interface ChangeReportFormatter {
   @AutoValue
   public abstract static class Input {
diff --git a/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java b/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
index 5866c57..4f6094e 100644
--- a/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
+++ b/java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java
@@ -22,7 +22,7 @@
 import com.google.inject.Inject;
 import java.util.Optional;
 
-/** Print a change description for use in git command-line progress. */
+/** Default formatter for change descriptions for use in git command-line progress. */
 public class DefaultChangeReportFormatter implements ChangeReportFormatter {
   private static final int SUBJECT_MAX_LENGTH = 80;
   private static final String SUBJECT_CROP_APPENDIX = "...";
diff --git a/java/com/google/gerrit/server/git/GarbageCollection.java b/java/com/google/gerrit/server/git/GarbageCollection.java
index 090d439..9b52f48 100644
--- a/java/com/google/gerrit/server/git/GarbageCollection.java
+++ b/java/com/google/gerrit/server/git/GarbageCollection.java
@@ -16,6 +16,7 @@
 
 import com.google.common.collect.Sets;
 import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.Nullable;
 import com.google.gerrit.common.data.GarbageCollectionResult;
 import com.google.gerrit.entities.Project;
 import com.google.gerrit.extensions.events.GarbageCollectorListener;
@@ -37,6 +38,7 @@
 import org.eclipse.jgit.lib.TextProgressMonitor;
 import org.eclipse.jgit.storage.pack.PackConfig;
 
+/** Serial execution of GC on a list of repositories. */
 public class GarbageCollection {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
@@ -69,8 +71,9 @@
     return run(projectNames, gcConfig.isAggressive(), writer);
   }
 
+  /** Runs GC on the given projects, serially. Progress is written to writer if non-null. */
   public GarbageCollectionResult run(
-      List<Project.NameKey> projectNames, boolean aggressive, PrintWriter writer) {
+      List<Project.NameKey> projectNames, boolean aggressive, @Nullable PrintWriter writer) {
     GarbageCollectionResult result = new GarbageCollectionResult();
     Set<Project.NameKey> projectsToGc = gcQueue.addAll(projectNames);
     for (Project.NameKey projectName :
diff --git a/java/com/google/gerrit/server/git/GarbageCollectionQueue.java b/java/com/google/gerrit/server/git/GarbageCollectionQueue.java
index e3a923b..5df9ab5 100644
--- a/java/com/google/gerrit/server/git/GarbageCollectionQueue.java
+++ b/java/com/google/gerrit/server/git/GarbageCollectionQueue.java
@@ -21,6 +21,7 @@
 import java.util.HashSet;
 import java.util.Set;
 
+/** A thread-safe list of projects scheduled for GC. */
 @Singleton
 public class GarbageCollectionQueue {
   private final Set<Project.NameKey> projectsScheduledForGc = new HashSet<>();
diff --git a/java/com/google/gerrit/server/restapi/account/GetAgreements.java b/java/com/google/gerrit/server/restapi/account/GetAgreements.java
index 5feca66..572b489 100644
--- a/java/com/google/gerrit/server/restapi/account/GetAgreements.java
+++ b/java/com/google/gerrit/server/restapi/account/GetAgreements.java
@@ -29,6 +29,8 @@
 import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.account.AccountResource;
 import com.google.gerrit.server.config.GerritServerConfig;
+import com.google.gerrit.server.permissions.GlobalPermission;
+import com.google.gerrit.server.permissions.PermissionBackend;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.project.ProjectCache;
 import com.google.gerrit.server.restapi.config.AgreementJson;
@@ -48,17 +50,20 @@
   private final ProjectCache projectCache;
   private final AgreementJson agreementJson;
   private final boolean agreementsEnabled;
+  private final PermissionBackend permissionBackend;
 
   @Inject
   GetAgreements(
       Provider<CurrentUser> self,
       ProjectCache projectCache,
       AgreementJson agreementJson,
+      PermissionBackend permissionBackend,
       @GerritServerConfig Config config) {
     this.self = self;
     this.projectCache = projectCache;
     this.agreementJson = agreementJson;
     this.agreementsEnabled = config.getBoolean("auth", "contributorAgreements", false);
+    this.permissionBackend = permissionBackend;
   }
 
   @Override
@@ -74,7 +79,11 @@
 
     IdentifiedUser user = self.get().asIdentifiedUser();
     if (user != resource.getUser()) {
-      throw new AuthException("not allowed to get contributor agreements");
+      try {
+        permissionBackend.user(user).check(GlobalPermission.ADMINISTRATE_SERVER);
+      } catch (AuthException e) {
+        throw new AuthException("not allowed to get contributor agreements", e);
+      }
     }
 
     List<AgreementInfo> results = new ArrayList<>();
diff --git a/java/com/google/gerrit/server/restapi/project/BanCommit.java b/java/com/google/gerrit/server/restapi/project/BanCommit.java
index a20d462..eb5473d 100644
--- a/java/com/google/gerrit/server/restapi/project/BanCommit.java
+++ b/java/com/google/gerrit/server/restapi/project/BanCommit.java
@@ -31,6 +31,7 @@
 import java.util.List;
 import org.eclipse.jgit.lib.ObjectId;
 
+/** The REST endpoint that marks commits as banned in a project. */
 @Singleton
 public class BanCommit implements RestModifyView<ProjectResource, BanCommitInput> {
   private final com.google.gerrit.server.git.BanCommit banCommit;
diff --git a/java/com/google/gerrit/server/restapi/project/GarbageCollect.java b/java/com/google/gerrit/server/restapi/project/GarbageCollect.java
index 25a2c90..c5423e6 100644
--- a/java/com/google/gerrit/server/restapi/project/GarbageCollect.java
+++ b/java/com/google/gerrit/server/restapi/project/GarbageCollect.java
@@ -43,6 +43,7 @@
 import java.util.Collections;
 import java.util.Optional;
 
+/** REST endpoint that executes GC on a project. */
 @RequiresCapability(GlobalCapability.RUN_GC)
 @Singleton
 public class GarbageCollect
diff --git a/java/com/google/gerrit/server/update/RetryHelper.java b/java/com/google/gerrit/server/update/RetryHelper.java
index c3a88f2..9f38156 100644
--- a/java/com/google/gerrit/server/update/RetryHelper.java
+++ b/java/com/google/gerrit/server/update/RetryHelper.java
@@ -87,7 +87,7 @@
     @Nullable
     abstract Duration timeout();
 
-    abstract Optional<String> caller();
+    abstract Optional<String> actionName();
 
     abstract Optional<Predicate<Throwable>> retryWithTrace();
 
@@ -99,7 +99,7 @@
 
       public abstract Builder timeout(Duration timeout);
 
-      public abstract Builder caller(String caller);
+      public abstract Builder actionName(String caller);
 
       public abstract Builder retryWithTrace(Predicate<Throwable> exceptionPredicate);
 
@@ -458,7 +458,7 @@
                   return true;
                 }
 
-                String actionName = opts.caller().orElse("N/A");
+                String actionName = opts.actionName().orElse("N/A");
 
                 // Exception hooks may identify additional exceptions for retry.
                 if (exceptionHooks.stream()
@@ -507,7 +507,7 @@
         logger.atFine().log("%s was attempted %d times", actionType, listener.getAttemptCount());
         metrics.attemptCounts.incrementBy(
             actionType,
-            opts.caller().orElse("N/A"),
+            opts.actionName().orElse("N/A"),
             listener.getCause().map(this::formatCause).orElse("_unknown"),
             listener.getAttemptCount() - 1);
       }
@@ -557,7 +557,7 @@
       if (e instanceof RetryException) {
         metrics.timeoutCount.increment(
             actionType,
-            opts.caller().orElse("N/A"),
+            opts.actionName().orElse("N/A"),
             e.getCause() != null ? formatCause(e.getCause()) : "_unknown");
       }
       if (e.getCause() != null) {
diff --git a/java/com/google/gerrit/server/update/RetryableAction.java b/java/com/google/gerrit/server/update/RetryableAction.java
index ae9ba99..75ebeb37 100644
--- a/java/com/google/gerrit/server/update/RetryableAction.java
+++ b/java/com/google/gerrit/server/update/RetryableAction.java
@@ -78,7 +78,7 @@
     this.retryHelper = requireNonNull(retryHelper, "retryHelper");
     this.actionType = requireNonNull(actionType, "actionType");
     this.action = requireNonNull(action, "action");
-    options.caller(requireNonNull(actionName, "actionName"));
+    options.actionName(requireNonNull(actionName, "actionName"));
   }
 
   /**
diff --git a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
index 3bb0338..11ca391 100644
--- a/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/accounts/AgreementsIT.java
@@ -178,6 +178,18 @@
   }
 
   @Test
+  public void listAgreementPermission() throws Exception {
+    assume().that(isContributorAgreementsEnabled()).isTrue();
+    requestScopeOperations.setApiUser(admin.id());
+    // Allowed.
+    gApi.accounts().id(user.id().get()).listAgreements();
+    requestScopeOperations.setApiUser(user.id());
+
+    // Not allowed.
+    assertThrows(AuthException.class, () -> gApi.accounts().id(admin.id().get()).listAgreements());
+  }
+
+  @Test
   public void signAgreementAsOtherUser() throws Exception {
     assume().that(isContributorAgreementsEnabled()).isTrue();
     assertThat(gApi.accounts().self().get().name).isNotEqualTo("admin");
diff --git a/package.json b/package.json
index 95edf0b..2656f74 100644
--- a/package.json
+++ b/package.json
@@ -15,7 +15,7 @@
   "scripts": {
     "start": "polygerrit-ui/run-server.sh",
     "test": "WCT_HEADLESS_MODE=1 WCT_ARGS='--verbose -l chrome' ./polygerrit-ui/app/run_test.sh",
-    "eslint": "./node_modules/eslint/bin/eslint.js --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js polygerrit-ui/app",
+    "eslint": "./node_modules/eslint/bin/eslint.js --ext .html,.js polygerrit-ui/app",
     "eslintfix": "npm run eslint -- --fix",
     "test-template": "./polygerrit-ui/app/run_template_test.sh",
     "polylint": "bazel test polygerrit-ui/app:polylint_test"
diff --git a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
index 5eb8b09..c6743e8 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
+++ b/polygerrit-ui/app/elements/diff/gr-diff-builder/gr-diff-builder.js
@@ -560,6 +560,10 @@
         isStartOfRange ? 'startOfRange' : '');
     const shaNode = this._createElement('span', 'sha');
     shaNode.innerText = commit.id.substr(0, 7);
+    shaNode.onclick = function() {
+      location.href = '/q/' + shaNode.innerText;
+    };
+
     blameNode.appendChild(shaNode);
     blameNode.append(` on ${date} by ${commit.author}`);
     return blameNode;
diff --git a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
index 3b64f55..92c2c37 100644
--- a/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
+++ b/polygerrit-ui/app/elements/diff/gr-diff/gr-diff.html
@@ -273,6 +273,8 @@
       }
       td.blame .sha {
         font-family: var(--monospace-font-family);
+        color: var(--link-color);
+        cursor: pointer;
       }
       .full-width td.blame {
         overflow: hidden;
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js
index 546b9f3..e48ef91 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-gerrit.js
@@ -156,4 +156,41 @@
   // Preloaded plugins should be installed after Gerrit.install() is set,
   // since plugin preloader substitutes Gerrit.install() temporarily.
   Gerrit._pluginLoader.installPreloadedPlugins();
+
+  // TODO(taoalpha): List all internal supported event names.
+  // Also convert this to inherited class once we move Gerrit to class.
+  Gerrit._eventEmitter = new EventEmitter();
+  ['addListener',
+    'dispatch',
+    'emit',
+    'off',
+    'on',
+    'once',
+    'removeAllListeners',
+    'removeListener',
+  ].forEach(method => {
+    /**
+     * Enabling EventEmitter interface on Gerrit.
+     *
+     * This will enable to signal across different parts of js code without relying on DOM,
+     * including core to core, plugin to plugin and also core to plugin.
+     *
+     * @example
+     *
+     * // Emit this event from pluginA
+     * Gerrit.install(pluginA => {
+     *   fetch("some-api").then(() => {
+     *     Gerrit.on("your-special-event", {plugin: pluginA});
+     *   });
+     * });
+     *
+     * // Listen on your-special-event from pluignB
+     * Gerrit.install(pluginB => {
+     *   Gerrit.on("your-special-event", ({plugin}) => {
+     *     // do something, plugin is pluginA
+     *   });
+     * });
+     */
+    Gerrit[method] = Gerrit._eventEmitter[method].bind(Gerrit._eventEmitter);
+  });
 })(window);
\ No newline at end of file
diff --git a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
index 9ff1297..6dc0309 100644
--- a/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
+++ b/polygerrit-ui/app/elements/shared/gr-js-api-interface/gr-public-js-api.js
@@ -381,40 +381,4 @@
   };
 
   window.Plugin = Plugin;
-  // TODO(taoalpha): List all internal supported event names.
-  // Also convert this to inherited class once we move Gerrit to class.
-  Gerrit._eventEmitter = new EventEmitter();
-  ['addListener',
-    'dispatch',
-    'emit',
-    'off',
-    'on',
-    'once',
-    'removeAllListeners',
-    'removeListener',
-  ].forEach(method => {
-    /**
-     * Enabling EventEmitter interface on Gerrit.
-     *
-     * This will enable to signal across different parts of js code without relying on DOM,
-     * including core to core, plugin to plugin and also core to plugin.
-     *
-     * @example
-     *
-     * // Emit this event from pluginA
-     * Gerrit.install(pluginA => {
-     *   fetch("some-api").then(() => {
-     *     Gerrit.on("your-special-event", {plugin: pluginA});
-     *   });
-     * });
-     *
-     * // Listen on your-special-event from pluignB
-     * Gerrit.install(pluginB => {
-     *   Gerrit.on("your-special-event", ({plugin}) => {
-     *     // do something, plugin is pluginA
-     *   });
-     * });
-     */
-    Gerrit[method] = Gerrit._eventEmitter[method].bind(Gerrit._eventEmitter);
-  });
 })(window);
diff --git a/polygerrit-ui/app/lint_test.sh b/polygerrit-ui/app/lint_test.sh
index 35939ba..bcd94ba 100755
--- a/polygerrit-ui/app/lint_test.sh
+++ b/polygerrit-ui/app/lint_test.sh
@@ -19,4 +19,4 @@
     exit 1
 fi
 
-${eslint_bin} --ignore-pattern 'bower_components/' --ignore-pattern 'gr-linked-text' --ignore-pattern 'scripts/vendor' --ext .html,.js .
+${eslint_bin} --ext .html,.js .
diff --git a/tools/dev-hooks/pre-commit b/tools/dev-hooks/pre-commit
new file mode 100755
index 0000000..b77f382
--- /dev/null
+++ b/tools/dev-hooks/pre-commit
@@ -0,0 +1,47 @@
+#!/bin/sh
+#
+# Part of Gerrit Code Review (https://www.gerritcodereview.com/)
+#
+# Copyright (C) 2019 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.
+
+
+# To enable this hook:
+# - copy this file or content to ".git/hooks/pre-commit"
+# - (optional if you copied this file) make it executable: `chmod +x .git/hooks/pre-commit`
+
+set -ue
+
+# gitroot, default to .
+gitroot=$(git rev-parse --show-cdup)
+gitroot=${gitroot:-.};
+
+# eslint
+eslint=${gitroot}/node_modules/eslint/bin/eslint.js
+
+# Run eslint over changed frontend code
+CHANGED_UI_FILES=$(git diff --cached --name-only -- '*.js' '*.html' | grep 'polygerrit-ui') && true
+if [ "${CHANGED_UI_FILES}" ]; then
+  if $eslint --fix ${CHANGED_UI_FILES}; then
+    # Add again in case lint fix modified some files
+    git add ${CHANGED_UI_FILES}
+    exit 0
+  else
+    echo "Failed to fix all linter issues.";
+    exit 1
+  fi
+else
+  echo "No UI files changed"
+  exit 0
+fi
\ No newline at end of file