diff options
21 files changed, 231 insertions, 35 deletions
diff --git a/activerecord/CHANGELOG.md b/activerecord/CHANGELOG.md index c0e3840b40..840ad0d7ce 100644 --- a/activerecord/CHANGELOG.md +++ b/activerecord/CHANGELOG.md @@ -1,3 +1,11 @@ +* Improve support for non Active Record objects on `validates_associated` + + Skipping `marked_for_destruction?` when the associated object does not responds + to it make easier to validate virtual associations built on top of Active Model + objects and/or serialized objects that implement a `valid?` instance method. + + *Kassio Borges*, *Lucas Mazza* + * Change connection management middleware to return a new response with a body proxy, rather than mutating the original. diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index dae468ba54..81c535d962 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -116,8 +116,7 @@ module ActiveRecord::Associations::Builder # :nodoc: end def self.add_destroy_callbacks(model, reflection) - name = reflection.name - model.after_destroy lambda { |o| o.association(name).handle_dependency } + model.after_destroy lambda { |o| o.association(reflection.name).handle_dependency } end def self.define_validations(model, reflection) diff --git a/activerecord/lib/active_record/associations/join_dependency.rb b/activerecord/lib/active_record/associations/join_dependency.rb index 9f183c3e7e..0e4e951269 100644 --- a/activerecord/lib/active_record/associations/join_dependency.rb +++ b/activerecord/lib/active_record/associations/join_dependency.rb @@ -32,7 +32,7 @@ module ActiveRecord @alias_cache[node][column] end - class Table < Struct.new(:node, :columns) + class Table < Struct.new(:node, :columns) # :nodoc: def table Arel::Nodes::TableAlias.new node.table, node.aliased_table_name end diff --git a/activerecord/lib/active_record/enum.rb b/activerecord/lib/active_record/enum.rb index 8fba6fcc35..7ded96f8fb 100644 --- a/activerecord/lib/active_record/enum.rb +++ b/activerecord/lib/active_record/enum.rb @@ -104,7 +104,7 @@ module ActiveRecord super end - class EnumType < Type::Value + class EnumType < Type::Value # :nodoc: def initialize(name, mapping) @name = name @mapping = mapping diff --git a/activerecord/lib/active_record/relation/record_fetch_warning.rb b/activerecord/lib/active_record/relation/record_fetch_warning.rb index 14e1bf89fa..0a1814b3dd 100644 --- a/activerecord/lib/active_record/relation/record_fetch_warning.rb +++ b/activerecord/lib/active_record/relation/record_fetch_warning.rb @@ -23,11 +23,13 @@ module ActiveRecord end end + # :stopdoc: ActiveSupport::Notifications.subscribe("sql.active_record") do |*args| payload = args.last QueryRegistry.queries << payload[:sql] end + # :startdoc: class QueryRegistry # :nodoc: extend ActiveSupport::PerThreadRegistry diff --git a/activerecord/lib/active_record/type_caster.rb b/activerecord/lib/active_record/type_caster.rb index 63ba10c289..accc339d00 100644 --- a/activerecord/lib/active_record/type_caster.rb +++ b/activerecord/lib/active_record/type_caster.rb @@ -2,6 +2,6 @@ require 'active_record/type_caster/map' require 'active_record/type_caster/connection' module ActiveRecord - module TypeCaster + module TypeCaster # :nodoc: end end diff --git a/activerecord/lib/active_record/type_caster/connection.rb b/activerecord/lib/active_record/type_caster/connection.rb index 868d08ed44..7ed8dcc313 100644 --- a/activerecord/lib/active_record/type_caster/connection.rb +++ b/activerecord/lib/active_record/type_caster/connection.rb @@ -1,6 +1,6 @@ module ActiveRecord module TypeCaster - class Connection + class Connection # :nodoc: def initialize(klass, table_name) @klass = klass @table_name = table_name diff --git a/activerecord/lib/active_record/type_caster/map.rb b/activerecord/lib/active_record/type_caster/map.rb index 4b1941351c..3a367b3999 100644 --- a/activerecord/lib/active_record/type_caster/map.rb +++ b/activerecord/lib/active_record/type_caster/map.rb @@ -1,6 +1,6 @@ module ActiveRecord module TypeCaster - class Map + class Map # :nodoc: def initialize(types) @types = types end diff --git a/activerecord/lib/active_record/validations/associated.rb b/activerecord/lib/active_record/validations/associated.rb index 32fbaf0a91..b14db85167 100644 --- a/activerecord/lib/active_record/validations/associated.rb +++ b/activerecord/lib/active_record/validations/associated.rb @@ -2,10 +2,16 @@ module ActiveRecord module Validations class AssociatedValidator < ActiveModel::EachValidator #:nodoc: def validate_each(record, attribute, value) - if Array.wrap(value).reject {|r| r.marked_for_destruction? || r.valid?}.any? - record.errors.add(attribute, :invalid, options.merge(:value => value)) + if Array(value).reject { |r| valid_object?(r) }.any? + record.errors.add(attribute, :invalid, options.merge(value: value)) end end + + private + + def valid_object?(record) + (record.respond_to?(:marked_for_destruction?) && record.marked_for_destruction?) || record.valid? + end end module ClassMethods diff --git a/activerecord/test/cases/validations/association_validation_test.rb b/activerecord/test/cases/validations/association_validation_test.rb index bff5ffa65e..584a3dc0d8 100644 --- a/activerecord/test/cases/validations/association_validation_test.rb +++ b/activerecord/test/cases/validations/association_validation_test.rb @@ -45,6 +45,18 @@ class AssociationValidationTest < ActiveRecord::TestCase assert t.valid? end + def test_validates_associated_without_marked_for_destruction + reply = Class.new do + def valid? + true + end + end + Topic.validates_associated(:replies) + t = Topic.new + t.define_singleton_method(:replies) { [reply.new] } + assert t.valid? + end + def test_validates_associated_with_custom_message_using_quotes Reply.validates_associated :topic, :message=> "This string contains 'single' and \"double\" quotes" Topic.validates_presence_of :content diff --git a/activesupport/CHANGELOG.md b/activesupport/CHANGELOG.md index 7c0f3eae80..d505218c7a 100644 --- a/activesupport/CHANGELOG.md +++ b/activesupport/CHANGELOG.md @@ -1,3 +1,12 @@ +* Updated `parameterize` to preserve the case of a string, optionally. + + Example: + + parameterize("Donald E. Knuth", separator: '_') # => "donald_e_knuth" + parameterize("Donald E. Knuth", preserve_case: true) # => "Donald-E-Knuth" + + *Swaathi Kakarla* + * `HashWithIndifferentAccess.new` respects the default value or proc on objects that respond to `#to_hash`. `.new_from_hash_copying_default` simply invokes `.new`. All calls to `.new_from_hash_copying_default` are replaced with `.new`. diff --git a/activesupport/lib/active_support/cache/strategy/local_cache.rb b/activesupport/lib/active_support/cache/strategy/local_cache.rb index 4e2d3e9875..d521061004 100644 --- a/activesupport/lib/active_support/cache/strategy/local_cache.rb +++ b/activesupport/lib/active_support/cache/strategy/local_cache.rb @@ -48,10 +48,6 @@ module ActiveSupport @data.clear end - def fetch(*args, &block) - @data.fetch(*args, &block) - end - def read_entry(key, options) @data[key] end @@ -64,6 +60,10 @@ module ActiveSupport def delete_entry(key, options) !!@data.delete(key) end + + def fetch_entry(key, options = nil) # :nodoc: + @data.fetch(key) { @data[key] = yield } + end end # Use a local cache for the duration of block. @@ -103,11 +103,7 @@ module ActiveSupport protected def read_entry(key, options) # :nodoc: if cache = local_cache - cache.fetch(key) do - entry = super - cache.write_entry(key, entry, options) - entry - end + cache.fetch_entry(key) { super } else super end diff --git a/activesupport/lib/active_support/core_ext/string/inflections.rb b/activesupport/lib/active_support/core_ext/string/inflections.rb index b2e713077c..cc71b8155d 100644 --- a/activesupport/lib/active_support/core_ext/string/inflections.rb +++ b/activesupport/lib/active_support/core_ext/string/inflections.rb @@ -164,8 +164,26 @@ class String # # <%= link_to(@person.name, person_path) %> # # => <a href="/person/1-donald-e-knuth">Donald E. Knuth</a> - def parameterize(sep = '-'.freeze) - ActiveSupport::Inflector.parameterize(self, sep) + # + # To preserve the case of the characters in a string, use the `preserve_case` argument. + # + # class Person + # def to_param + # "#{id}-#{name.parameterize(preserve_case: true)}" + # end + # end + # + # @person = Person.find(1) + # # => #<Person id: 1, name: "Donald E. Knuth"> + # + # <%= link_to(@person.name, person_path) %> + # # => <a href="/person/1-Donald-E-Knuth">Donald E. Knuth</a> + def parameterize(sep = :unused, separator: '-', preserve_case: false) + unless sep == :unused + ActiveSupport::Deprecation.warn("Passing the separator argument as a positional parameter is deprecated and will soon be removed. Use `separator: '#{sep}'` instead.") + separator = sep + end + ActiveSupport::Inflector.parameterize(self, separator: separator, preserve_case: preserve_case) end # Creates the name of a table like Rails does for models to table names. This method diff --git a/activesupport/lib/active_support/inflector/transliterate.rb b/activesupport/lib/active_support/inflector/transliterate.rb index 103207fb63..871cfb8a72 100644 --- a/activesupport/lib/active_support/inflector/transliterate.rb +++ b/activesupport/lib/active_support/inflector/transliterate.rb @@ -68,29 +68,44 @@ module ActiveSupport # # parameterize("Donald E. Knuth") # => "donald-e-knuth" # parameterize("^trés|Jolie-- ") # => "tres-jolie" - def parameterize(string, sep = '-') + # + # To use a custom separator, override the `separator` argument. + # + # parameterize("Donald E. Knuth", separator: '_') # => "donald_e_knuth" + # parameterize("^trés|Jolie-- ", separator: '_') # => "tres_jolie" + # + # To preserve the case of the characters in a string, use the `preserve_case` argument. + # + # parameterize("Donald E. Knuth", preserve_case: true) # => "Donald-E-Knuth" + # parameterize("^trés|Jolie-- ", preserve_case: true) # => "tres-Jolie" + # + def parameterize(string, sep = :unused, separator: '-', preserve_case: false) + unless sep == :unused + ActiveSupport::Deprecation.warn("Passing the separator argument as a positional parameter is deprecated and will soon be removed. Use `separator: '#{sep}'` instead.") + separator = sep + end # Replace accented chars with their ASCII equivalents. parameterized_string = transliterate(string) # Turn unwanted chars into the separator. - parameterized_string.gsub!(/[^a-z0-9\-_]+/i, sep) + parameterized_string.gsub!(/[^a-z0-9\-_]+/i, separator) - unless sep.nil? || sep.empty? - if sep == "-".freeze + unless separator.nil? || separator.empty? + if separator == "-".freeze re_duplicate_separator = /-{2,}/ re_leading_trailing_separator = /^-|-$/i else - re_sep = Regexp.escape(sep) + re_sep = Regexp.escape(separator) re_duplicate_separator = /#{re_sep}{2,}/ re_leading_trailing_separator = /^#{re_sep}|#{re_sep}$/i end # No more than one of the separator in a row. - parameterized_string.gsub!(re_duplicate_separator, sep) + parameterized_string.gsub!(re_duplicate_separator, separator) # Remove leading/trailing separator. parameterized_string.gsub!(re_leading_trailing_separator, ''.freeze) end - - parameterized_string.downcase! + + parameterized_string.downcase! unless preserve_case parameterized_string end end diff --git a/activesupport/test/caching_test.rb b/activesupport/test/caching_test.rb index 94e73b6df3..3629f5e64b 100644 --- a/activesupport/test/caching_test.rb +++ b/activesupport/test/caching_test.rb @@ -633,6 +633,13 @@ module LocalCacheBehavior end end + def test_local_cache_fetch + @cache.with_local_cache do + @cache.send(:local_cache).write 'foo', 'bar' + assert_equal 'bar', @cache.send(:local_cache).fetch('foo') + end + end + def test_local_cache_of_write_nil @cache.with_local_cache do assert @cache.write('foo', nil) @@ -1102,11 +1109,11 @@ class CacheStoreLoggerTest < ActiveSupport::TestCase def test_log_with_proc_namespace proc = Proc.new do "proc_namespace" - end + end @cache.fetch('foo', {:namespace => proc}) { 'bar' } assert_match %r{proc_namespace:foo}, @buffer.string end - + def test_mute_logging @cache.mute { @cache.fetch('foo') { 'bar' } } assert @buffer.string.blank? diff --git a/activesupport/test/core_ext/string_ext_test.rb b/activesupport/test/core_ext/string_ext_test.rb index 9cc7bb1a77..2e69816364 100644 --- a/activesupport/test/core_ext/string_ext_test.rb +++ b/activesupport/test/core_ext/string_ext_test.rb @@ -143,15 +143,49 @@ class StringInflectionsTest < ActiveSupport::TestCase end end + def test_string_parameterized_normal_preserve_case + StringToParameterizedPreserveCase.each do |normal, slugged| + assert_equal(normal.parameterize(preserve_case: true), slugged) + end + end + def test_string_parameterized_no_separator StringToParameterizeWithNoSeparator.each do |normal, slugged| - assert_equal(normal.parameterize(''), slugged) + assert_equal(normal.parameterize(separator: ''), slugged) + end + end + + def test_string_parameterized_no_separator_deprecated + StringToParameterizeWithNoSeparator.each do |normal, slugged| + assert_deprecated(/Passing the separator argument as a positional parameter is deprecated and will soon be removed. Use `separator: ''` instead./i) do + assert_equal(normal.parameterize(''), slugged) + end + end + end + + def test_string_parameterized_no_separator_preserve_case + StringToParameterizePreserveCaseWithNoSeparator.each do |normal, slugged| + assert_equal(normal.parameterize(separator: '', preserve_case: true), slugged) end end def test_string_parameterized_underscore StringToParameterizeWithUnderscore.each do |normal, slugged| - assert_equal(normal.parameterize('_'), slugged) + assert_equal(normal.parameterize(separator: '_'), slugged) + end + end + + def test_string_parameterized_underscore_deprecated + StringToParameterizeWithUnderscore.each do |normal, slugged| + assert_deprecated(/Passing the separator argument as a positional parameter is deprecated and will soon be removed. Use `separator: '_'` instead./i) do + assert_equal(normal.parameterize('_'), slugged) + end + end + end + + def test_string_parameterized_underscore_preserve_case + StringToParameterizePreserceCaseWithUnderscore.each do |normal, slugged| + assert_equal(normal.parameterize(separator: '_', preserve_case: true), slugged) end end diff --git a/activesupport/test/inflector_test.rb b/activesupport/test/inflector_test.rb index a0764f6d6b..06cd41c86d 100644 --- a/activesupport/test/inflector_test.rb +++ b/activesupport/test/inflector_test.rb @@ -269,14 +269,32 @@ class InflectorTest < ActiveSupport::TestCase def test_parameterize_with_custom_separator jruby_skip "UTF-8 to UTF8-MAC Converter is unavailable" StringToParameterizeWithUnderscore.each do |some_string, parameterized_string| - assert_equal(parameterized_string, ActiveSupport::Inflector.parameterize(some_string, '_')) + assert_equal(parameterized_string, ActiveSupport::Inflector.parameterize(some_string, separator: '_')) + end + end + + def test_parameterize_with_custom_separator_deprecated + jruby_skip "UTF-8 to UTF8-MAC Converter is unavailable" + StringToParameterizeWithUnderscore.each do |some_string, parameterized_string| + assert_deprecated(/Passing the separator argument as a positional parameter is deprecated and will soon be removed. Use `separator: '_'` instead./i) do + assert_equal(parameterized_string, ActiveSupport::Inflector.parameterize(some_string, '_')) + end end end def test_parameterize_with_multi_character_separator jruby_skip "UTF-8 to UTF8-MAC Converter is unavailable" StringToParameterized.each do |some_string, parameterized_string| - assert_equal(parameterized_string.gsub('-', '__sep__'), ActiveSupport::Inflector.parameterize(some_string, '__sep__')) + assert_equal(parameterized_string.gsub('-', '__sep__'), ActiveSupport::Inflector.parameterize(some_string, separator: '__sep__')) + end + end + + def test_parameterize_with_multi_character_separator_deprecated + jruby_skip "UTF-8 to UTF8-MAC Converter is unavailable" + StringToParameterized.each do |some_string, parameterized_string| + assert_deprecated(/Passing the separator argument as a positional parameter is deprecated and will soon be removed. Use `separator: '__sep__'` instead./i) do + assert_equal(parameterized_string.gsub('-', '__sep__'), ActiveSupport::Inflector.parameterize(some_string, '__sep__')) + end end end diff --git a/activesupport/test/inflector_test_cases.rb b/activesupport/test/inflector_test_cases.rb index e6898658b5..14fe97a986 100644 --- a/activesupport/test/inflector_test_cases.rb +++ b/activesupport/test/inflector_test_cases.rb @@ -174,6 +174,17 @@ module InflectorTestCases "Test with malformed utf8 \251" => "test-with-malformed-utf8" } + StringToParameterizedPreserveCase = { + "Donald E. Knuth" => "Donald-E-Knuth", + "Random text with *(bad)* characters" => "Random-text-with-bad-characters", + "Allow_Under_Scores" => "Allow_Under_Scores", + "Trailing bad characters!@#" => "Trailing-bad-characters", + "!@#Leading bad characters" => "Leading-bad-characters", + "Squeeze separators" => "Squeeze-separators", + "Test with + sign" => "Test-with-sign", + "Test with malformed utf8 \xA9" => "Test-with-malformed-utf8" + } + StringToParameterizeWithNoSeparator = { "Donald E. Knuth" => "donaldeknuth", "With-some-dashes" => "with-some-dashes", @@ -185,6 +196,17 @@ module InflectorTestCases "Test with malformed utf8 \251" => "testwithmalformedutf8" } + StringToParameterizePreserveCaseWithNoSeparator = { + "Donald E. Knuth" => "DonaldEKnuth", + "With-some-dashes" => "With-some-dashes", + "Random text with *(bad)* characters" => "Randomtextwithbadcharacters", + "Trailing bad characters!@#" => "Trailingbadcharacters", + "!@#Leading bad characters" => "Leadingbadcharacters", + "Squeeze separators" => "Squeezeseparators", + "Test with + sign" => "Testwithsign", + "Test with malformed utf8 \xA9" => "Testwithmalformedutf8" + } + StringToParameterizeWithUnderscore = { "Donald E. Knuth" => "donald_e_knuth", "Random text with *(bad)* characters" => "random_text_with_bad_characters", @@ -197,6 +219,18 @@ module InflectorTestCases "Test with malformed utf8 \251" => "test_with_malformed_utf8" } + StringToParameterizePreserceCaseWithUnderscore = { + "Donald E. Knuth" => "Donald_E_Knuth", + "Random text with *(bad)* characters" => "Random_text_with_bad_characters", + "With-some-dashes" => "With-some-dashes", + "Allow_Under_Scores" => "Allow_Under_Scores", + "Trailing bad characters!@#" => "Trailing_bad_characters", + "!@#Leading bad characters" => "Leading_bad_characters", + "Squeeze separators" => "Squeeze_separators", + "Test with + sign" => "Test_with_sign", + "Test with malformed utf8 \xA9" => "Test_with_malformed_utf8" + } + StringToParameterizedAndNormalized = { "Malmö" => "malmo", "Garçons" => "garcons", diff --git a/guides/source/active_support_core_extensions.md b/guides/source/active_support_core_extensions.md index f6fc255c24..181dca4b71 100644 --- a/guides/source/active_support_core_extensions.md +++ b/guides/source/active_support_core_extensions.md @@ -1710,6 +1710,20 @@ The method `parameterize` normalizes its receiver in a way that can be used in p "Kurt Gödel".parameterize # => "kurt-godel" ``` +To preserve the case of the string, set the `preserve_case` argument to true. By default, `preserve_case` is set to false. + +```ruby +"John Smith".parameterize(preserve_case: true) # => "John-Smith" +"Kurt Gödel".parameterize(preserve_case: true) # => "Kurt-Godel" +``` + +To use a custom separator, override the `separator` argument. + +```ruby +"John Smith".parameterize(separator: "_") # => "john\_smith" +"Kurt Gödel".parameterize(separator: "_") # => "kurt\_godel" +``` + In fact, the result string is wrapped in an instance of `ActiveSupport::Multibyte::Chars`. NOTE: Defined in `active_support/core_ext/string/inflections.rb`. diff --git a/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb b/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb index bc3c9b3f6b..17a86f376b 100644 --- a/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb +++ b/railties/lib/rails/generators/rails/scaffold_controller/templates/api_controller.rb @@ -1,5 +1,5 @@ <% if namespaced? -%> -require_dependency "<%= namespaced_file_path %>/application_controller" +require_dependency "<%= namespaced_path %>/application_controller" <% end -%> <% module_namespacing do -%> diff --git a/railties/test/generators/namespaced_generators_test.rb b/railties/test/generators/namespaced_generators_test.rb index c4ee6602c5..590f06e19a 100644 --- a/railties/test/generators/namespaced_generators_test.rb +++ b/railties/test/generators/namespaced_generators_test.rb @@ -396,4 +396,28 @@ class NamespacedScaffoldGeneratorTest < NamespacedGeneratorTestCase # Stylesheets (should not be removed) assert_file "app/assets/stylesheets/scaffold.css" end + + def test_api_scaffold_with_namespace_on_invoke + run_generator [ "admin/role", "name:string", "description:string", "--api" ] + + # Model + assert_file "app/models/test_app/admin.rb", /module TestApp\n module Admin/ + assert_file "app/models/test_app/admin/role.rb", /module TestApp\n class Admin::Role < ActiveRecord::Base/ + assert_file "test/models/test_app/admin/role_test.rb", /module TestApp\n class Admin::RoleTest < ActiveSupport::TestCase/ + assert_file "test/fixtures/test_app/admin/roles.yml" + assert_migration "db/migrate/create_test_app_admin_roles.rb" + + # Route + assert_file "config/routes.rb" do |route| + assert_match(/^ namespace :admin do\n resources :roles\n end$/, route) + end + + # Controller + assert_file "app/controllers/test_app/admin/roles_controller.rb" do |content| + assert_match(/module TestApp\n class Admin::RolesController < ApplicationController/, content) + assert_match(%r(require_dependency "test_app/application_controller"), content) + end + assert_file "test/controllers/test_app/admin/roles_controller_test.rb", + /module TestApp\n class Admin::RolesControllerTest < ActionController::TestCase/ + end end |