Remove redundant validates_unique where DB unique index already exists

app/models/runtime/app_model.rb

@@ -84,7 +84,7 @@ def around_save
     rescue Sequel::UniqueConstraintViolation => e
       raise e unless e.message.include?('apps_v3_space_guid_name_index')
 
-      errors.add(%i[space_guid name], :unique)
+      errors.add(%i[space_guid name], Sequel.lit("App with the name '#{name}' already exists."))
       raise validation_failed_error
     rescue Sequel::ForeignKeyConstraintViolation => e
       raise e unless e.message.include?('fk_apps_droplet_guid')
@@ -99,8 +99,6 @@ def validate
       validates_format APP_NAME_REGEX, :name
       validate_environment_variables
       validate_droplet_is_staged
-
-      validates_unique %i[space_guid name], message: Sequel.lit("App with the name '#{name}' already exists.")
     end
 
     def lifecycle_type

app/models/runtime/domain.rb

@@ -97,6 +97,8 @@ def around_save
 
     def validate
       validates_presence :name
+      # Keep validates_unique to check across all Domain subclasses (STI: SharedDomain, PrivateDomain).
+      # Without it, exact name duplicates would be caught by name_overlaps? below with a misleading error message.
       validates_unique :name, dataset: Domain.dataset
 
       validates_format CloudController::DomainDecorator::DOMAIN_REGEX, :name,

app/models/runtime/feature_flag.rb

@@ -43,9 +43,17 @@ class UndefinedFeatureFlagError < StandardError
     export_attributes :name, :enabled, :error_message
     import_attributes :name, :enabled, :error_message
 
+    def around_save
+      yield
+    rescue Sequel::UniqueConstraintViolation => e
+      raise e unless e.message.include?('feature_flags_name_index')
+
+      errors.add(:name, :unique)
+      raise validation_failed_error
+    end
+
     def validate
       validates_presence :name
-      validates_unique :name
       validates_presence :enabled
 
       validates_includes DEFAULT_FLAGS.keys.map(&:to_s), :name

app/models/runtime/helpers/organization_role_mixin.rb

@@ -24,7 +24,6 @@ def around_save
     end
 
     def validate
-      validates_unique %i[organization_id user_id]
       validates_presence :organization_id
       validates_presence :user_id
     end

app/models/runtime/helpers/space_role_mixin.rb

@@ -26,7 +26,6 @@ def around_save
     def validate
       validates_presence :space_id
       validates_presence :user_id
-      validates_unique %i[space_id user_id]
     end
   end
 end

app/models/runtime/isolation_segment_model.rb

@@ -25,14 +25,12 @@ def around_save
     rescue Sequel::UniqueConstraintViolation => e
       raise e unless e.message.include?('isolation_segment_name_unique_constraint')
 
-      errors.add(:name, :unique)
+      errors.add(:name, Sequel.lit('Isolation Segment names are case insensitive and must be unique'))
       raise validation_failed_error
     end
 
     def validate
       validates_format ISOLATION_SEGMENT_MODEL_REGEX, :name, message: Sequel.lit('Isolation Segment names can only contain non-blank unicode characters')
-
-      validates_unique [:name], message: Sequel.lit('Isolation Segment names are case insensitive and must be unique')
     end
 
     def is_shared_segment?

app/models/runtime/organization.rb

@@ -218,7 +218,6 @@ def around_save
 
     def validate
       validates_presence :name
-      validates_unique :name
       validates_format ORG_NAME_REGEX, :name
       validates_includes ORG_STATUS_VALUES, :status, allow_missing: true
 

app/models/runtime/quota_definition.rb

@@ -30,7 +30,6 @@ def around_save
 
     def validate
       validates_presence :name
-      validates_unique :name
       validates_presence :non_basic_services_allowed
       validates_presence :total_services
       validates_presence :total_routes

app/models/runtime/space.rb

@@ -229,7 +229,6 @@ def around_save
     def validate
       validates_presence :name
       validates_presence :organization
-      validates_unique %i[organization_id name]
       validates_format SPACE_NAME_REGEX, :name
 
       errors.add(:space_quota_definition, :invalid_organization) if space_quota_definition && space_quota_definition.organization_id != organization.id

app/models/runtime/space_quota_definition.rb

@@ -41,7 +41,6 @@ def validate
       validates_presence :total_routes
       validates_presence :memory_limit
       validates_presence :organization
-      validates_unique %i[organization_id name]
 
       validates_limit(:memory_limit, memory_limit)
       validates_limit(:instance_memory_limit, instance_memory_limit)

app/models/runtime/stack.rb

@@ -43,7 +43,6 @@ def around_save
 
     def validate
       validates_presence :name
-      validates_unique :name
       validates_includes StackStates::VALID_STATES, :state, allow_missing: true
     end
 

app/models/runtime/user.rb

@@ -87,7 +87,6 @@ def around_save
 
     def validate
       validates_presence :guid
-      validates_unique :guid
     end
 
     def validate_organization(org)

app/models/services/service_broker.rb

@@ -34,6 +34,8 @@ def validate
       validates_presence :broker_url
       validates_presence :auth_username
       validates_presence :auth_password
+      # Keep validates_unique as a pre-guard for ServiceBrokerRegistration
+      # to avoid unnecessary HTTP calls to the broker if the name is not unique.
       validates_unique :name, message: Sequel.lit('Name must be unique')
       validates_url :broker_url
       validates_url_no_basic_auth

app/models/services/service_dashboard_client.rb

@@ -2,9 +2,17 @@ module VCAP::CloudController
   class ServiceDashboardClient < Sequel::Model
     many_to_one :service_broker
 
+    def around_save
+      yield
+    rescue Sequel::UniqueConstraintViolation => e
+      raise e unless e.message.include?('s_d_clients_uaa_id_unique')
+
+      errors.add(:uaa_id, :unique)
+      raise validation_failed_error
+    end
+
     def validate
       validates_presence :uaa_id
-      validates_unique :uaa_id
     end
 
     class << self

app/models/services/service_plan.rb

@@ -75,13 +75,21 @@ def active?
 
     alias_method :broker_provided_id, :unique_id
 
+    def around_save
+      yield
+    rescue Sequel::UniqueConstraintViolation => e
+      raise e unless e.message.include?('svc_plan_svc_id_name_index')
+
+      errors.add(%i[name service_id], Sequel.lit("Plan names must be unique within a service. Service #{service.try(:label)} already has a plan named #{name}"))
+      raise validation_failed_error
+    end
+
     def validate
       validates_presence :name,                message: 'is required'
       validates_presence :description,         message: 'is required'
       validates_presence :free,                message: 'is required'
       validates_presence :service,             message: 'is required'
       validates_presence :unique_id,           message: 'is required'
-      validates_unique %i[service_id name], message: Sequel.lit("Plan names must be unique within a service. Service #{service.try(:label)} already has a plan named #{name}")
       validate_private_broker_plan_not_public
     end
 

app/models/services/service_plan_visibility.rb

@@ -6,10 +6,18 @@ class ServicePlanVisibility < Sequel::Model
     import_attributes :service_plan_guid, :organization_guid
     export_attributes :service_plan_guid, :organization_guid
 
+    def around_save
+      yield
+    rescue Sequel::UniqueConstraintViolation => e
+      raise e unless e.message.include?('spv_org_id_sp_id_index')
+
+      errors.add(%i[organization_id service_plan_id], :unique)
+      raise validation_failed_error
+    end
+
     def validate
       validates_presence :service_plan
       validates_presence :organization
-      validates_unique %i[organization_id service_plan_id]
       validate_plan_is_not_private
       validate_plan_is_not_public
     end

spec/unit/actions/app_create_spec.rb

@@ -200,23 +200,6 @@ module VCAP::CloudController
         end
       end
 
-      context 'when creating apps concurrently' do
-        it 'ensures one creation is successful and the other fails due to name conflict' do
-          # First request, should succeed
-          expect do
-            app_create.create(message, lifecycle)
-          end.not_to raise_error
-
-          # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-          allow_any_instance_of(AppModel).to receive(:validate).and_return(true)
-
-          # Second request, should fail with correct error
-          expect do
-            app_create.create(message, lifecycle)
-          end.to raise_error(CloudController::Errors::V3::ApiError)
-        end
-      end
-
       describe 'stack state validation' do
         let(:test_stack) { Stack.make(name: 'test-stack-for-validation') }
         let(:lifecycle_request) { { type: 'buildpack', data: { buildpacks: [buildpack_identifier], stack: test_stack.name } } }

spec/unit/actions/organization_create_spec.rb

@@ -110,26 +110,6 @@ module VCAP::CloudController
             end.to raise_error(OrganizationCreate::Error, "Organization '#{name}' already exists.")
           end
         end
-
-        context 'when creating organizations concurrently' do
-          let(:name) { 'Alfredo' }
-
-          it 'ensures one creation is successful and the other fails due to name conflict' do
-            # First request, should succeed
-            message = VCAP::CloudController::OrganizationUpdateMessage.new(name:)
-            expect do
-              org_create.create(message)
-            end.not_to raise_error
-
-            # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-            allow_any_instance_of(Organization).to receive(:validate).and_return(true)
-
-            # Second request, should fail with correct error
-            expect do
-              org_create.create(message)
-            end.to raise_error(OrganizationCreate::Error, "Organization 'Alfredo' already exists.")
-          end
-        end
       end
     end
   end

spec/unit/actions/service_broker_create_spec.rb

@@ -132,23 +132,6 @@ module CloudController
           end
         end
       end
-
-      describe 'when creating a service broker with the same name concurrently' do
-        it 'ensures one creation is successful and the other fails due to name conflict' do
-          # First request, should succeed
-          expect do
-            action.create(message)
-          end.not_to raise_error
-
-          # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-          allow_any_instance_of(ServiceBroker).to receive(:validate).and_return(true)
-
-          # Second request, should fail with correct error
-          expect do
-            action.create(message)
-          end.to raise_error(V3::ServiceBrokerCreate::InvalidServiceBroker)
-        end
-      end
     end
   end
 end

spec/unit/actions/space_create_spec.rb

@@ -79,26 +79,6 @@ module VCAP::CloudController
             end.to raise_error(SpaceCreate::Error, 'Name must be unique per organization')
           end
         end
-
-        context 'when creating spaces concurrently' do
-          let(:name) { 'Rose' }
-
-          it 'ensures one creation is successful and the other fails due to name conflict' do
-            # First request, should succeed
-            message = VCAP::CloudController::SpaceCreateMessage.new(name:)
-            expect do
-              SpaceCreate.new(user_audit_info:).create(org, message)
-            end.not_to raise_error
-
-            # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-            allow_any_instance_of(Space).to receive(:validate).and_return(true)
-
-            # Second request, should fail with correct error
-            expect do
-              SpaceCreate.new(user_audit_info:).create(org, message)
-            end.to raise_error(SpaceCreate::Error, 'Name must be unique per organization')
-          end
-        end
       end
     end
   end

spec/unit/actions/space_quotas_create_spec.rb

@@ -202,23 +202,6 @@ module VCAP::CloudController
           end
         end
       end
-
-      context 'when creating space quota with the same name concurrently' do
-        it 'ensures one creation is successful and the other fails due to name conflict' do
-          # First request, should succeed
-          expect do
-            space_quotas_create.create(message, organization: org)
-          end.not_to raise_error
-
-          # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-          allow_any_instance_of(SpaceQuotaDefinition).to receive(:validate).and_return(true)
-
-          # Second request, should fail with correct error
-          expect do
-            space_quotas_create.create(message, organization: org)
-          end.to raise_error(SpaceQuotasCreate::Error, "Space Quota 'my-name' already exists.")
-        end
-      end
     end
   end
 end

spec/unit/actions/stack_create_spec.rb

@@ -119,26 +119,6 @@ module VCAP::CloudController
           end.to raise_error(StackCreate::Error, 'Name must be unique')
         end
       end
-
-      context 'when creating stack with the same name concurrently' do
-        let(:name) { 'Gaby' }
-
-        it 'ensures one creation is successful and the other fails due to name conflict' do
-          message = VCAP::CloudController::StackCreateMessage.new(name:)
-          # First request, should succeed
-          expect do
-            stack_create.create(message)
-          end.not_to raise_error
-
-          # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-          allow_any_instance_of(Stack).to receive(:validate).and_return(true)
-
-          # Second request, should fail with correct error
-          expect do
-            stack_create.create(message)
-          end.to raise_error(StackCreate::Error, 'Name must be unique')
-        end
-      end
     end
   end
 end

spec/unit/actions/user_create_spec.rb

@@ -35,25 +35,6 @@ module VCAP::CloudController
         end
       end
 
-      context 'when creating users concurrently' do
-        let(:message) { UserCreateMessage.new({ guid: 'some-nice-user-gu-id' }) }
-
-        it 'ensures one creation is successful and the other fails due to name conflict' do
-          # First request, should succeed
-          expect do
-            subject.create(message:)
-          end.not_to raise_error
-
-          # Mock the validation for the second request to simulate the race condition and trigger a unique constraint violation
-          allow_any_instance_of(User).to receive(:validate).and_return(true)
-
-          # Second request, should fail with correct error
-          expect do
-            subject.create(message:)
-          end.to raise_error(UserCreate::Error, "User with guid 'some-nice-user-gu-id' already exists.")
-        end
-      end
-
       describe 'creating users' do
         before do
           allow(User).to receive_messages(create_uaa_shadow_user: { 'id' => guid }, get_user_id_by_username_and_origin: nil)