diff options
23 files changed, 172 insertions, 60 deletions
diff --git a/actionview/lib/action_view/helpers/tags/base.rb b/actionview/lib/action_view/helpers/tags/base.rb index bbb8c4d224..922d4c5390 100644 --- a/actionview/lib/action_view/helpers/tags/base.rb +++ b/actionview/lib/action_view/helpers/tags/base.rb @@ -138,7 +138,7 @@ module ActionView end def sanitized_value(value) - value.to_s.gsub(/\s/, "_").gsub(/[^-\w]/, "").downcase + value.to_s.gsub(/\s/, "_").gsub(/[^-[[:word:]]]/, "").mb_chars.downcase.to_s end def select_content_tag(option_tags, options, html_options) diff --git a/actionview/test/template/form_collections_helper_test.rb b/actionview/test/template/form_collections_helper_test.rb index aa72621c7d..bba529a98a 100644 --- a/actionview/test/template/form_collections_helper_test.rb +++ b/actionview/test/template/form_collections_helper_test.rb @@ -39,6 +39,13 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_select "label[for=user_active_no]", "No" end + test "collection radio generates labels for non-English values correctly" do + with_collection_radio_buttons :user, :title, ["Господин", "Госпожа"], :to_s, :to_s + + assert_select "input[type=radio]#user_title_господин" + assert_select "label[for=user_title_господин]", "Господин" + end + test "collection radio should sanitize collection values for labels correctly" do with_collection_radio_buttons :user, :name, ["$0.99", "$1.99"], :to_s, :to_s assert_select "label[for=user_name_099]", "$0.99" @@ -299,6 +306,13 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_select "label[for=user_name_199]", "$1.99" end + test "collection check boxes generates labels for non-English values correctly" do + with_collection_check_boxes :user, :title, ["Господин", "Госпожа"], :to_s, :to_s + + assert_select "input[type=checkbox]#user_title_господин" + assert_select "label[for=user_title_господин]", "Господин" + end + test "collection check boxes accepts html options as the last element of array" do collection = [[1, "Category 1", { class: "foo" }], [2, "Category 2", { class: "bar" }]] with_collection_check_boxes :user, :active, collection, :first, :second diff --git a/activerecord/lib/active_record/associations/collection_association.rb b/activerecord/lib/active_record/associations/collection_association.rb index ed2e6d1ae4..0001b804a8 100644 --- a/activerecord/lib/active_record/associations/collection_association.rb +++ b/activerecord/lib/active_record/associations/collection_association.rb @@ -304,15 +304,13 @@ module ActiveRecord return scope.to_a if skip_statement_cache?(scope) conn = klass.connection - sc = reflection.association_scope_cache(conn, owner) do - StatementCache.create(conn) { |params| - as = AssociationScope.create { params.bind } - target_scope.merge!(as.scope(self)) - } + sc = reflection.association_scope_cache(conn, owner) do |params| + as = AssociationScope.create { params.bind } + target_scope.merge!(as.scope(self)) end binds = AssociationScope.get_bind_values(owner, reflection.chain) - sc.execute(binds, klass, conn) do |record| + sc.execute(binds, conn) do |record| set_inverse_instance(record) end end diff --git a/activerecord/lib/active_record/associations/singular_association.rb b/activerecord/lib/active_record/associations/singular_association.rb index c1eee3c630..441bd715e4 100644 --- a/activerecord/lib/active_record/associations/singular_association.rb +++ b/activerecord/lib/active_record/associations/singular_association.rb @@ -41,15 +41,13 @@ module ActiveRecord return scope.take if skip_statement_cache?(scope) conn = klass.connection - sc = reflection.association_scope_cache(conn, owner) do - StatementCache.create(conn) { |params| - as = AssociationScope.create { params.bind } - target_scope.merge!(as.scope(self)).limit(1) - } + sc = reflection.association_scope_cache(conn, owner) do |params| + as = AssociationScope.create { params.bind } + target_scope.merge!(as.scope(self)).limit(1) end binds = AssociationScope.get_bind_values(owner, reflection.chain) - sc.execute(binds, klass, conn) do |record| + sc.execute(binds, conn) do |record| set_inverse_instance record end.first rescue ::RangeError diff --git a/activerecord/lib/active_record/attribute_methods/serialization.rb b/activerecord/lib/active_record/attribute_methods/serialization.rb index 6ed45d8737..acd47629dd 100644 --- a/activerecord/lib/active_record/attribute_methods/serialization.rb +++ b/activerecord/lib/active_record/attribute_methods/serialization.rb @@ -5,6 +5,16 @@ module ActiveRecord module Serialization extend ActiveSupport::Concern + class ColumnNotSerializableError < StandardError + def initialize(name, type) + super <<-EOS.strip_heredoc + Column `#{name}` of type #{type.class} does not support `serialize` feature. + Usually it means that you are trying to use `serialize` + on a column that already implements serialization natively. + EOS + end + end + module ClassMethods # If you have an attribute that needs to be saved to the database as an # object, and retrieved as the same object, then specify the name of that @@ -60,9 +70,23 @@ module ActiveRecord end decorate_attribute_type(attr_name, :serialize) do |type| + if type_incompatible_with_serialize?(type) + raise ColumnNotSerializableError.new(attr_name, type) + end + Type::Serialized.new(type, coder) end end + + private + + def type_incompatible_with_serialize?(type) + type.is_a?(ActiveRecord::Type::Json) || + ( + defined?(ActiveRecord::ConnectionAdapters::PostgreSQL) && + type.is_a?(ActiveRecord::ConnectionAdapters::PostgreSQL::OID::Array) + ) + end end end end diff --git a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb index 314a35207b..5febb5a59f 100644 --- a/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb +++ b/activerecord/lib/active_record/connection_adapters/abstract/database_statements.rb @@ -465,12 +465,12 @@ module ActiveRecord @binds = [] end - def << str + def <<(str) @parts << str self end - def add_bind obj + def add_bind(obj) @binds << obj @parts << Arel::Nodes::BindParam.new(1) self diff --git a/activerecord/lib/active_record/core.rb b/activerecord/lib/active_record/core.rb index cdd3b5114a..fbb4c671a5 100644 --- a/activerecord/lib/active_record/core.rb +++ b/activerecord/lib/active_record/core.rb @@ -2,6 +2,7 @@ require "active_support/core_ext/hash/indifferent_access" require "active_support/core_ext/string/filters" +require "concurrent/map" module ActiveRecord module Core @@ -149,7 +150,7 @@ module ActiveRecord end def initialize_find_by_cache # :nodoc: - @find_by_statement_cache = { true => {}.extend(Mutex_m), false => {}.extend(Mutex_m) } + @find_by_statement_cache = { true => Concurrent::Map.new, false => Concurrent::Map.new } end def inherited(child_class) # :nodoc: @@ -176,7 +177,7 @@ module ActiveRecord where(key => params.bind).limit(1) } - record = statement.execute([id], self, connection).first + record = statement.execute([id], connection).first unless record raise RecordNotFound.new("Couldn't find #{name} with '#{primary_key}'=#{id}", name, primary_key, id) @@ -208,7 +209,7 @@ module ActiveRecord where(wheres).limit(1) } begin - statement.execute(hash.values, self, connection).first + statement.execute(hash.values, connection).first rescue TypeError raise ActiveRecord::StatementInvalid rescue ::RangeError @@ -281,9 +282,7 @@ module ActiveRecord def cached_find_by_statement(key, &block) cache = @find_by_statement_cache[connection.prepared_statements] - cache[key] || cache.synchronize { - cache[key] ||= StatementCache.create(connection, &block) - } + cache.compute_if_absent(key) { StatementCache.create(connection, &block) } end def relation diff --git a/activerecord/lib/active_record/reflection.rb b/activerecord/lib/active_record/reflection.rb index efe56454d0..97f1a83033 100644 --- a/activerecord/lib/active_record/reflection.rb +++ b/activerecord/lib/active_record/reflection.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require "thread" require "active_support/core_ext/string/filters" require "active_support/deprecation" +require "concurrent/map" module ActiveRecord # = Active Record Reflection @@ -443,8 +443,7 @@ module ActiveRecord @type = options[:as] && (options[:foreign_type] || "#{options[:as]}_type") @foreign_type = options[:foreign_type] || "#{name}_type" @constructable = calculate_constructable(macro, options) - @association_scope_cache = {} - @scope_lock = Mutex.new + @association_scope_cache = Concurrent::Map.new if options[:class_name] && options[:class_name].class == Class ActiveSupport::Deprecation.warn(<<-MSG.squish) @@ -458,14 +457,12 @@ module ActiveRecord end end - def association_scope_cache(conn, owner) + def association_scope_cache(conn, owner, &block) key = conn.prepared_statements if polymorphic? key = [key, owner._read_attribute(@foreign_type)] end - @association_scope_cache[key] ||= @scope_lock.synchronize { - @association_scope_cache[key] ||= yield - } + @association_scope_cache.compute_if_absent(key) { StatementCache.create(conn, &block) } end def constructable? # :nodoc: diff --git a/activerecord/lib/active_record/statement_cache.rb b/activerecord/lib/active_record/statement_cache.rb index 2af7d00246..59acd63a0f 100644 --- a/activerecord/lib/active_record/statement_cache.rb +++ b/activerecord/lib/active_record/statement_cache.rb @@ -11,7 +11,7 @@ module ActiveRecord # The cached statement is executed by using the # {connection.execute}[rdoc-ref:ConnectionAdapters::DatabaseStatements#execute] method: # - # cache.execute([], Book, Book.connection) + # cache.execute([], Book.connection) # # The relation returned by the block is cached, and for each # {execute}[rdoc-ref:ConnectionAdapters::DatabaseStatements#execute] @@ -26,7 +26,7 @@ module ActiveRecord # # And pass the bind values as the first argument of +execute+ call. # - # cache.execute(["my book"], Book, Book.connection) + # cache.execute(["my book"], Book.connection) class StatementCache # :nodoc: class Substitute; end # :nodoc: @@ -87,21 +87,20 @@ module ActiveRecord end end - attr_reader :bind_map, :query_builder - def self.create(connection, block = Proc.new) relation = block.call Params.new query_builder, binds = connection.cacheable_query(self, relation.arel) bind_map = BindMap.new(binds) - new query_builder, bind_map + new(query_builder, bind_map, relation.klass) end - def initialize(query_builder, bind_map) + def initialize(query_builder, bind_map, klass) @query_builder = query_builder - @bind_map = bind_map + @bind_map = bind_map + @klass = klass end - def execute(params, klass, connection, &block) + def execute(params, connection, &block) bind_values = bind_map.bind params sql = query_builder.sql_for bind_values, connection @@ -114,5 +113,9 @@ module ActiveRecord when NilClass, Array, Range, Hash, Relation, Base then true end end + + protected + + attr_reader :query_builder, :bind_map, :klass end end diff --git a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb index 0174c7ea31..856fcc5897 100644 --- a/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb +++ b/activerecord/lib/rails/generators/active_record/migration/migration_generator.rb @@ -28,7 +28,7 @@ module ActiveRecord def set_local_assigns! @migration_template = "migration.rb" case file_name - when /^(add|remove)_.*_(?:to|from)_(.*)/ + when /^(add)_.*_to_(.*)/, /^(remove)_.*?_from_(.*)/ @migration_action = $1 @table_name = normalize_table_name($2) when /join_table/ diff --git a/activerecord/test/cases/adapters/postgresql/array_test.rb b/activerecord/test/cases/adapters/postgresql/array_test.rb index 8507dbb463..08b17f37e2 100644 --- a/activerecord/test/cases/adapters/postgresql/array_test.rb +++ b/activerecord/test/cases/adapters/postgresql/array_test.rb @@ -47,6 +47,15 @@ class PostgresqlArrayTest < ActiveRecord::PostgreSQLTestCase assert ratings_column.array? end + def test_not_compatible_with_serialize + new_klass = Class.new(PgArray) do + serialize :tags, Array + end + assert_raises(ActiveRecord::AttributeMethods::Serialization::ColumnNotSerializableError) do + new_klass.new + end + end + def test_default @connection.add_column "pg_arrays", "score", :integer, array: true, default: [4, 4, 2] PgArray.reset_column_information diff --git a/activerecord/test/cases/adapters/postgresql/json_test.rb b/activerecord/test/cases/adapters/postgresql/json_test.rb index aa5e03df41..79dcfe110c 100644 --- a/activerecord/test/cases/adapters/postgresql/json_test.rb +++ b/activerecord/test/cases/adapters/postgresql/json_test.rb @@ -33,6 +33,15 @@ module PostgresqlJSONSharedTestCases x.reload assert_equal ["foo" => "bar"], x.objects end + + def test_not_compatible_with_serialize_macro + new_klass = Class.new(klass) do + serialize :payload, JSON + end + assert_raises(ActiveRecord::AttributeMethods::Serialization::ColumnNotSerializableError) do + new_klass.new + end + end end class PostgresqlJSONTest < ActiveRecord::PostgreSQLTestCase diff --git a/activerecord/test/cases/statement_cache_test.rb b/activerecord/test/cases/statement_cache_test.rb index e2142267ea..1f715e41a6 100644 --- a/activerecord/test/cases/statement_cache_test.rb +++ b/activerecord/test/cases/statement_cache_test.rb @@ -21,9 +21,9 @@ module ActiveRecord Book.where(name: params.bind) end - b = cache.execute([ "my book" ], Book, Book.connection) + b = cache.execute([ "my book" ], Book.connection) assert_equal "my book", b[0].name - b = cache.execute([ "my other book" ], Book, Book.connection) + b = cache.execute([ "my other book" ], Book.connection) assert_equal "my other book", b[0].name end @@ -35,9 +35,9 @@ module ActiveRecord Book.where(id: params.bind) end - b = cache.execute([ b1.id ], Book, Book.connection) + b = cache.execute([ b1.id ], Book.connection) assert_equal b1.name, b[0].name - b = cache.execute([ b2.id ], Book, Book.connection) + b = cache.execute([ b2.id ], Book.connection) assert_equal b2.name, b[0].name end @@ -60,7 +60,7 @@ module ActiveRecord Book.create(name: "my book", author_id: 4) - books = cache.execute([], Book, Book.connection) + books = cache.execute([], Book.connection) assert_equal "my book", books[0].name end @@ -73,7 +73,7 @@ module ActiveRecord molecule = salty.molecules.create(name: "dioxane") molecule.electrons.create(name: "lepton") - liquids = cache.execute([], Book, Book.connection) + liquids = cache.execute([], Book.connection) assert_equal "salty", liquids[0].name end @@ -86,13 +86,13 @@ module ActiveRecord Book.create(name: "my book") end - first_books = cache.execute([], Book, Book.connection) + first_books = cache.execute([], Book.connection) 3.times do Book.create(name: "my book") end - additional_books = cache.execute([], Book, Book.connection) + additional_books = cache.execute([], Book.connection) assert first_books != additional_books end diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 938d4f9d31..6908882123 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,20 @@ +* Update `String#camelize` to provide feedback when wrong option is passed + + `String#camelize` was returning nil without any feedback when an + invalid option was passed as parameter. + + Previously: + + 'one_two'.camelize(true) + => nil + + Now: + + 'one_two'.camelize(true) + => ArgumentError: Invalid option, use either :upper or :lower. + + *Ricardo Díaz* + * Fix modulo operations involving durations Rails 5.1 introduce an `ActiveSupport::Duration::Scalar` class as a wrapper diff --git a/activesupport/lib/active_support/core_ext/load_error.rb b/activesupport/lib/active_support/core_ext/load_error.rb index c3939b5d0b..750f858fcc 100644 --- a/activesupport/lib/active_support/core_ext/load_error.rb +++ b/activesupport/lib/active_support/core_ext/load_error.rb @@ -1,13 +1,6 @@ # frozen_string_literal: true class LoadError - REGEXPS = [ - /^no such file to load -- (.+)$/i, - /^Missing \w+ (?:file\s*)?([^\s]+.rb)$/i, - /^Missing API definition file in (.+)$/i, - /^cannot load such file -- (.+)$/i, - ] - # Returns true if the given path name (except perhaps for the ".rb" # extension) is the missing file which caused the exception to be raised. def is_missing?(location) diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb index 846600c623..b5bb385033 100644 --- a/activesupport/lib/active_support/core_ext/string/inflections.rb +++ b/activesupport/lib/active_support/core_ext/string/inflections.rb @@ -94,6 +94,8 @@ class String ActiveSupport::Inflector.camelize(self, true) when :lower ActiveSupport::Inflector.camelize(self, false) + else + raise ArgumentError, "Invalid option, use either :upper or :lower." end end alias_method :camelcase, :camelize diff --git a/activesupport/lib/active_support/messages/metadata.rb b/activesupport/lib/active_support/messages/metadata.rb index db14ac0b1c..a45aecfcd0 100644 --- a/activesupport/lib/active_support/messages/metadata.rb +++ b/activesupport/lib/active_support/messages/metadata.rb @@ -1,4 +1,5 @@ # frozen_string_literal: true + require "time" module ActiveSupport diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb index 0afff5aa50..9fd6d8ac0f 100644 --- a/activesupport/test/core_ext/string_ext_test.rb +++ b/activesupport/test/core_ext/string_ext_test.rb @@ -108,6 +108,13 @@ class StringInflectionsTest < ActiveSupport::TestCase assert_equal("capital", "Capital".camelize(:lower)) end + def test_camelize_invalid_option + e = assert_raise ArgumentError do + "Capital".camelize(nil) + end + assert_equal("Invalid option, use either :upper or :lower.", e.message) + end + def test_dasherize UnderscoresToDashes.each do |underscored, dasherized| assert_equal(dasherized, underscored.dasherize) diff --git a/railties/lib/rails/app_loader.rb b/railties/lib/rails/app_loader.rb index 3dc4fe5414..9e4f715ca6 100644 --- a/railties/lib/rails/app_loader.rb +++ b/railties/lib/rails/app_loader.rb @@ -8,15 +8,26 @@ module Rails RUBY = Gem.ruby EXECUTABLES = ["bin/rails", "script/rails"] BUNDLER_WARNING = <<EOS -Looks like your app's ./bin/rails is a stub that was generated by Bundler. +Beginning in Rails 4, Rails ships with a `rails` binstub at ./bin/rails that +should be used instead of the Bundler-generated `rails` binstub. -In Rails #{Rails::VERSION::MAJOR}, your app's bin/ directory contains executables that are versioned -like any other source code, rather than stubs that are generated on demand. +If you are seeing this message, your binstub at ./bin/rails was generated by +Bundler instead of Rails. -Here's how to upgrade: +You might need to regenerate your `rails` binstub locally and add it to source +control: + + rails app:update:bin # Bear in mind this generates other binstubs + # too that you may or may not want (like yarn) + +If you already have Rails binstubs in source control, you might be +inadverently overwriting them during deployment by using bundle install +with the --binstubs option. + +If your application was created prior to Rails 4, here's how to upgrade: bundle config --delete bin # Turn off Bundler's stub generator - rails app:update:bin # Use the new Rails 5 executables + rails app:update:bin # Use the new Rails executables git add bin # Add bin/ to source control You may need to remove bin/ from your .gitignore as well. diff --git a/railties/lib/rails/generators/rails/app/templates/Gemfile b/railties/lib/rails/generators/rails/app/templates/Gemfile index 4b2842ef46..7b7bebc957 100644 --- a/railties/lib/rails/generators/rails/app/templates/Gemfile +++ b/railties/lib/rails/generators/rails/app/templates/Gemfile @@ -1,7 +1,11 @@ source 'https://rubygems.org' git_source(:github) { |repo| "https://github.com/#{repo}.git" } -ruby <%= "'#{RUBY_VERSION}'" %> +ruby <%= "'#{RUBY_VERSION}'" -%> + +<% unless gemfile_entries.first.comment -%> + +<% end -%> <% gemfile_entries.each do |gem| -%> <% if gem.comment -%> diff --git a/railties/lib/rails/generators/testing/assertions.rb b/railties/lib/rails/generators/testing/assertions.rb index 1cabf4e28c..9ac70f60c0 100644 --- a/railties/lib/rails/generators/testing/assertions.rb +++ b/railties/lib/rails/generators/testing/assertions.rb @@ -113,7 +113,11 @@ module Rails # # assert_field_default_value :string, "MyString" def assert_field_default_value(attribute_type, value) - assert_equal(value, create_generated_attribute(attribute_type).default) + if value.nil? + assert_nil(create_generated_attribute(attribute_type).default) + else + assert_equal(value, create_generated_attribute(attribute_type).default) + end end end end diff --git a/railties/test/generators/migration_generator_test.rb b/railties/test/generators/migration_generator_test.rb index 6fe6e4ca07..db0808750e 100644 --- a/railties/test/generators/migration_generator_test.rb +++ b/railties/test/generators/migration_generator_test.rb @@ -48,6 +48,17 @@ class MigrationGeneratorTest < Rails::Generators::TestCase end end + def test_add_migration_with_table_having_from_in_title + migration = "add_email_address_to_blacklisted_from_campaign" + run_generator [migration, "email_address:string"] + + assert_migration "db/migrate/#{migration}.rb" do |content| + assert_method :change, content do |change| + assert_match(/add_column :blacklisted_from_campaigns, :email_address, :string/, change) + end + end + end + def test_remove_migration_with_indexed_attribute migration = "remove_title_body_from_posts" run_generator [migration, "title:string:index", "body:text"] @@ -73,6 +84,17 @@ class MigrationGeneratorTest < Rails::Generators::TestCase end end + def test_remove_migration_with_table_having_to_in_title + migration = "remove_email_address_from_sent_to_user" + run_generator [migration, "email_address:string"] + + assert_migration "db/migrate/#{migration}.rb" do |content| + assert_method :change, content do |change| + assert_match(/remove_column :sent_to_users, :email_address, :string/, change) + end + end + end + def test_remove_migration_with_references_options migration = "remove_references_from_books" run_generator [migration, "author:belongs_to", "distributor:references{polymorphic}"] diff --git a/tasks/release.rb b/tasks/release.rb index 8644d1a9c0..6ff37fb415 100644 --- a/tasks/release.rb +++ b/tasks/release.rb @@ -234,7 +234,7 @@ task :announce do versions = ENV["VERSIONS"] ? ENV["VERSIONS"].split(",") : [ version ] versions = versions.sort.map { |v| Announcement::Version.new(v) } - raise "Only valid for patch releases" if versions.any?(&:major_or_security?) + raise "Only valid for patch releases" if versions.any?(&:major_or_security?) if versions.any?(&:rc?) require "date" |