From ac2444c3c9d020b2d0ef1b011b5bdb37d2b4af62 Mon Sep 17 00:00:00 2001 From: Michael Herold Date: Thu, 22 Oct 2020 20:51:14 -0500 Subject: Fix an imprecise spec for `Dash#replace` The check here doesn't make sense because it's testing the behavior of another method. The best way to check identity is but checking the `#object_id` of an object - thus, this check actually does what the test says it does. --- spec/hashie/dash_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/hashie/dash_spec.rb b/spec/hashie/dash_spec.rb index 3adfc8f..c64e400 100644 --- a/spec/hashie/dash_spec.rb +++ b/spec/hashie/dash_spec.rb @@ -344,7 +344,7 @@ describe DashTest do before { subject.replace(first_name: 'Cain') } it 'return self' do - expect(subject.replace(email: 'bar').to_hash).to eq(email: 'bar', count: 0) + expect(subject.replace(email: 'bar').object_id).to eq subject.object_id end it 'sets all specified keys to their corresponding values' do -- cgit v1.2.1 From b9a93916f538bfab57815a1d48740c826715ca5d Mon Sep 17 00:00:00 2001 From: Michael Herold Date: Thu, 22 Oct 2020 21:00:11 -0500 Subject: Expose `IndifferentAccess` conversion internally In some cases --- like writing meta-extensions of `IndifferentAccess` --- we need access to the ability to convert a key for the purposes of `IndifferentAccess`. Exposing this behavior as a module function makes this possible. --- lib/hashie/extensions/indifferent_access.rb | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/lib/hashie/extensions/indifferent_access.rb b/lib/hashie/extensions/indifferent_access.rb index d6b9da3..2ff5f28 100644 --- a/lib/hashie/extensions/indifferent_access.rb +++ b/lib/hashie/extensions/indifferent_access.rb @@ -25,6 +25,11 @@ module Hashie module IndifferentAccess include Hashie::Extensions::RubyVersionCheck + # @api private + def self.convert_key(key) + key.to_s + end + def self.included(base) Hashie::Extensions::Dash::IndifferentAccess.maybe_extend(base) @@ -68,7 +73,7 @@ module Hashie end def convert_key(key) - key.to_s + IndifferentAccess.convert_key(key) end # Iterates through the keys and values, reconverting them to -- cgit v1.2.1 From 98df995e2f0433d3d1abe46efd732bc8e20e31cc Mon Sep 17 00:00:00 2001 From: Michael Herold Date: Thu, 22 Oct 2020 21:07:14 -0500 Subject: Ensure all properties are set on exported Dash When exporting a Dash via `#to_h` or `#to_hash`, we expect all properties to be exported whether or not they are set. However, in the change that allows codependent properties to be nilified, we regressed the behavior of exporting all properties. There is a gotcha here, which I note in the tests for the specs. For posterity, MRI does not send the `#to_hash` method to anything that subclasses `Hash` when you double-splat it. Thus, we cannot override the behavior within MRI. For more information, see [this comment][1] where I detail the behavior of double-splat within MRI. Currently, JRuby also follows this behavior, but it's not guaranteed that other Rubies will. [1]: https://github.com/hashie/hashie/issues/353#issuecomment-363294886 --- CHANGELOG.md | 1 + lib/hashie/dash.rb | 7 +++ lib/hashie/extensions/dash/indifferent_access.rb | 9 +++ spec/hashie/dash_spec.rb | 76 ++++++++++++++++++++++++ 4 files changed, 93 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0db8b66..c1bc0de 100644 --- a/CHANGELOG.md +++ b/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). +* [#535](https://github.com/hashie/hashie/pull/535): Restored the exporting of all properties as part of `Dash#to_h` and `Dash#to_hash` - [@michaelherold](https://github.com/michaelherold). * [#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. diff --git a/lib/hashie/dash.rb b/lib/hashie/dash.rb index 785d94d..cfb151e 100644 --- a/lib/hashie/dash.rb +++ b/lib/hashie/dash.rb @@ -156,6 +156,13 @@ module Hashie self end + def to_h + defaults = ::Hash[self.class.properties.map { |prop| [prop, self.class.defaults[prop]] }] + + defaults.merge(self) + end + alias to_hash to_h + def update_attributes!(attributes) update_attributes(attributes) diff --git a/lib/hashie/extensions/dash/indifferent_access.rb b/lib/hashie/extensions/dash/indifferent_access.rb index 204cae6..50d4f93 100644 --- a/lib/hashie/extensions/dash/indifferent_access.rb +++ b/lib/hashie/extensions/dash/indifferent_access.rb @@ -19,6 +19,15 @@ module Hashie end private_class_method :requires_class_methods? + def to_h + defaults = ::Hash[self.class.properties.map do |prop| + [Hashie::Extensions::IndifferentAccess.convert_key(prop), self.class.defaults[prop]] + end] + + defaults.merge(self) + end + alias to_hash to_h + module ClassMethods # Check to see if the specified property has already been # defined. diff --git a/spec/hashie/dash_spec.rb b/spec/hashie/dash_spec.rb index c64e400..e75ad1a 100644 --- a/spec/hashie/dash_spec.rb +++ b/spec/hashie/dash_spec.rb @@ -416,6 +416,7 @@ describe DashTest do Class.new(Hashie::Dash) do property :a, required: -> { b.nil? }, message: 'is required if b is not set.' property :b, required: -> { a.nil? }, message: 'is required if a is not set.' + property :c, default: -> { 'c' } end end @@ -434,6 +435,44 @@ describe DashTest do it 'raises an error when neither property is set' do expect { codependent.new(a: nil, b: nil) }.to raise_error(ArgumentError) end + + context 'exporting nil values' do + describe '#to_h' do + it 'does not prune nil values' do + expect(codependent.new(a: 'hi', b: nil).to_h).to eq(a: 'hi', b: nil, c: 'c') + expect(codependent.new(a: 'hi', b: nil, c: nil).to_hash).to eq(a: 'hi', b: nil, c: 'c') + expect(codependent.new(a: 'hi', b: nil).merge(c: nil).to_h).to( + eq(a: 'hi', b: nil, c: nil) + ) + end + end + + describe '#to_hash' do + it 'does not prune nil values' do + expect(codependent.new(a: 'hi', b: nil).to_hash).to eq(a: 'hi', b: nil, c: 'c') + expect(codependent.new(a: 'hi', b: nil, c: nil).to_hash).to eq(a: 'hi', b: nil, c: 'c') + expect(codependent.new(a: 'hi', b: nil).merge(c: nil).to_hash).to( + eq(a: 'hi', b: nil, c: nil) + ) + end + end + + describe '**' do + # Note: This test is an implementation detail of MRI and may not hold for + # other Ruby interpreters. But it's important to note in the test suite + # because it can be surprising for people unfamiliar with the semantics of + # double-splatting. + # + # For more information, see [this link][1]: + # + # [1]: https://github.com/hashie/hashie/issues/353#issuecomment-363294886 + it 'prunes nil values because they are not set in the dash' do + dash = codependent.new(a: 'hi', b: nil) + + expect(**dash).to eq(a: 'hi', c: 'c') + end + end + end end end end @@ -482,6 +521,43 @@ describe Hashie::Dash, 'inheritance' do expect(@bottom.new).to have_key(:echo) expect(@bottom.new).to_not have_key('echo') end + + context 'exporting nil values' do + let(:test) do + Class.new(Hashie::Dash) do + property :foo + property :bar + end + end + + describe '#to_h' do + it 'does not prune nil values' do + expect(test.new(foo: 'hi', bar: nil).to_h).to eq(foo: 'hi', bar: nil) + end + end + + describe '#to_hash' do + it 'does not prune nil values' do + expect(test.new(foo: 'hi', bar: nil).to_hash).to eq(foo: 'hi', bar: nil) + end + end + + describe '**' do + # Note: This test is an implementation detail of MRI and may not hold for + # other Ruby interpreters. But it's important to note in the test suite + # because it can be surprising for people unfamiliar with the semantics of + # double-splatting. + # + # For more information, see [this link][1]: + # + # [1]: https://github.com/hashie/hashie/issues/353#issuecomment-363294886 + it 'prunes nil values because they are not set in the dash' do + dash = test.new(foo: 'hi', bar: nil) + + expect(**dash).to eq(foo: 'hi') + end + end + end end describe SubclassedTest do -- cgit v1.2.1