diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index d0322280d9..c0741bb572 100755 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1682,14 +1682,14 @@ module ActiveRecord relation = association.join_relation(relation) end - 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 = 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.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 = relation.conditions(construct_arel_limited_ids_condition(options, join_dependency)) if !using_limitable_reflections?(join_dependency.reflections) && ((scope && scope[:limit]) || options[:limit]) + relation = relation.limit(construct_limit(options[:limit], scope)) if using_limitable_reflections?(join_dependency.reflections) relation end @@ -1725,14 +1725,13 @@ module ActiveRecord relation = association.join_relation(relation) end - 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.select!(connection.distinct("#{connection.quote_table_name table_name}.#{primary_key}", construct_order(options[:order], scope(:find)).join(","))) + relation = 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)). + 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,12 +2221,12 @@ module ActiveRecord def join_relation(joining_relation, join = nil) if relation.is_a?(Array) - joining_relation.joins!(relation.first, join_type).on!(association_join.first) - joining_relation.joins!(relation.last, join_type).on!(association_join.last) + joining_relation. + joins(relation.first, join_type).on(association_join.first). + joins(relation.last, join_type).on(association_join.last) else - joining_relation.joins!(relation, join_type).on!(association_join) + joining_relation.joins(relation, join_type).on(association_join) end - joining_relation end protected diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2b79788b89..b23ce53d43 100755 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -869,15 +869,15 @@ module ActiveRecord #:nodoc: relation = arel_table if conditions = construct_conditions(conditions, scope) - relation.conditions!(Arel::SqlLiteral.new(conditions)) + relation = relation.conditions(Arel::SqlLiteral.new(conditions)) end - if options.has_key?(:limit) || (scope && scope[:limit]) + relation = 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.order!(construct_order(options[:order], scope)).limit!(construct_limit(options[:limit], scope)) + relation.order(construct_order(options[:order], scope)).limit(construct_limit(options[:limit], scope)) else - relation.order!(options[:order]) + relation.order(options[:order]) end relation.update(sanitize_sql_for_assignment(updates)) @@ -932,7 +932,7 @@ module ActiveRecord #:nodoc: # associations or call your before_* or +after_destroy+ callbacks, use the +destroy_all+ method instead. def delete_all(conditions = nil) if conditions - arel_table.conditions!(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 @@ -1535,7 +1535,7 @@ module ActiveRecord #:nodoc: def arel_table(table = nil) table = table_name if table.blank? - Relation.new(self, table) + Relation.new(self, Arel::Table.new(table)) end private @@ -1720,13 +1720,13 @@ module ActiveRecord #:nodoc: def construct_finder_arel(options = {}, scope = scope(:find)) # TODO add lock to Arel 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)) + 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)) @@ -1810,7 +1810,7 @@ module ActiveRecord #:nodoc: def build_association_joins(joins) join_dependency = ActiveRecord::Associations::ClassMethods::InnerJoinDependency.new(self, joins, nil) - relation = arel_table + relation = arel_table.relation join_dependency.join_associations.map { |association| if association.relation.is_a?(Array) [Arel::InnerJoin.new(relation, association.relation.first, association.association_join.first).joins(relation), @@ -2569,7 +2569,7 @@ module ActiveRecord #:nodoc: # be made (since they can't be persisted). def destroy unless new_record? - arel_table(true).conditions!(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 @@ -2864,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).conditions!(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 @@ -2952,7 +2952,7 @@ module ActiveRecord #:nodoc: end def arel_table(reload = nil) - @arel_table = Relation.new(self.class, self.class.table_name) if reload || @arel_table.nil? + @arel_table = Relation.new(self.class, Arel::Table.new(self.class.table_name)) if reload || @arel_table.nil? @arel_table end diff --git a/activerecord/lib/active_record/calculations.rb b/activerecord/lib/active_record/calculations.rb index cd2fbe9b79..40242333e5 100644 --- a/activerecord/lib/active_record/calculations.rb +++ b/activerecord/lib/active_record/calculations.rb @@ -147,11 +147,11 @@ module ActiveRecord construct_finder_arel_with_included_associations(options, join_dependency) else 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]) + 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.select!(operation == 'count' ? column.count(options[:distinct]) : column.send(operation)) + relation = 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,7 @@ module ActiveRecord options[:select] << ", #{group_field} AS #{group_alias}" - relation.select!(options[:select]) - relation.group!(construct_group(options[:group], options[:having], nil)) + relation = relation.select(options[:select]).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 9c7ba881fc..bba0d0844d 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -3,9 +3,9 @@ module ActiveRecord delegate :to_sql, :to => :relation attr_reader :relation, :klass - def initialize(klass, table = nil) + def initialize(klass, relation) @klass = klass - @relation = Arel::Table.new(table || @klass.table_name) + @relation = relation end def to_a @@ -21,60 +21,57 @@ module ActiveRecord to_a.first end - def select!(selection) - @relation = @relation.project(selection) if selection - self + def select(selects) + Relation.new(@klass, @relation.project(selects)) end - def on!(on) - @relation = @relation.on(on) if on - self + def group(groups) + Relation.new(@klass, @relation.group(groups)) end - def order!(order) - @relation = @relation.order(order) if order - self + def order(orders) + Relation.new(@klass, @relation.order(orders)) end - def group!(group) - @relation = @relation.group(group) if group - self + def limit(limits) + Relation.new(@klass, @relation.take(limits)) end - def limit!(limit) - @relation = @relation.take(limit) if limit - self + def offset(offsets) + Relation.new(@klass, @relation.skip(offsets)) end - def offset!(offset) - @relation = @relation.skip(offset) if offset - self + def on(join) + Relation.new(@klass, @relation.on(join)) end - def joins!(joins, join_type = nil) - if !joins.blank? - @relation = case joins - when String - @relation.join(joins) - when Hash, Array, Symbol - if @klass.send(:array_of_strings?, joins) - @relation.join(joins.join(' ')) + def joins(join, join_type = nil) + if join.blank? + self + else + join = case join + when String + @relation.join(join) + when Hash, Array, Symbol + if @klass.send(:array_of_strings?, join) + @relation.join(join.join(' ')) + else + @relation.join(@klass.send(:build_association_joins, join)) + end else - @relation.join(@klass.send(:build_association_joins, joins)) - end - else - @relation.join(joins, join_type) + @relation.join(join, join_type) end + Relation.new(@klass, join) end - self end - def conditions!(conditions) - if !conditions.blank? + def conditions(conditions) + if conditions.blank? + self + else conditions = @klass.send(:merge_conditions, conditions) if [String, Hash, Array].include?(conditions.class) - @relation = @relation.where(conditions) + Relation.new(@klass, @relation.where(conditions)) end - self end private diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b4f29f939a..df15c1a797 100755 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -1893,7 +1893,7 @@ class BasicsTest < ActiveRecord::TestCase end def test_all_with_conditions - assert_equal Developer.find(:all, :order => 'id desc'), Developer.all.order!('id desc').to_a + assert_equal Developer.find(:all, :order => 'id desc'), Developer.all.order('id desc').to_a end def test_find_ordered_last diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index e8b222f876..b017570b45 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -11,47 +11,47 @@ class RelationTest < ActiveRecord::TestCase fixtures :authors, :topics, :entrants, :developers, :companies, :developers_projects, :accounts, :categories, :categorizations, :posts def test_finding_with_conditions - assert_equal Author.find(:all, :conditions => "name = 'David'"), Author.all.conditions!("name = 'David'").to_a + assert_equal Author.find(:all, :conditions => "name = 'David'"), Author.all.conditions("name = 'David'").to_a end def test_finding_with_order - topics = Topic.all.order!('id') + topics = Topic.all.order('id') assert_equal 4, topics.size assert_equal topics(:first).title, topics.first.title end def test_finding_with_order_and_take - entrants = Entrant.all.order!("id ASC").limit!(2).to_a + entrants = Entrant.all.order("id ASC").limit(2).to_a assert_equal(2, entrants.size) assert_equal(entrants(:first).name, entrants.first.name) end def test_finding_with_order_limit_and_offset - entrants = Entrant.all.order!("id ASC").limit!(2).offset!(1) + entrants = Entrant.all.order("id ASC").limit(2).offset(1) assert_equal(2, entrants.size) assert_equal(entrants(:second).name, entrants.first.name) - entrants = Entrant.all.order!("id ASC").limit!(2).offset!(2) + entrants = Entrant.all.order("id ASC").limit(2).offset(2) assert_equal(1, entrants.size) assert_equal(entrants(:third).name, entrants.first.name) end def test_finding_with_group - developers = Developer.all.group!("salary").select!("salary").to_a + developers = Developer.all.group("salary").select("salary").to_a assert_equal 4, developers.size assert_equal 4, developers.map(&:salary).uniq.size end def test_finding_with_hash_conditions_on_joined_table - firms = DependentFirm.all.joins!(:account).conditions!({:name => 'RailsCore', :accounts => { :credit_limit => 55..60 }}).to_a + firms = DependentFirm.all.joins(:account).conditions({:name => 'RailsCore', :accounts => { :credit_limit => 55..60 }}).to_a assert_equal 1, firms.size assert_equal companies(:rails_core), firms.first end def test_find_all_with_join - developers_on_project_one = Developer.all.joins!('LEFT JOIN developers_projects ON developers.id = developers_projects.developer_id').conditions!('project_id=1').to_a + developers_on_project_one = Developer.all.joins('LEFT JOIN developers_projects ON developers.id = developers_projects.developer_id').conditions('project_id=1').to_a assert_equal 3, developers_on_project_one.length developer_names = developers_on_project_one.map { |d| d.name } @@ -60,11 +60,11 @@ class RelationTest < ActiveRecord::TestCase end def test_find_on_hash_conditions - assert_equal Topic.find(:all, :conditions => {:approved => false}), Topic.all.conditions!({ :approved => false }).to_a + assert_equal Topic.find(:all, :conditions => {:approved => false}), Topic.all.conditions({ :approved => false }).to_a end def test_joins_with_string_array - person_with_reader_and_post = Post.all.joins!([ + person_with_reader_and_post = Post.all.joins([ "INNER JOIN categorizations ON categorizations.post_id = posts.id", "INNER JOIN categories ON categories.id = categorizations.category_id AND categories.type = 'SpecialCategory'" ]