ReviewerResource: Don't inherit from ChangeResource

Prefer composition to inheritance. This one was particularly confusing
because of the two different users/controls (caller and reviewer).
Continue the disambiguating started by the previous commit and give
methods slightly more descriptive names.

Change-Id: I3ce101d32cb133698c20869649576808f3240d4b
This commit is contained in:
Dave Borowitz
2015-12-04 11:48:18 -05:00
parent 220c45bc17
commit ce5649619c
5 changed files with 40 additions and 24 deletions

View File

@@ -75,7 +75,7 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
throws AuthException, ResourceNotFoundException, OrmException, throws AuthException, ResourceNotFoundException, OrmException,
IOException { IOException {
ChangeControl control = rsrc.getControl(); ChangeControl control = rsrc.getControl();
Change.Id changeId = rsrc.getId(); Change.Id changeId = rsrc.getChangeId();
ReviewDb db = dbProvider.get(); ReviewDb db = dbProvider.get();
ChangeUpdate update = updateFactory.create(rsrc.getControl()); ChangeUpdate update = updateFactory.create(rsrc.getControl());
@@ -103,13 +103,13 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
if (del.isEmpty()) { if (del.isEmpty()) {
throw new ResourceNotFoundException(); throw new ResourceNotFoundException();
} }
ChangeUtil.bumpRowVersionNotLastUpdatedOn(rsrc.getId(), db); ChangeUtil.bumpRowVersionNotLastUpdatedOn(rsrc.getChangeId(), db);
db.patchSetApprovals().delete(del); db.patchSetApprovals().delete(del);
update.removeReviewer(rsrc.getReviewerUser().getAccountId()); update.removeReviewer(rsrc.getReviewerUser().getAccountId());
if (msg.length() > 0) { if (msg.length() > 0) {
ChangeMessage changeMessage = ChangeMessage changeMessage =
new ChangeMessage(new ChangeMessage.Key(rsrc.getId(), new ChangeMessage(new ChangeMessage.Key(rsrc.getChangeId(),
ChangeUtil.messageUUID(db)), ChangeUtil.messageUUID(db)),
control.getUser().getAccountId(), control.getUser().getAccountId(),
TimeUtil.nowTs(), rsrc.getChange().currentPatchSetId()); TimeUtil.nowTs(), rsrc.getChange().currentPatchSetId());
@@ -138,7 +138,8 @@ public class DeleteReviewer implements RestModifyView<ReviewerResource, Input> {
ReviewerResource rsrc) throws OrmException { ReviewerResource rsrc) throws OrmException {
final Account.Id user = rsrc.getReviewerUser().getAccountId(); final Account.Id user = rsrc.getReviewerUser().getAccountId();
return Iterables.filter( return Iterables.filter(
approvalsUtil.byChange(db, rsrc.getNotes()).values(), approvalsUtil.byChange(db, rsrc.getChangeResource().getNotes())
.values(),
new Predicate<PatchSetApproval>() { new Predicate<PatchSetApproval>() {
@Override @Override
public boolean apply(PatchSetApproval input) { public boolean apply(PatchSetApproval input) {

View File

@@ -70,11 +70,9 @@ public class DeleteVote implements RestModifyView<VoteResource, Input> {
public Response<?> apply(VoteResource rsrc, Input input) public Response<?> apply(VoteResource rsrc, Input input)
throws RestApiException, UpdateException { throws RestApiException, UpdateException {
ReviewerResource r = rsrc.getReviewer(); ReviewerResource r = rsrc.getReviewer();
ChangeControl ctl = r.getControl();
Change change = r.getChange(); Change change = r.getChange();
try (BatchUpdate bu = batchUpdateFactory.create(db.get(), try (BatchUpdate bu = batchUpdateFactory.create(db.get(),
change.getProject(), ctl.getUser().asIdentifiedUser(), change.getProject(), r.getControl().getUser(), TimeUtil.nowTs())) {
TimeUtil.nowTs())) {
bu.addOp(change.getId(), bu.addOp(change.getId(),
new Op(r.getReviewerUser().getAccountId(), rsrc.getLabel())); new Op(r.getReviewerUser().getAccountId(), rsrc.getLabel()));
bu.execute(); bu.execute();

View File

@@ -150,10 +150,11 @@ public class PostReviewers implements RestModifyView<ChangeResource, AddReviewer
private PostResult putAccount(ReviewerResource rsrc) throws OrmException, private PostResult putAccount(ReviewerResource rsrc) throws OrmException,
IOException { IOException {
Account member = rsrc.getReviewerUser().getAccount(); Account member = rsrc.getReviewerUser().getAccount();
ChangeControl control = rsrc.getControl(); ChangeControl control = rsrc.getReviewerControl();
PostResult result = new PostResult(); PostResult result = new PostResult();
if (isValidReviewer(member, control)) { if (isValidReviewer(member, control)) {
addReviewers(rsrc, result, ImmutableMap.of(member.getId(), control)); addReviewers(rsrc.getChangeResource(), result,
ImmutableMap.of(member.getId(), control));
} }
return result; return result;
} }

View File

@@ -68,7 +68,7 @@ public class ReviewerJson {
for (ReviewerResource rsrc : rsrcs) { for (ReviewerResource rsrc : rsrcs) {
ReviewerInfo info = format(new ReviewerInfo( ReviewerInfo info = format(new ReviewerInfo(
rsrc.getReviewerUser().getAccountId()), rsrc.getReviewerUser().getAccountId()),
rsrc.getUserControl()); rsrc.getReviewerControl());
loader.put(info); loader.put(info);
infos.add(info); infos.add(info);
} }

View File

@@ -14,48 +14,64 @@
package com.google.gerrit.server.change; package com.google.gerrit.server.change;
import com.google.gerrit.extensions.restapi.RestResource;
import com.google.gerrit.extensions.restapi.RestView; import com.google.gerrit.extensions.restapi.RestView;
import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.IdentifiedUser;
import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ChangeControl;
import com.google.inject.TypeLiteral; import com.google.inject.TypeLiteral;
import com.google.inject.assistedinject.Assisted; import com.google.inject.assistedinject.Assisted;
import com.google.inject.assistedinject.AssistedInject; import com.google.inject.assistedinject.AssistedInject;
public class ReviewerResource extends ChangeResource { public class ReviewerResource implements RestResource {
public static final TypeLiteral<RestView<ReviewerResource>> REVIEWER_KIND = public static final TypeLiteral<RestView<ReviewerResource>> REVIEWER_KIND =
new TypeLiteral<RestView<ReviewerResource>>() {}; new TypeLiteral<RestView<ReviewerResource>>() {};
public static interface Factory { public static interface Factory {
ReviewerResource create(ChangeResource rsrc, IdentifiedUser user); ReviewerResource create(ChangeResource change, Account.Id id);
ReviewerResource create(ChangeResource rsrc, Account.Id id);
} }
private final ChangeResource change;
private final IdentifiedUser user; private final IdentifiedUser user;
@AssistedInject @AssistedInject
ReviewerResource(@Assisted ChangeResource rsrc, ReviewerResource(IdentifiedUser.GenericFactory userFactory,
@Assisted IdentifiedUser user) { @Assisted ChangeResource change,
super(rsrc); @Assisted Account.Id id) {
this.user = user; this.change = change;
this.user = userFactory.create(id);
} }
@AssistedInject public ChangeResource getChangeResource() {
ReviewerResource(IdentifiedUser.GenericFactory userFactory, return change;
@Assisted ChangeResource rsrc, }
@Assisted Account.Id id) {
this(rsrc, userFactory.create(id)); public Change.Id getChangeId() {
return change.getId();
}
public Change getChange() {
return change.getChange();
} }
public IdentifiedUser getReviewerUser() { public IdentifiedUser getReviewerUser() {
return user; return user;
} }
/**
* @return the control for the caller's user (as opposed to the reviewer's
* user as returned by {@link #getReviewerControl()}).
*/
public ChangeControl getControl() {
return change.getControl();
}
/** /**
* @return the control for the reviewer's user (as opposed to the caller's * @return the control for the reviewer's user (as opposed to the caller's
* user as returned by {@link #getControl()}). * user as returned by {@link #getControl()}).
*/ */
public ChangeControl getUserControl() { public ChangeControl getReviewerControl() {
return getControl().forUser(user); return change.getControl().forUser(user);
} }
} }