From c99414ab4bed8b028d59b355d329d57b59bc6588 Mon Sep 17 00:00:00 2001
From: Jacek Centkowski <geminica.programs@gmail.com>
Date: Tue, 2 Mar 2021 14:45:41 +0100
Subject: [PATCH] Enforce repository size on pack rather than on object

Issue:
Originally max repository size was enforced by the quota plugin.
However, that functionality was a perfect candidate for core quota
implementation and as such was implemented in [1].

As a result, Gerrit core is responsible for making sure that the limit
is enforced and the plugin is responsible for providing the limit.
Unfortunately, Gerrit core implementation developed a fault as it
enforced the limit on the object size instead of the pack size leading
to the situation where multiple smaller objects would fall under the
limit where the pack they belong to should already exhaust it.

Solution:
Enforce available repository size tokens on pack size (as it was
done this way originally in plugin).

[1] https://gerrit-review.googlesource.com/c/gerrit/+/224728

Bug: issue 14118
Change-Id: I37e573971a76abe53a3e6456498940a4bb803d0c
---
 .../google/gerrit/acceptance/InProcessProtocol.java   |  2 +-
 .../server/git/receive/AsyncReceiveCommits.java       |  2 +-
 .../server/quota/RepositorySizeQuotaIT.java           | 11 ++++++-----
 3 files changed, 8 insertions(+), 7 deletions(-)

diff --git a/java/com/google/gerrit/acceptance/InProcessProtocol.java b/java/com/google/gerrit/acceptance/InProcessProtocol.java
index 69715a9cf8..83ed2020ee 100644
--- a/java/com/google/gerrit/acceptance/InProcessProtocol.java
+++ b/java/com/google/gerrit/acceptance/InProcessProtocol.java
@@ -337,7 +337,7 @@ class InProcessProtocol extends TestProtocol<Context> {
                 .project(req.project)
                 .availableTokens(REPOSITORY_SIZE_GROUP);
         availableTokens.throwOnError();
-        availableTokens.availableTokens().ifPresent(v -> rp.setMaxObjectSizeLimit(v));
+        availableTokens.availableTokens().ifPresent(rp::setMaxPackSizeLimit);
 
         ImmutableList<PostReceiveHook> hooks =
             ImmutableList.<PostReceiveHook>builder()
diff --git a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
index da2887f907..f49fbfdcff 100644
--- a/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
+++ b/java/com/google/gerrit/server/git/receive/AsyncReceiveCommits.java
@@ -327,7 +327,7 @@ public class AsyncReceiveCommits implements PreReceiveHook {
           REPOSITORY_SIZE_GROUP, projectName);
       throw new RuntimeException(e);
     }
-    availableTokens.availableTokens().ifPresent(v -> receivePack.setMaxObjectSizeLimit(v));
+    availableTokens.availableTokens().ifPresent(receivePack::setMaxPackSizeLimit);
   }
 
   /** Determine if the user can upload commits. */
diff --git a/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java b/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
index 0814230e30..018b4e608c 100644
--- a/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
+++ b/javatests/com/google/gerrit/acceptance/server/quota/RepositorySizeQuotaIT.java
@@ -34,7 +34,7 @@ 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.TooLargePackException;
 import org.eclipse.jgit.api.errors.TransportException;
 import org.junit.Before;
 import org.junit.Test;
@@ -77,7 +77,7 @@ public class RepositorySizeQuotaIT extends AbstractDaemonTest {
   @Test
   public void pushWithAvailableTokens() throws Exception {
     expect(quotaBackendWithResource.availableTokens(REPOSITORY_SIZE_GROUP))
-        .andReturn(singletonAggregation(ok(276L)))
+        .andReturn(singletonAggregation(ok(277L)))
         .times(2);
     expect(quotaBackendWithResource.requestTokens(eq(REPOSITORY_SIZE_GROUP), anyLong()))
         .andReturn(singletonAggregation(ok()));
@@ -101,11 +101,12 @@ public class RepositorySizeQuotaIT extends AbstractDaemonTest {
     try {
       pushCommit();
       assert_().fail("expected TooLargeObjectInPackException");
-    } catch (TooLargeObjectInPackException e) {
+    } catch (TooLargePackException e) {
       String msg = e.getMessage();
-      assertThat(msg).contains("Object too large");
       assertThat(msg)
-          .contains(String.format("Max object size limit is %d bytes.", availableTokens));
+          .contains(
+              String.format(
+                  "Pack exceeds the limit of %d bytes, rejecting the pack", availableTokens));
     }
     verify(quotaBackendWithUser);
     verify(quotaBackendWithResource);