Merge branch 'stable-2.11' into stable-2.12

* stable-2.11:
  Set version to 2.11.7
  Release notes for Gerrit 2.11.7
  Document new allowrcpt restriction on adding user email
  OutputStreamQuery: fix comments with current-patch-set option
  Add tests for ssh query command
  EmailReviewComments: Provide the current user instead of exception
  OutputStreamQuery: Take files into account when adding patch sets
  Update 2.11.6 release notes

Change-Id: I6b24624da6af024e340a672b2ac527d5510f5219

Documentation/rest-api-accounts.txt

@@ -496,6 +496,9 @@ For the development mode email addresses are directly added without
 confirmation. A Gerrit administrator may add an email address without
 confirmation by setting `no_confirmation` in the
 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.
 
 In the request body additional data for the email address can be
 provided as link:#email-input[EmailInput].

ReleaseNotes/ReleaseNotes-2.11.6.txt

@@ -90,6 +90,10 @@ to new Gerrit version.
 The default was 'No' which resulted in some sites not upgrading core
 plugins and running the wrong versions.
 
+* Fix reading of plugin documentation.
++
+Under some circumstances it was possible to fail with an IO error.
+
 * Replication
 
 ** Recursively include parent groups of groups specified in `authGroup`.

ReleaseNotes/ReleaseNotes-2.11.7.txt

@@ -0,0 +1,44 @@
+Release notes for Gerrit 2.11.7
+===============================
+
+Gerrit 2.11.7 is now available:
+
+link:https://gerrit-releases.storage.googleapis.com/gerrit-2.11.7.war[
+https://gerrit-releases.storage.googleapis.com/gerrit-2.11.7.war]
+
+There are no schema changes from link:ReleaseNotes-2.11.6.html[2.11.6].
+
+Bug Fixes
+---------
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3882[Issue 3882]:
+Fix 'No user on email thread' exception when label with group parameter is
+used in watched projects predicate.
+
+* link:https://code.google.com/p/gerrit/issues/detail?id=3877[Issue 3877]:
+Include files in output when using `gerrit query` with combination of
+search operators.
++
+A regression introduced in version 2.11.6 caused files to be omitted
+in the output.
+
+* Include comments in output when using `gerrit query` with the
+`--current-patch-set` option.
++
+Comments were added at the change level but were not added at the
+patch set level.
+
+* Honor the `sendemail.allowrcpt` setting when adding new email address.
++
+When adding a new email address via the UI or REST API, it was possible for
+the user to add an address that does not belong to a domain allowed by the
+`sendemail.allowrcpt` configuration. However, when sending the verification
+email, the recipient address was (correctly) dropped, and the email had no
+recipients. This resulted in an error from the SMTP server and an 'Internal
+server error' message to the user.
+
+* Remove unnecessary log messages.
++
+The messages 'Assuming empty group membership' and 'Skipping delivery of
+email' do not add any value and were filling up the error log.
+

ReleaseNotes/index.txt

@@ -9,6 +9,7 @@ Version 2.12.x
 [[2_11]]
 Version 2.11.x
 --------------
+* link:ReleaseNotes-2.11.7.html[2.11.7]
 * link:ReleaseNotes-2.11.6.html[2.11.6]
 * link:ReleaseNotes-2.11.5.html[2.11.5]
 * link:ReleaseNotes-2.11.4.html[2.11.4]

gerrit-acceptance-tests/src/test/java/com/google/gerrit/acceptance/ssh/QueryIT.java

@@ -0,0 +1,320 @@
+// Copyright (C) 2016 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.ssh;
+
+import static com.google.common.truth.Truth.assertThat;
+import static com.google.common.truth.Truth.assert_;
+
+import com.google.common.collect.Lists;
+import com.google.gerrit.acceptance.AbstractDaemonTest;
+import com.google.gerrit.acceptance.NoHttpd;
+import com.google.gerrit.acceptance.PushOneCommit;
+import com.google.gerrit.extensions.api.changes.AddReviewerInput;
+import com.google.gerrit.extensions.api.changes.ReviewInput;
+import com.google.gerrit.extensions.client.Side;
+import com.google.gerrit.server.data.ChangeAttribute;
+import com.google.gson.Gson;
+
+import org.junit.Test;
+
+import java.util.ArrayList;
+import java.util.HashMap;
+import java.util.List;
+
+@NoHttpd
+public class QueryIT extends AbstractDaemonTest {
+
+  private static Gson gson = new Gson();
+
+  @Test
+  public void testBasicQueryJSON() throws Exception {
+    String changeId1 = createChange().getChangeId();
+    String changeId2 = createChange().getChangeId();
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery("1234");
+    assertThat(changes.size()).isEqualTo(0);
+
+    changes = executeSuccessfulQuery(changeId1);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).project).isEqualTo(project.toString());
+    assertThat(changes.get(0).id).isEqualTo(changeId1);
+
+    changes = executeSuccessfulQuery(changeId1 + " OR " + changeId2);
+    assertThat(changes.size()).isEqualTo(2);
+    assertThat(changes.get(0).project).isEqualTo(project.toString());
+    assertThat(changes.get(0).id).isEqualTo(changeId2);
+    assertThat(changes.get(1).project).isEqualTo(project.toString());
+    assertThat(changes.get(1).id).isEqualTo(changeId1);
+
+    changes =
+        executeSuccessfulQuery("--start=1 " + changeId1 + " OR " + changeId2);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).project).isEqualTo(project.toString());
+    assertThat(changes.get(0).id).isEqualTo(changeId1);
+  }
+
+  @Test
+  public void testAllApprovalsOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNull();
+
+    changes = executeSuccessfulQuery("--all-approvals " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testAllReviewersOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    AddReviewerInput in = new AddReviewerInput();
+    in.reviewer = user.email;
+    gApi.changes().id(changeId).addReviewer(in);
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).allReviewers).isNull();
+
+    changes = executeSuccessfulQuery("--all-reviewers " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).allReviewers).isNotNull();
+    assertThat(changes.get(0).allReviewers.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommitMessageOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--commit-message " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).commitMessage).isNotNull();
+    assertThat(changes.get(0).commitMessage).contains(PushOneCommit.SUBJECT);
+  }
+
+  @Test
+  public void testCurrentPatchSetOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    amendChange(changeId);
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet).isNull();
+
+    changes = executeSuccessfulQuery("--current-patch-set " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.number).isEqualTo("2");
+
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    changes = executeSuccessfulQuery("--current-patch-set " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.approvals).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.approvals.size()).isEqualTo(1);
+
+  }
+
+  @Test
+  public void testPatchSetsOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    amendChange(changeId);
+    amendChange(changeId);
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNull();
+
+    changes = executeSuccessfulQuery("--patch-sets " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets).isNotNull();
+    assertThat(changes.get(0).patchSets.size()).isEqualTo(3);
+  }
+
+  @Test
+  public void shouldFailWithFilesWithoutPatchSetsOrCurrentPatchSetsOption()
+      throws Exception {
+    String changeId = createChange().getChangeId();
+    sshSession.exec("gerrit query --files " + changeId);
+    assertThat(sshSession.hasError()).isTrue();
+    assertThat(sshSession.getError()).contains(
+        "needs --patch-sets or --current-patch-set");
+  }
+
+  @Test
+  public void testFileOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--current-patch-set --files " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet.files).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.files.size()).isEqualTo(2);
+
+    changes = executeSuccessfulQuery("--patch-sets --files " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    changes =
+        executeSuccessfulQuery("--patch-sets --files --all-approvals "
+            + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+    assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommentOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).comments).isNull();
+
+    changes = executeSuccessfulQuery("--comments " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).comments).isNotNull();
+    assertThat(changes.get(0).comments.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommentOptionsInCurrentPatchSetJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    ReviewInput review = new ReviewInput();
+    ReviewInput.CommentInput comment = new ReviewInput.CommentInput();
+    comment.path = PushOneCommit.FILE_NAME;
+    comment.side = Side.REVISION;
+    comment.message = "comment 1";
+    review.comments = new HashMap<>();
+    review.comments.put(comment.path, Lists.newArrayList(comment));
+    gApi.changes().id(changeId).current().review(review);
+
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--current-patch-set " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet.comments).isNull();
+
+    changes =
+        executeSuccessfulQuery("--current-patch-set --comments " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).currentPatchSet.comments).isNotNull();
+    assertThat(changes.get(0).currentPatchSet.comments.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testCommentOptionInPatchSetsJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+
+    ReviewInput review = new ReviewInput();
+    ReviewInput.CommentInput comment = new ReviewInput.CommentInput();
+    comment.path = PushOneCommit.FILE_NAME;
+    comment.side = Side.REVISION;
+    comment.message = "comment 1";
+    review.comments = new HashMap<>();
+    review.comments.put(comment.path, Lists.newArrayList(comment));
+    gApi.changes().id(changeId).current().review(review);
+
+    List<ChangeAttribute> changes =
+        executeSuccessfulQuery("--patch-sets " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNull();
+
+    changes = executeSuccessfulQuery("--patch-sets --comments " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1);
+
+    changes =
+        executeSuccessfulQuery("--patch-sets --comments --files " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+
+    gApi.changes().id(changeId).current().review(ReviewInput.approve());
+    changes =
+        executeSuccessfulQuery("--patch-sets --comments --files --all-approvals "
+            + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).comments).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).comments.size()).isEqualTo(1);
+    assertThat(changes.get(0).patchSets.get(0).files).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).files.size()).isEqualTo(2);
+    assertThat(changes.get(0).patchSets.get(0).approvals).isNotNull();
+    assertThat(changes.get(0).patchSets.get(0).approvals.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testDependenciesOptionJSON() throws Exception {
+    String changeId1 = createChange().getChangeId();
+    String changeId2 = createChange().getChangeId();
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId1);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNull();
+
+    changes = executeSuccessfulQuery("--dependencies " + changeId1);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNull();
+
+    changes = executeSuccessfulQuery(changeId2);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNull();
+
+    changes = executeSuccessfulQuery("--dependencies " + changeId2);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).dependsOn).isNotNull();
+    assertThat(changes.get(0).dependsOn.size()).isEqualTo(1);
+  }
+
+  @Test
+  public void testSubmitRecordsOptionJSON() throws Exception {
+    String changeId = createChange().getChangeId();
+    List<ChangeAttribute> changes = executeSuccessfulQuery(changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).submitRecords).isNull();
+
+    changes = executeSuccessfulQuery("--submit-records " + changeId);
+    assertThat(changes.size()).isEqualTo(1);
+    assertThat(changes.get(0).submitRecords).isNotNull();
+    assertThat(changes.get(0).submitRecords.size()).isEqualTo(1);
+  }
+
+  private List<ChangeAttribute> executeSuccessfulQuery(String params)
+      throws Exception {
+    String rawResponse =
+        sshSession.exec("gerrit query --format=JSON " + params);
+    assert_().withFailureMessage(sshSession.getError())
+        .that(sshSession.hasError()).isFalse();
+    return getChanges(rawResponse);
+  }
+
+  private static List<ChangeAttribute> getChanges(String rawResponse) {
+    String[] lines = rawResponse.split("\\n");
+    List<ChangeAttribute> changes = new ArrayList<>(lines.length - 1);
+    for (int i = 0; i < lines.length - 1; i++) {
+      changes.add(gson.fromJson(lines[i], ChangeAttribute.class));
+    }
+    return changes;
+  }
+}

gerrit-server/src/main/java/com/google/gerrit/server/change/EmailReviewComments.java

@@ -17,13 +17,13 @@ package com.google.gerrit.server.change;
 import static com.google.gerrit.server.PatchLineCommentsUtil.PLC_ORDER;
 
 import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling;
-import com.google.gerrit.reviewdb.client.Account;
 import com.google.gerrit.reviewdb.client.Change;
 import com.google.gerrit.reviewdb.client.ChangeMessage;
 import com.google.gerrit.reviewdb.client.PatchLineComment;
 import com.google.gerrit.reviewdb.client.PatchSet;
 import com.google.gerrit.reviewdb.server.ReviewDb;
 import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.IdentifiedUser;
 import com.google.gerrit.server.git.SendEmailExecutor;
 import com.google.gerrit.server.mail.CommentSender;
 import com.google.gerrit.server.patch.PatchSetInfoFactory;
@@ -32,7 +32,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext;
 import com.google.gwtorm.server.OrmException;
 import com.google.gwtorm.server.SchemaFactory;
 import com.google.inject.Inject;
-import com.google.inject.OutOfScopeException;
 import com.google.inject.Provider;
 import com.google.inject.ProvisionException;
 import com.google.inject.assistedinject.Assisted;
@@ -51,7 +50,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
         NotifyHandling notify,
         Change change,
         PatchSet patchSet,
-        Account.Id authorId,
+        IdentifiedUser user,
         ChangeMessage message,
         List<PatchLineComment> comments);
   }
@@ -65,13 +64,13 @@ public class EmailReviewComments implements Runnable, RequestContext {
   private final NotifyHandling notify;
   private final Change change;
   private final PatchSet patchSet;
-  private final Account.Id authorId;
+  private final IdentifiedUser user;
   private final ChangeMessage message;
   private List<PatchLineComment> comments;
   private ReviewDb db;
 
   @Inject
-  EmailReviewComments (
+  EmailReviewComments(
       @SendEmailExecutor ExecutorService executor,
       PatchSetInfoFactory patchSetInfoFactory,
       CommentSender.Factory commentSenderFactory,
@@ -80,7 +79,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
       @Assisted NotifyHandling notify,
       @Assisted Change change,
       @Assisted PatchSet patchSet,
-      @Assisted Account.Id authorId,
+      @Assisted IdentifiedUser user,
       @Assisted ChangeMessage message,
       @Assisted List<PatchLineComment> comments) {
     this.sendEmailsExecutor = executor;
@@ -91,7 +90,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
     this.notify = notify;
     this.change = change;
     this.patchSet = patchSet;
-    this.authorId = authorId;
+    this.user = user;
     this.message = message;
     this.comments = PLC_ORDER.sortedCopy(comments);
   }
@@ -106,7 +105,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
     try {
 
       CommentSender cm = commentSenderFactory.create(notify, change.getId());
-      cm.setFrom(authorId);
+      cm.setFrom(user.getAccountId());
       cm.setPatchSet(patchSet, patchSetInfoFactory.get(change, patchSet));
       cm.setChangeMessage(message);
       cm.setPatchLineComments(comments);
@@ -129,7 +128,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
 
   @Override
   public CurrentUser getUser() {
-    throw new OutOfScopeException("No user on email thread");
+    return user.getRealUser();
   }
 
   @Override

gerrit-server/src/main/java/com/google/gerrit/server/change/PostReview.java

@@ -376,7 +376,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
             in.notify,
             change,
             ps,
-            user.getAccountId(),
+            user,
             message,
             comments).sendAsync();
       }

gerrit-server/src/main/java/com/google/gerrit/server/query/change/OutputStreamQuery.java

@@ -293,6 +293,10 @@ public class OutputStreamQuery {
           eventFactory.addPatchSetFileNames(c.currentPatchSet,
               d.change(), d.currentPatchSet());
         }
+        if (includeComments) {
+          eventFactory.addPatchSetComments(c.currentPatchSet,
+              d.publishedComments());
+        }
       }
     }
 
@@ -301,7 +305,7 @@ public class OutputStreamQuery {
       if (includePatchSets) {
         eventFactory.addPatchSets(db.get(), rw, c, d.patchSets(),
             includeApprovals ? d.approvals().asMap() : null,
-            labelTypes);
+            includeFiles, d.change(), labelTypes);
         for (PatchSetAttribute attribute : c.patchSets) {
           eventFactory.addPatchSetComments(
               attribute, d.publishedComments());