diff --git a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java index b3b4a300c8..ebf5f9e361 100644 --- a/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java +++ b/gerrit-common/src/main/java/com/google/gerrit/common/data/PatchDetailService.java @@ -28,7 +28,6 @@ import com.google.gwtjsonrpc.common.RpcImpl; import com.google.gwtjsonrpc.common.RpcImpl.Version; import com.google.gwtjsonrpc.common.VoidResult; -import java.util.List; import java.util.Set; @RpcImpl(version = Version.V2_0) @@ -63,16 +62,6 @@ public interface PatchDetailService extends RemoteJsonService { @SignInRequired void deleteDraftPatchSet(PatchSet.Id psid, AsyncCallback callback); - @Audit - @SignInRequired - void addReviewers(Change.Id id, List reviewers, boolean confirmed, - AsyncCallback callback); - - @Audit - @SignInRequired - void removeReviewer(Change.Id id, Account.Id reviewerId, - AsyncCallback callback); - void userApprovals(Set cids, Account.Id aid, AsyncCallback callback); diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java index 72cd3ccd8c..559c270270 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/SuggestServiceImpl.java @@ -33,8 +33,8 @@ import com.google.gerrit.server.account.AccountControl; import com.google.gerrit.server.account.AccountVisibility; import com.google.gerrit.server.account.GroupBackend; import com.google.gerrit.server.account.GroupMembers; +import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gerrit.server.patch.AddReviewer; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.NoSuchProjectException; @@ -273,7 +273,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements private boolean suggestGroupAsReviewer(final Project.NameKey project, final GroupReference group) throws OrmException { - if (!AddReviewer.isLegalReviewerGroup(group.getUUID())) { + if (!PostReviewers.isLegalReviewerGroup(group.getUUID())) { return false; } @@ -287,7 +287,7 @@ class SuggestServiceImpl extends BaseServiceImplementation implements final int maxAllowed = cfg.getInt("addreviewer", "maxAllowed", - AddReviewer.DEFAULT_MAX_REVIEWERS); + PostReviewers.DEFAULT_MAX_REVIEWERS); if (maxAllowed > 0 && members.size() > maxAllowed) { return false; } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewerHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewerHandler.java deleted file mode 100644 index 60d6e60fcf..0000000000 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/AddReviewerHandler.java +++ /dev/null @@ -1,61 +0,0 @@ -// Copyright (C) 2009 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.httpd.rpc.patch; - -import com.google.gerrit.common.data.ReviewerResult; -import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.patch.AddReviewer; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; - -import java.util.Collection; - -class AddReviewerHandler extends Handler { - interface Factory { - AddReviewerHandler create(Change.Id changeId, Collection reviewers, - boolean confirmed); - } - - private final AddReviewer.Factory addReviewerFactory; - private final ChangeDetailFactory.Factory changeDetailFactory; - - private final Change.Id changeId; - private final Collection reviewers; - private final boolean confirmed; - - @Inject - AddReviewerHandler(final AddReviewer.Factory addReviewerFactory, - final ChangeDetailFactory.Factory changeDetailFactory, - @Assisted final Change.Id changeId, - @Assisted final Collection reviewers, - @Assisted final boolean confirmed) { - - this.addReviewerFactory = addReviewerFactory; - this.changeDetailFactory = changeDetailFactory; - - this.changeId = changeId; - this.reviewers = reviewers; - this.confirmed = confirmed; - } - - @Override - public ReviewerResult call() throws Exception { - ReviewerResult result = addReviewerFactory.create(changeId, reviewers, confirmed).call(); - result.setChange(changeDetailFactory.create(changeId).call()); - return result; - } -} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java index 6888d4072b..1ab484f250 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchDetailServiceImpl.java @@ -21,7 +21,6 @@ import com.google.gerrit.common.data.ChangeDetail; import com.google.gerrit.common.data.PatchDetailService; import com.google.gerrit.common.data.PatchScript; import com.google.gerrit.common.data.ReviewResult; -import com.google.gerrit.common.data.ReviewerResult; import com.google.gerrit.common.errors.NoSuchEntityException; import com.google.gerrit.httpd.rpc.BaseServiceImplementation; import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory; @@ -52,7 +51,6 @@ import org.eclipse.jgit.errors.RepositoryNotFoundException; import java.io.IOException; import java.util.Collections; import java.util.HashMap; -import java.util.List; import java.util.Map; import java.util.Set; @@ -61,10 +59,8 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements private final ApprovalTypes approvalTypes; private final AccountInfoCacheFactory.Factory accountInfoCacheFactory; - private final AddReviewerHandler.Factory addReviewerHandlerFactory; private final ChangeControl.Factory changeControlFactory; private final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory; - private final RemoveReviewerHandler.Factory removeReviewerHandlerFactory; private final FunctionState.Factory functionStateFactory; private final PatchScriptFactory.Factory patchScriptFactoryFactory; private final SaveDraft.Factory saveDraftFactory; @@ -75,8 +71,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements final Provider currentUser, final ApprovalTypes approvalTypes, final AccountInfoCacheFactory.Factory accountInfoCacheFactory, - final AddReviewerHandler.Factory addReviewerHandlerFactory, - final RemoveReviewerHandler.Factory removeReviewerHandlerFactory, final ChangeControl.Factory changeControlFactory, final DeleteDraftPatchSet.Factory deleteDraftPatchSetFactory, final FunctionState.Factory functionStateFactory, @@ -87,8 +81,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements this.approvalTypes = approvalTypes; this.accountInfoCacheFactory = accountInfoCacheFactory; - this.addReviewerHandlerFactory = addReviewerHandlerFactory; - this.removeReviewerHandlerFactory = removeReviewerHandlerFactory; this.changeControlFactory = changeControlFactory; this.deleteDraftPatchSetFactory = deleteDraftPatchSetFactory; this.functionStateFactory = functionStateFactory; @@ -170,16 +162,6 @@ class PatchDetailServiceImpl extends BaseServiceImplementation implements }); } - public void addReviewers(final Change.Id id, final List reviewers, - final boolean confirmed, final AsyncCallback callback) { - addReviewerHandlerFactory.create(id, reviewers, confirmed).to(callback); - } - - public void removeReviewer(final Change.Id id, final Account.Id reviewerId, - final AsyncCallback callback) { - removeReviewerHandlerFactory.create(id, reviewerId).to(callback); - } - public void userApprovals(final Set cids, final Account.Id aid, final AsyncCallback callback) { run(callback, new Action() { diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java index 3e94b4c5d3..d1f5b245ac 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/PatchModule.java @@ -28,8 +28,6 @@ public class PatchModule extends RpcServletModule { install(new FactoryModule() { @Override protected void configure() { - factory(AddReviewerHandler.Factory.class); - factory(RemoveReviewerHandler.Factory.class); factory(PatchScriptFactory.Factory.class); factory(SaveDraft.Factory.class); } diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/RemoveReviewerHandler.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/RemoveReviewerHandler.java deleted file mode 100644 index 2b31c1cb6b..0000000000 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/patch/RemoveReviewerHandler.java +++ /dev/null @@ -1,59 +0,0 @@ -// Copyright (C) 2010 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.httpd.rpc.patch; - -import com.google.gerrit.common.data.ReviewerResult; -import com.google.gerrit.httpd.rpc.Handler; -import com.google.gerrit.httpd.rpc.changedetail.ChangeDetailFactory; -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.server.patch.RemoveReviewer; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; - -import java.util.Collections; - -/** - * Implement the remote logic that removes a reviewer from a change. - */ -class RemoveReviewerHandler extends Handler { - interface Factory { - RemoveReviewerHandler create(Change.Id changeId, Account.Id reviewerId); - } - - private final RemoveReviewer.Factory removeReviewerFactory; - private final Account.Id reviewerId; - private final Change.Id changeId; - private final ChangeDetailFactory.Factory changeDetailFactory; - - @Inject - RemoveReviewerHandler(final RemoveReviewer.Factory removeReviewerFactory, - final ChangeDetailFactory.Factory changeDetailFactory, - @Assisted Change.Id changeId, @Assisted Account.Id reviewerId) { - this.removeReviewerFactory = removeReviewerFactory; - this.changeId = changeId; - this.reviewerId = reviewerId; - this.changeDetailFactory = changeDetailFactory; - } - - @Override - public ReviewerResult call() throws Exception { - ReviewerResult result = removeReviewerFactory.create( - changeId, Collections.singleton(reviewerId)).call(); - result.setChange(changeDetailFactory.create(changeId).call()); - return result; - } - -} diff --git a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java index e071a0f4a8..251249f59a 100644 --- a/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java +++ b/gerrit-httpd/src/main/java/com/google/gerrit/httpd/rpc/project/ReviewProjectAccess.java @@ -27,14 +27,17 @@ import com.google.gerrit.reviewdb.server.ReviewDb; import com.google.gerrit.server.ChangeUtil; import com.google.gerrit.server.IdentifiedUser; import com.google.gerrit.server.account.GroupBackend; +import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.PostReviewers; import com.google.gerrit.server.git.GitRepositoryManager; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.ProjectConfig; -import com.google.gerrit.server.patch.AddReviewer; import com.google.gerrit.server.patch.PatchSetInfoFactory; +import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.ProjectControl; import com.google.gwtorm.server.OrmException; import com.google.inject.Inject; +import com.google.inject.Provider; import com.google.inject.assistedinject.Assisted; import org.eclipse.jgit.lib.ObjectId; @@ -58,14 +61,16 @@ public class ReviewProjectAccess extends ProjectAccessHandler { private final ReviewDb db; private final IdentifiedUser user; private final PatchSetInfoFactory patchSetInfoFactory; - private final AddReviewer.Factory addReviewerFactory; + private final Provider reviewersProvider; + private final ChangeControl.GenericFactory changeFactory; @Inject ReviewProjectAccess(final ProjectControl.Factory projectControlFactory, final GroupBackend groupBackend, final MetaDataUpdate.User metaDataUpdateFactory, final ReviewDb db, final IdentifiedUser user, final PatchSetInfoFactory patchSetInfoFactory, - final AddReviewer.Factory addReviewerFactory, + final Provider reviewersProvider, + final ChangeControl.GenericFactory changeFactory, @Assisted final Project.NameKey projectName, @Nullable @Assisted final ObjectId base, @@ -76,7 +81,8 @@ public class ReviewProjectAccess extends ProjectAccessHandler { this.db = db; this.user = user; this.patchSetInfoFactory = patchSetInfoFactory; - this.addReviewerFactory = addReviewerFactory; + this.reviewersProvider = reviewersProvider; + this.changeFactory = changeFactory; } @Override @@ -111,7 +117,7 @@ public class ReviewProjectAccess extends ProjectAccessHandler { insertAncestors(ps.getId(), commit); db.patchSets().insert(Collections.singleton(ps)); db.changes().insert(Collections.singleton(change)); - addProjectOwnersAsReviewers(changeId); + addProjectOwnersAsReviewers(change); db.commit(); } finally { db.rollback(); @@ -133,12 +139,15 @@ public class ReviewProjectAccess extends ProjectAccessHandler { db.patchSetAncestors().insert(toInsert); } - private void addProjectOwnersAsReviewers(final Change.Id changeId) { + private void addProjectOwnersAsReviewers(final Change change) { final String projectOwners = groupBackend.get(AccountGroup.PROJECT_OWNERS).getName(); try { - addReviewerFactory.create(changeId, Collections.singleton(projectOwners), - false).call(); + ChangeResource rsrc = + new ChangeResource(changeFactory.controlFor(change, user)); + PostReviewers.Input input = new PostReviewers.Input(); + input.reviewer = projectOwners; + reviewersProvider.get().apply(rsrc, input); } catch (Exception e) { // one of the owner groups is not visible to the user and this it why it // can't be added as reviewer diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java index a121248dba..0311089f79 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/DeleteReviewer.java @@ -32,8 +32,8 @@ import com.google.inject.Provider; import java.util.List; -class DeleteReviewer implements RestModifyView { - static class Input { +public class DeleteReviewer implements RestModifyView { + public static class Input { } private final Provider dbProvider; diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java index f83ab64bc2..ca06203d6d 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/PostReviewers.java @@ -57,13 +57,13 @@ import java.text.MessageFormat; import java.util.List; import java.util.Set; -class PostReviewers implements RestModifyView { +public class PostReviewers implements RestModifyView { public final static int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10; public final static int DEFAULT_MAX_REVIEWERS = 20; - static class Input { + public static class Input { @DefaultInput - String reviewer; + public String reviewer; Boolean confirmed; boolean confirmed() { diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java index 23fdeadab1..059b19332e 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerJson.java @@ -144,8 +144,8 @@ public class ReviewerJson { } public static class PostResult { - List reviewers; + public List reviewers; + public String error; Boolean confirm; - String error; } } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java index 98a4277327..f7b5228786 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/change/ReviewerResource.java @@ -26,7 +26,7 @@ public class ReviewerResource extends ChangeResource { public static final TypeLiteral> REVIEWER_KIND = new TypeLiteral>() {}; - static interface Factory { + public static interface Factory { ReviewerResource create(ChangeResource rsrc, IdentifiedUser user); ReviewerResource create(ChangeResource rsrc, Account.Id id); } diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java index f4254cf62a..0779cec9b3 100644 --- a/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java +++ b/gerrit-server/src/main/java/com/google/gerrit/server/config/GerritRequestModule.java @@ -26,7 +26,6 @@ import com.google.gerrit.server.git.BanCommit; import com.google.gerrit.server.git.MergeOp; import com.google.gerrit.server.git.MetaDataUpdate; import com.google.gerrit.server.git.SubmoduleOp; -import com.google.gerrit.server.patch.AddReviewer; import com.google.gerrit.server.patch.RemoveReviewer; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.CreateProject; @@ -56,7 +55,6 @@ public class GerritRequestModule extends FactoryModule { // Not really per-request, but dammit, I don't know where else to // easily park this stuff. // - factory(AddReviewer.Factory.class); factory(DeleteDraftPatchSet.Factory.class); factory(PublishDraft.Factory.class); factory(RemoveReviewer.Factory.class); diff --git a/gerrit-server/src/main/java/com/google/gerrit/server/patch/AddReviewer.java b/gerrit-server/src/main/java/com/google/gerrit/server/patch/AddReviewer.java deleted file mode 100644 index 707d3b9b1d..0000000000 --- a/gerrit-server/src/main/java/com/google/gerrit/server/patch/AddReviewer.java +++ /dev/null @@ -1,273 +0,0 @@ -// Copyright (C) 2009 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.server.patch; - -import com.google.gerrit.common.ChangeHooks; -import com.google.gerrit.common.data.ApprovalType; -import com.google.gerrit.common.data.ApprovalTypes; -import com.google.gerrit.common.data.ReviewerResult; -import com.google.gerrit.reviewdb.client.Account; -import com.google.gerrit.reviewdb.client.AccountGroup; -import com.google.gerrit.reviewdb.client.ApprovalCategory; -import com.google.gerrit.reviewdb.client.Change; -import com.google.gerrit.reviewdb.client.PatchSet; -import com.google.gerrit.reviewdb.client.PatchSetApproval; -import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.IdentifiedUser; -import com.google.gerrit.server.account.AccountCache; -import com.google.gerrit.server.account.AccountResolver; -import com.google.gerrit.server.account.GroupCache; -import com.google.gerrit.server.account.GroupMembers; -import com.google.gerrit.server.config.GerritServerConfig; -import com.google.gerrit.server.mail.AddReviewerSender; -import com.google.gerrit.server.project.ChangeControl; -import com.google.gwtorm.server.OrmException; -import com.google.inject.Inject; -import com.google.inject.assistedinject.Assisted; - -import org.eclipse.jgit.lib.Config; - -import java.util.ArrayList; -import java.util.Collection; -import java.util.HashSet; -import java.util.List; -import java.util.Set; -import java.util.concurrent.Callable; - -public class AddReviewer implements Callable { - public final static int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10; - public final static int DEFAULT_MAX_REVIEWERS = 20; - - public interface Factory { - AddReviewer create(Change.Id changeId, - Collection userNameOrEmailOrGroupNames, boolean confirmed); - } - - private final AddReviewerSender.Factory addReviewerSenderFactory; - private final AccountResolver accountResolver; - private final GroupCache groupCache; - private final GroupMembers.Factory groupMembersFactory; - private final ChangeControl.Factory changeControlFactory; - private final ReviewDb db; - private final IdentifiedUser currentUser; - private final IdentifiedUser.GenericFactory identifiedUserFactory; - private final ApprovalCategory.Id addReviewerCategoryId; - private final Config cfg; - private final ChangeHooks hooks; - private final AccountCache accountCache; - - private final Change.Id changeId; - private final Collection reviewers; - private final boolean confirmed; - - @Inject - AddReviewer(final AddReviewerSender.Factory addReviewerSenderFactory, - final AccountResolver accountResolver, final GroupCache groupCache, - final GroupMembers.Factory groupMembersFactory, - final ChangeControl.Factory changeControlFactory, final ReviewDb db, - final IdentifiedUser.GenericFactory identifiedUserFactory, - final IdentifiedUser currentUser, final ApprovalTypes approvalTypes, - final @GerritServerConfig Config cfg, final ChangeHooks hooks, - final AccountCache accountCache, - @Assisted final Change.Id changeId, - @Assisted final Collection reviewers, - @Assisted final boolean confirmed) { - this.addReviewerSenderFactory = addReviewerSenderFactory; - this.accountResolver = accountResolver; - this.groupCache = groupCache; - this.groupMembersFactory = groupMembersFactory; - this.db = db; - this.changeControlFactory = changeControlFactory; - this.identifiedUserFactory = identifiedUserFactory; - this.currentUser = currentUser; - this.cfg = cfg; - this.hooks = hooks; - this.accountCache = accountCache; - - final List allTypes = approvalTypes.getApprovalTypes(); - addReviewerCategoryId = - allTypes.get(allTypes.size() - 1).getCategory().getId(); - - this.changeId = changeId; - this.reviewers = reviewers; - this.confirmed = confirmed; - } - - @Override - public ReviewerResult call() throws Exception { - final Set reviewerIds = new HashSet(); - final ChangeControl control = changeControlFactory.validateFor(changeId); - - final ReviewerResult result = new ReviewerResult(); - for (final String reviewer : reviewers) { - final Account account = accountResolver.find(reviewer); - if (account == null) { - AccountGroup group = groupCache.get(new AccountGroup.NameKey(reviewer)); - - if (group == null) { - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.REVIEWER_NOT_FOUND, reviewer)); - continue; - } - - if (!isLegalReviewerGroup(group.getGroupUUID())) { - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.GROUP_NOT_ALLOWED, reviewer)); - continue; - } - - final Set members = groupMembersFactory.create(currentUser) - .listAccounts(group.getGroupUUID(), - control.getProject().getNameKey()); - if (members == null || members.size() == 0) { - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.GROUP_EMPTY, reviewer)); - continue; - } - - // if maxAllowed is set to 0, it is allowed to add any number of - // reviewers - final int maxAllowed = - cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS); - if (maxAllowed > 0 && members.size() > maxAllowed) { - result.setMemberCount(members.size()); - result.setAskForConfirmation(false); - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.GROUP_HAS_TOO_MANY_MEMBERS, reviewer)); - continue; - } - - // if maxWithoutCheck is set to 0, we never ask for confirmation - final int maxWithoutConfirmation = - cfg.getInt("addreviewer", "maxWithoutConfirmation", - DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK); - if (!confirmed && maxWithoutConfirmation > 0 - && members.size() > maxWithoutConfirmation) { - result.setMemberCount(members.size()); - result.setAskForConfirmation(true); - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.GROUP_HAS_TOO_MANY_MEMBERS, reviewer)); - continue; - } - - for (final Account member : members) { - if (member.isActive()) { - final IdentifiedUser user = - identifiedUserFactory.create(member.getId()); - // Does not account for draft status as a user might want to let a - // reviewer see a draft. - if (control.forUser(user).isRefVisible()) { - reviewerIds.add(member.getId()); - } - } - } - continue; - } - - if (!account.isActive()) { - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.ACCOUNT_INACTIVE, - formatUser(account, reviewer))); - continue; - } - - final IdentifiedUser user = identifiedUserFactory.create(account.getId()); - // Does not account for draft status as a user might want to let a - // reviewer see a draft. - if (!control.forUser(user).isRefVisible()) { - result.addError(new ReviewerResult.Error( - ReviewerResult.Error.Type.CHANGE_NOT_VISIBLE, - formatUser(account, reviewer))); - continue; - } - - reviewerIds.add(account.getId()); - } - - if (reviewerIds.isEmpty()) { - return result; - } - - // Add the reviewers to the database - // - final Set added = new HashSet(); - final List toInsert = new ArrayList(); - final Change change = control.getChange(); - final PatchSet.Id psid = change.currentPatchSetId(); - for (final Account.Id reviewer : reviewerIds) { - if (!exists(psid, reviewer)) { - // This reviewer has not entered an approval for this change yet. - // - final PatchSetApproval myca = - dummyApproval(control.getChange(), psid, reviewer); - toInsert.add(myca); - added.add(reviewer); - } - } - db.patchSetApprovals().insert(toInsert); - - // Execute hook for added reviewers - // - final PatchSet patchSet = db.patchSets().get(psid); - for (final Account.Id id : added) { - final Account account = accountCache.get(id).getAccount(); - hooks.doReviewerAddedHook(change, account, patchSet, db); - } - - // Email the reviewers - // - // The user knows they added themselves, don't bother emailing them. - added.remove(currentUser.getAccountId()); - if (!added.isEmpty()) { - final AddReviewerSender cm; - - cm = addReviewerSenderFactory.create(control.getChange()); - cm.setFrom(currentUser.getAccountId()); - cm.addReviewers(added); - cm.send(); - } - - return result; - } - - private String formatUser(Account account, String nameOrEmail) { - if (nameOrEmail.matches("^[1-9][0-9]*$")) { - return RemoveReviewer.formatUser(account, nameOrEmail); - } else { - return nameOrEmail; - } - } - - private boolean exists(final PatchSet.Id patchSetId, - final Account.Id reviewerId) throws OrmException { - return db.patchSetApprovals().byPatchSetUser(patchSetId, reviewerId) - .iterator().hasNext(); - } - - private PatchSetApproval dummyApproval(final Change change, - final PatchSet.Id patchSetId, final Account.Id reviewerId) { - final PatchSetApproval dummyApproval = - new PatchSetApproval(new PatchSetApproval.Key(patchSetId, reviewerId, - addReviewerCategoryId), (short) 0); - dummyApproval.cache(change); - return dummyApproval; - } - - public static boolean isLegalReviewerGroup(final AccountGroup.UUID groupUUID) { - return !(AccountGroup.ANONYMOUS_USERS.equals(groupUUID) - || AccountGroup.REGISTERED_USERS.equals(groupUUID)); - } -} diff --git a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java index 311300ee23..8e9bcac159 100644 --- a/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java +++ b/gerrit-sshd/src/main/java/com/google/gerrit/sshd/commands/SetReviewersCommand.java @@ -14,14 +14,16 @@ package com.google.gerrit.sshd.commands; -import com.google.gerrit.common.data.ReviewerResult; +import com.google.gerrit.extensions.restapi.ResourceNotFoundException; import com.google.gerrit.reviewdb.client.Account; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.reviewdb.client.PatchSet; import com.google.gerrit.reviewdb.client.RevId; import com.google.gerrit.reviewdb.server.ReviewDb; -import com.google.gerrit.server.patch.AddReviewer; -import com.google.gerrit.server.patch.RemoveReviewer; +import com.google.gerrit.server.change.ChangeResource; +import com.google.gerrit.server.change.DeleteReviewer; +import com.google.gerrit.server.change.PostReviewers; +import com.google.gerrit.server.change.ReviewerResource; import com.google.gerrit.server.project.ChangeControl; import com.google.gerrit.server.project.NoSuchChangeException; import com.google.gerrit.server.project.ProjectControl; @@ -30,6 +32,7 @@ import com.google.gerrit.sshd.SshCommand; import com.google.gwtorm.server.OrmException; import com.google.gwtorm.server.ResultSet; import com.google.inject.Inject; +import com.google.inject.Provider; import org.kohsuke.args4j.Argument; import org.kohsuke.args4j.Option; @@ -37,7 +40,6 @@ import org.slf4j.Logger; import org.slf4j.LoggerFactory; import java.io.IOException; -import java.text.MessageFormat; import java.util.ArrayList; import java.util.HashSet; import java.util.List; @@ -74,10 +76,13 @@ public class SetReviewersCommand extends SshCommand { private ReviewDb db; @Inject - private AddReviewer.Factory addReviewerFactory; + private ReviewerResource.Factory reviewerFactory; @Inject - private RemoveReviewer.Factory removeReviewerFactory; + private Provider postReviewersProvider; + + @Inject + private Provider deleteReviewerProvider; @Inject private ChangeControl.Factory changeControlFactory; @@ -104,62 +109,48 @@ public class SetReviewersCommand extends SshCommand { } private boolean modifyOne(Change.Id changeId) throws Exception { - changeControlFactory.validateFor(changeId); - - ReviewerResult result; + ChangeResource changeRsrc = + new ChangeResource(changeControlFactory.validateFor(changeId)); boolean ok = true; // Remove reviewers // - result = removeReviewerFactory.create(changeId, toRemove).call(); - ok &= result.getErrors().isEmpty(); - for (ReviewerResult.Error resultError : result.getErrors()) { - String message; - switch (resultError.getType()) { - case REMOVE_NOT_PERMITTED: - message = "not permitted to remove {0} from {1}"; - break; - case COULD_NOT_REMOVE: - message = "could not remove {0} from {1}"; - break; - default: - message = "could not remove {0}: {2}"; + DeleteReviewer delete = deleteReviewerProvider.get(); + for (Account.Id reviewer : toRemove) { + ReviewerResource rsrc = reviewerFactory.create(changeRsrc, reviewer); + String error = null;; + try { + delete.apply(rsrc, new DeleteReviewer.Input()); + } catch (ResourceNotFoundException e) { + error = String.format("could not remove %s: not found", reviewer); + } catch (Exception e) { + error = String.format("could not remove %s: %s", + reviewer, e.getMessage()); + } + if (error != null) { + ok = false; + writeError("error", error); } - writeError("error", MessageFormat.format(message, - resultError.getName(), changeId, resultError.getType())); } // Add reviewers // - result = - addReviewerFactory.create(changeId, toAdd, true).call(); - ok &= result.getErrors().isEmpty(); - for (ReviewerResult.Error resultError : result.getErrors()) { - String message; - switch (resultError.getType()) { - case REVIEWER_NOT_FOUND: - message = "account or group {0} not found"; - break; - case ACCOUNT_INACTIVE: - message = "account {0} inactive"; - break; - case CHANGE_NOT_VISIBLE: - message = "change {1} not visible to {0}"; - break; - case GROUP_EMPTY: - message = "group {0} is empty"; - break; - case GROUP_HAS_TOO_MANY_MEMBERS: - message = "group {0} has too many members"; - break; - case GROUP_NOT_ALLOWED: - message = "group {0} is not allowed as reviewer"; - break; - default: - message = "could not add {0}: {2}"; + PostReviewers post = postReviewersProvider.get(); + for (String reviewer : toAdd) { + PostReviewers.Input input = new PostReviewers.Input(); + input.reviewer = reviewer; + String error; + try { + error = post.apply(changeRsrc, input).error; + } catch (ResourceNotFoundException e) { + error = String.format("could not add %s: not found", reviewer); + } catch (Exception e) { + error = String.format("could not add %s: %s", reviewer, e.getMessage()); + } + if (error != null) { + ok = false; + writeError("error", error); } - writeError("error", MessageFormat.format(message, - resultError.getName(), changeId, resultError.getType())); } return ok;