Merge pull request #126 from 3scale/fix/copy-service-self-managed-on-…

…prem

Fix copying self managed service to on prem where invalid

lib/3scale_toolbox/entities/service.rb

@@ -10,17 +10,34 @@ class Service
 
       class << self
         def create(remote:, service:, system_name:)
-          svc_obj = remote.create_service copy_service_params(service, system_name)
-          unless svc_obj['errors'].nil?
-            raise ThreeScaleToolbox::Error, 'Service has not been saved. ' \
-              "Errors: #{svc_obj['errors']}"
+          svc_obj = create_service(
+            remote: remote,
+            service: copy_service_params(service, system_name)
+          )
+          if (errors = svc_obj['errors'])
+            raise ThreeScaleToolbox::Error, "Service has not been saved. Errors: #{errors}" \
           end
 
           new(id: svc_obj.fetch('id'), remote: remote)
         end
 
         private
 
+        def create_service(remote:, service:)
+          svc_obj = remote.create_service service
+
+          # Source and target remotes might not allow same set of deployment options
+          # Invalid deployment option check
+          # use default deployment_option
+          if (errors = svc_obj['errors']) &&
+             ThreeScaleToolbox::Helper.service_invalid_deployment_option?(errors)
+            service.delete('deployment_option')
+            svc_obj = remote.create_service(service)
+          end
+
+          svc_obj
+        end
+
         def copy_service_params(original, system_name)
           service_params = Helper.filter_params(VALID_PARAMS, original)
           service_params.tap do |hash|

lib/3scale_toolbox/helper.rb

@@ -32,6 +32,12 @@ def parse_uri(uri)
 
         uri_obj
       end
+
+      def service_invalid_deployment_option?(error)
+        Array(Hash(error)['deployment_option']).any? do |msg|
+          msg.match(/is not included in the list/)
+        end
+      end
     end
   end
 end

lib/3scale_toolbox/tasks/update_service_settings_task.rb

@@ -11,14 +11,31 @@ def initialize(source:, target:, target_name:)
 
       def call
         source_obj = source.show_service
-        response = target.update_service(target_service_params(source_obj))
-        raise ThreeScaleToolbox::Error, "Service has not been saved. Errors: #{response['errors']}" unless response['errors'].nil?
+        svc_obj = update_service target_service_params(source_obj)
+        if (errors = svc_obj['errors'])
+          raise ThreeScaleToolbox::Error, "Service has not been saved. Errors: #{errors}" \
+        end
 
         puts "updated service settings for service id #{source.id}..."
       end
 
       private
 
+      def update_service(service)
+        svc_obj = target.update_service service
+
+        # Source and target remotes might not allow same set of deployment options
+        # Invalid deployment option check
+        # use default deployment_option
+        if (errors = svc_obj['errors']) &&
+           ThreeScaleToolbox::Helper.service_invalid_deployment_option?(errors)
+          service.delete('deployment_option')
+          svc_obj = target.update_service(service)
+        end
+
+        svc_obj
+      end
+
       # system name only included when specified from options
       def target_service_params(source)
         target_svc_obj = ThreeScaleToolbox::Helper.filter_params(Entities::Service::VALID_PARAMS, source)

spec/unit/entities/service_spec.rb

@@ -3,22 +3,65 @@
 RSpec.describe ThreeScaleToolbox::Entities::Service do
   include_context :random_name
   let(:remote) { double('remote') }
+  let(:common_error_response) { { 'errors' => { 'comp' => 'error' } } }
+  let(:positive_response) { { 'errors' => nil, 'id' => 'some_id' } }
 
   context 'Service.create' do
     let(:system_name) { random_lowercase_name }
-    let(:service) { { 'name' => random_lowercase_name } }
+    let(:deployment_option) { 'hosted' }
+    let(:service) do
+      {
+        'name' => random_lowercase_name,
+        'deployment_option' => deployment_option
+      }
+    end
     let(:service_info) { { remote: remote, service: service, system_name: system_name } }
-    let(:expected_svc) { { 'name' => service['name'], 'system_name' => system_name } }
 
     it 'throws error on remote error' do
-      expect(remote).to receive(:create_service).with(expected_svc).and_return('errors' => true)
+      expect(remote).to receive(:create_service).and_return(common_error_response)
+      expect do
+        described_class.create(service_info)
+      end.to raise_error(ThreeScaleToolbox::Error, /Service has not been saved/)
+    end
+
+    context 'deployment mode invalid' do
+      let(:invalid_deployment_error_response) do
+        {
+          'errors' => {
+            'deployment_option' => ['is not included in the list']
+          }
+        }
+      end
+
+      it 'deployment config is removed' do
+        expect(remote).to receive(:create_service).with(hash_including('deployment_option'))
+                                                  .and_return(invalid_deployment_error_response)
+        expect(remote).to receive(:create_service).with(hash_excluding('deployment_option'))
+                                                  .and_return(positive_response)
+        service_obj = described_class.create(service_info)
+        expect(service_obj.id).to eq(positive_response['id'])
+      end
+
+      it 'throws error when second request returns error' do
+        expect(remote).to receive(:create_service).with(hash_including('deployment_option'))
+                                                  .and_return(invalid_deployment_error_response)
+        expect(remote).to receive(:create_service).with(hash_excluding('deployment_option'))
+                                                  .and_return(common_error_response)
+        expect do
+          described_class.create(service_info)
+        end.to raise_error(ThreeScaleToolbox::Error, /Service has not been saved/)
+      end
+    end
+
+    it 'throws deployment option error' do
+      expect(remote).to receive(:create_service).and_return(common_error_response)
       expect do
         described_class.create(service_info)
       end.to raise_error(ThreeScaleToolbox::Error, /Service has not been saved/)
     end
 
     it 'service instance is returned' do
-      expect(remote).to receive(:create_service).with(expected_svc).and_return('errors' => nil, 'id' => 'some_id')
+      expect(remote).to receive(:create_service).and_return(positive_response)
       service_obj = described_class.create(service_info)
       expect(service_obj.id).to eq('some_id')
       expect(service_obj.remote).to be(remote)

spec/unit/tasks/update_service_settings_task_spec.rb

@@ -5,12 +5,17 @@
     let(:source) { double('source') }
     let(:target) { double('target') }
     let(:target_id) { 2 }
+    let(:deployment_option) { 'hosted' }
     let(:service_settings) do
       {
         'name' => 'some_name',
-        'system_name' => 'old_system_name'
+        'system_name' => 'old_system_name',
+        'deployment_option' => deployment_option
       }
     end
+    let(:service_id) { '1000' }
+    let(:common_error_response) { { 'errors' => { 'comp' => 'error' } } }
+    let(:positive_response) { { 'errors' => nil, 'id' => 'some_id' } }
     let(:target_name) { 'new_name' }
     subject { described_class.new(source: source, target: target, target_name: target_name) }
 
@@ -20,14 +25,40 @@
 
     context 'remote respond with error' do
       it 'error raised' do
-        expect(target).to receive(:update_service).and_return('errors' => 'some error')
+        expect(target).to receive(:update_service).and_return(common_error_response)
+        expect { subject.call }.to raise_error(ThreeScaleToolbox::Error, /not been saved/)
+      end
+    end
+
+    context 'deployment mode invalid' do
+      let(:invalid_deployment_error_response) do
+        {
+          'errors' => {
+            'deployment_option' => ['is not included in the list']
+          }
+        }
+      end
+
+      it 'deployment config is set to hosted' do
+        expect(source).to receive(:id).and_return(service_id)
+        expect(target).to receive(:update_service).with(hash_including('deployment_option'))
+                                                  .and_return(invalid_deployment_error_response)
+        expect(target).to receive(:update_service).with(hash_excluding('deployment_option'))
+                                                  .and_return(positive_response)
+        expect { subject.call }.to output(/updated service settings for service id #{service_id}/).to_stdout
+      end
+
+      it 'throws error when second request returns error' do
+        expect(target).to receive(:update_service).with(hash_including('deployment_option'))
+                                                  .and_return(invalid_deployment_error_response)
+        expect(target).to receive(:update_service).with(hash_excluding('deployment_option'))
+                                                  .and_return(common_error_response)
         expect { subject.call }.to raise_error(ThreeScaleToolbox::Error, /not been saved/)
       end
     end
 
     context 'system_name not provided' do
       let(:target_name) { nil }
-      let(:service_id) { '7' }
       let(:expected_svc_settings) { service_settings.reject { |k, _| k == 'system_name' } }
 
       it 'service settings does not contain system_name' do
@@ -38,7 +69,6 @@
     end
 
     context 'system_name provided' do
-      let(:service_id) { '7' }
       let(:expected_svc_settings) { service_settings.merge('system_name' => target_name) }
 
       it 'service settings contains new provided system_name' do