diff options
75 files changed, 934 insertions, 517 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index b8bebe4d42..ab4fcd123d 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -28,3 +28,5 @@ engines: ratings: paths: - "**.rb" + +exclude_paths: [] diff --git a/actionmailbox/README.md b/actionmailbox/README.md index 9a47223d3b..593bd429ae 100644 --- a/actionmailbox/README.md +++ b/actionmailbox/README.md @@ -1,6 +1,6 @@ # Action Mailbox -Action Mailbox routes incoming emails to controller-like mailboxes for processing in Rails. It ships with ingresses for Amazon SES, Mailgun, Mandrill, Postmark, and SendGrid. You can also handle inbound mails directly via the built-in Exim, Postfix, and Qmail ingresses. +Action Mailbox routes incoming emails to controller-like mailboxes for processing in Rails. It ships with ingresses for Mailgun, Mandrill, Postmark, and SendGrid. You can also handle inbound mails directly via the built-in Exim, Postfix, and Qmail ingresses. The inbound emails are turned into `InboundEmail` records using Active Record and feature lifecycle tracking, storage of the original email on cloud storage via Active Storage, and responsible data handling with on-by-default incineration. diff --git a/actionmailbox/app/controllers/action_mailbox/base_controller.rb b/actionmailbox/app/controllers/action_mailbox/base_controller.rb index 92477b86a8..80a14355b7 100644 --- a/actionmailbox/app/controllers/action_mailbox/base_controller.rb +++ b/actionmailbox/app/controllers/action_mailbox/base_controller.rb @@ -7,10 +7,6 @@ module ActionMailbox before_action :ensure_configured - def self.prepare - # Override in concrete controllers to run code on load. - end - private def ensure_configured unless ActionMailbox.ingress == ingress_name diff --git a/actionmailbox/app/controllers/action_mailbox/ingresses/amazon/inbound_emails_controller.rb b/actionmailbox/app/controllers/action_mailbox/ingresses/amazon/inbound_emails_controller.rb deleted file mode 100644 index e0a187054e..0000000000 --- a/actionmailbox/app/controllers/action_mailbox/ingresses/amazon/inbound_emails_controller.rb +++ /dev/null @@ -1,54 +0,0 @@ -# frozen_string_literal: true - -module ActionMailbox - # Ingests inbound emails from Amazon's Simple Email Service (SES). - # - # Requires the full RFC 822 message in the +content+ parameter. Authenticates requests by validating their signatures. - # - # Returns: - # - # - <tt>204 No Content</tt> if an inbound email is successfully recorded and enqueued for routing to the appropriate mailbox - # - <tt>401 Unauthorized</tt> if the request's signature could not be validated - # - <tt>404 Not Found</tt> if Action Mailbox is not configured to accept inbound emails from SES - # - <tt>422 Unprocessable Entity</tt> if the request is missing the required +content+ parameter - # - <tt>500 Server Error</tt> if one of the Active Record database, the Active Storage service, or - # the Active Job backend is misconfigured or unavailable - # - # == Usage - # - # 1. Install the {aws-sdk-sns}[https://rubygems.org/gems/aws-sdk-sns] gem: - # - # # Gemfile - # gem "aws-sdk-sns", ">= 1.9.0", require: false - # - # 2. Tell Action Mailbox to accept emails from SES: - # - # # config/environments/production.rb - # config.action_mailbox.ingress = :amazon - # - # 3. {Configure SES}[https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-notifications.html] - # to deliver emails to your application via POST requests to +/rails/action_mailbox/amazon/inbound_emails+. - # If your application lived at <tt>https://example.com</tt>, you would specify the fully-qualified URL - # <tt>https://example.com/rails/action_mailbox/amazon/inbound_emails</tt>. - class Ingresses::Amazon::InboundEmailsController < BaseController - before_action :authenticate - - cattr_accessor :verifier - - def self.prepare - self.verifier ||= begin - require "aws-sdk-sns" - Aws::SNS::MessageVerifier.new - end - end - - def create - ActionMailbox::InboundEmail.create_and_extract_message_id! params.require(:content) - end - - private - def authenticate - head :unauthorized unless verifier.authentic?(request.body) - end - end -end diff --git a/actionmailbox/config/routes.rb b/actionmailbox/config/routes.rb index 517d2835af..1496d6f0b3 100644 --- a/actionmailbox/config/routes.rb +++ b/actionmailbox/config/routes.rb @@ -2,7 +2,6 @@ Rails.application.routes.draw do scope "/rails/action_mailbox", module: "action_mailbox/ingresses" do - post "/amazon/inbound_emails" => "amazon/inbound_emails#create", as: :rails_amazon_inbound_emails post "/mandrill/inbound_emails" => "mandrill/inbound_emails#create", as: :rails_mandrill_inbound_emails post "/postmark/inbound_emails" => "postmark/inbound_emails#create", as: :rails_postmark_inbound_emails post "/relay/inbound_emails" => "relay/inbound_emails#create", as: :rails_relay_inbound_emails diff --git a/actionmailbox/lib/action_mailbox/engine.rb b/actionmailbox/lib/action_mailbox/engine.rb index 039f04ac2f..bab3964d93 100644 --- a/actionmailbox/lib/action_mailbox/engine.rb +++ b/actionmailbox/lib/action_mailbox/engine.rb @@ -26,16 +26,7 @@ module ActionMailbox ActionMailbox.incinerate = app.config.action_mailbox.incinerate.nil? ? true : app.config.action_mailbox.incinerate ActionMailbox.incinerate_after = app.config.action_mailbox.incinerate_after || 30.days ActionMailbox.queues = app.config.action_mailbox.queues || {} - end - end - - initializer "action_mailbox.ingress" do |app| - config.to_prepare do - if ActionMailbox.ingress = app.config.action_mailbox.ingress.presence - if ingress_controller_class = "ActionMailbox::Ingresses::#{ActionMailbox.ingress.to_s.classify}::InboundEmailsController".safe_constantize - ingress_controller_class.prepare - end - end + ActionMailbox.ingress = app.config.action_mailbox.ingress end end end diff --git a/actionmailbox/lib/rails/generators/installer.rb b/actionmailbox/lib/rails/generators/installer.rb index 25cf528ef5..2864ea4e62 100644 --- a/actionmailbox/lib/rails/generators/installer.rb +++ b/actionmailbox/lib/rails/generators/installer.rb @@ -5,6 +5,6 @@ copy_file "#{__dir__}/mailbox/templates/application_mailbox.rb", "app/mailboxes/ environment <<~end_of_config, env: "production" # Prepare the ingress controller used to receive mail - # config.action_mailbox.ingress = :amazon + # config.action_mailbox.ingress = :postfix end_of_config diff --git a/actionmailbox/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb deleted file mode 100644 index e10985553e..0000000000 --- a/actionmailbox/test/controllers/ingresses/amazon/inbound_emails_controller_test.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -require "test_helper" - -ActionMailbox::Ingresses::Amazon::InboundEmailsController.verifier = - Module.new { def self.authentic?(message); true; end } - -class ActionMailbox::Ingresses::Amazon::InboundEmailsControllerTest < ActionDispatch::IntegrationTest - setup { ActionMailbox.ingress = :amazon } - - test "receiving an inbound email from Amazon" do - assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do - post rails_amazon_inbound_emails_url, params: { content: file_fixture("../files/welcome.eml").read }, as: :json - end - - assert_response :no_content - - inbound_email = ActionMailbox::InboundEmail.last - assert_equal file_fixture("../files/welcome.eml").read, inbound_email.raw_email.download - assert_equal "0CB459E0-0336-41DA-BC88-E6E28C697DDB@37signals.com", inbound_email.message_id - end -end diff --git a/actionmailbox/test/dummy/config/environments/production.rb b/actionmailbox/test/dummy/config/environments/production.rb index 1731582220..932858fdb6 100644 --- a/actionmailbox/test/dummy/config/environments/production.rb +++ b/actionmailbox/test/dummy/config/environments/production.rb @@ -1,9 +1,4 @@ Rails.application.configure do - # Prepare the ingress controller used to receive mail - # config.action_mailbox.ingress = :amazon - - # Verifies that versions and hashed value of the package contents in the project's package.json - config.webpacker.check_yarn_integrity = false # Settings specified here will take precedence over those in config/application.rb. # Code is not reloaded between requests. @@ -96,4 +91,10 @@ Rails.application.configure do # Do not dump schema after migrations. config.active_record.dump_schema_after_migration = false + + # Prepare the ingress controller used to receive mail + # config.action_mailbox.ingress = :postfix + + # Verifies that versions and hashed value of the package contents in the project's package.json + config.webpacker.check_yarn_integrity = false end diff --git a/actionpack/lib/action_controller/metal/params_wrapper.rb b/actionpack/lib/action_controller/metal/params_wrapper.rb index 09716f7588..e635abec8e 100644 --- a/actionpack/lib/action_controller/metal/params_wrapper.rb +++ b/actionpack/lib/action_controller/metal/params_wrapper.rb @@ -93,7 +93,7 @@ module ActionController end def model - super || synchronize { super || self.model = _default_wrap_model } + super || self.model = _default_wrap_model end def include diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 815f82a1f2..ae774b01f1 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -4,7 +4,6 @@ require "active_support/core_ext/hash/indifferent_access" require "active_support/core_ext/array/wrap" require "active_support/core_ext/string/filters" require "active_support/core_ext/object/to_query" -require "active_support/rescuable" require "action_dispatch/http/upload" require "rack/test" require "stringio" @@ -1092,9 +1091,6 @@ module ActionController # See ActionController::Parameters.require and ActionController::Parameters.permit # for more information. module StrongParameters - extend ActiveSupport::Concern - include ActiveSupport::Rescuable - # Returns a new ActionController::Parameters object that # has been instantiated with the <tt>request.parameters</tt>. def params diff --git a/actionview/lib/action_view/template/handlers/erb/erubi.rb b/actionview/lib/action_view/template/handlers/erb/erubi.rb index 307b852440..e602aa117a 100644 --- a/actionview/lib/action_view/template/handlers/erb/erubi.rb +++ b/actionview/lib/action_view/template/handlers/erb/erubi.rb @@ -25,7 +25,7 @@ module ActionView src = @src view = Class.new(ActionView::Base) { include action_view_erb_handler_context._routes.url_helpers - class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", @filename || "(erubi)", 0) + class_eval("define_method(:_template) { |local_assigns, output_buffer| #{src} }", defined?(@filename) ? @filename : "(erubi)", 0) }.empty view._run(:_template, nil, {}, ActionView::OutputBuffer.new) end diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 138c8a8ef4..c4e21b48ac 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,7 @@ +* Use individual execution counters when calculating retry delay. + + *Patrik Bóna* + * Make job argument assertions with `Time`, `ActiveSupport::TimeWithZone`, and `DateTime` work by dropping microseconds. Microsecond precision is lost during serialization. *Gannon McGibbon* diff --git a/activejob/lib/active_job/exceptions.rb b/activejob/lib/active_job/exceptions.rb index 9e00942a1c..35c1476368 100644 --- a/activejob/lib/active_job/exceptions.rb +++ b/activejob/lib/active_job/exceptions.rb @@ -54,7 +54,7 @@ module ActiveJob self.exception_executions[exceptions.to_s] = (exception_executions[exceptions.to_s] || 0) + 1 if exception_executions[exceptions.to_s] < attempts - retry_job wait: determine_delay(wait), queue: queue, priority: priority, error: error + retry_job wait: determine_delay(seconds_or_duration_or_algorithm: wait, executions: exception_executions[exceptions.to_s]), queue: queue, priority: priority, error: error else if block_given? instrument :retry_stopped, error: error do @@ -123,7 +123,7 @@ module ActiveJob end private - def determine_delay(seconds_or_duration_or_algorithm) + def determine_delay(seconds_or_duration_or_algorithm:, executions:) case seconds_or_duration_or_algorithm when :exponentially_longer (executions**4) + 2 diff --git a/activejob/test/cases/exceptions_test.rb b/activejob/test/cases/exceptions_test.rb index c88162bf58..840f4d40b5 100644 --- a/activejob/test/cases/exceptions_test.rb +++ b/activejob/test/cases/exceptions_test.rb @@ -140,6 +140,26 @@ class ExceptionsTest < ActiveSupport::TestCase ], JobBuffer.values end + test "use individual execution timers when calculating retry delay" do + travel_to Time.now + + exceptions_to_raise = %w(ExponentialWaitTenAttemptsError CustomWaitTenAttemptsError ExponentialWaitTenAttemptsError CustomWaitTenAttemptsError) + + RetryJob.perform_later exceptions_to_raise, 5, :log_scheduled_at + + assert_equal [ + "Raised ExponentialWaitTenAttemptsError for the 1st time", + "Next execution scheduled at #{(Time.now + 3.seconds).to_f}", + "Raised CustomWaitTenAttemptsError for the 2nd time", + "Next execution scheduled at #{(Time.now + 2.seconds).to_f}", + "Raised ExponentialWaitTenAttemptsError for the 3rd time", + "Next execution scheduled at #{(Time.now + 18.seconds).to_f}", + "Raised CustomWaitTenAttemptsError for the 4th time", + "Next execution scheduled at #{(Time.now + 4.seconds).to_f}", + "Successfully completed job" + ], JobBuffer.values + end + test "successfully retry job throwing one of two retryable exceptions" do RetryJob.perform_later "SecondRetryableErrorOfTwo", 3 diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 5c4670f393..0075cd3c01 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -286,12 +286,12 @@ module ActiveModel method_name = matcher.method_name(attr_name) unless instance_method_already_implemented?(method_name) - generate_method = "define_method_#{matcher.method_missing_target}" + generate_method = "define_method_#{matcher.target}" if respond_to?(generate_method, true) send(generate_method, attr_name.to_s) else - define_proxy_call true, generated_attribute_methods, method_name, matcher.method_missing_target, attr_name.to_s + define_proxy_call true, generated_attribute_methods, method_name, matcher.target, attr_name.to_s end end end @@ -355,14 +355,14 @@ module ActiveModel # Must try to match prefixes/suffixes first, or else the matcher with no prefix/suffix # will match every time. matchers = attribute_method_matchers.partition(&:plain?).reverse.flatten(1) - matchers.map { |method| method.match(method_name) }.compact + matchers.map { |matcher| matcher.match(method_name) }.compact end end # Define a method `name` in `mod` that dispatches to `send` # using the given `extra` args. This falls back on `define_method` # and `send` if the given names cannot be compiled. - def define_proxy_call(include_private, mod, name, send, *extra) + def define_proxy_call(include_private, mod, name, target, *extra) defn = if NAME_COMPILABLE_REGEXP.match?(name) "def #{name}(*args)" else @@ -371,34 +371,34 @@ module ActiveModel extra = (extra.map!(&:inspect) << "*args").join(", ") - target = if CALL_COMPILABLE_REGEXP.match?(send) - "#{"self." unless include_private}#{send}(#{extra})" + body = if CALL_COMPILABLE_REGEXP.match?(target) + "#{"self." unless include_private}#{target}(#{extra})" else - "send(:'#{send}', #{extra})" + "send(:'#{target}', #{extra})" end mod.module_eval <<-RUBY, __FILE__, __LINE__ + 1 #{defn} - #{target} + #{body} end RUBY end class AttributeMethodMatcher #:nodoc: - attr_reader :prefix, :suffix, :method_missing_target + attr_reader :prefix, :suffix, :target - AttributeMethodMatch = Struct.new(:target, :attr_name, :method_name) + AttributeMethodMatch = Struct.new(:target, :attr_name) def initialize(options = {}) @prefix, @suffix = options.fetch(:prefix, ""), options.fetch(:suffix, "") @regex = /^(?:#{Regexp.escape(@prefix)})(.*)(?:#{Regexp.escape(@suffix)})$/ - @method_missing_target = "#{@prefix}attribute#{@suffix}" + @target = "#{@prefix}attribute#{@suffix}" @method_name = "#{prefix}%s#{suffix}" end def match(method_name) if @regex =~ method_name - AttributeMethodMatch.new(method_missing_target, $1, method_name) + AttributeMethodMatch.new(target, $1) end end diff --git a/activemodel/lib/active_model/attributes.rb b/activemodel/lib/active_model/attributes.rb index c3a446098c..b7b2f35bcc 100644 --- a/activemodel/lib/active_model/attributes.rb +++ b/activemodel/lib/active_model/attributes.rb @@ -91,7 +91,7 @@ module ActiveModel @attributes.fetch_value(name) end - # Handle *= for method_missing. + # Dispatch target for <tt>*=</tt> attribute methods. def attribute=(attribute_name, value) write_attribute(attribute_name, value) end diff --git a/activemodel/lib/active_model/dirty.rb b/activemodel/lib/active_model/dirty.rb index ab2c9d04ae..35a587658c 100644 --- a/activemodel/lib/active_model/dirty.rb +++ b/activemodel/lib/active_model/dirty.rb @@ -165,17 +165,17 @@ module ActiveModel mutations_from_database.changed_attribute_names end - # Handles <tt>*_changed?</tt> for +method_missing+. + # Dispatch target for <tt>*_changed?</tt> attribute methods. def attribute_changed?(attr_name, **options) # :nodoc: mutations_from_database.changed?(attr_name.to_s, options) end - # Handles <tt>*_was</tt> for +method_missing+. + # Dispatch target for <tt>*_was</tt> attribute methods. def attribute_was(attr_name) # :nodoc: mutations_from_database.original_value(attr_name.to_s) end - # Handles <tt>*_previously_changed?</tt> for +method_missing+. + # Dispatch target for <tt>*_previously_changed?</tt> attribute methods. def attribute_previously_changed?(attr_name) # :nodoc: mutations_before_last_save.changed?(attr_name.to_s) end @@ -253,22 +253,22 @@ module ActiveModel @mutations_before_last_save ||= ActiveModel::NullMutationTracker.instance end - # Handles <tt>*_change</tt> for +method_missing+. + # Dispatch target for <tt>*_change</tt> attribute methods. def attribute_change(attr_name) mutations_from_database.change_to_attribute(attr_name.to_s) end - # Handles <tt>*_previous_change</tt> for +method_missing+. + # Dispatch target for <tt>*_previous_change</tt> attribute methods. def attribute_previous_change(attr_name) mutations_before_last_save.change_to_attribute(attr_name.to_s) end - # Handles <tt>*_will_change!</tt> for +method_missing+. + # Dispatch target for <tt>*_will_change!</tt> attribute methods. def attribute_will_change!(attr_name) mutations_from_database.force_change(attr_name.to_s) end - # Handles <tt>restore_*!</tt> for +method_missing+. + # Dispatch target for <tt>restore_*!</tt> attribute methods. def restore_attribute!(attr_name) attr_name = attr_name.to_s if attribute_changed?(attr_name) diff --git a/activemodel/test/cases/attribute_methods_test.rb b/activemodel/test/cases/attribute_methods_test.rb index ebb6cc542d..4e228032c3 100644 --- a/activemodel/test/cases/attribute_methods_test.rb +++ b/activemodel/test/cases/attribute_methods_test.rb @@ -264,6 +264,5 @@ class AttributeMethodsTest < ActiveModel::TestCase assert_equal "foo", match.attr_name assert_equal "attribute_test", match.target - assert_equal "foo_test", match.method_name end end diff --git a/activemodel/test/cases/validations/acceptance_validation_test.rb b/activemodel/test/cases/validations/acceptance_validation_test.rb index 7662f996ae..72baf6e7a7 100644 --- a/activemodel/test/cases/validations/acceptance_validation_test.rb +++ b/activemodel/test/cases/validations/acceptance_validation_test.rb @@ -7,21 +7,23 @@ require "models/reply" require "models/person" class AcceptanceValidationTest < ActiveModel::TestCase - def teardown - Topic.clear_validators! + teardown do + self.class.send(:remove_const, :TestClass) end def test_terms_of_service_agreement_no_acceptance - Topic.validates_acceptance_of(:terms_of_service) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service) - t = Topic.new("title" => "We should not be confirmed") + t = klass.new("title" => "We should not be confirmed") assert_predicate t, :valid? end def test_terms_of_service_agreement - Topic.validates_acceptance_of(:terms_of_service) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service) - t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + t = klass.new("title" => "We should be confirmed", "terms_of_service" => "") assert_predicate t, :invalid? assert_equal ["must be accepted"], t.errors[:terms_of_service] @@ -30,9 +32,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_eula - Topic.validates_acceptance_of(:eula, message: "must be abided") + klass = define_test_class(Topic) + klass.validates_acceptance_of(:eula, message: "must be abided") - t = Topic.new("title" => "We should be confirmed", "eula" => "") + t = klass.new("title" => "We should be confirmed", "eula" => "") assert_predicate t, :invalid? assert_equal ["must be abided"], t.errors[:eula] @@ -41,9 +44,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_terms_of_service_agreement_with_accept_value - Topic.validates_acceptance_of(:terms_of_service, accept: "I agree.") + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service, accept: "I agree.") - t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + t = klass.new("title" => "We should be confirmed", "terms_of_service" => "") assert_predicate t, :invalid? assert_equal ["must be accepted"], t.errors[:terms_of_service] @@ -52,9 +56,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_terms_of_service_agreement_with_multiple_accept_values - Topic.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."]) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service, accept: [1, "I concur."]) - t = Topic.new("title" => "We should be confirmed", "terms_of_service" => "") + t = klass.new("title" => "We should be confirmed", "terms_of_service" => "") assert_predicate t, :invalid? assert_equal ["must be accepted"], t.errors[:terms_of_service] @@ -66,9 +71,10 @@ class AcceptanceValidationTest < ActiveModel::TestCase end def test_validates_acceptance_of_for_ruby_class - Person.validates_acceptance_of :karma + klass = define_test_class(Person) + klass.validates_acceptance_of :karma - p = Person.new + p = klass.new p.karma = "" assert_predicate p, :invalid? @@ -76,13 +82,21 @@ class AcceptanceValidationTest < ActiveModel::TestCase p.karma = "1" assert_predicate p, :valid? - ensure - Person.clear_validators! end def test_validates_acceptance_of_true - Topic.validates_acceptance_of(:terms_of_service) + klass = define_test_class(Topic) + klass.validates_acceptance_of(:terms_of_service) - assert_predicate Topic.new(terms_of_service: true), :valid? + assert_predicate klass.new(terms_of_service: true), :valid? end + + private + + # Acceptance validator includes anonymous module into class, which cannot + # be cleared, so to avoid multiple inclusions we use a named subclass which + # we can remove in teardown. + def define_test_class(parent) + self.class.const_set(:TestClass, Class.new(parent)) + end end diff --git a/activemodel/test/cases/validations/i18n_validation_test.rb b/activemodel/test/cases/validations/i18n_validation_test.rb index eb03e837f1..35bb918f26 100644 --- a/activemodel/test/cases/validations/i18n_validation_test.rb +++ b/activemodel/test/cases/validations/i18n_validation_test.rb @@ -6,7 +6,7 @@ require "models/person" class I18nValidationTest < ActiveModel::TestCase def setup Person.clear_validators! - @person = Person.new + @person = person_class.new @old_load_path, @old_backend = I18n.load_path.dup, I18n.backend I18n.load_path.clear @@ -18,7 +18,9 @@ class I18nValidationTest < ActiveModel::TestCase end def teardown - Person.clear_validators! + person_class.clear_validators! + self.class.send(:remove_const, :Person) + @person_stub = nil I18n.load_path.replace @old_load_path I18n.backend = @old_backend I18n.backend.reload! @@ -28,14 +30,14 @@ class I18nValidationTest < ActiveModel::TestCase def test_full_message_encoding I18n.backend.store_translations("en", errors: { messages: { too_short: "猫舌" } }) - Person.validates_length_of :title, within: 3..5 + person_class.validates_length_of :title, within: 3..5 @person.valid? assert_equal ["Title 猫舌"], @person.errors.full_messages end def test_errors_full_messages_translates_human_attribute_name_for_model_attributes @person.errors.add(:name, "not found") - assert_called_with(Person, :human_attribute_name, ["name", default: "Name"], returns: "Person's name") do + assert_called_with(person_class, :human_attribute_name, ["name", default: "Name"], returns: "Person's name") do assert_equal ["Person's name not found"], @person.errors.full_messages end end @@ -52,7 +54,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { person: { attributes: { name: { format: "%{message}" } } } } } }) - person = Person.new + person = person_class.new assert_equal "Name cannot be blank", person.errors.full_message(:name, "cannot be blank") assert_equal "Name test cannot be blank", person.errors.full_message(:name_test, "cannot be blank") end @@ -63,7 +65,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { person: { attributes: { name: { format: "%{message}" } } } } } }) - person = Person.new + person = person_class.new assert_equal "cannot be blank", person.errors.full_message(:name, "cannot be blank") assert_equal "Name test cannot be blank", person.errors.full_message(:name_test, "cannot be blank") end @@ -74,7 +76,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { person: { format: "%{message}" } } } }) - person = Person.new + person = person_class.new assert_equal "cannot be blank", person.errors.full_message(:name, "cannot be blank") assert_equal "cannot be blank", person.errors.full_message(:name_test, "cannot be blank") end @@ -85,7 +87,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { attributes: { street: { format: "%{message}" } } } } } }) - person = Person.new + person = person_class.new assert_equal "cannot be blank", person.errors.full_message(:'contacts/addresses.street', "cannot be blank") assert_equal "Contacts/addresses country cannot be blank", person.errors.full_message(:'contacts/addresses.country', "cannot be blank") end @@ -96,7 +98,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { format: "%{message}" } } } }) - person = Person.new + person = person_class.new assert_equal "cannot be blank", person.errors.full_message(:'contacts/addresses.street', "cannot be blank") assert_equal "cannot be blank", person.errors.full_message(:'contacts/addresses.country', "cannot be blank") end @@ -107,7 +109,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { attributes: { street: { format: "%{message}" } } } } } }) - person = Person.new + person = person_class.new assert_equal "cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].street', "cannot be blank") assert_equal "Contacts/addresses country cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].country', "cannot be blank") end @@ -118,7 +120,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { format: "%{message}" } } } }) - person = Person.new + person = person_class.new assert_equal "cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].street', "cannot be blank") assert_equal "cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].country', "cannot be blank") end @@ -130,7 +132,7 @@ class I18nValidationTest < ActiveModel::TestCase attributes: { 'person/contacts/addresses': { country: "Country" } } }) - person = Person.new + person = person_class.new assert_equal "Contacts/addresses street cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].street', "cannot be blank") assert_equal "Country cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].country', "cannot be blank") end @@ -141,7 +143,7 @@ class I18nValidationTest < ActiveModel::TestCase I18n.backend.store_translations("en", activemodel: { errors: { models: { 'person/contacts/addresses': { attributes: { street: { format: "%{message}" } } } } } }) - person = Person.new + person = person_class.new assert_equal "Contacts[0]/addresses[0] street cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].street', "cannot be blank") assert_equal "Contacts[0]/addresses[0] country cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].country', "cannot be blank") end @@ -153,7 +155,7 @@ class I18nValidationTest < ActiveModel::TestCase attributes: { 'person/contacts[0]/addresses[0]': { country: "Country" } } }) - person = Person.new + person = person_class.new assert_equal "Contacts[0]/addresses[0] street cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].street', "cannot be blank") assert_equal "Country cannot be blank", person.errors.full_message(:'contacts[0]/addresses[0].country', "cannot be blank") end @@ -174,7 +176,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_confirmation_of on generated message #{name}" do - Person.validates_confirmation_of :title, validation_options + person_class.validates_confirmation_of :title, validation_options @person.title_confirmation = "foo" call = [:title_confirmation, :confirmation, generate_message_options.merge(attribute: "Title")] assert_called_with(@person.errors, :generate_message, call) do @@ -185,7 +187,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_acceptance_of on generated message #{name}" do - Person.validates_acceptance_of :title, validation_options.merge(allow_nil: false) + person_class.validates_acceptance_of :title, validation_options.merge(allow_nil: false) call = [:title, :accepted, generate_message_options] assert_called_with(@person.errors, :generate_message, call) do @person.valid? @@ -195,7 +197,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_presence_of on generated message #{name}" do - Person.validates_presence_of :title, validation_options + person_class.validates_presence_of :title, validation_options call = [:title, :blank, generate_message_options] assert_called_with(@person.errors, :generate_message, call) do @person.valid? @@ -205,7 +207,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_length_of for :within on generated message when too short #{name}" do - Person.validates_length_of :title, validation_options.merge(within: 3..5) + person_class.validates_length_of :title, validation_options.merge(within: 3..5) call = [:title, :too_short, generate_message_options.merge(count: 3)] assert_called_with(@person.errors, :generate_message, call) do @person.valid? @@ -215,7 +217,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_length_of for :too_long generated message #{name}" do - Person.validates_length_of :title, validation_options.merge(within: 3..5) + person_class.validates_length_of :title, validation_options.merge(within: 3..5) @person.title = "this title is too long" call = [:title, :too_long, generate_message_options.merge(count: 5)] assert_called_with(@person.errors, :generate_message, call) do @@ -226,7 +228,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_length_of for :is on generated message #{name}" do - Person.validates_length_of :title, validation_options.merge(is: 5) + person_class.validates_length_of :title, validation_options.merge(is: 5) call = [:title, :wrong_length, generate_message_options.merge(count: 5)] assert_called_with(@person.errors, :generate_message, call) do @person.valid? @@ -236,7 +238,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_format_of on generated message #{name}" do - Person.validates_format_of :title, validation_options.merge(with: /\A[1-9][0-9]*\z/) + person_class.validates_format_of :title, validation_options.merge(with: /\A[1-9][0-9]*\z/) @person.title = "72x" call = [:title, :invalid, generate_message_options.merge(value: "72x")] assert_called_with(@person.errors, :generate_message, call) do @@ -247,7 +249,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_inclusion_of on generated message #{name}" do - Person.validates_inclusion_of :title, validation_options.merge(in: %w(a b c)) + person_class.validates_inclusion_of :title, validation_options.merge(in: %w(a b c)) @person.title = "z" call = [:title, :inclusion, generate_message_options.merge(value: "z")] assert_called_with(@person.errors, :generate_message, call) do @@ -258,7 +260,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_inclusion_of using :within on generated message #{name}" do - Person.validates_inclusion_of :title, validation_options.merge(within: %w(a b c)) + person_class.validates_inclusion_of :title, validation_options.merge(within: %w(a b c)) @person.title = "z" call = [:title, :inclusion, generate_message_options.merge(value: "z")] assert_called_with(@person.errors, :generate_message, call) do @@ -269,7 +271,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_exclusion_of generated message #{name}" do - Person.validates_exclusion_of :title, validation_options.merge(in: %w(a b c)) + person_class.validates_exclusion_of :title, validation_options.merge(in: %w(a b c)) @person.title = "a" call = [:title, :exclusion, generate_message_options.merge(value: "a")] assert_called_with(@person.errors, :generate_message, call) do @@ -280,7 +282,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_exclusion_of using :within generated message #{name}" do - Person.validates_exclusion_of :title, validation_options.merge(within: %w(a b c)) + person_class.validates_exclusion_of :title, validation_options.merge(within: %w(a b c)) @person.title = "a" call = [:title, :exclusion, generate_message_options.merge(value: "a")] assert_called_with(@person.errors, :generate_message, call) do @@ -291,7 +293,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of generated message #{name}" do - Person.validates_numericality_of :title, validation_options + person_class.validates_numericality_of :title, validation_options @person.title = "a" call = [:title, :not_a_number, generate_message_options.merge(value: "a")] assert_called_with(@person.errors, :generate_message, call) do @@ -302,7 +304,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of for :only_integer on generated message #{name}" do - Person.validates_numericality_of :title, validation_options.merge(only_integer: true) + person_class.validates_numericality_of :title, validation_options.merge(only_integer: true) @person.title = "0.0" call = [:title, :not_an_integer, generate_message_options.merge(value: "0.0")] assert_called_with(@person.errors, :generate_message, call) do @@ -313,7 +315,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of for :odd on generated message #{name}" do - Person.validates_numericality_of :title, validation_options.merge(only_integer: true, odd: true) + person_class.validates_numericality_of :title, validation_options.merge(only_integer: true, odd: true) @person.title = 0 call = [:title, :odd, generate_message_options.merge(value: 0)] assert_called_with(@person.errors, :generate_message, call) do @@ -324,7 +326,7 @@ class I18nValidationTest < ActiveModel::TestCase COMMON_CASES.each do |name, validation_options, generate_message_options| test "validates_numericality_of for :less_than on generated message #{name}" do - Person.validates_numericality_of :title, validation_options.merge(only_integer: true, less_than: 0) + person_class.validates_numericality_of :title, validation_options.merge(only_integer: true, less_than: 0) @person.title = 1 call = [:title, :less_than, generate_message_options.merge(value: 1, count: 0)] assert_called_with(@person.errors, :generate_message, call) do @@ -369,67 +371,67 @@ class I18nValidationTest < ActiveModel::TestCase end set_expectations_for_validation "validates_confirmation_of", :confirmation do |person, options_to_merge| - Person.validates_confirmation_of :title, options_to_merge + person.class.validates_confirmation_of :title, options_to_merge person.title_confirmation = "foo" end set_expectations_for_validation "validates_acceptance_of", :accepted do |person, options_to_merge| - Person.validates_acceptance_of :title, options_to_merge.merge(allow_nil: false) + person.class.validates_acceptance_of :title, options_to_merge.merge(allow_nil: false) end set_expectations_for_validation "validates_presence_of", :blank do |person, options_to_merge| - Person.validates_presence_of :title, options_to_merge + person.class.validates_presence_of :title, options_to_merge end set_expectations_for_validation "validates_length_of", :too_short do |person, options_to_merge| - Person.validates_length_of :title, options_to_merge.merge(within: 3..5) + person.class.validates_length_of :title, options_to_merge.merge(within: 3..5) end set_expectations_for_validation "validates_length_of", :too_long do |person, options_to_merge| - Person.validates_length_of :title, options_to_merge.merge(within: 3..5) + person.class.validates_length_of :title, options_to_merge.merge(within: 3..5) person.title = "too long" end set_expectations_for_validation "validates_length_of", :wrong_length do |person, options_to_merge| - Person.validates_length_of :title, options_to_merge.merge(is: 5) + person.class.validates_length_of :title, options_to_merge.merge(is: 5) end set_expectations_for_validation "validates_format_of", :invalid do |person, options_to_merge| - Person.validates_format_of :title, options_to_merge.merge(with: /\A[1-9][0-9]*\z/) + person.class.validates_format_of :title, options_to_merge.merge(with: /\A[1-9][0-9]*\z/) end set_expectations_for_validation "validates_inclusion_of", :inclusion do |person, options_to_merge| - Person.validates_inclusion_of :title, options_to_merge.merge(in: %w(a b c)) + person.class.validates_inclusion_of :title, options_to_merge.merge(in: %w(a b c)) end set_expectations_for_validation "validates_exclusion_of", :exclusion do |person, options_to_merge| - Person.validates_exclusion_of :title, options_to_merge.merge(in: %w(a b c)) + person.class.validates_exclusion_of :title, options_to_merge.merge(in: %w(a b c)) person.title = "a" end set_expectations_for_validation "validates_numericality_of", :not_a_number do |person, options_to_merge| - Person.validates_numericality_of :title, options_to_merge + person.class.validates_numericality_of :title, options_to_merge person.title = "a" end set_expectations_for_validation "validates_numericality_of", :not_an_integer do |person, options_to_merge| - Person.validates_numericality_of :title, options_to_merge.merge(only_integer: true) + person.class.validates_numericality_of :title, options_to_merge.merge(only_integer: true) person.title = "1.0" end set_expectations_for_validation "validates_numericality_of", :odd do |person, options_to_merge| - Person.validates_numericality_of :title, options_to_merge.merge(only_integer: true, odd: true) + person.class.validates_numericality_of :title, options_to_merge.merge(only_integer: true, odd: true) person.title = 0 end set_expectations_for_validation "validates_numericality_of", :less_than do |person, options_to_merge| - Person.validates_numericality_of :title, options_to_merge.merge(only_integer: true, less_than: 0) + person.class.validates_numericality_of :title, options_to_merge.merge(only_integer: true, less_than: 0) person.title = 1 end def test_validations_with_message_symbol_must_translate I18n.backend.store_translations "en", errors: { messages: { custom_error: "I am a custom error" } } - Person.validates_presence_of :title, message: :custom_error + person_class.validates_presence_of :title, message: :custom_error @person.title = nil @person.valid? assert_equal ["I am a custom error"], @person.errors[:title] @@ -437,7 +439,7 @@ class I18nValidationTest < ActiveModel::TestCase def test_validates_with_message_symbol_must_translate_per_attribute I18n.backend.store_translations "en", activemodel: { errors: { models: { person: { attributes: { title: { custom_error: "I am a custom error" } } } } } } - Person.validates_presence_of :title, message: :custom_error + person_class.validates_presence_of :title, message: :custom_error @person.title = nil @person.valid? assert_equal ["I am a custom error"], @person.errors[:title] @@ -445,16 +447,20 @@ class I18nValidationTest < ActiveModel::TestCase def test_validates_with_message_symbol_must_translate_per_model I18n.backend.store_translations "en", activemodel: { errors: { models: { person: { custom_error: "I am a custom error" } } } } - Person.validates_presence_of :title, message: :custom_error + person_class.validates_presence_of :title, message: :custom_error @person.title = nil @person.valid? assert_equal ["I am a custom error"], @person.errors[:title] end def test_validates_with_message_string - Person.validates_presence_of :title, message: "I am a custom error" + person_class.validates_presence_of :title, message: "I am a custom error" @person.title = nil @person.valid? assert_equal ["I am a custom error"], @person.errors[:title] end + + def person_class + @person_stub ||= self.class.const_set(:Person, Class.new(Person)) + end end diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 1cf3f3352f..7ae6326fc3 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,14 @@ +* `change_column_comment` and `change_table_comment` are invertible only if + `to` and `from` options are specified. + + *Yoshiyuki Kinjo* + +* Don't call commit/rollback callbacks despite a record isn't saved. + + Fixes #29747. + + *Ryuta Kamizono* + * Fix circular `autosave: true` causes invalid records to be saved. Prior to the fix, when there was a circular series of `autosave: true` diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index 7c69cd65ee..3b4b243148 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -27,40 +27,32 @@ module ActiveRecord::Associations::Builder # :nodoc: "Please choose a different association name." end - extension = define_extensions model, name, &block - reflection = create_reflection model, name, scope, options, extension + reflection = create_reflection(model, name, scope, options, &block) define_accessors model, reflection define_callbacks model, reflection define_validations model, reflection reflection end - def self.create_reflection(model, name, scope, options, extension = nil) + def self.create_reflection(model, name, scope, options, &block) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) validate_options(options) - scope = build_scope(scope, extension) + extension = define_extensions(model, name, &block) + options[:extend] = [*options[:extend], extension] if extension + + scope = build_scope(scope) ActiveRecord::Reflection.create(macro, name, scope, options, model) end - def self.build_scope(scope, extension) - new_scope = scope - + def self.build_scope(scope) if scope && scope.arity == 0 - new_scope = proc { instance_exec(&scope) } - end - - if extension - new_scope = wrap_scope new_scope, extension + proc { instance_exec(&scope) } + else + scope end - - new_scope - end - - def self.wrap_scope(scope, extension) - scope end def self.macro diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index 5848cd9112..9fccfcce0c 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -66,17 +66,5 @@ module ActiveRecord::Associations::Builder # :nodoc: end CODE end - - def self.wrap_scope(scope, mod) - if scope - if scope.arity > 0 - proc { |owner| instance_exec(owner, &scope).extending(mod) } - else - proc { instance_exec(&scope).extending(mod) } - end - else - proc { extending(mod) } - end - end end end diff --git a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb index 5941f51a1a..dc239ff9ea 100644 --- a/activerecord/lib/active_record/attribute_methods/before_type_cast.rb +++ b/activerecord/lib/active_record/attribute_methods/before_type_cast.rb @@ -65,7 +65,7 @@ module ActiveRecord private - # Handle *_before_type_cast for method_missing. + # Dispatch target for <tt>*_before_type_cast</tt> attribute methods. def attribute_before_type_cast(attribute_name) read_attribute_before_type_cast(attribute_name) end diff --git a/activerecord/lib/active_record/attribute_methods/dirty.rb b/activerecord/lib/active_record/attribute_methods/dirty.rb index 920a7b2038..68ac8475b0 100644 --- a/activerecord/lib/active_record/attribute_methods/dirty.rb +++ b/activerecord/lib/active_record/attribute_methods/dirty.rb @@ -166,6 +166,30 @@ module ActiveRecord result end + def _touch_row(attribute_names, time) + @_touch_attr_names = Set.new(attribute_names) + + affected_rows = super + + changes = {} + @attributes.keys.each do |attr_name| + next if @_touch_attr_names.include?(attr_name) + + if attribute_changed?(attr_name) + changes[attr_name] = _read_attribute(attr_name) + _write_attribute(attr_name, attribute_was(attr_name)) + clear_attribute_change(attr_name) + end + end + + changes_applied + changes.each { |attr_name, value| _write_attribute(attr_name, value) } + + affected_rows + ensure + @_touch_attr_names = nil + end + def _update_record(attribute_names = attribute_names_for_partial_writes) affected_rows = super changes_applied diff --git a/activerecord/lib/active_record/attribute_methods/query.rb b/activerecord/lib/active_record/attribute_methods/query.rb index 6811f54b10..0cf67644af 100644 --- a/activerecord/lib/active_record/attribute_methods/query.rb +++ b/activerecord/lib/active_record/attribute_methods/query.rb @@ -32,7 +32,7 @@ module ActiveRecord end private - # Handle *? for method_missing. + # Dispatch target for <tt>*?</tt> attribute methods. def attribute?(attribute_name) query_attribute(attribute_name) end diff --git a/activerecord/lib/active_record/attribute_methods/write.rb b/activerecord/lib/active_record/attribute_methods/write.rb index 455e67e19b..d5ba2f42cb 100644 --- a/activerecord/lib/active_record/attribute_methods/write.rb +++ b/activerecord/lib/active_record/attribute_methods/write.rb @@ -58,7 +58,7 @@ module ActiveRecord value end - # Handle *= for method_missing. + # Dispatch target for <tt>*=</tt> attribute methods. def attribute=(attribute_name, value) _write_attribute(attribute_name, value) end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 4840307094..7041d92586 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -1185,12 +1185,22 @@ module ActiveRecord end # Changes the comment for a table or removes it if +nil+. - def change_table_comment(table_name, comment) + # + # Passing a hash containing +:from+ and +:to+ will make this change + # reversible in migration: + # + # change_table_comment(:posts, from: "old_comment", to: "new_comment") + def change_table_comment(table_name, comment_or_changes) raise NotImplementedError, "#{self.class} does not support changing table comments" end # Changes the comment for a column or removes it if +nil+. - def change_column_comment(table_name, column_name, comment) + # + # Passing a hash containing +:from+ and +:to+ will make this change + # reversible in migration: + # + # change_column_comment(:posts, :state, from: "old_comment", to: "new_comment") + def change_column_comment(table_name, column_name, comment_or_changes) raise NotImplementedError, "#{self.class} does not support changing column comments" end @@ -1374,11 +1384,37 @@ module ActiveRecord default_or_changes end end + alias :extract_new_comment_value :extract_new_default_value def can_remove_index_by_name?(options) options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty? end + def bulk_change_table(table_name, operations) + sql_fragments = [] + non_combinable_operations = [] + + operations.each do |command, args| + table, arguments = args.shift, args + method = :"#{command}_for_alter" + + if respond_to?(method, true) + sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) } + sql_fragments << sqls + non_combinable_operations.concat(procs) + else + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? + non_combinable_operations.each(&:call) + sql_fragments = [] + non_combinable_operations = [] + send(command, table, *arguments) + end + end + + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? + non_combinable_operations.each(&:call) + end + def add_column_for_alter(table_name, column_name, type, options = {}) td = create_table_definition(table_name) cd = td.new_column_definition(column_name, type, options) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb index ca8bbc14da..8b907759c6 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -63,7 +63,7 @@ module ActiveRecord /mariadb/i.match?(full_version) end - def supports_bulk_alter? #:nodoc: + def supports_bulk_alter? true end @@ -285,22 +285,8 @@ module ActiveRecord SQL end - def bulk_change_table(table_name, operations) #:nodoc: - sqls = operations.flat_map do |command, args| - table, arguments = args.shift, args - method = :"#{command}_for_alter" - - if respond_to?(method, true) - send(method, table, *arguments) - else - raise "Unknown method called : #{method}(#{arguments.inspect})" - end - end.join(", ") - - execute("ALTER TABLE #{quote_table_name(table_name)} #{sqls}") - end - - def change_table_comment(table_name, comment) #:nodoc: + def change_table_comment(table_name, comment_or_changes) # :nodoc: + comment = extract_new_comment_value(comment_or_changes) comment = "" if comment.nil? execute("ALTER TABLE #{quote_table_name(table_name)} COMMENT #{quote(comment)}") end @@ -356,7 +342,8 @@ module ActiveRecord change_column table_name, column_name, nil, null: null end - def change_column_comment(table_name, column_name, comment) #:nodoc: + def change_column_comment(table_name, column_name, comment_or_changes) # :nodoc: + comment = extract_new_comment_value(comment_or_changes) change_column table_name, column_name, nil, comment: comment end diff --git a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb index 56479f27bf..9167593064 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/type_metadata.rb @@ -10,12 +10,11 @@ module ActiveRecord def initialize(type_metadata, extra: "") super(type_metadata) - @type_metadata = type_metadata @extra = extra end def ==(other) - other.is_a?(MySQL::TypeMetadata) && + other.is_a?(TypeMetadata) && __getobj__ == other.__getobj__ && extra == other.extra end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb index ef98d2b37a..ec25bb1e19 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/column.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/column.rb @@ -4,8 +4,7 @@ module ActiveRecord module ConnectionAdapters module PostgreSQL class Column < ConnectionAdapters::Column # :nodoc: - delegate :array, :oid, :fmod, to: :sql_type_metadata - alias :array? :array + delegate :oid, :fmod, to: :sql_type_metadata def initialize(*, serial: nil, **) super @@ -15,6 +14,15 @@ module ActiveRecord def serial? @serial end + + def array + sql_type_metadata.sql_type.end_with?("[]") + end + alias :array? :array + + def sql_type + super.sub(/\[\]\z/, "") + end end end PostgreSQLColumn = PostgreSQL::Column # :nodoc: diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index c412d1f34c..40c5e51d92 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -368,31 +368,6 @@ module ActiveRecord SQL end - def bulk_change_table(table_name, operations) - sql_fragments = [] - non_combinable_operations = [] - - operations.each do |command, args| - table, arguments = args.shift, args - method = :"#{command}_for_alter" - - if respond_to?(method, true) - sqls, procs = Array(send(method, table, *arguments)).partition { |v| v.is_a?(String) } - sql_fragments << sqls - non_combinable_operations.concat(procs) - else - execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? - non_combinable_operations.each(&:call) - sql_fragments = [] - non_combinable_operations = [] - send(command, table, *arguments) - end - end - - execute "ALTER TABLE #{quote_table_name(table_name)} #{sql_fragments.join(", ")}" unless sql_fragments.empty? - non_combinable_operations.each(&:call) - end - # Renames a table. # Also renames a table's primary key sequence if the sequence name exists and # matches the Active Record default. @@ -443,14 +418,16 @@ module ActiveRecord end # Adds comment for given table column or drops it if +comment+ is a +nil+ - def change_column_comment(table_name, column_name, comment) # :nodoc: + def change_column_comment(table_name, column_name, comment_or_changes) # :nodoc: clear_cache! + comment = extract_new_comment_value(comment_or_changes) execute "COMMENT ON COLUMN #{quote_table_name(table_name)}.#{quote_column_name(column_name)} IS #{quote(comment)}" end # Adds comment for given table or drops it if +comment+ is a +nil+ - def change_table_comment(table_name, comment) # :nodoc: + def change_table_comment(table_name, comment_or_changes) # :nodoc: clear_cache! + comment = extract_new_comment_value(comment_or_changes) execute "COMMENT ON TABLE #{quote_table_name(table_name)} IS #{quote(comment)}" end @@ -675,7 +652,7 @@ module ActiveRecord precision: cast_type.precision, scale: cast_type.scale, ) - PostgreSQLTypeMetadata.new(simple_type, oid: oid, fmod: fmod) + PostgreSQL::TypeMetadata.new(simple_type, oid: oid, fmod: fmod) end def sequence_name_from_parts(table_name, column_name, suffix) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/type_metadata.rb b/activerecord/lib/active_record/connection_adapters/postgresql/type_metadata.rb index 403b3ead98..8bdec623af 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/type_metadata.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/type_metadata.rb @@ -3,39 +3,34 @@ module ActiveRecord # :stopdoc: module ConnectionAdapters - class PostgreSQLTypeMetadata < DelegateClass(SqlTypeMetadata) - undef to_yaml if method_defined?(:to_yaml) + module PostgreSQL + class TypeMetadata < DelegateClass(SqlTypeMetadata) + undef to_yaml if method_defined?(:to_yaml) - attr_reader :oid, :fmod, :array + attr_reader :oid, :fmod - def initialize(type_metadata, oid: nil, fmod: nil) - super(type_metadata) - @type_metadata = type_metadata - @oid = oid - @fmod = fmod - @array = /\[\]$/.match?(type_metadata.sql_type) - end - - def sql_type - super.gsub(/\[\]$/, "") - end + def initialize(type_metadata, oid: nil, fmod: nil) + super(type_metadata) + @oid = oid + @fmod = fmod + end - def ==(other) - other.is_a?(PostgreSQLTypeMetadata) && - __getobj__ == other.__getobj__ && - oid == other.oid && - fmod == other.fmod && - array == other.array - end - alias eql? == + def ==(other) + other.is_a?(TypeMetadata) && + __getobj__ == other.__getobj__ && + oid == other.oid && + fmod == other.fmod + end + alias eql? == - def hash - PostgreSQLTypeMetadata.hash ^ - __getobj__.hash ^ - oid.hash ^ - fmod.hash ^ - array.hash + def hash + TypeMetadata.hash ^ + __getobj__.hash ^ + oid.hash ^ + fmod.hash + end end end + PostgreSQLTypeMetadata = PostgreSQL::TypeMetadata end end diff --git a/activerecord/lib/active_record/connection_handling.rb b/activerecord/lib/active_record/connection_handling.rb index 53069cd899..6782833c5a 100644 --- a/activerecord/lib/active_record/connection_handling.rb +++ b/activerecord/lib/active_record/connection_handling.rb @@ -85,14 +85,14 @@ module ActiveRecord # based on the requested role: # # ActiveRecord::Base.connected_to(role: :writing) do - # Dog.create! # creates dog using dog connection + # Dog.create! # creates dog using dog writing connection # end # # ActiveRecord::Base.connected_to(role: :reading) do # Dog.create! # throws exception because we're on a replica # end # - # ActiveRecord::Base.connected_to(role: :unknown_ode) do + # ActiveRecord::Base.connected_to(role: :unknown_role) do # # raises exception due to non-existent role # end # @@ -100,11 +100,20 @@ module ActiveRecord # you can use +connected_to+ with a +database+ argument. The +database+ argument # expects a symbol that corresponds to the database key in your config. # - # This will connect to a new database for the queries inside the block. - # # ActiveRecord::Base.connected_to(database: :animals_slow_replica) do # Dog.run_a_long_query # runs a long query while connected to the +animals_slow_replica+ # end + # + # This will connect to a new database for the queries inside the block. By + # default the `:writing` role will be used since all connections must be assigned + # a role. If you would like to use a different role you can pass a hash to database: + # + # ActiveRecord::Base.connected_to(database: { readonly_slow: :animals_slow_replica }) do + # Dog.run_a_long_query # runs a long query while connected to the +animals_slow_replica+ + # using the readonly_slow role. + # end + # + # When using the database key a new connection will be established every time. def connected_to(database: nil, role: nil, &blk) if database && role raise ArgumentError, "connected_to can only accept a `database` or a `role` argument, but not both arguments." @@ -112,17 +121,14 @@ module ActiveRecord if database.is_a?(Hash) role, database = database.first role = role.to_sym - else - role = database.to_sym end config_hash = resolve_config_for_connection(database) handler = lookup_connection_handler(role) - with_handler(role) do - handler.establish_connection(config_hash) - yield - end + handler.establish_connection(config_hash) + + with_handler(role, &blk) elsif role with_handler(role.to_sym, &blk) else @@ -154,6 +160,7 @@ module ActiveRecord end def lookup_connection_handler(handler_key) # :nodoc: + handler_key ||= ActiveRecord::Base.writing_role connection_handlers[handler_key] ||= ActiveRecord::ConnectionAdapters::ConnectionHandler.new end diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index eb4b48bc37..6fed3e5c19 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -466,6 +466,7 @@ module ActiveRecord # Returns +true+ if the attributes hash has been frozen. def frozen? + sync_with_transaction_state @attributes.frozen? end @@ -583,7 +584,7 @@ module ActiveRecord end def thaw - if frozen? + if @attributes.frozen? @attributes = @attributes.dup end end diff --git a/activerecord/lib/active_record/locking/optimistic.rb b/activerecord/lib/active_record/locking/optimistic.rb index 4a3a31fc95..b7eecda59e 100644 --- a/activerecord/lib/active_record/locking/optimistic.rb +++ b/activerecord/lib/active_record/locking/optimistic.rb @@ -71,9 +71,8 @@ module ActiveRecord end def _touch_row(attribute_names, time) + @_touch_attr_names << self.class.locking_column if locking_enabled? super - ensure - clear_attribute_change(self.class.locking_column) if locking_enabled? end def _update_row(attribute_names, attempted_action = "update") diff --git a/activerecord/lib/active_record/migration/command_recorder.rb b/activerecord/lib/active_record/migration/command_recorder.rb index 8e7f596076..efed4b0e26 100644 --- a/activerecord/lib/active_record/migration/command_recorder.rb +++ b/activerecord/lib/active_record/migration/command_recorder.rb @@ -14,6 +14,8 @@ module ActiveRecord # * change_column # * change_column_default (must supply a :from and :to option) # * change_column_null + # * change_column_comment (must supply a :from and :to option) + # * change_table_comment (must supply a :from and :to option) # * create_join_table # * create_table # * disable_extension @@ -35,7 +37,8 @@ module ActiveRecord :change_column_default, :add_reference, :remove_reference, :transaction, :drop_join_table, :drop_table, :execute_block, :enable_extension, :disable_extension, :change_column, :execute, :remove_columns, :change_column_null, - :add_foreign_key, :remove_foreign_key + :add_foreign_key, :remove_foreign_key, + :change_column_comment, :change_table_comment ] include JoinTable @@ -244,6 +247,26 @@ module ActiveRecord [:add_foreign_key, reversed_args] end + def invert_change_column_comment(args) + table, column, options = *args + + unless options && options.is_a?(Hash) && options.has_key?(:from) && options.has_key?(:to) + raise ActiveRecord::IrreversibleMigration, "change_column_comment is only reversible if given a :from and :to option." + end + + [:change_column_comment, [table, column, from: options[:to], to: options[:from]]] + end + + def invert_change_table_comment(args) + table, options = *args + + unless options && options.is_a?(Hash) && options.has_key?(:from) && options.has_key?(:to) + raise ActiveRecord::IrreversibleMigration, "change_table_comment is only reversible if given a :from and :to option." + end + + [:change_table_comment, [table, from: options[:to], to: options[:from]]] + end + def respond_to_missing?(method, _) super || delegate.respond_to?(method) end diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index abc939826b..ff91218696 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -27,6 +27,16 @@ module ActiveRecord def invert_transaction(args, &block) [:transaction, args, block] end + + def invert_change_column_comment(args) + table_name, column_name, comment = args + [:change_column_comment, [table_name, column_name, from: comment, to: comment]] + end + + def invert_change_table_comment(args) + table_name, comment = args + [:change_table_comment, [table_name, from: comment, to: comment]] + end end def create_table(table_name, **options) diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index e05490753f..8bade8cd28 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -530,7 +530,6 @@ module ActiveRecord def destroy _raise_readonly_record_error if readonly? destroy_associations - self.class.connection.add_transaction_record(self) @_trigger_destroy_callback = if persisted? destroy_row > 0 else @@ -852,7 +851,9 @@ module ActiveRecord end attribute_names = timestamp_attributes_for_update_in_model - attribute_names |= names.map(&:to_s) + attribute_names |= names.map!(&:to_s).map! { |name| + self.class.attribute_alias?(name) ? self.class.attribute_alias(name) : name + } unless attribute_names.empty? affected_rows = _touch_row(attribute_names, time) @@ -880,8 +881,7 @@ module ActiveRecord time ||= current_time_from_proper_timezone attribute_names.each do |attr_name| - write_attribute(attr_name, time) - clear_attribute_change(attr_name) + _write_attribute(attr_name, time) end _update_row(attribute_names, "touch") diff --git a/activerecord/lib/active_record/touch_later.rb b/activerecord/lib/active_record/touch_later.rb index 5dc88fb26c..980e42664b 100644 --- a/activerecord/lib/active_record/touch_later.rb +++ b/activerecord/lib/active_record/touch_later.rb @@ -22,7 +22,7 @@ module ActiveRecord @_touch_time = current_time_from_proper_timezone surreptitiously_touch @_defer_touch_attrs - self.class.connection.add_transaction_record self + add_to_transaction # touch the parents as we are not calling the after_save callbacks self.class.reflect_on_all_associations(:belongs_to).each do |r| diff --git a/activerecord/lib/active_record/transactions.rb b/activerecord/lib/active_record/transactions.rb index a45d228298..634dc50376 100644 --- a/activerecord/lib/active_record/transactions.rb +++ b/activerecord/lib/active_record/transactions.rb @@ -355,18 +355,6 @@ module ActiveRecord clear_transaction_record_state end - # Add the record to the current transaction so that the #after_rollback and #after_commit callbacks - # can be called. - def add_to_transaction - if has_transactional_callbacks? - self.class.connection.add_transaction_record(self) - else - sync_with_transaction_state - set_transaction_state(self.class.connection.transaction_state) - end - remember_transaction_record_state - end - # Executes +method+ within a transaction and captures its return value as a # status flag. If the status is true the transaction is committed, otherwise # a ROLLBACK is issued. In any case the status flag is returned. @@ -376,9 +364,19 @@ module ActiveRecord def with_transaction_returning_status status = nil self.class.transaction do - add_to_transaction + unless has_transactional_callbacks? + sync_with_transaction_state + @transaction_state = self.class.connection.transaction_state + end + remember_transaction_record_state + status = yield raise ActiveRecord::Rollback unless status + ensure + if has_transactional_callbacks? && + (@_new_record_before_last_commit && !new_record? || _trigger_update_callback || _trigger_destroy_callback) + add_to_transaction + end end status end @@ -415,13 +413,14 @@ module ActiveRecord # Force to clear the transaction record state. def force_clear_transaction_record_state @_start_transaction_state.clear + @transaction_state = nil end # Restore the new record state and id of a record that was previously saved by a call to save_record_state. - def restore_transaction_record_state(force = false) + def restore_transaction_record_state(force_restore_state = false) unless @_start_transaction_state.empty? transaction_level = (@_start_transaction_state[:level] || 0) - 1 - if transaction_level < 1 || force + if transaction_level < 1 || force_restore_state restore_state = @_start_transaction_state thaw @new_record = restore_state[:new_record] @@ -449,8 +448,10 @@ module ActiveRecord end end - def set_transaction_state(state) - @transaction_state = state + # Add the record to the current transaction so that the #after_rollback and #after_commit + # callbacks can be called. + def add_to_transaction + self.class.connection.add_transaction_record(self) end def has_transactional_callbacks? @@ -470,19 +471,17 @@ module ActiveRecord # This method checks to see if the ActiveRecord object's state reflects # the TransactionState, and rolls back or commits the Active Record object # as appropriate. - # - # Since Active Record objects can be inside multiple transactions, this - # method recursively goes through the parent of the TransactionState and - # checks if the Active Record object reflects the state of the object. def sync_with_transaction_state - update_attributes_from_transaction_state(@transaction_state) - end - - def update_attributes_from_transaction_state(transaction_state) - if transaction_state && transaction_state.finalized? - restore_transaction_record_state(transaction_state.fully_rolledback?) if transaction_state.rolledback? - force_clear_transaction_record_state if transaction_state.fully_committed? - clear_transaction_record_state if transaction_state.fully_completed? + if @transaction_state && @transaction_state.finalized? + if @transaction_state.fully_committed? + force_clear_transaction_record_state + elsif @transaction_state.committed? + clear_transaction_record_state + elsif @transaction_state.rolledback? + force_restore_state = @transaction_state.fully_rolledback? + restore_transaction_record_state(force_restore_state) + clear_transaction_record_state + end end end end diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index ba04859bf0..ce2ed06c1d 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -490,6 +490,8 @@ module ActiveRecord @connection.truncate("posts") assert_equal 0, Post.count + ensure + reset_fixtures("posts") end def test_truncate_with_query_cache @@ -501,6 +503,7 @@ module ActiveRecord assert_equal 0, Post.count ensure + reset_fixtures("posts") @connection.disable_query_cache! end @@ -514,6 +517,8 @@ module ActiveRecord assert_equal 0, Post.count assert_equal 0, Author.count assert_equal 0, AuthorAddress.count + ensure + reset_fixtures("posts", "authors", "author_addresses") end def test_truncate_tables_with_query_cache @@ -529,6 +534,7 @@ module ActiveRecord assert_equal 0, Author.count assert_equal 0, AuthorAddress.count ensure + reset_fixtures("posts", "authors", "author_addresses") @connection.disable_query_cache! end @@ -551,6 +557,16 @@ module ActiveRecord assert_nothing_raised { sub.save! } end end + + private + + def reset_fixtures(*fixture_names) + ActiveRecord::FixtureSet.reset_cache + + fixture_names.each do |fixture_name| + ActiveRecord::FixtureSet.create_fixtures(FIXTURES_ROOT, fixture_name) + end + end end end diff --git a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb index 88c2ac5d0a..c2c357d0c1 100644 --- a/activerecord/test/cases/adapters/mysql2/active_schema_test.rb +++ b/activerecord/test/cases/adapters/mysql2/active_schema_test.rb @@ -10,7 +10,15 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase ActiveRecord::Base.connection.send(:default_row_format) ActiveRecord::Base.connection.singleton_class.class_eval do alias_method :execute_without_stub, :execute - def execute(sql, name = nil) sql end + def execute(sql, name = nil) + ActiveSupport::Notifications.instrumenter.instrument( + "sql.active_record", + sql: sql, + name: name, + connection: self) do + sql + end + end end end @@ -89,17 +97,19 @@ class Mysql2ActiveSchemaTest < ActiveRecord::Mysql2TestCase %w(SPATIAL FULLTEXT UNIQUE).each do |type| expected = "ALTER TABLE `people` ADD #{type} INDEX `index_people_on_last_name` (`last_name`)" - actual = ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| - t.index :last_name, type: type + assert_sql(expected) do + ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| + t.index :last_name, type: type + end end - assert_equal expected, actual end expected = "ALTER TABLE `people` ADD INDEX `index_people_on_last_name` USING btree (`last_name`(10)), ALGORITHM = COPY" - actual = ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| - t.index :last_name, length: 10, using: :btree, algorithm: :copy + assert_sql(expected) do + ActiveRecord::Base.connection.change_table(:people, bulk: true) do |t| + t.index :last_name, length: 10, using: :btree, algorithm: :copy + end end - assert_equal expected, actual end def test_drop_table diff --git a/activerecord/test/cases/ar_schema_test.rb b/activerecord/test/cases/ar_schema_test.rb index 9027cc582a..7aa6d089c5 100644 --- a/activerecord/test/cases/ar_schema_test.rb +++ b/activerecord/test/cases/ar_schema_test.rb @@ -160,7 +160,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end if subsecond_precision_supported? - def test_timestamps_sets_presicion_on_create_table + def test_timestamps_sets_precision_on_create_table ActiveRecord::Schema.define do create_table :has_timestamps do |t| t.timestamps @@ -171,7 +171,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase assert @connection.column_exists?(:has_timestamps, :updated_at, precision: 6, null: false) end - def test_timestamps_sets_presicion_on_change_table + def test_timestamps_sets_precision_on_change_table ActiveRecord::Schema.define do create_table :has_timestamps @@ -185,7 +185,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end if ActiveRecord::Base.connection.supports_bulk_alter? - def test_timestamps_sets_presicion_on_change_table_with_bulk + def test_timestamps_sets_precision_on_change_table_with_bulk ActiveRecord::Schema.define do create_table :has_timestamps @@ -199,7 +199,7 @@ class ActiveRecordSchemaTest < ActiveRecord::TestCase end end - def test_timestamps_sets_presicion_on_add_timestamps + def test_timestamps_sets_precision_on_add_timestamps ActiveRecord::Schema.define do create_table :has_timestamps add_timestamps :has_timestamps, default: Time.now diff --git a/activerecord/test/cases/associations/eager_load_nested_include_test.rb b/activerecord/test/cases/associations/eager_load_nested_include_test.rb index 525ad3197a..849939de75 100644 --- a/activerecord/test/cases/associations/eager_load_nested_include_test.rb +++ b/activerecord/test/cases/associations/eager_load_nested_include_test.rb @@ -110,10 +110,10 @@ class EagerLoadNestedIncludeWithMissingDataTest < ActiveRecord::TestCase end teardown do - @davey_mcdave.destroy - @first_post.destroy @first_comment.destroy @first_categorization.destroy + @davey_mcdave.destroy + @first_post.destroy end def test_missing_data_in_a_nested_include_should_not_cause_errors_when_constructing_objects diff --git a/activerecord/test/cases/associations/has_many_associations_test.rb b/activerecord/test/cases/associations/has_many_associations_test.rb index 6a7efe2121..32285f269a 100644 --- a/activerecord/test/cases/associations/has_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_many_associations_test.rb @@ -2578,22 +2578,22 @@ class HasManyAssociationsTest < ActiveRecord::TestCase test "association with extend option" do post = posts(:welcome) - assert_equal "lifo", post.comments_with_extend.author - assert_equal "hello", post.comments_with_extend.greeting + assert_equal "lifo", post.comments_with_extend.author + assert_equal "hello :)", post.comments_with_extend.greeting end test "association with extend option with multiple extensions" do post = posts(:welcome) - assert_equal "lifo", post.comments_with_extend_2.author - assert_equal "hullo", post.comments_with_extend_2.greeting + assert_equal "lifo", post.comments_with_extend_2.author + assert_equal "hullo :)", post.comments_with_extend_2.greeting end test "extend option affects per association" do post = posts(:welcome) - assert_equal "lifo", post.comments_with_extend.author - assert_equal "lifo", post.comments_with_extend_2.author - assert_equal "hello", post.comments_with_extend.greeting - assert_equal "hullo", post.comments_with_extend_2.greeting + assert_equal "lifo", post.comments_with_extend.author + assert_equal "lifo", post.comments_with_extend_2.author + assert_equal "hello :)", post.comments_with_extend.greeting + assert_equal "hullo :)", post.comments_with_extend_2.greeting end test "delete record with complex joins" do diff --git a/activerecord/test/cases/associations/inverse_associations_test.rb b/activerecord/test/cases/associations/inverse_associations_test.rb index da3a42e2b5..669e176dcb 100644 --- a/activerecord/test/cases/associations/inverse_associations_test.rb +++ b/activerecord/test/cases/associations/inverse_associations_test.rb @@ -8,6 +8,7 @@ require "models/zine" require "models/club" require "models/sponsor" require "models/rating" +require "models/post" require "models/comment" require "models/car" require "models/bulb" @@ -62,6 +63,14 @@ class AutomaticInverseFindingTests < ActiveRecord::TestCase assert_equal rating_reflection, comment_reflection.inverse_of, "The Comment reflection's inverse should be the Rating reflection" end + def test_has_many_and_belongs_to_should_find_inverse_automatically_for_extension_block + comment_reflection = Comment.reflect_on_association(:post) + post_reflection = Post.reflect_on_association(:comments) + + assert_predicate post_reflection, :has_inverse? + assert_equal comment_reflection, post_reflection.inverse_of + end + def test_has_many_and_belongs_to_should_find_inverse_automatically_for_sti author_reflection = Author.reflect_on_association(:posts) author_child_reflection = Author.reflect_on_association(:special_posts) diff --git a/activerecord/test/cases/callbacks_test.rb b/activerecord/test/cases/callbacks_test.rb index 4d6a112af5..b4026078f1 100644 --- a/activerecord/test/cases/callbacks_test.rb +++ b/activerecord/test/cases/callbacks_test.rb @@ -458,10 +458,6 @@ class CallbacksTest < ActiveRecord::TestCase [ :before_validation, :object ], [ :before_validation, :block ], [ :before_validation, :throwing_abort ], - [ :after_rollback, :block ], - [ :after_rollback, :object ], - [ :after_rollback, :proc ], - [ :after_rollback, :method ], ], david.history end diff --git a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb index 36591097b6..a2d289bf2f 100644 --- a/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb +++ b/activerecord/test/cases/connection_adapters/connection_handlers_multi_db_test.rb @@ -203,26 +203,53 @@ module ActiveRecord assert_equal "must provide a `database` or a `role`.", error.message end - def test_switching_connections_with_database_symbol + def test_switching_connections_with_database_symbol_uses_default_role previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env" config = { "default_env" => { - "readonly" => { adapter: "sqlite3", database: "db/readonly.sqlite3" }, + "animals" => { adapter: "sqlite3", database: "db/animals.sqlite3" }, "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } } } @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config - ActiveRecord::Base.connected_to(database: :readonly) do - assert_equal :readonly, ActiveRecord::Base.current_role - assert ActiveRecord::Base.connected_to?(role: :readonly) + ActiveRecord::Base.connected_to(database: :animals) do + assert_equal :writing, ActiveRecord::Base.current_role + assert ActiveRecord::Base.connected_to?(role: :writing) + + handler = ActiveRecord::Base.connection_handler + assert_equal handler, ActiveRecord::Base.connection_handlers[:writing] + + assert_not_nil pool = handler.retrieve_connection_pool("primary") + assert_equal(config["default_env"]["animals"], pool.spec.config) + end + ensure + ActiveRecord::Base.configurations = @prev_configs + ActiveRecord::Base.establish_connection(:arunit) + ENV["RAILS_ENV"] = previous_env + end + + def test_switching_connections_with_database_hash_uses_passed_role_and_database + previous_env, ENV["RAILS_ENV"] = ENV["RAILS_ENV"], "default_env" + + config = { + "default_env" => { + "animals" => { adapter: "sqlite3", database: "db/animals.sqlite3" }, + "primary" => { adapter: "sqlite3", database: "db/primary.sqlite3" } + } + } + @prev_configs, ActiveRecord::Base.configurations = ActiveRecord::Base.configurations, config + + ActiveRecord::Base.connected_to(database: { writing: :primary }) do + assert_equal :writing, ActiveRecord::Base.current_role + assert ActiveRecord::Base.connected_to?(role: :writing) handler = ActiveRecord::Base.connection_handler - assert_equal handler, ActiveRecord::Base.connection_handlers[:readonly] + assert_equal handler, ActiveRecord::Base.connection_handlers[:writing] assert_not_nil pool = handler.retrieve_connection_pool("primary") - assert_equal(config["default_env"]["readonly"], pool.spec.config) + assert_equal(config["default_env"]["primary"], pool.spec.config) end ensure ActiveRecord::Base.configurations = @prev_configs diff --git a/activerecord/test/cases/invertible_migration_test.rb b/activerecord/test/cases/invertible_migration_test.rb index d68cc40107..7f67b945f0 100644 --- a/activerecord/test/cases/invertible_migration_test.rb +++ b/activerecord/test/cases/invertible_migration_test.rb @@ -103,6 +103,32 @@ module ActiveRecord end end + class ChangeColumnComment1 < SilentMigration + def change + create_table("horses") do |t| + t.column :name, :string, comment: "Sekitoba" + end + end + end + + class ChangeColumnComment2 < SilentMigration + def change + change_column_comment :horses, :name, from: "Sekitoba", to: "Diomed" + end + end + + class ChangeTableComment1 < SilentMigration + def change + create_table("horses", comment: "Sekitoba") + end + end + + class ChangeTableComment2 < SilentMigration + def change + change_table_comment :horses, from: "Sekitoba", to: "Diomed" + end + end + class DisableExtension1 < SilentMigration def change enable_extension "hstore" @@ -290,6 +316,7 @@ module ActiveRecord def test_migrate_revert_change_column_default migration1 = ChangeColumnDefault1.new migration1.migrate(:up) + Horse.reset_column_information assert_equal "Sekitoba", Horse.new.name migration2 = ChangeColumnDefault2.new @@ -302,6 +329,38 @@ module ActiveRecord assert_equal "Sekitoba", Horse.new.name end + if ActiveRecord::Base.connection.supports_comments? + def test_migrate_revert_change_column_comment + migration1 = ChangeColumnComment1.new + migration1.migrate(:up) + Horse.reset_column_information + assert_equal "Sekitoba", Horse.columns_hash["name"].comment + + migration2 = ChangeColumnComment2.new + migration2.migrate(:up) + Horse.reset_column_information + assert_equal "Diomed", Horse.columns_hash["name"].comment + + migration2.migrate(:down) + Horse.reset_column_information + assert_equal "Sekitoba", Horse.columns_hash["name"].comment + end + + def test_migrate_revert_change_table_comment + connection = ActiveRecord::Base.connection + migration1 = ChangeTableComment1.new + migration1.migrate(:up) + assert_equal "Sekitoba", connection.table_comment("horses") + + migration2 = ChangeTableComment2.new + migration2.migrate(:up) + assert_equal "Diomed", connection.table_comment("horses") + + migration2.migrate(:down) + assert_equal "Sekitoba", connection.table_comment("horses") + end + end + if current_adapter?(:PostgreSQLAdapter) def test_migrate_enable_and_disable_extension migration1 = InvertibleMigration.new diff --git a/activerecord/test/cases/locking_test.rb b/activerecord/test/cases/locking_test.rb index 33bd74e114..04f9b26960 100644 --- a/activerecord/test/cases/locking_test.rb +++ b/activerecord/test/cases/locking_test.rb @@ -182,7 +182,9 @@ class OptimisticLockingTest < ActiveRecord::TestCase p1.touch assert_equal 1, p1.lock_version - assert_not p1.changed?, "Changes should have been cleared" + assert_not_predicate p1, :changed?, "Changes should have been cleared" + assert_predicate p1, :saved_changes? + assert_equal ["lock_version", "updated_at"], p1.saved_changes.keys.sort end def test_touch_stale_object @@ -193,6 +195,8 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_raises(ActiveRecord::StaleObjectError) do stale_person.touch end + + assert_not_predicate stale_person, :saved_changes? end def test_update_with_dirty_primary_key @@ -296,6 +300,9 @@ class OptimisticLockingTest < ActiveRecord::TestCase t1.touch assert_equal 1, t1.lock_version + assert_not_predicate t1, :changed? + assert_predicate t1, :saved_changes? + assert_equal ["lock_version", "updated_at"], t1.saved_changes.keys.sort end def test_touch_stale_object_with_lock_without_default @@ -307,6 +314,8 @@ class OptimisticLockingTest < ActiveRecord::TestCase assert_raises(ActiveRecord::StaleObjectError) do stale_object.touch end + + assert_not_predicate stale_object, :saved_changes? end def test_lock_without_default_should_work_with_null_in_the_database diff --git a/activerecord/test/cases/migration/command_recorder_test.rb b/activerecord/test/cases/migration/command_recorder_test.rb index 01f8628fc5..c9f3756b1f 100644 --- a/activerecord/test/cases/migration/command_recorder_test.rb +++ b/activerecord/test/cases/migration/command_recorder_test.rb @@ -182,6 +182,40 @@ module ActiveRecord assert_equal [:change_column_default, [:table, :column, from: false, to: true]], change end + if ActiveRecord::Base.connection.supports_comments? + def test_invert_change_column_comment + assert_raises(ActiveRecord::IrreversibleMigration) do + @recorder.inverse_of :change_column_comment, [:table, :column, "comment"] + end + end + + def test_invert_change_column_comment_with_from_and_to + change = @recorder.inverse_of :change_column_comment, [:table, :column, from: "old_value", to: "new_value"] + assert_equal [:change_column_comment, [:table, :column, from: "new_value", to: "old_value"]], change + end + + def test_invert_change_column_comment_with_from_and_to_with_nil + change = @recorder.inverse_of :change_column_comment, [:table, :column, from: nil, to: "new_value"] + assert_equal [:change_column_comment, [:table, :column, from: "new_value", to: nil]], change + end + + def test_invert_change_table_comment + assert_raises(ActiveRecord::IrreversibleMigration) do + @recorder.inverse_of :change_column_comment, [:table, :column, "comment"] + end + end + + def test_invert_change_table_comment_with_from_and_to + change = @recorder.inverse_of :change_table_comment, [:table, from: "old_value", to: "new_value"] + assert_equal [:change_table_comment, [:table, from: "new_value", to: "old_value"]], change + end + + def test_invert_change_table_comment_with_from_and_to_with_nil + change = @recorder.inverse_of :change_table_comment, [:table, from: nil, to: "new_value"] + assert_equal [:change_table_comment, [:table, from: "new_value", to: nil]], change + end + end + def test_invert_change_column_null add = @recorder.inverse_of :change_column_null, [:table, :column, true] assert_equal [:change_column_null, [:table, :column, false]], add diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 5753bd7117..726ccf925e 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -220,6 +220,35 @@ module ActiveRecord end end + if ActiveRecord::Base.connection.supports_comments? + def test_change_column_comment_can_be_reverted + migration = Class.new(ActiveRecord::Migration[5.2]) { + def migrate(x) + revert do + change_column_comment(:testings, :foo, "comment") + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + assert connection.column_exists?(:testings, :foo, comment: "comment") + end + + def test_change_table_comment_can_be_reverted + migration = Class.new(ActiveRecord::Migration[5.2]) { + def migrate(x) + revert do + change_table_comment(:testings, "comment") + end + end + }.new + + ActiveRecord::Migrator.new(:up, [migration]).migrate + + assert_equal "comment", connection.table_comment("testings") + end + end + if current_adapter?(:PostgreSQLAdapter) class Testing < ActiveRecord::Base end @@ -242,9 +271,9 @@ module ActiveRecord private def precision_implicit_default if current_adapter?(:Mysql2Adapter) - { presicion: 0 } + { precision: 0 } else - { presicion: nil } + { precision: nil } end end end diff --git a/activerecord/test/cases/timestamp_test.rb b/activerecord/test/cases/timestamp_test.rb index 75ecd6fc40..232e018e03 100644 --- a/activerecord/test/cases/timestamp_test.rb +++ b/activerecord/test/cases/timestamp_test.rb @@ -40,17 +40,25 @@ class TimestampTest < ActiveRecord::TestCase assert_not_equal @previously_updated_at, @developer.updated_at assert_equal previous_salary + 10000, @developer.salary - assert @developer.salary_changed?, "developer salary should have changed" - assert @developer.changed?, "developer should be marked as changed" + assert_predicate @developer, :salary_changed?, "developer salary should have changed" + assert_predicate @developer, :changed?, "developer should be marked as changed" + assert_equal ["salary"], @developer.changed + assert_predicate @developer, :saved_changes? + assert_equal ["updated_at", "updated_on"], @developer.saved_changes.keys.sort + @developer.reload assert_equal previous_salary, @developer.salary end def test_touching_a_record_with_default_scope_that_excludes_it_updates_its_timestamp developer = @developer.becomes(DeveloperCalledJamis) - developer.touch + assert_not_equal @previously_updated_at, developer.updated_at + assert_not_predicate developer, :changed? + assert_predicate developer, :saved_changes? + assert_equal ["updated_at", "updated_on"], developer.saved_changes.keys.sort + developer.reload assert_not_equal @previously_updated_at, developer.updated_at end diff --git a/activerecord/test/cases/transaction_callbacks_test.rb b/activerecord/test/cases/transaction_callbacks_test.rb index e88d20a453..53fe31e087 100644 --- a/activerecord/test/cases/transaction_callbacks_test.rb +++ b/activerecord/test/cases/transaction_callbacks_test.rb @@ -111,6 +111,32 @@ class TransactionCallbacksTest < ActiveRecord::TestCase assert_equal [:after_commit], @first.history end + def test_dont_call_any_callbacks_after_transaction_commits_for_invalid_record + @first.after_commit_block { |r| r.history << :after_commit } + @first.after_rollback_block { |r| r.history << :after_rollback } + + def @first.valid?(*) + false + end + + assert_not @first.save + assert_equal [], @first.history + end + + def test_dont_call_any_callbacks_after_explicit_transaction_commits_for_invalid_record + @first.after_commit_block { |r| r.history << :after_commit } + @first.after_rollback_block { |r| r.history << :after_rollback } + + def @first.valid?(*) + false + end + + @first.transaction do + assert_not @first.save + end + assert_equal [], @first.history + end + def test_only_call_after_commit_on_save_after_transaction_commits_for_saving_record record = TopicWithCallbacks.new(title: "New topic", written_on: Date.today) record.after_commit_block(:save) { |r| r.history << :after_save } @@ -598,7 +624,7 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase @topic.content = "foo" @topic.save! end - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) end assert_equal [:before_commit, :after_commit], @topic.history end @@ -608,7 +634,7 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase @topic.transaction(requires_new: true) do @topic.content = "foo" @topic.save! - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) end end assert_equal [:before_commit, :after_commit], @topic.history @@ -629,7 +655,7 @@ class TransactionEnrollmentCallbacksTest < ActiveRecord::TestCase @topic.content = "foo" @topic.save! end - @topic.class.connection.add_transaction_record(@topic) + @topic.send(:add_to_transaction) raise ActiveRecord::Rollback end assert_equal [:rollback], @topic.history diff --git a/activerecord/test/cases/transactions_test.rb b/activerecord/test/cases/transactions_test.rb index 410f07d3ab..7bad3de343 100644 --- a/activerecord/test/cases/transactions_test.rb +++ b/activerecord/test/cases/transactions_test.rb @@ -26,13 +26,15 @@ class TransactionTest < ActiveRecord::TestCase def test_raise_after_destroy assert_not_predicate @first, :frozen? - assert_raises(RuntimeError) { - Topic.transaction do - @first.destroy - assert_predicate @first, :frozen? - raise + assert_not_called(@first, :rolledback!) do + assert_raises(RuntimeError) do + Topic.transaction do + @first.destroy + assert_predicate @first, :frozen? + raise + end end - } + end assert_not_predicate @first, :frozen? end @@ -63,7 +65,7 @@ class TransactionTest < ActiveRecord::TestCase def test_add_to_null_transaction topic = Topic.new - topic.add_to_transaction + topic.send(:add_to_transaction) end def test_successful_with_return diff --git a/activerecord/test/cases/validations_test.rb b/activerecord/test/cases/validations_test.rb index 9a70934b7e..4f98a6b7fc 100644 --- a/activerecord/test/cases/validations_test.rb +++ b/activerecord/test/cases/validations_test.rb @@ -145,15 +145,17 @@ class ValidationsTest < ActiveRecord::TestCase end def test_validates_acceptance_of_with_undefined_attribute_methods - Topic.validates_acceptance_of(:approved) - topic = Topic.new(approved: true) - Topic.undefine_attribute_methods + klass = Class.new(Topic) + klass.validates_acceptance_of(:approved) + topic = klass.new(approved: true) + klass.undefine_attribute_methods assert topic.approved end def test_validates_acceptance_of_as_database_column - Topic.validates_acceptance_of(:approved) - topic = Topic.create("approved" => true) + klass = Class.new(Topic) + klass.validates_acceptance_of(:approved) + topic = klass.create("approved" => true) assert topic["approved"] end diff --git a/activerecord/test/models/post.rb b/activerecord/test/models/post.rb index 395b534c63..c34968590f 100644 --- a/activerecord/test/models/post.rb +++ b/activerecord/test/models/post.rb @@ -11,6 +11,10 @@ class Post < ActiveRecord::Base def author "lifo" end + + def greeting + super + " :)" + end end module NamedExtension2 diff --git a/activesupport/lib/active_support/cache/redis_cache_store.rb b/activesupport/lib/active_support/cache/redis_cache_store.rb index 9a55e49e27..87f9aa5346 100644 --- a/activesupport/lib/active_support/cache/redis_cache_store.rb +++ b/activesupport/lib/active_support/cache/redis_cache_store.rb @@ -361,6 +361,7 @@ module ActiveSupport def read_multi_mget(*names) options = names.extract_options! options = merged_options(options) + return {} if names == [] keys = names.map { |name| normalize_key(name, options) } diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 1d87f74347..790534cd3c 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -140,6 +140,12 @@ module ActiveSupport::Cache::RedisCacheStoreTests end end + def test_fetch_multi_without_names + assert_not_called(@cache.redis, :mget) do + @cache.fetch_multi() { } + end + end + def test_increment_expires_in assert_called_with @cache.redis, :incrby, [ "#{@namespace}:foo", 1 ] do assert_called_with @cache.redis, :expire, [ "#{@namespace}:foo", 60 ] do diff --git a/guides/source/action_mailbox_basics.md b/guides/source/action_mailbox_basics.md index c90892d456..de92401226 100644 --- a/guides/source/action_mailbox_basics.md +++ b/guides/source/action_mailbox_basics.md @@ -19,9 +19,9 @@ Introduction ------------ Action Mailbox routes incoming emails to controller-like mailboxes for -processing in Rails. It ships with ingresses for Amazon SES, Mailgun, Mandrill, -Postmark, and SendGrid. You can also handle inbound mails directly via the -built-in Exim, Postfix, and Qmail ingresses. +processing in Rails. It ships with ingresses for Mailgun, Mandrill, Postmark, +and SendGrid. You can also handle inbound mails directly via the built-in Exim, +Postfix, and Qmail ingresses. The inbound emails are turned into `InboundEmail` records using Active Record and feature lifecycle tracking, storage of the original email on cloud storage @@ -43,28 +43,6 @@ $ rails db:migrate ## Configuration -### Amazon SES - -Install the [`aws-sdk-sns`](https://rubygems.org/gems/aws-sdk-sns) gem: - -```ruby -# Gemfile -gem "aws-sdk-sns", ">= 1.9.0", require: false -``` - -Tell Action Mailbox to accept emails from SES: - -```ruby -# config/environments/production.rb -config.action_mailbox.ingress = :amazon -``` - -[Configure SES](https://docs.aws.amazon.com/ses/latest/DeveloperGuide/receiving-email-notifications.html) -to deliver emails to your application via POST requests to -`/rails/action_mailbox/amazon/inbound_emails`. If your application lived at -`https://example.com`, you would specify the fully-qualified URL -`https://example.com/rails/action_mailbox/amazon/inbound_emails`. - ### Exim Tell Action Mailbox to accept emails from an SMTP relay: diff --git a/guides/source/command_line.md b/guides/source/command_line.md index a83724f1bb..0091271363 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -481,6 +481,22 @@ lib/school.rb: * [ 17] [FIXME] ``` +#### Tags + +You can add more default tags to search for by using `config.annotations.register_tags`. It receives a list of tags. + +```ruby +config.annotations.register_tags("DEPRECATEME", "TESTME") +``` + +```bash +$ rails notes +app/controllers/admin/users_controller.rb: + * [ 20] [TODO] do A/B testing on this + * [ 42] [TESTME] this needs more functional tests + * [132] [DEPRECATEME] ensure this method is deprecated in next release +``` + #### Directories You can add more default directories to search from by using `config.annotations.register_directories`. It receives a list of directory names. diff --git a/guides/source/documents.yaml b/guides/source/documents.yaml index 25e4fdb4e6..1e67b2bce7 100644 --- a/guides/source/documents.yaml +++ b/guides/source/documents.yaml @@ -155,6 +155,11 @@ name: Using Rails for API-only Applications url: api_app.html description: This guide explains how to effectively use Rails to develop a JSON API application. + - + name: Active Record and PostgreSQL + work_in_progress: true + url: active_record_postgresql.html + description: This guide covers PostgreSQL specific usage of Active Record. - name: Extending Rails diff --git a/railties/CHANGELOG.md b/railties/CHANGELOG.md index ab400e5982..109c4836d5 100644 --- a/railties/CHANGELOG.md +++ b/railties/CHANGELOG.md @@ -1,3 +1,12 @@ +* New applications get `config.cache_classes = false` in `config/environments/test.rb` + unless `--skip-spring`. + + *Xavier Noria* + +* Autoloading during initialization is deprecated. + + *Xavier Noria* + * Only force `:async` ActiveJob adapter to `:inline` during seeding. *BatedUrGonnaDie* diff --git a/railties/lib/rails/application/finisher.rb b/railties/lib/rails/application/finisher.rb index 1cf44a480c..109c560c80 100644 --- a/railties/lib/rails/application/finisher.rb +++ b/railties/lib/rails/application/finisher.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require "active_support/core_ext/string/inflections" +require "active_support/core_ext/array/conversions" + module Rails class Application module Finisher @@ -21,6 +24,40 @@ module Rails end end + # This will become an error if/when we remove classic mode. The plan is + # autoloaders won't be configured up to this point in the finisher, so + # constants just won't be found, raising regular NameError exceptions. + initializer :warn_if_autoloaded, before: :let_zeitwerk_take_over do + next if config.cache_classes + next if ActiveSupport::Dependencies.autoloaded_constants.empty? + + autoloaded = ActiveSupport::Dependencies.autoloaded_constants + constants = "constant".pluralize(autoloaded.size) + enum = autoloaded.to_sentence + have = autoloaded.size == 1 ? "has" : "have" + these = autoloaded.size == 1 ? "This" : "These" + example = autoloaded.first + example_klass = example.constantize.class + + ActiveSupport::DescendantsTracker.clear + ActiveSupport::Dependencies.clear + + ActiveSupport::Deprecation.warn(<<~WARNING) + Initialization autoloaded the #{constants} #{enum}. + + Being able to do this is deprecated. Autoloading during initialization is going + to be an error condition in future versions of Rails. + + Reloading does not reboot the application, and therefore code executed during + initialization does not run again. So, if you reload #{example}, for example, + the expected changes won't be reflected in that stale #{example_klass} object. + + #{these} autoloaded #{constants} #{have} been unloaded. + + Please, check the "Autoloading and Reloading Constants" guide for solutions. + WARNING + end + initializer :let_zeitwerk_take_over do if config.autoloader == :zeitwerk require "active_support/dependencies/zeitwerk_integration" diff --git a/railties/lib/rails/command/environment_argument.rb b/railties/lib/rails/command/environment_argument.rb index 0cb3f1ce1e..9945fd1430 100644 --- a/railties/lib/rails/command/environment_argument.rb +++ b/railties/lib/rails/command/environment_argument.rb @@ -9,7 +9,9 @@ module Rails extend ActiveSupport::Concern included do - class_attribute :environment_desc, default: "Specifies the environment to run this #{self.command_name} under (test/development/production)." + no_commands do + class_attribute :environment_desc, default: "Specifies the environment to run this #{self.command_name} under (test/development/production)." + end class_option :environment, aliases: "-e", type: :string, desc: environment_desc end diff --git a/railties/lib/rails/commands/notes/notes_command.rb b/railties/lib/rails/commands/notes/notes_command.rb index 64b339b3cd..94cf183855 100644 --- a/railties/lib/rails/commands/notes/notes_command.rb +++ b/railties/lib/rails/commands/notes/notes_command.rb @@ -5,7 +5,7 @@ require "rails/source_annotation_extractor" module Rails module Command class NotesCommand < Base # :nodoc: - class_option :annotations, aliases: "-a", desc: "Filter by specific annotations, e.g. Foobar TODO", type: :array, default: %w(OPTIMIZE FIXME TODO) + class_option :annotations, aliases: "-a", desc: "Filter by specific annotations, e.g. Foobar TODO", type: :array, default: Rails::SourceAnnotationExtractor::Annotation.tags def perform(*) require_application_and_environment! diff --git a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt index 63ed3fa952..c66e349442 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/environments/test.rb.tt @@ -1,11 +1,16 @@ +# The test environment is used exclusively to run your application's +# test suite. You never need to work with it otherwise. Remember that +# your test database is "scratch space" for the test suite and is wiped +# and recreated between test runs. Don't rely on the data there! + Rails.application.configure do # Settings specified here will take precedence over those in config/application.rb. - - # The test environment is used exclusively to run your application's - # test suite. You never need to work with it otherwise. Remember that - # your test database is "scratch space" for the test suite and is wiped - # and recreated between test runs. Don't rely on the data there! + <%# Spring executes the reloaders when files change. %> + <%- if spring_install? -%> + config.cache_classes = false + <%- else -%> config.cache_classes = true + <%- end -%> # Do not eager load code on boot. This avoids loading your whole application # just for the purpose of running a single test. If you are using a tool that diff --git a/railties/lib/rails/source_annotation_extractor.rb b/railties/lib/rails/source_annotation_extractor.rb index d7170e6282..9ce22b96a6 100644 --- a/railties/lib/rails/source_annotation_extractor.rb +++ b/railties/lib/rails/source_annotation_extractor.rb @@ -29,6 +29,16 @@ module Rails directories.push(*dirs) end + def self.tags + @@tags ||= %w(OPTIMIZE FIXME TODO) + end + + # Registers additional tags + # Rails::SourceAnnotationExtractor::Annotation.register_tags("TESTME", "DEPRECATEME") + def self.register_tags(*additional_tags) + tags.push(*additional_tags) + end + def self.extensions @@extensions ||= {} end @@ -66,6 +76,8 @@ module Rails # Prints all annotations with tag +tag+ under the root directories +app+, # +config+, +db+, +lib+, and +test+ (recursively). # + # If +tag+ is <tt>nil</tt>, annotations with either default or registered tags are printed. + # # Specific directories can be explicitly set using the <tt>:dirs</tt> key in +options+. # # Rails::SourceAnnotationExtractor.enumerate 'TODO|FIXME', dirs: %w(app lib), tag: true @@ -75,7 +87,8 @@ module Rails # See <tt>#find_in</tt> for a list of file extensions that will be taken into account. # # This class method is the single entry point for the `rails notes` command. - def self.enumerate(tag, options = {}) + def self.enumerate(tag = nil, options = {}) + tag ||= Annotation.tags.join("|") extractor = new(tag) dirs = options.delete(:dirs) || Annotation.directories extractor.display(extractor.find(dirs), options) diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index b8e167b488..a2e3e781c0 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -1704,6 +1704,61 @@ module ApplicationTests end end + test "autoloading during initialization gets deprecation message and clearing if config.cache_classes is false" do + app_file "lib/c.rb", <<~EOS + class C + extend ActiveSupport::DescendantsTracker + end + + class X < C + end + EOS + + app_file "app/models/d.rb", <<~EOS + require "c" + + class D < C + end + EOS + + app_file "config/initializers/autoload.rb", "D" + + app "development" + + # TODO: Test deprecation message, assert_depcrecated { app "development" } + # does not collect it. + + assert_equal [X], C.descendants + assert_empty ActiveSupport::Dependencies.autoloaded_constants + end + + test "autoloading during initialization triggers nothing if config.cache_classes is true" do + app_file "lib/c.rb", <<~EOS + class C + extend ActiveSupport::DescendantsTracker + end + + class X < C + end + EOS + + app_file "app/models/d.rb", <<~EOS + require "c" + + class D < C + end + EOS + + app_file "config/initializers/autoload.rb", "D" + + app "production" + + # TODO: Test no deprecation message is issued. + + assert_equal [X, D], C.descendants + end + + test "raises with proper error message if no database configuration found" do FileUtils.rm("#{app_path}/config/database.yml") err = assert_raises RuntimeError do diff --git a/railties/test/application/rake/routes_test.rb b/railties/test/application/rake/routes_test.rb index 9879d1f047..dffdae7bde 100644 --- a/railties/test/application/rake/routes_test.rb +++ b/railties/test/application/rake/routes_test.rb @@ -20,7 +20,6 @@ module ApplicationTests assert_equal <<~MESSAGE, run_rake_routes Prefix Verb URI Pattern Controller#Action cart GET /cart(.:format) cart#show - rails_amazon_inbound_emails POST /rails/action_mailbox/amazon/inbound_emails(.:format) action_mailbox/ingresses/amazon/inbound_emails#create rails_mandrill_inbound_emails POST /rails/action_mailbox/mandrill/inbound_emails(.:format) action_mailbox/ingresses/mandrill/inbound_emails#create rails_postmark_inbound_emails POST /rails/action_mailbox/postmark/inbound_emails(.:format) action_mailbox/ingresses/postmark/inbound_emails#create rails_relay_inbound_emails POST /rails/action_mailbox/relay/inbound_emails(.:format) action_mailbox/ingresses/relay/inbound_emails#create diff --git a/railties/test/commands/notes_test.rb b/railties/test/commands/notes_test.rb index 147019e299..9182541413 100644 --- a/railties/test/commands/notes_test.rb +++ b/railties/test/commands/notes_test.rb @@ -121,6 +121,47 @@ class Rails::Command::NotesTest < ActiveSupport::TestCase OUTPUT end + test "displays results from additional tags added to the default tags from a config file" do + app_file "app/models/profile.rb", "# TESTME: some method to test" + app_file "app/controllers/hello_controller.rb", "# DEPRECATEME: this action is no longer needed" + app_file "db/some_seeds.rb", "# TODO: default tags such as TODO are still present" + + add_to_config 'config.annotations.register_tags "TESTME", "DEPRECATEME"' + + assert_equal <<~OUTPUT, run_notes_command + app/controllers/hello_controller.rb: + * [1] [DEPRECATEME] this action is no longer needed + + app/models/profile.rb: + * [1] [TESTME] some method to test + + db/some_seeds.rb: + * [1] [TODO] default tags such as TODO are still present + + OUTPUT + end + + test "does not display results from tags that are neither default nor registered" do + app_file "app/models/profile.rb", "# TESTME: some method to test" + app_file "app/controllers/hello_controller.rb", "# DEPRECATEME: this action is no longer needed" + app_file "db/some_seeds.rb", "# TODO: default tags such as TODO are still present" + app_file "db/some_other_seeds.rb", "# BAD: this note should not be listed" + + add_to_config 'config.annotations.register_tags "TESTME", "DEPRECATEME"' + + assert_equal <<~OUTPUT, run_notes_command + app/controllers/hello_controller.rb: + * [1] [DEPRECATEME] this action is no longer needed + + app/models/profile.rb: + * [1] [TESTME] some method to test + + db/some_seeds.rb: + * [1] [TODO] default tags such as TODO are still present + + OUTPUT + end + private def run_notes_command(args = []) rails "notes", args diff --git a/railties/test/commands/routes_test.rb b/railties/test/commands/routes_test.rb index b4f927060e..a2dbd944f5 100644 --- a/railties/test/commands/routes_test.rb +++ b/railties/test/commands/routes_test.rb @@ -62,7 +62,6 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase assert_equal <<~MESSAGE, run_routes_command([ "-g", "POST" ]) Prefix Verb URI Pattern Controller#Action POST /cart(.:format) cart#create - rails_amazon_inbound_emails POST /rails/action_mailbox/amazon/inbound_emails(.:format) action_mailbox/ingresses/amazon/inbound_emails#create rails_mandrill_inbound_emails POST /rails/action_mailbox/mandrill/inbound_emails(.:format) action_mailbox/ingresses/mandrill/inbound_emails#create rails_postmark_inbound_emails POST /rails/action_mailbox/postmark/inbound_emails(.:format) action_mailbox/ingresses/postmark/inbound_emails#create rails_relay_inbound_emails POST /rails/action_mailbox/relay/inbound_emails(.:format) action_mailbox/ingresses/relay/inbound_emails#create @@ -166,7 +165,6 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase assert_equal <<~MESSAGE, run_routes_command Prefix Verb URI Pattern Controller#Action - rails_amazon_inbound_emails POST /rails/action_mailbox/amazon/inbound_emails(.:format) action_mailbox/ingresses/amazon/inbound_emails#create rails_mandrill_inbound_emails POST /rails/action_mailbox/mandrill/inbound_emails(.:format) action_mailbox/ingresses/mandrill/inbound_emails#create rails_postmark_inbound_emails POST /rails/action_mailbox/postmark/inbound_emails(.:format) action_mailbox/ingresses/postmark/inbound_emails#create rails_relay_inbound_emails POST /rails/action_mailbox/relay/inbound_emails(.:format) action_mailbox/ingresses/relay/inbound_emails#create @@ -207,101 +205,96 @@ class Rails::Command::RoutesTest < ActiveSupport::TestCase URI | /cart(.:format) Controller#Action | cart#show --[ Route 2 ]-------------- - Prefix | rails_amazon_inbound_emails - Verb | POST - URI | /rails/action_mailbox/amazon/inbound_emails(.:format) - Controller#Action | action_mailbox/ingresses/amazon/inbound_emails#create - --[ Route 3 ]-------------- Prefix | rails_mandrill_inbound_emails Verb | POST URI | /rails/action_mailbox/mandrill/inbound_emails(.:format) Controller#Action | action_mailbox/ingresses/mandrill/inbound_emails#create - --[ Route 4 ]-------------- + --[ Route 3 ]-------------- Prefix | rails_postmark_inbound_emails Verb | POST URI | /rails/action_mailbox/postmark/inbound_emails(.:format) Controller#Action | action_mailbox/ingresses/postmark/inbound_emails#create - --[ Route 5 ]-------------- + --[ Route 4 ]-------------- Prefix | rails_relay_inbound_emails Verb | POST URI | /rails/action_mailbox/relay/inbound_emails(.:format) Controller#Action | action_mailbox/ingresses/relay/inbound_emails#create - --[ Route 6 ]-------------- + --[ Route 5 ]-------------- Prefix | rails_sendgrid_inbound_emails Verb | POST URI | /rails/action_mailbox/sendgrid/inbound_emails(.:format) Controller#Action | action_mailbox/ingresses/sendgrid/inbound_emails#create - --[ Route 7 ]-------------- + --[ Route 6 ]-------------- Prefix | rails_mailgun_inbound_emails Verb | POST URI | /rails/action_mailbox/mailgun/inbound_emails/mime(.:format) Controller#Action | action_mailbox/ingresses/mailgun/inbound_emails#create - --[ Route 8 ]-------------- + --[ Route 7 ]-------------- Prefix | rails_conductor_inbound_emails Verb | GET URI | /rails/conductor/action_mailbox/inbound_emails(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#index - --[ Route 9 ]-------------- + --[ Route 8 ]-------------- Prefix | Verb | POST URI | /rails/conductor/action_mailbox/inbound_emails(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#create - --[ Route 10 ]------------- + --[ Route 9 ]-------------- Prefix | new_rails_conductor_inbound_email Verb | GET URI | /rails/conductor/action_mailbox/inbound_emails/new(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#new - --[ Route 11 ]------------- + --[ Route 10 ]------------- Prefix | edit_rails_conductor_inbound_email Verb | GET URI | /rails/conductor/action_mailbox/inbound_emails/:id/edit(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#edit - --[ Route 12 ]------------- + --[ Route 11 ]------------- Prefix | rails_conductor_inbound_email Verb | GET URI | /rails/conductor/action_mailbox/inbound_emails/:id(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#show - --[ Route 13 ]------------- + --[ Route 12 ]------------- Prefix | Verb | PATCH URI | /rails/conductor/action_mailbox/inbound_emails/:id(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#update - --[ Route 14 ]------------- + --[ Route 13 ]------------- Prefix | Verb | PUT URI | /rails/conductor/action_mailbox/inbound_emails/:id(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#update - --[ Route 15 ]------------- + --[ Route 14 ]------------- Prefix | Verb | DELETE URI | /rails/conductor/action_mailbox/inbound_emails/:id(.:format) Controller#Action | rails/conductor/action_mailbox/inbound_emails#destroy - --[ Route 16 ]------------- + --[ Route 15 ]------------- Prefix | rails_conductor_inbound_email_reroute Verb | POST URI | /rails/conductor/action_mailbox/:inbound_email_id/reroute(.:format) Controller#Action | rails/conductor/action_mailbox/reroutes#create - --[ Route 17 ]------------- + --[ Route 16 ]------------- Prefix | rails_service_blob Verb | GET URI | /rails/active_storage/blobs/:signed_id/*filename(.:format) Controller#Action | active_storage/blobs#show - --[ Route 18 ]------------- + --[ Route 17 ]------------- Prefix | rails_blob_representation Verb | GET URI | /rails/active_storage/representations/:signed_blob_id/:variation_key/*filename(.:format) Controller#Action | active_storage/representations#show - --[ Route 19 ]------------- + --[ Route 18 ]------------- Prefix | rails_disk_service Verb | GET URI | /rails/active_storage/disk/:encoded_key/*filename(.:format) Controller#Action | active_storage/disk#show - --[ Route 20 ]------------- + --[ Route 19 ]------------- Prefix | update_rails_disk_service Verb | PUT URI | /rails/active_storage/disk/:encoded_token(.:format) Controller#Action | active_storage/disk#update - --[ Route 21 ]------------- + --[ Route 20 ]------------- Prefix | rails_direct_uploads Verb | POST URI | /rails/active_storage/direct_uploads(.:format) diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 3bdbc70990..8771b53071 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -345,39 +345,44 @@ class AppGeneratorTest < Rails::Generators::TestCase app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-spring"] - FileUtils.cd(app_root) do - quietly { system("bin/rails app:update") } - end + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, skip_spring: true }, { destination_root: app_root, shell: @shell } + generator.send(:app_const) + quietly { generator.send(:update_config_files) } - assert_no_file "#{app_root}/config/spring.rb" + assert_no_file "#{app_root}/config/spring.rb" + end end def test_app_update_does_not_generate_action_cable_contents_when_skip_action_cable_is_given app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-action-cable"] - FileUtils.cd(app_root) do - quietly { system("bin/rails app:update") } - end + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, skip_action_cable: true }, { destination_root: app_root, shell: @shell } + generator.send(:app_const) + quietly { generator.send(:update_config_files) } - assert_no_file "#{app_root}/config/cable.yml" - assert_file "#{app_root}/config/environments/production.rb" do |content| - assert_no_match(/config\.action_cable/, content) + assert_no_file "#{app_root}/config/cable.yml" + assert_file "#{app_root}/config/environments/production.rb" do |content| + assert_no_match(/config\.action_cable/, content) + end + assert_no_file "#{app_root}/test/channels/application_cable/connection_test.rb" end - - assert_no_file "#{app_root}/test/channels/application_cable/connection_test.rb" end def test_app_update_does_not_generate_bootsnap_contents_when_skip_bootsnap_is_given app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-bootsnap"] - FileUtils.cd(app_root) do - quietly { system("bin/rails app:update") } - end + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, skip_bootsnap: true }, { destination_root: app_root, shell: @shell } + generator.send(:app_const) + quietly { generator.send(:update_config_files) } - assert_file "#{app_root}/config/boot.rb" do |content| - assert_no_match(/require 'bootsnap\/setup'/, content) + assert_file "#{app_root}/config/boot.rb" do |content| + assert_no_match(/require 'bootsnap\/setup'/, content) + end end end @@ -396,46 +401,50 @@ class AppGeneratorTest < Rails::Generators::TestCase app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-active-storage"] - FileUtils.cd(app_root) do - quietly { system("bin/rails app:update") } - end + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, skip_active_storage: true }, { destination_root: app_root, shell: @shell } + generator.send(:app_const) + quietly { generator.send(:update_config_files) } - assert_file "#{app_root}/config/environments/development.rb" do |content| - assert_no_match(/config\.active_storage/, content) - end + assert_file "#{app_root}/config/environments/development.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end - assert_file "#{app_root}/config/environments/production.rb" do |content| - assert_no_match(/config\.active_storage/, content) - end + assert_file "#{app_root}/config/environments/production.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end - assert_file "#{app_root}/config/environments/test.rb" do |content| - assert_no_match(/config\.active_storage/, content) - end + assert_file "#{app_root}/config/environments/test.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end - assert_no_file "#{app_root}/config/storage.yml" + assert_no_file "#{app_root}/config/storage.yml" + end end def test_app_update_does_not_generate_active_storage_contents_when_skip_active_record_is_given app_root = File.join(destination_root, "myapp") run_generator [app_root, "--skip-active-record"] - FileUtils.cd(app_root) do - quietly { system("bin/rails app:update") } - end + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true, skip_active_record: true }, { destination_root: app_root, shell: @shell } + generator.send(:app_const) + quietly { generator.send(:update_config_files) } - assert_file "#{app_root}/config/environments/development.rb" do |content| - assert_no_match(/config\.active_storage/, content) - end + assert_file "#{app_root}/config/environments/development.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end - assert_file "#{app_root}/config/environments/production.rb" do |content| - assert_no_match(/config\.active_storage/, content) - end + assert_file "#{app_root}/config/environments/production.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end - assert_file "#{app_root}/config/environments/test.rb" do |content| - assert_no_match(/config\.active_storage/, content) - end + assert_file "#{app_root}/config/environments/test.rb" do |content| + assert_no_match(/config\.active_storage/, content) + end - assert_no_file "#{app_root}/config/storage.yml" + assert_no_file "#{app_root}/config/storage.yml" + end end def test_generator_skips_action_mailbox_when_skip_action_mailbox_is_given @@ -469,16 +478,17 @@ class AppGeneratorTest < Rails::Generators::TestCase end def test_app_update_does_not_change_config_target_version - run_generator + app_root = File.join(destination_root, "myapp") + run_generator [app_root, "--skip-spring"] - FileUtils.cd(destination_root) do + FileUtils.cd(app_root) do config = "config/application.rb" content = File.read(config) File.write(config, content.gsub(/config\.load_defaults #{Rails::VERSION::STRING.to_f}/, "config.load_defaults 5.1")) quietly { system("bin/rails app:update") } end - assert_file "config/application.rb", /\s+config\.load_defaults 5\.1/ + assert_file "#{app_root}/config/application.rb", /\s+config\.load_defaults 5\.1/ end def test_app_update_does_not_change_app_name_when_app_name_is_hyphenated_name @@ -495,13 +505,15 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_match(/hyphenated-app/, content) end - FileUtils.cd(app_root) do - quietly { system("bin/rails app:update") } - end + stub_rails_application(app_root) do + generator = Rails::Generators::AppGenerator.new ["rails"], { update: true }, { destination_root: app_root, shell: @shell } + generator.send(:app_const) + quietly { generator.send(:update_config_files) } - assert_file "#{app_root}/config/cable.yml" do |content| - assert_match(/hyphenated_app/, content) - assert_no_match(/hyphenated-app/, content) + assert_file "#{app_root}/config/cable.yml" do |content| + assert_match(/hyphenated_app/, content) + assert_no_match(/hyphenated-app/, content) + end end end @@ -827,6 +839,9 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_spring run_generator assert_gem "spring" + assert_file("config/environments/test.rb") do |contents| + assert_match("config.cache_classes = false", contents) + end end def test_bundler_binstub @@ -845,7 +860,7 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_spring_no_fork jruby_skip "spring doesn't run on JRuby" - assert_called_with(Process, :respond_to?, [[:fork], [:fork]], returns: false) do + assert_called_with(Process, :respond_to?, [[:fork], [:fork], [:fork]], returns: false) do run_generator assert_no_gem "spring" @@ -857,6 +872,9 @@ class AppGeneratorTest < Rails::Generators::TestCase assert_no_file "config/spring.rb" assert_no_gem "spring" + assert_file("config/environments/test.rb") do |contents| + assert_match("config.cache_classes = true", contents) + end end def test_spring_with_dev_option |