From 6508530710d4d53fd0498a4ce0e1727a8594c381 Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Mon, 25 Aug 2008 19:32:19 -0700 Subject: [PATCH] refactor dynamic finder name matching into its own class --- activerecord/lib/active_record.rb | 1 + activerecord/lib/active_record/base.rb | 121 ++++++++------------ .../lib/active_record/dynamic_finder_match.rb | 33 ++++++ activerecord/test/cases/finder_test.rb | 42 +++++++ 4 files changed, 125 insertions(+), 72 deletions(-) create mode 100644 activerecord/lib/active_record/dynamic_finder_match.rb diff --git a/activerecord/lib/active_record.rb b/activerecord/lib/active_record.rb index e9767c2..c47ca48 100644 --- a/activerecord/lib/active_record.rb +++ b/activerecord/lib/active_record.rb @@ -51,6 +51,7 @@ require 'active_record/calculations' require 'active_record/serialization' require 'active_record/attribute_methods' require 'active_record/dirty' +require 'active_record/dynamic_finder_match' ActiveRecord::Base.class_eval do extend ActiveRecord::QueryCache diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 2e139c5..523b619 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1354,8 +1354,8 @@ module ActiveRecord #:nodoc: end def respond_to?(method_id, include_private = false) - if match = matches_dynamic_finder?(method_id) || matches_dynamic_finder_with_initialize_or_create?(method_id) - return true if all_attributes_exists?(extract_attribute_names_from_match(match)) + if match = DynamicFinderMatch.match(method_id) + return true if all_attributes_exists?(match.attribute_names) end super end @@ -1674,88 +1674,65 @@ module ActiveRecord #:nodoc: # Each dynamic finder or initializer/creator is also defined in the class after it is first invoked, so that future # attempts to use it do not run through method_missing. def method_missing(method_id, *arguments) - if match = matches_dynamic_finder?(method_id) - finder = determine_finder(match) - - attribute_names = extract_attribute_names_from_match(match) + if match = DynamicFinderMatch.match(method_id) + attribute_names = match.attribute_names super unless all_attributes_exists?(attribute_names) - - self.class_eval %{ - def self.#{method_id}(*args) - options = args.extract_options! - attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) - finder_options = { :conditions => attributes } - validate_find_options(options) - set_readonly_option!(options) - - if options[:conditions] - with_scope(:find => finder_options) do - ActiveSupport::Deprecation.silence { send(:#{finder}, options) } + if match.finder? + finder = match.finder + self.class_eval %{ + def self.#{method_id}(*args) + options = args.extract_options! + attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) + finder_options = { :conditions => attributes } + validate_find_options(options) + set_readonly_option!(options) + + if options[:conditions] + with_scope(:find => finder_options) do + ActiveSupport::Deprecation.silence { send(:#{finder}, options) } + end + else + ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) } end - else - ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) } - end - end - }, __FILE__, __LINE__ - send(method_id, *arguments) - elsif match = matches_dynamic_finder_with_initialize_or_create?(method_id) - instantiator = determine_instantiator(match) - attribute_names = extract_attribute_names_from_match(match) - super unless all_attributes_exists?(attribute_names) - - self.class_eval %{ - def self.#{method_id}(*args) - guard_protected_attributes = false - - if args[0].is_a?(Hash) - guard_protected_attributes = true - attributes = args[0].with_indifferent_access - find_attributes = attributes.slice(*[:#{attribute_names.join(',:')}]) - else - find_attributes = attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) end + }, __FILE__, __LINE__ + send(method_id, *arguments) + elsif match.instantiator? + instantiator = match.instantiator + self.class_eval %{ + def self.#{method_id}(*args) + guard_protected_attributes = false + + if args[0].is_a?(Hash) + guard_protected_attributes = true + attributes = args[0].with_indifferent_access + find_attributes = attributes.slice(*[:#{attribute_names.join(',:')}]) + else + find_attributes = attributes = construct_attributes_from_arguments([:#{attribute_names.join(',:')}], args) + end - options = { :conditions => find_attributes } - set_readonly_option!(options) + options = { :conditions => find_attributes } + set_readonly_option!(options) - record = find_initial(options) + record = find_initial(options) - if record.nil? - record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) } - #{'yield(record) if block_given?'} - #{'record.save' if instantiator == :create} - record - else - record + if record.nil? + record = self.new { |r| r.send(:attributes=, attributes, guard_protected_attributes) } + #{'yield(record) if block_given?'} + #{'record.save' if instantiator == :create} + record + else + record + end end - end - }, __FILE__, __LINE__ - send(method_id, *arguments) + }, __FILE__, __LINE__ + send(method_id, *arguments) + end else super end end - def matches_dynamic_finder?(method_id) - /^find_(all_by|by)_([_a-zA-Z]\w*)$/.match(method_id.to_s) - end - - def matches_dynamic_finder_with_initialize_or_create?(method_id) - /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/.match(method_id.to_s) - end - - def determine_finder(match) - match.captures.first == 'all_by' ? :find_every : :find_initial - end - - def determine_instantiator(match) - match.captures.first == 'initialize' ? :new : :create - end - - def extract_attribute_names_from_match(match) - match.captures.last.split('_and_') - end - def construct_attributes_from_arguments(attribute_names, arguments) attributes = {} attribute_names.each_with_index { |name, idx| attributes[name] = arguments[idx] } diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb new file mode 100644 index 0000000..4618e77 --- /dev/null +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -0,0 +1,33 @@ +module ActiveRecord + class DynamicFinderMatch + def self.match(method) + df_match = self.new(method) + df_match.finder ? df_match : nil + end + + def initialize(method) + @finder = :find_initial + case method.to_s + when /^find_(all_by|by)_([_a-zA-Z]\w*)$/ + @finder = :find_every if $1 == 'all_by' + names = $2 + when /^find_or_(initialize|create)_by_([_a-zA-Z]\w*)$/ + @instantiator = $1 == 'initialize' ? :new : :create + names = $2 + else + @finder = nil + end + @attribute_names = names && names.split('_and_') + end + + attr_reader :finder, :attribute_names, :instantiator + + def finder? + !@finder.nil? && @instantiator.nil? + end + + def instantiator? + @finder == :find_initial && !@instantiator.nil? + end + end +end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index b97db73..875d56f 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -12,6 +12,48 @@ require 'models/customer' require 'models/job' require 'models/categorization' +class DynamicFinderMatchTest < ActiveRecord::TestCase + def test_find_no_match + assert_nil ActiveRecord::DynamicFinderMatch.match("not_a_finder") + end + + def test_find_by + match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location") + assert_not_nil match + assert match.finder? + assert_equal :find_initial, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_all_by + match = ActiveRecord::DynamicFinderMatch.match("find_all_by_age_and_sex_and_location") + assert_not_nil match + assert match.finder? + assert_equal :find_every, match.finder + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_or_initialize_by + match = ActiveRecord::DynamicFinderMatch.match("find_or_initialize_by_age_and_sex_and_location") + assert_not_nil match + assert !match.finder? + assert match.instantiator? + assert_equal :find_initial, match.finder + assert_equal :new, match.instantiator + assert_equal %w(age sex location), match.attribute_names + end + + def test_find_or_create_by + match = ActiveRecord::DynamicFinderMatch.match("find_or_create_by_age_and_sex_and_location") + assert_not_nil match + assert !match.finder? + assert match.instantiator? + assert_equal :find_initial, match.finder + assert_equal :create, match.instantiator + assert_equal %w(age sex location), match.attribute_names + end +end + class FinderTest < ActiveRecord::TestCase fixtures :companies, :topics, :entrants, :developers, :developers_projects, :posts, :comments, :accounts, :authors, :customers -- 1.6.0 From 87887866ba9710e03137deb1176e704d615362f6 Mon Sep 17 00:00:00 2001 From: Josh Susser Date: Mon, 25 Aug 2008 21:28:53 -0700 Subject: [PATCH] add dynamic finder bang version to raise RecordNotFound --- activerecord/lib/active_record/base.rb | 4 +++- .../lib/active_record/dynamic_finder_match.rb | 7 +++++++ activerecord/test/cases/finder_test.rb | 14 ++++++++++++++ 3 files changed, 24 insertions(+), 1 deletions(-) diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb index 523b619..5c30f80 100644 --- a/activerecord/lib/active_record/base.rb +++ b/activerecord/lib/active_record/base.rb @@ -1679,6 +1679,7 @@ module ActiveRecord #:nodoc: super unless all_attributes_exists?(attribute_names) if match.finder? finder = match.finder + bang = match.bang? self.class_eval %{ def self.#{method_id}(*args) options = args.extract_options! @@ -1687,13 +1688,14 @@ module ActiveRecord #:nodoc: validate_find_options(options) set_readonly_option!(options) - if options[:conditions] + #{'result = ' if bang}if options[:conditions] with_scope(:find => finder_options) do ActiveSupport::Deprecation.silence { send(:#{finder}, options) } end else ActiveSupport::Deprecation.silence { send(:#{finder}, options.merge(finder_options)) } end + #{'result || raise(RecordNotFound)' if bang} end }, __FILE__, __LINE__ send(method_id, *arguments) diff --git a/activerecord/lib/active_record/dynamic_finder_match.rb b/activerecord/lib/active_record/dynamic_finder_match.rb index 4618e77..b105b91 100644 --- a/activerecord/lib/active_record/dynamic_finder_match.rb +++ b/activerecord/lib/active_record/dynamic_finder_match.rb @@ -11,6 +11,9 @@ module ActiveRecord when /^find_(all_by|by)_([_a-zA-Z]\w*)$/ @finder = :find_every if $1 == 'all_by' 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 @@ -29,5 +32,9 @@ module ActiveRecord def instantiator? @finder == :find_initial && !@instantiator.nil? end + + def bang? + @bang + end end end diff --git a/activerecord/test/cases/finder_test.rb b/activerecord/test/cases/finder_test.rb index 875d56f..2ce49ed 100644 --- a/activerecord/test/cases/finder_test.rb +++ b/activerecord/test/cases/finder_test.rb @@ -25,6 +25,15 @@ class DynamicFinderMatchTest < ActiveRecord::TestCase assert_equal %w(age sex location), match.attribute_names end + def find_by_bang + match = ActiveRecord::DynamicFinderMatch.match("find_by_age_and_sex_and_location!") + assert_not_nil match + assert match.finder? + assert match.bang? + assert_equal :find_initial, match.finder + assert_equal %w(age sex location), match.attribute_names + end + def test_find_all_by match = ActiveRecord::DynamicFinderMatch.match("find_all_by_age_and_sex_and_location") assert_not_nil match @@ -482,6 +491,11 @@ class FinderTest < ActiveRecord::TestCase assert_nil Topic.find_by_title("The First Topic!") end + def test_find_by_one_attribute_bang + assert_equal topics(:first), Topic.find_by_title!("The First Topic") + assert_raises(ActiveRecord::RecordNotFound) { Topic.find_by_title!("The First Topic!") } + end + def test_find_by_one_attribute_caches_dynamic_finder # ensure this test can run independently of order class << Topic; self; end.send(:remove_method, :find_by_title) if Topic.public_methods.any? { |m| m.to_s == 'find_by_title' } -- 1.6.0