Merge "Send correct value for SuggestedReviewerInfo.confirm if limit is removed"
diff --git a/Documentation/dev-plugins.txt b/Documentation/dev-plugins.txt
index 82a9527..e84effd 100644
--- a/Documentation/dev-plugins.txt
+++ b/Documentation/dev-plugins.txt
@@ -2670,6 +2670,94 @@
are met, but marked as `OK`. If the requirements were not displayed, reviewers
would need to use their precious time to manually check that they were met.
+[[quota-enforcer]]
+== Quota Enforcer
+
+Gerrit provides an extension point that allows a plugin to enforce quota.
+link:quota.html[This documentation page] has a list of all quota requests that
+Gerrit core issues. Plugins can choose to respond to all or just a subset of
+requests. Some implementations might want to keep track of user quota in buckets,
+others might just check against instance or project state to enforce limits on how
+many projects can be created or how large a repository can become.
+
+Checking against instance state can be racy for concurrent requests as the server does not
+refill tokens if the action fails in a later stage (e.g. database failure). If
+plugins want to guarantee an absolute maximum on a resource, they have to do their own
+book-keeping.
+
+[source, java]
+----
+import com.google.server.quota.QuotaEnforcer;
+
+class ProjectLimiter implements QuotaEnforcer {
+ private final long maxNumberOfProjects = 100;
+ @Override
+ QuotaResponse requestTokens(String quotaGroup, QuotaRequestContext ctx, long numTokens) {
+ if (!"/projects/create".equals(quotaGroup)) {
+ return QuotaResponse.noOp();
+ }
+ // No deduction because we always check against the instance state (racy but fine for
+ // this plugin)
+ if (currentNumberOfProjects() + numTokens > maxNumberOfProjects) {
+ return QuotaResponse.error("too many projects");
+ }
+ return QuotaResponse.ok();
+ }
+
+ @Override
+ QuotaResponse dryRun(String quotaGroup, QuotaRequestContext ctx, long numTokens) {
+ // Since we are not keeping any state in this enforcer, we can simply call requestTokens().
+ return requestTokens(quotaGroup, ctx, numTokens);
+ }
+
+ void refill(String quotaGroup, QuotaRequestContext ctx, long numTokens) {
+ // No-op
+ }
+}
+----
+
+[source, java]
+----
+import com.google.server.quota.QuotaEnforcer;
+
+class ApiQpsEnforcer implements QuotaEnforcer {
+ // AutoRefillingPerUserBuckets is a imaginary bucket implementation that could be based on
+ // a loading cache or a commonly used bucketing algorithm.
+ private final AutoRefillingPerUserBuckets<CurrentUser, Long> buckets;
+ @Override
+ QuotaResponse requestTokens(String quotaGroup, QuotaRequestContext ctx, long numTokens) {
+ if (!quotaGroup.startsWith("/restapi/")) {
+ return QuotaResponse.noOp();
+ }
+ boolean success = buckets.deduct(ctx.user(), numTokens);
+ if (!success) {
+ return QuotaResponse.error("user sent too many qps, please wait for 5 minutes");
+ }
+ return QuotaResponse.ok();
+ }
+
+ @Override
+ QuotaResponse dryRun(String quotaGroup, QuotaRequestContext ctx, long numTokens) {
+ if (!quotaGroup.startsWith("/restapi/")) {
+ return QuotaResponse.noOp();
+ }
+ boolean success = buckets.checkOnly(ctx.user(), numTokens);
+ if (!success) {
+ return QuotaResponse.error("user sent too many qps, please wait for 5 minutes");
+ }
+ return QuotaResponse.ok();
+ }
+
+ @Override
+ void refill(String quotaGroup, QuotaRequestContext ctx, long numTokens) {
+ if (!quotaGroup.startsWith("/restapi/")) {
+ return;
+ }
+ buckets.add(ctx.user(), numTokens);
+ }
+}
+----
+
== SEE ALSO
diff --git a/Documentation/dev-release-subproject.txt b/Documentation/dev-release-subproject.txt
index 4886849..c9369b9 100644
--- a/Documentation/dev-release-subproject.txt
+++ b/Documentation/dev-release-subproject.txt
@@ -73,7 +73,7 @@
* Deploy the new release:
+
----
- mvn deploy
+ mvn deploy -DperformRelease=true
----
* Push the pom change(s) to the project's repository
diff --git a/Documentation/index.txt b/Documentation/index.txt
index 6011158..557cf90 100644
--- a/Documentation/index.txt
+++ b/Documentation/index.txt
@@ -85,6 +85,7 @@
.. link:js-api.html[JavaScript Plugin API]
.. link:config-validation.html[Validation Interfaces]
.. link:dev-stars.html[Starring Changes]
+.. link:quota.html[Quota Enforcement]
. link:dev-design.html[System Design]
. link:i18n-readme.html[i18n Support]
diff --git a/Documentation/quota.txt b/Documentation/quota.txt
new file mode 100644
index 0000000..a647e33
--- /dev/null
+++ b/Documentation/quota.txt
@@ -0,0 +1,50 @@
+= Gerrit Code Review - Quota
+
+Gerrit does not provide out of the box quota enforcement. However, it does
+support an extension mechanism for plugins to hook into to provide this
+functionality. The most prominent plugin is the
+link:https://gerrit.googlesource.com/plugins/quota/[Quota Plugin].
+
+This documentation is intended to be read by plugin developers. It contains all
+quota requests implemented in Gerrit-core as well as the metadata that they have
+associated.
+
+== Quota Groups
+
+The following quota groups are defined in core Gerrit:
+
+=== REST API
+[[rest-api]]
+
+The REST API enforces quota after the resource was parsed (if applicable) and before the
+endpoint's logic is executed. This enables quota enforcer implementations to throttle calls
+to specific endpoints while knowing the general context (user and top-level entity such as
+change, project or account).
+
+If the quota enforcer wants to throttle HTTP requests, they should use
+link:quota.html#http-requests[HTTP Requests] instead.
+
+The quota groups used for checking follow the exact definition of the endoint in the REST
+API, but remove all IDs. The schema is:
+
+/restapi/<ENDPOINT>:<HTTP-METHOD>
+
+Examples:
+
+[options="header",cols="1,6"]
+|=======================
+|HTTP call |Quota Group |Metadata
+|GET /a/changes/1/revisions/current/detail |/changes/revisions/detail:GET |CurrentUser, Change.Id, Project.NameKey
+|POST /a/changes/ |/changes/:POST |CurrentUser
+|GET /a/accounts/self/detail |/accounts/detail:GET |CurrentUser, Account.Id
+|=======================
+
+The user provided in the check's metadata is always the calling user (having the
+impersonation bit and real user set in case the user is impersonating another user).
+
+GERRIT
+------
+Part of link:index.html[Gerrit Code Review]
+
+SEARCHBOX
+---------
diff --git a/Documentation/rest-api-projects.txt b/Documentation/rest-api-projects.txt
index 49ab36f..238eda6 100644
--- a/Documentation/rest-api-projects.txt
+++ b/Documentation/rest-api-projects.txt
@@ -1192,21 +1192,19 @@
Content-Type: application/json; charset=UTF-8
{
- "remove": [
- {
- "refs/*": {
- "permissions": {
- "read": {
- "rules": {
- "c2ce4749a32ceb82cd6adcce65b8216e12afb41c": {
- "action": "ALLOW"
- }
+ "remove": {
+ "refs/*": {
+ "permissions": {
+ "read": {
+ "rules": {
+ "c2ce4749a32ceb82cd6adcce65b8216e12afb41c": {
+ "action": "ALLOW"
}
}
}
}
}
- ]
+ }
}
----
diff --git a/Documentation/rest-api.txt b/Documentation/rest-api.txt
index 8f6a47b..a8ab353 100644
--- a/Documentation/rest-api.txt
+++ b/Documentation/rest-api.txt
@@ -191,6 +191,11 @@
"`422 Unprocessable Entity`" is returned if the ID of a resource that is
specified in the request body cannot be resolved.
+==== 429 Too Many Requests
+"`429 Too Many Requests`" is returned if the request exhausted any set
+quota limits. Depending on the exhausted quota, the request may be retried
+with exponential backoff.
+
[[tracing]]
=== Request Tracing
For each REST endpoint tracing can be enabled by setting the
diff --git a/WORKSPACE b/WORKSPACE
index 6daa686..f0bdc70 100644
--- a/WORKSPACE
+++ b/WORKSPACE
@@ -1034,10 +1034,18 @@
sha1 = "4b7f0e0dc527fab032e9800ed231080fdc3ac015",
)
+TESTCONTAINERS_VERSION = "1.10.2"
+
maven_jar(
name = "testcontainers",
- artifact = "org.testcontainers:testcontainers:1.8.0",
- sha1 = "bc413912f7044f9f12aa0782853aef0a067ee52a",
+ artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION,
+ sha1 = "dfe35b1887685000fecee7f102bd8ce55643665c",
+)
+
+maven_jar(
+ name = "testcontainers-elasticsearch",
+ artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION,
+ sha1 = "c6eb4a3a0ad114929b659fa59c2ee9fe1c1d6a58",
)
maven_jar(
diff --git a/java/com/google/gerrit/acceptance/BUILD b/java/com/google/gerrit/acceptance/BUILD
index b0a39cf..7becf99 100644
--- a/java/com/google/gerrit/acceptance/BUILD
+++ b/java/com/google/gerrit/acceptance/BUILD
@@ -26,6 +26,7 @@
"//java/com/google/gerrit/pgm/util",
"//java/com/google/gerrit/reviewdb:server",
"//java/com/google/gerrit/server",
+ "//java/com/google/gerrit/server/audit",
"//java/com/google/gerrit/server/git/receive",
"//java/com/google/gerrit/server/project/testing:project-test-util",
"//java/com/google/gerrit/server/restapi",
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/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 e74c4b2..197a3b9 100644
--- a/java/com/google/gerrit/httpd/GitOverHttpServlet.java
+++ b/java/com/google/gerrit/httpd/GitOverHttpServlet.java
@@ -15,7 +15,7 @@
package com.google.gerrit.httpd;
import com.google.common.cache.Cache;
-import com.google.common.collect.ArrayListMultimap;
+import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Lists;
import com.google.gerrit.common.data.Capable;
@@ -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;
@@ -52,6 +53,7 @@
import java.util.Arrays;
import java.util.Collections;
import java.util.HashSet;
+import java.util.Map;
import java.util.Set;
import javax.servlet.Filter;
import javax.servlet.FilterChain;
@@ -61,6 +63,7 @@
import javax.servlet.ServletResponse;
import javax.servlet.http.HttpServletRequest;
import javax.servlet.http.HttpServletResponse;
+import javax.servlet.http.HttpServletResponseWrapper;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
import org.eclipse.jgit.http.server.GitServlet;
import org.eclipse.jgit.http.server.GitSmartHttpTools;
@@ -130,6 +133,55 @@
}
}
+ static class HttpServletResponseWithStatusWrapper extends HttpServletResponseWrapper {
+ private int responseStatus;
+
+ HttpServletResponseWithStatusWrapper(HttpServletResponse response) {
+ super(response);
+ /* Even if we could read the status from response, we assume that it is all
+ * fine because we entered the filter without any prior issues.
+ * When Google will have upgraded to Servlet 3.0, we could actually
+ * call response.getStatus() and the code will be clearer.
+ */
+ responseStatus = HttpServletResponse.SC_OK;
+ }
+
+ @Override
+ public void setStatus(int sc) {
+ responseStatus = sc;
+ super.setStatus(sc);
+ }
+
+ @SuppressWarnings("deprecation")
+ @Override
+ public void setStatus(int sc, String sm) {
+ responseStatus = sc;
+ super.setStatus(sc, sm);
+ }
+
+ @Override
+ public void sendError(int sc) throws IOException {
+ this.responseStatus = sc;
+ super.sendError(sc);
+ }
+
+ @Override
+ public void sendError(int sc, String msg) throws IOException {
+ this.responseStatus = sc;
+ super.sendError(sc, msg);
+ }
+
+ @Override
+ public void sendRedirect(String location) throws IOException {
+ this.responseStatus = HttpServletResponse.SC_MOVED_TEMPORARILY;
+ super.sendRedirect(location);
+ }
+
+ public int getResponseStatus() {
+ return responseStatus;
+ }
+ }
+
@Inject
GitOverHttpServlet(
Resolver resolver,
@@ -156,19 +208,15 @@
}
private static ListMultimap<String, String> extractParameters(HttpServletRequest request) {
-
- ListMultimap<String, String> multiMap = ArrayListMultimap.create();
- if (request.getQueryString() != null) {
- request
- .getParameterMap()
- .forEach(
- (k, v) -> {
- for (int i = 0; i < v.length; i++) {
- multiMap.put(k, v[i]);
- }
- });
+ if (request.getQueryString() == null) {
+ return ImmutableListMultimap.of();
}
- return multiMap;
+ // Explicit cast is required to compile under Servlet API 2.5, where the return type is raw Map.
+ @SuppressWarnings("cast")
+ Map<String, String[]> parameterMap = (Map<String, String[]>) request.getParameterMap();
+ ImmutableListMultimap.Builder<String, String> b = ImmutableListMultimap.builder();
+ parameterMap.forEach(b::putAll);
+ return b.build();
}
static class Resolver implements RepositoryResolver<HttpServletRequest> {
@@ -238,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;
@@ -267,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;
}
}
@@ -301,41 +347,48 @@
UploadPack up = (UploadPack) request.getAttribute(ServletUtils.ATTRIBUTE_HANDLER);
PermissionBackend.ForProject perm =
permissionBackend.currentUser().project(state.getNameKey());
+ HttpServletResponseWithStatusWrapper responseWrapper =
+ new HttpServletResponseWithStatusWrapper((HttpServletResponse) response);
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
+ String sessionId = httpRequest.getSession().getId();
+
try {
- perm.check(ProjectPermission.RUN_UPLOAD_PACK);
- } catch (AuthException e) {
- GitSmartHttpTools.sendError(
- (HttpServletRequest) request,
- (HttpServletResponse) response,
- HttpServletResponse.SC_FORBIDDEN,
- "upload-pack not permitted on this server");
- return;
- } catch (PermissionBackendException e) {
- throw new ServletException(e);
+ try {
+ perm.check(ProjectPermission.RUN_UPLOAD_PACK);
+ } catch (AuthException e) {
+ GitSmartHttpTools.sendError(
+ (HttpServletRequest) request,
+ responseWrapper,
+ HttpServletResponse.SC_FORBIDDEN,
+ "upload-pack not permitted on this server");
+ return;
+ } catch (PermissionBackendException e) {
+ responseWrapper.sendError(HttpServletResponse.SC_INTERNAL_SERVER_ERROR);
+ throw new ServletException(e);
+ }
+
+ // We use getRemoteHost() here instead of getRemoteAddr() because REMOTE_ADDR
+ // may have been overridden by a proxy server -- we'll try to avoid this.
+ UploadValidators uploadValidators =
+ uploadValidatorsFactory.create(state.getProject(), repo, request.getRemoteHost());
+ up.setPreUploadHook(
+ PreUploadHookChain.newChain(
+ Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
+ up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
+ next.doFilter(httpRequest, responseWrapper);
} finally {
- HttpServletRequest httpRequest = (HttpServletRequest) request;
- HttpServletResponse httpResponse = (HttpServletResponse) response;
groupAuditService.dispatch(
new HttpAuditEvent(
- httpRequest.getSession().getId(),
+ sessionId,
userProvider.get(),
extractWhat(httpRequest),
TimeUtil.nowMs(),
extractParameters(httpRequest),
httpRequest.getMethod(),
httpRequest,
- httpResponse.getStatus(),
- httpResponse));
+ responseWrapper.getResponseStatus(),
+ responseWrapper));
}
-
- // We use getRemoteHost() here instead of getRemoteAddr() because REMOTE_ADDR
- // may have been overridden by a proxy server -- we'll try to avoid this.
- UploadValidators uploadValidators =
- uploadValidatorsFactory.create(state.getProject(), repo, request.getRemoteHost());
- up.setPreUploadHook(
- PreUploadHookChain.newChain(Lists.newArrayList(up.getPreUploadHook(), uploadValidators)));
- up.setAdvertiseRefsHook(new DefaultAdvertiseRefsHook(perm, RefFilterOptions.defaults()));
- next.doFilter(request, response);
}
@Override
@@ -411,25 +464,28 @@
rp.getAdvertiseRefsHook().advertiseRefs(rp);
ProjectState state = (ProjectState) request.getAttribute(ATT_STATE);
+ HttpServletResponseWithStatusWrapper responseWrapper =
+ new HttpServletResponseWithStatusWrapper((HttpServletResponse) response);
+ HttpServletRequest httpRequest = (HttpServletRequest) request;
Capable canUpload;
try {
- permissionBackend
- .currentUser()
- .project(state.getNameKey())
- .check(ProjectPermission.RUN_RECEIVE_PACK);
- canUpload = arc.canUpload();
- } catch (AuthException e) {
- GitSmartHttpTools.sendError(
- (HttpServletRequest) request,
- (HttpServletResponse) response,
- HttpServletResponse.SC_FORBIDDEN,
- "receive-pack not permitted on this server");
- return;
- } catch (PermissionBackendException e) {
- throw new RuntimeException(e);
+ try {
+ permissionBackend
+ .currentUser()
+ .project(state.getNameKey())
+ .check(ProjectPermission.RUN_RECEIVE_PACK);
+ canUpload = arc.canUpload();
+ } catch (AuthException e) {
+ GitSmartHttpTools.sendError(
+ httpRequest,
+ responseWrapper,
+ HttpServletResponse.SC_FORBIDDEN,
+ "receive-pack not permitted on this server");
+ return;
+ } catch (PermissionBackendException e) {
+ throw new RuntimeException(e);
+ }
} finally {
- HttpServletRequest httpRequest = (HttpServletRequest) request;
- HttpServletResponse httpResponse = (HttpServletResponse) response;
groupAuditService.dispatch(
new HttpAuditEvent(
httpRequest.getSession().getId(),
@@ -439,26 +495,26 @@
extractParameters(httpRequest),
httpRequest.getMethod(),
httpRequest,
- httpResponse.getStatus(),
- httpResponse));
+ responseWrapper.getResponseStatus(),
+ responseWrapper));
}
if (canUpload != Capable.OK) {
GitSmartHttpTools.sendError(
- (HttpServletRequest) request,
- (HttpServletResponse) response,
+ httpRequest,
+ responseWrapper,
HttpServletResponse.SC_FORBIDDEN,
"\n" + canUpload.getMessage());
return;
}
if (!rp.isCheckReferencedObjectsAreReachable()) {
- chain.doFilter(request, response);
+ chain.doFilter(request, responseWrapper);
return;
}
if (!(userProvider.get().isIdentifiedUser())) {
- chain.doFilter(request, response);
+ chain.doFilter(request, responseWrapper);
return;
}
@@ -475,7 +531,7 @@
}
}
- chain.doFilter(request, response);
+ chain.doFilter(request, responseWrapper);
if (isGet) {
cache.put(cacheKey, Collections.unmodifiableSet(new HashSet<>(rp.getAdvertisedObjects())));
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
new file mode 100644
index 0000000..61132e7
--- /dev/null
+++ b/java/com/google/gerrit/httpd/restapi/RestApiQuotaEnforcer.java
@@ -0,0 +1,83 @@
+// Copyright (C) 2018 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.restapi;
+
+import com.google.gerrit.extensions.restapi.RestResource;
+import com.google.gerrit.server.account.AccountResource;
+import com.google.gerrit.server.change.ChangeResource;
+import com.google.gerrit.server.project.ProjectResource;
+import com.google.gerrit.server.quota.QuotaBackend;
+import com.google.gerrit.server.quota.QuotaException;
+import com.google.gerrit.util.http.RequestUtil;
+import javax.inject.Inject;
+import javax.servlet.http.HttpServletRequest;
+
+/**
+ * Enforces quota on specific REST API endpoints.
+ *
+ * <p>Examples:
+ *
+ * <ul>
+ * <li>GET /a/accounts/self/detail => /restapi/accounts/detail:GET
+ * <li>GET /changes/123/revisions/current/detail => /restapi/changes/revisions/detail:GET
+ * <li>PUT /changes/10/reviewed => /changes/reviewed:PUT
+ * </ul>
+ *
+ * <p>Adds context (change, project, account) to the quota check if the call is for an existing
+ * entity that was successfully parsed. This quota check is generally enforced after the resource
+ * was parsed, but before the view is executed. If a quota enforcer desires to throttle earlier,
+ * they should consider quota groups in the {@code /http/*} space.
+ */
+public class RestApiQuotaEnforcer {
+ private final QuotaBackend quotaBackend;
+
+ @Inject
+ RestApiQuotaEnforcer(QuotaBackend quotaBackend) {
+ this.quotaBackend = quotaBackend;
+ }
+
+ /** Enforce quota on a request not tied to any {@code RestResource}. */
+ void enforce(HttpServletRequest req) throws QuotaException {
+ String pathForQuotaReporting = RequestUtil.getRestPathWithoutIds(req);
+ quotaBackend
+ .currentUser()
+ .requestToken(quotaGroup(pathForQuotaReporting, req.getMethod()))
+ .throwOnError();
+ }
+
+ /** Enforce quota on a request for a given resource. */
+ void enforce(RestResource rsrc, HttpServletRequest req) throws QuotaException {
+ String pathForQuotaReporting = RequestUtil.getRestPathWithoutIds(req);
+ // Enrich the quota request we are operating on an interesting collection
+ QuotaBackend.WithResource report = quotaBackend.currentUser();
+ if (rsrc instanceof ChangeResource) {
+ ChangeResource changeResource = (ChangeResource) rsrc;
+ report =
+ quotaBackend.currentUser().change(changeResource.getId(), changeResource.getProject());
+ } else if (rsrc instanceof AccountResource) {
+ AccountResource accountResource = (AccountResource) rsrc;
+ report = quotaBackend.currentUser().account(accountResource.getUser().getAccountId());
+ } else if (rsrc instanceof ProjectResource) {
+ ProjectResource projectResource = (ProjectResource) rsrc;
+ report = quotaBackend.currentUser().project(projectResource.getNameKey());
+ }
+
+ report.requestToken(quotaGroup(pathForQuotaReporting, req.getMethod())).throwOnError();
+ }
+
+ private static String quotaGroup(String path, String method) {
+ return "/restapi" + path + ":" + method;
+ }
+}
diff --git a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
index ec71d8f..c9f3a77 100644
--- a/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
+++ b/java/com/google/gerrit/httpd/restapi/RestApiServlet.java
@@ -114,6 +114,7 @@
import com.google.gerrit.server.permissions.GlobalPermission;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
+import com.google.gerrit.server.quota.QuotaException;
import com.google.gerrit.server.update.UpdateException;
import com.google.gerrit.server.util.time.TimeUtil;
import com.google.gerrit.util.http.CacheHeaders;
@@ -227,6 +228,7 @@
final AuditService auditService;
final RestApiMetrics metrics;
final Pattern allowOrigin;
+ final RestApiQuotaEnforcer quotaChecker;
@Inject
Globals(
@@ -236,6 +238,7 @@
PermissionBackend permissionBackend,
AuditService auditService,
RestApiMetrics metrics,
+ RestApiQuotaEnforcer quotaChecker,
@GerritServerConfig Config cfg) {
this.currentUser = currentUser;
this.webSession = webSession;
@@ -243,6 +246,7 @@
this.permissionBackend = permissionBackend;
this.auditService = auditService;
this.metrics = metrics;
+ this.quotaChecker = quotaChecker;
allowOrigin = makeAllowOrigin(cfg);
}
@@ -317,6 +321,7 @@
viewData = new ViewData(null, null);
if (path.isEmpty()) {
+ globals.quotaChecker.enforce(req);
if (rc instanceof NeedsParams) {
((NeedsParams) rc).setParams(qp.params());
}
@@ -339,6 +344,7 @@
IdString id = path.remove(0);
try {
rsrc = rc.parse(rsrc, id);
+ globals.quotaChecker.enforce(rsrc, req);
if (path.isEmpty()) {
checkPreconditions(req);
}
@@ -346,6 +352,7 @@
if (!path.isEmpty()) {
throw e;
}
+ globals.quotaChecker.enforce(req);
if (isPost(req) || isPut(req)) {
RestView<RestResource> createView = rc.views().get(PluginName.GERRIT, "CREATE./");
@@ -602,6 +609,9 @@
status = SC_INTERNAL_SERVER_ERROR;
responseBytes = handleException(e, req, res);
}
+ } catch (QuotaException e) {
+ responseBytes =
+ replyError(req, res, status = 429, messageOr(e, "Quota limit reached"), e.caching(), e);
} catch (Exception e) {
status = SC_INTERNAL_SERVER_ERROR;
responseBytes = handleException(e, req, res);
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/pgm/init/GroupsOnInit.java b/java/com/google/gerrit/pgm/init/GroupsOnInit.java
index 8e06aa1..584d8af 100644
--- a/java/com/google/gerrit/pgm/init/GroupsOnInit.java
+++ b/java/com/google/gerrit/pgm/init/GroupsOnInit.java
@@ -25,7 +25,6 @@
import com.google.gerrit.pgm.init.api.InitFlags;
import com.google.gerrit.reviewdb.client.Account;
import com.google.gerrit.reviewdb.client.AccountGroup;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.GerritPersonIdentProvider;
import com.google.gerrit.server.config.AllUsersName;
import com.google.gerrit.server.config.GerritServerIdProvider;
@@ -37,7 +36,6 @@
import com.google.gerrit.server.group.db.GroupConfig;
import com.google.gerrit.server.group.db.GroupNameNotes;
import com.google.gerrit.server.group.db.InternalGroupUpdate;
-import com.google.gwtorm.server.OrmException;
import com.google.inject.Inject;
import java.io.File;
import java.io.IOException;
@@ -54,9 +52,8 @@
/**
* A database accessor for calls related to groups.
*
- * <p>All calls which read or write group related details to the database <strong>during
- * init</strong> (either ReviewDb or NoteDb) are gathered here. For non-init cases, use {@code
- * Groups} or {@code GroupsUpdate} instead.
+ * <p>All calls which read or write group related details to the NoteDb <strong>during init</strong>
+ * are gathered here. For non-init cases, use {@code Groups} or {@code GroupsUpdate} instead.
*
* <p>All methods of this class refer to <em>internal</em> groups.
*/
@@ -76,16 +73,14 @@
/**
* Returns the {@code AccountGroup} for the specified {@code GroupReference}.
*
- * @param db the {@code ReviewDb} instance to use for lookups
* @param groupReference the {@code GroupReference} of the group
* @return the {@code InternalGroup} represented by the {@code GroupReference}
- * @throws OrmException if the group couldn't be retrieved from ReviewDb
* @throws IOException if an error occurs while reading from NoteDb
* @throws ConfigInvalidException if the data in NoteDb is in an incorrect format
* @throws NoSuchGroupException if a group with such a name doesn't exist
*/
- public InternalGroup getExistingGroup(ReviewDb db, GroupReference groupReference)
- throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
+ public InternalGroup getExistingGroup(GroupReference groupReference)
+ throws NoSuchGroupException, IOException, ConfigInvalidException {
File allUsersRepoPath = getPathToAllUsersRepository();
if (allUsersRepoPath != null) {
try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
@@ -102,14 +97,11 @@
/**
* Returns {@code GroupReference}s for all internal groups.
*
- * @param db the {@code ReviewDb} instance to use for lookups
* @return a stream of the {@code GroupReference}s of all internal groups
- * @throws OrmException if an error occurs while reading from ReviewDb
* @throws IOException if an error occurs while reading from NoteDb
* @throws ConfigInvalidException if the data in NoteDb is in an incorrect format
*/
- public Stream<GroupReference> getAllGroupReferences(ReviewDb db)
- throws OrmException, IOException, ConfigInvalidException {
+ public Stream<GroupReference> getAllGroupReferences() throws IOException, ConfigInvalidException {
File allUsersRepoPath = getPathToAllUsersRepository();
if (allUsersRepoPath != null) {
try (Repository allUsersRepo = new FileRepository(allUsersRepoPath)) {
@@ -126,14 +118,12 @@
* <p><strong>Note</strong>: This method doesn't check whether the account exists! It also doesn't
* update the account index!
*
- * @param db the {@code ReviewDb} instance to update
* @param groupUuid the UUID of the group
* @param account the account to add
- * @throws OrmException if an error occurs while reading/writing from/to ReviewDb
* @throws NoSuchGroupException if the specified group doesn't exist
*/
- public void addGroupMember(ReviewDb db, AccountGroup.UUID groupUuid, Account account)
- throws OrmException, NoSuchGroupException, IOException, ConfigInvalidException {
+ public void addGroupMember(AccountGroup.UUID groupUuid, Account account)
+ throws NoSuchGroupException, IOException, ConfigInvalidException {
File allUsersRepoPath = getPathToAllUsersRepository();
if (allUsersRepoPath != null) {
try (Repository repository = new FileRepository(allUsersRepoPath)) {
diff --git a/java/com/google/gerrit/pgm/init/InitAdminUser.java b/java/com/google/gerrit/pgm/init/InitAdminUser.java
index f12fa50..f19cf39 100644
--- a/java/com/google/gerrit/pgm/init/InitAdminUser.java
+++ b/java/com/google/gerrit/pgm/init/InitAdminUser.java
@@ -26,7 +26,6 @@
import com.google.gerrit.pgm.init.api.InitStep;
import com.google.gerrit.pgm.init.api.SequencesOnInit;
import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.server.ReviewDb;
import com.google.gerrit.server.account.AccountSshKey;
import com.google.gerrit.server.account.AccountState;
import com.google.gerrit.server.account.externalids.ExternalId;
@@ -37,7 +36,6 @@
import com.google.gerrit.server.index.group.GroupIndex;
import com.google.gerrit.server.index.group.GroupIndexCollection;
import com.google.gerrit.server.util.time.TimeUtil;
-import com.google.gwtorm.server.SchemaFactory;
import com.google.inject.Inject;
import java.io.IOException;
import java.nio.file.Files;
@@ -57,7 +55,6 @@
private final ExternalIdsOnInit externalIds;
private final SequencesOnInit sequencesOnInit;
private final GroupsOnInit groupsOnInit;
- private SchemaFactory<ReviewDb> dbFactory;
private AccountIndexCollection accountIndexCollection;
private GroupIndexCollection groupIndexCollection;
@@ -84,11 +81,6 @@
@Override
public void run() {}
- @Inject(optional = true)
- void set(SchemaFactory<ReviewDb> dbFactory) {
- this.dbFactory = dbFactory;
- }
-
@Inject
void set(AccountIndexCollection accountIndexCollection) {
this.accountIndexCollection = accountIndexCollection;
@@ -106,58 +98,56 @@
return;
}
- try (ReviewDb db = dbFactory.open()) {
- if (!accounts.hasAnyAccount()) {
- ui.header("Gerrit Administrator");
- if (ui.yesno(true, "Create administrator user")) {
- Account.Id id = new Account.Id(sequencesOnInit.nextAccountId(db));
- String username = ui.readString("admin", "username");
- String name = ui.readString("Administrator", "name");
- String httpPassword = ui.readString("secret", "HTTP password");
- AccountSshKey sshKey = readSshKey(id);
- String email = readEmail(sshKey);
+ if (!accounts.hasAnyAccount()) {
+ ui.header("Gerrit Administrator");
+ if (ui.yesno(true, "Create administrator user")) {
+ Account.Id id = new Account.Id(sequencesOnInit.nextAccountId());
+ String username = ui.readString("admin", "username");
+ String name = ui.readString("Administrator", "name");
+ String httpPassword = ui.readString("secret", "HTTP password");
+ AccountSshKey sshKey = readSshKey(id);
+ String email = readEmail(sshKey);
- List<ExternalId> extIds = new ArrayList<>(2);
- extIds.add(ExternalId.createUsername(username, id, httpPassword));
+ List<ExternalId> extIds = new ArrayList<>(2);
+ extIds.add(ExternalId.createUsername(username, id, httpPassword));
- if (email != null) {
- extIds.add(ExternalId.createEmail(id, email));
- }
- externalIds.insert("Add external IDs for initial admin user", extIds);
+ if (email != null) {
+ extIds.add(ExternalId.createEmail(id, email));
+ }
+ externalIds.insert("Add external IDs for initial admin user", extIds);
- Account a = new Account(id, TimeUtil.nowTs());
- a.setFullName(name);
- a.setPreferredEmail(email);
- accounts.insert(a);
+ Account a = new Account(id, TimeUtil.nowTs());
+ a.setFullName(name);
+ a.setPreferredEmail(email);
+ accounts.insert(a);
- // Only two groups should exist at this point in time and hence iterating over all of them
- // is cheap.
- Optional<GroupReference> adminGroupReference =
- groupsOnInit
- .getAllGroupReferences(db)
- .filter(group -> group.getName().equals("Administrators"))
- .findAny();
- if (!adminGroupReference.isPresent()) {
- throw new NoSuchGroupException("Administrators");
- }
- GroupReference adminGroup = adminGroupReference.get();
- groupsOnInit.addGroupMember(db, adminGroup.getUUID(), a);
+ // Only two groups should exist at this point in time and hence iterating over all of them
+ // is cheap.
+ Optional<GroupReference> adminGroupReference =
+ groupsOnInit
+ .getAllGroupReferences()
+ .filter(group -> group.getName().equals("Administrators"))
+ .findAny();
+ if (!adminGroupReference.isPresent()) {
+ throw new NoSuchGroupException("Administrators");
+ }
+ GroupReference adminGroup = adminGroupReference.get();
+ groupsOnInit.addGroupMember(adminGroup.getUUID(), a);
- if (sshKey != null) {
- VersionedAuthorizedKeysOnInit authorizedKeys = authorizedKeysFactory.create(id).load();
- authorizedKeys.addKey(sshKey.sshPublicKey());
- authorizedKeys.save("Add SSH key for initial admin user\n");
- }
+ if (sshKey != null) {
+ VersionedAuthorizedKeysOnInit authorizedKeys = authorizedKeysFactory.create(id).load();
+ authorizedKeys.addKey(sshKey.sshPublicKey());
+ authorizedKeys.save("Add SSH key for initial admin user\n");
+ }
- AccountState as = AccountState.forAccount(new AllUsersName(allUsers.get()), a, extIds);
- for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) {
- accountIndex.replace(as);
- }
+ AccountState as = AccountState.forAccount(new AllUsersName(allUsers.get()), a, extIds);
+ for (AccountIndex accountIndex : accountIndexCollection.getWriteIndexes()) {
+ accountIndex.replace(as);
+ }
- InternalGroup adminInternalGroup = groupsOnInit.getExistingGroup(db, adminGroup);
- for (GroupIndex groupIndex : groupIndexCollection.getWriteIndexes()) {
- groupIndex.replace(adminInternalGroup);
- }
+ InternalGroup adminInternalGroup = groupsOnInit.getExistingGroup(adminGroup);
+ for (GroupIndex groupIndex : groupIndexCollection.getWriteIndexes()) {
+ groupIndex.replace(adminInternalGroup);
}
}
}
diff --git a/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java b/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java
index c9c3a64..1716a3c 100644
--- a/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java
+++ b/java/com/google/gerrit/pgm/init/api/SequencesOnInit.java
@@ -35,16 +35,14 @@
this.allUsersName = allUsersName;
}
- public int nextAccountId(ReviewDb db) throws OrmException {
- @SuppressWarnings("deprecation")
- RepoSequence.Seed accountSeed = db::nextAccountId;
+ public int nextAccountId() throws OrmException {
RepoSequence accountSeq =
new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
new Project.NameKey(allUsersName.get()),
Sequences.NAME_ACCOUNTS,
- accountSeed,
+ () -> ReviewDb.FIRST_ACCOUNT_ID,
1);
return accountSeq.next();
}
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDb.java b/java/com/google/gerrit/reviewdb/server/ReviewDb.java
index 8e4292c..90cd066 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDb.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDb.java
@@ -14,8 +14,6 @@
package com.google.gerrit.reviewdb.server;
-import com.google.gerrit.reviewdb.client.Account;
-import com.google.gerrit.reviewdb.client.AccountGroup;
import com.google.gerrit.reviewdb.client.Change;
import com.google.gwtorm.server.OrmException;
import com.google.gwtorm.server.Relation;
@@ -28,7 +26,6 @@
* <p>Root entities that are at the top level of some important data graph:
*
* <ul>
- * <li>{@link Account}: Per-user account registration, preferences, identity.
* <li>{@link Change}: All review information about a single proposed change.
* </ul>
*/
@@ -91,22 +88,8 @@
int FIRST_ACCOUNT_ID = 1000000;
- /**
- * Next unique id for a {@link Account}.
- *
- * @deprecated use {@link com.google.gerrit.server.Sequences#nextAccountId()}.
- */
- @Sequence(startWith = FIRST_ACCOUNT_ID)
- @Deprecated
- int nextAccountId() throws OrmException;
-
int FIRST_GROUP_ID = 1;
- /** Next unique id for a {@link AccountGroup}. */
- @Sequence(startWith = FIRST_GROUP_ID)
- @Deprecated
- int nextAccountGroupId() throws OrmException;
-
int FIRST_CHANGE_ID = 1;
/**
diff --git a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
index 202729e..c420254 100644
--- a/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
+++ b/java/com/google/gerrit/reviewdb/server/ReviewDbWrapper.java
@@ -136,18 +136,6 @@
@Override
@SuppressWarnings("deprecation")
- public int nextAccountId() throws OrmException {
- return delegate.nextAccountId();
- }
-
- @Override
- @SuppressWarnings("deprecation")
- public int nextAccountGroupId() throws OrmException {
- return delegate.nextAccountGroupId();
- }
-
- @Override
- @SuppressWarnings("deprecation")
public int nextChangeId() throws OrmException {
return delegate.nextChangeId();
}
diff --git a/java/com/google/gerrit/server/Sequences.java b/java/com/google/gerrit/server/Sequences.java
index fcf0759..70a02a8 100644
--- a/java/com/google/gerrit/server/Sequences.java
+++ b/java/com/google/gerrit/server/Sequences.java
@@ -93,11 +93,15 @@
new RepoSequence(
repoManager, gitRefUpdated, allProjects, NAME_CHANGES, changeSeed, changeBatchSize);
- RepoSequence.Seed groupSeed = () -> nextGroupId(db.get());
int groupBatchSize = 1;
groupSeq =
new RepoSequence(
- repoManager, gitRefUpdated, allUsers, NAME_GROUPS, groupSeed, groupBatchSize);
+ repoManager,
+ gitRefUpdated,
+ allUsers,
+ NAME_GROUPS,
+ () -> ReviewDb.FIRST_GROUP_ID,
+ groupBatchSize);
nextIdLatency =
metrics.newTimer(
@@ -158,9 +162,4 @@
private static int nextChangeId(ReviewDb db) throws OrmException {
return db.nextChangeId();
}
-
- @SuppressWarnings("deprecation")
- static int nextGroupId(ReviewDb db) throws OrmException {
- return db.nextAccountGroupId();
- }
}
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/change/ReviewerAdder.java b/java/com/google/gerrit/server/change/ReviewerAdder.java
index adbfe54..477db12 100644
--- a/java/com/google/gerrit/server/change/ReviewerAdder.java
+++ b/java/com/google/gerrit/server/change/ReviewerAdder.java
@@ -30,6 +30,7 @@
import com.google.common.collect.Lists;
import com.google.common.collect.Ordering;
import com.google.common.collect.Streams;
+import com.google.common.flogger.FluentLogger;
import com.google.gerrit.common.Nullable;
import com.google.gerrit.common.data.GroupDescription;
import com.google.gerrit.extensions.api.changes.AddReviewerInput;
@@ -89,6 +90,8 @@
import org.eclipse.jgit.lib.Config;
public class ReviewerAdder {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
public static final int DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK = 10;
public static final int DEFAULT_MAX_REVIEWERS = 20;
@@ -357,6 +360,8 @@
// reviewers
int maxAllowed = cfg.getInt("addreviewer", "maxAllowed", DEFAULT_MAX_REVIEWERS);
if (maxAllowed > 0 && members.size() > maxAllowed) {
+ logger.atFine().log(
+ "Adding %d group members is not allowed (maxAllowed = %d)", members.size(), maxAllowed);
return fail(
input,
FailureType.OTHER,
@@ -367,6 +372,9 @@
int maxWithoutConfirmation =
cfg.getInt("addreviewer", "maxWithoutConfirmation", DEFAULT_MAX_REVIEWERS_WITHOUT_CHECK);
if (!confirmed && maxWithoutConfirmation > 0 && members.size() > maxWithoutConfirmation) {
+ logger.atFine().log(
+ "Adding %d group members as reviewer requires confirmation (maxWithoutConfirmation = %d)",
+ members.size(), maxWithoutConfirmation);
return fail(
input,
FailureType.OTHER,
diff --git a/java/com/google/gerrit/server/config/GerritGlobalModule.java b/java/com/google/gerrit/server/config/GerritGlobalModule.java
index b4f9cc7..b26e875 100644
--- a/java/com/google/gerrit/server/config/GerritGlobalModule.java
+++ b/java/com/google/gerrit/server/config/GerritGlobalModule.java
@@ -170,6 +170,7 @@
import com.google.gerrit.server.query.change.ChangeQueryBuilder;
import com.google.gerrit.server.query.change.ChangeQueryProcessor;
import com.google.gerrit.server.query.change.ConflictsCacheImpl;
+import com.google.gerrit.server.quota.QuotaEnforcer;
import com.google.gerrit.server.restapi.change.SuggestReviewers;
import com.google.gerrit.server.restapi.group.GroupModule;
import com.google.gerrit.server.rules.DefaultSubmitRule;
@@ -394,6 +395,7 @@
DynamicItem.itemOf(binder(), MergeSuperSetComputation.class);
DynamicItem.itemOf(binder(), ProjectNameLockManager.class);
DynamicSet.setOf(binder(), SubmitRule.class);
+ DynamicSet.setOf(binder(), QuotaEnforcer.class);
DynamicMap.mapOf(binder(), MailFilter.class);
bind(MailFilter.class).annotatedWith(Exports.named("ListMailFilter")).to(ListMailFilter.class);
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/DefaultAdvertiseRefsHook.java b/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
index be8fcdb..f255ea2 100644
--- a/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
+++ b/java/com/google/gerrit/server/git/DefaultAdvertiseRefsHook.java
@@ -14,10 +14,10 @@
package com.google.gerrit.server.git;
-import com.google.common.collect.ImmutableMap;
import com.google.gerrit.server.permissions.PermissionBackend;
import com.google.gerrit.server.permissions.PermissionBackendException;
import java.io.IOException;
+import java.util.HashMap;
import java.util.List;
import java.util.Map;
import org.eclipse.jgit.lib.Ref;
@@ -49,13 +49,12 @@
if (prefixes.isEmpty() || prefixes.get(0).isEmpty()) {
refs = repo.getAllRefs();
} else {
- ImmutableMap.Builder<String, Ref> b = new ImmutableMap.Builder<>();
+ refs = new HashMap<>();
for (String prefix : prefixes) {
for (Ref ref : repo.getRefDatabase().getRefsByPrefix(prefix)) {
- b.put(ref.getName(), ref);
+ refs.put(ref.getName(), ref);
}
}
- refs = b.build();
}
return perm.filter(refs, repo, opts);
} catch (IOException | PermissionBackendException e) {
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/meta/MetaDataUpdate.java b/java/com/google/gerrit/server/git/meta/MetaDataUpdate.java
index bbe0c62..97beefd 100644
--- a/java/com/google/gerrit/server/git/meta/MetaDataUpdate.java
+++ b/java/com/google/gerrit/server/git/meta/MetaDataUpdate.java
@@ -22,6 +22,7 @@
import com.google.gerrit.server.git.GitRepositoryManager;
import com.google.inject.Inject;
import com.google.inject.Provider;
+import com.google.inject.Singleton;
import com.google.inject.assistedinject.Assisted;
import java.io.IOException;
import org.eclipse.jgit.errors.RepositoryNotFoundException;
@@ -33,21 +34,22 @@
/** Helps with the updating of a {@link VersionedMetaData}. */
public class MetaDataUpdate implements AutoCloseable {
+ @Singleton
public static class User {
private final InternalFactory factory;
private final GitRepositoryManager mgr;
- private final PersonIdent serverIdent;
+ private final Provider<PersonIdent> serverIdentProvider;
private final Provider<IdentifiedUser> identifiedUser;
@Inject
User(
InternalFactory factory,
GitRepositoryManager mgr,
- @GerritPersonIdent PersonIdent serverIdent,
+ @GerritPersonIdent Provider<PersonIdent> serverIdentProvider,
Provider<IdentifiedUser> identifiedUser) {
this.factory = factory;
this.mgr = mgr;
- this.serverIdent = serverIdent;
+ this.serverIdentProvider = serverIdentProvider;
this.identifiedUser = identifiedUser;
}
@@ -126,29 +128,31 @@
public MetaDataUpdate create(
Project.NameKey name, Repository repository, IdentifiedUser user, BatchRefUpdate batch) {
MetaDataUpdate md = factory.create(name, repository, batch);
- md.getCommitBuilder().setCommitter(serverIdent);
+ md.getCommitBuilder().setCommitter(serverIdentProvider.get());
md.setAuthor(user);
return md;
}
private PersonIdent createPersonIdent(IdentifiedUser user) {
+ PersonIdent serverIdent = serverIdentProvider.get();
return user.newCommitterIdent(serverIdent.getWhen(), serverIdent.getTimeZone());
}
}
+ @Singleton
public static class Server {
private final InternalFactory factory;
private final GitRepositoryManager mgr;
- private final PersonIdent serverIdent;
+ private final Provider<PersonIdent> serverIdentProvider;
@Inject
Server(
InternalFactory factory,
GitRepositoryManager mgr,
- @GerritPersonIdent PersonIdent serverIdent) {
+ @GerritPersonIdent Provider<PersonIdent> serverIdentProvider) {
this.factory = factory;
this.mgr = mgr;
- this.serverIdent = serverIdent;
+ this.serverIdentProvider = serverIdentProvider;
}
public MetaDataUpdate create(Project.NameKey name)
@@ -162,6 +166,7 @@
Repository repo = mgr.openRepository(name);
MetaDataUpdate md = factory.create(name, repo, batch);
md.setCloseRepository(true);
+ PersonIdent serverIdent = serverIdentProvider.get();
md.getCommitBuilder().setAuthor(serverIdent);
md.getCommitBuilder().setCommitter(serverIdent);
return md;
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/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
index b23fe71..ba11985 100644
--- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java
@@ -1670,6 +1670,11 @@
logger.atFine().log("Handling %s", RefNames.REFS_USERS_SELF);
ref = RefNames.refsUsers(user.getAccountId());
}
+ // Pushing changes for review usually requires that the target branch exists, but there is an
+ // exception for the branch to which HEAD points to and for refs/meta/config. Pushing for
+ // review to these branches is allowed even if the branch does not exist yet. This allows to
+ // push initial code for review to an empty repository and to review an initial project
+ // configuration.
if (!receivePack.getAdvertisedRefs().containsKey(ref)
&& !ref.equals(readHEAD(repo))
&& !ref.equals(RefNames.REFS_CONFIG)) {
@@ -1753,9 +1758,10 @@
reject(cmd, "cannot use merged with base");
return;
}
- RevCommit branchTip = readBranchTip(cmd, magicBranch.dest);
+ RevCommit branchTip = readBranchTip(magicBranch.dest);
if (branchTip == null) {
- return; // readBranchTip already rejected cmd.
+ reject(cmd, magicBranch.dest.get() + " not found");
+ return;
}
if (!walk.isMergedInto(tip, branchTip)) {
reject(cmd, "not merged into branch");
@@ -1793,12 +1799,21 @@
}
}
} else if (newChangeForAllNotInTarget) {
- RevCommit branchTip = readBranchTip(cmd, magicBranch.dest);
- if (branchTip == null) {
- return; // readBranchTip already rejected cmd.
+ RevCommit branchTip = readBranchTip(magicBranch.dest);
+ if (branchTip != null) {
+ magicBranch.baseCommit = Collections.singletonList(branchTip);
+ logger.atFine().log("Set baseCommit = %s", magicBranch.baseCommit.get(0).name());
+ } else {
+ // The target branch does not exist. Usually pushing changes for review requires that the
+ // target branch exists, but there is an exception for the branch to which HEAD points to
+ // and for refs/meta/config. Pushing for review to these branches is allowed even if the
+ // branch does not exist yet. This allows to push initial code for review to an empty
+ // repository and to review an initial project configuration.
+ if (!ref.equals(readHEAD(repo)) && !ref.equals(RefNames.REFS_CONFIG)) {
+ reject(cmd, magicBranch.dest.get() + " not found");
+ return;
+ }
}
- magicBranch.baseCommit = Collections.singletonList(branchTip);
- logger.atFine().log("Set baseCommit = %s", magicBranch.baseCommit.get(0).name());
}
} catch (IOException ex) {
logger.atWarning().withCause(ex).log(
@@ -1872,10 +1887,9 @@
}
}
- private RevCommit readBranchTip(ReceiveCommand cmd, Branch.NameKey branch) throws IOException {
+ private RevCommit readBranchTip(Branch.NameKey branch) throws IOException {
Ref r = allRefs().get(branch.get());
if (r == null) {
- reject(cmd, branch.get() + " not found");
return null;
}
return receivePack.getRevWalk().parseCommit(r.getObjectId());
@@ -2727,10 +2741,13 @@
/** prints a warning if the new PS has the same tree as the previous commit. */
private void sameTreeWarning() throws IOException {
- RevCommit newCommit = receivePack.getRevWalk().parseCommit(newCommitId);
+ RevWalk rw = receivePack.getRevWalk();
+ RevCommit newCommit = rw.parseCommit(newCommitId);
RevCommit priorCommit = revisions.inverse().get(priorPatchSet);
if (newCommit.getTree().equals(priorCommit.getTree())) {
+ rw.parseBody(newCommit);
+ rw.parseBody(priorCommit);
boolean messageEq =
Objects.equals(newCommit.getFullMessage(), priorCommit.getFullMessage());
boolean parentsEq = parentsEqual(newCommit, priorCommit);
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/project/ProjectConfig.java b/java/com/google/gerrit/server/project/ProjectConfig.java
index b16b076..36cffb3 100644
--- a/java/com/google/gerrit/server/project/ProjectConfig.java
+++ b/java/com/google/gerrit/server/project/ProjectConfig.java
@@ -229,7 +229,7 @@
private Map<String, LabelType> labelSections;
private ConfiguredMimeTypes mimeTypes;
private Map<Project.NameKey, SubscribeSection> subscribeSections;
- private List<CommentLinkInfoImpl> commentLinkSections;
+ private Map<String, CommentLinkInfoImpl> commentLinkSections;
private List<ValidationError> validationErrors;
private ObjectId rulesId;
private long maxObjectSizeLimit;
@@ -278,7 +278,7 @@
}
public void addCommentLinkSection(CommentLinkInfoImpl commentLink) {
- commentLinkSections.add(commentLink);
+ commentLinkSections.put(commentLink.name, commentLink);
}
private ProjectConfig(Project.NameKey projectName, @Nullable StoredConfig baseConfig) {
@@ -456,7 +456,7 @@
}
public Collection<CommentLinkInfoImpl> getCommentLinkSections() {
- return commentLinkSections;
+ return commentLinkSections.values();
}
public ConfiguredMimeTypes getMimeTypes() {
@@ -1001,10 +1001,10 @@
private void loadCommentLinkSections(Config rc) {
Set<String> subsections = rc.getSubsections(COMMENTLINK);
- commentLinkSections = new ArrayList<>(subsections.size());
+ commentLinkSections = new LinkedHashMap<>(subsections.size());
for (String name : subsections) {
try {
- commentLinkSections.add(buildCommentLink(rc, name, false));
+ commentLinkSections.put(name, buildCommentLink(rc, name, false));
} catch (PatternSyntaxException e) {
error(
new ValidationError(
@@ -1191,7 +1191,7 @@
private void saveCommentLinkSections(Config rc) {
if (commentLinkSections != null) {
- for (CommentLinkInfoImpl cm : commentLinkSections) {
+ for (CommentLinkInfoImpl cm : commentLinkSections.values()) {
rc.setString(COMMENTLINK, cm.name, KEY_MATCH, cm.match);
if (!Strings.isNullOrEmpty(cm.html)) {
rc.setString(COMMENTLINK, cm.name, KEY_HTML, cm.html);
diff --git a/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java b/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java
new file mode 100644
index 0000000..8aa86b1
--- /dev/null
+++ b/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java
@@ -0,0 +1,149 @@
+// Copyright (C) 2018 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.quota;
+
+import static com.google.common.base.Preconditions.checkState;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.flogger.FluentLogger;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project.NameKey;
+import com.google.gerrit.server.CurrentUser;
+import com.google.gerrit.server.plugincontext.PluginSetContext;
+import com.google.gerrit.server.plugincontext.PluginSetEntryContext;
+import java.util.ArrayList;
+import java.util.List;
+import javax.inject.Inject;
+import javax.inject.Provider;
+import javax.inject.Singleton;
+
+@Singleton
+public class DefaultQuotaBackend implements QuotaBackend {
+ private static final FluentLogger logger = FluentLogger.forEnclosingClass();
+
+ private final Provider<CurrentUser> userProvider;
+ private final PluginSetContext<QuotaEnforcer> quotaEnforcers;
+
+ @Inject
+ DefaultQuotaBackend(
+ Provider<CurrentUser> userProvider, PluginSetContext<QuotaEnforcer> quotaEnforcers) {
+ this.userProvider = userProvider;
+ this.quotaEnforcers = quotaEnforcers;
+ }
+
+ @Override
+ public WithUser currentUser() {
+ return new WithUser(quotaEnforcers, userProvider.get());
+ }
+
+ @Override
+ public WithUser user(CurrentUser user) {
+ return new WithUser(quotaEnforcers, user);
+ }
+
+ private static QuotaResponse.Aggregated request(
+ PluginSetContext<QuotaEnforcer> quotaEnforcers,
+ String quotaGroup,
+ QuotaRequestContext requestContext,
+ long numTokens,
+ boolean deduct) {
+ checkState(numTokens > 0, "numTokens must be a positive, non-zero long");
+
+ // PluginSets can change their content when plugins (de-)register. Copy the currently registered
+ // plugins so that we can iterate twice on a stable list.
+ List<PluginSetEntryContext<QuotaEnforcer>> enforcers = ImmutableList.copyOf(quotaEnforcers);
+ List<QuotaResponse> responses = new ArrayList<>(enforcers.size());
+ for (PluginSetEntryContext<QuotaEnforcer> enforcer : enforcers) {
+ try {
+ if (deduct) {
+ responses.add(enforcer.call(p -> p.requestTokens(quotaGroup, requestContext, numTokens)));
+ } else {
+ responses.add(enforcer.call(p -> p.dryRun(quotaGroup, requestContext, numTokens)));
+ }
+ } catch (RuntimeException e) {
+ logger.atSevere().withCause(e).log("exception while enforcing quota");
+ responses.add(QuotaResponse.error(e.getMessage()));
+ }
+ }
+
+ if (deduct && responses.stream().anyMatch(r -> r.status().isError())) {
+ // Roll back the quota request for all enforcers that deducted the quota (= the request
+ // succeeded). Don't touch failed enforcers as the interface contract said that failed
+ // requests should not be deducted.
+ for (int i = 0; i < responses.size(); i++) {
+ if (responses.get(i).status().isOk()) {
+ enforcers.get(i).run(p -> p.refill(quotaGroup, requestContext, numTokens));
+ }
+ }
+ }
+
+ logger.atFine().log(
+ "Quota request for %s with %s (deduction=%s) for %s token returned %s",
+ quotaGroup,
+ requestContext,
+ deduct ? "(deduction=yes)" : "(deduction=no)",
+ numTokens,
+ responses);
+ return new AutoValue_QuotaResponse_Aggregated(ImmutableList.copyOf(responses));
+ }
+
+ static class WithUser extends WithResource implements QuotaBackend.WithUser {
+ WithUser(PluginSetContext<QuotaEnforcer> quotaEnforcers, CurrentUser user) {
+ super(quotaEnforcers, QuotaRequestContext.builder().user(user).build());
+ }
+
+ @Override
+ public QuotaBackend.WithResource account(Account.Id account) {
+ QuotaRequestContext ctx = requestContext.toBuilder().account(account).build();
+ return new WithResource(quotaEnforcers, ctx);
+ }
+
+ @Override
+ public QuotaBackend.WithResource project(NameKey project) {
+ QuotaRequestContext ctx = requestContext.toBuilder().project(project).build();
+ return new WithResource(quotaEnforcers, ctx);
+ }
+
+ @Override
+ public QuotaBackend.WithResource change(Change.Id change, NameKey project) {
+ QuotaRequestContext ctx = requestContext.toBuilder().change(change).project(project).build();
+ return new WithResource(quotaEnforcers, ctx);
+ }
+ }
+
+ static class WithResource implements QuotaBackend.WithResource {
+ protected final QuotaRequestContext requestContext;
+ protected final PluginSetContext<QuotaEnforcer> quotaEnforcers;
+
+ private WithResource(
+ PluginSetContext<QuotaEnforcer> quotaEnforcers, QuotaRequestContext quotaRequestContext) {
+ this.quotaEnforcers = quotaEnforcers;
+ this.requestContext = quotaRequestContext;
+ }
+
+ @Override
+ public QuotaResponse.Aggregated requestTokens(String quotaGroup, long numTokens) {
+ return DefaultQuotaBackend.request(
+ quotaEnforcers, quotaGroup, requestContext, numTokens, true);
+ }
+
+ @Override
+ public QuotaResponse.Aggregated dryRun(String quotaGroup, long numTokens) {
+ return DefaultQuotaBackend.request(
+ quotaEnforcers, quotaGroup, requestContext, numTokens, false);
+ }
+ }
+}
diff --git a/java/com/google/gerrit/server/quota/QuotaBackend.java b/java/com/google/gerrit/server/quota/QuotaBackend.java
new file mode 100644
index 0000000..d4dc46d
--- /dev/null
+++ b/java/com/google/gerrit/server/quota/QuotaBackend.java
@@ -0,0 +1,86 @@
+// Copyright (C) 2018 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.quota;
+
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.CurrentUser;
+import com.google.inject.ImplementedBy;
+
+/**
+ * Backend interface to perform quota requests on. By default, this interface is backed by {@link
+ * DefaultQuotaBackend} which calls all plugins that implement {@link QuotaEnforcer}. A different
+ * implementation might be bound in tests. Plugins are not supposed to implement this interface, but
+ * bind a {@link QuotaEnforcer} implementation instead.
+ *
+ * <p>All quota requests require a quota group and a user. Enriching them with a top-level entity
+ * {@code Change, Project, Account} is optional but should be done if the request is targeted.
+ *
+ * <p>Example usage:
+ *
+ * <pre>
+ * quotaBackend.currentUser().project(projectName).requestToken("/projects/create").throwOnError();
+ * quotaBackend.user(user).requestToken("/restapi/config/put").throwOnError();
+ * QuotaResponse.Aggregated result = quotaBackend.currentUser().account(accountId).requestToken("/restapi/accounts/emails/validate");
+ * QuotaResponse.Aggregated result = quotaBackend.currentUser().project(projectName).requestTokens("/projects/git/upload", numBytesInPush);
+ * </pre>
+ *
+ * <p>All quota groups must be documented in {@code quota.txt} and detail the metadata that is
+ * provided (i.e. the parameters used to scope down the quota request).
+ */
+@ImplementedBy(DefaultQuotaBackend.class)
+public interface QuotaBackend {
+ /** Constructs a request for the current user. */
+ WithUser currentUser();
+
+ /**
+ * See {@link #currentUser()}. Use this method only if you can't guarantee that the request is for
+ * the current user (e.g. impersonation).
+ */
+ WithUser user(CurrentUser user);
+
+ /**
+ * An interface capable of issuing quota requests. Scope can be futher reduced by providing a
+ * top-level entity.
+ */
+ interface WithUser extends WithResource {
+ /** Scope the request down to an account. */
+ WithResource account(Account.Id account);
+
+ /** Scope the request down to a project. */
+ WithResource project(Project.NameKey project);
+
+ /** Scope the request down to a change. */
+ WithResource change(Change.Id change, Project.NameKey project);
+ }
+
+ /** An interface capable of issuing quota requests. */
+ interface WithResource {
+ /** Issues a single quota request for {@code 1} token. */
+ default QuotaResponse.Aggregated requestToken(String quotaGroup) {
+ return requestTokens(quotaGroup, 1);
+ }
+
+ /** Issues a single quota request for {@code numTokens} tokens. */
+ QuotaResponse.Aggregated requestTokens(String quotaGroup, long numTokens);
+
+ /**
+ * Issues a single quota request for {@code numTokens} tokens but signals the implementations
+ * not to deduct any quota yet. Can be used to do pre-flight requests where necessary
+ */
+ QuotaResponse.Aggregated dryRun(String quotaGroup, long tokens);
+ }
+}
diff --git a/java/com/google/gerrit/server/quota/QuotaEnforcer.java b/java/com/google/gerrit/server/quota/QuotaEnforcer.java
new file mode 100644
index 0000000..9c55e11
--- /dev/null
+++ b/java/com/google/gerrit/server/quota/QuotaEnforcer.java
@@ -0,0 +1,72 @@
+// Copyright (C) 2018 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.quota;
+
+import com.google.gerrit.extensions.annotations.ExtensionPoint;
+
+/**
+ * Allows plugins to enforce different types of quota.
+ *
+ * <p>Enforcing quotas can be helpful in many scenarios. For example:
+ *
+ * <ul>
+ * <li>Reducing the number of QPS a user can send to Gerrit on the REST API
+ * <li>Limiting the size of a repository (project)
+ * <li>Limiting the number of changes in a repository
+ * <li>Limiting the number of actions that have the potential for spam, abuse or flooding if not
+ * limited
+ * </ul>
+ *
+ * This endpoint gives plugins the capability to enforce any of these limits. The server will ask
+ * all plugins that registered this endpoint and collect all results. In case {@link
+ * #requestTokens(String, QuotaRequestContext, long)} was called and one or more plugins returned an
+ * erroneous result, the server will call {@link #refill(String, QuotaRequestContext, long)} on all
+ * plugins with the same parameters. Plugins that deducted tokens in the {@link
+ * #requestTokens(String, QuotaRequestContext, long)} call can refill them so that users don't get
+ * charged any quota for failed requests.
+ *
+ * <p>Not all implementations will need to deduct quota on {@link #requestTokens(String,
+ * QuotaRequestContext, long)}}. Implementations that work on top of instance-attributes, such as
+ * the number of projects per instance can choose not to keep any state and always check how many
+ * existing projects there are and if adding the inquired number would exceed the limit. In this
+ * case, {@link #requestTokens(String, QuotaRequestContext, long)} and {@link #dryRun(String,
+ * QuotaRequestContext, long)} share the same implementation and {@link #refill(String,
+ * QuotaRequestContext, long)} is a no-op.
+ */
+@ExtensionPoint
+public interface QuotaEnforcer {
+ /**
+ * Checks if there is at least {@code numTokens} quota to fulfil the request. Bucket-based
+ * implementations can deduct the inquired number of tokens from the bucket.
+ */
+ QuotaResponse requestTokens(String quotaGroup, QuotaRequestContext ctx, long numTokens);
+
+ /**
+ * Checks if there is at least {@code numTokens} quota to fulfil the request. This is a pre-flight
+ * request, implementations should not deduct tokens from a bucket, yet.
+ */
+ QuotaResponse dryRun(String quotaGroup, QuotaRequestContext ctx, long numTokens);
+
+ /**
+ * A previously requested and deducted quota has to be refilled (if possible) because the request
+ * failed other quota checks. Implementations can choose to leave this a no-op in case they are
+ * the first line of defence (e.g. always deduct HTTP quota even if the request failed for other
+ * quota issues so that the user gets throttled).
+ *
+ * <p>Will not be called if the {@link #requestTokens(String, QuotaRequestContext, long)} call
+ * returned {@link QuotaResponse.Status#NO_OP}.
+ */
+ void refill(String quotaGroup, QuotaRequestContext ctx, long numTokens);
+}
diff --git a/java/com/google/gerrit/server/quota/QuotaException.java b/java/com/google/gerrit/server/quota/QuotaException.java
new file mode 100644
index 0000000..be13c0ec0
--- /dev/null
+++ b/java/com/google/gerrit/server/quota/QuotaException.java
@@ -0,0 +1,29 @@
+// Copyright (C) 2018 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.quota;
+
+import com.google.gerrit.extensions.restapi.RestApiException;
+
+/**
+ * Exception that was encountered while checking if there is sufficient quota to fulfil the request.
+ * Can be propagated directly to the REST API.
+ */
+public class QuotaException extends RestApiException {
+ private static final long serialVersionUID = 1L;
+
+ public QuotaException(String reason) {
+ super(reason);
+ }
+}
diff --git a/java/com/google/gerrit/server/quota/QuotaRequestContext.java b/java/com/google/gerrit/server/quota/QuotaRequestContext.java
new file mode 100644
index 0000000..90b501c
--- /dev/null
+++ b/java/com/google/gerrit/server/quota/QuotaRequestContext.java
@@ -0,0 +1,54 @@
+// Copyright (C) 2018 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.quota;
+
+import com.google.auto.value.AutoValue;
+import com.google.gerrit.reviewdb.client.Account;
+import com.google.gerrit.reviewdb.client.Change;
+import com.google.gerrit.reviewdb.client.Project;
+import com.google.gerrit.server.AnonymousUser;
+import com.google.gerrit.server.CurrentUser;
+import java.util.Optional;
+
+@AutoValue
+public abstract class QuotaRequestContext {
+
+ public static Builder builder() {
+ return new AutoValue_QuotaRequestContext.Builder().user(new AnonymousUser());
+ }
+
+ public abstract CurrentUser user();
+
+ public abstract Optional<Project.NameKey> project();
+
+ public abstract Optional<Change.Id> change();
+
+ public abstract Optional<Account.Id> account();
+
+ public abstract Builder toBuilder();
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract QuotaRequestContext.Builder user(CurrentUser user);
+
+ public abstract QuotaRequestContext.Builder account(Account.Id account);
+
+ public abstract QuotaRequestContext.Builder project(Project.NameKey project);
+
+ public abstract QuotaRequestContext.Builder change(Change.Id change);
+
+ public abstract QuotaRequestContext build();
+ }
+}
diff --git a/java/com/google/gerrit/server/quota/QuotaResponse.java b/java/com/google/gerrit/server/quota/QuotaResponse.java
new file mode 100644
index 0000000..c239aaf
--- /dev/null
+++ b/java/com/google/gerrit/server/quota/QuotaResponse.java
@@ -0,0 +1,113 @@
+// Copyright (C) 2018 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.quota;
+
+import static com.google.common.collect.ImmutableList.toImmutableList;
+
+import com.google.auto.value.AutoValue;
+import com.google.common.base.Strings;
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Streams;
+import java.util.Optional;
+import java.util.stream.Collectors;
+
+@AutoValue
+public abstract class QuotaResponse {
+ public enum Status {
+ /** The quota requests succeeded. */
+ OK,
+
+ /**
+ * The quota succeeded, but was a no-op because the plugin does not enforce this quota group
+ * (equivalent to OK, but relevant for debugging).
+ */
+ NO_OP,
+
+ /**
+ * The requested quota could not be allocated. This status code is not used to indicate
+ * processing failures as these are propagated as {@code RuntimeException}s.
+ */
+ ERROR;
+
+ public boolean isOk() {
+ return this == OK;
+ }
+
+ public boolean isError() {
+ return this == ERROR;
+ }
+ }
+
+ public static QuotaResponse ok() {
+ return new AutoValue_QuotaResponse.Builder().status(Status.OK).build();
+ }
+
+ public static QuotaResponse noOp() {
+ return new AutoValue_QuotaResponse.Builder().status(Status.NO_OP).build();
+ }
+
+ public static QuotaResponse error(String message) {
+ return new AutoValue_QuotaResponse.Builder().status(Status.ERROR).message(message).build();
+ }
+
+ public abstract Status status();
+
+ public abstract Optional<String> message();
+
+ @AutoValue.Builder
+ public abstract static class Builder {
+ public abstract QuotaResponse.Builder status(Status status);
+
+ public abstract QuotaResponse.Builder message(String message);
+
+ public abstract QuotaResponse build();
+ }
+
+ @AutoValue
+ public abstract static class Aggregated {
+ protected abstract ImmutableList<QuotaResponse> responses();
+
+ public boolean hasError() {
+ return responses().stream().anyMatch(r -> r.status().isError());
+ }
+
+ public ImmutableList<QuotaResponse> all() {
+ return responses();
+ }
+
+ public ImmutableList<QuotaResponse> ok() {
+ return responses().stream().filter(r -> r.status().isOk()).collect(toImmutableList());
+ }
+
+ public ImmutableList<QuotaResponse> error() {
+ return responses().stream().filter(r -> r.status().isError()).collect(toImmutableList());
+ }
+
+ public String errorMessage() {
+ return error()
+ .stream()
+ .map(QuotaResponse::message)
+ .flatMap(Streams::stream)
+ .collect(Collectors.joining(", "));
+ }
+
+ public void throwOnError() throws QuotaException {
+ String errorMessage = errorMessage();
+ if (!Strings.isNullOrEmpty(errorMessage)) {
+ throw new QuotaException(errorMessage);
+ }
+ }
+ }
+}
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/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/server/schema/Schema_155.java b/java/com/google/gerrit/server/schema/Schema_155.java
index 812d7a6..e9372a5 100644
--- a/java/com/google/gerrit/server/schema/Schema_155.java
+++ b/java/com/google/gerrit/server/schema/Schema_155.java
@@ -40,15 +40,13 @@
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
- @SuppressWarnings("deprecation")
- RepoSequence.Seed accountSeed = db::nextAccountId;
RepoSequence accountSeq =
new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
allUsersName,
Sequences.NAME_ACCOUNTS,
- accountSeed,
+ () -> ReviewDb.FIRST_ACCOUNT_ID,
1);
// consume one account ID to ensure that the account sequence is initialized in NoteDb
diff --git a/java/com/google/gerrit/server/schema/Schema_163.java b/java/com/google/gerrit/server/schema/Schema_163.java
index 9eb5d5e..4b3659de 100644
--- a/java/com/google/gerrit/server/schema/Schema_163.java
+++ b/java/com/google/gerrit/server/schema/Schema_163.java
@@ -40,15 +40,13 @@
@Override
protected void migrateData(ReviewDb db, UpdateUI ui) throws OrmException, SQLException {
- @SuppressWarnings("deprecation")
- RepoSequence.Seed groupSeed = db::nextAccountGroupId;
RepoSequence groupSeq =
new RepoSequence(
repoManager,
GitReferenceUpdated.DISABLED,
allUsersName,
Sequences.NAME_GROUPS,
- groupSeed,
+ () -> ReviewDb.FIRST_GROUP_ID,
1);
// consume one account ID to ensure that the group sequence is initialized in NoteDb
diff --git a/java/com/google/gerrit/sshd/commands/KillCommand.java b/java/com/google/gerrit/sshd/commands/KillCommand.java
index ef12f5f..df74f86 100644
--- a/java/com/google/gerrit/sshd/commands/KillCommand.java
+++ b/java/com/google/gerrit/sshd/commands/KillCommand.java
@@ -43,7 +43,7 @@
@Inject private DeleteTask deleteTask;
@Argument(index = 0, multiValued = true, required = true, metaVar = "ID")
- private final List<String> taskIds = new ArrayList<>();
+ private List<String> taskIds = new ArrayList<>();
@Override
protected void run() {
diff --git a/java/com/google/gerrit/testing/DisabledReviewDb.java b/java/com/google/gerrit/testing/DisabledReviewDb.java
index 2bf95b0..d06beb9 100644
--- a/java/com/google/gerrit/testing/DisabledReviewDb.java
+++ b/java/com/google/gerrit/testing/DisabledReviewDb.java
@@ -95,16 +95,6 @@
}
@Override
- public int nextAccountId() {
- throw new Disabled();
- }
-
- @Override
- public int nextAccountGroupId() {
- throw new Disabled();
- }
-
- @Override
public int nextChangeId() {
throw new Disabled();
}
diff --git a/java/com/google/gerrit/testing/FakeGroupAuditService.java b/java/com/google/gerrit/testing/FakeGroupAuditService.java
index f2e85cd..ddf03f5 100644
--- a/java/com/google/gerrit/testing/FakeGroupAuditService.java
+++ b/java/com/google/gerrit/testing/FakeGroupAuditService.java
@@ -63,7 +63,10 @@
@Override
public void dispatch(AuditEvent action) {
- auditEvents.add(action);
+ synchronized (auditEvents) {
+ auditEvents.add(action);
+ auditEvents.notifyAll();
+ }
}
@Override
diff --git a/java/com/google/gerrit/util/http/RequestUtil.java b/java/com/google/gerrit/util/http/RequestUtil.java
index 2a359ca..2543ada 100644
--- a/java/com/google/gerrit/util/http/RequestUtil.java
+++ b/java/com/google/gerrit/util/http/RequestUtil.java
@@ -56,5 +56,38 @@
return pathInfo;
}
+ /**
+ * Trims leading '/' and 'a/'. Removes the context path, but keeps the servlet path. Removes all
+ * IDs from the rest of the URI.
+ *
+ * <p>The returned string is a good fit for cases where one wants the full context of the request
+ * without any identifiable data. For example: Logging or quota checks.
+ *
+ * <p>Examples:
+ *
+ * <ul>
+ * <li>/a/accounts/self/detail => /accounts/detail
+ * <li>/changes/123/revisions/current/detail => /changes/revisions/detail
+ * <li>/changes/ => /changes
+ * </ul>
+ */
+ public static String getRestPathWithoutIds(HttpServletRequest req) {
+ String encodedPathInfo = req.getRequestURI().substring(req.getContextPath().length());
+ if (encodedPathInfo.startsWith("/")) {
+ encodedPathInfo = encodedPathInfo.substring(1);
+ }
+ if (encodedPathInfo.startsWith("a/")) {
+ encodedPathInfo = encodedPathInfo.substring(2);
+ }
+
+ String[] parts = encodedPathInfo.split("/");
+ StringBuilder result = new StringBuilder(parts.length);
+ for (int i = 0; i < parts.length; i = i + 2) {
+ result.append("/");
+ result.append(parts[i]);
+ }
+ return result.toString();
+ }
+
private RequestUtil() {}
}
diff --git a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
index 6f25d28..18eb37a 100644
--- a/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
+++ b/javatests/com/google/gerrit/acceptance/api/group/GroupsIT.java
@@ -89,14 +89,13 @@
import java.lang.annotation.Retention;
import java.lang.annotation.Target;
import java.sql.Timestamp;
-import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collection;
-import java.util.Collections;
import java.util.List;
import java.util.Map;
import java.util.Objects;
import java.util.concurrent.TimeUnit;
+import java.util.stream.Stream;
import org.eclipse.jgit.internal.storage.dfs.InMemoryRepository;
import org.eclipse.jgit.junit.TestRepository;
import org.eclipse.jgit.lib.CommitBuilder;
@@ -334,9 +333,6 @@
String p = createUniqueGroup();
String g1 = createUniqueGroup();
String g2 = createUniqueGroup();
- List<String> groups = new ArrayList<>();
- groups.add(g1);
- groups.add(g2);
gApi.groups().id(p).addGroups(g1, g2);
assertIncludes(p, g1, g2);
}
@@ -952,8 +948,8 @@
}
/**
- * @Sandboxed is used by this test because it deletes a group reference which introduces an
- * inconsistency for the group storage. Once group deletion is supported, this test should be
+ * {@code @Sandboxed} is used by this test because it deletes a group reference which introduces
+ * an inconsistency for the group storage. Once group deletion is supported, this test should be
* updated to use the API instead.
*/
@Test
@@ -1001,8 +997,7 @@
TestAccount groupOwner = accountCreator.user2();
GroupInput in = new GroupInput();
in.name = name("group");
- in.members =
- Collections.singleton(groupOwner).stream().map(u -> u.id.toString()).collect(toList());
+ in.members = Stream.of(groupOwner).map(u -> u.id.toString()).collect(toList());
in.visibleToAll = true;
GroupInfo group = gApi.groups().create(in).get();
@@ -1045,7 +1040,7 @@
@Test
public void pushToGroupsBranchForNonAllUsersRepo() throws Exception {
- assertCreateGroupBranch(project, null);
+ assertCreateGroupBranch(project);
String groupRef =
RefNames.refsGroups(new AccountGroup.UUID(gApi.groups().create(name("foo")).get().id));
createBranch(project, groupRef);
@@ -1054,7 +1049,7 @@
@Test
public void pushToDeletedGroupsBranchForNonAllUsersRepo() throws Exception {
- assertCreateGroupBranch(project, null);
+ assertCreateGroupBranch(project);
String groupRef =
RefNames.refsDeletedGroups(
new AccountGroup.UUID(gApi.groups().create(name("foo")).get().id));
@@ -1092,8 +1087,7 @@
}
}
- private void assertCreateGroupBranch(Project.NameKey project, String expectedErrorOnCreate)
- throws Exception {
+ private void assertCreateGroupBranch(Project.NameKey project) throws Exception {
grant(project, RefNames.REFS_GROUPS + "*", Permission.CREATE, false, REGISTERED_USERS);
grant(project, RefNames.REFS_GROUPS + "*", Permission.PUSH, false, REGISTERED_USERS);
TestRepository<InMemoryRepository> repo = cloneProject(project);
@@ -1102,11 +1096,7 @@
.create(db, admin.getIdent(), repo, "Update group", "arbitraryFile.txt", "some content")
.setParents(ImmutableList.of())
.to(RefNames.REFS_GROUPS + name("bar"));
- if (expectedErrorOnCreate != null) {
- r.assertErrorStatus(expectedErrorOnCreate);
- } else {
- r.assertOkStatus();
- }
+ r.assertOkStatus();
}
@Test
@@ -1495,7 +1485,7 @@
private void assertMembers(String group, TestAccount... expectedMembers) throws Exception {
assertMembers(
gApi.groups().id(group).members(),
- TestAccount.names(expectedMembers).stream().toArray(String[]::new));
+ TestAccount.names(expectedMembers).toArray(new String[0]));
assertAccountInfos(Arrays.asList(expectedMembers), gApi.groups().id(group).members());
}
diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
index 228784b..7529ee3 100644
--- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
+++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java
@@ -221,6 +221,18 @@
@Test
@TestProjectInput(createEmptyCommit = false)
public void pushInitialCommitSeriesForMasterBranch() throws Exception {
+ testPushInitialCommitSeriesForMasterBranch();
+ }
+
+ @Test
+ @TestProjectInput(createEmptyCommit = false)
+ public void pushInitialCommitSeriesForMasterBranchWithCreateNewChangeForAllNotInTarget()
+ throws Exception {
+ enableCreateNewChangeForAllNotInTarget();
+ testPushInitialCommitSeriesForMasterBranch();
+ }
+
+ private void testPushInitialCommitSeriesForMasterBranch() throws Exception {
RevCommit c = testRepo.commit().message("Initial commit").insertChangeId().create();
String id = GitUtil.getChangeId(testRepo, c).get();
testRepo.reset(c);
@@ -1464,14 +1476,7 @@
@Test
public void pushSameCommitTwice() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
- u.save();
- }
+ enableCreateNewChangeForAllNotInTarget();
PushOneCommit push =
pushFactory.create(
@@ -1493,14 +1498,7 @@
@Test
public void pushSameCommitTwiceWhenIndexFailed() throws Exception {
- try (ProjectConfigUpdate u = updateProject(project)) {
- u.getConfig()
- .getProject()
- .setBooleanConfig(
- BooleanProjectConfig.CREATE_NEW_CHANGE_FOR_ALL_NOT_IN_TARGET,
- InheritableBoolean.TRUE);
- u.save();
- }
+ enableCreateNewChangeForAllNotInTarget();
PushOneCommit push =
pushFactory.create(
@@ -2356,6 +2354,113 @@
assertPushOk(pr, "refs/heads/permitted");
}
+ @Test
+ public void pushCommitsWithSameTreeNoChanges() throws Exception {
+ RevCommit c =
+ testRepo
+ .commit()
+ .message("Foo")
+ .parent(getHead(testRepo.getRepository()))
+ .insertChangeId()
+ .create();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ RevCommit amended = testRepo.amend(c).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains(
+ "warning: no changes between prior commit "
+ + c.abbreviate(7).name()
+ + " and new commit "
+ + amended.abbreviate(7).name());
+ }
+
+ @Test
+ public void pushCommitsWithSameTreeNoFilesChangedMessageUpdated() throws Exception {
+ RevCommit c =
+ testRepo
+ .commit()
+ .message("Foo")
+ .parent(getHead(testRepo.getRepository()))
+ .insertChangeId()
+ .create();
+ String id = GitUtil.getChangeId(testRepo, c).get();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ RevCommit amended =
+ testRepo.amend(c).message("Foo Bar").insertChangeId(id.substring(1)).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains(
+ "warning: " + amended.abbreviate(7).name() + ": no files changed, message updated");
+ }
+
+ @Test
+ public void pushCommitsWithSameTreeNoFilesChangedAuthorChanged() throws Exception {
+ RevCommit c =
+ testRepo
+ .commit()
+ .message("Foo")
+ .parent(getHead(testRepo.getRepository()))
+ .insertChangeId()
+ .create();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ RevCommit amended = testRepo.amend(c).author(user.getIdent()).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains(
+ "warning: " + amended.abbreviate(7).name() + ": no files changed, author changed");
+ }
+
+ @Test
+ public void pushCommitsWithSameTreeNoFilesChangedWasRebased() throws Exception {
+ RevCommit head = getHead(testRepo.getRepository());
+ RevCommit c = testRepo.commit().message("Foo").parent(head).insertChangeId().create();
+ testRepo.reset(c);
+
+ String r = "refs/for/master";
+ PushResult pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ testRepo.reset(head);
+ RevCommit newBase = testRepo.commit().message("Base").parent(head).insertChangeId().create();
+ testRepo.reset(newBase);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+
+ testRepo.reset(c);
+ RevCommit amended = testRepo.amend(c).parent(newBase).create();
+ testRepo.reset(amended);
+
+ pr = pushHead(testRepo, r, false);
+ assertPushOk(pr, r);
+ assertThat(pr.getMessages())
+ .contains("warning: " + amended.abbreviate(7).name() + ": no files changed, was rebased");
+ }
+
private DraftInput newDraft(String path, int line, String message) {
DraftInput d = new DraftInput();
d.path = path;
diff --git a/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java b/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
index 42e046a..90f4134 100644
--- a/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
+++ b/javatests/com/google/gerrit/acceptance/git/GitOverHttpServletIT.java
@@ -17,7 +17,9 @@
import static com.google.common.truth.Truth.assertThat;
import com.google.gerrit.server.AuditEvent;
+import com.google.gerrit.server.audit.HttpAuditEvent;
import java.util.Collections;
+import javax.servlet.http.HttpServletResponse;
import org.eclipse.jgit.transport.CredentialsProvider;
import org.eclipse.jgit.transport.RefSpec;
import org.eclipse.jgit.transport.UsernamePasswordCredentialsProvider;
@@ -25,6 +27,7 @@
import org.junit.Test;
public class GitOverHttpServletIT extends AbstractPushForReview {
+ private static final long AUDIT_EVENT_TIMEOUT = 500L;
@Before
public void beforeEach() throws Exception {
@@ -42,6 +45,7 @@
.setRemote("origin")
.setRefSpecs(new RefSpec("HEAD:refs/for/master"))
.call();
+ waitForAudit();
// Git smart protocol makes two requests:
// https://github.com/git/git/blob/master/Documentation/technical/http-protocol.txt
@@ -51,11 +55,13 @@
assertThat(e.who.getAccountId()).isEqualTo(admin.id);
assertThat(e.what).endsWith("/git-receive-pack");
assertThat(e.params).isEmpty();
+ assertThat(((HttpAuditEvent) e).httpStatus).isEqualTo(HttpServletResponse.SC_OK);
}
@Test
public void uploadPackAuditEventLog() throws Exception {
testRepo.git().fetch().call();
+ waitForAudit();
assertThat(auditService.auditEvents.size()).isEqualTo(1);
@@ -64,5 +70,12 @@
assertThat(e.params.get("service"))
.containsExactlyElementsIn(Collections.singletonList("git-upload-pack"));
assertThat(e.what).endsWith("service=git-upload-pack");
+ assertThat(((HttpAuditEvent) e).httpStatus).isEqualTo(HttpServletResponse.SC_OK);
+ }
+
+ private void waitForAudit() throws InterruptedException {
+ synchronized (auditService.auditEvents) {
+ auditService.auditEvents.wait(AUDIT_EVENT_TIMEOUT);
+ }
}
}
diff --git a/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java b/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java
index e603413..eb79c18 100644
--- a/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java
+++ b/javatests/com/google/gerrit/acceptance/ssh/SshCommandsIT.java
@@ -28,7 +28,6 @@
import com.google.gerrit.acceptance.Sandboxed;
import com.google.gerrit.acceptance.UseSsh;
import com.google.gerrit.common.data.GlobalCapability;
-import com.google.gerrit.sshd.Commands;
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
@@ -80,27 +79,35 @@
"stream-events",
"test-submit");
+ private static final ImmutableList<String> EMPTY = ImmutableList.of();
private static final ImmutableMap<String, List<String>> MASTER_COMMANDS =
- ImmutableMap.of(
- Commands.ROOT,
- Streams.concat(COMMON_ROOT_COMMANDS.stream(), MASTER_ONLY_ROOT_COMMANDS.stream())
- .sorted()
- .collect(toImmutableList()),
- "index",
- ImmutableList.of(
- "changes", "changes-in-project"), // "activate" and "start" are not included
- "logging",
- ImmutableList.of("ls", "set"),
- "plugin",
- ImmutableList.of("add", "enable", "install", "ls", "reload", "remove", "rm"),
- "test-submit",
- ImmutableList.of("rule", "type"));
+ ImmutableMap.<String, List<String>>builder()
+ .put("kill", EMPTY)
+ .put("ps", EMPTY)
+ // TODO(dpursehouse): Add "scp" and "suexec"
+ .put(
+ "gerrit",
+ Streams.concat(COMMON_ROOT_COMMANDS.stream(), MASTER_ONLY_ROOT_COMMANDS.stream())
+ .sorted()
+ .collect(toImmutableList()))
+ .put(
+ "gerrit index",
+ ImmutableList.of(
+ "changes", "changes-in-project")) // "activate" and "start" are not included
+ .put("gerrit logging", ImmutableList.of("ls", "set"))
+ .put(
+ "gerrit plugin",
+ ImmutableList.of("add", "enable", "install", "ls", "reload", "remove", "rm"))
+ .put("gerrit test-submit", ImmutableList.of("rule", "type"))
+ .build();
private static final ImmutableMap<String, List<String>> SLAVE_COMMANDS =
ImmutableMap.of(
- Commands.ROOT,
+ "kill",
+ EMPTY,
+ "gerrit",
COMMON_ROOT_COMMANDS,
- "plugin",
+ "gerrit plugin",
ImmutableList.of("add", "enable", "install", "ls", "reload", "remove", "rm"));
@Test
@@ -117,22 +124,30 @@
private void testCommandExecution(Map<String, List<String>> commands) throws Exception {
for (String root : commands.keySet()) {
- for (String command : commands.get(root)) {
- // We can't assert that adminSshSession.hasError() is false, because using the --help
- // option causes the usage info to be written to stderr. Instead, we assert on the
- // content of the stderr, which will always start with "gerrit command" when the --help
- // option is used.
- String cmd = String.format("gerrit%s%s %s", root.isEmpty() ? "" : " ", root, command);
- logger.atFine().log(cmd);
- adminSshSession.exec(String.format("%s --help", cmd));
- String response = adminSshSession.getError();
- assertWithMessage(String.format("command %s failed: %s", command, response))
- .that(response)
- .startsWith(cmd);
+ List<String> cmds = commands.get(root);
+ if (cmds.isEmpty()) {
+ testCommandExecution(root);
+ } else {
+ for (String cmd : cmds) {
+ testCommandExecution(String.format("%s %s", root, cmd));
+ }
}
}
}
+ private void testCommandExecution(String cmd) throws Exception {
+ // We can't assert that adminSshSession.hasError() is false, because using the --help
+ // option causes the usage info to be written to stderr. Instead, we assert on the
+ // content of the stderr, which will always start with "gerrit command" when the --help
+ // option is used.
+ logger.atFine().log(cmd);
+ adminSshSession.exec(String.format("%s --help", cmd));
+ String response = adminSshSession.getError();
+ assertWithMessage(String.format("command %s failed: %s", cmd, response))
+ .that(response)
+ .startsWith(cmd);
+ }
+
@Test
public void nonExistingCommandFails() throws Exception {
adminSshSession.exec("gerrit non-existing-command --help");
@@ -145,12 +160,12 @@
public void listCommands() throws Exception {
adminSshSession.exec("gerrit --help");
List<String> commands = parseCommandsFromGerritHelpText(adminSshSession.getError());
- assertThat(commands).containsExactlyElementsIn(MASTER_COMMANDS.get(Commands.ROOT)).inOrder();
+ assertThat(commands).containsExactlyElementsIn(MASTER_COMMANDS.get("gerrit")).inOrder();
restartAsSlave();
adminSshSession.exec("gerrit --help");
commands = parseCommandsFromGerritHelpText(adminSshSession.getError());
- assertThat(commands).containsExactlyElementsIn(SLAVE_COMMANDS.get(Commands.ROOT)).inOrder();
+ assertThat(commands).containsExactlyElementsIn(SLAVE_COMMANDS.get("gerrit")).inOrder();
}
private List<String> parseCommandsFromGerritHelpText(String helpText) {
diff --git a/javatests/com/google/gerrit/elasticsearch/BUILD b/javatests/com/google/gerrit/elasticsearch/BUILD
index 20ef2fa..61bd263 100644
--- a/javatests/com/google/gerrit/elasticsearch/BUILD
+++ b/javatests/com/google/gerrit/elasticsearch/BUILD
@@ -18,6 +18,7 @@
"//lib/httpcomponents:httpcore",
"//lib/jgit/org.eclipse.jgit:jgit",
"//lib/testcontainers",
+ "//lib/testcontainers:testcontainers-elasticsearch",
],
)
@@ -73,7 +74,10 @@
size = "large",
srcs = [src],
tags = ELASTICSEARCH_TAGS + ["flaky"],
- deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name],
+ deps = ELASTICSEARCH_DEPS + [QUERY_TESTS_DEP % name] + [
+ "//lib/httpcomponents:httpasyncclient",
+ "//lib/httpcomponents:httpclient",
+ ],
) for name, src in ELASTICSEARCH_TESTS_V7.items()]
junit_tests(
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
index 79e2853..14fe4de 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticContainer.java
@@ -14,21 +14,19 @@
package com.google.gerrit.elasticsearch;
-import com.google.common.collect.ImmutableSet;
-import java.util.Set;
import org.apache.http.HttpHost;
import org.junit.AssumptionViolatedException;
-import org.testcontainers.containers.GenericContainer;
+import org.testcontainers.elasticsearch.ElasticsearchContainer;
/* Helper class for running ES integration tests in docker container */
-public class ElasticContainer<SELF extends ElasticContainer<SELF>> extends GenericContainer<SELF> {
+public class ElasticContainer extends ElasticsearchContainer {
private static final int ELASTICSEARCH_DEFAULT_PORT = 9200;
- public static ElasticContainer<?> createAndStart(ElasticVersion version) {
+ public static ElasticContainer createAndStart(ElasticVersion version) {
// Assumption violation is not natively supported by Testcontainers.
// See https://github.com/testcontainers/testcontainers-java/issues/343
try {
- ElasticContainer<?> container = new ElasticContainer<>(version);
+ ElasticContainer container = new ElasticContainer(version);
container.start();
return container;
} catch (Throwable t) {
@@ -58,16 +56,6 @@
super(getImageName(version));
}
- @Override
- protected void configure() {
- addExposedPort(ELASTICSEARCH_DEFAULT_PORT);
- }
-
- @Override
- public Set<Integer> getLivenessCheckPortNumbers() {
- return ImmutableSet.of(getMappedPort(ELASTICSEARCH_DEFAULT_PORT));
- }
-
public HttpHost getHttpHost() {
return new HttpHost(getContainerIpAddress(), getMappedPort(ELASTICSEARCH_DEFAULT_PORT));
}
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
index 9f7b60c..020a158 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticTestUtils.java
@@ -58,7 +58,7 @@
public static Config getConfig(ElasticVersion version) {
ElasticNodeInfo elasticNodeInfo;
- ElasticContainer<?> container = ElasticContainer.createAndStart(version);
+ ElasticContainer container = ElasticContainer.createAndStart(version);
elasticNodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
String indicesPrefix = UUID.randomUUID().toString();
Config cfg = new Config();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
index 074debc..c8ce54a 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryAccountsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
index 89d0168..cfdfa98 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryChangesTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
index 79d3ca5..832a7bd 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryGroupsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java
index 247cb29..29d3fa4 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV5QueryProjectsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java
index 519a9c3..8833907 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryAccountsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
index 690734c..8ba753c 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryChangesTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java
index bf0095a..cecb085 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryGroupsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java
index f2227a9..47e9b10 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV6QueryProjectsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
index 7ac9f21..bddbbc9 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryAccountsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
index ed6f95b..5dcf159 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryChangesTest.java
@@ -21,7 +21,12 @@
import com.google.gerrit.testing.IndexConfig;
import com.google.inject.Guice;
import com.google.inject.Injector;
+import org.apache.http.client.methods.HttpPost;
+import org.apache.http.client.protocol.HttpClientContext;
+import org.apache.http.impl.nio.client.CloseableHttpAsyncClient;
+import org.apache.http.impl.nio.client.HttpAsyncClients;
import org.eclipse.jgit.lib.Config;
+import org.junit.After;
import org.junit.AfterClass;
import org.junit.BeforeClass;
@@ -32,7 +37,8 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
+ private static CloseableHttpAsyncClient client;
@BeforeClass
public static void startIndexService() {
@@ -43,6 +49,8 @@
container = ElasticContainer.createAndStart(ElasticVersion.V7_0);
nodeInfo = new ElasticNodeInfo(container.getHttpHost().getPort());
+ client = HttpAsyncClients.createDefault();
+ client.start();
}
@AfterClass
@@ -52,6 +60,16 @@
}
}
+ @After
+ public void closeIndex() {
+ client.execute(
+ new HttpPost(
+ String.format(
+ "http://localhost:%d/%s*/_close", nodeInfo.port, getSanitizedMethodName())),
+ HttpClientContext.create(),
+ null);
+ }
+
@Override
protected void initAfterLifecycleStart() throws Exception {
super.initAfterLifecycleStart();
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
index 5efaa4a..54be7b9 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryGroupsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
index 02b3641..e8b4a2c 100644
--- a/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
+++ b/javatests/com/google/gerrit/elasticsearch/ElasticV7QueryProjectsTest.java
@@ -32,7 +32,7 @@
}
private static ElasticNodeInfo nodeInfo;
- private static ElasticContainer<?> container;
+ private static ElasticContainer container;
@BeforeClass
public static void startIndexService() {
diff --git a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
index de2234e..0ea5241 100644
--- a/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
+++ b/javatests/com/google/gerrit/server/query/change/AbstractQueryChangesTest.java
@@ -1709,6 +1709,10 @@
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId();
assertQuery(q + " visibleto:" + user2.get(), change1);
+ String g1 = createGroup("group1", "Administrators");
+ gApi.groups().id(g1).addMembers("anotheruser");
+ assertQuery(q + " visibleto:" + g1, change1);
+
requestContext.setContext(
newRequestContext(
accountManager.authenticate(AuthRequest.forUser("anotheruser")).getAccountId()));
diff --git a/javatests/com/google/gerrit/util/http/RequestUtilTest.java b/javatests/com/google/gerrit/util/http/RequestUtilTest.java
index 0bf34e7..adda5e7 100644
--- a/javatests/com/google/gerrit/util/http/RequestUtilTest.java
+++ b/javatests/com/google/gerrit/util/http/RequestUtilTest.java
@@ -15,6 +15,8 @@
package com.google.gerrit.util.http;
import static com.google.common.truth.Truth.assertThat;
+import static com.google.gerrit.util.http.RequestUtil.getEncodedPathInfo;
+import static com.google.gerrit.util.http.RequestUtil.getRestPathWithoutIds;
import com.google.gerrit.testing.GerritBaseTests;
import com.google.gerrit.util.http.testutil.FakeHttpServletRequest;
@@ -22,36 +24,45 @@
public class RequestUtilTest extends GerritBaseTests {
@Test
- public void emptyContextPath() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/s", "/foo/bar")))
- .isEqualTo("/foo/bar");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/s", "/foo%2Fbar")))
- .isEqualTo("/foo%2Fbar");
+ public void getEncodedPathInfo_emptyContextPath() {
+ assertThat(getEncodedPathInfo(fakeRequest("", "/s", "/foo/bar"))).isEqualTo("/foo/bar");
+ assertThat(getEncodedPathInfo(fakeRequest("", "/s", "/foo%2Fbar"))).isEqualTo("/foo%2Fbar");
}
@Test
- public void emptyServletPath() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/c", "/foo/bar")))
- .isEqualTo("/foo/bar");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("", "/c", "/foo%2Fbar")))
- .isEqualTo("/foo%2Fbar");
+ public void getEncodedPathInfo_emptyServletPath() {
+ assertThat(getEncodedPathInfo(fakeRequest("", "/c", "/foo/bar"))).isEqualTo("/foo/bar");
+ assertThat(getEncodedPathInfo(fakeRequest("", "/c", "/foo%2Fbar"))).isEqualTo("/foo%2Fbar");
}
@Test
- public void trailingSlashes() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar/")))
- .isEqualTo("/foo/bar/");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar///")))
- .isEqualTo("/foo/bar/");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar/")))
- .isEqualTo("/foo%2Fbar/");
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar///")))
+ public void getEncodedPathInfo_trailingSlashes() {
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar/"))).isEqualTo("/foo/bar/");
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo/bar///"))).isEqualTo("/foo/bar/");
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar/"))).isEqualTo("/foo%2Fbar/");
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", "/foo%2Fbar///")))
.isEqualTo("/foo%2Fbar/");
}
@Test
public void emptyPathInfo() {
- assertThat(RequestUtil.getEncodedPathInfo(fakeRequest("/c", "/s", ""))).isNull();
+ assertThat(getEncodedPathInfo(fakeRequest("/c", "/s", ""))).isNull();
+ }
+
+ @Test
+ public void getRestPathWithoutIds_emptyContextPath() {
+ assertThat(getRestPathWithoutIds(fakeRequest("", "/a/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
+ assertThat(getRestPathWithoutIds(fakeRequest("", "/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
+ }
+
+ @Test
+ public void getRestPathWithoutIds_nonEmptyContextPath() {
+ assertThat(getRestPathWithoutIds(fakeRequest("/c", "/a/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
+ assertThat(getRestPathWithoutIds(fakeRequest("/c", "/accounts", "/123/test")))
+ .isEqualTo("/accounts/test");
}
private FakeHttpServletRequest fakeRequest(
diff --git a/lib/httpcomponents/BUILD b/lib/httpcomponents/BUILD
index a875eaf..74cca14 100644
--- a/lib/httpcomponents/BUILD
+++ b/lib/httpcomponents/BUILD
@@ -30,7 +30,10 @@
java_library(
name = "httpasyncclient",
data = ["//lib:LICENSE-Apache2.0"],
- visibility = ["//java/com/google/gerrit/elasticsearch:__pkg__"],
+ visibility = [
+ "//java/com/google/gerrit/elasticsearch:__pkg__",
+ "//javatests/com/google/gerrit/elasticsearch:__pkg__",
+ ],
exports = ["@httpasyncclient//jar"],
)
diff --git a/lib/testcontainers/BUILD b/lib/testcontainers/BUILD
index f99365d..5e2d039 100644
--- a/lib/testcontainers/BUILD
+++ b/lib/testcontainers/BUILD
@@ -35,3 +35,12 @@
"//lib/log:ext",
],
)
+
+java_library(
+ name = "testcontainers-elasticsearch",
+ testonly = 1,
+ data = ["//lib:LICENSE-testcontainers"],
+ visibility = ["//visibility:public"],
+ exports = ["@testcontainers-elasticsearch//jar"],
+ runtime_deps = [":testcontainers"],
+)
diff --git a/plugins/reviewnotes b/plugins/reviewnotes
index 041ac8d..863130d 160000
--- a/plugins/reviewnotes
+++ b/plugins/reviewnotes
@@ -1 +1 @@
-Subproject commit 041ac8d9fb0c0e5adff0aa37773a1931aced5a9a
+Subproject commit 863130de158605c19809956864870ff05d86b494
diff --git a/proto/BUILD b/proto/BUILD
index 00d725a..88445c1 100644
--- a/proto/BUILD
+++ b/proto/BUILD
@@ -9,13 +9,6 @@
deps = [":cache_proto"],
)
-genrule(
- name = "gen_reviewdb_proto",
- outs = ["reviewdb.proto"],
- cmd = "$(location //java/com/google/gerrit/proto:ProtoGen) -o $@",
- tools = ["//java/com/google/gerrit/proto:ProtoGen"],
-)
-
proto_library(
name = "reviewdb_proto",
srcs = [":reviewdb.proto"],
diff --git a/proto/reviewdb.proto b/proto/reviewdb.proto
new file mode 100644
index 0000000..bc45eea
--- /dev/null
+++ b/proto/reviewdb.proto
@@ -0,0 +1,211 @@
+// Copyright (C) 2018 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.
+
+syntax = "proto2";
+
+package devtools.gerritcodereview;
+
+option java_package = "com.google.gerrit.proto.reviewdb";
+
+// Serialized form of com.google.gerrit.reviewdb.client.Change.Id.
+// Next ID: 2
+message Change_Id {
+ required int32 id = 1;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.Change.Key.
+// Next ID: 2
+message Change_Key {
+ optional string id = 1;
+}
+
+// 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;
+
+ // Deleted fields, should not be reused:
+ reserved 6; // sortkey
+ reserved 9; // open
+ reserved 11; // nbrPatchSets
+ reserved 15; // lastSha1MergeTested
+ reserved 16; // mergeable
+}
+
+// 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;
+}
+
+// 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;
+}
+
+// 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;
+}
+
+// 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;
+
+ // 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;
+}
+
+// Serialized form of com.google.gerrit.reviewdb.client.Project.NameKey.
+// Next ID: 2
+message Project_NameKey {
+ optional string name = 1;
+}
+
+// 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;
+}
