Fix bug introduced by changeset 3679 which caused custom attribute? methods to be overridden. Also ensure that ? methods are defined even if read method is customised. (closes #3677) [jonathan@bluewire.net.nz]

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

3 files changed, 31 insertions, 9 deletions

diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index a4505733de..facb22c0e9 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -1676,8 +1676,9 @@ module ActiveRecord #:nodoc:
       # ActiveRecord::Base.generate_read_methods is set to true.
       def define_read_methods
         self.class.columns_hash.each do |name, column|
-          unless self.class.serialized_attributes[name] || respond_to_without_attributes?(name)
-            define_read_method(name.to_sym, name, column)
+          unless self.class.serialized_attributes[name]
+            define_read_method(name.to_sym, name, column) unless respond_to_without_attributes?(name)
+            define_question_method(name)     unless respond_to_without_attributes?("#{name}?")
           end
         end
       end
@@ -1686,19 +1687,30 @@ module ActiveRecord #:nodoc:
       def define_read_method(symbol, attr_name, column)
         cast_code = column.type_cast_code('v') if column
         access_code = cast_code ? "(v=@attributes['#{attr_name}']) && #{cast_code}" : "@attributes['#{attr_name}']"
-
+        
         unless attr_name.to_s == self.class.primary_key.to_s
           access_code = access_code.insert(0, "raise NoMethodError, 'missing attribute: #{attr_name}', caller unless @attributes.has_key?('#{attr_name}'); ")
           self.class.read_methods << attr_name
+        end
+        
+        evaluate_read_method attr_name, "def #{symbol}; #{access_code}; end"
+      end
+      
+      # Define an attribute ? method.
+      def define_question_method(attr_name)
+        unless attr_name.to_s == self.class.primary_key.to_s
           self.class.read_methods << "#{attr_name}?"
         end
-
+        
+        evaluate_read_method attr_name, "def #{attr_name}?; query_attribute('#{attr_name}'); end"
+      end
+      
+      # Evaluate the definition for an attribute reader or ? method
+      def evaluate_read_method(attr_name, method_definition)
         begin
-          self.class.class_eval("def #{symbol}; #{access_code}; end")
-          self.class.class_eval("def #{symbol}?; query_attribute('#{attr_name}'); end")
+          self.class.class_eval(method_definition)
         rescue SyntaxError => err
           self.class.read_methods.delete(attr_name)
-          self.class.read_methods.delete("#{attr_name}?")
           if logger
             logger.warn "Exception occured during reader method compilation."
             logger.warn "Maybe #{attr_name} is not a valid Ruby identifier?"
diff --git a/activerecord/test/base_test.rb b/activerecord/test/base_test.rb
index 35b00b878a..9f79fc9b13 100755
--- a/activerecord/test/base_test.rb
+++ b/activerecord/test/base_test.rb
@@ -266,7 +266,7 @@ class BasicsTest < Test::Unit::TestCase
     if ActiveRecord::Base.generate_read_methods
       assert_readers(Topic,  %w(type replies_count))
       assert_readers(Firm,   %w(type))
-      assert_readers(Client, %w(type))
+      assert_readers(Client, %w(type ruby_type rating?))
     else
       [Topic, Firm, Client].each {|klass| assert_equal klass.read_methods, {}}
     end
@@ -1282,8 +1282,9 @@ class BasicsTest < Test::Unit::TestCase
   
   private
     def assert_readers(model, exceptions)
-      expected_readers = Set.new(model.column_names - (model.serialized_attributes.keys + exceptions + ['id']))
+      expected_readers = Set.new(model.column_names - (model.serialized_attributes.keys + ['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/fixtures/company.rb b/activerecord/test/fixtures/company.rb
index b0694ff361..59a7cdd13e 100755
--- a/activerecord/test/fixtures/company.rb
+++ b/activerecord/test/fixtures/company.rb
@@ -57,6 +57,15 @@ class Client < Company
     end
     true
   end
+  
+  # Used to test that read and question methods are not generated for these attributes
+  def ruby_type
+    read_attribute :ruby_type
+  end
+  
+  def rating?
+    query_attribute :rating
+  end
 end