Beefed up tests for to/from mongo. Refactored attributes and to_mongo.

lib/mongomapper/document.rb

@@ -312,7 +312,6 @@ def update
         save_to_collection
       end
 
-      # collection.save returns mongoid
       def save_to_collection
         clear_custom_id_flag
         collection.save(to_mongo)

lib/mongomapper/embedded_document.rb

@@ -219,28 +219,40 @@ def attributes=(attrs)
 
       def attributes
         attrs = HashWithIndifferentAccess.new
-        self.class.keys.each_pair do |name, key|
-          value =
-            if key.embeddable?
-              if embedded_document = read_attribute(key.name)
-                embedded_document.attributes
-              end
-            else
-              read_attribute(key.name)
-            end
-
-          attrs[name] = value unless value.nil?
+
+        embedded_keys.each do |key|
+          puts key.inspect
+          attrs[key.name] = read_attribute(key.name).try(:attributes)
+        end
+
+        non_embedded_keys.each do |key|
+          attrs[key.name] = read_attribute(key.name)
+        end
+
+        embedded_associations.each do |association|
+          documents = instance_variable_get(association.ivar)
+          next if documents.nil?
+          attrs[association.name] = documents.collect { |doc| doc.attributes }
         end
-        attrs.merge!(embedded_association_attributes)
+
+        attrs
       end
 
       def to_mongo
         attrs = HashWithIndifferentAccess.new
-        self.class.keys.each_pair do |name, key|
+
+        _keys.each_pair do |name, key|
           value = key.set(read_attribute(key.name))
           attrs[name] = value unless value.nil?
         end
-        attrs.merge!(embedded_associations_to_mongo)
+
+        embedded_associations.each do |association|
+          if documents = instance_variable_get(association.ivar)
+            attrs[association.name] = documents.map { |document| document.to_mongo }
+          end
+        end
+
+        attrs
       end
 
       def [](name)
@@ -270,7 +282,7 @@ def using_custom_id?
       end
 
       def inspect
-        attributes_as_nice_string = self.class.keys.keys.collect do |name|
+        attributes_as_nice_string = key_names.collect do |name|
           "#{name}: #{read_attribute(name).inspect}"
         end.join(", ")
         "#<#{self.class} #{attributes_as_nice_string}>"
@@ -288,12 +300,28 @@ def update_attributes(attrs={})
       end
 
       private
+        def _keys
+          self.class.keys
+        end
+
+        def key_names
+          _keys.keys
+        end
+
+        def non_embedded_keys
+          _keys.values.select { |key| !key.embeddable? }
+        end
+
+        def embedded_keys
+          _keys.values.select { |key| key.embeddable? }
+        end
+
         def ensure_key_exists(name)
           self.class.key(name) unless respond_to?("#{name}=")
         end
 
         def read_attribute(name)
-          value = self.class.keys[name].get(instance_variable_get("@#{name}"))
+          value = _keys[name].get(instance_variable_get("@#{name}"))
           instance_variable_set "@#{name}", value if !frozen?
           value
         end
@@ -303,7 +331,7 @@ def read_attribute_before_typecast(name)
         end
 
         def write_attribute(name, value)
-          key = self.class.keys[name]          
+          key = _keys[name]          
           instance_variable_set "@#{name}_before_typecast", value
           instance_variable_set "@#{name}", key.set(value)
 
@@ -321,27 +349,7 @@ def embedded_associations
           end.map do |name, association|
             association
           end
-        end
-
-        def embedded_associations_to_mongo
-          returning HashWithIndifferentAccess.new do |attrs|
-            embedded_associations.each do |association|
-              documents = instance_variable_get(association.ivar)
-              next if documents.nil?
-              attrs[association.name] = documents.collect { |doc| doc.to_mongo }
-            end
-          end
-        end
-
-        def embedded_association_attributes
-          returning HashWithIndifferentAccess.new do |attrs|
-            embedded_associations.each do |association|
-              documents = instance_variable_get(association.ivar)
-              next if documents.nil?
-              attrs[association.name] = documents.collect { |doc| doc.attributes }
-            end
-          end
-        end
+        end        
     end # InstanceMethods
   end # EmbeddedDocument
 end # MongoMapper

lib/mongomapper/support.rb

@@ -31,7 +31,7 @@ def self.to_mongo(value)
 
 class String
   def self.to_mongo(value)
-    value.to_s
+    value.nil? ? nil : value.to_s
   end
 
   def self.from_mongo(value)
@@ -55,17 +55,13 @@ def self.from_mongo(value)
   end
 end
 
-class Hash  
-  def self.to_mongo(value)
-    value unless value.nil?
-  end
-
+class Hash
   def self.from_mongo(value)
     HashWithIndifferentAccess.new(value || {})
   end
 
   def to_mongo
-    Hash.to_mongo(self)
+    self
   end
 end
 

test/functional/test_document.rb

@@ -811,21 +811,6 @@ class ::Thing
         doc.date.should == Date.new(2009, 12, 1)
       end
     end
-
-    context "Saving an embedded document with Date key set" do
-      should "save the Date value as a Time object" do
-        Pet.class_eval do
-          key :date_of_birth, Date
-        end
-
-        doc = RealPerson.new
-        doc.pets = [Pet.new(:date_of_birth => "12/01/2009")]
-        doc.save
-
-        doc = RealPerson.find(doc.id)
-        doc.pets.last.date_of_birth.should == Date.new(2009, 12, 1)
-      end
-    end
   end
 
   context "Saving an existing document" do
@@ -851,7 +836,7 @@ class ::Thing
       from_db.age.should == 30
     end
 
-    should "allow to update custom attributes" do
+    should "allow updating custom attributes" do
       @doc = @document.new(:first_name => 'David', :age => '26', :gender => 'male')
       @doc.gender = 'Male'
       @doc.save
@@ -887,7 +872,7 @@ class ::Thing
       from_db.age.should == 30
     end
 
-    should "allow to update custom attributes" do
+    should "allow updating custom attributes" do
       @doc.update_attributes(:gender => 'mALe')
       from_db = @document.find(@doc.id)
       from_db.gender.should == 'mALe'
@@ -966,36 +951,6 @@ class ::Thing
     end
   end
 
-  context "embedded document persistence" do
-    setup do
-      @person = RealPerson.create
-    end
-
-    should "save the embedded document" do
-      pet = Pet.new :name => 'sparky'
-      @person.pets << pet
-      pet.save
-
-      doc = RealPerson.find(@person.id)
-      doc.pets.first.should == pet
-    end
-
-    should "update_attributes on the embedded document" do
-      pet = Pet.new(:name => 'sparky')
-      @person.pets << pet
-      pet.save
-
-      doc = RealPerson.find(@person.id)
-      pet = doc.pets.first
-      pet.update_attributes :name => 'koda'
-
-      doc = RealPerson.find(@person.id)
-      embedded = doc.pets.first
-      embedded.id.should == pet.id
-      embedded.name.should == 'koda'
-    end
-  end
-
   context "timestamping" do
     setup do
       @document.timestamps!

test/functional/test_embedded_document.rb

@@ -0,0 +1,32 @@
+require 'test_helper'
+require 'models'
+
+class EmbeddedDocumentTest < Test::Unit::TestCase    
+  should "save the embedded document" do
+    person = RealPerson.create
+
+    pet = Pet.new :name => 'sparky'
+    person.pets << pet
+    pet.save
+
+    doc = RealPerson.find(person.id)
+    doc.pets.first.should == pet
+  end
+
+  should "update_attributes on the embedded document" do
+    person = RealPerson.create
+
+    pet = Pet.new(:name => 'sparky')
+    person.pets << pet
+    pet.save
+
+    doc = RealPerson.find(person.id)
+    pet = doc.pets.first
+    pet.update_attributes :name => 'koda'
+
+    doc = RealPerson.find(person.id)
+    embedded = doc.pets.first
+    embedded.id.should == pet.id
+    embedded.name.should == 'koda'
+  end
+end

test/unit/test_embedded_document.rb

@@ -32,8 +32,8 @@ class OtherChild < Parent
   key :other_child, String
 end
 
-class EmbeddedDocumentTest < Test::Unit::TestCase
-  context "Including MongoMapper::EmbeddedDocument" do
+class EmbeddedDocumentTest < Test::Unit::TestCase 
+  context "Including MongoMapper::EmbeddedDocument in a class" do
     setup do
       @klass = Class.new do
         include MongoMapper::EmbeddedDocument
@@ -43,6 +43,36 @@ class EmbeddedDocumentTest < Test::Unit::TestCase
     should "add _id key" do
       @klass.keys['_id'].should_not be_nil
     end
+
+    context "#to_mongo" do
+      should "be nil if nil" do
+        @klass.to_mongo(nil).should be_nil
+      end
+
+      should "convert to_mongo for other values" do
+        doc = @klass.new(:foo => 'bar')
+        to_mongo = @klass.to_mongo(doc)
+        to_mongo.is_a?(Hash).should be_true
+        to_mongo['foo'].should == 'bar'
+      end
+    end
+
+    context "#from_mongo" do
+      should "be nil if nil" do
+        @klass.from_mongo(nil).should be_nil
+      end
+
+      should "be instance if instance of class" do
+        doc = @klass.new
+        @klass.from_mongo(doc).should == doc
+      end
+
+      should "be instance if hash of attributes" do
+        doc = @klass.from_mongo({:foo => 'bar'})
+        doc.instance_of?(@klass).should be_true
+        doc.foo.should == 'bar'
+      end
+    end
   end
 
   context "parent_model" do
@@ -368,18 +398,33 @@ def passwd
     end
 
     context "attributes" do
-      should "default to hash with _id" do
+      should "default to hash with all keys" do
         doc = @document.new
-        doc.attributes.keys.should == ['_id']
+        doc.attributes.keys.sort.should == ['_id', 'age', 'name']
       end
 
-      should "return all keys that aren't nil" do
+      should "return all keys with values" do
         doc = @document.new(:name => 'string', :age => nil)
-        doc.attributes.keys.sort.should == ['_id', 'name']
+        doc.attributes.keys.sort.should == ['_id', 'age', 'name']
         doc.attributes.values.should include('string')
+        doc.attributes.values.should include(nil)
       end
     end
-
+
+    context "to_mongo" do
+      should "default to hash with _id key" do
+        doc = @document.new
+        doc.to_mongo.keys.should == ['_id']
+      end
+
+      should "return all keys with non nil values" do
+        doc = @document.new(:name => 'string', :age => nil)
+        doc.to_mongo.keys.sort.should == ['_id', 'name']
+        doc.to_mongo.values.should include('string')
+        doc.to_mongo.values.should_not include(nil)
+      end
+    end
+
     should "convert dates into times" do
       document = Class.new(@document) do
         key :start_date, Date