Skip to content

Commit c8604dc

Browse files
authored
Remove redundant validates_unique where DB unique index already exists (#4930)
* feat: validate_unique removed - Model's that already has uniqueness constraint on db level are refactored. sequel validate_unique method removed from list. * feat: around_save added for existing unique index for service_plan * feat: around_save added for existing unique index for service_plan_visibility * feat: around_save added for existing unique index for service_dashboard_client model * feat: around_save added for existing unique index for feature_flag model * feat: around_save added for existing unique index for qutoa_definition model * fix: rubocop errors are fixed * fix: error message fixed for service_broker model * fix: service_broker validate_unique reintroduced because, it uses this validation in service_broker_registraion service to refraing http call invalid brokers * fix: service_broker validate_unique reintroduced because, it uses this validation in service_key_manager to refraing http call in service_key_create * fix: service_broker validation_unique is removed * fix: typos are fixed * fix: concurrent context tests are removed in unit/actions. Uniqueness tests are reintroduced in unit/models. * fix: changes reverted because if we remove validate_uniqueness, it fires name_overlap and that leads misleading error * fix: add comment explaining rescue Sequel::ValidationFailed in service broker registration The around_save hook on the ServiceBroker model catches unique constraint violations and adds errors to the model before raising ValidationFailed. The rescue block returns nil, and callers access errors via the delegated errors method. * fix: service_broker validation_unique removel is reverted. Since it is used as guard before calling some http endpoint in check in service_broker_registration. Comments are added
1 parent ae26719 commit c8604dc

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

44 files changed

+222
-190
lines changed

app/models/runtime/app_model.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -84,7 +84,7 @@ def around_save
8484
rescue Sequel::UniqueConstraintViolation => e
8585
raise e unless e.message.include?('apps_v3_space_guid_name_index')
8686

87-
errors.add(%i[space_guid name], :unique)
87+
errors.add(%i[space_guid name], Sequel.lit("App with the name '#{name}' already exists."))
8888
raise validation_failed_error
8989
rescue Sequel::ForeignKeyConstraintViolation => e
9090
raise e unless e.message.include?('fk_apps_droplet_guid')
@@ -99,8 +99,6 @@ def validate
9999
validates_format APP_NAME_REGEX, :name
100100
validate_environment_variables
101101
validate_droplet_is_staged
102-
103-
validates_unique %i[space_guid name], message: Sequel.lit("App with the name '#{name}' already exists.")
104102
end
105103

106104
def lifecycle_type

app/models/runtime/domain.rb

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -97,6 +97,8 @@ def around_save
9797

9898
def validate
9999
validates_presence :name
100+
# Keep validates_unique to check across all Domain subclasses (STI: SharedDomain, PrivateDomain).
101+
# Without it, exact name duplicates would be caught by name_overlaps? below with a misleading error message.
100102
validates_unique :name, dataset: Domain.dataset
101103

102104
validates_format CloudController::DomainDecorator::DOMAIN_REGEX, :name,

app/models/runtime/feature_flag.rb

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,9 +43,17 @@ class UndefinedFeatureFlagError < StandardError
4343
export_attributes :name, :enabled, :error_message
4444
import_attributes :name, :enabled, :error_message
4545

46+
def around_save
47+
yield
48+
rescue Sequel::UniqueConstraintViolation => e
49+
raise e unless e.message.include?('feature_flags_name_index')
50+
51+
errors.add(:name, :unique)
52+
raise validation_failed_error
53+
end
54+
4655
def validate
4756
validates_presence :name
48-
validates_unique :name
4957
validates_presence :enabled
5058

5159
validates_includes DEFAULT_FLAGS.keys.map(&:to_s), :name

app/models/runtime/helpers/organization_role_mixin.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,6 @@ def around_save
2424
end
2525

2626
def validate
27-
validates_unique %i[organization_id user_id]
2827
validates_presence :organization_id
2928
validates_presence :user_id
3029
end

app/models/runtime/helpers/space_role_mixin.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@ def around_save
2626
def validate
2727
validates_presence :space_id
2828
validates_presence :user_id
29-
validates_unique %i[space_id user_id]
3029
end
3130
end
3231
end

app/models/runtime/isolation_segment_model.rb

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -25,14 +25,12 @@ def around_save
2525
rescue Sequel::UniqueConstraintViolation => e
2626
raise e unless e.message.include?('isolation_segment_name_unique_constraint')
2727

28-
errors.add(:name, :unique)
28+
errors.add(:name, Sequel.lit('Isolation Segment names are case insensitive and must be unique'))
2929
raise validation_failed_error
3030
end
3131

3232
def validate
3333
validates_format ISOLATION_SEGMENT_MODEL_REGEX, :name, message: Sequel.lit('Isolation Segment names can only contain non-blank unicode characters')
34-
35-
validates_unique [:name], message: Sequel.lit('Isolation Segment names are case insensitive and must be unique')
3634
end
3735

3836
def is_shared_segment?

app/models/runtime/organization.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -218,7 +218,6 @@ def around_save
218218

219219
def validate
220220
validates_presence :name
221-
validates_unique :name
222221
validates_format ORG_NAME_REGEX, :name
223222
validates_includes ORG_STATUS_VALUES, :status, allow_missing: true
224223

app/models/runtime/quota_definition.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ def around_save
3030

3131
def validate
3232
validates_presence :name
33-
validates_unique :name
3433
validates_presence :non_basic_services_allowed
3534
validates_presence :total_services
3635
validates_presence :total_routes

app/models/runtime/space.rb

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -229,7 +229,6 @@ def around_save
229229
def validate
230230
validates_presence :name
231231
validates_presence :organization
232-
validates_unique %i[organization_id name]
233232
validates_format SPACE_NAME_REGEX, :name
234233

235234
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

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,6 @@ def validate
4141
validates_presence :total_routes
4242
validates_presence :memory_limit
4343
validates_presence :organization
44-
validates_unique %i[organization_id name]
4544

4645
validates_limit(:memory_limit, memory_limit)
4746
validates_limit(:instance_memory_limit, instance_memory_limit)

0 commit comments

Comments
 (0)