diff options
90 files changed, 961 insertions, 722 deletions
diff --git a/actionpack/CHANGELOG b/actionpack/CHANGELOG index 0ec42cc6de..6f8314109b 100644 --- a/actionpack/CHANGELOG +++ b/actionpack/CHANGELOG @@ -1,5 +1,7 @@ *Rails 3.1.0 (unreleased)* +* file_field automatically adds :multipart => true to the enclosing form. [Santiago Pastorino] + * Renames csrf_meta_tag -> csrf_meta_tags, and aliases csrf_meta_tag for backwards compatibility. [fxn] * Add Rack::Cache to the default stack. Create a Rails store that delegates to the Rails cache, so by default, whatever caching layer you are using will be used diff --git a/actionpack/Rakefile b/actionpack/Rakefile index d67c6f2410..521ee6913a 100644 --- a/actionpack/Rakefile +++ b/actionpack/Rakefile @@ -18,7 +18,8 @@ Rake::TestTask.new(:test_action_pack) do |t| # this will not happen automatically and the tests (as a whole) will error t.test_files = Dir.glob('test/{abstract,controller,dispatch,template}/**/*_test.rb').sort - # t.warning = true + #t.warning = true + t.verbose = true end namespace :test do diff --git a/actionpack/actionpack.gemspec b/actionpack/actionpack.gemspec index 75ac47105d..0e91b44914 100644 --- a/actionpack/actionpack.gemspec +++ b/actionpack/actionpack.gemspec @@ -26,7 +26,7 @@ Gem::Specification.new do |s| s.add_dependency('builder', '~> 2.1.2') s.add_dependency('i18n', '~> 0.4.1') s.add_dependency('rack', '~> 1.2.1') - s.add_dependency('rack-test', '~> 0.5.4') + s.add_dependency('rack-test', '~> 0.5.5') s.add_dependency('rack-mount', '~> 0.6.13') s.add_dependency('tzinfo', '~> 0.3.23') s.add_dependency('erubis', '~> 2.6.6') diff --git a/actionpack/lib/action_controller/metal/conditional_get.rb b/actionpack/lib/action_controller/metal/conditional_get.rb index 797ad846f6..8930c13a68 100644 --- a/actionpack/lib/action_controller/metal/conditional_get.rb +++ b/actionpack/lib/action_controller/metal/conditional_get.rb @@ -17,7 +17,7 @@ module ActionController # # def show # @article = Article.find(params[:id]) - # fresh_when(:etag => @article, :last_modified => @article.created_at.utc, :public => true) + # fresh_when(:etag => @article, :last_modified => @article.created_at, :public => true) # end # # This will render the show template if the request isn't sending a matching etag or @@ -48,7 +48,7 @@ module ActionController # def show # @article = Article.find(params[:id]) # - # if stale?(:etag => @article, :last_modified => @article.created_at.utc) + # if stale?(:etag => @article, :last_modified => @article.created_at) # @statistics = @article.really_expensive_call # respond_to do |format| # # all the supported formats diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index a5c9910d68..2b4a3b9392 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -22,7 +22,7 @@ module ActionController location = options.delete(:location) options.each do |key, value| - headers[key.to_s.dasherize.split(/-/).map { |v| v.capitalize }.join("-")] = value.to_s + headers[key.to_s.dasherize.split('-').each { |v| v[0] = v[0].chr.upcase }.join('-')] = value.to_s end self.status = status diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 1d74521e4f..251b1a8a8b 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -30,7 +30,7 @@ module ActionController # # # === Advanced \Basic example - # + # # Here is a more advanced \Basic example where only Atom feeds and the XML API is protected by HTTP authentication, # the regular HTML interface is protected by a session approach: # @@ -98,7 +98,7 @@ module ActionController # end # # === Notes - # + # # The +authenticate_or_request_with_http_digest+ block must return the user's password # or the ha1 digest hash so the framework can appropriately hash to check the user's # credentials. Returning +nil+ will cause authentication to fail. @@ -222,11 +222,10 @@ module ActionController end def decode_credentials(header) - header.to_s.gsub(/^Digest\s+/,'').split(',').inject({}) do |hash, pair| + Hash[header.to_s.gsub(/^Digest\s+/,'').split(',').map do |pair| key, value = pair.split('=', 2) - hash[key.strip.to_sym] = value.to_s.gsub(/^"|"$/,'').gsub(/'/, '') - hash - end + [key.strip.to_sym, value.to_s.gsub(/^"|"$/,'').gsub(/'/, '')] + end] end def authentication_header(controller, realm) diff --git a/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb b/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb index 0eaad2b911..85250721e7 100644 --- a/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb +++ b/actionpack/lib/action_controller/vendor/html-scanner/html/node.rb @@ -38,18 +38,14 @@ module HTML #:nodoc: private def keys_to_strings(hash) - hash.keys.inject({}) do |h,k| - h[k.to_s] = hash[k] - h - end + Hash[hash.keys.map {|k| [k.to_s, hash[k]]}] end def keys_to_symbols(hash) - hash.keys.inject({}) do |h,k| + Hash[hash.keys.map do |k| raise "illegal key #{k.inspect}" unless k.respond_to?(:to_sym) - h[k.to_sym] = hash[k] - h - end + [k.to_sym, hash[k]] + end] end end diff --git a/actionpack/lib/action_dispatch/http/upload.rb b/actionpack/lib/action_dispatch/http/upload.rb index 8ee4b81cdd..49465d820e 100644 --- a/actionpack/lib/action_dispatch/http/upload.rb +++ b/actionpack/lib/action_dispatch/http/upload.rb @@ -2,32 +2,27 @@ require 'active_support/core_ext/object/blank' module ActionDispatch module Http - module UploadedFile - def self.extended(object) - object.class_eval do - attr_accessor :original_path, :content_type - alias_method :local_path, :path if method_defined?(:path) - end - end + class UploadedFile < Tempfile + attr_accessor :original_filename, :content_type, :tempfile, :headers - # Take the basename of the upload's original filename. - # This handles the full Windows paths given by Internet Explorer - # (and perhaps other broken user agents) without affecting - # those which give the lone filename. - # The Windows regexp is adapted from Perl's File::Basename. - def original_filename - unless defined? @original_filename - @original_filename = - unless original_path.blank? - if original_path =~ /^(?:.*[:\\\/])?(.*)/m - $1 - else - File.basename original_path - end - end + def initialize(hash) + @original_filename = hash[:filename] + @content_type = hash[:type] + @headers = hash[:head] + + # To the untrained eye, this may appear as insanity. Given the alternatives, + # such as busting the method cache on every request or breaking backwards + # compatibility with is_a?(Tempfile), this solution is the best available + # option. + # + # TODO: Deprecate is_a?(Tempfile) and define a real API for this parameter + tempfile = hash[:tempfile] + tempfile.instance_variables.each do |ivar| + instance_variable_set(ivar, tempfile.instance_variable_get(ivar)) end - @original_filename end + + alias local_path path end module Upload @@ -35,11 +30,7 @@ module ActionDispatch # file upload hash with UploadedFile objects def normalize_parameters(value) if Hash === value && value.has_key?(:tempfile) - upload = value[:tempfile] - upload.extend(UploadedFile) - upload.original_path = value[:filename] - upload.content_type = value[:type] - upload + UploadedFile.new(value) else super end @@ -47,4 +38,4 @@ module ActionDispatch private :normalize_parameters end end -end
\ No newline at end of file +end diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 8a161b0837..fe85acb94e 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -149,6 +149,10 @@ module ActionDispatch controller = [@scope[:module], controller].compact.join("/").presence end + if controller.is_a?(String) && controller =~ %r{\A/} + raise ArgumentError, "controller name should not start with a slash" + end + controller = controller.to_s unless controller.is_a?(Regexp) action = action.to_s unless action.is_a?(Regexp) diff --git a/actionpack/lib/action_dispatch/routing/route.rb b/actionpack/lib/action_dispatch/routing/route.rb index aefebf8f80..f91a48e16c 100644 --- a/actionpack/lib/action_dispatch/routing/route.rb +++ b/actionpack/lib/action_dispatch/routing/route.rb @@ -21,11 +21,7 @@ module ActionDispatch conditions[:path_info] = ::Rack::Mount::Strexp.compile(path, requirements, SEPARATORS, anchor) end - @conditions = conditions.inject({}) { |h, (k, v)| - h[k] = Rack::Mount::RegexpWithNamedGroups.new(v) - h - } - + @conditions = Hash[conditions.map { |k,v| [k, Rack::Mount::RegexpWithNamedGroups.new(v)] }] @conditions.delete_if{ |k,v| k != :path_info && !valid_condition?(k) } @requirements.delete_if{ |k,v| !valid_condition?(k) } end diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 835ba03784..1a5f21bd09 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -217,27 +217,35 @@ module ActionDispatch self.valid_conditions.delete(:id) self.valid_conditions.push(:controller, :action) + @append = [] @disable_clear_and_finalize = false clear! end def draw(&block) clear! unless @disable_clear_and_finalize + eval_block(block) + finalize! unless @disable_clear_and_finalize + + nil + end + + def append(&block) + @append << block + end + def eval_block(block) mapper = Mapper.new(self) if default_scope mapper.with_default_scope(default_scope, &block) else mapper.instance_exec(&block) end - - finalize! unless @disable_clear_and_finalize - - nil end def finalize! return if @finalized + @append.each { |blk| eval_block(blk) } @finalized = true @set.freeze end @@ -558,13 +566,8 @@ module ActionDispatch keys = options.delete(:_positional_keys) keys -= options.keys if args.size < keys.size - 1 # take format into account - args = args.zip(keys).inject({}) do |h, (v, k)| - h[k] = v - h - end - # Tell url_for to skip default_url_options - options.merge!(args) + options.merge!(Hash[args.zip(keys).map { |v, k| [k, v] }]) end def rewrite_authentication(options) diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 590ebbf364..a681c9a5b6 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -257,17 +257,19 @@ module ActionDispatch end end + port = host.split(':')[1] + env = { :method => method, :params => parameters, "SERVER_NAME" => host.split(':')[0], - "SERVER_PORT" => (https? ? "443" : "80"), + "SERVER_PORT" => (port ? port : (https? ? "443" : "80")), "HTTPS" => https? ? "on" : "off", "rack.url_scheme" => https? ? "https" : "http", "REQUEST_URI" => path, - "HTTP_HOST" => host, + "HTTP_HOST" => [host, port].compact.join(':'), "REMOTE_ADDR" => remote_addr, "CONTENT_TYPE" => "application/x-www-form-urlencoded", "HTTP_ACCEPT" => accept diff --git a/actionpack/lib/action_view/helpers/asset_tag_helper.rb b/actionpack/lib/action_view/helpers/asset_tag_helper.rb index fdc40c8f2e..687cb83d75 100644 --- a/actionpack/lib/action_view/helpers/asset_tag_helper.rb +++ b/actionpack/lib/action_view/helpers/asset_tag_helper.rb @@ -827,7 +827,7 @@ module ActionView def expand_javascript_sources(sources, recursive = false) if sources.include?(:all) - all_javascript_files = collect_asset_files(config.javascripts_dir, ('**' if recursive), '*.js') + all_javascript_files = (collect_asset_files(config.javascripts_dir, ('**' if recursive), '*.js') - ['application']) << 'application' ((determine_source(:defaults, @@javascript_expansions).dup & all_javascript_files) + all_javascript_files).uniq else expanded_sources = sources.collect do |source| diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 3cc412de63..0937075edf 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -317,8 +317,11 @@ module ActionView options[:html] ||= {} options[:html][:remote] = options.delete(:remote) - output = form_tag(options.delete(:url) || {}, options.delete(:html) || {}) - output << fields_for(object_name, object, options, &proc) + builder = options[:parent_builder] = instantiate_builder(object_name, object, options, &proc) + fields_for = fields_for(object_name, object, options, &proc) + default_options = builder.multipart? ? { :multipart => true } : {} + output = form_tag(options.delete(:url) || {}, default_options.merge!(options.delete(:html) || {})) + output << fields_for output.safe_concat('</form>') end @@ -339,8 +342,8 @@ module ActionView options[:html] ||= {} options[:html].reverse_merge!(html_options) - options[:url] ||= options[:format] ? \ - polymorphic_path(object_or_array, :format => options.delete(:format)) : \ + options[:url] ||= options[:format] ? + polymorphic_path(object_or_array, :format => options.delete(:format)) : polymorphic_path(object_or_array) end @@ -529,22 +532,7 @@ module ActionView # <% end %> # <% end %> def fields_for(record, record_object = nil, options = nil, &block) - raise ArgumentError, "Missing block" unless block_given? - - options, record_object = record_object, nil if record_object.is_a?(Hash) - options ||= {} - - case record - when String, Symbol - object = record_object - object_name = record - else - object = record - object_name = ActiveModel::Naming.param_key(object) - end - - builder = options[:builder] || ActionView::Base.default_form_builder - capture(builder.new(object_name, object, self, options, block), &block) + capture(instantiate_builder(record, record_object, options, &block), &block) end # Returns a label tag tailored for labelling an input field for a specified attribute (identified by +method+) on an object @@ -850,6 +838,25 @@ module ActionView def range_field(object_name, method, options = {}) InstanceTag.new(object_name, method, self, options.delete(:object)).to_number_field_tag("range", options) end + + private + + def instantiate_builder(record, record_object = nil, options = nil, &block) + options, record_object = record_object, nil if record_object.is_a?(Hash) + options ||= {} + + case record + when String, Symbol + object = record_object + object_name = record + else + object = record + object_name = ActiveModel::Naming.param_key(object) + end + + builder = options[:builder] || ActionView::Base.default_form_builder + builder.new(object_name, object, self, options, block) + end end module InstanceTagMethods #:nodoc: @@ -858,9 +865,9 @@ module ActionView attr_reader :method_name, :object_name - DEFAULT_FIELD_OPTIONS = { "size" => 30 }.freeze - DEFAULT_RADIO_OPTIONS = { }.freeze - DEFAULT_TEXT_AREA_OPTIONS = { "cols" => 40, "rows" => 20 }.freeze + DEFAULT_FIELD_OPTIONS = { "size" => 30 } + DEFAULT_RADIO_OPTIONS = { } + DEFAULT_TEXT_AREA_OPTIONS = { "cols" => 40, "rows" => 20 } def initialize(object_name, method_name, template_object, object = nil) @object_name, @method_name = object_name.to_s.dup, method_name.to_s.dup @@ -1114,6 +1121,14 @@ module ActionView attr_accessor :object_name, :object, :options + attr_reader :multipart, :parent_builder + alias :multipart? :multipart + + def multipart=(multipart) + @multipart = multipart + parent_builder.multipart = multipart if parent_builder + end + def self.model_name @model_name ||= Struct.new(:partial_path).new(name.demodulize.underscore.sub!(/_builder$/, '')) end @@ -1125,6 +1140,7 @@ module ActionView def initialize(object_name, object, template, options, proc) @nested_child_index = {} @object_name, @object, @template, @options, @proc = object_name, object, template, options, proc + @parent_builder = options[:parent_builder] @default_options = @options ? @options.slice(:index) : {} if @object_name.to_s.match(/\[\]$/) if object ||= @template.instance_variable_get("@#{Regexp.last_match.pre_match}") and object.respond_to?(:to_param) @@ -1133,9 +1149,10 @@ module ActionView raise ArgumentError, "object[] naming but object param and @object var don't exist or don't respond to to_param: #{object.inspect}" end end + @multipart = nil end - (field_helpers - %w(label check_box radio_button fields_for hidden_field)).each do |selector| + (field_helpers - %w(label check_box radio_button fields_for hidden_field file_field)).each do |selector| class_eval <<-RUBY_EVAL, __FILE__, __LINE__ + 1 def #{selector}(method, options = {}) # def text_field(method, options = {}) @template.send( # @template.send( @@ -1157,10 +1174,9 @@ module ActionView index = "" end - if options[:builder] - args << {} unless args.last.is_a?(Hash) - args.last[:builder] ||= options[:builder] - end + args << {} unless args.last.is_a?(Hash) + args.last[:builder] ||= options[:builder] + args.last[:parent_builder] = self case record_or_name_or_array when String, Symbol @@ -1199,6 +1215,10 @@ module ActionView @template.hidden_field(@object_name, method, objectify_options(options)) end + def file_field(method, options = {}) + self.multipart = true + @template.file_field(@object_name, method, objectify_options(options)) + end # Add the submit button for the given form. When no value is given, it checks # if the object is a new resource or not to create the proper label: # diff --git a/actionpack/lib/action_view/test_case.rb b/actionpack/lib/action_view/test_case.rb index 2c2661df26..ff35fb7df4 100644 --- a/actionpack/lib/action_view/test_case.rb +++ b/actionpack/lib/action_view/test_case.rb @@ -189,11 +189,7 @@ module ActionView end def _assigns - _instance_variables.inject({}) do |hash, var| - name = var[1..-1].to_sym - hash[name] = instance_variable_get(var) - hash - end + _instance_variables.map { |var| [var[1..-1].to_sym, instance_variable_get(var)] } end def _routes diff --git a/actionpack/test/controller/assert_select_test.rb b/actionpack/test/controller/assert_select_test.rb index 2600dae3c5..f63321c78b 100644 --- a/actionpack/test/controller/assert_select_test.rb +++ b/actionpack/test/controller/assert_select_test.rb @@ -459,8 +459,8 @@ class AssertSelectTest < ActionController::TestCase assert_select_rjs :remove, "test1" - rescue Assertion - assert_equal "No RJS statement that removes 'test1' was rendered.", $!.message + rescue Assertion => e + assert_equal "No RJS statement that removes 'test1' was rendered.", e.message end def test_assert_select_rjs_for_remove_ignores_block @@ -491,8 +491,8 @@ class AssertSelectTest < ActionController::TestCase assert_select_rjs :show, "test1" - rescue Assertion - assert_equal "No RJS statement that shows 'test1' was rendered.", $!.message + rescue Assertion => e + assert_equal "No RJS statement that shows 'test1' was rendered.", e.message end def test_assert_select_rjs_for_show_ignores_block @@ -523,8 +523,8 @@ class AssertSelectTest < ActionController::TestCase assert_select_rjs :hide, "test1" - rescue Assertion - assert_equal "No RJS statement that hides 'test1' was rendered.", $!.message + rescue Assertion => e + assert_equal "No RJS statement that hides 'test1' was rendered.", e.message end def test_assert_select_rjs_for_hide_ignores_block @@ -555,8 +555,8 @@ class AssertSelectTest < ActionController::TestCase assert_select_rjs :toggle, "test1" - rescue Assertion - assert_equal "No RJS statement that toggles 'test1' was rendered.", $!.message + rescue Assertion => e + assert_equal "No RJS statement that toggles 'test1' was rendered.", e.message end def test_assert_select_rjs_for_toggle_ignores_block @@ -729,7 +729,7 @@ EOF end def render_rjs(&block) - @controller.response_with &block + @controller.response_with(&block) get :rjs end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 84b796ba72..a83f5155f8 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -637,7 +637,7 @@ class FragmentCachingTest < ActionController::TestCase @store.write('views/another_name', 'another_value') @store.write('views/primalgrasp', 'will not expire ;-)') - @controller.expire_fragment /name/ + @controller.expire_fragment(/name/) assert_nil @store.read('views/name') assert_nil @store.read('views/another_name') @@ -727,22 +727,22 @@ CACHED def test_fragment_caching_in_partials get :html_fragment_cached_with_partial assert_response :success - assert_match /Old fragment caching in a partial/, @response.body + assert_match(/Old fragment caching in a partial/, @response.body) assert_match "Old fragment caching in a partial", @store.read('views/test.host/functional_caching/html_fragment_cached_with_partial') end def test_render_inline_before_fragment_caching get :inline_fragment_cached assert_response :success - assert_match /Some inline content/, @response.body - assert_match /Some cached content/, @response.body + assert_match(/Some inline content/, @response.body) + assert_match(/Some cached content/, @response.body) assert_match "Some cached content", @store.read('views/test.host/functional_caching/inline_fragment_cached') end def test_fragment_caching_in_rjs_partials xhr :get, :js_fragment_cached_with_partial assert_response :success - assert_match /Old fragment caching in a partial/, @response.body + assert_match(/Old fragment caching in a partial/, @response.body) assert_match "Old fragment caching in a partial", @store.read('views/test.host/functional_caching/js_fragment_cached_with_partial') end diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index ab12c0bf17..42723c834e 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -488,6 +488,10 @@ class TestController < ActionController::Base head :x_custom_header => "something" end + def head_with_www_authenticate_header + head 'WWW-Authenticate' => 'something' + end + def head_with_status_code_first head :forbidden, :x_custom_header => "something" end @@ -1139,6 +1143,13 @@ class RenderTest < ActionController::TestCase assert_response :ok end + def test_head_with_www_authenticate_header + get :head_with_www_authenticate_header + assert_blank @response.body + assert_equal "something", @response.headers["WWW-Authenticate"] + assert_response :ok + end + def test_head_with_symbolic_status get :head_with_symbolic_status, :status => "ok" assert_equal 200, @response.status diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index b642adc06b..3daabf7a64 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -2128,6 +2128,38 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_raises(ActionController::RoutingError){ list_todo_path(:list_id => '2', :id => '1') } end + def test_controller_name_with_leading_slash_raise_error + assert_raise(ArgumentError) do + self.class.stub_controllers do |routes| + routes.draw { get '/feeds/:service', :to => '/feeds#show' } + end + end + + assert_raise(ArgumentError) do + self.class.stub_controllers do |routes| + routes.draw { get '/feeds/:service', :controller => '/feeds', :action => 'show' } + end + end + + assert_raise(ArgumentError) do + self.class.stub_controllers do |routes| + routes.draw { get '/api/feeds/:service', :to => '/api/feeds#show' } + end + end + + assert_raise(ArgumentError) do + self.class.stub_controllers do |routes| + routes.draw { controller("/feeds") { get '/feeds/:service', :to => :show } } + end + end + + assert_raise(ArgumentError) do + self.class.stub_controllers do |routes| + routes.draw { resources :feeds, :controller => '/feeds' } + end + end + end + private def with_test_routes yield @@ -2152,6 +2184,39 @@ private end end +class TestAppendingRoutes < ActionController::IntegrationTest + def simple_app(resp) + lambda { |e| [ 200, { 'Content-Type' => 'text/plain' }, [resp] ] } + end + + setup do + s = self + @app = ActionDispatch::Routing::RouteSet.new + @app.append do + match '/hello' => s.simple_app('fail') + match '/goodbye' => s.simple_app('goodbye') + end + + @app.draw do + match '/hello' => s.simple_app('hello') + end + end + + def test_goodbye_should_be_available + get '/goodbye' + assert_equal 'goodbye', @response.body + end + + def test_hello_should_not_be_overwritten + get '/hello' + assert_equal 'hello', @response.body + end + + def test_missing_routes_are_still_missing + get '/random' + assert_equal 404, @response.status + end +end class TestDefaultScope < ActionController::IntegrationTest module ::Blog diff --git a/actionpack/test/template/asset_tag_helper_test.rb b/actionpack/test/template/asset_tag_helper_test.rb index 2b83cfe1a9..ec28d4442c 100644 --- a/actionpack/test/template/asset_tag_helper_test.rb +++ b/actionpack/test/template/asset_tag_helper_test.rb @@ -89,8 +89,8 @@ class AssetTagHelperTest < ActionView::TestCase %(javascript_include_tag("bank", :lang => "vbscript")) => %(<script lang="vbscript" src="/javascripts/bank.js" type="text/javascript"></script>), %(javascript_include_tag("common.javascript", "/elsewhere/cools")) => %(<script src="/javascripts/common.javascript" type="text/javascript"></script>\n<script src="/elsewhere/cools.js" type="text/javascript"></script>), %(javascript_include_tag(:defaults)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), - %(javascript_include_tag(:all)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>), - %(javascript_include_tag(:all, :recursive => true)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>), + %(javascript_include_tag(:all)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), + %(javascript_include_tag(:all, :recursive => true)) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), %(javascript_include_tag(:defaults, "bank")) => %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), %(javascript_include_tag("bank", :defaults)) => %(<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/rails.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), @@ -628,7 +628,7 @@ class AssetTagHelperTest < ActionView::TestCase assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) assert_equal( - %(// prototype js\n\n// effects js\n\n// dragdrop js\n\n// controls js\n\n// application js\n\n// bank js\n\n// robber js\n\n// subdir js\n\n\n// version.1.0 js), + %(// prototype js\n\n// effects js\n\n// dragdrop js\n\n// controls js\n\n// bank js\n\n// robber js\n\n// subdir js\n\n\n// version.1.0 js\n\n// application js), IO.read(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) ) @@ -649,7 +649,7 @@ class AssetTagHelperTest < ActionView::TestCase assert File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) assert_equal( - %(// prototype js\n\n// effects js\n\n// dragdrop js\n\n// controls js\n\n// application js\n\n// bank js\n\n// robber js\n\n// version.1.0 js), + %(// prototype js\n\n// effects js\n\n// dragdrop js\n\n// controls js\n\n// bank js\n\n// robber js\n\n// version.1.0 js\n\n// application js), IO.read(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'combined.js')) ) @@ -686,24 +686,24 @@ class AssetTagHelperTest < ActionView::TestCase config.perform_caching = false assert_dom_equal( - %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>), + %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag(:all, :cache => true) ) assert_dom_equal( - %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>), + %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag(:all, :cache => true, :recursive => true) ) assert !File.exist?(File.join(ActionView::Helpers::AssetTagHelper::JAVASCRIPTS_DIR, 'all.js')) assert_dom_equal( - %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>), + %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag(:all, :cache => "money") ) assert_dom_equal( - %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>), + %(<script src="/javascripts/prototype.js" type="text/javascript"></script>\n<script src="/javascripts/effects.js" type="text/javascript"></script>\n<script src="/javascripts/dragdrop.js" type="text/javascript"></script>\n<script src="/javascripts/controls.js" type="text/javascript"></script>\n<script src="/javascripts/bank.js" type="text/javascript"></script>\n<script src="/javascripts/robber.js" type="text/javascript"></script>\n<script src="/javascripts/subdir/subdir.js" type="text/javascript"></script>\n<script src="/javascripts/version.1.0.js" type="text/javascript"></script>\n<script src="/javascripts/application.js" type="text/javascript"></script>), javascript_include_tag(:all, :cache => "money", :recursive => true) ) diff --git a/actionpack/test/template/form_helper_test.rb b/actionpack/test/template/form_helper_test.rb index 97a08d45ba..d40dd409b8 100644 --- a/actionpack/test/template/form_helper_test.rb +++ b/actionpack/test/template/form_helper_test.rb @@ -640,6 +640,12 @@ class FormHelperTest < ActionView::TestCase ) end + def test_form_for_requires_block + assert_raises(ArgumentError) do + form_for(:post, @post, :html => { :id => 'create-post' }) + end + end + def test_form_for assert_deprecated do form_for(:post, @post, :html => { :id => 'create-post' }) do |f| @@ -665,6 +671,45 @@ class FormHelperTest < ActionView::TestCase assert_dom_equal expected, output_buffer end + def test_form_for_with_file_field_generate_multipart + Post.send :attr_accessor, :file + + assert_deprecated do + form_for(:post, @post, :html => { :id => 'create-post' }) do |f| + concat f.file_field(:file) + end + end + + expected = + "<form accept-charset='UTF-8' action='/' id='create-post' method='post' enctype='multipart/form-data'>" + + snowman + + "<input name='post[file]' type='file' id='post_file' />" + + "</form>" + + assert_dom_equal expected, output_buffer + end + + def test_fields_for_with_file_field_generate_multipart + Comment.send :attr_accessor, :file + + assert_deprecated do + form_for(:post, @post) do |f| + concat f.fields_for(:comment, @post) { |c| + concat c.file_field(:file) + } + end + end + + expected = + "<form accept-charset='UTF-8' action='/' method='post' enctype='multipart/form-data'>" + + snowman + + "<input name='post[comment][file]' type='file' id='post_comment_file' />" + + "</form>" + + assert_dom_equal expected, output_buffer + end + + def test_form_for_with_format form_for(@post, :format => :json, :html => { :id => "edit_post_123", :class => "edit_post" }) do |f| concat f.label(:title) diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 1361d327b3..1dfd0b6132 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -115,7 +115,7 @@ module ActiveModel # person.name = 'bob' # person.changes # => { 'name' => ['bill', 'bob'] } def changes - changed.inject(HashWithIndifferentAccess.new){ |h, attr| h[attr] = attribute_change(attr); h } + HashWithIndifferentAccess[changed.map { |attr| [attr, attribute_change(attr)] }] end # Map of attributes that were changed when the model was saved. diff --git a/activemodel/lib/active_model/serialization.rb b/activemodel/lib/active_model/serialization.rb index 8f90ef64ba..37739b98a1 100644 --- a/activemodel/lib/active_model/serialization.rb +++ b/activemodel/lib/active_model/serialization.rb @@ -79,15 +79,8 @@ module ActiveModel attribute_names -= except end - method_names = Array.wrap(options[:methods]).inject([]) do |methods, name| - methods << name if respond_to?(name.to_s) - methods - end - - (attribute_names + method_names).inject({}) { |hash, name| - hash[name] = send(name) - hash - } + method_names = Array.wrap(options[:methods]).map { |n| n if respond_to?(n.to_s) }.compact + Hash[(attribute_names + method_names).map { |n| [n, send(n)] }] end end end diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index e89385e7e5..26a134568c 100644 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -76,10 +76,9 @@ module ActiveModel end def serializable_methods - Array.wrap(options[:methods]).inject([]) do |methods, name| - methods << self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s) - methods - end + Array.wrap(options[:methods]).map do |name| + self.class::MethodAttribute.new(name.to_s, @serializable) if @serializable.respond_to?(name.to_s) + end.compact end def serialize diff --git a/activemodel/lib/active_model/validations/acceptance.rb b/activemodel/lib/active_model/validations/acceptance.rb index d9dfe0c855..a7296d8e1d 100644 --- a/activemodel/lib/active_model/validations/acceptance.rb +++ b/activemodel/lib/active_model/validations/acceptance.rb @@ -14,8 +14,6 @@ module ActiveModel end def setup(klass) - # Note: instance_methods.map(&:to_s) is important for 1.9 compatibility - # as instance_methods returns symbols unlike 1.8 which returns strings. attr_readers = attributes.reject { |name| klass.attribute_method?(name) } attr_writers = attributes.reject { |name| klass.attribute_method?("#{name}=") } klass.send(:attr_reader, *attr_readers) diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG index 62f1470287..8f283e1117 100644 --- a/activerecord/CHANGELOG +++ b/activerecord/CHANGELOG @@ -1,7 +1,19 @@ *Rails 3.1.0 (unreleased)* -* No changes +* The following code: + Model.limit(10).scoping { Model.count } + + now generates the following SQL: + + SELECT COUNT(*) FROM models LIMIT 10 + + This may not return what you want. Instead, you may with to do something + like this: + + Model.limit(10).scoping { Model.all.size } + + [Aaron Patterson] *Rails 3.0.0 (August 29, 2010)* diff --git a/activerecord/lib/active_record/association_preload.rb b/activerecord/lib/active_record/association_preload.rb index 63224377a3..715c868598 100644 --- a/activerecord/lib/active_record/association_preload.rb +++ b/activerecord/lib/active_record/association_preload.rb @@ -209,23 +209,20 @@ module ActiveRecord records.each {|record| record.send("set_#{reflection.name}_target", nil)} if options[:through] through_records = preload_through_records(records, reflection, options[:through]) - through_reflection = reflections[options[:through]] - through_primary_key = through_reflection.primary_key_name + unless through_records.empty? + through_reflection = reflections[options[:through]] + through_primary_key = through_reflection.primary_key_name source = reflection.source_reflection.name through_records.first.class.preload_associations(through_records, source) if through_reflection.macro == :belongs_to - rev_id_to_record_map = construct_id_map(records, through_primary_key).first - rev_primary_key = through_reflection.klass.primary_key - through_records.each do |through_record| - add_preloaded_record_to_collection(rev_id_to_record_map[through_record[rev_primary_key].to_s], - reflection.name, through_record.send(source)) - end - else - through_records.each do |through_record| - add_preloaded_record_to_collection(id_to_record_map[through_record[through_primary_key].to_s], - reflection.name, through_record.send(source)) - end + id_to_record_map = construct_id_map(records, through_primary_key).first + through_primary_key = through_reflection.klass.primary_key + end + + through_records.each do |through_record| + add_preloaded_record_to_collection(id_to_record_map[through_record[through_primary_key].to_s], + reflection.name, through_record.send(source)) end end else @@ -299,9 +296,10 @@ module ActiveRecord options = reflection.options primary_key_name = reflection.primary_key_name + klasses_and_ids = {} + if options[:polymorphic] polymorph_type = options[:foreign_type] - klasses_and_ids = {} # Construct a mapping from klass to a list of ids to load and a mapping of those ids back # to their parent_records @@ -310,27 +308,20 @@ module ActiveRecord klass_id = record.send(primary_key_name) if klass_id id_map = klasses_and_ids[klass] ||= {} - id_list_for_klass_id = (id_map[klass_id.to_s] ||= []) - id_list_for_klass_id << record + (id_map[klass_id.to_s] ||= []) << record end end end - klasses_and_ids = klasses_and_ids.to_a else id_map = {} records.each do |record| key = record.send(primary_key_name) - if key - mapped_records = (id_map[key.to_s] ||= []) - mapped_records << record - end + (id_map[key.to_s] ||= []) << record if key end - klasses_and_ids = [[reflection.klass.name, id_map]] + klasses_and_ids[reflection.klass.name] = id_map unless id_map.empty? end - klasses_and_ids.each do |klass_and_id| - klass_name, id_map = *klass_and_id - next if id_map.empty? + klasses_and_ids.each do |klass_name, id_map| klass = klass_name.constantize table_name = klass.quoted_table_name diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index 4bf206d589..565ebf8197 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1416,8 +1416,8 @@ module ActiveRecord include Module.new { class_eval <<-RUBY, __FILE__, __LINE__ + 1 def destroy # def destroy - super # super #{reflection.name}.clear # posts.clear + super # super end # end RUBY } @@ -1841,7 +1841,7 @@ module ActiveRecord @reflections = [] @base_records_hash = {} @base_records_in_order = [] - @table_aliases = Hash.new { |aliases, table| aliases[table] = 0 } + @table_aliases = Hash.new(0) @table_aliases[base.table_name] = 1 build(associations) end @@ -1855,7 +1855,7 @@ module ActiveRecord end def join_associations - @joins[1..-1].to_a + @joins.last(@joins.length - 1) end def join_base @@ -1951,53 +1951,54 @@ module ActiveRecord def construct(parent, associations, joins, row) case associations - when Symbol, String - join = joins.detect{|j| j.reflection.name.to_s == associations.to_s && j.parent_table_name == parent.class.table_name } + when Symbol, String + join = joins.detect{|j| j.reflection.name.to_s == associations.to_s && j.parent_table_name == parent.class.table_name } + raise(ConfigurationError, "No such association") if join.nil? + + joins.delete(join) + construct_association(parent, join, row) + when Array + associations.each do |association| + construct(parent, association, joins, row) + end + when Hash + associations.sort_by { |k,_| k.to_s }.each do |name, assoc| + join = joins.detect{|j| j.reflection.name.to_s == name.to_s && j.parent_table_name == parent.class.table_name } raise(ConfigurationError, "No such association") if join.nil? + association = construct_association(parent, join, row) joins.delete(join) - construct_association(parent, join, row) - when Array - associations.each do |association| - construct(parent, association, joins, row) - end - when Hash - associations.keys.sort{|a,b|a.to_s<=>b.to_s}.each do |name| - join = joins.detect{|j| j.reflection.name.to_s == name.to_s && j.parent_table_name == parent.class.table_name } - raise(ConfigurationError, "No such association") if join.nil? - - association = construct_association(parent, join, row) - joins.delete(join) - construct(association, associations[name], joins, row) if association - end - else - raise ConfigurationError, associations.inspect + construct(association, assoc, joins, row) if association + end + else + raise ConfigurationError, associations.inspect end end def construct_association(record, join, row) - case join.reflection.macro + return if record.id.to_s != join.parent.record_id(row).to_s + + macro = join.reflection.macro + if macro == :has_one + return if record.instance_variable_defined?("@#{join.reflection.name}") + association = join.instantiate(row) unless row[join.aliased_primary_key].nil? + set_target_and_inverse(join, association, record) + else + return if row[join.aliased_primary_key].nil? + association = join.instantiate(row) + case macro when :has_many, :has_and_belongs_to_many collection = record.send(join.reflection.name) collection.loaded - - return nil if record.id.to_s != join.parent.record_id(row).to_s or row[join.aliased_primary_key].nil? - association = join.instantiate(row) collection.target.push(association) collection.__send__(:set_inverse_instance, association, record) - when :has_one - return if record.id.to_s != join.parent.record_id(row).to_s - return if record.instance_variable_defined?("@#{join.reflection.name}") - association = join.instantiate(row) unless row[join.aliased_primary_key].nil? - set_target_and_inverse(join, association, record) when :belongs_to - return if record.id.to_s != join.parent.record_id(row).to_s or row[join.aliased_primary_key].nil? - association = join.instantiate(row) set_target_and_inverse(join, association, record) else raise ConfigurationError, "unknown macro: #{join.reflection.macro}" + end end - return association + association end def set_target_and_inverse(join, association, record) @@ -2046,7 +2047,7 @@ module ActiveRecord end def extract_record(row) - column_names_with_alias.inject({}){|record, (cn, an)| record[cn] = row[an]; record} + Hash[column_names_with_alias.map{|cn, an| [cn, row[an]]}] end def record_id(row) @@ -2107,71 +2108,15 @@ module ActiveRecord def association_join return @join if @join - aliased_table = Arel::Table.new(table_name, :as => @aliased_table_name, :engine => arel_engine) - parent_table = Arel::Table.new(parent.table_name, :as => parent.aliased_table_name, :engine => arel_engine) - - @join = case reflection.macro - when :has_and_belongs_to_many - join_table = Arel::Table.new(options[:join_table], :as => aliased_join_table_name, :engine => arel_engine) - fk = options[:foreign_key] || reflection.active_record.to_s.foreign_key - klass_fk = options[:association_foreign_key] || klass.to_s.foreign_key + aliased_table = Arel::Table.new(table_name, :as => @aliased_table_name, + :engine => arel_engine, + :columns => klass.columns) - [ - join_table[fk].eq(parent_table[reflection.active_record.primary_key]), - aliased_table[klass.primary_key].eq(join_table[klass_fk]) - ] - when :has_many, :has_one - if reflection.options[:through] - join_table = Arel::Table.new(through_reflection.klass.table_name, :as => aliased_join_table_name, :engine => arel_engine) - jt_foreign_key = jt_as_extra = jt_source_extra = jt_sti_extra = nil - first_key = second_key = as_extra = nil - - if through_reflection.options[:as] # has_many :through against a polymorphic join - jt_foreign_key = through_reflection.options[:as].to_s + '_id' - jt_as_extra = join_table[through_reflection.options[:as].to_s + '_type'].eq(parent.active_record.base_class.name) - else - jt_foreign_key = through_reflection.primary_key_name - end + parent_table = Arel::Table.new(parent.table_name, :as => parent.aliased_table_name, + :engine => arel_engine, + :columns => parent.active_record.columns) - case source_reflection.macro - when :has_many - if source_reflection.options[:as] - first_key = "#{source_reflection.options[:as]}_id" - second_key = options[:foreign_key] || primary_key - as_extra = aliased_table["#{source_reflection.options[:as]}_type"].eq(source_reflection.active_record.base_class.name) - else - first_key = through_reflection.klass.base_class.to_s.foreign_key - second_key = options[:foreign_key] || primary_key - end - - unless through_reflection.klass.descends_from_active_record? - jt_sti_extra = join_table[through_reflection.active_record.inheritance_column].eq(through_reflection.klass.sti_name) - end - when :belongs_to - first_key = primary_key - if reflection.options[:source_type] - second_key = source_reflection.association_foreign_key - jt_source_extra = join_table[reflection.source_reflection.options[:foreign_type]].eq(reflection.options[:source_type]) - else - second_key = source_reflection.primary_key_name - end - end - - [ - [parent_table[parent.primary_key].eq(join_table[jt_foreign_key]), jt_as_extra, jt_source_extra, jt_sti_extra].reject{|x| x.blank? }, - aliased_table[first_key].eq(join_table[second_key]) - ] - elsif reflection.options[:as] - id_rel = aliased_table["#{reflection.options[:as]}_id"].eq(parent_table[parent.primary_key]) - type_rel = aliased_table["#{reflection.options[:as]}_type"].eq(parent.active_record.base_class.name) - [id_rel, type_rel] - else - foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key - [aliased_table[foreign_key].eq(parent_table[reflection.options[:primary_key] || parent.primary_key])] - end - when :belongs_to - [aliased_table[options[:primary_key] || reflection.klass.primary_key].eq(parent_table[options[:foreign_key] || reflection.primary_key_name])] - end + @join = send("build_#{reflection.macro}", aliased_table, parent_table) unless klass.descends_from_active_record? sti_column = aliased_table[klass.inheritance_column] @@ -2191,7 +2136,9 @@ module ActiveRecord end def relation - aliased = Arel::Table.new(table_name, :as => @aliased_table_name, :engine => arel_engine) + aliased = Arel::Table.new(table_name, :as => @aliased_table_name, + :engine => arel_engine, + :columns => klass.columns) if reflection.macro == :has_and_belongs_to_many [Arel::Table.new(options[:join_table], :as => aliased_join_table_name, :engine => arel_engine), aliased] @@ -2244,6 +2191,77 @@ module ActiveRecord def interpolate_sql(sql) instance_eval("%@#{sql.gsub('@', '\@')}@", __FILE__, __LINE__) end + + private + + def build_has_and_belongs_to_many(aliased_table, parent_table) + join_table = Arel::Table.new(options[:join_table], :as => aliased_join_table_name, :engine => arel_engine) + fk = options[:foreign_key] || reflection.active_record.to_s.foreign_key + klass_fk = options[:association_foreign_key] || klass.to_s.foreign_key + + [ + join_table[fk].eq(parent_table[reflection.active_record.primary_key]), + aliased_table[klass.primary_key].eq(join_table[klass_fk]) + ] + end + + def build_has_many(aliased_table, parent_table) + if reflection.options[:through] + join_table = Arel::Table.new(through_reflection.klass.table_name, + :as => aliased_join_table_name, + :engine => arel_engine) + jt_foreign_key = jt_as_extra = jt_source_extra = jt_sti_extra = nil + first_key = second_key = nil + + if through_reflection.options[:as] # has_many :through against a polymorphic join + as_key = through_reflection.options[:as].to_s + jt_foreign_key = as_key + '_id' + jt_as_extra = join_table[as_key + '_type'].eq(parent.active_record.base_class.name) + else + jt_foreign_key = through_reflection.primary_key_name + end + + case source_reflection.macro + when :has_many + second_key = options[:foreign_key] || primary_key + + if source_reflection.options[:as] + first_key = "#{source_reflection.options[:as]}_id" + else + first_key = through_reflection.klass.base_class.to_s.foreign_key + end + + unless through_reflection.klass.descends_from_active_record? + jt_sti_extra = join_table[through_reflection.active_record.inheritance_column].eq(through_reflection.klass.sti_name) + end + when :belongs_to + first_key = primary_key + if reflection.options[:source_type] + second_key = source_reflection.association_foreign_key + jt_source_extra = join_table[reflection.source_reflection.options[:foreign_type]].eq(reflection.options[:source_type]) + else + second_key = source_reflection.primary_key_name + end + end + + [ + [parent_table[parent.primary_key].eq(join_table[jt_foreign_key]), jt_as_extra, jt_source_extra, jt_sti_extra].compact, + aliased_table[first_key].eq(join_table[second_key]) + ] + elsif reflection.options[:as] + id_rel = aliased_table["#{reflection.options[:as]}_id"].eq(parent_table[parent.primary_key]) + type_rel = aliased_table["#{reflection.options[:as]}_type"].eq(parent.active_record.base_class.name) + [id_rel, type_rel] + else + foreign_key = options[:foreign_key] || reflection.active_record.name.foreign_key + [aliased_table[foreign_key].eq(parent_table[reflection.options[:primary_key] || parent.primary_key])] + end + end + alias :build_has_one :build_has_many + + def build_belongs_to(aliased_table, parent_table) + [aliased_table[options[:primary_key] || reflection.klass.primary_key].eq(parent_table[options[:foreign_key] || reflection.primary_key_name])] + end end end end diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb index 1221a56bbc..91e0a9f2f8 100644 --- a/activerecord/lib/active_record/associations/association_collection.rb +++ b/activerecord/lib/active_record/associations/association_collection.rb @@ -338,13 +338,12 @@ module ActiveRecord def uniq(collection = self) seen = Set.new - collection.inject([]) do |kept, record| + collection.map do |record| unless seen.include?(record.id) - kept << record seen << record.id + record end - kept - end + end.compact end # Replace this collection with +other_array+ diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb index c0ec65bd40..eb65234dfb 100644 --- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb +++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb @@ -49,23 +49,20 @@ module ActiveRecord timestamps = record_timestamp_columns(record) timezone = record.send(:current_time_from_proper_timezone) if timestamps.any? - attributes = columns.inject({}) do |attrs, column| + attributes = Hash[columns.map do |column| name = column.name - case name.to_s + value = case name.to_s when @reflection.primary_key_name.to_s - attrs[relation[name]] = @owner.id + @owner.id when @reflection.association_foreign_key.to_s - attrs[relation[name]] = record.id + record.id when *timestamps - attrs[relation[name]] = timezone + timezone else - if record.has_attribute?(name) - value = @owner.send(:quote_value, record[name], column) - attrs[relation[name]] = value unless value.nil? - end + @owner.send(:quote_value, record[name], column) if record.has_attribute?(name) end - attrs - end + [relation[name], value] unless value.nil? + end] relation.insert(attributes) end @@ -79,7 +76,7 @@ module ActiveRecord else relation = Arel::Table.new(@reflection.options[:join_table]) relation.where(relation[@reflection.primary_key_name].eq(@owner.id). - and(relation[@reflection.association_foreign_key].in(records.map { |x| x.id })) + and(relation[@reflection.association_foreign_key].in(records.map { |x| x.id }.compact)) ).delete end end diff --git a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb index a4e144f233..23195b02f7 100644 --- a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb +++ b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb @@ -13,10 +13,7 @@ module ActiveRecord # Returns a hash of attributes before typecasting and deserialization. def attributes_before_type_cast - self.attribute_names.inject({}) do |attrs, name| - attrs[name] = read_attribute_before_type_cast(name) - attrs - end + Hash[attribute_names.map { |name| [name, read_attribute_before_type_cast(name)] }] end private diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5da4eb169b..e86d4984a6 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -884,21 +884,13 @@ module ActiveRecord #:nodoc: # single-table inheritance model that makes it possible to create # objects of different types from the same table. def instantiate(record) - object = find_sti_class(record[inheritance_column]).allocate - - object.instance_variable_set(:@attributes, record) - object.instance_variable_set(:@attributes_cache, {}) - object.instance_variable_set(:@new_record, false) - object.instance_variable_set(:@readonly, false) - object.instance_variable_set(:@destroyed, false) - object.instance_variable_set(:@marked_for_destruction, false) - object.instance_variable_set(:@previously_changed, {}) - object.instance_variable_set(:@changed_attributes, {}) - - object.send(:_run_find_callbacks) - object.send(:_run_initialize_callbacks) - - object + find_sti_class(record[inheritance_column]).allocate.instance_eval do + @attributes, @attributes_cache, @previously_changed, @changed_attributes = record, {}, {}, {} + @new_record = @readonly = @destroyed = @marked_for_destruction = false + _run_find_callbacks + _run_initialize_callbacks + self + end end def find_sti_class(type_name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb index 8e74eff0ab..ec7035e540 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_specification.rb @@ -67,8 +67,8 @@ module ActiveRecord begin require "active_record/connection_adapters/#{spec[:adapter]}_adapter" - rescue LoadError - raise "Please install the #{spec[:adapter]} adapter: `gem install activerecord-#{spec[:adapter]}-adapter` (#{$!})" + rescue LoadError => e + raise "Please install the #{spec[:adapter]} adapter: `gem install activerecord-#{spec[:adapter]}-adapter` (#{e})" end adapter_method = "#{spec[:adapter]}_connection" 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 0245e63294..310423bb20 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -340,12 +340,10 @@ module ActiveRecord end if index_name.length > index_name_length - @logger.warn("Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{index_name_length} characters. Skipping.") - return + raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' is too long; the limit is #{index_name_length} characters" end if index_name_exists?(table_name, index_name, false) - @logger.warn("Index name '#{index_name}' on table '#{table_name}' already exists. Skipping.") - return + raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' already exists" end quoted_column_names = quoted_columns_for_index(column_names, options).join(", ") @@ -365,8 +363,7 @@ module ActiveRecord def remove_index(table_name, options = {}) index_name = index_name(table_name, options) unless index_name_exists?(table_name, index_name, true) - @logger.warn("Index name '#{index_name}' on table '#{table_name}' does not exist. Skipping.") - return + raise ArgumentError, "Index name '#{index_name}' on table '#{table_name}' does not exist" end remove_index!(table_name, index_name) end diff --git a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb index d797f9c6c3..e7f7b37b27 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql_adapter.rb @@ -19,11 +19,11 @@ module ActiveRecord begin require 'mysql' rescue LoadError - raise "!!! Missing the mysql gem. Add it to your Gemfile: gem 'mysql', '2.8.1'" + raise "!!! Missing the mysql2 gem. Add it to your Gemfile: gem 'mysql2'" end unless defined?(Mysql::Result) && Mysql::Result.method_defined?(:each_hash) - raise "!!! Outdated mysql gem. Upgrade to 2.8.1 or later. In your Gemfile: gem 'mysql', '2.8.1'" + raise "!!! Outdated mysql gem. Upgrade to 2.8.1 or later. In your Gemfile: gem 'mysql', '2.8.1'. Or use gem 'mysql2'" end end @@ -276,7 +276,7 @@ module ActiveRecord rows = [] result.each { |row| rows << row } result.free - @connection.more_results && @connection.next_result # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped + @connection.more_results && @connection.next_result # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped rows end @@ -358,10 +358,10 @@ module ActiveRecord sql = "SHOW TABLES" end - select_all(sql).inject("") do |structure, table| + select_all(sql).map do |table| table.delete('Table_type') - structure += select_one("SHOW CREATE TABLE #{quote_table_name(table.to_a.first.last)}")["Create Table"] + ";\n\n" - end + select_one("SHOW CREATE TABLE #{quote_table_name(table.to_a.first.last)}")["Create Table"] + ";\n\n" + end.join("") end def recreate_database(name, options = {}) #:nodoc: @@ -620,7 +620,7 @@ module ActiveRecord rows = [] result.each_hash { |row| rows << row } result.free - @connection.more_results && @connection.next_result # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped + @connection.more_results && @connection.next_result # invoking stored procedures with CLIENT_MULTI_RESULTS requires this to tidy up else connection will be dropped rows end diff --git a/activerecord/lib/active_record/nested_attributes.rb b/activerecord/lib/active_record/nested_attributes.rb index 33611b410c..18519a712b 100644 --- a/activerecord/lib/active_record/nested_attributes.rb +++ b/activerecord/lib/active_record/nested_attributes.rb @@ -386,7 +386,7 @@ module ActiveRecord association.to_a else attribute_ids = attributes_collection.map {|a| a['id'] || a[:id] }.compact - attribute_ids.present? ? association.all(:conditions => {association.primary_key => attribute_ids}) : [] + attribute_ids.empty? ? [] : association.all(:conditions => {association.primary_key => attribute_ids}) end attributes_collection.each do |attributes| diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 1b016f0895..f80e304c5d 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -279,10 +279,9 @@ module ActiveRecord # that a new instance, or one populated from a passed-in Hash, still has all the attributes # that instances loaded from the database would. def attributes_from_column_definition - self.class.columns.inject({}) do |attributes, column| - attributes[column.name] = column.default unless column.name == self.class.primary_key - attributes - end + Hash[self.class.columns.map do |column| + [column.name, column.default] unless column.name == self.class.primary_key + end] end end end diff --git a/activerecord/lib/active_record/railties/databases.rake b/activerecord/lib/active_record/railties/databases.rake index 8e5332214c..ccaa1f01f6 100644 --- a/activerecord/lib/active_record/railties/databases.rake +++ b/activerecord/lib/active_record/railties/databases.rake @@ -67,8 +67,8 @@ namespace :db do # Create the SQLite database ActiveRecord::Base.establish_connection(config) ActiveRecord::Base.connection - rescue - $stderr.puts $!, *($!.backtrace) + rescue Exception => e + $stderr.puts e, *(e.backtrace) $stderr.puts "Couldn't create database for #{config.inspect}" end end @@ -113,8 +113,8 @@ namespace :db do ActiveRecord::Base.establish_connection(config.merge('database' => 'postgres', 'schema_search_path' => 'public')) ActiveRecord::Base.connection.create_database(config['database'], config.merge('encoding' => @encoding)) ActiveRecord::Base.establish_connection(config) - rescue - $stderr.puts $!, *($!.backtrace) + rescue Exception => e + $stderr.puts e, *(e.backtrace) $stderr.puts "Couldn't create database for #{config.inspect}" end end @@ -299,8 +299,7 @@ namespace :db do desc 'Load the seed data from db/seeds.rb' task :seed => 'db:abort_if_pending_migrations' do - seed_file = File.join(Rails.root, 'db', 'seeds.rb') - load(seed_file) if File.exist?(seed_file) + Rails.application.load_seed end namespace :fixtures do diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index 5584397439..b41e935ed5 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -1,7 +1,7 @@ require 'active_support/core_ext/object/blank' module ActiveRecord - module Batches # :nodoc: + module Batches # Yields each record that was found by the find +options+. The find is # performed by find_in_batches with a batch size of 1000 (or as # specified by the <tt>:batch_size</tt> option). diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index 398ab75b69..12a2c6aec3 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -225,13 +225,11 @@ module ActiveRecord key_records = key_records.inject({}) { |hsh, r| hsh.merge(r.id => r) } end - calculated_data.inject(ActiveSupport::OrderedHash.new) do |all, row| - key = type_cast_calculated_value(row[group_alias], group_column) - key = key_records[key] if associated - value = row[aggregate_alias] - all[key] = type_cast_calculated_value(value, column_for(column_name), operation) - all - end + ActiveSupport::OrderedHash[calculated_data.map do |row| + key = type_cast_calculated_value(row[group_alias], group_column) + key = key_records[key] if associated + [key, type_cast_calculated_value(row[aggregate_alias], column_for(column_name), operation)] + end] end # Converts the given keys to the value that the database adapter returns as diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index de57725af2..6a33edeb97 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -222,7 +222,7 @@ module ActiveRecord def build_joins(relation, joins) association_joins = [] - joins = @joins_values.map {|j| j.respond_to?(:strip) ? j.strip : j}.uniq + joins = joins.map {|j| j.respond_to?(:strip) ? j.strip : j}.uniq joins.each do |join| association_joins << join if [Hash, Array, Symbol].include?(join.class) && !array_of_strings?(join) @@ -260,8 +260,8 @@ 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 + # 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) 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 8bb8edde0e..2bdf9d8971 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 @@ -425,7 +425,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase def test_removing_associations_on_destroy david = DeveloperWithBeforeDestroyRaise.find(1) assert !david.projects.empty? - assert_nothing_raised { david.destroy } + assert_raise(RuntimeError) { david.destroy } assert david.projects.empty? assert DeveloperWithBeforeDestroyRaise.connection.select_all("SELECT * FROM developers_projects WHERE developer_id = 1").empty? end diff --git a/activerecord/test/cases/associations/join_model_test.rb b/activerecord/test/cases/associations/join_model_test.rb index b83966e91c..f131dc01f6 100644 --- a/activerecord/test/cases/associations/join_model_test.rb +++ b/activerecord/test/cases/associations/join_model_test.rb @@ -44,6 +44,16 @@ class AssociationsJoinModelTest < ActiveRecord::TestCase assert !authors(:mary).unique_categorized_posts.loaded? end + def test_column_caching + # pre-heat our cache + Post.arel_table.columns + Comment.columns + + Post.connection.column_calls = 0 + 2.times { Post.joins(:comments).to_a } + assert_equal 0, Post.connection.column_calls + end + def test_has_many_uniq_through_find assert_equal 1, authors(:mary).unique_categorized_posts.find(:all).size end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index dcb1da7d91..d87f259f4b 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -659,6 +659,10 @@ class BasicsTest < ActiveRecord::TestCase cloned_topic.save assert !cloned_topic.new_record? assert_not_equal cloned_topic.id, topic.id + + cloned_topic.reload + # FIXME: I think this is poor behavior, and will fix it with #5686 + assert_equal({'a' => 'c'}.to_s, cloned_topic.title) end def test_clone_with_aggregate_of_same_name_as_attribute @@ -1433,19 +1437,4 @@ class BasicsTest < ActiveRecord::TestCase ensure Object.class_eval{ remove_const :UnloadablePost } if defined?(UnloadablePost) end - - protected - def with_env_tz(new_tz = 'US/Eastern') - old_tz, ENV['TZ'] = ENV['TZ'], new_tz - yield - ensure - old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ') - end - - def with_active_record_default_timezone(zone) - old_zone, ActiveRecord::Base.default_timezone = ActiveRecord::Base.default_timezone, zone - yield - ensure - ActiveRecord::Base.default_timezone = old_zone - end end diff --git a/activerecord/test/cases/date_time_test.rb b/activerecord/test/cases/date_time_test.rb index a8b4b7a096..3deb0dac99 100644 --- a/activerecord/test/cases/date_time_test.rb +++ b/activerecord/test/cases/date_time_test.rb @@ -4,17 +4,21 @@ require 'models/task' class DateTimeTest < ActiveRecord::TestCase def test_saves_both_date_and_time - time_values = [1807, 2, 10, 15, 30, 45] - # create DateTime value with local time zone offset - local_offset = Rational(Time.local_time(*time_values).utc_offset, 86400) - now = DateTime.civil(*(time_values + [local_offset])) + with_env_tz 'America/New_York' do + with_active_record_default_timezone :utc do + time_values = [1807, 2, 10, 15, 30, 45] + # create DateTime value with local time zone offset + local_offset = Rational(Time.local_time(*time_values).utc_offset, 86400) + now = DateTime.civil(*(time_values + [local_offset])) - task = Task.new - task.starting = now - task.save! + task = Task.new + task.starting = now + task.save! - # check against Time.local_time, since some platforms will return a Time instead of a DateTime - assert_equal Time.local_time(*time_values), Task.find(task.id).starting + # check against Time.local_time, since some platforms will return a Time instead of a DateTime + assert_equal Time.local_time(*time_values), Task.find(task.id).starting + end + end end def test_assign_empty_date_time diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 4bf3c25d28..2d3047c875 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -31,6 +31,20 @@ def current_adapter?(*types) end end +def with_env_tz(new_tz = 'US/Eastern') + old_tz, ENV['TZ'] = ENV['TZ'], new_tz + yield +ensure + old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ') +end + +def with_active_record_default_timezone(zone) + old_zone, ActiveRecord::Base.default_timezone = ActiveRecord::Base.default_timezone, zone + yield +ensure + ActiveRecord::Base.default_timezone = old_zone +end + ActiveRecord::Base.connection.class.class_eval do IGNORED_SQL = [/^PRAGMA/, /^SELECT currval/, /^SELECT CAST/, /^SELECT @@IDENTITY/, /^SELECT @@ROWCOUNT/, /^SAVEPOINT/, /^ROLLBACK TO SAVEPOINT/, /^RELEASE SAVEPOINT/, /SHOW FIELDS/] @@ -43,6 +57,18 @@ ActiveRecord::Base.connection.class.class_eval do alias_method_chain :execute, :query_record end +ActiveRecord::Base.connection.class.class_eval { + attr_accessor :column_calls + + def columns_with_calls(*args) + @column_calls ||= 0 + @column_calls += 1 + columns_without_calls(*args) + end + + alias_method_chain :columns, :calls +} + unless ENV['FIXTURE_DEBUG'] module ActiveRecord::TestFixtures::ClassMethods def try_to_load_dependency_with_silence(*args) diff --git a/activerecord/test/cases/migration_test.rb b/activerecord/test/cases/migration_test.rb index 03a8cc90f6..bcae46c7e8 100644 --- a/activerecord/test/cases/migration_test.rb +++ b/activerecord/test/cases/migration_test.rb @@ -127,16 +127,17 @@ if ActiveRecord::Base.connection.supports_migrations? def test_add_index_length_limit good_index_name = 'x' * Person.connection.index_name_length too_long_index_name = good_index_name + 'x' - assert_nothing_raised { Person.connection.add_index("people", "first_name", :name => too_long_index_name) } + assert_raise(ArgumentError) { Person.connection.add_index("people", "first_name", :name => too_long_index_name) } assert !Person.connection.index_name_exists?("people", too_long_index_name, false) assert_nothing_raised { Person.connection.add_index("people", "first_name", :name => good_index_name) } assert Person.connection.index_name_exists?("people", good_index_name, false) + Person.connection.remove_index("people", :name => good_index_name) end def test_remove_nonexistent_index # we do this by name, so OpenBase is a wash as noted above unless current_adapter?(:OpenBaseAdapter) - assert_nothing_raised { Person.connection.remove_index("people", "no_such_index") } + assert_raise(ArgumentError) { Person.connection.remove_index("people", "no_such_index") } end end @@ -154,7 +155,7 @@ if ActiveRecord::Base.connection.supports_migrations? def test_double_add_index unless current_adapter?(:OpenBaseAdapter) Person.connection.add_index('people', [:first_name], :name => 'some_idx') - assert_nothing_raised { Person.connection.add_index('people', [:first_name], :name => 'some_idx') } + assert_raise(ArgumentError) { Person.connection.add_index('people', [:first_name], :name => 'some_idx') } end end diff --git a/activerecord/test/cases/relation_scoping_test.rb b/activerecord/test/cases/relation_scoping_test.rb index f538d2a326..b571e9a8b8 100644 --- a/activerecord/test/cases/relation_scoping_test.rb +++ b/activerecord/test/cases/relation_scoping_test.rb @@ -161,7 +161,7 @@ class NestedRelationScopingTest < ActiveRecord::TestCase def test_merge_inner_scope_has_priority Developer.limit(5).scoping do Developer.limit(10).scoping do - assert_equal 10, Developer.count + assert_equal 10, Developer.all.size end end end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 649fe79fd3..3bc3671b77 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -679,24 +679,36 @@ class RelationTest < ActiveRecord::TestCase assert_equal Post.order(Post.arel_table[:title]).all, Post.order("title").all end - def test_relations_limit_with_conditions_or_limit - assert_equal Post.limit(2).size, Post.limit(2).all.size - end - def test_order_with_find_with_order - assert_equal 'zyke', Car.order('name desc').find(:first, :order => 'id').name + assert_equal 'zyke', CoolCar.order('name desc').find(:first, :order => 'id').name + assert_equal 'zyke', FastCar.order('name desc').find(:first, :order => 'id').name end def test_default_scope_order_with_named_scope_order - assert_equal 'zyke', Car.order_using_new_style.limit(1).first.name - assert_equal 'zyke', Car.order_using_old_style.limit(1).first.name + assert_equal 'zyke', CoolCar.order_using_new_style.limit(1).first.name + assert_equal 'zyke', CoolCar.order_using_old_style.limit(1).first.name + assert_equal 'zyke', FastCar.order_using_new_style.limit(1).first.name + assert_equal 'zyke', FastCar.order_using_old_style.limit(1).first.name end def test_order_using_scoping - car = Car.order('id DESC').scoping do - Car.find(:first, :order => 'id asc') + car1 = CoolCar.order('id DESC').scoping do + CoolCar.find(:first, :order => 'id asc') + end + assert_equal 'zyke', car1.name + + car2 = FastCar.order('id DESC').scoping do + FastCar.find(:first, :order => 'id asc') end - assert_equal 'zyke', car.name + assert_equal 'zyke', car2.name + end + + def test_unscoped_block_style + assert_equal 'honda', CoolCar.unscoped { CoolCar.order_using_new_style.limit(1).first.name} + assert_equal 'honda', CoolCar.unscoped { CoolCar.order_using_old_style.limit(1).first.name} + + assert_equal 'honda', FastCar.unscoped { FastCar.order_using_new_style.limit(1).first.name} + assert_equal 'honda', FastCar.unscoped { FastCar.order_using_old_style.limit(1).first.name} end def test_intersection_with_array diff --git a/activerecord/test/models/car.rb b/activerecord/test/models/car.rb index a1e351b54e..e7db3d3423 100644 --- a/activerecord/test/models/car.rb +++ b/activerecord/test/models/car.rb @@ -1,4 +1,5 @@ class Car < ActiveRecord::Base + has_many :bulbs has_many :tyres has_many :engines @@ -7,9 +8,15 @@ class Car < ActiveRecord::Base scope :incl_tyres, includes(:tyres) scope :incl_engines, includes(:engines) - default_scope :order => 'name desc' - scope :order_using_new_style, order('name asc') scope :order_using_old_style, :order => 'name asc' end + +class CoolCar < Car + default_scope :order => 'name desc' +end + +class FastCar < Car + default_scope order('name desc') +end diff --git a/activerecord/test/models/engine.rb b/activerecord/test/models/engine.rb index 751c3f02d1..851ff8c22b 100644 --- a/activerecord/test/models/engine.rb +++ b/activerecord/test/models/engine.rb @@ -1,3 +1,4 @@ class Engine < ActiveRecord::Base belongs_to :my_car, :class_name => 'Car', :foreign_key => 'car_id', :counter_cache => :engines_count end + diff --git a/activeresource/lib/active_resource/base.rb b/activeresource/lib/active_resource/base.rb index 7963aa462f..3016f62b80 100644 --- a/activeresource/lib/active_resource/base.rb +++ b/activeresource/lib/active_resource/base.rb @@ -589,8 +589,8 @@ module ActiveResource def prefix(options={}) "#{prefix_call}" end RUBY_EVAL end - rescue - logger.error "Couldn't set prefix: #{$!}\n #{code}" if logger + rescue Exception => e + logger.error "Couldn't set prefix: #{e}\n #{code}" if logger raise end @@ -987,10 +987,7 @@ module ActiveResource # not_ryan.hash # => {:not => "an ARes instance"} def clone # Clone all attributes except the pk and any nested ARes - cloned = attributes.reject {|k,v| k == self.class.primary_key || v.is_a?(ActiveResource::Base)}.inject({}) do |attrs, (k, v)| - attrs[k] = v.clone - attrs - end + cloned = Hash[attributes.reject {|k,v| k == self.class.primary_key || v.is_a?(ActiveResource::Base)}.map { |k, v| [k, v.clone] }] # Form the new resource - bypass initialize of resource with 'new' as that will call 'load' which # attempts to convert hashes into member objects and arrays into collections of objects. We want # the raw objects to be cloned so we bypass load by directly setting the attributes hash. @@ -1318,7 +1315,7 @@ module ActiveResource end def load_attributes_from_response(response) - if response['Content-Length'] != "0" && response.body.strip.size > 0 + if !response['Content-Length'].blank? && response['Content-Length'] != "0" && !response.body.nil? && response.body.strip.size > 0 load(self.class.format.decode(response.body)) end end diff --git a/activeresource/lib/active_resource/schema.rb b/activeresource/lib/active_resource/schema.rb index 5758ac9502..3fd37a9bb6 100644 --- a/activeresource/lib/active_resource/schema.rb +++ b/activeresource/lib/active_resource/schema.rb @@ -4,7 +4,7 @@ module ActiveResource # :nodoc: class Schema # :nodoc: # attributes can be known to be one of these types. They are easy to # cast to/from. - KNOWN_ATTRIBUTE_TYPES = %w( string integer float ) + KNOWN_ATTRIBUTE_TYPES = %w( string text integer float decimal datetime timestamp time date binary boolean ) # An array of attribute definitions, representing the attributes that # have been defined. @@ -39,8 +39,6 @@ module ActiveResource # :nodoc: # The following are the attribute types supported by Active Resource # migrations. - # TODO: We should eventually support all of these: - # %w( string text integer float decimal datetime timestamp time date binary boolean ).each do |attr_type| KNOWN_ATTRIBUTE_TYPES.each do |attr_type| class_eval <<-EOV, __FILE__, __LINE__ + 1 def #{attr_type.to_s}(*args) diff --git a/activeresource/test/abstract_unit.rb b/activeresource/test/abstract_unit.rb index fcb770d612..129efeb879 100644 --- a/activeresource/test/abstract_unit.rb +++ b/activeresource/test/abstract_unit.rb @@ -18,3 +18,101 @@ begin require 'ruby-debug' rescue LoadError end + +def setup_response + @default_request_headers = { 'Content-Type' => 'application/xml' } + @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') + @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') + @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') + @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') + @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") + @joe = { 'person' => { :id => 6, :name => 'Joe' }}.to_json + @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people') + @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people') + @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') + + # - deep nested resource - + # - Luis (Customer) + # - JK (Customer::Friend) + # - Mateo (Customer::Friend::Brother) + # - Edith (Customer::Friend::Brother::Child) + # - Martha (Customer::Friend::Brother::Child) + # - Felipe (Customer::Friend::Brother) + # - Bryan (Customer::Friend::Brother::Child) + # - Luke (Customer::Friend::Brother::Child) + # - Eduardo (Customer::Friend) + # - Sebas (Customer::Friend::Brother) + # - Andres (Customer::Friend::Brother::Child) + # - Jorge (Customer::Friend::Brother::Child) + # - Elsa (Customer::Friend::Brother) + # - Natacha (Customer::Friend::Brother::Child) + # - Milena (Customer::Friend::Brother) + # + @luis = {:id => 1, :name => 'Luis', + :friends => [{:name => 'JK', + :brothers => [{:name => 'Mateo', + :children => [{:name => 'Edith'},{:name => 'Martha'}]}, + {:name => 'Felipe', + :children => [{:name => 'Bryan'},{:name => 'Luke'}]}]}, + {:name => 'Eduardo', + :brothers => [{:name => 'Sebas', + :children => [{:name => 'Andres'},{:name => 'Jorge'}]}, + {:name => 'Elsa', + :children => [{:name => 'Natacha'}]}, + {:name => 'Milena', + :children => []}]}]}.to_xml(:root => 'customer') + # - resource with yaml array of strings; for ARs using serialize :bar, Array + @marty = <<-eof.strip + <?xml version=\"1.0\" encoding=\"UTF-8\"?> + <person> + <id type=\"integer\">5</id> + <name>Marty</name> + <colors type=\"yaml\">--- + - \"red\" + - \"green\" + - \"blue\" + </colors> + </person> + eof + + ActiveResource::HttpMock.respond_to do |mock| + mock.get "/people/1.xml", {}, @matz + mock.get "/people/2.xml", {}, @david + mock.get "/people/5.xml", {}, @marty + mock.get "/people/Greg.xml", {}, @greg + mock.get "/people/6.json", {}, @joe + mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 + mock.put "/people/1.xml", {}, nil, 204 + mock.delete "/people/1.xml", {}, nil, 200 + mock.delete "/people/2.xml", {}, nil, 400 + mock.get "/people/99.xml", {}, nil, 404 + mock.post "/people.xml", {}, @rick, 201, 'Location' => '/people/5.xml' + mock.get "/people.xml", {}, @people + mock.get "/people/1/addresses.xml", {}, @addresses + mock.get "/people/1/addresses/1.xml", {}, @addy + mock.get "/people/1/addresses/2.xml", {}, nil, 404 + mock.get "/people/2/addresses.xml", {}, nil, 404 + mock.get "/people/2/addresses/1.xml", {}, nil, 404 + mock.get "/people/Greg/addresses/1.xml", {}, @addy + mock.put "/people/1/addresses/1.xml", {}, nil, 204 + mock.delete "/people/1/addresses/1.xml", {}, nil, 200 + mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' + mock.get "/people//addresses.xml", {}, nil, 404 + mock.get "/people//addresses/1.xml", {}, nil, 404 + mock.put "/people//addresses/1.xml", {}, nil, 404 + mock.delete "/people//addresses/1.xml", {}, nil, 404 + mock.post "/people//addresses.xml", {}, nil, 404 + mock.head "/people/1.xml", {}, nil, 200 + mock.head "/people/Greg.xml", {}, nil, 200 + mock.head "/people/99.xml", {}, nil, 404 + mock.head "/people/1/addresses/1.xml", {}, nil, 200 + mock.head "/people/1/addresses/2.xml", {}, nil, 404 + mock.head "/people/2/addresses/1.xml", {}, nil, 404 + mock.head "/people/Greg/addresses/1.xml", {}, nil, 200 + # customer + mock.get "/customers/1.xml", {}, @luis + end + + Person.user = nil + Person.password = nil +end diff --git a/activeresource/test/cases/base/schema_test.rb b/activeresource/test/cases/base/schema_test.rb index 19861d7b32..37f30e4353 100644 --- a/activeresource/test/cases/base/schema_test.rb +++ b/activeresource/test/cases/base/schema_test.rb @@ -8,58 +8,13 @@ require "fixtures/street_address" ######################################################################## class SchemaTest < ActiveModel::TestCase def setup - @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') - @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') - @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') - @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') - @default_request_headers = { 'Content-Type' => 'application/xml' } - @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") - @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people') - @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people') - @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') - - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/people/1.xml", {}, @matz - mock.get "/people/2.xml", {}, @david - mock.get "/people/Greg.xml", {}, @greg - mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 - mock.get "/people/5.xml", {}, @rick - mock.put "/people/1.xml", {}, nil, 204 - mock.delete "/people/1.xml", {}, nil, 200 - mock.delete "/people/2.xml", {}, nil, 400 - mock.get "/people/99.xml", {}, nil, 404 - mock.post "/people.xml", {}, @rick, 201, 'Location' => '/people/5.xml' - mock.get "/people.xml", {}, @people - mock.get "/people/1/addresses.xml", {}, @addresses - mock.get "/people/1/addresses/1.xml", {}, @addy - mock.get "/people/1/addresses/2.xml", {}, nil, 404 - mock.get "/people/2/addresses/1.xml", {}, nil, 404 - mock.get "/people/Greg/addresses/1.xml", {}, @addy - mock.put "/people/1/addresses/1.xml", {}, nil, 204 - mock.delete "/people/1/addresses/1.xml", {}, nil, 200 - mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' - mock.get "/people//addresses.xml", {}, nil, 404 - mock.get "/people//addresses/1.xml", {}, nil, 404 - mock.put "/people//addressaddresseses/1.xml", {}, nil, 404 - mock.delete "/people//addresses/1.xml", {}, nil, 404 - mock.post "/people//addresses.xml", {}, nil, 404 - mock.head "/people/1.xml", {}, nil, 200 - mock.head "/people/Greg.xml", {}, nil, 200 - mock.head "/people/99.xml", {}, nil, 404 - mock.head "/people/1/addresses/1.xml", {}, nil, 200 - mock.head "/people/1/addresses/2.xml", {}, nil, 404 - mock.head "/people/2/addresses/1.xml", {}, nil, 404 - mock.head "/people/Greg/addresses/1.xml", {}, nil, 200 - end - - Person.user = nil - Person.password = nil + setup_response # find me in abstract_unit end + def teardown Person.schema = nil # hack to stop test bleedthrough... end - ##################################################### # Passing in a schema directly and returning it #### @@ -78,14 +33,23 @@ class SchemaTest < ActiveModel::TestCase end test "schema should accept a simple hash" do - new_schema = {'age' => 'integer', 'name' => 'string'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} + assert_nothing_raised { Person.schema = new_schema } assert_equal new_schema, Person.schema end test "schema should accept a hash with simple values" do - new_schema = {'age' => 'integer', 'name' => 'string', 'height' => 'float', 'mydatetime' => 'string'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} assert_nothing_raised { Person.schema = new_schema } assert_equal new_schema, Person.schema @@ -109,7 +73,12 @@ class SchemaTest < ActiveModel::TestCase end test "schema should accept nil and remove the schema" do - new_schema = {'age' => 'integer', 'name' => 'string'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} + assert_nothing_raised { Person.schema = new_schema } assert_equal new_schema, Person.schema # sanity check @@ -120,7 +89,12 @@ class SchemaTest < ActiveModel::TestCase test "schema should be with indifferent access" do - new_schema = {:age => :integer, 'name' => 'string'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} + new_schema_syms = new_schema.keys assert_nothing_raised { Person.schema = new_schema } @@ -156,7 +130,12 @@ class SchemaTest < ActiveModel::TestCase test "defining a schema should return it when asked" do assert Person.schema.blank?, "should have a blank class schema" - new_schema = {'name' => 'string', 'age' => 'integer', 'height' => 'float', 'weight' => 'float'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} + assert_nothing_raised { Person.schema = new_schema assert_equal new_schema, Person.schema, "should have saved the schema on the class" @@ -167,7 +146,11 @@ class SchemaTest < ActiveModel::TestCase test "defining a schema, then fetching a model should still match the defined schema" do # sanity checks assert Person.schema.blank?, "should have a blank class schema" - new_schema = {'name' => 'string', 'age' => 'integer', 'height' => 'float', 'weight' => 'float'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} matz = Person.find(1) assert !matz.schema.blank?, "should have some sort of schema on an instance variable" @@ -368,12 +351,16 @@ class SchemaTest < ActiveModel::TestCase end test "setting schema should set known attributes on class and instance" do - new_schema = {'age' => 'integer', 'name' => 'string'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} assert_nothing_raised { Person.schema = new_schema } - assert_equal new_schema.keys, Person.known_attributes - assert_equal new_schema.keys, Person.new.known_attributes + assert_equal new_schema.keys.sort, Person.known_attributes.sort + assert_equal new_schema.keys.sort, Person.new.known_attributes.sort end test "known attributes on a fetched resource should return all the attributes of the instance" do @@ -401,7 +388,11 @@ class SchemaTest < ActiveModel::TestCase test "setting schema then fetching should add schema attributes to the instance attributes" do # an attribute in common with fetched instance and one that isn't - new_schema = {'name' => 'string', 'my_strange_attribute' => 'string'} + new_schema = {'age' => 'integer', 'name' => 'string', + 'height' => 'float', 'bio' => 'text', + 'weight' => 'decimal', 'photo' => 'binary', + 'alive' => 'boolean', 'created_at' => 'timestamp', + 'thetime' => 'time', 'thedate' => 'date', 'mydatetime' => 'datetime'} assert_nothing_raised { Person.schema = new_schema } diff --git a/activeresource/test/cases/base_test.rb b/activeresource/test/cases/base_test.rb index 77135be146..b5914683e9 100644 --- a/activeresource/test/cases/base_test.rb +++ b/activeresource/test/cases/base_test.rb @@ -12,101 +12,8 @@ require 'mocha' class BaseTest < Test::Unit::TestCase def setup - @default_request_headers = { 'Content-Type' => 'application/xml' } - @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') - @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') - @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') - @addy = { :id => 1, :street => '12345 Street', :country => 'Australia' }.to_xml(:root => 'address') - @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") - @joe = { 'person' => { :id => 6, :name => 'Joe' }}.to_json - @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people') - @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people') - @addresses = [{ :id => 1, :street => '12345 Street', :country => 'Australia' }].to_xml(:root => 'addresses') - - # - deep nested resource - - # - Luis (Customer) - # - JK (Customer::Friend) - # - Mateo (Customer::Friend::Brother) - # - Edith (Customer::Friend::Brother::Child) - # - Martha (Customer::Friend::Brother::Child) - # - Felipe (Customer::Friend::Brother) - # - Bryan (Customer::Friend::Brother::Child) - # - Luke (Customer::Friend::Brother::Child) - # - Eduardo (Customer::Friend) - # - Sebas (Customer::Friend::Brother) - # - Andres (Customer::Friend::Brother::Child) - # - Jorge (Customer::Friend::Brother::Child) - # - Elsa (Customer::Friend::Brother) - # - Natacha (Customer::Friend::Brother::Child) - # - Milena (Customer::Friend::Brother) - # - @luis = {:id => 1, :name => 'Luis', - :friends => [{:name => 'JK', - :brothers => [{:name => 'Mateo', - :children => [{:name => 'Edith'},{:name => 'Martha'}]}, - {:name => 'Felipe', - :children => [{:name => 'Bryan'},{:name => 'Luke'}]}]}, - {:name => 'Eduardo', - :brothers => [{:name => 'Sebas', - :children => [{:name => 'Andres'},{:name => 'Jorge'}]}, - {:name => 'Elsa', - :children => [{:name => 'Natacha'}]}, - {:name => 'Milena', - :children => []}]}]}.to_xml(:root => 'customer') - # - resource with yaml array of strings; for ARs using serialize :bar, Array - @marty = <<-eof.strip - <?xml version=\"1.0\" encoding=\"UTF-8\"?> - <person> - <id type=\"integer\">5</id> - <name>Marty</name> - <colors type=\"yaml\">--- - - \"red\" - - \"green\" - - \"blue\" - </colors> - </person> - eof - - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/people/1.xml", {}, @matz - mock.get "/people/2.xml", {}, @david - mock.get "/people/6.json", {}, @joe - mock.get "/people/5.xml", {}, @marty - mock.get "/people/Greg.xml", {}, @greg - mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 - mock.put "/people/1.xml", {}, nil, 204 - mock.delete "/people/1.xml", {}, nil, 200 - mock.delete "/people/2.xml", {}, nil, 400 - mock.get "/people/99.xml", {}, nil, 404 - mock.post "/people.xml", {}, @rick, 201, 'Location' => '/people/5.xml' - mock.get "/people.xml", {}, @people - mock.get "/people/1/addresses.xml", {}, @addresses - mock.get "/people/1/addresses/1.xml", {}, @addy - mock.get "/people/1/addresses/2.xml", {}, nil, 404 - mock.get "/people/2/addresses/1.xml", {}, nil, 404 - mock.get "/people/Greg/addresses/1.xml", {}, @addy - mock.put "/people/1/addresses/1.xml", {}, nil, 204 - mock.delete "/people/1/addresses/1.xml", {}, nil, 200 - mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' - mock.get "/people//addresses.xml", {}, nil, 404 - mock.get "/people//addresses/1.xml", {}, nil, 404 - mock.put "/people//addresses/1.xml", {}, nil, 404 - mock.delete "/people//addresses/1.xml", {}, nil, 404 - mock.post "/people//addresses.xml", {}, nil, 404 - mock.head "/people/1.xml", {}, nil, 200 - mock.head "/people/Greg.xml", {}, nil, 200 - mock.head "/people/99.xml", {}, nil, 404 - mock.head "/people/1/addresses/1.xml", {}, nil, 200 - mock.head "/people/1/addresses/2.xml", {}, nil, 404 - mock.head "/people/2/addresses/1.xml", {}, nil, 404 - mock.head "/people/Greg/addresses/1.xml", {}, nil, 200 - # customer - mock.get "/customers/1.xml", {}, @luis - end - + setup_response # find me in abstract_unit @original_person_site = Person.site - Person.user = nil - Person.password = nil end def teardown @@ -713,6 +620,14 @@ class BaseTest < Test::Unit::TestCase assert_nil p.__send__(:id_from_response, resp) end + def test_load_attributes_from_response + p = Person.new + resp = ActiveResource::Response.new(nil) + resp['Content-Length'] = "100" + assert_nil p.__send__(:load_attributes_from_response, resp) + end + + def test_create_with_custom_prefix matzs_house = StreetAddress.new(:person_id => 1) matzs_house.save @@ -1041,7 +956,7 @@ class BaseTest < Test::Unit::TestCase ensure Person.element_name = old_elem_name end - + def test_to_xml_with_private_method_name_as_attribute assert_nothing_raised(ArgumentError) { Customer.new(:test => true).to_xml diff --git a/activeresource/test/cases/finder_test.rb b/activeresource/test/cases/finder_test.rb index 1f52868966..fd09ef46d7 100644 --- a/activeresource/test/cases/finder_test.rb +++ b/activeresource/test/cases/finder_test.rb @@ -8,101 +8,7 @@ require 'active_support/core_ext/hash/conversions' class FinderTest < Test::Unit::TestCase def setup - # TODO: refactor/DRY this setup - it's a copy of the BaseTest setup. - # We can probably put this into abstract_unit - @matz = { :id => 1, :name => 'Matz' }.to_xml(:root => 'person') - @david = { :id => 2, :name => 'David' }.to_xml(:root => 'person') - @greg = { :id => 3, :name => 'Greg' }.to_xml(:root => 'person') - @addy = { :id => 1, :street => '12345 Street' }.to_xml(:root => 'address') - @default_request_headers = { 'Content-Type' => 'application/xml' } - @rick = { :name => "Rick", :age => 25 }.to_xml(:root => "person") - @people = [{ :id => 1, :name => 'Matz' }, { :id => 2, :name => 'David' }].to_xml(:root => 'people') - @people_david = [{ :id => 2, :name => 'David' }].to_xml(:root => 'people') - @addresses = [{ :id => 1, :street => '12345 Street' }].to_xml(:root => 'addresses') - - # - deep nested resource - - # - Luis (Customer) - # - JK (Customer::Friend) - # - Mateo (Customer::Friend::Brother) - # - Edith (Customer::Friend::Brother::Child) - # - Martha (Customer::Friend::Brother::Child) - # - Felipe (Customer::Friend::Brother) - # - Bryan (Customer::Friend::Brother::Child) - # - Luke (Customer::Friend::Brother::Child) - # - Eduardo (Customer::Friend) - # - Sebas (Customer::Friend::Brother) - # - Andres (Customer::Friend::Brother::Child) - # - Jorge (Customer::Friend::Brother::Child) - # - Elsa (Customer::Friend::Brother) - # - Natacha (Customer::Friend::Brother::Child) - # - Milena (Customer::Friend::Brother) - # - @luis = {:id => 1, :name => 'Luis', - :friends => [{:name => 'JK', - :brothers => [{:name => 'Mateo', - :children => [{:name => 'Edith'},{:name => 'Martha'}]}, - {:name => 'Felipe', - :children => [{:name => 'Bryan'},{:name => 'Luke'}]}]}, - {:name => 'Eduardo', - :brothers => [{:name => 'Sebas', - :children => [{:name => 'Andres'},{:name => 'Jorge'}]}, - {:name => 'Elsa', - :children => [{:name => 'Natacha'}]}, - {:name => 'Milena', - :children => []}]}]}.to_xml(:root => 'customer') - # - resource with yaml array of strings; for ARs using serialize :bar, Array - @marty = <<-eof.strip - <?xml version=\"1.0\" encoding=\"UTF-8\"?> - <person> - <id type=\"integer\">5</id> - <name>Marty</name> - <colors type=\"yaml\">--- - - \"red\" - - \"green\" - - \"blue\" - </colors> - </person> - eof - - ActiveResource::HttpMock.respond_to do |mock| - mock.get "/people/1.xml", {}, @matz - mock.get "/people/2.xml", {}, @david - mock.get "/people/5.xml", {}, @marty - mock.get "/people/Greg.xml", {}, @greg - mock.get "/people/4.xml", {'key' => 'value'}, nil, 404 - mock.put "/people/1.xml", {}, nil, 204 - mock.delete "/people/1.xml", {}, nil, 200 - mock.delete "/people/2.xml", {}, nil, 400 - mock.get "/people/99.xml", {}, nil, 404 - mock.post "/people.xml", {}, @rick, 201, 'Location' => '/people/5.xml' - mock.get "/people.xml", {}, @people - mock.get "/people/1/addresses.xml", {}, @addresses - mock.get "/people/1/addresses/1.xml", {}, @addy - mock.get "/people/1/addresses/2.xml", {}, nil, 404 - mock.get "/people/2/addresses.xml", {}, nil, 404 - mock.get "/people/2/addresses/1.xml", {}, nil, 404 - mock.get "/people/Greg/addresses/1.xml", {}, @addy - mock.put "/people/1/addresses/1.xml", {}, nil, 204 - mock.delete "/people/1/addresses/1.xml", {}, nil, 200 - mock.post "/people/1/addresses.xml", {}, nil, 201, 'Location' => '/people/1/addresses/5' - mock.get "/people//addresses.xml", {}, nil, 404 - mock.get "/people//addresses/1.xml", {}, nil, 404 - mock.put "/people//addresses/1.xml", {}, nil, 404 - mock.delete "/people//addresses/1.xml", {}, nil, 404 - mock.post "/people//addresses.xml", {}, nil, 404 - mock.head "/people/1.xml", {}, nil, 200 - mock.head "/people/Greg.xml", {}, nil, 200 - mock.head "/people/99.xml", {}, nil, 404 - mock.head "/people/1/addresses/1.xml", {}, nil, 200 - mock.head "/people/1/addresses/2.xml", {}, nil, 404 - mock.head "/people/2/addresses/1.xml", {}, nil, 404 - mock.head "/people/Greg/addresses/1.xml", {}, nil, 200 - # customer - mock.get "/customers/1.xml", {}, @luis - end - - Person.user = nil - Person.password = nil + setup_response # find me in abstract_unit end def test_find_by_id diff --git a/activesupport/lib/active_support/cache.rb b/activesupport/lib/active_support/cache.rb index df35540b55..9098ffbfec 100644 --- a/activesupport/lib/active_support/cache.rb +++ b/activesupport/lib/active_support/cache.rb @@ -61,8 +61,8 @@ module ActiveSupport store_class = begin require "active_support/cache/#{store}" - rescue LoadError - raise "Could not find cache store adapter for #{store} (#{$!})" + rescue LoadError => e + raise "Could not find cache store adapter for #{store} (#{e})" else ActiveSupport::Cache.const_get(store_class_name) end diff --git a/activesupport/lib/active_support/core_ext/hash/conversions.rb b/activesupport/lib/active_support/core_ext/hash/conversions.rb index 7e929848a9..4e8ec5a3a8 100644 --- a/activesupport/lib/active_support/core_ext/hash/conversions.rb +++ b/activesupport/lib/active_support/core_ext/hash/conversions.rb @@ -114,10 +114,7 @@ class Hash elsif value['type'] && value.size == 1 && !value['type'].is_a?(::Hash) nil else - xml_value = value.inject({}) do |h,(k,v)| - h[k] = typecast_xml_value(v) - h - end + xml_value = Hash[value.map { |k,v| [k, typecast_xml_value(v)] }] # Turn { :files => { :file => #<StringIO> } into { :files => #<StringIO> } so it is compatible with # how multipart uploaded files from HTML appear @@ -136,10 +133,7 @@ class Hash def unrename_keys(params) case params.class.to_s when "Hash" - params.inject({}) do |h,(k,v)| - h[k.to_s.tr("-", "_")] = unrename_keys(v) - h - end + Hash[params.map { |k,v| [k.to_s.tr("-", "_"), unrename_keys(v)] } ] when "Array" params.map { |v| unrename_keys(v) } else diff --git a/activesupport/lib/active_support/core_ext/kernel/reporting.rb b/activesupport/lib/active_support/core_ext/kernel/reporting.rb index 5105c40e4d..37a827123a 100644 --- a/activesupport/lib/active_support/core_ext/kernel/reporting.rb +++ b/activesupport/lib/active_support/core_ext/kernel/reporting.rb @@ -59,4 +59,23 @@ module Kernel raise unless exception_classes.any? { |cls| e.kind_of?(cls) } end end + + # Captures the given stream and returns it: + # + # stream = capture(:stdout){ puts "Cool" } + # stream # => "Cool\n" + # + def capture(stream) + begin + stream = stream.to_s + eval "$#{stream} = StringIO.new" + yield + result = eval("$#{stream}").string + ensure + eval("$#{stream} = #{stream.upcase}") + end + + result + end + alias :silence :capture end diff --git a/activesupport/lib/active_support/core_ext/numeric/time.rb b/activesupport/lib/active_support/core_ext/numeric/time.rb index d32a9e0531..e73915ffcf 100644 --- a/activesupport/lib/active_support/core_ext/numeric/time.rb +++ b/activesupport/lib/active_support/core_ext/numeric/time.rb @@ -60,7 +60,7 @@ class Numeric alias :fortnight :fortnights # Reads best without arguments: 10.minutes.ago - def ago(time = ::Time.now) + def ago(time = ::Time.current) time - self end @@ -68,7 +68,7 @@ class Numeric alias :until :ago # Reads best with argument: 10.minutes.since(time) - def since(time = ::Time.now) + def since(time = ::Time.current) time + self end diff --git a/activesupport/lib/active_support/core_ext/object/instance_variables.rb b/activesupport/lib/active_support/core_ext/object/instance_variables.rb index 97b288c608..77a3cfc21d 100644 --- a/activesupport/lib/active_support/core_ext/object/instance_variables.rb +++ b/activesupport/lib/active_support/core_ext/object/instance_variables.rb @@ -10,10 +10,7 @@ class Object # # C.new(0, 1).instance_values # => {"x" => 0, "y" => 1} def instance_values #:nodoc: - instance_variables.inject({}) do |values, name| - values[name.to_s[1..-1]] = instance_variable_get(name) - values - end + Hash[instance_variables.map { |name| [name.to_s[1..-1], instance_variable_get(name)] }] end # Returns an array of instance variable names including "@". They are strings diff --git a/activesupport/lib/active_support/json/encoding.rb b/activesupport/lib/active_support/json/encoding.rb index 6e9d62bd16..c8cac52027 100644 --- a/activesupport/lib/active_support/json/encoding.rb +++ b/activesupport/lib/active_support/json/encoding.rb @@ -232,9 +232,8 @@ class Hash # use encoder as a proxy to call as_json on all values in the subset, to protect from circular references encoder = options && options[:encoder] || ActiveSupport::JSON::Encoding::Encoder.new(options) - pairs = subset.map { |k, v| [k.to_s, encoder.as_json(v)] } - result = self.is_a?(ActiveSupport::OrderedHash) ? ActiveSupport::OrderedHash.new : Hash.new - pairs.inject(result) { |hash, pair| hash[pair.first] = pair.last; hash } + result = self.is_a?(ActiveSupport::OrderedHash) ? ActiveSupport::OrderedHash : Hash + result[subset.map { |k, v| [k.to_s, encoder.as_json(v)] }] end def encode_json(encoder) diff --git a/activesupport/lib/active_support/testing/performance.rb b/activesupport/lib/active_support/testing/performance.rb index f7ddf6421d..64b436ba8c 100644 --- a/activesupport/lib/active_support/testing/performance.rb +++ b/activesupport/lib/active_support/testing/performance.rb @@ -58,16 +58,16 @@ begin metric.send(mode) { __send__ @method_name } rescue ::Test::Unit::AssertionFailedError => e add_failure(e.message, e.backtrace) - rescue StandardError, ScriptError - add_error($!) + rescue StandardError, ScriptError => e + add_error(e) ensure begin teardown run_callbacks :teardown, :enumerator => :reverse_each rescue ::Test::Unit::AssertionFailedError => e add_failure(e.message, e.backtrace) - rescue StandardError, ScriptError - add_error($!) + rescue StandardError, ScriptError => e + add_error(e) end end diff --git a/activesupport/test/core_ext/duration_test.rb b/activesupport/test/core_ext/duration_test.rb index b6456f0a30..bb453b8d7f 100644 --- a/activesupport/test/core_ext/duration_test.rb +++ b/activesupport/test/core_ext/duration_test.rb @@ -53,8 +53,8 @@ class DurationTest < ActiveSupport::TestCase flunk("no exception was raised") rescue ArgumentError => e assert_equal 'expected a time or date, got ""', e.message, "ensure ArgumentError is not being raised by dependencies.rb" - rescue Exception - flunk("ArgumentError should be raised, but we got #{$!.class} instead") + rescue Exception => e + flunk("ArgumentError should be raised, but we got #{e.class} instead") end end diff --git a/activesupport/test/core_ext/kernel_test.rb b/activesupport/test/core_ext/kernel_test.rb index 904d56a87e..ede9b0a6aa 100644 --- a/activesupport/test/core_ext/kernel_test.rb +++ b/activesupport/test/core_ext/kernel_test.rb @@ -52,6 +52,11 @@ class KernelTest < Test::Unit::TestCase class << o; @x = 1; end assert_equal 1, o.class_eval { @x } end + + def test_capture + assert_equal 'STDERR', capture(:stderr) {$stderr.print 'STDERR'} + assert_equal 'STDOUT', capture(:stdout) {print 'STDOUT'} + end end class KernelSuppressTest < Test::Unit::TestCase diff --git a/activesupport/test/core_ext/numeric_ext_test.rb b/activesupport/test/core_ext/numeric_ext_test.rb index e40b487753..6ef4e37b26 100644 --- a/activesupport/test/core_ext/numeric_ext_test.rb +++ b/activesupport/test/core_ext/numeric_ext_test.rb @@ -88,6 +88,44 @@ class NumericExtTimeAndDateTimeTest < Test::Unit::TestCase assert_equal Time.utc(2005,2,28,15,15,10), Time.utc(2004,2,29,15,15,10) + 1.year assert_equal DateTime.civil(2005,2,28,15,15,10), DateTime.civil(2004,2,29,15,15,10) + 1.year end + + def test_since_and_ago_anchored_to_time_now_when_time_zone_default_not_set + Time.zone_default = nil + with_env_tz 'US/Eastern' do + Time.stubs(:now).returns Time.local(2000) + # since + assert_equal false, 5.since.is_a?(ActiveSupport::TimeWithZone) + assert_equal Time.local(2000,1,1,0,0,5), 5.since + # ago + assert_equal false, 5.ago.is_a?(ActiveSupport::TimeWithZone) + assert_equal Time.local(1999,12,31,23,59,55), 5.ago + end + end + + def test_since_and_ago_anchored_to_time_zone_now_when_time_zone_default_set + Time.zone_default = ActiveSupport::TimeZone['Eastern Time (US & Canada)'] + with_env_tz 'US/Eastern' do + Time.stubs(:now).returns Time.local(2000) + # since + assert_equal true, 5.since.is_a?(ActiveSupport::TimeWithZone) + assert_equal Time.utc(2000,1,1,0,0,5), 5.since.time + assert_equal 'Eastern Time (US & Canada)', 5.since.time_zone.name + # ago + assert_equal true, 5.ago.is_a?(ActiveSupport::TimeWithZone) + assert_equal Time.utc(1999,12,31,23,59,55), 5.ago.time + assert_equal 'Eastern Time (US & Canada)', 5.ago.time_zone.name + end + ensure + Time.zone_default = nil + end + + protected + def with_env_tz(new_tz = 'US/Eastern') + old_tz, ENV['TZ'] = ENV['TZ'], new_tz + yield + ensure + old_tz ? ENV['TZ'] = old_tz : ENV.delete('TZ') + end end class NumericExtDateTest < Test::Unit::TestCase diff --git a/activesupport/test/ordered_hash_test.rb b/activesupport/test/ordered_hash_test.rb index f47e896487..50778b5864 100644 --- a/activesupport/test/ordered_hash_test.rb +++ b/activesupport/test/ordered_hash_test.rb @@ -217,8 +217,8 @@ class OrderedHashTest < Test::Unit::TestCase ActiveSupport::OrderedHash[1,2,3,4,5] flunk "Hash::[] should have raised an exception on initialization " + "with an odd number of parameters" - rescue - assert_equal "odd number of arguments for Hash", $!.message + rescue ArgumentError => e + assert_equal "odd number of arguments for Hash", e.message end end diff --git a/ci/ci_build.rb b/ci/ci_build.rb index 9539e47cdc..6bc0c69112 100755 --- a/ci/ci_build.rb +++ b/ci/ci_build.rb @@ -38,6 +38,7 @@ cd "#{root_dir}/activesupport" do # build_results[:activesupport_isolated] = rake 'test:isolated' end +system "sudo rm -R #{root_dir}/railties/tmp" cd "#{root_dir}/railties" do puts puts "[CruiseControl] Building RailTies" diff --git a/railties/guides/source/api_documentation_guidelines.textile b/railties/guides/source/api_documentation_guidelines.textile index d2ccb6475e..c2131ff450 100644 --- a/railties/guides/source/api_documentation_guidelines.textile +++ b/railties/guides/source/api_documentation_guidelines.textile @@ -29,7 +29,9 @@ Documentation has to be concise but comprehensive. Explore and document edge cas The proper names of Rails components have a space in between the words, like "Active Support". +ActiveRecord+ is a Ruby module, whereas Active Record is an ORM. Historically there has been lack of consistency regarding this, but we checked with David when docrails started. All Rails documentation consistently refer to Rails components by their proper name, and if in your next blog post or presentation you remember this tidbit and take it into account that'd be fenomenal :). -Spell names correctly: HTML, MySQL, JavaScript, ERb. Use the article "an" for "SQL", as in "an SQL statement". Also "an SQLite database". +Spell names correctly: Arel, Test::Unit, RSpec, HTML, MySQL, JavaScript, ERb. When in doubt, please have a look at some authoritative source like their official documentation. + +Use the article "an" for "SQL", as in "an SQL statement". Also "an SQLite database". h3. Example Code diff --git a/railties/guides/source/initialization.textile b/railties/guides/source/initialization.textile index b998066f4c..599ddccdd6 100644 --- a/railties/guides/source/initialization.textile +++ b/railties/guides/source/initialization.textile @@ -848,7 +848,7 @@ h5. +require "active_record"+ TODO: Why are +activesupport_path+ and +activemodel_path+ defined here? -The first three requires require ActiveSupport, Active Model and ARel in that order: +The first three requires require ActiveSupport, Active Model and Arel in that order: <ruby> require 'active_support' diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index b923fedab7..0e85e6d1d5 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -147,9 +147,11 @@ module Rails def default_middleware_stack ActionDispatch::MiddlewareStack.new.tap do |middleware| - require "action_dispatch/http/rack_cache" if config.action_dispatch.rack_cache + rack_cache = config.action_controller.perform_caching && config.action_dispatch.rack_cache - middleware.use ::Rack::Cache, config.action_dispatch.rack_cache if config.action_dispatch.rack_cache + require "action_dispatch/http/rack_cache" if rack_cache + + middleware.use ::Rack::Cache, rack_cache if rack_cache middleware.use ::ActionDispatch::Static, config.static_asset_paths if config.serve_static_assets middleware.use ::Rack::Lock if !config.allow_concurrency middleware.use ::Rack::Runtime diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index f9f06299a0..a0ecbc0fc8 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -59,7 +59,6 @@ module Rails def paths @paths ||= begin paths = super - paths.app.controllers << builtin_controller if builtin_controller paths.config.database "config/database.yml" paths.config.environment "config/environment.rb" paths.lib.templates "lib/templates" @@ -101,10 +100,6 @@ module Rails end end - def builtin_controller - File.expand_path('../info_routes', __FILE__) if Rails.env.development? - end - def log_level @log_level ||= Rails.env.production? ? :info : :debug end diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 8fd2aa0bce..b95df467c7 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -27,7 +27,9 @@ module Rails initializer :add_builtin_route do |app| if Rails.env.development? - app.routes_reloader.paths << File.expand_path('../../info_routes.rb', __FILE__) + app.routes.append do + match '/rails/info/properties' => "rails/info#properties" + end end end diff --git a/railties/lib/rails/code_statistics.rb b/railties/lib/rails/code_statistics.rb index 57e29a0045..78a4f00ad8 100644 --- a/railties/lib/rails/code_statistics.rb +++ b/railties/lib/rails/code_statistics.rb @@ -23,7 +23,7 @@ class CodeStatistics #:nodoc: private def calculate_statistics - @pairs.inject({}) { |stats, pair| stats[pair.first] = calculate_directory_statistics(pair.last); stats } + Hash[@pais.map { |pair| [pair.first, calculate_directory_statistics(pair.last)] }] end def calculate_directory_statistics(directory, pattern = /.*\.rb$/) diff --git a/railties/lib/rails/commands/runner.rb b/railties/lib/rails/commands/runner.rb index 54a9e6ec59..1a91d477ec 100644 --- a/railties/lib/rails/commands/runner.rb +++ b/railties/lib/rails/commands/runner.rb @@ -17,13 +17,13 @@ ARGV.clone.options do |opts| opts.separator "" opts.on("-h", "--help", - "Show this help message.") { $stderr.puts opts; exit } + "Show this help message.") { $stdout.puts opts; exit } if RbConfig::CONFIG['host_os'] !~ /mswin|mingw/ opts.separator "" opts.separator "You can also use runner as a shebang line for your scripts like this:" opts.separator "-------------------------------------------------------------" - opts.separator "#!/usr/bin/env #{File.expand_path($0)}" + opts.separator "#!/usr/bin/env #{File.expand_path($0)} runner" opts.separator "" opts.separator "Product.find(:all).each { |p| p.price *= 2 ; p.save! }" opts.separator "-------------------------------------------------------------" diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 10df9b3a6c..0620b8608e 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -211,12 +211,30 @@ module Rails # end # end # - # If engine is marked as namespaced, FooController has access only to helpers from engine and + # If engine is marked as isolated, FooController has access only to helpers from engine and # url_helpers from MyEngine::Engine.routes. # + # The next thing that changes in isolated engine is routes behaviour. Normally, when you namespace + # your controllers, you need to use scope or namespace method in routes. With isolated engine, + # the namespace is applied by default, so you can ignore it in routes. Further more, you don't need + # to use longer url helpers like "my_engine_articles_path". As the prefix is not set you can just use + # articles_path as you would normally do. + # + # To make that behaviour consistent with other parts of framework, isolated engine has influence also on + # ActiveModel::Naming. When you use namespaced model, like MyEngine::Article, it will normally + # use the prefix "my_engine". In isolated engine, the prefix will be ommited in most of the places, + # like url helpers or form fields. + # + # polymorphic_url(MyEngine::Article.new) #=> "articles_path" + # + # form_for(MyEngine::Article.new) do + # text_field :title #=> <input type="text" name="article[title]" id="article_title" /> + # end + # + # # Additionaly namespaced engine will set its name according to namespace, so in that case: - # MyEngine::Engine.engine_name #=> "my_engine" - # and it will set MyEngine.table_name_prefix to "my_engine_" + # MyEngine::Engine.engine_name #=> "my_engine" and it will set MyEngine.table_name_prefix + # to "my_engine_". # # == Using Engine's routes outside Engine # @@ -257,6 +275,21 @@ module Rails # # This code will use my_engine.user_path(@user) to generate proper route. # + # == Migrations & seed data + # + # Engines can have their own migrations. Default path for migrations is exactly the same + # as in application: db/migrate + # + # To use engine's migrations in application you can use rake task, which copies them to + # application's dir: + # + # rake railties:copy_migrations + # + # If your engine has migrations, you may also want to prepare data for the database in + # seeds.rb file. You can load that data using load_seed method, e.g. + # + # MyEngine::Engine.load_seed + # class Engine < Railtie autoload :Configurable, "rails/engine/configurable" autoload :Configuration, "rails/engine/configuration" @@ -380,6 +413,15 @@ module Rails @config ||= Engine::Configuration.new(find_root_with_flag("lib")) end + # Load data from db/seeds.rb file. It can be used in to load engines' + # seeds, e.g.: + # + # Blog::Engine.load_seed + def load_seed + seed_file = config.paths.db.seeds.to_a.first + load(seed_file) if File.exist?(seed_file) + end + # Add configured load paths to ruby load paths and remove duplicates. initializer :set_load_path, :before => :bootstrap_hook do _all_load_paths.reverse_each do |path| diff --git a/railties/lib/rails/engine/configuration.rb b/railties/lib/rails/engine/configuration.rb index 3ac8911ba8..d4d87be527 100644 --- a/railties/lib/rails/engine/configuration.rb +++ b/railties/lib/rails/engine/configuration.rb @@ -37,6 +37,7 @@ module Rails paths.vendor.plugins "vendor/plugins" paths.db "db" paths.db.migrate "db/migrate" + paths.db.seeds "db/seeds.rb" paths end end diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 76ef598d68..240810b8bd 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -140,10 +140,7 @@ module Rails lookup(lookups) - namespaces = subclasses.inject({}) do |hash, klass| - hash[klass.namespace] = klass - hash - end + namespaces = Hash[subclasses.map { |klass| [klass.namespace, klass] }] lookups.each do |namespace| klass = namespaces[namespace] diff --git a/railties/lib/rails/generators/migration.rb b/railties/lib/rails/generators/migration.rb index 9244307261..8d98909055 100644 --- a/railties/lib/rails/generators/migration.rb +++ b/railties/lib/rails/generators/migration.rb @@ -53,7 +53,11 @@ module Rails destination = self.class.migration_exists?(migration_dir, @migration_file_name) if behavior == :invoke - raise Error, "Another migration is already named #{@migration_file_name}: #{destination}" if destination + if destination && options.force? + remove_file(destination) + elsif destination + raise Error, "Another migration is already named #{@migration_file_name}: #{destination}" + end destination = File.join(migration_dir, "#{@migration_number}_#{@migration_file_name}.rb") end diff --git a/railties/lib/rails/generators/test_case.rb b/railties/lib/rails/generators/test_case.rb index 3376b422cb..cab8708be3 100644 --- a/railties/lib/rails/generators/test_case.rb +++ b/railties/lib/rails/generators/test_case.rb @@ -1,6 +1,7 @@ require 'active_support/core_ext/class/attribute' require 'active_support/core_ext/module/delegation' require 'active_support/core_ext/hash/reverse_merge' +require 'active_support/core_ext/kernel/reporting' require 'rails/generators' require 'fileutils' @@ -65,25 +66,6 @@ module Rails self.destination_root = path end - # Captures the given stream and returns it: - # - # stream = capture(:stdout){ puts "Cool" } - # stream # => "Cool\n" - # - def capture(stream) - begin - stream = stream.to_s - eval "$#{stream} = StringIO.new" - yield - result = eval("$#{stream}").string - ensure - eval("$#{stream} = #{stream.upcase}") - end - - result - end - alias :silence :capture - # Asserts a given file exists. You need to supply an absolute path or a path relative # to the configured destination: # diff --git a/railties/lib/rails/info_routes.rb b/railties/lib/rails/info_routes.rb deleted file mode 100644 index b5c4e4c1e0..0000000000 --- a/railties/lib/rails/info_routes.rb +++ /dev/null @@ -1,3 +0,0 @@ -Rails.application.routes.draw do - match '/rails/info/properties' => "rails/info#properties" -end diff --git a/railties/lib/rails/ruby_version_check.rb b/railties/lib/rails/ruby_version_check.rb index e8d1d1e039..4d57c5973c 100644 --- a/railties/lib/rails/ruby_version_check.rb +++ b/railties/lib/rails/ruby_version_check.rb @@ -14,8 +14,7 @@ elsif RUBY_VERSION > '1.9' and RUBY_VERSION < '1.9.2' $stderr.puts <<-end_message Rails 3 doesn't officially support Ruby 1.9.1 since recent stable - releases have segfaulted the test suite. Please upgrade to Ruby 1.9.2 - before Rails 3 is released! + releases have segfaulted the test suite. Please upgrade to Ruby 1.9.2. You're running #{RUBY_DESCRIPTION} diff --git a/railties/test/application/middleware/cache_test.rb b/railties/test/application/middleware/cache_test.rb index 655f9bcd55..5675cebfd9 100644 --- a/railties/test/application/middleware/cache_test.rb +++ b/railties/test/application/middleware/cache_test.rb @@ -24,7 +24,7 @@ module ApplicationTests end def simple_controller - controller :foo, <<-RUBY + controller :expires, <<-RUBY class ExpiresController < ApplicationController def expires_header expires_in 10, :public => !params[:private] @@ -55,6 +55,20 @@ module ApplicationTests RUBY end + def test_cache_is_disabled_in_dev_mode + simple_controller + app("development") + + get "/expires/expires_header" + assert_nil last_response.headers['X-Rack-Cache'] + + body = last_response.body + + get "/expires/expires_header" + assert_nil last_response.headers['X-Rack-Cache'] + assert_not_equal body, last_response.body + end + def test_cache_works_with_expires simple_controller diff --git a/railties/test/application/middleware_test.rb b/railties/test/application/middleware_test.rb index 0ce6d482a0..f9b594eb33 100644 --- a/railties/test/application/middleware_test.rb +++ b/railties/test/application/middleware_test.rb @@ -19,6 +19,33 @@ module ApplicationTests boot! assert_equal [ + "ActionDispatch::Static", + "Rack::Lock", + "ActiveSupport::Cache::Strategy::LocalCache", + "Rack::Runtime", + "Rails::Rack::Logger", + "ActionDispatch::ShowExceptions", + "ActionDispatch::RemoteIp", + "Rack::Sendfile", + "ActionDispatch::Callbacks", + "ActiveRecord::ConnectionAdapters::ConnectionManagement", + "ActiveRecord::QueryCache", + "ActionDispatch::Cookies", + "ActionDispatch::Session::CookieStore", + "ActionDispatch::Flash", + "ActionDispatch::ParamsParser", + "Rack::MethodOverride", + "ActionDispatch::Head", + "ActionDispatch::BestStandardsSupport" + ], middleware + end + + test "Rack::Cache is present when action_controller.perform_caching is set" do + add_to_config "config.action_controller.perform_caching = true" + + boot! + + assert_equal [ "Rack::Cache", "ActionDispatch::Static", "Rack::Lock", @@ -82,24 +109,24 @@ module ApplicationTests test "insert middleware after" do add_to_config "config.middleware.insert_after ActionDispatch::Static, Rack::Config" boot! - assert_equal "Rack::Config", middleware.third + assert_equal "Rack::Config", middleware.second end test "RAILS_CACHE does not respond to middleware" do add_to_config "config.cache_store = :memory_store" boot! - assert_equal "Rack::Runtime", middleware.fourth + assert_equal "Rack::Runtime", middleware.third end test "RAILS_CACHE does respond to middleware" do boot! - assert_equal "Rack::Runtime", middleware.fifth + assert_equal "Rack::Runtime", middleware.fourth end test "insert middleware before" do add_to_config "config.middleware.insert_before ActionDispatch::Static, Rack::Config" boot! - assert_equal "Rack::Config", middleware.second + assert_equal "Rack::Config", middleware.first end # x_sendfile_header middleware diff --git a/railties/test/application/routing_test.rb b/railties/test/application/routing_test.rb index bf2af11e9f..416a5de5b0 100644 --- a/railties/test/application/routing_test.rb +++ b/railties/test/application/routing_test.rb @@ -177,6 +177,34 @@ module ApplicationTests assert_equal 'admin::foo', last_response.body end + def test_reloads_appended_route_blocks + app_file 'config/routes.rb', <<-RUBY + AppTemplate::Application.routes.draw do + match ':controller#:action' + end + RUBY + + add_to_config <<-R + routes.append do + match '/win' => lambda { |e| [200, {'Content-Type'=>'text/plain'}, ['WIN']] } + end + R + + app 'development' + + get '/win' + assert_equal 'WIN', last_response.body + + app_file 'config/routes.rb', <<-R + AppTemplate::Application.routes.draw do + match 'lol' => 'hello#index' + end + R + + get '/win' + assert_equal 'WIN', last_response.body + end + {"development" => "baz", "production" => "bar"}.each do |mode, expected| test "reloads routes when configuration is changed in #{mode}" do controller :foo, <<-RUBY diff --git a/railties/test/application/runner_test.rb b/railties/test/application/runner_test.rb index 07a3d94120..292d1e247f 100644 --- a/railties/test/application/runner_test.rb +++ b/railties/test/application/runner_test.rb @@ -18,6 +18,10 @@ module ApplicationTests MODEL end + def test_should_include_runner_in_shebang_line_in_help + assert_match "/rails runner", Dir.chdir(app_path) { `bundle exec rails runner --help` } + end + def test_should_run_ruby_statement assert_match "42", Dir.chdir(app_path) { `bundle exec rails runner "puts User.count"` } end diff --git a/railties/test/generators/model_generator_test.rb b/railties/test/generators/model_generator_test.rb index f4a9a152c9..52e08cf2dd 100644 --- a/railties/test/generators/model_generator_test.rb +++ b/railties/test/generators/model_generator_test.rb @@ -165,6 +165,15 @@ class ModelGeneratorTest < Rails::Generators::TestCase assert_no_migration "db/migrate/create_accounts.rb" end + def test_existing_migration_is_removed_on_force + run_generator + old_migration = Dir["#{destination_root}/db/migrate/*_create_accounts.rb"].first + error = capture(:stderr) { run_generator ["Account", "--force"] } + assert_no_match /Another migration is already named create_foos/, error + assert_no_file old_migration + assert_migration 'db/migrate/create_accounts.rb' + end + def test_invokes_default_test_framework run_generator assert_file "test/unit/account_test.rb", /class AccountTest < ActiveSupport::TestCase/ diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 390c0ab543..79c7735019 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -19,6 +19,7 @@ RAILS_FRAMEWORK_ROOT = File.expand_path("#{File.dirname(__FILE__)}/../../..") # to run the tests require "#{RAILS_FRAMEWORK_ROOT}/activesupport/lib/active_support/testing/isolation" require "#{RAILS_FRAMEWORK_ROOT}/activesupport/lib/active_support/testing/declarative" +require "#{RAILS_FRAMEWORK_ROOT}/activesupport/lib/active_support/core_ext/kernel/reporting" module TestHelpers module Paths diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 6b90b32005..a9dd7d4c1b 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -4,19 +4,6 @@ require 'stringio' module RailtiesTest class EngineTest < Test::Unit::TestCase - # TODO: it's copied from generators/test_case, maybe make a module with such helpers? - def capture(stream) - begin - stream = stream.to_s - eval "$#{stream} = StringIO.new" - yield - result = eval("$#{stream}").string - ensure - eval("$#{stream} = #{stream.upcase}") - end - - result - end include ActiveSupport::Testing::Isolation include SharedTests @@ -578,7 +565,6 @@ module RailtiesTest env = Rack::MockRequest.env_for("/bukkits/posts/new") response = AppTemplate::Application.call(env) - p rack_body(response[2]) assert rack_body(response[2]) =~ /name="post\[title\]"/ end @@ -638,5 +624,24 @@ module RailtiesTest assert !File.exist?(File.join(app_path, 'public/bukkits')) end end + + test "loading seed data" do + @plugin.write "db/seeds.rb", <<-RUBY + Bukkits::Engine.config.bukkits_seeds_loaded = true + RUBY + + app_file "db/seeds.rb", <<-RUBY + Rails.application.config.app_seeds_loaded = true + RUBY + + boot_rails + + Rails.application.load_seed + assert Rails.application.config.app_seeds_loaded + assert_raise(NoMethodError) do Bukkits::Engine.config.bukkits_seeds_loaded end + + Bukkits::Engine.load_seed + assert Bukkits::Engine.config.bukkits_seeds_loaded + end end end |