diff --git a/CHANGELOG.md b/CHANGELOG.md index abded329..a4b7f233 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -39,6 +39,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 diff --git a/README.md b/README.md index 47608f63..3ad96b12 100644 --- a/README.md +++ b/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: diff --git a/lib/hashie/dash.rb b/lib/hashie/dash.rb index 460790b2..785d94de 100644 --- a/lib/hashie/dash.rb +++ b/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) diff --git a/lib/hashie/extensions/ignore_undeclared.rb b/lib/hashie/extensions/ignore_undeclared.rb index 9b506dd9..64cf0b61 100644 --- a/lib/hashie/extensions/ignore_undeclared.rb +++ b/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) diff --git a/spec/hashie/dash_spec.rb b/spec/hashie/dash_spec.rb index 5f3bfe39..3adfc8f8 100644 --- a/spec/hashie/dash_spec.rb +++ b/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 diff --git a/spec/hashie/extensions/ignore_undeclared_spec.rb b/spec/hashie/extensions/ignore_undeclared_spec.rb index 50c4f31a..6f2d818f 100644 --- a/spec/hashie/extensions/ignore_undeclared_spec.rb +++ b/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 diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb index 0bdf972e..5ac288a7 100644 --- a/spec/spec_helper.rb +++ b/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| diff --git a/spec/support/shared_examples.rb b/spec/support/shared_examples.rb new file mode 100644 index 00000000..df055927 --- /dev/null +++ b/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