diff options
18 files changed, 94 insertions, 64 deletions
diff --git a/actionview/test/template/form_collections_helper_test.rb b/actionview/test/template/form_collections_helper_test.rb index 68c83f2059..7a62b9d907 100644 --- a/actionview/test/template/form_collections_helper_test.rb +++ b/actionview/test/template/form_collections_helper_test.rb @@ -60,7 +60,7 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_no_select 'input[type=radio][value=other][disabled=disabled]' end - test 'collection radio accepts single disable item' do + test 'collection radio accepts single disabled item' do collection = [[1, true], [0, false]] with_collection_radio_buttons :user, :active, collection, :last, :first, :disabled => true @@ -300,7 +300,7 @@ class FormCollectionsHelperTest < ActionView::TestCase assert_no_select 'input[type=checkbox][value=2][disabled=disabled]' end - test 'collection check boxes accepts single disable item' do + test 'collection check boxes accepts single disabled item' do collection = (1..3).map{|i| [i, "Category #{i}"] } with_collection_check_boxes :user, :category_ids, collection, :first, :last, :disabled => 1 diff --git a/activerecord/lib/active_record/associations/builder/association.rb b/activerecord/lib/active_record/associations/builder/association.rb index f86d975592..3911d1b520 100644 --- a/activerecord/lib/active_record/associations/builder/association.rb +++ b/activerecord/lib/active_record/associations/builder/association.rb @@ -15,68 +15,67 @@ module ActiveRecord::Associations::Builder class Association #:nodoc: class << self attr_accessor :extensions + # TODO: This class accessor is needed to make activerecord-deprecated_finders work. + # We can move it to a constant in 5.0. + attr_accessor :valid_options end self.extensions = [] - # TODO: This class accessor is needed to make activerecord-deprecated_finders work. - # We can move it to a constant in 5.0. - cattr_accessor :valid_options, instance_accessor: false self.valid_options = [:class_name, :class, :foreign_key, :validate] + attr_reader :name, :scope, :options + def self.build(model, name, scope, options, &block) - extension = define_extensions model, name, &block - reflection = create_reflection model, name, scope, options, extension + builder = create_builder model, name, scope, options, &block + reflection = builder.build(model) define_accessors model, reflection define_callbacks model, reflection + builder.define_extensions model reflection end - def self.create_reflection(model, name, scope, options, extension = nil) + def self.create_builder(model, name, scope, options, &block) raise ArgumentError, "association names must be a Symbol" unless name.kind_of?(Symbol) + new(model, name, scope, options, &block) + end + + def initialize(model, name, scope, options) + # TODO: Move this to create_builder as soon we drop support to activerecord-deprecated_finders. if scope.is_a?(Hash) options = scope scope = nil end - validate_options(options) - - scope = build_scope(scope, extension) - - ActiveRecord::Reflection.create(macro, name, scope, options, model) - end + # TODO: Remove this model argument as soon we drop support to activerecord-deprecated_finders. + @name = name + @scope = scope + @options = options - def self.build_scope(scope, extension) - new_scope = scope + validate_options if scope && scope.arity == 0 - new_scope = proc { instance_exec(&scope) } - end - - if extension - new_scope = wrap_scope new_scope, extension + @scope = proc { instance_exec(&scope) } end - - new_scope end - def self.wrap_scope(scope, extension) - scope + def build(model) + ActiveRecord::Reflection.create(macro, name, scope, options, model) end - def self.macro + def macro raise NotImplementedError end - def self.build_valid_options(options) - self.valid_options + Association.extensions.flat_map(&:valid_options) + def valid_options + Association.valid_options + Association.extensions.flat_map(&:valid_options) end - def self.validate_options(options) - options.assert_valid_keys(build_valid_options(options)) + def validate_options + options.assert_valid_keys(valid_options) end - def self.define_extensions(model, name) + def define_extensions(model) end def self.define_callbacks(model, reflection) @@ -119,6 +118,8 @@ module ActiveRecord::Associations::Builder raise NotImplementedError end + private + def self.add_before_destroy_callbacks(model, reflection) unless valid_dependent_options.include? reflection.options[:dependent] raise ArgumentError, "The :dependent option must be one of #{valid_dependent_options}, but is :#{reflection.options[:dependent]}" diff --git a/activerecord/lib/active_record/associations/builder/belongs_to.rb b/activerecord/lib/active_record/associations/builder/belongs_to.rb index 7dd8a7deb1..62cc1e3a8d 100644 --- a/activerecord/lib/active_record/associations/builder/belongs_to.rb +++ b/activerecord/lib/active_record/associations/builder/belongs_to.rb @@ -1,10 +1,10 @@ module ActiveRecord::Associations::Builder class BelongsTo < SingularAssociation #:nodoc: - def self.macro + def macro :belongs_to end - def self.build_valid_options(options) + def valid_options super + [:foreign_type, :polymorphic, :touch, :counter_cache] end @@ -23,6 +23,8 @@ module ActiveRecord::Associations::Builder add_counter_cache_methods mixin end + private + def self.add_counter_cache_methods(mixin) return if mixin.method_defined? :belongs_to_counter_cache_after_create diff --git a/activerecord/lib/active_record/associations/builder/collection_association.rb b/activerecord/lib/active_record/associations/builder/collection_association.rb index b696914883..bc15a49996 100644 --- a/activerecord/lib/active_record/associations/builder/collection_association.rb +++ b/activerecord/lib/active_record/associations/builder/collection_association.rb @@ -7,11 +7,22 @@ module ActiveRecord::Associations::Builder CALLBACKS = [:before_add, :after_add, :before_remove, :after_remove] - def self.build_valid_options(options) + def valid_options super + [:table_name, :before_add, :after_add, :before_remove, :after_remove, :extend] end + attr_reader :block_extension + + def initialize(model, name, scope, options) + super + @mod = nil + if block_given? + @mod = Module.new(&Proc.new) + @scope = wrap_scope @scope, @mod + end + end + def self.define_callbacks(model, reflection) super name = reflection.name @@ -21,11 +32,10 @@ module ActiveRecord::Associations::Builder } end - def self.define_extensions(model, name) - if block_given? + def define_extensions(model) + if @mod extension_module_name = "#{model.name.demodulize}#{name.to_s.camelize}AssociationExtension" - extension = Module.new(&Proc.new) - model.parent.const_set(extension_module_name, extension) + model.parent.const_set(extension_module_name, @mod) end end @@ -68,7 +78,9 @@ module ActiveRecord::Associations::Builder CODE end - def self.wrap_scope(scope, mod) + private + + def wrap_scope(scope, mod) if scope proc { |owner| instance_exec(owner, &scope).extending(mod) } else diff --git a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb index 1c9c04b044..af596a3a64 100644 --- a/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_and_belongs_to_many.rb @@ -84,11 +84,11 @@ module ActiveRecord::Associations::Builder middle_name = [lhs_model.name.downcase.pluralize, association_name].join('_').gsub(/::/, '_').to_sym middle_options = middle_options join_model - - HasMany.create_reflection(lhs_model, - middle_name, - nil, - middle_options) + hm_builder = HasMany.create_builder(lhs_model, + middle_name, + nil, + middle_options) + hm_builder.build lhs_model end private diff --git a/activerecord/lib/active_record/associations/builder/has_many.rb b/activerecord/lib/active_record/associations/builder/has_many.rb index 4ec808bc77..7909b93622 100644 --- a/activerecord/lib/active_record/associations/builder/has_many.rb +++ b/activerecord/lib/active_record/associations/builder/has_many.rb @@ -1,10 +1,10 @@ module ActiveRecord::Associations::Builder class HasMany < CollectionAssociation #:nodoc: - def self.macro + def macro :has_many end - def self.build_valid_options(options) + def valid_options super + [:primary_key, :dependent, :as, :through, :source, :source_type, :inverse_of, :counter_cache] end diff --git a/activerecord/lib/active_record/associations/builder/has_one.rb b/activerecord/lib/active_record/associations/builder/has_one.rb index 216d6abec1..f359efd496 100644 --- a/activerecord/lib/active_record/associations/builder/has_one.rb +++ b/activerecord/lib/active_record/associations/builder/has_one.rb @@ -1,10 +1,10 @@ module ActiveRecord::Associations::Builder class HasOne < SingularAssociation #:nodoc: - def self.macro + def macro :has_one end - def self.build_valid_options(options) + def valid_options valid = super + [:order, :as] valid += [:through, :source, :source_type] if options[:through] valid @@ -14,6 +14,8 @@ module ActiveRecord::Associations::Builder [:destroy, :delete, :nullify, :restrict_with_error, :restrict_with_exception] end + private + def self.add_before_destroy_callbacks(model, reflection) super unless reflection.options[:through] end diff --git a/activerecord/lib/active_record/associations/builder/singular_association.rb b/activerecord/lib/active_record/associations/builder/singular_association.rb index 6a1a0073e6..e655c389a6 100644 --- a/activerecord/lib/active_record/associations/builder/singular_association.rb +++ b/activerecord/lib/active_record/associations/builder/singular_association.rb @@ -2,7 +2,7 @@ module ActiveRecord::Associations::Builder class SingularAssociation < Association #:nodoc: - def self.build_valid_options(options) + def valid_options super + [:remote, :dependent, :primary_key, :inverse_of] end diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index e650ebcf64..5caab09038 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -84,13 +84,18 @@ module ActiveRecord "#{finder}(#{attributes_hash})" end + # The parameters in the signature may have reserved Ruby words, in order + # to prevent errors, we start each param name with `_`. + # # Extended in activerecord-deprecated_finders def signature - attribute_names.join(', ') + attribute_names.map { |name| "_#{name}" }.join(', ') end + # Given that the parameters starts with `_`, the finder needs to use the + # same parameter name. def attributes_hash - "{" + attribute_names.map { |name| ":#{name} => #{name}" }.join(',') + "}" + "{" + attribute_names.map { |name| ":#{name} => _#{name}" }.join(',') + "}" end def finder diff --git a/activerecord/test/cases/associations/extension_test.rb b/activerecord/test/cases/associations/extension_test.rb index f8f2832ab1..4c1fdfdd9a 100644 --- a/activerecord/test/cases/associations/extension_test.rb +++ b/activerecord/test/cases/associations/extension_test.rb @@ -75,6 +75,7 @@ class AssociationsExtensionsTest < ActiveRecord::TestCase private def extend!(model) - ActiveRecord::Associations::Builder::HasMany.define_extensions(model, :association_name) { } + builder = ActiveRecord::Associations::Builder::HasMany.new(model, :association_name, nil, {}) { } + builder.define_extensions(model) end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 5e8f1c851f..7a00f7fa8f 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -12,6 +12,7 @@ require 'models/developer' require 'models/customer' require 'models/toy' require 'models/matey' +require 'models/dog' class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers, :categories, :categorizations @@ -641,6 +642,13 @@ class FinderTest < ActiveRecord::TestCase assert_raise(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") } end + def test_find_by_on_attribute_that_is_a_reserved_word + dog_alias = 'Dog' + dog = Dog.create(alias: dog_alias) + + assert_equal dog, Dog.find_by_alias(dog_alias) + end + def test_find_by_one_attribute_that_is_an_alias assert_equal topics(:first), Topic.find_by_heading("The First Topic") assert_nil Topic.find_by_heading("The First Topic!") diff --git a/activerecord/test/schema/schema.rb b/activerecord/test/schema/schema.rb index 92e2e4d409..ac546fc296 100644 --- a/activerecord/test/schema/schema.rb +++ b/activerecord/test/schema/schema.rb @@ -245,6 +245,7 @@ ActiveRecord::Schema.define do t.integer :trainer_id t.integer :breeder_id t.integer :dog_lover_id + t.string :alias end create_table :edges, force: true, id: false do |t| diff --git a/activesupport/lib/active_support/number_helper.rb b/activesupport/lib/active_support/number_helper.rb index c6658dba96..b169e3af01 100644 --- a/activesupport/lib/active_support/number_helper.rb +++ b/activesupport/lib/active_support/number_helper.rb @@ -1,4 +1,3 @@ - module ActiveSupport module NumberHelper extend ActiveSupport::Autoload @@ -343,6 +342,5 @@ module ActiveSupport def number_to_human(number, options = {}) NumberToHumanConverter.convert(number, options) end - end end diff --git a/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb b/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb index d1335f6910..78d2c9ae6e 100644 --- a/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_human_size_converter.rb @@ -1,6 +1,6 @@ module ActiveSupport module NumberHelper - class NumberToHumanSizeConverter < NumberConverter + class NumberToHumanSizeConverter < NumberConverter #:nodoc: STORAGE_UNITS = [:byte, :kb, :mb, :gb, :tb] self.namespace = :human diff --git a/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb b/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb index 4c33c30772..af2ee56d91 100644 --- a/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb +++ b/activesupport/lib/active_support/number_helper/number_to_phone_converter.rb @@ -1,6 +1,6 @@ module ActiveSupport module NumberHelper - class NumberToPhoneConverter < NumberConverter + class NumberToPhoneConverter < NumberConverter #:nodoc: def convert str = country_code(opts[:country_code]) str << convert_to_phone_number(number.to_s.strip) diff --git a/activesupport/test/abstract_unit.rb b/activesupport/test/abstract_unit.rb index 65de48a7f6..4600855998 100644 --- a/activesupport/test/abstract_unit.rb +++ b/activesupport/test/abstract_unit.rb @@ -15,7 +15,6 @@ silence_warnings do end require 'active_support/testing/autorun' -require 'empty_bool' ENV['NO_RELOAD'] = '1' require 'active_support' diff --git a/activesupport/test/core_ext/blank_test.rb b/activesupport/test/core_ext/blank_test.rb index a68c074777..3e3176f993 100644 --- a/activesupport/test/core_ext/blank_test.rb +++ b/activesupport/test/core_ext/blank_test.rb @@ -4,6 +4,14 @@ require 'abstract_unit' require 'active_support/core_ext/object/blank' class BlankTest < ActiveSupport::TestCase + class EmptyTrue + def empty?() true; end + end + + class EmptyFalse + def empty?() false; end + end + BLANK = [ EmptyTrue.new, nil, false, '', ' ', " \n\t \r ", ' ', [], {} ] NOT = [ EmptyFalse.new, Object.new, true, 0, 1, 'a', [nil], { nil => 0 } ] diff --git a/activesupport/test/empty_bool.rb b/activesupport/test/empty_bool.rb deleted file mode 100644 index 005b3523ef..0000000000 --- a/activesupport/test/empty_bool.rb +++ /dev/null @@ -1,7 +0,0 @@ -class EmptyTrue - def empty?() true; end -end - -class EmptyFalse - def empty?() false; end -end |