From aff928bacfb26f164c3f0ccbee7bcfa8228941cb Mon Sep 17 00:00:00 2001 From: Yves Senn Date: Mon, 27 May 2013 16:52:42 +0200 Subject: [PATCH] `implicit_readonly` is being removed in favor of calling `readonly` explicitly --- activerecord/CHANGELOG.md | 12 ++++++++++++ activerecord/lib/active_record/relation.rb | 6 +----- .../active_record/relation/query_methods.rb | 5 ----- .../inner_join_association_test.rb | 8 ++++---- activerecord/test/cases/readonly_test.rb | 18 ++++++++---------- 5 files changed, 25 insertions(+), 24 deletions(-) diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 3efd473627..5f63b6d7ae 100644 --- a/activerecord/CHANGELOG.md +++ b/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. diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 2439c3b440..9857ab0f8f 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -26,7 +26,6 @@ module ActiveRecord @klass = klass @table = table @values = values - @implicit_readonly = nil @loaded = false @default_scoped = false end @@ -582,10 +581,7 @@ module ActiveRecord 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 diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index d020f1ba52..fe8fec4dfa 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -864,8 +864,6 @@ module ActiveRecord return [] if joins.empty? - @implicit_readonly = true - joins.map do |join| case join when Array @@ -947,8 +945,6 @@ module ActiveRecord 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 @@ module ActiveRecord def build_select(arel, selects) unless selects.empty? - @implicit_readonly = false arel.project(*selects) else arel.project(@klass.arel_table[Arel.star]) diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index 918783e8f1..9baf94399a 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -46,10 +46,10 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase 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 diff --git a/activerecord/test/cases/readonly_test.rb b/activerecord/test/cases/readonly_test.rb index df076c97b4..2afd25c989 100644 --- a/activerecord/test/cases/readonly_test.rb +++ b/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/reader' 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 @@ class ReadOnlyTest < ActiveRecord::TestCase 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 @@ class ReadOnlyTest < ActiveRecord::TestCase # 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