Merge branch '271574_fix_exceeded_size' into 'master'

Avoid using the current's project excess twice

See merge request gitlab-org/gitlab!46017

ee/lib/ee/gitlab/repository_size_checker.rb

@@ -17,9 +17,10 @@ module EE
       override :exceeded_size
       # @param change_size [int] in bytes
       def exceeded_size(change_size = 0)
-        exceeded_size = super
-        exceeded_size -= remaining_additional_purchased_storage if additional_repo_storage_available?
-        exceeded_size
+        size = super
+        size -= remaining_additional_purchased_storage if additional_repo_storage_available?
+
+        [size, 0].max
       end
 
       private
@@ -38,8 +39,16 @@ module EE
         namespace&.additional_purchased_storage_size&.megabytes.to_i
       end
 
+      def current_project_excess
+        [current_size - limit, 0].max
+      end
+
+      def total_excess_without_current_project
+        total_repository_size_excess - current_project_excess
+      end
+
       def remaining_additional_purchased_storage
-        additional_purchased_storage - total_repository_size_excess
+        additional_purchased_storage - total_excess_without_current_project
       end
     end
   end

ee/spec/lib/ee/gitlab/repository_size_checker_spec.rb

@@ -136,41 +136,69 @@ RSpec.describe Gitlab::RepositorySizeChecker do
         stub_feature_flags(namespace_storage_limit: false)
       end
 
-      context 'when current size + total repository size excess are below or equal to the limit + additional purchased storage' do
-        let(:current_size) { 50 }
+      context 'with additional purchased storage' do
         let(:total_repository_size_excess) { 10 }
         let(:additional_purchased_storage) { 10 }
 
-        it 'returns zero' do
-          expect(subject.exceeded_size).to eq(0)
-        end
-      end
+        context 'when no change size provided' do
+          context 'when current size + total repository size excess is below the limit (additional purchase storage not used)' do
+            let(:current_size) { limit - 1 }
 
-      context 'when current size + total repository size excess are over the limit + additional purchased storage' do
-        let(:current_size) { 51 }
-        let(:total_repository_size_excess) { 10 }
-        let(:additional_purchased_storage) { 10 }
+            it 'returns zero' do
+              expect(subject.exceeded_size).to eq(0)
+            end
+          end
 
-        it 'returns 1' do
-          expect(subject.exceeded_size).to eq(1.megabytes)
-        end
-      end
+          context 'when current size + total repository size excess is equal to the limit (additional purchase storage not used)' do
+            let(:current_size) { limit }
+
+            it 'returns zero' do
+              expect(subject.exceeded_size).to eq(0)
+            end
+          end
+
+          context 'when there is remaining additional purchased storage (current size + other project excess use some additional purchased storage)' do
+            let(:current_size) { limit + 1 }
 
-      context 'when change size will be over the limit' do
-        let(:current_size) { 50 }
+            it 'returns zero' do
+              expect(subject.exceeded_size).to eq(0)
+            end
+          end
 
-        it 'returns 1' do
-          expect(subject.exceeded_size(1.megabytes)).to eq(1.megabytes)
+          context 'when additional purchased storage is depleted (current size + other project excess exceed additional purchased storage)' do
+            let(:total_repository_size_excess) { 15 }
+            let(:current_size) { 61 }
+
+            it 'returns a positive number' do
+              expect(subject.exceeded_size).to eq(5.megabytes)
+            end
+          end
         end
-      end
 
-      context 'when change size will not be over the limit' do
-        let(:current_size) { 49 }
+        context 'when a change size is provided' do
+          let(:change_size) { 1.megabyte }
+
+          context 'when current size + total repository size excess is below the limit (additional purchase storage not used)' do
+            let(:current_size) { limit - 1 }
+
+            it 'returns zero' do
+              expect(subject.exceeded_size(change_size)).to eq(0)
+            end
+          end
 
-        it 'returns zero' do
-          expect(subject.exceeded_size(1.megabytes)).to eq(0)
+          context 'when current size + total repository size excess is equal to the limit (additional purchase storage depleted)' do
+            let(:current_size) { limit }
+
+            it 'returns a positive number' do
+              expect(subject.exceeded_size(change_size)).to eq(1.megabyte)
+            end
+          end
         end
       end
+
+      context 'without additional purchased storage' do
+        include_examples 'checker size exceeded'
+      end
     end
 
     context 'with feature flag :additional_repo_storage_by_namespace disabled' do

lib/gitlab/repository_size_checker.rb

@@ -37,7 +37,9 @@ module Gitlab
 
     # @param change_size [int] in bytes
     def exceeded_size(change_size = 0)
-      current_size + change_size - limit
+      size = current_size + change_size - limit
+
+      [size, 0].max
     end
 
     def error_message

spec/support/shared_examples/lib/gitlab/repository_size_checker_shared_examples.rb

@@ -17,35 +17,58 @@ RSpec.shared_examples 'checker size not over limit' do
 end
 
 RSpec.shared_examples 'checker size exceeded' do
-  context 'when current size is below or equal to the limit' do
-    let(:current_size) { 50 }
+  context 'when no change size provided' do
+    context 'when current size is below the limit' do
+      let(:current_size) { limit - 1 }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size).to eq(0)
+      it 'returns zero' do
+        expect(subject.exceeded_size).to eq(0)
+      end
     end
-  end
 
-  context 'when current size is over the limit' do
-    let(:current_size) { 51 }
+    context 'when current size is equal to the limit' do
+      let(:current_size) { limit }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size).to eq(1.megabytes)
+      it 'returns zero' do
+        expect(subject.exceeded_size).to eq(0)
+      end
     end
-  end
 
-  context 'when change size will be over the limit' do
-    let(:current_size) { 50 }
+    context 'when current size is over the limit' do
+      let(:current_size) { limit + 1 }
+      let(:total_repository_size_excess) { 1 }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size(1.megabytes)).to eq(1.megabytes)
+      it 'returns a positive number' do
+        expect(subject.exceeded_size).to eq(1.megabyte)
+      end
     end
   end
 
-  context 'when change size will not be over the limit' do
-    let(:current_size) { 49 }
+  context 'when a change size is provided' do
+    let(:change_size) { 1.megabyte }
+
+    context 'when change size will be over the limit' do
+      let(:current_size) { limit }
+
+      it 'returns a positive number' do
+        expect(subject.exceeded_size(change_size)).to eq(1.megabyte)
+      end
+    end
+
+    context 'when change size will be at the limit' do
+      let(:current_size) { limit - 1 }
+
+      it 'returns zero' do
+        expect(subject.exceeded_size(change_size)).to eq(0)
+      end
+    end
+
+    context 'when change size will be under the limit' do
+      let(:current_size) { limit - 2 }
 
-    it 'returns zero' do
-      expect(subject.exceeded_size(1.megabytes)).to eq(0)
+      it 'returns zero' do
+        expect(subject.exceeded_size(change_size)).to eq(0)
+      end
     end
   end
 end