Change the implementation of ActiveRecord's attribute reader and writer methods:

 * Generate Reader and Writer methods which cache attribute values in hashes.  This is to avoid repeatedly parsing the same date or integer columns.
 * Move the attribute related methods out to attribute_methods.rb to de-clutter base.rb
 * Change exception raised when users use find with :select then try to access a skipped column.  Plugins could override missing_attribute() to lazily load the columns.
 * Move method definition to the class, instead of the instance
 * Always generate the readers, writers and predicate methods.


git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@7315 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

3 files changed, 10 insertions, 29 deletions

diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 99cb9ac0a1..9da5552c09 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -1470,10 +1470,12 @@ class HasAndBelongsToManyAssociationsTest < Test::Unit::TestCase
     assert project.respond_to?("name=")
     assert project.respond_to?("name?")
     assert project.respond_to?("joined_on")
-    assert project.respond_to?("joined_on=")
+    # given that the 'join attribute' won't be persisted, I don't
+    # think we should define the mutators
+    #assert project.respond_to?("joined_on=")
     assert project.respond_to?("joined_on?")
     assert project.respond_to?("access_level")
-    assert project.respond_to?("access_level=")
+    #assert project.respond_to?("access_level=")
     assert project.respond_to?("access_level?")
   end
 
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 2701a2692d..1b6714a495 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -344,24 +344,10 @@ class BasicsTest < Test::Unit::TestCase
     assert !object.int_value?
   end
 
-  def test_reader_generation
-    Topic.find(:first).title
-    Firm.find(:first).name
-    Client.find(:first).name
-    if ActiveRecord::Base.generate_read_methods
-      assert_readers(Topic,  %w(type replies_count))
-      assert_readers(Firm,   %w(type))
-      assert_readers(Client, %w(type ruby_type rating?))
-    else
-      [Topic, Firm, Client].each {|klass| assert_equal klass.read_methods, {}}
-    end
-  end
 
   def test_reader_for_invalid_column_names
-    # column names which aren't legal ruby ids
-    topic = Topic.find(:first)
-    topic.send(:define_read_method, "mumub-jumbo".to_sym, "mumub-jumbo", nil)
-    assert !Topic.read_methods.include?("mumub-jumbo")
+    Topic.send(:define_read_method, "mumub-jumbo".to_sym, "mumub-jumbo", nil)
+    assert !Topic.generated_methods.include?("mumub-jumbo")
   end
 
   def test_non_attribute_access_and_assignment
@@ -791,7 +777,7 @@ class BasicsTest < Test::Unit::TestCase
   
   def test_mass_assignment_protection_against_class_attribute_writers
     [:logger, :configurations, :primary_key_prefix_type, :table_name_prefix, :table_name_suffix, :pluralize_table_names, :colorize_logging,
-      :default_timezone, :allow_concurrency, :generate_read_methods, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method|
+      :default_timezone, :allow_concurrency, :schema_format, :verification_timeout, :lock_optimistically, :record_timestamps].each do |method|
       assert  Task.respond_to?(method)
       assert  Task.respond_to?("#{method}=")
       assert  Task.new.respond_to?(method)
@@ -1708,12 +1694,4 @@ class BasicsTest < Test::Unit::TestCase
     assert_equal %("#{t.written_on.to_s(:db)}"), t.attribute_for_inspect(:written_on)
     assert_equal '"This is some really long content, longer than 50 ch..."', t.attribute_for_inspect(:content)
   end
-
-  private
-    def assert_readers(model, exceptions)
-      expected_readers = Set.new(model.column_names - ['id'])
-      expected_readers += expected_readers.map { |col| "#{col}?" }
-      expected_readers -= exceptions
-      assert_equal expected_readers, model.read_methods
-    end
 end
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index 9c68b604cc..d9809d4b7e 100644
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -1,4 +1,5 @@
 require 'abstract_unit'
+require 'fixtures/author'
 require 'fixtures/comment'
 require 'fixtures/company'
 require 'fixtures/topic'
@@ -129,10 +130,10 @@ class FinderTest < Test::Unit::TestCase
 
   def test_find_only_some_columns
     topic = Topic.find(1, :select => "author_name")
-    assert_raises(NoMethodError) { topic.title }
+    assert_raises(ActiveRecord::MissingAttributeError) {topic.title}
     assert_equal "David", topic.author_name
     assert !topic.attribute_present?("title")
-    assert !topic.respond_to?("title")
+    #assert !topic.respond_to?("title")
     assert topic.attribute_present?("author_name")
     assert topic.respond_to?("author_name")
   end