Use finder options as relation method names to provide more familiar

naming. Use bang methods convention in methods that alter the
relation.

4 files changed, 65 insertions, 50 deletions

diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb
index 406f08e247..d0322280d9 100755
--- a/activerecord/lib/active_record/associations.rb
+++ b/activerecord/lib/active_record/associations.rb
@@ -1681,15 +1681,15 @@ module ActiveRecord
           for association in join_dependency.join_associations
             relation = association.join_relation(relation)
           end
-          relation.join(construct_join(options[:joins], scope))
 
-          relation.where(construct_conditions(options[:conditions], scope))
-          relation.where(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
+          relation.joins!(construct_join(options[:joins], scope)).
+            select!(column_aliases(join_dependency)).
+            group!(construct_group(options[:group], options[:having], scope)).
+            order!(construct_order(options[:order], scope)).
+            conditions!(construct_conditions(options[:conditions], scope))
 
-          relation.project(column_aliases(join_dependency))
-          relation.group(construct_group(options[:group], options[:having], scope))
-          relation.order(construct_order(options[:order], scope))
-          relation.take(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections)
+          relation.conditions!(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit])
+          relation.limit!(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections)
 
           relation
         end
@@ -1724,15 +1724,15 @@ module ActiveRecord
           for association in join_dependency.join_associations
             relation = association.join_relation(relation)
           end
-          relation.join(construct_join(options[:joins], scope))
 
-          relation.where(construct_conditions(options[:conditions], scope))
-          relation.project(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", construct_order(options[:order], scope(:find)).join(",")))
+          relation.joins!(construct_join(options[:joins], scope)).
+            conditions!(construct_conditions(options[:conditions], scope)).
+            group!(construct_group(options[:group], options[:having], scope)).
+            order!(construct_order(options[:order], scope)).
+            limit!(construct_limit(options[:limit], scope)).
+            offset!(construct_limit(options[:offset], scope))
 
-          relation.group(construct_group(options[:group], options[:having], scope))
-          relation.order(construct_order(options[:order], scope))
-          relation.take(construct_limit(options[:limit], scope))
-          relation.skip(construct_limit(options[:offset], scope))
+          relation.select!(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", construct_order(options[:order], scope(:find)).join(",")))
 
           sanitize_sql(relation.to_sql)
         end
@@ -2222,10 +2222,10 @@ module ActiveRecord
 
             def join_relation(joining_relation, join = nil)
               if relation.is_a?(Array)
-                joining_relation.join(relation.first, join_type).on(association_join.first)
-                joining_relation.join(relation.last, join_type).on(association_join.last)
+                joining_relation.joins!(relation.first, join_type).on!(association_join.first)
+                joining_relation.joins!(relation.last, join_type).on!(association_join.last)
               else
-                joining_relation.join(relation, join_type).on(association_join)
+                joining_relation.joins!(relation, join_type).on!(association_join)
               end
               joining_relation
             end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index a6832b47ef..2b79788b89 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -866,18 +866,18 @@ module ActiveRecord #:nodoc:
       def update_all(updates, conditions = nil, options = {})
         scope = scope(:find)
 
-        relation = arel_table.relation
+        relation = arel_table
 
         if conditions = construct_conditions(conditions, scope)
-          relation = relation.where(Arel::SqlLiteral.new(conditions))
+          relation.conditions!(Arel::SqlLiteral.new(conditions))
         end
 
         if options.has_key?(:limit) || (scope && scope[:limit])
           # Only take order from scope if limit is also provided by scope, this
           # is useful for updating a has_many association with a limit.
-          relation = relation.order(construct_order(options[:order], scope)).take(construct_limit(options[:limit], scope))
+          relation.order!(construct_order(options[:order], scope)).limit!(construct_limit(options[:limit], scope))
         else
-          relation = relation.order(construct_order(options[:order], nil))
+          relation.order!(options[:order])
         end
 
         relation.update(sanitize_sql_for_assignment(updates))
@@ -932,7 +932,7 @@ module ActiveRecord #:nodoc:
       # associations or call your <tt>before_*</tt> or +after_destroy+ callbacks, use the +destroy_all+ method instead.
       def delete_all(conditions = nil)
         if conditions
-          arel_table.where(Arel::SqlLiteral.new(construct_conditions(conditions, scope(:find)))).delete
+          arel_table.conditions!(Arel::SqlLiteral.new(construct_conditions(conditions, scope(:find)))).delete
         else
           arel_table.delete
         end
@@ -1719,15 +1719,14 @@ module ActiveRecord #:nodoc:
 
         def construct_finder_arel(options = {}, scope = scope(:find))
           # TODO add lock to Arel
-          arel_table(options[:from]).
-            join(construct_join(options[:joins], scope)).
-            where(construct_conditions(options[:conditions], scope)).
-            project(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))).
-            group(construct_group(options[:group], options[:having], scope)).
-            order(construct_order(options[:order], scope)).
-            take(construct_limit(options[:limit], scope)).
-            skip(construct_offset(options[:offset], scope)
-          )
+          relation = arel_table(options[:from]).
+            joins!(construct_join(options[:joins], scope)).
+            conditions!(construct_conditions(options[:conditions], scope)).
+            select!(options[:select] || (scope && scope[:select]) || default_select(options[:joins] || (scope && scope[:joins]))).
+            group!(construct_group(options[:group], options[:having], scope)).
+            order!(construct_order(options[:order], scope)).
+            limit!(construct_limit(options[:limit], scope)).
+            offset!(construct_offset(options[:offset], scope))
         end
 
         def construct_finder_sql(options, scope = scope(:find))
@@ -2570,7 +2569,7 @@ module ActiveRecord #:nodoc:
       # be made (since they can't be persisted).
       def destroy
         unless new_record?
-          arel_table(true).where(arel_table[self.class.primary_key].eq(id)).delete
+          arel_table(true).conditions!(arel_table[self.class.primary_key].eq(id)).delete
         end
 
         @destroyed = true
@@ -2865,7 +2864,7 @@ module ActiveRecord #:nodoc:
       def update(attribute_names = @attributes.keys)
         attributes_with_values = arel_attributes_values(false, false, attribute_names)
         return 0 if attributes_with_values.empty?
-        arel_table(true).where(arel_table[self.class.primary_key].eq(id)).update(attributes_with_values)
+        arel_table(true).conditions!(arel_table[self.class.primary_key].eq(id)).update(attributes_with_values)
       end
 
       # Creates a record with values matching those of the instance attributes
diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb
index 82a171c6ad..cd2fbe9b79 100644
--- a/activerecord/lib/active_record/calculations.rb
+++ b/activerecord/lib/active_record/calculations.rb
@@ -146,12 +146,12 @@ module ActiveRecord
             join_dependency = ActiveRecord::Associations::ClassMethods::JoinDependency.new(self, merged_includes, construct_join(options[:joins], scope))
             construct_finder_arel_with_included_associations(options, join_dependency)
           else
-            arel_table(options[:from]).
-              join(construct_join(options[:joins], scope)).
-              where(construct_conditions(options[:conditions], scope)).
-              order(options[:order]).
-              take(options[:limit]).
-              skip(options[:offset])
+            relation = arel_table(options[:from]).
+              joins!(construct_join(options[:joins], scope)).
+              conditions!(construct_conditions(options[:conditions], scope)).
+              order!(options[:order]).
+              limit!(options[:limit]).
+              offset!(options[:offset])
           end
           if options[:group]
             return execute_grouped_calculation(operation, column_name, options, relation)
@@ -171,7 +171,7 @@ module ActiveRecord
                                (column_name == :all ? "*" : column_name.to_s))
         end
 
-        relation.project(operation == 'count' ? column.count(options[:distinct]) : column.send(operation))
+        relation.select!(operation == 'count' ? column.count(options[:distinct]) : column.send(operation))
 
         type_cast_calculated_value(connection.select_value(relation.to_sql), column_for(column_name), operation)
       end
@@ -194,8 +194,8 @@ module ActiveRecord
 
         options[:select] <<  ", #{group_field} AS #{group_alias}"
 
-        relation.project(options[:select])
-        relation.group(construct_group(options[:group], options[:having], nil))
+        relation.select!(options[:select])
+        relation.group!(construct_group(options[:group], options[:having], nil))
 
         calculated_data = connection.select_all(relation.to_sql)
 
diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb
index 456de73250..bbbb1da210 100644
--- a/activerecord/lib/active_record/relation.rb
+++ b/activerecord/lib/active_record/relation.rb
@@ -1,7 +1,7 @@
 module ActiveRecord
   class Relation
     delegate :delete, :to_sql, :to => :relation
-    CLAUSES_METHODS = ["project", "group", "order", "take", "skip", "on"].freeze
+    CLAUSES_METHODS = ["project", "where", "group", "order", "take", "skip", "on"].freeze
     attr_reader :relation, :klass
 
     def initialize(klass, table = nil)
@@ -24,25 +24,41 @@ module ActiveRecord
 
     for clause in CLAUSES_METHODS
       class_eval %{
-        def #{clause}(_#{clause})
+        def #{clause}!(_#{clause})
           @relation = @relation.#{clause}(_#{clause}) if _#{clause}
           self
         end
       }
     end
 
-    def join(joins, join_type = nil)
-      if !joins.blank?
-        if [String, Hash, Array, Symbol].include?(joins.class)
-          @relation = @relation.join(@klass.send(:construct_join, joins, nil))
+
+    def select!(selection)
+      @relation = @relation.project(selection) if selection
+      self
+    end
+
+    def limit!(limit)
+      @relation = @relation.take(limit) if limit
+      self
+    end
+
+    def offset!(offset)
+      @relation = @relation.skip(offset) if offset
+      self
+    end
+
+    def joins!(join, join_type = nil)
+      if !join.blank?
+        if [String, Hash, Array, Symbol].include?(join.class)
+          @relation = @relation.join(@klass.send(:construct_join, join, nil))
         else
-          @relation = @relation.join(joins, join_type)
+          @relation = @relation.join(join, join_type)
         end
       end
       self
     end
 
-    def where(conditions)
+    def conditions!(conditions)
       if !conditions.blank?
         conditions = @klass.send(:merge_conditions, conditions) if [String, Hash, Array].include?(conditions.class)
         @relation = @relation.where(conditions)