From 0bca9255a6234606cbdd98880ccc241f192cfdae Mon Sep 17 00:00:00 2001 From: Caroline Artz Date: Mon, 4 May 2020 20:15:32 -0500 Subject: [PATCH] Changes to `Mash` initialization key string conversion. (#521) --- CHANGELOG.md | 2 + README.md | 23 ++++++++--- UPGRADING.md | 38 +++++++++++++++++++ lib/hashie/extensions/mash/symbolize_keys.rb | 10 ++--- lib/hashie/extensions/symbolize_keys.rb | 13 ++++++- lib/hashie/hash.rb | 4 +- lib/hashie/mash.rb | 19 +++++----- lib/hashie/version.rb | 2 +- .../extensions/mash/symbolize_keys_spec.rb | 24 ++++++++++-- spec/hashie/extensions/symbolize_keys_spec.rb | 5 ++- spec/hashie/hash_spec.rb | 6 +-- spec/hashie/mash_spec.rb | 22 ++++++----- 12 files changed, 127 insertions(+), 41 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index aecc4ac..616726c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,7 @@ scheme are considered to be bugs. ### Changed +* [#521](https://github.com/hashie/hashie/pull/499): Do not convert keys that cannot be represented as symbols to `String` in `Mash` initialization - [@carolineartz](https://github.com/carolineartz). * Your contribution here. ### Deprecated @@ -28,6 +29,7 @@ scheme are considered to be bugs. ### Fixed +* [#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). * Your contribution here. ### Security diff --git a/README.md b/README.md index 7d0f8ca..d417f5d 100644 --- a/README.md +++ b/README.md @@ -191,13 +191,13 @@ end ### KeyConversion -The KeyConversion extension gives you the convenience methods of `symbolize_keys` and `stringify_keys` along with their bang counterparts. You can also include just stringify or just symbolize with `Hashie::Extensions::StringifyKeys` or [`Hashie::Extensions::SymbolizeKeys`](#mash-extension-symbolizekeys). +The KeyConversion extension gives you the convenience methods of `symbolize_keys` and `stringify_keys` along with their bang counterparts. You can also include just stringify or just symbolize with `Hashie::Extensions::StringifyKeys` or `Hashie::Extensions::SymbolizeKeys`. Hashie also has a utility method for converting keys on a Hash without a mixin: ```ruby -Hashie.symbolize_keys! hash # => Symbolizes keys of hash. -Hashie.symbolize_keys hash # => Returns a copy of hash with keys symbolized. +Hashie.symbolize_keys! hash # => Symbolizes all string keys of hash. +Hashie.symbolize_keys hash # => Returns a copy of hash with string keys symbolized. Hashie.stringify_keys! hash # => Stringifies keys of hash. Hashie.stringify_keys hash # => Returns a copy of hash with keys stringified. ``` @@ -580,6 +580,19 @@ my_gem = MyGem.new(name: "Hashie", dependencies: { rake: "< 11", rspec: "~> 3.0" my_gem.dependencies.class #=> MyGem ``` +### How does Mash handle key types which cannot be symbolized? + +Mash preserves keys which cannot be converted *directly* to both a string and a symbol, such as numeric keys. Since Mash is conceived to provide psuedo-object functionality, handling keys which cannot represent a method call falls outside its scope of value. + +#### Example + +```ruby +Hashie::Mash.new('1' => 'one string', :'1' => 'one sym', 1 => 'one num') +# => {"1"=>"one sym", 1=>"one num"} +``` + +The symbol key `:'1'` is converted the string `'1'` to support indifferent access and consequently its value `'one sym'` will override the previously set `'one string'`. However, the subsequent key of `1` cannot directly convert to a symbol and therefore **not** converted to the string `'1'` that would otherwise override the previously set value of `'one sym'`. + ### What else can Mash do? Mash allows you also to transform any files into a Mash objects. @@ -642,7 +655,7 @@ Mash.load('data/user.csv', permitted_classes: [Symbol], permitted_symbols: [], a ### Mash Extension: KeepOriginalKeys -This extension can be mixed into a Mash to keep the form of any keys passed directly into the Mash. By default, Mash converts keys to strings to give indifferent access. This extension still allows indifferent access, but keeps the form of the keys to eliminate confusion when you're not expecting the keys to change. +This extension can be mixed into a Mash to keep the form of any keys passed directly into the Mash. By default, Mash converts symbol keys to strings to give indifferent access. This extension still allows indifferent access, but keeps the form of the keys to eliminate confusion when you're not expecting the keys to change. ```ruby class KeepingMash < ::Hashie::Mash @@ -701,7 +714,7 @@ safe_mash[:zip] = 'test' # => still ArgumentError ### Mash Extension: SymbolizeKeys -This extension can be mixed into a Mash to change the default behavior of converting keys to strings. After mixing this extension into a Mash, the Mash will convert all keys to symbols. It can be useful to use with keywords argument, which required symbol keys. +This extension can be mixed into a Mash to change the default behavior of converting keys to strings. After mixing this extension into a Mash, the Mash will convert all string keys to symbols. It can be useful to use with keywords argument, which required symbol keys. ```ruby class SymbolizedMash < ::Hashie::Mash diff --git a/UPGRADING.md b/UPGRADING.md index d516c97..b044cf1 100644 --- a/UPGRADING.md +++ b/UPGRADING.md @@ -1,6 +1,44 @@ Upgrading Hashie ================ +### Upgrading to 5.0.0 + +#### Mash initialization key conversion + +Mash initialization now only converts to string keys which can be represented as symbols. + +```ruby +Hashie::Mash.new( + {foo: "bar"} => "baz", + "1" => "one string", + :"1" => "one sym", + 1 => "one num" +) + +# Before +{"{:foo=>\"bar\"}"=>"baz", "1"=>"one num"} + +# After +{{:foo=>"bar"}=>"baz", "1"=>"one sym", 1=>"one num"} +``` + +#### Mash#dig with numeric keys + +`Hashie::Mash#dig` no longer considers numeric keys for indifferent access. + +```ruby +my_mash = Hashie::Mash.new("1" => "a") # => {"1"=>"a"} + +my_mash.dig("1") # => "a" +my_mash.dig(:"1") # => "a" + +# Before +my_mash.dig(1) # => "a" + +# After +my_mash.dig(1) # => nil +``` + ### Upgrading to 4.0.0 #### Non-destructive Hash methods called on Mash diff --git a/lib/hashie/extensions/mash/symbolize_keys.rb b/lib/hashie/extensions/mash/symbolize_keys.rb index 91a6b14..c0f2a4d 100644 --- a/lib/hashie/extensions/mash/symbolize_keys.rb +++ b/lib/hashie/extensions/mash/symbolize_keys.rb @@ -5,7 +5,7 @@ module Hashie # # @example # class LazyResponse < Hashie::Mash - # include Hashie::Extensions::Mash::SymbolizedKeys + # include Hashie::Extensions::Mash::SymbolizeKeys # end # # response = LazyResponse.new("id" => 123, "name" => "Rey").to_h @@ -24,13 +24,13 @@ module Hashie private - # Converts a key to a symbol + # Converts a key to a symbol, if possible # # @api private - # @param [String, Symbol] key the key to convert to a symbol - # @return [void] + # @param [] key the key to attempt convert to a symbol + # @return [Symbol, K] def convert_key(key) - key.to_sym + key.respond_to?(:to_sym) ? key.to_sym : key end end end diff --git a/lib/hashie/extensions/symbolize_keys.rb b/lib/hashie/extensions/symbolize_keys.rb index e4315bd..d1da9cb 100644 --- a/lib/hashie/extensions/symbolize_keys.rb +++ b/lib/hashie/extensions/symbolize_keys.rb @@ -46,7 +46,7 @@ module Hashie hash.extend(Hashie::Extensions::SymbolizeKeys) unless hash.respond_to?(:symbolize_keys!) hash.keys.each do |k| # rubocop:disable Performance/HashEachMethods symbolize_keys_recursively!(hash[k]) - hash[k.to_sym] = hash.delete(k) + hash[convert_key(k)] = hash.delete(k) end hash end @@ -61,6 +61,17 @@ module Hashie symbolize_keys!(new_hash) end end + + private + + # Converts a key to a symbol, if possible + # + # @api private + # @param [] key the key to attempt convert to a symbol + # @return [Symbol, K] + def convert_key(key) + key.respond_to?(:to_sym) ? key.to_sym : key + end end class << self diff --git a/lib/hashie/hash.rb b/lib/hashie/hash.rb index 7a97ef6..a2bd7ab 100644 --- a/lib/hashie/hash.rb +++ b/lib/hashie/hash.rb @@ -21,8 +21,8 @@ module Hashie assignment_key = if options[:stringify_keys] k.to_s - elsif options[:symbolize_keys] - k.to_s.to_sym + elsif options[:symbolize_keys] && k.respond_to?(:to_sym) + k.to_sym else k end diff --git a/lib/hashie/mash.rb b/lib/hashie/mash.rb index 96b24c9..6ab6e9a 100644 --- a/lib/hashie/mash.rb +++ b/lib/hashie/mash.rb @@ -121,8 +121,8 @@ module Hashie alias regular_reader [] alias regular_writer []= - # Retrieves an attribute set in the Mash. Will convert - # any key passed in to a string before retrieving. + # Retrieves an attribute set in the Mash. Will convert a key passed in + # as a symbol to a string before retrieving. def custom_reader(key) default_proc.call(self, key) if default_proc && !key?(key) value = regular_reader(convert_key(key)) @@ -130,14 +130,12 @@ module Hashie value end - # Sets an attribute in the Mash. Key will be converted to - # a string before it is set, and Hashes will be converted - # into Mashes for nesting purposes. + # Sets an attribute in the Mash. Symbol keys will be converted to + # strings before being set, and Hashes will be converted into Mashes + # for nesting purposes. def custom_writer(key, value, convert = true) #:nodoc: - key_as_symbol = (key = convert_key(key)).to_sym - - log_built_in_message(key_as_symbol) if log_collision?(key_as_symbol) - regular_writer(key, convert ? convert_value(value) : value) + log_built_in_message(key) if key.respond_to?(:to_sym) && log_collision?(key.to_sym) + regular_writer(convert_key(key), convert ? convert_value(value) : value) end alias [] custom_reader @@ -371,7 +369,7 @@ module Hashie end def convert_key(key) #:nodoc: - key.to_s + key.respond_to?(:to_sym) ? key.to_s : key end def convert_value(val, duping = false) #:nodoc: @@ -406,6 +404,7 @@ module Hashie end def log_collision?(method_key) + return unless method_key.is_a?(String) || method_key.is_a?(Symbol) return unless respond_to?(method_key) _, suffix = method_name_and_suffix(method_key) diff --git a/lib/hashie/version.rb b/lib/hashie/version.rb index b97bf9b..e4ffc63 100644 --- a/lib/hashie/version.rb +++ b/lib/hashie/version.rb @@ -1,3 +1,3 @@ module Hashie - VERSION = '4.1.0'.freeze + VERSION = '5.0.0'.freeze end diff --git a/spec/hashie/extensions/mash/symbolize_keys_spec.rb b/spec/hashie/extensions/mash/symbolize_keys_spec.rb index 9846737..bf58ea1 100644 --- a/spec/hashie/extensions/mash/symbolize_keys_spec.rb +++ b/spec/hashie/extensions/mash/symbolize_keys_spec.rb @@ -9,12 +9,30 @@ RSpec.describe Hashie::Extensions::Mash::SymbolizeKeys do end.to raise_error(ArgumentError) end - it 'symbolizes all keys in the Mash' do - my_mash = Class.new(Hashie::Mash) do + context 'when included in a Mash' do + class SymbolizedMash < Hashie::Mash include Hashie::Extensions::Mash::SymbolizeKeys end - expect(my_mash.new('test' => 'value').to_h).to eq(test: 'value') + it 'symbolizes string keys in the Mash' do + my_mash = SymbolizedMash.new('test' => 'value') + expect(my_mash.to_h).to eq(test: 'value') + end + + it 'preserves keys which cannot be symbolized' do + my_mash = SymbolizedMash.new( + '1' => 'symbolizable one', + 1 => 'one', + [1, 2, 3] => 'testing', + { 'test' => 'value' } => 'value' + ) + expect(my_mash.to_h).to eq( + :'1' => 'symbolizable one', + 1 => 'one', + [1, 2, 3] => 'testing', + { 'test' => 'value' } => 'value' + ) + end end context 'implicit to_hash on double splat' do diff --git a/spec/hashie/extensions/symbolize_keys_spec.rb b/spec/hashie/extensions/symbolize_keys_spec.rb index be345ea..34ac8d8 100644 --- a/spec/hashie/extensions/symbolize_keys_spec.rb +++ b/spec/hashie/extensions/symbolize_keys_spec.rb @@ -89,9 +89,10 @@ describe Hashie::Extensions::SymbolizeKeys do context 'singleton methods' do subject { Hash } let(:object) do - subject.new.merge('a' => 1, 'b' => { 'c' => 2 }).extend(Hashie::Extensions::SymbolizeKeys) + subject.new.merge('a' => 1, 'b' => { 'c' => 2 }, 1 => 'numeric key') + .extend(Hashie::Extensions::SymbolizeKeys) end - let(:expected_hash) { { a: 1, b: { c: 2 } } } + let(:expected_hash) { { a: 1, b: { c: 2 }, 1 => 'numeric key' } } describe '.symbolize_keys' do it 'does not raise error' do diff --git a/spec/hashie/hash_spec.rb b/spec/hashie/hash_spec.rb index 0282258..9e2b4e0 100644 --- a/spec/hashie/hash_spec.rb +++ b/spec/hashie/hash_spec.rb @@ -41,7 +41,7 @@ describe Hash do it '#to_hash with symbolize_keys set to true returns a hash with symbolized keys' do hash = Hashie::Hash['a' => 'hey', 123 => 'bob', 'array' => [1, 2, 3]] symbolized_hash = hash.to_hash(symbolize_keys: true) - expect(symbolized_hash).to eq(a: 'hey', :"123" => 'bob', array: [1, 2, 3]) + expect(symbolized_hash).to eq(a: 'hey', 123 => 'bob', array: [1, 2, 3]) end it "#to_hash should not blow up when #to_hash doesn't accept arguments" do @@ -112,9 +112,9 @@ describe Hash do expected = { a: 'hey', - :"123" => 'bob', + 123 => 'bob', array: [1, 2, 3], - subhash: { a: 'hey', b: 'bar', :'123' => 'bob', array: [1, 2, 3] } + subhash: { a: 'hey', b: 'bar', 123 => 'bob', array: [1, 2, 3] } } expect(symbolized_hash).to eq(expected) diff --git a/spec/hashie/mash_spec.rb b/spec/hashie/mash_spec.rb index b0aef57..4731985 100644 --- a/spec/hashie/mash_spec.rb +++ b/spec/hashie/mash_spec.rb @@ -613,6 +613,18 @@ describe Hashie::Mash do expect(converted.to_hash['a'].first.is_a?(Hash)).to be_truthy expect(converted.to_hash['a'].first['c'].first.is_a?(Hashie::Mash)).to be_falsy end + + it 'only stringifies keys which can be converted to symbols' do + initial_hash = { 1 => 'a', ['b'] => 2, 'c' => 3, d: 4 } + converted = Hashie::Mash.new(initial_hash) + expect(converted).to eq(1 => 'a', ['b'] => 2, 'c' => 3, 'd' => 4) + end + + it 'preserves keys which cannot be converted to symbols' do + initial_hash = { 1 => 'a', '1' => 'b', :'1' => 'c' } + converted = Hashie::Mash.new(initial_hash) + expect(converted).to eq(1 => 'a', '1' => 'c') + end end describe '#fetch' do @@ -900,7 +912,7 @@ describe Hashie::Mash do end it 'returns a mash with the keys and values inverted' do - expect(mash.invert).to eq('apple' => 'a', '4' => 'b') + expect(mash.invert).to eq('apple' => 'a', 4 => 'b') end context 'when using with subclass' do @@ -977,14 +989,6 @@ describe Hashie::Mash do expect(subject.dig('a', 'b')).to eq(1) end - context 'with numeric key' do - subject { described_class.new('1' => { b: 1 }) } - it 'accepts a numeric value as key' do - expect(subject.dig(1, :b)).to eq(1) - expect(subject.dig('1', :b)).to eq(1) - end - end - context 'when the Mash wraps a Hashie::Array' do it 'handles digging into an array' do mash = described_class.new(alphabet: { first_three: Hashie::Array['a', 'b', 'c'] })