Merge "ReceiveCommits: Log results of ReceiveCommands if tracing is enabled"
diff --git a/WORKSPACE b/WORKSPACE
index f0bdc70..fe1f341c 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -191,9 +191,9 @@
maven_jar(
name = "gwtorm-client",
- artifact = "com.google.gerrit:gwtorm:1.18",
- sha1 = "f326dec463439a92ccb32f05b38345e21d0b5ecf",
- src_sha1 = "e0b973d5cafef3d145fa80cdf032fcead1186d29",
+ artifact = "com.google.gerrit:gwtorm:1.20",
+ sha1 = "a4809769b710bc8ce3f203125630b8419f0e58b0",
+ src_sha1 = "cb63296276ce3228b2d83a37017a99e38ad8ed42",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java
index de8d10c..401d417 100644
--- a/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -39,6 +39,7 @@
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.RequestContext;
@@ -208,7 +209,7 @@
private static class Upload implements UploadPackFactory<Context> {
private final TransferConfig transferConfig;
- private final DynamicSet<UploadPackInitializer> uploadPackInitializers;
+ private final PluginSetContext<UploadPackInitializer> uploadPackInitializers;
private final DynamicSet<PreUploadHook> preUploadHooks;
private final UploadValidators.Factory uploadValidatorsFactory;
private final ThreadLocalRequestContext threadContext;
@@ -218,7 +219,7 @@
@Inject
Upload(
TransferConfig transferConfig,
- DynamicSet<UploadPackInitializer> uploadPackInitializers,
+ PluginSetContext<UploadPackInitializer> uploadPackInitializers,
DynamicSet<PreUploadHook> preUploadHooks,
UploadValidators.Factory uploadValidatorsFactory,
ThreadLocalRequestContext threadContext,
@@ -267,9 +268,7 @@
List<PreUploadHook> hooks = Lists.newArrayList(preUploadHooks);
hooks.add(uploadValidatorsFactory.create(projectState.getProject(), repo, "localhost-test"));
up.setPreUploadHook(PreUploadHookChain.newChain(hooks));
- for (UploadPackInitializer initializer : uploadPackInitializers) {
- initializer.init(req.project, up);
- }
+ uploadPackInitializers.runEach(initializer -> initializer.init(req.project, up));
return up;
}
}
@@ -279,7 +278,7 @@
private final ProjectCache projectCache;
private final AsyncReceiveCommits.Factory factory;
private final TransferConfig config;
- private final DynamicSet<ReceivePackInitializer> receivePackInitializers;
+ private final PluginSetContext<ReceivePackInitializer> receivePackInitializers;
private final DynamicSet<PostReceiveHook> postReceiveHooks;
private final ThreadLocalRequestContext threadContext;
private final PermissionBackend permissionBackend;
@@ -290,7 +289,7 @@
ProjectCache projectCache,
AsyncReceiveCommits.Factory factory,
TransferConfig config,
- DynamicSet<ReceivePackInitializer> receivePackInitializers,
+ PluginSetContext<ReceivePackInitializer> receivePackInitializers,
DynamicSet<PostReceiveHook> postReceiveHooks,
ThreadLocalRequestContext threadContext,
PermissionBackend permissionBackend) {
@@ -339,9 +338,8 @@
rp.setTimeout(config.getTimeout());
rp.setMaxObjectSizeLimit(config.getMaxObjectSizeLimit());
- for (ReceivePackInitializer initializer : receivePackInitializers) {
- initializer.init(projectState.getNameKey(), rp);
- }
+ receivePackInitializers.runEach(
+ initializer -> initializer.init(projectState.getNameKey(), rp));
rp.setPostReceiveHook(PostReceiveHookChain.newChain(Lists.newArrayList(postReceiveHooks)));
return rp;
diff --git a/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java b/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
index 5efdc81..8efb6ae 100644
--- a/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
+++ b/java/com/google/gerrit/acceptance/testsuite/ThrowingConsumer.java
@@ -17,4 +17,12 @@
@FunctionalInterface
public interface ThrowingConsumer<T> {
void accept(T t) throws Exception;
+
+ default void acceptAndThrowSilently(T t) {
+ try {
+ accept(t);
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/ThrowingFunction.java b/java/com/google/gerrit/acceptance/testsuite/ThrowingFunction.java
index d41672a..2337331 100644
--- a/java/com/google/gerrit/acceptance/testsuite/ThrowingFunction.java
+++ b/java/com/google/gerrit/acceptance/testsuite/ThrowingFunction.java
@@ -18,4 +18,12 @@
public interface ThrowingFunction<T, R> {
R apply(T value) throws Exception;
+
+ default R applyAndThrowSilently(T t) {
+ try {
+ return apply(t);
+ } catch (Exception e) {
+ throw new IllegalStateException(e);
+ }
+ }
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperations.java b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperations.java
index 61b7599..1791c73 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperations.java
@@ -65,7 +65,7 @@
*
* @return {@code true} if the account exists
*/
- boolean exists() throws Exception;
+ boolean exists();
/**
* Retrieves the account.
@@ -76,7 +76,7 @@
*
* @return the corresponding {@code TestAccount}
*/
- TestAccount get() throws Exception;
+ TestAccount get();
/**
* Starts the fluent chain to update an account. The returned builder can be used to specify how
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
index ebbcfe4..f35404f 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/AccountOperationsImpl.java
@@ -108,20 +108,27 @@
}
@Override
- public boolean exists() throws Exception {
- return accounts.get(accountId).isPresent();
+ public boolean exists() {
+ return getAccountState(accountId).isPresent();
}
@Override
- public TestAccount get() throws Exception {
+ public TestAccount get() {
AccountState account =
- accounts
- .get(accountId)
+ getAccountState(accountId)
.orElseThrow(
() -> new IllegalStateException("Tried to get non-existing test account"));
return toTestAccount(account);
}
+ private Optional<AccountState> getAccountState(Account.Id accountId) {
+ try {
+ return accounts.get(accountId);
+ } catch (IOException | ConfigInvalidException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
private TestAccount toTestAccount(AccountState accountState) {
Account account = accountState.getAccount();
return TestAccount.builder()
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java
index ab32409..f2414e0 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountCreation.java
@@ -88,9 +88,9 @@
abstract TestAccountCreation autoBuild();
- public Account.Id create() throws Exception {
+ public Account.Id create() {
TestAccountCreation accountUpdate = autoBuild();
- return accountUpdate.accountCreator().apply(accountUpdate);
+ return accountUpdate.accountCreator().applyAndThrowSilently(accountUpdate);
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
index 251f452..da599e7 100644
--- a/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
+++ b/java/com/google/gerrit/acceptance/testsuite/account/TestAccountUpdate.java
@@ -86,9 +86,9 @@
abstract TestAccountUpdate autoBuild();
- public void update() throws Exception {
+ public void update() {
TestAccountUpdate accountUpdate = autoBuild();
- accountUpdate.accountUpdater().accept(accountUpdate);
+ accountUpdate.accountUpdater().acceptAndThrowSilently(accountUpdate);
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/group/GroupOperations.java b/java/com/google/gerrit/acceptance/testsuite/group/GroupOperations.java
index f75ca2e..bf305ff 100644
--- a/java/com/google/gerrit/acceptance/testsuite/group/GroupOperations.java
+++ b/java/com/google/gerrit/acceptance/testsuite/group/GroupOperations.java
@@ -63,7 +63,7 @@
*
* @return {@code true} if the group exists
*/
- boolean exists() throws Exception;
+ boolean exists();
/**
* Retrieves the group.
@@ -74,7 +74,7 @@
*
* @return the corresponding {@code TestGroup}
*/
- TestGroup get() throws Exception;
+ TestGroup get();
/**
* Starts the fluent chain to update a group. The returned builder can be used to specify how
diff --git a/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java b/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java
index f9769c5..9d87895 100644
--- a/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java
+++ b/java/com/google/gerrit/acceptance/testsuite/group/GroupOperationsImpl.java
@@ -109,17 +109,25 @@
}
@Override
- public boolean exists() throws Exception {
- return groups.getGroup(groupUuid).isPresent();
+ public boolean exists() {
+ return getGroup(groupUuid).isPresent();
}
@Override
- public TestGroup get() throws Exception {
- Optional<InternalGroup> group = groups.getGroup(groupUuid);
+ public TestGroup get() {
+ Optional<InternalGroup> group = getGroup(groupUuid);
checkState(group.isPresent(), "Tried to get non-existing test group");
return toTestGroup(group.get());
}
+ private Optional<InternalGroup> getGroup(AccountGroup.UUID groupUuid) {
+ try {
+ return groups.getGroup(groupUuid);
+ } catch (IOException | ConfigInvalidException e) {
+ throw new IllegalStateException(e);
+ }
+ }
+
private TestGroup toTestGroup(InternalGroup internalGroup) {
return TestGroup.builder()
.groupUuid(internalGroup.getGroupUUID())
diff --git a/java/com/google/gerrit/acceptance/testsuite/group/TestGroupCreation.java b/java/com/google/gerrit/acceptance/testsuite/group/TestGroupCreation.java
index efed720..612ce2a 100644
--- a/java/com/google/gerrit/acceptance/testsuite/group/TestGroupCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/group/TestGroupCreation.java
@@ -104,9 +104,9 @@
*
* @return the UUID of the created group
*/
- public AccountGroup.UUID create() throws Exception {
+ public AccountGroup.UUID create() {
TestGroupCreation groupCreation = autoBuild();
- return groupCreation.groupCreator().apply(groupCreation);
+ return groupCreation.groupCreator().applyAndThrowSilently(groupCreation);
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/group/TestGroupUpdate.java b/java/com/google/gerrit/acceptance/testsuite/group/TestGroupUpdate.java
index 095a270..bc9d569 100644
--- a/java/com/google/gerrit/acceptance/testsuite/group/TestGroupUpdate.java
+++ b/java/com/google/gerrit/acceptance/testsuite/group/TestGroupUpdate.java
@@ -126,9 +126,9 @@
abstract TestGroupUpdate autoBuild();
/** Executes the group update as specified. */
- public void update() throws Exception {
+ public void update() {
TestGroupUpdate groupUpdater = autoBuild();
- groupUpdater.groupUpdater().accept(groupUpdater);
+ groupUpdater.groupUpdater().acceptAndThrowSilently(groupUpdater);
}
}
}
diff --git a/java/com/google/gerrit/acceptance/testsuite/project/TestProjectCreation.java b/java/com/google/gerrit/acceptance/testsuite/project/TestProjectCreation.java
index a0b130e..31af1d2 100644
--- a/java/com/google/gerrit/acceptance/testsuite/project/TestProjectCreation.java
+++ b/java/com/google/gerrit/acceptance/testsuite/project/TestProjectCreation.java
@@ -63,9 +63,9 @@
*
* @return the name of the created project
*/
- public Project.NameKey create() throws Exception {
+ public Project.NameKey create() {
TestProjectCreation creation = autoBuild();
- return creation.projectCreator().apply(creation);
+ return creation.projectCreator().applyAndThrowSilently(creation);
}
}
}
diff --git a/java/com/google/gerrit/elasticsearch/ElasticIndexVersionManager.java b/java/com/google/gerrit/elasticsearch/ElasticIndexVersionManager.java
index 8011efa..b9d86d5 100644
--- a/java/com/google/gerrit/elasticsearch/ElasticIndexVersionManager.java
+++ b/java/com/google/gerrit/elasticsearch/ElasticIndexVersionManager.java
@@ -17,7 +17,6 @@
import com.google.common.base.Strings;
import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.index.Index;
import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.Schema;
@@ -26,6 +25,7 @@
import com.google.gerrit.server.index.GerritIndexStatus;
import com.google.gerrit.server.index.OnlineUpgradeListener;
import com.google.gerrit.server.index.VersionManager;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -45,7 +45,7 @@
ElasticIndexVersionManager(
@GerritServerConfig Config cfg,
SitePaths sitePaths,
- DynamicSet<OnlineUpgradeListener> listeners,
+ PluginSetContext<OnlineUpgradeListener> listeners,
Collection<IndexDefinition<?, ?, ?>> defs,
ElasticIndexVersionDiscovery versionDiscovery) {
super(sitePaths, listeners, defs, VersionManager.getOnlineUpgrade(cfg));
diff --git a/java/com/google/gerrit/httpd/GitOverHttpServlet.java b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
index 2e1b562..197a3b9 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -38,6 +38,7 @@
import com.google.gerrit.server.permissions.PermissionBackend.RefFilterOptions;
import com.google.gerrit.server.permissions.PermissionBackendException;
import com.google.gerrit.server.permissions.ProjectPermission;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.ProjectCache;
import com.google.gerrit.server.project.ProjectState;
import com.google.gerrit.server.util.time.TimeUtil;
@@ -285,14 +286,14 @@
private final TransferConfig config;
private final DynamicSet<PreUploadHook> preUploadHooks;
private final DynamicSet<PostUploadHook> postUploadHooks;
- private final DynamicSet<UploadPackInitializer> uploadPackInitializers;
+ private final PluginSetContext<UploadPackInitializer> uploadPackInitializers;
@Inject
UploadFactory(
TransferConfig tc,
DynamicSet<PreUploadHook> preUploadHooks,
DynamicSet<PostUploadHook> postUploadHooks,
- DynamicSet<UploadPackInitializer> uploadPackInitializers) {
+ PluginSetContext<UploadPackInitializer> uploadPackInitializers) {
this.config = tc;
this.preUploadHooks = preUploadHooks;
this.postUploadHooks = postUploadHooks;
@@ -314,9 +315,7 @@
}
}
ProjectState state = (ProjectState) req.getAttribute(ATT_STATE);
- for (UploadPackInitializer initializer : uploadPackInitializers) {
- initializer.init(state.getNameKey(), up);
- }
+ uploadPackInitializers.runEach(initializer -> initializer.init(state.getNameKey(), up));
return up;
}
}
diff --git a/java/com/google/gerrit/httpd/UniversalWebLoginFilter.java b/java/com/google/gerrit/httpd/UniversalWebLoginFilter.java
index 4878006..6e32980 100644
--- a/java/com/google/gerrit/httpd/UniversalWebLoginFilter.java
+++ b/java/com/google/gerrit/httpd/UniversalWebLoginFilter.java
@@ -14,10 +14,11 @@
package com.google.gerrit.httpd;
-import com.google.gerrit.extensions.registration.DynamicItem;
import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.CurrentUser;
import com.google.gerrit.server.IdentifiedUser;
+import com.google.gerrit.server.plugincontext.PluginItemContext;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Provider;
import com.google.inject.Singleton;
@@ -34,8 +35,8 @@
import javax.servlet.http.HttpServletResponse;
public class UniversalWebLoginFilter implements Filter {
- private final DynamicItem<WebSession> session;
- private final DynamicSet<WebLoginListener> webLoginListeners;
+ private final PluginItemContext<WebSession> session;
+ private final PluginSetContext<WebLoginListener> webLoginListeners;
private final Provider<CurrentUser> userProvider;
public static ServletModule module() {
@@ -52,8 +53,8 @@
@Inject
public UniversalWebLoginFilter(
- DynamicItem<WebSession> session,
- DynamicSet<WebLoginListener> webLoginListeners,
+ PluginItemContext<WebSession> session,
+ PluginSetContext<WebLoginListener> webLoginListeners,
Provider<CurrentUser> userProvider) {
this.session = session;
this.webLoginListeners = webLoginListeners;
@@ -75,20 +76,18 @@
Optional<IdentifiedUser> loggedInUserAfter = loggedInUser();
if (!loggedInUserBefore.isPresent() && loggedInUserAfter.isPresent()) {
- for (WebLoginListener loginListener : webLoginListeners) {
- loginListener.onLogin(loggedInUserAfter.get(), httpRequest, wrappedResponse);
- }
+ webLoginListeners.runEach(
+ l -> l.onLogin(loggedInUserAfter.get(), httpRequest, wrappedResponse));
} else if (loggedInUserBefore.isPresent() && !loggedInUserAfter.isPresent()) {
- for (WebLoginListener loginListener : webLoginListeners) {
- loginListener.onLogout(loggedInUserBefore.get(), httpRequest, wrappedResponse);
- }
+ webLoginListeners.runEach(
+ l -> l.onLogout(loggedInUserBefore.get(), httpRequest, wrappedResponse));
}
wrappedResponse.play();
}
private Optional<IdentifiedUser> loggedInUser() {
- return session.get().isSignedIn()
+ return session.call(s -> s.isSignedIn())
? Optional.of(userProvider.get().asIdentifiedUser())
: Optional.empty();
}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java
index c7678c2..61132e7 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java
@@ -70,8 +70,8 @@
AccountResource accountResource = (AccountResource) rsrc;
report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId());
} else if (rsrc instanceof ProjectResource) {
- ProjectResource accountResource = (ProjectResource) rsrc;
- report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId());
+ ProjectResource projectResource = (ProjectResource) rsrc;
+ report = quotaBackend.currentUser().project(projectResource.getNameKey());
}
report.requestToken(quotaGroup(pathForQuotaReporting, req.getMethod())).throwOnError();
diff --git a/java/com/google/gerrit/lucene/LuceneVersionManager.java b/java/com/google/gerrit/lucene/LuceneVersionManager.java
index 63abea8..d3a1a24 100644
--- a/java/com/google/gerrit/lucene/LuceneVersionManager.java
+++ b/java/com/google/gerrit/lucene/LuceneVersionManager.java
@@ -16,7 +16,6 @@
import com.google.common.flogger.FluentLogger;
import com.google.common.primitives.Ints;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.index.Index;
import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.Schema;
@@ -25,6 +24,7 @@
import com.google.gerrit.server.index.GerritIndexStatus;
import com.google.gerrit.server.index.OnlineUpgradeListener;
import com.google.gerrit.server.index.VersionManager;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.io.IOException;
@@ -47,7 +47,7 @@
LuceneVersionManager(
@GerritServerConfig Config cfg,
SitePaths sitePaths,
- DynamicSet<OnlineUpgradeListener> listeners,
+ PluginSetContext<OnlineUpgradeListener> listeners,
Collection<IndexDefinition<?, ?, ?>> defs) {
super(sitePaths, listeners, defs, VersionManager.getOnlineUpgrade(cfg));
}
diff --git a/java/com/google/gerrit/server/auth/UniversalAuthBackend.java b/java/com/google/gerrit/server/auth/UniversalAuthBackend.java
index 94faeef..4e93ff2 100644
--- a/java/com/google/gerrit/server/auth/UniversalAuthBackend.java
+++ b/java/com/google/gerrit/server/auth/UniversalAuthBackend.java
@@ -16,7 +16,7 @@
import static java.util.Objects.requireNonNull;
-import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.ArrayList;
@@ -25,10 +25,10 @@
/** Universal implementation of the AuthBackend that works with the injected set of AuthBackends. */
@Singleton
public final class UniversalAuthBackend implements AuthBackend {
- private final DynamicSet<AuthBackend> authBackends;
+ private final PluginSetContext<AuthBackend> authBackends;
@Inject
- UniversalAuthBackend(DynamicSet<AuthBackend> authBackends) {
+ UniversalAuthBackend(PluginSetContext<AuthBackend> authBackends) {
this.authBackends = authBackends;
}
@@ -36,15 +36,16 @@
public AuthUser authenticate(AuthRequest request) throws AuthException {
List<AuthUser> authUsers = new ArrayList<>();
List<AuthException> authExs = new ArrayList<>();
- for (AuthBackend backend : authBackends) {
- try {
- authUsers.add(requireNonNull(backend.authenticate(request)));
- } catch (MissingCredentialsException ex) {
- // Not handled by this backend.
- } catch (AuthException ex) {
- authExs.add(ex);
- }
- }
+ authBackends.runEach(
+ backend -> {
+ try {
+ authUsers.add(requireNonNull(backend.authenticate(request)));
+ } catch (MissingCredentialsException ex) {
+ // Not handled by this backend.
+ } catch (AuthException ex) {
+ authExs.add(ex);
+ }
+ });
// Handle the valid responses
if (authUsers.size() == 1) {
diff --git a/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java b/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
index a7fdbbd..ee672cd 100644
--- a/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
+++ b/java/com/google/gerrit/server/cache/ForwardingRemovalListener.java
@@ -17,8 +17,8 @@
import com.google.common.base.Strings;
import com.google.common.cache.RemovalListener;
import com.google.common.cache.RemovalNotification;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.PluginName;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.assistedinject.Assisted;
@@ -35,13 +35,13 @@
ForwardingRemovalListener create(String cacheName);
}
- private final DynamicSet<CacheRemovalListener> listeners;
+ private final PluginSetContext<CacheRemovalListener> listeners;
private final String cacheName;
private String pluginName = PluginName.GERRIT;
@Inject
ForwardingRemovalListener(
- DynamicSet<CacheRemovalListener> listeners, @Assisted String cacheName) {
+ PluginSetContext<CacheRemovalListener> listeners, @Assisted String cacheName) {
this.listeners = listeners;
this.cacheName = cacheName;
}
@@ -56,8 +56,6 @@
@Override
@SuppressWarnings("unchecked")
public void onRemoval(RemovalNotification<K, V> notification) {
- for (CacheRemovalListener<K, V> l : listeners) {
- l.onRemoval(pluginName, cacheName, notification);
- }
+ listeners.runEach(l -> l.onRemoval(pluginName, cacheName, notification));
}
}
diff --git a/java/com/google/gerrit/server/config/GerritServerConfigReloader.java b/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
index 09c10740..5ecf6ed 100644
--- a/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
+++ b/java/com/google/gerrit/server/config/GerritServerConfigReloader.java
@@ -17,9 +17,9 @@
import com.google.common.collect.ArrayListMultimap;
import com.google.common.collect.Multimap;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.server.config.ConfigUpdatedEvent.ConfigUpdateEntry;
import com.google.gerrit.server.config.ConfigUpdatedEvent.UpdateResult;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@@ -29,11 +29,12 @@
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
private final GerritServerConfigProvider configProvider;
- private final DynamicSet<GerritConfigListener> configListeners;
+ private final PluginSetContext<GerritConfigListener> configListeners;
@Inject
GerritServerConfigReloader(
- GerritServerConfigProvider configProvider, DynamicSet<GerritConfigListener> configListeners) {
+ GerritServerConfigProvider configProvider,
+ PluginSetContext<GerritConfigListener> configListeners) {
this.configProvider = configProvider;
this.configListeners = configListeners;
}
@@ -53,9 +54,7 @@
public Multimap<UpdateResult, ConfigUpdateEntry> fireUpdatedConfigEvent(
ConfigUpdatedEvent event) {
Multimap<UpdateResult, ConfigUpdateEntry> updates = ArrayListMultimap.create();
- for (GerritConfigListener configListener : configListeners) {
- updates.putAll(configListener.configUpdated(event));
- }
+ configListeners.runEach(l -> updates.putAll(l.configUpdated(event)));
return updates;
}
}
diff --git a/java/com/google/gerrit/server/extensions/events/PluginEvent.java b/java/com/google/gerrit/server/extensions/events/PluginEvent.java
index 8680ab1..60d27c9 100644
--- a/java/com/google/gerrit/server/extensions/events/PluginEvent.java
+++ b/java/com/google/gerrit/server/extensions/events/PluginEvent.java
@@ -15,16 +15,16 @@
package com.google.gerrit.server.extensions.events;
import com.google.gerrit.extensions.events.PluginEventListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
@Singleton
public class PluginEvent {
- private final DynamicSet<PluginEventListener> listeners;
+ private final PluginSetContext<PluginEventListener> listeners;
@Inject
- PluginEvent(DynamicSet<PluginEventListener> listeners) {
+ PluginEvent(PluginSetContext<PluginEventListener> listeners) {
this.listeners = listeners;
}
@@ -33,9 +33,7 @@
return;
}
Event e = new Event(pluginName, type, data);
- for (PluginEventListener l : listeners) {
- l.onPluginEvent(e);
- }
+ listeners.runEach(l -> l.onPluginEvent(e));
}
private static class Event extends AbstractNoNotifyEvent implements PluginEventListener.Event {
diff --git a/java/com/google/gerrit/server/git/GarbageCollection.java b/java/com/google/gerrit/server/git/GarbageCollection.java
index 3624695..75c9012 100644
--- a/java/com/google/gerrit/server/git/GarbageCollection.java
+++ b/java/com/google/gerrit/server/git/GarbageCollection.java
@@ -18,10 +18,10 @@
import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.data.GarbageCollectionResult;
import com.google.gerrit.extensions.events.GarbageCollectorListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.reviewdb.client.Project;
import com.google.gerrit.server.config.GcConfig;
import com.google.gerrit.server.extensions.events.AbstractNoNotifyEvent;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import java.io.PrintWriter;
import java.util.List;
@@ -43,7 +43,7 @@
private final GitRepositoryManager repoManager;
private final GarbageCollectionQueue gcQueue;
private final GcConfig gcConfig;
- private final DynamicSet<GarbageCollectorListener> listeners;
+ private final PluginSetContext<GarbageCollectorListener> listeners;
public interface Factory {
GarbageCollection create();
@@ -54,7 +54,7 @@
GitRepositoryManager repoManager,
GarbageCollectionQueue gcQueue,
GcConfig config,
- DynamicSet<GarbageCollectorListener> listeners) {
+ PluginSetContext<GarbageCollectorListener> listeners) {
this.repoManager = repoManager;
this.gcQueue = gcQueue;
this.gcConfig = config;
@@ -113,13 +113,7 @@
return;
}
Event event = new Event(p, statistics);
- for (GarbageCollectorListener l : listeners) {
- try {
- l.onGarbageCollected(event);
- } catch (RuntimeException e) {
- logger.atWarning().withCause(e).log("Failure in GarbageCollectorListener");
- }
- }
+ listeners.runEach(l -> l.onGarbageCollected(event));
}
private static void logGcInfo(Project.NameKey projectName, String msg) {
diff --git a/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java b/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java
index 7adb21b..0f081be 100644
--- a/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java
+++ b/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java
@@ -14,7 +14,7 @@
package com.google.gerrit.server.git.receive;
-import com.google.gerrit.extensions.registration.DynamicSet;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import java.util.Collection;
import org.eclipse.jgit.transport.PostReceiveHook;
@@ -22,17 +22,15 @@
import org.eclipse.jgit.transport.ReceivePack;
class LazyPostReceiveHookChain implements PostReceiveHook {
- private final DynamicSet<PostReceiveHook> hooks;
+ private final PluginSetContext<PostReceiveHook> hooks;
@Inject
- LazyPostReceiveHookChain(DynamicSet<PostReceiveHook> hooks) {
+ LazyPostReceiveHookChain(PluginSetContext<PostReceiveHook> hooks) {
this.hooks = hooks;
}
@Override
public void onPostReceive(ReceivePack rp, Collection<ReceiveCommand> commands) {
- for (PostReceiveHook h : hooks) {
- h.onPostReceive(rp, commands);
- }
+ hooks.runEach(h -> h.onPostReceive(rp, commands));
}
}
diff --git a/java/com/google/gerrit/server/index/OnlineReindexer.java b/java/com/google/gerrit/server/index/OnlineReindexer.java
index ec0e1d4..8a1776d 100644
--- a/java/com/google/gerrit/server/index/OnlineReindexer.java
+++ b/java/com/google/gerrit/server/index/OnlineReindexer.java
@@ -18,11 +18,11 @@
import com.google.common.collect.Lists;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.index.Index;
import com.google.gerrit.index.IndexCollection;
import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.SiteIndexer;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import java.io.IOException;
import java.util.List;
import java.util.concurrent.atomic.AtomicBoolean;
@@ -35,7 +35,7 @@
private final SiteIndexer<K, V, I> batchIndexer;
private final int oldVersion;
private final int newVersion;
- private final DynamicSet<OnlineUpgradeListener> listeners;
+ private final PluginSetContext<OnlineUpgradeListener> listeners;
private I index;
private final AtomicBoolean running = new AtomicBoolean();
@@ -43,7 +43,7 @@
IndexDefinition<K, V, I> def,
int oldVersion,
int newVersion,
- DynamicSet<OnlineUpgradeListener> listeners) {
+ PluginSetContext<OnlineUpgradeListener> listeners) {
this.name = def.getName();
this.indexes = def.getIndexCollection();
this.batchIndexer = def.getSiteIndexer();
@@ -68,9 +68,7 @@
} finally {
running.set(false);
if (!ok) {
- for (OnlineUpgradeListener listener : listeners) {
- listener.onFailure(name, oldVersion, newVersion);
- }
+ listeners.runEach(listener -> listener.onFailure(name, oldVersion, newVersion));
}
}
}
@@ -94,9 +92,7 @@
}
private void reindex() throws IOException {
- for (OnlineUpgradeListener listener : listeners) {
- listener.onStart(name, oldVersion, newVersion);
- }
+ listeners.runEach(listener -> listener.onStart(name, oldVersion, newVersion));
index =
requireNonNull(
indexes.getWriteIndex(newVersion),
@@ -118,9 +114,7 @@
}
logger.atInfo().log("Reindex %s to version %s complete", name, version(index));
activateIndex();
- for (OnlineUpgradeListener listener : listeners) {
- listener.onSuccess(name, oldVersion, newVersion);
- }
+ listeners.runEach(listener -> listener.onSuccess(name, oldVersion, newVersion));
}
public void activateIndex() {
diff --git a/java/com/google/gerrit/server/index/VersionManager.java b/java/com/google/gerrit/server/index/VersionManager.java
index f37472c..3417379 100644
--- a/java/com/google/gerrit/server/index/VersionManager.java
+++ b/java/com/google/gerrit/server/index/VersionManager.java
@@ -21,13 +21,13 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Maps;
import com.google.gerrit.extensions.events.LifecycleListener;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.index.Index;
import com.google.gerrit.index.IndexCollection;
import com.google.gerrit.index.IndexDefinition;
import com.google.gerrit.index.IndexDefinition.IndexFactory;
import com.google.gerrit.index.Schema;
import com.google.gerrit.server.config.SitePaths;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.ProvisionException;
import java.io.IOException;
import java.util.Collection;
@@ -61,7 +61,7 @@
protected final String runReindexMsg;
protected final SitePaths sitePaths;
- private final DynamicSet<OnlineUpgradeListener> listeners;
+ private final PluginSetContext<OnlineUpgradeListener> listeners;
// The following fields must be accessed synchronized on this.
protected final Map<String, IndexDefinition<?, ?, ?>> defs;
@@ -69,7 +69,7 @@
protected VersionManager(
SitePaths sitePaths,
- DynamicSet<OnlineUpgradeListener> listeners,
+ PluginSetContext<OnlineUpgradeListener> listeners,
Collection<IndexDefinition<?, ?, ?>> defs,
boolean onlineUpgrade) {
this.sitePaths = sitePaths;
diff --git a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
index 01ac3e2..ff935d8 100644
--- a/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
+++ b/java/com/google/gerrit/server/notedb/rebuild/NoteDbMigrator.java
@@ -45,7 +45,6 @@
import com.google.common.util.concurrent.MoreExecutors;
import com.google.gerrit.common.FormatUtil;
import com.google.gerrit.common.Nullable;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.git.LockFailureException;
import com.google.gerrit.git.RefUpdateUtil;
import com.google.gerrit.reviewdb.client.Change;
@@ -72,6 +71,7 @@
import com.google.gerrit.server.notedb.PrimaryStorageMigrator.NoNoteDbStateException;
import com.google.gerrit.server.notedb.RepoSequence;
import com.google.gerrit.server.notedb.rebuild.ChangeRebuilder.NoPatchSetsException;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.project.NoSuchChangeException;
import com.google.gerrit.server.update.ChainedReceiveCommands;
import com.google.gerrit.server.util.ManualRequestContext;
@@ -150,7 +150,7 @@
private final WorkQueue workQueue;
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
- private final DynamicSet<NotesMigrationStateListener> listeners;
+ private final PluginSetContext<NotesMigrationStateListener> listeners;
private int threads;
private ImmutableList<Project.NameKey> projects = ImmutableList.of();
@@ -179,7 +179,7 @@
WorkQueue workQueue,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
- DynamicSet<NotesMigrationStateListener> listeners) {
+ PluginSetContext<NotesMigrationStateListener> listeners) {
// Reload gerrit.config/notedb.config on each migrator invocation, in case a previous
// migration in the same process modified the on-disk contents. This ensures the defaults for
// trial/autoMigrate get set correctly below.
@@ -390,7 +390,7 @@
private final ChangeRebuilderImpl rebuilder;
private final MutableNotesMigration globalNotesMigration;
private final PrimaryStorageMigrator primaryStorageMigrator;
- private final DynamicSet<NotesMigrationStateListener> listeners;
+ private final PluginSetContext<NotesMigrationStateListener> listeners;
private final ListeningExecutorService executor;
private final ImmutableList<Project.NameKey> projects;
@@ -416,7 +416,7 @@
ChangeRebuilderImpl rebuilder,
MutableNotesMigration globalNotesMigration,
PrimaryStorageMigrator primaryStorageMigrator,
- DynamicSet<NotesMigrationStateListener> listeners,
+ PluginSetContext<NotesMigrationStateListener> listeners,
ListeningExecutorService executor,
ImmutableList<Project.NameKey> projects,
ImmutableList<Change.Id> changes,
@@ -737,9 +737,7 @@
private void preStateChange(NotesMigrationState oldState, NotesMigrationState newState)
throws IOException {
- for (NotesMigrationStateListener listener : listeners) {
- listener.preStateChange(oldState, newState);
- }
+ listeners.runEach(l -> l.preStateChange(oldState, newState), IOException.class);
}
private void setControlFlags() throws MigrationException {
diff --git a/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java b/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java
index 4d89482..22cd84c 100644
--- a/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java
+++ b/java/com/google/gerrit/server/plugins/UniversalServerPluginProvider.java
@@ -15,8 +15,8 @@
package com.google.gerrit.server.plugins;
import com.google.common.flogger.FluentLogger;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.registration.PluginName;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.nio.file.Path;
@@ -28,10 +28,10 @@
class UniversalServerPluginProvider implements ServerPluginProvider {
private static final FluentLogger logger = FluentLogger.forEnclosingClass();
- private final DynamicSet<ServerPluginProvider> serverPluginProviders;
+ private final PluginSetContext<ServerPluginProvider> serverPluginProviders;
@Inject
- UniversalServerPluginProvider(DynamicSet<ServerPluginProvider> sf) {
+ UniversalServerPluginProvider(PluginSetContext<ServerPluginProvider> sf) {
this.serverPluginProviders = sf;
}
@@ -80,15 +80,16 @@
private List<ServerPluginProvider> providersForHandlingPlugin(Path srcPath) {
List<ServerPluginProvider> providers = new ArrayList<>();
- for (ServerPluginProvider serverPluginProvider : serverPluginProviders) {
- boolean handles = serverPluginProvider.handles(srcPath);
- logger.atFine().log(
- "File %s handled by %s ? => %s",
- srcPath, serverPluginProvider.getProviderPluginName(), handles);
- if (handles) {
- providers.add(serverPluginProvider);
- }
- }
+ serverPluginProviders.runEach(
+ serverPluginProvider -> {
+ boolean handles = serverPluginProvider.handles(srcPath);
+ logger.atFine().log(
+ "File %s handled by %s ? => %s",
+ srcPath, serverPluginProvider.getProviderPluginName(), handles);
+ if (handles) {
+ providers.add(serverPluginProvider);
+ }
+ });
return providers;
}
}
diff --git a/java/com/google/gerrit/server/restapi/account/CreateAccount.java b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
index 4185f36..2f9f2a9 100644
--- a/java/com/google/gerrit/server/restapi/account/CreateAccount.java
+++ b/java/com/google/gerrit/server/restapi/account/CreateAccount.java
@@ -27,7 +27,6 @@
import com.google.gerrit.extensions.annotations.RequiresCapability;
import com.google.gerrit.extensions.api.accounts.AccountInput;
import com.google.gerrit.extensions.common.AccountInfo;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.BadRequestException;
import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.ResourceConflictException;
@@ -51,6 +50,7 @@
import com.google.gerrit.server.group.db.InternalGroupUpdate;
import com.google.gerrit.server.mail.send.OutgoingEmailValidator;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.gerrit.server.ssh.SshKeyCache;
import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
@@ -73,7 +73,7 @@
private final SshKeyCache sshKeyCache;
private final Provider<AccountsUpdate> accountsUpdateProvider;
private final AccountLoader.Factory infoLoader;
- private final DynamicSet<AccountExternalIdCreator> externalIdCreators;
+ private final PluginSetContext<AccountExternalIdCreator> externalIdCreators;
private final Provider<GroupsUpdate> groupsUpdate;
private final OutgoingEmailValidator validator;
@@ -85,7 +85,7 @@
SshKeyCache sshKeyCache,
@UserInitiated Provider<AccountsUpdate> accountsUpdateProvider,
AccountLoader.Factory infoLoader,
- DynamicSet<AccountExternalIdCreator> externalIdCreators,
+ PluginSetContext<AccountExternalIdCreator> externalIdCreators,
@UserInitiated Provider<GroupsUpdate> groupsUpdate,
OutgoingEmailValidator validator) {
this.seq = seq;
@@ -131,9 +131,7 @@
}
extIds.add(ExternalId.createUsername(username, accountId, input.httpPassword));
- for (AccountExternalIdCreator c : externalIdCreators) {
- extIds.addAll(c.create(accountId, username, input.email));
- }
+ externalIdCreators.runEach(c -> extIds.addAll(c.create(accountId, username, input.email)));
try {
accountsUpdateProvider
diff --git a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
index f7959c8..854125c 100644
--- a/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
+++ b/java/com/google/gerrit/server/restapi/change/ReviewersUtil.java
@@ -402,7 +402,8 @@
return result;
}
- boolean needsConfirmation = result.size > maxAllowedWithoutConfirmation;
+ boolean needsConfirmation =
+ maxAllowedWithoutConfirmation > 0 && result.size > maxAllowedWithoutConfirmation;
if (needsConfirmation) {
logger.atFine().log(
"group %s needs confirmation to be added as reviewer, it has %d members",
diff --git a/java/com/google/gerrit/server/restapi/config/ListTopMenus.java b/java/com/google/gerrit/server/restapi/config/ListTopMenus.java
index 7a85bcd..c296a7d 100644
--- a/java/com/google/gerrit/server/restapi/config/ListTopMenus.java
+++ b/java/com/google/gerrit/server/restapi/config/ListTopMenus.java
@@ -14,10 +14,10 @@
package com.google.gerrit.server.restapi.config;
-import com.google.gerrit.extensions.registration.DynamicSet;
import com.google.gerrit.extensions.restapi.RestReadView;
import com.google.gerrit.extensions.webui.TopMenu;
import com.google.gerrit.server.config.ConfigResource;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
import com.google.inject.Inject;
import com.google.inject.Singleton;
import java.util.ArrayList;
@@ -25,19 +25,17 @@
@Singleton
class ListTopMenus implements RestReadView<ConfigResource> {
- private final DynamicSet<TopMenu> extensions;
+ private final PluginSetContext<TopMenu> extensions;
@Inject
- ListTopMenus(DynamicSet<TopMenu> extensions) {
+ ListTopMenus(PluginSetContext<TopMenu> extensions) {
this.extensions = extensions;
}
@Override
public List<TopMenu.MenuEntry> apply(ConfigResource resource) {
List<TopMenu.MenuEntry> entries = new ArrayList<>();
- for (TopMenu extension : extensions) {
- entries.addAll(extension.getEntries());
- }
+ extensions.runEach(extension -> entries.addAll(extension.getEntries()));
return entries;
}
}
diff --git a/java/com/google/gerrit/testing/GerritBaseTests.java b/java/com/google/gerrit/testing/GerritBaseTests.java
index 01fb85d..8f3a6a8 100644
--- a/java/com/google/gerrit/testing/GerritBaseTests.java
+++ b/java/com/google/gerrit/testing/GerritBaseTests.java
@@ -16,7 +16,7 @@
import com.google.common.base.CharMatcher;
import com.google.gwtorm.client.KeyUtil;
-import com.google.gwtorm.server.StandardKeyEncoder;
+import com.google.gwtorm.client.StandardKeyEncoder;
import org.junit.Ignore;
import org.junit.Rule;
import org.junit.rules.ExpectedException;
diff --git a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
index af3cf24..981ba34 100644
--- a/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
+++ b/javatests/com/google/gerrit/acceptance/rest/change/SuggestReviewersIT.java
@@ -14,6 +14,7 @@
package com.google.gerrit.acceptance.rest.change;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
import static com.google.common.truth.Truth.assertThat;
import static com.google.gerrit.server.group.SystemGroupBackend.ANONYMOUS_USERS;
import static java.util.stream.Collectors.toList;
@@ -23,33 +24,33 @@
import com.google.gerrit.acceptance.AbstractDaemonTest;
import com.google.gerrit.acceptance.GerritConfig;
import com.google.gerrit.acceptance.TestAccount;
+import com.google.gerrit.acceptance.testsuite.account.AccountOperations;
+import com.google.gerrit.acceptance.testsuite.group.GroupOperations;
import com.google.gerrit.acceptance.testsuite.project.ProjectOperations;
import com.google.gerrit.common.data.GlobalCapability;
import com.google.gerrit.extensions.api.accounts.EmailInput;
import com.google.gerrit.extensions.api.changes.ReviewInput;
import com.google.gerrit.extensions.common.ChangeInput;
-import com.google.gerrit.extensions.common.GroupInfo;
import com.google.gerrit.extensions.common.SuggestedReviewerInfo;
-import com.google.gerrit.extensions.restapi.IdString;
import com.google.gerrit.extensions.restapi.RestApiException;
-import com.google.gerrit.extensions.restapi.TopLevelResource;
+import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Project;
-import com.google.gerrit.server.group.InternalGroup;
-import com.google.gerrit.server.restapi.group.CreateGroup;
import com.google.inject.Inject;
-import java.util.Arrays;
import java.util.List;
+import java.util.Set;
+import java.util.stream.IntStream;
import org.junit.Before;
import org.junit.Test;
public class SuggestReviewersIT extends AbstractDaemonTest {
- @Inject private CreateGroup createGroup;
@Inject private ProjectOperations projectOperations;
+ @Inject private AccountOperations accountOperations;
+ @Inject private GroupOperations groupOperations;
- private InternalGroup group1;
- private InternalGroup group2;
- private InternalGroup group3;
+ private AccountGroup.UUID group1;
+ private AccountGroup.UUID group2;
+ private AccountGroup.UUID group3;
private TestAccount user1;
private TestAccount user2;
@@ -58,14 +59,14 @@
@Before
public void setUp() throws Exception {
- group1 = newGroup("users1");
- group2 = newGroup("users2");
- group3 = newGroup("users3");
-
- user1 = user("user1", "First1 Last1", group1);
- user2 = user("user2", "First2 Last2", group2);
- user3 = user("user3", "First3 Last3", group1, group2);
+ user1 = user("user1", "First1 Last1");
+ user2 = user("user2", "First2 Last2");
+ user3 = user("user3", "First3 Last3");
user4 = user("jdoe", "John Doe", "JDOE");
+
+ group1 = groupOperations.newGroup().name(name("users1")).members(user1.id, user3.id).create();
+ group2 = groupOperations.newGroup().name(name("users2")).members(user2.id, user3.id).create();
+ group3 = groupOperations.newGroup().name(name("users3")).members(user1.id).create();
}
@Test
@@ -107,7 +108,8 @@
assertReviewers(
reviewers, ImmutableList.of(user1, user2, user3), ImmutableList.of(group1, group2));
- reviewers = suggestReviewers(changeId, group3.getName(), 10);
+ String group3Name = groupOperations.group(group3).get().name();
+ reviewers = suggestReviewers(changeId, group3Name, 10);
assertReviewers(reviewers, ImmutableList.of(), ImmutableList.of(group3));
// Suggested accounts are ordered by activity. All users have no activity,
@@ -153,7 +155,7 @@
setApiUser(user3);
block("refs/*", "read", ANONYMOUS_USERS);
- allow("refs/*", "read", group1.getGroupUUID());
+ allow("refs/*", "read", group1);
reviewers = suggestReviewers(changeId, user2.username, 2);
assertThat(reviewers).isEmpty();
}
@@ -169,7 +171,7 @@
assertThat(reviewers).isEmpty();
setApiUser(user1); // Clear cached group info.
- allowGlobalCapabilities(group1.getGroupUUID(), GlobalCapability.VIEW_ALL_ACCOUNTS);
+ allowGlobalCapabilities(group1, GlobalCapability.VIEW_ALL_ACCOUNTS);
reviewers = suggestReviewers(changeId, user2.username, 2);
assertThat(reviewers).hasSize(1);
assertThat(Iterables.getOnlyElement(reviewers).account.name).isEqualTo(user2.fullName);
@@ -245,17 +247,11 @@
}
@Test
- @GerritConfig(name = "addreviewer.maxAllowed", value = "2")
+ @GerritConfig(name = "addreviewer.maxAllowed", value = "1")
@GerritConfig(name = "addreviewer.maxWithoutConfirmation", value = "1")
- public void suggestReviewersGroupSizeConsiderations() throws Exception {
- InternalGroup largeGroup = newGroup("large");
- InternalGroup mediumGroup = newGroup("medium");
-
- // Both groups have Administrator as a member. Add two users to large
- // group to push it past maxAllowed, and one to medium group to push it
- // past maxWithoutConfirmation.
- user("individual 0", "Test0 Last0", largeGroup, mediumGroup);
- user("individual 1", "Test1 Last1", largeGroup);
+ public void confirmationIsNeverRequestedForAccounts() throws Exception {
+ user("individual 0", "Test0 Last0");
+ user("individual 1", "Test1 Last1");
String changeId = createChange().getChangeId();
List<SuggestedReviewerInfo> reviewers;
@@ -267,21 +263,68 @@
reviewer = reviewers.get(0);
assertThat(reviewer.count).isEqualTo(1);
assertThat(reviewer.confirm).isNull();
+ }
+
+ @Test
+ @GerritConfig(name = "addreviewer.maxAllowed", value = "2")
+ @GerritConfig(name = "addreviewer.maxWithoutConfirmation", value = "1")
+ public void suggestReviewersGroupSizeConsiderations() throws Exception {
+ AccountGroup.UUID largeGroup = createGroupWithArbitraryMembers(3);
+ String largeGroupName = groupOperations.group(largeGroup).get().name();
+ AccountGroup.UUID mediumGroup = createGroupWithArbitraryMembers(2);
+ String mediumGroupName = groupOperations.group(mediumGroup).get().name();
+
+ String changeId = createChange().getChangeId();
+ List<SuggestedReviewerInfo> reviewers;
+ SuggestedReviewerInfo reviewer;
// Large group should never be suggested.
- reviewers = suggestReviewers(changeId, largeGroup.getName(), 10);
+ reviewers = suggestReviewers(changeId, largeGroupName, 10);
assertThat(reviewers).isEmpty();
// Medium group should be suggested with appropriate count and confirm.
- reviewers = suggestReviewers(changeId, mediumGroup.getName(), 10);
+ reviewers = suggestReviewers(changeId, mediumGroupName, 10);
assertThat(reviewers).hasSize(1);
reviewer = reviewers.get(0);
- assertThat(reviewer.group.name).isEqualTo(mediumGroup.getName());
+ assertThat(reviewer.group.id).isEqualTo(mediumGroup.get());
assertThat(reviewer.count).isEqualTo(2);
assertThat(reviewer.confirm).isTrue();
}
@Test
+ @GerritConfig(name = "addreviewer.maxAllowed", value = "20")
+ @GerritConfig(name = "addreviewer.maxWithoutConfirmation", value = "0")
+ public void confirmationIsNotNecessaryForLargeGroupWhenLimitIsRemoved() throws Exception {
+ String changeId = createChange().getChangeId();
+ int numMembers = 15;
+ AccountGroup.UUID largeGroup = createGroupWithArbitraryMembers(numMembers);
+ String groupName = groupOperations.group(largeGroup).get().name();
+
+ List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId, groupName, 10);
+
+ assertThat(reviewers).hasSize(1);
+ SuggestedReviewerInfo reviewer = Iterables.getOnlyElement(reviewers);
+ assertThat(reviewer.group.id).isEqualTo(largeGroup.get());
+ // Confirmation should not be necessary.
+ assertThat(reviewer.confirm).isNull();
+ }
+
+ @Test
+ @GerritConfig(name = "addreviewer.maxAllowed", value = "0")
+ public void largeGroupIsSuggestedWhenLimitIsRemoved() throws Exception {
+ String changeId = createChange().getChangeId();
+ int numMembers = 30;
+ AccountGroup.UUID largeGroup = createGroupWithArbitraryMembers(numMembers);
+ String groupName = groupOperations.group(largeGroup).get().name();
+
+ List<SuggestedReviewerInfo> reviewers = suggestReviewers(changeId, groupName, 10);
+
+ assertThat(reviewers).hasSize(1);
+ SuggestedReviewerInfo reviewer = Iterables.getOnlyElement(reviewers);
+ assertThat(reviewer.group.id).isEqualTo(largeGroup.get());
+ }
+
+ @Test
public void defaultReviewerSuggestion() throws Exception {
TestAccount user1 = user("customuser1", "User1");
TestAccount reviewer1 = user("customuser2", "User2");
@@ -492,21 +535,20 @@
return gApi.changes().id(changeId).suggestReviewers(query).withLimit(n).get();
}
- private InternalGroup newGroup(String name) throws Exception {
- GroupInfo group =
- createGroup.apply(TopLevelResource.INSTANCE, IdString.fromDecoded(name(name)), null);
- return group(new AccountGroup.UUID(group.id));
+ private AccountGroup.UUID createGroupWithArbitraryMembers(int numMembers) {
+ Set<Account.Id> members =
+ IntStream.rangeClosed(1, numMembers)
+ .mapToObj(i -> accountOperations.newAccount().create())
+ .collect(toImmutableSet());
+ return groupOperations.newGroup().members(members).create();
}
- private TestAccount user(String name, String fullName, String emailName, InternalGroup... groups)
- throws Exception {
- String[] groupNames = Arrays.stream(groups).map(InternalGroup::getName).toArray(String[]::new);
- return accountCreator.create(
- name(name), name(emailName) + "@example.com", fullName, groupNames);
+ private TestAccount user(String name, String fullName, String emailName) throws Exception {
+ return accountCreator.create(name(name), name(emailName) + "@example.com", fullName);
}
- private TestAccount user(String name, String fullName, InternalGroup... groups) throws Exception {
- return user(name, fullName, name, groups);
+ private TestAccount user(String name, String fullName) throws Exception {
+ return user(name, fullName, name);
}
private void reviewChange(String changeId) throws RestApiException {
@@ -527,10 +569,10 @@
return gApi.changes().create(ci).get().changeId;
}
- private void assertReviewers(
+ private static void assertReviewers(
List<SuggestedReviewerInfo> actual,
List<TestAccount> expectedUsers,
- List<InternalGroup> expectedGroups) {
+ List<AccountGroup.UUID> expectedGroups) {
List<Integer> actualAccountIds =
actual
.stream()
@@ -544,7 +586,7 @@
actual.stream().filter(i -> i.group != null).map(i -> i.group.id).collect(toList());
assertThat(actualGroupIds)
.containsExactlyElementsIn(
- expectedGroups.stream().map(g -> g.getGroupUUID().get()).collect(toList()))
+ expectedGroups.stream().map(AccountGroup.UUID::get).collect(toList()))
.inOrder();
}
}
diff --git a/plugins/replication b/plugins/replication
index 3ce09db..05b0999 160000
--- a/plugins/replication
+++ b/plugins/replication
@@ -1 +1 @@
-Subproject commit 3ce09db0cae09038f2924f1ff8f097e6c91c7a1e
+Subproject commit 05b0999845151b230d8fe0c38d43cd39bcba8093
diff --git a/proto/reviewdb.proto b/proto/reviewdb.proto
index 70dd91f..bc45eea 100644
--- a/proto/reviewdb.proto
+++ b/proto/reviewdb.proto
@@ -14,160 +14,198 @@
syntax = "proto2";
+package devtools.gerritcodereview;
+
option java_package = "com.google.gerrit.proto.reviewdb";
-package devtools.gerritcodereview;
+// Serialized form of com.google.gerrit.reviewdb.client.Change.Id.
+// Next ID: 2
message Change_Id {
- required int32 id = 1;
+ required int32 id = 1;
}
-message ChangeMessage_Key {
- required Change_Id change_id = 1;
- required string uuid = 2;
-}
-
-message PatchSet_Id {
- required Change_Id change_id = 1;
- required int32 patch_set_id = 2;
-}
-
-message Patch_Key {
- required PatchSet_Id patch_set_id = 1;
- required string file_name = 2;
-}
-
-message PatchLineComment_Key {
- required Patch_Key patch_key = 1;
- required string uuid = 2;
-}
-
-message Account_Id {
- required int32 id = 1;
-}
-
-message LabelId {
- required string id = 1;
-}
-
-message PatchSetApproval_Key {
- required PatchSet_Id patch_set_id = 1;
- required Account_Id account_id = 2;
- required LabelId category_id = 3;
-}
-
-message CurrentSchemaVersion_Key {
- required string one = 1;
-}
-
-message ChangeMessage {
- required ChangeMessage_Key key = 1;
- optional Account_Id author_id = 2;
- optional fixed64 written_on = 3;
- optional string message = 4;
- optional PatchSet_Id patchset = 5;
- optional string tag = 6;
- optional Account_Id real_author = 7;
-}
-
+// Serialized form of com.google.gerrit.reviewdb.client.Change.Key.
+// Next ID: 2
message Change_Key {
- optional string id = 1;
+ optional string id = 1;
}
-message Project_NameKey {
- optional string name = 1;
-}
-
-message Branch_NameKey {
- optional Project_NameKey project_name = 1;
- optional string branch_name = 2;
-}
-
+// Serialized form of com.google.gerrit.reviewdb.client.Change.
+// Next ID: 24
message Change {
- required Change_Id change_id = 1;
- optional Change_Key change_key = 2;
- optional int32 row_version = 3;
- optional fixed64 created_on = 4;
- optional fixed64 last_updated_on = 5;
- optional Account_Id owner_account_id = 7;
- optional Branch_NameKey dest = 8;
- optional uint32 status = 10;
- optional int32 current_patch_set_id = 12;
- optional string subject = 13;
- optional string topic = 14;
- optional string original_subject = 17;
- optional string submission_id = 18;
- optional Account_Id assignee = 19;
- optional bool is_private = 20;
- optional bool work_in_progress = 21;
- optional bool review_started = 22;
- optional Change_Id revert_of = 23;
- optional string note_db_state = 101;
+ required Change_Id change_id = 1;
+ optional Change_Key change_key = 2;
+ optional int32 row_version = 3;
+ optional fixed64 created_on = 4;
+ optional fixed64 last_updated_on = 5;
+ optional Account_Id owner_account_id = 7;
+ optional Branch_NameKey dest = 8;
+ optional uint32 status = 10;
+ optional int32 current_patch_set_id = 12;
+ optional string subject = 13;
+ optional string topic = 14;
+ optional string original_subject = 17;
+ optional string submission_id = 18;
+ optional Account_Id assignee = 19;
+ optional bool is_private = 20;
+ optional bool work_in_progress = 21;
+ optional bool review_started = 22;
+ optional Change_Id revert_of = 23;
+ optional string note_db_state = 101;
+
+ // Deleted fields, should not be reused:
+ reserved 6; // sortkey
+ reserved 9; // open
+ reserved 11; // nbrPatchSets
+ reserved 15; // lastSha1MergeTested
+ reserved 16; // mergeable
}
-message CommentRange {
- optional int32 start_line = 1;
- optional int32 start_character = 2;
- optional int32 end_line = 3;
- optional int32 end_character = 4;
+// Serialized form of com.google.gerrit.reviewdb.client.ChangeMessage.
+// Next ID: 3
+message ChangeMessage_Key {
+ required Change_Id change_id = 1;
+ required string uuid = 2;
}
-message PatchLineComment {
- required PatchLineComment_Key key = 1;
- optional int32 line_nbr = 2;
- optional Account_Id author_id = 3;
- optional fixed64 written_on = 4;
- optional uint32 status = 5;
- optional int32 side = 6;
- optional string message = 7;
- optional string parent_uuid = 8;
- optional CommentRange range = 9;
- optional string tag = 10;
- optional Account_Id real_author = 11;
- optional bool unresolved = 12;
+// Serialized form of com.google.gerrit.reviewdb.client.ChangeMessage.
+// Next ID: 8
+message ChangeMessage {
+ required ChangeMessage_Key key = 1;
+ optional Account_Id author_id = 2;
+ optional fixed64 written_on = 3;
+ optional string message = 4;
+ optional PatchSet_Id patchset = 5;
+ optional string tag = 6;
+ optional Account_Id real_author = 7;
}
-message PatchSetApproval {
- required PatchSetApproval_Key key = 1;
- optional int32 value = 2;
- optional fixed64 granted = 3;
- optional string tag = 6;
- optional Account_Id real_account_id = 7;
- optional bool post_submit = 8;
+// Serialized form of com.google.gerrit.reviewdb.client.Patch.Key.
+// Next ID: 3
+message Patch_Key {
+ required PatchSet_Id patch_set_id = 1;
+ required string file_name = 2;
}
-message RevId {
- optional string id = 1;
+// Serialized form of com.google.gerrit.reviewdb.client.PatchSet.Id.
+// Next ID: 3
+message PatchSet_Id {
+ required Change_Id change_id = 1;
+ required int32 patch_set_id = 2;
}
+// Serialized form of com.google.gerrit.reviewdb.client.PatchSet.
+// Next ID: 10
message PatchSet {
- required PatchSet_Id id = 1;
- optional RevId revision = 2;
- optional Account_Id uploader_account_id = 3;
- optional fixed64 created_on = 4;
- optional string groups = 6;
- optional string push_certificate = 8;
- optional string description = 9;
+ required PatchSet_Id id = 1;
+ optional RevId revision = 2;
+ optional Account_Id uploader_account_id = 3;
+ optional fixed64 created_on = 4;
+ optional string groups = 6;
+ optional string push_certificate = 8;
+ optional string description = 9;
+
+ // Deleted fields, should not be reused:
+ reserved 5; // draft
+ reserved 7; // pushCertficate
}
+// Serialized form of com.google.gerrit.reviewdb.client.PatchLineComment.Key.
+// Next ID: 3
+message PatchLineComment_Key {
+ required Patch_Key patch_key = 1;
+ required string uuid = 2;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.PatchLineComment.
+// Next ID: 13
+message PatchLineComment {
+ required PatchLineComment_Key key = 1;
+ optional int32 line_nbr = 2;
+ optional Account_Id author_id = 3;
+ optional fixed64 written_on = 4;
+ optional uint32 status = 5;
+ optional int32 side = 6;
+ optional string message = 7;
+ optional string parent_uuid = 8;
+ optional CommentRange range = 9;
+ optional string tag = 10;
+ optional Account_Id real_author = 11;
+ optional bool unresolved = 12;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.Account.Id.
+// Next ID: 2
+message Account_Id {
+ required int32 id = 1;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.LabelId.
+// Next ID: 2
+message LabelId {
+ required string id = 1;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.PatchSetApproval.Key.
+// Next ID: 4
+message PatchSetApproval_Key {
+ required PatchSet_Id patch_set_id = 1;
+ required Account_Id account_id = 2;
+ required LabelId category_id = 3;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.PatchSetApproval.
+// Next ID: 9
+message PatchSetApproval {
+ required PatchSetApproval_Key key = 1;
+ optional int32 value = 2;
+ optional fixed64 granted = 3;
+ optional string tag = 6;
+ optional Account_Id real_account_id = 7;
+ optional bool post_submit = 8;
+
+ // Deleted fields, should not be reused:
+ reserved 4; // changeOpen
+ reserved 5; // changeSortKey
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.CurrentSchemaVersion.Key.
+// Next ID: 2
+message CurrentSchemaVersion_Key {
+ required string one = 1;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.CurrentSchemaVersion.
+// Next ID: 3
message CurrentSchemaVersion {
- required CurrentSchemaVersion_Key singleton = 1;
- optional int32 version_nbr = 2;
+ required CurrentSchemaVersion_Key singleton = 1;
+ optional int32 version_nbr = 2;
}
-message AnyReviewDbPrimaryKey {
- optional CurrentSchemaVersion_Key schema_version = 1;
- optional Change_Id changes = 21;
- optional PatchSetApproval_Key patch_set_approvals = 22;
- optional ChangeMessage_Key change_messages = 23;
- optional PatchSet_Id patch_sets = 24;
- optional PatchLineComment_Key patch_comments = 26;
+// Serialized form of com.google.gerrit.reviewdb.client.Project.NameKey.
+// Next ID: 2
+message Project_NameKey {
+ optional string name = 1;
}
-message AnyReviewDb {
- optional CurrentSchemaVersion schema_version = 1;
- optional Change changes = 21;
- optional PatchSetApproval patch_set_approvals = 22;
- optional ChangeMessage change_messages = 23;
- optional PatchSet patch_sets = 24;
- optional PatchLineComment patch_comments = 26;
+// Serialized form of com.google.gerrit.reviewdb.client.Branch.NameKey.
+// Next ID: 3
+message Branch_NameKey {
+ optional Project_NameKey project_name = 1;
+ optional string branch_name = 2;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.CommentRange.
+// Next ID: 5
+message CommentRange {
+ optional int32 start_line = 1;
+ optional int32 start_character = 2;
+ optional int32 end_line = 3;
+ optional int32 end_character = 4;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.RevId.
+// Next ID: 2
+message RevId {
+ optional string id = 1;
}
