Merge branch '1583-fix-wrong-exceeding-value-when-push-is-rejected' into 'master'

Show correct exceeding limit value on LFS push when it's rejected

Closes #1583

See merge request !1105

app/controllers/concerns/lfs_request.rb

@@ -122,7 +122,7 @@ module LfsRequest
     def render_size_error
       render(
         json: {
-          message: Gitlab::RepositorySizeError.new(project).push_error,
+          message: Gitlab::RepositorySizeError.new(project).push_error(@exceeded_limit),
           documentation_url: help_url,
         },
         content_type: "application/vnd.git-lfs+json",
@@ -134,9 +134,11 @@ module LfsRequest
       return false unless project.size_limit_enabled?
       return @limit_exceeded if defined?(@limit_exceeded)
 
-      size_of_objects = objects.sum { |o| o[:size] }
+      lfs_push_size = objects.sum { |o| o[:size] }
+      size_with_lfs_push = project.repository_and_lfs_size + lfs_push_size
 
-      @limit_exceeded = (project.repository_and_lfs_size + size_of_objects) > project.actual_size_limit
+      @exceeded_limit = size_with_lfs_push - project.actual_size_limit
+      @limit_exceeded = @exceeded_limit > 0
     end
   end
 

lib/gitlab/repository_size_error.rb

@@ -20,8 +20,8 @@ module Gitlab
       "This merge request cannot be merged, #{base_message}"
     end
 
-    def push_error
-      "Your push has been rejected, #{base_message}. #{more_info_message}"
+    def push_error(exceeded_limit = nil)
+      "Your push has been rejected, #{base_message(exceeded_limit)}. #{more_info_message}"
     end
 
     def new_changes_error
@@ -38,8 +38,8 @@ module Gitlab
 
     private
 
-    def base_message
-      "because this repository has exceeded its size limit of #{limit} by #{size_to_remove}"
+    def base_message(exceeded_limit = nil)
+      "because this repository has exceeded its size limit of #{limit} by #{size_to_remove(exceeded_limit)}"
     end
 
     def current_size
@@ -50,8 +50,8 @@ module Gitlab
       format_number(project.actual_size_limit)
     end
 
-    def size_to_remove
-      format_number(project.size_to_remove)
+    def size_to_remove(exceeded_limit = nil)
+      format_number(exceeded_limit || project.size_to_remove)
     end
 
     def format_number(number)

spec/lib/gitlab/repository_size_error_spec.rb

@@ -32,8 +32,22 @@ describe Gitlab::RepositorySizeError, lib: true do
     end
 
     describe '#push_error' do
-      it 'returns the correct message' do
-        expect(message.push_error).to eq("Your push has been rejected, #{base_message}. #{message.more_info_message}")
+      context 'with exceeded_limit value' do
+        let(:rejection_message) do
+          'because this repository has exceeded its size limit of 10 MB by 15 MB'
+        end
+
+        it 'returns the correct message' do
+          expect(message.push_error(15.megabytes))
+            .to eq("Your push has been rejected, #{rejection_message}. #{message.more_info_message}")
+        end
+      end
+
+      context 'without exceeded_limit value' do
+        it 'returns the correct message' do
+          expect(message.push_error)
+            .to eq("Your push has been rejected, #{base_message}. #{message.more_info_message}")
+        end
       end
     end
 

spec/requests/lfs_http_spec.rb

@@ -663,11 +663,14 @@ describe 'Git LFS API and storage' do
           end
 
           context 'when pushing a lfs object that does not exist' do
+            let(:sample_size) { 150.megabytes }
+            let(:sample_oid) { '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897' }
+
             let(:body) do
               { 'operation' => 'upload',
                 'objects' => [
-                  { 'oid' => '91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897',
-                    'size' => 157507855
+                  { 'oid' => sample_oid,
+                    'size' => sample_size
                   }]
               }
             end
@@ -678,31 +681,43 @@ describe 'Git LFS API and storage' do
 
             it 'responds with upload hypermedia link' do
               expect(json_response['objects']).to be_kind_of(Array)
-              expect(json_response['objects'].first['oid']).to eq("91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897")
-              expect(json_response['objects'].first['size']).to eq(157507855)
-              expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/91eff75a492a3ed0dfcb544d7f31326bc4014c8551849c192fd1e48d4dd2c897/157507855")
+              expect(json_response['objects'].first['oid']).to eq(sample_oid)
+              expect(json_response['objects'].first['size']).to eq(sample_size)
+              expect(json_response['objects'].first['actions']['upload']['href']).to eq("#{Gitlab.config.gitlab.url}/#{project.path_with_namespace}.git/gitlab-lfs/objects/#{sample_oid}/#{sample_size}")
               expect(json_response['objects'].first['actions']['upload']['header']).to eq('Authorization' => authorization)
             end
 
             context 'and project is above the limit' do
               let(:update_lfs_permissions) do
-                allow_any_instance_of(Project).to receive(:above_size_limit?).and_return(true)
+                allow_any_instance_of(Project).to receive_messages(
+                  repository_and_lfs_size: 100.megabytes,
+                  actual_size_limit: 99.megabytes)
               end
 
               it 'responds with status 406' do
                 expect(response).to have_http_status(406)
               end
+
+              it 'show correct error message' do
+                expect(json_response['message']).to eql('Your push has been rejected, because this repository has exceeded its size limit of 99 MB by 1 MB. Please contact your GitLab administrator for more information.')
+              end
             end
 
             context 'and project will go over the limit' do
               let(:update_lfs_permissions) do
-                allow_any_instance_of(Project).to receive_messages(actual_size_limit: 145, size_limit_enabled?: true)
+                allow_any_instance_of(Project).to receive_messages(
+                  repository_and_lfs_size: 200.megabytes,
+                  actual_size_limit: 300.megabytes)
               end
 
               it 'responds with status 406' do
                 expect(response).to have_http_status(406)
                 expect(json_response['documentation_url']).to include('/help')
               end
+
+              it 'show correct error message' do
+                expect(json_response['message']).to eql('Your push has been rejected, because this repository has exceeded its size limit of 300 MB by 50 MB. Please contact your GitLab administrator for more information.')
+              end
             end
           end