Avoid validating `belongs_to` association if it has not changed

This commit is contained in:
fatkodima 2022-11-24 03:31:04 +02:00
parent eed5655d94
commit e5d15140d2
9 changed files with 110 additions and 1 deletions

View File

@ -1,3 +1,21 @@
* Avoid validating `belongs_to` association if it has not changed.
Previously, when updating a record, Active Record will perform an extra query to check for the presence of
`belongs_to` associations (if the presence is configured to be mandatory), even if that attribute hasn't changed.
Currently, only `belongs_to`-related columns are checked for presence. It is possible to have orphaned records with
this approach. To avoid this problem, you need to use a foreign key.
This behavior can be controlled by configuration:
```ruby
config.active_record.belongs_to_required_validates_foreign_key = false
```
and will be disabled by default with `load_defaults 7.1`.
*fatkodima*
* `has_one` and `belongs_to` associations now define a `reset_association` method
on the owner model (where `association` is the name of the association). This
method unloads the cached associate record, if any, and causes the next access

View File

@ -271,6 +271,9 @@ module ActiveRecord
singleton_class.attr_accessor :raise_on_assign_to_attr_readonly
self.raise_on_assign_to_attr_readonly = false
singleton_class.attr_accessor :belongs_to_required_validates_foreign_key
self.belongs_to_required_validates_foreign_key = true
##
# :singleton-method:
# Specify a threshold for the size of query result sets. If the number of

View File

@ -123,7 +123,20 @@ module ActiveRecord::Associations::Builder # :nodoc:
super
if required
model.validates_presence_of reflection.name, message: :required
if ActiveRecord.belongs_to_required_validates_foreign_key
model.validates_presence_of reflection.name, message: :required
else
condition = lambda { |record|
foreign_key = reflection.foreign_key
foreign_type = reflection.foreign_type
record.read_attribute(foreign_key).nil? ||
record.attribute_changed?(foreign_key) ||
(reflection.polymorphic? && (record.read_attribute(foreign_type).nil? || record.attribute_changed?(foreign_type)))
}
model.validates_presence_of reflection.name, message: :required, if: condition
end
end
end

View File

@ -37,6 +37,7 @@ module ActiveRecord
config.active_record.query_log_tags_format = :legacy
config.active_record.cache_query_log_tags = false
config.active_record.raise_on_assign_to_attr_readonly = false
config.active_record.belongs_to_required_validates_foreign_key = true
config.active_record.queues = ActiveSupport::InheritableOptions.new

View File

@ -1665,6 +1665,61 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase
assert_not comment.author_changed?
assert comment.author_previously_changed?
end
class ShipRequired < ActiveRecord::Base
self.table_name = "ships"
belongs_to :developer, required: true
end
test "runs parent presence check if parent changed or nil" do
david = developers(:david)
jamis = developers(:jamis)
ship = ShipRequired.create!(name: "Medusa", developer: david)
assert_equal david, ship.developer
assert_queries(2) do # UPDATE and SELECT to check developer presence
ship.update!(developer_id: jamis.id)
end
ship.update_column(:developer_id, nil)
ship.reload
assert_queries(2) do # UPDATE and SELECT to check developer presence
ship.update!(developer_id: david.id)
end
end
test "skips parent presence check if parent has not changed" do
david = developers(:david)
ship = ShipRequired.create!(name: "Medusa", developer: david)
ship.reload # unload developer association
assert_queries(1) do # UPDATE only, no SELECT to check developer presence
ship.update!(name: "Leviathan")
end
end
test "runs parent presence check if parent has not changed and belongs_to_required_validates_foreign_key is set" do
original_value = ActiveRecord.belongs_to_required_validates_foreign_key
ActiveRecord.belongs_to_required_validates_foreign_key = true
model = Class.new(ActiveRecord::Base) do
self.table_name = "ships"
def self.name; "Temp"; end
belongs_to :developer, required: true
end
david = developers(:david)
ship = model.create!(name: "Medusa", developer: david)
ship.reload # unload developer association
assert_queries(2) do # UPDATE and SELECT to check developer presence
ship.update!(name: "Leviathan")
end
ensure
ActiveRecord.belongs_to_required_validates_foreign_key = original_value
end
end
class BelongsToWithForeignKeyTest < ActiveRecord::TestCase

View File

@ -31,6 +31,7 @@ ARTest.connect
QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name("type")
ActiveRecord.raise_on_assign_to_attr_readonly = true
ActiveRecord.belongs_to_required_validates_foreign_key = false
def current_adapter?(*types)
types.any? do |type|

View File

@ -66,6 +66,7 @@ Below are the default values associated with each target version. In cases of co
- [`config.active_record.allow_deprecated_singular_associations_name`](#config-active-record-allow-deprecated-singular-associations-name): `false`
- [`config.active_record.query_log_tags_format`](#config-active-record-query-log-tags-format): `:sqlcommenter`
- [`config.active_record.raise_on_assign_to_attr_readonly`](#config-active-record-raise-on-assign-to-attr-readonly): `true`
- [`config.active_record.belongs_to_required_validates_foreign_key`](#config-active-record-belongs-to-required-validates-foreign-key): `false`
- [`config.active_record.run_commit_callbacks_on_first_saved_instances_in_transaction`](#config-active-record-run-commit-callbacks-on-first-saved-instances-in-transaction): `false`
- [`config.active_record.sqlite3_adapter_strict_strings_by_default`](#config-active-record-sqlite3-adapter-strict-strings-by-default): `true`
- [`config.active_support.default_message_encryptor_serializer`](#config-active-support-default-message-encryptor-serializer): `:json`
@ -1002,6 +1003,17 @@ The default value depends on the `config.load_defaults` target version:
| (original) | `nil` |
| 5.0 | `true` |
#### `config.active_record.belongs_to_required_validates_foreign_key`
Enable validating only parent-related columns for presence when the parent is mandatory.
The previous behavior was to validate the presence of the parent record, which performed an extra query
to get the parent every time the child record was updated, even when parent has not changed.
| Starting with version | The default value is |
| --------------------- | -------------------- |
| (original) | `true` |
| 7.1 | `false` |
#### `config.active_record.action_on_strict_loading_violation`
Enables raising or logging an exception if strict_loading is set on an

View File

@ -289,6 +289,7 @@ module Rails
active_record.sqlite3_adapter_strict_strings_by_default = true
active_record.query_log_tags_format = :sqlcommenter
active_record.raise_on_assign_to_attr_readonly = true
active_record.belongs_to_required_validates_foreign_key = false
end
if respond_to?(:action_dispatch)

View File

@ -112,3 +112,8 @@
# behavior would allow assignment but silently not persist changes to the
# database.
# Rails.application.config.active_record.raise_on_assign_to_attr_readonly = true
# Enable validating only parent-related columns for presence when the parent is mandatory.
# The previous behavior was to validate the presence of the parent record, which performed an extra query
# to get the parent every time the child record was updated, even when parent has not changed.
# Rails.application.config.active_record.belongs_to_required_validates_foreign_key = false