diff options
167 files changed, 1445 insertions, 418 deletions
@@ -25,7 +25,7 @@ the [Active Model](activemodel/README.rdoc) module. ## Controller layer The _**Controller layer**_ is responsible for handling incoming HTTP requests and -providing a suitable response. Usually this means returning HTML, but Rails controllers +providing a suitable response. Usually, this means returning HTML, but Rails controllers can also generate XML, JSON, PDFs, mobile-specific views, and more. Controllers load and manipulate models, and render view templates in order to generate the appropriate HTTP response. In Rails, incoming requests are routed by Action Dispatch to an appropriate controller, and @@ -37,7 +37,7 @@ are bundled together in [Action Pack](actionpack/README.rdoc). The _**View layer**_ is composed of "templates" that are responsible for providing appropriate representations of your application's resources. Templates can come in a variety of formats, but most view templates are HTML with embedded -Ruby code (ERB files). Views are typically rendered to generate a controller response, +Ruby code (ERB files). Views are typically rendered to generate a controller response or to generate the body of an email. In Rails, View generation is handled by [Action View](actionview/README.rdoc). ## Frameworks and libraries diff --git a/actioncable/test/client_test.rb b/actioncable/test/client_test.rb index bf141df458..eed514db10 100644 --- a/actioncable/test/client_test.rb +++ b/actioncable/test/client_test.rb @@ -91,7 +91,7 @@ class ClientTest < ActionCable::TestCase rescue RuntimeError => ex # Work around https://bugs.ruby-lang.org/issues/13239 - raise unless ex.message =~ /can't modify frozen IOError/ + raise unless ex.message.match?(/can't modify frozen IOError/) # Handle this as if it were the IOError: do the same as above. server.binder.close diff --git a/actionmailbox/CHANGELOG.md b/actionmailbox/CHANGELOG.md index bca3d1d9ed..0d4799a7b8 100644 --- a/actionmailbox/CHANGELOG.md +++ b/actionmailbox/CHANGELOG.md @@ -1,3 +1,7 @@ +* Fix Bcc header not being included with emails from `create_inbound_email_from` test helpers. + + *jduff* + * Add `ApplicationMailbox.mailbox_for` to expose mailbox routing. *James Dabbs* diff --git a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb index 8713f545f5..cd8b12dcde 100644 --- a/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb +++ b/actionmailbox/app/controllers/rails/conductor/action_mailbox/inbound_emails_controller.rb @@ -23,7 +23,7 @@ module Rails Mail.new(params.require(:mail).permit(:from, :to, :cc, :bcc, :in_reply_to, :subject, :body).to_h).tap do |mail| mail[:bcc]&.include_in_headers = true params[:mail][:attachments].to_a.each do |attachment| - mail.add_file(filename: attachment.path, content: attachment.read) + mail.add_file(filename: attachment.original_filename, content: attachment.read) end end end diff --git a/actionmailbox/lib/action_mailbox/test_helper.rb b/actionmailbox/lib/action_mailbox/test_helper.rb index d248fa8734..dc6a0229f7 100644 --- a/actionmailbox/lib/action_mailbox/test_helper.rb +++ b/actionmailbox/lib/action_mailbox/test_helper.rb @@ -14,7 +14,11 @@ module ActionMailbox # # create_inbound_email_from_mail(from: "david@loudthinking.com", subject: "Hello!") def create_inbound_email_from_mail(status: :processing, **mail_options) - create_inbound_email_from_source Mail.new(mail_options).to_s, status: status + mail = Mail.new(mail_options) + # Bcc header is not encoded by default + mail[:bcc].include_in_headers = true if mail[:bcc] + + create_inbound_email_from_source mail.to_s, status: status end # Create an +InboundEmail+ using the raw rfc822 +source+ as text. diff --git a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb index 6fc39c2433..33282d36f7 100644 --- a/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb +++ b/actionmailbox/test/controllers/rails/action_mailbox/inbound_emails_controller_test.rb @@ -30,6 +30,27 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa end end + test "create inbound email with bcc" do + with_rails_env("development") do + assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do + post rails_conductor_inbound_emails_path, params: { + mail: { + from: "Jason Fried <jason@37signals.com>", + bcc: "Replies <replies@example.com>", + subject: "Hey there", + body: "How's it going?" + } + } + end + + mail = ActionMailbox::InboundEmail.last.mail + assert_equal %w[ jason@37signals.com ], mail.from + assert_equal %w[ replies@example.com ], mail.bcc + assert_equal "Hey there", mail.subject + assert_equal "How's it going?", mail.body.decoded + end + end + test "create inbound email with attachments" do with_rails_env("development") do assert_difference -> { ActionMailbox::InboundEmail.count }, +1 do @@ -47,6 +68,7 @@ class Rails::Conductor::ActionMailbox::InboundEmailsControllerTest < ActionDispa mail = ActionMailbox::InboundEmail.last.mail assert_equal "Let's talk about these images:", mail.text_part.decoded assert_equal 2, mail.attachments.count + assert_equal %w[ avatar1.jpeg avatar2.jpeg ], mail.attachments.collect(&:filename) end end diff --git a/actionmailbox/test/dummy/config/initializers/backtrace_silencers.rb b/actionmailbox/test/dummy/config/initializers/backtrace_silencers.rb index 59385cdf37..3c56b21b3c 100644 --- a/actionmailbox/test/dummy/config/initializers/backtrace_silencers.rb +++ b/actionmailbox/test/dummy/config/initializers/backtrace_silencers.rb @@ -1,7 +1,7 @@ # Be sure to restart your server when you modify this file. # You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. -# Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } +# Rails.backtrace_cleaner.add_silencer { |line| /my_noisy_library/.match?(line) } # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. # Rails.backtrace_cleaner.remove_silencers! diff --git a/actionmailbox/test/unit/router_test.rb b/actionmailbox/test/unit/router_test.rb index 4dd3730604..7eb8e04a73 100644 --- a/actionmailbox/test/unit/router_test.rb +++ b/actionmailbox/test/unit/router_test.rb @@ -51,6 +51,15 @@ module ActionMailbox assert_equal inbound_email.mail, $processed_mail end + test "single string routing on bcc" do + @router.add_routes("first@example.com" => :first) + + inbound_email = create_inbound_email_from_mail(to: "someone@example.com", bcc: "first@example.com", subject: "This is a reply") + @router.route inbound_email + assert_equal "FirstMailbox", $processed_by + assert_equal inbound_email.mail, $processed_mail + end + test "single string routing case-insensitively" do @router.add_routes("first@example.com" => :first) diff --git a/actionmailer/lib/action_mailer/base.rb b/actionmailer/lib/action_mailer/base.rb index ba914c4813..78264d21d1 100644 --- a/actionmailer/lib/action_mailer/base.rb +++ b/actionmailer/lib/action_mailer/base.rb @@ -408,7 +408,7 @@ module ActionMailer # really useful if you need to validate a self-signed and/or a wildcard certificate. You can use the name # of an OpenSSL verify constant (<tt>'none'</tt> or <tt>'peer'</tt>) or directly the constant # (<tt>OpenSSL::SSL::VERIFY_NONE</tt> or <tt>OpenSSL::SSL::VERIFY_PEER</tt>). - # <tt>:ssl/:tls</tt> Enables the SMTP connection to use SMTP/TLS (SMTPS: SMTP over direct TLS connection) + # * <tt>:ssl/:tls</tt> Enables the SMTP connection to use SMTP/TLS (SMTPS: SMTP over direct TLS connection) # # * <tt>sendmail_settings</tt> - Allows you to override options for the <tt>:sendmail</tt> delivery method. # * <tt>:location</tt> - The location of the sendmail executable. Defaults to <tt>/usr/sbin/sendmail</tt>. @@ -737,7 +737,7 @@ module ActionMailer end class LateAttachmentsProxy < SimpleDelegator - def inline; _raise_error end + def inline; self end def []=(_name, _content); _raise_error end private diff --git a/actionmailer/test/base_test.rb b/actionmailer/test/base_test.rb index 4ea5634cd0..a6223964e5 100644 --- a/actionmailer/test/base_test.rb +++ b/actionmailer/test/base_test.rb @@ -267,6 +267,17 @@ class BaseTest < ActiveSupport::TestCase assert_match(/Can't add attachments after `mail` was called./, e.message) end + test "accessing inline attachments after mail was called works" do + class LateInlineAttachmentMailer < ActionMailer::Base + def welcome + mail body: "yay", from: "welcome@example.com", to: "to@example.com" + attachments.inline["invoice.pdf"] + end + end + + assert_nothing_raised { LateInlineAttachmentMailer.welcome.message } + end + test "adding inline attachments while rendering mail works" do class LateInlineAttachmentMailer < ActionMailer::Base def on_render diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index ca7ae6621b..90217755bc 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,27 @@ +* Allow system test screen shots to be taken more than once in + a test by prefixing the file name with an incrementing counter. + + Add an environment variable `RAILS_SYSTEM_TESTING_SCREENSHOT_HTML` to + enable saving of HTML during a screenshot in addition to the image. + This uses the same image name, with the extension replaced with `.html` + + *Tom Fakes* + +* Add `Vary: Accept` header when using `Accept` header for response + + For some requests like `/users/1`, Rails uses requests' `Accept` + header to determine what to return. And if we don't add `Vary` + in the response header, browsers might accidentally cache different + types of content, which would cause issues: e.g. javascript got displayed + instead of html content. This PR fixes these issues by adding `Vary: Accept` + in these types of requests. For more detailed problem description, please read: + + https://github.com/rails/rails/pull/36213 + + Fixes #25842 + + *Stan Lo* + * Fix IntegrationTest `follow_redirect!` to follow redirection using the same HTTP verb when following a 307 redirection. @@ -65,7 +89,7 @@ *Gustavo Gutierrez* -* Calling `ActionController::Parameters#transform_keys/!` without a block now returns +* Calling `ActionController::Parameters#transform_keys`/`!` without a block now returns an enumerator for the parameters instead of the underlying hash. *Eugene Kenny* diff --git a/actionpack/lib/abstract_controller/rendering.rb b/actionpack/lib/abstract_controller/rendering.rb index 8ba2b25552..280af50a44 100644 --- a/actionpack/lib/abstract_controller/rendering.rb +++ b/actionpack/lib/abstract_controller/rendering.rb @@ -28,6 +28,7 @@ module AbstractController else _set_rendered_content_type rendered_format end + _set_vary_header self.response_body = rendered_body end @@ -109,6 +110,9 @@ module AbstractController def _set_html_content_type # :nodoc: end + def _set_vary_header # :nodoc: + end + def _set_rendered_content_type(format) # :nodoc: end diff --git a/actionpack/lib/action_controller/metal/http_authentication.rb b/actionpack/lib/action_controller/metal/http_authentication.rb index 6a274d35cb..ec0c9ecc67 100644 --- a/actionpack/lib/action_controller/metal/http_authentication.rb +++ b/actionpack/lib/action_controller/metal/http_authentication.rb @@ -482,7 +482,7 @@ module ActionController def raw_params(auth) _raw_params = auth.sub(TOKEN_REGEX, "").split(/\s*#{AUTHN_PAIR_DELIMITERS}\s*/) - if !(_raw_params.first =~ %r{\A#{TOKEN_KEY}}) + if !(%r{\A#{TOKEN_KEY}}.match?(_raw_params.first)) _raw_params[0] = "#{TOKEN_KEY}#{_raw_params.first}" end diff --git a/actionpack/lib/action_controller/metal/instrumentation.rb b/actionpack/lib/action_controller/metal/instrumentation.rb index 6f7fc0d624..4c28a45c69 100644 --- a/actionpack/lib/action_controller/metal/instrumentation.rb +++ b/actionpack/lib/action_controller/metal/instrumentation.rb @@ -27,7 +27,7 @@ module ActionController path: request.fullpath } - ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload.dup) + ActiveSupport::Notifications.instrument("start_processing.action_controller", raw_payload) ActiveSupport::Notifications.instrument("process_action.action_controller", raw_payload) do |payload| super.tap do diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 5c6f7fe396..a993c76af9 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -142,7 +142,7 @@ module ActionController #:nodoc: # # You can set the variant in a +before_action+: # - # request.variant = :tablet if request.user_agent =~ /iPad/ + # request.variant = :tablet if /iPad/.match?(request.user_agent) # # Respond to variants in the action just like you respond to formats: # diff --git a/actionpack/lib/action_controller/metal/rendering.rb b/actionpack/lib/action_controller/metal/rendering.rb index efa5de313c..fd22c4fa64 100644 --- a/actionpack/lib/action_controller/metal/rendering.rb +++ b/actionpack/lib/action_controller/metal/rendering.rb @@ -77,6 +77,10 @@ module ActionController end end + def _set_vary_header + self.headers["Vary"] = "Accept" if request.should_apply_vary_header? + end + # Normalize arguments by catching blocks and setting them on :update. def _normalize_args(action = nil, options = {}, &blk) options = super diff --git a/actionpack/lib/action_controller/metal/request_forgery_protection.rb b/actionpack/lib/action_controller/metal/request_forgery_protection.rb index 5a5c04234b..e923afb17c 100644 --- a/actionpack/lib/action_controller/metal/request_forgery_protection.rb +++ b/actionpack/lib/action_controller/metal/request_forgery_protection.rb @@ -280,7 +280,7 @@ module ActionController #:nodoc: # Check for cross-origin JavaScript responses. def non_xhr_javascript_response? # :doc: - content_type =~ %r(\A(?:text|application)/javascript) && !request.xhr? + %r(\A(?:text|application)/javascript).match?(content_type) && !request.xhr? end AUTHENTICITY_TOKEN_LENGTH = 32 diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index 920ae52f2b..4c9eb20c65 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -225,7 +225,7 @@ module ActionController class << self def nested_attribute?(key, value) # :nodoc: - key =~ /\A-?\d+\z/ && (value.is_a?(Hash) || value.is_a?(Parameters)) + /\A-?\d+\z/.match?(key) && (value.is_a?(Hash) || value.is_a?(Parameters)) end end diff --git a/actionpack/lib/action_controller/renderer.rb b/actionpack/lib/action_controller/renderer.rb index dadf6d3445..b48c7a1afa 100644 --- a/actionpack/lib/action_controller/renderer.rb +++ b/actionpack/lib/action_controller/renderer.rb @@ -65,7 +65,7 @@ module ActionController def initialize(controller, env, defaults) @controller = controller @defaults = defaults - @env = normalize_keys defaults.merge(env) + @env = normalize_keys defaults, env end # Render templates with any options from ActionController::Base#render_to_string. @@ -97,8 +97,9 @@ module ActionController end private - def normalize_keys(env) + def normalize_keys(defaults, env) new_env = {} + defaults.each_pair { |k, v| new_env[rack_key_for(k)] = rack_value_for(k, v) } env.each_pair { |k, v| new_env[rack_key_for(k)] = rack_value_for(k, v) } new_env["rack.url_scheme"] = new_env["HTTPS"] == "on" ? "https" : "http" new_env @@ -120,7 +121,7 @@ module ActionController } def rack_key_for(key) - RACK_KEY_TRANSLATION.fetch(key, key.to_s) + RACK_KEY_TRANSLATION[key] || key.to_s end def rack_value_for(key, value) diff --git a/actionpack/lib/action_controller/test_case.rb b/actionpack/lib/action_controller/test_case.rb index 47e0099f20..1632ac3ae8 100644 --- a/actionpack/lib/action_controller/test_case.rb +++ b/actionpack/lib/action_controller/test_case.rb @@ -593,8 +593,8 @@ module ActionController private def scrub_env!(env) - env.delete_if { |k, v| k =~ /^(action_dispatch|rack)\.request/ } - env.delete_if { |k, v| k =~ /^action_dispatch\.rescue/ } + env.delete_if { |k, v| k.match?(/^(action_dispatch|rack)\.request/) } + env.delete_if { |k, v| k.match?(/^action_dispatch\.rescue/) } env.delete "action_dispatch.request.query_parameters" env.delete "action_dispatch.request.request_parameters" env["rack.input"] = StringIO.new diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index 9c430b57e3..e8cf1b95a5 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -33,7 +33,7 @@ module ActionDispatch #:nodoc: private def html_response?(headers) if content_type = headers[CONTENT_TYPE] - content_type =~ /html/ + /html/.match?(content_type) end end diff --git a/actionpack/lib/action_dispatch/http/feature_policy.rb b/actionpack/lib/action_dispatch/http/feature_policy.rb index 592b6e4393..78e982918d 100644 --- a/actionpack/lib/action_dispatch/http/feature_policy.rb +++ b/actionpack/lib/action_dispatch/http/feature_policy.rb @@ -33,7 +33,7 @@ module ActionDispatch #:nodoc: private def html_response?(headers) if content_type = headers[CONTENT_TYPE] - content_type =~ /html/ + /html/.match?(content_type) end end diff --git a/actionpack/lib/action_dispatch/http/filter_parameters.rb b/actionpack/lib/action_dispatch/http/filter_parameters.rb index 7a7a493f64..7ad1ba3e0e 100644 --- a/actionpack/lib/action_dispatch/http/filter_parameters.rb +++ b/actionpack/lib/action_dispatch/http/filter_parameters.rb @@ -23,7 +23,7 @@ module ActionDispatch # change { file: { code: "xxxx"} } # # env["action_dispatch.parameter_filter"] = -> (k, v) do - # v.reverse! if k =~ /secret/i + # v.reverse! if k.match?(/secret/i) # end # => reverses the value to all keys matching /secret/i module FilterParameters diff --git a/actionpack/lib/action_dispatch/http/filter_redirect.rb b/actionpack/lib/action_dispatch/http/filter_redirect.rb index d780d5f793..3bd1f5109d 100644 --- a/actionpack/lib/action_dispatch/http/filter_redirect.rb +++ b/actionpack/lib/action_dispatch/http/filter_redirect.rb @@ -27,7 +27,7 @@ module ActionDispatch if String === filter location.include?(filter) elsif Regexp === filter - location =~ filter + location.match?(filter) end end end diff --git a/actionpack/lib/action_dispatch/http/mime_negotiation.rb b/actionpack/lib/action_dispatch/http/mime_negotiation.rb index a2cac49082..6bf4e652d3 100644 --- a/actionpack/lib/action_dispatch/http/mime_negotiation.rb +++ b/actionpack/lib/action_dispatch/http/mime_negotiation.rb @@ -62,13 +62,7 @@ module ActionDispatch def formats fetch_header("action_dispatch.request.formats") do |k| - params_readable = begin - parameters[:format] - rescue *RESCUABLE_MIME_FORMAT_ERRORS - false - end - - v = if params_readable + v = if params_readable? Array(Mime[parameters[:format]]) elsif use_accept_header && valid_accept_header accepts @@ -153,12 +147,22 @@ module ActionDispatch order.include?(Mime::ALL) ? format : nil end + def should_apply_vary_header? + !params_readable? && use_accept_header && valid_accept_header + end + private BROWSER_LIKE_ACCEPTS = /,\s*\*\/\*|\*\/\*\s*,/ + def params_readable? # :doc: + parameters[:format] + rescue *RESCUABLE_MIME_FORMAT_ERRORS + false + end + def valid_accept_header # :doc: (xhr? && (accept.present? || content_mime_type)) || - (accept.present? && accept !~ BROWSER_LIKE_ACCEPTS) + (accept.present? && !accept.match?(BROWSER_LIKE_ACCEPTS)) end def use_accept_header # :doc: diff --git a/actionpack/lib/action_dispatch/http/mime_type.rb b/actionpack/lib/action_dispatch/http/mime_type.rb index ed1d50f3b9..60b78c0582 100644 --- a/actionpack/lib/action_dispatch/http/mime_type.rb +++ b/actionpack/lib/action_dispatch/http/mime_type.rb @@ -202,7 +202,7 @@ module Mime # For an input of <tt>'application'</tt>, returns <tt>[Mime[:html], Mime[:js], # Mime[:xml], Mime[:yaml], Mime[:atom], Mime[:json], Mime[:rss], Mime[:url_encoded_form]</tt>. def parse_data_with_trailing_star(type) - Mime::SET.select { |m| m =~ type } + Mime::SET.select { |m| m.match?(type) } end # This method is opposite of register method. @@ -283,8 +283,14 @@ module Mime @synonyms.any? { |synonym| synonym.to_s =~ regexp } || @string =~ regexp end + def match?(mime_type) + return false unless mime_type + regexp = Regexp.new(Regexp.quote(mime_type.to_s)) + @synonyms.any? { |synonym| synonym.to_s.match?(regexp) } || @string.match?(regexp) + end + def html? - symbol == :html || @string =~ /html/ + (symbol == :html) || /html/.match?(@string) end def all?; false; end diff --git a/actionpack/lib/action_dispatch/http/request.rb b/actionpack/lib/action_dispatch/http/request.rb index 4ac7c5c2bd..54dbb536c1 100644 --- a/actionpack/lib/action_dispatch/http/request.rb +++ b/actionpack/lib/action_dispatch/http/request.rb @@ -85,7 +85,7 @@ module ActionDispatch def controller_class_for(name) if name controller_param = name.underscore - const_name = "#{controller_param.camelize}Controller" + const_name = controller_param.camelize << "Controller" ActiveSupport::Dependencies.constantize(const_name) else PASS_NOT_FOUND @@ -265,7 +265,7 @@ module ActionDispatch # (case-insensitive), which may need to be manually added depending on the # choice of JavaScript libraries and frameworks. def xml_http_request? - get_header("HTTP_X_REQUESTED_WITH") =~ /XMLHttpRequest/i + /XMLHttpRequest/i.match?(get_header("HTTP_X_REQUESTED_WITH")) end alias :xhr? :xml_http_request? @@ -400,7 +400,7 @@ module ActionDispatch # True if the request came from localhost, 127.0.0.1, or ::1. def local? - LOCALHOST =~ remote_addr && LOCALHOST =~ remote_ip + LOCALHOST.match?(remote_addr) && LOCALHOST.match?(remote_ip) end def request_parameters=(params) diff --git a/actionpack/lib/action_dispatch/http/url.rb b/actionpack/lib/action_dispatch/http/url.rb index 3b0f6378ea..225ae0a497 100644 --- a/actionpack/lib/action_dispatch/http/url.rb +++ b/actionpack/lib/action_dispatch/http/url.rb @@ -133,7 +133,7 @@ module ActionDispatch end def named_host?(host) - IP_HOST_REGEXP !~ host + !IP_HOST_REGEXP.match?(host) end def normalize_protocol(protocol) diff --git a/actionpack/lib/action_dispatch/journey/route.rb b/actionpack/lib/action_dispatch/journey/route.rb index 4aee7a6f83..9184676801 100644 --- a/actionpack/lib/action_dispatch/journey/route.rb +++ b/actionpack/lib/action_dispatch/journey/route.rb @@ -148,7 +148,7 @@ module ActionDispatch end def glob? - !path.spec.grep(Nodes::Star).empty? + path.spec.any?(Nodes::Star) end def dispatcher? diff --git a/actionpack/lib/action_dispatch/middleware/cookies.rb b/actionpack/lib/action_dispatch/middleware/cookies.rb index 642f155085..96bdf570af 100644 --- a/actionpack/lib/action_dispatch/middleware/cookies.rb +++ b/actionpack/lib/action_dispatch/middleware/cookies.rb @@ -346,28 +346,6 @@ module ActionDispatch @cookies.map { |k, v| "#{escape(k)}=#{escape(v)}" }.join "; " end - def handle_options(options) # :nodoc: - if options[:expires].respond_to?(:from_now) - options[:expires] = options[:expires].from_now - end - - options[:path] ||= "/" - - if options[:domain] == :all || options[:domain] == "all" - # If there is a provided tld length then we use it otherwise default domain regexp. - domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP - - # If host is not ip and matches domain regexp. - # (ip confirms to domain regexp so we explicitly check for ip) - options[:domain] = if (request.host !~ /^[\d.]+$/) && (request.host =~ domain_regexp) - ".#{$&}" - end - elsif options[:domain].is_a? Array - # If host matches one of the supplied domains without a dot in front of it. - options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } - end - end - # Sets the cookie named +name+. The second argument may be the cookie's # value or a hash of options as documented above. def []=(name, options) @@ -447,6 +425,28 @@ module ActionDispatch def write_cookie?(cookie) request.ssl? || !cookie[:secure] || always_write_cookie end + + def handle_options(options) + if options[:expires].respond_to?(:from_now) + options[:expires] = options[:expires].from_now + end + + options[:path] ||= "/" + + if options[:domain] == :all || options[:domain] == "all" + # If there is a provided tld length then we use it otherwise default domain regexp. + domain_regexp = options[:tld_length] ? /([^.]+\.?){#{options[:tld_length]}}$/ : DOMAIN_REGEXP + + # If host is not ip and matches domain regexp. + # (ip confirms to domain regexp so we explicitly check for ip) + options[:domain] = if !request.host.match?(/^[\d.]+$/) && (request.host =~ domain_regexp) + ".#{$&}" + end + elsif options[:domain].is_a? Array + # If host matches one of the supplied domains without a dot in front of it. + options[:domain] = options[:domain].find { |domain| request.host.include? domain.sub(/^\./, "") } + end + end end class AbstractCookieJar # :nodoc: diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 00902ede21..34b213d14e 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -13,7 +13,7 @@ module ActionDispatch # # Requests can opt-out of redirection with +exclude+: # - # config.ssl_options = { redirect: { exclude: -> request { request.path =~ /healthcheck/ } } } + # config.ssl_options = { redirect: { exclude: -> request { /healthcheck/.match?(request.path) } } } # # Cookies will not be flagged as secure for excluded requests. # diff --git a/actionpack/lib/action_dispatch/middleware/static.rb b/actionpack/lib/action_dispatch/middleware/static.rb index 1f2f7757a3..eddcdbaeac 100644 --- a/actionpack/lib/action_dispatch/middleware/static.rb +++ b/actionpack/lib/action_dispatch/middleware/static.rb @@ -32,18 +32,13 @@ module ActionDispatch return false unless ::Rack::Utils.valid_path? path path = ::Rack::Utils.clean_path_info path - paths = [path, "#{path}#{ext}", "#{path}/#{@index}#{ext}"] + return ::Rack::Utils.escape_path(path).b if file_readable?(path) - if match = paths.detect { |p| - path = File.join(@root, p.b) - begin - File.file?(path) && File.readable?(path) - rescue SystemCallError - false - end - } - return ::Rack::Utils.escape_path(match).b - end + path_with_ext = path + ext + return ::Rack::Utils.escape_path(path_with_ext).b if file_readable?(path_with_ext) + + path << "/" << @index << ext + return ::Rack::Utils.escape_path(path).b if file_readable?(path) end def call(env) @@ -83,11 +78,11 @@ module ActionDispatch end def gzip_encoding_accepted?(request) - request.accept_encoding.any? { |enc, quality| enc =~ /\bgzip\b/i } + request.accept_encoding.any? { |enc, quality| /\bgzip\b/i.match?(enc) } end def gzip_file_path(path) - can_gzip_mime = content_type(path) =~ /\A(?:text\/|application\/javascript)/ + can_gzip_mime = /\A(?:text\/|application\/javascript)/.match?(content_type(path)) gzip_path = "#{path}.gz" if can_gzip_mime && File.exist?(File.join(@root, ::Rack::Utils.unescape_path(gzip_path))) gzip_path @@ -95,6 +90,12 @@ module ActionDispatch false end end + + def file_readable?(path) + file_path = File.join(@root, path.b) + File.file?(file_path) && File.readable?(file_path) + rescue SystemCallError + end end # This middleware will attempt to return the contents of a file's body from diff --git a/actionpack/lib/action_dispatch/routing/inspector.rb b/actionpack/lib/action_dispatch/routing/inspector.rb index 6e40a18009..bf286c299d 100644 --- a/actionpack/lib/action_dispatch/routing/inspector.rb +++ b/actionpack/lib/action_dispatch/routing/inspector.rb @@ -94,7 +94,7 @@ module ActionDispatch if filter @routes.select do |route| route_wrapper = RouteWrapper.new(route) - filter.any? { |default, value| route_wrapper.send(default) =~ value } + filter.any? { |default, value| value.match?(route_wrapper.send(default)) } end else @routes diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index d1100089b1..42e770c5bf 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -112,7 +112,7 @@ module ActionDispatch end def self.optional_format?(path, format) - format != false && path !~ OPTIONAL_FORMAT_REGEX + format != false && !path.match?(OPTIONAL_FORMAT_REGEX) end def initialize(set:, ast:, controller:, default_action:, to:, formatted:, via:, options_constraints:, anchor:, scope_params:, options:) @@ -367,7 +367,7 @@ module ActionDispatch def translate_controller(controller) return controller if Regexp === controller - return controller.to_s if controller =~ /\A[a-z_0-9][a-z_0-9\/]*\z/ + return controller.to_s if /\A[a-z_0-9][a-z_0-9\/]*\z/.match?(controller) yield end @@ -403,7 +403,7 @@ module ActionDispatch # for root cases, where the latter is the correct one. def self.normalize_path(path) path = Journey::Router::Utils.normalize_path(path) - path.gsub!(%r{/(\(+)/?}, '\1/') unless path =~ %r{^/(\(+[^)]+\)){1,}$} + path.gsub!(%r{/(\(+)/?}, '\1/') unless %r{^/(\(+[^)]+\)){1,}$}.match?(path) path end @@ -996,7 +996,7 @@ module ActionDispatch # # Requests to routes can be constrained based on specific criteria: # - # constraints(-> (req) { req.env["HTTP_USER_AGENT"] =~ /iPhone/ }) do + # constraints(-> (req) { /iPhone/.match?(req.env["HTTP_USER_AGENT"]) }) do # resources :iphones # end # @@ -1006,7 +1006,7 @@ module ActionDispatch # # class Iphone # def self.matches?(request) - # request.env["HTTP_USER_AGENT"] =~ /iPhone/ + # /iPhone/.match?(request.env["HTTP_USER_AGENT"]) # end # end # @@ -1833,7 +1833,7 @@ module ActionDispatch # and return nil in case it isn't. Otherwise, we pass the invalid name # forward so the underlying router engine treats it and raises an exception. if as.nil? - candidate unless candidate !~ /\A[_a-z]/i || has_named_route?(candidate) + candidate unless !candidate.match?(/\A[_a-z]/i) || has_named_route?(candidate) else candidate end @@ -1916,7 +1916,7 @@ module ActionDispatch end def using_match_shorthand?(path) - path =~ %r{^/?[-\w]+/[-\w/]+$} + %r{^/?[-\w]+/[-\w/]+$}.match?(path) end def decomposed_match(path, controller, options, _path, to, via, formatted, anchor, options_constraints) diff --git a/actionpack/lib/action_dispatch/routing/route_set.rb b/actionpack/lib/action_dispatch/routing/route_set.rb index 5b35b68c44..db8c54ba84 100644 --- a/actionpack/lib/action_dispatch/routing/route_set.rb +++ b/actionpack/lib/action_dispatch/routing/route_set.rb @@ -836,7 +836,7 @@ module ActionDispatch def recognize_path(path, environment = {}) method = (environment[:method] || "GET").to_s.upcase - path = Journey::Router::Utils.normalize_path(path) unless path =~ %r{://} + path = Journey::Router::Utils.normalize_path(path) unless %r{://}.match?(path) extras = environment[:extras] || {} begin diff --git a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb index 056ce51a61..cba053ee4c 100644 --- a/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb +++ b/actionpack/lib/action_dispatch/system_testing/test_helpers/screenshot_helper.rb @@ -9,10 +9,16 @@ module ActionDispatch # # +take_screenshot+ can be used at any point in your system tests to take # a screenshot of the current state. This can be useful for debugging or - # automating visual testing. + # automating visual testing. You can take multiple screenshots per test + # to investigate changes at different points during your test. These will be + # named with a sequential prefix (or 'failed' for failing tests) # # The screenshot will be displayed in your console, if supported. # + # You can set the +RAILS_SYSTEM_TESTING_SCREENSHOT_HTML+ environment variable to + # save the HTML from the page that is being screenhoted so you can investigate the + # elements on the page at the time of the screenshot + # # You can set the +RAILS_SYSTEM_TESTING_SCREENSHOT+ environment variable to # control the output. Possible values are: # * [+simple+ (default)] Only displays the screenshot path. @@ -22,6 +28,8 @@ module ActionDispatch # * [+artifact+] Display the screenshot in the terminal, using the terminal # artifact format (https://buildkite.github.io/terminal-to-html/inline-images/). def take_screenshot + increment_unique + save_html if save_html? save_image puts display_image end @@ -38,17 +46,48 @@ module ActionDispatch end private + attr_accessor :_screenshot_counter + + def save_html? + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT_HTML"] == "1" + end + + def increment_unique + @_screenshot_counter ||= 0 + @_screenshot_counter += 1 + end + + def unique + failed? ? "failures" : (_screenshot_counter || 0).to_s + end + def image_name - name = method_name[0...225] - failed? ? "failures_#{name}" : name + name = "#{unique}_#{method_name}" + name[0...225] end def image_path - @image_path ||= absolute_image_path.to_s + absolute_image_path.to_s + end + + def html_path + absolute_html_path.to_s + end + + def absolute_path + Rails.root.join("tmp/screenshots/#{image_name}") end def absolute_image_path - Rails.root.join("tmp/screenshots/#{image_name}.png") + "#{absolute_path}.png" + end + + def absolute_html_path + "#{absolute_path}.html" + end + + def save_html + page.save_page(absolute_html_path) end def save_image @@ -66,7 +105,8 @@ module ActionDispatch end def display_image - message = +"[Screenshot]: #{image_path}\n" + message = +"[Screenshot Image]: #{image_path}\n" + message << +"[Screenshot HTML]: #{html_path}\n" if save_html? case output_type when "artifact" diff --git a/actionpack/test/controller/integration_test.rb b/actionpack/test/controller/integration_test.rb index caacd66bd6..4f5f5b71ae 100644 --- a/actionpack/test/controller/integration_test.rb +++ b/actionpack/test/controller/integration_test.rb @@ -543,6 +543,32 @@ class IntegrationProcessTest < ActionDispatch::IntegrationTest end end + def test_setting_vary_header_when_request_is_xhr_with_accept_header + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_equal "Accept", response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_format_is_provided + with_test_route_set do + get "/get", params: { format: "json" } + assert_nil response.headers["Vary"] + end + end + + def test_not_setting_vary_header_when_ignore_accept_header_is_set + original_ignore_accept_header = ActionDispatch::Request.ignore_accept_header + ActionDispatch::Request.ignore_accept_header = true + + with_test_route_set do + get "/get", headers: { "Accept" => "application/json" }, xhr: true + assert_nil response.headers["Vary"] + end + ensure + ActionDispatch::Request.ignore_accept_header = original_ignore_accept_header + end + private def with_default_headers(headers) original = ActionDispatch::Response.default_headers @@ -580,7 +606,7 @@ class MetalIntegrationTest < ActionDispatch::IntegrationTest class Poller def self.call(env) - if env["PATH_INFO"] =~ /^\/success/ + if /^\/success/.match?(env["PATH_INFO"]) [200, { "Content-Type" => "text/plain", "Content-Length" => "12" }, ["Hello World!"]] else [404, { "Content-Type" => "text/plain", "Content-Length" => "0" }, []] diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index 1a7e7f6cbb..46ab7b4f74 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -139,7 +139,7 @@ class ACLogSubscriberTest < ActionController::TestCase def test_process_action_without_parameters get :show wait - assert_nil logs.detect { |l| l =~ /Parameters/ } + assert_nil logs.detect { |l| /Parameters/.match?(l) } end def test_process_action_with_parameters diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index a2a6c69dd3..dd76824413 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -4,6 +4,11 @@ require "abstract_unit" require "controller/fake_models" class TestControllerWithExtraEtags < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "test/with_implicit_template.erb" => "Hello explicitly!", + "test/hello_world.erb" => "Hello world!" + )] + def self.controller_name; "test"; end def self.controller_path; "test"; end @@ -37,6 +42,11 @@ class TestControllerWithExtraEtags < ActionController::Base end class ImplicitRenderTestController < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "implicit_render_test/hello_world.erb" => "Hello world!", + "implicit_render_test/empty_action_with_template.html.erb" => "<h1>Empty action rendered this implicitly.</h1>\n" + )] + def empty_action end @@ -46,6 +56,10 @@ end module Namespaced class ImplicitRenderTestController < ActionController::Base + self.view_paths = [ActionView::FixtureResolver.new( + "namespaced/implicit_render_test/hello_world.erb" => "Hello world!" + )] + def hello_world fresh_when(etag: "abc") end @@ -293,13 +307,15 @@ end module TemplateModificationHelper private def modify_template(name) - path = File.expand_path("../fixtures/#{name}.erb", __dir__) - original = File.read(path) - File.write(path, "#{original} Modified!") + hash = @controller.view_paths.first.instance_variable_get(:@hash) + key = name + ".erb" + original = hash[key] + hash[key] = "#{original} Modified!" ActionView::LookupContext::DetailsKey.clear yield ensure - File.write(path, original) + hash[key] = original + ActionView::LookupContext::DetailsKey.clear end end diff --git a/actionpack/test/dispatch/debug_exceptions_test.rb b/actionpack/test/dispatch/debug_exceptions_test.rb index fa629bc761..5d12b62fd4 100644 --- a/actionpack/test/dispatch/debug_exceptions_test.rb +++ b/actionpack/test/dispatch/debug_exceptions_test.rb @@ -544,8 +544,8 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest @app = DevelopmentApp Rails.stub :root, Pathname.new(".") do cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc| - bc.add_silencer { |line| line =~ /method_that_raises/ } - bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } + bc.add_silencer { |line| line.match?(/method_that_raises/) } + bc.add_silencer { |line| !line.match?(%r{test/dispatch/debug_exceptions_test.rb}) } end get "/framework_raises", headers: { "action_dispatch.backtrace_cleaner" => cleaner } @@ -596,7 +596,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest @app = DevelopmentApp Rails.stub :root, Pathname.new(".") do cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc| - bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } + bc.add_silencer { |line| !line.match?(%r{test/dispatch/debug_exceptions_test.rb}) } end get "/nested_exceptions", headers: { "action_dispatch.backtrace_cleaner" => cleaner } @@ -631,7 +631,7 @@ class DebugExceptionsTest < ActionDispatch::IntegrationTest @app = DevelopmentApp Rails.stub :root, Pathname.new(".") do cleaner = ActiveSupport::BacktraceCleaner.new.tap do |bc| - bc.add_silencer { |line| line !~ %r{test/dispatch/debug_exceptions_test.rb} } + bc.add_silencer { |line| !line.match?(%r{test/dispatch/debug_exceptions_test.rb}) } end get "/actionable_error", headers: { "action_dispatch.backtrace_cleaner" => cleaner } diff --git a/actionpack/test/dispatch/exception_wrapper_test.rb b/actionpack/test/dispatch/exception_wrapper_test.rb index 668469a01d..3c395ca5ee 100644 --- a/actionpack/test/dispatch/exception_wrapper_test.rb +++ b/actionpack/test/dispatch/exception_wrapper_test.rb @@ -21,7 +21,7 @@ module ActionDispatch setup do @cleaner = ActiveSupport::BacktraceCleaner.new @cleaner.remove_filters! - @cleaner.add_silencer { |line| line !~ /^lib/ } + @cleaner.add_silencer { |line| !line.match?(/^lib/) } end test "#source_extracts fetches source fragments for every backtrace entry" do diff --git a/actionpack/test/dispatch/mime_type_test.rb b/actionpack/test/dispatch/mime_type_test.rb index 50f6c06fee..b0b2aa0cc1 100644 --- a/actionpack/test/dispatch/mime_type_test.rb +++ b/actionpack/test/dispatch/mime_type_test.rb @@ -175,6 +175,12 @@ class MimeTypeTest < ActiveSupport::TestCase assert Mime[:html] =~ "application/xhtml+xml" end + test "match?" do + assert Mime[:js].match?("text/javascript") + assert Mime[:js].match?("application/javascript") + assert_not Mime[:js].match?("text/html") + end + test "can be initialized with wildcards" do assert_equal "*/*", Mime::Type.new("*/*").to_s assert_equal "text/*", Mime::Type.new("text/*").to_s diff --git a/actionpack/test/dispatch/prefix_generation_test.rb b/actionpack/test/dispatch/prefix_generation_test.rb index 63c147cb1b..f20043b9ac 100644 --- a/actionpack/test/dispatch/prefix_generation_test.rb +++ b/actionpack/test/dispatch/prefix_generation_test.rb @@ -304,7 +304,7 @@ module TestGenerationPrefix assert_equal "/omg/blog/posts/1", path end - test "[OBJECT] generating engine's route with named helpers" do + test "[OBJECT] generating engine's route with named route helpers" do path = engine_object.posts_path assert_equal "/awesome/blog/posts", path diff --git a/actionpack/test/dispatch/request/json_params_parsing_test.rb b/actionpack/test/dispatch/request/json_params_parsing_test.rb index 2a48a12497..bbf98912f3 100644 --- a/actionpack/test/dispatch/request/json_params_parsing_test.rb +++ b/actionpack/test/dispatch/request/json_params_parsing_test.rb @@ -68,7 +68,7 @@ class JsonParamsParsingTest < ActionDispatch::IntegrationTest post "/parse", params: json, headers: { "CONTENT_TYPE" => "application/json", "action_dispatch.show_exceptions" => true, "action_dispatch.logger" => ActiveSupport::Logger.new(output) } assert_response :bad_request output.rewind && err = output.read - assert err =~ /Error occurred while parsing request parameters/ + assert err.match?(/Error occurred while parsing request parameters/) end end diff --git a/actionpack/test/dispatch/request_test.rb b/actionpack/test/dispatch/request_test.rb index 0ec8dd25e0..806b7d0559 100644 --- a/actionpack/test/dispatch/request_test.rb +++ b/actionpack/test/dispatch/request_test.rb @@ -865,12 +865,28 @@ class RequestFormat < BaseRequestTest assert_not_predicate request.format, :json? end - test "format does not throw exceptions when malformed parameters" do + test "format does not throw exceptions when malformed GET parameters" do request = stub_request("QUERY_STRING" => "x[y]=1&x[y][][w]=2") assert request.formats assert_predicate request.format, :html? end + test "format does not throw exceptions when invalid POST parameters" do + body = "{record:{content:127.0.0.1}}" + request = stub_request( + "REQUEST_METHOD" => "POST", + "CONTENT_LENGTH" => body.length, + "CONTENT_TYPE" => "application/json", + "rack.input" => StringIO.new(body), + "action_dispatch.logger" => Logger.new(output = StringIO.new) + ) + assert request.formats + assert request.format.html? + + output.rewind && (err = output.read) + assert_match(/Error occurred while parsing request parameters/, err) + end + test "formats with xhr request" do request = stub_request "HTTP_X_REQUESTED_WITH" => "XMLHttpRequest", "QUERY_STRING" => "" diff --git a/actionpack/test/dispatch/routing_test.rb b/actionpack/test/dispatch/routing_test.rb index b67b1dd347..d4a667a13a 100644 --- a/actionpack/test/dispatch/routing_test.rb +++ b/actionpack/test/dispatch/routing_test.rb @@ -12,7 +12,7 @@ class TestRoutingMapper < ActionDispatch::IntegrationTest class IpRestrictor def self.matches?(request) - request.ip =~ /192\.168\.1\.1\d\d/ + /192\.168\.1\.1\d\d/.match?(request.ip) end end @@ -3823,7 +3823,7 @@ private end def method_missing(method, *args, &block) - if method.to_s =~ /_(path|url)$/ + if method.to_s.match?(/_(path|url)$/) @app.routes.url_helpers.send(method, *args, &block) else super @@ -5137,7 +5137,7 @@ class TestRecognizePath < ActionDispatch::IntegrationTest end def matches?(request) - request.path_parameters[key] =~ pattern + pattern.match?(request.path_parameters[key]) end end @@ -5147,8 +5147,8 @@ class TestRecognizePath < ActionDispatch::IntegrationTest get "/hash/:foo", to: "pages#show", constraints: { foo: /foo/ } get "/hash/:bar", to: "pages#show", constraints: { bar: /bar/ } - get "/proc/:foo", to: "pages#show", constraints: proc { |r| r.path_parameters[:foo] =~ /foo/ } - get "/proc/:bar", to: "pages#show", constraints: proc { |r| r.path_parameters[:bar] =~ /bar/ } + get "/proc/:foo", to: "pages#show", constraints: proc { |r| /foo/.match?(r.path_parameters[:foo]) } + get "/proc/:bar", to: "pages#show", constraints: proc { |r| /bar/.match?(r.path_parameters[:bar]) } get "/class/:foo", to: "pages#show", constraints: PageConstraint.new(:foo, /foo/) get "/class/:bar", to: "pages#show", constraints: PageConstraint.new(:bar, /bar/) diff --git a/actionpack/test/dispatch/ssl_test.rb b/actionpack/test/dispatch/ssl_test.rb index baf46e7c7e..44fe433c5f 100644 --- a/actionpack/test/dispatch/ssl_test.rb +++ b/actionpack/test/dispatch/ssl_test.rb @@ -42,7 +42,7 @@ class RedirectSSLTest < SSLTest end test "exclude can avoid redirect" do - excluding = { exclude: -> request { request.path =~ /healthcheck/ } } + excluding = { exclude: -> request { request.path.match?(/healthcheck/) } } assert_not_redirected "http://example.org/healthcheck", redirect: excluding assert_redirected from: "http://example.org/", redirect: excluding @@ -209,7 +209,7 @@ class SecureCookiesTest < SSLTest end def test_cookies_as_not_secure_with_exclude - excluding = { exclude: -> request { request.domain =~ /example/ } } + excluding = { exclude: -> request { /example/.match?(request.domain) } } get headers: { "Set-Cookie" => DEFAULT }, ssl_options: { redirect: excluding } assert_cookies(*DEFAULT.split("\n")) diff --git a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb index b0b36f9d74..5d69a887ef 100644 --- a/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb +++ b/actionpack/test/dispatch/system_testing/screenshot_helper_test.rb @@ -6,60 +6,93 @@ require "capybara/dsl" require "selenium/webdriver" class ScreenshotHelperTest < ActiveSupport::TestCase + def setup + @new_test = DrivenBySeleniumWithChrome.new("x") + @new_test.send("_screenshot_counter=", nil) + end + test "image path is saved in tmp directory" do - new_test = DrivenBySeleniumWithChrome.new("x") + Rails.stub :root, Pathname.getwd do + assert_equal Rails.root.join("tmp/screenshots/0_x.png").to_s, @new_test.send(:image_path) + end + end + + test "image path unique counter is changed when incremented" do + @new_test.send(:increment_unique) Rails.stub :root, Pathname.getwd do - assert_equal Rails.root.join("tmp/screenshots/x.png").to_s, new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/1_x.png").to_s, @new_test.send(:image_path) end end - test "image path includes failures text if test did not pass" do - new_test = DrivenBySeleniumWithChrome.new("x") + # To allow multiple screenshots in same test + test "image path unique counter generates different path in same test" do + Rails.stub :root, Pathname.getwd do + @new_test.send(:increment_unique) + assert_equal Rails.root.join("tmp/screenshots/1_x.png").to_s, @new_test.send(:image_path) + + @new_test.send(:increment_unique) + assert_equal Rails.root.join("tmp/screenshots/2_x.png").to_s, @new_test.send(:image_path) + end + end + test "image path includes failures text if test did not pass" do Rails.stub :root, Pathname.getwd do - new_test.stub :passed?, false do - assert_equal Rails.root.join("tmp/screenshots/failures_x.png").to_s, new_test.send(:image_path) + @new_test.stub :passed?, false do + assert_equal Rails.root.join("tmp/screenshots/failures_x.png").to_s, @new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/failures_x.html").to_s, @new_test.send(:html_path) end end end test "image path does not include failures text if test skipped" do - new_test = DrivenBySeleniumWithChrome.new("x") - Rails.stub :root, Pathname.getwd do - new_test.stub :passed?, false do - new_test.stub :skipped?, true do - assert_equal Rails.root.join("tmp/screenshots/x.png").to_s, new_test.send(:image_path) + @new_test.stub :passed?, false do + @new_test.stub :skipped?, true do + assert_equal Rails.root.join("tmp/screenshots/0_x.png").to_s, @new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/0_x.html").to_s, @new_test.send(:html_path) end end end end - test "image name truncates names over 225 characters" do - new_test = DrivenBySeleniumWithChrome.new("x" * 400) + test "image name truncates names over 225 characters including counter" do + long_test = DrivenBySeleniumWithChrome.new("x" * 400) Rails.stub :root, Pathname.getwd do - assert_equal Rails.root.join("tmp/screenshots/#{"x" * 225}.png").to_s, new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/0_#{"x" * 223}.png").to_s, long_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/0_#{"x" * 223}.html").to_s, long_test.send(:html_path) end end test "defaults to simple output for the screenshot" do - new_test = DrivenBySeleniumWithChrome.new("x") - assert_equal "simple", new_test.send(:output_type) + assert_equal "simple", @new_test.send(:output_type) + end + + test "display_image return html path when RAILS_SYSTEM_TESTING_SCREENSHOT_HTML environment" do + original_html_setting = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT_HTML"] + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT_HTML"] = "1" + + assert @new_test.send(:save_html?) + + Rails.stub :root, Pathname.getwd do + @new_test.stub :passed?, false do + assert_match %r|\[Screenshot HTML\].+?tmp/screenshots/failures_x\.html|, @new_test.send(:display_image) + end + end + ensure + ENV["RAILS_SYSTEM_TESTING_SCREENSHOT_HTML"] = original_html_setting end test "display_image return artifact format when specify RAILS_SYSTEM_TESTING_SCREENSHOT environment" do original_output_type = ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] ENV["RAILS_SYSTEM_TESTING_SCREENSHOT"] = "artifact" - new_test = DrivenBySeleniumWithChrome.new("x") - - assert_equal "artifact", new_test.send(:output_type) + assert_equal "artifact", @new_test.send(:output_type) Rails.stub :root, Pathname.getwd do - new_test.stub :passed?, false do - assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, new_test.send(:display_image) + @new_test.stub :passed?, false do + assert_match %r|url=artifact://.+?tmp/screenshots/failures_x\.png|, @new_test.send(:display_image) end end ensure @@ -67,10 +100,8 @@ class ScreenshotHelperTest < ActiveSupport::TestCase end test "image path returns the absolute path from root" do - new_test = DrivenBySeleniumWithChrome.new("x") - Rails.stub :root, Pathname.getwd.join("..") do - assert_equal Rails.root.join("tmp/screenshots/x.png").to_s, new_test.send(:image_path) + assert_equal Rails.root.join("tmp/screenshots/0_x.png").to_s, @new_test.send(:image_path) end end end diff --git a/actiontext/lib/action_text/attachables/remote_image.rb b/actiontext/lib/action_text/attachables/remote_image.rb index 650b11862b..c5819ef1f5 100644 --- a/actiontext/lib/action_text/attachables/remote_image.rb +++ b/actiontext/lib/action_text/attachables/remote_image.rb @@ -14,7 +14,7 @@ module ActionText private def content_type_is_image?(content_type) - content_type.to_s =~ /^image(\/.+|$)/ + content_type.to_s.match?(/^image(\/.+|$)/) end def attributes_from_node(node) diff --git a/actiontext/lib/action_text/attachment_gallery.rb b/actiontext/lib/action_text/attachment_gallery.rb index 45afbff058..48ba9f830c 100644 --- a/actiontext/lib/action_text/attachment_gallery.rb +++ b/actiontext/lib/action_text/attachment_gallery.rb @@ -23,7 +23,7 @@ module ActionText Fragment.wrap(content).find_all(SELECTOR).select do |node| node.children.all? do |child| if child.text? - child.text =~ /\A(\n|\ )*\z/ + /\A(\n|\ )*\z/.match?(child.text) else child.matches? ATTACHMENT_SELECTOR end diff --git a/actiontext/package.json b/actiontext/package.json index 594cfe16eb..ee44b75d2c 100644 --- a/actiontext/package.json +++ b/actiontext/package.json @@ -24,6 +24,6 @@ "@rails/activestorage": "^6.0.0-alpha" }, "peerDependencies": { - "trix": "^1.0.0" + "trix": "^1.2.0" } } diff --git a/actiontext/test/dummy/config/initializers/backtrace_silencers.rb b/actiontext/test/dummy/config/initializers/backtrace_silencers.rb index 59385cdf37..3c56b21b3c 100644 --- a/actiontext/test/dummy/config/initializers/backtrace_silencers.rb +++ b/actiontext/test/dummy/config/initializers/backtrace_silencers.rb @@ -1,7 +1,7 @@ # Be sure to restart your server when you modify this file. # You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. -# Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } +# Rails.backtrace_cleaner.add_silencer { |line| /my_noisy_library/.match?(line) } # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. # Rails.backtrace_cleaner.remove_silencers! diff --git a/actionview/CHANGELOG.md b/actionview/CHANGELOG.md index 504b1d3e98..ca3ce1476a 100644 --- a/actionview/CHANGELOG.md +++ b/actionview/CHANGELOG.md @@ -1,3 +1,7 @@ +* Added `phone_to` helper method to create a link from mobile numbers + + *Pietro Moro* + * annotated_source_code returns an empty array so TemplateErrors without a template in the backtrace are surfaced properly by DebugExceptions. diff --git a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee index 736cab08db..9e11bfa7ed 100644 --- a/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee +++ b/actionview/app/assets/javascripts/rails-ujs/utils/form.coffee @@ -11,6 +11,7 @@ Rails.serializeElement = (element, additionalParam) -> inputs.forEach (input) -> return if !input.name || input.disabled + return if input.closest('fieldset[disabled]') if matches(input, 'select') toArray(input.options).forEach (option) -> params.push(name: input.name, value: option.value) if option.selected diff --git a/actionview/lib/action_view/digestor.rb b/actionview/lib/action_view/digestor.rb index 97a6da3634..cdf436ccae 100644 --- a/actionview/lib/action_view/digestor.rb +++ b/actionview/lib/action_view/digestor.rb @@ -9,10 +9,11 @@ module ActionView class << self # Supported options: # - # * <tt>name</tt> - Template name - # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> - # * <tt>dependencies</tt> - An array of dependent views - def digest(name:, format:, finder:, dependencies: nil) + # * <tt>name</tt> - Template name + # * <tt>format</tt> - Template format + # * <tt>finder</tt> - An instance of <tt>ActionView::LookupContext</tt> + # * <tt>dependencies</tt> - An array of dependent views + def digest(name:, format: nil, finder:, dependencies: nil) if dependencies.nil? || dependencies.empty? cache_key = "#{name}.#{format}" else diff --git a/actionview/lib/action_view/helpers/form_options_helper.rb b/actionview/lib/action_view/helpers/form_options_helper.rb index a7747456a4..cf8f7de931 100644 --- a/actionview/lib/action_view/helpers/form_options_helper.rb +++ b/actionview/lib/action_view/helpers/form_options_helper.rb @@ -566,9 +566,10 @@ module ActionView # an ActiveSupport::TimeZone. # # By default, +model+ is the ActiveSupport::TimeZone constant (which can - # be obtained in Active Record as a value object). The only requirement - # is that the +model+ parameter be an object that responds to +all+, and - # returns an array of objects that represent time zones. + # be obtained in Active Record as a value object). The +model+ parameter + # must respond to +all+ and return an array of objects that represent time + # zones; each object must respond to +name+. If a Regexp is given it will + # attempt to match the zones using <code>match?</code> method. # # NOTE: Only the option tags are returned, you have to wrap this call in # a regular HTML select tag. @@ -580,7 +581,7 @@ module ActionView if priority_zones if priority_zones.is_a?(Regexp) - priority_zones = zones.select { |z| z =~ priority_zones } + priority_zones = zones.select { |z| z.match?(priority_zones) } end zone_options.safe_concat options_for_select(convert_zones[priority_zones], selected) diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index 8203a43239..980a89a7b6 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -228,7 +228,7 @@ module ActionView # pluralize(2, 'Person', locale: :de) # # => 2 Personen def pluralize(count, singular, plural_arg = nil, plural: plural_arg, locale: I18n.locale) - word = if count == 1 || count.to_s =~ /^1(\.0+)?$/ + word = if count == 1 || count.to_s.match?(/^1(\.0+)?$/) singular else plural || singular.pluralize(locale) diff --git a/actionview/lib/action_view/helpers/url_helper.rb b/actionview/lib/action_view/helpers/url_helper.rb index 85fd549177..1b05d4aa71 100644 --- a/actionview/lib/action_view/helpers/url_helper.rb +++ b/actionview/lib/action_view/helpers/url_helper.rb @@ -619,6 +619,53 @@ module ActionView content_tag("a", name || phone_number, html_options, &block) end + # Creates a TEL anchor link tag to the specified +phone_number+, which is + # also used as the name of the link unless +name+ is specified. Additional + # HTML attributes for the link can be passed in +html_options+. + # + # When clicked, the default app to make calls is opened, and it + # is prepopulated with the passed phone number and optional + # +country_code+ value. + # + # +phone_to+ has an optional +country_code+ option which automatically adds the country + # code as well as the + sign in the phone numer that gets prepopulated, + # for example if +country_code: "01"+ +\+01+ will be prepended to the + # phone numer, by passing special keys to +html_options+. + # + # ==== Options + # * <tt>:country_code</tt> - Prepends the country code to the number + # + # ==== Examples + # phone_to "1234567890" + # # => <a href="tel:1234567890">1234567890</a> + # + # phone_to "1234567890", "Phone me" + # # => <a href="tel:134567890">Phone me</a> + # + # phone_to "1234567890", "Phone me", country_code: "01" + # # => <a href="tel:+015155555785">Phone me</a> + # + # You can use a block as well if your link target is hard to fit into the name parameter. \ERB example: + # + # <%= phone_to "1234567890" do %> + # <strong>Phone me:</strong> + # <% end %> + # # => <a href="tel:1234567890"> + # <strong>Phone me:</strong> + # </a> + def phone_to(phone_number, name = nil, html_options = {}, &block) + html_options, name = name, nil if block_given? + html_options = (html_options || {}).stringify_keys + + country_code = html_options.delete("country_code").presence + country_code = country_code.nil? ? "" : "+#{ERB::Util.url_encode(country_code)}" + + encoded_phone_number = ERB::Util.url_encode(phone_number) + html_options["href"] = "tel:#{country_code}#{encoded_phone_number}" + + content_tag("a", name || phone_number, html_options, &block) + end + private def convert_options_to_data_attributes(options, html_options) if html_options @@ -642,7 +689,7 @@ module ActionView end def add_method_to_attributes!(html_options, method) - if method_not_get_method?(method) && html_options["rel"] !~ /nofollow/ + if method_not_get_method?(method) && !html_options["rel"]&.match?(/nofollow/) if html_options["rel"].blank? html_options["rel"] = "nofollow" else diff --git a/actionview/lib/action_view/layouts.rb b/actionview/lib/action_view/layouts.rb index be21ff0e5d..74e8e80e07 100644 --- a/actionview/lib/action_view/layouts.rb +++ b/actionview/lib/action_view/layouts.rb @@ -395,7 +395,7 @@ module ActionView end def _normalize_layout(value) - value.is_a?(String) && value !~ /\blayouts/ ? "layouts/#{value}" : value + value.is_a?(String) && !value.match?(/\blayouts/) ? "layouts/#{value}" : value end # Returns the default layout for this controller. diff --git a/actionview/lib/action_view/lookup_context.rb b/actionview/lib/action_view/lookup_context.rb index 211fbc8e6c..f050d54e27 100644 --- a/actionview/lib/action_view/lookup_context.rb +++ b/actionview/lib/action_view/lookup_context.rb @@ -29,7 +29,7 @@ module ActionView Accessors.define_method(:"default_#{name}", &block) Accessors.module_eval <<-METHOD, __FILE__, __LINE__ + 1 def #{name} - @details.fetch(:#{name}, []) + @details[:#{name}] || [] end def #{name}=(value) diff --git a/actionview/lib/action_view/testing/resolvers.rb b/actionview/lib/action_view/testing/resolvers.rb index 99443663dd..03eac29bb4 100644 --- a/actionview/lib/action_view/testing/resolvers.rb +++ b/actionview/lib/action_view/testing/resolvers.rb @@ -7,10 +7,15 @@ module ActionView #:nodoc: # file system. This is used internally by Rails' own test suite, and is # useful for testing extensions that have no way of knowing what the file # system will look like at runtime. - class FixtureResolver < PathResolver + class FixtureResolver < OptimizedFileSystemResolver def initialize(hash = {}, pattern = nil) - super(pattern) + super("") + if pattern + ActiveSupport::Deprecation.warn "Specifying a custom path for #{self.class} is deprecated. Implement a custom Resolver subclass instead." + @pattern = pattern + end @hash = hash + @path = "" end def data @@ -23,25 +28,32 @@ module ActionView #:nodoc: private def query(path, exts, _, locals, cache:) - query = +"" - EXTENSIONS.each do |ext, prefix| - query << "(" << exts[ext].map { |e| e && Regexp.escape("#{prefix}#{e}") }.join("|") << "|)" - end - query = /^(#{Regexp.escape(path)})#{query}$/ + regex = build_regex(path, exts) - templates = [] - @hash.each do |_path, source| - next unless query.match?(_path) + @hash.select do |_path, _| + ("/" + _path).match?(regex) + end.map do |_path, source| handler, format, variant = extract_handler_and_format_and_variant(_path) - templates << Template.new(source, _path, handler, + + Template.new(source, _path, handler, virtual_path: path.virtual, format: format, variant: variant, locals: locals ) + end.sort_by do |t| + match = ("/" + t.identifier).match(regex) + EXTENSIONS.keys.reverse.map do |ext| + if ext == :variants && exts[ext] == :any + match[ext].nil? ? 0 : 1 + elsif match[ext].nil? + exts[ext].length + else + found = match[ext].to_sym + exts[ext].index(found) + end + end end - - templates.sort_by { |t| -t.identifier.match(/^#{query}$/).captures.compact_blank.size } end end diff --git a/actionview/test/activerecord/relation_cache_test.rb b/actionview/test/activerecord/relation_cache_test.rb index 6fe83dcb9a..26f3bfbcbc 100644 --- a/actionview/test/activerecord/relation_cache_test.rb +++ b/actionview/test/activerecord/relation_cache_test.rb @@ -17,9 +17,24 @@ class RelationCacheTest < ActionView::TestCase end def test_cache_relation_other - cache(Project.all) { concat("Hello World") } + assert_queries(1) do + cache(Project.all) { concat("Hello World") } + end assert_equal "Hello World", controller.cache_store.read("views/test/hello_world:fa9482a68ce25bf7589b8eddad72f736/projects-#{Project.count}") end def view_cache_dependencies; []; end + + def assert_queries(num) + ActiveRecord::Base.connection.materialize_transactions + count = 0 + + ActiveSupport::Notifications.subscribe("sql.active_record") do |_name, _start, _finish, _id, payload| + count += 1 unless ["SCHEMA", "TRANSACTION"].include? payload[:name] + end + + result = yield + assert_equal num, count, "#{count} instead of #{num} queries were executed." + result + end end diff --git a/actionview/test/template/form_options_helper_test.rb b/actionview/test/template/form_options_helper_test.rb index d7a7b95ab3..273646b077 100644 --- a/actionview/test/template/form_options_helper_test.rb +++ b/actionview/test/template/form_options_helper_test.rb @@ -47,6 +47,7 @@ class FormOptionsHelperTest < ActionView::TestCase FakeZone = Struct.new(:name) do def to_s; name; end def =~(_re); end + def match?(_re); end end module ClassMethods @@ -1266,6 +1267,7 @@ class FormOptionsHelperTest < ActionView::TestCase @fake_timezones.each do |tz| def tz.=~(re); %(A D).include?(name) end + def tz.match?(re); %(A D).include?(name) end end html = time_zone_select("firm", "time_zone", /A|D/) diff --git a/actionview/test/template/output_safety_helper_test.rb b/actionview/test/template/output_safety_helper_test.rb index faeeded1c8..b2c6ae99ed 100644 --- a/actionview/test/template/output_safety_helper_test.rb +++ b/actionview/test/template/output_safety_helper_test.rb @@ -43,7 +43,9 @@ class OutputSafetyHelperTest < ActionView::TestCase joined = safe_join(["a", "b"]) assert_equal "ab", joined - $, = "|" + silence_warnings do + $, = "|" + end joined = safe_join(["a", "b"]) assert_equal "a|b", joined ensure @@ -108,7 +110,9 @@ class OutputSafetyHelperTest < ActionView::TestCase test "to_sentence is not affected by $," do separator_was = $, - $, = "|" + silence_warnings do + $, = "|" + end begin assert_equal "one and two", to_sentence(["one", "two"]) assert_equal "one, two, and three", to_sentence(["one", "two", "three"]) diff --git a/actionview/test/template/testing/fixture_resolver_test.rb b/actionview/test/template/testing/fixture_resolver_test.rb index 6d0317e0c9..a6066e94c6 100644 --- a/actionview/test/template/testing/fixture_resolver_test.rb +++ b/actionview/test/template/testing/fixture_resolver_test.rb @@ -27,4 +27,26 @@ class FixtureResolverTest < ActiveSupport::TestCase assert_equal :html, templates.first.format assert_equal "variant", templates.first.variant end + + def test_should_match_locales + resolver = ActionView::FixtureResolver.new("arbitrary/path.erb" => "this text", "arbitrary/path.fr.erb" => "ce texte") + en = resolver.find_all("path", "arbitrary", false, locale: [:en], formats: [:html], variants: [], handlers: [:erb]) + fr = resolver.find_all("path", "arbitrary", false, locale: [:fr], formats: [:html], variants: [], handlers: [:erb]) + + assert_equal 1, en.size + assert_equal 2, fr.size + + assert_equal "this text", en[0].source + assert_equal "ce texte", fr[0].source + assert_equal "this text", fr[1].source + end + + def test_should_return_all_variants_for_any + resolver = ActionView::FixtureResolver.new("arbitrary/path.html.erb" => "this html", "arbitrary/path.html+varient.erb" => "this text") + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: [], handlers: [:erb]) + assert_equal 1, templates.size, "expected one template" + assert_equal "this html", templates.first.source + templates = resolver.find_all("path", "arbitrary", false, locale: [], formats: [:html], variants: :any, handlers: [:erb]) + assert_equal 2, templates.size, "expected all templates" + end end diff --git a/actionview/test/template/url_helper_test.rb b/actionview/test/template/url_helper_test.rb index bce6e7f370..ba5ae837f1 100644 --- a/actionview/test/template/url_helper_test.rb +++ b/actionview/test/template/url_helper_test.rb @@ -657,7 +657,7 @@ class UrlHelperTest < ActiveSupport::TestCase ) end - def test_mail_with_options + def test_mail_to_with_options assert_dom_equal( %{<a href="mailto:me@example.com?cc=ccaddress%40example.com&bcc=bccaddress%40example.com&body=This%20is%20the%20body%20of%20the%20message.&subject=This%20is%20an%20example%20email&reply-to=foo%40bar.com">My email</a>}, mail_to("me@example.com", "My email", cc: "ccaddress@example.com", bcc: "bccaddress@example.com", subject: "This is an example email", body: "This is the body of the message.", reply_to: "foo@bar.com") @@ -731,45 +731,109 @@ class UrlHelperTest < ActiveSupport::TestCase ) end - def test_sms_with_img + def test_sms_to_with_img assert_dom_equal %{<a href="sms:15155555785;"><img src="/feedback.png" /></a>}, sms_to("15155555785", raw('<img src="/feedback.png" />')) end - def test_sms_with_html_safe_string + def test_sms_to_with_html_safe_string assert_dom_equal( %{<a href="sms:1%2B5155555785;">1+5155555785</a>}, sms_to(raw("1+5155555785")) ) end - def test_sms_with_nil + def test_sms_to_with_nil assert_dom_equal( %{<a href="sms:;"></a>}, sms_to(nil) ) end - def test_sms_returns_html_safe_string + def test_sms_to_returns_html_safe_string assert_predicate sms_to("15155555785"), :html_safe? end - def test_sms_with_block + def test_sms_to_with_block assert_dom_equal %{<a href="sms:15155555785;"><span>Text me</span></a>}, sms_to("15155555785") { content_tag(:span, "Text me") } end - def test_sms_with_block_and_options + def test_sms_to_with_block_and_options assert_dom_equal %{<a class="special" href="sms:15155555785;?&body=Hello%20from%20Jim"><span>Text me</span></a>}, sms_to("15155555785", body: "Hello from Jim", class: "special") { content_tag(:span, "Text me") } end - def test_sms_does_not_modify_html_options_hash + def test_sms_to_does_not_modify_html_options_hash options = { class: "special" } sms_to "15155555785", "ME!", options assert_equal({ class: "special" }, options) end + def test_phone_to + assert_dom_equal %{<a href="tel:1234567890">1234567890</a>}, + phone_to("1234567890") + assert_dom_equal %{<a href="tel:1234567890">Bob</a>}, + phone_to("1234567890", "Bob") + assert_dom_equal( + %{<a class="phoner" href="tel:1234567890">Bob</a>}, + phone_to("1234567890", "Bob", "class" => "phoner") + ) + assert_equal phone_to("1234567890", "Bob", "class" => "admin"), + phone_to("1234567890", "Bob", class: "admin") + end + + def test_phone_to_with_options + assert_dom_equal( + %{<a class="example-class" href="tel:+011234567890">Phone</a>}, + phone_to("1234567890", "Phone", class: "example-class", country_code: "01") + ) + + assert_dom_equal( + %{<a href="tel:+011234567890">Phone</a>}, + phone_to("1234567890", "Phone", country_code: "01") + ) + end + + def test_phone_to_with_img + assert_dom_equal %{<a href="tel:1234567890"><img src="/feedback.png" /></a>}, + phone_to("1234567890", raw('<img src="/feedback.png" />')) + end + + def test_phone_to_with_html_safe_string + assert_dom_equal( + %{<a href="tel:1%2B234567890">1+234567890</a>}, + phone_to(raw("1+234567890")) + ) + end + + def test_phone_to_with_nil + assert_dom_equal( + %{<a href="tel:"></a>}, + phone_to(nil) + ) + end + + def test_phone_to_returns_html_safe_string + assert_predicate phone_to("1234567890"), :html_safe? + end + + def test_phone_to_with_block + assert_dom_equal %{<a href="tel:1234567890"><span>Phone</span></a>}, + phone_to("1234567890") { content_tag(:span, "Phone") } + end + + def test_phone_to_with_block_and_options + assert_dom_equal %{<a class="special" href="tel:+011234567890"><span>Phone</span></a>}, + phone_to("1234567890", country_code: "01", class: "special") { content_tag(:span, "Phone") } + end + + def test_phone_to_does_not_modify_html_options_hash + options = { class: "special" } + phone_to "1234567890", "ME!", options + assert_equal({ class: "special" }, options) + end + def protect_against_forgery? request_forgery end diff --git a/actionview/test/ujs/public/test/data-remote.js b/actionview/test/ujs/public/test/data-remote.js index 16ea114f3b..f95a57f795 100644 --- a/actionview/test/ujs/public/test/data-remote.js +++ b/actionview/test/ujs/public/test/data-remote.js @@ -490,4 +490,22 @@ asyncTest('changing a select option without "data-url" attribute still fires aja setTimeout(function() { start() }, 20) }) +asyncTest('inputs inside disabled fieldset are not submitted on remote forms', 3, function() { + $('form') + .append('<fieldset>\ + <input name="description" value="A wise man" />\ + </fieldset>') + .append('<fieldset disabled="disabled">\ + <input name="age" />\ + </fieldset>') + .bindNative('ajax:success', function(e, data, status, xhr) { + equal(data.params.user_name, 'john') + equal(data.params.description, 'A wise man') + equal(data.params.age, undefined) + + start() + }) + .triggerNative('submit') +}) + })() diff --git a/activejob/CHANGELOG.md b/activejob/CHANGELOG.md index 2f0d72ccb9..542a4eb2db 100644 --- a/activejob/CHANGELOG.md +++ b/activejob/CHANGELOG.md @@ -1,3 +1,6 @@ +* `assert_enqueued_with` and `assert_performed_with` can now test jobs with relative delay. + + *Vlado Cingel* Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/activejob/CHANGELOG.md) for previous changes. diff --git a/activejob/lib/active_job/test_helper.rb b/activejob/lib/active_job/test_helper.rb index 463020a332..5b1af7da22 100644 --- a/activejob/lib/active_job/test_helper.rb +++ b/activejob/lib/active_job/test_helper.rb @@ -630,7 +630,7 @@ module ActiveJob def prepare_args_for_assertion(args) args.dup.tap do |arguments| - arguments[:at] = arguments[:at].to_f if arguments[:at] + arguments[:at] = round_time_arguments(arguments[:at]) if arguments[:at] arguments[:args] = round_time_arguments(arguments[:args]) if arguments[:args] end end @@ -650,6 +650,7 @@ module ActiveJob def deserialize_args_for_assertion(job) job.dup.tap do |new_job| + new_job[:at] = round_time_arguments(Time.at(new_job[:at])) if new_job[:at] new_job[:args] = ActiveJob::Arguments.deserialize(new_job[:args]) if new_job[:args] end end diff --git a/activejob/test/cases/test_helper_test.rb b/activejob/test/cases/test_helper_test.rb index 32471cfacc..34dfd8eb56 100644 --- a/activejob/test/cases/test_helper_test.rb +++ b/activejob/test/cases/test_helper_test.rb @@ -621,6 +621,12 @@ class EnqueuedJobsTest < ActiveJob::TestCase end end + def test_assert_enqueued_with_with_relative_at_option + assert_enqueued_with(job: HelloJob, at: 5.minutes.from_now) do + HelloJob.set(wait: 5.minutes).perform_later + end + end + def test_assert_enqueued_with_with_no_block_with_at_option HelloJob.set(wait_until: Date.tomorrow.noon).perform_later assert_enqueued_with(job: HelloJob, at: Date.tomorrow.noon) @@ -1663,6 +1669,18 @@ class PerformedJobsTest < ActiveJob::TestCase end end + def test_assert_performed_with_with_relative_at_option + assert_performed_with(job: HelloJob, at: 5.minutes.from_now) do + HelloJob.set(wait: 5.minutes).perform_later + end + + assert_raise ActiveSupport::TestCase::Assertion do + assert_performed_with(job: HelloJob, at: 2.minutes.from_now) do + HelloJob.set(wait: 1.minute).perform_later + end + end + end + def test_assert_performed_with_without_block_with_at_option HelloJob.set(wait_until: Date.tomorrow.noon).perform_later diff --git a/activemodel/lib/active_model/attribute_methods.rb b/activemodel/lib/active_model/attribute_methods.rb index 415f1f679b..1a4e0b8e59 100644 --- a/activemodel/lib/active_model/attribute_methods.rb +++ b/activemodel/lib/active_model/attribute_methods.rb @@ -352,11 +352,7 @@ module ActiveModel def attribute_method_matchers_matching(method_name) attribute_method_matchers_cache.compute_if_absent(method_name) do - # Bump plain matcher to last place so that only methods that do not - # match any other pattern match the actual attribute name. - # This is currently only needed to support legacy usage. - matchers = attribute_method_matchers.partition(&:plain?).reverse.flatten(1) - matchers.map { |matcher| matcher.match(method_name) }.compact + attribute_method_matchers.map { |matcher| matcher.match(method_name) }.compact end end @@ -406,10 +402,6 @@ module ActiveModel def method_name(attr_name) @method_name % attr_name end - - def plain? - prefix.empty? && suffix.empty? - end end end diff --git a/activemodel/lib/active_model/errors.rb b/activemodel/lib/active_model/errors.rb index 42c004ce31..c91ac69603 100644 --- a/activemodel/lib/active_model/errors.rb +++ b/activemodel/lib/active_model/errors.rb @@ -100,7 +100,7 @@ module ActiveModel def copy!(other) # :nodoc: @errors = other.errors.deep_dup @errors.each { |error| - error.instance_variable_set("@base", @base) + error.instance_variable_set(:@base, @base) } end diff --git a/activemodel/lib/active_model/type/helpers/timezone.rb b/activemodel/lib/active_model/type/helpers/timezone.rb index cf87b9715b..b0477aec32 100644 --- a/activemodel/lib/active_model/type/helpers/timezone.rb +++ b/activemodel/lib/active_model/type/helpers/timezone.rb @@ -7,7 +7,7 @@ module ActiveModel module Helpers # :nodoc: all module Timezone def is_utc? - ::Time.zone_default.nil? || ::Time.zone_default =~ "UTC" + ::Time.zone_default.nil? || ::Time.zone_default.match?("UTC") end def default_timezone diff --git a/activemodel/lib/active_model/validations/format.rb b/activemodel/lib/active_model/validations/format.rb index bea57415b0..a473440b4e 100644 --- a/activemodel/lib/active_model/validations/format.rb +++ b/activemodel/lib/active_model/validations/format.rb @@ -6,7 +6,7 @@ module ActiveModel def validate_each(record, attribute, value) if options[:with] regexp = option_call(record, :with) - record_error(record, attribute, :with, value) if value.to_s !~ regexp + record_error(record, attribute, :with, value) unless regexp.match?(value.to_s) elsif options[:without] regexp = option_call(record, :without) record_error(record, attribute, :without, value) if regexp.match?(value.to_s) diff --git a/activemodel/lib/active_model/validations/validates.rb b/activemodel/lib/active_model/validations/validates.rb index 97612d474d..8906837aa8 100644 --- a/activemodel/lib/active_model/validations/validates.rb +++ b/activemodel/lib/active_model/validations/validates.rb @@ -27,7 +27,7 @@ module ActiveModel # class EmailValidator < ActiveModel::EachValidator # def validate_each(record, attribute, value) # record.errors.add attribute, (options[:message] || "is not an email") unless - # value =~ /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i + # /\A([^@\s]+)@((?:[-a-z0-9]+\.)+[a-z]{2,})\z/i.match?(value) # end # end # @@ -47,7 +47,7 @@ module ActiveModel # # class TitleValidator < ActiveModel::EachValidator # def validate_each(record, attribute, value) - # record.errors.add attribute, "must start with 'the'" unless value =~ /\Athe/i + # record.errors.add attribute, "must start with 'the'" unless /\Athe/i.match?(value) # end # end # diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 2af48f99db..56a8796318 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,21 @@ +* Preserve user supplied joins order as much as possible. + + Fixes #36761, #34328, #24281, #12953. + + *Ryuta Kamizono* + +* Allow `matches_regex` and `does_not_match_regexp` on the MySQL Arel visitor. + + *James Pearson* + +* Allow specifying fixtures to be ignored by setting `ignore` in YAML file's '_fixture' section. + + *Tongfei Gao* + +* Make the DATABASE_URL env variable only affect the primary connection. Add new env variables for multiple databases. + + *John Crepezzi*, *Eileen Uchitelle* + * Add a warning for enum elements with 'not_' prefix. class Foo @@ -56,5 +74,4 @@ *Michael Duchemin* - Please check [6-0-stable](https://github.com/rails/rails/blob/6-0-stable/activerecord/CHANGELOG.md) for previous changes. diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 6ad4c75fb5..2072f93194 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -62,7 +62,7 @@ module ActiveRecord::Associations::Builder # :nodoc: def middle_reflection(join_model) middle_name = [lhs_model.name.downcase.pluralize, - association_name].join("_").gsub("::", "_").to_sym + association_name.to_s].sort.join("_").gsub("::", "_").to_sym middle_options = middle_options join_model HasMany.create_reflection(lhs_model, diff --git a/activerecord/lib/active_record/associations/collection_proxy.rb b/activerecord/lib/active_record/associations/collection_proxy.rb index 0db0ad8595..404a7c02ba 100644 --- a/activerecord/lib/active_record/associations/collection_proxy.rb +++ b/activerecord/lib/active_record/associations/collection_proxy.rb @@ -100,7 +100,7 @@ module ActiveRecord # converting them into an array and iterating through them using # Array#select. # - # person.pets.select { |pet| pet.name =~ /oo/ } + # person.pets.select { |pet| /oo/.match?(pet.name) } # # => [ # # #<Pet id: 2, name: "Spook", person_id: 1>, # # #<Pet id: 3, name: "Choo-Choo", person_id: 1> diff --git a/activerecord/lib/active_record/attribute_methods/query.rb b/activerecord/lib/active_record/attribute_methods/query.rb index 0cf67644af..5a21e36cc7 100644 --- a/activerecord/lib/active_record/attribute_methods/query.rb +++ b/activerecord/lib/active_record/attribute_methods/query.rb @@ -17,7 +17,7 @@ module ActiveRecord when false, nil then false else if !type_for_attribute(attr_name) { false } - if Numeric === value || value !~ /[^0-9]/ + if Numeric === value || !value.match?(/[^0-9]/) !value.to_i.zero? else return false if ActiveModel::Type::Boolean::FALSE_VALUES.include?(value) 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 ef1eef6b69..0fe16270ed 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -572,7 +572,8 @@ module ActiveRecord end end - # See https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html + # See https://dev.mysql.com/doc/refman/5.7/en/server-error-reference.html + ER_DB_CREATE_EXISTS = 1007 ER_DUP_ENTRY = 1062 ER_NOT_NULL_VIOLATION = 1048 ER_NO_REFERENCED_ROW = 1216 @@ -592,6 +593,8 @@ module ActiveRecord def translate_exception(exception, message:, sql:, binds:) case error_number(exception) + when ER_DB_CREATE_EXISTS + DatabaseAlreadyExists.new(message, sql: sql, binds: binds) when ER_DUP_ENTRY RecordNotUnique.new(message, sql: sql, binds: binds) when ER_NO_REFERENCED_ROW, ER_ROW_IS_REFERENCED, ER_ROW_IS_REFERENCED_2, ER_NO_REFERENCED_ROW_2 diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 0732b69f81..20041f0c85 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -275,7 +275,7 @@ module ActiveRecord # hash and merges with the rest of the hash. # Connection details inside of the "url" key win any merge conflicts def resolve_hash_connection(spec) - if spec["url"] && spec["url"] !~ /^jdbc:/ + if spec["url"] && !spec["url"].match?(/^jdbc:/) connection_hash = resolve_url_connection(spec.delete("url")) spec.merge!(connection_hash) end diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 0a7c6d8ac4..f33ba87435 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -467,6 +467,7 @@ module ActiveRecord UNIQUE_VIOLATION = "23505" SERIALIZATION_FAILURE = "40001" DEADLOCK_DETECTED = "40P01" + DUPLICATE_DATABASE = "42P04" LOCK_NOT_AVAILABLE = "55P03" QUERY_CANCELED = "57014" @@ -488,6 +489,8 @@ module ActiveRecord SerializationFailure.new(message, sql: sql, binds: binds) when DEADLOCK_DETECTED Deadlocked.new(message, sql: sql, binds: binds) + when DUPLICATE_DATABASE + DatabaseAlreadyExists.new(message, sql: sql, binds: binds) when LOCK_NOT_AVAILABLE LockWaitTimeout.new(message, sql: sql, binds: binds) when QUERY_CANCELED diff --git a/activerecord/lib/active_record/database_configurations.rb b/activerecord/lib/active_record/database_configurations.rb index 8baa0f5af6..fcf13a910c 100644 --- a/activerecord/lib/active_record/database_configurations.rb +++ b/activerecord/lib/active_record/database_configurations.rb @@ -91,6 +91,19 @@ module ActiveRecord end alias :blank? :empty? + def each + throw_getter_deprecation(:each) + configurations.each { |config| + yield [config.env_name, config.config] + } + end + + def first + throw_getter_deprecation(:first) + config = configurations.first + [config.env_name, config.config] + end + private def env_with_configs(env = nil) if env @@ -168,17 +181,21 @@ module ActiveRecord end def environment_url_config(env, spec_name, config) - url = ENV["DATABASE_URL"] + url = environment_value_for(spec_name) return unless url ActiveRecord::DatabaseConfigurations::UrlConfig.new(env, spec_name, url, config) end + def environment_value_for(spec_name) + spec_env_key = "#{spec_name.upcase}_DATABASE_URL" + url = ENV[spec_env_key] + url ||= ENV["DATABASE_URL"] if spec_name == "primary" + url + end + def method_missing(method, *args, &blk) case method - when :each, :first - throw_getter_deprecation(method) - configurations.send(method, *args, &blk) when :fetch throw_getter_deprecation(method) configs_for(env_name: args.first) diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index 20cc987d6e..509f21c9a5 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -187,6 +187,10 @@ module ActiveRecord class NoDatabaseError < StatementInvalid end + # Raised when creating a database if it exists. + class DatabaseAlreadyExists < StatementInvalid + end + # Raised when PostgreSQL returns 'cached plan must not change result type' and # we cannot retry gracefully (e.g. inside a transaction) class PreparedStatementCacheExpired < StatementInvalid @@ -334,7 +338,7 @@ module ActiveRecord # See the following: # # * https://www.postgresql.org/docs/current/static/transaction-iso.html - # * https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html#error_er_lock_deadlock + # * https://dev.mysql.com/doc/refman/5.7/en/server-error-reference.html#error_er_lock_deadlock class TransactionRollbackError < StatementInvalid end diff --git a/activerecord/lib/active_record/explain_subscriber.rb b/activerecord/lib/active_record/explain_subscriber.rb index a86217abc0..ce209092f5 100644 --- a/activerecord/lib/active_record/explain_subscriber.rb +++ b/activerecord/lib/active_record/explain_subscriber.rb @@ -26,7 +26,7 @@ module ActiveRecord payload[:exception] || payload[:cached] || IGNORED_PAYLOADS.include?(payload[:name]) || - payload[:sql] !~ EXPLAINED_SQLS + !payload[:sql].match?(EXPLAINED_SQLS) end ActiveSupport::Notifications.subscribe("sql.active_record", new) diff --git a/activerecord/lib/active_record/fixture_set/file.rb b/activerecord/lib/active_record/fixture_set/file.rb index f1ea0e022f..b2030a5bb9 100644 --- a/activerecord/lib/active_record/fixture_set/file.rb +++ b/activerecord/lib/active_record/fixture_set/file.rb @@ -29,6 +29,10 @@ module ActiveRecord config_row["model_class"] end + def ignored_fixtures + config_row["ignore"] + end + private def rows @rows ||= raw_rows.reject { |fixture_name, _| fixture_name == "_fixture" } @@ -40,7 +44,7 @@ module ActiveRecord if row row.last else - { 'model_class': nil } + { 'model_class': nil, 'ignore': nil } end end end diff --git a/activerecord/lib/active_record/fixtures.rb b/activerecord/lib/active_record/fixtures.rb index 046ed0e95c..3df4b3c87f 100644 --- a/activerecord/lib/active_record/fixtures.rb +++ b/activerecord/lib/active_record/fixtures.rb @@ -420,6 +420,29 @@ module ActiveRecord # # Any fixture labeled "DEFAULTS" is safely ignored. # + # Besides using "DEFAULTS", you can also specify what fixtures will + # be ignored by setting "ignore" in "_fixture" section. + # + # # users.yml + # _fixture: + # ignore: + # - base + # # or use "ignore: base" when there is only one fixture needs to be ignored. + # + # base: &base + # admin: false + # introduction: "This is a default description" + # + # admin: + # <<: *base + # admin: true + # + # visitor: + # <<: *base + # + # In the above example, 'base' will be ignored when creating fixtures. + # This can be used for common attributes inheriting. + # # == Configure the fixture model class # # It's possible to set the fixture's model class directly in the YAML file. @@ -614,7 +637,7 @@ module ActiveRecord end end - attr_reader :table_name, :name, :fixtures, :model_class, :config + attr_reader :table_name, :name, :fixtures, :model_class, :ignored_fixtures, :config def initialize(_, name, class_name, path, config = ActiveRecord::Base) @name = name @@ -647,8 +670,8 @@ module ActiveRecord # Returns a hash of rows to be inserted. The key is the table, the value is # a list of rows to insert to that table. def table_rows - # allow a standard key to be used for doing defaults in YAML - fixtures.delete("DEFAULTS") + # allow specifying fixtures to be ignored by setting `ignore` in `_fixture` section + fixtures.except!(*ignored_fixtures) TableRows.new( table_name, @@ -667,6 +690,21 @@ module ActiveRecord end end + def ignored_fixtures=(base) + @ignored_fixtures = + case base + when Array + base + when String + [base] + else + [] + end + + @ignored_fixtures << "DEFAULTS" unless @ignored_fixtures.include?("DEFAULTS") + @ignored_fixtures.compact + end + # Loads the fixtures from the YAML file at +path+. # If the file sets the +model_class+ and current instance value is not set, # it uses the file value. @@ -678,6 +716,7 @@ module ActiveRecord yaml_files.each_with_object({}) do |file, fixtures| FixtureSet::File.open(file) do |fh| self.model_class ||= fh.model_class if fh.model_class + self.ignored_fixtures ||= fh.ignored_fixtures fh.each do |fixture_name, row| fixtures[fixture_name] = ActiveRecord::Fixture.new(row, model_class) end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 323b01ab2d..4dfe8655fe 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -566,10 +566,10 @@ module ActiveRecord def becomes(klass) became = klass.allocate became.send(:initialize) - became.instance_variable_set("@attributes", @attributes) - became.instance_variable_set("@mutations_from_database", @mutations_from_database ||= nil) - became.instance_variable_set("@new_record", new_record?) - became.instance_variable_set("@destroyed", destroyed?) + became.instance_variable_set(:@attributes, @attributes) + became.instance_variable_set(:@mutations_from_database, @mutations_from_database ||= nil) + became.instance_variable_set(:@new_record, new_record?) + became.instance_variable_set(:@destroyed, destroyed?) became.errors.copy!(errors) became end @@ -809,7 +809,7 @@ module ActiveRecord self.class.unscoped { self.class.find(id) } end - @attributes = fresh_object.instance_variable_get("@attributes") + @attributes = fresh_object.instance_variable_get(:@attributes) @new_record = false self end diff --git a/activerecord/lib/active_record/relation/query_methods.rb b/activerecord/lib/active_record/relation/query_methods.rb index 6957ba052b..29e2e2cedc 100644 --- a/activerecord/lib/active_record/relation/query_methods.rb +++ b/activerecord/lib/active_record/relation/query_methods.rb @@ -1095,26 +1095,44 @@ module ActiveRecord end def build_left_outer_joins(manager, outer_joins, aliases) - buckets = { association_join: valid_association_list(outer_joins) } + buckets = Hash.new { |h, k| h[k] = [] } + buckets[:association_join] = valid_association_list(outer_joins) build_join_query(manager, buckets, Arel::Nodes::OuterJoin, aliases) end def build_joins(manager, joins, aliases) + buckets = Hash.new { |h, k| h[k] = [] } + unless left_outer_joins_values.empty? left_joins = valid_association_list(left_outer_joins_values.flatten) - joins.unshift construct_join_dependency(left_joins, Arel::Nodes::OuterJoin) + buckets[:stashed_join] << construct_join_dependency(left_joins, Arel::Nodes::OuterJoin) end - buckets = joins.group_by do |join| + joins.map! do |join| + if join.is_a?(String) + table.create_string_join(Arel.sql(join.strip)) unless join.blank? + else + join + end + end.compact_blank! + + while joins.first.is_a?(Arel::Nodes::Join) + join_node = joins.shift + if join_node.is_a?(Arel::Nodes::StringJoin) && !buckets[:stashed_join].empty? + buckets[:join_node] << join_node + else + buckets[:leading_join] << join_node + end + end + + joins.each do |join| case join - when String - :string_join when Hash, Symbol, Array - :association_join + buckets[:association_join] << join when ActiveRecord::Associations::JoinDependency - :stashed_join + buckets[:stashed_join] << join when Arel::Nodes::Join - :join_node + buckets[:join_node] << join else raise "unknown class: %s" % join.class.name end @@ -1124,25 +1142,21 @@ module ActiveRecord end def build_join_query(manager, buckets, join_type, aliases) - buckets.default = [] - association_joins = buckets[:association_join] stashed_joins = buckets[:stashed_join] + leading_joins = buckets[:leading_join].tap(&:uniq!) join_nodes = buckets[:join_node].tap(&:uniq!) - string_joins = buckets[:string_join].compact_blank!.map!(&:strip).tap(&:uniq!) - - string_joins.map! { |join| table.create_string_join(Arel.sql(join)) } join_sources = manager.join_sources - join_sources.concat(join_nodes) unless join_nodes.empty? + join_sources.concat(leading_joins) unless leading_joins.empty? unless association_joins.empty? && stashed_joins.empty? - alias_tracker = alias_tracker(join_nodes + string_joins, aliases) + alias_tracker = alias_tracker(leading_joins + join_nodes, aliases) join_dependency = construct_join_dependency(association_joins, join_type) join_sources.concat(join_dependency.join_constraints(stashed_joins, alias_tracker)) end - join_sources.concat(string_joins) unless string_joins.empty? + join_sources.concat(join_nodes) unless join_nodes.empty? end def build_select(arel) diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 5d1ce19829..300e67b0aa 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -4,7 +4,6 @@ require "active_record/database_configurations" module ActiveRecord module Tasks # :nodoc: - class DatabaseAlreadyExists < StandardError; end # :nodoc: class DatabaseNotSupported < StandardError; end # :nodoc: # ActiveRecord::Tasks::DatabaseTasks is a utility class, which encapsulates diff --git a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index e3efeb75b5..b9a8ccb22c 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -15,12 +15,6 @@ module ActiveRecord establish_connection configuration_without_database connection.create_database configuration["database"], creation_options establish_connection configuration - rescue ActiveRecord::StatementInvalid => error - if connection.error_number(error.cause) == ER_DB_CREATE_EXISTS - raise DatabaseAlreadyExists - else - raise - end end def drop diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index 626ffdfdf9..fc37db216d 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -21,12 +21,6 @@ module ActiveRecord connection.create_database configuration["database"], configuration.merge("encoding" => encoding) establish_connection configuration - rescue ActiveRecord::StatementInvalid => error - if error.cause.is_a?(PG::DuplicateDatabase) - raise DatabaseAlreadyExists - else - raise - end end def drop diff --git a/activerecord/lib/arel/visitors/mysql.rb b/activerecord/lib/arel/visitors/mysql.rb index dd77cfdf66..6cb866715f 100644 --- a/activerecord/lib/arel/visitors/mysql.rb +++ b/activerecord/lib/arel/visitors/mysql.rb @@ -48,6 +48,14 @@ module Arel # :nodoc: all visit_Arel_Nodes_IsNotDistinctFrom o, collector end + def visit_Arel_Nodes_Regexp(o, collector) + infix_value o, collector, " REGEXP " + end + + def visit_Arel_Nodes_NotRegexp(o, collector) + infix_value o, collector, " NOT REGEXP " + end + # In the simple case, MySQL allows us to place JOINs directly into the UPDATE # query. However, this does not allow for LIMIT, OFFSET and ORDER. To support # these, we must use a subquery. diff --git a/activerecord/lib/arel/visitors/oracle.rb b/activerecord/lib/arel/visitors/oracle.rb index aab66301ef..c54aec71a6 100644 --- a/activerecord/lib/arel/visitors/oracle.rb +++ b/activerecord/lib/arel/visitors/oracle.rb @@ -9,7 +9,7 @@ module Arel # :nodoc: all # if need to select first records without ORDER BY and GROUP BY and without DISTINCT # then can use simple ROWNUM in WHERE clause - if o.limit && o.orders.empty? && o.cores.first.groups.empty? && !o.offset && o.cores.first.set_quantifier.class.to_s !~ /Distinct/ + if o.limit && o.orders.empty? && o.cores.first.groups.empty? && !o.offset && !o.cores.first.set_quantifier.class.to_s.match?(/Distinct/) o.cores.last.wheres.push Nodes::LessThanOrEqual.new( Nodes::SqlLiteral.new("ROWNUM"), o.limit.expr ) diff --git a/activerecord/test/cases/arel/nodes/node_test.rb b/activerecord/test/cases/arel/nodes/node_test.rb index f1e0ce1ea9..8a9ecd84ca 100644 --- a/activerecord/test/cases/arel/nodes/node_test.rb +++ b/activerecord/test/cases/arel/nodes/node_test.rb @@ -14,7 +14,7 @@ module Arel }.grep(Class).each do |klass| next if Nodes::SqlLiteral == klass next if Nodes::BindParam == klass - next if klass.name =~ /^Arel::Nodes::(?:Test|.*Test$)/ + next if /^Arel::Nodes::(?:Test|.*Test$)/.match?(klass.name) assert klass.ancestors.include?(Nodes::Node), klass.name end end diff --git a/activerecord/test/cases/arel/select_manager_test.rb b/activerecord/test/cases/arel/select_manager_test.rb index 526fe6787a..4512d8e8a6 100644 --- a/activerecord/test/cases/arel/select_manager_test.rb +++ b/activerecord/test/cases/arel/select_manager_test.rb @@ -514,7 +514,7 @@ module Arel assert_equal "bar", join.right end - it "should create join nodes with a outer join klass" do + it "should create join nodes with an outer join klass" do relation = Arel::SelectManager.new join = relation.create_join "foo", "bar", Arel::Nodes::OuterJoin assert_kind_of Arel::Nodes::OuterJoin, join diff --git a/activerecord/test/cases/arel/visitors/mysql_test.rb b/activerecord/test/cases/arel/visitors/mysql_test.rb index 5f37587957..05dccd126e 100644 --- a/activerecord/test/cases/arel/visitors/mysql_test.rb +++ b/activerecord/test/cases/arel/visitors/mysql_test.rb @@ -104,6 +104,52 @@ module Arel sql.must_be_like %{ NOT "users"."name" <=> NULL } end end + + describe "Nodes::Regexp" do + before do + @table = Table.new(:users) + @attr = @table[:id] + end + + it "should know how to visit" do + node = @table[:name].matches_regexp("foo.*") + node.must_be_kind_of Nodes::Regexp + compile(node).must_be_like %{ + "users"."name" REGEXP 'foo.*' + } + end + + it "can handle subqueries" do + subquery = @table.project(:id).where(@table[:name].matches_regexp("foo.*")) + node = @attr.in subquery + compile(node).must_be_like %{ + "users"."id" IN (SELECT id FROM "users" WHERE "users"."name" REGEXP 'foo.*') + } + end + end + + describe "Nodes::NotRegexp" do + before do + @table = Table.new(:users) + @attr = @table[:id] + end + + it "should know how to visit" do + node = @table[:name].does_not_match_regexp("foo.*") + node.must_be_kind_of Nodes::NotRegexp + compile(node).must_be_like %{ + "users"."name" NOT REGEXP 'foo.*' + } + end + + it "can handle subqueries" do + subquery = @table.project(:id).where(@table[:name].does_not_match_regexp("foo.*")) + node = @attr.in subquery + compile(node).must_be_like %{ + "users"."id" IN (SELECT id FROM "users" WHERE "users"."name" NOT REGEXP 'foo.*') + } + end + end end end end diff --git a/activerecord/test/cases/associations/belongs_to_associations_test.rb b/activerecord/test/cases/associations/belongs_to_associations_test.rb index 3525fa2ab8..6bd305306f 100644 --- a/activerecord/test/cases/associations/belongs_to_associations_test.rb +++ b/activerecord/test/cases/associations/belongs_to_associations_test.rb @@ -60,11 +60,8 @@ class BelongsToAssociationsTest < ActiveRecord::TestCase end def test_belongs_to_does_not_use_order_by - ActiveRecord::SQLCounter.clear_log - Client.find(3).firm - ensure - sql_log = ActiveRecord::SQLCounter.log - assert sql_log.all? { |sql| /order by/i !~ sql }, "ORDER BY was used in the query: #{sql_log}" + sql_log = capture_sql { Client.find(3).firm } + assert sql_log.all? { |sql| !/order by/i.match?(sql) }, "ORDER BY was used in the query: #{sql_log}" end def test_belongs_to_with_primary_key diff --git a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb index 25cfa0a723..de9742b250 100644 --- a/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb +++ b/activerecord/test/cases/associations/has_and_belongs_to_many_associations_test.rb @@ -700,10 +700,17 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal ["id"], developers(:david).projects.select(:id).first.attributes.keys end + def test_join_middle_table_alias + assert_equal( + 2, + Project.includes(:developers_projects).where.not("developers_projects.joined_on": nil).to_a.size + ) + end + def test_join_table_alias assert_equal( 3, - Developer.includes(projects: :developers).where.not("projects_developers_projects_join.joined_on": nil).to_a.size + Developer.includes(projects: :developers).where.not("developers_projects_projects_join.joined_on": nil).to_a.size ) end @@ -716,7 +723,7 @@ class HasAndBelongsToManyAssociationsTest < ActiveRecord::TestCase assert_equal( 3, - Developer.includes(projects: :developers).where.not("projects_developers_projects_join.joined_on": nil).group(group.join(",")).to_a.size + Developer.includes(projects: :developers).where.not("developers_projects_projects_join.joined_on": nil).group(group.join(",")).to_a.size ) end diff --git a/activerecord/test/cases/associations/has_one_associations_test.rb b/activerecord/test/cases/associations/has_one_associations_test.rb index b1fcf47528..6c2a09c296 100644 --- a/activerecord/test/cases/associations/has_one_associations_test.rb +++ b/activerecord/test/cases/associations/has_one_associations_test.rb @@ -37,11 +37,8 @@ class HasOneAssociationsTest < ActiveRecord::TestCase end def test_has_one_does_not_use_order_by - ActiveRecord::SQLCounter.clear_log - companies(:first_firm).account - ensure - sql_log = ActiveRecord::SQLCounter.log - assert sql_log.all? { |sql| /order by/i !~ sql }, "ORDER BY was used in the query: #{sql_log}" + sql_log = capture_sql { companies(:first_firm).account } + assert sql_log.all? { |sql| !/order by/i.match?(sql) }, "ORDER BY was used in the query: #{sql_log}" end def test_has_one_cache_nils diff --git a/activerecord/test/cases/associations/inner_join_association_test.rb b/activerecord/test/cases/associations/inner_join_association_test.rb index e0dac01f4a..b65af4b819 100644 --- a/activerecord/test/cases/associations/inner_join_association_test.rb +++ b/activerecord/test/cases/associations/inner_join_association_test.rb @@ -13,7 +13,7 @@ require "models/tag" class InnerJoinAssociationTest < ActiveRecord::TestCase fixtures :authors, :author_addresses, :essays, :posts, :comments, :categories, :categories_posts, :categorizations, - :taggings, :tags + :taggings, :tags, :people def test_construct_finder_sql_applies_aliases_tables_on_association_conditions result = Author.joins(:thinking_posts, :welcome_posts).to_a @@ -36,16 +36,37 @@ class InnerJoinAssociationTest < ActiveRecord::TestCase end def test_construct_finder_sql_does_not_table_name_collide_with_string_joins - sql = Person.joins(:agents).joins("JOIN people agents_people ON agents_people.primary_contact_id = people.id").to_sql - assert_match(/agents_people_2/i, sql) + string_join = <<~SQL + JOIN people agents_people ON agents_people.primary_contact_id = agents_people_2.id AND agents_people.id > agents_people_2.id + SQL + + expected = people(:susan) + assert_sql(/agents_people_2/i) do + assert_equal [expected], Person.joins(:agents).joins(string_join) + end end def test_construct_finder_sql_does_not_table_name_collide_with_aliased_joins - people = Person.arel_table - agents = people.alias("agents_people") - constraint = agents[:primary_contact_id].eq(people[:id]) - sql = Person.joins(:agents).joins(agents.create_join(agents, agents.create_on(constraint))).to_sql - assert_match(/agents_people_2/i, sql) + agents = Person.arel_table.alias("agents_people") + agents_2 = Person.arel_table.alias("agents_people_2") + constraint = agents[:primary_contact_id].eq(agents_2[:id]).and(agents[:id].gt(agents_2[:id])) + + expected = people(:susan) + assert_sql(/agents_people_2/i) do + assert_equal [expected], Person.joins(:agents).joins(agents.create_join(agents, agents.create_on(constraint))) + end + end + + def test_user_supplied_joins_order_should_be_preserved + string_join = <<~SQL + JOIN people agents_people_2 ON agents_people_2.primary_contact_id = people.id + SQL + agents = Person.arel_table.alias("agents_people") + agents_2 = Person.arel_table.alias("agents_people_2") + constraint = agents[:primary_contact_id].eq(agents_2[:id]).and(agents[:id].gt(agents_2[:id])) + + expected = people(:susan) + assert_equal [expected], Person.joins(string_join).joins(agents.create_join(agents, agents.create_on(constraint))) end def test_construct_finder_sql_ignores_empty_joins_hash diff --git a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb index 95e57f42e3..ee2972101f 100644 --- a/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb +++ b/activerecord/test/cases/connection_adapters/merge_and_resolve_default_url_config_test.rb @@ -334,6 +334,8 @@ module ActiveRecord } } + configs = ActiveRecord::DatabaseConfigurations.new(config) + actual = configs.configs_for(env_name: "default_env", spec_name: "primary").config expected = { "adapter" => "postgresql", "database" => "foo", @@ -341,11 +343,37 @@ module ActiveRecord "pool" => 5 } - ["primary", "animals"].each do |spec_name| - configs = ActiveRecord::DatabaseConfigurations.new(config) - actual = configs.configs_for(env_name: "default_env", spec_name: spec_name).config - assert_equal expected, actual - end + assert_equal expected, actual + + configs = ActiveRecord::DatabaseConfigurations.new(config) + actual = configs.configs_for(env_name: "default_env", spec_name: "animals").config + expected = { "pool" => 5 } + + assert_equal expected, actual + end + + def test_separate_database_env_vars + ENV["DATABASE_URL"] = "postgres://localhost/foo" + ENV["PRIMARY_DATABASE_URL"] = "postgres://localhost/primary" + ENV["ANIMALS_DATABASE_URL"] = "postgres://localhost/animals" + + config = { + "default_env" => { + "primary" => { "pool" => 5 }, + "animals" => { "pool" => 5 } + } + } + + configs = ActiveRecord::DatabaseConfigurations.new(config) + actual = configs.configs_for(env_name: "default_env", spec_name: "primary").config + assert_equal "primary", actual["database"] + + configs = ActiveRecord::DatabaseConfigurations.new(config) + actual = configs.configs_for(env_name: "default_env", spec_name: "animals").config + assert_equal "animals", actual["database"] + ensure + ENV.delete("PRIMARY_DATABASE_URL") + ENV.delete("ANIMALS_DATABASE_URL") end def test_does_not_change_other_environments diff --git a/activerecord/test/cases/database_configurations_test.rb b/activerecord/test/cases/database_configurations_test.rb index ed8151f01a..714b9d75c5 100644 --- a/activerecord/test/cases/database_configurations_test.rb +++ b/activerecord/test/cases/database_configurations_test.rb @@ -80,17 +80,20 @@ class LegacyDatabaseConfigurationsTest < ActiveRecord::TestCase def test_each_is_deprecated assert_deprecated do - ActiveRecord::Base.configurations.each do |db_config| - assert_equal "primary", db_config.spec_name + all_configs = ActiveRecord::Base.configurations.values + ActiveRecord::Base.configurations.each do |env_name, config| + assert_includes ["arunit", "arunit2", "arunit_without_prepared_statements"], env_name + assert_includes all_configs, config end end end def test_first_is_deprecated + first_config = ActiveRecord::Base.configurations.configurations.map(&:config).first assert_deprecated do - db_config = ActiveRecord::Base.configurations.first - assert_equal "arunit", db_config.env_name - assert_equal "primary", db_config.spec_name + env_name, config = ActiveRecord::Base.configurations.first + assert_equal "arunit", env_name + assert_equal first_config, config end end diff --git a/activerecord/test/cases/enum_test.rb b/activerecord/test/cases/enum_test.rb index 8673a99c45..0ae156320a 100644 --- a/activerecord/test/cases/enum_test.rb +++ b/activerecord/test/cases/enum_test.rb @@ -580,7 +580,9 @@ class EnumTest < ActiveRecord::TestCase def self.name "Book" end - enum status: [:sent, :not_sent] + silence_warnings do + enum status: [:sent, :not_sent] + end end assert_match(expected_message, logger.logged(:warn).first) diff --git a/activerecord/test/cases/fixtures_test.rb b/activerecord/test/cases/fixtures_test.rb index a7f01e898e..0861d938c5 100644 --- a/activerecord/test/cases/fixtures_test.rb +++ b/activerecord/test/cases/fixtures_test.rb @@ -67,7 +67,7 @@ class FixturesTest < ActiveRecord::TestCase end def call(_, _, _, _, values) - @events << values[:sql] if values[:sql] =~ /INSERT/ + @events << values[:sql] if /INSERT/.match?(values[:sql]) end end @@ -1279,6 +1279,33 @@ class CustomNameForFixtureOrModelTest < ActiveRecord::TestCase end end +class IgnoreFixturesTest < ActiveRecord::TestCase + fixtures :other_books, :parrots + + test "ignores books fixtures" do + assert_raise(StandardError) { other_books(:published) } + assert_raise(StandardError) { other_books(:published_paperback) } + assert_raise(StandardError) { other_books(:published_ebook) } + + assert_equal 2, Book.count + assert_equal "Agile Web Development with Rails", other_books(:awdr).name + assert_equal "published", other_books(:awdr).status + assert_equal "paperback", other_books(:awdr).format + assert_equal "english", other_books(:awdr).language + + assert_equal "Ruby for Rails", other_books(:rfr).name + assert_equal "ebook", other_books(:rfr).format + assert_equal "published", other_books(:rfr).status + end + + test "ignores parrots fixtures" do + assert_raise(StandardError) { parrots(:DEFAULT) } + assert_raise(StandardError) { parrots(:DEAD_PARROT) } + + assert_equal "DeadParrot", parrots(:polly).parrot_sti_class + end +end + class FixturesWithDefaultScopeTest < ActiveRecord::TestCase fixtures :bulbs diff --git a/activerecord/test/cases/tasks/mysql_rake_test.rb b/activerecord/test/cases/tasks/mysql_rake_test.rb index ac3c5bc26e..4b039395e8 100644 --- a/activerecord/test/cases/tasks/mysql_rake_test.rb +++ b/activerecord/test/cases/tasks/mysql_rake_test.rb @@ -93,11 +93,9 @@ if current_adapter?(:Mysql2Adapter) with_stubbed_connection_establish_connection do ActiveRecord::Base.connection.stub( :create_database, - proc { raise ActiveRecord::StatementInvalid } + proc { raise ActiveRecord::DatabaseAlreadyExists } ) do - ActiveRecord::Base.connection.stub(:error_number, 1007) do - ActiveRecord::Tasks::DatabaseTasks.create @configuration - end + ActiveRecord::Tasks::DatabaseTasks.create @configuration assert_equal "Database 'my-app-db' already exists\n", $stderr.string end diff --git a/activerecord/test/cases/tasks/postgresql_rake_test.rb b/activerecord/test/cases/tasks/postgresql_rake_test.rb index f9df650687..d74ba0580d 100644 --- a/activerecord/test/cases/tasks/postgresql_rake_test.rb +++ b/activerecord/test/cases/tasks/postgresql_rake_test.rb @@ -129,7 +129,7 @@ if current_adapter?(:PostgreSQLAdapter) with_stubbed_connection_establish_connection do ActiveRecord::Base.connection.stub( :create_database, - proc { raise ActiveRecord::Tasks::DatabaseAlreadyExists } + proc { raise ActiveRecord::DatabaseAlreadyExists } ) do ActiveRecord::Tasks::DatabaseTasks.create @configuration diff --git a/activerecord/test/fixtures/other_books.yml b/activerecord/test/fixtures/other_books.yml new file mode 100644 index 0000000000..62806c03d7 --- /dev/null +++ b/activerecord/test/fixtures/other_books.yml @@ -0,0 +1,26 @@ +_fixture: + model_class: Book + ignore: + - PUBLISHED + - PUBLISHED_PAPERBACK + - PUBLISHED_EBOOK + +PUBLISHED: &PUBLISHED + status: :published + +PUBLISHED_PAPERBACK: &PUBLISHED_PAPERBACK + <<: *PUBLISHED + format: "paperback" + language: :english + +PUBLISHED_EBOOK: &PUBLISHED_EBOOK + <<: *PUBLISHED + format: "ebook" + +awdr: + <<: *PUBLISHED_PAPERBACK + name: "Agile Web Development with Rails" + +rfr: + <<: *PUBLISHED_EBOOK + name: "Ruby for Rails" diff --git a/activerecord/test/fixtures/parrots.yml b/activerecord/test/fixtures/parrots.yml index 8425ef98e0..4f0a090e03 100644 --- a/activerecord/test/fixtures/parrots.yml +++ b/activerecord/test/fixtures/parrots.yml @@ -1,3 +1,9 @@ +_fixture: + ignore: DEAD_PARROT + +DEAD_PARROT: &DEAD_PARROT + parrot_sti_class: DeadParrot + george: name: "Curious George" treasures: diamond, sapphire @@ -17,7 +23,7 @@ polly: name: $LABEL killer: blackbeard treasures: sapphire, ruby - parrot_sti_class: DeadParrot + <<: *DEAD_PARROT DEFAULTS: &DEFAULTS treasures: sapphire, ruby diff --git a/activestorage/test/dummy/config/initializers/backtrace_silencers.rb b/activestorage/test/dummy/config/initializers/backtrace_silencers.rb index d0f0d3b5df..8452ef9236 100644 --- a/activestorage/test/dummy/config/initializers/backtrace_silencers.rb +++ b/activestorage/test/dummy/config/initializers/backtrace_silencers.rb @@ -2,7 +2,7 @@ # Be sure to restart your server when you modify this file. # You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. -# Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } +# Rails.backtrace_cleaner.add_silencer { |line| /my_noisy_library/.match?(line) } # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. # Rails.backtrace_cleaner.remove_silencers! diff --git a/activestorage/test/fixtures/files/racecar.tif b/activestorage/test/fixtures/files/racecar.tif Binary files differindex 0a11b22896..97430741e2 100644 --- a/activestorage/test/fixtures/files/racecar.tif +++ b/activestorage/test/fixtures/files/racecar.tif diff --git a/activesupport/lib/active_support/backtrace_cleaner.rb b/activesupport/lib/active_support/backtrace_cleaner.rb index f55e821e10..6273012808 100644 --- a/activesupport/lib/active_support/backtrace_cleaner.rb +++ b/activesupport/lib/active_support/backtrace_cleaner.rb @@ -16,7 +16,7 @@ module ActiveSupport # # bc = ActiveSupport::BacktraceCleaner.new # bc.add_filter { |line| line.gsub(Rails.root.to_s, '') } # strip the Rails.root prefix - # bc.add_silencer { |line| line =~ /puma|rubygems/ } # skip any lines from puma or rubygems + # bc.add_silencer { |line| /puma|rubygems/.match?(line) } # skip any lines from puma or rubygems # bc.clean(exception.backtrace) # perform the cleanup # # To reconfigure an existing BacktraceCleaner (like the default one in Rails) @@ -65,7 +65,7 @@ module ActiveSupport # for a given line, it will be excluded from the clean backtrace. # # # Will reject all lines that include the word "puma", like "/gems/puma/server.rb" or "/app/my_puma_server/rb" - # backtrace_cleaner.add_silencer { |line| line =~ /puma/ } + # backtrace_cleaner.add_silencer { |line| /puma/.match?(line) } def add_silencer(&block) @silencers << block end diff --git a/activesupport/lib/active_support/core_ext/name_error.rb b/activesupport/lib/active_support/core_ext/name_error.rb index 6d37cd9dfd..c51aec0e03 100644 --- a/activesupport/lib/active_support/core_ext/name_error.rb +++ b/activesupport/lib/active_support/core_ext/name_error.rb @@ -15,7 +15,7 @@ class NameError # We should use original_message message instead. message = respond_to?(:original_message) ? original_message : self.message - if /undefined local variable or method/ !~ message + unless /undefined local variable or method/.match?(message) $1 if /((::)?([A-Z]\w*)(::[A-Z]\w*)*)$/ =~ message end end diff --git a/activesupport/lib/active_support/core_ext/string/access.rb b/activesupport/lib/active_support/core_ext/string/access.rb index 4ca24028b0..f6a14c08bc 100644 --- a/activesupport/lib/active_support/core_ext/string/access.rb +++ b/activesupport/lib/active_support/core_ext/string/access.rb @@ -44,7 +44,7 @@ class String # str.from(0).to(-1) # => "hello" # str.from(1).to(-2) # => "ell" def from(position) - self[position..-1] + self[position, length] end # Returns a substring from the beginning of the string to the given position. @@ -61,7 +61,8 @@ class String # str.from(0).to(-1) # => "hello" # str.from(1).to(-2) # => "ell" def to(position) - self[0..position] + position += size if position < 0 + self[0, position + 1] || +"" end # Returns the first character. If a limit is supplied, returns a substring @@ -75,17 +76,7 @@ class String # str.first(0) # => "" # str.first(6) # => "hello" def first(limit = 1) - ActiveSupport::Deprecation.warn( - "Calling String#first with a negative integer limit " \ - "will raise an ArgumentError in Rails 6.1." - ) if limit < 0 - if limit == 0 - "" - elsif limit >= size - dup - else - to(limit - 1) - end + self[0, limit] || raise(ArgumentError, "negative limit") end # Returns the last character of the string. If a limit is supplied, returns a substring @@ -99,16 +90,6 @@ class String # str.last(0) # => "" # str.last(6) # => "hello" def last(limit = 1) - ActiveSupport::Deprecation.warn( - "Calling String#last with a negative integer limit " \ - "will raise an ArgumentError in Rails 6.1." - ) if limit < 0 - if limit == 0 - "" - elsif limit >= size - dup - else - from(-limit) - end + self[[length - limit, 0].max, limit] || raise(ArgumentError, "negative limit") end end diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index 923d6ad228..6d71a67a46 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -793,7 +793,6 @@ module ActiveSupport #:nodoc: end private - # Returns the original name of a class or module even if `name` has been # overridden. def real_mod_name(mod) diff --git a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb index f75083a05a..2cf55713bd 100644 --- a/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb +++ b/activesupport/lib/active_support/dependencies/zeitwerk_integration.rb @@ -42,6 +42,17 @@ module ActiveSupport end end + module RequireDependency + def require_dependency(filename) + filename = filename.to_path if filename.respond_to?(:to_path) + if abspath = ActiveSupport::Dependencies.search_for_file(filename) + require abspath + else + require filename + end + end + end + module Inflector def self.camelize(basename, _abspath) basename.camelize @@ -90,7 +101,7 @@ module ActiveSupport def decorate_dependencies Dependencies.unhook! Dependencies.singleton_class.prepend(Decorations) - Object.class_eval { alias_method :require_dependency, :require } + Object.prepend(RequireDependency) end end end diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 18f3f53879..d33c79d60e 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -269,32 +269,36 @@ module ActiveSupport # NameError is raised when the name is not in CamelCase or the constant is # unknown. def constantize(camel_cased_word) - names = camel_cased_word.split("::") - - # Trigger a built-in NameError exception including the ill-formed constant in the message. - Object.const_get(camel_cased_word) if names.empty? - - # Remove the first blank element in case of '::ClassName' notation. - names.shift if names.size > 1 && names.first.empty? - - names.inject(Object) do |constant, name| - if constant == Object - constant.const_get(name) - else - candidate = constant.const_get(name) - next candidate if constant.const_defined?(name, false) - next candidate unless Object.const_defined?(name) - - # Go down the ancestors to check if it is owned directly. The check - # stops when we reach Object or the end of ancestors tree. - constant = constant.ancestors.inject(constant) do |const, ancestor| - break const if ancestor == Object - break ancestor if ancestor.const_defined?(name, false) - const + if camel_cased_word.blank? || !camel_cased_word.include?('::') + Object.const_get(camel_cased_word) + else + names = camel_cased_word.split("::") + + # Trigger a built-in NameError exception including the ill-formed constant in the message. + Object.const_get(camel_cased_word) if names.empty? + + # Remove the first blank element in case of '::ClassName' notation. + names.shift if names.size > 1 && names.first.empty? + + names.inject(Object) do |constant, name| + if constant == Object + constant.const_get(name) + else + candidate = constant.const_get(name) + next candidate if constant.const_defined?(name, false) + next candidate unless Object.const_defined?(name) + + # Go down the ancestors to check if it is owned directly. The check + # stops when we reach Object or the end of ancestors tree. + constant = constant.ancestors.inject(constant) do |const, ancestor| + break const if ancestor == Object + break ancestor if ancestor.const_defined?(name, false) + const + end + + # owner is in Object, so raise + constant.const_get(name, false) end - - # owner is in Object, so raise - constant.const_get(name, false) end end end diff --git a/activesupport/lib/active_support/inflector/transliterate.rb b/activesupport/lib/active_support/inflector/transliterate.rb index ec6e9ccb59..3ba2d93ed8 100644 --- a/activesupport/lib/active_support/inflector/transliterate.rb +++ b/activesupport/lib/active_support/inflector/transliterate.rb @@ -56,14 +56,39 @@ module ActiveSupport # # transliterate('Jürgen', locale: :de) # # => "Juergen" + # + # Transliteration is restricted to UTF-8, US-ASCII and GB18030 strings + # Other encodings will raise an ArgumentError. def transliterate(string, replacement = "?", locale: nil) raise ArgumentError, "Can only transliterate strings. Received #{string.class.name}" unless string.is_a?(String) - I18n.transliterate( + allowed_encodings = [Encoding::UTF_8, Encoding::US_ASCII, Encoding::GB18030] + raise ArgumentError, "Can not transliterate strings with #{string.encoding} encoding" unless allowed_encodings.include?(string.encoding) + + input_encoding = string.encoding + + # US-ASCII is a subset of UTF-8 so we'll force encoding as UTF-8 if + # US-ASCII is given. This way we can let tidy_bytes handle the string + # in the same way as we do for UTF-8 + string.force_encoding(Encoding::UTF_8) if string.encoding == Encoding::US_ASCII + + # GB18030 is Unicode compatible but is not a direct mapping so needs to be + # transcoded. Using invalid/undef :replace will result in loss of data in + # the event of invalid characters, but since tidy_bytes will replace + # invalid/undef with a "?" we're safe to do the same beforehand + string.encode!(Encoding::UTF_8, invalid: :replace, undef: :replace) if string.encoding == Encoding::GB18030 + + transliterated = I18n.transliterate( ActiveSupport::Multibyte::Unicode.tidy_bytes(string).unicode_normalize(:nfc), replacement: replacement, locale: locale ) + + # Restore the string encoding of the input if it was not UTF-8. + # Apply invalid/undef :replace as tidy_bytes does + transliterated.encode!(input_encoding, invalid: :replace, undef: :replace) if input_encoding != transliterated.encoding + + transliterated end # Replaces special characters in a string so that it may be used as part of @@ -91,7 +116,7 @@ module ActiveSupport # If the optional parameter +locale+ is specified, # the word will be parameterized as a word of that language. # By default, this parameter is set to <tt>nil</tt> and it will use - # the configured <tt>I18n.locale<tt>. + # the configured <tt>I18n.locale</tt>. def parameterize(string, separator: "-", preserve_case: false, locale: nil) # Replace accented chars with their ASCII equivalents. parameterized_string = transliterate(string, locale: locale) diff --git a/activesupport/lib/active_support/logger.rb b/activesupport/lib/active_support/logger.rb index b8555c887b..2576bee4ff 100644 --- a/activesupport/lib/active_support/logger.rb +++ b/activesupport/lib/active_support/logger.rb @@ -14,7 +14,7 @@ module ActiveSupport # ActiveSupport::Logger.logger_outputs_to?(logger, STDOUT) # # => true def self.logger_outputs_to?(logger, *sources) - logdev = logger.instance_variable_get("@logdev") + logdev = logger.instance_variable_get(:@logdev) logger_source = logdev.dev if logdev.respond_to?(:dev) sources.any? { |source| source == logger_source } end diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index 2ba3936cae..868cb70423 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -48,7 +48,7 @@ module ActiveSupport #:nodoc: alias to_s wrapped_string alias to_str wrapped_string - delegate :<=>, :=~, :acts_like_string?, to: :wrapped_string + delegate :<=>, :=~, :match?, :acts_like_string?, to: :wrapped_string # Creates a new Chars instance by wrapping _string_. def initialize(string) diff --git a/activesupport/lib/active_support/option_merger.rb b/activesupport/lib/active_support/option_merger.rb index ab9ca727f6..8e54b11be6 100644 --- a/activesupport/lib/active_support/option_merger.rb +++ b/activesupport/lib/active_support/option_merger.rb @@ -5,7 +5,7 @@ require "active_support/core_ext/hash/deep_merge" module ActiveSupport class OptionMerger #:nodoc: instance_methods.each do |method| - undef_method(method) if method !~ /^(__|instance_eval|class|object_id)/ + undef_method(method) unless method.match?(/^(__|instance_eval|class|object_id)/) end def initialize(context, options) diff --git a/activesupport/lib/active_support/parameter_filter.rb b/activesupport/lib/active_support/parameter_filter.rb index e1cd7c46c1..f4c4f2d2fb 100644 --- a/activesupport/lib/active_support/parameter_filter.rb +++ b/activesupport/lib/active_support/parameter_filter.rb @@ -22,7 +22,7 @@ module ActiveSupport # change { file: { code: "xxxx"} } # # ActiveSupport::ParameterFilter.new([-> (k, v) do - # v.reverse! if k =~ /secret/i + # v.reverse! if /secret/i.match?(k) # end]) # => reverses the value to all keys matching /secret/i class ParameterFilter diff --git a/activesupport/lib/active_support/per_thread_registry.rb b/activesupport/lib/active_support/per_thread_registry.rb index eb92fb4371..bf5a3b97ae 100644 --- a/activesupport/lib/active_support/per_thread_registry.rb +++ b/activesupport/lib/active_support/per_thread_registry.rb @@ -40,7 +40,7 @@ module ActiveSupport # If the class has an initializer, it must accept no arguments. module PerThreadRegistry def self.extended(object) - object.instance_variable_set "@per_thread_registry_key", object.name.freeze + object.instance_variable_set :@per_thread_registry_key, object.name.freeze end def instance diff --git a/activesupport/lib/active_support/secure_compare_rotator.rb b/activesupport/lib/active_support/secure_compare_rotator.rb index 14a0aee947..269703c34a 100644 --- a/activesupport/lib/active_support/secure_compare_rotator.rb +++ b/activesupport/lib/active_support/secure_compare_rotator.rb @@ -37,14 +37,13 @@ module ActiveSupport @value = value end - def secure_compare!(other_value, on_rotation: @rotation) + def secure_compare!(other_value, on_rotation: @on_rotation) secure_compare(@value, other_value) || run_rotations(on_rotation) { |wrapper| wrapper.secure_compare!(other_value) } || raise(InvalidMatch) end private - def build_rotation(previous_value, _options) self.class.new(previous_value) end diff --git a/activesupport/lib/active_support/tagged_logging.rb b/activesupport/lib/active_support/tagged_logging.rb index d8a86d997e..75a7ca24c2 100644 --- a/activesupport/lib/active_support/tagged_logging.rb +++ b/activesupport/lib/active_support/tagged_logging.rb @@ -32,15 +32,18 @@ module ActiveSupport def push_tags(*tags) tags.flatten.reject(&:blank?).tap do |new_tags| + @tags_text = nil current_tags.concat new_tags end end def pop_tags(size = 1) + @tags_text = nil current_tags.pop size end def clear_tags! + @tags_text = nil current_tags.clear end @@ -51,11 +54,13 @@ module ActiveSupport end def tags_text - tags = current_tags - if tags.one? - "[#{tags[0]}] " - elsif tags.any? - tags.collect { |tag| "[#{tag}] " }.join + @tags_text ||= begin + tags = current_tags + if tags.one? + "[#{tags[0]}] " + elsif tags.any? + tags.collect { |tag| "[#{tag}] " }.join + end end end end diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index d9e033e23b..90830b89bd 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -334,6 +334,13 @@ module ActiveSupport re === name || re === MAPPING[name] end + # Compare #name and TZInfo identifier to a supplied regexp, returning +true+ + # if a match is found. + def match?(re) + (re == name) || (re == MAPPING[name]) || + ((Regexp === re) && (re.match?(name) || re.match?(MAPPING[name]))) + end + # Returns a textual representation of this time zone. def to_s "(GMT#{formatted_offset}) #{name}" diff --git a/activesupport/test/cache/stores/mem_cache_store_test.rb b/activesupport/test/cache/stores/mem_cache_store_test.rb index 3917d14d1c..0a5e20ed46 100644 --- a/activesupport/test/cache/stores/mem_cache_store_test.rb +++ b/activesupport/test/cache/stores/mem_cache_store_test.rb @@ -9,7 +9,7 @@ require "dalli" # connection pool testing. class SlowDalliClient < Dalli::Client def get(key, options = {}) - if key =~ /latency/ + if /latency/.match?(key) sleep 3 else super diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index a2177e0476..0ea37b15d5 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -15,7 +15,7 @@ Redis::Connection.drivers.append(driver) # connection pool testing. class SlowRedis < Redis def get(key, options = {}) - if key =~ /latency/ + if /latency/.match?(key) sleep 3 else super diff --git a/activesupport/test/callbacks_test.rb b/activesupport/test/callbacks_test.rb index cc37c4fa99..221a8a5731 100644 --- a/activesupport/test/callbacks_test.rb +++ b/activesupport/test/callbacks_test.rb @@ -256,7 +256,7 @@ module CallbacksTest end def respond_to_missing?(sym) - sym =~ /^(log|wrap)_/ || super + sym.match?(/^(log|wrap)_/) || super end end diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb index c5a000b67a..af8f9c9b09 100644 --- a/activesupport/test/core_ext/string_ext_test.rb +++ b/activesupport/test/core_ext/string_ext_test.rb @@ -455,6 +455,8 @@ class StringAccessTest < ActiveSupport::TestCase test "#to with negative Integer, position is counted from the end" do assert_equal "hell", "hello".to(-2) + assert_equal "h", "hello".to(-5) + assert_equal "", "hello".to(-7) end test "#from and #to can be combined" do @@ -480,12 +482,16 @@ class StringAccessTest < ActiveSupport::TestCase assert_not_same different_string, string end - test "#first with negative Integer is deprecated" do - string = "hello" - message = "Calling String#first with a negative integer limit " \ - "will raise an ArgumentError in Rails 6.1." - assert_deprecated(message) do - string.first(-1) + test "#first with Integer returns a non-frozen string" do + string = "he" + (0..string.length + 1).each do |limit| + assert_not string.first(limit).frozen? + end + end + + test "#first with negative Integer raises ArgumentError" do + assert_raise ArgumentError do + "hello".first(-1) end end @@ -507,12 +513,16 @@ class StringAccessTest < ActiveSupport::TestCase assert_not_same different_string, string end - test "#last with negative Integer is deprecated" do - string = "hello" - message = "Calling String#last with a negative integer limit " \ - "will raise an ArgumentError in Rails 6.1." - assert_deprecated(message) do - string.last(-1) + test "#last with Integer returns a non-frozen string" do + string = "he" + (0..string.length + 1).each do |limit| + assert_not string.last(limit).frozen? + end + end + + test "#last with negative Integer raises ArgumentError" do + assert_raise ArgumentError do + "hello".last(-1) end end diff --git a/activesupport/test/message_encryptor_test.rb b/activesupport/test/message_encryptor_test.rb index 097aa8b5f8..f807497709 100644 --- a/activesupport/test/message_encryptor_test.rb +++ b/activesupport/test/message_encryptor_test.rb @@ -174,7 +174,7 @@ class MessageEncryptorTest < ActiveSupport::TestCase def test_on_rotation_can_be_passed_at_the_constructor_level older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign(encoded: "message") - rotated = false + rotated = rotated = false # double assigning to suppress "assigned but unused variable" warning encryptor = ActiveSupport::MessageEncryptor.new(@secret, on_rotation: proc { rotated = true }) encryptor.rotate secrets[:older], "older sign" @@ -188,7 +188,7 @@ class MessageEncryptorTest < ActiveSupport::TestCase def test_on_rotation_option_takes_precedence_over_the_one_given_in_constructor older_message = ActiveSupport::MessageEncryptor.new(secrets[:older], "older sign").encrypt_and_sign(encoded: "message") - rotated = false + rotated = rotated = false # double assigning to suppress "assigned but unused variable" warning encryptor = ActiveSupport::MessageEncryptor.new(@secret, on_rotation: proc { rotated = true }) encryptor.rotate secrets[:older], "older sign" diff --git a/activesupport/test/multibyte_chars_test.rb b/activesupport/test/multibyte_chars_test.rb index e0e0d9afc0..b0027047df 100644 --- a/activesupport/test/multibyte_chars_test.rb +++ b/activesupport/test/multibyte_chars_test.rb @@ -204,6 +204,12 @@ class MultibyteCharsUTF8BehaviourTest < ActiveSupport::TestCase assert_equal 3, (@chars =~ /わ/u) end + def test_match_should_return_boolean_for_regexp_match + assert_not @chars.match?(/wrong/u) + assert @chars.match?(/こに/u) + assert @chars.match?(/ち/u) + end + def test_should_use_character_offsets_for_insert_offsets assert_equal "", (+"").mb_chars.insert(0, "") assert_equal "こわにちわ", @chars.insert(1, "わ") diff --git a/activesupport/test/parameter_filter_test.rb b/activesupport/test/parameter_filter_test.rb index e680a22479..510921cf95 100644 --- a/activesupport/test/parameter_filter_test.rb +++ b/activesupport/test/parameter_filter_test.rb @@ -22,7 +22,7 @@ class ParameterFilterTest < ActiveSupport::TestCase filter_words << "blah" filter_words << lambda { |key, value| - value.reverse! if key =~ /bargain/ + value.reverse! if /bargain/.match?(key) } filter_words << lambda { |key, value, original_params| value.replace("world!") if original_params["barg"]["blah"] == "bar" && key == "hello" @@ -61,7 +61,7 @@ class ParameterFilterTest < ActiveSupport::TestCase filter_words << "blah" filter_words << lambda { |key, value| - value.reverse! if key =~ /bargain/ + value.reverse! if /bargain/.match?(key) } filter_words << lambda { |key, value, original_params| value.replace("world!") if original_params["barg"]["blah"] == "bar" && key == "hello" diff --git a/activesupport/test/secure_compare_rotator_test.rb b/activesupport/test/secure_compare_rotator_test.rb index 8acf13e38f..d80faea128 100644 --- a/activesupport/test/secure_compare_rotator_test.rb +++ b/activesupport/test/secure_compare_rotator_test.rb @@ -41,4 +41,17 @@ class SecureCompareRotatorTest < ActiveSupport::TestCase assert_equal(true, wrapper.secure_compare!("and_another_one", on_rotation: -> { @witness = true })) end end + + test "#secure_compare! calls the on_rotation proc that given in constructor" do + @witness = nil + + wrapper = ActiveSupport::SecureCompareRotator.new("old_secret", on_rotation: -> { @witness = true }) + wrapper.rotate("new_secret") + wrapper.rotate("another_secret") + wrapper.rotate("and_another_one") + + assert_changes(:@witness, from: nil, to: true) do + assert_equal(true, wrapper.secure_compare!("and_another_one")) + end + end end diff --git a/activesupport/test/time_zone_test.rb b/activesupport/test/time_zone_test.rb index f948c363df..6ca3afd561 100644 --- a/activesupport/test/time_zone_test.rb +++ b/activesupport/test/time_zone_test.rb @@ -717,6 +717,13 @@ class TimeZoneTest < ActiveSupport::TestCase assert zone !~ /Nonexistent_Place/ end + def test_zone_match? + zone = ActiveSupport::TimeZone["Eastern Time (US & Canada)"] + assert zone.match?(/Eastern/) + assert zone.match?(/New_York/) + assert_not zone.match?(/Nonexistent_Place/) + end + def test_to_s assert_equal "(GMT+05:30) New Delhi", ActiveSupport::TimeZone["New Delhi"].to_s end diff --git a/activesupport/test/transliterate_test.rb b/activesupport/test/transliterate_test.rb index 9e29a93ea0..2e02b5e938 100644 --- a/activesupport/test/transliterate_test.rb +++ b/activesupport/test/transliterate_test.rb @@ -57,4 +57,53 @@ class TransliterateTest < ActiveSupport::TestCase end assert_equal "Can only transliterate strings. Received Object", exception.message end + + def test_transliterate_handles_strings_with_valid_utf8_encodings + string = String.new("A", encoding: Encoding::UTF_8) + assert_equal "A", ActiveSupport::Inflector.transliterate(string) + end + + def test_transliterate_handles_strings_with_valid_us_ascii_encodings + string = String.new("A", encoding: Encoding::US_ASCII) + transcoded = ActiveSupport::Inflector.transliterate(string) + assert_equal "A", transcoded + assert_equal Encoding::US_ASCII, transcoded.encoding + end + + def test_transliterate_handles_strings_with_valid_gb18030_encodings + string = String.new("A", encoding: Encoding::GB18030) + transcoded = ActiveSupport::Inflector.transliterate(string) + assert_equal "A", transcoded + assert_equal Encoding::GB18030, transcoded.encoding + end + + def test_transliterate_handles_strings_with_incompatible_encodings + incompatible_encodings = Encoding.list - [ + Encoding::UTF_8, + Encoding::US_ASCII, + Encoding::GB18030 + ] + incompatible_encodings.each do |encoding| + string = String.new("", encoding: encoding) + exception = assert_raises ArgumentError do + ActiveSupport::Inflector.transliterate(string) + end + assert_equal "Can not transliterate strings with #{encoding} encoding", exception.message + end + end + + def test_transliterate_handles_strings_with_invalid_utf8_bytes + string = String.new("\255", encoding: Encoding::UTF_8) + assert_equal "?", ActiveSupport::Inflector.transliterate(string) + end + + def test_transliterate_handles_strings_with_invalid_us_ascii_bytes + string = String.new("\255", encoding: Encoding::US_ASCII) + assert_equal "?", ActiveSupport::Inflector.transliterate(string) + end + + def test_transliterate_handles_strings_with_invalid_gb18030_bytes + string = String.new("\255", encoding: Encoding::GB18030) + assert_equal "?", ActiveSupport::Inflector.transliterate(string) + end end diff --git a/guides/rails_guides/kindle.rb b/guides/rails_guides/kindle.rb index 8a0361ff4c..fc8e6b97e6 100644 --- a/guides/rails_guides/kindle.rb +++ b/guides/rails_guides/kindle.rb @@ -36,7 +36,7 @@ module Kindle frontmatter = [] html_pages.delete_if { |x| if /(toc|welcome|copyright).html/.match?(x) - frontmatter << x unless x =~ /toc/ + frontmatter << x unless /toc/.match?(x) true end } diff --git a/guides/source/action_controller_overview.md b/guides/source/action_controller_overview.md index f8367283fc..a5d097637e 100644 --- a/guides/source/action_controller_overview.md +++ b/guides/source/action_controller_overview.md @@ -34,7 +34,7 @@ Controller Naming Convention The naming convention of controllers in Rails favors pluralization of the last word in the controller's name, although it is not strictly required (e.g. `ApplicationController`). For example, `ClientsController` is preferable to `ClientController`, `SiteAdminsController` is preferable to `SiteAdminController` or `SitesAdminsController`, and so on. -Following this convention will allow you to use the default route generators (e.g. `resources`, etc) without needing to qualify each `:path` or `:controller`, and will keep URL and path helpers' usage consistent throughout your application. See [Layouts & Rendering Guide](layouts_and_rendering.html) for more details. +Following this convention will allow you to use the default route generators (e.g. `resources`, etc) without needing to qualify each `:path` or `:controller`, and will keep named route helpers' usage consistent throughout your application. See [Layouts & Rendering Guide](layouts_and_rendering.html) for more details. NOTE: The controller naming convention differs from the naming convention of models, which are expected to be named in singular form. diff --git a/guides/source/action_mailer_basics.md b/guides/source/action_mailer_basics.md index f600cf29ce..9f4a567f96 100644 --- a/guides/source/action_mailer_basics.md +++ b/guides/source/action_mailer_basics.md @@ -743,7 +743,7 @@ files (environment.rb, production.rb, etc...) | Configuration | Description | |---------------|-------------| |`logger`|Generates information on the mailing run if available. Can be set to `nil` for no logging. Compatible with both Ruby's own `Logger` and `Log4r` loggers.| -|`smtp_settings`|Allows detailed configuration for `:smtp` delivery method:<ul><li>`:address` - Allows you to use a remote mail server. Just change it from its default `"localhost"` setting.</li><li>`:port` - On the off chance that your mail server doesn't run on port 25, you can change it.</li><li>`:domain` - If you need to specify a HELO domain, you can do it here.</li><li>`:user_name` - If your mail server requires authentication, set the username in this setting.</li><li>`:password` - If your mail server requires authentication, set the password in this setting.</li><li>`:authentication` - If your mail server requires authentication, you need to specify the authentication type here. This is a symbol and one of `:plain` (will send the password in the clear), `:login` (will send password Base64 encoded) or `:cram_md5` (combines a Challenge/Response mechanism to exchange information and a cryptographic Message Digest 5 algorithm to hash important information)</li><li>`:enable_starttls_auto` - Detects if STARTTLS is enabled in your SMTP server and starts to use it. Defaults to `true`.</li><li>`:openssl_verify_mode` - When using TLS, you can set how OpenSSL checks the certificate. This is really useful if you need to validate a self-signed and/or a wildcard certificate. You can use the name of an OpenSSL verify constant ('none' or 'peer') or directly the constant (`OpenSSL::SSL::VERIFY_NONE` or `OpenSSL::SSL::VERIFY_PEER`).</li></ul>| +|`smtp_settings`|Allows detailed configuration for `:smtp` delivery method:<ul><li>`:address` - Allows you to use a remote mail server. Just change it from its default `"localhost"` setting.</li><li>`:port` - On the off chance that your mail server doesn't run on port 25, you can change it.</li><li>`:domain` - If you need to specify a HELO domain, you can do it here.</li><li>`:user_name` - If your mail server requires authentication, set the username in this setting.</li><li>`:password` - If your mail server requires authentication, set the password in this setting.</li><li>`:authentication` - If your mail server requires authentication, you need to specify the authentication type here. This is a symbol and one of `:plain` (will send the password in the clear), `:login` (will send password Base64 encoded) or `:cram_md5` (combines a Challenge/Response mechanism to exchange information and a cryptographic Message Digest 5 algorithm to hash important information)</li><li>`:enable_starttls_auto` - Detects if STARTTLS is enabled in your SMTP server and starts to use it. Defaults to `true`.</li><li>`:openssl_verify_mode` - When using TLS, you can set how OpenSSL checks the certificate. This is really useful if you need to validate a self-signed and/or a wildcard certificate. You can use the name of an OpenSSL verify constant ('none' or 'peer') or directly the constant (`OpenSSL::SSL::VERIFY_NONE` or `OpenSSL::SSL::VERIFY_PEER`).</li><li>`:ssl/:tls` - Enables the SMTP connection to use SMTP/TLS (SMTPS: SMTP over direct TLS connection)</li></ul>| |`sendmail_settings`|Allows you to override options for the `:sendmail` delivery method.<ul><li>`:location` - The location of the sendmail executable. Defaults to `/usr/sbin/sendmail`.</li><li>`:arguments` - The command line arguments to be passed to sendmail. Defaults to `-i`.</li></ul>| |`raise_delivery_errors`|Whether or not errors should be raised if the email fails to be delivered. This only works if the external email server is configured for immediate delivery.| |`delivery_method`|Defines a delivery method. Possible values are:<ul><li>`:smtp` (default), can be configured by using `config.action_mailer.smtp_settings`.</li><li>`:sendmail`, can be configured by using `config.action_mailer.sendmail_settings`.</li><li>`:file`: save emails to files; can be configured by using `config.action_mailer.file_settings`.</li><li>`:test`: save emails to `ActionMailer::Base.deliveries` array.</li></ul>See [API docs](https://api.rubyonrails.org/classes/ActionMailer/Base.html) for more info.| diff --git a/guides/source/autoloading_and_reloading_constants.md b/guides/source/autoloading_and_reloading_constants.md index 8cd2d353de..212cbfaf43 100644 --- a/guides/source/autoloading_and_reloading_constants.md +++ b/guides/source/autoloading_and_reloading_constants.md @@ -90,7 +90,7 @@ INFO. Autoload paths are called _root directories_ in Zeitwerk documentation, bu Within an autoload path, file names must match the constants they define as documented [here](https://github.com/fxn/zeitwerk#file-structure). -By default, the autoload paths of an application consist of all the subdirectories of `app` that exist when the application boots ---except for `aasets`, `javascripts`, `views`,--- plus the autoload paths of engines it might depend on. +By default, the autoload paths of an application consist of all the subdirectories of `app` that exist when the application boots ---except for `assets`, `javascripts`, `views`,--- plus the autoload paths of engines it might depend on. For example, if `UsersHelper` is implemented in `app/helpers/users_helper.rb`, the module is autoloadable, you do not need (and should not write) a `require` call for it: diff --git a/guides/source/command_line.md b/guides/source/command_line.md index 60d0de17bc..4acc25bdc2 100644 --- a/guides/source/command_line.md +++ b/guides/source/command_line.md @@ -368,7 +368,7 @@ irb(main):001:0> Inside the `rails console` you have access to the `app` and `helper` instances. -With the `app` method you can access URL and path helpers, as well as do requests. +With the `app` method you can access named route helpers, as well as do requests. ```bash >> app.root_path diff --git a/guides/source/engines.md b/guides/source/engines.md index 8961a079b5..b3ac243af9 100644 --- a/guides/source/engines.md +++ b/guides/source/engines.md @@ -264,7 +264,7 @@ contains a file called `application_helper.rb`. This file will provide any common functionality for the helpers of the engine. The `blorgh` directory is where the other helpers for the engine will go. By placing them within this namespaced directory, you prevent them from possibly clashing with -identically-named helpers within other engines or even within the +identically-named route helpers within other engines or even within the application. Within the `app/jobs` directory there is a `blorgh` directory that diff --git a/guides/source/routing.md b/guides/source/routing.md index 4aeb9ee585..161984c993 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -210,7 +210,7 @@ end This will create a number of routes for each of the `articles` and `comments` controller. For `Admin::ArticlesController`, Rails will create: -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | ------------------------ | ---------------------- | ---------------------------- | | GET | /admin/articles | admin/articles#index | admin_articles_path | | GET | /admin/articles/new | admin/articles#new | new_admin_article_path | @@ -250,7 +250,7 @@ resources :articles, path: '/admin/articles' In each of these cases, the named routes remain the same as if you did not use `scope`. In the last case, the following paths map to `ArticlesController`: -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | ------------------------ | -------------------- | ---------------------- | | GET | /admin/articles | articles#index | articles_path | | GET | /admin/articles/new | articles#new | new_article_path | @@ -373,7 +373,7 @@ end The comments resource here will have the following routes generated for it: -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | -------------------------------------------- | ----------------- | ------------------------ | | GET | /articles/:article_id/comments(.:format) | comments#index | article_comments_path | | POST | /articles/:article_id/comments(.:format) | comments#create | article_comments_path | @@ -383,7 +383,7 @@ The comments resource here will have the following routes generated for it: | PATCH/PUT | /sekret/comments/:id(.:format) | comments#update | comment_path | | DELETE | /sekret/comments/:id(.:format) | comments#destroy | comment_path | -The `:shallow_prefix` option adds the specified parameter to the named helpers: +The `:shallow_prefix` option adds the specified parameter to the named route helpers: ```ruby scope shallow_prefix: "sekret" do @@ -395,7 +395,7 @@ end The comments resource here will have the following routes generated for it: -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | -------------------------------------------- | ----------------- | --------------------------- | | GET | /articles/:article_id/comments(.:format) | comments#index | article_comments_path | | POST | /articles/:article_id/comments(.:format) | comments#create | article_comments_path | @@ -638,7 +638,7 @@ You can specify a name for any route using the `:as` option: get 'exit', to: 'sessions#destroy', as: :logout ``` -This will create `logout_path` and `logout_url` as named helpers in your application. Calling `logout_path` will return `/exit` +This will create `logout_path` and `logout_url` as named route helpers in your application. Calling `logout_path` will return `/exit` You can also use this to override routing methods defined by resources, like this: @@ -934,7 +934,7 @@ resources :photos, controller: 'images' will recognize incoming paths beginning with `/photos` but route to the `Images` controller: -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | ---------------- | ----------------- | -------------------- | | GET | /photos | images#index | photos_path | | GET | /photos/new | images#new | new_photo_path | @@ -982,7 +982,7 @@ NOTE: Of course, you can use the more advanced constraints available in non-reso TIP: By default the `:id` parameter doesn't accept dots - this is because the dot is used as a separator for formatted routes. If you need to use a dot within an `:id` add a constraint which overrides this - for example `id: /[^\/]+/` allows anything except a slash. -### Overriding the Named Helpers +### Overriding the Named Route Helpers The `:as` option lets you override the normal naming for the named route helpers. For example: @@ -992,7 +992,7 @@ resources :photos, as: 'images' will recognize incoming paths beginning with `/photos` and route the requests to `PhotosController`, but use the value of the `:as` option to name the helpers. -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | ---------------- | ----------------- | -------------------- | | GET | /photos | photos#index | images_path | | GET | /photos/new | photos#new | new_image_path | @@ -1097,7 +1097,7 @@ end Rails now creates routes to the `CategoriesController`. -| HTTP Verb | Path | Controller#Action | Named Helper | +| HTTP Verb | Path | Controller#Action | Named Route Helper | | --------- | -------------------------- | ------------------ | ----------------------- | | GET | /kategorien | categories#index | categories_path | | GET | /kategorien/neu | categories#new | new_category_path | diff --git a/guides/source/working_with_javascript_in_rails.md b/guides/source/working_with_javascript_in_rails.md index 8cf8efefd0..28af1cd88d 100644 --- a/guides/source/working_with_javascript_in_rails.md +++ b/guides/source/working_with_javascript_in_rails.md @@ -14,6 +14,7 @@ After reading this guide, you will know: * How Rails' built-in helpers assist you. * How to handle Ajax on the server side. * The Turbolinks gem. +* How to include your Cross-Site Request Forgery token in request headers ------------------------------------------------------------------------------- @@ -524,6 +525,23 @@ For more details, including other events you can bind to, check out [the Turbolinks README](https://github.com/turbolinks/turbolinks/blob/master/README.md). +Cross-Site Request Forgery (CSRF) token in Ajax +---- + +When using another library to make Ajax calls, it is necessary to add +the security token as a default header for Ajax calls in your library. To get +the token: + +```javascript +var token = document.getElementsByName('csrf-token')[0].content +``` + +You can then submit this token as a `X-CSRF-Token` header for your +Ajax request. You do not need to add a CSRF token for GET requests, +only non-GET ones. + +You can read more about about Cross-Site Request Forgery in [Security](https://guides.rubyonrails.org/security.html#cross-site-request-forgery-csrf) + Other Resources --------------- diff --git a/railties/lib/rails/code_statistics.rb b/railties/lib/rails/code_statistics.rb index 09082282f3..aa5c0d0b5b 100644 --- a/railties/lib/rails/code_statistics.rb +++ b/railties/lib/rails/code_statistics.rb @@ -44,7 +44,7 @@ class CodeStatistics #:nodoc: Dir.foreach(directory) do |file_name| path = "#{directory}/#{file_name}" - if File.directory?(path) && (/^\./ !~ file_name) + if File.directory?(path) && !(/^\./.match?(file_name)) stats.add(calculate_directory_statistics(path, pattern)) elsif file_name&.match?(pattern) stats.add_by_file_path(path) diff --git a/railties/lib/rails/code_statistics_calculator.rb b/railties/lib/rails/code_statistics_calculator.rb index 85f86bdbd0..8dd415d9d1 100644 --- a/railties/lib/rails/code_statistics_calculator.rb +++ b/railties/lib/rails/code_statistics_calculator.rb @@ -58,20 +58,20 @@ class CodeStatisticsCalculator #:nodoc: @lines += 1 if comment_started - if patterns[:end_block_comment] && line =~ patterns[:end_block_comment] + if patterns[:end_block_comment] && patterns[:end_block_comment].match?(line) comment_started = false end next else - if patterns[:begin_block_comment] && line =~ patterns[:begin_block_comment] + if patterns[:begin_block_comment] && patterns[:begin_block_comment].match?(line) comment_started = true next end end - @classes += 1 if patterns[:class] && line =~ patterns[:class] - @methods += 1 if patterns[:method] && line =~ patterns[:method] - if line !~ /^\s*$/ && (patterns[:line_comment].nil? || line !~ patterns[:line_comment]) + @classes += 1 if patterns[:class] && patterns[:class].match?(line) + @methods += 1 if patterns[:method] && patterns[:method].match?(line) + if !line.match?(/^\s*$/) && (patterns[:line_comment].nil? || !line.match?(patterns[:line_comment])) @code_lines += 1 end end diff --git a/railties/lib/rails/command/base.rb b/railties/lib/rails/command/base.rb index a22b198c66..415bab199f 100644 --- a/railties/lib/rails/command/base.rb +++ b/railties/lib/rails/command/base.rb @@ -52,7 +52,7 @@ module Rails def inherited(base) #:nodoc: super - if base.name && base.name !~ /Base$/ + if base.name && !base.name.match?(/Base$/) Rails::Command.subclasses << base end end diff --git a/railties/lib/rails/command/behavior.rb b/railties/lib/rails/command/behavior.rb index 7fb2a99e67..90650059f4 100644 --- a/railties/lib/rails/command/behavior.rb +++ b/railties/lib/rails/command/behavior.rb @@ -44,7 +44,7 @@ module Rails require path return rescue LoadError => e - raise unless e.message =~ /#{Regexp.escape(path)}$/ + raise unless /#{Regexp.escape(path)}$/.match?(e.message) rescue Exception => e warn "[WARNING] Could not load #{command_type} #{path.inspect}. Error: #{e.message}.\n#{e.backtrace.join("\n")}" end diff --git a/railties/lib/rails/command/environment_argument.rb b/railties/lib/rails/command/environment_argument.rb index 9945fd1430..df3cc1b2bb 100644 --- a/railties/lib/rails/command/environment_argument.rb +++ b/railties/lib/rails/command/environment_argument.rb @@ -28,7 +28,7 @@ module Rails if available_environments.include? env env else - %w( production development test ).detect { |e| e =~ /^#{env}/ } || env + %w( production development test ).detect { |e| /^#{env}/.match?(e) } || env end end diff --git a/railties/lib/rails/command/helpers/pretty_credentials.rb b/railties/lib/rails/command/helpers/pretty_credentials.rb new file mode 100644 index 0000000000..873ed0e825 --- /dev/null +++ b/railties/lib/rails/command/helpers/pretty_credentials.rb @@ -0,0 +1,55 @@ +# frozen_string_literal: true + +require "fileutils" + +module Rails + module Command + module Helpers + module PrettyCredentials + Error = Class.new(StandardError) + + def opt_in_pretty_credentials + unless already_answered? || already_opted_in? + answer = yes?("Would you like to make the credentials diff from git more readable in the future? [Y/n]") + end + + opt_in! if answer + FileUtils.touch(tracker) unless answer.nil? + rescue Error + say("Couldn't setup git to prettify the credentials diff") + end + + private + def already_answered? + tracker.exist? + end + + def already_opted_in? + system_call("git config --get 'diff.rails_credentials.textconv'", accepted_codes: [0, 1]) + end + + def opt_in! + system_call("git config diff.rails_credentials.textconv 'bin/rails credentials:show'", accepted_codes: [0]) + + git_attributes = Rails.root.join(".gitattributes") + File.open(git_attributes, "a+") do |file| + file.write(<<~EOM) + config/credentials/*.yml.enc diff=rails_credentials + config/credentials.yml.enc diff=rails_credentials + EOM + end + end + + def tracker + Rails.root.join("tmp", "rails_pretty_credentials") + end + + def system_call(command_line, accepted_codes:) + result = system(command_line) + raise(Error) if accepted_codes.exclude?($?.exitstatus) + result + end + end + end + end +end diff --git a/railties/lib/rails/commands/credentials/credentials_command.rb b/railties/lib/rails/commands/credentials/credentials_command.rb index e23a1b3008..772e105007 100644 --- a/railties/lib/rails/commands/credentials/credentials_command.rb +++ b/railties/lib/rails/commands/credentials/credentials_command.rb @@ -2,12 +2,15 @@ require "active_support" require "rails/command/helpers/editor" +require "rails/command/helpers/pretty_credentials" require "rails/command/environment_argument" +require "pathname" module Rails module Command class CredentialsCommand < Rails::Command::Base # :nodoc: include Helpers::Editor + include Helpers::PrettyCredentials include EnvironmentArgument self.environment_desc = "Uses credentials from config/credentials/:environment.yml.enc encrypted by config/credentials/:environment.key key" @@ -34,20 +37,29 @@ module Rails end say "File encrypted and saved." + opt_in_pretty_credentials rescue ActiveSupport::MessageEncryptor::InvalidMessage say "Couldn't decrypt #{content_path}. Perhaps you passed the wrong key?" end - def show - extract_environment_option_from_argument(default_environment: nil) + def show(git_textconv_path = nil) + if git_textconv_path + default_environment = extract_environment_from_path(git_textconv_path) + fallback_message = File.read(git_textconv_path) + end + + extract_environment_option_from_argument(default_environment: default_environment) require_application! - say credentials.read.presence || missing_credentials_message + say credentials(git_textconv_path).read.presence || fallback_message || missing_credentials_message + rescue => e + raise(e) unless git_textconv_path + fallback_message end private - def credentials - Rails.application.encrypted(content_path, key_path: key_path) + def credentials(content = nil) + Rails.application.encrypted(content || content_path, key_path: key_path) end def ensure_encryption_key_has_been_added @@ -77,7 +89,6 @@ module Rails end end - def content_path options[:environment] ? "config/credentials/#{options[:environment]}.yml.enc" : "config/credentials.yml.enc" end @@ -86,6 +97,17 @@ module Rails options[:environment] ? "config/credentials/#{options[:environment]}.key" : "config/master.key" end + def extract_environment_from_path(path) + regex = %r{ + ([A-Za-z0-9]+) # match the environment + (?<!credentials) # don't match if file contains the word "credentials" + # in such case, the environment should be the default one + \.yml\.enc # look for `.yml.enc` file extension + }x + path.match(regex) + + Regexp.last_match(1) + end def encryption_key_file_generator require "rails/generators" diff --git a/railties/lib/rails/engine.rb b/railties/lib/rails/engine.rb index 46f1d38b96..b4c0028b1f 100644 --- a/railties/lib/rails/engine.rb +++ b/railties/lib/rails/engine.rb @@ -362,7 +362,7 @@ module Rails base.called_from = begin call_stack = caller_locations.map { |l| l.absolute_path || l.path } - File.dirname(call_stack.detect { |p| p !~ %r[railties[\w.-]*/lib/rails|rack[\w.-]*/lib/rack] }) + File.dirname(call_stack.detect { |p| !p.match?(%r[railties[\w.-]*/lib/rails|rack[\w.-]*/lib/rack]) }) end end diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index dbfb7337f0..ed0215bda9 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -400,7 +400,7 @@ module Rails end def os_supports_listen_out_of_the_box? - RbConfig::CONFIG["host_os"] =~ /darwin|linux/ + /darwin|linux/.match?(RbConfig::CONFIG["host_os"]) end def run_bundle diff --git a/railties/lib/rails/generators/base.rb b/railties/lib/rails/generators/base.rb index a153923ce3..1d3c947cb0 100644 --- a/railties/lib/rails/generators/base.rb +++ b/railties/lib/rails/generators/base.rb @@ -233,7 +233,7 @@ module Rails # Invoke source_root so the default_source_root is set. base.source_root - if base.name && base.name !~ /Base$/ + if base.name && !base.name.match?(/Base$/) Rails::Generators.subclasses << base Rails::Generators.templates_path.each do |path| diff --git a/railties/lib/rails/generators/generated_attribute.rb b/railties/lib/rails/generators/generated_attribute.rb index 4e348be9be..377a5dfc65 100644 --- a/railties/lib/rails/generators/generated_attribute.rb +++ b/railties/lib/rails/generators/generated_attribute.rb @@ -131,7 +131,7 @@ module Rails end def foreign_key? - !!(name =~ /_id$/) + /_id$/.match?(name) end def reference? diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt index cf5462f7dc..f13dab59b1 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile.tt +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile.tt @@ -72,7 +72,7 @@ group :test do # Easy installation and use of web drivers to run system tests with browsers gem 'webdrivers' end -<%- end -%> +<%- end -%> # Windows does not include zoneinfo files, so bundle the tzinfo-data gem gem 'tzinfo-data', platforms: [:mingw, :mswin, :x64_mingw, :jruby] diff --git a/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt b/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt index b8c1f21c0b..437bf84ce3 100644 --- a/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt +++ b/railties/lib/rails/generators/rails/app/templates/app/views/layouts/application.html.erb.tt @@ -2,6 +2,7 @@ <html> <head> <title><%= camelized %></title> + <meta name="viewport" content="width=device-width,initial-scale=1"> <%%= csrf_meta_tags %> <%%= csp_meta_tag %> diff --git a/railties/lib/rails/generators/rails/app/templates/config/initializers/backtrace_silencers.rb.tt b/railties/lib/rails/generators/rails/app/templates/config/initializers/backtrace_silencers.rb.tt index 59385cdf37..3c56b21b3c 100644 --- a/railties/lib/rails/generators/rails/app/templates/config/initializers/backtrace_silencers.rb.tt +++ b/railties/lib/rails/generators/rails/app/templates/config/initializers/backtrace_silencers.rb.tt @@ -1,7 +1,7 @@ # Be sure to restart your server when you modify this file. # You can add backtrace silencers for libraries that you're using but don't wish to see in your backtraces. -# Rails.backtrace_cleaner.add_silencer { |line| line =~ /my_noisy_library/ } +# Rails.backtrace_cleaner.add_silencer { |line| /my_noisy_library/.match?(line) } # You can also remove all the silencers if you're trying to debug a problem that might stem from framework code. # Rails.backtrace_cleaner.remove_silencers! diff --git a/railties/lib/rails/test_unit/runner.rb b/railties/lib/rails/test_unit/runner.rb index 7b294751fc..b8bce8c772 100644 --- a/railties/lib/rails/test_unit/runner.rb +++ b/railties/lib/rails/test_unit/runner.rb @@ -61,7 +61,7 @@ module Rails private def extract_filters(argv) # Extract absolute and relative paths but skip -n /.*/ regexp filters. - argv.select { |arg| arg =~ %r%^/?\w+/% && !arg.end_with?("/") }.map do |path| + argv.select { |arg| %r%^/?\w+/%.match?(arg) && !arg.end_with?("/") }.map do |path| case when /(:\d+)+$/.match?(path) file, *lines = path.split(":") diff --git a/railties/test/application/assets_test.rb b/railties/test/application/assets_test.rb index a7dd233f3d..651897d8f3 100644 --- a/railties/test/application/assets_test.rb +++ b/railties/test/application/assets_test.rb @@ -311,7 +311,7 @@ module ApplicationTests manifest = Dir["#{app_path}/public/assets/.sprockets-manifest-*.json"].first assets = ActiveSupport::JSON.decode(File.read(manifest)) - assert asset_path = assets["assets"].find { |(k, _)| k && k =~ /.png/ }[1] + assert asset_path = assets["assets"].find { |(k, _)| /.png/.match?(k) }[1] # Load app env app "development" diff --git a/railties/test/application/configuration_test.rb b/railties/test/application/configuration_test.rb index 38dda20bec..cca86bc13a 100644 --- a/railties/test/application/configuration_test.rb +++ b/railties/test/application/configuration_test.rb @@ -456,7 +456,7 @@ module ApplicationTests test "filter_parameters should be able to set via config.filter_parameters" do add_to_config <<-RUBY config.filter_parameters += [ :foo, 'bar', lambda { |key, value| - value = value.reverse if key =~ /baz/ + value = value.reverse if /baz/.match?(key) }] RUBY @@ -790,7 +790,7 @@ module ApplicationTests end get "/" - assert last_response.body =~ /csrf\-param/ + assert /csrf\-param/.match?(last_response.body) end test "default form builder specified as a string" do diff --git a/railties/test/application/rake_test.rb b/railties/test/application/rake_test.rb index fe56e3d076..e8456e8c19 100644 --- a/railties/test/application/rake_test.rb +++ b/railties/test/application/rake_test.rb @@ -162,7 +162,6 @@ module ApplicationTests rails "generate", "scaffold", "user", "username:string", "password:string" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") @@ -194,7 +193,6 @@ module ApplicationTests rails "generate", "scaffold", "LineItems", "product:references", "cart:belongs_to" with_rails_env("test") do rails("db:migrate") - rails("webpacker:compile") end output = rails("test") diff --git a/railties/test/application/zeitwerk_integration_test.rb b/railties/test/application/zeitwerk_integration_test.rb index ff8c06b479..d03fa3d0c6 100644 --- a/railties/test/application/zeitwerk_integration_test.rb +++ b/railties/test/application/zeitwerk_integration_test.rb @@ -150,6 +150,35 @@ class ZeitwerkIntegrationTest < ActiveSupport::TestCase assert_empty deps.autoloaded_constants end + [true, false].each do |add_aps_to_lp| + test "require_dependency looks autoload paths up (#{add_aps_to_lp})" do + add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}" + app_file "app/models/user.rb", "class User; end" + boot + + assert require_dependency("user") + end + + test "require_dependency handles absolute paths correctly (#{add_aps_to_lp})" do + add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}" + app_file "app/models/user.rb", "class User; end" + boot + + assert require_dependency("#{app_path}/app/models/user.rb") + end + + test "require_dependency supports arguments that repond to to_path (#{add_aps_to_lp})" do + add_to_config "config.add_autoload_paths_to_load_path = #{add_aps_to_lp}" + app_file "app/models/user.rb", "class User; end" + boot + + user = Object.new + def user.to_path; "user"; end + + assert require_dependency(user) + end + end + test "eager loading loads the application code" do $zeitwerk_integration_test_user = false $zeitwerk_integration_test_post = false diff --git a/railties/test/commands/credentials_test.rb b/railties/test/commands/credentials_test.rb index 0ee36081c0..3dec6fbe10 100644 --- a/railties/test/commands/credentials_test.rb +++ b/railties/test/commands/credentials_test.rb @@ -4,6 +4,8 @@ require "isolation/abstract_unit" require "env_helpers" require "rails/command" require "rails/commands/credentials/credentials_command" +require "fileutils" +require "tempfile" class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase include ActiveSupport::Testing::Isolation, EnvHelpers @@ -88,10 +90,107 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase assert_match(/secret_key_base/, output) end + test "edit ask the user to opt in to pretty credentials" do + assert_match(/Would you like to make the credentials diff from git/, run_edit_command) + end + + test "edit doesn't ask the user to opt in to pretty credentials when alreasy asked" do + app_file("tmp/rails_pretty_credentials", "") + + assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + end + + test "edit doesn't ask the user to opt in when user already opted in" do + content = <<~EOM + [diff "rails_credentials"] + textconv = bin/rails credentials:show + EOM + app_file(".git/config", content) + + assert_no_match(/Would you like to make the credentials diff from git/, run_edit_command) + end + + test "edit ask the user to opt in to pretty credentials, user accepts" do + file = Tempfile.open("credentials_test") + file.write("y") + file.rewind + + run_edit_command(stdin: file.path) + + git_attributes = app_path(".gitattributes") + expected = <<~EOM + config/credentials/*.yml.enc diff=rails_credentials + config/credentials.yml.enc diff=rails_credentials + EOM + assert(File.exist?(git_attributes)) + assert_equal(expected, File.read(git_attributes)) + Dir.chdir(app_path) do + assert_equal("bin/rails credentials:show\n", `git config --get 'diff.rails_credentials.textconv'`) + end + ensure + file.close! + end + + test "edit ask the user to opt in to pretty credentials, user refuses" do + file = Tempfile.open("credentials_test") + file.write("n") + file.rewind + + run_edit_command(stdin: file.path) + + git_attributes = app_path(".gitattributes") + assert_not(File.exist?(git_attributes)) + ensure + file.close! + end + test "show credentials" do assert_match(/access_key_id: 123/, run_show_command) end + test "show command when argument is provided (from git diff left file)" do + run_edit_command(environment: "development") + + assert_match(/access_key_id: 123/, run_show_command("config/credentials/development.yml.enc")) + end + + test "show command when argument is provided (from git diff right file)" do + run_edit_command(environment: "development") + + dir = Dir.mktmpdir + file_path = File.join(dir, "KnAM4a_development.yml.enc") + file_content = File.read(app_path("config", "credentials", "development.yml.enc")) + File.write(file_path, file_content) + + assert_match(/access_key_id: 123/, run_show_command(file_path)) + ensure + FileUtils.rm_rf(dir) + end + + test "show command when argument is provided (git diff) and filename is the master credentials" do + assert_match(/access_key_id: 123/, run_show_command("config/credentials.yml.enc")) + end + + test "show command when argument is provided (git diff) and master key is not available" do + remove_file "config/master.key" + + raw_content = File.read(app_path("config", "credentials.yml.enc")) + assert_match(raw_content, run_show_command("config/credentials.yml.enc")) + end + + test "show command when argument is provided (git diff) return the raw encrypted content in an error occurs" do + run_edit_command(environment: "development") + + dir = Dir.mktmpdir + file_path = File.join(dir, "20190807development.yml.enc") + file_content = File.read(app_path("config", "credentials", "development.yml.enc")) + File.write(file_path, file_content) + + assert_match(file_content, run_show_command(file_path)) + ensure + FileUtils.rm_rf(dir) + end + test "show command raises error when require_master_key is specified and key does not exist" do remove_file "config/master.key" add_to_config "config.require_master_key = true" @@ -128,8 +227,9 @@ class Rails::Command::CredentialsCommandTest < ActiveSupport::TestCase end end - def run_show_command(environment: nil, **options) + def run_show_command(path = nil, environment: nil, **options) args = environment ? ["--environment", environment] : [] + args.unshift(path) rails "credentials:show", args, **options end end diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 5b439fdcba..43461036f3 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -678,7 +678,7 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_inclusion_of_listen_related_configuration_by_default run_generator - if RbConfig::CONFIG["host_os"] =~ /darwin|linux/ + if /darwin|linux/.match?(RbConfig::CONFIG["host_os"]) assert_listen_related_configuration else assert_no_listen_related_configuration @@ -690,7 +690,7 @@ class AppGeneratorTest < Rails::Generators::TestCase Object.const_set(:RUBY_ENGINE, "MyRuby") run_generator - if RbConfig::CONFIG["host_os"] =~ /darwin|linux/ + if /darwin|linux/.match?(RbConfig::CONFIG["host_os"]) assert_listen_related_configuration else assert_no_listen_related_configuration @@ -708,7 +708,7 @@ class AppGeneratorTest < Rails::Generators::TestCase def test_evented_file_update_checker_config run_generator assert_file "config/environments/development.rb" do |content| - if RbConfig::CONFIG["host_os"] =~ /darwin|linux/ + if /darwin|linux/.match?(RbConfig::CONFIG["host_os"]) assert_match(/^\s*config\.file_watcher = ActiveSupport::EventedFileUpdateChecker/, content) else assert_match(/^\s*# config\.file_watcher = ActiveSupport::EventedFileUpdateChecker/, content) diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 0fe62df8ba..6077ba3ee7 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -301,7 +301,7 @@ module TestHelpers # stderr:: true to pass STDERR output straight to the "real" STDERR. # By default, the STDERR and STDOUT of the process will be # combined in the returned string. - def rails(*args, allow_failure: false, stderr: false) + def rails(*args, allow_failure: false, stderr: false, stdin: File::NULL) args = args.flatten fork = true @@ -328,7 +328,7 @@ module TestHelpers out_read.close err_read.close if err_read - $stdin.reopen(File::NULL, "r") + $stdin.reopen(stdin, "r") $stdout.reopen(out_write) $stderr.reopen(err_write) diff --git a/railties/test/railties/engine_test.rb b/railties/test/railties/engine_test.rb index 7f05b9d9cf..06836844f0 100644 --- a/railties/test/railties/engine_test.rb +++ b/railties/test/railties/engine_test.rb @@ -110,7 +110,7 @@ module RailtiesTest assert_no_match(/\d+_create_users/, output.join("\n")) - bukkits_migration_order = output.index(output.detect { |o| /NOTE: Migration \d+_create_sessions\.rb from bukkits has been skipped/ =~ o }) + bukkits_migration_order = output.index(output.detect { |o| /NOTE: Migration \d+_create_sessions\.rb from bukkits has been skipped/.match?(o) }) assert_not_nil bukkits_migration_order, "Expected migration to be skipped" end end |