`implicit_readonly` is being removed in favor of calling `readonly` explicitly

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