diff options
40 files changed, 356 insertions, 156 deletions
diff --git a/actionpack/lib/action_controller/metal/live.rb b/actionpack/lib/action_controller/metal/live.rb index 27b3eb4e58..6804e577f0 100644 --- a/actionpack/lib/action_controller/metal/live.rb +++ b/actionpack/lib/action_controller/metal/live.rb @@ -223,12 +223,6 @@ module ActionController jar.write self unless committed? end - def before_sending - super - request.cookie_jar.commit! - headers.freeze - end - def build_buffer(response, body) buf = Live::Buffer.new response body.each { |part| buf.write part } diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index b64f660ec5..b8d395854c 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -47,15 +47,10 @@ module Mime def const_missing(sym) ext = sym.downcase if Mime[ext] - ActiveSupport::Deprecation.warn <<-eow -Accessing mime types via constants is deprecated. Please change: - - `Mime::#{sym}` - -to: - - `Mime[:#{ext}]` - eow + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Accessing mime types via constants is deprecated. + Please change `Mime::#{sym}` to `Mime[:#{ext}]`. + MSG Mime[ext] else super @@ -65,15 +60,10 @@ to: def const_defined?(sym, inherit = true) ext = sym.downcase if Mime[ext] - ActiveSupport::Deprecation.warn <<-eow -Accessing mime types via constants is deprecated. Please change: - - `Mime.const_defined?(#{sym})` - -to: - - `Mime[:#{ext}]` - eow + ActiveSupport::Deprecation.warn(<<-MSG.squish) + Accessing mime types via constants is deprecated. + Please change `Mime.const_defined?(#{sym})` to `Mime[:#{ext}]`. + MSG true else super diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 3280799647..29cf821090 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -49,6 +49,10 @@ module ActionDispatch METHOD end + def self.empty + new({}) + end + def initialize(env) super @method = nil @@ -59,6 +63,9 @@ module ActionDispatch @ip = nil end + def commit_cookie_jar! # :nodoc: + end + def check_path_parameters! # If any of the path parameters has an invalid encoding then # raise since it's likely to trigger errors further on. diff --git a/actionpack/lib/action_dispatch/http/response.rb b/actionpack/lib/action_dispatch/http/response.rb index f0127aa276..9b11111a67 100644 --- a/actionpack/lib/action_dispatch/http/response.rb +++ b/actionpack/lib/action_dispatch/http/response.rb @@ -412,6 +412,8 @@ module ActionDispatch # :nodoc: end def before_sending + headers.freeze + request.commit_cookie_jar! unless committed? end def build_buffer(response, body) diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 65baf117ba..3477aa8b29 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -12,6 +12,12 @@ module ActionDispatch end # :stopdoc: + prepend Module.new { + def commit_cookie_jar! + cookie_jar.commit! + end + } + def have_cookie_jar? has_header? 'action_dispatch.cookies'.freeze end diff --git a/actionpack/lib/action_dispatch/testing/assertions/response.rb b/actionpack/lib/action_dispatch/testing/assertions/response.rb index eab20b075d..5af052afb4 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/response.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/response.rb @@ -27,6 +27,8 @@ module ActionDispatch # # Asserts that the response code was status code 401 (unauthorized) # assert_response 401 def assert_response(type, message = nil) + message ||= generate_response_message(type) + if Symbol === type if [:success, :missing, :redirect, :error].include?(type) assert_predicate @response, RESPONSE_PREDICATES[type], message @@ -82,6 +84,19 @@ module ActionDispatch handle._compute_redirect_to_location(@request, fragment) end end + + def generate_response_message(type) + message = "Expected response to be a <#{type}>, but was" + + if @response.redirection? + redirect_is = normalize_argument_to_redirection(@response.location) + message << " a redirect to <#{redirect_is}>" + else + message << " <#{@response.response_code}>" + end + + message + end end end end diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 790f9ea5d2..711ca10419 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -375,6 +375,7 @@ module ActionDispatch @request = ActionDispatch::Request.new(session.last_request.env) response = _mock_session.last_response @response = ActionDispatch::TestResponse.from_response(response) + @response.request = @request @html_document = nil @url_options = nil diff --git a/actionpack/test/assertions/response_assertions_test.rb b/actionpack/test/assertions/response_assertions_test.rb index 82c747680d..6c7036aa1a 100644 --- a/actionpack/test/assertions/response_assertions_test.rb +++ b/actionpack/test/assertions/response_assertions_test.rb @@ -6,7 +6,12 @@ module ActionDispatch class ResponseAssertionsTest < ActiveSupport::TestCase include ResponseAssertions - FakeResponse = Struct.new(:response_code) do + FakeResponse = Struct.new(:response_code, :location) do + def initialize(*) + super + self.location ||= "http://test.example.com/posts" + end + [:successful, :not_found, :redirection, :server_error].each do |sym| define_method("#{sym}?") do sym == response_code @@ -58,6 +63,16 @@ module ActionDispatch assert_response :succezz } end + + def test_message_when_response_is_redirect_but_asserted_for_status_other_than_redirect + @response = FakeResponse.new :redirection, "http://test.host/posts/redirect/1" + error = assert_raises(Minitest::Assertion) do + assert_response :success + end + + expected = "Expected response to be a <success>, but was a redirect to <http://test.host/posts/redirect/1>." + assert_match expected, error.message + end end end end diff --git a/actionpack/test/controller/base_test.rb b/actionpack/test/controller/base_test.rb index fb60dbd993..e3f669dbb5 100644 --- a/actionpack/test/controller/base_test.rb +++ b/actionpack/test/controller/base_test.rb @@ -93,7 +93,7 @@ end class ControllerInstanceTests < ActiveSupport::TestCase def setup @empty = EmptyController.new - @empty.set_request!(ActionDispatch::Request.new({})) + @empty.set_request!(ActionDispatch::Request.empty) @empty.set_response!(EmptyController.make_response!(@empty.request)) @contained = Submodule::ContainedEmptyController.new @empty_controllers = [@empty, @contained] diff --git a/actionpack/test/controller/new_base/bare_metal_test.rb b/actionpack/test/controller/new_base/bare_metal_test.rb index e61f4d241b..c226fa57ee 100644 --- a/actionpack/test/controller/new_base/bare_metal_test.rb +++ b/actionpack/test/controller/new_base/bare_metal_test.rb @@ -26,7 +26,7 @@ module BareMetalTest test "response_body value is wrapped in an array when the value is a String" do controller = BareController.new - controller.set_request!(ActionDispatch::Request.new({})) + controller.set_request!(ActionDispatch::Request.empty) controller.set_response!(BareController.make_response!(controller.request)) controller.index assert_equal ["Hello world"], controller.response_body diff --git a/actionpack/test/dispatch/cookies_test.rb b/actionpack/test/dispatch/cookies_test.rb index 84c244c72a..dfcef14344 100644 --- a/actionpack/test/dispatch/cookies_test.rb +++ b/actionpack/test/dispatch/cookies_test.rb @@ -7,7 +7,7 @@ class CookieJarTest < ActiveSupport::TestCase attr_reader :request def setup - @request = ActionDispatch::Request.new({}) + @request = ActionDispatch::Request.empty end def test_fetch diff --git a/actionpack/test/dispatch/live_response_test.rb b/actionpack/test/dispatch/live_response_test.rb index 160cdc1582..e4475f4233 100644 --- a/actionpack/test/dispatch/live_response_test.rb +++ b/actionpack/test/dispatch/live_response_test.rb @@ -6,7 +6,7 @@ module ActionController class ResponseTest < ActiveSupport::TestCase def setup @response = Live::Response.new - @response.request = ActionDispatch::Request.new({}) #yolo + @response.request = ActionDispatch::Request.empty end def test_header_merge diff --git a/actionpack/test/dispatch/request/session_test.rb b/actionpack/test/dispatch/request/session_test.rb index ae0e7e93ed..7dcbcc5c21 100644 --- a/actionpack/test/dispatch/request/session_test.rb +++ b/actionpack/test/dispatch/request/session_test.rb @@ -7,7 +7,7 @@ module ActionDispatch attr_reader :req def setup - @req = ActionDispatch::Request.new({}) + @req = ActionDispatch::Request.empty end def test_create_adds_itself_to_env diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 08c4554721..7dd9d05e62 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -4,9 +4,13 @@ class BaseRequestTest < ActiveSupport::TestCase def setup @env = { :ip_spoofing_check => true, - :tld_length => 1, "rack.input" => "foo" } + @original_tld_length = ActionDispatch::Http::URL.tld_length + end + + def teardown + ActionDispatch::Http::URL.tld_length = @original_tld_length end def url_for(options = {}) @@ -19,9 +23,9 @@ class BaseRequestTest < ActiveSupport::TestCase ip_spoofing_check = env.key?(:ip_spoofing_check) ? env.delete(:ip_spoofing_check) : true @trusted_proxies ||= nil ip_app = ActionDispatch::RemoteIp.new(Proc.new { }, ip_spoofing_check, @trusted_proxies) - tld_length = env.key?(:tld_length) ? env.delete(:tld_length) : 1 + ActionDispatch::Http::URL.tld_length = env.delete(:tld_length) if env.key?(:tld_length) + ip_app.call(env) - ActionDispatch::Http::URL.tld_length = tld_length env = @env.merge(env) ActionDispatch::Request.new(env) @@ -254,15 +258,6 @@ end class RequestDomain < BaseRequestTest test "domains" do - request = stub_request 'HTTP_HOST' => 'www.rubyonrails.org' - assert_equal "rubyonrails.org", request.domain - - request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk" - assert_equal "rubyonrails.co.uk", request.domain(2) - - request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk", :tld_length => 2 - assert_equal "rubyonrails.co.uk", request.domain - request = stub_request 'HTTP_HOST' => "192.168.1.200" assert_nil request.domain @@ -271,25 +266,18 @@ class RequestDomain < BaseRequestTest request = stub_request 'HTTP_HOST' => "192.168.1.200.com" assert_equal "200.com", request.domain - end - test "subdomains" do - request = stub_request 'HTTP_HOST' => "www.rubyonrails.org" - assert_equal %w( www ), request.subdomains - assert_equal "www", request.subdomain + request = stub_request 'HTTP_HOST' => 'www.rubyonrails.org' + assert_equal "rubyonrails.org", request.domain request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk" - assert_equal %w( www ), request.subdomains(2) - assert_equal "www", request.subdomain(2) - - request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk" - assert_equal %w( dev www ), request.subdomains(2) - assert_equal "dev.www", request.subdomain(2) + assert_equal "rubyonrails.co.uk", request.domain(2) - request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk", :tld_length => 2 - assert_equal %w( dev www ), request.subdomains - assert_equal "dev.www", request.subdomain + request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk", :tld_length => 2 + assert_equal "rubyonrails.co.uk", request.domain + end + test "subdomains" do request = stub_request 'HTTP_HOST' => "foobar.foobar.com" assert_equal %w( foobar ), request.subdomains assert_equal "foobar", request.subdomain @@ -309,6 +297,22 @@ class RequestDomain < BaseRequestTest request = stub_request 'HTTP_HOST' => nil assert_equal [], request.subdomains assert_equal "", request.subdomain + + request = stub_request 'HTTP_HOST' => "www.rubyonrails.org" + assert_equal %w( www ), request.subdomains + assert_equal "www", request.subdomain + + request = stub_request 'HTTP_HOST' => "www.rubyonrails.co.uk" + assert_equal %w( www ), request.subdomains(2) + assert_equal "www", request.subdomain(2) + + request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk" + assert_equal %w( dev www ), request.subdomains(2) + assert_equal "dev.www", request.subdomain(2) + + request = stub_request 'HTTP_HOST' => "dev.www.rubyonrails.co.uk", :tld_length => 2 + assert_equal %w( dev www ), request.subdomains + assert_equal "dev.www", request.subdomain end end diff --git a/actionpack/test/dispatch/response_test.rb b/actionpack/test/dispatch/response_test.rb index 981d820ccf..c37679bc5f 100644 --- a/actionpack/test/dispatch/response_test.rb +++ b/actionpack/test/dispatch/response_test.rb @@ -5,6 +5,7 @@ require 'rack/content_length' class ResponseTest < ActiveSupport::TestCase def setup @response = ActionDispatch::Response.create + @response.request = ActionDispatch::Request.empty end def test_can_wait_until_commit @@ -39,6 +40,7 @@ class ResponseTest < ActiveSupport::TestCase def test_response_body_encoding body = ["hello".encode(Encoding::UTF_8)] response = ActionDispatch::Response.new 200, {}, body + response.request = ActionDispatch::Request.empty assert_equal Encoding::UTF_8, response.body.encoding end @@ -261,6 +263,7 @@ class ResponseTest < ActiveSupport::TestCase test "can be explicitly destructured into status, headers and an enumerable body" do response = ActionDispatch::Response.new(404, { 'Content-Type' => 'text/plain' }, ['Not Found']) + response.request = ActionDispatch::Request.empty status, headers, body = *response assert_equal 404, status @@ -356,6 +359,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest resp.cache_control[:public] = true resp.etag = '123' resp.body = 'Hello' + resp.request = ActionDispatch::Request.empty }.to_a } @@ -392,6 +396,7 @@ class ResponseIntegrationTest < ActionDispatch::IntegrationTest resp.charset = 'utf-16' resp.content_type = Mime[:xml] resp.body = 'Hello' + resp.request = ActionDispatch::Request.empty }.to_a } diff --git a/activemodel/test/cases/translation_test.rb b/activemodel/test/cases/translation_test.rb index cedc812ec7..2c89388f14 100644 --- a/activemodel/test/cases/translation_test.rb +++ b/activemodel/test/cases/translation_test.rb @@ -88,6 +88,11 @@ class ActiveModelI18nTests < ActiveModel::TestCase assert_equal 'child model', Child.model_name.human end + def test_translated_model_with_namespace + I18n.backend.store_translations 'en', activemodel: { models: { 'person/gender': 'gender model' } } + assert_equal 'gender model', Person::Gender.model_name.human + end + def test_translated_model_names_with_ancestors_fallback I18n.backend.store_translations 'en', activemodel: { models: { person: 'person model' } } assert_equal 'person model', Child.model_name.human diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index fdc95f718a..56a3232ee9 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,21 @@ +* Introduce after_{create,update,delete}_commit callbacks. + + Before: + + after_commit :add_to_index_later, on: :create + after_commit :update_in_index_later, on: :update + after_commit :remove_from_index_later, on: :destroy + + After: + + after_create_commit :add_to_index_later + after_update_commit :update_in_index_later + after_destroy_commit :remove_from_index_later + + Fixes #22515. + + *Genadi Samokovarov* + * Respect the column default values for `inheritance_column` when instantiating records through the base class. diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 81c535d962..f02d146e89 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -106,8 +106,7 @@ module ActiveRecord::Associations::Builder # :nodoc: touch = reflection.options[:touch] callback = lambda { |record| - touch_method = touching_delayed_records? ? :touch : :touch_later - BelongsTo.touch_record(record, foreign_key, n, touch, touch_method) + BelongsTo.touch_record(record, foreign_key, n, touch, belongs_to_touch_method) } model.after_save callback, if: :changed? diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 46c6d8c293..9e566031b8 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -298,6 +298,7 @@ module ActiveRecord # * \Validations are skipped. # * \Callbacks are skipped. # * +updated_at+/+updated_on+ are not updated. + # * However, attributes are serialized with the same rules as ActiveRecord::Relation#update_all # # This method raises an ActiveRecord::ActiveRecordError when called on new # objects, or when at least one of the attributes is marked as readonly. @@ -566,5 +567,9 @@ module ActiveRecord ensure @_association_destroy_exception = nil end + + def belongs_to_touch_method + :touch + end end end diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index f100476374..2cf19c76c5 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -347,9 +347,8 @@ module ActiveRecord # Updates all records in the current relation with details given. This method constructs a single SQL UPDATE # statement and sends it straight to the database. It does not instantiate the involved models and it does not - # trigger Active Record callbacks or validations. Values passed to #update_all will not go through - # Active Record's type-casting behavior. It should receive only values that can be passed as-is to the SQL - # database. + # trigger Active Record callbacks or validations. However, values passed to #update_all will still go through + # Active Record's normal type casting and serialization. # # ==== Parameters # diff --git a/activerecord/lib/active_record/touch_later.rb b/activerecord/lib/active_record/touch_later.rb index 4352a0ffea..9a80a63e28 100644 --- a/activerecord/lib/active_record/touch_later.rb +++ b/activerecord/lib/active_record/touch_later.rb @@ -16,6 +16,13 @@ module ActiveRecord surreptitiously_touch @_defer_touch_attrs self.class.connection.add_transaction_record self + + # touch the parents as we are not calling the after_save callbacks + self.class.reflect_on_all_associations(:belongs_to).each do |r| + if touch = r.options[:touch] + ActiveRecord::Associations::Builder::BelongsTo.touch_record(self, r.foreign_key, r.name, touch, :touch_later) + end + end end def touch(*names, time: nil) # :nodoc: @@ -26,6 +33,7 @@ module ActiveRecord end private + def surreptitiously_touch(attrs) attrs.each { |attr| write_attribute attr, @_touch_time } clear_attribute_changes attrs @@ -33,9 +41,8 @@ module ActiveRecord def touch_deferred_attributes if has_defer_touch_attrs? && persisted? - @_touching_delayed_records = true touch(*@_defer_touch_attrs, time: @_touch_time) - @_touching_delayed_records, @_defer_touch_attrs, @_touch_time = nil, nil, nil + @_defer_touch_attrs, @_touch_time = nil, nil end end @@ -43,8 +50,9 @@ module ActiveRecord defined?(@_defer_touch_attrs) && @_defer_touch_attrs.present? end - def touching_delayed_records? - defined?(@_touching_delayed_records) && @_touching_delayed_records + def belongs_to_touch_method + :touch_later end + end end diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index 8de82feae3..38ab1f3fc6 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -233,6 +233,24 @@ module ActiveRecord set_callback(:commit, :after, *args, &block) end + # Shortcut for +after_commit :hook, on: :create+. + def after_create_commit(*args, &block) + set_options_for_callbacks!(args, on: :create) + set_callback(:commit, :after, *args, &block) + end + + # Shortcut for +after_commit :hook, on: :update+. + def after_update_commit(*args, &block) + set_options_for_callbacks!(args, on: :update) + set_callback(:commit, :after, *args, &block) + end + + # Shortcut for +after_commit :hook, on: :destroy+. + def after_destroy_commit(*args, &block) + set_options_for_callbacks!(args, on: :destroy) + set_callback(:commit, :after, *args, &block) + end + # This callback is called after a create, update, or destroy are rolled back. # # Please check the documentation of #after_commit for options. @@ -268,9 +286,11 @@ module ActiveRecord private - def set_options_for_callbacks!(args) - options = args.last - if options.is_a?(Hash) && options[:on] + def set_options_for_callbacks!(args, enforced_options = {}) + options = args.extract_options!.merge!(enforced_options) + args << options + + if options[:on] fire_on = Array(options[:on]) assert_valid_transaction_action(fire_on) options[:if] = Array(options[:if]) diff --git a/activerecord/test/cases/adapters/postgresql/geometric_test.rb b/activerecord/test/cases/adapters/postgresql/geometric_test.rb index 8d0c5bf23f..3b97cb4ad4 100644 --- a/activerecord/test/cases/adapters/postgresql/geometric_test.rb +++ b/activerecord/test/cases/adapters/postgresql/geometric_test.rb @@ -262,7 +262,9 @@ class PostgreSQLGeometricLineTest < ActiveRecord::PostgreSQLTestCase end teardown do - @connection.drop_table 'postgresql_lines', if_exists: true + if defined?(@connection) + @connection.drop_table 'postgresql_lines', if_exists: true + end end def test_geometric_line_type diff --git a/activerecord/test/cases/touch_later_test.rb b/activerecord/test/cases/touch_later_test.rb index 7058f4fbe2..07dbb4b8f8 100644 --- a/activerecord/test/cases/touch_later_test.rb +++ b/activerecord/test/cases/touch_later_test.rb @@ -95,8 +95,6 @@ class TouchLaterTest < ActiveRecord::TestCase end def test_touching_three_deep - skip "Pending from #19324" - previous_tree_updated_at = trees(:root).updated_at previous_grandparent_updated_at = nodes(:grandparent).updated_at previous_parent_updated_at = nodes(:parent_a).updated_at diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index f2229939c8..637f89196e 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -35,9 +35,9 @@ class TransactionCallbacksTest < ActiveRecord::TestCase has_many :replies, class_name: "ReplyWithCallbacks", foreign_key: "parent_id" after_commit { |record| record.do_after_commit(nil) } - after_commit(on: :create) { |record| record.do_after_commit(:create) } - after_commit(on: :update) { |record| record.do_after_commit(:update) } - after_commit(on: :destroy) { |record| record.do_after_commit(:destroy) } + after_create_commit { |record| record.do_after_commit(:create) } + after_update_commit { |record| record.do_after_commit(:update) } + after_destroy_commit { |record| record.do_after_commit(:destroy) } after_rollback { |record| record.do_after_rollback(nil) } after_rollback(on: :create) { |record| record.do_after_rollback(:create) } after_rollback(on: :update) { |record| record.do_after_rollback(:update) } diff --git a/activesupport/lib/active_support/cache/mem_cache_store.rb b/activesupport/lib/active_support/cache/mem_cache_store.rb index 36f1ba2713..174913365a 100644 --- a/activesupport/lib/active_support/cache/mem_cache_store.rb +++ b/activesupport/lib/active_support/cache/mem_cache_store.rb @@ -36,13 +36,13 @@ module ActiveSupport end def write_entry(key, entry, options) # :nodoc: - retval = super - if options[:raw] && local_cache && retval + if options[:raw] && local_cache raw_entry = Entry.new(entry.value.to_s) raw_entry.expires_at = entry.expires_at - local_cache.write_entry(key, raw_entry, options) + super(key, raw_entry, options) + else + super end - retval end end diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 7798c7ec60..c53f9c1039 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -42,8 +42,8 @@ module ActiveSupport listeners_for(name).each { |s| s.start(name, id, payload) } end - def finish(name, id, payload) - listeners_for(name).each { |s| s.finish(name, id, payload) } + def finish(name, id, payload, listeners = listeners_for(name)) + listeners.each { |s| s.finish(name, id, payload) } end def publish(name, *args) diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index 075ddc2382..67f2ee1a7f 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -15,14 +15,15 @@ module ActiveSupport # and publish it. Notice that events get sent even if an error occurs # in the passed-in block. def instrument(name, payload={}) - start name, payload + # some of the listeners might have state + listeners_state = start name, payload begin yield payload rescue Exception => e payload[:exception] = [e.class.name, e.message] raise e ensure - finish name, payload + finish_with_state listeners_state, name, payload end end @@ -36,6 +37,10 @@ module ActiveSupport @notifier.finish name, @id, payload end + def finish_with_state(listeners_state, name, payload) + @notifier.finish name, @id, payload, listeners_state + end + private def unique_id diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 7bef73136c..a1bd2d5356 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -655,6 +655,14 @@ module LocalCacheBehavior end end + def test_local_cache_of_read_nil + @cache.with_local_cache do + assert_equal nil, @cache.read('foo') + @cache.send(:bypass_local_cache) { @cache.write 'foo', 'bar' } + assert_equal nil, @cache.read('foo') + end + end + def test_local_cache_of_delete @cache.with_local_cache do @cache.write('foo', 'bar') diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index f729f0a95b..d9cc392ac9 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -42,6 +42,21 @@ module Notifications ActiveSupport::Notifications.instrument(name) assert_equal expected, events end + + def test_subsribing_to_instrumentation_while_inside_it + # the repro requires that there are no evented subscribers for the "foo" event, + # so we have to duplicate some of the setup code + old_notifier = ActiveSupport::Notifications.notifier + ActiveSupport::Notifications.notifier = ActiveSupport::Notifications::Fanout.new + + ActiveSupport::Notifications.subscribe('foo', TestSubscriber.new) + + ActiveSupport::Notifications.instrument('foo') do + ActiveSupport::Notifications.subscribe('foo') {} + end + ensure + ActiveSupport::Notifications.notifier = old_notifier + end end class UnsubscribeTest < TestCase diff --git a/guides/source/active_record_callbacks.md b/guides/source/active_record_callbacks.md index 13989a3b33..b5ad3e9411 100644 --- a/guides/source/active_record_callbacks.md +++ b/guides/source/active_record_callbacks.md @@ -412,4 +412,23 @@ end NOTE: the `:on` option specifies when a callback will be fired. If you don't supply the `:on` option the callback will fire for every action. +Since using `after_commit` callback only on create, update or delete is +common, there are aliases for those operations: + +* `after_create_commit` +* `after_update_commit` +* `after_destroy_commit` + +```ruby +class PictureFile < ActiveRecord::Base + after_destroy_commit :delete_picture_file_from_disk + + def delete_picture_file_from_disk + if File.exist?(filepath) + File.delete(filepath) + end + end +end +``` + WARNING. 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/railties/CHANGELOG.md b/railties/CHANGELOG.md index d20ec75b61..709e473b58 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,10 @@ +* Add Command infrastructure to replace rake. + + Also move `rake dev:cache` to new infrastructure. You'll need to use + `rails dev:cache` to toggle development caching from now on. + + *Chuck Callebs* + * Allow use of minitest-rails gem with Rails test runner. Fixes #22455. diff --git a/railties/lib/rails/command.rb b/railties/lib/rails/command.rb new file mode 100644 index 0000000000..6587984b53 --- /dev/null +++ b/railties/lib/rails/command.rb @@ -0,0 +1,70 @@ +require 'rails/commands/commands_tasks' + +module Rails + class Command + attr_reader :argv + + def initialize(argv = []) + @argv = argv + + @option_parser = build_option_parser + @options = {} + end + + def self.run(task_name, argv) + command_name = command_name_for(task_name) + + if command = command_for(command_name) + command.new(argv).run(command_name) + else + Rails::CommandsTasks.new(argv).run_command!(task_name) + end + end + + def run(command_name) + parse_options_for(command_name) + @option_parser.parse! @argv + + public_send(command_name) + end + + def self.options_for(command_name, &options_to_parse) + @@command_options[command_name] = options_to_parse + end + + def self.set_banner(command_name, banner) + options_for(command_name) { |opts, _| opts.banner = banner } + end + + private + @@commands = [] + @@command_options = {} + + def parse_options_for(command_name) + @@command_options.fetch(command_name, proc {}).call(@option_parser, @options) + end + + def build_option_parser + OptionParser.new do |opts| + opts.on('-h', '--help', 'Show this help.') do + puts opts + exit + end + end + end + + def self.inherited(command) + @@commands << command + end + + def self.command_name_for(task_name) + task_name.gsub(':', '_').to_sym + end + + def self.command_for(command_name) + @@commands.find do |command| + command.public_instance_methods.include?(command_name) + end + end + end +end diff --git a/railties/lib/rails/commands.rb b/railties/lib/rails/commands.rb index 12bd73db24..b9c4e02ca0 100644 --- a/railties/lib/rails/commands.rb +++ b/railties/lib/rails/commands.rb @@ -13,6 +13,7 @@ aliases = { command = ARGV.shift command = aliases[command] || command -require 'rails/commands/commands_tasks' +require 'rails/command' +require 'rails/commands/dev_cache' -Rails::CommandsTasks.new(ARGV).run_command!(command) +Rails::Command.run(command, ARGV) diff --git a/railties/lib/rails/commands/commands_tasks.rb b/railties/lib/rails/commands/commands_tasks.rb index 685d55eea8..7e6b49e2a3 100644 --- a/railties/lib/rails/commands/commands_tasks.rb +++ b/railties/lib/rails/commands/commands_tasks.rb @@ -36,10 +36,9 @@ EOT def run_command!(command) command = parse_command(command) + if COMMAND_WHITELIST.include?(command) send(command) - else - write_error_message(command) end end @@ -151,26 +150,6 @@ EOT puts HELP_MESSAGE end - # Output an error message stating that the attempted command is not a valid rails command. - # Run the attempted command as a rake command with the --dry-run flag. If successful, suggest - # to the user that they possibly meant to run the given rails command as a rake command. - # Append the help message. - # - # Example: - # $ rails db:migrate - # Error: Command 'db:migrate' not recognized - # Did you mean: `$ rake db:migrate` ? - # (Help message output) - # - def write_error_message(command) - puts "Error: Command '#{command}' not recognized" - if %x{rake #{command} --dry-run 2>&1 } && $?.success? - puts "Did you mean: `$ rake #{command}` ?\n\n" - end - write_help_message - exit(1) - end - def parse_command(command) case command when '--version', '-v' diff --git a/railties/lib/rails/commands/dev_cache.rb b/railties/lib/rails/commands/dev_cache.rb new file mode 100644 index 0000000000..ec96e8f630 --- /dev/null +++ b/railties/lib/rails/commands/dev_cache.rb @@ -0,0 +1,21 @@ +require 'rails/command' + +module Rails + module Commands + # This is a wrapper around the Rails dev:cache command + class DevCache < Command + set_banner :dev_cache, 'Toggle development mode caching on/off' + def dev_cache + if File.exist? 'tmp/caching-dev.txt' + File.delete 'tmp/caching-dev.txt' + puts 'Development mode is no longer being cached.' + else + FileUtils.touch 'tmp/caching-dev.txt' + puts 'Development mode is now being cached.' + end + + FileUtils.touch 'tmp/restart.txt' + end + end + end +end diff --git a/railties/lib/rails/tasks.rb b/railties/lib/rails/tasks.rb index d3e33584d7..d60eaf6f4f 100644 --- a/railties/lib/rails/tasks.rb +++ b/railties/lib/rails/tasks.rb @@ -3,7 +3,6 @@ require 'rake' # Load Rails Rakefile extensions %w( annotations - dev framework initializers log diff --git a/railties/lib/rails/tasks/dev.rake b/railties/lib/rails/tasks/dev.rake deleted file mode 100644 index 4593100465..0000000000 --- a/railties/lib/rails/tasks/dev.rake +++ /dev/null @@ -1,14 +0,0 @@ -namespace :dev do - desc 'Toggle development mode caching on/off' - task :cache do - if File.exist? 'tmp/caching-dev.txt' - File.delete 'tmp/caching-dev.txt' - puts 'Development mode is no longer being cached.' - else - FileUtils.touch 'tmp/caching-dev.txt' - puts 'Development mode is now being cached.' - end - - FileUtils.touch 'tmp/restart.txt' - end -end diff --git a/railties/test/application/rake/dev_test.rb b/railties/test/application/rake/dev_test.rb deleted file mode 100644 index 28d8b22a37..0000000000 --- a/railties/test/application/rake/dev_test.rb +++ /dev/null @@ -1,35 +0,0 @@ -require 'isolation/abstract_unit' - -module ApplicationTests - module RakeTests - class RakeDevTest < ActiveSupport::TestCase - include ActiveSupport::Testing::Isolation - - def setup - build_app - boot_rails - end - - def teardown - teardown_app - end - - test 'dev:cache creates file and outputs message' do - Dir.chdir(app_path) do - output = `rake dev:cache` - assert File.exist?('tmp/caching-dev.txt') - assert_match(/Development mode is now being cached/, output) - end - end - - test 'dev:cache deletes file and outputs message' do - Dir.chdir(app_path) do - output = `rake dev:cache` - output = `rake dev:cache` - assert_not File.exist?('tmp/caching-dev.txt') - assert_match(/Development mode is no longer being cached/, output) - end - end - end - end -end diff --git a/railties/test/commands/dev_cache_test.rb b/railties/test/commands/dev_cache_test.rb new file mode 100644 index 0000000000..f3612070c6 --- /dev/null +++ b/railties/test/commands/dev_cache_test.rb @@ -0,0 +1,32 @@ +require_relative '../isolation/abstract_unit' + +module CommandsTests + class DevCacheTest < ActiveSupport::TestCase + include ActiveSupport::Testing::Isolation + + def setup + build_app + end + + def teardown + teardown_app + end + + test 'dev:cache creates file and outputs message' do + Dir.chdir(app_path) do + output = `rails dev:cache` + assert File.exist?('tmp/caching-dev.txt') + assert_match(%r{Development mode is now being cached}, output) + end + end + + test 'dev:cache deletes file and outputs message' do + Dir.chdir(app_path) do + output = `rails dev:cache` + output = `rails dev:cache` + assert_not File.exist?('tmp/caching-dev.txt') + assert_match(%r{Development mode is no longer being cached}, output) + end + end + end +end |