Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  Fix the access-path for AbstractGitCommand subclasses
  JGit metrics: expose total load time for block cache entries
  Format bzl files with buildifier version 1.0.0
  Bump Bazel version to 2.2.0
  Upgrade jackson-core to 2.10.3
  Add support for going to linNum in inline editor
  Add support for fsharp in highlighting syntax
  Update git submodules
  Add sendemail.denyrcpt functionality

Change-Id: Iceb8cf8639b4a0532a112d04a79f80f762d63e8d

.bazelversion

@@ -1 +1 @@
-2.1.0
+2.2.0

Documentation/config-gerrit.txt

@@ -4203,6 +4203,19 @@ email address, that one address is added to the white list.
 If set to a domain name, any address at that domain can receive
 email from Gerrit.
 +
+If allowrcpt is configured, The set of allowed recipients is:
+`allowrcpt - denyrcpt`.
++
+By default, unset, permitting delivery to any email address.
+
+[[sendemail.denyrcpt]]sendemail.denyrcpt::
++
+If present, each value adds one entry to the blacklist of email
+addresses that Gerrit can send email to.  If set to a complete
+email address, that one address is added to the blacklist.
+If set to a domain name, any address at that domain can *not* receive
+email from Gerrit.
++
 By default, unset, permitting delivery to any email address.
 
 [[sendemail.includeDiff]]sendemail.includeDiff::

Documentation/dev-contributing.txt

@@ -171,7 +171,7 @@ To format Java source code, Gerrit uses the
 link:https://github.com/google/google-java-format[`google-java-format`]
 tool (version 1.7), and to format Bazel BUILD, WORKSPACE and .bzl files the
 link:https://github.com/bazelbuild/buildtools/tree/master/buildifier[`buildifier`]
-tool (version 0.29.0). Unused dependencies are found and removed using the
+tool (version 1.0.0). Unused dependencies are found and removed using the
 link:https://github.com/bazelbuild/buildtools/tree/master/unused_deps[`unused_deps`]
 build tool, a sibling of `buildifier`.
 

Documentation/metrics.txt

@@ -137,9 +137,10 @@ topic submissions that concluded successfully.
 
 === JGit
 
-* `jgit/block_cache/cache_used`: Bytes of memory retained in JGit block cache.
+* `jgit/block_cache/cache_used` : Bytes of memory retained in JGit block cache.
-* `jgit/block_cache/open_files`: File handles held open by JGit block cache.
+* `jgit/block_cache/open_files` : File handles held open by JGit block cache.
-* `avg_load_time` Average time to load a cache entry for JGit block cache.
+* `avg_load_time` : Average time to load a cache entry for JGit block cache.
+* `total_load_time` : Total time to load cache entries for JGit block cache.
 * `eviction_count` : Cache evictions for JGit block cache.
 * `eviction_ratio` : Cache eviction ratio for JGit block cache.
 * `hit_count` : Cache hits for JGit block cache.

Documentation/rest-api-accounts.txt

@@ -639,6 +639,9 @@ link:#email-input[EmailInput].
 If link:config-gerrit.html#sendemail.allowrcpt[sendemail.allowrcpt] is
 configured, the added email address must belong to a domain that is
 allowed, unless `no_confirmation` is set.
+If link:config-gerrit.html#sendemail.denyrcpt[sendemail.denyrcpt]
+is configured, make sure that the added email address is *not* disallowed or
+belongs to a domain that is disallowed.
 
 The link:#email-input[EmailInput] object in the request body may
 contain additional options for the email address.

java/com/google/gerrit/metrics/proc/JGitMetricModule.java

@@ -65,6 +65,17 @@ public class JGitMetricModule extends MetricModule {
           }
         });
 
+    metrics.newCallbackMetric(
+        "jgit/block_cache/total_load_time",
+        Long.class,
+        new Description("Total time to load cache entries for JGit block cache.").setGauge(),
+        new Supplier<Long>() {
+          @Override
+          public Long get() {
+            return WindowCacheStats.getStats().getTotalLoadTime();
+          }
+        });
+
     metrics.newCallbackMetric(
         "jgit/block_cache/eviction_count",
         Long.class,

java/com/google/gerrit/server/mail/send/OutgoingEmail.java

@@ -497,9 +497,7 @@ public abstract class OutgoingEmail {
     if (addr != null && addr.getEmail() != null && addr.getEmail().length() > 0) {
       if (!args.validator.isValid(addr.getEmail())) {
         logger.atWarning().log("Not emailing %s (invalid email address)", addr.getEmail());
-      } else if (!args.emailSender.canEmail(addr.getEmail())) {
+      } else if (args.emailSender.canEmail(addr.getEmail())) {
-        logger.atWarning().log("Not emailing %s (prohibited by allowrcpt)", addr.getEmail());
-      } else {
         if (!smtpRcptTo.add(addr)) {
           if (!override) {
             return;

java/com/google/gerrit/server/mail/send/SmtpEmailSender.java

@@ -16,6 +16,7 @@ package com.google.gerrit.server.mail.send;
 
 import static java.nio.charset.StandardCharsets.UTF_8;
 
+import com.google.common.flogger.FluentLogger;
 import com.google.common.io.BaseEncoding;
 import com.google.common.primitives.Ints;
 import com.google.gerrit.common.Nullable;
@@ -56,6 +57,8 @@ public class SmtpEmailSender implements EmailSender {
   /** The socket's connect timeout (0 = infinite timeout) */
   private static final int DEFAULT_CONNECT_TIMEOUT = 0;
 
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
   public static class Module extends AbstractModule {
     @Override
     protected void configure() {
@@ -73,6 +76,7 @@ public class SmtpEmailSender implements EmailSender {
   private Encryption smtpEncryption;
   private boolean sslVerify;
   private Set<String> allowrcpt;
+  private Set<String> denyrcpt;
   private String importance;
   private int expiryDays;
 
@@ -117,6 +121,9 @@ public class SmtpEmailSender implements EmailSender {
     Set<String> rcpt = new HashSet<>();
     Collections.addAll(rcpt, cfg.getStringList("sendemail", null, "allowrcpt"));
     allowrcpt = Collections.unmodifiableSet(rcpt);
+    Set<String> rcptdeny = new HashSet<>();
+    Collections.addAll(rcptdeny, cfg.getStringList("sendemail", null, "denyrcpt"));
+    denyrcpt = Collections.unmodifiableSet(rcptdeny);
     importance = cfg.getString("sendemail", null, "importance");
     expiryDays = cfg.getInt("sendemail", null, "expiryDays", 0);
   }
@@ -129,22 +136,47 @@ public class SmtpEmailSender implements EmailSender {
   @Override
   public boolean canEmail(String address) {
     if (!isEnabled()) {
+      logger.atWarning().log("Not emailing %s (email is disabled)", address);
       return false;
     }
 
+    String domain = address.substring(address.lastIndexOf('@') + 1);
+    if (isDenied(address, domain)) {
+      return false;
+    }
+
+    return isAllowed(address, domain);
+  }
+
+  private boolean isDenied(String address, String domain) {
+
+    if (denyrcpt.isEmpty()) {
+      return false;
+    }
+
+    if (denyrcpt.contains(address)
+        || denyrcpt.contains(domain)
+        || denyrcpt.contains("@" + domain)) {
+      logger.atWarning().log("Not emailing %s (prohibited by sendemail.denyrcpt)", address);
+      return true;
+    }
+
+    return false;
+  }
+
+  private boolean isAllowed(String address, String domain) {
+
     if (allowrcpt.isEmpty()) {
       return true;
     }
 
-    if (allowrcpt.contains(address)) {
+    if (allowrcpt.contains(address)
-      return true;
+        || allowrcpt.contains(domain)
-    }
+        || allowrcpt.contains("@" + domain)) {
-
-    String domain = address.substring(address.lastIndexOf('@') + 1);
-    if (allowrcpt.contains(domain) || allowrcpt.contains("@" + domain)) {
       return true;
     }
 
+    logger.atWarning().log("Not emailing %s (prohibited by sendemail.allowrcpt)", address);
     return false;
   }
 

java/com/google/gerrit/sshd/AbstractGitCommand.java

@@ -36,12 +36,12 @@ public abstract class AbstractGitCommand extends BaseCommand {
 
   @Inject private GitRepositoryManager repoManager;
 
-  @Inject private SshSession session;
-
   @Inject private SshScope.Context context;
 
   @Inject private IdentifiedUser.GenericFactory userFactory;
 
+  @Inject protected SshSession session;
+
   protected Repository repo;
   protected Project.NameKey projectName;
   protected Project project;

java/com/google/gerrit/sshd/commands/Receive.java

@@ -20,7 +20,7 @@ import com.google.common.flogger.FluentLogger;
 import com.google.gerrit.common.data.Capable;
 import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.CurrentUser;
 import com.google.gerrit.server.git.DefaultAdvertiseRefsHook;
 import com.google.gerrit.server.git.receive.AsyncReceiveCommits;
 import com.google.gerrit.server.notedb.ReviewerStateInternal;
@@ -29,7 +29,6 @@ import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.sshd.AbstractGitCommand;
 import com.google.gerrit.sshd.CommandMetaData;
-import com.google.gerrit.sshd.SshSession;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.util.ArrayList;
@@ -50,8 +49,6 @@ final class Receive extends AbstractGitCommand {
   private static final FluentLogger logger = FluentLogger.forEnclosingClass();
 
   @Inject private AsyncReceiveCommits.Factory factory;
-  @Inject private IdentifiedUser currentUser;
-  @Inject private SshSession session;
   @Inject private PermissionBackend permissionBackend;
 
   private final SetMultimap<ReviewerStateInternal, Account.Id> reviewers =
@@ -77,6 +74,7 @@ final class Receive extends AbstractGitCommand {
 
   @Override
   protected void runImpl() throws IOException, Failure {
+    CurrentUser currentUser = session.getUser();
     try {
       permissionBackend
           .user(currentUser)
@@ -88,7 +86,8 @@ final class Receive extends AbstractGitCommand {
       throw new Failure(1, "fatal: unable to check permissions " + e);
     }
 
-    AsyncReceiveCommits arc = factory.create(projectState, currentUser, repo, null);
+    AsyncReceiveCommits arc =
+        factory.create(projectState, currentUser.asIdentifiedUser(), repo, null);
 
     try {
       Capable r = arc.canUpload();

java/com/google/gerrit/sshd/commands/Upload.java

@@ -27,7 +27,6 @@ import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
 import com.google.gerrit.server.permissions.PermissionBackendException;
 import com.google.gerrit.server.permissions.ProjectPermission;
 import com.google.gerrit.sshd.AbstractGitCommand;
-import com.google.gerrit.sshd.SshSession;
 import com.google.inject.Inject;
 import java.io.IOException;
 import java.util.List;
@@ -45,7 +44,6 @@ final class Upload extends AbstractGitCommand {
   @Inject private DynamicSet<PostUploadHook> postUploadHooks;
   @Inject private DynamicSet<UploadPackInitializer> uploadPackInitializers;
   @Inject private UploadValidators.Factory uploadValidatorsFactory;
-  @Inject private SshSession session;
   @Inject private PermissionBackend permissionBackend;
 
   private PackStatistics stats;

javatests/com/google/gerrit/acceptance/git/AbstractForcePush.java

@@ -21,7 +21,6 @@ import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.OK;
 import static org.eclipse.jgit.transport.RemoteRefUpdate.Status.REJECTED_OTHER_REASON;
 
 import com.google.gerrit.acceptance.AbstractDaemonTest;
-import com.google.gerrit.acceptance.NoHttpd;
 import com.google.gerrit.acceptance.PushOneCommit;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.api.projects.BranchInput;
@@ -31,8 +30,7 @@ import org.eclipse.jgit.transport.PushResult;
 import org.eclipse.jgit.transport.RemoteRefUpdate;
 import org.junit.Test;
 
-@NoHttpd
+public abstract class AbstractForcePush extends AbstractDaemonTest {
-public class ForcePushIT extends AbstractDaemonTest {
 
   @Test
   public void forcePushNotAllowed() throws Exception {

javatests/com/google/gerrit/acceptance/git/HttpForcePushIT.java

@@ -0,0 +1,29 @@
+// Copyright (C) 2020 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.google.gerrit.acceptance.git;
+
+import com.google.gerrit.acceptance.GitUtil;
+import org.eclipse.jgit.transport.CredentialsProvider;
+import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
+import org.junit.Before;
+
+public class HttpForcePushIT extends AbstractForcePush {
+  @Before
+  public void cloneProjectOverHttp() throws Exception {
+    CredentialsProvider.setDefault(
+        new UsernamePasswordCredentialsProvider(admin.username(), admin.httpPassword()));
+    testRepo = GitUtil.cloneProject(project, admin.getHttpUrl(server) + "/" + project.get());
+  }
+}

javatests/com/google/gerrit/acceptance/git/SshForcePushIT.java

@@ -0,0 +1,29 @@
+// Copyright (C) 2020 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.google.gerrit.acceptance.git;
+
+import com.google.gerrit.acceptance.GitUtil;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.UseSsh;
+import org.junit.Before;
+
+@NoHttpd
+@UseSsh
+public class SshForcePushIT extends AbstractForcePush {
+  @Before
+  public void cloneProjectOverSsh() throws Exception {
+    testRepo = GitUtil.cloneProject(project, adminSshSession.getUrl() + "/" + project.get());
+  }
+}

polygerrit-ui/app/elements/core/gr-router/gr-router.js

@@ -132,8 +132,8 @@
     // eslint-disable-next-line max-len
     DIFF: /^\/c\/(.+)\/\+\/(\d+)(\/((-?\d+|edit)(\.\.(\d+|edit))?(\/(.+))))\/?$/,
 
-    // Matches /c/<project>/+/<changeNum>/[<patchNum|edit>]/<path>,edit
+    // Matches /c/<project>/+/<changeNum>/[<patchNum|edit>]/<path>,edit[#lineNum]
-    DIFF_EDIT: /^\/c\/(.+)\/\+\/(\d+)\/(\d+|edit)\/(.+),edit$/,
+    DIFF_EDIT: /^\/c\/(.+)\/\+\/(\d+)\/(\d+|edit)\/(.+),edit(\#\d+)?$/,
 
     // Matches non-project-relative
     // /c/<changeNum>/[<basePatchNum>..]<patchNum>/<path>.
@@ -1367,6 +1367,7 @@
         changeNum: ctx.params[1],
         patchNum: ctx.params[2],
         path: ctx.params[3],
+        lineNum: ctx.hash,
         view: Gerrit.Nav.View.EDIT,
       });
     },

polygerrit-ui/app/elements/core/gr-router/gr-router_test.html

@@ -1535,6 +1535,37 @@ limitations under the License.
             view: Gerrit.Nav.View.EDIT,
             path: 'foo/bar/baz',
             patchNum: 3,
+            lineNum: undefined,
+          };
+
+          element._handleDiffEditRoute(ctx);
+          assert.isFalse(redirectStub.called);
+          assert.isTrue(normalizeRangeSpy.calledOnce);
+          assert.deepEqual(normalizeRangeSpy.lastCall.args[0], appParams);
+          assert.isFalse(normalizeRangeSpy.lastCall.returnValue);
+          assert.deepEqual(setParamsStub.lastCall.args[0], appParams);
+        });
+
+        test('_handleDiffEditRoute with lineNum', () => {
+          const normalizeRangeSpy =
+              sandbox.spy(element, '_normalizePatchRangeParams');
+          sandbox.stub(element.$.restAPI, 'setInProjectLookup');
+          const ctx = {
+            params: [
+              'foo/bar', // 0 Project
+              1234, // 1 Change number
+              3, // 2 Patch num
+              'foo/bar/baz', // 3 File path
+            ],
+            hash: 4,
+          };
+          const appParams = {
+            project: 'foo/bar',
+            changeNum: 1234,
+            view: Gerrit.Nav.View.EDIT,
+            path: 'foo/bar/baz',
+            patchNum: 3,
+            lineNum: 4,
           };
 
           element._handleDiffEditRoute(ctx);

polygerrit-ui/app/elements/diff/gr-syntax-layer/gr-syntax-layer.js

@@ -46,6 +46,7 @@
     'text/x-elm': 'elm',
     'text/x-erlang': 'erlang',
     'text/x-fortran': 'fortran',
+    'text/x-fsharp': 'fsharp',
     'text/x-go': 'go',
     'text/x-groovy': 'groovy',
     'text/x-haml': 'haml',

polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.html

@@ -116,6 +116,7 @@ limitations under the License.
         <gr-endpoint-param name="fileContent" value="[[_newContent]]"></gr-endpoint-param>
         <gr-endpoint-param name="prefs" value="[[_prefs]]"></gr-endpoint-param>
         <gr-endpoint-param name="fileType" value="[[_type]]"></gr-endpoint-param>
+        <gr-endpoint-param name="lineNum" value="[[_lineNum]]"></gr-endpoint-param>
         <gr-default-editor id="file" file-content="[[_newContent]]"></gr-default-editor>
       </gr-endpoint-decorator>
     </div>

polygerrit-ui/app/elements/edit/gr-editor-view/gr-editor-view.js

@@ -71,6 +71,7 @@
         computed: '_computeSaveDisabled(_content, _newContent, _saving)',
       },
       _prefs: Object,
+      _lineNum: Number,
     },
 
     behaviors: [
@@ -109,6 +110,7 @@
       this._changeNum = value.changeNum;
       this._path = value.path;
       this._patchNum = value.patchNum || this.EDIT_NAME;
+      this._lineNum = value.lineNum;
 
       // NOTE: This may be called before attachment (e.g. while parentElement is
       // null). Fire title-change in an async so that, if attachment to the DOM

tools/bzl/asciidoc.bzl

@@ -1,7 +1,7 @@
 def documentation_attributes():
     return [
         "toc2",
-        'newline="\\n"',
+        "newline=\"\\n\"",
         'asterisk="*"',
         'plus="+"',
         'caret="^"',

tools/bzl/js.bzl

@@ -528,7 +528,7 @@ def polygerrit_plugin(name, app, srcs = [], deps = [], externs = [], assets = No
                 cmd = "cp $(SRCS) $(@D)",
                 output_to_bindir = True,
             )
-            static_files += [":" + name + "_copy_assets"]
+            static_files.append(":" + name + "_copy_assets")
 
     native.filegroup(
         name = name,

tools/nongoogle.bzl

@@ -100,8 +100,8 @@ def declare_nongoogle_deps():
 
     maven_jar(
         name = "jackson-core",
-        artifact = "com.fasterxml.jackson.core:jackson-core:2.10.2",
+        artifact = "com.fasterxml.jackson.core:jackson-core:2.10.3",
-        sha1 = "73d4322a6bda684f676a2b5fe918361c4e5c7cca",
+        sha1 = "f7ee7b55c7d292ac72fbaa7648c089f069c938d2",
     )
 
     # Test-only dependencies below.