Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

Fix #7191. Remove unnecessary transaction when assigning has_one associations. #7286

Merged
merged 1 commit into from

2 participants

@kennyj
Collaborator

Please see #7191. If target is equal to record or target and record are nil, we should not use transaction.

@rafaelfranca rafaelfranca merged commit d1835db into rails:master
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Aug 7, 2012
  1. @kennyj
This page is out of date. Refresh to see the latest.
View
28 activerecord/lib/active_record/associations/has_one_association.rb
@@ -7,19 +7,21 @@ def replace(record, save = true)
raise_on_type_mismatch(record) if record
load_target
- reflection.klass.transaction do
- if target && target != record
- remove_target!(options[:dependent]) unless target.destroyed?
- end
-
- if record
- set_owner_attributes(record)
- set_inverse_instance(record)
-
- if owner.persisted? && save && !record.save
- nullify_owner_attributes(record)
- set_owner_attributes(target) if target
- raise RecordNotSaved, "Failed to save the new associated #{reflection.name}."
+ # If target and record are nil, or target is equal to record,
+ # we don't need to have transaction.
+ if (target || record) && target != record
+ reflection.klass.transaction do
+ remove_target!(options[:dependent]) if target && !target.destroyed?
+
+ if record
+ set_owner_attributes(record)
+ set_inverse_instance(record)
+
+ if owner.persisted? && save && !record.save
+ nullify_owner_attributes(record)
+ set_owner_attributes(target) if target
+ raise RecordNotSaved, "Failed to save the new associated #{reflection.name}."
+ end
end
end
end
View
13 activerecord/test/cases/associations/has_one_associations_test.rb
@@ -535,4 +535,17 @@ def test_building_has_one_association_with_dependent_restrict
ensure
ActiveRecord::Base.dependent_restrict_raises = option_before
end
+
+ def test_has_one_transaction
+ company = companies(:first_firm)
+ account = Account.find(1)
+
+ company.account # force loading
+ assert_no_queries { company.account = account }
+
+ company.account = nil
+ assert_no_queries { company.account = nil }
+ account = Account.find(2)
+ assert_queries { company.account = account }
+ end
end
Something went wrong with that request. Please try again.