Merge pull request #537 from michaelherold/dash-consistency

Fix inconsistencies with Dash defaults

CHANGELOG.md

@@ -41,6 +41,7 @@ Any violations of this scheme are considered to be bugs.
 * [#516](https://github.com/hashie/hashie/issues/516): Fixed `NoMethodError` raised when including `Hashie::Extensions::Mash::SymbolizeKeys` and `Hashie::Extensions::SymbolizeKeys` in mashes/hashes with non string or symbol keys - [@carolineartz](https://github.com/carolineartz).
 * [#531](https://github.com/hashie/hashie/pull/531): Fixed [slice doesn't work using symbols](https://github.com/hashie/hashie/issues/529) using hash with `IndifferentAccess` extension - [@gnomex](https://github.com/gnomex).
 * [#533](https://github.com/hashie/hashie/pull/533): Fixed `NoMethodError: undefined method `to_json'` at `hashie/dash_spec` - [@gnomex](https://github.com/gnomex).
+* [#537](https://github.com/hashie/hashie/pull/537): Fixed inconsistencies with handling defaults in `Dash` with and without `IgnoreUnclared` mixed in - [@michaelherold](https://github.com/michaelherold).
 * Your contribution here.
 
 ### Security

README.md

@@ -875,6 +875,27 @@ p = Tricky.new('trick' => 'two')
 p.trick # => NoMethodError
 ```
 
+If you would like to update a Dash and use any default values set in the case of a `nil` value, use `#update_attributes!`.
+
+```ruby
+class WithDefaults < Hashie::Dash
+  property :description, default: 'none'
+end
+
+dash = WithDefaults.new
+dash.description  #=> 'none'
+
+dash.description = 'You committed one of the classic blunders!'
+dash.description  #=> 'You committed one of the classic blunders!'
+
+dash.description = nil
+dash.description  #=> nil
+
+dash.description = 'Only slightly less known is ...'
+dash.update_attributes!(description: nil)
+dash.description  #=> 'none'
+```
+
 ### Potential Gotchas
 
 Because Dashes are subclasses of the built-in Ruby Hash class, the double-splat operator takes the Dash as-is without any conversion. This can lead to strange behavior when you use the double-splat operator on a Dash as the first part of a keyword list or built Hash. For example:

lib/hashie/dash.rb

@@ -104,19 +104,6 @@ def self.required?(name)
     def initialize(attributes = {}, &block)
       super(&block)
 
-      self.class.defaults.each_pair do |prop, value|
-        self[prop] = begin
-          val = value.dup
-          if val.is_a?(Proc)
-            val.arity == 1 ? val.call(self) : val.call
-          else
-            val
-          end
-        rescue TypeError
-          value
-        end
-      end
-
       initialize_attributes(attributes)
       assert_required_attributes_set!
     end
@@ -173,13 +160,19 @@ def update_attributes!(attributes)
       update_attributes(attributes)
 
       self.class.defaults.each_pair do |prop, value|
-        next unless self[prop].nil?
+        next unless fetch(prop, nil).nil?
         self[prop] = begin
-          value.dup
+          val = value.dup
+          if val.is_a?(Proc)
+            val.arity == 1 ? val.call(self) : val.call
+          else
+            val
+          end
         rescue TypeError
           value
         end
       end
+
       assert_required_attributes_set!
     end
 
@@ -189,7 +182,7 @@ def initialize_attributes(attributes)
       return unless attributes
 
       cleaned_attributes = attributes.reject { |_attr, value| value.nil? }
-      update_attributes(cleaned_attributes)
+      update_attributes!(cleaned_attributes)
     end
 
     def update_attributes(attributes)

lib/hashie/extensions/ignore_undeclared.rb

@@ -31,12 +31,11 @@ module Extensions
     module IgnoreUndeclared
       def initialize_attributes(attributes)
         return unless attributes
+
         klass = self.class
-        translations = klass.respond_to?(:translations) && klass.translations
-        attributes.each_pair do |att, value|
-          next unless klass.property?(att) || (translations && translations.include?(att))
-          self[att] = value
-        end
+        translations = klass.respond_to?(:translations) && klass.translations || []
+
+        super(attributes.select { |attr, _| klass.property?(attr) || translations.include?(attr) })
       end
 
       def property_exists?(property)

spec/hashie/dash_spec.rb

@@ -606,3 +606,13 @@ class DashWithMethodAccess < Hashie::Dash
     it { is_expected.to eq true }
   end
 end
+
+RSpec.describe Hashie::Dash do
+  let(:test) do
+    Class.new(Hashie::Dash) do
+      property :description, default: ''
+    end
+  end
+
+  include_examples 'Dash default handling', :description
+end

spec/hashie/extensions/ignore_undeclared_spec.rb

@@ -27,6 +27,18 @@ class ForgivingTrash < Hashie::Trash
       hash = subject.new(city: 'Toronto')
       expect { hash.country = 'Canada' }.to raise_error(NoMethodError)
     end
+
+    context 'with a default value' do
+      let(:test) do
+        Class.new(Hashie::Trash) do
+          include Hashie::Extensions::IgnoreUndeclared
+
+          property :description, from: :desc, default: ''
+        end
+      end
+
+      include_examples 'Dash default handling', :description, :desc
+    end
   end
 
   context 'combined with DeepMerge' do

spec/spec_helper.rb

@@ -13,6 +13,8 @@
 require './spec/support/logger'
 require './spec/support/matchers'
 
+Dir[File.expand_path(File.join(__dir__, 'support', '**', '*'))].sort.each { |file| require file }
+
 RSpec.configure do |config|
   config.extend RubyVersionCheck
   config.expect_with :rspec do |expect|

spec/support/shared_examples.rb

@@ -0,0 +1,26 @@
+RSpec.shared_examples 'Dash default handling' do |property, name = property|
+  it 'uses the default when initializing' do
+    expect(test.new(name => nil).public_send(property)).to eq ''
+  end
+
+  it 'allows you to set the value to nil with the hash writer' do
+    trash = test.new(name => 'foo')
+    trash[name] = nil
+
+    expect(trash.public_send(property)).to be_nil
+  end
+
+  it 'allows you to set the value to nil with the method writer' do
+    trash = test.new(name => 'foo')
+    trash[name] = nil
+
+    expect(trash.public_send(property)).to be_nil
+  end
+
+  it 'uses the default when updating with defaults' do
+    trash = test.new(name => 'foo')
+    trash.update_attributes!(name => nil)
+
+    expect(trash.public_send(property)).to eq ''
+  end
+end