Correctly convert repository_size_limit if it is present

Ensure empty string & non-numeric values also result in an error.

app/models/application_setting.rb

@@ -262,7 +262,8 @@ class ApplicationSetting < ActiveRecord::Base
       elasticsearch_aws: false,
       elasticsearch_aws_region: ENV['ELASTIC_REGION'] || 'us-east-1',
       usage_ping_enabled: true,
-      minimum_mirror_sync_time: Gitlab::Mirror::FIFTEEN
+      minimum_mirror_sync_time: Gitlab::Mirror::FIFTEEN,
+      repository_size_limit: 0
     }
   end
 

app/services/application_settings/update_service.rb

@@ -3,7 +3,7 @@ module ApplicationSettings
     def execute
       # Repository size limit comes as MB from the view
       limit = @params.delete(:repository_size_limit)
-      @application_setting.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      @application_setting.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       @application_setting.update(@params)
     end

app/services/groups/create_service.rb

@@ -15,7 +15,7 @@ module Groups
 
       # Repository size limit comes as MB from the view
       limit = params.delete(:repository_size_limit)
-      @group.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      @group.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       if @group.parent && !can?(current_user, :admin_group, @group.parent)
         @group.parent = nil

app/services/groups/update_service.rb

@@ -14,7 +14,7 @@ module Groups
 
       # Repository size limit comes as MB from the view
       limit = @params.delete(:repository_size_limit)
-      group.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      group.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       group.assign_attributes(params)
 

app/services/projects/create_service.rb

@@ -147,7 +147,7 @@ module Projects
 
     def set_repository_size_limit_as_bytes
       limit = params.delete(:repository_size_limit)
-      @project.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      @project.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
     end
 
     def set_project_name_from_path

app/services/projects/update_service.rb

@@ -15,7 +15,7 @@ module Projects
 
       # Repository size limit comes as MB from the view
       limit = params.delete(:repository_size_limit)
-      project.repository_size_limit = (limit.to_i.megabytes if limit.present?)
+      project.repository_size_limit = Gitlab::Utils.try_megabytes_to_bytes(limit) if limit
 
       new_branch = params.delete(:default_branch)
       new_repository_storage = params.delete(:repository_storage)

lib/gitlab/utils.rb

@@ -21,5 +21,11 @@ module Gitlab
 
       nil
     end
+
+    def try_megabytes_to_bytes(size)
+      Integer(size).megabytes
+    rescue ArgumentError
+      size
+    end
   end
 end

spec/controllers/admin/application_settings_controller_spec.rb

@@ -37,28 +37,32 @@ describe Admin::ApplicationSettingsController do
       expect(ApplicationSetting.current.restricted_visibility_levels).to be_empty
     end
 
-    context 'with valid repository_size_limit' do
-      subject { put :update, application_setting: { repository_size_limit: '100' } }
+    it 'updates repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '100' }
 
-      it 'redirect to application settings page' do
-        is_expected.to redirect_to(admin_application_settings_path)
-      end
+      expect(response).to redirect_to(admin_application_settings_path)
+      expect(response).to set_flash[:notice].to('Application settings saved successfully')
+    end
+
+    it 'does not accept negative repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '-100' }
 
-      it 'set flash notice' do
-        is_expected.to set_flash[:notice].to('Application settings saved successfully')
-      end
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
     end
 
-    context 'with invalid repository_size_limit' do
-      subject! { put :update, application_setting: { repository_size_limit: '-100' } }
+    it 'does not accept invalid repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: 'one thousand' }
+
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
+    end
 
-      it 'render show template' do
-        is_expected.to render_template(:show)
-      end
+    it 'does not accept empty repository_size_limit' do
+      put :update, application_setting: { repository_size_limit: '' }
 
-      it 'assigned @application_settings has errors' do
-        expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
-      end
+      expect(response).to render_template(:show)
+      expect(assigns(:application_setting).errors[:repository_size_limit]).to be_present
     end
   end