removing code complexity concerning attribute lookup.

14 files changed, 68 insertions, 107 deletions

diff --git a/lib/active_relation/primitives/attribute.rb b/lib/active_relation/primitives/attribute.rb
index e6df5cd87c..893feefef2 100644
--- a/lib/active_relation/primitives/attribute.rb
+++ b/lib/active_relation/primitives/attribute.rb
@@ -5,6 +5,10 @@ module ActiveRelation
     def initialize(relation, name, aliaz = nil, ancestor = nil)
       @relation, @name, @alias, @ancestor = relation, name, aliaz, ancestor
     end
+    
+    def alias_or_name
+      @alias || name
+    end
 
     module Transformations
       def as(aliaz = nil)
@@ -96,9 +100,11 @@ module ActiveRelation
       strategy.attribute prefix, name, self.alias
     end
     
-    private
-    delegate :hash, :to => :relation
+    def hash
+      relation.hash + name.hash
+    end
     
+    private
     def prefix
       relation.prefix_for(self)
     end
diff --git a/lib/active_relation/primitives/expression.rb b/lib/active_relation/primitives/expression.rb
index a64219e9d1..ff07c40109 100644
--- a/lib/active_relation/primitives/expression.rb
+++ b/lib/active_relation/primitives/expression.rb
@@ -15,12 +15,10 @@ module ActiveRelation
       end
     
       def as(aliaz)
-        # key line -- note self
         Expression.new(attribute, function_sql, aliaz, self)
       end
       
       def to_attribute
-        # key line -- note self
         Attribute.new(relation, @alias, nil, self)
       end
     end
@@ -39,7 +37,6 @@ module ActiveRelation
       !(history & other.send(:history)).empty?
     end
 
-    private
     def hash
       attribute.hash + function_sql.hash
     end
diff --git a/lib/active_relation/relations/compound.rb b/lib/active_relation/relations/compound.rb
index 776620732d..befb7f26d2 100644
--- a/lib/active_relation/relations/compound.rb
+++ b/lib/active_relation/relations/compound.rb
@@ -8,10 +8,5 @@ module ActiveRelation
     def attributes
       relation.attributes.collect { |a| a.substitute(self) }
     end
-    
-    protected
-    def attribute_for_name(name)
-      relation[name].substitute(self) rescue nil
-    end
   end
 end
\ No newline at end of file
diff --git a/lib/active_relation/relations/join.rb b/lib/active_relation/relations/join.rb
index 0327b5cd87..5b2efe727f 100644
--- a/lib/active_relation/relations/join.rb
+++ b/lib/active_relation/relations/join.rb
@@ -42,14 +42,6 @@ module ActiveRelation
         (relation2.send(:selects) unless relation2.aggregation?)
       ].compact.flatten
     end
-    
-    def attribute_for_name(name)
-      (relation1[name] || relation2[name])
-    end
-
-    def attribute_for_attribute(attribute)
-      (relation1[attribute] || relation2[attribute])
-    end
    
     def table_sql
       relation1.aggregation?? relation1.to_sql(Sql::Aggregation.new) : relation1.send(:table_sql)
diff --git a/lib/active_relation/relations/relation.rb b/lib/active_relation/relations/relation.rb
index 0f770e674c..c5409bc9ff 100644
--- a/lib/active_relation/relations/relation.rb
+++ b/lib/active_relation/relations/relation.rb
@@ -109,7 +109,7 @@ module ActiveRelation
     end
     
     def attribute_for_name(name)
-      nil
+      attributes.detect { |a| a.alias_or_name == name }
     end
     
     def attribute_for_attribute(attribute)
diff --git a/lib/active_relation/relations/rename.rb b/lib/active_relation/relations/rename.rb
index 8d92e9422c..c41cd1bdc5 100644
--- a/lib/active_relation/relations/rename.rb
+++ b/lib/active_relation/relations/rename.rb
@@ -19,26 +19,9 @@ module ActiveRelation
       relation.attributes.collect(&method(:substitute))
     end
     
-    protected
-    def attribute_for_name(name)
-      case
-      when referring_by_autonym?(name) then nil
-      when referring_by_pseudonym?(name) then substitute(relation[attribute])
-      else relation[name].substitute(self) rescue nil
-      end
-    end
-
     private
     def substitute(attribute)
       (attribute =~ self.attribute ? attribute.as(pseudonym) : attribute).substitute(self) rescue nil
     end
-
-    def referring_by_autonym?(name)
-      relation[name] == relation[attribute]
-    end
-    
-    def referring_by_pseudonym?(name)
-      name == pseudonym
-    end
   end
 end
\ No newline at end of file
diff --git a/lib/active_relation/relations/table.rb b/lib/active_relation/relations/table.rb
index 9e58404a51..d6ab45dcea 100644
--- a/lib/active_relation/relations/table.rb
+++ b/lib/active_relation/relations/table.rb
@@ -7,7 +7,9 @@ module ActiveRelation
     end
 
     def attributes
-      attributes_by_name.values
+      @attributes ||= connection.columns(name, "#{name} Columns").collect do |column|
+        Attribute.new(self, column.name.to_sym)
+      end
     end
 
     def qualify
@@ -23,21 +25,11 @@ module ActiveRelation
     end
     
     protected    
-    def attribute_for_name(name)
-      attributes_by_name[name.to_s]
-    end
-    
     def table_sql
       "#{quote_table_name(name)}"
     end
 
     private
-    def attributes_by_name
-      @attributes_by_name ||= connection.columns(name, "#{name} Columns").inject({}) do |attributes_by_name, column|
-        attributes_by_name.merge(column.name => Attribute.new(self, column.name.to_sym))
-      end
-    end
-
     def qualifications
       attributes.zip(attributes.collect(&:qualified_name)).to_hash
     end
diff --git a/spec/active_relation/integration/scratch_spec.rb b/spec/active_relation/integration/scratch_spec.rb
index a31c2ea9f6..4b02b22a11 100644
--- a/spec/active_relation/integration/scratch_spec.rb
+++ b/spec/active_relation/integration/scratch_spec.rb
@@ -101,7 +101,7 @@ describe 'ActiveRelation', 'A proposed refactoring to ActiveRecord, introducing
         for an association on an individual row' do
       users_cameras = photo_belongs_to_camera(user_has_many_photos(@users))
       users_cameras.to_sql.should be_like("""
-        SELECT `users`.`name`, `users`.`id`, `photos`.`id`, `photos`.`user_id`, `photos`.`camera_id`, `cameras`.`id`
+        SELECT `users`.`id`, `users`.`name`, `photos`.`id`, `photos`.`user_id`, `photos`.`camera_id`, `cameras`.`id`
         FROM `users`
           LEFT OUTER JOIN `photos`
             ON `users`.`id` = `photos`.`user_id`
@@ -113,7 +113,7 @@ describe 'ActiveRelation', 'A proposed refactoring to ActiveRecord, introducing
     it 'is trivial to disambiguate columns' do
       users_cameras = photo_belongs_to_camera(user_has_many_photos(@users)).qualify
       users_cameras.to_sql.should be_like("""
-        SELECT `users`.`name` AS 'users.name', `users`.`id` AS 'users.id', `photos`.`id` AS 'photos.id', `photos`.`user_id` AS 'photos.user_id', `photos`.`camera_id` AS 'photos.camera_id', `cameras`.`id` AS 'cameras.id'
+        SELECT `users`.`id` AS 'users.id', `users`.`name` AS 'users.name', `photos`.`id` AS 'photos.id', `photos`.`user_id` AS 'photos.user_id', `photos`.`camera_id` AS 'photos.camera_id', `cameras`.`id` AS 'cameras.id'
         FROM `users`
           LEFT OUTER JOIN `photos`
             ON `users`.`id` = `photos`.`user_id`
@@ -124,13 +124,13 @@ describe 'ActiveRelation', 'A proposed refactoring to ActiveRecord, introducing
     
     it 'allows arbitrary sql to be passed through' do
       @users.outer_join(@photos).on("asdf").to_sql.should be_like("""
-        SELECT `users`.`name`, `users`.`id`, `photos`.`id`, `photos`.`user_id`, `photos`.`camera_id`
+        SELECT `users`.`id`, `users`.`name`, `photos`.`id`, `photos`.`user_id`, `photos`.`camera_id`
         FROM `users`
           LEFT OUTER JOIN `photos`
             ON asdf
       """)
       @users.select("asdf").to_sql.should be_like("""
-        SELECT `users`.`name`, `users`.`id`
+        SELECT `users`.`id`, `users`.`name`
         FROM `users`
         WHERE asdf
       """)
diff --git a/spec/active_relation/relations/join_spec.rb b/spec/active_relation/relations/join_spec.rb
index 2df349dcd3..3b2ad88123 100644
--- a/spec/active_relation/relations/join_spec.rb
+++ b/spec/active_relation/relations/join_spec.rb
@@ -3,8 +3,8 @@ require File.join(File.dirname(__FILE__), '..', '..', 'spec_helper')
 module ActiveRelation
   describe Join do
     before do
-      @relation1 = Table.new(:foo)
-      @relation2 = Table.new(:bar)
+      @relation1 = Table.new(:users)
+      @relation2 = Table.new(:photos)
       @predicate = Equality.new(@relation1[:id], @relation2[:id])
     end
 
@@ -55,20 +55,20 @@ module ActiveRelation
       describe 'with simple relations' do
         it 'manufactures sql joining the two tables on the predicate' do
           Join.new("INNER JOIN", @relation1, @relation2, @predicate).to_sql.should be_like("""
-            SELECT `foo`.`name`, `foo`.`id`, `bar`.`name`, `bar`.`foo_id`, `bar`.`id`
-            FROM `foo`
-              INNER JOIN `bar` ON `foo`.`id` = `bar`.`id`
+            SELECT `users`.`id`, `users`.`name`, `photos`.`id`, `photos`.`user_id`, `photos`.`camera_id`
+            FROM `users`
+              INNER JOIN `photos` ON `users`.`id` = `photos`.`id`
           """)
         end
 
         it 'manufactures sql joining the two tables, merging any selects' do
           Join.new("INNER JOIN", @relation1.select(@relation1[:id].equals(1)),
                                  @relation2.select(@relation2[:id].equals(2)), @predicate).to_sql.should be_like("""
-            SELECT `foo`.`name`, `foo`.`id`, `bar`.`name`, `bar`.`foo_id`, `bar`.`id`
-            FROM `foo`
-              INNER JOIN `bar` ON `foo`.`id` = `bar`.`id`
-            WHERE `foo`.`id` = 1
-              AND `bar`.`id` = 2
+            SELECT `users`.`id`, `users`.`name`, `photos`.`id`, `photos`.`user_id`, `photos`.`camera_id`
+            FROM `users`
+              INNER JOIN `photos` ON `users`.`id` = `photos`.`id`
+            WHERE `users`.`id` = 1
+              AND `photos`.`id` = 2
           """)          
         end
       end
@@ -84,7 +84,7 @@ module ActiveRelation
         describe 'with the aggregation on the right' do
           it 'manufactures sql joining the left table to a derived table' do
             Join.new("INNER JOIN", @relation, @aggregate_relation, @predicate).to_sql.should be_like("""
-              SELECT `users`.`name`, `users`.`id`, `photo_count`.`user_id`, `photo_count`.`cnt`
+              SELECT `users`.`id`, `users`.`name`, `photo_count`.`user_id`, `photo_count`.`cnt`
               FROM `users`
                 INNER JOIN (SELECT `photos`.`user_id`, COUNT(`photos`.`id`) AS `cnt` FROM `photos` GROUP BY `photos`.`user_id`) AS `photo_count`
                   ON `photo_count`.`user_id` = `users`.`id`
@@ -95,7 +95,7 @@ module ActiveRelation
         describe 'with the aggregation on the left' do
           it 'manufactures sql joining the right table to a derived table' do
             Join.new("INNER JOIN", @aggregate_relation, @relation, @predicate).to_sql.should be_like("""
-              SELECT `photo_count`.`user_id`, `photo_count`.`cnt`, `users`.`name`, `users`.`id`
+              SELECT `photo_count`.`user_id`, `photo_count`.`cnt`, `users`.`id`, `users`.`name`
               FROM (SELECT `photos`.`user_id`, COUNT(`photos`.`id`) AS `cnt` FROM `photos` GROUP BY `photos`.`user_id`) AS `photo_count`
                 INNER JOIN `users`
                   ON `photo_count`.`user_id` = `users`.`id`
@@ -105,7 +105,7 @@ module ActiveRelation
 
         it "keeps selects on the aggregation within the derived table" do
           Join.new("INNER JOIN", @relation, @aggregate_relation.select(@aggregate_relation[:user_id].equals(1)), @predicate).to_sql.should be_like("""
-            SELECT `users`.`name`, `users`.`id`, `photo_count`.`user_id`, `photo_count`.`cnt`
+            SELECT `users`.`id`, `users`.`name`, `photo_count`.`user_id`, `photo_count`.`cnt`
             FROM `users`
               INNER JOIN (SELECT `photos`.`user_id`, COUNT(`photos`.`id`) AS `cnt` FROM `photos` WHERE `photos`.`user_id` = 1 GROUP BY `photos`.`user_id`) AS `photo_count`
                 ON `photo_count`.`user_id` = `users`.`id`
diff --git a/spec/active_relation/relations/order_spec.rb b/spec/active_relation/relations/order_spec.rb
index 204558daec..12b9761431 100644
--- a/spec/active_relation/relations/order_spec.rb
+++ b/spec/active_relation/relations/order_spec.rb
@@ -3,25 +3,23 @@ require File.join(File.dirname(__FILE__), '..', '..', 'spec_helper')
 module ActiveRelation
   describe Order do
     before do
-      @relation1 = Table.new(:foo)
-      @relation2 = Table.new(:bar)
-      @attribute1 = @relation1[:id]
-      @attribute2 = @relation2[:id]
+      @relation = Table.new(:users)
+      @attribute = @relation[:id]
     end
 
     describe '#qualify' do
       it "distributes over the relation and attributes" do
-        Order.new(@relation1, @attribute1).qualify. \
-          should == Order.new(@relation1.qualify, @attribute1.qualify)
+        Order.new(@relation, @attribute).qualify. \
+          should == Order.new(@relation.qualify, @attribute.qualify)
       end
     end
   
     describe '#to_sql' do
       it "manufactures sql with an order clause" do
-        Order.new(@relation1, @attribute1).to_sql.should be_like("""
-          SELECT `foo`.`name`, `foo`.`id`
-          FROM `foo`
-          ORDER BY `foo`.`id`
+        Order.new(@relation, @attribute).to_sql.should be_like("""
+          SELECT `users`.`id`, `users`.`name`
+          FROM `users`
+          ORDER BY `users`.`id`
         """)
       end
     end
diff --git a/spec/active_relation/relations/range_spec.rb b/spec/active_relation/relations/range_spec.rb
index 51171c759b..a2f84c2e88 100644
--- a/spec/active_relation/relations/range_spec.rb
+++ b/spec/active_relation/relations/range_spec.rb
@@ -3,25 +3,23 @@ require File.join(File.dirname(__FILE__), '..', '..', 'spec_helper')
 module ActiveRelation
   describe Range do
     before do
-      @relation1 = Table.new(:foo)
-      @relation2 = Table.new(:bar)
-      @range1 = 1..2
-      @range2 = 4..9
+      @relation = Table.new(:users)
+      @range = 4..9
     end
 
     describe '#qualify' do
       it "distributes over the relation" do
-        Range.new(@relation1, @range1).qualify.should == Range.new(@relation1.qualify, @range1)
+        Range.new(@relation, @range).qualify.should == Range.new(@relation.qualify, @range)
       end
     end
   
     describe '#to_sql' do
       it "manufactures sql with limit and offset" do
-        range_size = @range2.last - @range2.first + 1
-        range_start = @range2.first
-        Range.new(@relation1, @range2).to_s.should be_like("""
-          SELECT `foo`.`name`, `foo`.`id`
-          FROM `foo`
+        range_size = @range.last - @range.first + 1
+        range_start = @range.first
+        Range.new(@relation, @range).to_s.should be_like("""
+          SELECT `users`.`id`, `users`.`name`
+          FROM `users`
           LIMIT #{range_size}
           OFFSET #{range_start}
         """)
diff --git a/spec/active_relation/relations/rename_spec.rb b/spec/active_relation/relations/rename_spec.rb
index ebe95f13b9..695e51c191 100644
--- a/spec/active_relation/relations/rename_spec.rb
+++ b/spec/active_relation/relations/rename_spec.rb
@@ -3,15 +3,15 @@ require File.join(File.dirname(__FILE__), '..', '..', 'spec_helper')
 module ActiveRelation
   describe Rename do
     before do
-      @relation1 = Table.new(:foo)
-      @relation2 = Table.new(:bar)
+      @relation1 = Table.new(:users)
+      @relation2 = Table.new(:photos)
       @renamed_relation = Rename.new(@relation1, @relation1[:id] => :schmid)
     end
 
     describe '#initialize' do
       it "manufactures nested rename relations if multiple renames are provided" do
         Rename.new(@relation1, @relation1[:id] => :humpty, @relation1[:name] => :dumpty). \
-          should == Rename.new(Rename.new(@relation1, @relation1[:id] => :humpty), @relation1[:name] => :dumpty)
+          should == Rename.new(Rename.new(@relation1, @relation1[:name] => :dumpty), @relation1[:id] => :humpty)
       end
     end
     
@@ -50,6 +50,7 @@ module ActiveRelation
 
       describe 'when given an', Expression do
         it "manufactures a substituted and renamed expression if the expression is within the relation" do
+          pending
         end
       end
       
@@ -97,8 +98,8 @@ module ActiveRelation
     describe '#to_sql' do
       it 'manufactures sql renaming the attribute' do
         @renamed_relation.to_sql.should be_like("""
-          SELECT `foo`.`name`, `foo`.`id` AS 'schmid'
-          FROM `foo`
+          SELECT `users`.`id` AS 'schmid', `users`.`name`
+          FROM `users`
         """)
       end
     end
diff --git a/spec/active_relation/relations/selection_spec.rb b/spec/active_relation/relations/selection_spec.rb
index 95d9e68625..66ad54de19 100644
--- a/spec/active_relation/relations/selection_spec.rb
+++ b/spec/active_relation/relations/selection_spec.rb
@@ -3,39 +3,38 @@ require File.join(File.dirname(__FILE__), '..', '..', 'spec_helper')
 module ActiveRelation
   describe Selection do
     before do
-      @relation1 = Table.new(:foo)
-      @relation2 = Table.new(:bar)
-      @predicate1 = Equality.new(@relation1[:id], @relation2[:foo_id])
-      @predicate2 = LessThan.new(@relation1[:age], 2)
+      @relation = Table.new(:users)
+      @predicate = Equality.new(@relation[:id], 1)
     end
   
     describe '#initialize' do
       it "manufactures nested selection relations if multiple predicates are provided" do
-        Selection.new(@relation1, @predicate1, @predicate2). \
-          should == Selection.new(Selection.new(@relation1, @predicate2), @predicate1)
+        @predicate2 = LessThan.new(@relation[:age], 2)
+        Selection.new(@relation, @predicate, @predicate2). \
+          should == Selection.new(Selection.new(@relation, @predicate2), @predicate)
       end
     end
   
     describe '#qualify' do
       it "distributes over the relation and predicates" do
-        Selection.new(@relation1, @predicate1).qualify. \
-          should == Selection.new(@relation1.qualify, @predicate1.qualify)
+        Selection.new(@relation, @predicate).qualify. \
+          should == Selection.new(@relation.qualify, @predicate.qualify)
       end
     end
   
     describe '#to_sql' do
       it "manufactures sql with where clause conditions" do
-        Selection.new(@relation1, @predicate1).to_sql.should be_like("""
-          SELECT `foo`.`name`, `foo`.`id`
-          FROM `foo`
-          WHERE `foo`.`id` = `bar`.`foo_id`
+        Selection.new(@relation, @predicate).to_sql.should be_like("""
+          SELECT `users`.`id`, `users`.`name`
+          FROM `users`
+          WHERE `users`.`id` = 1
         """)
       end
     
       it "allows arbitrary sql" do
-        Selection.new(@relation1, "asdf").to_sql.should be_like("""
-          SELECT `foo`.`name`, `foo`.`id`
-          FROM `foo`
+        Selection.new(@relation, "asdf").to_sql.should be_like("""
+          SELECT `users`.`id`, `users`.`name`
+          FROM `users`
           WHERE asdf
         """)
       end
diff --git a/spec/active_relation/relations/table_spec.rb b/spec/active_relation/relations/table_spec.rb
index e48d259fb8..03b4836e80 100644
--- a/spec/active_relation/relations/table_spec.rb
+++ b/spec/active_relation/relations/table_spec.rb
@@ -40,7 +40,7 @@ module ActiveRelation
     describe '#to_sql' do
       it "manufactures a simple select query" do
         @table.to_sql.should be_like("""
-          SELECT `users`.`name`, `users`.`id`
+          SELECT `users`.`id`, `users`.`name`
           FROM `users`
         """)
       end
@@ -61,8 +61,8 @@ module ActiveRelation
     describe '#attributes' do
       it 'manufactures attributes corresponding to columns in the table' do
         @table.attributes.should == [
-          Attribute.new(@table, :name),
-          Attribute.new(@table, :id)
+          Attribute.new(@table, :id),
+          Attribute.new(@table, :name)
         ]
       end
     end