Merge branch 'stable-2.16' into stable-3.0

* stable-2.16:
  Upgrade elasticsearch-rest-client to 7.3.1
  Add .gitreview file
  DeleteDraftComments: Don't update change modified timestamp
  ChangeIT: set submittableAfterLosingPermissions private
  Rebase: Don't swallow caught exception
  Output NoteDb migration progress to Flogger
  Update git submodules
  DefaultChangeReportFormatter: Make constructor and urlFormatter visible
  StarredChangesUtil: Fix NPE when ref to be deleted doesn't exist
  StarredChangesUtil: Throw LockFailureException on LOCK_FAILURE
  Add test for creating a change on a non-existing base change
  Rebase: Do not fail with 500 ISE if non-existing change is given as base
  Fix detecting changes of parent trees when computing change kind for merge commit
  Remove duplicate descriptions of fields in Requirement JSON entity
  InternalAccountQuery: Add back the oneByExternalId method
  Set version to 2.16.11-SNAPSHOT

Update the newly added .gitreview file to point to stable-3.0.

Change-Id: I717026f02a6a7a14dcab72a166f25c2196a6a6ae

.gitreview

@@ -0,0 +1,5 @@
+[gerrit]
+host=gerrit-review.googlesource.com
+scheme=https
+project=gerrit.git
+defaultbranch=stable-3.0

Documentation/dev-contributing.txt

@@ -144,6 +144,11 @@ by clicking the 'Obtain Password' link on the
 link:https://gerrit-review.googlesource.com/#/settings/http-password[HTTP
 Password tab of the user settings page].
 
+Alternately, you may use the
+link:https://pypi.org/project/git-review/[git-review] tool to submit changes
+to Gerrit. If you do, it will set up the Change-Id hook and `gerrit` remote
+for you. You will still need to do the HTTP access step.
+
 [[style]]
 === Style
 

Documentation/dev-release.txt

@@ -251,7 +251,10 @@ link:https://gerrit-review.googlesource.com/admin/repos/gerrit,branches[
 Gerrit Web UI] or by push.
 
 * Push the commits done on `stable-$version` to `refs/for/stable-$version` and
-get them merged
+get them merged.
+
+* Create a change updating the `defaultbranch` field in the `.gitreview`
+to match the branch name created.
 
 
 [[push-tag]]

Documentation/rest-api-changes.txt

@@ -6831,13 +6831,6 @@ oldest. Empty if there are no related changes.
 === Requirement
 The `Requirement` entity contains information about a requirement relative to a change.
 
-type:: Alphanumerical (plus hyphens or underscores) string to identify what the requirement is and
-why it was triggered. Can be seen as a class: requirements sharing the same type were created for a
-similar reason, and the data structure will follow one set of rules.
-
-data:: (Optional) Additional key-value data linked to this requirement. This is used in templates to
-render rich status messages.
-
 [options="header",cols="1,^1,5"]
 |===========================
 |Field Name      | |Description

WORKSPACE

@@ -1056,8 +1056,8 @@ maven_jar(
 # and httpasyncclient as necessary.
 maven_jar(
     name = "elasticsearch-rest-client",
-    artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.3.0",
-    sha1 = "3cdc211c8efb72c202107b40dee356f4f2f0f9bd",
+    artifact = "org.elasticsearch.client:elasticsearch-rest-client:7.3.1",
+    sha1 = "f5793c89b50a159cbb3e15e17bb981ff854cbe51",
 )
 
 maven_jar(

java/com/google/gerrit/common/UsedAt.java

@@ -31,6 +31,7 @@ public @interface UsedAt {
   /** Enumeration of projects that call a method that would otherwise be private. */
   enum Project {
     GOOGLE,
+    COLLABNET,
     PLUGIN_CHECKS,
     PLUGIN_DELETE_PROJECT,
     PLUGIN_SERVICEUSER,

java/com/google/gerrit/extensions/restapi/UnprocessableEntityException.java

@@ -21,4 +21,8 @@ public class UnprocessableEntityException extends RestApiException {
   public UnprocessableEntityException(String msg) {
     super(msg);
   }
+
+  public UnprocessableEntityException(String msg, Throwable cause) {
+    super(msg, cause);
+  }
 }

java/com/google/gerrit/server/StarredChangesUtil.java

@@ -32,6 +32,7 @@ import com.google.common.flogger.FluentLogger;
 import com.google.common.primitives.Ints;
 import com.google.gerrit.common.Nullable;
 import com.google.gerrit.exceptions.StorageException;
+import com.google.gerrit.git.LockFailureException;
 import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.Project;
@@ -248,10 +249,14 @@ public class StarredChangesUtil {
       batchUpdate.execute(rw, NullProgressMonitor.INSTANCE);
       for (ReceiveCommand command : batchUpdate.getCommands()) {
         if (command.getResult() != ReceiveCommand.Result.OK) {
-          throw new IOException(
+          String message =
               String.format(
                   "Unstar change %d failed, ref %s could not be deleted: %s",
-                  changeId.get(), command.getRefName(), command.getResult()));
+                  changeId.get(), command.getRefName(), command.getResult());
+          if (command.getResult() == ReceiveCommand.Result.LOCK_FAILURE) {
+            throw new LockFailureException(message, batchUpdate);
+          }
+          throw new IOException(message);
         }
       }
       indexer.index(project, changeId);

java/com/google/gerrit/server/change/ChangeKindCacheImpl.java

@@ -211,13 +211,13 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
         rw.parseBody(next);
 
         if (!next.getFullMessage().equals(prior.getFullMessage())) {
-          if (isSameDeltaAndTree(prior, next)) {
+          if (isSameDeltaAndTree(rw, prior, next)) {
             return ChangeKind.NO_CODE_CHANGE;
           }
           return ChangeKind.REWORK;
         }
 
-        if (isSameDeltaAndTree(prior, next)) {
+        if (isSameDeltaAndTree(rw, prior, next)) {
           return ChangeKind.NO_CHANGE;
         }
 
@@ -281,7 +281,8 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
       return FluentIterable.from(Arrays.asList(parents)).skip(1).toSet();
     }
 
-    private static boolean isSameDeltaAndTree(RevCommit prior, RevCommit next) {
+    private static boolean isSameDeltaAndTree(RevWalk rw, RevCommit prior, RevCommit next)
+        throws IOException {
       if (!Objects.equals(next.getTree(), prior.getTree())) {
         return false;
       }
@@ -295,6 +296,10 @@ public class ChangeKindCacheImpl implements ChangeKindCache {
       // Make sure that the prior/next delta is the same - not just the tree.
       // This is done by making sure that the parent trees are equal.
       for (int i = 0; i < prior.getParentCount(); i++) {
+        // Parse parent commits so that their trees are available.
+        rw.parseCommit(prior.getParent(i));
+        rw.parseCommit(next.getParent(i));
+
         if (!Objects.equals(next.getParent(i).getTree(), prior.getParent(i).getTree())) {
           return false;
         }

java/com/google/gerrit/server/git/DefaultChangeReportFormatter.java

@@ -29,10 +29,10 @@ public class DefaultChangeReportFormatter implements ChangeReportFormatter {
   private static final int SUBJECT_CROP_RANGE = 10;
   private static final String NEW_CHANGE_INDICATOR = " [NEW]";
 
-  private final DynamicItem<UrlFormatter> urlFormatter;
+  protected final DynamicItem<UrlFormatter> urlFormatter;
 
   @Inject
-  DefaultChangeReportFormatter(DynamicItem<UrlFormatter> urlFormatter) {
+  public DefaultChangeReportFormatter(DynamicItem<UrlFormatter> urlFormatter) {
     this.urlFormatter = urlFormatter;
   }
 

java/com/google/gerrit/server/query/account/InternalAccountQuery.java

@@ -17,10 +17,13 @@ package com.google.gerrit.server.query.account;
 import static java.util.stream.Collectors.toList;
 import static java.util.stream.Collectors.toSet;
 
+import com.google.common.base.Joiner;
 import com.google.common.collect.ArrayListMultimap;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableListMultimap;
 import com.google.common.collect.Multimap;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.common.UsedAt;
 import com.google.gerrit.index.FieldDef;
 import com.google.gerrit.index.IndexConfig;
 import com.google.gerrit.index.Schema;
@@ -42,6 +45,8 @@ import java.util.Set;
  * holding on to a single instance.
  */
 public class InternalAccountQuery extends InternalQuery<AccountState, InternalAccountQuery> {
+  private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
   @Inject
   InternalAccountQuery(
       AccountQueryProcessor queryProcessor,
@@ -62,6 +67,22 @@ public class InternalAccountQuery extends InternalQuery<AccountState, InternalAc
     return query(AccountPredicates.externalIdIncludingSecondaryEmails(externalId.toString()));
   }
 
+  @UsedAt(UsedAt.Project.COLLABNET)
+  public AccountState oneByExternalId(ExternalId.Key externalId) {
+    List<AccountState> accountStates = byExternalId(externalId);
+    if (accountStates.size() == 1) {
+      return accountStates.get(0);
+    } else if (accountStates.size() > 0) {
+      StringBuilder msg = new StringBuilder();
+      msg.append("Ambiguous external ID ").append(externalId).append(" for accounts: ");
+      Joiner.on(", ")
+          .appendTo(
+              msg, accountStates.stream().map(AccountState.ACCOUNT_ID_FUNCTION).collect(toList()));
+      logger.atWarning().log(msg.toString());
+    }
+    return null;
+  }
+
   public List<AccountState> byFullName(String fullName) {
     return query(AccountPredicates.fullName(fullName));
   }

java/com/google/gerrit/server/restapi/change/Rebase.java

@@ -25,6 +25,7 @@ import com.google.gerrit.extensions.restapi.AuthException;
 import com.google.gerrit.extensions.restapi.ResourceConflictException;
 import com.google.gerrit.extensions.restapi.RestApiException;
 import com.google.gerrit.extensions.restapi.RestModifyView;
+import com.google.gerrit.extensions.restapi.UnprocessableEntityException;
 import com.google.gerrit.extensions.webui.UiAction;
 import com.google.gerrit.reviewdb.client.Branch;
 import com.google.gerrit.reviewdb.client.Change;
@@ -154,11 +155,18 @@ public class Rebase extends RetryingRestModifyView<RevisionResource, RebaseInput
       return destRef.getObjectId();
     }
 
-    Base base = rebaseUtil.parseBase(rsrc, str);
-    if (base == null) {
-      throw new ResourceConflictException(
-          "base revision is missing from the destination branch: " + str);
+    Base base;
+    try {
+      base = rebaseUtil.parseBase(rsrc, str);
+      if (base == null) {
+        throw new ResourceConflictException(
+            "base revision is missing from the destination branch: " + str);
+      }
+    } catch (NoSuchChangeException e) {
+      throw new UnprocessableEntityException(
+          String.format("Base change not found: %s", input.base), e);
     }
+
     PatchSet.Id baseId = base.patchSet().getId();
     if (change.getId().equals(baseId.getParentKey())) {
       throw new ResourceConflictException("cannot rebase change onto itself");

javatests/com/google/gerrit/acceptance/api/change/ChangeIT.java

@@ -75,10 +75,12 @@ import com.google.gerrit.common.data.LabelFunction;
 import com.google.gerrit.common.data.LabelType;
 import com.google.gerrit.common.data.Permission;
 import com.google.gerrit.extensions.annotations.Exports;
+import com.google.gerrit.extensions.api.accounts.DeleteDraftCommentsInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerInput;
 import com.google.gerrit.extensions.api.changes.AddReviewerResult;
 import com.google.gerrit.extensions.api.changes.DeleteReviewerInput;
 import com.google.gerrit.extensions.api.changes.DeleteVoteInput;
+import com.google.gerrit.extensions.api.changes.DraftApi;
 import com.google.gerrit.extensions.api.changes.DraftInput;
 import com.google.gerrit.extensions.api.changes.NotifyHandling;
 import com.google.gerrit.extensions.api.changes.NotifyInfo;
@@ -830,6 +832,16 @@ public class ChangeIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).current().rebase();
   }
 
+  @Test
+  public void rebaseOnNonExistingChange() throws Exception {
+    String changeId = createChange().getChangeId();
+    RebaseInput in = new RebaseInput();
+    in.base = "999999";
+    exception.expect(UnprocessableEntityException.class);
+    exception.expectMessage("Base change not found: " + in.base);
+    gApi.changes().id(changeId).rebase(in);
+  }
+
   @Test
   public void rebaseOnChangeNumber() throws Exception {
     String branchTip = testRepo.getRepository().exactRef("HEAD").getObjectId().name();
@@ -3803,7 +3815,7 @@ public class ChangeIT extends AbstractDaemonTest {
     submittableAfterLosingPermissions("Label");
   }
 
-  public void submittableAfterLosingPermissions(String label) throws Exception {
+  private void submittableAfterLosingPermissions(String label) throws Exception {
     String codeReviewLabel = "Code-Review";
     AccountGroup.UUID registered = SystemGroupBackend.REGISTERED_USERS;
     try (ProjectConfigUpdate u = updateProject(project)) {
@@ -3858,6 +3870,46 @@ public class ChangeIT extends AbstractDaemonTest {
     gApi.changes().id(changeId).current().submit();
   }
 
+  @Test
+  public void draftCommentsShouldNotUpdateChangeTimestamp() throws Exception {
+    String changeId = createNewChange();
+    Timestamp changeTs = getChangeLastUpdate(changeId);
+    DraftApi draftApi = addDraftComment(changeId);
+    assertThat(getChangeLastUpdate(changeId)).isEqualTo(changeTs);
+    draftApi.delete();
+    assertThat(getChangeLastUpdate(changeId)).isEqualTo(changeTs);
+  }
+
+  @Test
+  public void deletingAllDraftCommentsShouldNotUpdateChangeTimestamp() throws Exception {
+    String changeId = createNewChange();
+    Timestamp changeTs = getChangeLastUpdate(changeId);
+    addDraftComment(changeId);
+    assertThat(getChangeLastUpdate(changeId)).isEqualTo(changeTs);
+    gApi.accounts().self().deleteDraftComments(new DeleteDraftCommentsInput());
+    assertThat(getChangeLastUpdate(changeId)).isEqualTo(changeTs);
+  }
+
+  private Timestamp getChangeLastUpdate(String changeId) throws RestApiException {
+    Timestamp changeTs = gApi.changes().id(changeId).get().updated;
+    return changeTs;
+  }
+
+  private String createNewChange() throws Exception {
+    TestRepository<InMemoryRepository> userRepo = cloneProject(project, user);
+    PushOneCommit.Result result =
+        pushFactory.create(user.newIdent(), userRepo).to("refs/for/master");
+    String changeId = result.getChangeId();
+    return changeId;
+  }
+
+  private DraftApi addDraftComment(String changeId) throws RestApiException {
+    DraftInput comment = new DraftInput();
+    comment.message = "foo";
+    comment.path = "/foo";
+    return gApi.changes().id(changeId).current().createDraft(comment);
+  }
+
   private String getCommitMessage(String changeId) throws RestApiException, IOException {
     return gApi.changes().id(changeId).current().file("/COMMIT_MSG").content().asString();
   }

javatests/com/google/gerrit/acceptance/api/change/StickyApprovalsIT.java

@@ -250,6 +250,23 @@ public class StickyApprovalsIT extends AbstractDaemonTest {
     assertNotSticky(EnumSet.of(REWORK, NO_CODE_CHANGE, TRIVIAL_REBASE));
   }
 
+  @Test
+  public void notStickyWithCopyOnNoChangeWhenSecondParentIsUpdated() throws Exception {
+    try (ProjectConfigUpdate u = updateProject(project)) {
+      u.getConfig().getLabelSections().get("Code-Review").setCopyAllScoresIfNoChange(true);
+      u.save();
+    }
+
+    String changeId = createChangeForMergeCommit();
+    vote(admin, changeId, 2, 1);
+    vote(user, changeId, -2, -1);
+
+    updateSecondParent(changeId);
+    ChangeInfo c = detailedChange(changeId);
+    assertVotes(c, admin, 0, 0, null);
+    assertVotes(c, user, 0, 0, null);
+  }
+
   @Test
   public void removedVotesNotSticky() throws Exception {
     try (ProjectConfigUpdate u = updateProject(project)) {
@@ -513,6 +530,24 @@ public class StickyApprovalsIT extends AbstractDaemonTest {
     assertThat(getChangeKind(changeId)).isEqualTo(MERGE_FIRST_PARENT_UPDATE);
   }
 
+  private void updateSecondParent(String changeId) throws Exception {
+    ChangeInfo c = detailedChange(changeId);
+    List<CommitInfo> parents = c.revisions.get(c.currentRevision).commit.parents;
+    String parent1 = parents.get(0).commit;
+    String parent2 = parents.get(1).commit;
+    RevCommit commitParent1 = testRepo.getRevWalk().parseCommit(ObjectId.fromString(parent1));
+
+    testRepo.reset(parent2);
+    PushOneCommit.Result newParent2 = createChange("new parent 2", "p2-2.txt", "content 2-2");
+
+    PushOneCommit merge = pushFactory.create(admin.newIdent(), testRepo, changeId);
+    merge.setParents(ImmutableList.of(commitParent1, newParent2.getCommit()));
+    PushOneCommit.Result result = merge.to("refs/for/master");
+    result.assertOkStatus();
+
+    assertThat(getChangeKind(changeId)).isEqualTo(REWORK);
+  }
+
   private String cherryPick(String changeId, ChangeKind changeKind) throws Exception {
     switch (changeKind) {
       case REWORK:

javatests/com/google/gerrit/acceptance/rest/change/CreateChangeIT.java

@@ -250,6 +250,14 @@ public class CreateChangeIT extends AbstractDaemonTest {
         String.format("Commit %s doesn't exist on ref refs/heads/foo", input.baseCommit));
   }
 
+  @Test
+  public void createChangeOnNonExistingBaseChangeFails() throws Exception {
+    ChangeInput input = newChangeInput(ChangeStatus.NEW);
+    input.baseChange = "999999";
+    assertCreateFails(
+        input, UnprocessableEntityException.class, "Base change not found: " + input.baseChange);
+  }
+
   @Test
   public void createChangeWithoutAccessToParentCommitFails() throws Exception {
     Map<String, PushOneCommit.Result> results =

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

@@ -59,7 +59,7 @@ public class ElasticContainer extends ElasticsearchContainer {
       case V7_2:
         return "blacktop/elasticsearch:7.2.1";
       case V7_3:
-        return "blacktop/elasticsearch:7.3.0";
+        return "blacktop/elasticsearch:7.3.1";
     }
     throw new IllegalStateException("No tests for version: " + version.name());
   }

plugins/replication

@@ -1 +1 @@
-Subproject commit 3054e61afb6ebf18dbddcbe6a2fbcf4196f0039f
+Subproject commit 82682da3eca5dae923bfb404280788a796391f1a