Added extension capabilities to has_many and has_and_belongs_to_many proxies [DHH] Added find_or_create_by_X as a second type of dynamic finder that'll create the record if it doesn't already exist [DHH] Added constrain scoping for creates using a hash of attributes bound to the :creation key [DHH]

git-svn-id: http://svn-commit.rubyonrails.org/rails/trunk@2872 5ecf4fe2-1ee6-0310-87b1-e25e094e27de

11 files changed, 151 insertions, 47 deletions

diff --git a/activerecord/CHANGELOG b/activerecord/CHANGELOG
index cc718dafdc..0c15716116 100644
--- a/activerecord/CHANGELOG
+++ b/activerecord/CHANGELOG
@@ -1,14 +1,24 @@
 *SVN*
 
-* Omit internal dtproperties table from SQLServer table list.  #2729 [rtomayko@gmail.com]
-
-* Quote column names in generated SQL.  #2728 [rtomayko@gmail.com]
+* Added constrain scoping for creates using a hash of attributes bound to the :creation key [DHH]. Example:
 
-* Correct the pure-Ruby MySQL 4.1.1 shim's version test.  #2718 [Jeremy Kemper]
+    Comment.constrain(:creation => { :post_id => 5 }) do
+      # Associated with :post_id
+      Comment.create :body => "Hello world"
+    end
+  
+  This is rarely used directly, but allows for find_or_create on associations. So you can do:
+  
+    # If the tag doesn't exist, a new one is created that's associated with the person
+    person.tags.find_or_create_by_name("Summer")
 
-* Add Model.create! to match existing model.save! method.  When save! raises RecordInvalid, you can catch the exception, retrieve the invalid record (invalid_exception.record), and see its errors (invalid_exception.record.errors).  [Jeremy Kemper]
+* Added find_or_create_by_X as a second type of dynamic finder that'll create the record if it doesn't already exist [DHH]. Example:
 
-* Correct fixture behavior when table name pluralization is off.  #2719 [Rick Bradley <rick@rickbradley.com>]
+    # No 'Summer' tag exists
+    Tag.find_or_create_by_name("Summer") # equal to Tag.create(:name => "Summer")
+    
+    # Now the 'Summer' tag does exist
+    Tag.find_or_create_by_name("Summer") # equal to Tag.find_by_name("Summer")
 
 * Added extension capabilities to has_many and has_and_belongs_to_many proxies [DHH]. Example:
 
@@ -17,19 +27,27 @@
         def find_or_create_by_name(name)
           first_name, *last_name = name.split
           last_name = last_name.join " "
-    
-          find_by_first_name_and_last_name(first_name, last_name) ||
-            create({ :first_name => first_name, :last_name => last_name })
+
+          find_or_create_by_first_name_and_last_name(first_name, last_name)
         end
       }
     end
- 
+
     person = Account.find(:first).people.find_or_create_by_name("David Heinemeier Hansson")
     person.first_name # => "David"
     person.last_name  # => "Heinemeier Hansson"
- 
+
   Note that the anoymous module must be declared using brackets, not do/end (due to order of evaluation).
- 
+
+* Omit internal dtproperties table from SQLServer table list.  #2729 [rtomayko@gmail.com]
+
+* Quote column names in generated SQL.  #2728 [rtomayko@gmail.com]
+
+* Correct the pure-Ruby MySQL 4.1.1 shim's version test.  #2718 [Jeremy Kemper]
+
+* Add Model.create! to match existing model.save! method.  When save! raises RecordInvalid, you can catch the exception, retrieve the invalid record (invalid_exception.record), and see its errors (invalid_exception.record.errors).  [Jeremy Kemper]
+
+* Correct fixture behavior when table name pluralization is off.  #2719 [Rick Bradley <rick@rickbradley.com>]
 
 * Changed :dbfile to :database for SQLite adapter for consistency (old key still works as an alias) #2644 [Dan Peterson]
 
diff --git a/activerecord/lib/active_record/associations/association_collection.rb b/activerecord/lib/active_record/associations/association_collection.rb
index cbfbdf1541..7ee567e0b4 100644
--- a/activerecord/lib/active_record/associations/association_collection.rb
+++ b/activerecord/lib/active_record/associations/association_collection.rb
@@ -124,14 +124,6 @@ module ActiveRecord
       end
 
       private
-        def method_missing(method, *args, &block)
-          if @target.respond_to?(method) or (not @association_class.respond_to?(method) and Class.respond_to?(method))
-            super
-          else
-            @association_class.constrain(:conditions => @finder_sql, :joins => @join_sql, :readonly => false) { @association_class.send(method, *args, &block) }
-          end
-        end
-              
         def raise_on_type_mismatch(record)
           raise ActiveRecord::AssociationTypeMismatch, "#{@association_class} expected, got #{record.class}" unless record.is_a?(@association_class)
         end
diff --git a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
index 1b7adc3f39..2972aa0248 100644
--- a/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_and_belongs_to_many_association.rb
@@ -76,6 +76,16 @@ module ActiveRecord
       end
       
       protected
+        def method_missing(method, *args, &block)
+          if @target.respond_to?(method) || (!@association_class.respond_to?(method) && Class.respond_to?(method))
+            super
+          else
+            @association_class.constrain(:conditions => @finder_sql, :joins => @join_sql, :readonly => false) do
+              @association_class.send(method, *args, &block)
+            end
+          end
+        end
+            
         def find_target
           if @options[:finder_sql]
             records = @association_class.find_by_sql(@finder_sql)
diff --git a/activerecord/lib/active_record/associations/has_many_association.rb b/activerecord/lib/active_record/associations/has_many_association.rb
index f5f27dc410..b04cb81a5c 100644
--- a/activerecord/lib/active_record/associations/has_many_association.rb
+++ b/activerecord/lib/active_record/associations/has_many_association.rb
@@ -85,6 +85,20 @@ module ActiveRecord
       end
       
       protected
+        def method_missing(method, *args, &block)
+          if @target.respond_to?(method) || (!@association_class.respond_to?(method) && Class.respond_to?(method))
+            super
+          else
+            @association_class.constrain(
+                :conditions => @finder_sql, 
+                :joins      => @join_sql, 
+                :readonly   => false,
+                :creation   => { @association_class_primary_key_name => @owner.id }) do
+              @association_class.send(method, *args, &block)
+            end
+          end
+        end
+            
         def find_target
           find_all
         end
diff --git a/activerecord/lib/active_record/base.rb b/activerecord/lib/active_record/base.rb
index 4aaa05a6c9..3fc55e0783 100755
--- a/activerecord/lib/active_record/base.rb
+++ b/activerecord/lib/active_record/base.rb
@@ -140,7 +140,7 @@ module ActiveRecord #:nodoc:
   #
   # == Dynamic attribute-based finders
   #
-  # Dynamic attribute-based finders are a cleaner way of getting objects by simple queries without turning to SQL. They work by
+  # Dynamic attribute-based finders are a cleaner way of getting (and/or creating) objects by simple queries without turning to SQL. They work by
   # appending the name of an attribute to <tt>find_by_</tt> or <tt>find_all_by_</tt>, so you get finders like Person.find_by_user_name,
   # Person.find_all_by_last_name, Payment.find_by_transaction_id. So instead of writing
   # <tt>Person.find(:first, ["user_name = ?", user_name])</tt>, you just do <tt>Person.find_by_user_name(user_name)</tt>.
@@ -155,6 +155,15 @@ module ActiveRecord #:nodoc:
   # is actually Payment.find_all_by_amount(amount, options). And the full interface to Person.find_by_user_name is
   # actually Person.find_by_user_name(user_name, options). So you could call <tt>Payment.find_all_by_amount(50, :order => "created_on")</tt>.
   #
+  # The same dynamic finder style can be used to create the object if it doesn't already exist. This dynamic finder is called with
+  # <tt>find_or_create_by_</tt> and will return the object if it already exists and otherwise creates it, then returns it. Example:
+  #
+  #   # No 'Summer' tag exists
+  #   Tag.find_or_create_by_name("Summer") # equal to Tag.create(:name => "Summer")
+  #   
+  #   # Now the 'Summer' tag does exist
+  #   Tag.find_or_create_by_name("Summer") # equal to Tag.find_by_name("Summer")
+  #
   # == Saving arrays, hashes, and other non-mappable objects in text columns
   #
   # Active Record can serialize any object in text columns using YAML. To do so, you must specify this with a call to the class method +serialize+.
@@ -451,6 +460,8 @@ module ActiveRecord #:nodoc:
         if attributes.is_a?(Array)
           attributes.collect { |attr| create(attr) }
         else
+          attributes.reverse_merge!(scope_constraints[:creation]) if scope_constraints[:creation]
+
           object = new(attributes)
           object.save
           object
@@ -838,10 +849,10 @@ module ActiveRecord #:nodoc:
       #   end
       def constrain(options = {})
         options = options.dup
-        if !options[:joins].blank? and !options.has_key?(:readonly)
-          options[:readonly] = true
-        end
+        options[:readonly] = true if !options[:joins].blank? && !options.has_key?(:readonly)
+
         self.scope_constraints = options
+
         yield if block_given?
       ensure 
         self.scope_constraints = nil
@@ -948,27 +959,54 @@ module ActiveRecord #:nodoc:
         # It's even possible to use all the additional parameters to find. For example, the full interface for find_all_by_amount
         # is actually find_all_by_amount(amount, options).
         def method_missing(method_id, *arguments)
-          method_name = method_id.id2name
+          if match = /find_(all_by|by)_([_a-zA-Z]\w*)/.match(method_id.to_s)
+            finder = determine_finder(match)
 
-          if md = /find_(all_by|by)_([_a-zA-Z]\w*)/.match(method_id.to_s)
-            finder = md.captures.first == 'all_by' ? :all : :first
-            attributes = md.captures.last.split('_and_')
-            attributes.each { |attr_name| super unless column_methods_hash.include?(attr_name.to_sym) }
+            attribute_names = extract_attribute_names_from_match(match)
+            super unless all_attributes_exists?(attribute_names)
 
-            attr_index = -1
-            conditions = attributes.collect { |attr_name| attr_index += 1; "#{table_name}.#{attr_name} #{attribute_condition(arguments[attr_index])} " }.join(" AND ")
+            conditions = construct_conditions_from_arguments(attribute_names, arguments)
 
-            if arguments[attributes.length].is_a?(Hash)
-              find(finder, { :conditions => [conditions, *arguments[0...attributes.length]] }.update(arguments[attributes.length]))
+            if arguments[attribute_names.length].is_a?(Hash)
+              find(finder, { :conditions => conditions }.update(arguments[attribute_names.length]))
             else
-              # deprecated API
-              send("find_#{finder}", [conditions, *arguments[0...attributes.length]], *arguments[attributes.length..-1])
+              send("find_#{finder}", conditions, *arguments[attribute_names.length..-1]) # deprecated API
             end
+          elsif match = /find_or_create_by_([_a-zA-Z]\w*)/.match(method_id.to_s)
+            attribute_names = extract_attribute_names_from_match(match)
+            super unless all_attributes_exists?(attribute_names)
+
+            find(:first, :conditions => construct_conditions_from_arguments(attribute_names, arguments)) || 
+              create(construct_attributes_from_arguments(attribute_names, arguments))
           else
             super
           end
         end
 
+        def determine_finder(match)
+          match.captures.first == 'all_by' ? :all : :first
+        end
+
+        def extract_attribute_names_from_match(match)
+          match.captures.last.split('_and_')
+        end
+
+        def construct_conditions_from_arguments(attribute_names, arguments)
+          conditions = []
+          attribute_names.each_with_index { |name, idx| conditions << "#{table_name}.#{name} #{attribute_condition(arguments[idx])} " }
+          [ conditions.join(" AND "), *arguments[0...attribute_names.length] ]
+        end
+        
+        def construct_attributes_from_arguments(attribute_names, arguments)
+          attributes = {}
+          attribute_names.each_with_index { |name, idx| attributes[name] = arguments[idx] }
+          attributes
+        end
+
+        def all_attributes_exists?(attribute_names)
+          attribute_names.all? { |name| column_methods_hash.include?(name.to_sym) }
+        end
+
         def attribute_condition(argument)
           case argument
             when nil   then "IS ?"
@@ -1691,4 +1729,4 @@ module ActiveRecord #:nodoc:
         value
       end
   end
-end
+end
\ No newline at end of file
diff --git a/activerecord/test/associations_extensions_test.rb b/activerecord/test/associations_extensions_test.rb
index 92bdb86eb5..fa9674a149 100644
--- a/activerecord/test/associations_extensions_test.rb
+++ b/activerecord/test/associations_extensions_test.rb
@@ -1,9 +1,11 @@
 require 'abstract_unit'
+require 'fixtures/post'
+require 'fixtures/comment'
 require 'fixtures/project'
 require 'fixtures/developer'
 
 class AssociationsExtensionsTest < Test::Unit::TestCase
-  fixtures :projects, :developers
+  fixtures :projects, :developers, :comments, :posts
 
   def test_extension_on_habtm
     assert_equal projects(:action_controller), developers(:david).projects.find_most_recent
diff --git a/activerecord/test/associations_test.rb b/activerecord/test/associations_test.rb
index 6a4beb4135..fc2afb721f 100755
--- a/activerecord/test/associations_test.rb
+++ b/activerecord/test/associations_test.rb
@@ -490,6 +490,14 @@ class HasManyAssociationsTest < Test::Unit::TestCase
     assert_equal 3, companies(:first_firm).clients_of_firm(true).size
   end
 
+  def test_find_or_create
+    number_of_clients = companies(:first_firm).clients.size
+    the_client = companies(:first_firm).clients.find_or_create_by_name("Yet another client")
+    assert_equal number_of_clients + 1, companies(:first_firm, :refresh).clients.size
+    assert_equal the_client, companies(:first_firm).clients.find_or_create_by_name("Yet another client")
+    assert_equal number_of_clients + 1, companies(:first_firm, :refresh).clients.size
+  end
+
   def test_deleting
     force_signal37_to_load_all_clients_of_firm
     companies(:first_firm).clients_of_firm.delete(companies(:first_firm).clients_of_firm.first)
diff --git a/activerecord/test/conditions_scoping_test.rb b/activerecord/test/conditions_scoping_test.rb
index a997ee9e85..cf8b670c26 100644
--- a/activerecord/test/conditions_scoping_test.rb
+++ b/activerecord/test/conditions_scoping_test.rb
@@ -5,7 +5,7 @@ require 'fixtures/post'
 require 'fixtures/category'
 
 class ConditionsScopingTest < Test::Unit::TestCase
-  fixtures :developers
+  fixtures :developers, :comments, :posts
   
   def test_set_conditions
     Developer.constrain(:conditions => 'just a test...') do
@@ -42,6 +42,17 @@ class ConditionsScopingTest < Test::Unit::TestCase
     end        
   end
 
+  def test_scoped_create
+    new_comment = nil
+
+    VerySpecialComment.constrain(:creation => { :post_id => 1 }) do
+      assert_equal({ :post_id => 1 }, Thread.current[:constraints][VerySpecialComment][:creation])
+      new_comment = VerySpecialComment.create :body => "Wonderful world"
+    end
+    
+    assert Post.find(1).comments.include?(new_comment)
+  end
+
   def test_immutable_constraint
     options = { :conditions => "name = 'David'" }
     Developer.constrain(options) do
diff --git a/activerecord/test/finder_test.rb b/activerecord/test/finder_test.rb
index 7c30308a48..dc3464727e 100644
--- a/activerecord/test/finder_test.rb
+++ b/activerecord/test/finder_test.rb
@@ -278,6 +278,20 @@ class FinderTest < Test::Unit::TestCase
     assert_equal "Mary", topics[0].author_name
   end
 
+  def test_find_or_create_from_one_attribute
+    number_of_companies = Company.count
+    sig38 = Company.find_or_create_by_name("38signals")
+    assert_equal number_of_companies + 1, Company.count
+    assert_equal sig38, Company.find_or_create_by_name("38signals")
+  end
+
+  def test_find_or_create_from_two_attributes
+    number_of_companies = Company.count
+    sig38 = Company.find_or_create_by_name("38signals")
+    assert_equal number_of_companies + 1, Company.count
+    assert_equal sig38, Company.find_or_create_by_name("38signals")
+  end
+
   def test_find_with_bad_sql
     assert_raises(ActiveRecord::StatementInvalid) { Topic.find_by_sql "select 1 from badtable" }
   end
diff --git a/activerecord/test/fixtures/comment.rb b/activerecord/test/fixtures/comment.rb
index 0605fd7046..fd3a43ff5a 100644
--- a/activerecord/test/fixtures/comment.rb
+++ b/activerecord/test/fixtures/comment.rb
@@ -10,18 +10,14 @@ class Comment < ActiveRecord::Base
   end
 end
 
-class SpecialComment < Comment; 
-
+class SpecialComment < Comment
   def self.what_are_you
     'a special comment...'
   end
-  
-end;
+end
 
-class VerySpecialComment < Comment; 
-  
+class VerySpecialComment < Comment
   def self.what_are_you
     'a very special comment...'
   end
-  
-end;
+end
\ No newline at end of file
diff --git a/activerecord/test/fixtures/post.rb b/activerecord/test/fixtures/post.rb
index 6163ec90f8..1a34823ca0 100644
--- a/activerecord/test/fixtures/post.rb
+++ b/activerecord/test/fixtures/post.rb
@@ -11,8 +11,9 @@ class Post < ActiveRecord::Base
     end
   }
 
+  has_one  :very_special_comment
+  has_many :special_comments
 
-  has_many   :special_comments, :class_name => "SpecialComment"
   has_and_belongs_to_many :categories
   has_and_belongs_to_many :special_categories, :join_table => "categories_posts"