From 1be248e246948c43867e1d512502cbfdfcae3dd6 Mon Sep 17 00:00:00 2001 From: Paul McMahon Date: Sat, 28 Jan 2012 10:32:33 +0900 Subject: Extract different DynamicFinderMatch subclasses --- .../lib/active_record/dynamic_finder_match.rb | 65 +++++++++++++--------- 1 file changed, 39 insertions(+), 26 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index b309df9b1b..0cdc49ae14 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -6,33 +6,17 @@ module ActiveRecord # class DynamicFinderMatch def self.match(method) - finder = :first - bang = false - instantiator = nil - - case method.to_s - when /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ - finder = :last if $1 == 'last_' - finder = :all if $1 == 'all_' - names = $2 - when /^find_by_([_a-zA-Z]\w*)\!$/ - bang = true - names = $1 - when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ - instantiator = $1 == 'initialize' ? :new : :create - names = $2 - else - return nil + [ FindBy, FindByBang, FindOrInitializeCreateBy ].each do |klass| + o = klass.match(method.to_s) + return o if o end - - new(finder, instantiator, bang, names.split('_and_')) + nil end - def initialize(finder, instantiator, bang, attribute_names) + def initialize(finder, names, instantiator = nil) @finder = finder @instantiator = instantiator - @bang = bang - @attribute_names = attribute_names + @attribute_names = names.split('_and_') end attr_reader :finder, :attribute_names, :instantiator @@ -41,16 +25,45 @@ module ActiveRecord @finder && !@instantiator end + def creator? + @finder == :first && @instantiator == :create + end + def instantiator? - @finder == :first && @instantiator + @instantiator end - def creator? - @finder == :first && @instantiator == :create + def bang? + false + end + end + + class FindBy < DynamicFinderMatch + def self.match(method) + if method =~ /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ + new($1 == 'last_' ? :last : $1 == 'all_' ? :all : :first, $2) + end + end + end + + class FindByBang < DynamicFinderMatch + def self.match(method) + if method =~ /^find_by_([_a-zA-Z]\w*)\!$/ + new(:first, $1) + end end def bang? - @bang + true + end + end + + class FindOrInitializeCreateBy < DynamicFinderMatch + def self.match(method) + instantiator = nil + if method =~ /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ + new(:first, $2, $1 == 'initialize' ? :new : :create) + end end end end -- cgit v1.2.3 From fe12497e4d8ad292ffbcb4486a26b8802c19d65d Mon Sep 17 00:00:00 2001 From: Paul McMahon Date: Sat, 28 Jan 2012 10:42:44 +0900 Subject: Move argument validation into match --- activerecord/lib/active_record/dynamic_finder_match.rb | 8 ++++++++ activerecord/lib/active_record/dynamic_matchers.rb | 2 +- activerecord/lib/active_record/dynamic_scope_match.rb | 4 ++++ 3 files changed, 13 insertions(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 0cdc49ae14..2b4f1bbf92 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -36,6 +36,10 @@ module ActiveRecord def bang? false end + + def valid_arguments?(arguments) + arguments.size >= @attribute_names.size + end end class FindBy < DynamicFinderMatch @@ -65,5 +69,9 @@ module ActiveRecord new(:first, $2, $1 == 'initialize' ? :new : :create) end end + + def valid_arguments?(arguments) + arguments.size == 1 && arguments.first.is_a?(Hash) || super + end end end diff --git a/activerecord/lib/active_record/dynamic_matchers.rb b/activerecord/lib/active_record/dynamic_matchers.rb index b6b8e24436..60ce3dd4f1 100644 --- a/activerecord/lib/active_record/dynamic_matchers.rb +++ b/activerecord/lib/active_record/dynamic_matchers.rb @@ -25,7 +25,7 @@ module ActiveRecord if match = (DynamicFinderMatch.match(method_id) || DynamicScopeMatch.match(method_id)) attribute_names = match.attribute_names super unless all_attributes_exists?(attribute_names) - if !(match.is_a?(DynamicFinderMatch) && match.instantiator? && arguments.first.is_a?(Hash)) && arguments.size < attribute_names.size + unless match.valid_arguments?(arguments) method_trace = "#{__FILE__}:#{__LINE__}:in `#{method_id}'" backtrace = [method_trace] + caller raise ArgumentError, "wrong number of arguments (#{arguments.size} for #{attribute_names.size})", backtrace diff --git a/activerecord/lib/active_record/dynamic_scope_match.rb b/activerecord/lib/active_record/dynamic_scope_match.rb index c832e927d6..a502155aac 100644 --- a/activerecord/lib/active_record/dynamic_scope_match.rb +++ b/activerecord/lib/active_record/dynamic_scope_match.rb @@ -19,5 +19,9 @@ module ActiveRecord attr_reader :scope, :attribute_names alias :scope? :scope + + def valid_arguments?(arguments) + arguments.size >= @attribute_names.size + end end end -- cgit v1.2.3 From 2352b50e71d3165128317c7927eb0163d87ba992 Mon Sep 17 00:00:00 2001 From: Paul McMahon Date: Sun, 29 Jan 2012 13:27:47 +0900 Subject: Decouple finding matching class from instantiation --- .../lib/active_record/dynamic_finder_match.rb | 51 ++++++++++++---------- 1 file changed, 29 insertions(+), 22 deletions(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 2b4f1bbf92..638b68bc52 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -6,17 +6,23 @@ module ActiveRecord # class DynamicFinderMatch def self.match(method) - [ FindBy, FindByBang, FindOrInitializeCreateBy ].each do |klass| - o = klass.match(method.to_s) - return o if o + method = method.to_s + klass = [FindBy, FindByBang, FindOrInitializeCreateBy].find do |klass| + klass.matches?(method) end - nil + klass.try(:new, method) end - def initialize(finder, names, instantiator = nil) - @finder = finder - @instantiator = instantiator - @attribute_names = names.split('_and_') + def self.matches?(method) + method =~ self::METHOD_PATTERN + end + + def initialize(method) + @finder = :first + @instantiator = nil + match_data = method.match(self.class::METHOD_PATTERN) + @attribute_names = match_data[-1].split("_and_") + initialize_from_match_data(match_data) end attr_reader :finder, :attribute_names, :instantiator @@ -40,22 +46,24 @@ module ActiveRecord def valid_arguments?(arguments) arguments.size >= @attribute_names.size end + + private + + def initialize_from_match_data(match_data) + end end class FindBy < DynamicFinderMatch - def self.match(method) - if method =~ /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ - new($1 == 'last_' ? :last : $1 == 'all_' ? :all : :first, $2) - end + METHOD_PATTERN = /^find_(all_|last_)?by_([_a-zA-Z]\w*)$/ + + def initialize_from_match_data(match_data) + @finder = :last if match_data[1] == 'last_' + @finder = :all if match_data[1] == 'all_' end end class FindByBang < DynamicFinderMatch - def self.match(method) - if method =~ /^find_by_([_a-zA-Z]\w*)\!$/ - new(:first, $1) - end - end + METHOD_PATTERN = /^find_by_([_a-zA-Z]\w*)\!$/ def bang? true @@ -63,11 +71,10 @@ module ActiveRecord end class FindOrInitializeCreateBy < DynamicFinderMatch - def self.match(method) - instantiator = nil - if method =~ /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ - new(:first, $2, $1 == 'initialize' ? :new : :create) - end + METHOD_PATTERN = /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ + + def initialize_from_match_data(match_data) + @instantiator = match_data[1] == 'initialize' ? :new : :create end def valid_arguments?(arguments) -- cgit v1.2.3 From b08bf9979232f40309a826947f8ff85d8ccbaa8a Mon Sep 17 00:00:00 2001 From: Paul McMahon Date: Tue, 31 Jan 2012 09:32:42 +0900 Subject: Use conditional instead of try --- activerecord/lib/active_record/dynamic_finder_match.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'activerecord/lib') diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 638b68bc52..e266fbb2e9 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -10,7 +10,7 @@ module ActiveRecord klass = [FindBy, FindByBang, FindOrInitializeCreateBy].find do |klass| klass.matches?(method) end - klass.try(:new, method) + klass.new(method) if klass end def self.matches?(method) -- cgit v1.2.3