Merge pull request #3469 from sap-contributions/fix-service-route-bin…

…ding-failed-state-handling

Throw different error when route binding status is delete_failed or delete_in_progess / Treat route binding in status create_failed as non-existent

app/actions/service_route_binding_create.rb

@@ -15,7 +15,9 @@ def initialize(user_audit_info, audit_hash)
       PERMITTED_BINDING_ATTRIBUTES = [:route_service_url].freeze
 
       def precursor(service_instance, route, message:)
-        validate!(service_instance, route)
+        validate_service_instance!(service_instance, route)
+        binding = RouteBinding.first(service_instance:, route:)
+        validate_binding!(binding)
 
         binding_details = {
           service_instance:,
@@ -24,6 +26,7 @@ def precursor(service_instance, route, message:)
 
         RouteBinding.new.tap do |b|
           RouteBinding.db.transaction do
+            binding.destroy if binding
             b.save_with_attributes_and_new_operation(
               binding_details,
               CREATE_INITIAL_OPERATION
@@ -43,19 +46,25 @@ def event_repository
         )
       end
 
-      def validate!(service_instance, route)
+      def validate_service_instance!(service_instance, route)
         not_supported! unless service_instance.route_service?
         not_bindable! unless service_instance.bindable?
         route_is_internal! if route.try(:internal?)
         space_mismatch! unless route.space == service_instance.space
-        already_exists! if route.service_instance == service_instance
-        already_bound! if route.service_instance
+        already_bound! if route.service_instance && route.service_instance != service_instance
         return unless service_instance.managed_instance?
 
         service_instance_not_found! if service_instance.create_failed?
         operation_in_progress! if service_instance.operation_in_progress?
       end
 
+      def validate_binding!(binding)
+        return unless binding
+
+        already_exists! if binding.create_succeeded? || binding.create_in_progress?
+        incomplete_deletion! if binding.delete_failed? || binding.delete_in_progress?
+      end
+
       def permitted_binding_attributes
         PERMITTED_BINDING_ATTRIBUTES
       end
@@ -87,6 +96,10 @@ def not_bindable!
       def already_exists!
         raise RouteBindingAlreadyExists.new('The route and service instance are already bound')
       end
+
+      def incomplete_deletion!
+        raise UnprocessableCreate.new('The binding is getting deleted or its deletion failed')
+      end
     end
   end
 end

spec/unit/actions/service_route_binding_create_spec.rb

@@ -79,15 +79,83 @@ module V3
           end
 
           context 'route binding already exists' do
-            it 'raises an error' do
-              RouteBinding.make(service_instance:, route:)
+            let!(:binding) { RouteBinding.make(service_instance:, route:) }
+
+            context 'when no last service route binding operation exists' do
+              it 'raises an error' do
+                expect do
+                  action.precursor(service_instance, route, message:)
+                end.to raise_error(
+                  ServiceRouteBindingCreate::RouteBindingAlreadyExists,
+                  'The route and service instance are already bound'
+                )
+              end
+            end
 
-              expect do
-                action.precursor(service_instance, route, message:)
-              end.to raise_error(
-                ServiceRouteBindingCreate::RouteBindingAlreadyExists,
-                'The route and service instance are already bound'
-              )
+            context "when the last service route binding operation is in 'create succeeded' state" do
+              before do
+                binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'succeeded' })
+              end
+
+              it 'raises an error' do
+                expect { action.precursor(service_instance, route, message:) }.to raise_error(
+                  ServiceRouteBindingCreate::RouteBindingAlreadyExists,
+                  'The route and service instance are already bound'
+                )
+              end
+            end
+
+            context "when the last service route binding operation is in 'create in progress' state" do
+              before do
+                binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'in progress' })
+              end
+
+              it 'raises an error' do
+                expect { action.precursor(service_instance, route, message:) }.to raise_error(
+                  ServiceRouteBindingCreate::RouteBindingAlreadyExists,
+                  'The route and service instance are already bound'
+                )
+              end
+            end
+
+            context "when the last service route binding operation is in 'create failed' state" do
+              before do
+                binding.save_with_attributes_and_new_operation({}, { type: 'create', state: 'failed' })
+              end
+
+              it 'deletes the existing binding and creates a new one' do
+                b = action.precursor(service_instance, route, message:)
+
+                expect(b.guid).not_to eq(binding.guid)
+                expect(b).to be_create_in_progress
+                expect { binding.reload }.to raise_error Sequel::NoExistingObject
+              end
+            end
+
+            context "when the last service route binding operation is in 'delete in progress' state" do
+              before do
+                binding.save_with_attributes_and_new_operation({}, { type: 'delete', state: 'in progress' })
+              end
+
+              it 'raises an error' do
+                expect { action.precursor(service_instance, route, message:) }.to raise_error(
+                  ServiceRouteBindingCreate::UnprocessableCreate,
+                  'The binding is getting deleted or its deletion failed'
+                )
+              end
+            end
+
+            context "when the last service route binding operation is in 'delete failed' state" do
+              before do
+                binding.save_with_attributes_and_new_operation({}, { type: 'delete', state: 'failed' })
+              end
+
+              it 'raises an error' do
+                expect { action.precursor(service_instance, route, message:) }.to raise_error(
+                  ServiceRouteBindingCreate::UnprocessableCreate,
+                  'The binding is getting deleted or its deletion failed'
+                )
+              end
             end
           end