Merge branch 'stable-3.3'

* stable-3.3:
  Update git submodules
  Bazel: Consume bazel-toolchains release from bazel mirror
  Align the recommended buildifier version with CI's
  crafting-changes: Add some info on Optional/Nullable
  Fix typo in event documentation section
  Bump bazel-toolchains to 5.0.0
  Set version to 2.16.28-SNAPSHOT
  Set version to 2.16.27
  ForRef#check should permit internal users to read all refs
  Fix NPE on trying to send email for user without email address
  Update git submodules
  Update highlightjs to 10.6.0
  Use NoteDb sequence seed for groups sequence initialization
  GroupsOnInit: Remove unused dependency on ReviewDb
  Use NoteDb sequence seed for accounts sequence initialization
  Do not use ReviewDb anymore when is disabled
  Elasticsearch: Discontinue EOL versions 7.2 and 7.3 support
  e2e-tests: Add SubmitChangeInBranch scenario
  Update developers based on gerritcodereview's list
  ForRef#check should permit internal users to read all refs

Change-Id: Id555d1b7b63d5cd0a27ff186be2931ff50dd442d

Documentation/config-gerrit.txt

@@ -3397,7 +3397,7 @@ Defaults to all available options minus `CHANGE_ACTIONS`,
 config is backwards compatible with what the default was before the config
 was added.
 
-[[event.comment-added.publishPatchSetLevelComment]][event.comment-added.publishPatchSetLevelComment::
+[[event.comment-added.publishPatchSetLevelComment]]event.comment-added.publishPatchSetLevelComment::
 +
 Add patch set level comment as event comment. Without this option, patch set
 level comment will not be included in the event comment attribute. Given that

Documentation/dev-crafting-changes.txt

@@ -147,7 +147,7 @@ To format Java source code, Gerrit uses the
 link:https://github.com/google/google-java-format[`google-java-format`,role=external,window=_blank]
 tool (version 1.7), and to format Bazel BUILD, WORKSPACE and .bzl files the
 link:https://github.com/bazelbuild/buildtools/tree/master/buildifier[`buildifier`,role=external,window=_blank]
-tool (version 3.5.0). Unused dependencies are found and removed using the
+tool (version 4.0.0). Unused dependencies are found and removed using the
 link:https://github.com/bazelbuild/buildtools/tree/master/unused_deps[`unused_deps`,role=external,window=_blank]
 build tool, a sibling of `buildifier`.
 
@@ -160,6 +160,9 @@ run `./tools/setup_gjf.sh` to download a local copy and set up a
 wrapper script. If you run your own copy, please use the same version,
 as there may be slight differences between versions.
 
+[[code-rules]]
+== Code Rules
+=== Final
 When to use `final` modifier and when not (in new code):
 
 Always:
@@ -181,6 +184,12 @@ Never:
   be removed
   * method parameters: similar to local variables
 
+=== Optional / Nullable
+Recommended:
+
+  * Optionals in arguments are discouraged (use @Nullable instead)
+  * Return types should be objects or Optionals of objects, but not null/nullable
+
 [[code-organization]]
 == Code Organization
 

WORKSPACE

@@ -34,11 +34,11 @@ load("//tools:nongoogle.bzl", "TESTCONTAINERS_VERSION", "declare_nongoogle_deps"
 
 http_archive(
     name = "bazel_toolchains",
-    sha256 = "726b5423e1c7a3866a3a6d68e7123b4a955e9fcbe912a51e0f737e6dab1d0af2",
-    strip_prefix = "bazel-toolchains-3.1.0",
+    sha256 = "1adf7a8e9901287c644dcf9ca08dd8d67a69df94bedbd57a841490a84dc1e9ed",
+    strip_prefix = "bazel-toolchains-5.0.0",
     urls = [
-        "https://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/releases/download/3.1.0/bazel-toolchains-3.1.0.tar.gz",
-        "https://github.com/bazelbuild/bazel-toolchains/releases/download/3.1.0/bazel-toolchains-3.1.0.tar.gz",
+        "https://mirror.bazel.build/github.com/bazelbuild/bazel-toolchains/archive/v5.0.0.tar.gz",
+        "https://github.com/bazelbuild/bazel-toolchains/archive/v5.0.0.tar.gz",
     ],
 )
 

e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/CreateChange-body.json

@@ -1,5 +1,5 @@
 {
   "project": "${project}",
-  "branch": "master",
+  "branch": "${branch}",
   "subject": "Change"
 }

e2e-tests/src/test/resources/data/com/google/gerrit/scenarios/SubmitChangeInBranch.json

@@ -0,0 +1,5 @@
+[
+  {
+    "url": "HTTP_SCHEME://HOSTNAME:HTTP_PORT/a/changes/"
+  }
+]

e2e-tests/src/test/scala/com/google/gerrit/scenarios/ApproveChange.scala

@@ -19,10 +19,15 @@ import io.gatling.core.feeder.FeederBuilder
 import io.gatling.core.structure.ScenarioBuilder
 import io.gatling.http.Predef.http
 
+import scala.collection.mutable
+
 class ApproveChange extends GerritSimulation {
-  private val data: FeederBuilder = jsonFile(resource).convert(keys).queue
+  private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
+  private var numbersCopy: mutable.Queue[Int] = mutable.Queue[Int]()
   private var createChange: Option[CreateChange] = None
 
+  override def relativeRuntimeWeight = 10
+
   def this(createChange: CreateChange) {
     this()
     this.createChange = Some(createChange)
@@ -32,7 +37,10 @@ class ApproveChange extends GerritSimulation {
       .feed(data)
       .exec(session => {
         if (createChange.nonEmpty) {
-          session.set("number", createChange.get.number)
+          if (numbersCopy.isEmpty) {
+            numbersCopy = createChange.get.numbers.clone()
+          }
+          session.set("number", numbersCopy.dequeue())
         } else {
           session
         }

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateBranch.scala

@@ -19,18 +19,27 @@ import io.gatling.core.feeder.FeederBuilder
 import io.gatling.core.structure.ScenarioBuilder
 import io.gatling.http.Predef._
 
+import scala.collection.mutable
 import scala.concurrent.duration._
 
 class CreateBranch extends ProjectSimulation {
   private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
   private val branchIdKey = "branchId"
   private var counter = 0
+  var branches: mutable.Queue[String] = mutable.Queue[String]()
 
-  private val test: ScenarioBuilder = scenario(uniqueName)
+  def this(projectName: String) {
+    this()
+    this.projectName = projectName
+  }
+
+  val test: ScenarioBuilder = scenario(uniqueName)
       .feed(data)
       .exec(session => {
         counter += 1
-        session.set(branchIdKey, "branch-" + counter)
+        val branchId = "branch-" + counter
+        branches += branchId
+        session.set(branchIdKey, branchId)
       })
       .exec(http(uniqueName)
           .post("${url}${" + branchIdKey + "}")

e2e-tests/src/test/scala/com/google/gerrit/scenarios/CreateChange.scala

@@ -25,18 +25,37 @@ import scala.concurrent.duration._
 class CreateChange extends ProjectSimulation {
   private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
   private val numberKey = "_number"
+  private val weightPerUser = 0.1
+  private var createBranch: Option[CreateBranch] = None
+  private var branchesCopy: mutable.Queue[String] = mutable.Queue[String]()
   var number = 0
   var numbers: mutable.Queue[Int] = mutable.Queue[Int]()
 
-  override def relativeRuntimeWeight = 2
+  override def relativeRuntimeWeight: Int = 2 + (numberOfUsers * weightPerUser).toInt
 
   def this(projectName: String) {
     this()
     this.projectName = projectName
   }
 
+  def this(projectName: String, createBranch: CreateBranch) {
+    this()
+    this.projectName = projectName
+    this.createBranch = Some(createBranch)
+  }
+
   val test: ScenarioBuilder = scenario(uniqueName)
       .feed(data)
+      .exec(session => {
+        var branchId = "master"
+        if (createBranch.nonEmpty) {
+          if (branchesCopy.isEmpty) {
+            branchesCopy = createBranch.get.branches.clone()
+          }
+          branchId = branchesCopy.dequeue()
+        }
+        session.set("branch", branchId)
+      })
       .exec(httpRequest
           .body(ElFileBody(body)).asJson
           .check(regex("\"" + numberKey + "\":(\\d+),").saveAs(numberKey)))

e2e-tests/src/test/scala/com/google/gerrit/scenarios/SubmitChangeInBranch.scala

@@ -0,0 +1,74 @@
+// Copyright (C) 2021 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.scenarios
+
+import io.gatling.core.Predef.{atOnceUsers, _}
+import io.gatling.core.feeder.FeederBuilder
+import io.gatling.core.structure.ScenarioBuilder
+import io.gatling.http.Predef.http
+
+import scala.collection.mutable
+import scala.concurrent.duration._
+
+class SubmitChangeInBranch extends GerritSimulation {
+  private val data: FeederBuilder = jsonFile(resource).convert(keys).circular
+  private var changesCopy: mutable.Queue[Int] = mutable.Queue[Int]()
+  private val projectName = className
+
+  override def relativeRuntimeWeight = 10
+
+  private val test: ScenarioBuilder = scenario(uniqueName)
+      .feed(data)
+      .exec(session => {
+        if (changesCopy.isEmpty) {
+          changesCopy = createChange.numbers.clone()
+        }
+        session.set("number", changesCopy.dequeue())
+      })
+      .exec(http(uniqueName).post("${url}${number}/submit"))
+
+  private val createProject = new CreateProject(projectName)
+  private val createBranch = new CreateBranch(projectName)
+  private val createChange = new CreateChange(projectName, createBranch)
+  private val approveChange = new ApproveChange(createChange)
+  private val deleteProject = new DeleteProject(projectName)
+
+  setUp(
+    createProject.test.inject(
+      nothingFor(stepWaitTime(createProject) seconds),
+      atOnceUsers(single)
+    ),
+    createBranch.test.inject(
+      nothingFor(stepWaitTime(createBranch) seconds),
+      atOnceUsers(numberOfUsers)
+    ),
+    createChange.test.inject(
+      nothingFor(stepWaitTime(createChange) seconds),
+      atOnceUsers(numberOfUsers)
+    ),
+    approveChange.test.inject(
+      nothingFor(stepWaitTime(approveChange) seconds),
+      atOnceUsers(numberOfUsers)
+    ),
+    test.inject(
+      nothingFor(stepWaitTime(this) seconds),
+      atOnceUsers(numberOfUsers)
+    ),
+    deleteProject.test.inject(
+      nothingFor(stepWaitTime(deleteProject) seconds),
+      atOnceUsers(single)
+    ),
+  ).protocols(httpProtocol)
+}

java/com/google/gerrit/elasticsearch/ElasticVersion.java

@@ -18,8 +18,6 @@ import com.google.common.base.Joiner;
 import java.util.regex.Pattern;
 
 public enum ElasticVersion {
-  V7_2("7.2.*"),
-  V7_3("7.3.*"),
   V7_4("7.4.*"),
   V7_5("7.5.*"),
   V7_6("7.6.*"),

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

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.mail.send;
 
 import com.google.common.base.Joiner;
-import com.google.gerrit.entities.Address;
 import com.google.gerrit.exceptions.EmailException;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.server.IdentifiedUser;
@@ -68,7 +67,7 @@ public class AddKeySender extends OutgoingEmail {
     super.init();
     setHeader("Subject", String.format("[Gerrit Code Review] New %s Keys Added", getKeyType()));
     setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
-    add(RecipientType.TO, Address.create(getEmail()));
+    add(RecipientType.TO, user.getAccountId());
   }
 
   @Override

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

@@ -15,7 +15,6 @@
 package com.google.gerrit.server.mail.send;
 
 import com.google.common.base.Joiner;
-import com.google.gerrit.entities.Address;
 import com.google.gerrit.exceptions.EmailException;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.server.IdentifiedUser;
@@ -71,7 +70,7 @@ public class DeleteKeySender extends OutgoingEmail {
     super.init();
     setHeader("Subject", String.format("[Gerrit Code Review] %s Keys Deleted", getKeyType()));
     setMessageId(messageIdGenerator.fromAccountUpdate(user.getAccountId()));
-    add(RecipientType.TO, Address.create(getEmail()));
+    add(RecipientType.TO, user.getAccountId());
   }
 
   @Override

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

@@ -14,7 +14,6 @@
 
 package com.google.gerrit.server.mail.send;
 
-import com.google.gerrit.entities.Address;
 import com.google.gerrit.exceptions.EmailException;
 import com.google.gerrit.extensions.api.changes.RecipientType;
 import com.google.gerrit.server.IdentifiedUser;
@@ -51,7 +50,7 @@ public class HttpPasswordUpdateSender extends OutgoingEmail {
     setMessageId(
         messageIdGenerator.fromReasonAccountIdAndTimestamp(
             "HTTP_password_change", user.getAccountId(), TimeUtil.now()));
-    add(RecipientType.TO, Address.create(getEmail()));
+    add(RecipientType.TO, user.getAccountId());
   }
 
   @Override

javatests/com/google/gerrit/elasticsearch/ElasticContainer.java

@@ -39,10 +39,6 @@ public class ElasticContainer extends ElasticsearchContainer {
 
   private static String getImageName(ElasticVersion version) {
     switch (version) {
-      case V7_2:
-        return "blacktop/elasticsearch:7.2.1";
-      case V7_3:
-        return "blacktop/elasticsearch:7.3.2";
       case V7_4:
         return "blacktop/elasticsearch:7.4.2";
       case V7_5:

javatests/com/google/gerrit/elasticsearch/ElasticVersionTest.java

@@ -22,12 +22,6 @@ import org.junit.Test;
 public class ElasticVersionTest {
   @Test
   public void supportedVersion() throws Exception {
-    assertThat(ElasticVersion.forVersion("7.2.0")).isEqualTo(ElasticVersion.V7_2);
-    assertThat(ElasticVersion.forVersion("7.2.1")).isEqualTo(ElasticVersion.V7_2);
-
-    assertThat(ElasticVersion.forVersion("7.3.0")).isEqualTo(ElasticVersion.V7_3);
-    assertThat(ElasticVersion.forVersion("7.3.1")).isEqualTo(ElasticVersion.V7_3);
-
     assertThat(ElasticVersion.forVersion("7.4.0")).isEqualTo(ElasticVersion.V7_4);
     assertThat(ElasticVersion.forVersion("7.4.1")).isEqualTo(ElasticVersion.V7_4);
 

tools/maven/gerrit-acceptance-framework_pom.xml

@@ -40,6 +40,9 @@
     <developer>
       <name>Edwin Kempin</name>
     </developer>
+    <developer>
+      <name>Gal Paikin</name>
+    </developer>
     <developer>
       <name>Han-Wen Nienhuys</name>
     </developer>
@@ -49,6 +52,9 @@
     <developer>
       <name>Jacek Centkowski</name>
     </developer>
+    <developer>
+      <name>Joerg Zieren</name>
+    </developer>
     <developer>
       <name>Luca Milanesio</name>
     </developer>
@@ -61,6 +67,9 @@
     <developer>
       <name>Matthias Sohn</name>
     </developer>
+    <developer>
+      <name>Milutin Kristofic</name>
+    </developer>
     <developer>
       <name>Nasser Grainawi</name>
     </developer>

tools/maven/gerrit-extension-api_pom.xml

@@ -40,6 +40,9 @@
     <developer>
       <name>Edwin Kempin</name>
     </developer>
+    <developer>
+      <name>Gal Paikin</name>
+    </developer>
     <developer>
       <name>Han-Wen Nienhuys</name>
     </developer>
@@ -49,6 +52,9 @@
     <developer>
       <name>Jacek Centkowski</name>
     </developer>
+    <developer>
+      <name>Joerg Zieren</name>
+    </developer>
     <developer>
       <name>Luca Milanesio</name>
     </developer>
@@ -61,6 +67,9 @@
     <developer>
       <name>Matthias Sohn</name>
     </developer>
+    <developer>
+      <name>Milutin Kristofic</name>
+    </developer>
     <developer>
       <name>Nasser Grainawi</name>
     </developer>

tools/maven/gerrit-plugin-api_pom.xml

@@ -40,6 +40,9 @@
     <developer>
       <name>Edwin Kempin</name>
     </developer>
+    <developer>
+      <name>Gal Paikin</name>
+    </developer>
     <developer>
       <name>Han-Wen Nienhuys</name>
     </developer>
@@ -49,6 +52,9 @@
     <developer>
       <name>Jacek Centkowski</name>
     </developer>
+    <developer>
+      <name>Joerg Zieren</name>
+    </developer>
     <developer>
       <name>Luca Milanesio</name>
     </developer>
@@ -61,6 +67,9 @@
     <developer>
       <name>Matthias Sohn</name>
     </developer>
+    <developer>
+      <name>Milutin Kristofic</name>
+    </developer>
     <developer>
       <name>Nasser Grainawi</name>
     </developer>

tools/maven/gerrit-war_pom.xml

@@ -40,6 +40,9 @@
     <developer>
       <name>Edwin Kempin</name>
     </developer>
+    <developer>
+      <name>Gal Paikin</name>
+    </developer>
     <developer>
       <name>Han-Wen Nienhuys</name>
     </developer>
@@ -49,6 +52,9 @@
     <developer>
       <name>Jacek Centkowski</name>
     </developer>
+    <developer>
+      <name>Joerg Zieren</name>
+    </developer>
     <developer>
       <name>Luca Milanesio</name>
     </developer>
@@ -61,6 +67,9 @@
     <developer>
       <name>Matthias Sohn</name>
     </developer>
+    <developer>
+      <name>Milutin Kristofic</name>
+    </developer>
     <developer>
       <name>Nasser Grainawi</name>
     </developer>