Browse files

Merge pull request #10769 from senny/10615_join_should_not_return_rea…

…donly_records

using `#joins` does not imply `readonly = true`.
  • Loading branch information...
2 parents c3890d3 + aff928b commit 5a687e930ffd4785be7af4816f97000fc3e948b3 @rafaelfranca rafaelfranca committed May 27, 2013
View
12 activerecord/CHANGELOG.md
@@ -1,3 +1,15 @@
+* Usage of `implicit_readonly` is being removed`. Please use `readonly` method
+ explicitly to mark records as `readonly.
+ Fixes #10615.
+
+ Example:
+
+ user = User.joins(:todos).select("users.*, todos.title as todos_title").readonly(true).first
+ user.todos_title = 'clean pet'
+ user.save! # will raise error
+
+ *Yves Senn*
+
* Fix the `:primary_key` option for `has_many` associations.
Fixes #10693.
View
14 activerecord/lib/active_record/relation.rb
@@ -26,7 +26,6 @@ def initialize(klass, table, values = {})
@klass = klass
@table = table
@values = values
- @implicit_readonly = nil
@loaded = false
@default_scoped = false
end
@@ -76,10 +75,10 @@ def insert(values) # :nodoc:
def update_record(values, id, id_was) # :nodoc:
substitutes, binds = substitute_values values
um = @klass.unscoped.where(@klass.arel_table[@klass.primary_key].eq(id_was || id)).arel.compile_update(substitutes)
-
+
@klass.connection.update(
- um,
- 'SQL',
+ um,
+ 'SQL',
binds)
end
@@ -94,7 +93,7 @@ def substitute_values(values) # :nodoc:
end
[substitutes, binds]
- end
+ end
# Initializes new record from relation while maintaining the current
# scope.
@@ -582,10 +581,7 @@ def exec_queries
ActiveRecord::Associations::Preloader.new(@records, associations).run
end
- # @readonly_value is true only if set explicitly. @implicit_readonly is true if there
- # are JOINS and no explicit SELECT.
- readonly = readonly_value.nil? ? @implicit_readonly : readonly_value
- @records.each { |record| record.readonly! } if readonly
+ @records.each { |record| record.readonly! } if readonly_value
else
@records = default_scoped.to_a
end
View
5 activerecord/lib/active_record/relation/query_methods.rb
@@ -864,8 +864,6 @@ def custom_join_ast(table, joins)
return [] if joins.empty?
- @implicit_readonly = true
-
joins.map do |join|
case join
when Array
@@ -947,8 +945,6 @@ def build_joins(manager, joins)
join_dependency.graft(*stashed_association_joins)
- @implicit_readonly = true unless association_joins.empty? && stashed_association_joins.empty?
-
# FIXME: refactor this to build an AST
join_dependency.join_associations.each do |association|
association.join_to(manager)
@@ -961,7 +957,6 @@ def build_joins(manager, joins)
def build_select(arel, selects)
unless selects.empty?
- @implicit_readonly = false
arel.project(*selects)
else
arel.project(@klass.arel_table[Arel.star])
View
8 activerecord/test/cases/associations/inner_join_association_test.rb
@@ -46,10 +46,10 @@ def test_join_conditions_allow_nil_associations
assert_equal 2, authors.count
end
- def test_find_with_implicit_inner_joins_honors_readonly_without_select
- authors = Author.joins(:posts).to_a
- assert !authors.empty?, "expected authors to be non-empty"
- assert authors.all? {|a| a.readonly? }, "expected all authors to be readonly"
+ def test_find_with_implicit_inner_joins_without_select_does_not_imply_readonly
+ authors = Author.joins(:posts)
+ assert_not authors.empty?, "expected authors to be non-empty"
+ assert authors.none? {|a| a.readonly? }, "expected no authors to be readonly"
end
def test_find_with_implicit_inner_joins_honors_readonly_with_select
View
18 activerecord/test/cases/readonly_test.rb
@@ -1,4 +1,5 @@
require "cases/helper"
+require 'models/author'
require 'models/post'
require 'models/comment'
require 'models/developer'
@@ -7,7 +8,7 @@
require 'models/person'
class ReadOnlyTest < ActiveRecord::TestCase
- fixtures :posts, :comments, :developers, :projects, :developers_projects, :people, :readers
+ fixtures :authors, :posts, :comments, :developers, :projects, :developers_projects, :people, :readers
def test_cant_save_readonly_record
dev = Developer.find(1)
@@ -34,15 +35,12 @@ def test_find_with_readonly_option
Developer.readonly.each { |d| assert d.readonly? }
end
+ def test_find_with_joins_option_does_not_imply_readonly
+ Developer.joins(' ').each { |d| assert_not d.readonly? }
+ Developer.joins(' ').readonly(true).each { |d| assert d.readonly? }
- def test_find_with_joins_option_implies_readonly
- # Blank joins don't count.
- Developer.joins(' ').each { |d| assert !d.readonly? }
- Developer.joins(' ').readonly(false).each { |d| assert !d.readonly? }
-
- # Others do.
- Developer.joins(', projects').each { |d| assert d.readonly? }
- Developer.joins(', projects').readonly(false).each { |d| assert !d.readonly? }
+ Developer.joins(', projects').each { |d| assert_not d.readonly? }
+ Developer.joins(', projects').readonly(true).each { |d| assert d.readonly? }
end
def test_has_many_find_readonly
@@ -87,7 +85,7 @@ def test_readonly_scoping
# conflicting column names
unless current_adapter?(:OracleAdapter)
Post.joins(', developers').scoping do
- assert Post.find(1).readonly?
+ assert_not Post.find(1).readonly?
assert Post.readonly.find(1).readonly?
assert !Post.readonly(false).find(1).readonly?
end

0 comments on commit 5a687e9

Please sign in to comment.