diff options
130 files changed, 2733 insertions, 3341 deletions
diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index 2b533ad054..c9f10b359b 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -508,10 +508,7 @@ module ActionMailer def process(*args) #:nodoc: lookup_context.skip_default_locale! - generated_mail = super - unless generated_mail - @_message = NullMail.new - end + @_message = NullMail.new unless super end class NullMail #:nodoc: diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index 85a83ed7d9..e5bd21ea7b 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,29 @@ ## Rails 4.0.0 (unreleased) ## +* Sweepers was extracted from Action Controller as `rails-observers` gem. + + *Rafael Mendonça França* + +* Add option flag to `CacheHelper#cache` to manually bypass automatic template digests: + + <% cache project, skip_digest: true do %> + ... + <% end %> + + *Drew Ulmer* + +* No sort Hash options in #grouped_options_for_select. *Sergey Kojin* + +* Accept symbols as #send_data :disposition value *Elia Schito* + +* Add i18n scope to distance_of_time_in_words. *Steve Klabnik* + +* `assert_template`: + - is no more passing with empty string. + - is now validating option keys. It accepts: `:layout`, `:partial`, `:locals` and `:count`. + + *Roberto Soares* + * Allow setting a symbol as path in scope on routes. This is now allowed: scope :api do diff --git a/actionpack/lib/action_controller/caching.rb b/actionpack/lib/action_controller/caching.rb index 462f147371..177da1c8a0 100644 --- a/actionpack/lib/action_controller/caching.rb +++ b/actionpack/lib/action_controller/caching.rb @@ -6,10 +6,10 @@ module ActionController # \Caching is a cheap way of speeding up slow applications by keeping the result of # calculations, renderings, and database calls around for subsequent requests. # - # You can read more about each approach and the sweeping assistance by clicking the + # You can read more about each approach and the by clicking the # modules below. # - # Note: To turn off all caching and sweeping, set + # Note: To turn off all caching, set # config.action_controller.perform_caching = false. # # == \Caching stores @@ -30,8 +30,6 @@ module ActionController eager_autoload do autoload :Fragments - autoload :Sweeper, 'action_controller/caching/sweeping' - autoload :Sweeping, 'action_controller/caching/sweeping' end module ConfigMethods @@ -54,7 +52,6 @@ module ActionController include ConfigMethods include Fragments - include Sweeping if defined?(ActiveRecord) included do extend ConfigMethods diff --git a/actionpack/lib/action_controller/caching/sweeping.rb b/actionpack/lib/action_controller/caching/sweeping.rb deleted file mode 100644 index 317ac74b40..0000000000 --- a/actionpack/lib/action_controller/caching/sweeping.rb +++ /dev/null @@ -1,116 +0,0 @@ -module ActionController - module Caching - # Sweepers are the terminators of the caching world and responsible for expiring - # caches when Active Record objects change. They do this by being half-observers, - # half-filters and implementing callbacks for both roles. - # - # class ListSweeper < ActionController::Caching::Sweeper - # observe List, Item - # - # def after_save(record) - # list = record.is_a?(List) ? record : record.list - # expire_page(controller: 'lists', action: %w( show public feed ), id: list.id) - # expire_action(controller: 'lists', action: 'all') - # list.shares.each { |share| expire_page(controller: 'lists', action: 'show', id: share.url_key) } - # end - # end - # - # The sweeper is assigned in the controllers that wish to have its job performed using - # the +cache_sweeper+ class method: - # - # class ListsController < ApplicationController - # caches_action :index, :show, :public, :feed - # cache_sweeper :list_sweeper, only: [ :edit, :destroy, :share ] - # end - # - # In the example above, four actions are cached and three actions are responsible for expiring those caches. - # - # You can also name an explicit class in the declaration of a sweeper, which is needed - # if the sweeper is in a module: - # - # class ListsController < ApplicationController - # caches_action :index, :show, :public, :feed - # cache_sweeper OpenBar::Sweeper, only: [ :edit, :destroy, :share ] - # end - module Sweeping - extend ActiveSupport::Concern - - module ClassMethods # :nodoc: - def cache_sweeper(*sweepers) - configuration = sweepers.extract_options! - - sweepers.each do |sweeper| - ActiveRecord::Base.observers << sweeper if defined?(ActiveRecord) and defined?(ActiveRecord::Base) - sweeper_instance = (sweeper.is_a?(Symbol) ? Object.const_get(sweeper.to_s.classify) : sweeper).instance - - if sweeper_instance.is_a?(Sweeper) - around_filter(sweeper_instance, :only => configuration[:only]) - else - after_filter(sweeper_instance, :only => configuration[:only]) - end - end - end - end - end - - if defined?(ActiveRecord) and defined?(ActiveRecord::Observer) - class Sweeper < ActiveRecord::Observer # :nodoc: - attr_accessor :controller - - def initialize(*args) - super - @controller = nil - end - - def before(controller) - self.controller = controller - callback(:before) if controller.perform_caching - true # before method from sweeper should always return true - end - - def after(controller) - self.controller = controller - callback(:after) if controller.perform_caching - end - - def around(controller) - before(controller) - yield - after(controller) - ensure - clean_up - end - - protected - # gets the action cache path for the given options. - def action_path_for(options) - Actions::ActionCachePath.new(controller, options).path - end - - # Retrieve instance variables set in the controller. - def assigns(key) - controller.instance_variable_get("@#{key}") - end - - private - def clean_up - # Clean up, so that the controller can be collected after this request - self.controller = nil - end - - def callback(timing) - controller_callback_method_name = "#{timing}_#{controller.controller_name.underscore}" - action_callback_method_name = "#{controller_callback_method_name}_#{controller.action_name}" - - __send__(controller_callback_method_name) if respond_to?(controller_callback_method_name, true) - __send__(action_callback_method_name) if respond_to?(action_callback_method_name, true) - end - - def method_missing(method, *arguments, &block) - return super unless @controller - @controller.__send__(method, *arguments, &block) - end - end - end - end -end diff --git a/actionpack/lib/action_controller/metal/data_streaming.rb b/actionpack/lib/action_controller/metal/data_streaming.rb index 334943818c..75c4d3ef99 100644 --- a/actionpack/lib/action_controller/metal/data_streaming.rb +++ b/actionpack/lib/action_controller/metal/data_streaming.rb @@ -150,6 +150,7 @@ module ActionController #:nodoc: disposition = options.fetch(:disposition, DEFAULT_SEND_FILE_DISPOSITION) unless disposition.nil? + disposition = disposition.to_s disposition += %(; filename="#{options[:filename]}") if options[:filename] headers['Content-Disposition'] = disposition end diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index a475d4bdff..c9f1d8dcb4 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -2,7 +2,7 @@ require 'active_support/core_ext/hash/slice' require 'active_support/core_ext/hash/except' require 'active_support/core_ext/module/anonymous' require 'active_support/core_ext/struct' -require 'action_dispatch/http/mime_types' +require 'action_dispatch/http/mime_type' module ActionController # Wraps the parameters hash into a nested hash. This will allow clients to submit diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index da640502a2..386075bd30 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -70,8 +70,7 @@ module ActionController # Also, sets the +permitted+ attribute to the default value of # <tt>ActionController::Parameters.permit_all_parameters</tt>. # - # class Person - # include ActiveRecord::Base + # class Person < ActiveRecord::Base # end # # params = ActionController::Parameters.new(name: 'Francesco') @@ -125,10 +124,10 @@ module ActionController # <tt>ActionController::ParameterMissing</tt> error. # # ActionController::Parameters.new(person: { name: 'Francesco' }).require(:person) - # # => {"name"=>"Francesco"} + # # => {"name"=>"Francesco"} # # ActionController::Parameters.new(person: nil).require(:person) - # # => ActionController::ParameterMissing: param not found: person + # # => ActionController::ParameterMissing: param not found: person # # ActionController::Parameters.new(person: {}).require(:person) # # => ActionController::ParameterMissing: param not found: person @@ -188,7 +187,7 @@ module ActionController # # => {} # # params.require(:person).permit(contact: :phone) - # # => {"contact"=>{"phone"=>"555-1234"}} + # # => {"contact"=>{"phone"=>"555-1234"}} # # params.require(:person).permit(contact: [ :email, :phone ]) # # => {"contact"=>{"email"=>"none@test.com", "phone"=>"555-1234"}} @@ -204,6 +203,8 @@ module ActionController end keys.grep(/\A#{Regexp.escape(filter)}\(\d+[if]?\)\z/) { |key| params[key] = self[key] } when Hash then + filter = filter.with_indifferent_access + self.slice(*filter.keys).each do |key, values| return unless values diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index be8055955d..7b48870090 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -94,7 +94,7 @@ module ActionController matches_template = case options when String - rendered.any? do |t, num| + !options.empty? && rendered.any? do |t, num| options_splited = options.split(File::SEPARATOR) t_splited = t.split(File::SEPARATOR) t_splited.last(options_splited.size) == options_splited @@ -106,6 +106,8 @@ module ActionController end assert matches_template, msg when Hash + options.assert_valid_keys(:layout, :partial, :locals, :count) + if options.key?(:layout) expected_layout = options[:layout] msg = message || sprintf("expecting layout <%s> but action rendered <%s>", diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index 05cbcf709e..0c19b493ab 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -152,7 +152,7 @@ module ActionDispatch end def conditions - { :path_info => @path }.merge(constraints).merge(request_method_condition) + { :path_info => @path }.merge!(constraints).merge!(request_method_condition) end def requirements @@ -252,7 +252,7 @@ module ActionDispatch def defaults_from_constraints(constraints) url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.slice(*url_keys).select{ |k, v| v.is_a?(String) || v.is_a?(Fixnum) } + constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } end end @@ -285,7 +285,7 @@ module ActionDispatch # of most Rails applications, this is beneficial. def root(options = {}) options = { :to => options } if options.is_a?(String) - match '/', { :as => :root, :via => :get }.merge(options) + match '/', { :as => :root, :via => :get }.merge!(options) end # Matches a url pattern to one or more routes. Any symbols in a pattern @@ -641,19 +641,16 @@ module ActionDispatch # resources :posts # end def scope(*args) - options = args.extract_options! - options = options.dup - - options[:path] = args.flatten.join('/') if args.any? + options = args.extract_options!.dup recover = {} + options[:path] = args.flatten.join('/') if args.any? options[:constraints] ||= {} - unless options[:constraints].is_a?(Hash) - block, options[:constraints] = options[:constraints], {} - end if options[:constraints].is_a?(Hash) (options[:defaults] ||= {}).reverse_merge!(defaults_from_constraints(options[:constraints])) + else + block, options[:constraints] = options[:constraints], {} end scope_options.each do |option| @@ -663,8 +660,8 @@ module ActionDispatch end end - recover[:block] = @scope[:blocks] - @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) + recover[:blocks] = @scope[:blocks] + @scope[:blocks] = merge_blocks_scope(@scope[:blocks], block) recover[:options] = @scope[:options] @scope[:options] = merge_options_scope(@scope[:options], options) @@ -672,12 +669,7 @@ module ActionDispatch yield self ensure - scope_options.each do |option| - @scope[option] = recover[option] if recover.has_key?(option) - end - - @scope[:options] = recover[:options] - @scope[:blocks] = recover[:block] + @scope.merge!(recover) end # Scopes routes to a specific controller @@ -853,7 +845,7 @@ module ActionDispatch end def merge_options_scope(parent, child) #:nodoc: - (parent || {}).except(*override_keys(child)).merge(child) + (parent || {}).except(*override_keys(child)).merge!(child) end def merge_shallow_scope(parent, child) #:nodoc: @@ -866,7 +858,7 @@ module ActionDispatch def defaults_from_constraints(constraints) url_keys = [:protocol, :subdomain, :domain, :host, :port] - constraints.slice(*url_keys).select{ |k, v| v.is_a?(String) || v.is_a?(Fixnum) } + constraints.select { |k, v| url_keys.include?(k) && (v.is_a?(String) || v.is_a?(Fixnum)) } end end diff --git a/actionpack/lib/action_dispatch/routing/redirection.rb b/actionpack/lib/action_dispatch/routing/redirection.rb index d70063d0e9..d751e04e6a 100644 --- a/actionpack/lib/action_dispatch/routing/redirection.rb +++ b/actionpack/lib/action_dispatch/routing/redirection.rb @@ -75,7 +75,7 @@ module ActionDispatch :port => request.optional_port, :path => request.path, :params => request.query_parameters - }.merge options + }.merge! options if !params.empty? && url_options[:path].match(/%\{\w*\}/) url_options[:path] = (url_options[:path] % escape_path(params)) diff --git a/actionpack/lib/action_dispatch/testing/test_process.rb b/actionpack/lib/action_dispatch/testing/test_process.rb index 9ad5a1bc1d..e657283cec 100644 --- a/actionpack/lib/action_dispatch/testing/test_process.rb +++ b/actionpack/lib/action_dispatch/testing/test_process.rb @@ -26,17 +26,19 @@ module ActionDispatch @response.redirect_url end - # Shortcut for <tt>Rack::Test::UploadedFile.new(ActionController::TestCase.fixture_path + path, type)</tt>: + # Shortcut for <tt>Rack::Test::UploadedFile.new(File.join(ActionController::TestCase.fixture_path, path), type)</tt>: # - # post :change_avatar, avatar: fixture_file_upload('/files/spongebob.png', 'image/png') + # post :change_avatar, avatar: fixture_file_upload('files/spongebob.png', 'image/png') # # To upload binary files on Windows, pass <tt>:binary</tt> as the last parameter. # This will not affect other platforms: # - # post :change_avatar, avatar: fixture_file_upload('/files/spongebob.png', 'image/png', :binary) + # post :change_avatar, avatar: fixture_file_upload('files/spongebob.png', 'image/png', :binary) def fixture_file_upload(path, mime_type = nil, binary = false) - fixture_path = self.class.fixture_path if self.class.respond_to?(:fixture_path) - Rack::Test::UploadedFile.new("#{fixture_path}#{path}", mime_type, binary) + if self.class.respond_to?(:fixture_path) && self.class.fixture_path + path = File.join(self.class.fixture_path, path) + end + Rack::Test::UploadedFile.new(path, mime_type, binary) end end end diff --git a/actionpack/lib/action_view/helpers/cache_helper.rb b/actionpack/lib/action_view/helpers/cache_helper.rb index ddac87a37d..db920ae7a4 100644 --- a/actionpack/lib/action_view/helpers/cache_helper.rb +++ b/actionpack/lib/action_view/helpers/cache_helper.rb @@ -52,6 +52,13 @@ module ActionView # Additionally, the digestor will automatically look through your template file for # explicit and implicit dependencies, and include those as part of the digest. # + # The digestor can be bypassed by passing skip_digest: true as an option to the cache call: + # + # <% cache project, skip_digest: true do %> + # <b>All the topics on this project</b> + # <%= render project.topics %> + # <% end %> + # # ==== Implicit dependencies # # Most template dependencies can be derived from calls to render in the template itself. @@ -105,7 +112,7 @@ module ActionView # Now all you'll have to do is change that timestamp when the helper method changes. def cache(name = {}, options = nil, &block) if controller.perform_caching - safe_concat(fragment_for(fragment_name_with_digest(name), options, &block)) + safe_concat(fragment_for(cache_fragment_name(name, options), options, &block)) else yield end @@ -113,6 +120,22 @@ module ActionView nil end + # This helper returns the name of a cache key for a given fragment cache + # call. By supplying skip_digest: true to cache, the digestion of cache + # fragments can be manually bypassed. This is useful when cache fragments + # cannot be manually expired unless you know the exact key which is the + # case when using memcached. + def cache_fragment_name(name = {}, options = nil) + skip_digest = options && options[:skip_digest] + + if skip_digest + name + else + fragment_name_with_digest(name) + end + end + + private def fragment_name_with_digest(name) #:nodoc: if @virtual_path [ @@ -124,7 +147,6 @@ module ActionView end end - private # TODO: Create an object that has caching read/write on it def fragment_for(name = {}, options = nil, &block) #:nodoc: if fragment = controller.read_fragment(name, options) diff --git a/actionpack/lib/action_view/helpers/form_helper.rb b/actionpack/lib/action_view/helpers/form_helper.rb index 6abf1e1751..c79d30ea88 100644 --- a/actionpack/lib/action_view/helpers/form_helper.rb +++ b/actionpack/lib/action_view/helpers/form_helper.rb @@ -702,6 +702,11 @@ module ActionView # <% end %> # ... # <% end %> + # + # Note that fields_for will automatically generate a hidden field + # to store the ID of the record. There are circumstances where this + # hidden field is not needed and you can pass <tt>hidden_field_id: false</tt> + # to prevent fields_for from rendering it automatically. def fields_for(record_name, record_object = nil, options = {}, &block) builder = instantiate_builder(record_name, record_object, options) output = capture(builder, &block) diff --git a/actionpack/lib/action_view/helpers/form_options_helper.rb b/actionpack/lib/action_view/helpers/form_options_helper.rb index 46ebe60ec2..9310a90f0f 100644 --- a/actionpack/lib/action_view/helpers/form_options_helper.rb +++ b/actionpack/lib/action_view/helpers/form_options_helper.rb @@ -482,15 +482,15 @@ module ActionView # grouped_options_for_select(grouped_options) # # Possible output: + # <optgroup label="North America"> + # <option value="US">United States</option> + # <option value="Canada">Canada</option> + # </optgroup> # <optgroup label="Europe"> # <option value="Denmark">Denmark</option> # <option value="Germany">Germany</option> # <option value="France">France</option> # </optgroup> - # <optgroup label="North America"> - # <option value="US">United States</option> - # <option value="Canada">Canada</option> - # </optgroup> # # Sample usage (divider): # grouped_options = [ @@ -530,8 +530,6 @@ module ActionView body.safe_concat content_tag(:option, prompt_text(prompt), :value => "") end - grouped_options = grouped_options.sort if grouped_options.is_a?(Hash) - grouped_options.each do |container| if divider label = divider diff --git a/actionpack/lib/action_view/helpers/url_helper.rb b/actionpack/lib/action_view/helpers/url_helper.rb index fd671c9c07..fa516cf91b 100644 --- a/actionpack/lib/action_view/helpers/url_helper.rb +++ b/actionpack/lib/action_view/helpers/url_helper.rb @@ -498,7 +498,7 @@ module ActionView # True if the current request URI was generated by the given +options+. # # ==== Examples - # Let's say we're in the <tt>/shop/checkout?order=desc</tt> action. + # Let's say we're in the <tt>http://www.example.com/shop/checkout?order=desc</tt> action. # # current_page?(action: 'process') # # => false @@ -515,7 +515,13 @@ module ActionView # current_page?(controller: 'library', action: 'checkout') # # => false # - # Let's say we're in the <tt>/shop/checkout?order=desc&page=1</tt> action. + # current_page?('http://www.example.com/shop/checkout') + # # => true + # + # current_page?('/shop/checkout') + # # => true + # + # Let's say we're in the <tt>http://www.example.com/shop/checkout?order=desc&page=1</tt> action. # # current_page?(action: 'process') # # => false @@ -538,7 +544,7 @@ module ActionView # current_page?(controller: 'library', action: 'checkout') # # => false # - # Let's say we're in the <tt>/products</tt> action with method POST in case of invalid product. + # Let's say we're in the <tt>http://www.example.com/products</tt> action with method POST in case of invalid product. # # current_page?(controller: 'product', action: 'index') # # => false diff --git a/actionpack/lib/action_view/vendor/html-scanner.rb b/actionpack/lib/action_view/vendor/html-scanner.rb index 879b31e60e..775b827529 100644 --- a/actionpack/lib/action_view/vendor/html-scanner.rb +++ b/actionpack/lib/action_view/vendor/html-scanner.rb @@ -1,4 +1,4 @@ -$LOAD_PATH << "#{File.dirname(__FILE__)}/html-scanner" +$LOAD_PATH.unshift "#{File.dirname(__FILE__)}/html-scanner" module HTML extend ActiveSupport::Autoload diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 4f5b2895c9..95bff0a204 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -25,7 +25,6 @@ require 'active_support/dependencies' require 'active_model' require 'active_record' require 'action_controller/caching' -require 'action_controller/caching/sweeping' require 'pp' # require 'pp' early to prevent hidden_methods from not picking up the pretty-print methods until too late diff --git a/actionpack/test/controller/action_pack_assertions_test.rb b/actionpack/test/controller/action_pack_assertions_test.rb index ca542eb7e2..b94f45bfe7 100644 --- a/actionpack/test/controller/action_pack_assertions_test.rb +++ b/actionpack/test/controller/action_pack_assertions_test.rb @@ -430,6 +430,12 @@ end class AssertTemplateTest < ActionController::TestCase tests ActionPackAssertionsController + def test_with_invalid_hash_keys_raises_argument_error + assert_raise(ArgumentError) do + assert_template foo: "bar" + end + end + def test_with_partial get :partial assert_template :partial => '_partial' @@ -447,6 +453,20 @@ class AssertTemplateTest < ActionController::TestCase end end + def test_with_empty_string_fails_when_template_rendered + get :hello_world + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template "" + end + end + + def test_with_empty_string_fails_when_no_template_rendered + get :nothing + assert_raise(ActiveSupport::TestCase::Assertion) do + assert_template "" + end + end + def test_passes_with_correct_string get :hello_world assert_template 'hello_world' diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index 65c18dfb64..2428cd7433 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -164,6 +164,9 @@ class FunctionalCachingController < CachingController format.xml end end + + def fragment_cached_without_digest + end end class FunctionalFragmentCachingTest < ActionController::TestCase @@ -200,6 +203,15 @@ CACHED @store.read("views/test.host/functional_caching/html_fragment_cached_with_partial/#{template_digest("functional_caching/_partial", "html")}")) end + def test_skipping_fragment_cache_digesting + get :fragment_cached_without_digest, :format => "html" + assert_response :success + expected_body = "<body>\n<p>ERB</p>\n</body>\n" + + assert_equal expected_body, @response.body + assert_equal "<p>ERB</p>", @store.read("views/nodigest") + end + def test_render_inline_before_fragment_caching get :inline_fragment_cached assert_response :success diff --git a/actionpack/test/controller/filters_test.rb b/actionpack/test/controller/filters_test.rb index d203601771..1c59dd5953 100644 --- a/actionpack/test/controller/filters_test.rb +++ b/actionpack/test/controller/filters_test.rb @@ -499,18 +499,6 @@ class FilterTest < ActionController::TestCase end - class ::AppSweeper < ActionController::Caching::Sweeper; end - class SweeperTestController < ActionController::Base - cache_sweeper :app_sweeper - def show - render :text => 'hello world' - end - - def error - raise StandardError.new - end - end - class ImplicitActionsController < ActionController::Base before_filter :find_only, :only => :edit before_filter :find_except, :except => :edit @@ -526,35 +514,6 @@ class FilterTest < ActionController::TestCase end end - def test_sweeper_should_not_ignore_no_method_error - sweeper = ActionController::Caching::Sweeper.send(:new) - assert_raise NoMethodError do - sweeper.send_not_defined - end - end - - def test_sweeper_should_not_block_rendering - response = test_process(SweeperTestController) - assert_equal 'hello world', response.body - end - - def test_sweeper_should_clean_up_if_exception_is_raised - assert_raise StandardError do - test_process(SweeperTestController, 'error') - end - assert_nil AppSweeper.instance.controller - end - - def test_before_method_of_sweeper_should_always_return_true - sweeper = ActionController::Caching::Sweeper.send(:new) - assert sweeper.before(TestController.new) - end - - def test_after_method_of_sweeper_should_always_return_nil - sweeper = ActionController::Caching::Sweeper.send(:new) - assert_nil sweeper.after(TestController.new) - end - def test_non_yielding_around_filters_not_returning_false_do_not_raise controller = NonYieldingAroundFilterController.new controller.instance_variable_set "@filter_return_value", true diff --git a/actionpack/test/controller/parameters/nested_parameters_test.rb b/actionpack/test/controller/parameters/nested_parameters_test.rb index d287e79cba..6df849c4e2 100644 --- a/actionpack/test/controller/parameters/nested_parameters_test.rb +++ b/actionpack/test/controller/parameters/nested_parameters_test.rb @@ -36,6 +36,31 @@ class NestedParametersTest < ActiveSupport::TestCase assert_nil permitted[:magazine] end + test "permitted nested parameters with a string or a symbol as a key" do + params = ActionController::Parameters.new({ + book: { + 'authors' => [ + { name: 'William Shakespeare', born: '1564-04-26' }, + { name: 'Christopher Marlowe' } + ] + } + }) + + permitted = params.permit book: [ { 'authors' => [ :name ] } ] + + assert_equal 'William Shakespeare', permitted[:book]['authors'][0][:name] + assert_equal 'William Shakespeare', permitted[:book][:authors][0][:name] + assert_equal 'Christopher Marlowe', permitted[:book]['authors'][1][:name] + assert_equal 'Christopher Marlowe', permitted[:book][:authors][1][:name] + + permitted = params.permit book: [ { authors: [ :name ] } ] + + assert_equal 'William Shakespeare', permitted[:book]['authors'][0][:name] + assert_equal 'William Shakespeare', permitted[:book][:authors][0][:name] + assert_equal 'Christopher Marlowe', permitted[:book]['authors'][1][:name] + assert_equal 'Christopher Marlowe', permitted[:book][:authors][1][:name] + end + test "nested arrays with strings" do params = ActionController::Parameters.new({ :book => { diff --git a/actionpack/test/controller/send_file_test.rb b/actionpack/test/controller/send_file_test.rb index 8bf3096888..5e9053da5b 100644 --- a/actionpack/test/controller/send_file_test.rb +++ b/actionpack/test/controller/send_file_test.rb @@ -114,6 +114,18 @@ class SendFileTest < ActionController::TestCase assert_equal 'private', h['Cache-Control'] end + def test_send_file_headers_with_disposition_as_a_symbol + options = { + :type => Mime::PNG, + :disposition => :disposition, + :filename => 'filename' + } + + @controller.headers = {} + @controller.send(:send_file_headers!, options) + assert_equal 'disposition; filename="filename"', @controller.headers['Content-Disposition'] + end + def test_send_file_headers_with_mime_lookup_with_symbol options = { :type => :png diff --git a/actionpack/test/controller/sweeper_test.rb b/actionpack/test/controller/sweeper_test.rb deleted file mode 100644 index 0561efc62f..0000000000 --- a/actionpack/test/controller/sweeper_test.rb +++ /dev/null @@ -1,16 +0,0 @@ -require 'abstract_unit' - - -class SweeperTest < ActionController::TestCase - - class ::AppSweeper < ActionController::Caching::Sweeper; end - - def test_sweeper_should_not_ignore_unknown_method_calls - sweeper = ActionController::Caching::Sweeper.send(:new) - assert_raise NameError do - sweeper.instance_eval do - some_method_that_doesnt_exist - end - end - end -end diff --git a/actionpack/test/controller/test_case_test.rb b/actionpack/test/controller/test_case_test.rb index 8990fc34d6..bdca1d4d77 100644 --- a/actionpack/test/controller/test_case_test.rb +++ b/actionpack/test/controller/test_case_test.rb @@ -818,6 +818,18 @@ XML assert_equal '159528', @response.body end + def test_fixture_file_upload_relative_to_fixture_path + TestCaseTest.stubs(:fixture_path).returns(FILES_DIR) + uploaded_file = fixture_file_upload("mona_lisa.jpg", "image/jpg") + assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read + end + + def test_fixture_file_upload_ignores_nil_fixture_path + TestCaseTest.stubs(:fixture_path).returns(nil) + uploaded_file = fixture_file_upload("#{FILES_DIR}/mona_lisa.jpg", "image/jpg") + assert_equal File.open("#{FILES_DIR}/mona_lisa.jpg", READ_PLAIN).read, uploaded_file.read + end + def test_action_dispatch_uploaded_file_upload filename = 'mona_lisa.jpg' path = "#{FILES_DIR}/#{filename}" diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index 0a59d3cf9e..4f5d8fdb7c 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -20,592 +20,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end end - stub_controllers do |routes| - Routes = routes - Routes.draw do - default_url_options :host => "rubyonrails.org" - resources_path_names :correlation_indexes => "info_about_correlation_indexes" - + def test_logout + draw do controller :sessions do - get 'login' => :new - post 'login' => :create delete 'logout' => :destroy end - - resource :session do - get :create - post :reset - - resource :info - - member do - get :crush - end - end - - scope "bookmark", :controller => "bookmarks", :as => :bookmark do - get :new, :path => "build" - post :create, :path => "create", :as => "" - put :update - get :remove, :action => :destroy, :as => :remove - end - - scope "pagemark", :controller => "pagemarks", :as => :pagemark do - get "new", :path => "build" - post "create", :as => "" - put "update" - get "remove", :action => :destroy, :as => :remove - end - - get 'account/logout' => redirect("/logout"), :as => :logout_redirect - get 'account/login', :to => redirect("/login") - get 'secure', :to => redirect("/secure/login") - - get 'mobile', :to => redirect(:subdomain => 'mobile') - get 'documentation', :to => redirect(:domain => 'example-documentation.com', :path => '') - get 'new_documentation', :to => redirect(:path => '/documentation/new') - get 'super_new_documentation', :to => redirect(:host => 'super-docs.com') - - get 'stores/:name', :to => redirect(:subdomain => 'stores', :path => '/%{name}') - get 'stores/:name(*rest)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{rest}') - - get 'youtube_favorites/:youtube_id/:name', :to => redirect(YoutubeFavoritesRedirector) - - constraints(lambda { |req| true }) do - get 'account/overview' - end - - get '/account/nested/overview' - get 'sign_in' => "sessions#new" - - get 'account/modulo/:name', :to => redirect("/%{name}s") - get 'account/proc/:name', :to => redirect {|params, req| "/#{params[:name].pluralize}" } - get 'account/proc_req' => redirect {|params, req| "/#{req.method}" } - - get 'account/google' => redirect('http://www.google.com/', :status => 302) - - match 'openid/login', :via => [:get, :post], :to => "openid#login" - - controller(:global) do - get 'global/hide_notice' - get 'global/export', :to => :export, :as => :export_request - get '/export/:id/:file', :to => :export, :as => :export_download, :constraints => { :file => /.*/ } - get 'global/:action' - end - - get "/local/:action", :controller => "local" - - get "/projects/status(.:format)" - get "/404", :to => lambda { |env| [404, {"Content-Type" => "text/plain"}, ["NOT FOUND"]] } - - constraints(:ip => /192\.168\.1\.\d\d\d/) do - get 'admin' => "queenbee#index" - end - - constraints ::TestRoutingMapper::IpRestrictor do - get 'admin/accounts' => "queenbee#accounts" - end - - get 'admin/passwords' => "queenbee#passwords", :constraints => ::TestRoutingMapper::IpRestrictor - - scope 'pt', :as => 'pt' do - resources :projects, :path_names => { :edit => 'editar', :new => 'novo' }, :path => 'projetos' do - post :preview, :on => :new - put :close, :on => :member, :path => 'fechar' - get :open, :on => :new, :path => 'abrir' - end - resource :admin, :path_names => { :new => 'novo', :activate => 'ativar' }, :path => 'administrador' do - post :preview, :on => :new - put :activate, :on => :member - end - resources :products, :path_names => { :new => 'novo' } do - new do - post :preview - end - end - end - - resources :projects, :controller => :project do - resources :involvements, :attachments - get :correlation_indexes, :on => :collection - - resources :participants do - put :update_all, :on => :collection - end - - resources :companies do - resources :people - resource :avatar, :controller => :avatar - end - - resources :images, :as => :funny_images do - post :revise, :on => :member - end - - resource :manager, :as => :super_manager do - post :fire - end - - resources :people do - nested do - scope "/:access_token" do - resource :avatar - end - end - - member do - get 'some_path_with_name' - put :accessible_projects - post :resend, :generate_new_password - end - end - - resources :posts do - get :archive, :toggle_view, :on => :collection - post :preview, :on => :member - - resource :subscription - - resources :comments do - post :preview, :on => :collection - end - end - - post 'new', :action => 'new', :on => :collection, :as => :new - end - - resources :replies do - collection do - get 'page/:page' => 'replies#index', :page => %r{\d+} - get ':page' => 'replies#index', :page => %r{\d+} - end - - new do - post :preview - end - - member do - put :answer, :to => :mark_as_answer - delete :answer, :to => :unmark_as_answer - end - end - - resources :posts, :only => [:index, :show] do - namespace :admin do - root :to => "index#index" - end - resources :comments, :except => :destroy do - get "views" => "comments#views", :as => :views - end - end - - resource :past, :only => :destroy - resource :present, :only => :update - resource :future, :only => :create - resources :relationships, :only => [:create, :destroy] - resources :friendships, :only => [:update] - - shallow do - namespace :api do - resources :teams do - resources :players - resource :captain - end - end - end - - scope '/hello' do - shallow do - resources :notes do - resources :trackbacks - end - end - end - - resources :threads, :shallow => true do - resource :owner - resources :messages do - resources :comments do - member do - post :preview - end - end - end - end - - resources :sheep do - get "_it", :on => :member - end - - resources :clients do - namespace :google do - resource :account do - namespace :secret do - resource :info - end - end - end - end - - resources :customers do - get :recent, :on => :collection - get "profile", :on => :member - get "secret/profile" => "customers#secret", :on => :member - post "preview" => "customers#preview", :as => :another_preview, :on => :new - resource :avatar do - get "thumbnail" => "avatars#thumbnail", :as => :thumbnail, :on => :member - end - resources :invoices do - get "outstanding" => "invoices#outstanding", :on => :collection - get "overdue", :to => :overdue, :on => :collection - get "print" => "invoices#print", :as => :print, :on => :member - post "preview" => "invoices#preview", :as => :preview, :on => :new - get "aged/:months", :on => :collection, :action => :aged, :as => :aged - end - resources :notes, :shallow => true do - get "preview" => "notes#preview", :as => :preview, :on => :new - get "print" => "notes#print", :as => :print, :on => :member - end - get "inactive", :on => :collection - post "deactivate", :on => :member - get "old", :on => :collection, :as => :stale - get "export" - end - - namespace :api do - resources :customers do - get "recent" => "customers#recent", :as => :recent, :on => :collection - get "profile" => "customers#profile", :as => :profile, :on => :member - post "preview" => "customers#preview", :as => :preview, :on => :new - end - scope(':version', :version => /.+/) do - resources :users, :id => /.+?/, :format => /json|xml/ - end - - get "products/list" - end - - get 'sprockets.js' => ::TestRoutingMapper::SprocketsApp - - get 'people/:id/update', :to => 'people#update', :as => :update_person - get '/projects/:project_id/people/:id/update', :to => 'people#update', :as => :update_project_person - - # misc - get 'articles/:year/:month/:day/:title', :to => "articles#show", :as => :article - - # default params - get 'inline_pages/(:id)', :to => 'pages#show', :id => 'home' - get 'default_pages/(:id)', :to => 'pages#show', :defaults => { :id => 'home' } - defaults :id => 'home' do - get 'scoped_pages/(:id)', :to => 'pages#show' - end - - namespace :account do - get 'shorthand' - get 'description', :to => :description, :as => "description" - get ':action/callback', :action => /twitter|github/, :to => "callbacks", :as => :callback - resource :subscription, :credit, :credit_card - - root :to => "account#index" - - namespace :admin do - resource :subscription - end - end - - namespace :forum do - resources :products, :path => '' do - resources :questions - end - end - - namespace :users, :path => 'usuarios' do - root :to => 'home#index' - end - - controller :articles do - scope '/articles', :as => 'article' do - scope :path => '/:title', :title => /[a-z]+/, :as => :with_title do - get '/:id', :to => :with_id, :as => "" - end - end - end - - scope ':access_token', :constraints => { :access_token => /\w{5,5}/ } do - resources :rooms - end - - get '/info' => 'projects#info', :as => 'info' - - namespace :admin do - scope '(:locale)', :locale => /en|pl/ do - resources :descriptions - end - end - - scope '(:locale)', :locale => /en|pl/ do - get "registrations/new" - resources :descriptions - root :to => 'projects#index' - end - - scope :only => [:index, :show] do - resources :products, :constraints => { :id => /\d{4}/ } do - root :to => "products#root" - get :favorite, :on => :collection - resources :images - end - resource :account - end - - resource :dashboard, :constraints => { :ip => /192\.168\.1\.\d{1,3}/ } - - resource :token, :module => :api - scope :module => :api do - resources :errors, :shallow => true do - resources :notices - end - end - - scope :path => 'api' do - resource :me - get '/' => 'mes#index' - scope :v2 do - resource :me, as: 'v2_me' - get '/' => 'mes#index' - end - - scope :v3, :admin do - resource :me, as: 'v3_me' - end - end - - get "(/:username)/followers" => "followers#index" - get "/groups(/user/:username)" => "groups#index" - get "(/user/:username)/photos" => "photos#index" - - scope '(groups)' do - scope '(discussions)' do - resources :messages - end - end - - get "whatever/:controller(/:action(/:id))", :id => /\d+/ - - resource :profile do - get :settings - - new do - post :preview - end - end - - resources :content - - namespace :transport do - resources :taxis - end - - namespace :medical do - resource :taxis - end - - scope :constraints => { :id => /\d+/ } do - get '/tickets', :to => 'tickets#index', :as => :tickets - end - - scope :constraints => { :id => /\d{4}/ } do - resources :movies do - resources :reviews - resource :trailer - end - end - - namespace :private do - root :to => redirect('/private/index') - get "index", :to => 'private#index' - end - - scope :only => [:index, :show] do - namespace :only do - resources :clubs do - resources :players - resource :chairman - end - end - end - - scope :except => [:new, :create, :edit, :update, :destroy] do - namespace :except do - resources :clubs do - resources :players - resource :chairman - end - end - end - - namespace :wiki do - resources :articles, :id => /[^\/]+/ do - resources :comments, :only => [:create, :new] - end - end - - resources :wiki_pages, :path => :pages - resource :wiki_account, :path => :my_account - - scope :only => :show do - namespace :only do - resources :sectors, :only => :index do - resources :companies do - scope :only => :index do - resources :divisions - end - scope :except => [:show, :update, :destroy] do - resources :departments - end - end - resource :leader - resources :managers, :except => [:show, :update, :destroy] - end - end - end - - scope :except => :index do - namespace :except do - resources :sectors, :except => [:show, :update, :destroy] do - resources :companies do - scope :except => [:show, :update, :destroy] do - resources :divisions - end - scope :only => :index do - resources :departments - end - end - resource :leader - resources :managers, :only => :index - end - end - end - - resources :sections, :id => /.+/ do - get :preview, :on => :member - end - - resources :profiles, :param => :username, :username => /[a-z]+/ do - get :details, :on => :member - resources :messages - end - - resources :orders do - constraints :download => /[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}/ do - resources :downloads, :param => :download, :shallow => true - end - end - - scope :as => "routes" do - get "/c/:id", :as => :collision, :to => "collision#show" - get "/collision", :to => "collision#show" - get "/no_collision", :to => "collision#show", :as => nil - - get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" - get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" - end - - get '/purchases/:token/:filename', - :to => 'purchases#fetch', - :token => /[[:alnum:]]{10}/, - :filename => /(.+)/, - :as => :purchase - - resources :lists, :id => /([A-Za-z0-9]{25})|default/ do - resources :todos, :id => /\d+/ - end - - scope '/countries/:country', :constraints => lambda { |params, req| %w(all France).include?(params[:country]) } do - get '/', :to => 'countries#index' - get '/cities', :to => 'countries#cities' - end - - get '/countries/:country/(*other)', :to => redirect{ |params, req| params[:other] ? "/countries/all/#{params[:other]}" : '/countries/all' } - - get '/:locale/*file.:format', :to => 'files#show', :file => /path\/to\/existing\/file/ - - scope '/italians' do - get '/writers', :to => 'italians#writers', :constraints => ::TestRoutingMapper::IpRestrictor - get '/sculptors', :to => 'italians#sculptors' - get '/painters/:painter', :to => 'italians#painters', :constraints => {:painter => /michelangelo/} - end - end - end - - class TestAltApp < ActionDispatch::IntegrationTest - class AltRequest - def initialize(env) - @env = env - end - - def path_info - "/" - end - - def request_method - "GET" - end - - def ip - "127.0.0.1" - end - - def x_header - @env["HTTP_X_HEADER"] || "" - end - end - - class XHeader - def call(env) - [200, {"Content-Type" => "text/html"}, ["XHeader"]] - end - end - - class AltApp - def call(env) - [200, {"Content-Type" => "text/html"}, ["Alternative App"]] - end - end - - AltRoutes = ActionDispatch::Routing::RouteSet.new(AltRequest) - AltRoutes.draw do - get "/" => TestRoutingMapper::TestAltApp::XHeader.new, :constraints => {:x_header => /HEADER/} - get "/" => TestRoutingMapper::TestAltApp::AltApp.new - end - - def app - AltRoutes end - def test_alt_request_without_header - get "/" - assert_equal "Alternative App", @response.body - end - - def test_alt_request_with_matched_header - get "/", {}, "HTTP_X_HEADER" => "HEADER" - assert_equal "XHeader", @response.body - end - - def test_alt_request_with_unmatched_header - get "/", {}, "HTTP_X_HEADER" => "NON_MATCH" - assert_equal "Alternative App", @response.body - end - end - - def app - Routes - end - - include Routes.url_helpers - - def test_logout delete '/logout' assert_equal 'sessions#destroy', @response.body @@ -614,6 +35,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_login + draw do + default_url_options :host => "rubyonrails.org" + + controller :sessions do + get 'login' => :new + post 'login' => :create + end + end + get '/login' assert_equal 'sessions#new', @response.body assert_equal '/login', login_path @@ -624,39 +54,59 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest assert_equal '/login', url_for(:controller => 'sessions', :action => 'create', :only_path => true) assert_equal '/login', url_for(:controller => 'sessions', :action => 'new', :only_path => true) - assert_equal 'http://rubyonrails.org/login', Routes.url_for(:controller => 'sessions', :action => 'create') - assert_equal 'http://rubyonrails.org/login', Routes.url_helpers.login_url + assert_equal 'http://rubyonrails.org/login', url_for(:controller => 'sessions', :action => 'create') + assert_equal 'http://rubyonrails.org/login', login_url end def test_login_redirect + draw do + get 'account/login', :to => redirect("/login") + end + get '/account/login' verify_redirect 'http://www.example.com/login' end def test_logout_redirect_without_to + draw do + get 'account/logout' => redirect("/logout"), :as => :logout_redirect + end + assert_equal '/account/logout', logout_redirect_path get '/account/logout' verify_redirect 'http://www.example.com/logout' end def test_namespace_redirect + draw do + namespace :private do + root :to => redirect('/private/index') + get "index", :to => 'private#index' + end + end + get '/private' verify_redirect 'http://www.example.com/private/index' end def test_namespace_with_controller_segment assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw do - namespace :admin do - get '/:controller(/:action(/:id(.:format)))' - end + draw do + namespace :admin do + get '/:controller(/:action(/:id(.:format)))' end end end end def test_session_singleton_resource + draw do + resource :session do + get :create + post :reset + end + end + get '/session' assert_equal 'sessions#create', @response.body assert_equal '/session', session_path @@ -684,68 +134,126 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_session_info_nested_singleton_resource + draw do + resource :session do + resource :info + end + end + get '/session/info' assert_equal 'infos#show', @response.body assert_equal '/session/info', session_info_path end def test_member_on_resource + draw do + resource :session do + member do + get :crush + end + end + end + get '/session/crush' assert_equal 'sessions#crush', @response.body assert_equal '/session/crush', crush_session_path end def test_redirect_modulo + draw do + get 'account/modulo/:name', :to => redirect("/%{name}s") + end + get '/account/modulo/name' verify_redirect 'http://www.example.com/names' end def test_redirect_proc + draw do + get 'account/proc/:name', :to => redirect {|params, req| "/#{params[:name].pluralize}" } + end + get '/account/proc/person' verify_redirect 'http://www.example.com/people' end def test_redirect_proc_with_request + draw do + get 'account/proc_req' => redirect {|params, req| "/#{req.method}" } + end + get '/account/proc_req' verify_redirect 'http://www.example.com/GET' end def test_redirect_hash_with_subdomain + draw do + get 'mobile', :to => redirect(:subdomain => 'mobile') + end + get '/mobile' verify_redirect 'http://mobile.example.com/mobile' end def test_redirect_hash_with_domain_and_path + draw do + get 'documentation', :to => redirect(:domain => 'example-documentation.com', :path => '') + end + get '/documentation' verify_redirect 'http://www.example-documentation.com' end def test_redirect_hash_with_path + draw do + get 'new_documentation', :to => redirect(:path => '/documentation/new') + end + get '/new_documentation' verify_redirect 'http://www.example.com/documentation/new' end def test_redirect_hash_with_host + draw do + get 'super_new_documentation', :to => redirect(:host => 'super-docs.com') + end + get '/super_new_documentation?section=top' verify_redirect 'http://super-docs.com/super_new_documentation?section=top' end def test_redirect_hash_path_substitution + draw do + get 'stores/:name', :to => redirect(:subdomain => 'stores', :path => '/%{name}') + end + get '/stores/iernest' verify_redirect 'http://stores.example.com/iernest' end def test_redirect_hash_path_substitution_with_catch_all + draw do + get 'stores/:name(*rest)', :to => redirect(:subdomain => 'stores', :path => '/%{name}%{rest}') + end + get '/stores/iernest/products' verify_redirect 'http://stores.example.com/iernest/products' end def test_redirect_class + draw do + get 'youtube_favorites/:youtube_id/:name', :to => redirect(YoutubeFavoritesRedirector) + end + get '/youtube_favorites/oHg5SJYRHA0/rick-rolld' verify_redirect 'http://www.youtube.com/watch?v=oHg5SJYRHA0' end def test_openid + draw do + match 'openid/login', :via => [:get, :post], :to => "openid#login" + end + get '/openid/login' assert_equal 'openid#login', @response.body @@ -754,6 +262,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_bookmarks + draw do + scope "bookmark", :controller => "bookmarks", :as => :bookmark do + get :new, :path => "build" + post :create, :path => "create", :as => "" + put :update + get :remove, :action => :destroy, :as => :remove + end + end + get '/bookmark/build' assert_equal 'bookmarks#new', @response.body assert_equal '/bookmark/build', bookmark_new_path @@ -772,6 +289,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_pagemarks + draw do + scope "pagemark", :controller => "pagemarks", :as => :pagemark do + get "new", :path => "build" + post "create", :as => "" + put "update" + get "remove", :action => :destroy, :as => :remove + end + end + get '/pagemark/build' assert_equal 'pagemarks#new', @response.body assert_equal '/pagemark/build', pagemark_new_path @@ -790,6 +316,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_admin + draw do + constraints(:ip => /192\.168\.1\.\d\d\d/) do + get 'admin' => "queenbee#index" + end + + constraints ::TestRoutingMapper::IpRestrictor do + get 'admin/accounts' => "queenbee#accounts" + end + + get 'admin/passwords' => "queenbee#passwords", :constraints => ::TestRoutingMapper::IpRestrictor + end + get '/admin', {}, {'REMOTE_ADDR' => '192.168.1.100'} assert_equal 'queenbee#index', @response.body @@ -810,6 +348,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_global + draw do + controller(:global) do + get 'global/hide_notice' + get 'global/export', :to => :export, :as => :export_request + get '/export/:id/:file', :to => :export, :as => :export_download, :constraints => { :file => /.*/ } + get 'global/:action' + end + end + get '/global/dashboard' assert_equal 'global#dashboard', @response.body @@ -828,12 +375,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_local + draw do + get "/local/:action", :controller => "local" + end + get '/local/dashboard' assert_equal 'local#dashboard', @response.body end # tests the use of dup in url_for def test_url_for_with_no_side_effects + draw do + get "/projects/status(.:format)" + end + # without dup, additional (and possibly unwanted) values will be present in the options (eg. :host) original_options = {:controller => 'projects', :action => 'status'} options = original_options.dup @@ -845,6 +400,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_url_for_does_not_modify_controller + draw do + get "/projects/status(.:format)" + end + controller = '/projects' options = {:controller => controller, :action => 'status', :only_path => true} url = url_for(options) @@ -855,6 +414,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest # tests the arguments modification free version of define_hash_access def test_named_route_with_no_side_effects + draw do + resources :customers do + get "profile", :on => :member + end + end + original_options = { :host => 'test.host' } options = original_options.dup @@ -865,11 +430,19 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_status + draw do + get "/projects/status(.:format)" + end + assert_equal '/projects/status', url_for(:controller => 'projects', :action => 'status', :only_path => true) assert_equal '/projects/status.json', url_for(:controller => 'projects', :action => 'status', :format => 'json', :only_path => true) end def test_projects + draw do + resources :projects, :controller => :project + end + get '/projects' assert_equal 'project#index', @response.body assert_equal '/projects', projects_path @@ -903,12 +476,24 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_with_post_action_and_new_path_on_collection + draw do + resources :projects, :controller => :project do + post 'new', :action => 'new', :on => :collection, :as => :new + end + end + post '/projects/new' assert_equal "project#new", @response.body assert_equal "/projects/new", new_projects_path end def test_projects_involvements + draw do + resources :projects, :controller => :project do + resources :involvements, :attachments + end + end + get '/projects/1/involvements' assert_equal 'involvements#index', @response.body assert_equal '/projects/1/involvements', project_involvements_path(:project_id => '1') @@ -933,12 +518,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_attachments + draw do + resources :projects, :controller => :project do + resources :involvements, :attachments + end + end + get '/projects/1/attachments' assert_equal 'attachments#index', @response.body assert_equal '/projects/1/attachments', project_attachments_path(:project_id => '1') end def test_projects_participants + draw do + resources :projects, :controller => :project do + resources :participants do + put :update_all, :on => :collection + end + end + end + get '/projects/1/participants' assert_equal 'participants#index', @response.body assert_equal '/projects/1/participants', project_participants_path(:project_id => '1') @@ -949,6 +548,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_companies + draw do + resources :projects, :controller => :project do + resources :companies do + resources :people + resource :avatar, :controller => :avatar + end + end + end + get '/projects/1/companies' assert_equal 'companies#index', @response.body assert_equal '/projects/1/companies', project_companies_path(:project_id => '1') @@ -963,6 +571,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_project_manager + draw do + resources :projects do + resource :manager, :as => :super_manager do + post :fire + end + end + end + get '/projects/1/manager' assert_equal 'managers#show', @response.body assert_equal '/projects/1/manager', project_super_manager_path(:project_id => '1') @@ -977,6 +593,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_project_images + draw do + resources :projects do + resources :images, :as => :funny_images do + post :revise, :on => :member + end + end + end + get '/projects/1/images' assert_equal 'images#index', @response.body assert_equal '/projects/1/images', project_funny_images_path(:project_id => '1') @@ -991,6 +615,23 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_people + draw do + resources :projects do + resources :people do + nested do + scope "/:access_token" do + resource :avatar + end + end + + member do + put :accessible_projects + post :resend, :generate_new_password + end + end + end + end + get '/projects/1/people' assert_equal 'people#index', @response.body assert_equal '/projects/1/people', project_people_path(:project_id => '1') @@ -1017,12 +658,35 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_projects_with_resources_path_names + draw do + resources_path_names :correlation_indexes => "info_about_correlation_indexes" + + resources :projects do + get :correlation_indexes, :on => :collection + end + end + get '/projects/info_about_correlation_indexes' - assert_equal 'project#correlation_indexes', @response.body + assert_equal 'projects#correlation_indexes', @response.body assert_equal '/projects/info_about_correlation_indexes', correlation_indexes_projects_path end def test_projects_posts + draw do + resources :projects do + resources :posts do + get :archive, :toggle_view, :on => :collection + post :preview, :on => :member + + resource :subscription + + resources :comments do + post :preview, :on => :collection + end + end + end + end + get '/projects/1/posts' assert_equal 'posts#index', @response.body assert_equal '/projects/1/posts', project_posts_path(:project_id => '1') @@ -1053,6 +717,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_replies + draw do + resources :replies do + member do + put :answer, :to => :mark_as_answer + delete :answer, :to => :unmark_as_answer + end + end + end + put '/replies/1/answer' assert_equal 'replies#mark_as_answer', @response.body @@ -1061,6 +734,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resource_routes_with_only_and_except + draw do + resources :posts, :only => [:index, :show] do + resources :comments, :except => :destroy + end + end + get '/posts' assert_equal 'posts#index', @response.body assert_equal '/posts', posts_path @@ -1084,6 +763,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resource_routes_only_create_update_destroy + draw do + resource :past, :only => :destroy + resource :present, :only => :update + resource :future, :only => :create + end + delete '/past' assert_equal 'pasts#destroy', @response.body assert_equal '/past', past_path @@ -1102,6 +787,11 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resources_routes_only_create_update_destroy + draw do + resources :relationships, :only => [:create, :destroy] + resources :friendships, :only => [:update] + end + post '/relationships' assert_equal 'relationships#create', @response.body assert_equal '/relationships', relationships_path @@ -1120,12 +810,22 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resource_with_slugs_in_ids + draw do + resources :posts + end + get '/posts/rails-rocks' assert_equal 'posts#show', @response.body assert_equal '/posts/rails-rocks', post_path(:id => 'rails-rocks') end def test_resources_for_uncountable_names + draw do + resources :sheep do + get "_it", :on => :member + end + end + assert_equal '/sheep', sheep_index_path assert_equal '/sheep/1', sheep_path(1) assert_equal '/sheep/new', new_sheep_path @@ -1135,25 +835,26 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest def test_resource_does_not_modify_passed_options options = {:id => /.+?/, :format => /json|xml/} - self.class.stub_controllers do |routes| - routes.draw do - resource :user, options - end - end + draw { resource :user, options } assert_equal({:id => /.+?/, :format => /json|xml/}, options) end def test_resources_does_not_modify_passed_options options = {:id => /.+?/, :format => /json|xml/} - self.class.stub_controllers do |routes| - routes.draw do - resources :users, options - end - end + draw { resources :users, options } assert_equal({:id => /.+?/, :format => /json|xml/}, options) end def test_path_names + draw do + scope 'pt', :as => 'pt' do + resources :projects, :path_names => { :edit => 'editar', :new => 'novo' }, :path => 'projetos' + resource :admin, :path_names => { :new => 'novo', :activate => 'ativar' }, :path => 'administrador' do + put :activate, :on => :member + end + end + end + get '/pt/projetos' assert_equal 'projects#index', @response.body assert_equal '/pt/projetos', pt_projects_path @@ -1176,6 +877,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_path_option_override + draw do + scope 'pt', :as => 'pt' do + resources :projects, :path_names => { :new => 'novo' }, :path => 'projetos' do + put :close, :on => :member, :path => 'fechar' + get :open, :on => :new, :path => 'abrir' + end + end + end + get '/pt/projetos/novo/abrir' assert_equal 'projects#open', @response.body assert_equal '/pt/projetos/novo/abrir', open_new_pt_project_path @@ -1186,11 +896,19 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_sprockets + draw do + get 'sprockets.js' => ::TestRoutingMapper::SprocketsApp + end + get '/sprockets.js' assert_equal 'javascripts', @response.body end def test_update_person_route + draw do + get 'people/:id/update', :to => 'people#update', :as => :update_person + end + get '/people/1/update' assert_equal 'people#update', @response.body @@ -1198,6 +916,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_update_project_person + draw do + get '/projects/:project_id/people/:id/update', :to => 'people#update', :as => :update_project_person + end + get '/projects/1/people/2/update' assert_equal 'people#update', @response.body @@ -1205,6 +927,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_forum_products + draw do + namespace :forum do + resources :products, :path => '' do + resources :questions + end + end + end + get '/forum' assert_equal 'forum/products#index', @response.body assert_equal '/forum', forum_products_path @@ -1223,6 +953,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_articles_perma + draw do + get 'articles/:year/:month/:day/:title', :to => "articles#show", :as => :article + end + get '/articles/2009/08/18/rails-3' assert_equal 'articles#show', @response.body @@ -1230,6 +964,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_account_namespace + draw do + namespace :account do + resource :subscription, :credit, :credit_card + end + end + get '/account/subscription' assert_equal 'account/subscriptions#show', @response.body assert_equal '/account/subscription', account_subscription_path @@ -1244,12 +984,32 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_nested_namespace + draw do + namespace :account do + namespace :admin do + resource :subscription + end + end + end + get '/account/admin/subscription' assert_equal 'account/admin/subscriptions#show', @response.body assert_equal '/account/admin/subscription', account_admin_subscription_path end def test_namespace_nested_in_resources + draw do + resources :clients do + namespace :google do + resource :account do + namespace :secret do + resource :info + end + end + end + end + end + get '/clients/1/google/account' assert_equal '/clients/1/google/account', client_google_account_path(1) assert_equal 'google/accounts#show', @response.body @@ -1260,12 +1020,28 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_namespace_with_options + draw do + namespace :users, :path => 'usuarios' do + root :to => 'home#index' + end + end + get '/usuarios' assert_equal '/usuarios', users_root_path assert_equal 'users/home#index', @response.body end def test_articles_with_id + draw do + controller :articles do + scope '/articles', :as => 'article' do + scope :path => '/:title', :title => /[a-z]+/, :as => :with_title do + get '/:id', :to => :with_id, :as => "" + end + end + end + end + get '/articles/rails/1' assert_equal 'articles#with_id', @response.body @@ -1276,6 +1052,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_access_token_rooms + draw do + scope ':access_token', :constraints => { :access_token => /\w{5,5}/ } do + resources :rooms + end + end + get '/12345/rooms' assert_equal 'rooms#index', @response.body @@ -1287,40 +1069,91 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_root + draw do + root :to => 'projects#index' + end + assert_equal '/', root_path get '/' assert_equal 'projects#index', @response.body end + def test_scoped_root + draw do + scope '(:locale)', :locale => /en|pl/ do + root :to => 'projects#index' + end + end + + assert_equal '/en', root_path(:locale => 'en') + get '/en' + assert_equal 'projects#index', @response.body + end + def test_index + draw do + get '/info' => 'projects#info', :as => 'info' + end + assert_equal '/info', info_path get '/info' assert_equal 'projects#info', @response.body end def test_match_shorthand_with_no_scope + draw do + get 'account/overview' + end + assert_equal '/account/overview', account_overview_path get '/account/overview' assert_equal 'account#overview', @response.body end def test_match_shorthand_inside_namespace + draw do + namespace :account do + get 'shorthand' + end + end + assert_equal '/account/shorthand', account_shorthand_path get '/account/shorthand' assert_equal 'account#shorthand', @response.body end def test_match_shorthand_inside_namespace_with_controller + draw do + namespace :api do + get "products/list" + end + end + assert_equal '/api/products/list', api_products_list_path get '/api/products/list' assert_equal 'api/products#list', @response.body end def test_dynamically_generated_helpers_on_collection_do_not_clobber_resources_url_helper + draw do + resources :replies do + collection do + get 'page/:page' => 'replies#index', :page => %r{\d+} + get ':page' => 'replies#index', :page => %r{\d+} + end + end + end + assert_equal '/replies', replies_path end def test_scoped_controller_with_namespace_and_action + draw do + namespace :account do + get ':action/callback', :action => /twitter|github/, :to => "callbacks", :as => :callback + end + end + assert_equal '/account/twitter/callback', account_callback_path("twitter") get '/account/twitter/callback' assert_equal 'account/callbacks#twitter', @response.body @@ -1330,23 +1163,39 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_convention_match_nested_and_with_leading_slash + draw do + get '/account/nested/overview' + end + assert_equal '/account/nested/overview', account_nested_overview_path get '/account/nested/overview' assert_equal 'account/nested#overview', @response.body end def test_convention_with_explicit_end + draw do + get 'sign_in' => "sessions#new" + end + get '/sign_in' assert_equal 'sessions#new', @response.body assert_equal '/sign_in', sign_in_path end def test_redirect_with_complete_url_and_status + draw do + get 'account/google' => redirect('http://www.google.com/', :status => 302) + end + get '/account/google' verify_redirect 'http://www.google.com/', 302 end def test_redirect_with_port + draw do + get 'account/login', :to => redirect("/login") + end + previous_host, self.host = self.host, 'www.example.com:3000' get '/account/login' @@ -1356,6 +1205,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_normalize_namespaced_matches + draw do + namespace :account do + get 'description', :to => :description, :as => "description" + end + end + assert_equal '/account/description', account_description_path get '/account/description' @@ -1363,18 +1218,36 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_namespaced_roots + draw do + namespace :account do + root :to => "account#index" + end + end + assert_equal '/account', account_root_path get '/account' assert_equal 'account/account#index', @response.body end def test_optional_scoped_root + draw do + scope '(:locale)', :locale => /en|pl/ do + root :to => 'projects#index' + end + end + assert_equal '/en', root_path("en") get '/en' assert_equal 'projects#index', @response.body end def test_optional_scoped_path + draw do + scope '(:locale)', :locale => /en|pl/ do + resources :descriptions + end + end + assert_equal '/en/descriptions', descriptions_path("en") assert_equal '/descriptions', descriptions_path(nil) assert_equal '/en/descriptions/1', description_path("en", 1) @@ -1394,6 +1267,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_nested_optional_scoped_path + draw do + namespace :admin do + scope '(:locale)', :locale => /en|pl/ do + resources :descriptions + end + end + end + assert_equal '/admin/en/descriptions', admin_descriptions_path("en") assert_equal '/admin/descriptions', admin_descriptions_path(nil) assert_equal '/admin/en/descriptions/1', admin_description_path("en", 1) @@ -1413,6 +1294,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_nested_optional_path_shorthand + draw do + scope '(:locale)', :locale => /en|pl/ do + get "registrations/new" + end + end + get '/registrations/new' assert_nil @request.params[:locale] @@ -1421,6 +1308,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_default_params + draw do + get 'inline_pages/(:id)', :to => 'pages#show', :id => 'home' + get 'default_pages/(:id)', :to => 'pages#show', :defaults => { :id => 'home' } + + defaults :id => 'home' do + get 'scoped_pages/(:id)', :to => 'pages#show' + end + end + get '/inline_pages' assert_equal 'home', @request.params[:id] @@ -1432,6 +1328,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resource_constraints + draw do + resources :products, :constraints => { :id => /\d{4}/ } do + root :to => "products#root" + get :favorite, :on => :collection + resources :images + end + + resource :dashboard, :constraints => { :ip => /192\.168\.1\.\d{1,3}/ } + end + get '/products/1' assert_equal 'pass', @response.headers['X-Cascade'] get '/products' @@ -1455,18 +1361,35 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_root_works_in_the_resources_scope + draw do + resources :products do + root :to => "products#root" + end + end + get '/products' assert_equal 'products#root', @response.body assert_equal '/products', products_root_path end def test_module_scope + draw do + resource :token, :module => :api + end + get '/token' assert_equal 'api/tokens#show', @response.body assert_equal '/token', token_path end def test_path_scope + draw do + scope :path => 'api' do + resource :me + get '/' => 'mes#index' + end + end + get '/api/me' assert_equal 'mes#show', @response.body assert_equal '/api/me', me_path @@ -1476,6 +1399,19 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_symbol_scope + draw do + scope :path => 'api' do + scope :v2 do + resource :me, as: 'v2_me' + get '/' => 'mes#index' + end + + scope :v3, :admin do + resource :me, as: 'v3_me' + end + end + end + get '/api/v2/me' assert_equal 'mes#show', @response.body assert_equal '/api/v2/me', v2_me_path @@ -1488,6 +1424,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_url_generator_for_generic_route + draw do + get "whatever/:controller(/:action(/:id))" + end + get 'whatever/foo/bar' assert_equal 'foo#bar', @response.body @@ -1496,6 +1436,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_url_generator_for_namespaced_generic_route + draw do + get "whatever/:controller(/:action(/:id))", :id => /\d+/ + end + get 'whatever/foo/bar/show' assert_equal 'foo/bar#show', @response.body @@ -1509,11 +1453,37 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest url_for(:controller => "foo/bar", :action => "show", :id => '1') end - def test_assert_recognizes_account_overview - assert_recognizes({:controller => "account", :action => "overview"}, "/account/overview") - end - def test_resource_new_actions + draw do + resources :replies do + new do + post :preview + end + end + + scope 'pt', :as => 'pt' do + resources :projects, :path_names => { :new => 'novo' }, :path => 'projetos' do + post :preview, :on => :new + end + + resource :admin, :path_names => { :new => 'novo' }, :path => 'administrador' do + post :preview, :on => :new + end + + resources :products, :path_names => { :new => 'novo' } do + new do + post :preview + end + end + end + + resource :profile do + new do + post :preview + end + end + end + assert_equal '/replies/new/preview', preview_new_reply_path assert_equal '/pt/projetos/novo/preview', preview_new_pt_project_path assert_equal '/pt/administrador/novo/preview', preview_new_pt_admin_path @@ -1537,13 +1507,27 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resource_merges_options_from_scope - assert_raise(NameError) { new_account_path } + draw do + scope :only => :show do + resource :account + end + end + + assert_raise(NoMethodError) { new_account_path } get '/account/new' assert_equal 404, status end def test_resources_merges_options_from_scope + draw do + scope :only => [:index, :show] do + resources :products do + resources :images + end + end + end + assert_raise(NoMethodError) { edit_product_path('1') } get '/products/1/edit' @@ -1556,6 +1540,28 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_shallow_nested_resources + draw do + shallow do + namespace :api do + resources :teams do + resources :players + resource :captain + end + end + end + + resources :threads, :shallow => true do + resource :owner + resources :messages do + resources :comments do + member do + post :preview + end + end + end + end + end + get '/api/teams' assert_equal 'api/teams#index', @response.body assert_equal '/api/teams', api_teams_path @@ -1658,6 +1664,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_shallow_nested_resources_within_scope + draw do + scope '/hello' do + shallow do + resources :notes do + resources :trackbacks + end + end + end + end + get '/hello/notes/1/trackbacks' assert_equal 'trackbacks#index', @response.body assert_equal '/hello/notes/1/trackbacks', note_trackbacks_path(:note_id => 1) @@ -1709,6 +1725,36 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_custom_resource_routes_are_scoped + draw do + resources :customers do + get :recent, :on => :collection + get "profile", :on => :member + get "secret/profile" => "customers#secret", :on => :member + post "preview" => "customers#preview", :as => :another_preview, :on => :new + resource :avatar do + get "thumbnail" => "avatars#thumbnail", :as => :thumbnail, :on => :member + end + resources :invoices do + get "outstanding" => "invoices#outstanding", :on => :collection + get "overdue", :to => :overdue, :on => :collection + get "print" => "invoices#print", :as => :print, :on => :member + post "preview" => "invoices#preview", :as => :preview, :on => :new + end + resources :notes, :shallow => true do + get "preview" => "notes#preview", :as => :preview, :on => :new + get "print" => "notes#print", :as => :print, :on => :member + end + end + + namespace :api do + resources :customers do + get "recent" => "customers#recent", :as => :recent, :on => :collection + get "profile" => "customers#profile", :as => :profile, :on => :member + post "preview" => "customers#preview", :as => :preview, :on => :new + end + end + end + assert_equal '/customers/recent', recent_customers_path assert_equal '/customers/1/profile', profile_customer_path(:id => '1') assert_equal '/customers/1/secret/profile', secret_profile_customer_path(:id => '1') @@ -1731,6 +1777,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_shallow_nested_routes_ignore_module + draw do + scope :module => :api do + resources :errors, :shallow => true do + resources :notices + end + end + end + get '/errors/1/notices' assert_equal 'api/notices#index', @response.body assert_equal '/errors/1/notices', error_notices_path(:error_id => '1') @@ -1741,6 +1795,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_non_greedy_regexp + draw do + namespace :api do + scope(':version', :version => /.+/) do + resources :users, :id => /.+?/, :format => /json|xml/ + end + end + end + get '/api/1.0/users' assert_equal 'api/users#index', @response.body assert_equal '/api/1.0/users', api_users_path(:version => '1.0') @@ -1763,16 +1825,28 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_glob_parameter_accepts_regexp + draw do + get '/:locale/*file.:format', :to => 'files#show', :file => /path\/to\/existing\/file/ + end + get '/en/path/to/existing/file.html' assert_equal 200, @response.status end def test_resources_controller_name_is_not_pluralized + draw do + resources :content + end + get '/content' assert_equal 'content#index', @response.body end def test_url_generator_for_optional_prefix_dynamic_segment + draw do + get "(/:username)/followers" => "followers#index" + end + get '/bob/followers' assert_equal 'followers#index', @response.body assert_equal 'http://www.example.com/bob/followers', @@ -1785,6 +1859,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_url_generator_for_optional_suffix_static_and_dynamic_segment + draw do + get "/groups(/user/:username)" => "groups#index" + end + get '/groups/user/bob' assert_equal 'groups#index', @response.body assert_equal 'http://www.example.com/groups/user/bob', @@ -1797,6 +1875,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_url_generator_for_optional_prefix_static_and_dynamic_segment + draw do + get "(/user/:username)/photos" => "photos#index" + end + get 'user/bob/photos' assert_equal 'photos#index', @response.body assert_equal 'http://www.example.com/user/bob/photos', @@ -1809,6 +1891,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_url_recognition_for_optional_static_segments + draw do + scope '(groups)' do + scope '(discussions)' do + resources :messages + end + end + end + get '/groups/discussions/messages' assert_equal 'messages#index', @response.body @@ -1835,12 +1925,27 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_router_removes_invalid_conditions + draw do + scope :constraints => { :id => /\d+/ } do + get '/tickets', :to => 'tickets#index', :as => :tickets + end + end + get '/tickets' assert_equal 'tickets#index', @response.body assert_equal '/tickets', tickets_path end def test_constraints_are_merged_from_scope + draw do + scope :constraints => { :id => /\d{4}/ } do + resources :movies do + resources :reviews + resource :trailer + end + end + end + get '/movies/0001' assert_equal 'movies#show', @response.body assert_equal '/movies/0001', movie_path(:id => '0001') @@ -1875,6 +1980,17 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_only_should_be_read_from_scope + draw do + scope :only => [:index, :show] do + namespace :only do + resources :clubs do + resources :players + resource :chairman + end + end + end + end + get '/only/clubs' assert_equal 'only/clubs#index', @response.body assert_equal '/only/clubs', only_clubs_path @@ -1901,6 +2017,17 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_except_should_be_read_from_scope + draw do + scope :except => [:new, :create, :edit, :update, :destroy] do + namespace :except do + resources :clubs do + resources :players + resource :chairman + end + end + end + end + get '/except/clubs' assert_equal 'except/clubs#index', @response.body assert_equal '/except/clubs', except_clubs_path @@ -1927,6 +2054,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_only_option_should_override_scope + draw do + scope :only => :show do + namespace :only do + resources :sectors, :only => :index + end + end + end + get '/only/sectors' assert_equal 'only/sectors#index', @response.body assert_equal '/only/sectors', only_sectors_path @@ -1937,6 +2072,17 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_only_option_should_not_inherit + draw do + scope :only => :show do + namespace :only do + resources :sectors, :only => :index do + resources :companies + resource :leader + end + end + end + end + get '/only/sectors/1/companies/2' assert_equal 'only/companies#show', @response.body assert_equal '/only/sectors/1/companies/2', only_sector_company_path(:sector_id => '1', :id => '2') @@ -1947,6 +2093,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_except_option_should_override_scope + draw do + scope :except => :index do + namespace :except do + resources :sectors, :except => [:show, :update, :destroy] + end + end + end + get '/except/sectors' assert_equal 'except/sectors#index', @response.body assert_equal '/except/sectors', except_sectors_path @@ -1957,6 +2111,17 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_except_option_should_not_inherit + draw do + scope :except => :index do + namespace :except do + resources :sectors, :except => [:show, :update, :destroy] do + resources :companies + resource :leader + end + end + end + end + get '/except/sectors/1/companies/2' assert_equal 'except/companies#show', @response.body assert_equal '/except/sectors/1/companies/2', except_sector_company_path(:sector_id => '1', :id => '2') @@ -1967,6 +2132,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_except_option_should_override_scoped_only + draw do + scope :only => :show do + namespace :only do + resources :sectors, :only => :index do + resources :managers, :except => [:show, :update, :destroy] + end + end + end + end + get '/only/sectors/1/managers' assert_equal 'only/managers#index', @response.body assert_equal '/only/sectors/1/managers', only_sector_managers_path(:sector_id => '1') @@ -1977,6 +2152,16 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_only_option_should_override_scoped_except + draw do + scope :except => :index do + namespace :except do + resources :sectors, :except => [:show, :update, :destroy] do + resources :managers, :only => :index + end + end + end + end + get '/except/sectors/1/managers' assert_equal 'except/managers#index', @response.body assert_equal '/except/sectors/1/managers', except_sector_managers_path(:sector_id => '1') @@ -1987,6 +2172,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_only_scope_should_override_parent_scope + draw do + scope :only => :show do + namespace :only do + resources :sectors, :only => :index do + resources :companies do + scope :only => :index do + resources :divisions + end + end + end + end + end + end + get '/only/sectors/1/companies/2/divisions' assert_equal 'only/divisions#index', @response.body assert_equal '/only/sectors/1/companies/2/divisions', only_sector_company_divisions_path(:sector_id => '1', :company_id => '2') @@ -1997,6 +2196,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_except_scope_should_override_parent_scope + draw do + scope :except => :index do + namespace :except do + resources :sectors, :except => [:show, :update, :destroy] do + resources :companies do + scope :except => [:show, :update, :destroy] do + resources :divisions + end + end + end + end + end + end + get '/except/sectors/1/companies/2/divisions' assert_equal 'except/divisions#index', @response.body assert_equal '/except/sectors/1/companies/2/divisions', except_sector_company_divisions_path(:sector_id => '1', :company_id => '2') @@ -2007,6 +2220,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_except_scope_should_override_parent_only_scope + draw do + scope :only => :show do + namespace :only do + resources :sectors, :only => :index do + resources :companies do + scope :except => [:show, :update, :destroy] do + resources :departments + end + end + end + end + end + end + get '/only/sectors/1/companies/2/departments' assert_equal 'only/departments#index', @response.body assert_equal '/only/sectors/1/companies/2/departments', only_sector_company_departments_path(:sector_id => '1', :company_id => '2') @@ -2017,6 +2244,20 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_only_scope_should_override_parent_except_scope + draw do + scope :except => :index do + namespace :except do + resources :sectors, :except => [:show, :update, :destroy] do + resources :companies do + scope :only => :index do + resources :departments + end + end + end + end + end + end + get '/except/sectors/1/companies/2/departments' assert_equal 'except/departments#index', @response.body assert_equal '/except/sectors/1/companies/2/departments', except_sector_company_departments_path(:sector_id => '1', :company_id => '2') @@ -2027,6 +2268,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resources_are_not_pluralized + draw do + namespace :transport do + resources :taxis + end + end + get '/transport/taxis' assert_equal 'transport/taxis#index', @response.body assert_equal '/transport/taxis', transport_taxis_path @@ -2054,6 +2301,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_singleton_resources_are_not_singularized + draw do + namespace :medical do + resource :taxis + end + end + get '/medical/taxis/new' assert_equal 'medical/taxis#new', @response.body assert_equal '/medical/taxis/new', new_medical_taxis_path @@ -2077,6 +2330,12 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_greedy_resource_id_regexp_doesnt_match_edit_and_custom_action + draw do + resources :sections, :id => /.+/ do + get :preview, :on => :member + end + end + get '/sections/1/edit' assert_equal 'sections#edit', @response.body assert_equal '/sections/1/edit', edit_section_path(:id => '1') @@ -2087,6 +2346,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resource_constraints_are_pushed_to_scope + draw do + namespace :wiki do + resources :articles, :id => /[^\/]+/ do + resources :comments, :only => [:create, :new] + end + end + end + get '/wiki/articles/Ruby_on_Rails_3.0' assert_equal 'wiki/articles#show', @response.body assert_equal '/wiki/articles/Ruby_on_Rails_3.0', wiki_article_path(:id => 'Ruby_on_Rails_3.0') @@ -2101,6 +2368,11 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_resources_path_can_be_a_symbol + draw do + resources :wiki_pages, :path => :pages + resource :wiki_account, :path => :my_account + end + get '/pages' assert_equal 'wiki_pages#index', @response.body assert_equal '/pages', wiki_pages_path @@ -2115,6 +2387,10 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_redirect_https + draw do + get 'secure', :to => redirect("/secure/login") + end + with_https do get '/secure' verify_redirect 'https://www.example.com/secure/login' @@ -2122,6 +2398,15 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_symbolized_path_parameters_is_not_stale + draw do + scope '/countries/:country', :constraints => lambda { |params, req| %w(all France).include?(params[:country]) } do + get '/', :to => 'countries#index' + get '/cities', :to => 'countries#cities' + end + + get '/countries/:country/(*other)', :to => redirect{ |params, req| params[:other] ? "/countries/all/#{params[:other]}" : '/countries/all' } + end + get '/countries/France' assert_equal 'countries#index', @response.body @@ -2136,6 +2421,14 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_constraints_block_not_carried_to_following_routes + draw do + scope '/italians' do + get '/writers', :to => 'italians#writers', :constraints => ::TestRoutingMapper::IpRestrictor + get '/sculptors', :to => 'italians#sculptors' + get '/painters/:painter', :to => 'italians#painters', :constraints => {:painter => /michelangelo/} + end + end + get '/italians/writers' assert_equal 'Not Found', @response.body @@ -2150,6 +2443,18 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_custom_resource_actions_defined_using_string + draw do + resources :customers do + resources :invoices do + get "aged/:months", :on => :collection, :action => :aged, :as => :aged + end + + get "inactive", :on => :collection + post "deactivate", :on => :member + get "old", :on => :collection, :as => :stale + end + end + get '/customers/inactive' assert_equal 'customers#inactive', @response.body assert_equal '/customers/inactive', inactive_customers_path @@ -2168,18 +2473,38 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_route_defined_in_resources_scope_level + draw do + resources :customers do + get "export" + end + end + get '/customers/1/export' assert_equal 'customers#export', @response.body assert_equal '/customers/1/export', customer_export_path(:customer_id => '1') end def test_named_character_classes_in_regexp_constraints + draw do + get '/purchases/:token/:filename', + :to => 'purchases#fetch', + :token => /[[:alnum:]]{10}/, + :filename => /(.+)/, + :as => :purchase + end + get '/purchases/315004be7e/Ruby_on_Rails_3.pdf' assert_equal 'purchases#fetch', @response.body assert_equal '/purchases/315004be7e/Ruby_on_Rails_3.pdf', purchase_path(:token => '315004be7e', :filename => 'Ruby_on_Rails_3.pdf') end def test_nested_resource_constraints + draw do + resources :lists, :id => /([A-Za-z0-9]{25})|default/ do + resources :todos, :id => /\d+/ + end + end + get '/lists/01234012340123401234fffff' assert_equal 'lists#show', @response.body assert_equal '/lists/01234012340123401234fffff', list_path(:id => '01234012340123401234fffff') @@ -2194,6 +2519,17 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_named_routes_collision_is_avoided_unless_explicitly_given_as + draw do + scope :as => "routes" do + get "/c/:id", :as => :collision, :to => "collision#show" + get "/collision", :to => "collision#show" + get "/no_collision", :to => "collision#show", :as => nil + + get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" + get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" + end + end + assert_equal "/c/1", routes_collision_path(1) assert_equal "/fc/1", routes_forced_collision_path(1) end @@ -2204,86 +2540,100 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_explicitly_avoiding_the_named_route + draw do + scope :as => "routes" do + get "/c/:id", :as => :collision, :to => "collision#show" + get "/collision", :to => "collision#show" + get "/no_collision", :to => "collision#show", :as => nil + + get "/fc/:id", :as => :forced_collision, :to => "forced_collision#show" + get "/forced_collision", :as => :forced_collision, :to => "forced_collision#show" + end + end + assert !respond_to?(:routes_no_collision_path) 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 + draw { get '/feeds/:service', :to => '/feeds#show' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/feeds/:service', :controller => '/feeds', :action => 'show' } - end + draw { get '/feeds/:service', :controller => '/feeds', :action => 'show' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/api/feeds/:service', :to => '/api/feeds#show' } - end + draw { get '/api/feeds/:service', :to => '/api/feeds#show' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { controller("/feeds") { get '/feeds/:service', :to => :show } } - end + draw { controller("/feeds") { get '/feeds/:service', :to => :show } } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { resources :feeds, :controller => '/feeds' } - end + draw { resources :feeds, :controller => '/feeds' } end end def test_invalid_route_name_raises_error assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/products', :to => 'products#index', :as => 'products ' } - end + draw { get '/products', :to => 'products#index', :as => 'products ' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/products', :to => 'products#index', :as => ' products' } - end + draw { get '/products', :to => 'products#index', :as => ' products' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/products', :to => 'products#index', :as => 'products!' } - end + draw { get '/products', :to => 'products#index', :as => 'products!' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/products', :to => 'products#index', :as => 'products index' } - end + draw { get '/products', :to => 'products#index', :as => 'products index' } end assert_raise(ArgumentError) do - self.class.stub_controllers do |routes| - routes.draw { get '/products', :to => 'products#index', :as => '1products' } - end + draw { get '/products', :to => 'products#index', :as => '1products' } end end def test_nested_route_in_nested_resource + draw do + resources :posts, :only => [:index, :show] do + resources :comments, :except => :destroy do + get "views" => "comments#views", :as => :views + end + end + end + get "/posts/1/comments/2/views" assert_equal "comments#views", @response.body assert_equal "/posts/1/comments/2/views", post_comment_views_path(:post_id => '1', :comment_id => '2') end def test_root_in_deeply_nested_scope + draw do + resources :posts, :only => [:index, :show] do + namespace :admin do + root :to => "index#index" + end + end + end + get "/posts/1/admin" assert_equal "admin/index#index", @response.body assert_equal "/posts/1/admin", post_admin_root_path(:post_id => '1') end def test_custom_param + draw do + resources :profiles, :param => :username do + get :details, :on => :member + resources :messages + end + end + get '/profiles/bob' assert_equal 'profiles#show', @response.body assert_equal 'bob', @request.params[:username] @@ -2297,6 +2647,13 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_custom_param_constraint + draw do + resources :profiles, :param => :username, :username => /[a-z]+/ do + get :details, :on => :member + resources :messages + end + end + get '/profiles/bob1' assert_equal 404, @response.status @@ -2308,12 +2665,41 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest end def test_shallow_custom_param + draw do + resources :orders do + constraints :download => /[0-9a-f]{8}-(?:[0-9a-f]{4}-){3}[0-9a-f]{12}/ do + resources :downloads, :param => :download, :shallow => true + end + end + end + get '/downloads/0c0c0b68-d24b-11e1-a861-001ff3fffe6f.zip' assert_equal 'downloads#show', @response.body assert_equal '0c0c0b68-d24b-11e1-a861-001ff3fffe6f', @request.params[:download] end private + + def draw(&block) + self.class.stub_controllers do |routes| + @app = routes + @app.default_url_options = { host: 'www.example.com' } + @app.draw(&block) + end + end + + def url_for(options = {}) + @app.url_helpers.url_for(options) + end + + def method_missing(method, *args, &block) + if method.to_s =~ /_(path|url)$/ + @app.url_helpers.send(method, *args, &block) + else + super + end + end + def with_https old_https = https? https! @@ -2333,6 +2719,67 @@ private end end +class TestAltApp < ActionDispatch::IntegrationTest + class AltRequest + def initialize(env) + @env = env + end + + def path_info + "/" + end + + def request_method + "GET" + end + + def ip + "127.0.0.1" + end + + def x_header + @env["HTTP_X_HEADER"] || "" + end + end + + class XHeader + def call(env) + [200, {"Content-Type" => "text/html"}, ["XHeader"]] + end + end + + class AltApp + def call(env) + [200, {"Content-Type" => "text/html"}, ["Alternative App"]] + end + end + + AltRoutes = ActionDispatch::Routing::RouteSet.new(AltRequest) + AltRoutes.draw do + get "/" => TestAltApp::XHeader.new, :constraints => {:x_header => /HEADER/} + get "/" => TestAltApp::AltApp.new + end + + def app + AltRoutes + end + + def test_alt_request_without_header + get "/" + assert_equal "Alternative App", @response.body + end + + def test_alt_request_with_matched_header + get "/", {}, "HTTP_X_HEADER" => "HEADER" + assert_equal "XHeader", @response.body + end + + def test_alt_request_with_unmatched_header + get "/", {}, "HTTP_X_HEADER" => "NON_MATCH" + assert_equal "Alternative App", @response.body + end +end + class TestAppendingRoutes < ActionDispatch::IntegrationTest def simple_app(resp) lambda { |e| [ 200, { 'Content-Type' => 'text/plain' }, [resp] ] } diff --git a/actionpack/test/fixtures/functional_caching/fragment_cached_without_digest.html.erb b/actionpack/test/fixtures/functional_caching/fragment_cached_without_digest.html.erb new file mode 100644 index 0000000000..3125583a28 --- /dev/null +++ b/actionpack/test/fixtures/functional_caching/fragment_cached_without_digest.html.erb @@ -0,0 +1,3 @@ +<body> +<%= cache 'nodigest', skip_digest: true do %><p>ERB</p><% end %> +</body> diff --git a/actionpack/test/template/form_options_helper_test.rb b/actionpack/test/template/form_options_helper_test.rb index 54ab8b4d3a..c5efed3195 100644 --- a/actionpack/test/template/form_options_helper_test.rb +++ b/actionpack/test/template/form_options_helper_test.rb @@ -346,7 +346,7 @@ class FormOptionsHelperTest < ActionView::TestCase def test_optgroups_with_with_options_with_hash assert_dom_equal( - "<optgroup label=\"Europe\"><option value=\"Denmark\">Denmark</option>\n<option value=\"Germany\">Germany</option></optgroup><optgroup label=\"North America\"><option value=\"United States\">United States</option>\n<option value=\"Canada\">Canada</option></optgroup>", + "<optgroup label=\"North America\"><option value=\"United States\">United States</option>\n<option value=\"Canada\">Canada</option></optgroup><optgroup label=\"Europe\"><option value=\"Denmark\">Denmark</option>\n<option value=\"Germany\">Germany</option></optgroup>", grouped_options_for_select({'North America' => ['United States','Canada'], 'Europe' => ['Denmark','Germany']}) ) end diff --git a/activemodel/CHANGELOG.md b/activemodel/CHANGELOG.md index 133bb558a9..aacef97858 100644 --- a/activemodel/CHANGELOG.md +++ b/activemodel/CHANGELOG.md @@ -1,5 +1,20 @@ ## Rails 4.0.0 (unreleased) ## +* Observers was extracted from Active Model as `rails-observers` gem. + + *Rafael Mendonça França* + +* Specify type of singular association during serialization *Steve Klabnik* + +* Fixed length validator to correctly handle nil values. Fixes #7180. + + *Michal Zima* + +* Removed dispensable `require` statements. Make sure to require `active_model` before requiring + individual parts of the framework. + + *Yves Senn* + * Use BCrypt's MIN_COST in the test environment for speedier tests when using `has_secure_pasword`. *Brian Cardarella + Jeremy Kemper + Trevor Turk* diff --git a/activemodel/lib/active_model.rb b/activemodel/lib/active_model.rb index f757ba9843..0a32c5af05 100644 --- a/activemodel/lib/active_model.rb +++ b/activemodel/lib/active_model.rb @@ -40,8 +40,6 @@ module ActiveModel autoload :DeprecatedMassAssignmentSecurity autoload :Name, 'active_model/naming' autoload :Naming - autoload :Observer, 'active_model/observing' - autoload :Observing autoload :SecurePassword autoload :Serialization autoload :TestCase diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 86eaad830e..af11da1351 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -8,7 +8,7 @@ module ActiveModel # # user = User.first # user.pets.select(:id).first.user_id - # # => ActiveModel::MissingAttributeError: missing attribute: user_id + # # => ActiveModel::MissingAttributeError: missing attribute: user_id class MissingAttributeError < NoMethodError end # == Active \Model Attribute Methods @@ -202,7 +202,7 @@ module ActiveModel # person.name # => "Bob" # person.nickname # => "Bob" # person.name_short? # => true - # person.nickname_short? # => true + # person.nickname_short? # => true def alias_attribute(new_name, old_name) self.attribute_aliases = attribute_aliases.merge(new_name.to_s => old_name.to_s) attribute_method_matchers.each do |matcher| diff --git a/activemodel/lib/active_model/callbacks.rb b/activemodel/lib/active_model/callbacks.rb index eab94554cc..c52e4947ae 100644 --- a/activemodel/lib/active_model/callbacks.rb +++ b/activemodel/lib/active_model/callbacks.rb @@ -1,5 +1,3 @@ -require 'active_support/callbacks' - module ActiveModel # == Active \Model \Callbacks # diff --git a/activemodel/lib/active_model/conversion.rb b/activemodel/lib/active_model/conversion.rb index 42de32e63e..1f5d23dd8e 100644 --- a/activemodel/lib/active_model/conversion.rb +++ b/activemodel/lib/active_model/conversion.rb @@ -1,5 +1,3 @@ -require 'active_support/inflector' - module ActiveModel # == Active \Model Conversions # diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index 9d09353ef2..d47c3ae1bb 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -1,4 +1,3 @@ -require 'active_model/attribute_methods' require 'active_support/hash_with_indifferent_access' require 'active_support/core_ext/object/duplicable' diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index 264880eecd..6887f6d781 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -1,4 +1,3 @@ -require 'active_support/inflector' require 'active_support/core_ext/hash/except' require 'active_support/core_ext/module/introspection' @@ -56,8 +55,8 @@ module ActiveModel # end # # BlogPost.model_name <=> 'BlogPost' # => 0 - # BlogPost.model_name <=> 'Blog' # => 1 - # BlogPost.model_name <=> 'BlogPosts' # => -1 + # BlogPost.model_name <=> 'Blog' # => 1 + # BlogPost.model_name <=> 'BlogPosts' # => -1 ## # :method: =~ diff --git a/activemodel/lib/active_model/observer_array.rb b/activemodel/lib/active_model/observer_array.rb deleted file mode 100644 index 77bc0f71e3..0000000000 --- a/activemodel/lib/active_model/observer_array.rb +++ /dev/null @@ -1,152 +0,0 @@ -require 'set' - -module ActiveModel - # Stores the enabled/disabled state of individual observers for - # a particular model class. - class ObserverArray < Array - attr_reader :model_class - def initialize(model_class, *args) #:nodoc: - @model_class = model_class - super(*args) - end - - # Returns +true+ if the given observer is disabled for the model class, - # +false+ otherwise. - def disabled_for?(observer) #:nodoc: - disabled_observers.include?(observer.class) - end - - # Disables one or more observers. This supports multiple forms: - # - # ORM.observers.disable :all - # # => disables all observers for all models subclassed from - # # an ORM base class that includes ActiveModel::Observing - # # e.g. ActiveRecord::Base - # - # ORM.observers.disable :user_observer - # # => disables the UserObserver - # - # User.observers.disable AuditTrail - # # => disables the AuditTrail observer for User notifications. - # # Other models will still notify the AuditTrail observer. - # - # ORM.observers.disable :observer_1, :observer_2 - # # => disables Observer1 and Observer2 for all models. - # - # User.observers.disable :all do - # # all user observers are disabled for - # # just the duration of the block - # end - def disable(*observers, &block) - set_enablement(false, observers, &block) - end - - # Enables one or more observers. This supports multiple forms: - # - # ORM.observers.enable :all - # # => enables all observers for all models subclassed from - # # an ORM base class that includes ActiveModel::Observing - # # e.g. ActiveRecord::Base - # - # ORM.observers.enable :user_observer - # # => enables the UserObserver - # - # User.observers.enable AuditTrail - # # => enables the AuditTrail observer for User notifications. - # # Other models will not be affected (i.e. they will not - # # trigger notifications to AuditTrail if previously disabled) - # - # ORM.observers.enable :observer_1, :observer_2 - # # => enables Observer1 and Observer2 for all models. - # - # User.observers.enable :all do - # # all user observers are enabled for - # # just the duration of the block - # end - # - # Note: all observers are enabled by default. This method is only - # useful when you have previously disabled one or more observers. - def enable(*observers, &block) - set_enablement(true, observers, &block) - end - - protected - - def disabled_observers #:nodoc: - @disabled_observers ||= Set.new - end - - def observer_class_for(observer) #:nodoc: - return observer if observer.is_a?(Class) - - if observer.respond_to?(:to_sym) # string/symbol - observer.to_s.camelize.constantize - else - raise ArgumentError, "#{observer} was not a class or a " + - "lowercase, underscored class name as expected." - end - end - - def start_transaction #:nodoc: - disabled_observer_stack.push(disabled_observers.dup) - each_subclass_array do |array| - array.start_transaction - end - end - - def disabled_observer_stack #:nodoc: - @disabled_observer_stack ||= [] - end - - def end_transaction #:nodoc: - @disabled_observers = disabled_observer_stack.pop - each_subclass_array do |array| - array.end_transaction - end - end - - def transaction #:nodoc: - start_transaction - - begin - yield - ensure - end_transaction - end - end - - def each_subclass_array #:nodoc: - model_class.descendants.each do |subclass| - yield subclass.observers - end - end - - def set_enablement(enabled, observers) #:nodoc: - if block_given? - transaction do - set_enablement(enabled, observers) - yield - end - else - observers = ActiveModel::Observer.descendants if observers == [:all] - observers.each do |obs| - klass = observer_class_for(obs) - - unless klass < ActiveModel::Observer - raise ArgumentError.new("#{obs} does not refer to a valid observer") - end - - if enabled - disabled_observers.delete(klass) - else - disabled_observers << klass - end - end - - each_subclass_array do |array| - array.set_enablement(enabled, observers) - end - end - end - end -end diff --git a/activemodel/lib/active_model/observing.rb b/activemodel/lib/active_model/observing.rb deleted file mode 100644 index 5f1c99ce62..0000000000 --- a/activemodel/lib/active_model/observing.rb +++ /dev/null @@ -1,374 +0,0 @@ -require 'singleton' -require 'active_model/observer_array' -require 'active_support/core_ext/module/aliasing' -require 'active_support/core_ext/module/remove_method' -require 'active_support/core_ext/string/inflections' -require 'active_support/core_ext/enumerable' -require 'active_support/core_ext/object/try' -require 'active_support/descendants_tracker' - -module ActiveModel - # == Active \Model Observers Activation - module Observing - extend ActiveSupport::Concern - - included do - extend ActiveSupport::DescendantsTracker - end - - module ClassMethods - # Activates the observers assigned. - # - # class ORM - # include ActiveModel::Observing - # end - # - # # Calls PersonObserver.instance - # ORM.observers = :person_observer - # - # # Calls Cacher.instance and GarbageCollector.instance - # ORM.observers = :cacher, :garbage_collector - # - # # Same as above, just using explicit class references - # ORM.observers = Cacher, GarbageCollector - # - # Note: Setting this does not instantiate the observers yet. - # <tt>instantiate_observers</tt> is called during startup, and before - # each development request. - def observers=(*values) - observers.replace(values.flatten) - end - - # Gets an array of observers observing this model. The array also provides - # +enable+ and +disable+ methods that allow you to selectively enable and - # disable observers (see ActiveModel::ObserverArray.enable and - # ActiveModel::ObserverArray.disable for more on this). - # - # class ORM - # include ActiveModel::Observing - # end - # - # ORM.observers = :cacher, :garbage_collector - # ORM.observers # => [:cacher, :garbage_collector] - # ORM.observers.class # => ActiveModel::ObserverArray - def observers - @observers ||= ObserverArray.new(self) - end - - # Returns the current observer instances. - # - # class Foo - # include ActiveModel::Observing - # - # attr_accessor :status - # end - # - # class FooObserver < ActiveModel::Observer - # def on_spec(record, *args) - # record.status = true - # end - # end - # - # Foo.observers = FooObserver - # Foo.instantiate_observers - # - # Foo.observer_instances # => [#<FooObserver:0x007fc212c40820>] - def observer_instances - @observer_instances ||= [] - end - - # Instantiate the global observers. - # - # class Foo - # include ActiveModel::Observing - # - # attr_accessor :status - # end - # - # class FooObserver < ActiveModel::Observer - # def on_spec(record, *args) - # record.status = true - # end - # end - # - # Foo.observers = FooObserver - # - # foo = Foo.new - # foo.status = false - # foo.notify_observers(:on_spec) - # foo.status # => false - # - # Foo.instantiate_observers # => [FooObserver] - # - # foo = Foo.new - # foo.status = false - # foo.notify_observers(:on_spec) - # foo.status # => true - def instantiate_observers - observers.each { |o| instantiate_observer(o) } - end - - # Add a new observer to the pool. The new observer needs to respond to - # <tt>update</tt>, otherwise it raises an +ArgumentError+ exception. - # - # class Foo - # include ActiveModel::Observing - # end - # - # class FooObserver < ActiveModel::Observer - # end - # - # Foo.add_observer(FooObserver.instance) - # - # Foo.observers_instance - # # => [#<FooObserver:0x007fccf55d9390>] - def add_observer(observer) - unless observer.respond_to? :update - raise ArgumentError, "observer needs to respond to 'update'" - end - observer_instances << observer - end - - # Fires notifications to model's observers. - # - # def save - # notify_observers(:before_save) - # ... - # notify_observers(:after_save) - # end - # - # Custom notifications can be sent in a similar fashion: - # - # notify_observers(:custom_notification, :foo) - # - # This will call <tt>custom_notification</tt>, passing as arguments - # the current object and <tt>:foo</tt>. - def notify_observers(*args) - observer_instances.each { |observer| observer.update(*args) } - end - - # Returns the total number of instantiated observers. - # - # class Foo - # include ActiveModel::Observing - # - # attr_accessor :status - # end - # - # class FooObserver < ActiveModel::Observer - # def on_spec(record, *args) - # record.status = true - # end - # end - # - # Foo.observers = FooObserver - # Foo.observers_count # => 0 - # Foo.instantiate_observers - # Foo.observers_count # => 1 - def observers_count - observer_instances.size - end - - # <tt>count_observers</tt> is deprecated. Use #observers_count. - def count_observers - msg = "count_observers is deprecated in favor of observers_count" - ActiveSupport::Deprecation.warn msg - observers_count - end - - protected - def instantiate_observer(observer) #:nodoc: - # string/symbol - if observer.respond_to?(:to_sym) - observer = observer.to_s.camelize.constantize - end - if observer.respond_to?(:instance) - observer.instance - else - raise ArgumentError, - "#{observer} must be a lowercase, underscored class name (or " + - "the class itself) responding to the method :instance. " + - "Example: Person.observers = :big_brother # calls " + - "BigBrother.instance" - end - end - - # Notify observers when the observed class is subclassed. - def inherited(subclass) #:nodoc: - super - notify_observers :observed_class_inherited, subclass - end - end - - # Notify a change to the list of observers. - # - # class Foo - # include ActiveModel::Observing - # - # attr_accessor :status - # end - # - # class FooObserver < ActiveModel::Observer - # def on_spec(record, *args) - # record.status = true - # end - # end - # - # Foo.observers = FooObserver - # Foo.instantiate_observers # => [FooObserver] - # - # foo = Foo.new - # foo.status = false - # foo.notify_observers(:on_spec) - # foo.status # => true - # - # See ActiveModel::Observing::ClassMethods.notify_observers for more - # information. - def notify_observers(method, *extra_args) - self.class.notify_observers(method, self, *extra_args) - end - end - - # == Active \Model Observers - # - # Observer classes respond to life cycle callbacks to implement trigger-like - # behavior outside the original class. This is a great way to reduce the - # clutter that normally comes when the model class is burdened with - # functionality that doesn't pertain to the core responsibility of the - # class. - # - # class CommentObserver < ActiveModel::Observer - # def after_save(comment) - # Notifications.comment('admin@do.com', 'New comment was posted', comment).deliver - # end - # end - # - # This Observer sends an email when a <tt>Comment#save</tt> is finished. - # - # class ContactObserver < ActiveModel::Observer - # def after_create(contact) - # contact.logger.info('New contact added!') - # end - # - # def after_destroy(contact) - # contact.logger.warn("Contact with an id of #{contact.id} was destroyed!") - # end - # end - # - # This Observer uses logger to log when specific callbacks are triggered. - # - # == \Observing a class that can't be inferred - # - # Observers will by default be mapped to the class with which they share a - # name. So <tt>CommentObserver</tt> will be tied to observing <tt>Comment</tt>, - # <tt>ProductManagerObserver</tt> to <tt>ProductManager</tt>, and so on. If - # you want to name your observer differently than the class you're interested - # in observing, you can use the <tt>Observer.observe</tt> class method which - # takes either the concrete class (<tt>Product</tt>) or a symbol for that - # class (<tt>:product</tt>): - # - # class AuditObserver < ActiveModel::Observer - # observe :account - # - # def after_update(account) - # AuditTrail.new(account, 'UPDATED') - # end - # end - # - # If the audit observer needs to watch more than one kind of object, this can - # be specified with multiple arguments: - # - # class AuditObserver < ActiveModel::Observer - # observe :account, :balance - # - # def after_update(record) - # AuditTrail.new(record, 'UPDATED') - # end - # end - # - # The <tt>AuditObserver</tt> will now act on both updates to <tt>Account</tt> - # and <tt>Balance</tt> by treating them both as records. - # - # If you're using an Observer in a Rails application with Active Record, be - # sure to read about the necessary configuration in the documentation for - # ActiveRecord::Observer. - class Observer - include Singleton - extend ActiveSupport::DescendantsTracker - - class << self - # Attaches the observer to the supplied model classes. - # - # class AuditObserver < ActiveModel::Observer - # observe :account, :balance - # end - # - # AuditObserver.observed_classes # => [Account, Balance] - def observe(*models) - models.flatten! - models.collect! { |model| model.respond_to?(:to_sym) ? model.to_s.camelize.constantize : model } - singleton_class.redefine_method(:observed_classes) { models } - end - - # Returns an array of Classes to observe. - # - # AccountObserver.observed_classes # => [Account] - # - # You can override this instead of using the +observe+ helper. - # - # class AuditObserver < ActiveModel::Observer - # def self.observed_classes - # [Account, Balance] - # end - # end - def observed_classes - Array(observed_class) - end - - # Returns the class observed by default. It's inferred from the observer's - # class name. - # - # PersonObserver.observed_class # => Person - # AccountObserver.observed_class # => Account - def observed_class - name[/(.*)Observer/, 1].try :constantize - end - end - - # Start observing the declared classes and their subclasses. - # Called automatically by the instance method. - def initialize #:nodoc: - observed_classes.each { |klass| add_observer!(klass) } - end - - def observed_classes #:nodoc: - self.class.observed_classes - end - - # Send observed_method(object) if the method exists and - # the observer is enabled for the given object's class. - def update(observed_method, object, *extra_args, &block) #:nodoc: - return if !respond_to?(observed_method) || disabled_for?(object) - send(observed_method, object, *extra_args, &block) - end - - # Special method sent by the observed class when it is inherited. - # Passes the new subclass. - def observed_class_inherited(subclass) #:nodoc: - self.class.observe(observed_classes + [subclass]) - add_observer!(subclass) - end - - protected - def add_observer!(klass) #:nodoc: - klass.add_observer(self) - end - - # Returns true if notifications are disabled for this object. - def disabled_for?(object) #:nodoc: - klass = object.class - return false unless klass.respond_to?(:observers) - klass.observers.disabled_for?(self) - end - end -end diff --git a/activemodel/lib/active_model/serializers/xml.rb b/activemodel/lib/active_model/serializers/xml.rb index fb6093cce5..4a17a63e20 100755 --- a/activemodel/lib/active_model/serializers/xml.rb +++ b/activemodel/lib/active_model/serializers/xml.rb @@ -149,7 +149,12 @@ module ActiveModel end else merged_options[:root] = association.to_s - records.to_xml(merged_options) + + unless records.class.to_s.underscore == association.to_s + merged_options[:type] = records.class.name + end + + records.to_xml merged_options end end diff --git a/activemodel/lib/active_model/validations.rb b/activemodel/lib/active_model/validations.rb index 2524b8d065..2db4a25f61 100644 --- a/activemodel/lib/active_model/validations.rb +++ b/activemodel/lib/active_model/validations.rb @@ -1,9 +1,6 @@ require 'active_support/core_ext/array/extract_options' require 'active_support/core_ext/hash/keys' require 'active_support/core_ext/hash/except' -require 'active_model/errors' -require 'active_model/validations/callbacks' -require 'active_model/validator' module ActiveModel diff --git a/activemodel/lib/active_model/validations/callbacks.rb b/activemodel/lib/active_model/validations/callbacks.rb index a8fb4fdfc2..e28ad2841b 100644 --- a/activemodel/lib/active_model/validations/callbacks.rb +++ b/activemodel/lib/active_model/validations/callbacks.rb @@ -1,5 +1,3 @@ -require 'active_support/callbacks' - module ActiveModel module Validations # == Active \Model Validation Callbacks @@ -85,8 +83,8 @@ module ActiveModel # person = Person.new # person.name = '' # person.valid? # => false - # person.status # => false - # person.name = 'bob' + # person.status # => false + # person.name = 'bob' # person.valid? # => true # person.status # => true def after_validation(*args, &block) diff --git a/activemodel/lib/active_model/validations/length.rb b/activemodel/lib/active_model/validations/length.rb index 70ef589cd7..675fb5f1e5 100644 --- a/activemodel/lib/active_model/validations/length.rb +++ b/activemodel/lib/active_model/validations/length.rb @@ -14,6 +14,10 @@ module ActiveModel options[:minimum], options[:maximum] = range.min, range.max end + if options[:allow_blank] == false && options[:minimum].nil? && options[:is].nil? + options[:minimum] = 1 + end + super end @@ -40,7 +44,10 @@ module ActiveModel CHECKS.each do |key, validity_check| next unless check_value = options[key] - next if value_length.send(validity_check, check_value) + + if !value.nil? || skip_nil_check?(key) + next if value_length.send(validity_check, check_value) + end errors_options[:count] = check_value @@ -58,6 +65,10 @@ module ActiveModel options[:tokenizer].call(value) end || value end + + def skip_nil_check?(key) + key == :maximum && options[:allow_nil].nil? && options[:allow_blank].nil? + end end module HelperMethods @@ -79,7 +90,8 @@ module ActiveModel # # Configuration options: # * <tt>:minimum</tt> - The minimum size of the attribute. - # * <tt>:maximum</tt> - The maximum size of the attribute. + # * <tt>:maximum</tt> - The maximum size of the attribute. Allows +nil+ by + # default if not used with :minimum. # * <tt>:is</tt> - The exact size of the attribute. # * <tt>:within</tt> - A range specifying the minimum and maximum size of # the attribute. diff --git a/activemodel/test/cases/observer_array_test.rb b/activemodel/test/cases/observer_array_test.rb deleted file mode 100644 index fc5f18008b..0000000000 --- a/activemodel/test/cases/observer_array_test.rb +++ /dev/null @@ -1,220 +0,0 @@ -require 'cases/helper' -require 'models/observers' - -class ObserverArrayTest < ActiveModel::TestCase - def teardown - ORM.observers.enable :all - Budget.observers.enable :all - Widget.observers.enable :all - end - - def assert_observer_notified(model_class, observer_class) - observer_class.instance.before_save_invocations.clear - model_instance = model_class.new - model_instance.save - assert_equal [model_instance], observer_class.instance.before_save_invocations - end - - def assert_observer_not_notified(model_class, observer_class) - observer_class.instance.before_save_invocations.clear - model_instance = model_class.new - model_instance.save - assert_equal [], observer_class.instance.before_save_invocations - end - - test "all observers are enabled by default" do - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can disable individual observers using a class constant" do - ORM.observers.disable WidgetObserver - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can enable individual observers using a class constant" do - ORM.observers.disable :all - ORM.observers.enable AuditTrail - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can disable individual observers using a symbol" do - ORM.observers.disable :budget_observer - - assert_observer_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can enable individual observers using a symbol" do - ORM.observers.disable :all - ORM.observers.enable :audit_trail - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can disable multiple observers at a time" do - ORM.observers.disable :widget_observer, :budget_observer - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can enable multiple observers at a time" do - ORM.observers.disable :all - ORM.observers.enable :widget_observer, :budget_observer - - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_not_notified Budget, AuditTrail - end - - test "can disable all observers using :all" do - ORM.observers.disable :all - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_not_notified Budget, AuditTrail - end - - test "can enable all observers using :all" do - ORM.observers.disable :all - ORM.observers.enable :all - - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can disable observers on individual models without affecting those observers on other models" do - Widget.observers.disable :all - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can enable observers on individual models without affecting those observers on other models" do - ORM.observers.disable :all - Budget.observers.enable AuditTrail - - assert_observer_not_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can disable observers for the duration of a block" do - yielded = false - ORM.observers.disable :budget_observer do - yielded = true - assert_observer_notified Widget, WidgetObserver - assert_observer_not_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - assert yielded - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "can enable observers for the duration of a block" do - yielded = false - Widget.observers.disable :all - - Widget.observers.enable :all do - yielded = true - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - assert yielded - assert_observer_not_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "raises an appropriate error when a developer accidentally enables or disables the wrong class (i.e. Widget instead of WidgetObserver)" do - assert_raise ArgumentError do - ORM.observers.enable :widget - end - - assert_raise ArgumentError do - ORM.observers.enable Widget - end - - assert_raise ArgumentError do - ORM.observers.disable :widget - end - - assert_raise ArgumentError do - ORM.observers.disable Widget - end - end - - test "allows #enable at the superclass level to override #disable at the subclass level when called last" do - Widget.observers.disable :all - ORM.observers.enable :all - - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - test "allows #disable at the superclass level to override #enable at the subclass level when called last" do - Budget.observers.enable :audit_trail - ORM.observers.disable :audit_trail - - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_not_notified Budget, AuditTrail - end - - test "can use the block form at different levels of the hierarchy" do - yielded = false - Widget.observers.disable :all - - ORM.observers.enable :all do - yielded = true - assert_observer_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end - - assert yielded - assert_observer_not_notified Widget, WidgetObserver - assert_observer_notified Budget, BudgetObserver - assert_observer_not_notified Widget, AuditTrail - assert_observer_notified Budget, AuditTrail - end -end - diff --git a/activemodel/test/cases/observing_test.rb b/activemodel/test/cases/observing_test.rb deleted file mode 100644 index ade6026602..0000000000 --- a/activemodel/test/cases/observing_test.rb +++ /dev/null @@ -1,181 +0,0 @@ -require 'cases/helper' - -class ObservedModel - include ActiveModel::Observing - - class Observer - end -end - -class FooObserver < ActiveModel::Observer - class << self - public :new - end - - attr_accessor :stub - - def on_spec(record, *args) - stub.event_with(record, *args) if stub - end - - def around_save(record) - yield :in_around_save - end -end - -class Foo - include ActiveModel::Observing -end - -class ObservingTest < ActiveModel::TestCase - def setup - ObservedModel.observers.clear - end - - test "initializes model with no cached observers" do - assert ObservedModel.observers.empty?, "Not empty: #{ObservedModel.observers.inspect}" - end - - test "stores cached observers in an array" do - ObservedModel.observers << :foo - assert ObservedModel.observers.include?(:foo), ":foo not in #{ObservedModel.observers.inspect}" - end - - test "flattens array of assigned cached observers" do - ObservedModel.observers = [[:foo], :bar] - assert ObservedModel.observers.include?(:foo), ":foo not in #{ObservedModel.observers.inspect}" - assert ObservedModel.observers.include?(:bar), ":bar not in #{ObservedModel.observers.inspect}" - end - - test "uses an ObserverArray so observers can be disabled" do - ObservedModel.observers = [:foo, :bar] - assert ObservedModel.observers.is_a?(ActiveModel::ObserverArray) - end - - test "instantiates observer names passed as strings" do - ObservedModel.observers << 'foo_observer' - FooObserver.expects(:instance) - ObservedModel.instantiate_observers - end - - test "instantiates observer names passed as symbols" do - ObservedModel.observers << :foo_observer - FooObserver.expects(:instance) - ObservedModel.instantiate_observers - end - - test "instantiates observer classes" do - ObservedModel.observers << ObservedModel::Observer - ObservedModel::Observer.expects(:instance) - ObservedModel.instantiate_observers - end - - test "raises an appropriate error when a developer accidentally adds the wrong class (i.e. Widget instead of WidgetObserver)" do - assert_raise ArgumentError do - ObservedModel.observers = ['string'] - ObservedModel.instantiate_observers - end - assert_raise ArgumentError do - ObservedModel.observers = [:string] - ObservedModel.instantiate_observers - end - assert_raise ArgumentError do - ObservedModel.observers = [String] - ObservedModel.instantiate_observers - end - end - - test "passes observers to subclasses" do - FooObserver.instance - bar = Class.new(Foo) - assert_equal Foo.observers_count, bar.observers_count - end -end - -class ObserverTest < ActiveModel::TestCase - def setup - ObservedModel.observers = :foo_observer - FooObserver.singleton_class.instance_eval do - alias_method :original_observed_classes, :observed_classes - end - end - - def teardown - FooObserver.singleton_class.instance_eval do - undef_method :observed_classes - alias_method :observed_classes, :original_observed_classes - end - end - - test "guesses implicit observable model name" do - assert_equal Foo, FooObserver.observed_class - end - - test "tracks implicit observable models" do - instance = FooObserver.new - assert_equal [Foo], instance.observed_classes - end - - test "tracks explicit observed model class" do - FooObserver.observe ObservedModel - instance = FooObserver.new - assert_equal [ObservedModel], instance.observed_classes - end - - test "tracks explicit observed model as string" do - FooObserver.observe 'observed_model' - instance = FooObserver.new - assert_equal [ObservedModel], instance.observed_classes - end - - test "tracks explicit observed model as symbol" do - FooObserver.observe :observed_model - instance = FooObserver.new - assert_equal [ObservedModel], instance.observed_classes - end - - test "calls existing observer event" do - foo = Foo.new - FooObserver.instance.stub = stub - FooObserver.instance.stub.expects(:event_with).with(foo) - Foo.notify_observers(:on_spec, foo) - end - - test "calls existing observer event from the instance" do - foo = Foo.new - FooObserver.instance.stub = stub - FooObserver.instance.stub.expects(:event_with).with(foo) - foo.notify_observers(:on_spec) - end - - test "passes extra arguments" do - foo = Foo.new - FooObserver.instance.stub = stub - FooObserver.instance.stub.expects(:event_with).with(foo, :bar) - Foo.send(:notify_observers, :on_spec, foo, :bar) - end - - test "skips nonexistent observer event" do - foo = Foo.new - Foo.notify_observers(:whatever, foo) - end - - test "update passes a block on to the observer" do - yielded_value = nil - FooObserver.instance.update(:around_save, Foo.new) do |val| - yielded_value = val - end - assert_equal :in_around_save, yielded_value - end - - test "observe redefines observed_classes class method" do - class BarObserver < ActiveModel::Observer - observe :foo - end - - assert_equal [Foo], BarObserver.observed_classes - - BarObserver.observe(ObservedModel) - assert_equal [ObservedModel], BarObserver.observed_classes - end -end diff --git a/activemodel/test/cases/railtie_test.rb b/activemodel/test/cases/railtie_test.rb index f89a288f8f..a0cd1402b1 100644 --- a/activemodel/test/cases/railtie_test.rb +++ b/activemodel/test/cases/railtie_test.rb @@ -5,10 +5,11 @@ class RailtieTest < ActiveModel::TestCase include ActiveSupport::Testing::Isolation def setup - require 'rails/all' + require 'active_model/railtie' - @app ||= Class.new(::Rails::Application).tap do |app| - app.config.eager_load = false + @app ||= Class.new(::Rails::Application) do + config.eager_load = false + config.logger = Logger.new(STDOUT) end end diff --git a/activemodel/test/cases/serializers/json_serialization_test.rb b/activemodel/test/cases/serializers/json_serialization_test.rb index fd4d068354..9134c4980c 100644 --- a/activemodel/test/cases/serializers/json_serialization_test.rb +++ b/activemodel/test/cases/serializers/json_serialization_test.rb @@ -157,11 +157,8 @@ class JsonSerializationTest < ActiveModel::TestCase test "as_json should keep the default order in the hash" do json = @contact.as_json - keys = json.keys - %w(name age created_at awesome preferences).each_with_index do |field, index| - assert_equal keys.index(field), index - end + assert_equal %w(name age created_at awesome preferences), json.keys end test "from_json should work without a root (class attribute)" do diff --git a/activemodel/test/cases/serializers/xml_serialization_test.rb b/activemodel/test/cases/serializers/xml_serialization_test.rb index 90ddf8ff0c..99a9c1fe33 100755 --- a/activemodel/test/cases/serializers/xml_serialization_test.rb +++ b/activemodel/test/cases/serializers/xml_serialization_test.rb @@ -6,12 +6,12 @@ require 'ostruct' class Contact include ActiveModel::Serializers::Xml - attr_accessor :address, :friends + attr_accessor :address, :friends, :contact remove_method :attributes if method_defined?(:attributes) def attributes - instance_values.except("address", "friends") + instance_values.except("address", "friends", "contact") end end @@ -56,6 +56,9 @@ class XmlSerializationTest < ActiveModel::TestCase @contact.address.zip = 11111 @contact.address.apt_number = 35 @contact.friends = [Contact.new, Contact.new] + @related_contact = SerializableContact.new + @related_contact.name = "related" + @contact.contact = @related_contact end test "should serialize default root" do @@ -256,4 +259,9 @@ class XmlSerializationTest < ActiveModel::TestCase assert_match %r{<address>}, xml assert_match %r{<apt-number type="integer">}, xml end + + test "association with sti" do + xml = @contact.to_xml(include: :contact) + assert xml.include?(%(<contact type="SerializableContact">)) + end end diff --git a/activemodel/test/cases/validations/length_validation_test.rb b/activemodel/test/cases/validations/length_validation_test.rb index 113bfd6337..1a40ca8efc 100644 --- a/activemodel/test/cases/validations/length_validation_test.rb +++ b/activemodel/test/cases/validations/length_validation_test.rb @@ -375,4 +375,43 @@ class LengthValidationTest < ActiveModel::TestCase t.author_name = "A very long author name that should still be valid." * 100 assert t.valid? end + + def test_validates_length_of_using_maximum_should_not_allow_nil_when_nil_not_allowed + Topic.validates_length_of :title, :maximum => 10, :allow_nil => false + t = Topic.new + assert t.invalid? + end + + def test_validates_length_of_using_maximum_should_not_allow_nil_and_empty_string_when_blank_not_allowed + Topic.validates_length_of :title, :maximum => 10, :allow_blank => false + t = Topic.new + assert t.invalid? + + t.title = "" + assert t.invalid? + end + + def test_validates_length_of_using_both_minimum_and_maximum_should_not_allow_nil + Topic.validates_length_of :title, :minimum => 5, :maximum => 10 + t = Topic.new + assert t.invalid? + end + + def test_validates_length_of_using_minimum_0_should_not_allow_nil + Topic.validates_length_of :title, :minimum => 0 + t = Topic.new + assert t.invalid? + + t.title = "" + assert t.valid? + end + + def test_validates_length_of_using_is_0_should_not_allow_nil + Topic.validates_length_of :title, :is => 0 + t = Topic.new + assert t.invalid? + + t.title = "" + assert t.valid? + end end diff --git a/activemodel/test/models/observers.rb b/activemodel/test/models/observers.rb deleted file mode 100644 index 3729b3435e..0000000000 --- a/activemodel/test/models/observers.rb +++ /dev/null @@ -1,27 +0,0 @@ -class ORM - include ActiveModel::Observing - - def save - notify_observers :before_save - end - - class Observer < ActiveModel::Observer - def before_save_invocations - @before_save_invocations ||= [] - end - - def before_save(record) - before_save_invocations << record - end - end -end - -class Widget < ORM; end -class Budget < ORM; end -class WidgetObserver < ORM::Observer; end -class BudgetObserver < ORM::Observer; end -class AuditTrail < ORM::Observer - observe :widget, :budget -end - -ORM.instantiate_observers diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1a34ed441f..130d0f05d2 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,5 +1,26 @@ ## Rails 4.0.0 (unreleased) ## +* Add STI support to init and building associations. + Allows you to do BaseClass.new(:type => "SubClass") as well as + parent.children.build(:type => "SubClass") or parent.build_child + to initialize an STI subclass. Ensures that the class name is a + valid class and that it is in the ancestors of the super class + that the association is expecting. + + *Jason Rush* + +* Observers was extracted from Active Record as `rails-observers` gem. + + *Rafael Mendonça França* + +* Ensure that associations take a symbol argument. *Steve Klabnik* + +* Fix dirty attribute checks for `TimeZoneConversion` with nil and blank + datetime attributes. Setting a nil datetime to a blank string should not + result in a change being flagged. Fix #8310 + + *Alisdair McDiarmid* + * Prevent mass assignment to the type column of polymorphic associations when using `build` Fix #8265 @@ -452,11 +473,11 @@ *kennyj* -* Use inversed parent for first and last child of has_many association. +* Use inversed parent for first and last child of `has_many` association. *Ravil Bayramgalin* -* Fix Column.microseconds and Column.fast_string_to_date to avoid converting +* Fix `Column.microseconds` and `Column.fast_string_to_time` to avoid converting timestamp seconds to a float, since it occasionally results in inaccuracies with microsecond-precision times. Fixes #7352. diff --git a/activerecord/README.rdoc b/activerecord/README.rdoc index cc8942809c..9fc6785d99 100644 --- a/activerecord/README.rdoc +++ b/activerecord/README.rdoc @@ -80,17 +80,6 @@ A short rundown of some of the major features: {Learn more}[link:classes/ActiveRecord/Callbacks.html] -* Observers that react to changes in a model. - - class CommentObserver < ActiveRecord::Observer - def after_create(comment) # is called just after Comment#save - CommentMailer.new_comment_email('david@loudthinking.com', comment).deliver - end - end - - {Learn more}[link:classes/ActiveRecord/Observer.html] - - * Inheritance hierarchies. class Company < ActiveRecord::Base; end diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index 45122539f1..822da84d19 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -45,7 +45,6 @@ module ActiveRecord autoload :Migrator, 'active_record/migration' autoload :ModelSchema autoload :NestedAttributes - autoload :Observer autoload :Persistence autoload :QueryCache autoload :Querying diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 1df876bf62..5c37f42794 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -13,6 +13,8 @@ module ActiveRecord::Associations::Builder end def initialize(model, name, scope, options) + raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) + @model = model @name = name diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index 1548e68cea..832b963052 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -34,7 +34,7 @@ module ActiveRecord reload end - CollectionProxy.new(self) + CollectionProxy.new(klass, self) end # Implements the writer method, e.g. foo.items= for Foo.has_many :items diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 07a5d07256..7f9628499c 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -30,9 +30,9 @@ module ActiveRecord class CollectionProxy < Relation delegate(*(ActiveRecord::Calculations.public_instance_methods - [:count]), to: :scope) - def initialize(association) #:nodoc: + def initialize(klass, association) #:nodoc: @association = association - super association.klass, association.klass.arel_table + super klass, klass.arel_table merge! association.scope(nullify: false) end diff --git a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb index 427c61079a..47a8b576c0 100644 --- a/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb +++ b/activerecord/lib/active_record/attribute_methods/time_zone_conversion.rb @@ -34,6 +34,7 @@ module ActiveRecord if create_time_zone_conversion_attribute?(attr_name, columns_hash[attr_name]) method_body, line = <<-EOV, __LINE__ + 1 def #{attr_name}=(original_time) + original_time = nil if original_time.blank? time = original_time unless time.acts_like?(:time) time = time.is_a?(String) ? Time.zone.parse(time) : time.to_time rescue time diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 5eacb8f143..aab832c2f7 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -13,6 +13,7 @@ require 'active_support/core_ext/string/behavior' require 'active_support/core_ext/kernel/singleton_class' require 'active_support/core_ext/module/introspection' require 'active_support/core_ext/object/duplicable' +require 'active_support/core_ext/class/subclasses' require 'arel' require 'active_record/errors' require 'active_record/log_subscriber' @@ -320,7 +321,6 @@ module ActiveRecord #:nodoc: # So it's possible to assign a logger to the class through <tt>Base.logger=</tt> which will then be used by all # instances in the current object space. class Base - extend ActiveModel::Observing::ClassMethods extend ActiveModel::Naming extend ActiveSupport::Benchmarkable @@ -348,7 +348,6 @@ module ActiveRecord #:nodoc: include Locking::Pessimistic include AttributeMethods include Callbacks - include ActiveModel::Observing include Timestamp include Associations include ActiveModel::SecurePassword diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index db0db272a6..b5a8011ca4 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -490,6 +490,9 @@ module ActiveRecord # determine the connection pool that they should use. class ConnectionHandler def initialize + # These hashes are keyed by klass.name, NOT klass. Keying them by klass + # alone would lead to memory leaks in development mode as all previous + # instances of the class would stay in memory. @owner_to_pool = Hash.new { |h,k| h[k] = {} } @class_to_pool = Hash.new { |h,k| h[k] = {} } end @@ -508,7 +511,7 @@ module ActiveRecord def establish_connection(owner, spec) @class_to_pool.clear - owner_to_pool[owner] = ConnectionAdapters::ConnectionPool.new(spec) + owner_to_pool[owner.name] = ConnectionAdapters::ConnectionPool.new(spec) end # Returns true if there are any active connections among the connection @@ -554,7 +557,7 @@ module ActiveRecord # can be used as an argument for establish_connection, for easily # re-establishing the connection. def remove_connection(owner) - if pool = owner_to_pool.delete(owner) + if pool = owner_to_pool.delete(owner.name) @class_to_pool.clear pool.automatic_reconnect = false pool.disconnect! @@ -572,13 +575,13 @@ module ActiveRecord # but that's ok since the nil case is not the common one that we wish to optimise # for. def retrieve_connection_pool(klass) - class_to_pool[klass] ||= begin + class_to_pool[klass.name] ||= begin until pool = pool_for(klass) klass = klass.superclass break unless klass <= Base end - class_to_pool[klass] = pool + class_to_pool[klass.name] = pool end end @@ -593,21 +596,21 @@ module ActiveRecord end def pool_for(owner) - owner_to_pool.fetch(owner) { + owner_to_pool.fetch(owner.name) { if ancestor_pool = pool_from_any_process_for(owner) # A connection was established in an ancestor process that must have # subsequently forked. We can't reuse the connection, but we can copy # the specification and establish a new connection with it. establish_connection owner, ancestor_pool.spec else - owner_to_pool[owner] = nil + owner_to_pool[owner.name] = nil end } end def pool_from_any_process_for(owner) - owner_to_pool = @owner_to_pool.values.find { |v| v[owner] } - owner_to_pool && owner_to_pool[owner] + owner_to_pool = @owner_to_pool.values.find { |v| v[owner.name] } + owner_to_pool && owner_to_pool[owner.name] end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 4f3eebce7d..c3d15ca929 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -287,7 +287,7 @@ module ActiveRecord # Inserts the given fixture into the table. Overridden in adapters that require # something beyond a simple insert (eg. Oracle). def insert_fixture(fixture, table_name) - columns = Hash[columns(table_name).map { |c| [c.name, c] }] + columns = schema_cache.columns_hash(table_name) key_list = [] value_list = fixture.map do |name, value| diff --git a/activerecord/lib/active_record/connection_adapters/schema_cache.rb b/activerecord/lib/active_record/connection_adapters/schema_cache.rb index aad1f9a7ef..5839d1d3b4 100644 --- a/activerecord/lib/active_record/connection_adapters/schema_cache.rb +++ b/activerecord/lib/active_record/connection_adapters/schema_cache.rb @@ -1,7 +1,7 @@ module ActiveRecord module ConnectionAdapters class SchemaCache - attr_reader :columns, :columns_hash, :primary_keys, :tables, :version + attr_reader :primary_keys, :tables, :version attr_accessor :connection def initialize(conn) @@ -30,6 +30,25 @@ module ActiveRecord end end + # Get the columns for a table + def columns(table = nil) + if table + @columns[table] + else + @columns + end + end + + # Get the columns for a table as a hash, key is the column name + # value is the column object. + def columns_hash(table = nil) + if table + @columns_hash[table] + else + @columns_hash + end + end + # Clears out internal caches def clear! @columns.clear diff --git a/activerecord/lib/active_record/explain.rb b/activerecord/lib/active_record/explain.rb index af772996f1..70683eb731 100644 --- a/activerecord/lib/active_record/explain.rb +++ b/activerecord/lib/active_record/explain.rb @@ -6,11 +6,12 @@ module ActiveRecord base.mattr_accessor :auto_explain_threshold_in_seconds, instance_accessor: false end - # If auto explain is enabled, this method triggers EXPLAIN logging for the - # queries triggered by the block if it takes more than the threshold as a - # whole. That is, the threshold is not checked against each individual - # query, but against the duration of the entire block. This approach is - # convenient for relations. + # If the database adapter supports explain and auto explain is enabled, + # this method triggers EXPLAIN logging for the queries triggered by the + # block if it takes more than the threshold as a whole. That is, the + # threshold is not checked against each individual query, but against the + # duration of the entire block. This approach is convenient for relations. + # # The available_queries_for_explain thread variable collects the queries # to be explained. If the value is nil, it means queries are not being @@ -21,7 +22,7 @@ module ActiveRecord threshold = auto_explain_threshold_in_seconds current = Thread.current - if threshold && current[:available_queries_for_explain].nil? + if connection.supports_explain? && threshold && current[:available_queries_for_explain].nil? begin queries = current[:available_queries_for_explain] = [] start = Time.now diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 7922bbcfa0..c5ad14722e 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -872,11 +872,7 @@ module ActiveRecord end def teardown_fixtures - return unless defined?(ActiveRecord) && !ActiveRecord::Base.configurations.blank? - - unless run_in_transaction? - ActiveRecord::FixtureSet.reset_cache - end + return if ActiveRecord::Base.configurations.blank? # Rollback changes if a transaction is active. if run_in_transaction? @@ -884,7 +880,10 @@ module ActiveRecord connection.rollback_transaction if connection.transaction_open? end @fixture_connections.clear + else + ActiveRecord::FixtureSet.reset_cache end + ActiveRecord::Base.clear_active_connections! end diff --git a/activerecord/lib/active_record/inheritance.rb b/activerecord/lib/active_record/inheritance.rb index a448fa1f5c..6ab67fdece 100644 --- a/activerecord/lib/active_record/inheritance.rb +++ b/activerecord/lib/active_record/inheritance.rb @@ -9,6 +9,19 @@ module ActiveRecord end module ClassMethods + # Determines if one of the attributes passed in is the inheritance column, + # and if the inheritance column is attr accessible, it initializes an + # instance of the given subclass instead of the base class + def new(*args, &block) + if (attrs = args.first).is_a?(Hash) + if subclass = subclass_from_attrs(attrs) + return subclass.new(*args, &block) + end + end + # Delegate to the original .new + super + end + # True if this isn't a concrete subclass needing a STI type condition. def descends_from_active_record? if self == Base @@ -79,15 +92,6 @@ module ActiveRecord store_full_sti_class ? name : name.demodulize end - # Finder methods must instantiate through this method to work with the - # single-table inheritance model that makes it possible to create - # objects of different types from the same table. - def instantiate(record, column_types = {}) - sti_class = find_sti_class(record[inheritance_column]) - column_types = sti_class.decorate_columns(column_types) - sti_class.allocate.init_with('attributes' => record, 'column_types' => column_types) - end - protected # Returns the class type of the record using the current module as a prefix. So descendants of @@ -119,24 +123,33 @@ module ActiveRecord private + # Called by +instantiate+ to decide which class to use for a new + # record instance. For single-table inheritance, we check the record + # for a +type+ column and return the corresponding class. + def discriminate_class_for_record(record) + if using_single_table_inheritance?(record) + find_sti_class(record[inheritance_column]) + else + super + end + end + + def using_single_table_inheritance?(record) + record[inheritance_column].present? && columns_hash.include?(inheritance_column) + end + def find_sti_class(type_name) - if type_name.blank? || !columns_hash.include?(inheritance_column) - self + if store_full_sti_class + ActiveSupport::Dependencies.constantize(type_name) else - begin - if store_full_sti_class - ActiveSupport::Dependencies.constantize(type_name) - else - compute_type(type_name) - end - rescue NameError - raise SubclassNotFound, - "The single-table inheritance mechanism failed to locate the subclass: '#{type_name}'. " + - "This error is raised because the column '#{inheritance_column}' is reserved for storing the class in case of inheritance. " + - "Please rename this column if you didn't intend it to be used for storing the inheritance class " + - "or overwrite #{name}.inheritance_column to use another column for that information." - end + compute_type(type_name) end + rescue NameError + raise SubclassNotFound, + "The single-table inheritance mechanism failed to locate the subclass: '#{type_name}'. " + + "This error is raised because the column '#{inheritance_column}' is reserved for storing the class in case of inheritance. " + + "Please rename this column if you didn't intend it to be used for storing the inheritance class " + + "or overwrite #{name}.inheritance_column to use another column for that information." end def type_condition(table = arel_table) @@ -145,6 +158,19 @@ module ActiveRecord sti_column.in(sti_names) end + + # Detect the subclass from the inheritance column of attrs. If the inheritance column value + # is not self or a valid subclass, raises ActiveRecord::SubclassNotFound + # If this is a StrongParameters hash, and access to inheritance_column is not permitted, + # this will ignore the inheritance column and return nil + def subclass_from_attrs(attrs) + subclass_name = attrs.with_indifferent_access[inheritance_column] + return nil if subclass_name.blank? || subclass_name == self.name + unless subclass = subclasses.detect { |sub| sub.name == subclass_name } + raise ActiveRecord::SubclassNotFound.new("Invalid single-table inheritance type: #{subclass_name} is not a subclass of #{name}") + end + subclass + end end private diff --git a/activerecord/lib/active_record/observer.rb b/activerecord/lib/active_record/observer.rb deleted file mode 100644 index 6b2f6f98a5..0000000000 --- a/activerecord/lib/active_record/observer.rb +++ /dev/null @@ -1,126 +0,0 @@ - -module ActiveRecord - # = Active Record Observer - # - # Observer classes respond to life cycle callbacks to implement trigger-like - # behavior outside the original class. This is a great way to reduce the - # clutter that normally comes when the model class is burdened with - # functionality that doesn't pertain to the core responsibility of the - # class. Example: - # - # class CommentObserver < ActiveRecord::Observer - # def after_save(comment) - # Notifications.comment("admin@do.com", "New comment was posted", comment).deliver - # end - # end - # - # This Observer sends an email when a Comment#save is finished. - # - # class ContactObserver < ActiveRecord::Observer - # def after_create(contact) - # contact.logger.info('New contact added!') - # end - # - # def after_destroy(contact) - # contact.logger.warn("Contact with an id of #{contact.id} was destroyed!") - # end - # end - # - # This Observer uses logger to log when specific callbacks are triggered. - # - # == Observing a class that can't be inferred - # - # Observers will by default be mapped to the class with which they share a name. So CommentObserver will - # be tied to observing Comment, ProductManagerObserver to ProductManager, and so on. If you want to name your observer - # differently than the class you're interested in observing, you can use the Observer.observe class method which takes - # either the concrete class (Product) or a symbol for that class (:product): - # - # class AuditObserver < ActiveRecord::Observer - # observe :account - # - # def after_update(account) - # AuditTrail.new(account, "UPDATED") - # end - # end - # - # If the audit observer needs to watch more than one kind of object, this can be specified with multiple arguments: - # - # class AuditObserver < ActiveRecord::Observer - # observe :account, :balance - # - # def after_update(record) - # AuditTrail.new(record, "UPDATED") - # end - # end - # - # The AuditObserver will now act on both updates to Account and Balance by treating them both as records. - # - # == Available callback methods - # - # The observer can implement callback methods for each of the methods described in the Callbacks module. - # - # == Storing Observers in Rails - # - # If you're using Active Record within Rails, observer classes are usually stored in app/models with the - # naming convention of app/models/audit_observer.rb. - # - # == Configuration - # - # In order to activate an observer, list it in the <tt>config.active_record.observers</tt> configuration - # setting in your <tt>config/application.rb</tt> file. - # - # config.active_record.observers = :comment_observer, :signup_observer - # - # Observers will not be invoked unless you define these in your application configuration. - # - # If you are using Active Record outside Rails, activate the observers explicitly in a configuration or - # environment file: - # - # ActiveRecord::Base.add_observer CommentObserver.instance - # ActiveRecord::Base.add_observer SignupObserver.instance - # - # == Loading - # - # Observers register themselves in the model class they observe, since it is the class that - # notifies them of events when they occur. As a side-effect, when an observer is loaded its - # corresponding model class is loaded. - # - # Up to (and including) Rails 2.0.2 observers were instantiated between plugins and - # application initializers. Now observers are loaded after application initializers, - # so observed models can make use of extensions. - # - # If by any chance you are using observed models in the initialization you can still - # load their observers by calling <tt>ModelObserver.instance</tt> before. Observers are - # singletons and that call instantiates and registers them. - # - class Observer < ActiveModel::Observer - - protected - - def observed_classes - klasses = super - klasses + klasses.map { |klass| klass.descendants }.flatten - end - - def add_observer!(klass) - super - define_callbacks klass - end - - def define_callbacks(klass) - observer = self - observer_name = observer.class.name.underscore.gsub('/', '__') - - ActiveRecord::Callbacks::CALLBACKS.each do |callback| - next unless respond_to?(callback) - callback_meth = :"_notify_#{observer_name}_for_#{callback}" - unless klass.respond_to?(callback_meth) - klass.send(:define_method, callback_meth) do |&block| - observer.update(callback, self, &block) - end - klass.send(callback, callback_meth) - end - end - end - end -end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index eed49e17b1..94c109e72b 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -38,6 +38,32 @@ module ActiveRecord object end end + + # Given an attributes hash, +instantiate+ returns a new instance of + # the appropriate class. + # + # For example, +Post.all+ may return Comments, Messages, and Emails + # by storing the record's subclass in a +type+ attribute. By calling + # +instantiate+ instead of +new+, finder methods ensure they get new + # instances of the appropriate class for each record. + # + # See +ActiveRecord::Inheritance#discriminate_class_for_record+ to see + # how this "single-table" inheritance mapping is implemented. + def instantiate(record, column_types = {}) + klass = discriminate_class_for_record(record) + column_types = klass.decorate_columns(column_types) + klass.allocate.init_with('attributes' => record, 'column_types' => column_types) + end + + private + # Called by +instantiate+ to decide which class to use for a new + # record instance. + # + # See +ActiveRecord::Inheritance#discriminate_class_for_record+ for + # the single-table inheritance discriminator. + def discriminate_class_for_record(record) + self + end end # Returns true if this object hasn't been saved yet -- that is, a record @@ -102,7 +128,7 @@ module ActiveRecord # record's primary key, and no callbacks are executed. # # To enforce the object's +before_destroy+ and +after_destroy+ - # callbacks, Observer methods, or any <tt>:dependent</tt> association + # callbacks or any <tt>:dependent</tt> association # options, use <tt>#destroy</tt>. def delete self.class.delete(id) if persisted? diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 5464ca6066..1081b82bc6 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -115,6 +115,18 @@ module ActiveRecord See http://edgeguides.rubyonrails.org/security.html#mass-assignment for more information EOF end + + unless app.config.active_record.delete(:observers).nil? + ActiveSupport::Deprecation.warn <<-EOF.strip_heredoc, [] + Active Record Observers has been extracted out of Rails into a gem. + Please use callbacks or add `rails-observers` to your Gemfile to use observers. + + To disable this message remove the `observers` option from your + `config/application.rb` or from your initializers. + + See http://edgeguides.rubyonrails.org/4_0_release_notes.html for more information + EOF + end ensure ActiveSupport::Deprecation.behavior = old_behavior end @@ -136,6 +148,13 @@ module ActiveRecord end end + initializer "active_record.validate_explain_support" do |app| + if app.config.active_record[:auto_explain_threshold_in_seconds] && + !ActiveRecord::Base.connection.supports_explain? + warn "auto_explain_threshold_in_seconds is set but will be ignored because your adapter does not support this feature. Please unset the configuration to avoid this warning." + end + end + # Expose database runtime to controller for logging. initializer "active_record.log_runtime" do |app| require "active_record/railties/controller_runtime" @@ -161,15 +180,5 @@ module ActiveRecord path = app.paths["db"].first config.watchable_files.concat ["#{path}/schema.rb", "#{path}/structure.sql"] end - - config.after_initialize do |app| - ActiveSupport.on_load(:active_record) do - instantiate_observers - - ActionDispatch::Reloader.to_prepare do - ActiveRecord::Base.instantiate_observers - end - end - end end end diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index 0103de4cbd..bcfcb061f2 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -179,7 +179,7 @@ module ActiveRecord @collection = [:has_many, :has_and_belongs_to_many].include?(macro) end - # Returns a new, unsaved instance of the associated class. +options+ will + # Returns a new, unsaved instance of the associated class. +attributes+ will # be passed to the class's constructor. def build_association(attributes, &block) klass.new(attributes, &block) diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index 3ee55c580e..0df895eb67 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -31,6 +31,14 @@ module ActiveRecord @default_scoped = false end + def initialize_copy(other) + # This method is a hot spot, so for now, use Hash[] to dup the hash. + # https://bugs.ruby-lang.org/issues/7166 + @values = Hash[@values] + @values[:bind] = @values[:bind].dup if @values.key? :bind + reset + end + def insert(values) primary_key_value = nil @@ -90,14 +98,6 @@ module ActiveRecord scoping { @klass.new(*args, &block) } end - def initialize_copy(other) - # This method is a hot spot, so for now, use Hash[] to dup the hash. - # https://bugs.ruby-lang.org/issues/7166 - @values = Hash[@values] - @values[:bind] = @values[:bind].dup if @values.key? :bind - reset - end - alias build new # Tries to create a new record with the same scoped attributes @@ -315,11 +315,9 @@ module ActiveRecord # Destroys the records matching +conditions+ by instantiating each # record and calling its +destroy+ method. Each object's callbacks are - # executed (including <tt>:dependent</tt> association options and - # +before_destroy+/+after_destroy+ Observer methods). Returns the + # executed (including <tt>:dependent</tt> association options). Returns the # collection of objects that were destroyed; each will be frozen, to - # reflect that no changes should be made (since they can't be - # persisted). + # reflect that no changes should be made (since they can't be persisted). # # Note: Instantiation, callback execution, and deletion of each # record can be time consuming when you're removing many records at @@ -419,8 +417,7 @@ module ActiveRecord # Deletes the row with a primary key matching the +id+ argument, using a # SQL +DELETE+ statement, and returns the number of rows deleted. Active # Record objects are not instantiated, so the object's callbacks are not - # executed, including any <tt>:dependent</tt> association options or - # Observer methods. + # executed, including any <tt>:dependent</tt> association options. # # You can delete multiple rows at once by passing an Array of <tt>id</tt>s. # diff --git a/activerecord/lib/active_record/relation/delegation.rb b/activerecord/lib/active_record/relation/delegation.rb index dbfa92bbbd..2184625e22 100644 --- a/activerecord/lib/active_record/relation/delegation.rb +++ b/activerecord/lib/active_record/relation/delegation.rb @@ -1,27 +1,113 @@ -require 'thread' +require 'active_support/concern' +require 'mutex_m' module ActiveRecord module Delegation # :nodoc: - # Set up common delegations for performance (avoids method_missing) + extend ActiveSupport::Concern + + # This module creates compiled delegation methods dynamically at runtime, which makes + # subsequent calls to that method faster by avoiding method_missing. The delegations + # may vary depending on the klass of a relation, so we create a subclass of Relation + # for each different klass, and the delegations are compiled into that subclass only. + delegate :to_xml, :to_yaml, :length, :collect, :map, :each, :all?, :include?, :to_ary, :to => :to_a delegate :table_name, :quoted_table_name, :primary_key, :quoted_primary_key, :connection, :columns_hash, :auto_explain_threshold_in_seconds, :to => :klass - @@delegation_mutex = Mutex.new + module ClassSpecificRelation + extend ActiveSupport::Concern - def self.delegate_to_scoped_klass(method) - if method.to_s =~ /\A[a-zA-Z_]\w*[!?]?\z/ - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - scoping { @klass.#{method}(*args, &block) } + included do + @delegation_mutex = Mutex.new + end + + module ClassMethods + def name + superclass.name + end + + def delegate_to_scoped_klass(method) + @delegation_mutex.synchronize do + return if method_defined?(method) + + if method.to_s =~ /\A[a-zA-Z_]\w*[!?]?\z/ + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + scoping { @klass.#{method}(*args, &block) } + end + RUBY + else + module_eval <<-RUBY, __FILE__, __LINE__ + 1 + def #{method}(*args, &block) + scoping { @klass.send(#{method.inspect}, *args, &block) } + end + RUBY + end end - RUBY - else - module_eval <<-RUBY, __FILE__, __LINE__ + 1 - def #{method}(*args, &block) - scoping { @klass.send(#{method.inspect}, *args, &block) } + end + + def delegate(method, opts = {}) + @delegation_mutex.synchronize do + return if method_defined?(method) + super end - RUBY + end + end + + protected + + def method_missing(method, *args, &block) + if @klass.respond_to?(method) + self.class.delegate_to_scoped_klass(method) + scoping { @klass.send(method, *args, &block) } + elsif Array.method_defined?(method) + self.class.delegate method, :to => :to_a + to_a.send(method, *args, &block) + elsif arel.respond_to?(method) + self.class.delegate method, :to => :arel + arel.send(method, *args, &block) + else + super + end + end + end + + module ClassMethods + # This hash is keyed by klass.name to avoid memory leaks in development mode + @@subclasses = Hash.new { |h, k| h[k] = {} }.extend(Mutex_m) + + def new(klass, *args) + relation = relation_class_for(klass).allocate + relation.__send__(:initialize, klass, *args) + relation + end + + # Cache the constants in @@subclasses because looking them up via const_get + # make instantiation significantly slower. + def relation_class_for(klass) + if klass && klass.name + if subclass = @@subclasses.synchronize { @@subclasses[self][klass.name] } + subclass + else + subclass = const_get("#{name.gsub('::', '_')}_#{klass.name.gsub('::', '_')}", false) + @@subclasses.synchronize { @@subclasses[self][klass.name] = subclass } + subclass + end + else + ActiveRecord::Relation + end + end + + # Check const_defined? in case another thread has already defined the constant. + # I am not sure whether this is strictly necessary. + def const_missing(name) + @@subclasses.synchronize { + if const_defined?(name) + const_get(name) + else + const_set(name, Class.new(self) { include ClassSpecificRelation }) + end + } end end @@ -35,28 +121,10 @@ module ActiveRecord def method_missing(method, *args, &block) if @klass.respond_to?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate_to_scoped_klass(method) - end - end - scoping { @klass.send(method, *args, &block) } elsif Array.method_defined?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate method, :to => :to_a - end - end - to_a.send(method, *args, &block) elsif arel.respond_to?(method) - @@delegation_mutex.synchronize do - unless ::ActiveRecord::Delegation.method_defined?(method) - ::ActiveRecord::Delegation.delegate method, :to => :arel - end - end - arel.send(method, *args, &block) else super diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index b3712b4ad6..a480ddec9e 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -66,8 +66,7 @@ module ActiveRecord args.empty? ? self : spawn.includes!(*args) end - # Like #includes, but modifies the relation in place. - def includes!(*args) + def includes!(*args) # :nodoc: args.reject! {|a| a.blank? } self.includes_values = (includes_values + args).flatten.uniq @@ -84,8 +83,7 @@ module ActiveRecord args.blank? ? self : spawn.eager_load!(*args) end - # Like #eager_load, but modifies relation in place. - def eager_load!(*args) + def eager_load!(*args) # :nodoc: self.eager_load_values += args self end @@ -98,8 +96,7 @@ module ActiveRecord args.blank? ? self : spawn.preload!(*args) end - # Like #preload, but modifies relation in place. - def preload!(*args) + def preload!(*args) # :nodoc: self.preload_values += args self end @@ -116,8 +113,7 @@ module ActiveRecord args.blank? ? self : spawn.references!(*args) end - # Like #references, but modifies relation in place. - def references!(*args) + def references!(*args) # :nodoc: args.flatten! self.references_values = (references_values + args.map!(&:to_s)).uniq @@ -162,8 +158,7 @@ module ActiveRecord end end - # Like #select, but modifies relation in place. - def select!(*fields) + def select!(*fields) # :nodoc: self.select_values += fields.flatten self end @@ -184,8 +179,7 @@ module ActiveRecord args.blank? ? self : spawn.group!(*args) end - # Like #group, but modifies relation in place. - def group!(*args) + def group!(*args) # :nodoc: args.flatten! self.group_values += args @@ -215,8 +209,7 @@ module ActiveRecord args.blank? ? self : spawn.order!(*args) end - # Like #order, but modifies relation in place. - def order!(*args) + def order!(*args) # :nodoc: args.flatten! validate_order_args args @@ -241,8 +234,7 @@ module ActiveRecord args.blank? ? self : spawn.reorder!(*args) end - # Like #reorder, but modifies relation in place. - def reorder!(*args) + def reorder!(*args) # :nodoc: args.flatten! validate_order_args args @@ -259,8 +251,7 @@ module ActiveRecord args.compact.blank? ? self : spawn.joins!(*args.flatten) end - # Like #joins, but modifies relation in place. - def joins!(*args) + def joins!(*args) # :nodoc: self.joins_values += args self end @@ -269,7 +260,7 @@ module ActiveRecord spawn.bind!(value) end - def bind!(value) + def bind!(value) # :nodoc: self.bind_values += [value] self end @@ -386,9 +377,7 @@ module ActiveRecord opts.blank? ? self : spawn.where!(opts, *rest) end - # #where! is identical to #where, except that instead of returning a new relation, it adds - # the condition to the existing relation. - def where!(opts, *rest) + def where!(opts, *rest) # :nodoc: references!(PredicateBuilder.references(opts)) if Hash === opts self.where_values += build_where(opts, rest) @@ -403,8 +392,7 @@ module ActiveRecord opts.blank? ? self : spawn.having!(opts, *rest) end - # Like #having, but modifies relation in place. - def having!(opts, *rest) + def having!(opts, *rest) # :nodoc: references!(PredicateBuilder.references(opts)) if Hash === opts self.having_values += build_where(opts, rest) @@ -420,8 +408,7 @@ module ActiveRecord spawn.limit!(value) end - # Like #limit, but modifies relation in place. - def limit!(value) + def limit!(value) # :nodoc: self.limit_value = value self end @@ -437,8 +424,7 @@ module ActiveRecord spawn.offset!(value) end - # Like #offset, but modifies relation in place. - def offset!(value) + def offset!(value) # :nodoc: self.offset_value = value self end @@ -449,8 +435,7 @@ module ActiveRecord spawn.lock!(locks) end - # Like #lock, but modifies relation in place. - def lock!(locks = true) + def lock!(locks = true) # :nodoc: case locks when String, TrueClass, NilClass self.lock_value = locks || true @@ -494,8 +479,7 @@ module ActiveRecord extending(NullRelation) end - # Like #none, but modifies relation in place. - def none! + def none! # :nodoc: extending!(NullRelation) end @@ -509,8 +493,7 @@ module ActiveRecord spawn.readonly!(value) end - # Like #readonly, but modifies relation in place. - def readonly!(value = true) + def readonly!(value = true) # :nodoc: self.readonly_value = value self end @@ -532,12 +515,7 @@ module ActiveRecord spawn.create_with!(value) end - # Like #create_with but modifies the relation in place. Raises - # +ImmutableRelation+ if the relation has already been loaded. - # - # users = User.all.create_with!(name: 'Oscar') - # users.new.name # => 'Oscar' - def create_with!(value) + def create_with!(value) # :nodoc: self.create_with_value = value ? create_with_value.merge(value) : {} self end @@ -560,7 +538,7 @@ module ActiveRecord end # Like #from, but modifies relation in place. - def from!(value, subquery_name = nil) + def from!(value, subquery_name = nil) # :nodoc: self.from_value = [value, subquery_name] self end @@ -580,7 +558,7 @@ module ActiveRecord end # Like #uniq, but modifies relation in place. - def uniq!(value = true) + def uniq!(value = true) # :nodoc: self.uniq_value = value self end @@ -629,8 +607,7 @@ module ActiveRecord end end - # Like #extending, but modifies relation in place. - def extending!(*modules, &block) + def extending!(*modules, &block) # :nodoc: modules << Module.new(&block) if block_given? self.extending_values += modules.flatten @@ -646,8 +623,7 @@ module ActiveRecord spawn.reverse_order! end - # Like #reverse_order, but modifies relation in place. - def reverse_order! + def reverse_order! # :nodoc: self.reverse_order_value = !reverse_order_value self end diff --git a/activerecord/lib/active_record/relation/spawn_methods.rb b/activerecord/lib/active_record/relation/spawn_methods.rb index 62dda542ab..352dee3826 100644 --- a/activerecord/lib/active_record/relation/spawn_methods.rb +++ b/activerecord/lib/active_record/relation/spawn_methods.rb @@ -40,8 +40,7 @@ module ActiveRecord end end - # Like #merge, but applies changes in place. - def merge!(other) + def merge!(other) # :nodoc: if !other.is_a?(Relation) && other.respond_to?(:to_proc) instance_exec(&other) else diff --git a/activerecord/lib/rails/generators/active_record/observer/observer_generator.rb b/activerecord/lib/rails/generators/active_record/observer/observer_generator.rb deleted file mode 100644 index e7445d03a2..0000000000 --- a/activerecord/lib/rails/generators/active_record/observer/observer_generator.rb +++ /dev/null @@ -1,15 +0,0 @@ -require 'rails/generators/active_record' - -module ActiveRecord - module Generators # :nodoc: - class ObserverGenerator < Base # :nodoc: - check_class_collision :suffix => "Observer" - - def create_observer_file - template 'observer.rb', File.join('app/models', class_path, "#{file_name}_observer.rb") - end - - hook_for :test_framework - end - end -end diff --git a/activerecord/lib/rails/generators/active_record/observer/templates/observer.rb b/activerecord/lib/rails/generators/active_record/observer/templates/observer.rb deleted file mode 100644 index eaa256a9bd..0000000000 --- a/activerecord/lib/rails/generators/active_record/observer/templates/observer.rb +++ /dev/null @@ -1,4 +0,0 @@ -<% module_namespacing do -%> -class <%= class_name %>Observer < ActiveRecord::Observer -end -<% end -%> diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 5f7825783b..04d0f755b6 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -109,6 +109,34 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase assert_equal apple.id, citibank.firm_id end + def test_building_the_belonging_object_with_implicit_sti_base_class + account = Account.new + company = account.build_firm + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_belonging_object_with_explicit_sti_base_class + account = Account.new + company = account.build_firm(:type => "Company") + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_belonging_object_with_sti_subclass + account = Account.new + company = account.build_firm(:type => "Firm") + assert_kind_of Firm, company, "Expected #{company.class} to be a Firm" + end + + def test_building_the_belonging_object_with_an_invalid_type + account = Account.new + assert_raise(ActiveRecord::SubclassNotFound) { account.build_firm(:type => "InvalidType") } + end + + def test_building_the_belonging_object_with_an_unrelated_type + account = Account.new + assert_raise(ActiveRecord::SubclassNotFound) { account.build_firm(:type => "Account") } + end + def test_building_the_belonging_object_with_primary_key client = Client.create(:name => "Primary key client") apple = client.build_firm_with_primary_key("name" => "Apple") diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 2ded97582d..d25aca760f 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -144,6 +144,34 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal 'defaulty', bulb.name end + def test_building_the_associated_object_with_implicit_sti_base_class + firm = DependentFirm.new + company = firm.companies.build + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_explicit_sti_base_class + firm = DependentFirm.new + company = firm.companies.build(:type => "Company") + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_sti_subclass + firm = DependentFirm.new + company = firm.companies.build(:type => "Client") + assert_kind_of Client, company, "Expected #{company.class} to be a Client" + end + + def test_building_the_associated_object_with_an_invalid_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.companies.build(:type => "Invalid") } + end + + def test_building_the_associated_object_with_an_unrelated_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.companies.build(:type => "Account") } + end + def test_association_keys_bypass_attribute_protection car = Car.create(:name => 'honda') @@ -1579,7 +1607,7 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [tagging], post.taggings end - def test_build_with_polymotphic_has_many_does_not_allow_to_override_type_and_id + def test_build_with_polymorphic_has_many_does_not_allow_to_override_type_and_id welcome = posts(:welcome) tagging = welcome.taggings.build(:taggable_id => 99, :taggable_type => 'ShouldNotChange') diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index ea1cfa0805..4ed09a3bf7 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -6,6 +6,8 @@ require 'models/ship' require 'models/pirate' require 'models/car' require 'models/bulb' +require 'models/author' +require 'models/post' class HasOneAssociationsTest < ActiveRecord::TestCase self.use_transactional_fixtures = false unless supports_savepoints? @@ -212,6 +214,34 @@ class HasOneAssociationsTest < ActiveRecord::TestCase } end + def test_building_the_associated_object_with_implicit_sti_base_class + firm = DependentFirm.new + company = firm.build_company + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_explicit_sti_base_class + firm = DependentFirm.new + company = firm.build_company(:type => "Company") + assert_kind_of Company, company, "Expected #{company.class} to be a Company" + end + + def test_building_the_associated_object_with_sti_subclass + firm = DependentFirm.new + company = firm.build_company(:type => "Client") + assert_kind_of Client, company, "Expected #{company.class} to be a Client" + end + + def test_building_the_associated_object_with_an_invalid_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.build_company(:type => "Invalid") } + end + + def test_building_the_associated_object_with_an_unrelated_type + firm = DependentFirm.new + assert_raise(ActiveRecord::SubclassNotFound) { firm.build_company(:type => "Account") } + end + def test_build_and_create_should_not_happen_within_scope pirate = pirates(:blackbeard) scoped_count = pirate.association(:foo_bulb).scope.where_values.count diff --git a/activerecord/test/cases/associations_test.rb b/activerecord/test/cases/associations_test.rb index c0f1945cec..d7f25f760e 100644 --- a/activerecord/test/cases/associations_test.rb +++ b/activerecord/test/cases/associations_test.rb @@ -289,6 +289,14 @@ class OverridingAssociationsTest < ActiveRecord::TestCase DifferentPeopleList.reflect_on_association(:has_one) ) end + + def test_requires_symbol_argument + assert_raises ArgumentError do + Class.new(Post) do + belongs_to "author" + end + end + end end class GeneratedMethodsTest < ActiveRecord::TestCase diff --git a/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb b/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb index 3e3d6e2769..1eb9bf60e1 100644 --- a/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb +++ b/activerecord/test/cases/connection_adapters/abstract_adapter_test.rb @@ -10,19 +10,18 @@ module ActiveRecord end def test_in_use? - # FIXME: change to refute in Rails 4.0 / mt - assert !adapter.in_use?, 'adapter is not in use' + refute adapter.in_use?, 'adapter is not in use' assert adapter.lease, 'lease adapter' assert adapter.in_use?, 'adapter is in use' end def test_lease_twice assert adapter.lease, 'should lease adapter' - assert !adapter.lease, 'should not lease adapter' + refute adapter.lease, 'should not lease adapter' end def test_last_use - assert !adapter.last_use + refute adapter.last_use adapter.lease assert adapter.last_use end @@ -31,7 +30,7 @@ module ActiveRecord assert adapter.lease, 'lease adapter' assert adapter.in_use?, 'adapter is in use' adapter.expire - assert !adapter.in_use?, 'adapter is in use' + refute adapter.in_use?, 'adapter is in use' end def test_close @@ -45,7 +44,7 @@ module ActiveRecord # Close should put the adapter back in the pool adapter.close - assert !adapter.in_use? + refute adapter.in_use? assert_equal adapter, pool.connection end diff --git a/activerecord/test/cases/connection_adapters/connection_handler_test.rb b/activerecord/test/cases/connection_adapters/connection_handler_test.rb index 2ddabe058f..3e33b30144 100644 --- a/activerecord/test/cases/connection_adapters/connection_handler_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handler_test.rb @@ -4,8 +4,8 @@ module ActiveRecord module ConnectionAdapters class ConnectionHandlerTest < ActiveRecord::TestCase def setup - @klass = Class.new(Base) - @subklass = Class.new(@klass) + @klass = Class.new(Base) { def self.name; 'klass'; end } + @subklass = Class.new(@klass) { def self.name; 'subklass'; end } @handler = ConnectionHandler.new @pool = @handler.establish_connection(@klass, Base.connection_pool.spec) @@ -36,13 +36,11 @@ module ActiveRecord end def test_retrieve_connection_pool_uses_superclass_pool_after_subclass_establish_and_remove - @handler.establish_connection 'north america', Base.connection_pool.spec - assert_same @handler.retrieve_connection_pool(@klass), - @handler.retrieve_connection_pool(@subklass) + sub_pool = @handler.establish_connection(@subklass, Base.connection_pool.spec) + assert_same sub_pool, @handler.retrieve_connection_pool(@subklass) @handler.remove_connection @subklass - assert_same @handler.retrieve_connection_pool(@klass), - @handler.retrieve_connection_pool(@subklass) + assert_same @pool, @handler.retrieve_connection_pool(@subklass) end def test_connection_pools diff --git a/activerecord/test/cases/dirty_test.rb b/activerecord/test/cases/dirty_test.rb index d4fc5f204b..55ee066cda 100644 --- a/activerecord/test/cases/dirty_test.rb +++ b/activerecord/test/cases/dirty_test.rb @@ -12,7 +12,7 @@ class Pirate # Just reopening it, not defining it after_update :check_changes private - # after_save/update in sweepers, observers, and the model itself + # after_save/update and the model itself # can end up checking dirty status and acting on the results def check_changes if self.changed? @@ -203,6 +203,20 @@ class DirtyTest < ActiveRecord::TestCase end end + def test_nullable_datetime_not_marked_as_changed_if_new_value_is_blank + in_time_zone 'Edinburgh' do + target = Class.new(ActiveRecord::Base) + target.table_name = 'topics' + + topic = target.create + assert_nil topic.written_on + + topic.written_on = "" + assert_nil topic.written_on + assert !topic.written_on_changed? + end + end + def test_integer_zero_to_string_zero_not_marked_as_changed pirate = Pirate.new pirate.parrot_id = 0 diff --git a/activerecord/test/cases/explain_test.rb b/activerecord/test/cases/explain_test.rb index 6dce8ccdd1..a7e5fdf709 100644 --- a/activerecord/test/cases/explain_test.rb +++ b/activerecord/test/cases/explain_test.rb @@ -108,6 +108,16 @@ if ActiveRecord::Base.connection.supports_explain? assert_equal expected, base.exec_explain(queries) end + def test_unsupported_connection_adapter + connection.stubs(:supports_explain?).returns(false) + + base.logger.expects(:warn).never + + with_threshold(0) do + Car.where(:name => 'honda').to_a + end + end + def test_silence_auto_explain base.expects(:collecting_sqls_for_explain).never base.logger.expects(:warn).never diff --git a/activerecord/test/cases/forbidden_attributes_protection_test.rb b/activerecord/test/cases/forbidden_attributes_protection_test.rb index 9a2172f41e..490b599fb6 100644 --- a/activerecord/test/cases/forbidden_attributes_protection_test.rb +++ b/activerecord/test/cases/forbidden_attributes_protection_test.rb @@ -1,6 +1,7 @@ require 'cases/helper' require 'active_support/core_ext/hash/indifferent_access' require 'models/person' +require 'models/company' class ProtectedParams < ActiveSupport::HashWithIndifferentAccess attr_accessor :permitted @@ -40,6 +41,20 @@ class ForbiddenAttributesProtectionTest < ActiveRecord::TestCase assert_equal 'm', person.gender end + def test_forbidden_attributes_cannot_be_used_for_sti_inheritance_column + params = ProtectedParams.new(type: 'Client') + assert_raises(ActiveModel::ForbiddenAttributesError) do + Company.new(params) + end + end + + def test_permitted_attributes_can_be_used_for_sti_inheritance_column + params = ProtectedParams.new(type: 'Client') + params.permit! + person = Company.new(params) + assert_equal person.class, Client + end + def test_regular_hash_should_still_be_used_for_mass_assignment person = Person.new(first_name: 'Guille', gender: 'm') diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 1bff005510..3a315d843b 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -22,8 +22,6 @@ ActiveSupport::Deprecation.debug = true # Connect to the database ARTest.connect -require 'support/mysql' - # Quote "type" if it's a reserved word for the current connection. QUOTED_TYPE = ActiveRecord::Base.connection.quote_column_name('type') diff --git a/activerecord/test/cases/inheritance_test.rb b/activerecord/test/cases/inheritance_test.rb index aab7aa51dd..189066eb41 100644 --- a/activerecord/test/cases/inheritance_test.rb +++ b/activerecord/test/cases/inheritance_test.rb @@ -156,6 +156,29 @@ class InheritanceTest < ActiveRecord::TestCase assert_kind_of Cabbage, savoy end + def test_inheritance_new_with_default_class + company = Company.new + assert_equal Company, company.class + end + + def test_inheritance_new_with_base_class + company = Company.new(:type => 'Company') + assert_equal Company, company.class + end + + def test_inheritance_new_with_subclass + firm = Company.new(:type => 'Firm') + assert_equal Firm, firm.class + end + + def test_new_with_invalid_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.new(:type => 'InvalidType') } + end + + def test_new_with_unrelated_type + assert_raise(ActiveRecord::SubclassNotFound) { Company.new(:type => 'Account') } + end + def test_inheritance_condition assert_equal 10, Company.count assert_equal 2, Firm.count diff --git a/activerecord/test/cases/lifecycle_test.rb b/activerecord/test/cases/lifecycle_test.rb deleted file mode 100644 index 0b78f2e46b..0000000000 --- a/activerecord/test/cases/lifecycle_test.rb +++ /dev/null @@ -1,256 +0,0 @@ -require 'cases/helper' -require 'models/topic' -require 'models/developer' -require 'models/reply' -require 'models/minimalistic' -require 'models/comment' - -class SpecialDeveloper < Developer; end - -class DeveloperObserver < ActiveRecord::Observer - def calls - @calls ||= [] - end - - def before_save(developer) - calls << developer - end -end - -class SalaryChecker < ActiveRecord::Observer - observe :special_developer - attr_accessor :last_saved - - def before_save(developer) - return developer.salary > 80000 - end - - module Implementation - def after_save(developer) - self.last_saved = developer - end - end - include Implementation - -end - -class TopicaAuditor < ActiveRecord::Observer - observe :topic - - attr_reader :topic - - def after_find(topic) - @topic = topic - end -end - -class TopicObserver < ActiveRecord::Observer - attr_reader :topic - - def after_find(topic) - @topic = topic - end - - # Create an after_save callback, so a notify_observer hook is created - # on :topic. - def after_save(nothing) - end -end - -class MinimalisticObserver < ActiveRecord::Observer - attr_reader :minimalistic - - def after_find(minimalistic) - @minimalistic = minimalistic - end -end - -class MultiObserver < ActiveRecord::Observer - attr_reader :record - - def self.observed_class() [ Topic, Developer ] end - - cattr_reader :last_inherited - @@last_inherited = nil - - def observed_class_inherited_with_testing(subclass) - observed_class_inherited_without_testing(subclass) - @@last_inherited = subclass - end - - alias_method_chain :observed_class_inherited, :testing - - def after_find(record) - @record = record - end -end - -class ValidatedComment < Comment - attr_accessor :callers - - before_validation :record_callers - - after_validation do - record_callers - end - - def record_callers - callers << self.class if callers - end -end - -class ValidatedCommentObserver < ActiveRecord::Observer - attr_accessor :callers - - def after_validation(model) - callers << self.class if callers - end -end - - -class AroundTopic < Topic -end - -class AroundTopicObserver < ActiveRecord::Observer - observe :around_topic - def topic_ids - @topic_ids ||= [] - end - - def around_save(topic) - topic_ids << topic.id - yield(topic) - topic_ids << topic.id - end -end - -class LifecycleTest < ActiveRecord::TestCase - fixtures :topics, :developers, :minimalistics - - def test_before_destroy - topic = Topic.find(1) - assert_difference 'Topic.count', -(1 + topic.replies.size) do - topic.destroy - end - end - - def test_auto_observer - topic_observer = TopicaAuditor.instance - assert_nil TopicaAuditor.observed_class - assert_equal [Topic], TopicaAuditor.observed_classes.to_a - - topic = Topic.find(1) - assert_equal topic.title, topic_observer.topic.title - end - - def test_inferred_auto_observer - topic_observer = TopicObserver.instance - assert_equal Topic, TopicObserver.observed_class - - topic = Topic.find(1) - assert_equal topic.title, topic_observer.topic.title - end - - def test_observing_two_classes - multi_observer = MultiObserver.instance - - topic = Topic.find(1) - assert_equal topic.title, multi_observer.record.title - - developer = Developer.find(1) - assert_equal developer.name, multi_observer.record.name - end - - def test_observing_subclasses - multi_observer = MultiObserver.instance - - developer = SpecialDeveloper.find(1) - assert_equal developer.name, multi_observer.record.name - - klass = Class.new(Developer) - assert_equal klass, multi_observer.last_inherited - - developer = klass.find(1) - assert_equal developer.name, multi_observer.record.name - end - - def test_after_find_can_be_observed_when_its_not_defined_on_the_model - observer = MinimalisticObserver.instance - assert_equal Minimalistic, MinimalisticObserver.observed_class - - minimalistic = Minimalistic.find(1) - assert_equal minimalistic, observer.minimalistic - end - - def test_after_find_can_be_observed_when_its_defined_on_the_model - observer = TopicObserver.instance - assert_equal Topic, TopicObserver.observed_class - - topic = Topic.find(1) - assert_equal topic, observer.topic - end - - def test_invalid_observer - assert_raise(ArgumentError) { Topic.observers = Object.new; Topic.instantiate_observers } - end - - test "model callbacks fire before observers are notified" do - callers = [] - - comment = ValidatedComment.new - comment.callers = ValidatedCommentObserver.instance.callers = callers - - comment.valid? - assert_equal [ValidatedComment, ValidatedComment, ValidatedCommentObserver], callers, - "model callbacks did not fire before observers were notified" - end - - test "able to save developer" do - SalaryChecker.instance # activate - developer = SpecialDeveloper.new :name => 'Roger', :salary => 100000 - assert developer.save, "developer with normal salary failed to save" - end - - test "unable to save developer with low salary" do - SalaryChecker.instance # activate - developer = SpecialDeveloper.new :name => 'Rookie', :salary => 50000 - assert !developer.save, "allowed to save a developer with too low salary" - end - - test "able to call methods defined with included module" do # https://rails.lighthouseapp.com/projects/8994/tickets/6065-activerecordobserver-is-not-aware-of-method-added-by-including-modules - SalaryChecker.instance # activate - developer = SpecialDeveloper.create! :name => 'Roger', :salary => 100000 - assert_equal developer, SalaryChecker.instance.last_saved - end - - test "around filter from observer should accept block" do - observer = AroundTopicObserver.instance - topic = AroundTopic.new - topic.save - assert_nil observer.topic_ids.first - assert_not_nil observer.topic_ids.last - end - - test "able to disable observers" do - observer = DeveloperObserver.instance # activate - observer.calls.clear - - ActiveRecord::Base.observers.disable DeveloperObserver do - Developer.create! :name => 'Ancestor', :salary => 100000 - SpecialDeveloper.create! :name => 'Descendent', :salary => 100000 - end - - assert_equal [], observer.calls - end - - def test_observer_is_called_once - observer = DeveloperObserver.instance # activate - observer.calls.clear - - developer = Developer.create! :name => 'Ancestor', :salary => 100000 - special_developer = SpecialDeveloper.create! :name => 'Descendent', :salary => 100000 - - assert_equal [developer, special_developer], observer.calls - end - -end diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 98e278df82..92dc575d37 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -6,26 +6,26 @@ module ActiveRecord class RelationTest < ActiveRecord::TestCase fixtures :posts, :comments - class FakeKlass < Struct.new(:table_name) + class FakeKlass < Struct.new(:table_name, :name) end def test_construction relation = nil assert_nothing_raised do - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b end - assert_equal :a, relation.klass + assert_equal FakeKlass, relation.klass assert_equal :b, relation.table assert !relation.loaded, 'relation is not loaded' end def test_responds_to_model_and_returns_klass - relation = Relation.new :a, :b - assert_equal :a, relation.model + relation = Relation.new FakeKlass, :b + assert_equal FakeKlass, relation.model end def test_initialize_single_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b (Relation::SINGLE_VALUE_METHODS - [:create_with]).each do |method| assert_nil relation.send("#{method}_value"), method.to_s end @@ -33,19 +33,19 @@ module ActiveRecord end def test_multi_value_initialize - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b Relation::MULTI_VALUE_METHODS.each do |method| assert_equal [], relation.send("#{method}_values"), method.to_s end end def test_extensions - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal [], relation.extensions end def test_empty_where_values_hash - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal({}, relation.where_values_hash) relation.where! :hello @@ -79,7 +79,7 @@ module ActiveRecord end def test_scope_for_create - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal({}, relation.scope_for_create) end @@ -110,31 +110,31 @@ module ActiveRecord end def test_empty_eager_loading? - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert !relation.eager_loading? end def test_eager_load_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation.eager_load! :b assert relation.eager_loading? end def test_references_values - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b assert_equal [], relation.references_values relation = relation.references(:foo).references(:omg, :lol) assert_equal ['foo', 'omg', 'lol'], relation.references_values end def test_references_values_dont_duplicate - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation = relation.references(:foo).references(:foo) assert_equal ['foo'], relation.references_values end test 'merging a hash into a relation' do - relation = Relation.new :a, :b + relation = Relation.new FakeKlass, :b relation = relation.merge where: :lol, readonly: true assert_equal [:lol], relation.where_values @@ -142,7 +142,7 @@ module ActiveRecord end test 'merging an empty hash into a relation' do - assert_equal [], Relation.new(:a, :b).merge({}).where_values + assert_equal [], Relation.new(FakeKlass, :b).merge({}).where_values end test 'merging a hash with unknown keys raises' do @@ -150,7 +150,7 @@ module ActiveRecord end test '#values returns a dup of the values' do - relation = Relation.new(:a, :b).where! :foo + relation = Relation.new(FakeKlass, :b).where! :foo values = relation.values values[:where] = nil @@ -158,18 +158,18 @@ module ActiveRecord end test 'relations can be created with a values hash' do - relation = Relation.new(:a, :b, where: [:foo]) + relation = Relation.new(FakeKlass, :b, where: [:foo]) assert_equal [:foo], relation.where_values end test 'merging a single where value' do - relation = Relation.new(:a, :b) + relation = Relation.new(FakeKlass, :b) relation.merge!(where: :foo) assert_equal [:foo], relation.where_values end test 'merging a hash interpolates conditions' do - klass = stub + klass = stub_everything klass.stubs(:sanitize_sql).with(['foo = ?', 'bar']).returns('foo = bar') relation = Relation.new(klass, :b) @@ -179,8 +179,11 @@ module ActiveRecord end class RelationMutationTest < ActiveSupport::TestCase + class FakeKlass < Struct.new(:table_name, :name) + end + def relation - @relation ||= Relation.new :a, :b + @relation ||= Relation.new FakeKlass, :b end (Relation::MULTI_VALUE_METHODS - [:references, :extending]).each do |method| diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index c34aeaf925..0cd838c0b0 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -1452,4 +1452,33 @@ class RelationTest < ActiveRecord::TestCase assert_equal expected, result end end + + test "delegations do not leak to other classes" do + Topic.all.by_lifo + assert Topic.all.class.method_defined?(:by_lifo) + assert !Post.all.respond_to?(:by_lifo) + end + + class OMGTopic < ActiveRecord::Base + self.table_name = 'topics' + + def self.__omg__ + "omgtopic" + end + end + + test "delegations do not clash across classes" do + begin + class ::Array + def __omg__ + "array" + end + end + + assert_equal "array", Topic.all.__omg__ + assert_equal "omgtopic", OMGTopic.all.__omg__ + ensure + Array.send(:remove_method, :__omg__) + end + end end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index 961ba8d9ba..2ddc449c12 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -247,87 +247,6 @@ class TransactionCallbacksTest < ActiveRecord::TestCase end -class TransactionObserverCallbacksTest < ActiveRecord::TestCase - self.use_transactional_fixtures = false - fixtures :topics - - class TopicWithObserverAttached < ActiveRecord::Base - self.table_name = :topics - def history - @history ||= [] - end - end - - class TopicWithObserverAttachedObserver < ActiveRecord::Observer - def after_commit(record) - record.history.push "after_commit" - end - - def after_rollback(record) - record.history.push "after_rollback" - end - end - - def test_after_commit_called - assert TopicWithObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithObserverAttached.new - topic.save! - - assert_equal %w{ after_commit }, topic.history - end - - def test_after_rollback_called - assert TopicWithObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithObserverAttached.new - - Topic.transaction do - topic.save! - raise ActiveRecord::Rollback - end - - assert topic.id.nil? - assert !topic.persisted? - assert_equal %w{ after_rollback }, topic.history - end - - class TopicWithManualRollbackObserverAttached < ActiveRecord::Base - self.table_name = :topics - def history - @history ||= [] - end - end - - class TopicWithManualRollbackObserverAttachedObserver < ActiveRecord::Observer - def after_save(record) - record.history.push "after_save" - raise ActiveRecord::Rollback - end - end - - def test_after_save_called_with_manual_rollback - assert TopicWithManualRollbackObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithManualRollbackObserverAttached.new - - assert !topic.save - assert_equal nil, topic.id - assert !topic.persisted? - assert_equal %w{ after_save }, topic.history - end - def test_after_save_called_with_manual_rollback_bang - assert TopicWithManualRollbackObserverAttachedObserver.instance, 'should have observer' - - topic = TopicWithManualRollbackObserverAttached.new - - topic.save! - assert_equal nil, topic.id - assert !topic.persisted? - assert_equal %w{ after_save }, topic.history - end -end - class SaveFromAfterCommitBlockTest < ActiveRecord::TestCase self.use_transactional_fixtures = false diff --git a/activerecord/test/models/author.rb b/activerecord/test/models/author.rb index 77f4a2ec87..6935cfb0ea 100644 --- a/activerecord/test/models/author.rb +++ b/activerecord/test/models/author.rb @@ -1,5 +1,6 @@ class Author < ActiveRecord::Base has_many :posts + has_one :post has_many :very_special_comments, :through => :posts has_many :posts_with_comments, -> { includes(:comments) }, :class_name => "Post" has_many :popular_grouped_posts, -> { includes(:comments).group("type").having("SUM(comments_count) > 1").select("type") }, :class_name => "Post" diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 17b17724e8..3ca8f69646 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -111,6 +111,7 @@ end class DependentFirm < Company has_one :account, :foreign_key => "firm_id", :dependent => :nullify has_many :companies, :foreign_key => 'client_of', :dependent => :nullify + has_one :company, :foreign_key => 'client_of', :dependent => :nullify end class RestrictedFirm < Company diff --git a/activerecord/test/schema/postgresql_specific_schema.rb b/activerecord/test/schema/postgresql_specific_schema.rb index 0cfde83778..ab2a63d3ea 100644 --- a/activerecord/test/schema/postgresql_specific_schema.rb +++ b/activerecord/test/schema/postgresql_specific_schema.rb @@ -152,7 +152,7 @@ _SQL ); _SQL -begin + begin execute <<_SQL CREATE TABLE postgresql_partitioned_table_parent ( id SERIAL PRIMARY KEY, @@ -174,14 +174,14 @@ begin BEFORE INSERT ON postgresql_partitioned_table_parent FOR EACH ROW EXECUTE PROCEDURE partitioned_insert_trigger(); _SQL -rescue ActiveRecord::StatementInvalid => e - if e.message =~ /language "plpgsql" does not exist/ - execute "CREATE LANGUAGE 'plpgsql';" - retry - else - raise e + rescue ActiveRecord::StatementInvalid => e + if e.message =~ /language "plpgsql" does not exist/ + execute "CREATE LANGUAGE 'plpgsql';" + retry + else + raise e + end end -end begin execute <<_SQL @@ -190,9 +190,10 @@ end data xml ); _SQL -rescue #This version of PostgreSQL either has no XML support or is was not compiled with XML support: skipping table + rescue #This version of PostgreSQL either has no XML support or is was not compiled with XML support: skipping table end + # This table is to verify if the :limit option is being ignored for text and binary columns create_table :limitless_fields, force: true do |t| t.binary :binary, limit: 100_000 t.text :text, limit: 100_000 diff --git a/activerecord/test/support/mysql.rb b/activerecord/test/support/mysql.rb deleted file mode 100644 index 7a66415e64..0000000000 --- a/activerecord/test/support/mysql.rb +++ /dev/null @@ -1,11 +0,0 @@ -if defined?(Mysql) - class Mysql - class Error - # This monkey patch fixes annoy warning with mysql-2.8.1.gem when executing testcases. - def errno_with_fix_warnings - silence_warnings { errno_without_fix_warnings } - end - alias_method_chain :errno, :fix_warnings - end - end -end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 504ebcb2fe..450669968b 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,5 +1,23 @@ ## Rails 4.0.0 (unreleased) ## +* Patched Marshal#load to work with constant autoloading. + Fixes autoloading with cache stores that relay on Marshal(MemCacheStore and FileStore). [fixes #8167] + + *Uriel Katz* + +* Make `Time.zone.parse` to work with JavaScript format date strings. *Andrew White* + +* Add `DateTime#seconds_until_end_of_day` and `Time#seconds_until_end_of_day` + as a complement for `seconds_from_midnight`; useful when setting expiration + times for caches, e.g.: + + <% cache('dashboard', expires_in: Date.current.seconds_until_end_of_day) do %> + ... + + *Olek Janiszewski* + +* No longer proxy ActiveSupport::Multibyte#class. *Steve Klabnik* + * Deprecate `ActiveSupport::TestCase#pending` method, use `skip` from MiniTest instead. *Carlos Antonio da Silva* * `XmlMini.with_backend` now may be safely used with threads: @@ -63,7 +81,7 @@ *Jeremy Kemper* -* Add logger.push_tags and .pop_tags to complement logger.tagged: +* Add `logger.push_tags` and `.pop_tags` to complement logger.tagged: class Job def before diff --git a/activesupport/lib/active_support/cache/file_store.rb b/activesupport/lib/active_support/cache/file_store.rb index 2c1ad60d44..8e265ad863 100644 --- a/activesupport/lib/active_support/cache/file_store.rb +++ b/activesupport/lib/active_support/cache/file_store.rb @@ -1,3 +1,4 @@ +require 'active_support/core_ext/marshal' require 'active_support/core_ext/file/atomic' require 'active_support/core_ext/string/conversions' require 'uri/common' diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 17450fe4d0..712db2c75a 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -6,6 +6,7 @@ rescue LoadError => e end require 'digest/md5' +require 'active_support/core_ext/marshal' module ActiveSupport module Cache diff --git a/activesupport/lib/active_support/core_ext/date_time/calculations.rb b/activesupport/lib/active_support/core_ext/date_time/calculations.rb index 0c6437b02b..f77d444479 100644 --- a/activesupport/lib/active_support/core_ext/date_time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/date_time/calculations.rb @@ -32,6 +32,15 @@ class DateTime sec + (min * 60) + (hour * 3600) end + # Returns the number of seconds until 23:59:59. + # + # DateTime.new(2012, 8, 29, 0, 0, 0).seconds_until_end_of_day # => 86399 + # DateTime.new(2012, 8, 29, 12, 34, 56).seconds_until_end_of_day # => 41103 + # DateTime.new(2012, 8, 29, 23, 59, 59).seconds_until_end_of_day # => 0 + def seconds_until_end_of_day + end_of_day.to_i - to_i + end + # Returns a new DateTime where one or more of the elements have been changed # according to the +options+ parameter. The time options (<tt>:hour</tt>, # <tt>:minute</tt>, <tt>:sec</tt>) reset cascadingly, so if only the hour is diff --git a/activesupport/lib/active_support/core_ext/marshal.rb b/activesupport/lib/active_support/core_ext/marshal.rb new file mode 100644 index 0000000000..fec3051c0c --- /dev/null +++ b/activesupport/lib/active_support/core_ext/marshal.rb @@ -0,0 +1,21 @@ +module Marshal + class << self + def load_with_autoloading(source) + begin + load_without_autoloading(source) + rescue ArgumentError, NameError => exc + if exc.message.match(%r|undefined class/module (.+)|) + # try loading the class/module + $1.constantize + # if it is a IO we need to go back to read the object + source.rewind if source.respond_to?(:rewind) + retry + else + raise exc + end + end + end + + alias_method_chain :load, :autoloading + end +end
\ No newline at end of file diff --git a/activesupport/lib/active_support/core_ext/string/multibyte.rb b/activesupport/lib/active_support/core_ext/string/multibyte.rb index 4e7824ad74..a124202936 100644 --- a/activesupport/lib/active_support/core_ext/string/multibyte.rb +++ b/activesupport/lib/active_support/core_ext/string/multibyte.rb @@ -6,7 +6,7 @@ class String # # +mb_chars+ is a multibyte safe proxy for string methods. # - # In Ruby 1.8 and older it creates and returns an instance of the ActiveSupport::Multibyte::Chars class which + # It creates and returns an instance of the ActiveSupport::Multibyte::Chars class which # encapsulates the original string. A Unicode safe version of all the String methods are defined on this proxy # class. If the proxy class doesn't respond to a certain method, it's forwarded to the encapsulated string. # @@ -17,9 +17,6 @@ class String # name.mb_chars.reverse.to_s # => "rellüM sualC" # name.mb_chars.length # => 12 # - # In Ruby 1.9 and newer +mb_chars+ returns +self+ because String is (mostly) encoding aware. This means that - # it becomes easy to run one version of your code on multiple Ruby versions. - # # == Method chaining # # All the methods on the Chars proxy which normally return a string will return a Chars object. This allows @@ -36,11 +33,7 @@ class String # For more information about the methods defined on the Chars proxy see ActiveSupport::Multibyte::Chars. For # information about how to change the default Multibyte behavior see ActiveSupport::Multibyte. def mb_chars - if ActiveSupport::Multibyte.proxy_class.consumes?(self) - ActiveSupport::Multibyte.proxy_class.new(self) - else - self - end + ActiveSupport::Multibyte.proxy_class.new(self) end def is_utf8? diff --git a/activesupport/lib/active_support/core_ext/time/calculations.rb b/activesupport/lib/active_support/core_ext/time/calculations.rb index 931851d40e..46c9f05c15 100644 --- a/activesupport/lib/active_support/core_ext/time/calculations.rb +++ b/activesupport/lib/active_support/core_ext/time/calculations.rb @@ -62,6 +62,15 @@ class Time to_i - change(:hour => 0).to_i + (usec / 1.0e+6) end + # Returns the number of seconds until 23:59:59. + # + # Time.new(2012, 8, 29, 0, 0, 0).seconds_until_end_of_day # => 86399 + # Time.new(2012, 8, 29, 12, 34, 56).seconds_until_end_of_day # => 41103 + # Time.new(2012, 8, 29, 23, 59, 59).seconds_until_end_of_day # => 0 + def seconds_until_end_of_day + end_of_day.to_i - to_i + end + # Returns a new Time where one or more of the elements have been changed according # to the +options+ parameter. The time options (<tt>:hour</tt>, <tt>:min</tt>, # <tt>:sec</tt>, <tt>:usec</tt>) reset cascadingly, so if only the hour is passed, diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index c75fb46263..b816ecae5a 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -644,46 +644,58 @@ module ActiveSupport #:nodoc: normalized = const.to_s.sub(/\A::/, '') normalized.sub!(/\A(Object::)+/, '') - constants = normalized.split('::') - to_remove = constants.pop - parent_name = constants.empty? ? 'Object' : constants.join('::') + constants = normalized.split('::') + to_remove = constants.pop - if parent = safe_constantize(parent_name) - log "removing constant #{const}" - - # In an autoloaded user.rb like this - # - # autoload :Foo, 'foo' - # - # class User < ActiveRecord::Base - # end - # - # we correctly register "Foo" as being autoloaded. But if the app does - # not use the "Foo" constant we need to be careful not to trigger - # loading "foo.rb" ourselves. While #const_defined? and #const_get? do - # require the file, #autoload? and #remove_const don't. + if constants.empty? + parent = Object + else + # This method is robust to non-reachable constants. # - # We are going to remove the constant nonetheless ---which exists as - # far as Ruby is concerned--- because if the user removes the macro - # call from a class or module that were not autoloaded, as in the - # example above with Object, accessing to that constant must err. - unless parent.autoload?(to_remove) - begin - constantized = parent.const_get(to_remove, false) - rescue NameError - log "the constant #{const} is not reachable anymore, skipping" - return - else - constantized.before_remove_const if constantized.respond_to?(:before_remove_const) - end - end + # Non-reachable constants may be passed if some of the parents were + # autoloaded and already removed. It is easier to do a sanity check + # here than require the caller to be clever. We check the parent + # rather than the very const argument because we do not want to + # trigger Kernel#autoloads, see the comment below. + parent_name = constants.join('::') + return unless qualified_const_defined?(parent_name) + parent = constantize(parent_name) + end + log "removing constant #{const}" + + # In an autoloaded user.rb like this + # + # autoload :Foo, 'foo' + # + # class User < ActiveRecord::Base + # end + # + # we correctly register "Foo" as being autoloaded. But if the app does + # not use the "Foo" constant we need to be careful not to trigger + # loading "foo.rb" ourselves. While #const_defined? and #const_get? do + # require the file, #autoload? and #remove_const don't. + # + # We are going to remove the constant nonetheless ---which exists as + # far as Ruby is concerned--- because if the user removes the macro + # call from a class or module that were not autoloaded, as in the + # example above with Object, accessing to that constant must err. + unless parent.autoload?(to_remove) begin - parent.instance_eval { remove_const to_remove } + constantized = parent.const_get(to_remove, false) rescue NameError log "the constant #{const} is not reachable anymore, skipping" + return + else + constantized.before_remove_const if constantized.respond_to?(:before_remove_const) end end + + begin + parent.instance_eval { remove_const to_remove } + rescue NameError + log "the constant #{const} is not reachable anymore, skipping" + end end protected diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 0207f53238..1a37d6f2a4 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -278,18 +278,23 @@ module ActiveSupport # Time.zone.now # => Fri, 31 Dec 1999 14:00:00 HST -10:00 # Time.zone.parse('22:30:00') # => Fri, 31 Dec 1999 22:30:00 HST -10:00 def parse(str, now=now) - date_parts = Date._parse(str) - return if date_parts.empty? - time = Time.parse(str, now) rescue DateTime.parse(str) - - if date_parts[:offset].nil? - if date_parts[:hour] && time.hour != date_parts[:hour] - time = DateTime.parse(str) - end - - ActiveSupport::TimeWithZone.new(nil, self, time) + parts = Date._parse(str, false) + return if parts.empty? + + time = Time.new( + parts.fetch(:year, now.year), + parts.fetch(:mon, now.month), + parts.fetch(:mday, now.day), + parts.fetch(:hour, now.hour), + parts.fetch(:min, now.min), + parts.fetch(:sec, now.sec) + parts.fetch(:sec_fraction, 0), + parts.fetch(:offset, 0) + ) + + if parts[:offset] + TimeWithZone.new(time.utc, self) else - time.in_time_zone(self) + TimeWithZone.new(nil, self, time) end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index ed903746c8..a2e2c51283 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -1,6 +1,7 @@ require 'logger' require 'abstract_unit' require 'active_support/cache' +require 'dependecies_test_helpers' class CacheKeyTest < ActiveSupport::TestCase def test_entry_legacy_optional_ivars @@ -562,6 +563,45 @@ module LocalCacheBehavior end end +module AutoloadingCacheBehavior + include DependeciesTestHelpers + def test_simple_autoloading + with_autoloading_fixtures do + @cache.write('foo', E.new) + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of E, @cache.read('foo') + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + end + + def test_two_classes_autoloading + with_autoloading_fixtures do + @cache.write('foo', [E.new, ClassFolder.new]) + end + + remove_constants(:E, :ClassFolder) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + loaded = @cache.read('foo') + assert_kind_of Array, loaded + assert_equal 2, loaded.size + assert_kind_of E, loaded[0] + assert_kind_of ClassFolder, loaded[1] + end + + remove_constants(:E, :ClassFolder) + ActiveSupport::Dependencies.clear + end +end + class FileStoreTest < ActiveSupport::TestCase def setup Dir.mkdir(cache_dir) unless File.exist?(cache_dir) @@ -585,6 +625,7 @@ class FileStoreTest < ActiveSupport::TestCase include LocalCacheBehavior include CacheDeleteMatchedBehavior include CacheIncrementDecrementBehavior + include AutoloadingCacheBehavior def test_clear filepath = File.join(cache_dir, ".gitkeep") @@ -745,6 +786,7 @@ class MemCacheStoreTest < ActiveSupport::TestCase include LocalCacheBehavior include CacheIncrementDecrementBehavior include EncodedKeyCacheBehavior + include AutoloadingCacheBehavior def test_raw_values cache = ActiveSupport::Cache.lookup_store(:mem_cache_store, :raw => true) diff --git a/activesupport/test/core_ext/date_time_ext_test.rb b/activesupport/test/core_ext/date_time_ext_test.rb index b1d1e8ecb4..3353465c1c 100644 --- a/activesupport/test/core_ext/date_time_ext_test.rb +++ b/activesupport/test/core_ext/date_time_ext_test.rb @@ -61,6 +61,14 @@ class DateTimeExtCalculationsTest < ActiveSupport::TestCase assert_equal 86399,DateTime.civil(2005,1,1,23,59,59).seconds_since_midnight end + def test_seconds_until_end_of_day + assert_equal 0, DateTime.civil(2005,1,1,23,59,59).seconds_until_end_of_day + assert_equal 1, DateTime.civil(2005,1,1,23,59,58).seconds_until_end_of_day + assert_equal 60, DateTime.civil(2005,1,1,23,58,59).seconds_until_end_of_day + assert_equal 3660, DateTime.civil(2005,1,1,22,58,59).seconds_until_end_of_day + assert_equal 86399, DateTime.civil(2005,1,1,0,0,0).seconds_until_end_of_day + end + def test_beginning_of_day assert_equal DateTime.civil(2005,2,4,0,0,0), DateTime.civil(2005,2,4,10,10,10).beginning_of_day end diff --git a/activesupport/test/core_ext/marshal_test.rb b/activesupport/test/core_ext/marshal_test.rb new file mode 100644 index 0000000000..ac79b15fa8 --- /dev/null +++ b/activesupport/test/core_ext/marshal_test.rb @@ -0,0 +1,124 @@ +require 'abstract_unit' +require 'active_support/core_ext/marshal' +require 'dependecies_test_helpers' + +class MarshalTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + include DependeciesTestHelpers + + def teardown + ActiveSupport::Dependencies.clear + remove_constants(:E, :ClassFolder) + end + + test "that Marshal#load still works" do + sanity_data = ["test", [1, 2, 3], {a: [1, 2, 3]}, ActiveSupport::TestCase] + sanity_data.each do |obj| + dumped = Marshal.dump(obj) + assert_equal Marshal.load_without_autoloading(dumped), Marshal.load(dumped) + end + end + + test "that a missing class is autoloaded from string" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump(E.new) + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of E, Marshal.load(dumped) + end + end + + test "that classes in sub modules work" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump(ClassFolder::ClassFolderSubclass.new) + end + + remove_constants(:ClassFolder) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of ClassFolder::ClassFolderSubclass, Marshal.load(dumped) + end + end + + test "that more than one missing class is autoloaded" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump([E.new, ClassFolder.new]) + end + + remove_constants(:E, :ClassFolder) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + loaded = Marshal.load(dumped) + assert_equal 2, loaded.size + assert_kind_of E, loaded[0] + assert_kind_of ClassFolder, loaded[1] + end + end + + test "that a real missing class is causing an exception" do + dumped = nil + with_autoloading_fixtures do + dumped = Marshal.dump(E.new) + end + + remove_constants(:E) + ActiveSupport::Dependencies.clear + + assert_raise(NameError) do + Marshal.load(dumped) + end + end + + test "when first class is autoloaded and second not" do + dumped = nil + class SomeClass + end + + with_autoloading_fixtures do + dumped = Marshal.dump([E.new, SomeClass.new]) + end + + remove_constants(:E) + self.class.send(:remove_const, :SomeClass) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_raise(NameError) do + Marshal.load(dumped) + end + + assert_nothing_raised("E failed to load while we expect only SomeClass to fail loading") do + E.new + end + + assert_raise(NameError, "We expected SomeClass to not be loaded but it is!") do + SomeClass.new + end + end + end + + test "loading classes from files trigger autoloading" do + Tempfile.open("object_serializer_test") do |f| + with_autoloading_fixtures do + Marshal.dump(E.new, f) + end + + f.rewind + remove_constants(:E) + ActiveSupport::Dependencies.clear + + with_autoloading_fixtures do + assert_kind_of E, Marshal.load(f) + end + end + end +end
\ No newline at end of file diff --git a/activesupport/test/core_ext/time_ext_test.rb b/activesupport/test/core_ext/time_ext_test.rb index 6d6757a1b6..0e75104fc6 100644 --- a/activesupport/test/core_ext/time_ext_test.rb +++ b/activesupport/test/core_ext/time_ext_test.rb @@ -57,6 +57,54 @@ class TimeExtCalculationsTest < ActiveSupport::TestCase end end + def test_seconds_until_end_of_day + assert_equal 0, Time.local(2005,1,1,23,59,59).seconds_until_end_of_day + assert_equal 1, Time.local(2005,1,1,23,59,58).seconds_until_end_of_day + assert_equal 60, Time.local(2005,1,1,23,58,59).seconds_until_end_of_day + assert_equal 3660, Time.local(2005,1,1,22,58,59).seconds_until_end_of_day + assert_equal 86399, Time.local(2005,1,1,0,0,0).seconds_until_end_of_day + end + + def test_seconds_until_end_of_day_at_daylight_savings_time_start + with_env_tz 'US/Eastern' do + # dt: US: 2005 April 3rd 2:00am ST => April 3rd 3:00am DT + assert_equal 21*3600, Time.local(2005,4,3,1,59,59).seconds_until_end_of_day, 'just before DST start' + assert_equal 21*3600-2, Time.local(2005,4,3,3,0,1).seconds_until_end_of_day, 'just after DST start' + end + + with_env_tz 'NZ' do + # dt: New Zealand: 2006 October 1st 2:00am ST => October 1st 3:00am DT + assert_equal 21*3600, Time.local(2006,10,1,1,59,59).seconds_until_end_of_day, 'just before DST start' + assert_equal 21*3600-2, Time.local(2006,10,1,3,0,1).seconds_until_end_of_day, 'just after DST start' + end + end + + def test_seconds_until_end_of_day_at_daylight_savings_time_end + with_env_tz 'US/Eastern' do + # st: US: 2005 October 30th 2:00am DT => October 30th 1:00am ST + # avoid setting a time between 1:00 and 2:00 since that requires specifying whether DST is active + assert_equal 24*3600, Time.local(2005,10,30,0,59,59).seconds_until_end_of_day, 'just before DST end' + assert_equal 22*3600-2, Time.local(2005,10,30,2,0,1).seconds_until_end_of_day, 'just after DST end' + + # now set a time between 1:00 and 2:00 by specifying whether DST is active + # uses: Time.local( sec, min, hour, day, month, year, wday, yday, isdst, tz ) + assert_equal 24*3600-30*60-1, Time.local(0,30,1,30,10,2005,0,0,true,ENV['TZ']).seconds_until_end_of_day, 'before DST end' + assert_equal 23*3600-30*60-1, Time.local(0,30,1,30,10,2005,0,0,false,ENV['TZ']).seconds_until_end_of_day, 'after DST end' + end + + with_env_tz 'NZ' do + # st: New Zealand: 2006 March 19th 3:00am DT => March 19th 2:00am ST + # avoid setting a time between 2:00 and 3:00 since that requires specifying whether DST is active + assert_equal 23*3600, Time.local(2006,3,19,1,59,59).seconds_until_end_of_day, 'just before DST end' + assert_equal 21*3600-2, Time.local(2006,3,19,3,0,1).seconds_until_end_of_day, 'just after DST end' + + # now set a time between 2:00 and 3:00 by specifying whether DST is active + # uses: Time.local( sec, min, hour, day, month, year, wday, yday, isdst, tz ) + assert_equal 23*3600-30*60-1, Time.local(0,30,2,19,3,2006,0,0,true, ENV['TZ']).seconds_until_end_of_day, 'before DST end' + assert_equal 22*3600-30*60-1, Time.local(0,30,2,19,3,2006,0,0,false,ENV['TZ']).seconds_until_end_of_day, 'after DST end' + end + end + def test_beginning_of_day assert_equal Time.local(2005,2,4,0,0,0), Time.local(2005,2,4,10,10,10).beginning_of_day with_env_tz 'US/Eastern' do diff --git a/activesupport/test/dependecies_test_helpers.rb b/activesupport/test/dependecies_test_helpers.rb new file mode 100644 index 0000000000..4b46d32fb4 --- /dev/null +++ b/activesupport/test/dependecies_test_helpers.rb @@ -0,0 +1,27 @@ +module DependeciesTestHelpers + def with_loading(*from) + old_mechanism, ActiveSupport::Dependencies.mechanism = ActiveSupport::Dependencies.mechanism, :load + this_dir = File.dirname(__FILE__) + parent_dir = File.dirname(this_dir) + path_copy = $LOAD_PATH.dup + $LOAD_PATH.unshift(parent_dir) unless $LOAD_PATH.include?(parent_dir) + prior_autoload_paths = ActiveSupport::Dependencies.autoload_paths + ActiveSupport::Dependencies.autoload_paths = from.collect { |f| "#{this_dir}/#{f}" } + yield + ensure + $LOAD_PATH.replace(path_copy) + ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths + ActiveSupport::Dependencies.mechanism = old_mechanism + ActiveSupport::Dependencies.explicitly_unloadable_constants = [] + end + + def with_autoloading_fixtures(&block) + with_loading 'autoloading_fixtures', &block + end + + def remove_constants(*constants) + constants.each do |constant| + Object.send(:remove_const, constant) if Object.const_defined?(constant) + end + end +end
\ No newline at end of file diff --git a/activesupport/test/dependencies_test.rb b/activesupport/test/dependencies_test.rb index 67bd6669c5..952c82f9d8 100644 --- a/activesupport/test/dependencies_test.rb +++ b/activesupport/test/dependencies_test.rb @@ -1,6 +1,7 @@ require 'abstract_unit' require 'pp' require 'active_support/dependencies' +require 'dependecies_test_helpers' module ModuleWithMissing mattr_accessor :missing_count @@ -19,25 +20,7 @@ class DependenciesTest < ActiveSupport::TestCase ActiveSupport::Dependencies.clear end - def with_loading(*from) - old_mechanism, ActiveSupport::Dependencies.mechanism = ActiveSupport::Dependencies.mechanism, :load - this_dir = File.dirname(__FILE__) - parent_dir = File.dirname(this_dir) - path_copy = $LOAD_PATH.dup - $LOAD_PATH.unshift(parent_dir) unless $LOAD_PATH.include?(parent_dir) - prior_autoload_paths = ActiveSupport::Dependencies.autoload_paths - ActiveSupport::Dependencies.autoload_paths = from.collect { |f| "#{this_dir}/#{f}" } - yield - ensure - $LOAD_PATH.replace(path_copy) - ActiveSupport::Dependencies.autoload_paths = prior_autoload_paths - ActiveSupport::Dependencies.mechanism = old_mechanism - ActiveSupport::Dependencies.explicitly_unloadable_constants = [] - end - - def with_autoloading_fixtures(&block) - with_loading 'autoloading_fixtures', &block - end + include DependeciesTestHelpers def test_depend_on_path skip "LoadError#path does not exist" if RUBY_VERSION < '2.0.0' @@ -938,6 +921,16 @@ class DependenciesTest < ActiveSupport::TestCase assert !defined?(ShouldNotBeAutoloaded) end + def test_remove_constant_does_not_autoload_already_removed_parents_as_a_side_effect + with_autoloading_fixtures do + ::A + ::A::B + ActiveSupport::Dependencies.remove_constant('A') + ActiveSupport::Dependencies.remove_constant('A::B') + assert !defined?(A) + end + end + def test_load_once_constants_should_not_be_unloaded with_autoloading_fixtures do ActiveSupport::Dependencies.autoload_once_paths = ActiveSupport::Dependencies.autoload_paths diff --git a/activesupport/test/multibyte_chars_test.rb b/activesupport/test/multibyte_chars_test.rb index ef289692bc..0088a06c34 100644 --- a/activesupport/test/multibyte_chars_test.rb +++ b/activesupport/test/multibyte_chars_test.rb @@ -47,7 +47,7 @@ class MultibyteCharsTest < ActiveSupport::TestCase end def test_methods_are_forwarded_to_wrapped_string_for_byte_strings - assert_equal BYTE_STRING.class, BYTE_STRING.mb_chars.class + assert_equal BYTE_STRING.length, BYTE_STRING.mb_chars.length end def test_forwarded_method_with_non_string_result_should_be_returned_vertabim @@ -673,6 +673,9 @@ class MultibyteCharsExtrasTest < ActiveSupport::TestCase assert_equal "𥤤", chars(byte_string).tidy_bytes(true) end + def test_class_is_not_forwarded + assert_equal BYTE_STRING.dup.mb_chars.class, ActiveSupport::Multibyte::Chars + end private diff --git a/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb index bfd6863e40..bdeb80a294 100644 --- a/activesupport/test/time_zone_test.rb +++ b/activesupport/test/time_zone_test.rb @@ -178,8 +178,8 @@ class TimeZoneTest < ActiveSupport::TestCase def test_parse_with_old_date zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)'] - twz = zone.parse('1850-12-31 19:00:00') - assert_equal [0,0,19,31,12,1850], twz.to_a[0,6] + twz = zone.parse('1883-12-31 19:00:00') + assert_equal [0,0,19,31,12,1883], twz.to_a[0,6] assert_equal zone, twz.time_zone end @@ -204,21 +204,25 @@ class TimeZoneTest < ActiveSupport::TestCase end def test_parse_should_not_black_out_system_timezone_dst_jump - zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)'] - zone.stubs(:now).returns(zone.now) - Time.stubs(:parse).with('2012-03-25 03:29', zone.now). - returns(Time.local(0,29,4,25,3,2012,nil,nil,true,"+03:00")) - twz = zone.parse('2012-03-25 03:29') - assert_equal [0, 29, 3, 25, 3, 2012], twz.to_a[0,6] + with_env_tz('EET') do + zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)'] + twz = zone.parse('2012-03-25 03:29:00') + assert_equal [0, 29, 3, 25, 3, 2012], twz.to_a[0,6] + end end def test_parse_should_black_out_app_timezone_dst_jump - zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)'] - zone.stubs(:now).returns(zone.now) - Time.stubs(:parse).with('2012-03-11 02:29', zone.now). - returns(Time.local(0,29,2,11,3,2012,nil,nil,false,"+02:00")) - twz = zone.parse('2012-03-11 02:29') - assert_equal [0, 29, 3, 11, 3, 2012], twz.to_a[0,6] + with_env_tz('EET') do + zone = ActiveSupport::TimeZone['Pacific Time (US & Canada)'] + twz = zone.parse('2012-03-11 02:29:00') + assert_equal [0, 29, 3, 11, 3, 2012], twz.to_a[0,6] + end + end + + def test_parse_with_javascript_date + zone = ActiveSupport::TimeZone['Eastern Time (US & Canada)'] + twz = zone.parse("Mon May 28 2012 00:00:00 GMT-0700 (PDT)") + assert_equal Time.utc(2012, 5, 28, 7, 0, 0), twz.utc end def test_utc_offset_lazy_loaded_from_tzinfo_when_not_passed_in_to_initialize diff --git a/guides/CHANGELOG.md b/guides/CHANGELOG.md index 11fe1c5efa..e9f7ff9d68 100644 --- a/guides/CHANGELOG.md +++ b/guides/CHANGELOG.md @@ -1,5 +1,7 @@ ## Rails 4.0.0 (unreleased) ## +* Split Validations and Callbacks guide into two. *Steve Klabnik* + * New guide _Working with JavaScript in Rails_. *Steve Klabnik* * Guides updated to reflect new test locations. *Mike Moore* diff --git a/guides/code/getting_started/config/application.rb b/guides/code/getting_started/config/application.rb index d2cd5c028b..d53c9fd8bc 100644 --- a/guides/code/getting_started/config/application.rb +++ b/guides/code/getting_started/config/application.rb @@ -18,9 +18,6 @@ module Blog # Custom directories with classes and modules you want to be autoloadable. # config.autoload_paths += %W(#{config.root}/extras) - # Activate observers that should always be running. - # config.active_record.observers = :cacher, :garbage_collector, :forum_observer - # Set Time.zone default to the specified zone and make Active Record auto-convert to this zone. # Run "rake -D time" for a list of tasks for finding time zone names. Default is UTC. # config.time_zone = 'Central Time (US & Canada)' diff --git a/guides/source/4_0_release_notes.md b/guides/source/4_0_release_notes.md index ecb8dd04f5..b4442130ac 100644 --- a/guides/source/4_0_release_notes.md +++ b/guides/source/4_0_release_notes.md @@ -7,6 +7,7 @@ Highlights in Rails 4.0: (WIP) * Strong Parameters * Queue API * Caching Improvements +* ActionController::Live These release notes cover the major changes, but do not include each bug-fix and changes. If you want to see everything, check out the [list of commits](https://github.com/rails/rails/commits/master) in the main Rails repository on GitHub. @@ -69,6 +70,20 @@ $ ruby /path/to/rails/railties/bin/rails new myapp --dev Major Features -------------- +Moved to a Plugin +----------------- + +With Rails 4 several pieces have been extracted. While Rails won't ship with these features anymore, +you can simply add the extracted plugin to your `Gemfile` to bring the functionality back. + +* Hash-based & Dynamic finder methods ([Github](https://github.com/rails/activerecord-deprecated_finders)) +* Mass assignment protection in Active Record models ([Github](https://github.com/rails/protected_attributes), [Pull Request](https://github.com/rails/rails/pull/7251)) +* ActiveRecord::SessionStore ([Github](https://github.com/rails/activerecord-session_store), [Pull Request](https://github.com/rails/rails/pull/7436)) +* Active Record Observers ([Github](https://github.com/rails/rails-observers), [Commit](https://github.com/rails/rails/commit/39e85b3b90c58449164673909a6f1893cba290b2)) +* Active Resource ([Github](https://github.com/rails/activeresource), [Pull Request](https://github.com/rails/rails/pull/572), [Blog](http://yetimedia.tumblr.com/post/35233051627/activeresource-is-dead-long-live-activeresource)) +* Action Caching ([Github](https://github.com/rails/actionpack-action_caching), [Pull Request](https://github.com/rails/rails/pull/7833)) +* Page Caching ([Github](https://github.com/rails/actionpack-page_caching), [Pull Request](https://github.com/rails/rails/pull/7833)) + Documentation ------------- @@ -85,7 +100,7 @@ Railties * Add `.rake` to list of file extensions included by `rake notes` and `rake notes:custom`. -* New test locations `test/models`, `test/helpers`, `test/controllers`, and `test/mailers`. Corresponding rake tasks added as well. +* New test locations `test/models`, `test/helpers`, `test/controllers`, and `test/mailers`. Corresponding rake tasks added as well. ([Pull Request](https://github.com/rails/rails/pull/7878)) * Set a different cache per environment for assets pipeline through `config.assets.cache`. @@ -167,7 +182,7 @@ Action Mailer * Raise an `ActionView::MissingTemplate` exception when no implicit template could be found. -* Asynchronously send messages via the Rails Queue. +* Asynchronously send messages via the Rails Queue. ([Pull Request](https://github.com/rails/rails/pull/6839)) * Delivery Options (such as SMTP Settings) can now be set dynamically per mailer action. @@ -197,6 +212,8 @@ Action Pack If you add the above code, you can use `<%= error %>` in an erb, and `redirect_to /foo, :error => 'message'` in a controller. +* Encrypted Cookies + Sign using Derived Keys. ([Pull Request](https://github.com/rails/rails/pull/8112)) + * Remove Active Model dependency from Action Pack. * Support unicode characters in routes. Route will be automatically escaped, so instead of manually escaping: @@ -294,6 +311,8 @@ Action Pack * Show routes in exception page while debugging a `RoutingError` in development. +* Helper methods for HTML5 inputs. ([Pull Request](https://github.com/rails/rails/pull/6359)) + * Include `mounted_helpers` (helpers for accessing mounted engines) in `ActionDispatch::IntegrationTest` by default. * Added `ActionDispatch::SSL` middleware that when included force all the requests to be under HTTPS protocol. @@ -577,7 +596,7 @@ Active Record store :settings, accessors: [ :color, :homepage ], coder: JSON ``` -* `mysql` and `mysql2` connections will set `SQL_MODE=STRICT_ALL_TABLES` by default to avoid silent data loss. This can be disabled by specifying `strict: false` in `config/database.yml`. +* `mysql` and `mysql2` connections will set `SQL_MODE=STRICT_ALL_TABLES` by default to avoid silent data loss. This can be disabled by specifying `strict: false` in `config/database.yml`. ([Pull Request](https://github.com/rails/rails/pull/6069)) * Added default order to `ActiveRecord::Base#first` to assure consistent results among different database engines. Introduced `ActiveRecord::Base#take` as a replacement to the old behavior. @@ -614,7 +633,7 @@ Active Record * Remove IdentityMap - IdentityMap has never graduated to be an "enabled-by-default" feature, due to some inconsistencies with associations, as described in this [commit](https://github.com/rails/rails/commit/302c912bf6bcd0fa200d964ec2dc4a44abe328a6). Hence the removal from the codebase, until such issues are fixed. -* Added a feature to dump/load internal state of `SchemaCache` instance because we want to boot more quickly when we have many models. +* Added a feature to dump/load internal state of `SchemaCache` instance because we want to boot more quickly when we have many models. ([Pull Request](https://github.com/rails/rails/pull/5162)) ```ruby # execute rake task. @@ -685,6 +704,8 @@ Active Record * PostgreSQL hstore types are automatically deserialized from the database. +* Support for array datatype in PostgreSQL. ([Pull Request](https://github.com/rails/rails/pull/7547)) + * Added `#update_columns` method which updates the attributes from the passed-in hash without calling save, hence skipping validations and callbacks. `ActiveRecordError` will be raised when called on new objects or when at least one of the attributes is marked as read only. ```ruby @@ -819,7 +840,7 @@ Active Model * `ConfirmationValidator` error messages will attach to `:#{attribute}_confirmation` instead of `attribute`. -* Added `ActiveModel::Model`, a mixin to make Ruby objects work with Action Pack out of the box. +* Added `ActiveModel::Model`, a mixin to make Ruby objects work with Action Pack out of the box. ([Pull Request](https://github.com/rails/rails/pull/5253)) * `ActiveModel::Errors#to_json` supports a new parameter `:full_messages`. diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md new file mode 100644 index 0000000000..c45f3f2e6a --- /dev/null +++ b/guides/source/active_record_callbacks.md @@ -0,0 +1,361 @@ +Active Record Callbacks +======================= + +This guide teaches you how to hook into the life cycle of your Active Record +objects. + +After reading this guide and trying out the presented concepts, we hope that you'll be able to: + +* Understand the life cycle of Active Record objects +* Create callback methods that respond to events in the object life cycle +* Create special classes that encapsulate common behavior for your callbacks + +-------------------------------------------------------------------------------- + +The Object Life Cycle +--------------------- + +During the normal operation of a Rails application, objects may be created, updated, and destroyed. Active Record provides hooks into this <em>object life cycle</em> so that you can control your application and its data. + +Callbacks allow you to trigger logic before or after an alteration of an object's state. + +Callbacks Overview +------------------ + +Callbacks are methods that get called at certain moments of an object's life cycle. With callbacks it is possible to write code that will run whenever an Active Record object is created, saved, updated, deleted, validated, or loaded from the database. + +### Callback Registration + +In order to use the available callbacks, you need to register them. You can implement the callbacks as ordinary methods and use a macro-style class method to register them as callbacks: + +```ruby +class User < ActiveRecord::Base + validates :login, :email, presence: true + + before_validation :ensure_login_has_a_value + + protected + def ensure_login_has_a_value + if login.nil? + self.login = email unless email.blank? + end + end +end +``` + +The macro-style class methods can also receive a block. Consider using this style if the code inside your block is so short that it fits in a single line: + +```ruby +class User < ActiveRecord::Base + validates :login, :email, presence: true + + before_create do |user| + user.name = user.login.capitalize if user.name.blank? + end +end +``` + +Callbacks can also be registered to only fire on certain lifecycle events: + +```ruby +class User < ActiveRecord::Base + before_validation :normalize_name, on: :create + + # :on takes an array as well + after_validation :set_location, on: [ :create, :update ] + + protected + def normalize_name + self.name = self.name.downcase.titleize + end + + def set_location + self.location = LocationService.query(self) + end +end +``` + +It is considered good practice to declare callback methods as protected or private. If left public, they can be called from outside of the model and violate the principle of object encapsulation. + +Available Callbacks +------------------- + +Here is a list with all the available Active Record callbacks, listed in the same order in which they will get called during the respective operations: + +### Creating an Object + +* `before_validation` +* `after_validation` +* `before_save` +* `around_save` +* `before_create` +* `around_create` +* `after_create` +* `after_save` + +### Updating an Object + +* `before_validation` +* `after_validation` +* `before_save` +* `around_save` +* `before_update` +* `around_update` +* `after_update` +* `after_save` + +### Destroying an Object + +* `before_destroy` +* `around_destroy` +* `after_destroy` + +WARNING. `after_save` runs both on create and update, but always _after_ the more specific callbacks `after_create` and `after_update`, no matter the order in which the macro calls were executed. + +### `after_initialize` and `after_find` + +The `after_initialize` callback will be called whenever an Active Record object is instantiated, either by directly using `new` or when a record is loaded from the database. It can be useful to avoid the need to directly override your Active Record `initialize` method. + +The `after_find` callback will be called whenever Active Record loads a record from the database. `after_find` is called before `after_initialize` if both are defined. + +The `after_initialize` and `after_find` callbacks have no `before_*` counterparts, but they can be registered just like the other Active Record callbacks. + +```ruby +class User < ActiveRecord::Base + after_initialize do |user| + puts "You have initialized an object!" + end + + after_find do |user| + puts "You have found an object!" + end +end + +>> User.new +You have initialized an object! +=> #<User id: nil> + +>> User.first +You have found an object! +You have initialized an object! +=> #<User id: 1> +``` + +Running Callbacks +----------------- + +The following methods trigger callbacks: + +* `create` +* `create!` +* `decrement!` +* `destroy` +* `destroy_all` +* `increment!` +* `save` +* `save!` +* `save(validate: false)` +* `toggle!` +* `update` +* `update_attribute` +* `update_attributes` +* `update_attributes!` +* `valid?` + +Additionally, the `after_find` callback is triggered by the following finder methods: + +* `all` +* `first` +* `find` +* `find_all_by_*` +* `find_by_*` +* `find_by_*!` +* `find_by_sql` +* `last` + +The `after_initialize` callback is triggered every time a new object of the class is initialized. + +NOTE: The `find_all_by_*`, `find_by_*` and `find_by_*!` methods are dynamic finders generated automatically for every attribute. Learn more about them at the [Dynamic finders section](active_record_querying.html#dynamic-finders) + +Skipping Callbacks +------------------ + +Just as with validations, it is also possible to skip callbacks. These methods should be used with caution, however, because important business rules and application logic may be kept in callbacks. Bypassing them without understanding the potential implications may lead to invalid data. + +* `decrement` +* `decrement_counter` +* `delete` +* `delete_all` +* `increment` +* `increment_counter` +* `toggle` +* `touch` +* `update_column` +* `update_columns` +* `update_all` +* `update_counters` + +Halting Execution +----------------- + +As you start registering new callbacks for your models, they will be queued for execution. This queue will include all your model's validations, the registered callbacks, and the database operation to be executed. + +The whole callback chain is wrapped in a transaction. If any <em>before</em> callback method returns exactly `false` or raises an exception, the execution chain gets halted and a ROLLBACK is issued; <em>after</em> callbacks can only accomplish that by raising an exception. + +WARNING. Raising an arbitrary exception may break code that expects `save` and its friends not to fail like that. The `ActiveRecord::Rollback` exception is thought precisely to tell Active Record a rollback is going on. That one is internally captured but not reraised. + +Relational Callbacks +-------------------- + +Callbacks work through model relationships, and can even be defined by them. Suppose an example where a user has many posts. A user's posts should be destroyed if the user is destroyed. Let's add an `after_destroy` callback to the `User` model by way of its relationship to the `Post` model: + +```ruby +class User < ActiveRecord::Base + has_many :posts, dependent: :destroy +end + +class Post < ActiveRecord::Base + after_destroy :log_destroy_action + + def log_destroy_action + puts 'Post destroyed' + end +end + +>> user = User.first +=> #<User id: 1> +>> user.posts.create! +=> #<Post id: 1, user_id: 1> +>> user.destroy +Post destroyed +=> #<User id: 1> +``` + +Conditional Callbacks +--------------------- + +As with validations, we can also make the calling of a callback method conditional on the satisfaction of a given predicate. We can do this using the `:if` and `:unless` options, which can take a symbol, a string, a `Proc` or an `Array`. You may use the `:if` option when you want to specify under which conditions the callback **should** be called. If you want to specify the conditions under which the callback **should not** be called, then you may use the `:unless` option. + +### Using `:if` and `:unless` with a `Symbol` + +You can associate the `:if` and `:unless` options with a symbol corresponding to the name of a predicate method that will get called right before the callback. When using the `:if` option, the callback won't be executed if the predicate method returns false; when using the `:unless` option, the callback won't be executed if the predicate method returns true. This is the most common option. Using this form of registration it is also possible to register several different predicates that should be called to check if the callback should be executed. + +```ruby +class Order < ActiveRecord::Base + before_save :normalize_card_number, if: :paid_with_card? +end +``` + +### Using `:if` and `:unless` with a String + +You can also use a string that will be evaluated using `eval` and hence needs to contain valid Ruby code. You should use this option only when the string represents a really short condition: + +```ruby +class Order < ActiveRecord::Base + before_save :normalize_card_number, if: "paid_with_card?" +end +``` + +### Using `:if` and `:unless` with a `Proc` + +Finally, it is possible to associate `:if` and `:unless` with a `Proc` object. This option is best suited when writing short validation methods, usually one-liners: + +```ruby +class Order < ActiveRecord::Base + before_save :normalize_card_number, + if: Proc.new { |order| order.paid_with_card? } +end +``` + +### Multiple Conditions for Callbacks + +When writing conditional callbacks, it is possible to mix both `:if` and `:unless` in the same callback declaration: + +```ruby +class Comment < ActiveRecord::Base + after_create :send_email_to_author, if: :author_wants_emails?, + unless: Proc.new { |comment| comment.post.ignore_comments? } +end +``` + +Callback Classes +---------------- + +Sometimes the callback methods that you'll write will be useful enough to be reused by other models. Active Record makes it possible to create classes that encapsulate the callback methods, so it becomes very easy to reuse them. + +Here's an example where we create a class with an `after_destroy` callback for a `PictureFile` model: + +```ruby +class PictureFileCallbacks + def after_destroy(picture_file) + if File.exists?(picture_file.filepath) + File.delete(picture_file.filepath) + end + end +end +``` + +When declared inside a class, as above, the callback methods will receive the model object as a parameter. We can now use the callback class in the model: + +```ruby +class PictureFile < ActiveRecord::Base + after_destroy PictureFileCallbacks.new +end +``` + +Note that we needed to instantiate a new `PictureFileCallbacks` object, since we declared our callback as an instance method. This is particularly useful if the callbacks make use of the state of the instantiated object. Often, however, it will make more sense to declare the callbacks as class methods: + +```ruby +class PictureFileCallbacks + def self.after_destroy(picture_file) + if File.exists?(picture_file.filepath) + File.delete(picture_file.filepath) + end + end +end +``` + +If the callback method is declared this way, it won't be necessary to instantiate a `PictureFileCallbacks` object. + +```ruby +class PictureFile < ActiveRecord::Base + after_destroy PictureFileCallbacks +end +``` + +You can declare as many callbacks as you want inside your callback classes. + +Transaction Callbacks +--------------------- + +There are two additional callbacks that are triggered by the completion of a database transaction: `after_commit` and `after_rollback`. These callbacks are very similar to the `after_save` callback except that they don't execute until after database changes have either been committed or rolled back. They are most useful when your active record models need to interact with external systems which are not part of the database transaction. + +Consider, for example, the previous example where the `PictureFile` model needs to delete a file after the corresponding record is destroyed. If anything raises an exception after the `after_destroy` callback is called and the transaction rolls back, the file will have been deleted and the model will be left in an inconsistent state. For example, suppose that `picture_file_2` in the code below is not valid and the `save!` method raises an error. + +```ruby +PictureFile.transaction do + picture_file_1.destroy + picture_file_2.save! +end +``` + +By using the `after_commit` callback we can account for this case. + +```ruby +class PictureFile < ActiveRecord::Base + attr_accessor :delete_file + + after_destroy do |picture_file| + picture_file.delete_file = picture_file.filepath + end + + after_commit do |picture_file| + if picture_file.delete_file && File.exist?(picture_file.delete_file) + File.delete(picture_file.delete_file) + picture_file.delete_file = nil + end + end +end +``` + +The `after_commit` and `after_rollback` callbacks are guaranteed to be called for all models created, updated, or destroyed within a transaction block. If any exceptions are raised within one of these callbacks, they will be ignored so that they don't interfere with the other callbacks. As such, if your callback code could raise an exception, you'll need to rescue it and handle it appropriately within the callback. diff --git a/guides/source/active_record_validations_callbacks.md b/guides/source/active_record_validations.md index 18b73d8805..541b1a1e84 100644 --- a/guides/source/active_record_validations_callbacks.md +++ b/guides/source/active_record_validations.md @@ -1,7 +1,8 @@ -Active Record Validations and Callbacks -======================================= +Active Record Validations +========================= -This guide teaches you how to hook into the life cycle of your Active Record objects. You will learn how to validate the state of objects before they go into the database, and how to perform custom operations at certain points in the object life cycle. +This guide teaches you how to validate the state of objects before they go into +the database using Active Record's validations feature. After reading this guide, you will know: @@ -11,17 +12,9 @@ After reading this guide, you will know: * Work with the error messages generated by the validation process. * Create callback methods that respond to events in the object life cycle. * Create special classes that encapsulate common behavior for your callbacks. -* Create Observers that respond to life cycle events outside of the original class. -------------------------------------------------------------------------------- -The Object Life Cycle ---------------------- - -During the normal operation of a Rails application, objects may be created, updated, and destroyed. Active Record provides hooks into this <em>object life cycle</em> so that you can control your application and its data. - -Validations allow you to ensure that only valid data is stored in your database. Callbacks and observers allow you to trigger logic before or after an alteration of an object's state. - Validations Overview -------------------- @@ -973,408 +966,3 @@ end The result looks like the following:  - -Callbacks Overview ------------------- - -Callbacks are methods that get called at certain moments of an object's life cycle. With callbacks it is possible to write code that will run whenever an Active Record object is created, saved, updated, deleted, validated, or loaded from the database. - -### Callback Registration - -In order to use the available callbacks, you need to register them. You can implement the callbacks as ordinary methods and use a macro-style class method to register them as callbacks: - -```ruby -class User < ActiveRecord::Base - validates :login, :email, presence: true - - before_validation :ensure_login_has_a_value - - protected - def ensure_login_has_a_value - if login.nil? - self.login = email unless email.blank? - end - end -end -``` - -The macro-style class methods can also receive a block. Consider using this style if the code inside your block is so short that it fits in a single line: - -```ruby -class User < ActiveRecord::Base - validates :login, :email, presence: true - - before_create do |user| - user.name = user.login.capitalize if user.name.blank? - end -end -``` - -Callbacks can also be registered to only fire on certain lifecycle events: - -```ruby -class User < ActiveRecord::Base - before_validation :normalize_name, on: :create - - # :on takes an array as well - after_validation :set_location, on: [ :create, :update ] - - protected - def normalize_name - self.name = self.name.downcase.titleize - end - - def set_location - self.location = LocationService.query(self) - end -end -``` - -It is considered good practice to declare callback methods as protected or private. If left public, they can be called from outside of the model and violate the principle of object encapsulation. - -Available Callbacks -------------------- - -Here is a list with all the available Active Record callbacks, listed in the same order in which they will get called during the respective operations: - -### Creating an Object - -* `before_validation` -* `after_validation` -* `before_save` -* `around_save` -* `before_create` -* `around_create` -* `after_create` -* `after_save` - -### Updating an Object - -* `before_validation` -* `after_validation` -* `before_save` -* `around_save` -* `before_update` -* `around_update` -* `after_update` -* `after_save` - -### Destroying an Object - -* `before_destroy` -* `around_destroy` -* `after_destroy` - -WARNING. `after_save` runs both on create and update, but always _after_ the more specific callbacks `after_create` and `after_update`, no matter the order in which the macro calls were executed. - -### `after_initialize` and `after_find` - -The `after_initialize` callback will be called whenever an Active Record object is instantiated, either by directly using `new` or when a record is loaded from the database. It can be useful to avoid the need to directly override your Active Record `initialize` method. - -The `after_find` callback will be called whenever Active Record loads a record from the database. `after_find` is called before `after_initialize` if both are defined. - -The `after_initialize` and `after_find` callbacks have no `before_*` counterparts, but they can be registered just like the other Active Record callbacks. - -```ruby -class User < ActiveRecord::Base - after_initialize do |user| - puts "You have initialized an object!" - end - - after_find do |user| - puts "You have found an object!" - end -end - ->> User.new -You have initialized an object! -=> #<User id: nil> - ->> User.first -You have found an object! -You have initialized an object! -=> #<User id: 1> -``` - -Running Callbacks ------------------ - -The following methods trigger callbacks: - -* `create` -* `create!` -* `decrement!` -* `destroy` -* `destroy_all` -* `increment!` -* `save` -* `save!` -* `save(validate: false)` -* `toggle!` -* `update` -* `update_attribute` -* `update_attributes` -* `update_attributes!` -* `valid?` - -Additionally, the `after_find` callback is triggered by the following finder methods: - -* `all` -* `first` -* `find` -* `find_all_by_*` -* `find_by_*` -* `find_by_*!` -* `find_by_sql` -* `last` - -The `after_initialize` callback is triggered every time a new object of the class is initialized. - -NOTE: The `find_all_by_*`, `find_by_*` and `find_by_*!` methods are dynamic finders generated automatically for every attribute. Learn more about them at the [Dynamic finders section](active_record_querying.html#dynamic-finders) - -Skipping Callbacks ------------------- - -Just as with validations, it is also possible to skip callbacks. These methods should be used with caution, however, because important business rules and application logic may be kept in callbacks. Bypassing them without understanding the potential implications may lead to invalid data. - -* `decrement` -* `decrement_counter` -* `delete` -* `delete_all` -* `increment` -* `increment_counter` -* `toggle` -* `touch` -* `update_column` -* `update_columns` -* `update_all` -* `update_counters` - -Halting Execution ------------------ - -As you start registering new callbacks for your models, they will be queued for execution. This queue will include all your model's validations, the registered callbacks, and the database operation to be executed. - -The whole callback chain is wrapped in a transaction. If any <em>before</em> callback method returns exactly `false` or raises an exception, the execution chain gets halted and a ROLLBACK is issued; <em>after</em> callbacks can only accomplish that by raising an exception. - -WARNING. Raising an arbitrary exception may break code that expects `save` and its friends not to fail like that. The `ActiveRecord::Rollback` exception is thought precisely to tell Active Record a rollback is going on. That one is internally captured but not reraised. - -Relational Callbacks --------------------- - -Callbacks work through model relationships, and can even be defined by them. Suppose an example where a user has many posts. A user's posts should be destroyed if the user is destroyed. Let's add an `after_destroy` callback to the `User` model by way of its relationship to the `Post` model: - -```ruby -class User < ActiveRecord::Base - has_many :posts, dependent: :destroy -end - -class Post < ActiveRecord::Base - after_destroy :log_destroy_action - - def log_destroy_action - puts 'Post destroyed' - end -end - ->> user = User.first -=> #<User id: 1> ->> user.posts.create! -=> #<Post id: 1, user_id: 1> ->> user.destroy -Post destroyed -=> #<User id: 1> -``` - -Conditional Callbacks ---------------------- - -As with validations, we can also make the calling of a callback method conditional on the satisfaction of a given predicate. We can do this using the `:if` and `:unless` options, which can take a symbol, a string, a `Proc` or an `Array`. You may use the `:if` option when you want to specify under which conditions the callback **should** be called. If you want to specify the conditions under which the callback **should not** be called, then you may use the `:unless` option. - -### Using `:if` and `:unless` with a `Symbol` - -You can associate the `:if` and `:unless` options with a symbol corresponding to the name of a predicate method that will get called right before the callback. When using the `:if` option, the callback won't be executed if the predicate method returns false; when using the `:unless` option, the callback won't be executed if the predicate method returns true. This is the most common option. Using this form of registration it is also possible to register several different predicates that should be called to check if the callback should be executed. - -```ruby -class Order < ActiveRecord::Base - before_save :normalize_card_number, if: :paid_with_card? -end -``` - -### Using `:if` and `:unless` with a String - -You can also use a string that will be evaluated using `eval` and hence needs to contain valid Ruby code. You should use this option only when the string represents a really short condition: - -```ruby -class Order < ActiveRecord::Base - before_save :normalize_card_number, if: "paid_with_card?" -end -``` - -### Using `:if` and `:unless` with a `Proc` - -Finally, it is possible to associate `:if` and `:unless` with a `Proc` object. This option is best suited when writing short validation methods, usually one-liners: - -```ruby -class Order < ActiveRecord::Base - before_save :normalize_card_number, - if: Proc.new { |order| order.paid_with_card? } -end -``` - -### Multiple Conditions for Callbacks - -When writing conditional callbacks, it is possible to mix both `:if` and `:unless` in the same callback declaration: - -```ruby -class Comment < ActiveRecord::Base - after_create :send_email_to_author, if: :author_wants_emails?, - unless: Proc.new { |comment| comment.post.ignore_comments? } -end -``` - -Callback Classes ----------------- - -Sometimes the callback methods that you'll write will be useful enough to be reused by other models. Active Record makes it possible to create classes that encapsulate the callback methods, so it becomes very easy to reuse them. - -Here's an example where we create a class with an `after_destroy` callback for a `PictureFile` model: - -```ruby -class PictureFileCallbacks - def after_destroy(picture_file) - if File.exists?(picture_file.filepath) - File.delete(picture_file.filepath) - end - end -end -``` - -When declared inside a class, as above, the callback methods will receive the model object as a parameter. We can now use the callback class in the model: - -```ruby -class PictureFile < ActiveRecord::Base - after_destroy PictureFileCallbacks.new -end -``` - -Note that we needed to instantiate a new `PictureFileCallbacks` object, since we declared our callback as an instance method. This is particularly useful if the callbacks make use of the state of the instantiated object. Often, however, it will make more sense to declare the callbacks as class methods: - -```ruby -class PictureFileCallbacks - def self.after_destroy(picture_file) - if File.exists?(picture_file.filepath) - File.delete(picture_file.filepath) - end - end -end -``` - -If the callback method is declared this way, it won't be necessary to instantiate a `PictureFileCallbacks` object. - -```ruby -class PictureFile < ActiveRecord::Base - after_destroy PictureFileCallbacks -end -``` - -You can declare as many callbacks as you want inside your callback classes. - -Observers ---------- - -Observers are similar to callbacks, but with important differences. Whereas callbacks can pollute a model with code that isn't directly related to its purpose, observers allow you to add the same functionality without changing the code of the model. For example, it could be argued that a `User` model should not include code to send registration confirmation emails. Whenever you use callbacks with code that isn't directly related to your model, you may want to consider creating an observer instead. - -### Creating Observers - -For example, imagine a `User` model where we want to send an email every time a new user is created. Because sending emails is not directly related to our model's purpose, we should create an observer to contain the code implementing this functionality. - -```bash -$ rails generate observer User -``` - -generates `app/models/user_observer.rb` containing the observer class `UserObserver`: - -```ruby -class UserObserver < ActiveRecord::Observer -end -``` - -You may now add methods to be called at the desired occasions: - -```ruby -class UserObserver < ActiveRecord::Observer - def after_create(model) - # code to send confirmation email... - end -end -``` - -As with callback classes, the observer's methods receive the observed model as a parameter. - -### Registering Observers - -Observers are conventionally placed inside of your `app/models` directory and registered in your application's `config/application.rb` file. For example, the `UserObserver` above would be saved as `app/models/user_observer.rb` and registered in `config/application.rb` this way: - -```ruby -# Activate observers that should always be running. -config.active_record.observers = :user_observer -``` - -As usual, settings in `config/environments` take precedence over those in `config/application.rb`. So, if you prefer that an observer doesn't run in all environments, you can simply register it in a specific environment instead. - -### Sharing Observers - -By default, Rails will simply strip "Observer" from an observer's name to find the model it should observe. However, observers can also be used to add behavior to more than one model, and thus it is possible to explicitly specify the models that our observer should observe: - -```ruby -class MailerObserver < ActiveRecord::Observer - observe :registration, :user - - def after_create(model) - # code to send confirmation email... - end -end -``` - -In this example, the `after_create` method will be called whenever a `Registration` or `User` is created. Note that this new `MailerObserver` would also need to be registered in `config/application.rb` in order to take effect: - -```ruby -# Activate observers that should always be running. -config.active_record.observers = :mailer_observer -``` - -Transaction Callbacks ---------------------- - -There are two additional callbacks that are triggered by the completion of a database transaction: `after_commit` and `after_rollback`. These callbacks are very similar to the `after_save` callback except that they don't execute until after database changes have either been committed or rolled back. They are most useful when your active record models need to interact with external systems which are not part of the database transaction. - -Consider, for example, the previous example where the `PictureFile` model needs to delete a file after the corresponding record is destroyed. If anything raises an exception after the `after_destroy` callback is called and the transaction rolls back, the file will have been deleted and the model will be left in an inconsistent state. For example, suppose that `picture_file_2` in the code below is not valid and the `save!` method raises an error. - -```ruby -PictureFile.transaction do - picture_file_1.destroy - picture_file_2.save! -end -``` - -By using the `after_commit` callback we can account for this case. - -```ruby -class PictureFile < ActiveRecord::Base - attr_accessor :delete_file - - after_destroy do |picture_file| - picture_file.delete_file = picture_file.filepath - end - - after_commit do |picture_file| - if picture_file.delete_file && File.exist?(picture_file.delete_file) - File.delete(picture_file.delete_file) - picture_file.delete_file = nil - end - end -end -``` - -The `after_commit` and `after_rollback` callbacks are guaranteed to be called for all models created, updated, or destroyed within a transaction block. If any exceptions are raised within one of these callbacks, they will be ignored so that they don't interfere with the other callbacks. As such, if your callback code could raise an exception, you'll need to rescue it and handle it appropriately within the callback. diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index 2baf8c2515..775da2a85c 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -2067,14 +2067,6 @@ The sum of an empty receiver can be customized in this form as well: [].sum(1) {|n| n**3} # => 1 ``` -The method `ActiveRecord::Observer#observed_subclasses` for example is implemented this way: - -```ruby -def observed_subclasses - observed_classes.sum([]) { |klass| klass.send(:subclasses) } -end -``` - NOTE: Defined in `active_support/core_ext/enumerable.rb`. ### `index_by` @@ -3728,6 +3720,25 @@ The auxiliary file is written in a standard directory for temporary files, but y NOTE: Defined in `active_support/core_ext/file/atomic.rb`. +Extensions to `Marshal` +----------------------- + +### `load` + +Active Support adds constant autoloading support to `load`. + +For example, the file cache store deserializes this way: + +```ruby +File.open(file_name) { |f| Marshal.load(f) } +``` + +If the cached data refers to a constant that is unknown at that point, the autoloading mechanism is triggered and if it succeeds the deserialization is retried transparently. + +WARNING. If the argument is an `IO` it needs to respond to `rewind` to be able to retry. Regular files respond to `rewind`. + +NOTE: Defined in `active_support/core_ext/marshal.rb`. + Extensions to `Logger` ---------------------- diff --git a/guides/source/caching_with_rails.md b/guides/source/caching_with_rails.md index 12115a6a47..e737dcab83 100644 --- a/guides/source/caching_with_rails.md +++ b/guides/source/caching_with_rails.md @@ -67,8 +67,6 @@ class ProductsController < ActionController end ``` -If you want a more complicated expiration scheme, you can use cache sweepers to expire cached objects when things change. This is covered in the section on Sweepers. - By default, page caching automatically gzips files (for example, to `products.html.gz` if user requests `/products`) to reduce the size of data transmitted (web servers are typically configured to use a moderate compression ratio as a compromise, but since precompilation happens once, compression ratio is maximum). Nginx is able to serve compressed content directly from disk by enabling `gzip_static`: @@ -176,102 +174,6 @@ This fragment is then available to all actions in the `ProductsController` using expire_fragment('all_available_products') ``` -### Sweepers - -Cache sweeping is a mechanism which allows you to get around having a ton of `expire_{page,action,fragment}` calls in your code. It does this by moving all the work required to expire cached content into an `ActionController::Caching::Sweeper` subclass. This class is an observer and looks for changes to an Active Record object via callbacks, and when a change occurs it expires the caches associated with that object in an around or after filter. - -TIP: Sweepers rely on the use of Active Record and Active Record Observers. The object you are observing must be an Active Record model. - -Continuing with our Product controller example, we could rewrite it with a sweeper like this: - -```ruby -class ProductSweeper < ActionController::Caching::Sweeper - observe Product # This sweeper is going to keep an eye on the Product model - - # If our sweeper detects that a Product was created call this - def after_create(product) - expire_cache_for(product) - end - - # If our sweeper detects that a Product was updated call this - def after_update(product) - expire_cache_for(product) - end - - # If our sweeper detects that a Product was deleted call this - def after_destroy(product) - expire_cache_for(product) - end - - private - def expire_cache_for(product) - # Expire the index page now that we added a new product - expire_page(controller: 'products', action: 'index') - - # Expire a fragment - expire_fragment('all_available_products') - end -end -``` - -You may notice that the actual product gets passed to the sweeper, so if we were caching the edit action for each product, we could add an expire method which specifies the page we want to expire: - -```ruby -expire_action(controller: 'products', action: 'edit', id: product.id) -``` - -Then we add it to our controller to tell it to call the sweeper when certain actions are called. So, if we wanted to expire the cached content for the list and edit actions when the create action was called, we could do the following: - -```ruby -class ProductsController < ActionController - - before_filter :authenticate - caches_action :index - cache_sweeper :product_sweeper - - def index - @products = Product.all - end - -end -``` - -Sometimes it is necessary to disambiguate the controller when you call `expire_action`, such as when there are two identically named controllers in separate namespaces: - -```ruby -class ProductsController < ActionController - caches_action :index - - def index - @products = Product.all - end -end - -module Admin - class ProductsController < ActionController - cache_sweeper :product_sweeper - - def new - @product = Product.new - end - - def create - @product = Product.create(params[:product]) - end - end -end - -class ProductSweeper < ActionController::Caching::Sweeper - observe Product - - def after_create(product) - expire_action(controller: '/products', action: 'index') - end -end -``` - -Note the use of '/products' here rather than 'products'. If you wanted to expire an action cache for the `Admin::ProductsController`, you would use 'admin/products' instead. - ### SQL Caching Query caching is a Rails feature that caches the result set returned by each query so that if Rails encounters the same query again for that request, it will use the cached result set as opposed to running the query against the database again. diff --git a/guides/source/configuring.md b/guides/source/configuring.md index f01051a3fe..dba2be3b71 100644 --- a/guides/source/configuring.md +++ b/guides/source/configuring.md @@ -39,7 +39,7 @@ config.filter_parameters += [:password] This is a setting for Rails itself. If you want to pass settings to individual Rails components, you can do so via the same `config` object in `config/application.rb`: ```ruby -config.active_record.observers = [:hotel_observer, :review_observer] +config.active_record.schema_format = :ruby ``` Rails will use that particular setting to configure Active Record. @@ -616,7 +616,7 @@ Rails.application.config.before_initialize do end ``` -WARNING: Some parts of your application, notably observers and routing, are not yet set up at the point where the `after_initialize` block is called. +WARNING: Some parts of your application, notably routing, are not yet set up at the point where the `after_initialize` block is called. ### `Rails::Railtie#initializer` diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 19425765b8..d7c648681d 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -13,9 +13,13 @@ url: migrations.html description: This guide covers how you can use Active Record migrations to alter your database in a structured and organized manner. - - name: Active Record Validations and Callbacks - url: active_record_validations_callbacks.html - description: This guide covers how you can use Active Record validations and callbacks. + name: Active Record Validations + url: active_record_validations.html + description: This guide covers how you can use Active Record validations + - + name: Active Record Callbacks + url: active_record_callbacks.html + description: This guide covers how you can use Active Record callbacks. - name: Active Record Associations url: association_basics.html diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index f6721c617f..725295004f 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -106,7 +106,7 @@ module Rails # # The <tt>Application</tt> class adds a couple more paths to this set. And as in your # <tt>Application</tt>, all folders under +app+ are automatically added to the load path. - # If you have an <tt>app/observers</tt> folder for example, it will be added by default. + # If you have an <tt>app/services/tt> folder for example, it will be added by default. # # == Endpoint # diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 367f9288b8..d9a91b74d1 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -172,13 +172,11 @@ module Rails "resource_route", "#{orm}:migration", "#{orm}:model", - "#{orm}:observer", "#{test}:controller", "#{test}:helper", "#{test}:integration", "#{test}:mailer", "#{test}:model", - "#{test}:observer", "#{test}:scaffold", "#{test}:view", "#{test}:performance", diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index 5c4e81431c..b96ee9295e 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -78,7 +78,7 @@ module Rails # end # # environment(nil, env: "development") do - # "config.active_record.observers = :cacher" + # "config.autoload_paths += %W(#{config.root}/extras)" # end def environment(data=nil, options={}, &block) sentinel = /class [a-z_:]+ < Rails::Application/i diff --git a/railties/lib/rails/generators/named_base.rb b/railties/lib/rails/generators/named_base.rb index 84f8f76838..cc10fd9177 100644 --- a/railties/lib/rails/generators/named_base.rb +++ b/railties/lib/rails/generators/named_base.rb @@ -169,10 +169,10 @@ module Rails # # ==== Examples # - # check_class_collision suffix: "Observer" + # check_class_collision suffix: "Decorator" # # If the generator is invoked with class name Admin, it will check for - # the presence of "AdminObserver". + # the presence of "AdminDecorator". # def self.check_class_collision(options={}) define_method :check_class_collision do diff --git a/railties/lib/rails/generators/rails/observer/USAGE b/railties/lib/rails/generators/rails/observer/USAGE deleted file mode 100644 index 177ff49e4a..0000000000 --- a/railties/lib/rails/generators/rails/observer/USAGE +++ /dev/null @@ -1,12 +0,0 @@ -Description: - Stubs out a new observer. Pass the observer name, either CamelCased or - under_scored, as an argument. - - This generator only invokes your ORM and test framework generators. - -Example: - `rails generate observer Account` - - For ActiveRecord and TestUnit it creates: - Observer: app/models/account_observer.rb - TestUnit: test/models/account_observer_test.rb diff --git a/railties/lib/rails/generators/rails/observer/observer_generator.rb b/railties/lib/rails/generators/rails/observer/observer_generator.rb deleted file mode 100644 index 7a4d701ac6..0000000000 --- a/railties/lib/rails/generators/rails/observer/observer_generator.rb +++ /dev/null @@ -1,7 +0,0 @@ -module Rails - module Generators - class ObserverGenerator < NamedBase # :nodoc: - hook_for :orm, required: true - end - end -end diff --git a/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile b/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile index 7448b386c5..d69f943a72 100644 --- a/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile +++ b/railties/lib/rails/generators/rails/plugin_new/templates/Gemfile @@ -1,4 +1,4 @@ -source "http://rubygems.org" +source "https://rubygems.org" <% if options[:skip_gemspec] -%> <%= '# ' if options.dev? || options.edge? -%>gem "rails", "~> <%= Rails::VERSION::STRING %>" diff --git a/railties/lib/rails/generators/test_unit/observer/observer_generator.rb b/railties/lib/rails/generators/test_unit/observer/observer_generator.rb deleted file mode 100644 index 64fe694a8b..0000000000 --- a/railties/lib/rails/generators/test_unit/observer/observer_generator.rb +++ /dev/null @@ -1,13 +0,0 @@ -require 'rails/generators/test_unit' - -module TestUnit # :nodoc: - module Generators # :nodoc: - class ObserverGenerator < Base # :nodoc: - check_class_collision suffix: "ObserverTest" - - def create_test_files - template 'unit_test.rb', File.join('test/models', class_path, "#{file_name}_observer_test.rb") - end - end - end -end diff --git a/railties/lib/rails/generators/test_unit/observer/templates/unit_test.rb b/railties/lib/rails/generators/test_unit/observer/templates/unit_test.rb deleted file mode 100644 index 28aa23626a..0000000000 --- a/railties/lib/rails/generators/test_unit/observer/templates/unit_test.rb +++ /dev/null @@ -1,9 +0,0 @@ -require 'test_helper' - -<% module_namespacing do -%> -class <%= class_name %>ObserverTest < ActiveSupport::TestCase - # test "the truth" do - # assert true - # end -end -<% end -%> diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index b9d18f4582..ae1127b509 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -582,27 +582,6 @@ module ApplicationTests assert app.config.colorize_logging end - test "config.active_record.observers" do - add_to_config <<-RUBY - config.active_record.observers = :foo_observer - RUBY - - app_file 'app/models/foo.rb', <<-RUBY - class Foo < ActiveRecord::Base - end - RUBY - - app_file 'app/models/foo_observer.rb', <<-RUBY - class FooObserver < ActiveRecord::Observer - end - RUBY - - require "#{app_path}/config/environment" - - ActiveRecord::Base - assert defined?(FooObserver) - end - test "config.session_store with :active_record_store with activerecord-session_store gem" do begin make_basic_app do |app| diff --git a/railties/test/application/console_test.rb b/railties/test/application/console_test.rb index f372afa51c..3cb3643e3a 100644 --- a/railties/test/application/console_test.rb +++ b/railties/test/application/console_test.rb @@ -95,21 +95,4 @@ class ConsoleTest < ActiveSupport::TestCase load_environment(true) assert value end - - def test_active_record_does_not_panic_when_referencing_an_observed_constant - add_to_config "config.active_record.observers = :user_observer" - - app_file "app/models/user.rb", <<-MODEL - class User < ActiveRecord::Base - end - MODEL - - app_file "app/models/user_observer.rb", <<-MODEL - class UserObserver < ActiveRecord::Observer - end - MODEL - - load_environment - assert_nothing_raised { User } - end end diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index c6aea03d8c..f2234ab111 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -110,7 +110,6 @@ module ApplicationTests app_name = File.basename(app_path) app_dir = File.dirname(app_path) moved_app_name = app_name + '_moved' - moved_app_path = "#{app_path}/#{moved_app_name}" Dir.chdir(app_dir) do # Go from "./app/" to "./app/app_moved" @@ -250,28 +249,6 @@ module ApplicationTests assert !File.exists?(File.join(app_path, 'db', 'schema_cache.dump')) end - def test_load_activerecord_base_when_we_use_observers - Dir.chdir(app_path) do - `bundle exec rails g model user; - bundle exec rake db:migrate; - bundle exec rails g observer user;` - - add_to_config "config.active_record.observers = :user_observer" - - assert_equal "0", `bundle exec rails r "puts User.count"`.strip - - app_file "lib/tasks/count_user.rake", <<-RUBY - namespace :user do - task count: :environment do - puts User.count - end - end - RUBY - - assert_equal "0", `bundle exec rake user:count`.strip - end - end - def test_copy_templates Dir.chdir(app_path) do `bundle exec rake rails:templates:copy` diff --git a/railties/test/generators/namespaced_generators_test.rb b/railties/test/generators/namespaced_generators_test.rb index 9e7626647e..a4d8b3d1b0 100644 --- a/railties/test/generators/namespaced_generators_test.rb +++ b/railties/test/generators/namespaced_generators_test.rb @@ -1,18 +1,19 @@ require 'generators/generators_test_helper' require 'rails/generators/rails/controller/controller_generator' require 'rails/generators/rails/model/model_generator' -require 'rails/generators/rails/observer/observer_generator' require 'rails/generators/mailer/mailer_generator' require 'rails/generators/rails/scaffold/scaffold_generator' class NamespacedGeneratorTestCase < Rails::Generators::TestCase + include GeneratorsTestHelper + def setup + super Rails::Generators.namespace = TestApp end end class NamespacedControllerGeneratorTest < NamespacedGeneratorTestCase - include GeneratorsTestHelper arguments %w(Account foo bar) tests Rails::Generators::ControllerGenerator @@ -81,7 +82,6 @@ class NamespacedControllerGeneratorTest < NamespacedGeneratorTestCase end class NamespacedModelGeneratorTest < NamespacedGeneratorTestCase - include GeneratorsTestHelper arguments %w(Account name:string age:integer) tests Rails::Generators::ModelGenerator @@ -141,29 +141,7 @@ class NamespacedModelGeneratorTest < NamespacedGeneratorTestCase end end -class NamespacedObserverGeneratorTest < NamespacedGeneratorTestCase - include GeneratorsTestHelper - arguments %w(account) - tests Rails::Generators::ObserverGenerator - - def test_invokes_default_orm - run_generator - assert_file "app/models/test_app/account_observer.rb", /module TestApp/, / class AccountObserver < ActiveRecord::Observer/ - end - - def test_invokes_default_orm_with_class_path - run_generator ["admin/account"] - assert_file "app/models/test_app/admin/account_observer.rb", /module TestApp/, / class Admin::AccountObserver < ActiveRecord::Observer/ - end - - def test_invokes_default_test_framework - run_generator - assert_file "test/models/test_app/account_observer_test.rb", /module TestApp/, / class AccountObserverTest < ActiveSupport::TestCase/ - end -end - class NamespacedMailerGeneratorTest < NamespacedGeneratorTestCase - include GeneratorsTestHelper arguments %w(notifier foo bar) tests Rails::Generators::MailerGenerator diff --git a/railties/test/generators/observer_generator_test.rb b/railties/test/generators/observer_generator_test.rb deleted file mode 100644 index 1231827466..0000000000 --- a/railties/test/generators/observer_generator_test.rb +++ /dev/null @@ -1,27 +0,0 @@ -require 'generators/generators_test_helper' -require 'rails/generators/rails/observer/observer_generator' - -class ObserverGeneratorTest < Rails::Generators::TestCase - include GeneratorsTestHelper - arguments %w(account) - - def test_invokes_default_orm - run_generator - assert_file "app/models/account_observer.rb", /class AccountObserver < ActiveRecord::Observer/ - end - - def test_invokes_default_orm_with_class_path - run_generator ["admin/account"] - assert_file "app/models/admin/account_observer.rb", /class Admin::AccountObserver < ActiveRecord::Observer/ - end - - def test_invokes_default_test_framework - run_generator - assert_file "test/models/account_observer_test.rb", /class AccountObserverTest < ActiveSupport::TestCase/ - end - - def test_logs_if_the_test_framework_cannot_be_found - content = run_generator ["account", "--test-framework=rspec"] - assert_match(/rspec \[not found\]/, content) - end -end |