diff options
34 files changed, 378 insertions, 467 deletions
diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index fd8b38054e..880263ce87 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,5 +1,11 @@ ## Rails 4.0.0 (unreleased) ## +* Add 'X-Frame-Options' => 'SAMEORIGIN' and + 'X-XSS-Protection' => '1; mode=block' + as default headers. + + *Egor Homakov* + * Allow data attributes to be set as a first-level option for form_for, so you can write `form_for @record, data: { behavior: 'autosave' }` instead of `form_for @record, html: { data: { behavior: 'autosave' } }` *DHH* * Deprecate `button_to_function` and `link_to_function` helpers. diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 43a9e3aa9d..d8fe43b5af 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -33,13 +33,13 @@ module ActionController module Live class Buffer < ActionDispatch::Response::Buffer #:nodoc: def initialize(response) - super(response, Queue.new) + super(response, SizedQueue.new(10)) end def write(string) unless @response.committed? @response.headers["Cache-Control"] = "no-cache" - @response.headers.delete("Content-Length") + @response.headers.delete "Content-Length" end super @@ -47,13 +47,13 @@ module ActionController def each while str = @buf.pop - yield(str) + yield str end end def close super - @buf.push(nil) + @buf.push nil end end @@ -78,7 +78,7 @@ module ActionController end def initialize(status = 200, header = {}, body = []) - header = Header.new(self, header) + header = Header.new self, header super(status, header, body) end @@ -89,11 +89,11 @@ module ActionController private - def build_buffer(response, body) - buf = Live::Buffer.new(response) - body.each { |part| buf.write(part) } - buf - end + def build_buffer(response, body) + buf = Live::Buffer.new response + body.each { |part| buf.write part } + buf + end end def process(name) diff --git a/actionpack/lib/action_dispatch/http/parameters.rb b/actionpack/lib/action_dispatch/http/parameters.rb index bcfd0b0d00..9a7b5bc8c7 100644 --- a/actionpack/lib/action_dispatch/http/parameters.rb +++ b/actionpack/lib/action_dispatch/http/parameters.rb @@ -4,6 +4,11 @@ require 'active_support/core_ext/hash/indifferent_access' module ActionDispatch module Http module Parameters + def initialize(env) + super + @symbolized_path_params = nil + end + # Returns both GET and POST \parameters in a single hash. def parameters @env["action_dispatch.request.parameters"] ||= begin diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 1377e53ce8..d24c7c7f3f 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -38,6 +38,17 @@ module ActionDispatch METHOD end + def initialize(env) + super + @method = nil + @request_method = nil + @remote_ip = nil + @original_fullpath = nil + @fullpath = nil + @ip = nil + @uuid = nil + end + def key?(key) @env.key?(key) end diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index d336808e7c..5014ad80aa 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -58,6 +58,7 @@ module ActionDispatch # :nodoc: LOCATION = "Location".freeze cattr_accessor(:default_charset) { "utf-8" } + cattr_accessor(:default_headers) include Rack::Response::Helpers include ActionDispatch::Http::Cache::Response @@ -96,6 +97,10 @@ module ActionDispatch # :nodoc: def initialize(status = 200, header = {}, body = []) super() + if self.class.default_headers.respond_to?(:merge) + header = self.class.default_headers.merge(header) + end + self.body, self.header, self.status = body, header, status @sending_file = false diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 4266ec042e..8aa02ec482 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -87,6 +87,12 @@ module ActionDispatch end end + def initialize(env) + super + @protocol = nil + @port = nil + end + # Returns the complete URL used for this request. def url protocol + host_with_port + fullpath diff --git a/actionpack/lib/action_dispatch/railtie.rb b/actionpack/lib/action_dispatch/railtie.rb index 62f906219c..e7f3f07390 100644 --- a/actionpack/lib/action_dispatch/railtie.rb +++ b/actionpack/lib/action_dispatch/railtie.rb @@ -23,6 +23,7 @@ module ActionDispatch ActionDispatch::Http::URL.tld_length = app.config.action_dispatch.tld_length ActionDispatch::Request.ignore_accept_header = app.config.action_dispatch.ignore_accept_header ActionDispatch::Response.default_charset = app.config.action_dispatch.default_charset || app.config.encoding + ActionDispatch::Response.default_headers = app.config.action_dispatch.default_headers ActionDispatch::ExceptionWrapper.rescue_responses.merge!(config.action_dispatch.rescue_responses) ActionDispatch::ExceptionWrapper.rescue_templates.merge!(config.action_dispatch.rescue_templates) diff --git a/actionpack/lib/action_view/helpers/sanitize_helper.rb b/actionpack/lib/action_view/helpers/sanitize_helper.rb index a727b910e5..aaf0e0344a 100644 --- a/actionpack/lib/action_view/helpers/sanitize_helper.rb +++ b/actionpack/lib/action_view/helpers/sanitize_helper.rb @@ -78,7 +78,7 @@ module ActionView # strip_tags("<div id='top-bar'>Welcome to my website!</div>") # # => Welcome to my website! def strip_tags(html) - self.class.full_sanitizer.sanitize(html).try(:html_safe) + self.class.full_sanitizer.sanitize(html) end # Strips all link tags from +text+ leaving just the link text. diff --git a/actionpack/test/abstract_unit.rb b/actionpack/test/abstract_unit.rb index 56cebee678..b914bbce4d 100644 --- a/actionpack/test/abstract_unit.rb +++ b/actionpack/test/abstract_unit.rb @@ -85,39 +85,28 @@ module RenderERBUtils end end -module SetupOnce - extend ActiveSupport::Concern - - included do - cattr_accessor :setup_once_block - self.setup_once_block = nil - - setup :run_setup_once - end +SharedTestRoutes = ActionDispatch::Routing::RouteSet.new - module ClassMethods - def setup_once(&block) - self.setup_once_block = block +module ActionDispatch + module SharedRoutes + def before_setup + @routes = SharedTestRoutes + super end end - private - def run_setup_once - if self.setup_once_block - self.setup_once_block.call - self.setup_once_block = nil - end + # Hold off drawing routes until all the possible controller classes + # have been loaded. + module DrawOnce + class << self + attr_accessor :drew end -end + self.drew = false -SharedTestRoutes = ActionDispatch::Routing::RouteSet.new + def before_setup + super + return if DrawOnce.drew -module ActiveSupport - class TestCase - include SetupOnce - # Hold off drawing routes until all the possible controller classes - # have been loaded. - setup_once do SharedTestRoutes.draw do get ':controller(/:action)' end @@ -125,10 +114,18 @@ module ActiveSupport ActionDispatch::IntegrationTest.app.routes.draw do get ':controller(/:action)' end + + DrawOnce.drew = true end end end +module ActiveSupport + class TestCase + include ActionDispatch::DrawOnce + end +end + class RoutedRackApp attr_reader :routes @@ -159,9 +156,7 @@ class BasicController end class ActionDispatch::IntegrationTest < ActiveSupport::TestCase - setup do - @routes = SharedTestRoutes - end + include ActionDispatch::SharedRoutes def self.build_app(routes = nil) RoutedRackApp.new(routes || ActionDispatch::Routing::RouteSet.new) do |middleware| @@ -290,10 +285,7 @@ module ActionController class TestCase include ActionDispatch::TestProcess - - setup do - @routes = SharedTestRoutes - end + include ActionDispatch::SharedRoutes end end @@ -304,9 +296,7 @@ module ActionView class TestCase # Must repeat the setup because AV::TestCase is a duplication # of AC::TestCase - setup do - @routes = SharedTestRoutes - end + include ActionDispatch::SharedRoutes end end diff --git a/actionpack/test/controller/caching_test.rb b/actionpack/test/controller/caching_test.rb index d5afef9086..0efba5b77f 100644 --- a/actionpack/test/controller/caching_test.rb +++ b/actionpack/test/controller/caching_test.rb @@ -349,15 +349,18 @@ class ActionCachingMockController end class ActionCacheTest < ActionController::TestCase + tests ActionCachingTestController + def setup super - reset! + @request.host = 'hostname.com' FileUtils.mkdir_p(FILE_STORE_PATH) @path_class = ActionController::Caching::Actions::ActionCachePath @mock_controller = ActionCachingMockController.new end def teardown + super FileUtils.rm_rf(File.dirname(FILE_STORE_PATH)) end @@ -367,7 +370,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_equal cached_time, @response.body assert fragment_exist?('hostname.com/action_caching_test') - reset! get :index assert_response :success @@ -380,7 +382,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_equal cached_time, @response.body assert !fragment_exist?('hostname.com/action_caching_test/destroy') - reset! get :destroy assert_response :success @@ -395,7 +396,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_not_equal cached_time, @response.body assert fragment_exist?('hostname.com/action_caching_test/with_layout') - reset! get :with_layout assert_response :success @@ -410,7 +410,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_not_equal cached_time, @response.body assert fragment_exist?('hostname.com/action_caching_test/layout_false') - reset! get :layout_false assert_response :success @@ -425,7 +424,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_not_equal cached_time, @response.body assert fragment_exist?('hostname.com/action_caching_test/with_layout_proc_param') - reset! get :with_layout_proc_param, :layout => false assert_response :success @@ -440,7 +438,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_not_equal cached_time, @response.body assert fragment_exist?('hostname.com/action_caching_test/with_layout_proc_param') - reset! get :with_layout_proc_param, :layout => true assert_response :success @@ -477,7 +474,6 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_equal cached_time, @response.body assert fragment_exist?('test.host/custom/show') - reset! get :show assert_response :success @@ -488,7 +484,6 @@ class ActionCacheTest < ActionController::TestCase get :edit assert_response :success assert fragment_exist?('test.host/edit') - reset! get :edit, :id => 1 assert_response :success @@ -499,22 +494,18 @@ class ActionCacheTest < ActionController::TestCase get :index assert_response :success cached_time = content_to_cache - reset! get :index assert_response :success assert_equal cached_time, @response.body - reset! get :expire assert_response :success - reset! get :index assert_response :success new_cached_time = content_to_cache assert_not_equal cached_time, @response.body - reset! get :index assert_response :success @@ -524,12 +515,10 @@ class ActionCacheTest < ActionController::TestCase def test_cache_expiration_isnt_affected_by_request_format get :index cached_time = content_to_cache - reset! @request.request_uri = "/action_caching_test/expire.xml" get :expire, :format => :xml assert_response :success - reset! get :index assert_response :success @@ -539,12 +528,10 @@ class ActionCacheTest < ActionController::TestCase def test_cache_expiration_with_url_string get :index cached_time = content_to_cache - reset! @request.request_uri = "/action_caching_test/expire_with_url_string" get :expire_with_url_string assert_response :success - reset! get :index assert_response :success @@ -557,23 +544,17 @@ class ActionCacheTest < ActionController::TestCase assert_response :success jamis_cache = content_to_cache - reset! - @request.host = 'david.hostname.com' get :index assert_response :success david_cache = content_to_cache assert_not_equal jamis_cache, @response.body - reset! - @request.host = 'jamis.hostname.com' get :index assert_response :success assert_equal jamis_cache, @response.body - reset! - @request.host = 'david.hostname.com' get :index assert_response :success @@ -583,8 +564,6 @@ class ActionCacheTest < ActionController::TestCase def test_redirect_is_not_cached get :redirected assert_response :redirect - reset! - get :redirected assert_response :redirect end @@ -592,8 +571,6 @@ class ActionCacheTest < ActionController::TestCase def test_forbidden_is_not_cached get :forbidden assert_response :forbidden - reset! - get :forbidden assert_response :forbidden end @@ -609,17 +586,14 @@ class ActionCacheTest < ActionController::TestCase cached_time = content_to_cache assert_equal cached_time, @response.body assert fragment_exist?('hostname.com/action_caching_test/index.xml') - reset! get :index, :format => 'xml' assert_response :success assert_equal cached_time, @response.body assert_equal 'application/xml', @response.content_type - reset! get :expire_xml assert_response :success - reset! get :index, :format => 'xml' assert_response :success @@ -724,14 +698,6 @@ class ActionCacheTest < ActionController::TestCase assigns(:cache_this) end - def reset! - @request = ActionController::TestRequest.new - @response = ActionController::TestResponse.new - @controller = ActionCachingTestController.new - @controller.singleton_class.send(:include, @routes.url_helpers) - @request.host = 'hostname.com' - end - def fragment_exist?(path) @controller.fragment_exist?(path) end diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 2467654a70..347b3b3b5a 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -190,7 +190,7 @@ class CookiesTest < ActionController::TestCase def test_setting_the_same_value_to_permanent_cookie request.cookies[:user_name] = 'Jamie' get :set_permanent_cookie - assert response.cookies, 'user_name' => 'Jamie' + assert_equal response.cookies, 'user_name' => 'Jamie' end def test_setting_with_escapable_characters diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index e2903d4b36..71609d7340 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -176,6 +176,33 @@ class ResponseTest < ActiveSupport::TestCase ActionDispatch::Response.default_charset = original end end + + test "read x_frame_options and x_xss_protection" do + ActionDispatch::Response.default_headers = { + 'X-Frame-Options' => 'DENY', + 'X-XSS-Protection' => '1;' + } + resp = ActionDispatch::Response.new.tap { |response| + response.body = 'Hello' + } + resp.to_a + + assert_equal('DENY', resp.headers['X-Frame-Options']) + assert_equal('1;', resp.headers['X-XSS-Protection']) + end + + test "read custom default_header" do + ActionDispatch::Response.default_headers = { + 'X-XX-XXXX' => 'Here is my phone number' + } + resp = ActionDispatch::Response.new.tap { |response| + response.body = 'Hello' + } + resp.to_a + + assert_equal('Here is my phone number', resp.headers['X-XX-XXXX']) + end + end class ResponseIntegrationTest < ActionDispatch::IntegrationTest diff --git a/actionpack/test/template/sanitize_helper_test.rb b/actionpack/test/template/sanitize_helper_test.rb index 4182af590e..7626cdf386 100644 --- a/actionpack/test/template/sanitize_helper_test.rb +++ b/actionpack/test/template/sanitize_helper_test.rb @@ -40,9 +40,9 @@ class SanitizeHelperTest < ActionView::TestCase [nil, '', ' '].each do |blank| stripped = strip_tags(blank) assert_equal blank, stripped - assert stripped.html_safe? unless blank.nil? end - assert strip_tags("<script>").html_safe? + assert_equal "", strip_tags("<script>") + assert_equal "something <img onerror=alert(1337)", ERB::Util.html_escape(strip_tags("something <img onerror=alert(1337)")) end def test_sanitize_is_marked_safe diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index a99d7fdde8..e6237ef437 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -486,24 +486,13 @@ * Added the `ActiveRecord::NullRelation` class implementing the null object pattern for the Relation class. *Juanjo Bazán* -* Added deprecation for the `:dependent => :restrict` association option. +* Added new `:dependent => :restrict_with_error` option. This will add + an error to the model, rather than raising an exception. - Please note: - - * Up until now `has_many` and `has_one`, `:dependent => :restrict` - option raised a `DeleteRestrictionError` at the time of destroying - the object. Instead, it will add an error on the model. - - * To fix this warning, make sure your code isn't relying on a - `DeleteRestrictionError` and then add - `config.active_record.dependent_restrict_raises = false` to your - application config. - - * New rails application would be generated with the - `config.active_record.dependent_restrict_raises = false` in the - application config. + The `:restrict` option is renamed to `:restrict_with_exception` to + make this distinction explicit. - *Manoj Kumar* + *Manoj Kumar & Jon Leighton* * Added `create_join_table` migration helper to create HABTM join tables diff --git a/activerecord/lib/active_record/associations.rb b/activerecord/lib/active_record/associations.rb index f5ee4f3ebe..d9857b8fbd 100644 --- a/activerecord/lib/active_record/associations.rb +++ b/activerecord/lib/active_record/associations.rb @@ -1094,12 +1094,14 @@ module ActiveRecord # [:primary_key] # Specify the method that returns the primary key used for the association. By default this is +id+. # [:dependent] - # If set to <tt>:destroy</tt> all the associated objects are destroyed - # alongside this object by calling their +destroy+ method. If set to <tt>:delete_all</tt> all associated - # objects are deleted *without* calling their +destroy+ method. If set to <tt>:nullify</tt> all associated - # objects' foreign keys are set to +NULL+ *without* calling their +save+ callbacks. If set to - # <tt>:restrict</tt> an error will be added to the object, preventing its deletion, if any associated - # objects are present. + # Controls what happens to the associated objects when + # their owner is destroyed: + # + # * <tt>:destroy</tt> causes all the associated objects to also be destroyed + # * <tt>:delete_all</tt> causes all the asssociated objects to be deleted directly from the database (so callbacks will not execute) + # * <tt>:nullify</tt> causes the foreign keys to be set to +NULL+. Callbacks are not executed. + # * <tt>:restrict_with_exception</tt> causes an exception to be raised if there are any associated records + # * <tt>:restrict_with_error</tt> causes an error to be added to the owner if there are any associated objects # # If using with the <tt>:through</tt> option, the association on the join model must be # a +belongs_to+, and the records which get deleted are the join records, rather than @@ -1203,11 +1205,14 @@ module ActiveRecord # from the association name. So <tt>has_one :manager</tt> will by default be linked to the Manager class, but # if the real class name is Person, you'll have to specify it with this option. # [:dependent] - # If set to <tt>:destroy</tt>, the associated object is destroyed when this object is. If set to - # <tt>:delete</tt>, the associated object is deleted *without* calling its destroy method. - # If set to <tt>:nullify</tt>, the associated object's foreign key is set to +NULL+. - # If set to <tt>:restrict</tt>, an error will be added to the object, preventing its deletion, if an - # associated object is present. + # Controls what happens to the associated objects when + # their owner is destroyed: + # + # * <tt>:destroy</tt> causes all the associated objects to also be destroyed + # * <tt>:delete</tt> causes all the asssociated objects to be deleted directly from the database (so callbacks will not execute) + # * <tt>:nullify</tt> causes the foreign keys to be set to +NULL+. Callbacks are not executed. + # * <tt>:restrict_with_exception</tt> causes an exception to be raised if there are any associated records + # * <tt>:restrict_with_error</tt> causes an error to be added to the owner if there are any associated objects # [:foreign_key] # Specify the foreign key used for the association. By default this is guessed to be the name # of this class in lower-case and "_id" suffixed. So a Person class that makes a +has_one+ association diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index ddfc6f6c05..75f72c1a46 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -2,6 +2,11 @@ module ActiveRecord # = Active Record Belongs To Associations module Associations class BelongsToAssociation < SingularAssociation #:nodoc: + + def handle_dependency + target.send(options[:dependent]) if load_target + end + def replace(record) raise_on_type_mismatch(record) if record diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index c3f32b5ed9..cae640648b 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -39,6 +39,7 @@ module ActiveRecord::Associations::Builder def build validate_options define_accessors + configure_dependency if options[:dependent] @reflection = model.create_reflection(macro, name, scope, options, model) super # provides an extension point @reflection @@ -52,70 +53,51 @@ module ActiveRecord::Associations::Builder Association.valid_options end - private + def validate_options + options.assert_valid_keys(valid_options) + end - def validate_options - options.assert_valid_keys(valid_options) - end + def define_accessors + define_readers + define_writers + end - def define_accessors - define_readers - define_writers + def define_readers + name = self.name + mixin.redefine_method(name) do |*params| + association(name).reader(*params) end + end - def define_readers - name = self.name - mixin.redefine_method(name) do |*params| - association(name).reader(*params) - end + def define_writers + name = self.name + mixin.redefine_method("#{name}=") do |value| + association(name).writer(value) end + end - def define_writers - name = self.name - mixin.redefine_method("#{name}=") do |value| - association(name).writer(value) - end + def configure_dependency + unless valid_dependent_options.include? options[:dependent] + raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{options[:dependent]}" end - def check_valid_dependent!(dependent, valid_options) - unless valid_options.include?(dependent) - valid_options_message = valid_options.map(&:inspect).to_sentence( - words_connector: ', ', two_words_connector: ' or ', last_word_connector: ' or ') - - raise ArgumentError, "The :dependent option expects either " \ - "#{valid_options_message} (#{dependent.inspect})" - end + if options[:dependent] == :restrict + ActiveSupport::Deprecation.warn( + "The :restrict option is deprecated. Please use :restrict_with_exception instead, which " \ + "provides the same functionality." + ) end - def dependent_restrict_raises? - ActiveRecord::Base.dependent_restrict_raises == true + name = self.name + mixin.redefine_method("#{macro}_dependent_for_#{name}") do + association(name).handle_dependency end - def dependent_restrict_deprecation_warning - if dependent_restrict_raises? - msg = "In the next release, `:dependent => :restrict` will not raise a `DeleteRestrictionError`. "\ - "Instead, it will add an error on the model. To fix this warning, make sure your code " \ - "isn't relying on a `DeleteRestrictionError` and then add " \ - "`config.active_record.dependent_restrict_raises = false` to your application config." - ActiveSupport::Deprecation.warn msg - end - end + model.before_destroy "#{macro}_dependent_for_#{name}" + end - def define_restrict_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - has_one_macro = association(name).reflection.macro == :has_one - if has_one_macro ? !send(name).nil? : send(name).exists? - if dependent_restrict_raises? - raise ActiveRecord::DeleteRestrictionError.new(name) - else - key = has_one_macro ? "one" : "many" - errors.add(:base, :"restrict_dependent_destroy.#{key}", - :record => self.class.human_attribute_name(name).downcase) - return false - end - end - end - end + def valid_dependent_options + raise NotImplementedError + end end end diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index f205a456f7..acfc3073a9 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: def macro @@ -17,72 +16,58 @@ module ActiveRecord::Associations::Builder reflection = super add_counter_cache_callbacks(reflection) if options[:counter_cache] add_touch_callbacks(reflection) if options[:touch] - configure_dependency reflection end - private + def add_counter_cache_callbacks(reflection) + cache_column = reflection.counter_cache_column + name = self.name - def add_counter_cache_callbacks(reflection) - cache_column = reflection.counter_cache_column - name = self.name + method_name = "belongs_to_counter_cache_after_create_for_#{name}" + mixin.redefine_method(method_name) do + record = send(name) + record.class.increment_counter(cache_column, record.id) unless record.nil? + end + model.after_create(method_name) - method_name = "belongs_to_counter_cache_after_create_for_#{name}" - mixin.redefine_method(method_name) do + method_name = "belongs_to_counter_cache_before_destroy_for_#{name}" + mixin.redefine_method(method_name) do + unless marked_for_destruction? record = send(name) - record.class.increment_counter(cache_column, record.id) unless record.nil? + record.class.decrement_counter(cache_column, record.id) unless record.nil? end - model.after_create(method_name) - - method_name = "belongs_to_counter_cache_before_destroy_for_#{name}" - mixin.redefine_method(method_name) do - unless marked_for_destruction? - record = send(name) - record.class.decrement_counter(cache_column, record.id) unless record.nil? - end - end - model.before_destroy(method_name) - - model.send(:module_eval, - "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)", __FILE__, __LINE__ - ) end + model.before_destroy(method_name) + + model.send(:module_eval, + "#{reflection.class_name}.send(:attr_readonly,\"#{cache_column}\".intern) if defined?(#{reflection.class_name}) && #{reflection.class_name}.respond_to?(:attr_readonly)", __FILE__, __LINE__ + ) + end - def add_touch_callbacks(reflection) - name = self.name - method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}" - touch = options[:touch] + def add_touch_callbacks(reflection) + name = self.name + method_name = "belongs_to_touch_after_save_or_destroy_for_#{name}" + touch = options[:touch] - mixin.redefine_method(method_name) do - record = send(name) + mixin.redefine_method(method_name) do + record = send(name) - unless record.nil? - if touch == true - record.touch - else - record.touch(touch) - end + unless record.nil? + if touch == true + record.touch + else + record.touch(touch) end end - - model.after_save(method_name) - model.after_touch(method_name) - model.after_destroy(method_name) end - def configure_dependency - if dependent = options[:dependent] - check_valid_dependent! dependent, [:destroy, :delete] + model.after_save(method_name) + model.after_touch(method_name) + model.after_destroy(method_name) + end - method_name = "belongs_to_dependent_#{dependent}_for_#{name}" - model.send(:class_eval, <<-eoruby, __FILE__, __LINE__ + 1) - def #{method_name} - association = #{name} - association.#{dependent} if association - end - eoruby - model.after_destroy method_name - end - end + def valid_dependent_options + [:destroy, :delete] + end end end diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 3fb0a57450..40c13dae43 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class CollectionAssociation < Association #:nodoc: CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] @@ -34,53 +33,51 @@ module ActiveRecord::Associations::Builder end end - private - - def wrap_block_extension - if block_extension - @extension_module = mod = Module.new(&block_extension) - silence_warnings do - model.parent.const_set(extension_module_name, mod) - end + def wrap_block_extension + if block_extension + @extension_module = mod = Module.new(&block_extension) + silence_warnings do + model.parent.const_set(extension_module_name, mod) + end - prev_scope = @scope + prev_scope = @scope - if prev_scope - @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) } - else - @scope = proc { extending(mod) } - end + if prev_scope + @scope = proc { |owner| instance_exec(owner, &prev_scope).extending(mod) } + else + @scope = proc { extending(mod) } end end + end - def extension_module_name - @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" - end + def extension_module_name + @extension_module_name ||= "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" + end - def define_callback(callback_name) - full_callback_name = "#{callback_name}_for_#{name}" + def define_callback(callback_name) + full_callback_name = "#{callback_name}_for_#{name}" - # TODO : why do i need method_defined? I think its because of the inheritance chain - model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name) - model.send("#{full_callback_name}=", Array(options[callback_name.to_sym])) - end + # TODO : why do i need method_defined? I think its because of the inheritance chain + model.class_attribute full_callback_name.to_sym unless model.method_defined?(full_callback_name) + model.send("#{full_callback_name}=", Array(options[callback_name.to_sym])) + end - def define_readers - super + def define_readers + super - name = self.name - mixin.redefine_method("#{name.to_s.singularize}_ids") do - association(name).ids_reader - end + name = self.name + mixin.redefine_method("#{name.to_s.singularize}_ids") do + association(name).ids_reader end + end - def define_writers - super + def define_writers + super - name = self.name - mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids| - association(name).ids_writer(ids) - end + name = self.name + mixin.redefine_method("#{name.to_s.singularize}_ids=") do |ids| + association(name).ids_writer(ids) end + end end end diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 8df28ad876..c2b53b9b2c 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -24,18 +24,16 @@ module ActiveRecord::Associations::Builder end end - private - - def define_destroy_hook - name = self.name - model.send(:include, Module.new { - class_eval <<-RUBY, __FILE__, __LINE__ + 1 - def destroy_associations - association(#{name.to_sym.inspect}).delete_all - super - end - RUBY - }) - end + def define_destroy_hook + name = self.name + model.send(:include, Module.new { + class_eval <<-RUBY, __FILE__, __LINE__ + 1 + def destroy_associations + association(#{name.to_sym.inspect}).delete_all + super + end + RUBY + }) + end end end diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 9e60dbc30b..ab8225460a 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: def macro @@ -9,52 +8,8 @@ module ActiveRecord::Associations::Builder super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of] end - def build - reflection = super - configure_dependency - reflection + def valid_dependent_options + [:destroy, :delete_all, :nullify, :restrict, :restrict_with_error, :restrict_with_exception] end - - private - - def configure_dependency - if dependent = options[:dependent] - check_valid_dependent! dependent, [:destroy, :delete_all, :nullify, :restrict] - dependent_restrict_deprecation_warning if dependent == :restrict - - send("define_#{dependent}_dependency_method") - model.before_destroy dependency_method_name - end - end - - def define_destroy_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - send(name).each do |o| - # No point in executing the counter update since we're going to destroy the parent anyway - o.mark_for_destruction - end - - send(name).delete_all - end - end - - def define_delete_all_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - association(name).delete_all - end - end - - def define_nullify_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - send(name).delete_all - end - end - - def dependency_method_name - "has_many_dependent_for_#{name}" - end end end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 9c84f1913a..0da564f402 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,4 +1,3 @@ - module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: def macro @@ -15,35 +14,12 @@ module ActiveRecord::Associations::Builder !options[:through] end - def build - reflection = super - configure_dependency unless options[:through] - reflection + def configure_dependency + super unless options[:through] end - private - - def configure_dependency - if dependent = options[:dependent] - check_valid_dependent! dependent, [:destroy, :delete, :nullify, :restrict] - dependent_restrict_deprecation_warning if dependent == :restrict - - send("define_#{dependent}_dependency_method") - model.before_destroy dependency_method_name - end - end - - def define_destroy_dependency_method - name = self.name - mixin.redefine_method(dependency_method_name) do - association(name).delete - end - end - alias :define_delete_dependency_method :define_destroy_dependency_method - alias :define_nullify_dependency_method :define_destroy_dependency_method - - def dependency_method_name - "has_one_dependent_#{options[:dependent]}_for_#{name}" - end + def valid_dependent_options + [:destroy, :delete, :nullify, :restrict, :restrict_with_error, :restrict_with_exception] + end end end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index 90a4b7c2ef..0f96929a29 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -13,22 +13,20 @@ module ActiveRecord::Associations::Builder define_constructors if constructable? end - private + def define_constructors + name = self.name - def define_constructors - name = self.name - - mixin.redefine_method("build_#{name}") do |*params, &block| - association(name).build(*params, &block) - end + mixin.redefine_method("build_#{name}") do |*params, &block| + association(name).build(*params, &block) + end - mixin.redefine_method("create_#{name}") do |*params, &block| - association(name).create(*params, &block) - end + mixin.redefine_method("create_#{name}") do |*params, &block| + association(name).create(*params, &block) + end - mixin.redefine_method("create_#{name}!") do |*params, &block| - association(name).create!(*params, &block) - end + mixin.redefine_method("create_#{name}!") do |*params, &block| + association(name).create!(*params, &block) end + end end end diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb index 7a363c896e..74864d271f 100644 --- a/activerecord/lib/active_record/associations/has_many_association.rb +++ b/activerecord/lib/active_record/associations/has_many_association.rb @@ -7,6 +7,28 @@ module ActiveRecord # is provided by its child HasManyThroughAssociation. class HasManyAssociation < CollectionAssociation #:nodoc: + def handle_dependency + case options[:dependent] + when :restrict, :restrict_with_exception + raise ActiveRecord::DeleteRestrictionError.new(reflection.name) unless empty? + + when :restrict_with_error + unless empty? + record = klass.human_attribute_name(reflection.name).downcase + owner.errors.add(:base, :"restrict_dependent_destroy.many", record: record) + false + end + + else + if options[:dependent] == :destroy + # No point in executing the counter update since we're going to destroy the parent anyway + load_target.each(&:mark_for_destruction) + end + + delete_all + end + end + def insert_record(record, validate = true, raise = false) set_owner_attributes(record) diff --git a/activerecord/lib/active_record/associations/has_one_association.rb b/activerecord/lib/active_record/associations/has_one_association.rb index 191c4e8e39..dd7da59a86 100644 --- a/activerecord/lib/active_record/associations/has_one_association.rb +++ b/activerecord/lib/active_record/associations/has_one_association.rb @@ -3,23 +3,43 @@ module ActiveRecord # = Active Record Belongs To Has One Association module Associations class HasOneAssociation < SingularAssociation #:nodoc: - def replace(record, save = true) - raise_on_type_mismatch(record) if record - load_target - reflection.klass.transaction do - if target && target != record - remove_target!(options[:dependent]) unless target.destroyed? + def handle_dependency + case options[:dependent] + when :restrict, :restrict_with_exception + raise ActiveRecord::DeleteRestrictionError.new(reflection.name) if load_target + + when :restrict_with_error + if load_target + record = klass.human_attribute_name(reflection.name).downcase + owner.errors.add(:base, :"restrict_dependent_destroy.one", record: record) + false end - if record - set_owner_attributes(record) - set_inverse_instance(record) + else + delete + end + end - if owner.persisted? && save && !record.save - nullify_owner_attributes(record) - set_owner_attributes(target) if target - raise RecordNotSaved, "Failed to save the new associated #{reflection.name}." + def replace(record, save = true) + raise_on_type_mismatch(record) if record + load_target + + # If target and record are nil, or target is equal to record, + # we don't need to have transaction. + if (target || record) && target != record + reflection.klass.transaction do + remove_target!(options[:dependent]) if target && !target.destroyed? + + if record + set_owner_attributes(record) + set_inverse_instance(record) + + if owner.persisted? && save && !record.save + nullify_owner_attributes(record) + set_owner_attributes(target) if target + raise RecordNotSaved, "Failed to save the new associated #{reflection.name}." + end end end end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index 1145d2138c..0fddfdf0cb 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -82,15 +82,6 @@ module ActiveRecord # The connection handler config_attribute :connection_handler - ## - # :singleton-method: - # Specifies whether or not has_many or has_one association option - # :dependent => :restrict raises an exception. If set to true, the - # ActiveRecord::DeleteRestrictionError exception will be raised - # along with a DEPRECATION WARNING. If set to false, an error would - # be added to the model instead. - config_attribute :dependent_restrict_raises - %w(logger configurations default_timezone schema_format timestamped_migrations).each do |name| config_attribute name, global: true end diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 43440c1146..04714f42e9 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -1091,9 +1091,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase end def test_restrict - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true - firm = RestrictedFirm.create!(:name => 'restrict') firm.companies.create(:name => 'child') @@ -1101,15 +1098,25 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } assert RestrictedFirm.exists?(:name => 'restrict') assert firm.companies.exists?(:name => 'child') - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end - def test_restrict_when_dependent_restrict_raises_config_set_to_false - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = false + def test_restrict_is_deprecated + klass = Class.new(ActiveRecord::Base) + assert_deprecated { klass.has_many :posts, dependent: :restrict } + end - firm = RestrictedFirm.create!(:name => 'restrict') + def test_restrict_with_exception + firm = RestrictedWithExceptionFirm.create!(:name => 'restrict') + firm.companies.create(:name => 'child') + + assert !firm.companies.empty? + assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } + assert RestrictedWithExceptionFirm.exists?(:name => 'restrict') + assert firm.companies.exists?(:name => 'child') + end + + def test_restrict_with_error + firm = RestrictedWithErrorFirm.create!(:name => 'restrict') firm.companies.create(:name => 'child') assert !firm.companies.empty? @@ -1119,10 +1126,8 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert !firm.errors.empty? assert_equal "Cannot delete record because dependent companies exist", firm.errors[:base].first - assert RestrictedFirm.exists?(:name => 'restrict') + assert RestrictedWithErrorFirm.exists?(:name => 'restrict') assert firm.companies.exists?(:name => 'child') - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end def test_included_in_collection @@ -1602,18 +1607,6 @@ class HasManyAssociationsTest < ActiveRecord::TestCase assert_equal [bulb1, bulb3], result end - def test_building_has_many_association_with_restrict_dependency - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true - - klass = Class.new(ActiveRecord::Base) - - assert_deprecated { klass.has_many :companies, :dependent => :restrict } - assert_not_deprecated { klass.has_many :companies } - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before - end - def test_collection_association_with_private_kernel_method firm = companies(:first_firm) assert_equal [accounts(:signals37)], firm.accounts.open diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index 112735839f..8bc633f2b5 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -156,10 +156,7 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_nothing_raised { firm.destroy } end - def test_dependence_with_restrict - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true - + def test_restrict firm = RestrictedFirm.create!(:name => 'restrict') firm.create_account(:credit_limit => 10) @@ -168,38 +165,26 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } assert RestrictedFirm.exists?(:name => 'restrict') assert firm.account.present? - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end - def test_dependence_with_restrict_with_dependent_restrict_raises_config_set_to_false - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = false + def test_restrict_is_deprecated + klass = Class.new(ActiveRecord::Base) + assert_deprecated { klass.has_one :post, dependent: :restrict } + end - firm = RestrictedFirm.create!(:name => 'restrict') + def test_restrict_with_exception + firm = RestrictedWithExceptionFirm.create!(:name => 'restrict') firm.create_account(:credit_limit => 10) assert_not_nil firm.account - firm.destroy - - assert !firm.errors.empty? - assert_equal "Cannot delete record because a dependent account exists", firm.errors[:base].first - assert RestrictedFirm.exists?(:name => 'restrict') + assert_raise(ActiveRecord::DeleteRestrictionError) { firm.destroy } + assert RestrictedWithExceptionFirm.exists?(:name => 'restrict') assert firm.account.present? - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before end - def test_dependence_with_restrict_with_dependent_restrict_raises_config_set_to_false_and_attribute_name - old_backend = I18n.backend - I18n.backend = I18n::Backend::Simple.new - I18n.backend.store_translations 'en', :activerecord => {:attributes => {:restricted_firm => {:account => "account model"}}} - - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = false - - firm = RestrictedFirm.create!(:name => 'restrict') + def test_restrict_with_error + firm = RestrictedWithErrorFirm.create!(:name => 'restrict') firm.create_account(:credit_limit => 10) assert_not_nil firm.account @@ -207,12 +192,9 @@ class HasOneAssociationsTest < ActiveRecord::TestCase firm.destroy assert !firm.errors.empty? - assert_equal "Cannot delete record because a dependent account model exists", firm.errors[:base].first - assert RestrictedFirm.exists?(:name => 'restrict') + assert_equal "Cannot delete record because a dependent account exists", firm.errors[:base].first + assert RestrictedWithErrorFirm.exists?(:name => 'restrict') assert firm.account.present? - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before - I18n.backend = old_backend end def test_successful_build_association @@ -524,15 +506,16 @@ class HasOneAssociationsTest < ActiveRecord::TestCase assert_equal car.id, bulb.attributes_after_initialize['car_id'] end - def test_building_has_one_association_with_dependent_restrict - option_before = ActiveRecord::Base.dependent_restrict_raises - ActiveRecord::Base.dependent_restrict_raises = true + def test_has_one_transaction + company = companies(:first_firm) + account = Account.find(1) - klass = Class.new(ActiveRecord::Base) + company.account # force loading + assert_no_queries { company.account = account } - assert_deprecated { klass.has_one :account, :dependent => :restrict } - assert_not_deprecated { klass.has_one :account } - ensure - ActiveRecord::Base.dependent_restrict_raises = option_before + company.account = nil + assert_no_queries { company.account = nil } + account = Account.find(2) + assert_queries { company.account = account } end end diff --git a/activerecord/test/cases/helper.rb b/activerecord/test/cases/helper.rb index 018064233a..4c6d4666ed 100644 --- a/activerecord/test/cases/helper.rb +++ b/activerecord/test/cases/helper.rb @@ -19,9 +19,6 @@ require 'support/connection' # Show backtraces for deprecated behavior for quicker cleanup. ActiveSupport::Deprecation.debug = true -# Avoid deprecation warning setting dependent_restrict_raises to false. The default is true -ActiveRecord::Base.dependent_restrict_raises = false - # Connect to the database ARTest.connect diff --git a/activerecord/test/models/company.rb b/activerecord/test/models/company.rb index 5bfbb5e855..75f38d275c 100644 --- a/activerecord/test/models/company.rb +++ b/activerecord/test/models/company.rb @@ -115,8 +115,20 @@ class DependentFirm < Company end class RestrictedFirm < Company - has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict - has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict + ActiveSupport::Deprecation.silence do + has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict + has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict + end +end + +class RestrictedWithExceptionFirm < Company + has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict_with_exception + has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict_with_exception +end + +class RestrictedWithErrorFirm < Company + has_one :account, -> { order("id") }, :foreign_key => "firm_id", :dependent => :restrict_with_error + has_many :companies, -> { order("id") }, :foreign_key => 'client_of', :dependent => :restrict_with_error end class Client < Company diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 3e5a53b0ff..6229b89a91 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,13 +1,6 @@ ## Rails 4.0.0 (unreleased) ## -* ActiveSupport::JSON::Variable is deprecated. Define your own #as_json and #encode_json methods - for custom JSON string literals. *Erich Menge* - -* Add String#indent. *fxn & Ace Suares* - -* Inflections can now be defined per locale. `singularize` and `pluralize` accept locale as an extra argument. *David Celis* - -* `Object#try` will now return nil instead of raise a NoMethodError if the receiving object does not implement the method, but you can still get the old behavior by using the new `Object#try!` *DHH* +* `ERB::Util.html_escape` now escapes single quotes. *Santiago Pastorino* * `Time#change` now works with time values with offsets other than UTC or the local time zone. *Andrew White* diff --git a/railties/lib/rails/generators/rails/app/templates/config/application.rb b/railties/lib/rails/generators/rails/app/templates/config/application.rb index 1ee90e88f2..a952ff7fb0 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/application.rb +++ b/railties/lib/rails/generators/rails/app/templates/config/application.rb @@ -41,6 +41,11 @@ module <%= app_const_base %> # Configure sensitive parameters which will be filtered from the log file. config.filter_parameters += [:password] + config.action_dispatch.default_headers = { + 'X-Frame-Options' => 'SAMEORIGIN', + 'X-XSS-Protection' => '1; mode=block' + } + # Use SQL instead of Active Record's schema dumper when creating the database. # This is necessary if your schema can't be completely dumped by the schema dumper, # like if you have constraints or database-specific column types. @@ -51,11 +56,6 @@ module <%= app_const_base %> # in your app. As such, your models will need to explicitly whitelist or blacklist accessible # parameters by using an attr_accessible or attr_protected declaration. <%= comment_if :skip_active_record %>config.active_record.whitelist_attributes = true - - # Specifies whether or not has_many or has_one association option :dependent => :restrict raises - # an exception. If set to true, then an ActiveRecord::DeleteRestrictionError exception would be - # raised. If set to false, then an error will be added on the model instead. - <%= comment_if :skip_active_record %>config.active_record.dependent_restrict_raises = false <% unless options.skip_sprockets? -%> # Enable the asset pipeline. diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 907065f75c..c294bfb238 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -213,7 +213,6 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_file "config/database.yml" assert_file "config/application.rb", /#\s+require\s+["']active_record\/railtie["']/ assert_file "config/application.rb", /#\s+config\.active_record\.whitelist_attributes = true/ - assert_file "config/application.rb", /#\s+config\.active_record\.dependent_restrict_raises = false/ assert_file "test/test_helper.rb" do |helper_content| assert_no_match(/fixtures :all/, helper_content) end @@ -367,11 +366,6 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_file "config/application.rb", /config\.active_record\.whitelist_attributes = true/ end - def test_active_record_dependent_restrict_raises_is_present_application_config - run_generator - assert_file "config/application.rb", /config\.active_record\.dependent_restrict_raises = false/ - end - def test_pretend_option output = run_generator [File.join(destination_root, "myapp"), "--pretend"] assert_no_match(/run bundle install/, output) diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 6071cd3f39..8f04692aef 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -253,7 +253,6 @@ module TestHelpers :activerecord] - arr if to_remove.include? :activerecord remove_from_config "config.active_record.whitelist_attributes = true" - remove_from_config "config.active_record.dependent_restrict_raises = false" end $:.reject! {|path| path =~ %r'/(#{to_remove.join('|')})/' } end |