diff options
57 files changed, 397 insertions, 418 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 3e3b963a47..e93e7ff82f 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/**/*' @@ -179,3 +184,12 @@ Style/Semicolon: # Prefer Foo.method over Foo::method Style/ColonMethodCall: Enabled: true + +Style/TrivialAccessors: + Enabled: true + +Performance/FlatMap: + Enabled: true + +Performance/RedundantMerge: + 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) diff --git a/actioncable/test/javascript/vendor/mock-socket.js b/actioncable/test/javascript/vendor/mock-socket.js index b465c8b53f..4564cebc40 100644 --- a/actioncable/test/javascript/vendor/mock-socket.js +++ b/actioncable/test/javascript/vendor/mock-socket.js @@ -178,7 +178,7 @@ if (root.IPv6 === this) { root.IPv6 = _IPv6; } - + return this; } @@ -461,7 +461,6 @@ }(this, function (punycode, IPv6, SLD, root) { 'use strict'; /*global location, escape, unescape */ - // FIXME: v2.0.0 renamce non-camelCase properties to uppercase /*jshint camelcase: false */ // save current URI variable, if any diff --git a/actionmailer/lib/action_mailer/inline_preview_interceptor.rb b/actionmailer/lib/action_mailer/inline_preview_interceptor.rb index 8a12f805cc..2b97ac5b94 100644 --- a/actionmailer/lib/action_mailer/inline_preview_interceptor.rb +++ b/actionmailer/lib/action_mailer/inline_preview_interceptor.rb @@ -40,9 +40,7 @@ module ActionMailer end private - def message - @message - end + attr_reader :message def html_part @html_part ||= message.html_part diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index a6f395d33b..7af29f8dca 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -793,9 +793,7 @@ module ActionController protected attr_reader :parameters - def permitted=(new_permitted) - @permitted = new_permitted - end + attr_writer :permitted def fields_for_style? @parameters.all? { |k, v| k =~ /\A-?\d+\z/ && (v.is_a?(Hash) || v.is_a?(Parameters)) } diff --git a/actionpack/test/controller/http_digest_authentication_test.rb b/actionpack/test/controller/http_digest_authentication_test.rb index 3f211cd60d..b133afb343 100644 --- a/actionpack/test/controller/http_digest_authentication_test.rb +++ b/actionpack/test/controller/http_digest_authentication_test.rb @@ -272,7 +272,7 @@ class HttpDigestAuthenticationTest < ActionController::TestCase credentials.merge!(options) path_info = @request.env["PATH_INFO"].to_s uri = options[:uri] || path_info - credentials.merge!(uri: uri) + credentials[:uri] = uri @request.env["ORIGINAL_FULLPATH"] = path_info ActionController::HttpAuthentication::Digest.encode_credentials(method, credentials, password, options[:password_is_ha1]) end diff --git a/actionpack/test/controller/log_subscriber_test.rb b/actionpack/test/controller/log_subscriber_test.rb index be455642de..0562c16284 100644 --- a/actionpack/test/controller/log_subscriber_test.rb +++ b/actionpack/test/controller/log_subscriber_test.rb @@ -82,9 +82,7 @@ module Another @last_payload = payload end - def last_payload - @last_payload - end + attr_reader :last_payload end end diff --git a/actionpack/test/controller/resources_test.rb b/actionpack/test/controller/resources_test.rb index 3688fdbeee..d336b96eff 100644 --- a/actionpack/test/controller/resources_test.rb +++ b/actionpack/test/controller/resources_test.rb @@ -1322,7 +1322,7 @@ class ResourcesTest < ActionController::TestCase def assert_resource_allowed_routes(controller, options, shallow_options, allowed, not_allowed, path = controller) shallow_path = "#{path}/#{shallow_options[:id]}" format = options[:format] && ".#{options[:format]}" - options.merge!(controller: controller) + options[:controller] = controller shallow_options.merge!(options) assert_whether_allowed(allowed, not_allowed, options, "index", "#{path}#{format}", :get) @@ -1336,7 +1336,7 @@ class ResourcesTest < ActionController::TestCase def assert_singleton_resource_allowed_routes(controller, options, allowed, not_allowed, path = controller.singularize) format = options[:format] && ".#{options[:format]}" - options.merge!(controller: controller) + options[:controller] = controller assert_whether_allowed(allowed, not_allowed, options, "new", "#{path}/new#{format}", :get) assert_whether_allowed(allowed, not_allowed, options, "create", "#{path}#{format}", :post) diff --git a/actionpack/test/dispatch/header_test.rb b/actionpack/test/dispatch/header_test.rb index 3a265a056b..bd2a5b35fb 100644 --- a/actionpack/test/dispatch/header_test.rb +++ b/actionpack/test/dispatch/header_test.rb @@ -156,7 +156,7 @@ class HeaderTest < ActiveSupport::TestCase env = { "HTTP_REFERER" => "/" } headers = make_headers(env) headers["Referer"] = "http://example.com/" - headers.merge! "CONTENT_TYPE" => "text/plain" + headers["CONTENT_TYPE"] = "text/plain" assert_equal({ "HTTP_REFERER" => "http://example.com/", "CONTENT_TYPE" => "text/plain" }, env) end diff --git a/actionview/test/template/form_helper/form_with_test.rb b/actionview/test/template/form_helper/form_with_test.rb index f07e494ee3..c30176349c 100644 --- a/actionview/test/template/form_helper/form_with_test.rb +++ b/actionview/test/template/form_helper/form_with_test.rb @@ -318,7 +318,8 @@ class FormWithActsLikeFormForTest < FormWithTest @url_for_options = object if object.is_a?(Hash) && object[:use_route].blank? && object[:controller].blank? - object.merge!(controller: "main", action: "index") + object[:controller] = "main" + object[:action] = "index" end super diff --git a/actionview/test/template/form_helper_test.rb b/actionview/test/template/form_helper_test.rb index 5244204e42..38bb1e1d24 100644 --- a/actionview/test/template/form_helper_test.rb +++ b/actionview/test/template/form_helper_test.rb @@ -168,7 +168,8 @@ class FormHelperTest < ActionView::TestCase @url_for_options = object if object.is_a?(Hash) && object[:use_route].blank? && object[:controller].blank? - object.merge!(controller: "main", action: "index") + object[:controller] = "main" + object[:action] = "index" end super diff --git a/activemodel/test/cases/dirty_test.rb b/activemodel/test/cases/dirty_test.rb index b120e68027..b38d84fff2 100644 --- a/activemodel/test/cases/dirty_test.rb +++ b/activemodel/test/cases/dirty_test.rb @@ -14,37 +14,23 @@ class DirtyTest < ActiveModel::TestCase @status = "initialized" end - def name - @name - end + attr_reader :name, :color, :size, :status def name=(val) name_will_change! @name = val end - def color - @color - end - def color=(val) color_will_change! unless val == @color @color = val end - def size - @size - end - def size=(val) attribute_will_change!(:size) unless val == @size @size = val end - def status - @status - end - def status=(val) status_will_change! unless val == @status @status = val diff --git a/activerecord/lib/active_record/connection_adapters/connection_specification.rb b/activerecord/lib/active_record/connection_adapters/connection_specification.rb index 901717ae3d..204691006c 100644 --- a/activerecord/lib/active_record/connection_adapters/connection_specification.rb +++ b/activerecord/lib/active_record/connection_adapters/connection_specification.rb @@ -57,9 +57,7 @@ module ActiveRecord private - def uri - @uri - end + attr_reader :uri def uri_parser @uri_parser ||= URI::Parser.new diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb index 2087938d7c..1cf210d85b 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -36,7 +36,7 @@ module ActiveRecord end indexes.last[-2] << row[:Column_name] - indexes.last[-1][:lengths].merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] + indexes.last[-1][:lengths][row[:Column_name]] = row[:Sub_part].to_i if row[:Sub_part] indexes.last[-1][:orders].merge!(row[:Column_name] => :desc) if row[:Collation] == "D" end end diff --git a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb index 4c57bd48ab..544d720428 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql2_adapter.rb @@ -117,7 +117,7 @@ module ActiveRecord end def configure_connection - @connection.query_options.merge!(as: :array) + @connection.query_options[:as] = :array super end 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/tasks/mysql_database_tasks.rb b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb index e697fa6def..eddc6fa223 100644 --- a/activerecord/lib/active_record/tasks/mysql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/mysql_database_tasks.rb @@ -68,9 +68,7 @@ module ActiveRecord private - def configuration - @configuration - end + attr_reader :configuration def configuration_without_database configuration.merge("database" => nil) diff --git a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb index 647e066137..533e6953a4 100644 --- a/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/postgresql_database_tasks.rb @@ -90,9 +90,7 @@ module ActiveRecord private - def configuration - @configuration - end + attr_reader :configuration def encoding configuration["encoding"] || DEFAULT_ENCODING diff --git a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb index dfe599c4dd..d0bad05176 100644 --- a/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb +++ b/activerecord/lib/active_record/tasks/sqlite_database_tasks.rb @@ -60,13 +60,7 @@ module ActiveRecord private - def configuration - @configuration - end - - def root - @root - end + attr_reader :configuration, :root def run_cmd(cmd, args, out) fail run_cmd_error(cmd, args) unless Kernel.system(cmd, *args, out: out) 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/tasks/database_tasks_test.rb b/activerecord/test/cases/tasks/database_tasks_test.rb index b9cc08c446..a6efe3fa5e 100644 --- a/activerecord/test/cases/tasks/database_tasks_test.rb +++ b/activerecord/test/cases/tasks/database_tasks_test.rb @@ -152,7 +152,7 @@ module ActiveRecord end def test_ignores_configurations_without_databases - @configurations["development"].merge!("database" => nil) + @configurations["development"]["database"] = nil with_stubbed_configurations_establish_connection do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -162,7 +162,7 @@ module ActiveRecord end def test_ignores_remote_databases - @configurations["development"].merge!("host" => "my.server.tld") + @configurations["development"]["host"] = "my.server.tld" with_stubbed_configurations_establish_connection do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -172,7 +172,7 @@ module ActiveRecord end def test_warning_for_remote_databases - @configurations["development"].merge!("host" => "my.server.tld") + @configurations["development"]["host"] = "my.server.tld" with_stubbed_configurations_establish_connection do ActiveRecord::Tasks::DatabaseTasks.create_all @@ -183,7 +183,7 @@ module ActiveRecord end def test_creates_configurations_with_local_ip - @configurations["development"].merge!("host" => "127.0.0.1") + @configurations["development"]["host"] = "127.0.0.1" with_stubbed_configurations_establish_connection do assert_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -193,7 +193,7 @@ module ActiveRecord end def test_creates_configurations_with_local_host - @configurations["development"].merge!("host" => "localhost") + @configurations["development"]["host"] = "localhost" with_stubbed_configurations_establish_connection do assert_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -203,7 +203,7 @@ module ActiveRecord end def test_creates_configurations_with_blank_hosts - @configurations["development"].merge!("host" => nil) + @configurations["development"]["host"] = nil with_stubbed_configurations_establish_connection do assert_called(ActiveRecord::Tasks::DatabaseTasks, :create) do @@ -463,7 +463,7 @@ module ActiveRecord end def test_ignores_configurations_without_databases - @configurations[:development].merge!("database" => nil) + @configurations[:development]["database"] = nil ActiveRecord::Base.stub(:configurations, @configurations) do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -473,7 +473,7 @@ module ActiveRecord end def test_ignores_remote_databases - @configurations[:development].merge!("host" => "my.server.tld") + @configurations[:development]["host"] = "my.server.tld" ActiveRecord::Base.stub(:configurations, @configurations) do assert_not_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -483,7 +483,7 @@ module ActiveRecord end def test_warning_for_remote_databases - @configurations[:development].merge!("host" => "my.server.tld") + @configurations[:development]["host"] = "my.server.tld" ActiveRecord::Base.stub(:configurations, @configurations) do ActiveRecord::Tasks::DatabaseTasks.drop_all @@ -494,7 +494,7 @@ module ActiveRecord end def test_drops_configurations_with_local_ip - @configurations[:development].merge!("host" => "127.0.0.1") + @configurations[:development]["host"] = "127.0.0.1" ActiveRecord::Base.stub(:configurations, @configurations) do assert_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -504,7 +504,7 @@ module ActiveRecord end def test_drops_configurations_with_local_host - @configurations[:development].merge!("host" => "localhost") + @configurations[:development]["host"] = "localhost" ActiveRecord::Base.stub(:configurations, @configurations) do assert_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do @@ -514,7 +514,7 @@ module ActiveRecord end def test_drops_configurations_with_blank_hosts - @configurations[:development].merge!("host" => nil) + @configurations[:development]["host"] = nil ActiveRecord::Base.stub(:configurations, @configurations) do assert_called(ActiveRecord::Tasks::DatabaseTasks, :drop) do 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 b021b5f2d0..fa15e0451d 100644 --- a/activestorage/app/jobs/active_storage/purge_job.rb +++ b/activestorage/app/jobs/active_storage/purge_job.rb @@ -2,7 +2,7 @@ # Provides asynchronous purging of ActiveStorage::Blob records via ActiveStorage::Blob#purge_later. class ActiveStorage::PurgeJob < ActiveStorage::BaseJob - discard_on ActiveRecord::RecordNotFound, ActiveRecord::InvalidForeignKey + discard_on ActiveRecord::RecordNotFound def perform(blob) blob.purge diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index bf87598a66..e7f2615b0f 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -207,6 +207,7 @@ class ActiveStorage::Blob < ActiveRecord::Base def purge destroy delete + rescue ActiveRecord::InvalidForeignKey end # Enqueues an ActiveStorage::PurgeJob to call #purge. This is the recommended way to purge blobs from a transaction, 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/test/models/attached/many_test.rb b/activestorage/test/models/attached/many_test.rb index 334254d099..3b563b3fc8 100644 --- a/activestorage/test/models/attached/many_test.rb +++ b/activestorage/test/models/attached/many_test.rb @@ -468,6 +468,33 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "purging attachment with shared blobs" do + [ + create_blob(filename: "funky.jpg"), + create_blob(filename: "town.jpg"), + create_blob(filename: "worm.jpg") + ].tap do |blobs| + @user.highlights.attach blobs + assert @user.highlights.attached? + + another_user = User.create!(name: "John") + shared_blobs = [blobs.second, blobs.third] + another_user.highlights.attach shared_blobs + assert another_user.highlights.attached? + + @user.highlights.purge + assert_not @user.highlights.attached? + + assert_not ActiveStorage::Blob.exists?(blobs.first.id) + assert ActiveStorage::Blob.exists?(blobs.second.id) + assert ActiveStorage::Blob.exists?(blobs.third.id) + + assert_not ActiveStorage::Blob.service.exist?(blobs.first.key) + assert ActiveStorage::Blob.service.exist?(blobs.second.key) + assert ActiveStorage::Blob.service.exist?(blobs.third.key) + end + end + test "purging later" do [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| @user.highlights.attach blobs @@ -485,6 +512,35 @@ class ActiveStorage::ManyAttachedTest < ActiveSupport::TestCase end end + test "purging attachment later with shared blobs" do + [ + create_blob(filename: "funky.jpg"), + create_blob(filename: "town.jpg"), + create_blob(filename: "worm.jpg") + ].tap do |blobs| + @user.highlights.attach blobs + assert @user.highlights.attached? + + another_user = User.create!(name: "John") + shared_blobs = [blobs.second, blobs.third] + another_user.highlights.attach shared_blobs + assert another_user.highlights.attached? + + perform_enqueued_jobs do + @user.highlights.purge_later + end + + assert_not @user.highlights.attached? + assert_not ActiveStorage::Blob.exists?(blobs.first.id) + assert ActiveStorage::Blob.exists?(blobs.second.id) + assert ActiveStorage::Blob.exists?(blobs.third.id) + + assert_not ActiveStorage::Blob.service.exist?(blobs.first.key) + assert ActiveStorage::Blob.service.exist?(blobs.second.key) + assert ActiveStorage::Blob.service.exist?(blobs.third.key) + end + end + test "purging dependent attachment later on destroy" do [ create_blob(filename: "funky.jpg"), create_blob(filename: "town.jpg") ].tap do |blobs| @user.highlights.attach blobs diff --git a/activestorage/test/models/attached/one_test.rb b/activestorage/test/models/attached/one_test.rb index 3333fd9323..561c3e9d23 100644 --- a/activestorage/test/models/attached/one_test.rb +++ b/activestorage/test/models/attached/one_test.rb @@ -412,6 +412,22 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase end end + test "purging an attachment with a shared blob" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + assert @user.avatar.attached? + + another_user = User.create!(name: "John") + another_user.avatar.attach blob + assert another_user.avatar.attached? + + @user.avatar.purge + assert_not @user.avatar.attached? + assert ActiveStorage::Blob.exists?(blob.id) + assert ActiveStorage::Blob.service.exist?(blob.key) + end + end + test "purging later" do create_blob(filename: "funky.jpg").tap do |blob| @user.avatar.attach blob @@ -427,6 +443,25 @@ class ActiveStorage::OneAttachedTest < ActiveSupport::TestCase end end + test "purging an attachment later with shared blob" do + create_blob(filename: "funky.jpg").tap do |blob| + @user.avatar.attach blob + assert @user.avatar.attached? + + another_user = User.create!(name: "John") + another_user.avatar.attach blob + assert another_user.avatar.attached? + + perform_enqueued_jobs do + @user.avatar.purge_later + end + + assert_not @user.avatar.attached? + assert ActiveStorage::Blob.exists?(blob.id) + assert ActiveStorage::Blob.service.exist?(blob.key) + end + end + test "purging dependent attachment later on destroy" do create_blob(filename: "funky.jpg").tap do |blob| @user.avatar.attach blob diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index c2e7aae13a..88c106a08b 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -174,10 +174,10 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase assert_not ActiveStorage::Blob.service.exist?(variant.key) end - test "purge fails when attachments exist" do + test "purge does nothing when attachments exist" do create_blob.tap do |blob| User.create! name: "DHH", avatar: blob - assert_raises(ActiveRecord::InvalidForeignKey) { blob.purge } + assert_no_difference(-> { ActiveStorage::Blob.count }) { blob.purge } assert ActiveStorage::Blob.service.exist?(blob.key) 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/callbacks.rb b/activesupport/lib/active_support/callbacks.rb index a1b841ec3d..c266b432c0 100644 --- a/activesupport/lib/active_support/callbacks.rb +++ b/activesupport/lib/active_support/callbacks.rb @@ -497,9 +497,7 @@ module ActiveSupport arg.halted || !@user_conditions.all? { |c| c.call(arg.target, arg.value) } end - def nested - @nested - end + attr_reader :nested def final? !@call_template @@ -578,7 +576,7 @@ module ActiveSupport end protected - def chain; @chain; end + attr_reader :chain private diff --git a/activesupport/lib/active_support/message_encryptor.rb b/activesupport/lib/active_support/message_encryptor.rb index 8b73270894..404404cad1 100644 --- a/activesupport/lib/active_support/message_encryptor.rb +++ b/activesupport/lib/active_support/message_encryptor.rb @@ -210,9 +210,7 @@ module ActiveSupport OpenSSL::Cipher.new(@cipher) end - def verifier - @verifier - end + attr_reader :verifier def aead_mode? @aead_mode ||= new_cipher.authenticated? 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..455b7a44a6 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,25 @@ module ActiveSupport def parent_of?(event) @children.include? event end + + private + def now + Process.clock_gettime(Process::CLOCK_MONOTONIC) + end + + def now_cpu + Process.clock_gettime(Process::CLOCK_PROCESS_CPUTIME_ID) + 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/parallelization.rb b/activesupport/lib/active_support/testing/parallelization.rb index 59c8486f41..1caac1feb3 100644 --- a/activesupport/lib/active_support/testing/parallelization.rb +++ b/activesupport/lib/active_support/testing/parallelization.rb @@ -26,25 +26,21 @@ module ActiveSupport def pop; @queue.pop; end end - @after_fork_hooks = [] + @@after_fork_hooks = [] def self.after_fork_hook(&blk) - @after_fork_hooks << blk + @@after_fork_hooks << blk end - def self.after_fork_hooks - @after_fork_hooks - end + cattr_reader :after_fork_hooks - @run_cleanup_hooks = [] + @@run_cleanup_hooks = [] def self.run_cleanup_hook(&blk) - @run_cleanup_hooks << blk + @@run_cleanup_hooks << blk end - def self.run_cleanup_hooks - @run_cleanup_hooks - end + cattr_reader :run_cleanup_hooks def initialize(queue_size) @queue_size = queue_size diff --git a/activesupport/lib/active_support/values/time_zone.rb b/activesupport/lib/active_support/values/time_zone.rb index 792c88415c..fd07a3a6a2 100644 --- a/activesupport/lib/active_support/values/time_zone.rb +++ b/activesupport/lib/active_support/values/time_zone.rb @@ -265,7 +265,7 @@ module ActiveSupport private def load_country_zones(code) country = TZInfo::Country.get(code) - country.zone_identifiers.map do |tz_id| + country.zone_identifiers.flat_map do |tz_id| if MAPPING.value?(tz_id) MAPPING.inject([]) do |memo, (key, value)| memo << self[key] if value == tz_id @@ -274,7 +274,7 @@ module ActiveSupport else create(tz_id, nil, TZInfo::Timezone.new(tz_id)) end - end.flatten(1).sort! + end.sort! end def zones_map diff --git a/activesupport/test/cache/stores/redis_cache_store_test.rb b/activesupport/test/cache/stores/redis_cache_store_test.rb index 3b873de383..305a2c184d 100644 --- a/activesupport/test/cache/stores/redis_cache_store_test.rb +++ b/activesupport/test/cache/stores/redis_cache_store_test.rb @@ -103,9 +103,7 @@ module ActiveSupport::Cache::RedisCacheStoreTests private def build(**kwargs) - ActiveSupport::Cache::RedisCacheStore.new(driver: DRIVER, **kwargs).tap do |cache| - cache.redis - end + ActiveSupport::Cache::RedisCacheStore.new(driver: DRIVER, **kwargs).tap(&:redis) end end diff --git a/activesupport/test/log_subscriber_test.rb b/activesupport/test/log_subscriber_test.rb index 2af9b1de30..4f413b9627 100644 --- a/activesupport/test/log_subscriber_test.rb +++ b/activesupport/test/log_subscriber_test.rb @@ -75,6 +75,17 @@ 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 + assert_operator event.duration, :>, 0 + assert_operator event.cpu_time, :>, 0 + assert_operator event.idle_time, :>, 0 + assert_operator event.allocations, :>, 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/source/active_storage_overview.md b/guides/source/active_storage_overview.md index 182d784595..6933717c2b 100644 --- a/guides/source/active_storage_overview.md +++ b/guides/source/active_storage_overview.md @@ -36,10 +36,10 @@ files. ## Setup Active Storage uses two tables in your application’s database named -`active_storage_blobs` and `active_storage_attachments`. After upgrading your -application to Rails 5.2, run `rails active_storage:install` to generate a -migration that creates these tables. Use `rails db:migrate` to run the -migration. +`active_storage_blobs` and `active_storage_attachments`. After creating a new +application (or upgrading your application to Rails 5.2), run +`rails active_storage:install` to generate a migration that creates these +tables. Use `rails db:migrate` to run the migration. Declare Active Storage services in `config/storage.yml`. For each service your application uses, provide a name and the requisite configuration. The example diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index e1dbe39137..a4f7e6f601 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -651,7 +651,7 @@ def upload end ``` -Once a file has been uploaded, there are a multitude of potential tasks, ranging from where to store the files (on disk, Amazon S3, etc) and associating them with models to resizing image files and generating thumbnails. The intricacies of this are beyond the scope of this guide, but there are several libraries designed to assist with these. Two of the better known ones are [CarrierWave](https://github.com/jnicklas/carrierwave) and [Paperclip](https://github.com/thoughtbot/paperclip). +Once a file has been uploaded, there are a multitude of potential tasks, ranging from where to store the files (on disk, Amazon S3, etc) and associating them with models to resizing image files and generating thumbnails. [Active Storage](https://guides.rubyonrails.org/active_storage_overview.html) is designed to assist with these tasks. NOTE: If the user has not selected a file the corresponding parameter will be an empty string. diff --git a/railties/lib/rails/application.rb b/railties/lib/rails/application.rb index 76a8ccb8f1..31d0d65a30 100644 --- a/railties/lib/rails/application.rb +++ b/railties/lib/rails/application.rb @@ -373,9 +373,7 @@ module Rails @config ||= Application::Configuration.new(self.class.find_root(self.class.called_from)) end - def config=(configuration) #:nodoc: - @config = configuration - end + attr_writer :config # Returns secrets added to config/secrets.yml. # @@ -413,9 +411,7 @@ module Rails end end - def secrets=(secrets) #:nodoc: - @secrets = secrets - end + attr_writer :secrets # The secret_key_base is used as the input secret to the application's key generator, which in turn # is used to create all MessageVerifiers/MessageEncryptors, including the ones that sign and encrypt cookies. diff --git a/railties/lib/rails/application/configuration.rb b/railties/lib/rails/application/configuration.rb index bba573499d..9c54cc1f37 100644 --- a/railties/lib/rails/application/configuration.rb +++ b/railties/lib/rails/application/configuration.rb @@ -146,9 +146,7 @@ module Rails @debug_exception_response_format || :default end - def debug_exception_response_format=(value) - @debug_exception_response_format = value - end + attr_writer :debug_exception_response_format def paths @paths ||= begin diff --git a/railties/lib/rails/configuration.rb b/railties/lib/rails/configuration.rb index d3a54d9364..e8741a50ba 100644 --- a/railties/lib/rails/configuration.rb +++ b/railties/lib/rails/configuration.rb @@ -79,13 +79,7 @@ module Rails end protected - def operations - @operations - end - - def delete_operations - @delete_operations - end + attr_reader :operations, :delete_operations end class Generators #:nodoc: diff --git a/railties/lib/rails/generators.rb b/railties/lib/rails/generators.rb index 2a41403557..ed672ae48e 100644 --- a/railties/lib/rails/generators.rb +++ b/railties/lib/rails/generators.rb @@ -126,7 +126,7 @@ module Rails ) if ARGV.first == "mailer" - options[:rails].merge!(template_engine: :erb) + options[:rails][:template_engine] = :erb end end diff --git a/railties/lib/rails/generators/actions.rb b/railties/lib/rails/generators/actions.rb index d85bbfb03e..ae395708cb 100644 --- a/railties/lib/rails/generators/actions.rb +++ b/railties/lib/rails/generators/actions.rb @@ -298,7 +298,7 @@ module Rails sudo = options[:sudo] && !Gem.win_platform? ? "sudo " : "" config = { verbose: false } - config.merge!(capture: options[:capture]) if options[:capture] + config[:capture] = options[:capture] if options[:capture] in_root { run("#{sudo}#{extify(executor)} #{command} RAILS_ENV=#{env}", config) } end diff --git a/railties/test/app_loader_test.rb b/railties/test/app_loader_test.rb index 93ed68fabb..0deb1a76df 100644 --- a/railties/test/app_loader_test.rb +++ b/railties/test/app_loader_test.rb @@ -9,12 +9,12 @@ class AppLoaderTest < ActiveSupport::TestCase @loader ||= Class.new do extend Rails::AppLoader - def self.exec_arguments - @exec_arguments - end + class << self + attr_accessor :exec_arguments - def self.exec(*args) - @exec_arguments = args + def exec(*args) + self.exec_arguments = args + end end end end diff --git a/railties/test/commands/console_test.rb b/railties/test/commands/console_test.rb index b7cdb8229e..0b2fe204f8 100644 --- a/railties/test/commands/console_test.rb +++ b/railties/test/commands/console_test.rb @@ -151,7 +151,8 @@ class Rails::ConsoleTest < ActiveSupport::TestCase def build_app(console) mocked_console = Class.new do - attr_reader :sandbox, :console + attr_accessor :sandbox + attr_reader :console def initialize(console) @console = console @@ -161,10 +162,6 @@ class Rails::ConsoleTest < ActiveSupport::TestCase self end - def sandbox=(arg) - @sandbox = arg - end - def load_console end end |