diff options
98 files changed, 590 insertions, 496 deletions
diff --git a/.codeclimate.yml b/.codeclimate.yml index 7d4ec1c54f..7fd35d8241 100644 --- a/.codeclimate.yml +++ b/.codeclimate.yml @@ -23,7 +23,7 @@ checks: engines: rubocop: enabled: true - channel: rubocop-0-54 + channel: rubocop-0-58 ratings: paths: diff --git a/.rubocop.yml b/.rubocop.yml index d1e8f03f13..3e15d34dbd 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -1,5 +1,3 @@ -require: './ci/custom_cops/lib/custom_cops' - AllCops: TargetRubyVersion: 2.4 # RuboCop has a bunch of cops enabled by default. This setting tells RuboCop @@ -11,13 +9,20 @@ AllCops: - 'actionpack/lib/action_dispatch/journey/parser.rb' - 'railties/test/fixtures/tmp/**/*' -# Prefer assert_not_x over refute_x -CustomCops/RefuteNot: - Include: +Performance: + Exclude: - '**/test/**/*' +Rails: + Enabled: true + # Prefer assert_not over assert ! -CustomCops/AssertNot: +Rails/AssertNot: + Include: + - '**/test/**/*' + +# Prefer assert_not_x over refute_x +Rails/RefuteMethods: Include: - '**/test/**/*' @@ -188,3 +193,12 @@ Performance/FlatMap: Performance/RedundantMerge: Enabled: true + +Performance/StartWith: + Enabled: true + +Performance/EndWith: + Enabled: true + +Performance/RegexpMatch: + Enabled: true diff --git a/Gemfile.lock b/Gemfile.lock index 9c28735a0d..259221979e 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -280,6 +280,8 @@ GEM mini_magick (~> 4.0) ruby-vips (>= 2.0.10, < 3) io-like (0.3.0) + jaro_winkler (1.5.1) + jaro_winkler (1.5.1-java) jdbc-mysql (5.1.46) jdbc-postgres (42.1.4) jdbc-sqlite3 (3.20.1) @@ -347,13 +349,13 @@ GEM mini_portile2 (~> 2.3.0) os (0.9.6) parallel (1.12.1) - parser (2.5.1.0) + parser (2.5.1.2) ast (~> 2.4.0) path_expander (1.0.3) pg (1.0.0) pg (1.0.0-x64-mingw32) pg (1.0.0-x86-mingw32) - powerpack (0.1.1) + powerpack (0.1.2) psych (3.0.2) public_suffix (3.0.2) puma (3.11.4) @@ -400,9 +402,10 @@ GEM resque (~> 1.26) rufus-scheduler (~> 3.2) retriable (3.1.1) - rubocop (0.56.0) + rubocop (0.58.2) + jaro_winkler (~> 1.5.1) parallel (~> 1.10) - parser (>= 2.5) + parser (>= 2.5, != 2.5.1.1) powerpack (~> 0.1) rainbow (>= 2.2.2, < 4.0) ruby-progressbar (~> 1.7) @@ -485,7 +488,7 @@ GEM uber (0.1.0) uglifier (4.1.10) execjs (>= 0.3.0, < 3) - unicode-display_width (1.3.3) + unicode-display_width (1.4.0) useragent (0.16.10) vegas (0.1.11) rack (>= 1.0.0) @@ -4,7 +4,7 @@ Rails is a web-application framework that includes everything needed to create database-backed web applications according to the -[Model-View-Controller (MVC)](http://en.wikipedia.org/wiki/Model-view-controller) +[Model-View-Controller (MVC)](https://en.wikipedia.org/wiki/Model-view-controller) pattern. Understanding the MVC pattern is key to understanding Rails. MVC divides your @@ -79,7 +79,7 @@ and may also be used independently outside Rails. the following resources handy: * [Getting Started with Rails](https://guides.rubyonrails.org/getting_started.html) * [Ruby on Rails Guides](https://guides.rubyonrails.org) - * [The API Documentation](http://api.rubyonrails.org) + * [The API Documentation](https://api.rubyonrails.org) * [Ruby on Rails Tutorial](https://www.railstutorial.org/book) ## Contributing @@ -87,13 +87,13 @@ and may also be used independently outside Rails. [](https://www.codetriage.com/rails/rails) We encourage you to contribute to Ruby on Rails! Please check out the -[Contributing to Ruby on Rails guide](http://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html) for guidelines about how to proceed. [Join us!](http://contributors.rubyonrails.org) +[Contributing to Ruby on Rails guide](https://edgeguides.rubyonrails.org/contributing_to_ruby_on_rails.html) for guidelines about how to proceed. [Join us!](https://contributors.rubyonrails.org) Trying to report a possible security vulnerability in Rails? Please -check out our [security policy](http://rubyonrails.org/security/) for +check out our [security policy](https://rubyonrails.org/security/) for guidelines about how to proceed. -Everyone interacting in Rails and its sub-projects' codebases, issue trackers, chat rooms, and mailing lists is expected to follow the Rails [code of conduct](http://rubyonrails.org/conduct/). +Everyone interacting in Rails and its sub-projects' codebases, issue trackers, chat rooms, and mailing lists is expected to follow the Rails [code of conduct](https://rubyonrails.org/conduct/). ## Code Status diff --git a/actioncable/Rakefile b/actioncable/Rakefile index 226d171104..fb75d14363 100644 --- a/actioncable/Rakefile +++ b/actioncable/Rakefile @@ -57,7 +57,7 @@ namespace :assets do end print "[verify] #{file} is a UMD module " - if pathname.read =~ /module\.exports.*define\.amd/m + if /module\.exports.*define\.amd/m.match?(pathname.read) puts "[OK]" else $stderr.puts "[FAIL]" diff --git a/actionpack/CHANGELOG.md b/actionpack/CHANGELOG.md index f7e9104627..a5497aa055 100644 --- a/actionpack/CHANGELOG.md +++ b/actionpack/CHANGELOG.md @@ -1,3 +1,18 @@ +* Raises `ActionController::RespondToMismatchError` with confliciting `respond_to` invocations. + + `respond_to` can match multiple types and lead to undefined behavior when + multiple invocations are made and the types do not match: + + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end + + *Patrick Toomey* + * `ActionDispatch::Http::UploadedFile` now delegates `to_path` to its tempfile. This allows uploaded file objects to be passed directly to `File.read` diff --git a/actionpack/lib/abstract_controller/helpers.rb b/actionpack/lib/abstract_controller/helpers.rb index 35b462bc92..3191584770 100644 --- a/actionpack/lib/abstract_controller/helpers.rb +++ b/actionpack/lib/abstract_controller/helpers.rb @@ -17,7 +17,7 @@ module AbstractController @path = "helpers/#{path}.rb" set_backtrace error.backtrace - if error.path =~ /^#{path}(\.rb)?$/ + if /^#{path}(\.rb)?$/.match?(error.path) super("Missing helper file helpers/%s.rb" % path) else raise error diff --git a/actionpack/lib/action_controller/metal/exceptions.rb b/actionpack/lib/action_controller/metal/exceptions.rb index ce9eb209fe..30034be018 100644 --- a/actionpack/lib/action_controller/metal/exceptions.rb +++ b/actionpack/lib/action_controller/metal/exceptions.rb @@ -51,6 +51,24 @@ module ActionController class UnknownFormat < ActionControllerError #:nodoc: end + # Raised when a nested respond_to is triggered and the content types of each + # are incompatible. For exampe: + # + # respond_to do |outer_type| + # outer_type.js do + # respond_to do |inner_type| + # inner_type.html { render body: "HTML" } + # end + # end + # end + class RespondToMismatchError < ActionControllerError + DEFAULT_MESSAGE = "respond_to was called multiple times and matched with conflicting formats in this action. Please note that you may only call respond_to and match on a single format per action." + + def initialize(message = nil) + super(message || DEFAULT_MESSAGE) + end + end + class MissingExactTemplate < UnknownFormat #:nodoc: end end diff --git a/actionpack/lib/action_controller/metal/head.rb b/actionpack/lib/action_controller/metal/head.rb index bac9bc5e5f..3c84bebb85 100644 --- a/actionpack/lib/action_controller/metal/head.rb +++ b/actionpack/lib/action_controller/metal/head.rb @@ -38,7 +38,7 @@ module ActionController self.response_body = "" if include_content?(response_code) - self.content_type = content_type || (Mime[formats.first] if formats) + self.content_type = content_type || (Mime[formats.first] if formats) || Mime[:html] response.charset = false end diff --git a/actionpack/lib/action_controller/metal/mime_responds.rb b/actionpack/lib/action_controller/metal/mime_responds.rb index 2233b93406..2b55b9347c 100644 --- a/actionpack/lib/action_controller/metal/mime_responds.rb +++ b/actionpack/lib/action_controller/metal/mime_responds.rb @@ -197,6 +197,9 @@ module ActionController #:nodoc: yield collector if block_given? if format = collector.negotiate_format(request) + if content_type && content_type != format + raise ActionController::RespondToMismatchError + end _process_format(format) _set_rendered_content_type format response = collector.response diff --git a/actionpack/lib/action_dispatch/http/headers.rb b/actionpack/lib/action_dispatch/http/headers.rb index c3c2a9d8c5..6c7d24d2d0 100644 --- a/actionpack/lib/action_dispatch/http/headers.rb +++ b/actionpack/lib/action_dispatch/http/headers.rb @@ -121,7 +121,7 @@ module ActionDispatch # not contained within the headers hash. def env_name(key) key = key.to_s - if key =~ HTTP_HEADER + if HTTP_HEADER.match?(key) key = key.upcase.tr("-", "_") key = "HTTP_" + key unless CGI_VARIABLES.include?(key) end diff --git a/actionpack/lib/action_dispatch/middleware/ssl.rb b/actionpack/lib/action_dispatch/middleware/ssl.rb index 240269d1c7..190e54223e 100644 --- a/actionpack/lib/action_dispatch/middleware/ssl.rb +++ b/actionpack/lib/action_dispatch/middleware/ssl.rb @@ -113,7 +113,7 @@ module ActionDispatch cookies = cookies.split("\n".freeze) headers["Set-Cookie".freeze] = cookies.map { |cookie| - if cookie !~ /;\s*secure\s*(;|$)/i + if !/;\s*secure\s*(;|$)/i.match?(cookie) "#{cookie}; secure" else cookie diff --git a/actionpack/lib/action_dispatch/routing/mapper.rb b/actionpack/lib/action_dispatch/routing/mapper.rb index aeaae3c1dd..ff325afc54 100644 --- a/actionpack/lib/action_dispatch/routing/mapper.rb +++ b/actionpack/lib/action_dispatch/routing/mapper.rb @@ -279,7 +279,7 @@ module ActionDispatch def verify_regexp_requirements(requirements) requirements.each do |requirement| - if requirement.source =~ ANCHOR_CHARACTERS_REGEX + if ANCHOR_CHARACTERS_REGEX.match?(requirement.source) raise ArgumentError, "Regexp anchor characters are not allowed in routing requirements: #{requirement.inspect}" end @@ -333,7 +333,7 @@ module ActionDispatch end def split_to(to) - if to =~ /#/ + if /#/.match?(to) to.split("#") else [] @@ -342,7 +342,7 @@ module ActionDispatch def add_controller_module(controller, modyoule) if modyoule && !controller.is_a?(Regexp) - if controller =~ %r{\A/} + if %r{\A/}.match?(controller) controller[1..-1] else [modyoule, controller].compact.join("/") @@ -1588,7 +1588,7 @@ module ActionDispatch when Symbol options[:action] = to when String - if to =~ /#/ + if /#/.match?(to) options[:to] = to else options[:controller] = to @@ -1914,7 +1914,7 @@ module ActionDispatch default_action = options.delete(:action) || @scope[:action] - if action =~ /^[\w\-\/]+$/ + if /^[\w\-\/]+$/.match?(action) default_action ||= action.tr("-", "_") unless action.include?("/") else action = nil diff --git a/actionpack/lib/action_dispatch/testing/assertions/routing.rb b/actionpack/lib/action_dispatch/testing/assertions/routing.rb index 5390581139..77cb311630 100644 --- a/actionpack/lib/action_dispatch/testing/assertions/routing.rb +++ b/actionpack/lib/action_dispatch/testing/assertions/routing.rb @@ -78,7 +78,7 @@ module ActionDispatch # # Asserts that the generated route gives us our custom route # assert_generates "changesets/12", { controller: 'scm', action: 'show_diff', revision: "12" } def assert_generates(expected_path, options, defaults = {}, extras = {}, message = nil) - if expected_path =~ %r{://} + if %r{://}.match?(expected_path) fail_on(URI::InvalidURIError, message) do uri = URI.parse(expected_path) expected_path = uri.path.to_s.empty? ? "/" : uri.path @@ -189,7 +189,7 @@ module ActionDispatch request = ActionController::TestRequest.create @controller.class - if path =~ %r{://} + if %r{://}.match?(path) fail_on(URI::InvalidURIError, msg) do uri = URI.parse(path) request.env["rack.url_scheme"] = uri.scheme || "http" diff --git a/actionpack/lib/action_dispatch/testing/integration.rb b/actionpack/lib/action_dispatch/testing/integration.rb index 7637febd1c..45439a3bb1 100644 --- a/actionpack/lib/action_dispatch/testing/integration.rb +++ b/actionpack/lib/action_dispatch/testing/integration.rb @@ -217,7 +217,7 @@ module ActionDispatch method = :post end - if path =~ %r{://} + if %r{://}.match?(path) path = build_expanded_path(path) do |location| https! URI::HTTPS === location if location.scheme diff --git a/actionpack/test/controller/mime/respond_to_test.rb b/actionpack/test/controller/mime/respond_to_test.rb index 771eccb29b..1163775d3c 100644 --- a/actionpack/test/controller/mime/respond_to_test.rb +++ b/actionpack/test/controller/mime/respond_to_test.rb @@ -102,6 +102,26 @@ class RespondToController < ActionController::Base end end + def using_conflicting_nested_js_then_html + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.html { render body: "HTML" } + end + end + end + end + + def using_non_conflicting_nested_js_then_js + respond_to do |outer_type| + outer_type.js do + respond_to do |inner_type| + inner_type.js { render body: "JS" } + end + end + end + end + def custom_type_handling respond_to do |type| type.html { render body: "HTML" } @@ -430,6 +450,20 @@ class RespondToControllerTest < ActionController::TestCase assert_equal "<p>Hello world!</p>\n", @response.body end + def test_using_conflicting_nested_js_then_html + @request.accept = "*/*" + assert_raises(ActionController::RespondToMismatchError) do + get :using_conflicting_nested_js_then_html + end + end + + def test_using_non_conflicting_nested_js_then_js + @request.accept = "*/*" + get :using_non_conflicting_nested_js_then_js + assert_equal "text/javascript", @response.content_type + assert_equal "JS", @response.body + end + def test_with_atom_content_type @request.accept = "" @request.env["CONTENT_TYPE"] = "application/atom+xml" diff --git a/actionpack/test/controller/render_test.rb b/actionpack/test/controller/render_test.rb index 6e3bd0596b..306b245bd1 100644 --- a/actionpack/test/controller/render_test.rb +++ b/actionpack/test/controller/render_test.rb @@ -250,6 +250,15 @@ class TestController < ActionController::Base head 204 end + def head_default_content_type + # simulating path like "/1.foobar" + request.formats = [] + + respond_to do |format| + format.any { head 200 } + end + end + private def set_variable_for_layout @@ -814,6 +823,11 @@ class HeadRenderTest < ActionController::TestCase get :head_and_return end end + + def test_head_default_content_type + post :head_default_content_type + assert_equal "text/html", @response.header["Content-Type"] + end end class HttpCacheForeverTest < ActionController::TestCase diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 43e63eb996..bd2a5b35fb 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -105,20 +105,16 @@ class HeaderTest < ActiveSupport::TestCase end test "#merge! headers with mutation" do - # rubocop:disable Performance/RedundantMerge @headers.merge!("Host" => "http://example.test", "Content-Type" => "text/html") - # rubocop:enable Performance/RedundantMerge assert_equal({ "HTTP_HOST" => "http://example.test", "CONTENT_TYPE" => "text/html", "HTTP_REFERER" => "/some/page" }, @headers.env) end test "#merge! env with mutation" do - # rubocop:disable Performance/RedundantMerge @headers.merge!("HTTP_HOST" => "http://first.com", "CONTENT_TYPE" => "text/html") - # rubocop:enable Performance/RedundantMerge assert_equal({ "HTTP_HOST" => "http://first.com", "CONTENT_TYPE" => "text/html", "HTTP_REFERER" => "/some/page" }, @headers.env) diff --git a/actionview/Rakefile b/actionview/Rakefile index bdfd96c141..7851a2b6bf 100644 --- a/actionview/Rakefile +++ b/actionview/Rakefile @@ -107,7 +107,7 @@ namespace :assets do end print "[verify] #{file} is a UMD module " - if pathname.read =~ /module\.exports.*define\.amd/m + if /module\.exports.*define\.amd/m.match?(pathname.read) puts "[OK]" else $stderr.puts "[FAIL]" diff --git a/actionview/app/assets/javascripts/rails-ujs/start.coffee b/actionview/app/assets/javascripts/rails-ujs/start.coffee index 55595ac96f..32a915ac0b 100644 --- a/actionview/app/assets/javascripts/rails-ujs/start.coffee +++ b/actionview/app/assets/javascripts/rails-ujs/start.coffee @@ -9,7 +9,8 @@ } = Rails # For backward compatibility -if jQuery? and jQuery.ajax? and not jQuery.rails +if jQuery? and jQuery.ajax? + throw new Error('If you load both jquery_ujs and rails-ujs, use rails-ujs only.') if jQuery.rails jQuery.rails = Rails jQuery.ajaxPrefilter (options, originalOptions, xhr) -> CSRFProtection(xhr) unless options.crossDomain diff --git a/actionview/lib/action_view/helpers/tags/color_field.rb b/actionview/lib/action_view/helpers/tags/color_field.rb index c5f0bb6bbb..39ab1285c3 100644 --- a/actionview/lib/action_view/helpers/tags/color_field.rb +++ b/actionview/lib/action_view/helpers/tags/color_field.rb @@ -15,7 +15,7 @@ module ActionView def validate_color_string(string) regex = /#[0-9a-fA-F]{6}/ - if regex.match(string) + if regex.match?(string) string.downcase else "#000000" diff --git a/actionview/lib/action_view/helpers/text_helper.rb b/actionview/lib/action_view/helpers/text_helper.rb index 34138de00e..77a1c1fed9 100644 --- a/actionview/lib/action_view/helpers/text_helper.rb +++ b/actionview/lib/action_view/helpers/text_helper.rb @@ -188,7 +188,7 @@ module ActionView unless separator.empty? text.split(separator).each do |value| - if value.match(regex) + if value.match?(regex) phrase = value break end diff --git a/activemodel/lib/active_model/naming.rb b/activemodel/lib/active_model/naming.rb index dfccd03cd8..983401801f 100644 --- a/activemodel/lib/active_model/naming.rb +++ b/activemodel/lib/active_model/naming.rb @@ -111,6 +111,22 @@ module ActiveModel # BlogPost.model_name.eql?('Blog Post') # => false ## + # :method: match? + # + # :call-seq: + # match?(regexp) + # + # Equivalent to <tt>String#match?</tt>. Match the class name against the + # given regexp. Returns +true+ if there is a match, otherwise +false+. + # + # class BlogPost + # extend ActiveModel::Naming + # end + # + # BlogPost.model_name.match?(/Post/) # => true + # BlogPost.model_name.match?(/\d/) # => false + + ## # :method: to_s # # :call-seq: @@ -131,7 +147,7 @@ module ActiveModel # to_str() # # Equivalent to +to_s+. - delegate :==, :===, :<=>, :=~, :"!~", :eql?, :to_s, + delegate :==, :===, :<=>, :=~, :"!~", :eql?, :match?, :to_s, :to_str, :as_json, to: :name # Returns a new ActiveModel::Name instance. By default, the +namespace+ diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index 456f569718..d1ae41ab97 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,9 +1,3 @@ -* Don't impose primary key order if limit() has already been supplied. - - Fixes #23607 - - *Brian Christian* - * Add environment & load_config dependency to `bin/rake db:seed` to enable seed load in environments without Rails and custom DB configuration diff --git a/activerecord/lib/active_record/associations/belongs_to_association.rb b/activerecord/lib/active_record/associations/belongs_to_association.rb index 08f450278d..3d4ad1dd5b 100644 --- a/activerecord/lib/active_record/associations/belongs_to_association.rb +++ b/activerecord/lib/active_record/associations/belongs_to_association.rb @@ -67,7 +67,7 @@ module ActiveRecord if target && !stale_target? target.increment!(reflection.counter_cache_column, by, touch: reflection.options[:touch]) else - klass.update_counters(target_id, reflection.counter_cache_column => by, touch: reflection.options[:touch]) + counter_cache_target.update_counters(reflection.counter_cache_column => by, touch: reflection.options[:touch]) end end end @@ -112,12 +112,9 @@ module ActiveRecord inverse && inverse.has_one? end - def target_id - if options[:primary_key] - owner.send(reflection.name).try(:id) - else - owner._read_attribute(reflection.foreign_key) - end + def counter_cache_target + primary_key = reflection.association_primary_key(klass) + klass.unscoped.where!(primary_key => owner._read_attribute(reflection.foreign_key)) end def stale_state diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 4b6cb76081..b5fb0092d7 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -48,14 +48,12 @@ module ActiveRecord::Associations::Builder # :nodoc: foreign_key_was = attribute_before_last_save foreign_key foreign_key = attribute_in_database foreign_key - if foreign_key && model.respond_to?(:increment_counter) - foreign_key = counter_cache_target(reflection, model, foreign_key) - model.increment_counter(cache_column, foreign_key) + if foreign_key && model < ActiveRecord::Base + counter_cache_target(reflection, model, foreign_key).update_counters(cache_column => 1) end - if foreign_key_was && model_was.respond_to?(:decrement_counter) - foreign_key_was = counter_cache_target(reflection, model_was, foreign_key_was) - model_was.decrement_counter(cache_column, foreign_key_was) + if foreign_key_was && model_was < ActiveRecord::Base + counter_cache_target(reflection, model_was, foreign_key_was).update_counters(cache_column => -1) end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb index c730584902..f721e91203 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/connection_pool.rb @@ -1011,8 +1011,8 @@ module ActiveRecord # Returns true if a connection that's accessible to this class has # already been opened. def connected?(spec_name) - conn = retrieve_connection_pool(spec_name) - conn && conn.connected? + pool = retrieve_connection_pool(spec_name) + pool && pool.connected? end # Remove the connection for this class. This will close the active diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 8bdf1712b1..a4748dbeda 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -81,7 +81,9 @@ module ActiveRecord alias :in_use? :owner def self.type_cast_config_to_integer(config) - if config =~ SIMPLE_INT + if config.is_a?(Integer) + config + elsif SIMPLE_INT.match?(config) config.to_i else config 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 284b38ed7b..9de8242a58 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -619,6 +619,7 @@ module ActiveRecord ER_DUP_ENTRY = 1062 ER_NOT_NULL_VIOLATION = 1048 ER_DO_NOT_HAVE_DEFAULT = 1364 + ER_ROW_IS_REFERENCED_2 = 1451 ER_NO_REFERENCED_ROW_2 = 1452 ER_DATA_TOO_LONG = 1406 ER_OUT_OF_RANGE = 1264 @@ -633,7 +634,7 @@ module ActiveRecord case error_number(exception) when ER_DUP_ENTRY RecordNotUnique.new(message) - when ER_NO_REFERENCED_ROW_2 + when ER_ROW_IS_REFERENCED_2, ER_NO_REFERENCED_ROW_2 InvalidForeignKey.new(message) when ER_CANNOT_ADD_FOREIGN mismatched_foreign_key(message) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb index e20e5f2914..26b8113b0d 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -751,7 +751,7 @@ module ActiveRecord def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) - scope[:type] ||= "'r','v','m','f'" # (r)elation/table, (v)iew, (m)aterialized view, (f)oreign table + scope[:type] ||= "'r','v','m','p','f'" # (r)elation/table, (v)iew, (m)aterialized view, (p)artitioned table, (f)oreign table sql = "SELECT c.relname FROM pg_class c LEFT JOIN pg_namespace n ON n.oid = c.relnamespace".dup sql << " WHERE n.nspname = #{scope[:schema]}" @@ -765,7 +765,7 @@ module ActiveRecord type = \ case type when "BASE TABLE" - "'r'" + "'r','p'" when "VIEW" "'v','m'" when "FOREIGN TABLE" diff --git a/activerecord/lib/active_record/counter_cache.rb b/activerecord/lib/active_record/counter_cache.rb index 0d8748d7e6..c7f0077a76 100644 --- a/activerecord/lib/active_record/counter_cache.rb +++ b/activerecord/lib/active_record/counter_cache.rb @@ -102,27 +102,7 @@ module ActiveRecord # # `updated_at` = '2016-10-13T09:59:23-05:00' # # WHERE id IN (10, 15) def update_counters(id, counters) - touch = counters.delete(:touch) - - updates = counters.map do |counter_name, value| - operator = value < 0 ? "-" : "+" - quoted_column = connection.quote_column_name(counter_name) - "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{value.abs}" - end - - if touch - names = touch if touch != true - touch_updates = touch_attributes_with_time(*names) - updates << sanitize_sql_for_assignment(touch_updates) unless touch_updates.empty? - end - - if id.is_a?(Relation) && self == id.klass - relation = id - else - relation = unscoped.where!(primary_key => id) - end - - relation.update_all updates.join(", ") + unscoped.where!(primary_key => id).update_counters(counters) end # Increment a numeric field by one, via a direct SQL update. diff --git a/activerecord/lib/active_record/errors.rb b/activerecord/lib/active_record/errors.rb index c2a180c939..f61bc7b9e8 100644 --- a/activerecord/lib/active_record/errors.rb +++ b/activerecord/lib/active_record/errors.rb @@ -111,7 +111,8 @@ module ActiveRecord class RecordNotUnique < WrappedDatabaseException end - # Raised when a record cannot be inserted or updated because it references a non-existent record. + # Raised when a record cannot be inserted or updated because it references a non-existent record, + # or when a record cannot be deleted because a parent record references it. class InvalidForeignKey < WrappedDatabaseException end diff --git a/activerecord/lib/active_record/log_subscriber.rb b/activerecord/lib/active_record/log_subscriber.rb index cf884bc6da..1ae6840921 100644 --- a/activerecord/lib/active_record/log_subscriber.rb +++ b/activerecord/lib/active_record/log_subscriber.rb @@ -100,36 +100,21 @@ module ActiveRecord end def log_query_source - source_line, line_number = extract_callstack(caller_locations) + location = extract_query_source_location(caller_locations) - if source_line - if defined?(::Rails.root) - app_root = "#{::Rails.root}/" - source_line = source_line.sub(app_root, "") - end + if location + source = "#{location.path}:#{location.lineno}" + source = source.sub("#{::Rails.root}/", "") if defined?(::Rails.root) - logger.debug(" ↳ #{ source_line }:#{ line_number }") + logger.debug(" ↳ #{source}") end end - def extract_callstack(callstack) - line = callstack.find do |frame| - frame.absolute_path && !ignored_callstack(frame.absolute_path) - end - - offending_line = line || callstack.first - - [ - offending_line.path, - offending_line.lineno - ] - end - - RAILS_GEM_ROOT = File.expand_path("../../..", __dir__) + "/" + RAILS_GEM_ROOT = File.expand_path("../../..", __dir__) + "/" + PATHS_TO_IGNORE = /\A(#{RAILS_GEM_ROOT}|#{RbConfig::CONFIG["rubylibdir"]})/ - def ignored_callstack(path) - path.start_with?(RAILS_GEM_ROOT) || - path.start_with?(RbConfig::CONFIG["rubylibdir"]) + def extract_query_source_location(locations) + locations.find { |line| line.absolute_path && !line.absolute_path.match?(PATHS_TO_IGNORE) } end end end diff --git a/activerecord/lib/active_record/persistence.rb b/activerecord/lib/active_record/persistence.rb index 155d67fd8f..05963e5546 100644 --- a/activerecord/lib/active_record/persistence.rb +++ b/activerecord/lib/active_record/persistence.rb @@ -107,13 +107,11 @@ module ActiveRecord # When running callbacks is not needed for each record update, # it is preferred to use {update_all}[rdoc-ref:Relation#update_all] # for updating all records in a single query. - def update(id = :all, attributes) + def update(id, attributes) if id.is_a?(Array) id.map { |one_id| find(one_id) }.each_with_index { |object, idx| object.update(attributes[idx]) } - elsif id == :all - all.each { |record| record.update(attributes) } else if ActiveRecord::Base === id raise ArgumentError, diff --git a/activerecord/lib/active_record/relation.rb b/activerecord/lib/active_record/relation.rb index e7224658c4..2d3e1eaa08 100644 --- a/activerecord/lib/active_record/relation.rb +++ b/activerecord/lib/active_record/relation.rb @@ -43,6 +43,12 @@ module ActiveRecord klass.arel_attribute(name, table) end + def bind_attribute(name, value) # :nodoc: + attr = arel_attribute(name) + bind = predicate_builder.build_bind_attribute(attr.name, value) + yield attr, bind + end + # Initializes new record from relation while maintaining the current # scope. # @@ -369,6 +375,28 @@ module ActiveRecord @klass.connection.update stmt, "#{@klass} Update All" end + def update(attributes) # :nodoc: + each { |record| record.update(attributes) } + end + + def update_counters(counters) # :nodoc: + touch = counters.delete(:touch) + + updates = counters.map do |counter_name, value| + operator = value < 0 ? "-" : "+" + quoted_column = connection.quote_column_name(counter_name) + "#{quoted_column} = COALESCE(#{quoted_column}, 0) #{operator} #{value.abs}" + end + + if touch + names = touch if touch != true + touch_updates = klass.touch_attributes_with_time(*names) + updates << klass.sanitize_sql_for_assignment(touch_updates) unless touch_updates.empty? + end + + update_all updates.join(", ") + end + # Touches all records in the current relation without instantiating records first with the updated_at/on attributes # set to the current time or the time specified. # This method can be passed attribute names and an optional time argument. diff --git a/activerecord/lib/active_record/relation/batches.rb b/activerecord/lib/active_record/relation/batches.rb index ec4bb06c57..9c579843b1 100644 --- a/activerecord/lib/active_record/relation/batches.rb +++ b/activerecord/lib/active_record/relation/batches.rb @@ -251,8 +251,9 @@ module ActiveRecord end end - bind = primary_key_bind(primary_key_offset) - batch_relation = relation.where(arel_attribute(primary_key).gt(bind)) + batch_relation = relation.where( + bind_attribute(primary_key, primary_key_offset) { |attr, bind| attr.gt(bind) } + ) end end @@ -265,15 +266,11 @@ module ActiveRecord end def apply_start_limit(relation, start) - relation.where(arel_attribute(primary_key).gteq(primary_key_bind(start))) + relation.where(bind_attribute(primary_key, start) { |attr, bind| attr.gteq(bind) }) end def apply_finish_limit(relation, finish) - relation.where(arel_attribute(primary_key).lteq(primary_key_bind(finish))) - end - - def primary_key_bind(value) - predicate_builder.build_bind_attribute(primary_key, value) + relation.where(bind_attribute(primary_key, finish) { |attr, bind| attr.lteq(bind) }) end def batch_order diff --git a/activerecord/lib/active_record/relation/calculations.rb b/activerecord/lib/active_record/relation/calculations.rb index f215c95f51..40fe39fa9d 100644 --- a/activerecord/lib/active_record/relation/calculations.rb +++ b/activerecord/lib/active_record/relation/calculations.rb @@ -245,7 +245,7 @@ module ActiveRecord if distinct && (group_values.any? || select_values.empty? && order_values.empty?) column_name = primary_key end - elsif column_name =~ /\s*DISTINCT[\s(]+/i + elsif /\s*DISTINCT[\s(]+/i.match?(column_name.to_s) distinct = nil end end diff --git a/activerecord/lib/active_record/relation/finder_methods.rb b/activerecord/lib/active_record/relation/finder_methods.rb index 93f3b67686..c5562c1ff0 100644 --- a/activerecord/lib/active_record/relation/finder_methods.rb +++ b/activerecord/lib/active_record/relation/finder_methods.rb @@ -550,7 +550,7 @@ module ActiveRecord end def ordered_relation - if order_values.empty? && primary_key && limit_value.blank? + if order_values.empty? && primary_key order(arel_attribute(primary_key).asc) else self diff --git a/activerecord/lib/active_record/tasks/database_tasks.rb b/activerecord/lib/active_record/tasks/database_tasks.rb index 521375954b..d943796024 100644 --- a/activerecord/lib/active_record/tasks/database_tasks.rb +++ b/activerecord/lib/active_record/tasks/database_tasks.rb @@ -248,6 +248,7 @@ module ActiveRecord def load_schema(configuration, format = ActiveRecord::Base.schema_format, file = nil, environment = env, spec_name = "primary") # :nodoc: file ||= dump_filename(spec_name, format) + verbose_was, Migration.verbose = Migration.verbose, verbose? && ENV["VERBOSE"] check_schema_file(file) ActiveRecord::Base.establish_connection(configuration) @@ -261,6 +262,8 @@ module ActiveRecord end ActiveRecord::InternalMetadata.create_table ActiveRecord::InternalMetadata[:environment] = environment + ensure + Migration.verbose = verbose_was end def schema_file(format = ActiveRecord::Base.schema_format) diff --git a/activerecord/lib/active_record/test_databases.rb b/activerecord/lib/active_record/test_databases.rb index 606a3b0fb5..5b4efe22c9 100644 --- a/activerecord/lib/active_record/test_databases.rb +++ b/activerecord/lib/active_record/test_databases.rb @@ -5,28 +5,27 @@ require "active_support/testing/parallelization" module ActiveRecord module TestDatabases # :nodoc: ActiveSupport::Testing::Parallelization.after_fork_hook do |i| - create_and_migrate(i, spec_name: Rails.env) + create_and_load_schema(i, spec_name: Rails.env) end - ActiveSupport::Testing::Parallelization.run_cleanup_hook do |i| - drop(i, spec_name: Rails.env) + ActiveSupport::Testing::Parallelization.run_cleanup_hook do |_| + drop(spec_name: Rails.env) end - def self.create_and_migrate(i, spec_name:) + def self.create_and_load_schema(i, spec_name:) old, ENV["VERBOSE"] = ENV["VERBOSE"], "false" connection_spec = ActiveRecord::Base.configurations[spec_name] connection_spec["database"] += "-#{i}" ActiveRecord::Tasks::DatabaseTasks.create(connection_spec) - ActiveRecord::Base.establish_connection(connection_spec) - ActiveRecord::Tasks::DatabaseTasks.migrate + ActiveRecord::Tasks::DatabaseTasks.load_schema(connection_spec) ensure ActiveRecord::Base.establish_connection(ActiveRecord::Base.configurations[Rails.env]) ENV["VERBOSE"] = old end - def self.drop(i, spec_name:) + def self.drop(spec_name:) old, ENV["VERBOSE"] = ENV["VERBOSE"], "false" connection_spec = ActiveRecord::Base.configurations[spec_name] diff --git a/activerecord/test/cases/adapter_test.rb b/activerecord/test/cases/adapter_test.rb index 79642f5871..59b99351d1 100644 --- a/activerecord/test/cases/adapter_test.rb +++ b/activerecord/test/cases/adapter_test.rb @@ -304,6 +304,8 @@ module ActiveRecord class AdapterForeignKeyTest < ActiveRecord::TestCase self.use_transactional_tests = false + fixtures :fk_test_has_pk + def setup @connection = ActiveRecord::Base.connection end @@ -322,7 +324,7 @@ module ActiveRecord assert_not_nil error.cause end - def test_foreign_key_violations_are_translated_to_specific_exception + def test_foreign_key_violations_on_insert_are_translated_to_specific_exception error = assert_raises(ActiveRecord::InvalidForeignKey) do insert_into_fk_test_has_fk end @@ -330,6 +332,16 @@ module ActiveRecord assert_not_nil error.cause end + def test_foreign_key_violations_on_delete_are_translated_to_specific_exception + insert_into_fk_test_has_fk fk_id: 1 + + error = assert_raises(ActiveRecord::InvalidForeignKey) do + @connection.execute "DELETE FROM fk_test_has_pk WHERE pk_id = 1" + end + + assert_not_nil error.cause + end + def test_disable_referential_integrity assert_nothing_raised do @connection.disable_referential_integrity do @@ -342,14 +354,13 @@ module ActiveRecord end private - - def insert_into_fk_test_has_fk + def insert_into_fk_test_has_fk(fk_id: 0) # Oracle adapter uses prefetched primary key values from sequence and passes them to connection adapter insert method if @connection.prefetch_primary_key? id_value = @connection.next_sequence_value(@connection.default_sequence_name("fk_test_has_fk", "id")) - @connection.execute "INSERT INTO fk_test_has_fk (id,fk_id) VALUES (#{id_value},0)" + @connection.execute "INSERT INTO fk_test_has_fk (id,fk_id) VALUES (#{id_value},#{fk_id})" else - @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (0)" + @connection.execute "INSERT INTO fk_test_has_fk (fk_id) VALUES (#{fk_id})" end end end diff --git a/activerecord/test/cases/adapters/postgresql/partitions_test.rb b/activerecord/test/cases/adapters/postgresql/partitions_test.rb new file mode 100644 index 0000000000..0ac9ca1200 --- /dev/null +++ b/activerecord/test/cases/adapters/postgresql/partitions_test.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +require "cases/helper" + +class PostgreSQLPartitionsTest < ActiveRecord::PostgreSQLTestCase + def setup + @connection = ActiveRecord::Base.connection + end + + def teardown + @connection.drop_table "partitioned_events", if_exists: true + end + + def test_partitions_table_exists + skip unless ActiveRecord::Base.connection.postgresql_version >= 100000 + @connection.create_table :partitioned_events, force: true, id: false, + options: "partition by range (issued_at)" do |t| + t.timestamp :issued_at + end + assert @connection.table_exists?("partitioned_events") + end +end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 2f256dd36a..e73c88dd5d 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -463,6 +463,7 @@ class FinderTest < ActiveRecord::TestCase expected = topics(:first) expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.first + assert_equal expected, Topic.limit(5).first end def test_model_class_responds_to_first_bang @@ -485,6 +486,7 @@ class FinderTest < ActiveRecord::TestCase expected = topics(:second) expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.second + assert_equal expected, Topic.limit(5).second end def test_model_class_responds_to_second_bang @@ -507,6 +509,7 @@ class FinderTest < ActiveRecord::TestCase expected = topics(:third) expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.third + assert_equal expected, Topic.limit(5).third end def test_model_class_responds_to_third_bang @@ -529,6 +532,7 @@ class FinderTest < ActiveRecord::TestCase expected = topics(:fourth) expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.fourth + assert_equal expected, Topic.limit(5).fourth end def test_model_class_responds_to_fourth_bang @@ -551,6 +555,7 @@ class FinderTest < ActiveRecord::TestCase expected = topics(:fifth) expected.touch # PostgreSQL changes the default order if no order clause is used assert_equal expected, Topic.fifth + assert_equal expected, Topic.limit(5).fifth end def test_model_class_responds_to_fifth_bang @@ -713,6 +718,14 @@ class FinderTest < ActiveRecord::TestCase assert_equal comments.limit(2).to_a.first(3), comments.limit(2).first(3) end + def test_first_have_determined_order_by_default + expected = [companies(:second_client), companies(:another_client)] + clients = Client.where(name: expected.map(&:name)) + + assert_equal expected, clients.first(2) + assert_equal expected, clients.limit(5).first(2) + end + def test_take_and_first_and_last_with_integer_should_return_an_array assert_kind_of Array, Topic.take(5) assert_kind_of Array, Topic.first(5) @@ -1265,21 +1278,6 @@ class FinderTest < ActiveRecord::TestCase end end - def test_first_and_last_with_limit_for_order_without_primary_key - # While Topic.first should impose an ordering by primary key, - # Topic.limit(n).first should not - - Topic.first.touch # PostgreSQL changes the default order if no order clause is used - - assert_equal Topic.limit(1).to_a.first, Topic.limit(1).first - assert_equal Topic.limit(2).to_a.first, Topic.limit(2).first - assert_equal Topic.limit(2).to_a.first(2), Topic.limit(2).first(2) - - assert_equal Topic.limit(1).to_a.last, Topic.limit(1).last - assert_equal Topic.limit(2).to_a.last, Topic.limit(2).last - assert_equal Topic.limit(2).to_a.last(2), Topic.limit(2).last(2) - end - def test_finder_with_offset_string assert_nothing_raised { Topic.offset("3").to_a } end diff --git a/activerecord/test/cases/log_subscriber_test.rb b/activerecord/test/cases/log_subscriber_test.rb index e2742ed33e..f0126fdb0d 100644 --- a/activerecord/test/cases/log_subscriber_test.rb +++ b/activerecord/test/cases/log_subscriber_test.rb @@ -177,11 +177,25 @@ class LogSubscriberTest < ActiveRecord::TestCase logger = TestDebugLogSubscriber.new logger.sql(Event.new(0, sql: "hi mom!")) + assert_equal 2, @logger.logged(:debug).size assert_match(/↳/, @logger.logged(:debug).last) ensure ActiveRecord::Base.verbose_query_logs = false end + def test_verbose_query_with_ignored_callstack + ActiveRecord::Base.verbose_query_logs = true + + logger = TestDebugLogSubscriber.new + def logger.extract_query_source_location(*); nil; end + + logger.sql(Event.new(0, sql: "hi mom!")) + assert_equal 1, @logger.logged(:debug).size + assert_no_match(/↳/, @logger.logged(:debug).last) + ensure + ActiveRecord::Base.verbose_query_logs = false + end + def test_verbose_query_logs_disabled_by_default logger = TestDebugLogSubscriber.new logger.sql(Event.new(0, sql: "hi mom!")) diff --git a/activerecord/test/cases/relation_test.rb b/activerecord/test/cases/relation_test.rb index 3ca15640fd..fbeb617b29 100644 --- a/activerecord/test/cases/relation_test.rb +++ b/activerecord/test/cases/relation_test.rb @@ -187,7 +187,7 @@ module ActiveRecord end relation = Relation.new(klass) - relation.merge!(where: ["foo = ?", "bar"]) # rubocop:disable Performance/RedundantMerge + relation.merge!(where: ["foo = ?", "bar"]) assert_equal Relation::WhereClause.new(["foo = bar"]), relation.where_clause end diff --git a/activerecord/test/cases/relations_test.rb b/activerecord/test/cases/relations_test.rb index 952d2dd5d9..5412ab5def 100644 --- a/activerecord/test/cases/relations_test.rb +++ b/activerecord/test/cases/relations_test.rb @@ -30,7 +30,9 @@ class RelationTest < ActiveRecord::TestCase class TopicWithCallbacks < ActiveRecord::Base self.table_name = :topics + cattr_accessor :topic_count before_update { |topic| topic.author_name = "David" if topic.author_name.blank? } + after_update { |topic| topic.class.topic_count = topic.class.count } end def test_do_not_double_quote_string_id @@ -1576,6 +1578,8 @@ class RelationTest < ActiveRecord::TestCase topics = TopicWithCallbacks.where(id: [topic1.id, topic2.id]) topics.update(title: "adequaterecord") + assert_equal TopicWithCallbacks.count, TopicWithCallbacks.topic_count + assert_equal "adequaterecord", topic1.reload.title assert_equal "adequaterecord", topic2.reload.title # Testing that the before_update callbacks have run diff --git a/activestorage/app/assets/javascripts/activestorage.js b/activestorage/app/assets/javascripts/activestorage.js index a22f644238..d3fd795a3a 100644 --- a/activestorage/app/assets/javascripts/activestorage.js +++ b/activestorage/app/assets/javascripts/activestorage.js @@ -484,7 +484,7 @@ }, { key: "readNextChunk", value: function readNextChunk() { - if (this.chunkIndex < this.chunkCount) { + if (this.chunkIndex < this.chunkCount || this.chunkIndex == 0 && this.chunkCount == 0) { var start = this.chunkIndex * this.chunkSize; var end = Math.min(start + this.chunkSize, this.file.size); var bytes = fileSlice.call(this.file, start, end); diff --git a/activestorage/app/javascript/activestorage/file_checksum.js b/activestorage/app/javascript/activestorage/file_checksum.js index ffaec1a128..a9dbef69ea 100644 --- a/activestorage/app/javascript/activestorage/file_checksum.js +++ b/activestorage/app/javascript/activestorage/file_checksum.js @@ -39,7 +39,7 @@ export class FileChecksum { } readNextChunk() { - if (this.chunkIndex < this.chunkCount) { + if (this.chunkIndex < this.chunkCount || (this.chunkIndex == 0 && this.chunkCount == 0)) { const start = this.chunkIndex * this.chunkSize const end = Math.min(start + this.chunkSize, this.file.size) const bytes = fileSlice.call(this.file, start, end) diff --git a/activestorage/app/jobs/active_storage/purge_job.rb b/activestorage/app/jobs/active_storage/purge_job.rb index fa15e0451d..2604977bf1 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -3,6 +3,7 @@ # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. class ActiveStorage::PurgeJob < ActiveStorage::BaseJob discard_on ActiveRecord::RecordNotFound + retry_on ActiveRecord::Deadlocked, attempts: 10, wait: :exponentially_longer def perform(blob) blob.purge diff --git a/activestorage/app/models/active_storage/attachment.rb b/activestorage/app/models/active_storage/attachment.rb index 1c4dc25094..4bdd1c0224 100644 --- a/activestorage/app/models/active_storage/attachment.rb +++ b/activestorage/app/models/active_storage/attachment.rb @@ -20,13 +20,13 @@ class ActiveStorage::Attachment < ActiveRecord::Base # Synchronously deletes the attachment and {purges the blob}[rdoc-ref:ActiveStorage::Blob#purge]. def purge delete - blob.purge + blob&.purge end # Deletes the attachment and {enqueues a background job}[rdoc-ref:ActiveStorage::Blob#purge_later] to purge the blob. def purge_later delete - blob.purge_later + blob&.purge_later end private @@ -39,7 +39,7 @@ class ActiveStorage::Attachment < ActiveRecord::Base end def purge_dependent_blob_later - blob.purge_later if dependent == :purge_later + blob&.purge_later if dependent == :purge_later end diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb index a4e148c1a5..6c0b4c30e7 100644 --- a/activestorage/lib/active_storage/log_subscriber.rb +++ b/activestorage/lib/active_storage/log_subscriber.rb @@ -14,6 +14,8 @@ module ActiveStorage info event, color("Downloaded file from key: #{key_in(event)}", BLUE) end + alias_method :service_streaming_download, :service_download + def service_delete(event) info event, color("Deleted file from key: #{key_in(event)}", RED) end diff --git a/activestorage/lib/active_storage/previewer.rb b/activestorage/lib/active_storage/previewer.rb index fb202f029a..95a041fd16 100644 --- a/activestorage/lib/active_storage/previewer.rb +++ b/activestorage/lib/active_storage/previewer.rb @@ -2,8 +2,8 @@ module ActiveStorage # This is an abstract base class for previewers, which generate images from blobs. See - # ActiveStorage::Previewer::PDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for examples of - # concrete subclasses. + # ActiveStorage::Previewer::MuPDFPreviewer and ActiveStorage::Previewer::VideoPreviewer for + # examples of concrete subclasses. class Previewer attr_reader :blob diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index fdde01d4a3..eb46973509 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -1,11 +1,7 @@ # frozen_string_literal: true gem "google-cloud-storage", "~> 1.11" - require "google/cloud/storage" -require "net/http" - -require "active_support/core_ext/object/to_query" module ActiveStorage # Wraps the Google Cloud Storage as an Active Storage service. See ActiveStorage::Service for the generic API @@ -61,7 +57,13 @@ module ActiveStorage def delete_prefixed(prefix) instrument :delete_prefixed, prefix: prefix do - bucket.files(prefix: prefix).all(&:delete) + bucket.files(prefix: prefix).all do |file| + begin + file.delete + rescue Google::Cloud::NotFoundError + # Ignore concurrently-deleted files + end + end end end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 24ad72c45d..8808b38aac 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,41 @@ +* Add "event object" support to the notification system. + Before this change, end users were forced to create hand made artisanal + event objects on their own, like this: + + ActiveSupport::Notifications.subscribe('wait') do |*args| + @event = ActiveSupport::Notifications::Event.new(*args) + end + + ActiveSupport::Notifications.instrument('wait') do + sleep 1 + end + + @event.duration # => 1000.138 + + After this change, if the block passed to `subscribe` only takes one + parameter, the framework will yield an event object to the block. Now + end users are no longer required to make their own: + + ActiveSupport::Notifications.subscribe('wait') do |event| + @event = event + end + + ActiveSupport::Notifications.instrument('wait') do + sleep 1 + end + + p @event.allocations # => 7 + p @event.cpu_time # => 0.256 + p @event.idle_time # => 1003.2399 + + Now you can enjoy event objects without making them yourself. Neat! + + *Aaron "t.lo" Patterson* + +* Add cpu_time, idle_time, and allocations to Event + + *Eileen M. Uchitelle*, *Aaron Patterson* + * RedisCacheStore: support key expiry in increment/decrement. Pass `:expires_in` to `#increment` and `#decrement` to set a Redis EXPIRE on the key. diff --git a/activesupport/lib/active_support/configurable.rb b/activesupport/lib/active_support/configurable.rb index 4d6f7819bb..2610114d8f 100644 --- a/activesupport/lib/active_support/configurable.rb +++ b/activesupport/lib/active_support/configurable.rb @@ -3,7 +3,6 @@ require "active_support/concern" require "active_support/ordered_options" require "active_support/core_ext/array/extract_options" -require "active_support/core_ext/regexp" module ActiveSupport # Configurable provides a <tt>config</tt> method to store and retrieve diff --git a/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb b/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb index 01fee0fb74..281eaa7d5f 100644 --- a/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb +++ b/activesupport/lib/active_support/core_ext/module/attribute_accessors.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/array/extract_options" -require "active_support/core_ext/regexp" # Extends the module object with class/module and instance accessors for # class/module attributes, just like the native attr* accessors for instance diff --git a/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb b/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb index 4b9b6ea9bd..b1788a000b 100644 --- a/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb +++ b/activesupport/lib/active_support/core_ext/module/attribute_accessors_per_thread.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/core_ext/array/extract_options" -require "active_support/core_ext/regexp" # Extends the module object with class/module and instance accessors for # class/module attributes, just like the native attr* accessors for instance @@ -137,7 +136,7 @@ class Module # Or pass <tt>instance_accessor: false</tt>, to opt out both instance methods. # # class Current - # mattr_accessor :user, instance_accessor: false + # thread_mattr_accessor :user, instance_accessor: false # end # # Current.new.user = "DHH" # => NoMethodError diff --git a/activesupport/lib/active_support/core_ext/module/delegation.rb b/activesupport/lib/active_support/core_ext/module/delegation.rb index 7f42f44efb..3128539112 100644 --- a/activesupport/lib/active_support/core_ext/module/delegation.rb +++ b/activesupport/lib/active_support/core_ext/module/delegation.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "set" -require "active_support/core_ext/regexp" class Module # Error generated by +delegate+ when a method is called on +nil+ and +allow_nil+ diff --git a/activesupport/lib/active_support/core_ext/object/blank.rb b/activesupport/lib/active_support/core_ext/object/blank.rb index 2ca431ab10..ad6a9c6146 100644 --- a/activesupport/lib/active_support/core_ext/object/blank.rb +++ b/activesupport/lib/active_support/core_ext/object/blank.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/regexp" require "concurrent/map" class Object diff --git a/activesupport/lib/active_support/dependencies.rb b/activesupport/lib/active_support/dependencies.rb index a02cefc78e..9dc2c46880 100644 --- a/activesupport/lib/active_support/dependencies.rb +++ b/activesupport/lib/active_support/dependencies.rb @@ -345,7 +345,7 @@ module ActiveSupport #:nodoc: end def require_or_load(file_name, const_path = nil) - file_name = $` if file_name =~ /\.rb\z/ + file_name = file_name.chomp(".rb") expanded = File.expand_path(file_name) return if loaded.include?(expanded) @@ -395,7 +395,7 @@ module ActiveSupport #:nodoc: # constant paths which would cause Dependencies to attempt to load this # file. def loadable_constants_for_path(path, bases = autoload_paths) - path = $` if path =~ /\.rb\z/ + path = path.chomp(".rb") expanded_path = File.expand_path(path) paths = [] diff --git a/activesupport/lib/active_support/deprecation/method_wrappers.rb b/activesupport/lib/active_support/deprecation/method_wrappers.rb index 5be893d281..81482092fe 100644 --- a/activesupport/lib/active_support/deprecation/method_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/method_wrappers.rb @@ -1,6 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/module/aliasing" require "active_support/core_ext/array/extract_options" module ActiveSupport @@ -54,23 +53,26 @@ module ActiveSupport deprecator = options.delete(:deprecator) || self method_names += options.keys - mod = Module.new do - method_names.each do |method_name| - define_method(method_name) do |*args, &block| - deprecator.deprecation_warning(method_name, options[method_name]) - super(*args, &block) - end + method_names.each do |method_name| + aliased_method, punctuation = method_name.to_s.sub(/([?!=])$/, ""), $1 + with_method = "#{aliased_method}_with_deprecation#{punctuation}" + without_method = "#{aliased_method}_without_deprecation#{punctuation}" - case - when target_module.protected_method_defined?(method_name) - protected method_name - when target_module.private_method_defined?(method_name) - private method_name - end + target_module.send(:define_method, with_method) do |*args, &block| + deprecator.deprecation_warning(method_name, options[method_name]) + send(without_method, *args, &block) end - end - target_module.prepend(mod) + target_module.send(:alias_method, without_method, method_name) + target_module.send(:alias_method, method_name, with_method) + + case + when target_module.protected_method_defined?(without_method) + target_module.send(:protected, method_name) + when target_module.private_method_defined?(without_method) + target_module.send(:private, method_name) + end + end end end end diff --git a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb index 896c0d2d8e..56f1e23136 100644 --- a/activesupport/lib/active_support/deprecation/proxy_wrappers.rb +++ b/activesupport/lib/active_support/deprecation/proxy_wrappers.rb @@ -1,7 +1,5 @@ # frozen_string_literal: true -require "active_support/core_ext/regexp" - module ActiveSupport class Deprecation class DeprecationProxy #:nodoc: diff --git a/activesupport/lib/active_support/duration/iso8601_parser.rb b/activesupport/lib/active_support/duration/iso8601_parser.rb index 1847eeaa86..414f727705 100644 --- a/activesupport/lib/active_support/duration/iso8601_parser.rb +++ b/activesupport/lib/active_support/duration/iso8601_parser.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "strscan" -require "active_support/core_ext/regexp" module ActiveSupport class Duration diff --git a/activesupport/lib/active_support/inflector/inflections.rb b/activesupport/lib/active_support/inflector/inflections.rb index 7e5dff1d6d..2b86d233e5 100644 --- a/activesupport/lib/active_support/inflector/inflections.rb +++ b/activesupport/lib/active_support/inflector/inflections.rb @@ -2,7 +2,6 @@ require "concurrent/map" require "active_support/core_ext/array/prepend_and_append" -require "active_support/core_ext/regexp" require "active_support/i18n" require "active_support/deprecation" diff --git a/activesupport/lib/active_support/inflector/methods.rb b/activesupport/lib/active_support/inflector/methods.rb index 339b93b8da..7359de762a 100644 --- a/activesupport/lib/active_support/inflector/methods.rb +++ b/activesupport/lib/active_support/inflector/methods.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/inflections" -require "active_support/core_ext/regexp" module ActiveSupport # The Inflector transforms words from singular to plural, class names to table diff --git a/activesupport/lib/active_support/multibyte/chars.rb b/activesupport/lib/active_support/multibyte/chars.rb index 8152b8fd22..499a206f49 100644 --- a/activesupport/lib/active_support/multibyte/chars.rb +++ b/activesupport/lib/active_support/multibyte/chars.rb @@ -4,7 +4,6 @@ require "active_support/json" require "active_support/core_ext/string/access" require "active_support/core_ext/string/behavior" require "active_support/core_ext/module/delegation" -require "active_support/core_ext/regexp" module ActiveSupport #:nodoc: module Multibyte #:nodoc: diff --git a/activesupport/lib/active_support/notifications/fanout.rb b/activesupport/lib/active_support/notifications/fanout.rb index 25aab175b4..4e4ca70942 100644 --- a/activesupport/lib/active_support/notifications/fanout.rb +++ b/activesupport/lib/active_support/notifications/fanout.rb @@ -70,12 +70,29 @@ module ActiveSupport module Subscribers # :nodoc: def self.new(pattern, listener) + subscriber_class = Timed + if listener.respond_to?(:start) && listener.respond_to?(:finish) - subscriber = Evented.new pattern, listener + subscriber_class = Evented else - subscriber = Timed.new pattern, listener + # Doing all this to detect a block like `proc { |x| }` vs + # `proc { |*x| }` or `proc { |**x| }` + if listener.respond_to?(:parameters) + params = listener.parameters + if params.length == 1 && params.first.first == :opt + subscriber_class = EventObject + end + end end + wrap_all pattern, subscriber_class.new(pattern, listener) + end + + def self.event_object_subscriber(pattern, block) + wrap_all pattern, EventObject.new(pattern, block) + end + + def self.wrap_all(pattern, subscriber) unless pattern AllMessages.new(subscriber) else @@ -130,6 +147,27 @@ module ActiveSupport end end + class EventObject < Evented + def start(name, id, payload) + stack = Thread.current[:_event_stack] ||= [] + event = build_event name, id, payload + event.start! + stack.push event + end + + def finish(name, id, payload) + stack = Thread.current[:_event_stack] + event = stack.pop + event.finish! + @delegate.call event + end + + private + def build_event(name, id, payload) + ActiveSupport::Notifications::Event.new name, nil, nil, id, payload + end + end + class AllMessages # :nodoc: def initialize(delegate) @delegate = delegate diff --git a/activesupport/lib/active_support/notifications/instrumenter.rb b/activesupport/lib/active_support/notifications/instrumenter.rb index e99f5ee688..f8344912bb 100644 --- a/activesupport/lib/active_support/notifications/instrumenter.rb +++ b/activesupport/lib/active_support/notifications/instrumenter.rb @@ -63,6 +63,42 @@ module ActiveSupport @end = ending @children = [] @duration = nil + @cpu_time_start = nil + @cpu_time_finish = nil + @allocation_count_start = 0 + @allocation_count_finish = 0 + end + + # Record information at the time this event starts + def start! + @time = now + @cpu_time_start = now_cpu + @allocation_count_start = now_allocations + end + + # Record information at the time this event finishes + def finish! + @cpu_time_finish = now_cpu + @end = now + @allocation_count_finish = now_allocations + end + + # Returns the CPU time (in milliseconds) passed since the call to + # +start!+ and the call to +finish!+ + def cpu_time + (@cpu_time_finish - @cpu_time_start) * 1000 + end + + # Returns the idle time time (in milliseconds) passed since the call to + # +start!+ and the call to +finish!+ + def idle_time + duration - cpu_time + end + + # Returns the number of allocations made since the call to +start!+ and + # the call to +finish!+ + def allocations + @allocation_count_finish - @allocation_count_start end # Returns the difference in milliseconds between when the execution of the @@ -88,6 +124,31 @@ module ActiveSupport def parent_of?(event) @children.include? event end + + private + def now + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end + + if defined?(Process::CLOCK_PROCESS_CPUTIME_ID) + def now_cpu + Process.clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID) + end + else + def now_cpu + 0 + end + end + + if defined?(JRUBY_VERSION) + def now_allocations + 0 + end + else + def now_allocations + GC.stat :total_allocated_objects + end + end end end end diff --git a/activesupport/lib/active_support/subscriber.rb b/activesupport/lib/active_support/subscriber.rb index 8ad39f7a05..5a4c3d74af 100644 --- a/activesupport/lib/active_support/subscriber.rb +++ b/activesupport/lib/active_support/subscriber.rb @@ -79,7 +79,8 @@ module ActiveSupport end def start(name, id, payload) - e = ActiveSupport::Notifications::Event.new(name, Time.now, nil, id, payload) + e = ActiveSupport::Notifications::Event.new(name, nil, nil, id, payload) + e.start! parent = event_stack.last parent << e if parent @@ -87,9 +88,8 @@ module ActiveSupport end def finish(name, id, payload) - finished = Time.now - event = event_stack.pop - event.end = finished + event = event_stack.pop + event.finish! event.payload.merge!(payload) method = name.split(".".freeze).first @@ -97,7 +97,6 @@ module ActiveSupport end private - def event_stack SubscriberQueueRegistry.instance.get_queue(@queue_key) end diff --git a/activesupport/lib/active_support/testing/deprecation.rb b/activesupport/lib/active_support/testing/deprecation.rb index f655435729..18d63d2780 100644 --- a/activesupport/lib/active_support/testing/deprecation.rb +++ b/activesupport/lib/active_support/testing/deprecation.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "active_support/deprecation" -require "active_support/core_ext/regexp" module ActiveSupport module Testing diff --git a/activesupport/test/deprecation/method_wrappers_test.rb b/activesupport/test/deprecation/method_wrappers_test.rb index 439e117c1d..b04bce7a11 100644 --- a/activesupport/test/deprecation/method_wrappers_test.rb +++ b/activesupport/test/deprecation/method_wrappers_test.rb @@ -55,4 +55,39 @@ class MethodWrappersTest < ActiveSupport::TestCase assert(@klass.private_method_defined?(:old_private_method)) end + + def test_deprecate_class_method + mod = Module.new do + extend self + + def old_method + "abc" + end + end + ActiveSupport::Deprecation.deprecate_methods(mod, old_method: :new_method) + + warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/ + assert_deprecated(warning) { assert_equal "abc", mod.old_method } + end + + def test_deprecate_method_when_class_extends_module + mod = Module.new do + def old_method + "abc" + end + end + @klass.extend mod + ActiveSupport::Deprecation.deprecate_methods(mod, old_method: :new_method) + + warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/ + assert_deprecated(warning) { assert_equal "abc", @klass.old_method } + end + + def test_method_with_without_deprecation_is_exposed + ActiveSupport::Deprecation.deprecate_methods(@klass, old_method: :new_method) + + warning = /old_method is deprecated and will be removed from Rails \d.\d \(use new_method instead\)/ + assert_deprecated(warning) { assert_equal "abc", @klass.new.old_method_with_deprecation } + assert_equal "abc", @klass.new.old_method_without_deprecation + end end diff --git a/activesupport/test/json/encoding_test.rb b/activesupport/test/json/encoding_test.rb index 340a2abf75..ebc5df14dd 100644 --- a/activesupport/test/json/encoding_test.rb +++ b/activesupport/test/json/encoding_test.rb @@ -3,7 +3,6 @@ require "securerandom" require "abstract_unit" require "active_support/core_ext/string/inflections" -require "active_support/core_ext/regexp" require "active_support/json" require "active_support/time" require "time_zone_test_helpers" diff --git a/activesupport/test/log_subscriber_test.rb b/activesupport/test/log_subscriber_test.rb index 2af9b1de30..7f05459493 100644 --- a/activesupport/test/log_subscriber_test.rb +++ b/activesupport/test/log_subscriber_test.rb @@ -75,6 +75,22 @@ class SyncLogSubscriberTest < ActiveSupport::TestCase assert_kind_of ActiveSupport::Notifications::Event, @log_subscriber.event end + def test_event_attributes + ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber + instrument "some_event.my_log_subscriber" + wait + event = @log_subscriber.event + if defined?(JRUBY_VERSION) + assert_equal 0, event.cpu_time + assert_equal 0, event.allocations + else + assert_operator event.cpu_time, :>, 0 + assert_operator event.allocations, :>, 0 + end + assert_operator event.duration, :>, 0 + assert_operator event.idle_time, :>, 0 + end + def test_does_not_send_the_event_if_it_doesnt_match_the_class ActiveSupport::LogSubscriber.attach_to :my_log_subscriber, @log_subscriber instrument "unknown_event.my_log_subscriber" diff --git a/activesupport/test/notifications_test.rb b/activesupport/test/notifications_test.rb index d035f993f7..62817a839a 100644 --- a/activesupport/test/notifications_test.rb +++ b/activesupport/test/notifications_test.rb @@ -26,6 +26,42 @@ module Notifications end end + class SubscribeEventObjects < TestCase + def test_subscribe_events + events = [] + @notifier.subscribe do |event| + events << event + end + + ActiveSupport::Notifications.instrument("foo") + event = events.first + assert event, "should have an event" + assert_operator event.allocations, :>, 0 + assert_operator event.cpu_time, :>, 0 + assert_operator event.idle_time, :>, 0 + assert_operator event.duration, :>, 0 + end + + def test_subscribe_via_top_level_api + old_notifier = ActiveSupport::Notifications.notifier + ActiveSupport::Notifications.notifier = ActiveSupport::Notifications::Fanout.new + + event = nil + ActiveSupport::Notifications.subscribe("foo") do |e| + event = e + end + + ActiveSupport::Notifications.instrument("foo") do + 100.times { Object.new } # allocate at least 100 objects + end + + assert event + assert_operator event.allocations, :>=, 100 + ensure + ActiveSupport::Notifications.notifier = old_notifier + end + end + class SubscribedTest < TestCase def test_subscribed name = "foo" diff --git a/ci/custom_cops/bin/test b/ci/custom_cops/bin/test deleted file mode 100755 index 495ffec83a..0000000000 --- a/ci/custom_cops/bin/test +++ /dev/null @@ -1,5 +0,0 @@ -#!/usr/bin/env ruby -# frozen_string_literal: true - -COMPONENT_ROOT = File.expand_path("..", __dir__) -require_relative "../../../tools/test" diff --git a/ci/custom_cops/lib/custom_cops.rb b/ci/custom_cops/lib/custom_cops.rb deleted file mode 100644 index 157b8247e4..0000000000 --- a/ci/custom_cops/lib/custom_cops.rb +++ /dev/null @@ -1,4 +0,0 @@ -# frozen_string_literal: true - -require_relative "custom_cops/refute_not" -require_relative "custom_cops/assert_not" diff --git a/ci/custom_cops/lib/custom_cops/assert_not.rb b/ci/custom_cops/lib/custom_cops/assert_not.rb deleted file mode 100644 index 8b49d3eac2..0000000000 --- a/ci/custom_cops/lib/custom_cops/assert_not.rb +++ /dev/null @@ -1,40 +0,0 @@ -# frozen_string_literal: true - -module CustomCops - # Enforces the use of `assert_not` over `assert !`. - # - # @example - # # bad - # assert !x - # assert ! x - # - # # good - # assert_not x - # - class AssertNot < RuboCop::Cop::Cop - MSG = "Prefer `assert_not` over `assert !`" - - def_node_matcher :offensive?, "(send nil? :assert (send ... :!) ...)" - - def on_send(node) - add_offense(node) if offensive?(node) - end - - def autocorrect(node) - expression = node.loc.expression - - ->(corrector) do - corrector.replace( - expression, - corrected_source(expression.source) - ) - end - end - - private - - def corrected_source(source) - source.gsub(/^assert(\(| ) *! */, "assert_not\\1") - end - end -end diff --git a/ci/custom_cops/lib/custom_cops/refute_not.rb b/ci/custom_cops/lib/custom_cops/refute_not.rb deleted file mode 100644 index 3e89e0fd32..0000000000 --- a/ci/custom_cops/lib/custom_cops/refute_not.rb +++ /dev/null @@ -1,71 +0,0 @@ -# frozen_string_literal: true - -module CustomCops - # Enforces the use of `#assert_not` methods over `#refute` methods. - # - # @example - # # bad - # refute false - # refute_empty [1, 2, 3] - # refute_equal true, false - # - # # good - # assert_not false - # assert_not_empty [1, 2, 3] - # assert_not_equal true, false - # - class RefuteNot < RuboCop::Cop::Cop - MSG = "Prefer `%<assert_method>s` over `%<refute_method>s`" - - CORRECTIONS = { - refute: "assert_not", - refute_empty: "assert_not_empty", - refute_equal: "assert_not_equal", - refute_in_delta: "assert_not_in_delta", - refute_in_epsilon: "assert_not_in_epsilon", - refute_includes: "assert_not_includes", - refute_instance_of: "assert_not_instance_of", - refute_kind_of: "assert_not_kind_of", - refute_nil: "assert_not_nil", - refute_operator: "assert_not_operator", - refute_predicate: "assert_not_predicate", - refute_respond_to: "assert_not_respond_to", - refute_same: "assert_not_same", - refute_match: "assert_no_match" - }.freeze - - OFFENSIVE_METHODS = CORRECTIONS.keys.freeze - - def_node_matcher :offensive?, "(send nil? #offensive_method? ...)" - - def on_send(node) - return unless offensive?(node) - - message = offense_message(node.method_name) - add_offense(node, location: :selector, message: message) - end - - def autocorrect(node) - ->(corrector) do - corrector.replace( - node.loc.selector, - CORRECTIONS[node.method_name] - ) - end - end - - private - - def offensive_method?(method_name) - OFFENSIVE_METHODS.include?(method_name) - end - - def offense_message(method_name) - format( - MSG, - refute_method: method_name, - assert_method: CORRECTIONS[method_name] - ) - end - end -end diff --git a/ci/custom_cops/test/custom_cops/assert_not_test.rb b/ci/custom_cops/test/custom_cops/assert_not_test.rb deleted file mode 100644 index abb151aeb4..0000000000 --- a/ci/custom_cops/test/custom_cops/assert_not_test.rb +++ /dev/null @@ -1,42 +0,0 @@ -# frozen_string_literal: true - -require "support/cop_helper" -require_relative "../../lib/custom_cops/assert_not" - -class AssertNotTest < ActiveSupport::TestCase - include CopHelper - - setup do - @cop = CustomCops::AssertNot.new - end - - test "rejects 'assert !'" do - inspect_source @cop, "assert !x" - assert_offense @cop, "^^^^^^^^^ Prefer `assert_not` over `assert !`" - end - - test "rejects 'assert !' with a complex value" do - inspect_source @cop, "assert !a.b(c)" - assert_offense @cop, "^^^^^^^^^^^^^^ Prefer `assert_not` over `assert !`" - end - - test "autocorrects `assert !`" do - corrected = autocorrect_source(@cop, "assert !false") - assert_equal "assert_not false", corrected - end - - test "autocorrects `assert !` with extra spaces" do - corrected = autocorrect_source(@cop, "assert ! false") - assert_equal "assert_not false", corrected - end - - test "autocorrects `assert !` with parentheses" do - corrected = autocorrect_source(@cop, "assert(!false)") - assert_equal "assert_not(false)", corrected - end - - test "accepts `assert_not`" do - inspect_source @cop, "assert_not x" - assert_empty @cop.offenses - end -end diff --git a/ci/custom_cops/test/custom_cops/refute_not_test.rb b/ci/custom_cops/test/custom_cops/refute_not_test.rb deleted file mode 100644 index f0f6eaeda0..0000000000 --- a/ci/custom_cops/test/custom_cops/refute_not_test.rb +++ /dev/null @@ -1,66 +0,0 @@ -# frozen_string_literal: true - -require "support/cop_helper" -require_relative "../../lib/custom_cops/refute_not" - -class RefuteNotTest < ActiveSupport::TestCase - include CopHelper - - setup do - @cop = CustomCops::RefuteNot.new - end - - { - refute: :assert_not, - refute_empty: :assert_not_empty, - refute_equal: :assert_not_equal, - refute_in_delta: :assert_not_in_delta, - refute_in_epsilon: :assert_not_in_epsilon, - refute_includes: :assert_not_includes, - refute_instance_of: :assert_not_instance_of, - refute_kind_of: :assert_not_kind_of, - refute_nil: :assert_not_nil, - refute_operator: :assert_not_operator, - refute_predicate: :assert_not_predicate, - refute_respond_to: :assert_not_respond_to, - refute_same: :assert_not_same, - refute_match: :assert_no_match - }.each do |refute_method, assert_method| - test "rejects `#{refute_method}` with a single argument" do - inspect_source(@cop, "#{refute_method} a") - assert_offense @cop, offense_message(refute_method, assert_method) - end - - test "rejects `#{refute_method}` with multiple arguments" do - inspect_source(@cop, "#{refute_method} a, b, c") - assert_offense @cop, offense_message(refute_method, assert_method) - end - - test "autocorrects `#{refute_method}` with a single argument" do - corrected = autocorrect_source(@cop, "#{refute_method} a") - assert_equal "#{assert_method} a", corrected - end - - test "autocorrects `#{refute_method}` with multiple arguments" do - corrected = autocorrect_source(@cop, "#{refute_method} a, b, c") - assert_equal "#{assert_method} a, b, c", corrected - end - - test "accepts `#{assert_method}` with a single argument" do - inspect_source(@cop, "#{assert_method} a") - assert_empty @cop.offenses - end - - test "accepts `#{assert_method}` with multiple arguments" do - inspect_source(@cop, "#{assert_method} a, b, c") - assert_empty @cop.offenses - end - end - - private - - def offense_message(refute_method, assert_method) - carets = "^" * refute_method.to_s.length - "#{carets} Prefer `#{assert_method}` over `#{refute_method}`" - end -end diff --git a/ci/custom_cops/test/support/cop_helper.rb b/ci/custom_cops/test/support/cop_helper.rb deleted file mode 100644 index c2c6b969dd..0000000000 --- a/ci/custom_cops/test/support/cop_helper.rb +++ /dev/null @@ -1,47 +0,0 @@ -# frozen_string_literal: true - -require "rubocop" - -module CopHelper - def inspect_source(cop, source) - processed_source = parse_source(source) - raise "Error parsing example code" unless processed_source.valid_syntax? - investigate(cop, processed_source) - processed_source - end - - def autocorrect_source(cop, source) - cop.instance_variable_get(:@options)[:auto_correct] = true - processed_source = inspect_source(cop, source) - rewrite(cop, processed_source) - end - - def assert_offense(cop, expected_message) - assert_not_empty( - cop.offenses, - "Expected offense with message \"#{expected_message}\", but got no offense" - ) - - offense = cop.offenses.first - carets = "^" * offense.column_length - - assert_equal expected_message, "#{carets} #{offense.message}" - end - - private - TARGET_RUBY_VERSION = 2.4 - - def parse_source(source) - RuboCop::ProcessedSource.new(source, TARGET_RUBY_VERSION) - end - - def rewrite(cop, processed_source) - RuboCop::Cop::Corrector.new(processed_source.buffer, cop.corrections) - .rewrite - end - - def investigate(cop, processed_source) - RuboCop::Cop::Commissioner.new([cop], [], raise_error: true) - .investigate(processed_source) - end -end diff --git a/guides/Rakefile b/guides/Rakefile index 84e18e0972..4116e6f9cc 100644 --- a/guides/Rakefile +++ b/guides/Rakefile @@ -30,7 +30,7 @@ namespace :guides do unless Kindlerb.kindlegen_available? abort "Please run `setupkindlerb` to install kindlegen" end - unless `convert` =~ /convert/ + unless /convert/.match?(`convert`) abort "Please install ImageMagick" end ENV["KINDLE"] = "1" diff --git a/guides/rails_guides/kindle.rb b/guides/rails_guides/kindle.rb index d370541d2e..8a0361ff4c 100644 --- a/guides/rails_guides/kindle.rb +++ b/guides/rails_guides/kindle.rb @@ -35,7 +35,7 @@ module Kindle def generate_front_matter(html_pages) frontmatter = [] html_pages.delete_if { |x| - if x =~ /(toc|welcome|copyright).html/ + if /(toc|welcome|copyright).html/.match?(x) frontmatter << x unless x =~ /toc/ true end diff --git a/guides/rails_guides/markdown.rb b/guides/rails_guides/markdown.rb index 84f95eec68..61b371363e 100644 --- a/guides/rails_guides/markdown.rb +++ b/guides/rails_guides/markdown.rb @@ -69,7 +69,7 @@ module RailsGuides end def extract_raw_header_and_body - if @raw_body =~ /^\-{40,}$/ + if /^\-{40,}$/.match?(@raw_body) @raw_header, _, @raw_body = @raw_body.partition(/^\-{40,}$/).map(&:strip) end end @@ -89,7 +89,7 @@ module RailsGuides hierarchy = [] doc.children.each do |node| - if node.name =~ /^h[3-6]$/ + if /^h[3-6]$/.match?(node.name) case node.name when "h3" hierarchy = [node] diff --git a/guides/rails_guides/markdown/renderer.rb b/guides/rails_guides/markdown/renderer.rb index 78820a7856..8095b8c898 100644 --- a/guides/rails_guides/markdown/renderer.rb +++ b/guides/rails_guides/markdown/renderer.rb @@ -35,7 +35,7 @@ HTML def paragraph(text) if text =~ %r{^NOTE:\s+Defined\s+in\s+<code>(.*?)</code>\.?$} %(<div class="note"><p>Defined in <code><a href="#{github_file_url($1)}">#{$1}</a></code>.</p></div>) - elsif text =~ /^(TIP|IMPORTANT|CAUTION|WARNING|NOTE|INFO|TODO)[.:]/ + elsif /^(TIP|IMPORTANT|CAUTION|WARNING|NOTE|INFO|TODO)[.:]/.match?(text) convert_notes(text) elsif text.include?("DO NOT READ THIS FILE ON GITHUB") elsif text =~ /^\[<sup>(\d+)\]:<\/sup> (.+)$/ @@ -110,7 +110,7 @@ HTML end def api_link(url) - if url =~ %r{http://api\.rubyonrails\.org/v\d+\.} + if %r{http://api\.rubyonrails\.org/v\d+\.}.match?(url) url elsif edge url.sub("api", "edgeapi") diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 6233708ad5..a2890b9b7a 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -368,7 +368,7 @@ end **`:start`** -By default, records are fetched in ascending order of the primary key, which must be an integer. The `:start` option allows you to configure the first ID of the sequence whenever the lowest ID is not the one you need. This would be useful, for example, if you wanted to resume an interrupted batch process, provided you saved the last processed ID as a checkpoint. +By default, records are fetched in ascending order of the primary key. The `:start` option allows you to configure the first ID of the sequence whenever the lowest ID is not the one you need. This would be useful, for example, if you wanted to resume an interrupted batch process, provided you saved the last processed ID as a checkpoint. For example, to send newsletters only to users with the primary key starting from 2000: diff --git a/guides/source/routing.md b/guides/source/routing.md index 23a5538d9b..8c69e2600b 100644 --- a/guides/source/routing.md +++ b/guides/source/routing.md @@ -506,7 +506,7 @@ resources :photos do end ``` -This will recognize `/photos/1/preview` with GET, and route to the `preview` action of `PhotosController`, with the resource id value passed in `params[:id]`. It will also create the `photo_preview_url` and `photo_preview_path` helpers. +This will recognize `/photos/1/preview` with GET, and route to the `preview` action of `PhotosController`, with the resource id value passed in `params[:id]`. It will also create the `preview_photo_url` and `preview_photo_path` helpers. Within the block of member routes, each route name specifies the HTTP verb will be recognized. You can use `get`, `patch`, `put`, `post`, or `delete` here diff --git a/guides/source/testing.md b/guides/source/testing.md index b3c16f3e7c..01cda8e6e4 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -105,7 +105,7 @@ class ArticleTest < ActiveSupport::TestCase The `ArticleTest` class defines a _test case_ because it inherits from `ActiveSupport::TestCase`. `ArticleTest` thus has all the methods available from `ActiveSupport::TestCase`. Later in this guide, we'll see some of the methods it gives us. Any method defined within a class inherited from `Minitest::Test` -(which is the superclass of `ActiveSupport::TestCase`) that begins with `test_` (case sensitive) is simply called a test. So, methods defined as `test_password` and `test_valid_password` are legal test names and are run automatically when the test case is run. +(which is the superclass of `ActiveSupport::TestCase`) that begins with `test_` is simply called a test. So, methods defined as `test_password` and `test_valid_password` are legal test names and are run automatically when the test case is run. Rails also adds a `test` method that takes a test name and a block. It generates a normal `Minitest::Unit` test with method names prefixed with `test_`. So you don't have to worry about naming the methods, and you can write something like: diff --git a/railties/lib/rails/app_loader.rb b/railties/lib/rails/app_loader.rb index 20eb75d95c..aabcc5970c 100644 --- a/railties/lib/rails/app_loader.rb +++ b/railties/lib/rails/app_loader.rb @@ -49,7 +49,7 @@ EOS if exe = find_executable contents = File.read(exe) - if contents =~ /(APP|ENGINE)_PATH/ + if /(APP|ENGINE)_PATH/.match?(contents) exec RUBY, exe, *ARGV break # non reachable, hack to be able to stub exec in the test suite elsif exe.end_with?("bin/rails") && contents.include?("This file was generated by Bundler") diff --git a/railties/lib/rails/code_statistics.rb b/railties/lib/rails/code_statistics.rb index 9c447c366f..19d331ff30 100644 --- a/railties/lib/rails/code_statistics.rb +++ b/railties/lib/rails/code_statistics.rb @@ -46,7 +46,7 @@ class CodeStatistics #:nodoc: if File.directory?(path) && (/^\./ !~ file_name) stats.add(calculate_directory_statistics(path, pattern)) - elsif file_name =~ pattern + elsif file_name&.match?(pattern) stats.add_by_file_path(path) end end diff --git a/railties/lib/rails/commands/secrets/secrets_command.rb b/railties/lib/rails/commands/secrets/secrets_command.rb index 3d2c2cc7c6..2eebc0f35f 100644 --- a/railties/lib/rails/commands/secrets/secrets_command.rb +++ b/railties/lib/rails/commands/secrets/secrets_command.rb @@ -42,7 +42,7 @@ module Rails rescue Rails::Secrets::MissingKeyError => error say error.message rescue Errno::ENOENT => error - if error.message =~ /secrets\.yml\.enc/ + if /secrets\.yml\.enc/.match?(error.message) deprecate_in_favor_of_credentials_and_exit else raise diff --git a/railties/lib/rails/generators/app_base.rb b/railties/lib/rails/generators/app_base.rb index f51542f3ec..985e9ab263 100644 --- a/railties/lib/rails/generators/app_base.rb +++ b/railties/lib/rails/generators/app_base.rb @@ -376,7 +376,7 @@ module Rails comment = "See https://github.com/rails/execjs#readme for more supported runtimes" if defined?(JRUBY_VERSION) GemfileEntry.version "therubyrhino", nil, comment - elsif RUBY_PLATFORM =~ /mingw|mswin/ + elsif RUBY_PLATFORM.match?(/mingw|mswin/) GemfileEntry.version "duktape", nil, comment else GemfileEntry.new "mini_racer", nil, comment, { platforms: :ruby }, true diff --git a/railties/lib/rails/generators/rails/app/app_generator.rb b/railties/lib/rails/generators/rails/app/app_generator.rb index d5d87f3dfd..83c5c9f297 100644 --- a/railties/lib/rails/generators/rails/app/app_generator.rb +++ b/railties/lib/rails/generators/rails/app/app_generator.rb @@ -515,7 +515,7 @@ module Rails end def valid_const? - if app_const =~ /^\d/ + if /^\d/.match?(app_const) raise Error, "Invalid application name #{original_app_name}. Please give a name which does not start with numbers." elsif RESERVED_NAMES.include?(original_app_name) raise Error, "Invalid application name #{original_app_name}. Please give a " \ diff --git a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb index a83c911806..8cc42325bb 100644 --- a/railties/lib/rails/generators/rails/plugin/plugin_generator.rb +++ b/railties/lib/rails/generators/rails/plugin/plugin_generator.rb @@ -385,11 +385,11 @@ task default: :test end def valid_const? - if original_name =~ /-\d/ + if /-\d/.match?(original_name) raise Error, "Invalid plugin name #{original_name}. Please give a name which does not contain a namespace starting with numeric characters." - elsif original_name =~ /[^\w-]+/ + elsif /[^\w-]+/.match?(original_name) raise Error, "Invalid plugin name #{original_name}. Please give a name which uses only alphabetic, numeric, \"_\" or \"-\" characters." - elsif camelized =~ /^\d/ + elsif /^\d/.match?(camelized) raise Error, "Invalid plugin name #{original_name}. Please give a name which does not start with numbers." elsif RESERVED_NAMES.include?(name) raise Error, "Invalid plugin name #{original_name}. Please give a " \ diff --git a/railties/lib/rails/rack/logger.rb b/railties/lib/rails/rack/logger.rb index 4ea7e40319..3a95b55811 100644 --- a/railties/lib/rails/rack/logger.rb +++ b/railties/lib/rails/rack/logger.rb @@ -50,7 +50,7 @@ module Rails 'Started %s "%s" for %s at %s' % [ request.request_method, request.filtered_path, - request.ip, + request.remote_ip, Time.now.to_default_s ] end diff --git a/railties/lib/rails/test_unit/runner.rb b/railties/lib/rails/test_unit/runner.rb index de5744c662..2fa7573bdf 100644 --- a/railties/lib/rails/test_unit/runner.rb +++ b/railties/lib/rails/test_unit/runner.rb @@ -63,7 +63,7 @@ module Rails # Extract absolute and relative paths but skip -n /.*/ regexp filters. argv.select { |arg| arg =~ %r%^/?\w+/% && !arg.end_with?("/") }.map do |path| case - when path =~ /(:\d+)+$/ + when /(:\d+)+$/.match?(path) file, *lines = path.split(":") filters << [ file, lines ] file diff --git a/railties/test/application/rack/logger_test.rb b/railties/test/application/rack/logger_test.rb index d949a48366..ea425d5fa5 100644 --- a/railties/test/application/rack/logger_test.rb +++ b/railties/test/application/rack/logger_test.rb @@ -53,6 +53,12 @@ module ApplicationTests wait assert_match 'Started HEAD "/"', logs end + + test "logger logs correct remote IP address" do + get "/", {}, { "REMOTE_ADDR" => "127.0.0.1", "HTTP_X_FORWARDED_FOR" => "1.2.3.4" } + wait + assert_match 'Started GET "/" for 1.2.3.4', logs + end end end end diff --git a/tasks/release.rb b/tasks/release.rb index cbda9a3798..f13342b90c 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -89,7 +89,7 @@ npm_version = version.gsub(/\./).with_index { |s, i| i >= 2 ? "-" : s } if File.exist?("#{framework}/package.json") Dir.chdir("#{framework}") do - npm_tag = version =~ /[a-z]/ ? "pre" : "latest" + npm_tag = /[a-z]/.match?(version) ? "pre" : "latest" sh "npm publish --tag #{npm_tag}" end end |