diff options
31 files changed, 615 insertions, 36 deletions
diff --git a/Documentation/cmd-stream-events.txt b/Documentation/cmd-stream-events.txt index 8f24a47e03..1ab67d87d5 100644 --- a/Documentation/cmd-stream-events.txt +++ b/Documentation/cmd-stream-events.txt @@ -112,7 +112,8 @@ patchSet:: link:json.html#patchSet[patchSet attribute] submitter:: link:json.html#account[account attribute] -newRev:: The resulting revision of the merge. +newRev:: The state (revision) of the target branch after the operation that +closed the change was completed. eventCreatedOn:: Time in seconds since the UNIX epoch when this event was created. diff --git a/Documentation/config-gerrit.txt b/Documentation/config-gerrit.txt index 91cb87f0fa..a8eb875a50 100644 --- a/Documentation/config-gerrit.txt +++ b/Documentation/config-gerrit.txt @@ -4271,7 +4271,8 @@ Number of threads to use when executing SSH command requests. If additional requests are received while all threads are busy they are queued and serviced in a first-come-first-served order. + -By default, 2x the number of CPUs available to the JVM. +By default, 2x the number of CPUs available to the JVM (but at least 4 +threads). + [NOTE] When SSH daemon is enabled then this setting also defines the max number of @@ -1072,18 +1072,18 @@ maven_jar( sha1 = "0f5a654e4675769c716e5b387830d19b501ca191", ) -TESTCONTAINERS_VERSION = "1.11.4" +TESTCONTAINERS_VERSION = "1.12.0" maven_jar( name = "testcontainers", artifact = "org.testcontainers:testcontainers:" + TESTCONTAINERS_VERSION, - sha1 = "b0c70b1a3608f43deafba7649b344a422a442585", + sha1 = "ac89643ce1ddde504da09172086aba0c7df10bff", ) maven_jar( name = "testcontainers-elasticsearch", artifact = "org.testcontainers:elasticsearch:" + TESTCONTAINERS_VERSION, - sha1 = "faab09a8876b8dbb326cbc10bbaa5ea86f5f5299", + sha1 = "cd9020f1803396c45ef935312bf232f9b17332b0", ) maven_jar( diff --git a/java/com/google/gerrit/acceptance/EventRecorder.java b/java/com/google/gerrit/acceptance/EventRecorder.java index 6c6e1bfe2e..cab6b58b37 100644 --- a/java/com/google/gerrit/acceptance/EventRecorder.java +++ b/java/com/google/gerrit/acceptance/EventRecorder.java @@ -16,6 +16,7 @@ package com.google.gerrit.acceptance; import static com.google.common.truth.Truth.assertThat; +import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.FluentIterable; import com.google.common.collect.ImmutableList; import com.google.common.collect.LinkedListMultimap; @@ -109,7 +110,8 @@ public class EventRecorder { return events; } - private ImmutableList<ChangeMergedEvent> getChangeMergedEvents( + @VisibleForTesting + public ImmutableList<ChangeMergedEvent> getChangeMergedEvents( String project, String branch, int expectedSize) { String key = refEventKey(ChangeMergedEvent.TYPE, project, branch); if (expectedSize == 0) { diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java index 4a203be639..d0ed6732e7 100644 --- a/java/com/google/gerrit/acceptance/InProcessProtocol.java +++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java @@ -14,6 +14,10 @@ package com.google.gerrit.acceptance; +import static com.google.gerrit.server.git.receive.LazyPostReceiveHookChain.affectsSize; +import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP; + +import com.google.common.collect.ImmutableList; import com.google.common.collect.Lists; import com.google.gerrit.acceptance.InProcessProtocol.Context; import com.google.gerrit.common.data.Capable; @@ -40,6 +44,9 @@ 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.quota.QuotaBackend; +import com.google.gerrit.server.quota.QuotaException; +import com.google.gerrit.server.quota.QuotaResponse; import com.google.gerrit.server.util.RequestContext; import com.google.gerrit.server.util.RequestScopePropagator; import com.google.gerrit.server.util.ThreadLocalRequestContext; @@ -261,6 +268,7 @@ class InProcessProtocol extends TestProtocol<Context> { private final DynamicSet<PostReceiveHook> postReceiveHooks; private final ThreadLocalRequestContext threadContext; private final PermissionBackend permissionBackend; + private final QuotaBackend quotaBackend; @Inject Receive( @@ -271,7 +279,8 @@ class InProcessProtocol extends TestProtocol<Context> { PluginSetContext<ReceivePackInitializer> receivePackInitializers, DynamicSet<PostReceiveHook> postReceiveHooks, ThreadLocalRequestContext threadContext, - PermissionBackend permissionBackend) { + PermissionBackend permissionBackend, + QuotaBackend quotaBackend) { this.userProvider = userProvider; this.projectCache = projectCache; this.factory = factory; @@ -280,6 +289,7 @@ class InProcessProtocol extends TestProtocol<Context> { this.postReceiveHooks = postReceiveHooks; this.threadContext = threadContext; this.permissionBackend = permissionBackend; + this.quotaBackend = quotaBackend; } @Override @@ -319,10 +329,35 @@ class InProcessProtocol extends TestProtocol<Context> { receivePackInitializers.runEach( initializer -> initializer.init(projectState.getNameKey(), rp)); + QuotaResponse.Aggregated availableTokens = + quotaBackend + .user(identifiedUser) + .project(req.project) + .availableTokens(REPOSITORY_SIZE_GROUP); + availableTokens.throwOnError(); + availableTokens.availableTokens().ifPresent(v -> rp.setMaxObjectSizeLimit(v)); - rp.setPostReceiveHook(PostReceiveHookChain.newChain(Lists.newArrayList(postReceiveHooks))); + ImmutableList<PostReceiveHook> hooks = + ImmutableList.<PostReceiveHook>builder() + .add( + (pack, commands) -> { + if (affectsSize(pack, commands)) { + try { + quotaBackend + .user(identifiedUser) + .project(req.project) + .requestTokens(REPOSITORY_SIZE_GROUP, pack.getPackSize()) + .throwOnError(); + } catch (QuotaException e) { + throw new RuntimeException(e); + } + } + }) + .addAll(postReceiveHooks) + .build(); + rp.setPostReceiveHook(PostReceiveHookChain.newChain(hooks)); return rp; - } catch (IOException | PermissionBackendException e) { + } catch (IOException | PermissionBackendException | QuotaException e) { throw new RuntimeException(e); } } diff --git a/java/com/google/gerrit/json/SqlTimestampDeserializer.java b/java/com/google/gerrit/json/SqlTimestampDeserializer.java index 0148226284..e1cf38210a 100644 --- a/java/com/google/gerrit/json/SqlTimestampDeserializer.java +++ b/java/com/google/gerrit/json/SqlTimestampDeserializer.java @@ -44,7 +44,15 @@ class SqlTimestampDeserializer implements JsonDeserializer<Timestamp>, JsonSeria throw new JsonParseException("Expected string for timestamp type"); } - return JavaSqlTimestampHelper.parseTimestamp(p.getAsString()); + String input = p.getAsString(); + if (input.trim().isEmpty()) { + // Magic timestamp to indicate no timestamp. (-> null object) + // Always create a new object as timestamps are mutable. Don't use TimeUtil.never() to not + // introduce an undesired dependency. + return new Timestamp(0); + } + + return JavaSqlTimestampHelper.parseTimestamp(input); } @Override diff --git a/java/com/google/gerrit/server/account/AccountCacheImpl.java b/java/com/google/gerrit/server/account/AccountCacheImpl.java index 5a3c7749e1..af8d8b0c06 100644 --- a/java/com/google/gerrit/server/account/AccountCacheImpl.java +++ b/java/com/google/gerrit/server/account/AccountCacheImpl.java @@ -98,7 +98,7 @@ public class AccountCacheImpl implements AccountCache { return byId.get(accountId); } catch (ExecutionException e) { logger.atWarning().withCause(e).log("Cannot load AccountState for ID %s", accountId); - return null; + return Optional.empty(); } } @@ -146,7 +146,7 @@ public class AccountCacheImpl implements AccountCache { .orElseGet(Optional::empty); } catch (IOException | ConfigInvalidException e) { logger.atWarning().withCause(e).log("Cannot load AccountState for username %s", username); - return null; + return Optional.empty(); } } diff --git a/java/com/google/gerrit/server/change/NotifyResolver.java b/java/com/google/gerrit/server/change/NotifyResolver.java index 787958022a..62c0fdf300 100644 --- a/java/com/google/gerrit/server/change/NotifyResolver.java +++ b/java/com/google/gerrit/server/change/NotifyResolver.java @@ -59,7 +59,6 @@ public class NotifyResolver { public abstract NotifyHandling handling(); - // TODO(dborowitz): Should be ImmutableSetMultimap. public abstract ImmutableSetMultimap<RecipientType, Account.Id> accounts(); public Result withHandling(NotifyHandling notifyHandling) { diff --git a/java/com/google/gerrit/server/config/ThreadSettingsConfig.java b/java/com/google/gerrit/server/config/ThreadSettingsConfig.java index 6cb32cc0e5..c20e0a4ea4 100644 --- a/java/com/google/gerrit/server/config/ThreadSettingsConfig.java +++ b/java/com/google/gerrit/server/config/ThreadSettingsConfig.java @@ -28,7 +28,7 @@ public class ThreadSettingsConfig { @Inject ThreadSettingsConfig(@GerritServerConfig Config cfg) { int cores = Runtime.getRuntime().availableProcessors(); - sshdThreads = cfg.getInt("sshd", "threads", 2 * cores); + sshdThreads = cfg.getInt("sshd", "threads", Math.max(4, 2 * cores)); httpdMaxThreads = cfg.getInt("httpd", "maxThreads", 25); int defaultDatabasePoolLimit = sshdThreads + httpdMaxThreads + 2; databasePoolLimit = cfg.getInt("database", "poolLimit", defaultDatabasePoolLimit); diff --git a/java/com/google/gerrit/server/git/MergedByPushOp.java b/java/com/google/gerrit/server/git/MergedByPushOp.java index 6052a87438..355586229a 100644 --- a/java/com/google/gerrit/server/git/MergedByPushOp.java +++ b/java/com/google/gerrit/server/git/MergedByPushOp.java @@ -49,7 +49,10 @@ public class MergedByPushOp implements BatchUpdateOp { public interface Factory { MergedByPushOp create( - RequestScopePropagator requestScopePropagator, PatchSet.Id psId, String refName); + RequestScopePropagator requestScopePropagator, + PatchSet.Id psId, + @Assisted("refName") String refName, + @Assisted("mergeResultRevId") String mergeResultRevId); } private final RequestScopePropagator requestScopePropagator; @@ -62,6 +65,7 @@ public class MergedByPushOp implements BatchUpdateOp { private final PatchSet.Id psId; private final String refName; + private final String mergeResultRevId; private Change change; private boolean correctBranch; @@ -79,7 +83,8 @@ public class MergedByPushOp implements BatchUpdateOp { ChangeMerged changeMerged, @Assisted RequestScopePropagator requestScopePropagator, @Assisted PatchSet.Id psId, - @Assisted String refName) { + @Assisted("refName") String refName, + @Assisted("mergeResultRevId") String mergeResultRevId) { this.patchSetInfoFactory = patchSetInfoFactory; this.cmUtil = cmUtil; this.mergedSenderFactory = mergedSenderFactory; @@ -89,6 +94,7 @@ public class MergedByPushOp implements BatchUpdateOp { this.requestScopePropagator = requestScopePropagator; this.psId = psId; this.refName = refName; + this.mergeResultRevId = mergeResultRevId; } public String getMergedIntoRef() { @@ -184,8 +190,7 @@ public class MergedByPushOp implements BatchUpdateOp { } })); - changeMerged.fire( - change, patchSet, ctx.getAccount(), patchSet.commitId().name(), ctx.getWhen()); + changeMerged.fire(change, patchSet, ctx.getAccount(), mergeResultRevId, ctx.getWhen()); } private PatchSetInfo getPatchSetInfo(ChangeContext ctx) throws IOException { diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java index bce5b0a3e9..b8a2aed6ff 100644 --- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java @@ -14,6 +14,7 @@ package com.google.gerrit.server.git.receive; +import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP; import static java.util.concurrent.TimeUnit.NANOSECONDS; import com.google.common.flogger.FluentLogger; @@ -45,6 +46,9 @@ import com.google.gerrit.server.permissions.ProjectPermission; import com.google.gerrit.server.project.ContributorAgreementsChecker; import com.google.gerrit.server.project.ProjectState; import com.google.gerrit.server.query.change.InternalChangeQuery; +import com.google.gerrit.server.quota.QuotaBackend; +import com.google.gerrit.server.quota.QuotaException; +import com.google.gerrit.server.quota.QuotaResponse; import com.google.gerrit.server.util.MagicBranch; import com.google.gerrit.server.util.RequestScopePropagator; import com.google.inject.Inject; @@ -92,6 +96,7 @@ public class AsyncReceiveCommits implements PreReceiveHook { public static class Module extends PrivateModule { @Override public void configure() { + install(new FactoryModuleBuilder().build(LazyPostReceiveHookChain.Factory.class)); install(new FactoryModuleBuilder().build(AsyncReceiveCommits.Factory.class)); expose(AsyncReceiveCommits.Factory.class); // Don't expose the binding for ReceiveCommits.Factory. All callers should @@ -257,9 +262,10 @@ public class AsyncReceiveCommits implements PreReceiveHook { RequestScopePropagator scopePropagator, ReceiveConfig receiveConfig, TransferConfig transferConfig, - Provider<LazyPostReceiveHookChain> lazyPostReceive, + LazyPostReceiveHookChain.Factory lazyPostReceive, ContributorAgreementsChecker contributorAgreements, Metrics metrics, + QuotaBackend quotaBackend, @Named(TIMEOUT_NAME) long timeoutMillis, @Assisted ProjectState projectState, @Assisted IdentifiedUser user, @@ -288,7 +294,7 @@ public class AsyncReceiveCommits implements PreReceiveHook { receivePack.setRefFilter(new ReceiveRefFilter()); receivePack.setAllowPushOptions(true); receivePack.setPreReceiveHook(this); - receivePack.setPostReceiveHook(lazyPostReceive.get()); + receivePack.setPostReceiveHook(lazyPostReceive.create(user, projectName)); // If the user lacks READ permission, some references may be filtered and hidden from view. // Check objects mentioned inside the incoming pack file are reachable from visible refs. @@ -310,6 +316,17 @@ public class AsyncReceiveCommits implements PreReceiveHook { factory.create( projectState, user, receivePack, allRefsWatcher, messageSender, resultChangeIds); receiveCommits.init(); + QuotaResponse.Aggregated availableTokens = + quotaBackend.user(user).project(projectName).availableTokens(REPOSITORY_SIZE_GROUP); + try { + availableTokens.throwOnError(); + } catch (QuotaException e) { + logger.atWarning().withCause(e).log( + "Quota %s availableTokens request failed for project %s", + REPOSITORY_SIZE_GROUP, projectName); + throw new RuntimeException(e); + } + availableTokens.availableTokens().ifPresent(v -> receivePack.setMaxObjectSizeLimit(v)); } /** Determine if the user can upload commits. */ diff --git a/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java b/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java index 0f081bebb5..8e200eb67a 100644 --- a/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java +++ b/java/com/google/gerrit/server/git/receive/LazyPostReceiveHookChain.java @@ -14,23 +14,78 @@ package com.google.gerrit.server.git.receive; +import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP; + +import com.google.common.flogger.FluentLogger; +import com.google.gerrit.reviewdb.client.Project; +import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.plugincontext.PluginSetContext; +import com.google.gerrit.server.quota.QuotaBackend; +import com.google.gerrit.server.quota.QuotaResponse; import com.google.inject.Inject; +import com.google.inject.assistedinject.Assisted; import java.util.Collection; import org.eclipse.jgit.transport.PostReceiveHook; import org.eclipse.jgit.transport.ReceiveCommand; import org.eclipse.jgit.transport.ReceivePack; -class LazyPostReceiveHookChain implements PostReceiveHook { +/** + * Class is responsible for calling all registered post-receive hooks. In addition, in case when + * repository size quota is defined, it requests tokens (pack size) that were received. This is the + * final step of enforcing repository size quota that deducts token from available tokens. + */ +public class LazyPostReceiveHookChain implements PostReceiveHook { + interface Factory { + LazyPostReceiveHookChain create(CurrentUser user, Project.NameKey project); + } + + private static final FluentLogger logger = FluentLogger.forEnclosingClass(); + private final PluginSetContext<PostReceiveHook> hooks; + private final QuotaBackend quotaBackend; + private final CurrentUser user; + private final Project.NameKey project; @Inject - LazyPostReceiveHookChain(PluginSetContext<PostReceiveHook> hooks) { + LazyPostReceiveHookChain( + PluginSetContext<PostReceiveHook> hooks, + QuotaBackend quotaBackend, + @Assisted CurrentUser user, + @Assisted Project.NameKey project) { this.hooks = hooks; + this.quotaBackend = quotaBackend; + this.user = user; + this.project = project; } @Override public void onPostReceive(ReceivePack rp, Collection<ReceiveCommand> commands) { hooks.runEach(h -> h.onPostReceive(rp, commands)); + if (affectsSize(rp, commands)) { + QuotaResponse.Aggregated a = + quotaBackend + .user(user) + .project(project) + .requestTokens(REPOSITORY_SIZE_GROUP, rp.getPackSize()); + if (a.hasError()) { + String msg = + String.format( + "%s request failed for project %s with [%s]", + REPOSITORY_SIZE_GROUP, project, a.errorMessage()); + logger.atWarning().log(msg); + throw new RuntimeException(msg); + } + } + } + + public static boolean affectsSize(ReceivePack rp, Collection<ReceiveCommand> commands) { + if (rp.getPackSize() > 0L) { + for (ReceiveCommand cmd : commands) { + if (cmd.getType() != ReceiveCommand.Type.DELETE) { + return true; + } + } + } + return false; } } diff --git a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java index f8e6751594..38e60d4601 100644 --- a/java/com/google/gerrit/server/git/receive/ReceiveCommits.java +++ b/java/com/google/gerrit/server/git/receive/ReceiveCommits.java @@ -2952,6 +2952,7 @@ class ReceiveCommits { projectState, notes.getChange().getDest(), checkMergedInto, + checkMergedInto ? inputCommand.getNewId().name() : null, priorPatchSet, priorCommit, psId, @@ -3187,6 +3188,9 @@ class ReceiveCommits { int limit = receiveConfig.maxBatchCommits; int n = 0; for (RevCommit c; (c = walk.next()) != null; ) { + // Even if skipValidation is set, we still get here when at least one plugin + // commit validator requires to validate all commits. In this case, however, + // we don't need to check the commit limit. if (++n > limit && !skipValidation) { logger.atFine().log("Number of new commits exceeds limit of %d", limit); reject( @@ -3263,7 +3267,8 @@ class ReceiveCommits { bu.addOp(notes.get().getChangeId(), setPrivateOpFactory.create(false, null)); bu.addOp( psId.changeId(), - mergedByPushOpFactory.create(requestScopePropagator, psId, refName)); + mergedByPushOpFactory.create( + requestScopePropagator, psId, refName, newTip.getId().getName())); continue COMMIT; } } @@ -3297,7 +3302,8 @@ class ReceiveCommits { bu.addOp( id, mergedByPushOpFactory - .create(requestScopePropagator, req.psId, refName) + .create( + requestScopePropagator, req.psId, refName, newTip.getId().getName()) .setPatchSetProvider(req.replaceOp::getPatchSet)); bu.addOp(id, new ChangeProgressOp(progress)); ids.add(id); diff --git a/java/com/google/gerrit/server/git/receive/ReplaceOp.java b/java/com/google/gerrit/server/git/receive/ReplaceOp.java index 05a0e0dd3c..5e1f1782cd 100644 --- a/java/com/google/gerrit/server/git/receive/ReplaceOp.java +++ b/java/com/google/gerrit/server/git/receive/ReplaceOp.java @@ -101,6 +101,7 @@ public class ReplaceOp implements BatchUpdateOp { ProjectState projectState, BranchNameKey dest, boolean checkMergedInto, + @Nullable String mergeResultRevId, @Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId, @Assisted("priorCommitId") ObjectId priorCommit, @Assisted("patchSetId") PatchSet.Id patchSetId, @@ -133,6 +134,7 @@ public class ReplaceOp implements BatchUpdateOp { private final ProjectState projectState; private final BranchNameKey dest; private final boolean checkMergedInto; + private final String mergeResultRevId; private final PatchSet.Id priorPatchSetId; private final ObjectId priorCommitId; private final PatchSet.Id patchSetId; @@ -177,6 +179,7 @@ public class ReplaceOp implements BatchUpdateOp { @Assisted ProjectState projectState, @Assisted BranchNameKey dest, @Assisted boolean checkMergedInto, + @Assisted @Nullable String mergeResultRevId, @Assisted("priorPatchSetId") PatchSet.Id priorPatchSetId, @Assisted("priorCommitId") ObjectId priorCommitId, @Assisted("patchSetId") PatchSet.Id patchSetId, @@ -205,6 +208,7 @@ public class ReplaceOp implements BatchUpdateOp { this.projectState = projectState; this.dest = dest; this.checkMergedInto = checkMergedInto; + this.mergeResultRevId = mergeResultRevId; this.priorPatchSetId = priorPatchSetId; this.priorCommitId = priorCommitId.copy(); this.patchSetId = patchSetId; @@ -231,7 +235,8 @@ public class ReplaceOp implements BatchUpdateOp { String mergedInto = findMergedInto(ctx, dest.branch(), commit); if (mergedInto != null) { mergedByPushOp = - mergedByPushOpFactory.create(requestScopePropagator, patchSetId, mergedInto); + mergedByPushOpFactory.create( + requestScopePropagator, patchSetId, mergedInto, mergeResultRevId); } } diff --git a/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java b/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java index 479bb529f3..d39e55ca80 100644 --- a/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java +++ b/java/com/google/gerrit/server/quota/DefaultQuotaBackend.java @@ -24,6 +24,7 @@ import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.server.CurrentUser; import com.google.gerrit.server.plugincontext.PluginSetContext; import com.google.gerrit.server.plugincontext.PluginSetEntryContext; +import com.google.gerrit.server.quota.QuotaResponse.Aggregated; import com.google.inject.Inject; import com.google.inject.Provider; import com.google.inject.Singleton; @@ -100,6 +101,20 @@ public class DefaultQuotaBackend implements QuotaBackend { return QuotaResponse.Aggregated.create(ImmutableList.copyOf(responses)); } + private static QuotaResponse.Aggregated availableTokens( + PluginSetContext<QuotaEnforcer> quotaEnforcers, + String quotaGroup, + QuotaRequestContext requestContext) { + // 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) { + responses.add(enforcer.call(p -> p.availableTokens(quotaGroup, requestContext))); + } + return QuotaResponse.Aggregated.create(responses); + } + private static void refillAfterErrorOrException( List<PluginSetEntryContext<QuotaEnforcer>> enforcers, List<QuotaResponse> collectedResponses, @@ -158,5 +173,10 @@ public class DefaultQuotaBackend implements QuotaBackend { return DefaultQuotaBackend.request( quotaEnforcers, quotaGroup, requestContext, numTokens, false); } + + @Override + public Aggregated availableTokens(String quotaGroup) { + return DefaultQuotaBackend.availableTokens(quotaEnforcers, quotaGroup, requestContext); + } } } diff --git a/java/com/google/gerrit/server/quota/QuotaBackend.java b/java/com/google/gerrit/server/quota/QuotaBackend.java index d4dc46dd78..11ce61f74e 100644 --- a/java/com/google/gerrit/server/quota/QuotaBackend.java +++ b/java/com/google/gerrit/server/quota/QuotaBackend.java @@ -82,5 +82,22 @@ public interface QuotaBackend { * not to deduct any quota yet. Can be used to do pre-flight requests where necessary */ QuotaResponse.Aggregated dryRun(String quotaGroup, long tokens); + + /** + * Requests a minimum number of tokens available in implementations. This is a pre-flight check + * for the exceptional case when the requested number of tokens is not known in advance but + * boundary can be specified. For instance, when the commit is received its size is not known + * until the transfer happens however one can specify how many bytes can be accepted to meet the + * repository size quota. + * + * <p>By definition, this is not an allocating request, therefore, it should be followed by the + * call to {@link #requestTokens(String, long)} when the size gets determined so that quota + * could be properly adjusted. It is in developer discretion to ensure that it gets called. + * There might be a case when particular quota gets temporarily overbooked when multiple + * requests are performed but the following calls to {@link #requestTokens(String, long)} will + * fail at the moment when a quota is exhausted. It is not a subject of quota backend to reclaim + * tokens that were used due to overbooking. + */ + QuotaResponse.Aggregated availableTokens(String quotaGroup); } } diff --git a/java/com/google/gerrit/server/quota/QuotaEnforcer.java b/java/com/google/gerrit/server/quota/QuotaEnforcer.java index 9c55e11f0c..4c1de14197 100644 --- a/java/com/google/gerrit/server/quota/QuotaEnforcer.java +++ b/java/com/google/gerrit/server/quota/QuotaEnforcer.java @@ -60,6 +60,19 @@ public interface QuotaEnforcer { QuotaResponse dryRun(String quotaGroup, QuotaRequestContext ctx, long numTokens); /** + * Returns available tokens that can be later requested. + * + * <p>This is used as a pre-flight check for the exceptional case when the requested number of + * tokens is not known in advance. Implementation should not deduct tokens from a bucket. It + * should be followed by a call to {@link #requestTokens(String, QuotaRequestContext, long)} with + * the number of tokens that were eventually used. It is in {@link QuotaBackend} callers + * discretion to ensure that {@link + * com.google.gerrit.server.quota.QuotaBackend.WithResource#requestTokens(String, long)} is + * called. + */ + QuotaResponse availableTokens(String quotaGroup, QuotaRequestContext ctx); + + /** * 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 diff --git a/java/com/google/gerrit/server/quota/QuotaGroupDefinitions.java b/java/com/google/gerrit/server/quota/QuotaGroupDefinitions.java new file mode 100644 index 0000000000..5110538866 --- /dev/null +++ b/java/com/google/gerrit/server/quota/QuotaGroupDefinitions.java @@ -0,0 +1,25 @@ +// Copyright (C) 2019 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; + +public class QuotaGroupDefinitions { + /** + * Definition of repository size quota group. {@link QuotaEnforcer} implementations for repository + * size quota have to act on requests with this group name. + */ + public static final String REPOSITORY_SIZE_GROUP = "/repository:size"; + + private QuotaGroupDefinitions() {} +} diff --git a/java/com/google/gerrit/server/quota/QuotaResponse.java b/java/com/google/gerrit/server/quota/QuotaResponse.java index 99a7c1d86a..940f731327 100644 --- a/java/com/google/gerrit/server/quota/QuotaResponse.java +++ b/java/com/google/gerrit/server/quota/QuotaResponse.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.Streams; import java.util.Collection; import java.util.Optional; +import java.util.OptionalLong; import java.util.stream.Collectors; @AutoValue @@ -55,6 +56,10 @@ public abstract class QuotaResponse { return new AutoValue_QuotaResponse.Builder().status(Status.OK).build(); } + public static QuotaResponse ok(long tokens) { + return new AutoValue_QuotaResponse.Builder().status(Status.OK).availableTokens(tokens).build(); + } + public static QuotaResponse noOp() { return new AutoValue_QuotaResponse.Builder().status(Status.NO_OP).build(); } @@ -65,12 +70,16 @@ public abstract class QuotaResponse { public abstract Status status(); + public abstract Optional<Long> availableTokens(); + public abstract Optional<String> message(); @AutoValue.Builder public abstract static class Builder { public abstract QuotaResponse.Builder status(Status status); + public abstract QuotaResponse.Builder availableTokens(Long tokens); + public abstract QuotaResponse.Builder message(String message); public abstract QuotaResponse build(); @@ -96,6 +105,13 @@ public abstract class QuotaResponse { return responses().stream().filter(r -> r.status().isOk()).collect(toImmutableList()); } + public OptionalLong availableTokens() { + return responses().stream() + .filter(r -> r.status().isOk() && r.availableTokens().isPresent()) + .mapToLong(r -> r.availableTokens().get()) + .min(); + } + public ImmutableList<QuotaResponse> error() { return responses().stream().filter(r -> r.status().isError()).collect(toImmutableList()); } diff --git a/java/com/google/gerrit/server/util/time/BUILD b/java/com/google/gerrit/server/util/time/BUILD index 1d1305d266..c7cd89ec78 100644 --- a/java/com/google/gerrit/server/util/time/BUILD +++ b/java/com/google/gerrit/server/util/time/BUILD @@ -3,6 +3,7 @@ java_library( srcs = glob(["**/*.java"]), visibility = ["//visibility:public"], deps = [ + "//java/com/google/gerrit/common:annotations", "//lib:guava", "//lib/jgit/org.eclipse.jgit:jgit", ], diff --git a/java/com/google/gerrit/server/util/time/TimeUtil.java b/java/com/google/gerrit/server/util/time/TimeUtil.java index 645dbb92f8..b9d2d8af98 100644 --- a/java/com/google/gerrit/server/util/time/TimeUtil.java +++ b/java/com/google/gerrit/server/util/time/TimeUtil.java @@ -15,6 +15,8 @@ package com.google.gerrit.server.util.time; import com.google.common.annotations.VisibleForTesting; +import com.google.gerrit.common.UsedAt; +import com.google.gerrit.common.UsedAt.Project; import java.sql.Timestamp; import java.time.Instant; import java.util.function.LongSupplier; @@ -43,6 +45,17 @@ public class TimeUtil { return new Timestamp(nowMs()); } + /** + * Returns the magic timestamp representing no specific time. + * + * <p>This "null object" is helpful in contexts where using {@code null} directly is not possible. + */ + @UsedAt(Project.PLUGIN_CHECKS) + public static Timestamp never() { + // Always create a new object as timestamps are mutable. + return new Timestamp(0); + } + public static Timestamp truncateToSecond(Timestamp t) { return new Timestamp((t.getTime() / 1000) * 1000); } diff --git a/java/com/google/gerrit/sshd/commands/LsUserRefs.java b/java/com/google/gerrit/sshd/commands/LsUserRefs.java index eb94044952..ae3d59e587 100644 --- a/java/com/google/gerrit/sshd/commands/LsUserRefs.java +++ b/java/com/google/gerrit/sshd/commands/LsUserRefs.java @@ -91,7 +91,7 @@ public class LsUserRefs extends SshCommand { try { Map<String, Ref> refsMap = permissionBackend - .user(user) + .user(ctx.getUser()) .project(projectName) .filter(repo.getRefDatabase().getRefs(), repo, RefFilterOptions.defaults()); diff --git a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java index c85aa9353e..f82dcd7ee9 100644 --- a/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java +++ b/javatests/com/google/gerrit/acceptance/git/AbstractPushForReview.java @@ -2250,6 +2250,34 @@ public abstract class AbstractPushForReview extends AbstractDaemonTest { @GerritConfig(name = "receive.maxBatchCommits", value = "2") @Test public void maxBatchCommits() throws Exception { + testMaxBatchCommits(); + } + + @GerritConfig(name = "receive.maxBatchCommits", value = "2") + @Test + public void maxBatchCommitsWithDefaultValidator() throws Exception { + TestValidator validator = new TestValidator(); + RegistrationHandle handle = commitValidators.add("test-validator", validator); + try { + testMaxBatchCommits(); + } finally { + handle.remove(); + } + } + + @GerritConfig(name = "receive.maxBatchCommits", value = "2") + @Test + public void maxBatchCommitsWithValidateAllCommitsValidator() throws Exception { + TestValidator validator = new TestValidator(true); + RegistrationHandle handle = commitValidators.add("test-validator", validator); + try { + testMaxBatchCommits(); + } finally { + handle.remove(); + } + } + + private void testMaxBatchCommits() throws Exception { List<RevCommit> commits = new ArrayList<>(); commits.addAll(initChanges(2)); String master = "refs/heads/master"; diff --git a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java b/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java index 392499210e..c7f3abee63 100644 --- a/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java +++ b/javatests/com/google/gerrit/acceptance/git/SubmitOnPushIT.java @@ -38,10 +38,12 @@ import com.google.gerrit.reviewdb.client.PatchSetApproval; import com.google.gerrit.reviewdb.client.Project; import com.google.gerrit.reviewdb.client.RefNames; import com.google.gerrit.server.ApprovalsUtil; +import com.google.gerrit.server.events.ChangeMergedEvent; import com.google.gerrit.server.notedb.ChangeNotes; import com.google.gerrit.server.query.change.ChangeData; import com.google.gerrit.testing.FakeEmailSender.Message; import com.google.inject.Inject; +import java.util.List; import org.eclipse.jgit.api.errors.GitAPIException; import org.eclipse.jgit.api.errors.InvalidRemoteException; import org.eclipse.jgit.api.errors.TransportException; @@ -202,6 +204,23 @@ public class SubmitOnPushIT extends AbstractDaemonTest { } @Test + public void correctNewRevOnMergeByPushToBranch() throws Exception { + projectOperations + .project(project) + .forUpdate() + .add(allow(Permission.PUSH).ref("refs/heads/master").group(adminGroupUuid())) + .update(); + push("refs/for/master", PushOneCommit.SUBJECT, "one.txt", "One"); + PushOneCommit.Result r = push("refs/for/master", PushOneCommit.SUBJECT, "two.txt", "Two"); + startEventRecorder(); + git().push().setRefSpecs(new RefSpec(r.getCommit().name() + ":refs/heads/master")).call(); + List<ChangeMergedEvent> changeMergedEvents = + eventRecorder.getChangeMergedEvents(project.get(), "refs/heads/master", 2); + assertThat(changeMergedEvents.get(0).newRev).isEqualTo(r.getPatchSet().commitId().name()); + assertThat(changeMergedEvents.get(1).newRev).isEqualTo(r.getPatchSet().commitId().name()); + } + + @Test public void mergeOnPushToBranchWithChangeMergedInOther() throws Exception { enableCreateNewChangeForAllNotInTarget(); String master = "refs/heads/master"; diff --git a/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java b/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java index 47d23a4fba..7d4b95e619 100644 --- a/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java +++ b/javatests/com/google/gerrit/acceptance/server/quota/DefaultQuotaBackendIT.java @@ -22,6 +22,8 @@ import static org.easymock.EasyMock.resetToStrict; import com.google.gerrit.acceptance.AbstractDaemonTest; import com.google.gerrit.extensions.annotations.Exports; +import com.google.gerrit.extensions.common.ChangeInfo; +import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.reviewdb.client.Change; import com.google.gerrit.server.IdentifiedUser; @@ -93,7 +95,7 @@ public class DefaultQuotaBackendIT extends AbstractDaemonTest { @Test public void requestTokenForUserAndChange() throws Exception { - Change.Id changeId = createChange().getChange().getId(); + Change.Id changeId = retrieveChangeId(); QuotaRequestContext ctx = QuotaRequestContext.builder() .user(identifiedAdmin) @@ -126,6 +128,58 @@ public class DefaultQuotaBackendIT extends AbstractDaemonTest { } @Test + public void availableTokensForUserAndAccount() { + QuotaRequestContext ctx = + QuotaRequestContext.builder().user(identifiedAdmin).account(user.id()).build(); + QuotaResponse r = QuotaResponse.ok(10L); + expect(quotaEnforcer.availableTokens("testGroup", ctx)).andReturn(r); + replay(quotaEnforcer); + assertThat(quotaBackend.user(identifiedAdmin).account(user.id()).availableTokens("testGroup")) + .isEqualTo(singletonAggregation(r)); + } + + @Test + public void availableTokensForUserAndProject() { + QuotaRequestContext ctx = + QuotaRequestContext.builder().user(identifiedAdmin).project(project).build(); + QuotaResponse r = QuotaResponse.ok(10L); + expect(quotaEnforcer.availableTokens("testGroup", ctx)).andReturn(r); + replay(quotaEnforcer); + assertThat(quotaBackend.user(identifiedAdmin).project(project).availableTokens("testGroup")) + .isEqualTo(singletonAggregation(r)); + } + + @Test + public void availableTokensForUserAndChange() throws Exception { + Change.Id changeId = retrieveChangeId(); + QuotaRequestContext ctx = + QuotaRequestContext.builder() + .user(identifiedAdmin) + .change(changeId) + .project(project) + .build(); + QuotaResponse r = QuotaResponse.ok(10L); + expect(quotaEnforcer.availableTokens("testGroup", ctx)).andReturn(r); + replay(quotaEnforcer); + assertThat( + quotaBackend + .user(identifiedAdmin) + .change(changeId, project) + .availableTokens("testGroup")) + .isEqualTo(singletonAggregation(r)); + } + + @Test + public void availableTokens() { + QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); + QuotaResponse r = QuotaResponse.ok(10L); + expect(quotaEnforcer.availableTokens("testGroup", ctx)).andReturn(r); + replay(quotaEnforcer); + assertThat(quotaBackend.user(identifiedAdmin).availableTokens("testGroup")) + .isEqualTo(singletonAggregation(r)); + } + + @Test public void requestTokenError() throws Exception { QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); expect(quotaEnforcer.requestTokens("testGroup", ctx, 1)) @@ -139,15 +193,47 @@ public class DefaultQuotaBackendIT extends AbstractDaemonTest { } @Test + public void availableTokensError() throws Exception { + QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); + expect(quotaEnforcer.availableTokens("testGroup", ctx)) + .andReturn(QuotaResponse.error("failed")); + replay(quotaEnforcer); + QuotaResponse.Aggregated result = + quotaBackend.user(identifiedAdmin).availableTokens("testGroup"); + assertThat(result).isEqualTo(singletonAggregation(QuotaResponse.error("failed"))); + QuotaException thrown = assertThrows(QuotaException.class, () -> result.throwOnError()); + assertThat(thrown).hasMessageThat().contains("failed"); + } + + @Test public void requestTokenPluginThrowsAndRethrows() { QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); expect(quotaEnforcer.requestTokens("testGroup", ctx, 1)).andThrow(new NullPointerException()); replay(quotaEnforcer); + assertThrows( NullPointerException.class, () -> quotaBackend.user(identifiedAdmin).requestToken("testGroup")); } + @Test + public void availableTokensPluginThrowsAndRethrows() { + QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); + expect(quotaEnforcer.availableTokens("testGroup", ctx)).andThrow(new NullPointerException()); + replay(quotaEnforcer); + + assertThrows( + NullPointerException.class, + () -> quotaBackend.user(identifiedAdmin).availableTokens("testGroup")); + } + + private Change.Id retrieveChangeId() throws Exception { + // use REST API so that repository size quota doesn't have to be stubbed + ChangeInfo changeInfo = + gApi.changes().create(new ChangeInput(project.get(), "master", "test")).get(); + return Change.id(changeInfo._number); + } + private static QuotaResponse.Aggregated singletonAggregation(QuotaResponse response) { return QuotaResponse.Aggregated.create(Collections.singleton(response)); } diff --git a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java index 1caaf005a1..4f47927c77 100644 --- a/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java +++ b/javatests/com/google/gerrit/acceptance/server/quota/MultipleQuotaPluginsIT.java @@ -33,6 +33,7 @@ import com.google.gerrit.server.quota.QuotaRequestContext; import com.google.gerrit.server.quota.QuotaResponse; import com.google.inject.Inject; import com.google.inject.Module; +import java.util.OptionalLong; import org.easymock.EasyMock; import org.junit.Before; import org.junit.Test; @@ -123,4 +124,34 @@ public class MultipleQuotaPluginsIT extends AbstractDaemonTest { QuotaResponse.Aggregated.create( ImmutableList.of(QuotaResponse.error("fail"), QuotaResponse.noOp()))); } + + @Test + public void minimumAvailableTokens() { + QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); + expect(quotaEnforcerA.availableTokens("testGroup", ctx)).andReturn(QuotaResponse.ok(20L)); + expect(quotaEnforcerB.availableTokens("testGroup", ctx)).andReturn(QuotaResponse.ok(10L)); + + replay(quotaEnforcerA); + replay(quotaEnforcerB); + + OptionalLong tokens = + quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens(); + assertThat(tokens.isPresent()).isTrue(); + assertThat(tokens.getAsLong()).isEqualTo(10L); + } + + @Test + public void ignoreNoOpForAvailableTokens() { + QuotaRequestContext ctx = QuotaRequestContext.builder().user(identifiedAdmin).build(); + expect(quotaEnforcerA.availableTokens("testGroup", ctx)).andReturn(QuotaResponse.noOp()); + expect(quotaEnforcerB.availableTokens("testGroup", ctx)).andReturn(QuotaResponse.ok(20L)); + + replay(quotaEnforcerA); + replay(quotaEnforcerB); + + OptionalLong tokens = + quotaBackend.user(identifiedAdmin).availableTokens("testGroup").availableTokens(); + assertThat(tokens.isPresent()).isTrue(); + assertThat(tokens.getAsLong()).isEqualTo(20L); + } } diff --git a/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java b/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java new file mode 100644 index 0000000000..05b3b83a22 --- /dev/null +++ b/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java @@ -0,0 +1,140 @@ +// Copyright (C) 2019 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.acceptance.server.quota; + +import static com.google.common.truth.Truth.assertThat; +import static com.google.common.truth.Truth.assertWithMessage; +import static com.google.gerrit.server.quota.QuotaGroupDefinitions.REPOSITORY_SIZE_GROUP; +import static com.google.gerrit.server.quota.QuotaResponse.ok; +import static org.easymock.EasyMock.anyLong; +import static org.easymock.EasyMock.eq; +import static org.easymock.EasyMock.expect; +import static org.easymock.EasyMock.replay; +import static org.easymock.EasyMock.resetToStrict; +import static org.easymock.EasyMock.verify; + +import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.acceptance.UseLocalDisk; +import com.google.gerrit.extensions.config.FactoryModule; +import com.google.gerrit.server.CurrentUser; +import com.google.gerrit.server.quota.QuotaBackend; +import com.google.gerrit.server.quota.QuotaResponse; +import com.google.inject.Module; +import java.util.Collections; +import org.easymock.EasyMock; +import org.eclipse.jgit.api.errors.TooLargeObjectInPackException; +import org.eclipse.jgit.api.errors.TransportException; +import org.junit.Before; +import org.junit.Test; + +@UseLocalDisk +public class RepositorySizeQuotaIT extends AbstractDaemonTest { + private static final QuotaBackend.WithResource quotaBackendWithResource = + EasyMock.createStrictMock(QuotaBackend.WithResource.class); + private static final QuotaBackend.WithUser quotaBackendWithUser = + EasyMock.createStrictMock(QuotaBackend.WithUser.class); + + @Override + public Module createModule() { + return new FactoryModule() { + @Override + public void configure() { + bind(QuotaBackend.class) + .toInstance( + new QuotaBackend() { + @Override + public WithUser currentUser() { + return quotaBackendWithUser; + } + + @Override + public WithUser user(CurrentUser user) { + return quotaBackendWithUser; + } + }); + } + }; + } + + @Before + public void setUp() { + resetToStrict(quotaBackendWithResource); + resetToStrict(quotaBackendWithUser); + } + + @Test + public void pushWithAvailableTokens() throws Exception { + expect(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP)) + .andReturn(singletonAggregation(ok(276L))) + .times(2); + expect(quotaBackendWithResource.requestTokens(eq(REPOSITORY_SIZE_GROUP), anyLong())) + .andReturn(singletonAggregation(ok())); + expect(quotaBackendWithUser.project(project)).andReturn(quotaBackendWithResource).anyTimes(); + replay(quotaBackendWithResource); + replay(quotaBackendWithUser); + pushCommit(); + verify(quotaBackendWithUser); + verify(quotaBackendWithResource); + } + + @Test + public void pushWithNotSufficientTokens() throws Exception { + long availableTokens = 1L; + expect(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP)) + .andReturn(singletonAggregation(ok(availableTokens))) + .anyTimes(); + expect(quotaBackendWithUser.project(project)).andReturn(quotaBackendWithResource).anyTimes(); + replay(quotaBackendWithResource); + replay(quotaBackendWithUser); + try { + pushCommit(); + assertWithMessage("expected TooLargeObjectInPackException").fail(); + } catch (TooLargeObjectInPackException e) { + String msg = e.getMessage(); + assertThat(msg).contains("Object too large"); + assertThat(msg) + .contains(String.format("Max object size limit is %d bytes.", availableTokens)); + } + verify(quotaBackendWithUser); + verify(quotaBackendWithResource); + } + + @Test + public void errorGettingAvailableTokens() throws Exception { + String msg = "quota error"; + expect(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP)) + .andReturn(singletonAggregation(QuotaResponse.error(msg))) + .anyTimes(); + expect(quotaBackendWithUser.project(project)).andReturn(quotaBackendWithResource).anyTimes(); + replay(quotaBackendWithResource); + replay(quotaBackendWithUser); + try { + pushCommit(); + assertWithMessage("expected TransportException").fail(); + } catch (TransportException e) { + // TransportException has not much info about the cause + } + verify(quotaBackendWithUser); + verify(quotaBackendWithResource); + } + + private void pushCommit() throws Exception { + createCommitAndPush(testRepo, "refs/heads/master", "test 01", "file.test", "some content"); + } + + private static QuotaResponse.Aggregated singletonAggregation(QuotaResponse response) { + return QuotaResponse.Aggregated.create(Collections.singleton(response)); + } +} diff --git a/javatests/com/google/gerrit/acceptance/server/quota/RestApiQuotaIT.java b/javatests/com/google/gerrit/acceptance/server/quota/RestApiQuotaIT.java index a07569044d..802f55f1db 100644 --- a/javatests/com/google/gerrit/acceptance/server/quota/RestApiQuotaIT.java +++ b/javatests/com/google/gerrit/acceptance/server/quota/RestApiQuotaIT.java @@ -20,6 +20,7 @@ import static org.easymock.EasyMock.reset; import static org.easymock.EasyMock.verify; import com.google.gerrit.acceptance.AbstractDaemonTest; +import com.google.gerrit.extensions.common.ChangeInfo; import com.google.gerrit.extensions.common.ChangeInput; import com.google.gerrit.extensions.config.FactoryModule; import com.google.gerrit.reviewdb.client.Change; @@ -68,7 +69,7 @@ public class RestApiQuotaIT extends AbstractDaemonTest { @Test public void changeDetail() throws Exception { - Change.Id changeId = createChange().getChange().getId(); + Change.Id changeId = retrieveChangeId(); expect(quotaBackendWithResource.requestToken("/restapi/changes/detail:GET")) .andReturn(singletonAggregation(QuotaResponse.ok())); replay(quotaBackendWithResource); @@ -81,7 +82,7 @@ public class RestApiQuotaIT extends AbstractDaemonTest { @Test public void revisionDetail() throws Exception { - Change.Id changeId = createChange().getChange().getId(); + Change.Id changeId = retrieveChangeId(); expect(quotaBackendWithResource.requestToken("/restapi/changes/revisions/actions:GET")) .andReturn(singletonAggregation(QuotaResponse.ok())); replay(quotaBackendWithResource); @@ -130,6 +131,13 @@ public class RestApiQuotaIT extends AbstractDaemonTest { adminRestSession.get("/config/server/version").assertStatus(429); } + private Change.Id retrieveChangeId() throws Exception { + // use REST API so that repository size quota doesn't have to be stubbed + ChangeInfo changeInfo = + gApi.changes().create(new ChangeInput(project.get(), "master", "test")).get(); + return Change.id(changeInfo._number); + } + private static QuotaResponse.Aggregated singletonAggregation(QuotaResponse response) { return QuotaResponse.Aggregated.create(Collections.singleton(response)); } diff --git a/javatests/com/google/gerrit/json/BUILD b/javatests/com/google/gerrit/json/BUILD index 2d95652632..575f575bd2 100644 --- a/javatests/com/google/gerrit/json/BUILD +++ b/javatests/com/google/gerrit/json/BUILD @@ -5,7 +5,9 @@ junit_tests( srcs = glob(["*.java"]), deps = [ "//java/com/google/gerrit/json", + "//java/com/google/gerrit/server/util/time", "//java/com/google/gerrit/testing:gerrit-test-util", + "//lib:gson", "//lib:guava", "//lib/truth", ], diff --git a/javatests/com/google/gerrit/json/SqlTimestampDeserializerTest.java b/javatests/com/google/gerrit/json/SqlTimestampDeserializerTest.java new file mode 100644 index 0000000000..2699c3b793 --- /dev/null +++ b/javatests/com/google/gerrit/json/SqlTimestampDeserializerTest.java @@ -0,0 +1,33 @@ +// Copyright (C) 2019 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.json; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.gerrit.server.util.time.TimeUtil; +import com.google.gson.JsonPrimitive; +import java.sql.Timestamp; +import org.junit.Test; + +public class SqlTimestampDeserializerTest { + + private final SqlTimestampDeserializer deserializer = new SqlTimestampDeserializer(); + + @Test + public void emptyStringIsDeserializedToMagicTimestamp() { + Timestamp timestamp = deserializer.deserialize(new JsonPrimitive(""), Timestamp.class, null); + assertThat(timestamp).isEqualTo(TimeUtil.never()); + } +} diff --git a/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor.html b/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor.html index c21e84d5a8..5dbafae261 100644 --- a/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor.html +++ b/polygerrit-ui/app/elements/settings/gr-gpg-editor/gr-gpg-editor.html @@ -27,9 +27,6 @@ limitations under the License. <template> <style include="shared-styles"></style> <style include="gr-form-styles"> - .statusHeader { - width: 4em; - } .keyHeader { width: 9em; } @@ -54,10 +51,6 @@ limitations under the License. #existing { margin-bottom: 1em; } - #existing .commentColumn { - min-width: 27em; - width: auto; - } </style> <div class="gr-form-styles"> <fieldset id="existing"> |