diff options
36 files changed, 334 insertions, 128 deletions
diff --git a/actionpack/lib/action_controller/metal.rb b/actionpack/lib/action_controller/metal.rb index 7fcc4e7211..ace1aabe03 100644 --- a/actionpack/lib/action_controller/metal.rb +++ b/actionpack/lib/action_controller/metal.rb @@ -87,6 +87,7 @@ module ActionController @_status = 200 @_request = nil @_response = nil + @_routes = nil super end diff --git a/actionpack/lib/action_controller/metal/renderers.rb b/actionpack/lib/action_controller/metal/renderers.rb index 0be07cd1fc..f9b226b7c9 100644 --- a/actionpack/lib/action_controller/metal/renderers.rb +++ b/actionpack/lib/action_controller/metal/renderers.rb @@ -71,7 +71,7 @@ module ActionController end add :json do |json, options| - json = ActiveSupport::JSON.encode(json, options) unless json.respond_to?(:to_str) + json = json.to_json(options) unless json.respond_to?(:to_str) json = "#{options[:callback]}(#{json})" unless options[:callback].blank? self.content_type ||= Mime::JSON self.response_body = json diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 71a320ce9c..5d18dfe369 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -302,9 +302,9 @@ module ActionDispatch extend ActiveSupport::Concern include UrlFor - @routes = routes + @_routes = routes class << self - delegate :url_for, :to => '@routes' + delegate :url_for, :to => '@_routes' end extend routes.named_routes.module @@ -316,7 +316,7 @@ module ActionDispatch singleton_class.send(:redefine_method, :_routes) { routes } end - define_method(:_routes) { @_routes ||= nil; @_routes || routes } + define_method(:_routes) { @_routes || routes } end helpers diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 43cbba8a0a..83434a9340 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -395,12 +395,12 @@ module ActionView # <b>Note:</b> Only the <tt><optgroup></tt> and <tt><option></tt> tags are returned, so you still have to # wrap the output in an appropriate <tt><select></tt> tag. def option_groups_from_collection_for_select(collection, group_method, group_label_method, option_key_method, option_value_method, selected_key = nil) - collection.inject("") do |options_for_select, group| + collection.map do |group| group_label_string = eval("group.#{group_label_method}") - options_for_select += "<optgroup label=\"#{html_escape(group_label_string)}\">" - options_for_select += options_from_collection_for_select(eval("group.#{group_method}"), option_key_method, option_value_method, selected_key) - options_for_select += '</optgroup>' - end.html_safe + "<optgroup label=\"#{html_escape(group_label_string)}\">" + + options_from_collection_for_select(eval("group.#{group_method}"), option_key_method, option_value_method, selected_key) + + '</optgroup>' + end.join.html_safe end # Returns a string of <tt><option></tt> tags, like <tt>options_for_select</tt>, but diff --git a/actionpack/test/controller/render_json_test.rb b/actionpack/test/controller/render_json_test.rb index 5958b18d80..6dd2a9f23d 100644 --- a/actionpack/test/controller/render_json_test.rb +++ b/actionpack/test/controller/render_json_test.rb @@ -9,6 +9,10 @@ class RenderJsonTest < ActionController::TestCase hash.except!(*options[:except]) if options[:except] hash end + + def to_json(options = {}) + super :except => [:c, :e] + end end class TestController < ActionController::Base @@ -49,6 +53,10 @@ class RenderJsonTest < ActionController::TestCase def render_json_with_extra_options render :json => JsonRenderable.new, :except => [:c, :e] end + + def render_json_without_options + render :json => JsonRenderable.new + end end tests TestController @@ -109,4 +117,9 @@ class RenderJsonTest < ActionController::TestCase assert_equal '{"a":"b"}', @response.body assert_equal 'application/json', @response.content_type end + + def test_render_json_calls_to_json_from_object + get :render_json_without_options + assert_equal '{"a":"b"}', @response.body + end end diff --git a/activemodel/test/cases/callbacks_test.rb b/activemodel/test/cases/callbacks_test.rb index 64dc7b5026..069d907fb2 100644 --- a/activemodel/test/cases/callbacks_test.rb +++ b/activemodel/test/cases/callbacks_test.rb @@ -81,4 +81,34 @@ class CallbacksTest < ActiveModel::TestCase assert !ModelCallbacks.respond_to?(:around_empty) assert !ModelCallbacks.respond_to?(:after_empty) end + + class Violin + attr_reader :history + def initialize + @history = [] + end + extend ActiveModel::Callbacks + define_model_callbacks :create + def callback1; self.history << 'callback1'; end + def callback2; self.history << 'callback2'; end + def create + _run_create_callbacks {} + self + end + end + class Violin1 < Violin + after_create :callback1, :callback2 + end + class Violin2 < Violin + after_create :callback1 + after_create :callback2 + end + + test "after_create callbacks with both callbacks declared in one line" do + assert_equal ["callback1", "callback2"], Violin1.new.create.history + end + test "after_create callbacks with both callbacks declared in differnt lines" do + assert_equal ["callback1", "callback2"], Violin2.new.create.history + end + end diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index f5291b180e..2157a0aded 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1014,8 +1014,9 @@ module ActiveRecord #:nodoc: end def all_attributes_exists?(attribute_names) - attribute_names = expand_attribute_names_for_aggregates(attribute_names) - attribute_names.all? { |name| column_methods_hash.include?(name.to_sym) } + expand_attribute_names_for_aggregates(attribute_names).all? { |name| + column_methods_hash.include?(name.to_sym) + } end protected @@ -1383,10 +1384,7 @@ MSG ensure_proper_type - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| self.send("#{att}=", value) } if create_with - end + populate_with_current_scope_attributes self.attributes = attributes unless attributes.nil? result = yield self if block_given? @@ -1415,10 +1413,7 @@ MSG @new_record = true ensure_proper_type - if scope = self.class.send(:current_scoped_methods) - create_with = scope.scope_for_create - create_with.each { |att,value| self.send("#{att}=", value) } if create_with - end + populate_with_current_scope_attributes end # Returns a String, which Action Pack uses for constructing an URL to this @@ -1807,6 +1802,13 @@ MSG return string unless string.is_a?(String) && string =~ /^---/ YAML::load(string) rescue string end + + def populate_with_current_scope_attributes + if scope = self.class.send(:current_scoped_methods) + create_with = scope.scope_for_create + create_with.each { |att,value| self.respond_to?(:"#{att}=") && self.send("#{att}=", value) } if create_with + end + end end Base.class_eval do diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 6c29e67e17..ce6782aac7 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -327,14 +327,12 @@ module ActiveRecord # # Note: SQLite doesn't support index length def add_index(table_name, column_name, options = {}) - options[:name] = options[:name].to_s if options.key?(:name) - column_names = Array.wrap(column_name) index_name = index_name(table_name, :column => column_names) if Hash === options # legacy support, since this param was a string index_type = options[:unique] ? "UNIQUE" : "" - index_name = options[:name] || index_name + index_name = options[:name].to_s if options.key?(:name) else index_type = options end @@ -404,7 +402,8 @@ module ActiveRecord # as there's no way to determine the correct answer in that case. def index_name_exists?(table_name, index_name, default) return default unless respond_to?(:indexes) - indexes(table_name).detect { |i| i.name == index_name.to_s } + index_name = index_name.to_s + indexes(table_name).detect { |i| i.name == index_name } end # Returns a string of <tt>CREATE TABLE</tt> SQL statement(s) for recreating the diff --git a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb index 0571e0cd14..c0cc7ba20d 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite_adapter.rb @@ -360,8 +360,8 @@ module ActiveRecord end def copy_table_contents(from, to, columns, rename = {}) #:nodoc: - column_mappings = Hash[*columns.map {|name| [name, name]}.flatten] - rename.inject(column_mappings) {|map, a| map[a.last] = a.first; map} + column_mappings = Hash[columns.map {|name| [name, name]}] + rename.each { |a| column_mappings[a.last] = a.first } from_columns = columns(from).collect {|col| col.name} columns = columns.find_all{|col| from_columns.include?(column_mappings[col])} quoted_columns = columns.map { |col| quote_column_name(col) } * ',' diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 533bc331ae..b309df9b1b 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -6,40 +6,43 @@ module ActiveRecord # class DynamicFinderMatch def self.match(method) - df_match = self.new(method) - df_match.finder ? df_match : nil - end - - def initialize(method) - @finder = :first - @bang = false - @instantiator = nil + finder = :first + bang = false + instantiator = nil case method.to_s - when /^find_(all_by|last_by|by)_([_a-zA-Z]\w*)$/ - @finder = :last if $1 == 'last_by' - @finder = :all if $1 == 'all_by' + when /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ + finder = :last if $1 == 'last_' + finder = :all if $1 == 'all_' names = $2 when /^find_by_([_a-zA-Z]\w*)\!$/ - @bang = true + bang = true names = $1 when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ - @instantiator = $1 == 'initialize' ? :new : :create + instantiator = $1 == 'initialize' ? :new : :create names = $2 else - @finder = nil + return nil end - @attribute_names = names && names.split('_and_') + + new(finder, instantiator, bang, names.split('_and_')) + end + + def initialize(finder, instantiator, bang, attribute_names) + @finder = finder + @instantiator = instantiator + @bang = bang + @attribute_names = attribute_names end attr_reader :finder, :attribute_names, :instantiator def finder? - !@finder.nil? && @instantiator.nil? + @finder && !@instantiator end def instantiator? - @finder == :first && !@instantiator.nil? + @finder == :first && @instantiator end def creator? diff --git a/activerecord/lib/active_record/dynamic_scope_match.rb b/activerecord/lib/active_record/dynamic_scope_match.rb index 61c3ea0e7f..c832e927d6 100644 --- a/activerecord/lib/active_record/dynamic_scope_match.rb +++ b/activerecord/lib/active_record/dynamic_scope_match.rb @@ -8,25 +8,16 @@ module ActiveRecord # scope except that it's dynamic. class DynamicScopeMatch def self.match(method) - ds_match = self.new(method) - ds_match.scope ? ds_match : nil + return unless method.to_s =~ /^scoped_by_([_a-zA-Z]\w*)$/ + new(true, $1 && $1.split('_and_')) end - def initialize(method) - @scope = true - case method.to_s - when /^scoped_by_([_a-zA-Z]\w*)$/ - names = $1 - else - @scope = nil - end - @attribute_names = names && names.split('_and_') + def initialize(scope, attribute_names) + @scope = scope + @attribute_names = attribute_names end attr_reader :scope, :attribute_names - - def scope? - !@scope.nil? - end + alias :scope? :scope end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 826031a3e3..6fb723f2f5 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -704,11 +704,9 @@ class Fixtures < (RUBY_VERSION < '1.9' ? YAML::Omap : Hash) end def read_yaml_fixture_files - yaml_string = "" - Dir["#{@fixture_path}/**/*.yml"].select { |f| test(?f, f) }.each do |subfixture_path| - yaml_string << IO.read(subfixture_path) - end - yaml_string << IO.read(yaml_file_path) + yaml_string = (Dir["#{@fixture_path}/**/*.yml"].select { |f| + File.file?(f) + } + [yaml_file_path]).map { |file_path| IO.read(file_path) }.join if yaml = parse_yaml_string(yaml_string) # If the file is an ordered map, extract its children. diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 7372ab3278..bdd940f3ee 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -377,7 +377,12 @@ module ActiveRecord end if attributes_collection.is_a? Hash - attributes_collection = attributes_collection.sort_by { |index, _| index.to_i }.map { |_, attributes| attributes } + keys = attributes_collection.keys + attributes_collection = if keys.include?('id') || keys.include?(:id) + Array.wrap(attributes_collection) + else + attributes_collection.sort_by { |i, _| i.to_i }.map { |_, attributes| attributes } + end end association = send(association_name) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 478f1e8ef1..04ba5b291e 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -326,7 +326,11 @@ module ActiveRecord def scope_for_create @scope_for_create ||= begin - @create_with_value || where_values_hash + if @create_with_value + @create_with_value.reverse_merge(where_values_hash) + else + where_values_hash + end end end @@ -358,15 +362,6 @@ module ActiveRecord scoping { @klass.send(method, *args, &block) } elsif arel.respond_to?(method) arel.send(method, *args, &block) - elsif match = DynamicFinderMatch.match(method) - attributes = match.attribute_names - super unless @klass.send(:all_attributes_exists?, attributes) - - if match.finder? - find_by_attributes(match, attributes, *args) - elsif match.instantiator? - find_or_instantiator_by_attributes(match, attributes, *args, &block) - end else super end diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 12a2c6aec3..03862c78e4 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -191,7 +191,11 @@ module ActiveRecord end # Postgresql doesn't like ORDER BY when there are no GROUP BY - relation = except(:order).select(operation == 'count' ? column.count(distinct) : column.send(operation)) + relation = except(:order) + select_value = operation == 'count' ? column.count(distinct) : column.send(operation) + + relation.select_values = [select_value] + type_cast_calculated_value(@klass.connection.select_value(relation.to_sql), column_for(column_name), operation) end @@ -208,21 +212,22 @@ module ActiveRecord aggregate_alias = column_alias_for(operation, column_name) select_statement = if operation == 'count' && column_name == :all - "COUNT(*) AS count_all" + ["COUNT(*) AS count_all"] else - Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias).to_sql + [Arel::Attribute.new(@klass.unscoped.table, column_name).send(operation).as(aggregate_alias)] end - select_statement << ", #{group_field} AS #{group_alias}" + select_statement << "#{group_field} AS #{group_alias}" - relation = except(:group).select(select_statement).group(group) + relation = except(:group).group(group) + relation.select_values = select_statement calculated_data = @klass.connection.select_all(relation.to_sql) if association key_ids = calculated_data.collect { |row| row[group_alias] } key_records = association.klass.base_class.find(key_ids) - key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } + key_records = Hash[key_records.map { |r| [r.id, r] }] end ActiveSupport::OrderedHash[calculated_data.map do |row| diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index c97b1a24d2..2e0a2effc2 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -36,7 +36,7 @@ module ActiveRecord to_a.select {|*block_args| value.call(*block_args) } else relation = clone - relation.select_values += [value] + relation.select_values += Array.wrap(value) relation end end @@ -149,7 +149,7 @@ module ActiveRecord end def custom_join_sql(*joins) - arel = table + arel = table.select_manager joins.each do |join| next if join.blank? @@ -157,16 +157,13 @@ module ActiveRecord @implicit_readonly = true case join - when Hash, Array, Symbol - if array_of_strings?(join) - join_string = join.join(' ') - arel = arel.join(Arel::SqlLiteral.new(join_string)) - end + when Array + join = Arel.sql(join.join(' ')) if array_of_strings?(join) when String - arel = arel.join(Arel::SqlLiteral.new(join)) - else - arel = arel.join(join) + join = Arel.sql(join) end + + arel.join(join) end arel.joins(arel) @@ -178,13 +175,8 @@ module ActiveRecord arel = build_joins(arel, @joins_values) unless @joins_values.empty? (@where_values - ['']).uniq.each do |where| - case where - when Arel::SqlLiteral - arel = arel.where(where) - else - sql = where.is_a?(String) ? where : where.to_sql - arel = arel.where(Arel::SqlLiteral.new("(#{sql})")) - end + where = Arel.sql(where) if String === where + arel = arel.where(Arel::Nodes::Grouping.new(where)) end arel = arel.having(*@having_values.uniq.reject{|h| h.blank?}) unless @having_values.empty? @@ -259,16 +251,7 @@ module ActiveRecord def build_select(arel, selects) unless selects.empty? @implicit_readonly = false - # TODO: fix this ugly hack, we should refactor the callers to get an Arel compatible array. - # Before this change we were passing to Arel the last element only, and Arel is capable of handling an array - case select = selects.last - when Arel::Expression, Arel::SqlLiteral - arel.project(select) - when /^COUNT\(/ - arel.project(Arel::SqlLiteral.new(select)) - else - arel.project(*selects) - end + arel.project(*selects) else arel.project(Arel::SqlLiteral.new(@klass.quoted_table_name + '.*')) end @@ -282,15 +265,9 @@ module ActiveRecord end def reverse_sql_order(order_query) - order_query.join(', ').split(',').collect { |s| - if s.match(/\s(asc|ASC)$/) - s.gsub(/\s(asc|ASC)$/, ' DESC') - elsif s.match(/\s(desc|DESC)$/) - s.gsub(/\s(desc|DESC)$/, ' ASC') - else - s + ' DESC' - end - } + order_query.join(', ').split(',').collect do |s| + s.gsub!(/\sasc\Z/i, ' DESC') || s.gsub!(/\sdesc\Z/i, ' ASC') || s.concat(' DESC') + end end def array_of_strings?(o) diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 742513230e..cbaa4990f7 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -474,4 +474,9 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase Author.belongs_to :special_author_address, :dependent => :restrict end end + + def test_attributes_are_being_set_when_initialized_from_belongs_to_association_with_where_clause + new_firm = accounts(:signals37).build_firm(:name => 'Apple') + assert_equal new_firm.name, "Apple" + end end diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 2bdf9d8971..c0be7dfdcc 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -848,4 +848,14 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_queries(0) { david.projects.columns; david.projects.columns } end + def test_attributes_are_being_set_when_initialized_from_habm_association_with_where_clause + new_developer = projects(:action_controller).developers.where(:name => "Marcelo").build + assert_equal new_developer.name, "Marcelo" + end + + def test_attributes_are_being_set_when_initialized_from_habm_association_with_multiple_where_clauses + new_developer = projects(:action_controller).developers.where(:name => "Marcelo").where(:salary => 90_000).build + assert_equal new_developer.name, "Marcelo" + assert_equal new_developer.salary, 90_000 + end end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index efabf74e13..720b7fc386 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1255,4 +1255,16 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end EOF end + + def test_attributes_are_being_set_when_initialized_from_has_many_association_with_where_clause + new_comment = posts(:welcome).comments.where(:body => "Some content").build + assert_equal new_comment.body, "Some content" + end + + def test_attributes_are_being_set_when_initialized_from_has_many_association_with_multiple_where_clauses + new_comment = posts(:welcome).comments.where(:body => "Some content").where(:type => 'SpecialComment').build + assert_equal new_comment.body, "Some content" + assert_equal new_comment.type, "SpecialComment" + assert_equal new_comment.post_id, posts(:welcome).id + end end diff --git a/activerecord/test/cases/associations/has_many_through_associations_test.rb b/activerecord/test/cases/associations/has_many_through_associations_test.rb index 45f8bd64eb..0dac633852 100644 --- a/activerecord/test/cases/associations/has_many_through_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_through_associations_test.rb @@ -421,4 +421,18 @@ class HasManyThroughAssociationsTest < ActiveRecord::TestCase assert_raises(ActiveRecord::RecordNotFound) {company.developer_ids= ids} end + def test_build_a_model_from_hm_through_association_with_where_clause + assert_nothing_raised { books(:awdr).subscribers.where(:nick => "marklazz").build } + end + + def test_attributes_are_being_set_when_initialized_from_hm_through_association_with_where_clause + new_subscriber = books(:awdr).subscribers.where(:nick => "marklazz").build + assert_equal new_subscriber.nick, "marklazz" + end + + def test_attributes_are_being_set_when_initialized_from_hm_through_association_with_multiple_where_clauses + new_subscriber = books(:awdr).subscribers.where(:nick => "marklazz").where(:name => 'Marcelo Giorgi').build + assert_equal new_subscriber.nick, "marklazz" + assert_equal new_subscriber.name, "Marcelo Giorgi" + end end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index e959ed46cc..b522be3fe0 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -326,4 +326,9 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert !account.new_record? assert_equal 500, account.credit_limit end + + def test_attributes_are_being_set_when_initialized_from_has_one_association_with_where_clause + new_account = companies(:first_firm).build_account(:firm_name => 'Account') + assert_equal new_account.firm_name, "Account" + end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index b434b0faa8..16fd9a7465 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -69,6 +69,24 @@ class BasicsTest < ActiveRecord::TestCase end end + def test_use_table_engine_for_quoting_where + relation = Topic.where(Topic.arel_table[:id].eq(1)) + engine = relation.table.engine + + fakepool = Class.new(Struct.new(:spec)) { + def with_connection; yield self; end + def connection_pool; self; end + def quote_table_name(*args); raise "lol quote_table_name"; end + } + + relation.table.engine = fakepool.new(engine.connection_pool.spec) + + error = assert_raises(RuntimeError) { relation.to_a } + assert_match('lol', error.message) + ensure + relation.table.engine = engine + end + def test_preserving_time_objects assert_kind_of( Time, Topic.find(1).bonus_time, diff --git a/activerecord/test/cases/dynamic_finder_match_test.rb b/activerecord/test/cases/dynamic_finder_match_test.rb new file mode 100644 index 0000000000..64bf6cb508 --- /dev/null +++ b/activerecord/test/cases/dynamic_finder_match_test.rb @@ -0,0 +1,49 @@ +require "cases/helper" + +module ActiveRecord + class DynamicFinderMatchTest < ActiveRecord::TestCase + def test_find_by + m = DynamicFinderMatch.match(:find_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_all_by + m = DynamicFinderMatch.match(:find_all_by_foo) + assert_equal :all, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_last_by + m = DynamicFinderMatch.match(:find_last_by_foo) + assert_equal :last, m.finder + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_by! + m = DynamicFinderMatch.match(:find_by_foo!) + assert_equal :first, m.finder + assert m.bang?, 'should be banging' + assert_equal %w{ foo }, m.attribute_names + end + + def test_find_or_create + m = DynamicFinderMatch.match(:find_or_create_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + assert_equal :create, m.instantiator + end + + def test_find_or_initialize + m = DynamicFinderMatch.match(:find_or_initialize_by_foo) + assert_equal :first, m.finder + assert_equal %w{ foo }, m.attribute_names + assert_equal :new, m.instantiator + end + + def test_garbage + assert !DynamicFinderMatch.match(:fooo), 'should be false' + assert !DynamicFinderMatch.match(:find_by), 'should be false' + end + end +end diff --git a/activerecord/test/cases/method_scoping_test.rb b/activerecord/test/cases/method_scoping_test.rb index ffe16ffdfa..f3d3d62830 100644 --- a/activerecord/test/cases/method_scoping_test.rb +++ b/activerecord/test/cases/method_scoping_test.rb @@ -219,7 +219,7 @@ class MethodScopingTest < ActiveRecord::TestCase new_comment = nil VerySpecialComment.send(:with_scope, :create => { :post_id => 1 }) do - assert_equal({:post_id => 1}, VerySpecialComment.scoped.send(:scope_for_create)) + assert_equal({:post_id => 1, :type => 'VerySpecialComment' }, VerySpecialComment.scoped.send(:scope_for_create)) new_comment = VerySpecialComment.create :body => "Wonderful world" end diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 8c75500998..5242d78033 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -124,6 +124,13 @@ if ActiveRecord::Base.connection.supports_migrations? end end + def test_index_symbol_names + assert_nothing_raised { Person.connection.add_index :people, :primary_contact_id, :name => :symbol_index_name } + assert Person.connection.index_exists?(:people, :primary_contact_id, :name => :symbol_index_name) + assert_nothing_raised { Person.connection.remove_index :people, :name => :symbol_index_name } + assert !Person.connection.index_exists?(:people, :primary_contact_id, :name => :symbol_index_name) + end + def test_add_index_length_limit good_index_name = 'x' * Person.connection.index_name_length too_long_index_name = good_index_name + 'x' diff --git a/activerecord/test/cases/nested_attributes_test.rb b/activerecord/test/cases/nested_attributes_test.rb index 62e073ba8c..8382ca048b 100644 --- a/activerecord/test/cases/nested_attributes_test.rb +++ b/activerecord/test/cases/nested_attributes_test.rb @@ -115,7 +115,7 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_difference('Ship.count') { pirate.save! } end - def test_reject_if_with_a_proc_which_returns_true_always + def test_reject_if_with_a_proc_which_returns_true_always_for_has_one Pirate.accepts_nested_attributes_for :ship, :reject_if => proc {|attributes| true } pirate = Pirate.new(:catchphrase => "Stop wastin' me time") ship = pirate.create_ship(:name => 's1') @@ -123,6 +123,22 @@ class TestNestedAttributesInGeneral < ActiveRecord::TestCase assert_equal 's1', ship.reload.name end + def test_reject_if_with_a_proc_which_returns_true_always_for_has_many + Man.accepts_nested_attributes_for :interests, :reject_if => proc {|attributes| true } + man = Man.create(:name => "John") + interest = man.interests.create(:topic => 'photography') + man.update_attributes({:interests_attributes => { :topic => 'gardening', :id => interest.id } }) + assert_equal 'photography', interest.reload.topic + end + + def test_has_many_association_updating_a_single_record + Man.accepts_nested_attributes_for(:interests) + man = Man.create(:name => 'John') + interest = man.interests.create(:topic => 'photography') + man.update_attributes({:interests_attributes => {:topic => 'gardening', :id => interest.id}}) + assert_equal 'gardening', interest.reload.topic + end + end class TestNestedAttributesOnAHasOneAssociation < ActiveRecord::TestCase diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 3bc3671b77..d642aeed8b 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -32,6 +32,11 @@ class RelationTest < ActiveRecord::TestCase assert_equal 5, Post.where(:id => post_authors).size end + def test_dynamic_finder + x = Post.where('author_id = ?', 1) + assert x.klass.respond_to?(:find_by_id), '@klass should handle dynamic finders' + end + def test_multivalue_where posts = Post.where('author_id = ? AND id = ?', 1, 1) assert_equal 1, posts.to_a.size diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index f9d791dcf3..b5b46d7431 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -1371,8 +1371,9 @@ module ActiveResource namespaces = module_names[0, module_names.size-1].map do |module_name| receiver = receiver.const_get(module_name) end - if namespace = namespaces.reverse.detect { |ns| ns.const_defined?(resource_name) } - return namespace.const_get(resource_name) + const_args = RUBY_VERSION < "1.9" ? [resource_name] : [resource_name, false] + if namespace = namespaces.reverse.detect { |ns| ns.const_defined?(*const_args) } + return namespace.const_get(*const_args) else raise NameError end @@ -1388,8 +1389,9 @@ module ActiveResource self.class.const_get(resource_name) end rescue NameError - if self.class.const_defined?(resource_name) - resource = self.class.const_get(resource_name) + const_args = RUBY_VERSION < "1.9" ? [resource_name] : [resource_name, false] + if self.class.const_defined?(*const_args) + resource = self.class.const_get(*const_args) else resource = self.class.const_set(resource_name, Class.new(ActiveResource::Base)) end diff --git a/activeresource/test/abstract_unit.rb b/activeresource/test/abstract_unit.rb index 544aede002..195f93f2a6 100644 --- a/activeresource/test/abstract_unit.rb +++ b/activeresource/test/abstract_unit.rb @@ -75,6 +75,10 @@ def setup_response </person> eof + @startup_sound = { + :name => "Mac Startup Sound", :author => { :name => "Jim Reekes" } + }.to_xml(:root => 'sound') + ActiveResource::HttpMock.respond_to do |mock| mock.get "/people/1.xml", {}, @matz mock.get "/people/2.xml", {}, @david @@ -112,6 +116,8 @@ def setup_response mock.head "/people/Greg/addresses/1.xml", {}, nil, 200 # customer mock.get "/customers/1.xml", {}, @luis + # sound + mock.get "/sounds/1.xml", {}, @startup_sound end Person.user = nil diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index 18c732b2ab..abf4259a54 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -1097,4 +1097,9 @@ class BaseTest < Test::Unit::TestCase plan.save! assert_equal 10.00, plan.price end + + def test_namespacing + sound = Asset::Sound.find(1) + assert_equal "Asset::Sound::Author", sound.author.class.to_s + end end diff --git a/activeresource/test/fixtures/sound.rb b/activeresource/test/fixtures/sound.rb index 5c0ef5d55c..d9d2b99fcd 100644 --- a/activeresource/test/fixtures/sound.rb +++ b/activeresource/test/fixtures/sound.rb @@ -1,5 +1,9 @@ -module Asset +module Asset class Sound < ActiveResource::Base self.site = "http://37s.sunrise.i:3000" end end + +# to test namespacing in a module +class Author +end
\ No newline at end of file diff --git a/activesupport/lib/active_support/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index d811c3b2f0..0c1d46c7ec 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -482,7 +482,7 @@ module ActiveSupport chain.delete_if {|c| c.matches?(type, filter) } end - options[:prepend] ? chain.unshift(*mapped) : chain.push(*mapped) + options[:prepend] ? chain.unshift(*(mapped.reverse)) : chain.push(*mapped) end end diff --git a/activesupport/lib/active_support/core_ext/object/to_param.rb b/activesupport/lib/active_support/core_ext/object/to_param.rb index f2e7c2351e..ecb2bca82c 100644 --- a/activesupport/lib/active_support/core_ext/object/to_param.rb +++ b/activesupport/lib/active_support/core_ext/object/to_param.rb @@ -35,7 +35,8 @@ end class Hash # Converts a hash into a string suitable for use as a URL query string. An optional <tt>namespace</tt> can be - # passed to enclose the param names (see example below). + # passed to enclose the param names (see example below). The string pairs "key=value" that conform the query + # string are sorted lexicographically in ascending order. # # ==== Examples # { :name => 'David', :nationality => 'Danish' }.to_param # => "name=David&nationality=Danish" @@ -44,6 +45,6 @@ class Hash def to_param(namespace = nil) collect do |key, value| value.to_query(namespace ? "#{namespace}[#{key}]" : key) - end * '&' + end.sort * '&' end end diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index 292383e3d8..51b28b6a43 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -149,6 +149,27 @@ module CallbacksTest end end + class AfterSaveConditionalPerson < Record + after_save Proc.new { |r| r.history << [:after_save, :string1] } + after_save Proc.new { |r| r.history << [:after_save, :string2] } + def save + run_callbacks :save + end + end + + class AfterSaveConditionalPersonCallbackTest < Test::Unit::TestCase + def test_after_save_runs_in_the_reverse_order + person = AfterSaveConditionalPerson.new + person.save + assert_equal [ + [:after_save, :string2], + [:after_save, :string1] + ], person.history + end + end + + + class ConditionalPerson < Record # proc before_save Proc.new { |r| r.history << [:before_save, :proc] }, :if => Proc.new { |r| true } @@ -352,6 +373,8 @@ module CallbacksTest end end + + class ResetCallbackTest < Test::Unit::TestCase def test_save_conditional_person person = CleanPerson.new diff --git a/activesupport/test/core_ext/hash_ext_test.rb b/activesupport/test/core_ext/hash_ext_test.rb index e5438745e0..0f35eb9e78 100644 --- a/activesupport/test/core_ext/hash_ext_test.rb +++ b/activesupport/test/core_ext/hash_ext_test.rb @@ -462,20 +462,24 @@ class HashExtToParamTests < Test::Unit::TestCase assert_equal '', {}.to_param assert_equal 'hello=world', { :hello => "world" }.to_param assert_equal 'hello=10', { "hello" => 10 }.to_param - assert_equal 'hello=world&say_bye=true', ActiveSupport::OrderedHash[:hello, "world", "say_bye", true].to_param + assert_equal 'hello=world&say_bye=true', {:hello => "world", "say_bye" => true}.to_param end def test_number_hash - assert_equal '10=20&30=40&50=60', ActiveSupport::OrderedHash[10, 20, 30, 40, 50, 60].to_param + assert_equal '10=20&30=40&50=60', {10 => 20, 30 => 40, 50 => 60}.to_param end def test_to_param_hash - assert_equal 'custom=param-1&custom2=param2-1', ActiveSupport::OrderedHash[ToParam.new('custom'), ToParam.new('param'), ToParam.new('custom2'), ToParam.new('param2')].to_param + assert_equal 'custom2=param2-1&custom=param-1', {ToParam.new('custom') => ToParam.new('param'), ToParam.new('custom2') => ToParam.new('param2')}.to_param end def test_to_param_hash_escapes_its_keys_and_values assert_equal 'param+1=A+string+with+%2F+characters+%26+that+should+be+%3F+escaped', { 'param 1' => 'A string with / characters & that should be ? escaped' }.to_param end + + def test_to_param_orders_by_key_in_ascending_order + assert_equal 'a=2&b=1&c=0', ActiveSupport::OrderedHash[*%w(b 1 c 0 a 2)].to_param + end end class HashToXmlTest < Test::Unit::TestCase diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index d553c09484..1dbf27d978 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -11,6 +11,7 @@ gem 'rails', '<%= Rails::VERSION::STRING %>' # Bundle edge Rails instead: # gem 'rails', :git => 'git://github.com/rails/rails.git' +# gem 'arel', :git => 'git://github.com/rails/arel.git' <%- end -%> <% unless options[:skip_active_record] -%> |