diff options
51 files changed, 747 insertions, 286 deletions
diff --git a/.travis.yml b/.travis.yml index 290e0b5f2b..fd3b3f9002 100644 --- a/.travis.yml +++ b/.travis.yml @@ -20,10 +20,6 @@ addons: sources: - sourceline: "ppa:mc3man/trusty-media" - sourceline: "ppa:ubuntuhandbook1/apps" - packages: - - ffmpeg - - mupdf - - mupdf-tools bundler_args: --without test --jobs 3 --retry 3 before_install: @@ -42,6 +38,7 @@ before_script: # Decodes to e.g. `export VARIABLE=VALUE` - $(base64 --decode <<< "ZXhwb3J0IFNBVUNFX0FDQ0VTU19LRVk9YTAzNTM0M2YtZTkyMi00MGIzLWFhM2MtMDZiM2VhNjM1YzQ4") - $(base64 --decode <<< "ZXhwb3J0IFNBVUNFX1VTRVJOQU1FPXJ1YnlvbnJhaWxz") + - if [[ $GEM = *ast* ]] ; then sudo apt-get update && sudo apt-get -y install ffmpeg mupdf mupdf-tools ; fi script: 'ci/travis.rb' diff --git a/actionpack/lib/action_controller/metal/strong_parameters.rb b/actionpack/lib/action_controller/metal/strong_parameters.rb index ef7c4c4c16..a56ac749f8 100644 --- a/actionpack/lib/action_controller/metal/strong_parameters.rb +++ b/actionpack/lib/action_controller/metal/strong_parameters.rb @@ -335,7 +335,7 @@ module ActionController # the same way as <tt>Hash#each_pair</tt>. def each_pair(&block) @parameters.each_pair do |key, value| - yield key, convert_hashes_to_parameters(key, value) + yield [key, convert_hashes_to_parameters(key, value)] end end alias_method :each, :each_pair diff --git a/actionpack/lib/action_dispatch/http/content_security_policy.rb b/actionpack/lib/action_dispatch/http/content_security_policy.rb index d10d4faf3d..4883e23d24 100644 --- a/actionpack/lib/action_dispatch/http/content_security_policy.rb +++ b/actionpack/lib/action_dispatch/http/content_security_policy.rb @@ -1,5 +1,7 @@ # frozen_string_literal: true +require "active_support/core_ext/object/deep_dup" + module ActionDispatch #:nodoc: class ContentSecurityPolicy class Middleware @@ -110,7 +112,7 @@ module ActionDispatch #:nodoc: end def initialize_copy(other) - @directives = copy_directives(other.directives) + @directives = other.directives.deep_dup end DIRECTIVES.each do |name, directive| @@ -174,10 +176,6 @@ module ActionDispatch #:nodoc: end private - def copy_directives(directives) - directives.transform_values { |sources| sources.map(&:dup) } - end - def apply_mappings(sources) sources.map do |source| case source diff --git a/actionpack/test/controller/parameters/accessors_test.rb b/actionpack/test/controller/parameters/accessors_test.rb index 43cabae7d2..154430d4b0 100644 --- a/actionpack/test/controller/parameters/accessors_test.rb +++ b/actionpack/test/controller/parameters/accessors_test.rb @@ -51,6 +51,14 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each returns key,value array for block with arity 1" do + @params.each do |arg| + assert_kind_of Array, arg + assert_equal "person", arg[0] + assert_kind_of ActionController::Parameters, arg[1] + end + end + test "each_pair carries permitted status" do @params.permit! @params.each_pair { |key, value| assert(value.permitted?) if key == "person" } @@ -60,6 +68,14 @@ class ParametersAccessorsTest < ActiveSupport::TestCase @params.each_pair { |key, value| assert_not(value.permitted?) if key == "person" } end + test "each_pair returns key,value array for block with arity 1" do + @params.each_pair do |arg| + assert_kind_of Array, arg + assert_equal "person", arg[0] + assert_kind_of ActionController::Parameters, arg[1] + end + end + test "empty? returns true when params contains no key/value pairs" do params = ActionController::Parameters.new assert params.empty? diff --git a/actionpack/test/controller/routing_test.rb b/actionpack/test/controller/routing_test.rb index f09051b306..71b01c36a7 100644 --- a/actionpack/test/controller/routing_test.rb +++ b/actionpack/test/controller/routing_test.rb @@ -213,7 +213,7 @@ class LegacyRouteSetTests < ActiveSupport::TestCase assert_equal expected, ActiveSupport::JSON.decode(get(u)) end - def test_regexp_precidence + def test_regexp_precedence rs.draw do get "/whois/:domain", constraints: { domain: /\w+\.[\w\.]+/ }, diff --git a/actionpack/test/dispatch/content_security_policy_test.rb b/actionpack/test/dispatch/content_security_policy_test.rb index 8a1ac066e8..7c4a65a633 100644 --- a/actionpack/test/dispatch/content_security_policy_test.rb +++ b/actionpack/test/dispatch/content_security_policy_test.rb @@ -14,6 +14,15 @@ class ContentSecurityPolicyTest < ActiveSupport::TestCase assert_equal "script-src 'self';", @policy.build end + def test_dup + @policy.img_src :self + @policy.block_all_mixed_content + @policy.upgrade_insecure_requests + @policy.sandbox + copied = @policy.dup + assert_equal copied.build, @policy.build + end + def test_mappings @policy.script_src :data assert_equal "script-src data:;", @policy.build diff --git a/actionpack/test/dispatch/session/cookie_store_test.rb b/actionpack/test/dispatch/session/cookie_store_test.rb index 2c43a8c29f..e34426a471 100644 --- a/actionpack/test/dispatch/session/cookie_store_test.rb +++ b/actionpack/test/dispatch/session/cookie_store_test.rb @@ -8,11 +8,14 @@ require "active_support/messages/rotation_configuration" class CookieStoreTest < ActionDispatch::IntegrationTest SessionKey = "_myapp_session" SessionSecret = "b3c631c314c0bbca50c1b2843150fe33" - Generator = ActiveSupport::LegacyKeyGenerator.new(SessionSecret) + SessionSalt = "authenticated encrypted cookie" + + Generator = ActiveSupport::KeyGenerator.new(SessionSecret, iterations: 1000) Rotations = ActiveSupport::Messages::RotationConfiguration.new - Verifier = ActiveSupport::MessageVerifier.new(SessionSecret, digest: "SHA1") - SignedBar = Verifier.generate(foo: "bar", session_id: SecureRandom.hex(16)) + Encryptor = ActiveSupport::MessageEncryptor.new( + Generator.generate_key(SessionSalt, 32), cipher: "aes-256-gcm", serializer: Marshal + ) class TestController < ActionController::Base def no_session_access @@ -25,12 +28,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def set_session_value session[:foo] = "bar" - render plain: Rack::Utils.escape(Verifier.generate(session.to_hash)) - end - - def set_session_value_expires_in_five_hours - session[:foo] = "bar" - render plain: Rack::Utils.escape(Verifier.generate(session.to_hash, expires_in: 5.hours)) + render body: nil end def get_session_value @@ -72,19 +70,35 @@ class CookieStoreTest < ActionDispatch::IntegrationTest end end + def parse_cookie_from_header + cookie_matches = headers["Set-Cookie"].match(/#{SessionKey}=([^;]+)/) + cookie_matches && cookie_matches[1] + end + + def assert_session_cookie(cookie_string, contents) + assert_includes headers["Set-Cookie"], cookie_string + + session_value = parse_cookie_from_header + session_data = Encryptor.decrypt_and_verify(Rack::Utils.unescape(session_value)) rescue nil + + assert_not_nil session_data, "session failed to decrypt" + assert_equal session_data.slice(*contents.keys), contents + end + def test_setting_session_value with_test_route_set do get "/set_session_value" + assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" end end def test_getting_session_value with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/get_session_value" + assert_response :success assert_equal 'foo: "bar"', response.body end @@ -92,8 +106,9 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_getting_session_id with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/persistent_session_id" + assert_response :success assert_equal 32, response.body.size session_id = response.body @@ -106,8 +121,12 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_disregards_tampered_sessions with_test_route_set do - cookies[SessionKey] = "BAh7BjoIZm9vIghiYXI%3D--123456780" + encryptor = ActiveSupport::MessageEncryptor.new("A" * 32, cipher: "aes-256-gcm", serializer: Marshal) + + cookies[SessionKey] = encryptor.encrypt_and_sign("foo" => "bar", "session_id" => "abc") + get "/get_session_value" + assert_response :success assert_equal "foo: nil", response.body end @@ -135,19 +154,19 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_does_set_secure_cookies_over_https with_test_route_set(secure: true) do get "/set_session_value", headers: { "HTTPS" => "on" } + assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; secure; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; secure; HttpOnly", "foo" => "bar" end end # {:foo=>#<SessionAutoloadTest::Foo bar:"baz">, :session_id=>"ce8b0752a6ab7c7af3cdb8a80e6b9e46"} - SignedSerializedCookie = "BAh7BzoIZm9vbzodU2Vzc2lvbkF1dG9sb2FkVGVzdDo6Rm9vBjoJQGJhciIIYmF6Og9zZXNzaW9uX2lkIiVjZThiMDc1MmE2YWI3YzdhZjNjZGI4YTgwZTZiOWU0Ng==--2bf3af1ae8bd4e52b9ac2099258ace0c380e601c" + EncryptedSerializedCookie = "9RZ2Fij0qLveUwM4s+CCjGqhpjyUC8jiBIf/AiBr9M3TB8xh2vQZtvSOMfN3uf6oYbbpIDHAcOFIEl69FcW1ozQYeSrCLonYCazoh34ZdYskIQfGwCiSYleVXG1OD9Z4jFqeVArw4Ewm0paOOPLbN1rc6A==--I359v/KWdZ1ok0ey--JFFhuPOY7WUo6tB/eP05Aw==" def test_deserializes_unloaded_classes_on_get_id with_test_route_set do with_autoload_path "session_autoload_test" do - cookies[SessionKey] = SignedSerializedCookie + cookies[SessionKey] = EncryptedSerializedCookie get "/get_session_id" assert_response :success assert_equal "id: ce8b0752a6ab7c7af3cdb8a80e6b9e46", response.body, "should auto-load unloaded class" @@ -158,7 +177,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_deserializes_unloaded_classes_on_get_value with_test_route_set do with_autoload_path "session_autoload_test" do - cookies[SessionKey] = SignedSerializedCookie + cookies[SessionKey] = EncryptedSerializedCookie get "/get_session_value" assert_response :success assert_equal 'foo: #<SessionAutoloadTest::Foo bar:"baz">', response.body, "should auto-load unloaded class" @@ -197,8 +216,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest get "/set_session_value" assert_response :success session_payload = response.body - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/call_reset_session" assert_response :success @@ -216,8 +234,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set do get "/set_session_value" assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/get_class_after_reset_session" assert_response :success @@ -239,8 +256,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set do get "/set_session_value" assert_response :success - assert_equal "_myapp_session=#{response.body}; path=/; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; HttpOnly", "foo" => "bar" get "/call_session_clear" assert_response :success @@ -253,7 +269,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_persistent_session_id with_test_route_set do - cookies[SessionKey] = SignedBar + get "/set_session_value" get "/persistent_session_id" assert_response :success assert_equal 32, response.body.size @@ -268,8 +284,7 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def test_setting_session_id_to_nil_is_respected with_test_route_set do - cookies[SessionKey] = SignedBar - + get "/set_session_value" get "/get_session_id" sid = response.body assert_equal 36, sid.size @@ -283,31 +298,53 @@ class CookieStoreTest < ActionDispatch::IntegrationTest with_test_route_set(expire_after: 5.hours) do # First request accesses the session time = Time.local(2008, 4, 24) + Time.stub :now, time do expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - cookies[SessionKey] = SignedBar + get "/set_session_value" - get "/set_session_value_expires_in_five_hours" assert_response :success - - cookie_body = response.body - assert_equal "_myapp_session=#{cookie_body}; path=/; expires=#{expected_expiry}; HttpOnly", - headers["Set-Cookie"] + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" end # Second request does not access the session - time = Time.local(2008, 4, 25) + time = time + 3.hours Time.stub :now, time do expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - cookies[SessionKey] = SignedBar - get "/no_session_access" + assert_response :success + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" + end + end + end + + def test_session_store_with_expire_after_does_not_accept_expired_session + with_test_route_set(expire_after: 5.hours) do + # First request accesses the session + time = Time.local(2017, 11, 12) + + Time.stub :now, time do + expected_expiry = (time + 5.hours).gmtime.strftime("%a, %d %b %Y %H:%M:%S -0000") - assert_equal "_myapp_session=#{cookies[SessionKey]}; path=/; expires=#{expected_expiry}; HttpOnly", - headers["Set-Cookie"] + get "/set_session_value" + get "/get_session_value" + + assert_response :success + assert_equal 'foo: "bar"', response.body + assert_session_cookie "path=/; expires=#{expected_expiry}; HttpOnly", "foo" => "bar" + end + + # Second request is beyond the expiry time and the session is invalidated + time += 5.hours + 1.minute + + Time.stub :now, time do + get "/get_session_value" + + assert_response :success + assert_equal "foo: nil", response.body end end end @@ -347,8 +384,14 @@ class CookieStoreTest < ActionDispatch::IntegrationTest def get(path, *args) args[0] ||= {} args[0][:headers] ||= {} - args[0][:headers]["action_dispatch.key_generator"] ||= Generator - args[0][:headers]["action_dispatch.cookies_rotations"] ||= Rotations + args[0][:headers].tap do |config| + config["action_dispatch.secret_key_base"] = SessionSecret + config["action_dispatch.authenticated_encrypted_cookie_salt"] = SessionSalt + config["action_dispatch.use_authenticated_cookie_encryption"] = true + + config["action_dispatch.key_generator"] ||= Generator + config["action_dispatch.cookies_rotations"] ||= Rotations + end super(path, *args) end diff --git a/activejob/lib/active_job/core.rb b/activejob/lib/active_job/core.rb index c4e12fc518..879746fc01 100644 --- a/activejob/lib/active_job/core.rb +++ b/activejob/lib/active_job/core.rb @@ -97,17 +97,23 @@ module ActiveJob # ==== Examples # # class DeliverWebhookJob < ActiveJob::Base + # attr_writer :attempt_number + # + # def attempt_number + # @attempt_number ||= 0 + # end + # # def serialize - # super.merge('attempt_number' => (@attempt_number || 0) + 1) + # super.merge('attempt_number' => attempt_number + 1) # end # # def deserialize(job_data) # super - # @attempt_number = job_data['attempt_number'] + # self.attempt_number = job_data['attempt_number'] # end # - # rescue_from(TimeoutError) do |exception| - # raise exception if @attempt_number > 5 + # rescue_from(Timeout::Error) do |exception| + # raise exception if attempt_number > 5 # retry_job(wait: 10) # end # end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb index b66009bdc6..0594b4b485 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_definitions.rb @@ -6,7 +6,7 @@ module ActiveRecord # this type are typically created and returned by methods in database # adapters. e.g. ActiveRecord::ConnectionAdapters::MySQL::SchemaStatements#indexes class IndexDefinition # :nodoc: - attr_reader :table, :name, :unique, :columns, :lengths, :orders, :where, :type, :using, :opclass, :comment + attr_reader :table, :name, :unique, :columns, :lengths, :orders, :opclasses, :where, :type, :using, :comment def initialize( table, name, @@ -14,24 +14,33 @@ module ActiveRecord columns = [], lengths: {}, orders: {}, + opclasses: {}, where: nil, type: nil, using: nil, - opclass: {}, comment: nil ) @table = table @name = name @unique = unique @columns = columns - @lengths = lengths - @orders = orders + @lengths = concise_options(lengths) + @orders = concise_options(orders) + @opclasses = concise_options(opclasses) @where = where @type = type @using = using - @opclass = opclass @comment = comment end + + private + def concise_options(options) + if columns.size == options.size && options.values.uniq.size == 1 + options.values.first + else + options + end + end end # Abstract representation of a column definition. Instances of this type @@ -87,6 +96,11 @@ module ActiveRecord options[:primary_key] != default_primary_key end + def validate? + options.fetch(:validate, true) + end + alias validated? validate? + def defined_for?(to_table_ord = nil, to_table: nil, **options) if to_table_ord self.to_table == to_table_ord.to_s @@ -206,6 +220,7 @@ module ActiveRecord :decimal, :float, :integer, + :json, :string, :text, :time, diff --git a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb index 7ae32d0ced..4f58b0242c 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/schema_statements.rb @@ -600,7 +600,7 @@ module ActiveRecord # to provide these in a migration's +change+ method so it can be reverted. # In that case, +type+ and +options+ will be used by #add_column. def remove_column(table_name, column_name, type = nil, options = {}) - execute "ALTER TABLE #{quote_table_name(table_name)} DROP #{quote_column_name(column_name)}" + execute "ALTER TABLE #{quote_table_name(table_name)} #{remove_column_for_alter(table_name, column_name, type, options)}" end # Changes the column's definition according to the new options. @@ -964,6 +964,8 @@ module ActiveRecord # Action that happens <tt>ON DELETE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+ # [<tt>:on_update</tt>] # Action that happens <tt>ON UPDATE</tt>. Valid values are +:nullify+, +:cascade+ and +:restrict+ + # [<tt>:validate</tt>] + # (Postgres only) Specify whether or not the constraint should be validated. Defaults to +true+. def add_foreign_key(from_table, to_table, options = {}) return unless supports_foreign_keys? @@ -1195,17 +1197,18 @@ module ActiveRecord end def add_index_sort_order(quoted_columns, **options) - if order = options[:order] - case order - when Hash - order = order.symbolize_keys - quoted_columns.each { |name, column| column << " #{order[name].upcase}" if order[name].present? } - when String - quoted_columns.each { |name, column| column << " #{order.upcase}" if order.present? } - end + orders = options_for_index_columns(options[:order]) + quoted_columns.each do |name, column| + column << " #{orders[name].upcase}" if orders[name].present? end + end - quoted_columns + def options_for_index_columns(options) + if options.is_a?(Hash) + options.symbolize_keys + else + Hash.new { |hash, column| hash[column] = options } + end end # Overridden by the MySQL adapter for supporting index lengths and by @@ -1363,6 +1366,20 @@ module ActiveRecord options.is_a?(Hash) && options.key?(:name) && options.except(:name, :algorithm).empty? end + def add_column_for_alter(table_name, column_name, type, options = {}) + td = create_table_definition(table_name) + cd = td.new_column_definition(column_name, type, options) + schema_creation.accept(AddColumnDefinition.new(cd)) + end + + def remove_column_for_alter(table_name, column_name, type = nil, options = {}) + "DROP COLUMN #{quote_column_name(column_name)}" + end + + def remove_columns_for_alter(table_name, *column_names) + column_names.map { |column_name| remove_column_for_alter(table_name, column_name) } + end + def insert_versions_sql(versions) sm_table = quote_table_name(ActiveRecord::SchemaMigration.table_name) diff --git a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb index 8993c517a6..fc80d332f9 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_adapter.rb @@ -272,6 +272,11 @@ module ActiveRecord false end + # Does this adapter support creating invalid constraints? + def supports_validate_constraints? + false + end + # Does this adapter support creating foreign key constraints # in the same statement as creating the table? def supports_foreign_keys_in_create? 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 ede8a9c1e2..479131caad 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract_mysql_adapter.rb @@ -299,7 +299,7 @@ module ActiveRecord def bulk_change_table(table_name, operations) #:nodoc: sqls = operations.flat_map do |command, args| table, arguments = args.shift, args - method = :"#{command}_sql" + method = :"#{command}_for_alter" if respond_to?(method, true) send(method, table, *arguments) @@ -372,11 +372,11 @@ module ActiveRecord end def change_column(table_name, column_name, type, options = {}) #:nodoc: - execute("ALTER TABLE #{quote_table_name(table_name)} #{change_column_sql(table_name, column_name, type, options)}") + execute("ALTER TABLE #{quote_table_name(table_name)} #{change_column_for_alter(table_name, column_name, type, options)}") end def rename_column(table_name, column_name, new_column_name) #:nodoc: - execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_sql(table_name, column_name, new_column_name)}") + execute("ALTER TABLE #{quote_table_name(table_name)} #{rename_column_for_alter(table_name, column_name, new_column_name)}") rename_column_indexes(table_name, column_name, new_column_name) end @@ -605,25 +605,6 @@ module ActiveRecord end end - def add_index_length(quoted_columns, **options) - if length = options[:length] - case length - when Hash - length = length.symbolize_keys - quoted_columns.each { |name, column| column << "(#{length[name]})" if length[name].present? } - when Integer - quoted_columns.each { |name, column| column << "(#{length})" } - end - end - - quoted_columns - end - - def add_options_for_index_columns(quoted_columns, **options) - quoted_columns = add_index_length(quoted_columns, options) - super - end - # See https://dev.mysql.com/doc/refman/5.7/en/error-messages-server.html ER_DUP_ENTRY = 1062 ER_NOT_NULL_VIOLATION = 1048 @@ -671,13 +652,7 @@ module ActiveRecord end end - def add_column_sql(table_name, column_name, type, options = {}) - td = create_table_definition(table_name) - cd = td.new_column_definition(column_name, type, options) - schema_creation.accept(AddColumnDefinition.new(cd)) - end - - def change_column_sql(table_name, column_name, type, options = {}) + def change_column_for_alter(table_name, column_name, type, options = {}) column = column_for(table_name, column_name) type ||= column.sql_type @@ -698,7 +673,7 @@ module ActiveRecord schema_creation.accept(ChangeColumnDefinition.new(cd, column.name)) end - def rename_column_sql(table_name, column_name, new_column_name) + def rename_column_for_alter(table_name, column_name, new_column_name) column = column_for(table_name, column_name) options = { default: column.default, @@ -712,31 +687,23 @@ module ActiveRecord schema_creation.accept(ChangeColumnDefinition.new(cd, column.name)) end - def remove_column_sql(table_name, column_name, type = nil, options = {}) - "DROP #{quote_column_name(column_name)}" - end - - def remove_columns_sql(table_name, *column_names) - column_names.map { |column_name| remove_column_sql(table_name, column_name) } - end - - def add_index_sql(table_name, column_name, options = {}) + def add_index_for_alter(table_name, column_name, options = {}) index_name, index_type, index_columns, _, index_algorithm, index_using = add_index_options(table_name, column_name, options) index_algorithm[0, 0] = ", " if index_algorithm.present? "ADD #{index_type} INDEX #{quote_column_name(index_name)} #{index_using} (#{index_columns})#{index_algorithm}" end - def remove_index_sql(table_name, options = {}) + def remove_index_for_alter(table_name, options = {}) index_name = index_name_for_remove(table_name, options) "DROP INDEX #{quote_column_name(index_name)}" end - def add_timestamps_sql(table_name, options = {}) - [add_column_sql(table_name, :created_at, :datetime, options), add_column_sql(table_name, :updated_at, :datetime, options)] + def add_timestamps_for_alter(table_name, options = {}) + [add_column_for_alter(table_name, :created_at, :datetime, options), add_column_for_alter(table_name, :updated_at, :datetime, options)] end - def remove_timestamps_sql(table_name, options = {}) - [remove_column_sql(table_name, :updated_at), remove_column_sql(table_name, :created_at)] + def remove_timestamps_for_alter(table_name, options = {}) + [remove_column_for_alter(table_name, :updated_at), remove_column_for_alter(table_name, :created_at)] end # MySQL is too stupid to create a temporary table for use subquery, so we have diff --git a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb index da25e4863c..2ed4ad16ae 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_definitions.rb @@ -32,10 +32,6 @@ module ActiveRecord args.each { |name| column(name, :longtext, options) } end - def json(*args, **options) - args.each { |name| column(name, :json, options) } - end - def unsigned_integer(*args, **options) args.each { |name| column(name, :unsigned_integer, options) } end 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 a15c7d1787..ce50590651 100644 --- a/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/mysql/schema_statements.rb @@ -22,23 +22,26 @@ module ActiveRecord index_using = mysql_index_type end - indexes << IndexDefinition.new( + indexes << [ row[:Table], row[:Key_name], row[:Non_unique].to_i == 0, + [], + lengths: {}, + orders: {}, type: index_type, using: index_using, comment: row[:Index_comment].presence - ) + ] end - indexes.last.columns << row[:Column_name] - indexes.last.lengths.merge!(row[:Column_name] => row[:Sub_part].to_i) if row[:Sub_part] - indexes.last.orders.merge!(row[:Column_name] => :desc) if row[:Collation] == "D" + 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][:orders].merge!(row[:Column_name] => :desc) if row[:Collation] == "D" end end - indexes + indexes.map { |index| IndexDefinition.new(*index) } end def remove_column(table_name, column_name, type = nil, options = {}) @@ -103,6 +106,18 @@ module ActiveRecord super unless specifier == "RESTRICT" end + def add_index_length(quoted_columns, **options) + lengths = options_for_index_columns(options[:length]) + quoted_columns.each do |name, column| + column << "(#{lengths[name]})" if lengths[name].present? + end + end + + def add_options_for_index_columns(quoted_columns, **options) + quoted_columns = add_index_length(quoted_columns, options) + super + end + def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb index 59f661da25..8e381a92cf 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_creation.rb @@ -5,6 +5,18 @@ module ActiveRecord module PostgreSQL class SchemaCreation < AbstractAdapter::SchemaCreation # :nodoc: private + def visit_AlterTable(o) + super << o.constraint_validations.map { |fk| visit_ValidateConstraint fk }.join(" ") + end + + def visit_AddForeignKey(o) + super.dup.tap { |sql| sql << " NOT VALID" unless o.validate? } + end + + def visit_ValidateConstraint(name) + "VALIDATE CONSTRAINT #{quote_column_name(name)}" + end + def add_column_options!(sql, options) if options[:collation] sql << " COLLATE \"#{options[:collation]}\"" diff --git a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb index 75622eb304..6047217fcd 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_definitions.rb @@ -95,10 +95,6 @@ module ActiveRecord args.each { |name| column(name, :int8range, options) } end - def json(*args, **options) - args.each { |name| column(name, :json, options) } - end - def jsonb(*args, **options) args.each { |name| column(name, :jsonb, options) } end @@ -192,6 +188,19 @@ module ActiveRecord class Table < ActiveRecord::ConnectionAdapters::Table include ColumnMethods end + + class AlterTable < ActiveRecord::ConnectionAdapters::AlterTable + attr_reader :constraint_validations + + def initialize(td) + super + @constraint_validations = [] + end + + def validate_constraint(name) + @constraint_validations << name + end + end end end 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 15a1dfd102..bf5fbb30e1 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql/schema_statements.rb @@ -109,6 +109,9 @@ module ActiveRecord using, expressions, where = inddef.scan(/ USING (\w+?) \((.+?)\)(?: WHERE (.+))?\z/).flatten + orders = {} + opclasses = {} + if indkey.include?(0) columns = expressions else @@ -119,21 +122,12 @@ module ActiveRecord AND a.attnum IN (#{indkey.join(",")}) SQL - orders = {} - opclasses = {} - # add info on sort order (only desc order is explicitly specified, asc is the default) # and non-default opclasses expressions.scan(/(\w+)(?: (?!DESC)(\w+))?(?: (DESC))?/).each do |column, opclass, desc| - opclasses[column] = opclass if opclass + opclasses[column] = opclass.to_sym if opclass orders[column] = :desc if desc end - - # Use a string for the opclass description (instead of a hash) when all columns - # have the same opclass specified. - if columns.count == opclasses.count && opclasses.values.uniq.count == 1 - opclasses = opclasses.values.first - end end IndexDefinition.new( @@ -142,9 +136,9 @@ module ActiveRecord unique, columns, orders: orders, + opclasses: opclasses, where: where, using: using.to_sym, - opclass: opclasses, comment: comment.presence ) end @@ -400,50 +394,23 @@ module ActiveRecord def change_column(table_name, column_name, type, options = {}) #:nodoc: clear_cache! - quoted_table_name = quote_table_name(table_name) - quoted_column_name = quote_column_name(column_name) - sql_type = type_to_sql(type, options) - sql = "ALTER TABLE #{quoted_table_name} ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}".dup - if options[:collation] - sql << " COLLATE \"#{options[:collation]}\"" - end - if options[:using] - sql << " USING #{options[:using]}" - elsif options[:cast_as] - cast_as_type = type_to_sql(options[:cast_as], options) - sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" - end - execute sql - - change_column_default(table_name, column_name, options[:default]) if options.key?(:default) - change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) - change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + sqls, procs = change_column_for_alter(table_name, column_name, type, options) + execute "ALTER TABLE #{quote_table_name(table_name)} #{sqls.join(", ")}" + procs.each(&:call) end # Changes the default value of a table column. def change_column_default(table_name, column_name, default_or_changes) # :nodoc: - clear_cache! - column = column_for(table_name, column_name) - return unless column - - default = extract_new_default_value(default_or_changes) - alter_column_query = "ALTER TABLE #{quote_table_name(table_name)} ALTER COLUMN #{quote_column_name(column_name)} %s" - if default.nil? - # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will - # cast the default to the columns type, which leaves us with a default like "default NULL::character varying". - execute alter_column_query % "DROP DEFAULT" - else - execute alter_column_query % "SET DEFAULT #{quote_default_expression(default, column)}" - end + execute "ALTER TABLE #{quote_table_name(table_name)} #{change_column_default_for_alter(table_name, column_name, default_or_changes)}" end def change_column_null(table_name, column_name, null, default = nil) #:nodoc: clear_cache! unless null || default.nil? column = column_for(table_name, column_name) - execute("UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(default, column)} WHERE #{quote_column_name(column_name)} IS NULL") if column + execute "UPDATE #{quote_table_name(table_name)} SET #{quote_column_name(column_name)}=#{quote_default_expression(default, column)} WHERE #{quote_column_name(column_name)} IS NULL" if column end - execute("ALTER TABLE #{quote_table_name(table_name)} ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL") + execute "ALTER TABLE #{quote_table_name(table_name)} #{change_column_null_for_alter(table_name, column_name, null, default)}" end # Adds comment for given table column or drops it if +comment+ is a +nil+ @@ -507,7 +474,7 @@ module ActiveRecord def foreign_keys(table_name) scope = quoted_scope(table_name) fk_info = exec_query(<<-SQL.strip_heredoc, "SCHEMA") - SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete + SELECT t2.oid::regclass::text AS to_table, a1.attname AS column, a2.attname AS primary_key, c.conname AS name, c.confupdtype AS on_update, c.confdeltype AS on_delete, c.convalidated AS valid FROM pg_constraint c JOIN pg_class t1 ON c.conrelid = t1.oid JOIN pg_class t2 ON c.confrelid = t2.oid @@ -529,6 +496,7 @@ module ActiveRecord options[:on_delete] = extract_foreign_key_action(row["on_delete"]) options[:on_update] = extract_foreign_key_action(row["on_update"]) + options[:validate] = row["valid"] ForeignKeyDefinition.new(table_name, row["to_table"], options) end @@ -589,6 +557,43 @@ module ActiveRecord PostgreSQL::SchemaDumper.create(self, options) end + # Validates the given constraint. + # + # Validates the constraint named +constraint_name+ on +accounts+. + # + # validate_constraint :accounts, :constraint_name + def validate_constraint(table_name, constraint_name) + return unless supports_validate_constraints? + + at = create_alter_table table_name + at.validate_constraint constraint_name + + execute schema_creation.accept(at) + end + + # Validates the given foreign key. + # + # Validates the foreign key on +accounts.branch_id+. + # + # validate_foreign_key :accounts, :branches + # + # Validates the foreign key on +accounts.owner_id+. + # + # validate_foreign_key :accounts, column: :owner_id + # + # Validates the foreign key named +special_fk_name+ on the +accounts+ table. + # + # validate_foreign_key :accounts, name: :special_fk_name + # + # The +options+ hash accepts the same keys as SchemaStatements#add_foreign_key. + def validate_foreign_key(from_table, options_or_to_table = {}) + return unless supports_validate_constraints? + + fk_name_to_validate = foreign_key_for!(from_table, options_or_to_table).name + + validate_constraint from_table, fk_name_to_validate + end + private def schema_creation PostgreSQL::SchemaCreation.new(self) @@ -598,6 +603,10 @@ module ActiveRecord PostgreSQL::TableDefinition.new(*args) end + def create_alter_table(name) + PostgreSQL::AlterTable.new create_table_definition(name) + end + def new_column_from_field(table_name, field) column_name, type, default, notnull, oid, fmod, collation, comment = field type_metadata = fetch_type_metadata(column_name, type, oid.to_i, fmod.to_i) @@ -637,6 +646,66 @@ module ActiveRecord end end + def change_column_sql(table_name, column_name, type, options = {}) + quoted_column_name = quote_column_name(column_name) + sql_type = type_to_sql(type, options) + sql = "ALTER COLUMN #{quoted_column_name} TYPE #{sql_type}".dup + if options[:collation] + sql << " COLLATE \"#{options[:collation]}\"" + end + if options[:using] + sql << " USING #{options[:using]}" + elsif options[:cast_as] + cast_as_type = type_to_sql(options[:cast_as], options) + sql << " USING CAST(#{quoted_column_name} AS #{cast_as_type})" + end + + sql + end + + def change_column_for_alter(table_name, column_name, type, options = {}) + sqls = [change_column_sql(table_name, column_name, type, options)] + procs = [] + sqls << change_column_default_for_alter(table_name, column_name, options[:default]) if options.key?(:default) + sqls << change_column_null_for_alter(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + procs << Proc.new { change_column_comment(table_name, column_name, options[:comment]) } if options.key?(:comment) + + [sqls, procs] + end + + + # Changes the default value of a table column. + def change_column_default_for_alter(table_name, column_name, default_or_changes) # :nodoc: + column = column_for(table_name, column_name) + return unless column + + default = extract_new_default_value(default_or_changes) + alter_column_query = "ALTER COLUMN #{quote_column_name(column_name)} %s" + if default.nil? + # <tt>DEFAULT NULL</tt> results in the same behavior as <tt>DROP DEFAULT</tt>. However, PostgreSQL will + # cast the default to the columns type, which leaves us with a default like "default NULL::character varying". + alter_column_query % "DROP DEFAULT" + else + alter_column_query % "SET DEFAULT #{quote_default_expression(default, column)}" + end + end + + def change_column_null_for_alter(table_name, column_name, null, default = nil) #:nodoc: + "ALTER #{quote_column_name(column_name)} #{null ? 'DROP' : 'SET'} NOT NULL" + end + + def add_index_opclass(quoted_columns, **options) + opclasses = options_for_index_columns(options[:opclass]) + quoted_columns.each do |name, column| + column << " #{opclasses[name]}" if opclasses[name].present? + end + end + + def add_options_for_index_columns(quoted_columns, **options) + quoted_columns = add_index_opclass(quoted_columns, options) + super + end + def data_source_sql(name = nil, type: nil) scope = quoted_scope(name, type: type) scope[:type] ||= "'r','v','m'" # (r)elation/table, (v)iew, (m)aterialized view diff --git a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb index 9397481c4c..23fc69d649 100644 --- a/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/postgresql_adapter.rb @@ -142,6 +142,10 @@ module ActiveRecord true end + def supports_validate_constraints? + true + end + def supports_views? true end @@ -386,24 +390,6 @@ module ActiveRecord end private - - def add_index_opclass(column_names, options = {}) - opclass = - if options[:opclass].is_a?(Hash) - options[:opclass].symbolize_keys - else - Hash.new { |hash, column| hash[column] = options[:opclass].to_s } - end - column_names.each do |name, column| - column << " #{opclass[name]}" if opclass[name].present? - end - end - - def add_options_for_index_columns(quoted_columns, **options) - quoted_columns = add_index_opclass(quoted_columns, options) - super - end - # See https://www.postgresql.org/docs/current/static/errcodes-appendix.html VALUE_LIMIT_VIOLATION = "22001" NUMERIC_VALUE_OUT_OF_RANGE = "22003" diff --git a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb index 3197b522b3..65aba20052 100644 --- a/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb +++ b/activerecord/lib/active_record/connection_adapters/sqlite3_adapter.rb @@ -70,7 +70,8 @@ module ActiveRecord time: { name: "time" }, date: { name: "date" }, binary: { name: "blob" }, - boolean: { name: "boolean" } + boolean: { name: "boolean" }, + json: { name: "json" }, } ## @@ -134,6 +135,10 @@ module ActiveRecord true end + def supports_json? + true + end + def supports_multi_insert? sqlite_version >= "3.7.11" end @@ -369,6 +374,10 @@ module ActiveRecord end private + def initialize_type_map(m = type_map) + super + register_class_with_limit m, %r(int)i, SQLite3Integer + end def table_structure(table_name) structure = exec_query("PRAGMA table_info(#{quote_table_name(table_name)})", "SCHEMA") @@ -528,6 +537,17 @@ module ActiveRecord def configure_connection execute("PRAGMA foreign_keys = ON", "SCHEMA") end + + class SQLite3Integer < Type::Integer # :nodoc: + private + def _limit + # INTEGER storage class can be stored 8 bytes value. + # See https://www.sqlite.org/datatype3.html#storage_classes_and_datatypes + limit || 8 + end + end + + ActiveRecord::Type.register(:integer, SQLite3Integer, adapter: :sqlite3) end ActiveSupport.run_load_hooks(:active_record_sqlite3adapter, SQLite3Adapter) end diff --git a/activerecord/lib/active_record/migration.rb b/activerecord/lib/active_record/migration.rb index 15e9c09ffb..5c10d4ff24 100644 --- a/activerecord/lib/active_record/migration.rb +++ b/activerecord/lib/active_record/migration.rb @@ -1045,7 +1045,7 @@ module ActiveRecord new(:up, migrations(migrations_paths), nil) end - def get_all_versions(connection = Base.connection) + def get_all_versions if SchemaMigration.table_exists? SchemaMigration.all_versions.map(&:to_i) else @@ -1053,12 +1053,13 @@ module ActiveRecord end end - def current_version(connection = Base.connection) - get_all_versions(connection).max || 0 + def current_version(connection = nil) + get_all_versions.max || 0 + rescue ActiveRecord::NoDatabaseError end - def needs_migration?(connection = Base.connection) - (migrations(migrations_paths).collect(&:version) - get_all_versions(connection)).size > 0 + def needs_migration?(connection = nil) + (migrations(migrations_paths).collect(&:version) - get_all_versions).size > 0 end def any_migrations? diff --git a/activerecord/lib/active_record/migration/compatibility.rb b/activerecord/lib/active_record/migration/compatibility.rb index c979aaf0a0..bd8c054c28 100644 --- a/activerecord/lib/active_record/migration/compatibility.rb +++ b/activerecord/lib/active_record/migration/compatibility.rb @@ -16,6 +16,18 @@ module ActiveRecord V5_2 = Current class V5_1 < V5_2 + def change_column(table_name, column_name, type, options = {}) + if adapter_name == "PostgreSQL" + clear_cache! + sql = connection.send(:change_column_sql, table_name, column_name, type, options) + execute "ALTER TABLE #{quote_table_name(table_name)} #{sql}" + change_column_default(table_name, column_name, options[:default]) if options.key?(:default) + change_column_null(table_name, column_name, options[:null], options[:default]) if options.key?(:null) + change_column_comment(table_name, column_name, options[:comment]) if options.key?(:comment) + else + super + end + end end class V5_0 < V5_1 diff --git a/activerecord/lib/active_record/railtie.rb b/activerecord/lib/active_record/railtie.rb index 9ee8425e1b..4538ed6a5f 100644 --- a/activerecord/lib/active_record/railtie.rb +++ b/activerecord/lib/active_record/railtie.rb @@ -90,12 +90,15 @@ module ActiveRecord filename = File.join(app.config.paths["db"].first, "schema_cache.yml") if File.file?(filename) + current_version = ActiveRecord::Migrator.current_version + next if current_version.nil? + cache = YAML.load(File.read(filename)) - if cache.version == ActiveRecord::Migrator.current_version + if cache.version == current_version connection.schema_cache = cache connection_pool.schema_cache = cache.dup else - warn "Ignoring db/schema_cache.yml because it has expired. The current schema version is #{ActiveRecord::Migrator.current_version}, but the one in the cache is #{cache.version}." + warn "Ignoring db/schema_cache.yml because it has expired. The current schema version is #{current_version}, but the one in the cache is #{cache.version}." end end end diff --git a/activerecord/lib/active_record/schema_dumper.rb b/activerecord/lib/active_record/schema_dumper.rb index 8cb2851557..16ccba6b6c 100644 --- a/activerecord/lib/active_record/schema_dumper.rb +++ b/activerecord/lib/active_record/schema_dumper.rb @@ -184,12 +184,12 @@ HEADER "name: #{index.name.inspect}", ] index_parts << "unique: true" if index.unique - index_parts << "length: { #{format_options(index.lengths)} }" if index.lengths.present? - index_parts << "order: { #{format_options(index.orders)} }" if index.orders.present? + index_parts << "length: #{format_index_parts(index.lengths)}" if index.lengths.present? + index_parts << "order: #{format_index_parts(index.orders)}" if index.orders.present? + index_parts << "opclass: #{format_index_parts(index.opclasses)}" if index.opclasses.present? index_parts << "where: #{index.where.inspect}" if index.where index_parts << "using: #{index.using.inspect}" if !@connection.default_index_type?(index) index_parts << "type: #{index.type.inspect}" if index.type - index_parts << "opclass: #{index.opclass.inspect}" if index.opclass.present? index_parts << "comment: #{index.comment.inspect}" if index.comment index_parts end @@ -232,6 +232,14 @@ HEADER options.map { |key, value| "#{key}: #{value.inspect}" }.join(", ") end + def format_index_parts(options) + if options.is_a?(Hash) + "{ #{format_options(options)} }" + else + options.inspect + end + end + def remove_prefix_and_suffix(table) prefix = Regexp.escape(@options[:table_name_prefix].to_s) suffix = Regexp.escape(@options[:table_name_suffix].to_s) diff --git a/activerecord/test/cases/adapters/mysql2/connection_test.rb b/activerecord/test/cases/adapters/mysql2/connection_test.rb index e61c70848a..13b4096671 100644 --- a/activerecord/test/cases/adapters/mysql2/connection_test.rb +++ b/activerecord/test/cases/adapters/mysql2/connection_test.rb @@ -174,10 +174,10 @@ class Mysql2ConnectionTest < ActiveRecord::Mysql2TestCase assert_equal "SCHEMA", @subscriber.logged[0][1] end - def test_logs_name_rename_column_sql + def test_logs_name_rename_column_for_alter @connection.execute "CREATE TABLE `bar_baz` (`foo` varchar(255))" @subscriber.logged.clear - @connection.send(:rename_column_sql, "bar_baz", "foo", "foo2") + @connection.send(:rename_column_for_alter, "bar_baz", "foo", "foo2") assert_equal "SCHEMA", @subscriber.logged[0][1] ensure @connection.execute "DROP TABLE `bar_baz`" diff --git a/activerecord/test/cases/adapters/postgresql/schema_test.rb b/activerecord/test/cases/adapters/postgresql/schema_test.rb index a6ae18a826..1126908761 100644 --- a/activerecord/test/cases/adapters/postgresql/schema_test.rb +++ b/activerecord/test/cases/adapters/postgresql/schema_test.rb @@ -459,7 +459,7 @@ class SchemaTest < ActiveRecord::PostgreSQLTestCase assert_equal :btree, index_d.using assert_equal :gin, index_e.using - assert_equal :desc, index_d.orders[INDEX_D_COLUMN] + assert_equal :desc, index_d.orders end end @@ -520,7 +520,7 @@ class SchemaIndexOpclassTest < ActiveRecord::PostgreSQLTestCase output = dump_table_schema "trains" - assert_match(/opclass: "text_pattern_ops"/, output) + assert_match(/opclass: :text_pattern_ops/, output) end def test_non_default_opclass_is_dumped @@ -528,7 +528,7 @@ class SchemaIndexOpclassTest < ActiveRecord::PostgreSQLTestCase output = dump_table_schema "trains" - assert_match(/opclass: \{"description"=>"text_pattern_ops"\}/, output) + assert_match(/opclass: \{ description: :text_pattern_ops \}/, output) end end diff --git a/activerecord/test/cases/adapters/sqlite3/json_test.rb b/activerecord/test/cases/adapters/sqlite3/json_test.rb index 568a524058..6f247fcd22 100644 --- a/activerecord/test/cases/adapters/sqlite3/json_test.rb +++ b/activerecord/test/cases/adapters/sqlite3/json_test.rb @@ -9,8 +9,8 @@ class SQLite3JSONTest < ActiveRecord::SQLite3TestCase def setup super @connection.create_table("json_data_type") do |t| - t.column "payload", :json, default: {} - t.column "settings", :json + t.json "payload", default: {} + t.json "settings" end end diff --git a/activerecord/test/cases/base_test.rb b/activerecord/test/cases/base_test.rb index d79afa2ee9..3497f6aae4 100644 --- a/activerecord/test/cases/base_test.rb +++ b/activerecord/test/cases/base_test.rb @@ -891,11 +891,9 @@ class BasicsTest < ActiveRecord::TestCase assert_equal 2147483648, company.rating end - unless current_adapter?(:SQLite3Adapter) - def test_bignum_pk - company = Company.create!(id: 2147483648, name: "foo") - assert_equal company, Company.find(company.id) - end + def test_bignum_pk + company = Company.create!(id: 2147483648, name: "foo") + assert_equal company, Company.find(company.id) end # TODO: extend defaults tests to other databases! diff --git a/activerecord/test/cases/json_attribute_test.rb b/activerecord/test/cases/json_attribute_test.rb index 63f3c77fc3..afc39d0420 100644 --- a/activerecord/test/cases/json_attribute_test.rb +++ b/activerecord/test/cases/json_attribute_test.rb @@ -19,14 +19,14 @@ class JsonAttributeTest < ActiveRecord::TestCase def setup super @connection.create_table("json_data_type") do |t| - t.text "payload" - t.text "settings" + t.string "payload" + t.string "settings" end end private def column_type - :text + :string end def klass diff --git a/activerecord/test/cases/json_shared_test_cases.rb b/activerecord/test/cases/json_shared_test_cases.rb index a71485982c..b0c0f2c283 100644 --- a/activerecord/test/cases/json_shared_test_cases.rb +++ b/activerecord/test/cases/json_shared_test_cases.rb @@ -23,25 +23,23 @@ module JSONSharedTestCases def test_column column = klass.columns_hash["payload"] assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type + assert_type_match column_type, column.sql_type type = klass.type_for_attribute("payload") assert_not type.binary? end def test_change_table_supports_json - skip unless @connection.supports_json? @connection.change_table("json_data_type") do |t| t.public_send column_type, "users" end klass.reset_column_information column = klass.columns_hash["users"] assert_equal column_type, column.type - assert_equal column_type.to_s, column.sql_type + assert_type_match column_type, column.sql_type end def test_schema_dumping - skip unless @connection.supports_json? output = dump_table_schema("json_data_type") assert_match(/t\.#{column_type}\s+"settings"/, output) end @@ -68,26 +66,26 @@ module JSONSharedTestCases end def test_rewrite - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + @connection.execute(insert_statement_per_database('{"k":"v"}')) x = klass.first x.payload = { '"a\'' => "b" } assert x.save! end def test_select - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k":"v"}')|) + @connection.execute(insert_statement_per_database('{"k":"v"}')) x = klass.first assert_equal({ "k" => "v" }, x.payload) end def test_select_multikey - @connection.execute(%q|insert into json_data_type (payload) VALUES ('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')|) + @connection.execute(insert_statement_per_database('{"k1":"v1", "k2":"v2", "k3":[1,2,3]}')) x = klass.first assert_equal({ "k1" => "v1", "k2" => "v2", "k3" => [1, 2, 3] }, x.payload) end def test_null_json - @connection.execute("insert into json_data_type (payload) VALUES(null)") + @connection.execute(insert_statement_per_database("null")) x = klass.first assert_nil(x.payload) end @@ -109,13 +107,13 @@ module JSONSharedTestCases end def test_select_array_json_value - @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + @connection.execute(insert_statement_per_database('["v0",{"k1":"v1"}]')) x = klass.first assert_equal(["v0", { "k1" => "v1" }], x.payload) end def test_rewrite_array_json_value - @connection.execute(%q|insert into json_data_type (payload) VALUES ('["v0",{"k1":"v1"}]')|) + @connection.execute(insert_statement_per_database('["v0",{"k1":"v1"}]')) x = klass.first x.payload = ["v1", { "k2" => "v2" }, "v3"] assert x.save! @@ -255,4 +253,17 @@ module JSONSharedTestCases def klass JsonDataType end + + def assert_type_match(type, sql_type) + native_type = ActiveRecord::Base.connection.native_database_types[type][:name] + assert_match %r(\A#{native_type}\b), sql_type + end + + def insert_statement_per_database(values) + if current_adapter?(:OracleAdapter) + "insert into json_data_type (id, payload) VALUES (json_data_type_seq.nextval, '#{values}')" + else + "insert into json_data_type (payload) VALUES ('#{values}')" + end + end end diff --git a/activerecord/test/cases/migration/change_schema_test.rb b/activerecord/test/cases/migration/change_schema_test.rb index 7b0644e9c0..38a906c8f5 100644 --- a/activerecord/test/cases/migration/change_schema_test.rb +++ b/activerecord/test/cases/migration/change_schema_test.rb @@ -264,19 +264,18 @@ module ActiveRecord t.column :foo, :timestamp end - klass = Class.new(ActiveRecord::Base) - klass.table_name = "testings" + column = connection.columns(:testings).find { |c| c.name == "foo" } - assert_equal :datetime, klass.columns_hash["foo"].type + assert_equal :datetime, column.type if current_adapter?(:PostgreSQLAdapter) - assert_equal "timestamp without time zone", klass.columns_hash["foo"].sql_type + assert_equal "timestamp without time zone", column.sql_type elsif current_adapter?(:Mysql2Adapter) - assert_equal "timestamp", klass.columns_hash["foo"].sql_type + assert_equal "timestamp", column.sql_type elsif current_adapter?(:OracleAdapter) - assert_equal "TIMESTAMP(6)", klass.columns_hash["foo"].sql_type + assert_equal "TIMESTAMP(6)", column.sql_type else - assert_equal klass.connection.type_to_sql("datetime"), klass.columns_hash["foo"].sql_type + assert_equal connection.type_to_sql("datetime"), column.sql_type end end diff --git a/activerecord/test/cases/migration/compatibility_test.rb b/activerecord/test/cases/migration/compatibility_test.rb index 2fef2f796e..cc2391f349 100644 --- a/activerecord/test/cases/migration/compatibility_test.rb +++ b/activerecord/test/cases/migration/compatibility_test.rb @@ -16,7 +16,7 @@ module ActiveRecord ActiveRecord::Migration.verbose = false connection.create_table :testings do |t| - t.column :foo, :string, limit: 100 + t.column :foo, :string, limit: 5 t.column :bar, :string, limit: 100 end end @@ -126,6 +126,25 @@ module ActiveRecord end assert_match(/LegacyMigration < ActiveRecord::Migration\[4\.2\]/, e.message) end + + if current_adapter?(:PostgreSQLAdapter) + class Testing < ActiveRecord::Base + end + + def test_legacy_change_column_with_null_executes_update + migration = Class.new(ActiveRecord::Migration[5.1]) { + def migrate(x) + change_column :testings, :foo, :string, limit: 10, null: false, default: "foobar" + end + }.new + + Testing.create! + ActiveRecord::Migrator.new(:up, [migration]).migrate + assert_equal ["foobar"], Testing.all.map(&:foo) + ensure + ActiveRecord::Base.clear_cache! + end + end end end end diff --git a/activerecord/test/cases/migration/foreign_key_test.rb b/activerecord/test/cases/migration/foreign_key_test.rb index 499d072de5..079be04946 100644 --- a/activerecord/test/cases/migration/foreign_key_test.rb +++ b/activerecord/test/cases/migration/foreign_key_test.rb @@ -227,6 +227,74 @@ if ActiveRecord::Base.connection.supports_foreign_keys? end end + if ActiveRecord::Base.connection.supports_validate_constraints? + def test_add_invalid_foreign_key + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + refute fk.validated? + end + + def test_validate_foreign_key_infers_column + @connection.add_foreign_key :astronauts, :rockets, validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, :rockets + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_column + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, column: "rocket_id" + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_symbol_column + @connection.add_foreign_key :astronauts, :rockets, column: :rocket_id, validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, column: :rocket_id + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_key_by_name + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk", validate: false + refute @connection.foreign_keys("astronauts").first.validated? + + @connection.validate_foreign_key :astronauts, name: "fancy_named_fk" + assert @connection.foreign_keys("astronauts").first.validated? + end + + def test_validate_foreign_non_existing_foreign_key_raises + assert_raises ArgumentError do + @connection.validate_foreign_key :astronauts, :rockets + end + end + + def test_validate_constraint_by_name + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", name: "fancy_named_fk", validate: false + + @connection.validate_constraint :astronauts, "fancy_named_fk" + assert @connection.foreign_keys("astronauts").first.validated? + end + else + # Foreign key should still be created, but should not be invalid + def test_add_invalid_foreign_key + @connection.add_foreign_key :astronauts, :rockets, column: "rocket_id", validate: false + + foreign_keys = @connection.foreign_keys("astronauts") + assert_equal 1, foreign_keys.size + + fk = foreign_keys.first + assert fk.validated? + end + end + def test_schema_dumping @connection.add_foreign_key :astronauts, :rockets output = dump_table_schema "astronauts" diff --git a/activerecord/test/cases/schema_dumper_test.rb b/activerecord/test/cases/schema_dumper_test.rb index ac5092f1c1..a612ce9bb2 100644 --- a/activerecord/test/cases/schema_dumper_test.rb +++ b/activerecord/test/cases/schema_dumper_test.rb @@ -177,14 +177,14 @@ class SchemaDumperTest < ActiveRecord::TestCase def test_schema_dumps_index_columns_in_right_order index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*company_index/).first.strip - if current_adapter?(:PostgreSQLAdapter, :SQLite3Adapter) - assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition - elsif current_adapter?(:Mysql2Adapter) + if current_adapter?(:Mysql2Adapter) if ActiveRecord::Base.connection.supports_index_sort_order? assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }, order: { rating: :desc }', index_definition else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", length: { type: 10 }', index_definition end + elsif ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index", order: { rating: :desc }', index_definition else assert_equal 't.index ["firm_id", "type", "rating"], name: "company_index"', index_definition end @@ -199,6 +199,24 @@ class SchemaDumperTest < ActiveRecord::TestCase end end + def test_schema_dumps_index_sort_order + index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_rating/).first.strip + if ActiveRecord::Base.connection.supports_index_sort_order? + assert_equal 't.index ["name", "rating"], name: "index_companies_on_name_and_rating", order: :desc', index_definition + else + assert_equal 't.index ["name", "rating"], name: "index_companies_on_name_and_rating"', index_definition + end + end + + def test_schema_dumps_index_length + index_definition = dump_table_schema("companies").split(/\n/).grep(/t\.index.*_name_and_description/).first.strip + if current_adapter?(:Mysql2Adapter) + assert_equal 't.index ["name", "description"], name: "index_companies_on_name_and_description", length: 10', index_definition + else + assert_equal 't.index ["name", "description"], name: "index_companies_on_name_and_description"', index_definition + end + end + def test_schema_dump_should_honor_nonstandard_primary_keys output = standard_dump match = output.match(%r{create_table "movies"(.*)do}) diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index a4505a4892..bf66846840 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -205,6 +205,8 @@ ActiveRecord::Schema.define do t.bigint :rating, default: 1 t.integer :account_id t.string :description, default: "" + t.index [:name, :rating], order: :desc + t.index [:name, :description], length: 10 t.index [:firm_id, :type, :rating], name: "company_index", length: { type: 10 }, order: { rating: :desc } t.index [:firm_id, :type], name: "company_partial_index", where: "(rating > 10)" t.index :name, name: "company_name_index", using: :btree diff --git a/activestorage/app/models/active_storage/blob.rb b/activestorage/app/models/active_storage/blob.rb index 2aa05d665e..acaf22fac1 100644 --- a/activestorage/app/models/active_storage/blob.rb +++ b/activestorage/app/models/active_storage/blob.rb @@ -270,7 +270,8 @@ class ActiveStorage::Blob < ActiveRecord::Base # deleted as well or you will essentially have a dead reference. It's recommended to use the +#purge+ and +#purge_later+ # methods in most circumstances. def delete - service.delete key + service.delete(key) + service.delete_prefixed("variants/#{key}/") if image? end # Deletes the file on the service and then destroys the blob record. This is the recommended way to dispose of unwanted diff --git a/activestorage/lib/active_storage/log_subscriber.rb b/activestorage/lib/active_storage/log_subscriber.rb index 5cbf4bd1a5..a4e148c1a5 100644 --- a/activestorage/lib/active_storage/log_subscriber.rb +++ b/activestorage/lib/active_storage/log_subscriber.rb @@ -18,6 +18,10 @@ module ActiveStorage info event, color("Deleted file from key: #{key_in(event)}", RED) end + def service_delete_prefixed(event) + info event, color("Deleted files by key prefix: #{event.payload[:prefix]}", RED) + end + def service_exist(event) debug event, color("Checked if file exists at key: #{key_in(event)} (#{event.payload[:exist] ? "yes" : "no"})", BLUE) end diff --git a/activestorage/lib/active_storage/service.rb b/activestorage/lib/active_storage/service.rb index aa150e4d8a..c8f675db86 100644 --- a/activestorage/lib/active_storage/service.rb +++ b/activestorage/lib/active_storage/service.rb @@ -78,6 +78,11 @@ module ActiveStorage raise NotImplementedError end + # Delete files at keys starting with the +prefix+. + def delete_prefixed(prefix) + raise NotImplementedError + end + # Return +true+ if a file exists at the +key+. def exist?(key) raise NotImplementedError @@ -104,10 +109,10 @@ module ActiveStorage end private - def instrument(operation, key, payload = {}, &block) + def instrument(operation, payload = {}, &block) ActiveSupport::Notifications.instrument( "service_#{operation}.active_storage", - payload.merge(key: key, service: service_name), &block) + payload.merge(service: service_name), &block) end def service_name diff --git a/activestorage/lib/active_storage/service/azure_storage_service.rb b/activestorage/lib/active_storage/service/azure_storage_service.rb index f3877ad9c9..19b09991b3 100644 --- a/activestorage/lib/active_storage/service/azure_storage_service.rb +++ b/activestorage/lib/active_storage/service/azure_storage_service.rb @@ -19,7 +19,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin blobs.create_block_blob(container, key, io, content_md5: checksum) rescue Azure::Core::Http::HTTPError @@ -30,11 +30,11 @@ module ActiveStorage def download(key, &block) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do stream(key, &block) end else - instrument :download, key do + instrument :download, key: key do _, io = blobs.get_blob(container, key) io.force_encoding(Encoding::BINARY) end @@ -42,7 +42,7 @@ module ActiveStorage end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin blobs.delete_blob(container, key) rescue Azure::Core::Http::HTTPError @@ -51,8 +51,24 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + marker = nil + + loop do + results = blobs.list_blobs(container, prefix: prefix, marker: marker) + + results.each do |blob| + blobs.delete_blob(container, blob.name) + end + + break unless marker = results.continuation_token.presence + end + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = blob_for(key).present? payload[:exist] = answer answer @@ -60,7 +76,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| base_url = url_for(key) generated_url = signer.signed_uri( URI(base_url), false, @@ -77,7 +93,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| base_url = url_for(key) generated_url = signer.signed_uri(URI(base_url), false, permissions: "rw", expiry: format_expiry(expires_in)).to_s diff --git a/activestorage/lib/active_storage/service/disk_service.rb b/activestorage/lib/active_storage/service/disk_service.rb index 52eaba4e7b..a8728c5bc3 100644 --- a/activestorage/lib/active_storage/service/disk_service.rb +++ b/activestorage/lib/active_storage/service/disk_service.rb @@ -16,7 +16,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do IO.copy_stream(io, make_path_for(key)) ensure_integrity_of(key, checksum) if checksum end @@ -24,7 +24,7 @@ module ActiveStorage def download(key) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do File.open(path_for(key), "rb") do |file| while data = file.read(64.kilobytes) yield data @@ -32,14 +32,14 @@ module ActiveStorage end end else - instrument :download, key do + instrument :download, key: key do File.binread path_for(key) end end end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin File.delete path_for(key) rescue Errno::ENOENT @@ -48,8 +48,16 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + Dir.glob(path_for("#{prefix}*")).each do |path| + FileUtils.rm_rf(path) + end + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = File.exist? path_for(key) payload[:exist] = answer answer @@ -57,7 +65,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| verified_key_with_expiration = ActiveStorage.verifier.generate(key, expires_in: expires_in, purpose: :blob_key) generated_url = @@ -77,7 +85,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| verified_token_with_expiration = ActiveStorage.verifier.generate( { key: key, diff --git a/activestorage/lib/active_storage/service/gcs_service.rb b/activestorage/lib/active_storage/service/gcs_service.rb index fd9916634a..c13ce4786d 100644 --- a/activestorage/lib/active_storage/service/gcs_service.rb +++ b/activestorage/lib/active_storage/service/gcs_service.rb @@ -14,7 +14,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin bucket.create_file(io, key, md5: checksum) rescue Google::Cloud::InvalidArgumentError @@ -25,7 +25,7 @@ module ActiveStorage # FIXME: Download in chunks when given a block. def download(key) - instrument :download, key do + instrument :download, key: key do io = file_for(key).download io.rewind @@ -38,7 +38,7 @@ module ActiveStorage end def delete(key) - instrument :delete, key do + instrument :delete, key: key do begin file_for(key).delete rescue Google::Cloud::NotFoundError @@ -47,8 +47,14 @@ module ActiveStorage end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + bucket.files(prefix: prefix).all(&:delete) + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = file_for(key).exists? payload[:exist] = answer answer @@ -56,7 +62,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, content_type:, disposition:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = file_for(key).signed_url expires: expires_in, query: { "response-content-disposition" => content_disposition_with(type: disposition, filename: filename), "response-content-type" => content_type @@ -69,7 +75,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = bucket.signed_url key, method: "PUT", expires: expires_in, content_type: content_type, content_md5: checksum diff --git a/activestorage/lib/active_storage/service/mirror_service.rb b/activestorage/lib/active_storage/service/mirror_service.rb index 39e922f7ab..7eca8ce7f4 100644 --- a/activestorage/lib/active_storage/service/mirror_service.rb +++ b/activestorage/lib/active_storage/service/mirror_service.rb @@ -35,6 +35,11 @@ module ActiveStorage perform_across_services :delete, key end + # Delete files at keys starting with the +prefix+ on all services. + def delete_prefixed(prefix) + perform_across_services :delete_prefixed, prefix + end + private def each_service(&block) [ primary, *mirrors ].each(&block) diff --git a/activestorage/lib/active_storage/service/s3_service.rb b/activestorage/lib/active_storage/service/s3_service.rb index 6957119780..c95672f338 100644 --- a/activestorage/lib/active_storage/service/s3_service.rb +++ b/activestorage/lib/active_storage/service/s3_service.rb @@ -17,7 +17,7 @@ module ActiveStorage end def upload(key, io, checksum: nil) - instrument :upload, key, checksum: checksum do + instrument :upload, key: key, checksum: checksum do begin object_for(key).put(upload_options.merge(body: io, content_md5: checksum)) rescue Aws::S3::Errors::BadDigest @@ -28,24 +28,30 @@ module ActiveStorage def download(key, &block) if block_given? - instrument :streaming_download, key do + instrument :streaming_download, key: key do stream(key, &block) end else - instrument :download, key do + instrument :download, key: key do object_for(key).get.body.read.force_encoding(Encoding::BINARY) end end end def delete(key) - instrument :delete, key do + instrument :delete, key: key do object_for(key).delete end end + def delete_prefixed(prefix) + instrument :delete_prefixed, prefix: prefix do + bucket.objects(prefix: prefix).batch_delete! + end + end + def exist?(key) - instrument :exist, key do |payload| + instrument :exist, key: key do |payload| answer = object_for(key).exists? payload[:exist] = answer answer @@ -53,7 +59,7 @@ module ActiveStorage end def url(key, expires_in:, filename:, disposition:, content_type:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = object_for(key).presigned_url :get, expires_in: expires_in.to_i, response_content_disposition: content_disposition_with(type: disposition, filename: filename), response_content_type: content_type @@ -65,7 +71,7 @@ module ActiveStorage end def url_for_direct_upload(key, expires_in:, content_type:, content_length:, checksum:) - instrument :url, key do |payload| + instrument :url, key: key do |payload| generated_url = object_for(key).presigned_url :put, expires_in: expires_in.to_i, content_type: content_type, content_length: content_length, content_md5: checksum diff --git a/activestorage/test/models/blob_test.rb b/activestorage/test/models/blob_test.rb index 6e815997ba..f94e65ed77 100644 --- a/activestorage/test/models/blob_test.rb +++ b/activestorage/test/models/blob_test.rb @@ -41,13 +41,21 @@ class ActiveStorage::BlobTest < ActiveSupport::TestCase end end - test "purge removes from external service" do + test "purge deletes file from external service" do blob = create_blob blob.purge assert_not ActiveStorage::Blob.service.exist?(blob.key) end + test "purge deletes variants from external service" do + blob = create_file_blob + variant = blob.variant(resize: "100>").processed + + blob.purge + assert_not ActiveStorage::Blob.service.exist?(variant.key) + end + private def expected_url_for(blob, disposition: :inline) query_string = { content_type: blob.content_type, disposition: "#{disposition}; #{blob.filename.parameters}" }.to_param diff --git a/activestorage/test/service/shared_service_tests.rb b/activestorage/test/service/shared_service_tests.rb index ade91ab89a..ce28c4393a 100644 --- a/activestorage/test/service/shared_service_tests.rb +++ b/activestorage/test/service/shared_service_tests.rb @@ -75,5 +75,22 @@ module ActiveStorage::Service::SharedServiceTests @service.delete SecureRandom.base58(24) end end + + test "deleting by prefix" do + begin + @service.upload("a/a/a", StringIO.new(FIXTURE_DATA)) + @service.upload("a/a/b", StringIO.new(FIXTURE_DATA)) + @service.upload("a/b/a", StringIO.new(FIXTURE_DATA)) + + @service.delete_prefixed("a/a/") + assert_not @service.exist?("a/a/a") + assert_not @service.exist?("a/a/b") + assert @service.exist?("a/b/a") + ensure + @service.delete("a/a/a") + @service.delete("a/a/b") + @service.delete("a/b/a") + end + end end end diff --git a/activesupport/lib/active_support/core_ext/module/concerning.rb b/activesupport/lib/active_support/core_ext/module/concerning.rb index 370a948eea..800bf213cc 100644 --- a/activesupport/lib/active_support/core_ext/module/concerning.rb +++ b/activesupport/lib/active_support/core_ext/module/concerning.rb @@ -22,7 +22,7 @@ class Module # # == Using comments: # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -42,7 +42,7 @@ class Module # # Noisy syntax. # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -70,7 +70,7 @@ class Module # increased overhead can be a reasonable tradeoff even if it reduces our # at-a-glance perception of how things work. # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -82,7 +82,7 @@ class Module # By quieting the mix-in noise, we arrive at a natural, low-ceremony way to # separate bite-sized concerns. # - # class Todo + # class Todo < ApplicationRecord # # Other todo implementation # # ... # @@ -101,7 +101,7 @@ class Module # end # # Todo.ancestors - # # => [Todo, Todo::EventTracking, Object] + # # => [Todo, Todo::EventTracking, ApplicationRecord, Object] # # This small step has some wonderful ripple effects. We can # * grok the behavior of our class in one glance, diff --git a/guides/source/active_record_querying.md b/guides/source/active_record_querying.md index 3786343fc3..4e28e31a53 100644 --- a/guides/source/active_record_querying.md +++ b/guides/source/active_record_querying.md @@ -801,7 +801,7 @@ The SQL that would be executed: SELECT * FROM articles WHERE id > 10 ORDER BY id DESC # Original query without `only` -SELECT "articles".* FROM "articles" WHERE (id > 10) ORDER BY id desc LIMIT 20 +SELECT * FROM articles WHERE id > 10 ORDER BY id DESC LIMIT 20 ``` @@ -820,14 +820,14 @@ Article.find(10).comments.reorder('name') The SQL that would be executed: ```sql -SELECT * FROM articles WHERE id = 10 +SELECT * FROM articles WHERE id = 10 LIMIT 1 SELECT * FROM comments WHERE article_id = 10 ORDER BY name ``` In the case where the `reorder` clause is not used, the SQL executed would be: ```sql -SELECT * FROM articles WHERE id = 10 +SELECT * FROM articles WHERE id = 10 LIMIT 1 SELECT * FROM comments WHERE article_id = 10 ORDER BY posted_at DESC ``` @@ -1091,7 +1091,7 @@ This produces: ```sql SELECT articles.* FROM articles - INNER JOIN categories ON articles.category_id = categories.id + INNER JOIN categories ON categories.id = articles.category_id INNER JOIN comments ON comments.article_id = articles.id ``` @@ -1871,14 +1871,14 @@ All calculation methods work directly on a model: ```ruby Client.count -# SELECT count(*) AS count_all FROM clients +# SELECT COUNT(*) FROM clients ``` Or on a relation: ```ruby Client.where(first_name: 'Ryan').count -# SELECT count(*) AS count_all FROM clients WHERE (first_name = 'Ryan') +# SELECT COUNT(*) FROM clients WHERE (first_name = 'Ryan') ``` You can also use various finder methods on a relation for performing complex calculations: @@ -1890,9 +1890,9 @@ Client.includes("orders").where(first_name: 'Ryan', orders: { status: 'received' Which will execute: ```sql -SELECT count(DISTINCT clients.id) AS count_all FROM clients - LEFT OUTER JOIN orders ON orders.client_id = clients.id WHERE - (clients.first_name = 'Ryan' AND orders.status = 'received') +SELECT COUNT(DISTINCT clients.id) FROM clients + LEFT OUTER JOIN orders ON orders.client_id = clients.id + WHERE (clients.first_name = 'Ryan' AND orders.status = 'received') ``` ### Count diff --git a/guides/source/form_helpers.md b/guides/source/form_helpers.md index 4ce67df93a..f8ec389b01 100644 --- a/guides/source/form_helpers.md +++ b/guides/source/form_helpers.md @@ -920,7 +920,7 @@ When an association accepts nested attributes `fields_for` renders its block onc ```ruby def new @person = Person.new - 2.times { @person.addresses.build} + 2.times { @person.addresses.build } end ``` diff --git a/guides/source/testing.md b/guides/source/testing.md index e0a2d281d9..9692f50b6e 100644 --- a/guides/source/testing.md +++ b/guides/source/testing.md @@ -319,6 +319,8 @@ specify to make your test failure messages clearer. | `assert_not_includes( collection, obj, [msg] )` | Ensures that `obj` is not in `collection`.| | `assert_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are within `delta` of each other.| | `assert_not_in_delta( expected, actual, [delta], [msg] )` | Ensures that the numbers `expected` and `actual` are not within `delta` of each other.| +| `assert_in_epsilon ( expected, actual, [epsilon], [msg] )` | Ensures that the numbers `expected` and `actual` have a relative error less than `epsilon`.| +| `assert_not_in_epsilon ( expected, actual, [epsilon], [msg] )` | Ensures that the numbers `expected` and `actual` don't have a relative error less than `epsilon`.| | `assert_throws( symbol, [msg] ) { block }` | Ensures that the given block throws the symbol.| | `assert_raises( exception1, exception2, ... ) { block }` | Ensures that the given block raises one of the given exceptions.| | `assert_instance_of( class, obj, [msg] )` | Ensures that `obj` is an instance of `class`.| diff --git a/railties/test/application/rake/dbs_test.rb b/railties/test/application/rake/dbs_test.rb index 0235210fdd..2082e9fa9f 100644 --- a/railties/test/application/rake/dbs_test.rb +++ b/railties/test/application/rake/dbs_test.rb @@ -98,6 +98,20 @@ module ApplicationTests end end + test "db:create works when schema cache exists and database does not exist" do + use_postgresql + + begin + rails %w(db:create db:migrate db:schema:cache:dump) + + rails "db:drop" + rails "db:create" + assert_equal 0, $?.exitstatus + ensure + rails "db:drop" rescue nil + end + end + test "db:drop failure because database does not exist" do output = rails("db:drop:_unsafe", "--trace") assert_match(/does not exist/, output) diff --git a/railties/test/generators/app_generator_test.rb b/railties/test/generators/app_generator_test.rb index 774fd0f315..96803db838 100644 --- a/railties/test/generators/app_generator_test.rb +++ b/railties/test/generators/app_generator_test.rb @@ -310,7 +310,7 @@ class AppGeneratorTest < Rails::Generators::TestCase case command when "active_storage:install" @binstub_called += 1 - assert_equal 1, @binstub_called, "active_storage:install expected to be called once, but was called #{@install_called} times." + assert_equal 1, @binstub_called, "active_storage:install expected to be called once, but was called #{@binstub_called} times" end end @@ -743,6 +743,41 @@ class AppGeneratorTest < Rails::Generators::TestCase end end + def test_webpack_option + command_check = -> command, *_ do + @called ||= 0 + if command == "webpacker:install" + @called += 1 + assert_equal 1, @called, "webpacker:install expected to be called once, but was called #{@called} times." + end + end + + generator([destination_root], webpack: "webpack").stub(:rails_command, command_check) do + quietly { generator.invoke_all } + end + + assert_gem "webpacker" + end + + def test_webpack_option_with_js_framework + command_check = -> command, *_ do + case command + when "webpacker:install" + @webpacker ||= 0 + @webpacker += 1 + assert_equal 1, @webpacker, "webpacker:install expected to be called once, but was called #{@webpacker} times." + when "webpacker:install:react" + @react ||= 0 + @react += 1 + assert_equal 1, @react, "webpacker:install:react expected to be called once, but was called #{@react} times." + end + end + + generator([destination_root], webpack: "react").stub(:rails_command, command_check) do + quietly { generator.invoke_all } + end + end + def test_generator_if_skip_turbolinks_is_given run_generator [destination_root, "--skip-turbolinks"] diff --git a/railties/test/isolation/abstract_unit.rb b/railties/test/isolation/abstract_unit.rb index 7522237a38..5b1c06d4e5 100644 --- a/railties/test/isolation/abstract_unit.rb +++ b/railties/test/isolation/abstract_unit.rb @@ -381,6 +381,21 @@ module TestHelpers $:.reject! { |path| path =~ %r'/(#{to_remove.join('|')})/' } end + + def use_postgresql + File.open("#{app_path}/config/database.yml", "w") do |f| + f.puts <<-YAML + default: &default + adapter: postgresql + pool: 5 + database: railties_test + development: + <<: *default + test: + <<: *default + YAML + end + end end end |