EmailReviewComments: Provide the current user instead of exception

The review comments are sent in a separate thread, and when invoked by
a context that needs the current user it fails because getCurrentUser()
raises OutOfScopeException.

Modify EmailReciewComments sender and its Factory.create() to accept an
IdentifiedUser rather than an Account.Id, and use it to return from the
getCurrentUser() method instead of throwing an exception.

Test plan:

- Create a group named "test-group", visible to all users.
- Create a project named "test-project".
- As "user-a", add a project watch on "test-project" with the
  search query "label:Code-Review=+1,group=test-group".
- As "user-b", add a Code-Review+1 score on a change on the
  "test-project" project.

Bug: Issue 3882
Change-Id: Iebcded01d7f72156f2b292d7b55848efc6e03dc2
This commit is contained in:
David Pursehouse
2016-02-08 17:10:45 +09:00
parent 478a04fbcb
commit e39cb56881
2 changed files with 9 additions and 10 deletions

View File

@@ -16,13 +16,13 @@ package com.google.gerrit.server.change;
import com.google.common.collect.Lists; import com.google.common.collect.Lists;
import com.google.gerrit.extensions.api.changes.ReviewInput.NotifyHandling; 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.Change;
import com.google.gerrit.reviewdb.client.ChangeMessage; import com.google.gerrit.reviewdb.client.ChangeMessage;
import com.google.gerrit.reviewdb.client.PatchLineComment; import com.google.gerrit.reviewdb.client.PatchLineComment;
import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.PatchSet;
import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.git.EmailReviewCommentsExecutor; import com.google.gerrit.server.git.EmailReviewCommentsExecutor;
import com.google.gerrit.server.git.WorkQueue.Executor; import com.google.gerrit.server.git.WorkQueue.Executor;
import com.google.gerrit.server.mail.CommentSender; import com.google.gerrit.server.mail.CommentSender;
@@ -32,7 +32,6 @@ import com.google.gerrit.server.util.ThreadLocalRequestContext;
import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.SchemaFactory; import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject; import com.google.inject.Inject;
import com.google.inject.OutOfScopeException;
import com.google.inject.Provider; import com.google.inject.Provider;
import com.google.inject.ProvisionException; import com.google.inject.ProvisionException;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
@@ -52,7 +51,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
NotifyHandling notify, NotifyHandling notify,
Change change, Change change,
PatchSet patchSet, PatchSet patchSet,
Account.Id authorId, IdentifiedUser user,
ChangeMessage message, ChangeMessage message,
List<PatchLineComment> comments); List<PatchLineComment> comments);
} }
@@ -66,13 +65,13 @@ public class EmailReviewComments implements Runnable, RequestContext {
private final NotifyHandling notify; private final NotifyHandling notify;
private final Change change; private final Change change;
private final PatchSet patchSet; private final PatchSet patchSet;
private final Account.Id authorId; private final IdentifiedUser user;
private final ChangeMessage message; private final ChangeMessage message;
private List<PatchLineComment> comments; private List<PatchLineComment> comments;
private ReviewDb db; private ReviewDb db;
@Inject @Inject
EmailReviewComments ( EmailReviewComments(
@EmailReviewCommentsExecutor final Executor executor, @EmailReviewCommentsExecutor final Executor executor,
PatchSetInfoFactory patchSetInfoFactory, PatchSetInfoFactory patchSetInfoFactory,
CommentSender.Factory commentSenderFactory, CommentSender.Factory commentSenderFactory,
@@ -81,7 +80,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
@Assisted NotifyHandling notify, @Assisted NotifyHandling notify,
@Assisted Change change, @Assisted Change change,
@Assisted PatchSet patchSet, @Assisted PatchSet patchSet,
@Assisted Account.Id authorId, @Assisted IdentifiedUser user,
@Assisted ChangeMessage message, @Assisted ChangeMessage message,
@Assisted List<PatchLineComment> comments) { @Assisted List<PatchLineComment> comments) {
this.sendEmailsExecutor = executor; this.sendEmailsExecutor = executor;
@@ -92,7 +91,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
this.notify = notify; this.notify = notify;
this.change = change; this.change = change;
this.patchSet = patchSet; this.patchSet = patchSet;
this.authorId = authorId; this.user = user;
this.message = message; this.message = message;
this.comments = comments; this.comments = comments;
} }
@@ -130,7 +129,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
}); });
CommentSender cm = commentSenderFactory.create(notify, change); CommentSender cm = commentSenderFactory.create(notify, change);
cm.setFrom(authorId); cm.setFrom(user.getAccountId());
cm.setPatchSet(patchSet, patchSetInfoFactory.get(change, patchSet)); cm.setPatchSet(patchSet, patchSetInfoFactory.get(change, patchSet));
cm.setChangeMessage(message); cm.setChangeMessage(message);
cm.setPatchLineComments(comments); cm.setPatchLineComments(comments);
@@ -153,7 +152,7 @@ public class EmailReviewComments implements Runnable, RequestContext {
@Override @Override
public CurrentUser getCurrentUser() { public CurrentUser getCurrentUser() {
throw new OutOfScopeException("No user on email thread"); return user.getRealUser();
} }
@Override @Override

View File

@@ -191,7 +191,7 @@ public class PostReview implements RestModifyView<RevisionResource, ReviewInput>
input.notify, input.notify,
change, change,
revision.getPatchSet(), revision.getPatchSet(),
revision.getAccountId(), revision.getUser(),
message, message,
comments).sendAsync(); comments).sendAsync();
} }