Incorporate review feedback56883-add-to-whitelist

- Change parameter that ApplicationSetting::UpdateService accepts, from
outbound_local_requests_whitelist to
add_to_outbound_local_requests_whitelist to differentiate it better
from outbound_local_requests_whitelist_raw which overwrites the existing
ApplicationSetting.outbound_local_requests_whitelist.

4 files changed, 26 insertions, 11 deletions

diff --git a/app/models/application_setting_implementation.rb b/app/models/application_setting_implementation.rb
index 54b2cf65e92..67af50aa188 100644
--- a/app/models/application_setting_implementation.rb
+++ b/app/models/application_setting_implementation.rb
@@ -163,10 +163,7 @@ module ApplicationSettingImplementation
     self.outbound_local_requests_whitelist = domain_strings_to_array(values)
   end
 
-  def add_to_outbound_local_requests_whitelist(values)
-    values_array = Array(values).reject(&:empty?)
-    return if values_array.empty?
-
+  def add_to_outbound_local_requests_whitelist(values_array)
     clear_memoization(:outbound_local_requests_whitelist_arrays)
 
     self.outbound_local_requests_whitelist ||= []
diff --git a/app/services/application_settings/update_service.rb b/app/services/application_settings/update_service.rb
index 5fd6f8f531f..471df6e2d0c 100644
--- a/app/services/application_settings/update_service.rb
+++ b/app/services/application_settings/update_service.rb
@@ -15,7 +15,7 @@ module ApplicationSettings
 
       update_terms(@params.delete(:terms))
 
-      add_to_outbound_local_requests_whitelist(@params.delete(:outbound_local_requests_whitelist))
+      add_to_outbound_local_requests_whitelist(@params.delete(:add_to_outbound_local_requests_whitelist))
 
       if params.key?(:performance_bar_allowed_group_path)
         params[:performance_bar_allowed_group_id] = performance_bar_allowed_group_id
@@ -35,7 +35,10 @@ module ApplicationSettings
     end
 
     def add_to_outbound_local_requests_whitelist(values)
-      @application_setting.add_to_outbound_local_requests_whitelist(values)
+      values_array = Array(values).reject(&:empty?)
+      return if values_array.empty?
+
+      @application_setting.add_to_outbound_local_requests_whitelist(values_array)
     end
 
     def update_terms(terms)
diff --git a/spec/services/application_settings/update_service_spec.rb b/spec/services/application_settings/update_service_spec.rb
index 85fd4a353a3..adb5219d691 100644
--- a/spec/services/application_settings/update_service_spec.rb
+++ b/spec/services/application_settings/update_service_spec.rb
@@ -63,7 +63,7 @@ describe ApplicationSettings::UpdateService do
   end
 
   describe 'updating outbound_local_requests_whitelist' do
-    context 'when param outbound_local_requests_whitelist is blank' do
+    context 'when params is blank' do
       let(:params) { {} }
 
       it 'does not add to whitelist' do
@@ -73,12 +73,12 @@ describe ApplicationSettings::UpdateService do
       end
     end
 
-    context 'when param outbound_local_requests_whitelist contains values' do
+    context 'when param add_to_outbound_local_requests_whitelist contains values' do
       before do
         application_settings.outbound_local_requests_whitelist = ['127.0.0.1']
       end
 
-      let(:params) { { outbound_local_requests_whitelist: ['example.com'] } }
+      let(:params) { { add_to_outbound_local_requests_whitelist: ['example.com', ''] } }
 
       it 'adds to whitelist' do
         expect { subject.execute }.to change {
diff --git a/spec/support/shared_examples/application_setting_examples.rb b/spec/support/shared_examples/application_setting_examples.rb
index ed3b222ca7d..fc0a645ee3c 100644
--- a/spec/support/shared_examples/application_setting_examples.rb
+++ b/spec/support/shared_examples/application_setting_examples.rb
@@ -61,6 +61,7 @@ RSpec.shared_examples 'application settings examples' do
 
     it 'clears outbound_local_requests_whitelist_arrays memoization' do
       setting.outbound_local_requests_whitelist_raw = 'example.com'
+
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
         [], ['example.com']
       )
@@ -96,7 +97,7 @@ RSpec.shared_examples 'application settings examples' do
       setting.outbound_local_requests_whitelist = ['example.com']
 
       setting.add_to_outbound_local_requests_whitelist(
-        ['example.com', '127.0.0.1', 'gitlab.com', '']
+        ['example.com', '127.0.0.1', 'gitlab.com']
       )
 
       expect(setting.outbound_local_requests_whitelist).to contain_exactly(
@@ -104,8 +105,22 @@ RSpec.shared_examples 'application settings examples' do
         '127.0.0.1',
         'gitlab.com'
       )
+    end
+
+    it 'clears outbound_local_requests_whitelist_arrays memoization' do
+      setting.outbound_local_requests_whitelist = ['example.com']
+
+      expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
+        [],
+        ['example.com']
+      )
+
+      setting.add_to_outbound_local_requests_whitelist(
+        ['example.com', 'gitlab.com']
+      )
+
       expect(setting.outbound_local_requests_whitelist_arrays).to contain_exactly(
-        [Gitlab::Utils.string_to_ip_object('127.0.0.1')],
+        [],
         ['example.com', 'gitlab.com']
       )
     end