From 760d8558458379a59c3b061d38256f694ff4ca1b Mon Sep 17 00:00:00 2001
From: Sam Lown <me@samlown.com>
Date: Tue, 5 Apr 2011 20:41:24 +0200
Subject: [PATCH] Fixing and testing proxyable with associations and validation

---
 Gemfile.lock                                  | 27 +++----
 VERSION                                       |  2 +-
 history.txt                                   |  1 +
 lib/couchrest/model/associations.rb           | 58 +++++----------
 lib/couchrest/model/proxyable.rb              | 19 +++--
 lib/couchrest/model/validations/uniqueness.rb |  2 +-
 lib/couchrest/model/views.rb                  |  2 +-
 spec/couchrest/assocations_spec.rb            | 41 ++++++++---
 spec/couchrest/proxyable_spec.rb              | 73 ++++++++++++-------
 spec/couchrest/validations_spec.rb            | 12 ++-
 10 files changed, 135 insertions(+), 102 deletions(-)

diff --git a/Gemfile.lock b/Gemfile.lock
index 29e8b1c..d0a19df 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -12,9 +12,9 @@ GEM
   remote: http://rubygems.org/
   specs:
     abstract (1.0.0)
-    actionpack (3.0.4)
-      activemodel (= 3.0.4)
-      activesupport (= 3.0.4)
+    actionpack (3.0.5)
+      activemodel (= 3.0.5)
+      activesupport (= 3.0.5)
       builder (~> 2.1.2)
       erubis (~> 2.6.6)
       i18n (~> 0.4)
@@ -22,11 +22,11 @@ GEM
       rack-mount (~> 0.6.13)
       rack-test (~> 0.5.7)
       tzinfo (~> 0.3.23)
-    activemodel (3.0.4)
-      activesupport (= 3.0.4)
+    activemodel (3.0.5)
+      activesupport (= 3.0.5)
       builder (~> 2.1.2)
       i18n (~> 0.4)
-    activesupport (3.0.4)
+    activesupport (3.0.5)
     builder (2.1.2)
     couchrest (1.0.2)
       json (~> 1.5.1)
@@ -39,13 +39,13 @@ GEM
     json (1.5.1)
     mime-types (1.16)
     rack (1.2.1)
-    rack-mount (0.6.13)
+    rack-mount (0.6.14)
       rack (>= 1.0.0)
     rack-test (0.5.7)
       rack (>= 1.0)
-    railties (3.0.4)
-      actionpack (= 3.0.4)
-      activesupport (= 3.0.4)
+    railties (3.0.5)
+      actionpack (= 3.0.5)
+      activesupport (= 3.0.5)
       rake (>= 0.8.7)
       thor (~> 0.14.4)
     rake (0.8.7)
@@ -60,17 +60,12 @@ GEM
       diff-lcs (~> 1.1.2)
     rspec-mocks (2.3.0)
     thor (0.14.6)
-    tzinfo (0.3.24)
+    tzinfo (0.3.26)
 
 PLATFORMS
   ruby
 
 DEPENDENCIES
-  activemodel (~> 3.0.0)
-  couchrest (~> 1.0.2)
   couchrest_model!
-  mime-types (~> 1.15)
   rack-test (>= 0.5.7)
-  railties (~> 3.0.0)
   rspec (>= 2.0.0)
-  tzinfo (~> 0.3.22)
diff --git a/VERSION b/VERSION
index 743bc7c..054dfe3 100644
--- a/VERSION
+++ b/VERSION
@@ -1 +1 @@
-1.1.0.beta
+1.1.0.beta2
diff --git a/history.txt b/history.txt
index 98b9c15..7d4d1b5 100644
--- a/history.txt
+++ b/history.txt
@@ -2,6 +2,7 @@
 
 * Minor enhancements:
   * Time handling improved in accordance with CouchRest 1.0.3. Always set to UTC.
+  * Refinements to associations and uniqueness validation for proxy (based on issue found by Gleb Kanterov)
 
 == 1.1.0.beta
 
diff --git a/lib/couchrest/model/associations.rb b/lib/couchrest/model/associations.rb
index b047aec..ed2c23c 100644
--- a/lib/couchrest/model/associations.rb
+++ b/lib/couchrest/model/associations.rb
@@ -36,15 +36,12 @@ module CouchRest
         # it can be set with the :proxy_name option.
         #
         def belongs_to(attrib, *options)
+          opts = merge_belongs_to_association_options(attrib, options.first)
 
-          opts = merge_belongs_to_assocation_options(attrib, options.first)
+          property(opts[:foreign_key], opts)
 
-          prop = property(opts[:foreign_key], opts)
-
-          create_belongs_to_getter(attrib, prop, opts)
-          create_belongs_to_setter(attrib, prop, opts)
-
-          prop
+          create_belongs_to_getter(attrib, opts)
+          create_belongs_to_setter(attrib, opts)
         end
 
         # Provide access to a collection of objects where the associated
@@ -87,41 +84,27 @@ module CouchRest
         # frequently! Use with prudence.
         #
         def collection_of(attrib, *options)
-
-          opts = merge_belongs_to_assocation_options(attrib, options.first)
+          opts = merge_belongs_to_association_options(attrib, options.first)
           opts[:foreign_key] = opts[:foreign_key].pluralize
           opts[:readonly] = true
 
-          prop = property(opts[:foreign_key], [], opts)
+          property(opts[:foreign_key], [], opts)
 
-          create_collection_of_property_setter(attrib, prop, opts)
-          create_collection_of_getter(attrib, prop, opts)
-          create_collection_of_setter(attrib, prop, opts)
-
-          prop
+          create_collection_of_property_setter(attrib, opts)
+          create_collection_of_getter(attrib, opts)
+          create_collection_of_setter(attrib, opts)
         end
 
 
         private
 
-        def merge_belongs_to_assocation_options(attrib, options = nil)
+        def merge_belongs_to_association_options(attrib, options = nil)
           opts = {
-            :foreign_key => attrib.to_s + '_id',
-            :class_name => attrib.to_s.camelcase,
+            :foreign_key => attrib.to_s.singularize + '_id',
+            :class_name => attrib.to_s.singularize.camelcase,
             :proxy_name => attrib.to_s.pluralize
           }
-
-          # merge the options
-          case options
-          when Hash
-            opts.merge!(options)
-          end
-          # Get a class name
-          begin
-            opts[:class] = opts[:class_name].constantize
-          rescue NameError
-            raise NameError, "Unable to convert class name into Constant for #{self.name}##{attrib}"
-          end
+          opts.merge!(options) if options.is_a?(Hash)
 
           # Generate a string for the proxy method call
           # Assumes that the proxy_owner_method from "proxyable" is available.
@@ -129,14 +112,14 @@ module CouchRest
             opts[:proxy] = if proxy_owner_method
               "self.#{proxy_owner_method}.#{opts[:proxy_name]}"
             else
-              opts[:class_name].constantize
+              opts[:class_name]
             end
           end
 
           opts
         end
 
-        def create_belongs_to_getter(attrib, property, options)
+        def create_belongs_to_getter(attrib, options)
           class_eval <<-EOS, __FILE__, __LINE__ + 1
             def #{attrib}
               @#{attrib} ||= #{options[:foreign_key]}.nil? ? nil : #{options[:proxy]}.get(self.#{options[:foreign_key]})
@@ -144,7 +127,7 @@ module CouchRest
           EOS
         end
 
-        def create_belongs_to_setter(attrib, property, options)
+        def create_belongs_to_setter(attrib, options)
           class_eval <<-EOS, __FILE__, __LINE__ + 1
             def #{attrib}=(value)
               self.#{options[:foreign_key]} = value.nil? ? nil : value.id
@@ -155,7 +138,7 @@ module CouchRest
 
         ### collection_of support methods
 
-        def create_collection_of_property_setter(attrib, property, options)
+        def create_collection_of_property_setter(attrib, options)
           # ensure CollectionOfProxy is nil, ready to be reloaded on request
           class_eval <<-EOS, __FILE__, __LINE__ + 1
             def #{options[:foreign_key]}=(value)
@@ -165,18 +148,17 @@ module CouchRest
           EOS
         end
 
-        def create_collection_of_getter(attrib, property, options)
-          base = options[:proxy] || options[:class_name]
+        def create_collection_of_getter(attrib, options)
           class_eval <<-EOS, __FILE__, __LINE__ + 1
             def #{attrib}(reload = false)
               return @#{attrib} unless @#{attrib}.nil? or reload
-              ary = self.#{options[:foreign_key]}.collect{|i| #{base}.get(i)}
+              ary = self.#{options[:foreign_key]}.collect{|i| #{options[:proxy]}.get(i)}
               @#{attrib} = ::CouchRest::CollectionOfProxy.new(ary, self, '#{options[:foreign_key]}')
             end
           EOS
         end
 
-        def create_collection_of_setter(attrib, property, options)
+        def create_collection_of_setter(attrib, options)
           class_eval <<-EOS, __FILE__, __LINE__ + 1
             def #{attrib}=(value)
               @#{attrib} = ::CouchRest::CollectionOfProxy.new(value, self, '#{options[:foreign_key]}')
diff --git a/lib/couchrest/model/proxyable.rb b/lib/couchrest/model/proxyable.rb
index 76fdf2c..965589c 100644
--- a/lib/couchrest/model/proxyable.rb
+++ b/lib/couchrest/model/proxyable.rb
@@ -6,14 +6,11 @@ module CouchRest
 
       module ClassMethods
 
-        attr_reader :proxy_owner_method
-
         # Define a collection that will use the base model for the database connection
         # details.
         def proxy_for(assoc_name, options = {})
           db_method = options[:database_method] || "proxy_database"
           options[:class_name] ||= assoc_name.to_s.singularize.camelize
-          attr_accessor :model_proxy
           class_eval <<-EOS, __FILE__, __LINE__ + 1
             def #{assoc_name}
               unless respond_to?('#{db_method}')
@@ -27,8 +24,16 @@ module CouchRest
         def proxied_by(model_name, options = {})
           raise "Model can only be proxied once or ##{model_name} already defined" if method_defined?(model_name) || !proxy_owner_method.nil?
           self.proxy_owner_method = model_name
+          attr_accessor :model_proxy
           attr_accessor model_name
         end
+
+        # Define an a class variable accessor ready to be inherited and unique
+        # for each Class using the base.
+        # Perhaps there is a shorter way of writing this.
+        def proxy_owner_method=(name); @proxy_owner_method = name; end
+        def proxy_owner_method; @proxy_owner_method; end
+
       end
 
       class ModelProxy
@@ -132,10 +137,10 @@ module CouchRest
         # Update the document's proxy details, specifically, the fields that
         # link back to the original document.
         def proxy_update(doc)
-          if doc
-            doc.database = @database if doc.respond_to?(:database=)
-            doc.model_proxy = self if doc.respond_to?(:model_proxy=)
-            doc.send("#{owner_name}=", owner) if doc.respond_to?("#{owner_name}=")
+          if doc && doc.is_a?(model)
+            doc.database = @database
+            doc.model_proxy = self
+            doc.send("#{owner_name}=", owner)
           end
           doc
         end
diff --git a/lib/couchrest/model/validations/uniqueness.rb b/lib/couchrest/model/validations/uniqueness.rb
index 2130428..4cf6b90 100644
--- a/lib/couchrest/model/validations/uniqueness.rb
+++ b/lib/couchrest/model/validations/uniqueness.rb
@@ -16,7 +16,7 @@ module CouchRest
         def validate_each(document, attribute, value)
           view_name = options[:view].nil? ? "by_#{attribute}" : options[:view]
 
-          model = (respond_to?(:model_proxy) && model_proxy ? model_proxy : @model)
+          model = (document.respond_to?(:model_proxy) && document.model_proxy ? document.model_proxy : @model)
           # Determine the base of the search
           base = options[:proxy].nil? ? model : document.instance_eval(options[:proxy])
 
diff --git a/lib/couchrest/model/views.rb b/lib/couchrest/model/views.rb
index 6157102..a72d5da 100644
--- a/lib/couchrest/model/views.rb
+++ b/lib/couchrest/model/views.rb
@@ -100,7 +100,7 @@ module CouchRest
           query = query.dup # Modifications made on copy!
           db = query.delete(:database) || database
           refresh_design_doc(db)
-          query[:raw] = true if query[:reduce]        
+          query[:raw] = true if query[:reduce]
           raw = query.delete(:raw)
           fetch_view_with_docs(db, name, query, raw, &block)
         end
diff --git a/spec/couchrest/assocations_spec.rb b/spec/couchrest/assocations_spec.rb
index 36e0e5d..cb3b0c4 100644
--- a/spec/couchrest/assocations_spec.rb
+++ b/spec/couchrest/assocations_spec.rb
@@ -5,6 +5,35 @@ require File.join(FIXTURE_PATH, 'more', 'sale_invoice')
 
 describe "Assocations" do
 
+  describe ".merge_belongs_to_association_options" do
+    before :all do
+      def SaleInvoice.merge_assoc_opts(*args)
+        merge_belongs_to_association_options(*args)
+      end
+    end
+
+    it "should return a default set of options" do
+      o = SaleInvoice.merge_assoc_opts(:cat)
+      o[:foreign_key].should eql('cat_id')
+      o[:class_name].should eql('Cat')
+      o[:proxy_name].should eql('cats')
+      o[:proxy].should eql('Cat') # same as class name
+    end
+
+    it "should merge with provided options" do
+      o = SaleInvoice.merge_assoc_opts(:cat, :foreign_key => 'somecat_id', :proxy => 'some_cats')
+      o[:foreign_key].should eql('somecat_id')
+      o[:proxy].should eql('some_cats')
+    end
+
+    it "should generate a proxy string if proxied" do
+      SaleInvoice.stub!(:proxy_owner_method).twice.and_return('company')
+      o = SaleInvoice.merge_assoc_opts(:cat)
+      o[:proxy].should eql('self.company.cats')
+    end
+    
+  end
+
   describe "of type belongs to" do
 
     before :each do
@@ -43,14 +72,6 @@ describe "Assocations" do
       @invoice.client
     end
 
-    it "should raise error if class name does not exist" do
-      lambda do
-        class TestBadAssoc < CouchRest::Model::Base
-          belongs_to :test_bad_item
-        end
-      end.should raise_error(NameError, /TestBadAssoc#test_bad_item/)
-    end
-
     it "should allow override of foreign key" do
       @invoice.respond_to?(:alternate_client).should be_true
       @invoice.respond_to?("alternate_client=").should be_true
@@ -78,8 +99,8 @@ describe "Assocations" do
     end
 
     it "should create an associated property and collection proxy" do
-      @invoice.respond_to?('entry_ids')
-      @invoice.respond_to?('entry_ids=')
+      @invoice.respond_to?('entry_ids').should be_true
+      @invoice.respond_to?('entry_ids=').should be_true
       @invoice.entries.class.should eql(::CouchRest::CollectionOfProxy)
     end
 
diff --git a/spec/couchrest/proxyable_spec.rb b/spec/couchrest/proxyable_spec.rb
index 4873c5d..d67307b 100644
--- a/spec/couchrest/proxyable_spec.rb
+++ b/spec/couchrest/proxyable_spec.rb
@@ -13,16 +13,27 @@ end
 
 describe "Proxyable" do
 
-  it "should provide #model_proxy method" do
-    DummyProxyable.new.should respond_to(:model_proxy)
-  end
-
   describe "class methods" do
 
+    before(:each) do
+      @class = DummyProxyable.clone
+    end
+
+    describe ".proxy_owner_method" do
+      it "should provide proxy_owner_method accessors" do
+        @class.should respond_to(:proxy_owner_method)
+        @class.should respond_to(:proxy_owner_method=)
+      end
+      it "should work as expected" do
+        @class.proxy_owner_method = "foo"
+        @class.proxy_owner_method.should eql("foo")
+      end
+    end
+
     describe ".proxy_for" do
 
       it "should be provided" do
-        DummyProxyable.should respond_to(:proxy_for)
+        @class.should respond_to(:proxy_for)
       end
 
       it "should create a new method" do
@@ -54,42 +65,53 @@ describe "Proxyable" do
         end
 
         it "should raise an error if the database method is missing" do
-          DummyProxyable.proxy_for(:cats)
-          @obj = DummyProxyable.new
+          @class.proxy_for(:cats)
+          @obj = @class.new
           @obj.should_receive(:respond_to?).with('proxy_database').and_return(false)
           lambda { @obj.cats }.should raise_error(StandardError, "Missing #proxy_database method for proxy")
         end
 
         it "should raise an error if custom database method missing" do
-          DummyProxyable.proxy_for(:proxy_kittens, :database_method => "foobardom")
-          @obj = DummyProxyable.new
+          @class.proxy_for(:proxy_kittens, :database_method => "foobardom")
+          @obj = @class.new
           lambda { @obj.proxy_kittens }.should raise_error(StandardError, "Missing #foobardom method for proxy")
         end
-
-
       end
-
     end
 
     describe ".proxied_by" do
       it "should be provided" do
-        DummyProxyable.should respond_to(:proxied_by)
+        @class.should respond_to(:proxied_by)
       end
 
       it "should add an attribute accessor" do
-        DummyProxyable.proxied_by(:foobar)
-        DummyProxyable.new.should respond_to(:foobar)
+        @class.proxied_by(:foobar)
+        @class.new.should respond_to(:foobar)
+      end
+
+      it "should provide #model_proxy method" do
+        @class.proxied_by(:foobar)
+        @class.new.should respond_to(:model_proxy)
+      end
+
+      it "should set the proxy_owner_method" do
+        @class.proxied_by(:foobar)
+        @class.proxy_owner_method.should eql(:foobar)
       end
 
       it "should raise an error if model name pre-defined" do
-        lambda { DummyProxyable.proxied_by(:object_id) }.should raise_error
+        lambda { @class.proxied_by(:object_id) }.should raise_error
+      end
+
+      it "should raise an error if object already has a proxy" do
+        @class.proxied_by(:department)
+        lambda { @class.proxied_by(:company) }.should raise_error
       end
     end
-
   end
 
   describe "ModelProxy" do
-    
+
     before :all do
       @klass = CouchRest::Model::Proxyable::ModelProxy
     end
@@ -241,22 +263,18 @@ describe "Proxyable" do
       describe "#proxy_update" do
         it "should set returned doc fields" do
           doc = mock(:Document)
-          doc.should_receive(:respond_to?).with(:database=).and_return(true)
+          doc.should_receive(:is_a?).with(Cat).and_return(true)
           doc.should_receive(:database=).with('database')
-          doc.should_receive(:respond_to?).with(:model_proxy=).and_return(true)
           doc.should_receive(:model_proxy=).with(@obj)
-          doc.should_receive(:respond_to?).with('owner_name=').and_return(true)
           doc.should_receive(:send).with('owner_name=', 'owner')
           @obj.send(:proxy_update, doc).should eql(doc)
         end
 
-        it "should not fail if some fields missing" do
-          doc = mock(:Document)
-          doc.should_receive(:respond_to?).with(:database=).and_return(true)
-          doc.should_receive(:database=).with('database')
-          doc.should_receive(:respond_to?).with(:model_proxy=).and_return(false)
+        it "should not set anything if matching document not provided" do
+          doc = mock(:DocumentFoo)
+          doc.should_receive(:is_a?).with(Cat).and_return(false)
+          doc.should_not_receive(:database=)
           doc.should_not_receive(:model_proxy=)
-          doc.should_receive(:respond_to?).with('owner_name=').and_return(false)
           doc.should_not_receive(:owner_name=)
           @obj.send(:proxy_update, doc).should eql(doc)
         end
@@ -309,6 +327,7 @@ describe "Proxyable" do
 
     it "should allow creation of new entries" do
       inv = @company.proxyable_invoices.new(:client => "Lorena", :total => 35)
+      inv.database.should_not be_nil
       inv.save.should be_true
       @company.proxyable_invoices.count.should eql(1)
       @company.proxyable_invoices.first.client.should eql("Lorena")
diff --git a/spec/couchrest/validations_spec.rb b/spec/couchrest/validations_spec.rb
index 661e563..85f20c7 100644
--- a/spec/couchrest/validations_spec.rb
+++ b/spec/couchrest/validations_spec.rb
@@ -77,7 +77,17 @@ describe "Validations" do
       end
     end
 
- 
+    context "when proxied" do
+      it "should lookup the model_proxy" do
+        mp = mock(:ModelProxy)
+        mp.should_receive(:view).and_return({'rows' => []})
+        @obj = WithUniqueValidation.new(:title => 'test 8')
+        @obj.stub!(:model_proxy).twice.and_return(mp)
+        @obj.valid?
+      end
+
+    end
+
   end
 
 end