RestApiModule: Move bindings for non-REST-endpoint-classes into a separate module
At Google we would like to have more control over the REST API that we
expose. For this we intent to install a custom RestApiModule that binds
only the REST endpoints that we need. If we do this the problem is that
we need to take care to bind all the non-REST-endpoint-classes
internally that are currently bound by RestApiModule, which can easily
go out of sync and hence is error-prone. To avoid this move the bindings
for non-REST-endpoint-classes out of RestApiModule into a separate
module. Then at Google we can just use that new module, while we replace
the RestApiModule that only bind REST endpoints.
Release-Notes: skip
Change-Id: Id46aa5f9a673938ced87dae8c569914ad70eed11
Signed-off-by: Edwin Kempin <ekempin@google.com>
Bug: Google b/289489297
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index 0fb9310..7086447 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -202,6 +202,7 @@
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
import com.google.gerrit.server.quota.QuotaEnforcer;
+import com.google.gerrit.server.restapi.RestModule;
import com.google.gerrit.server.restapi.change.OnPostReview;
import com.google.gerrit.server.restapi.change.SuggestReviewers;
import com.google.gerrit.server.restapi.group.GroupModule;
@@ -289,6 +290,7 @@
install(new DefaultSubmitRuleModule());
install(new IgnoreSelfApprovalRuleModule());
install(new ReceiveCommitsModule());
+ install(new RestModule());
install(new SshAddressesModule());
install(new FileInfoJsonModule());
install(ThreadLocalRequestContext.module());
diff --git a/java/com/google/gerrit/server/restapi/RestApiModule.java b/java/com/google/gerrit/server/restapi/RestApiModule.java
index dffcf44..73991c9 100644
--- a/java/com/google/gerrit/server/restapi/RestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/RestApiModule.java
@@ -24,6 +24,12 @@
import com.google.gerrit.server.restapi.project.ProjectRestApiModule;
import com.google.inject.AbstractModule;
+/**
+ * Module to bind REST API endpoints.
+ *
+ * <p>Classes that are needed by the REST layer, but which are not REST API endpoints, should be
+ * bound in {@link RestModule}.
+ */
public class RestApiModule extends AbstractModule {
@Override
protected void configure() {
diff --git a/java/com/google/gerrit/server/restapi/RestModule.java b/java/com/google/gerrit/server/restapi/RestModule.java
new file mode 100644
index 0000000..0dcb4c8
--- /dev/null
+++ b/java/com/google/gerrit/server/restapi/RestModule.java
@@ -0,0 +1,133 @@
+// Copyright (C) 2023 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.restapi;
+
+import com.google.gerrit.extensions.config.FactoryModule;
+import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.gpg.PublicKeyStore;
+import com.google.gerrit.gpg.UnimplementedPublicKeyStoreProvider;
+import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.ServerInitiated;
+import com.google.gerrit.server.UserInitiated;
+import com.google.gerrit.server.account.AccountLoader;
+import com.google.gerrit.server.account.AccountsUpdate;
+import com.google.gerrit.server.change.AddReviewersOp;
+import com.google.gerrit.server.change.AddToAttentionSetOp;
+import com.google.gerrit.server.change.ChangeInserter;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.change.DeleteChangeOp;
+import com.google.gerrit.server.change.DeleteReviewerByEmailOp;
+import com.google.gerrit.server.change.DeleteReviewerOp;
+import com.google.gerrit.server.change.EmailReviewComments;
+import com.google.gerrit.server.change.PatchSetInserter;
+import com.google.gerrit.server.change.RebaseChangeOp;
+import com.google.gerrit.server.change.RemoveFromAttentionSetOp;
+import com.google.gerrit.server.change.ReviewerResource;
+import com.google.gerrit.server.change.SetCherryPickOp;
+import com.google.gerrit.server.change.SetCustomKeyedValuesOp;
+import com.google.gerrit.server.change.SetHashtagsOp;
+import com.google.gerrit.server.change.SetPrivateOp;
+import com.google.gerrit.server.change.SetTopicOp;
+import com.google.gerrit.server.change.WorkInProgressOp;
+import com.google.gerrit.server.comment.CommentContextLoader;
+import com.google.gerrit.server.config.GerritConfigListener;
+import com.google.gerrit.server.group.db.GroupsUpdate;
+import com.google.gerrit.server.project.RefValidationHelper;
+import com.google.gerrit.server.restapi.change.DeleteVoteOp;
+import com.google.gerrit.server.restapi.change.PostReviewOp;
+import com.google.gerrit.server.restapi.change.PreviewFix;
+import com.google.gerrit.server.restapi.project.CreateProject;
+import com.google.gerrit.server.restapi.project.ProjectNode;
+import com.google.gerrit.server.restapi.project.SetParent;
+import com.google.gerrit.server.util.AttentionSetEmail;
+import com.google.gerrit.server.validators.ProjectCreationValidationListener;
+import com.google.inject.Provides;
+import com.google.inject.multibindings.OptionalBinder;
+
+/**
+ * Module to bind classes that are needed but the REST layer, but which are not REST endpoints.
+ *
+ * <p>REST endpoints should be bound in {@link RestApiModule}.
+ */
+public class RestModule extends FactoryModule {
+ @Override
+ protected void configure() {
+ factory(AccountLoader.Factory.class);
+ factory(AddReviewersOp.Factory.class);
+ factory(AddToAttentionSetOp.Factory.class);
+ factory(AttentionSetEmail.Factory.class);
+ factory(ChangeInserter.Factory.class);
+ factory(ChangeResource.Factory.class);
+ factory(CommentContextLoader.Factory.class);
+ factory(DeleteChangeOp.Factory.class);
+ factory(DeleteReviewerByEmailOp.Factory.class);
+ factory(DeleteReviewerOp.Factory.class);
+ factory(DeleteVoteOp.Factory.class);
+ factory(EmailReviewComments.Factory.class);
+ factory(GroupsUpdate.Factory.class);
+ factory(PatchSetInserter.Factory.class);
+ factory(PostReviewOp.Factory.class);
+ factory(PreviewFix.Factory.class);
+ factory(ProjectNode.Factory.class);
+ factory(RebaseChangeOp.Factory.class);
+ factory(RefValidationHelper.Factory.class);
+ factory(RemoveFromAttentionSetOp.Factory.class);
+ factory(ReviewerResource.Factory.class);
+ factory(SetCherryPickOp.Factory.class);
+ factory(SetCustomKeyedValuesOp.Factory.class);
+ factory(SetHashtagsOp.Factory.class);
+ factory(SetPrivateOp.Factory.class);
+ factory(SetTopicOp.Factory.class);
+ factory(WorkInProgressOp.Factory.class);
+
+ DynamicSet.bind(binder(), GerritConfigListener.class).to(SetParent.class);
+ DynamicSet.bind(binder(), ProjectCreationValidationListener.class)
+ .to(CreateProject.ValidBranchListener.class);
+
+ OptionalBinder.newOptionalBinder(binder(), PublicKeyStore.class)
+ .setDefault()
+ .toProvider(UnimplementedPublicKeyStoreProvider.class);
+ }
+
+ @Provides
+ @ServerInitiated
+ AccountsUpdate provideServerInitiatedAccountsUpdate(
+ @AccountsUpdate.AccountsUpdateLoader.WithReindex
+ AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory) {
+ return accountsUpdateFactory.createWithServerIdent();
+ }
+
+ @Provides
+ @UserInitiated
+ AccountsUpdate provideUserInitiatedAccountsUpdate(
+ @AccountsUpdate.AccountsUpdateLoader.WithReindex
+ AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory,
+ IdentifiedUser currentUser) {
+ return accountsUpdateFactory.create(currentUser);
+ }
+
+ @Provides
+ @ServerInitiated
+ GroupsUpdate provideServerInitiatedGroupsUpdate(GroupsUpdate.Factory groupsUpdateFactory) {
+ return groupsUpdateFactory.createWithServerIdent();
+ }
+
+ @Provides
+ @UserInitiated
+ GroupsUpdate provideUserInitiatedGroupsUpdate(
+ GroupsUpdate.Factory groupsUpdateFactory, IdentifiedUser currentUser) {
+ return groupsUpdateFactory.create(currentUser);
+ }
+}
diff --git a/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java b/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
index 821d16e..53edfdb 100644
--- a/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/account/AccountRestApiModule.java
@@ -23,14 +23,6 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.gerrit.gpg.PublicKeyStore;
-import com.google.gerrit.gpg.UnimplementedPublicKeyStoreProvider;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.ServerInitiated;
-import com.google.gerrit.server.UserInitiated;
-import com.google.gerrit.server.account.AccountsUpdate;
-import com.google.inject.Provides;
-import com.google.inject.multibindings.OptionalBinder;
public class AccountRestApiModule extends RestApiModule {
@Override
@@ -39,10 +31,6 @@
bind(Capabilities.class);
bind(StarredChanges.Create.class);
- OptionalBinder.newOptionalBinder(binder(), PublicKeyStore.class)
- .setDefault()
- .toProvider(UnimplementedPublicKeyStoreProvider.class);
-
DynamicMap.mapOf(binder(), ACCOUNT_KIND);
DynamicMap.mapOf(binder(), CAPABILITY_KIND);
DynamicMap.mapOf(binder(), EMAIL_KIND);
@@ -113,21 +101,4 @@
// The gpgkeys REST endpoints are bound via GpgApiModule.
// The oauthtoken REST endpoint is bound via OAuthRestModule.
}
-
- @Provides
- @ServerInitiated
- AccountsUpdate provideServerInitiatedAccountsUpdate(
- @AccountsUpdate.AccountsUpdateLoader.WithReindex
- AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory) {
- return accountsUpdateFactory.createWithServerIdent();
- }
-
- @Provides
- @UserInitiated
- AccountsUpdate provideUserInitiatedAccountsUpdate(
- @AccountsUpdate.AccountsUpdateLoader.WithReindex
- AccountsUpdate.AccountsUpdateLoader accountsUpdateFactory,
- IdentifiedUser currentUser) {
- return accountsUpdateFactory.create(currentUser);
- }
}
diff --git a/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java b/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java
index 6b121f6..2ac24c6 100644
--- a/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/change/ChangeRestApiModule.java
@@ -29,29 +29,8 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.gerrit.server.account.AccountLoader;
-import com.google.gerrit.server.change.AddReviewersOp;
-import com.google.gerrit.server.change.AddToAttentionSetOp;
-import com.google.gerrit.server.change.ChangeInserter;
-import com.google.gerrit.server.change.ChangeResource;
-import com.google.gerrit.server.change.DeleteChangeOp;
-import com.google.gerrit.server.change.DeleteReviewerByEmailOp;
-import com.google.gerrit.server.change.DeleteReviewerOp;
-import com.google.gerrit.server.change.EmailReviewComments;
-import com.google.gerrit.server.change.PatchSetInserter;
-import com.google.gerrit.server.change.RebaseChangeOp;
-import com.google.gerrit.server.change.RemoveFromAttentionSetOp;
-import com.google.gerrit.server.change.ReviewerResource;
-import com.google.gerrit.server.change.SetCherryPickOp;
-import com.google.gerrit.server.change.SetCustomKeyedValuesOp;
-import com.google.gerrit.server.change.SetHashtagsOp;
-import com.google.gerrit.server.change.SetPrivateOp;
-import com.google.gerrit.server.change.SetTopicOp;
-import com.google.gerrit.server.change.WorkInProgressOp;
-import com.google.gerrit.server.comment.CommentContextLoader;
import com.google.gerrit.server.restapi.change.Reviewed.DeleteReviewed;
import com.google.gerrit.server.restapi.change.Reviewed.PutReviewed;
-import com.google.gerrit.server.util.AttentionSetEmail;
public class ChangeRestApiModule extends RestApiModule {
@Override
@@ -211,30 +190,5 @@
post(VOTE_KIND, "delete").to(DeleteVote.class);
post(CHANGE_KIND, "wip").to(SetWorkInProgress.class);
-
- factory(AccountLoader.Factory.class);
- factory(AddReviewersOp.Factory.class);
- factory(AddToAttentionSetOp.Factory.class);
- factory(AttentionSetEmail.Factory.class);
- factory(ChangeInserter.Factory.class);
- factory(ChangeResource.Factory.class);
- factory(CommentContextLoader.Factory.class);
- factory(DeleteChangeOp.Factory.class);
- factory(DeleteReviewerByEmailOp.Factory.class);
- factory(DeleteReviewerOp.Factory.class);
- factory(DeleteVoteOp.Factory.class);
- factory(EmailReviewComments.Factory.class);
- factory(PatchSetInserter.Factory.class);
- factory(PostReviewOp.Factory.class);
- factory(PreviewFix.Factory.class);
- factory(RebaseChangeOp.Factory.class);
- factory(RemoveFromAttentionSetOp.Factory.class);
- factory(ReviewerResource.Factory.class);
- factory(SetCherryPickOp.Factory.class);
- factory(SetCustomKeyedValuesOp.Factory.class);
- factory(SetHashtagsOp.Factory.class);
- factory(SetPrivateOp.Factory.class);
- factory(SetTopicOp.Factory.class);
- factory(WorkInProgressOp.Factory.class);
}
}
diff --git a/java/com/google/gerrit/server/restapi/change/PostReviewOp.java b/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
index 8479d91..a47e179 100644
--- a/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
+++ b/java/com/google/gerrit/server/restapi/change/PostReviewOp.java
@@ -97,7 +97,7 @@
import org.eclipse.jgit.lib.Config;
public class PostReviewOp implements BatchUpdateOp {
- interface Factory {
+ public interface Factory {
PostReviewOp create(
ProjectState projectState, PatchSet.Id psId, ReviewInput in, Account.Id reviewerId);
}
diff --git a/java/com/google/gerrit/server/restapi/group/GroupRestApiModule.java b/java/com/google/gerrit/server/restapi/group/GroupRestApiModule.java
index 1eaa6a2..f115374 100644
--- a/java/com/google/gerrit/server/restapi/group/GroupRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/group/GroupRestApiModule.java
@@ -20,17 +20,12 @@
import com.google.gerrit.extensions.registration.DynamicMap;
import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.gerrit.server.IdentifiedUser;
-import com.google.gerrit.server.ServerInitiated;
-import com.google.gerrit.server.UserInitiated;
-import com.google.gerrit.server.group.db.GroupsUpdate;
import com.google.gerrit.server.restapi.group.AddMembers.CreateMember;
import com.google.gerrit.server.restapi.group.AddMembers.UpdateMember;
import com.google.gerrit.server.restapi.group.AddSubgroups.CreateSubgroup;
import com.google.gerrit.server.restapi.group.AddSubgroups.UpdateSubgroup;
import com.google.gerrit.server.restapi.group.DeleteMembers.DeleteMember;
import com.google.gerrit.server.restapi.group.DeleteSubgroups.DeleteSubgroup;
-import com.google.inject.Provides;
public class GroupRestApiModule extends RestApiModule {
@@ -77,20 +72,5 @@
put(GROUP_KIND, "options").to(PutOptions.class);
get(GROUP_KIND, "owner").to(GetOwner.class);
put(GROUP_KIND, "owner").to(PutOwner.class);
-
- factory(GroupsUpdate.Factory.class);
- }
-
- @Provides
- @ServerInitiated
- GroupsUpdate provideServerInitiatedGroupsUpdate(GroupsUpdate.Factory groupsUpdateFactory) {
- return groupsUpdateFactory.createWithServerIdent();
- }
-
- @Provides
- @UserInitiated
- GroupsUpdate provideUserInitiatedGroupsUpdate(
- GroupsUpdate.Factory groupsUpdateFactory, IdentifiedUser currentUser) {
- return groupsUpdateFactory.create(currentUser);
}
}
diff --git a/java/com/google/gerrit/server/restapi/project/CreateProject.java b/java/com/google/gerrit/server/restapi/project/CreateProject.java
index cfdadd9..04819d8 100644
--- a/java/com/google/gerrit/server/restapi/project/CreateProject.java
+++ b/java/com/google/gerrit/server/restapi/project/CreateProject.java
@@ -227,7 +227,7 @@
return branch;
}
- static class ValidBranchListener implements ProjectCreationValidationListener {
+ public static class ValidBranchListener implements ProjectCreationValidationListener {
@Override
public void validateNewProject(CreateProjectArgs args) throws ValidationException {
for (String branch : args.branch) {
diff --git a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
index a680ec4..5a38766 100644
--- a/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
+++ b/java/com/google/gerrit/server/restapi/project/ProjectRestApiModule.java
@@ -25,12 +25,8 @@
import static com.google.gerrit.server.project.TagResource.TAG_KIND;
import com.google.gerrit.extensions.registration.DynamicMap;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestApiModule;
-import com.google.gerrit.server.config.GerritConfigListener;
-import com.google.gerrit.server.project.RefValidationHelper;
import com.google.gerrit.server.restapi.change.CherryPickCommit;
-import com.google.gerrit.server.validators.ProjectCreationValidationListener;
public class ProjectRestApiModule extends RestApiModule {
@@ -49,10 +45,6 @@
DynamicMap.mapOf(binder(), SUBMIT_REQUIREMENT_KIND);
DynamicMap.mapOf(binder(), TAG_KIND);
- DynamicSet.bind(binder(), GerritConfigListener.class).to(SetParent.class);
- DynamicSet.bind(binder(), ProjectCreationValidationListener.class)
- .to(CreateProject.ValidBranchListener.class);
-
create(PROJECT_KIND).to(CreateProject.class);
get(PROJECT_KIND).to(GetProject.class);
put(PROJECT_KIND).to(PutProject.class);
@@ -129,9 +121,6 @@
delete(TAG_KIND).to(DeleteTag.class);
post(PROJECT_KIND, "tags:delete").to(DeleteTags.class);
-
- factory(RefValidationHelper.Factory.class);
- factory(ProjectNode.Factory.class);
}
/** Separately bind batch functionality. */
