RUBY-185 fix JRuby null pointer exception on embedded docs

Better HashWithIndifferentAccess tests

ext/java/src/org/jbson/RubyBSONEncoder.java

@@ -214,7 +214,10 @@ public class RubyBSONEncoder extends BSONEncoder {
 
         if ( DEBUG ) {
           System.out.println( "\t put thing : " + name );
-          System.out.println( "\t class : " + val.getClass().getName() );
+          if( val == null )
+            System.out.println( "\t class : null value" );
+          else
+            System.out.println( "\t class : " + val.getClass().getName() );
         }
 
         if ( name.equals( "$where") && val instanceof String ) {
@@ -256,6 +259,9 @@ public class RubyBSONEncoder extends BSONEncoder {
         else if ( val instanceof byte[] )
             putBinary( name , (byte[])val );
 
+        else if ( val == null )
+            putNull(name);
+
         else if ( val.getClass().isArray() )
             putIterable( name , Arrays.asList( (Object[])val ) );
 

test/bson/bson_test.rb

@@ -12,9 +12,11 @@ rescue LoadError
   warn 'Mocking time with zone'
   module ActiveSupport
     class TimeWithZone
+      def initialize(utc_time, zone)
+      end
     end
   end
-  Zone = ActiveSupport::TimeWithZone.new
+  Zone = ActiveSupport::TimeWithZone.new(Time.now.utc, 'EST')
 end
 
 class BSONTest < Test::Unit::TestCase
@@ -167,7 +169,11 @@ class BSONTest < Test::Unit::TestCase
   def test_object
     doc = {'doc' => {'age' => 42, 'name' => 'Spongebob', 'shoe_size' => 9.5}}
     assert_doc_pass(doc)
-    bson = BSON::BSON_CODER.serialize(doc)
+  end
+
+  def test_embedded_document_with_nil
+    doc = {'doc' => {'age' => 42, 'name' => nil, 'shoe_size' => 9.5}}
+    assert_doc_pass(doc)
   end
 
   def test_oid
@@ -507,22 +513,4 @@ class BSONTest < Test::Unit::TestCase
     end
   end
 
-  # Mocking this class for testing
-  class ::HashWithIndifferentAccess < Hash; end
-
-  def test_keep_id_with_hash_with_indifferent_access
-    doc = HashWithIndifferentAccess.new
-    embedded = HashWithIndifferentAccess.new
-    embedded['_id'] = ObjectId.new
-    doc['_id']      = ObjectId.new
-    doc['embedded'] = [embedded]
-    @encoder.serialize(doc, false, true).to_a
-    assert doc.has_key?("_id")
-    assert doc['embedded'][0].has_key?("_id")
-
-    doc['_id'] = ObjectId.new
-    @encoder.serialize(doc, false, true).to_a
-    assert doc.has_key?("_id")
-  end
-
 end

test/bson/hash_with_indifferent_access_test.rb

@@ -0,0 +1,38 @@
+# encoding:utf-8
+require './test/test_helper'
+require './test/support/hash_with_indifferent_access'
+
+class HashWithIndifferentAccessTest < Test::Unit::TestCase
+  include BSON
+
+  def setup
+    @encoder = BSON::BSON_CODER
+  end
+
+  def test_document
+    doc = HashWithIndifferentAccess.new
+    doc['foo'] = 1
+    doc['bar'] = 'baz'
+
+    bson = @encoder.serialize(doc)
+    assert_equal doc, @encoder.deserialize(bson.to_s)
+  end
+
+  def test_embedded_document
+    jimmy = HashWithIndifferentAccess.new
+    jimmy['name']     = 'Jimmy'
+    jimmy['species'] = 'Siberian Husky'
+
+    stats = HashWithIndifferentAccess.new
+    stats['eyes'] = 'blue'
+
+    person = HashWithIndifferentAccess.new
+    person['_id'] = BSON::ObjectId.new
+    person['name'] = 'Mr. Pet Lover'
+    person['pets'] = [jimmy, {'name' => 'Sasha'}]
+    person['stats'] = stats
+
+    bson = @encoder.serialize(person)
+    assert_equal person, @encoder.deserialize(bson.to_s)
+  end
+end

test/support/hash_with_indifferent_access.rb

@@ -0,0 +1,154 @@
+# Note: HashWithIndifferentAccess is so commonly used
+# that we always need to make sure that the driver works
+# with it.
+require File.join(File.dirname(__FILE__), 'keys.rb')
+
+# This class has dubious semantics and we only have it so that
+# people can write params[:key] instead of params['key']
+# and they get the same value for both keys.
+
+module ActiveSupport
+  class HashWithIndifferentAccess < Hash
+    def extractable_options?
+      true
+    end
+
+    def initialize(constructor = {})
+      if constructor.is_a?(Hash)
+        super()
+        update(constructor)
+      else
+        super(constructor)
+      end
+    end
+
+    def default(key = nil)
+      if key.is_a?(Symbol) && include?(key = key.to_s)
+        self[key]
+      else
+        super
+      end
+    end
+
+    def self.new_from_hash_copying_default(hash)
+      ActiveSupport::HashWithIndifferentAccess.new(hash).tap do |new_hash|
+        new_hash.default = hash.default
+      end
+    end
+
+    alias_method :regular_writer, :[]= unless method_defined?(:regular_writer)
+    alias_method :regular_update, :update unless method_defined?(:regular_update)
+
+    # Assigns a new value to the hash:
+    #
+    #   hash = HashWithIndifferentAccess.new
+    #   hash[:key] = "value"
+    #
+    def []=(key, value)
+      regular_writer(convert_key(key), convert_value(value))
+    end
+
+    # Updates the instantized hash with values from the second:
+    #
+    #   hash_1 = HashWithIndifferentAccess.new
+    #   hash_1[:key] = "value"
+    #
+    #   hash_2 = HashWithIndifferentAccess.new
+    #   hash_2[:key] = "New Value!"
+    #
+    #   hash_1.update(hash_2) # => {"key"=>"New Value!"}
+    #
+    def update(other_hash)
+      other_hash.each_pair { |key, value| regular_writer(convert_key(key), convert_value(value)) }
+      self
+    end
+
+    alias_method :merge!, :update
+
+    # Checks the hash for a key matching the argument passed in:
+    #
+    #   hash = HashWithIndifferentAccess.new
+    #   hash["key"] = "value"
+    #   hash.key? :key  # => true
+    #   hash.key? "key" # => true
+    #
+    def key?(key)
+      super(convert_key(key))
+    end
+
+    alias_method :include?, :key?
+    alias_method :has_key?, :key?
+    alias_method :member?, :key?
+
+    # Fetches the value for the specified key, same as doing hash[key]
+    def fetch(key, *extras)
+      super(convert_key(key), *extras)
+    end
+
+    # Returns an array of the values at the specified indices:
+    #
+    #   hash = HashWithIndifferentAccess.new
+    #   hash[:a] = "x"
+    #   hash[:b] = "y"
+    #   hash.values_at("a", "b") # => ["x", "y"]
+    #
+    def values_at(*indices)
+      indices.collect {|key| self[convert_key(key)]}
+    end
+
+    # Returns an exact copy of the hash.
+    def dup
+      HashWithIndifferentAccess.new(self)
+    end
+
+    # Merges the instantized and the specified hashes together, giving precedence to the values from the second hash
+    # Does not overwrite the existing hash.
+    def merge(hash)
+      self.dup.update(hash)
+    end
+
+    # Performs the opposite of merge, with the keys and values from the first hash taking precedence over the second.
+    # This overloaded definition prevents returning a regular hash, if reverse_merge is called on a HashWithDifferentAccess.
+    def reverse_merge(other_hash)
+      super self.class.new_from_hash_copying_default(other_hash)
+    end
+
+    def reverse_merge!(other_hash)
+      replace(reverse_merge( other_hash ))
+    end
+
+    # Removes a specified key from the hash.
+    def delete(key)
+      super(convert_key(key))
+    end
+
+    def stringify_keys!; self end
+    def stringify_keys; dup end
+    undef :symbolize_keys!
+    def symbolize_keys; to_hash.symbolize_keys end
+    def to_options!; self end
+
+    # Convert to a Hash with String keys.
+    def to_hash
+      Hash.new(default).merge!(self)
+    end
+
+    protected
+      def convert_key(key)
+        key.kind_of?(Symbol) ? key.to_s : key
+      end
+
+      def convert_value(value)
+        case value
+        when Hash
+          self.class.new_from_hash_copying_default(value)
+        when Array
+          value.collect { |e| e.is_a?(Hash) ? self.class.new_from_hash_copying_default(e) : e }
+        else
+          value
+        end
+      end
+  end
+end
+
+HashWithIndifferentAccess = ActiveSupport::HashWithIndifferentAccess

test/support/keys.rb

@@ -0,0 +1,45 @@
+class Hash
+  # Return a new hash with all keys converted to strings.
+  def stringify_keys
+    dup.stringify_keys!
+  end
+
+  # Destructively convert all keys to strings.
+  def stringify_keys!
+    keys.each do |key|
+      self[key.to_s] = delete(key)
+    end
+    self
+  end
+
+  # Return a new hash with all keys converted to symbols, as long as
+  # they respond to +to_sym+.
+  def symbolize_keys
+    dup.symbolize_keys!
+  end
+
+  # Destructively convert all keys to symbols, as long as they respond
+  # to +to_sym+.
+  def symbolize_keys!
+    keys.each do |key|
+      self[(key.to_sym rescue key) || key] = delete(key)
+    end
+    self
+  end
+
+  alias_method :to_options,  :symbolize_keys
+  alias_method :to_options!, :symbolize_keys!
+
+  # Validate all keys in a hash match *valid keys, raising ArgumentError on a mismatch.
+  # Note that keys are NOT treated indifferently, meaning if you use strings for keys but assert symbols
+  # as keys, this will fail.
+  #
+  # ==== Examples
+  #   { :name => "Rob", :years => "28" }.assert_valid_keys(:name, :age) # => raises "ArgumentError: Unknown key(s): years"
+  #   { :name => "Rob", :age => "28" }.assert_valid_keys("name", "age") # => raises "ArgumentError: Unknown key(s): name, age"
+  #   { :name => "Rob", :age => "28" }.assert_valid_keys(:name, :age) # => passes, raises nothing
+  def assert_valid_keys(*valid_keys)
+    unknown_keys = keys - [valid_keys].flatten
+    raise(ArgumentError, "Unknown key(s): #{unknown_keys.join(", ")}") unless unknown_keys.empty?
+  end
+end

test/test_helper.rb

@@ -21,7 +21,9 @@ end
 
 require 'bson_ext/cbson' if ENV['C_EXT']
 
-MONGO_TEST_DB = 'mongo-ruby-test'
+unless defined? MONGO_TEST_DB
+  MONGO_TEST_DB = 'mongo-ruby-test'
+end
 
 class Test::Unit::TestCase
   include Mongo