From c071e4f9fdcd41b078e6be405f9cf54480b76c8e Mon Sep 17 00:00:00 2001 From: Michael Herold Date: Wed, 22 Feb 2017 19:37:07 -0600 Subject: [PATCH] Add an extension to maintain original Mash keys (#326) One of the behaviors of Mash that we see regularly surprise users is that Mash stringifies any keys passed into it. This leads to unexpected lack of synergy between Mash and its cousins (particularly Dash), since the property DSLs do not handle indifferent key access. This extension ensures that the original keys are kept inside the Mash's data structure, at the expense of more costly logic for fetching information indifferently. I have included a benchmark that compares the two. The benchmark shows that when you are passing string keys into a Mash, using this extension will actually be _faster_ than the default implementation, but that the reverse is true when passing symbol keys. In #296, I tried to do this universally for all Mashes, which slowed down the fetching behavior for Mash significantly. I like this attempt much better because it allows users to opt into the new behavior if they want it, while still keeping the default implementation as-is. Fixes #196 by giving the option of keeping the original structure of the Mash when using it with Dash. Fixes #246 by giving the option of opting into keeping the original keys. Closes #296 by giving a more flexible path forward that doesn't change the semantics of the main Mash class. --- .rubocop_todo.yml | 8 +-- CHANGELOG.md | 1 + Gemfile | 1 + README.md | 19 +++++++ benchmarks/keep_original_mash_keys.rb | 28 ++++++++++ lib/hashie.rb | 1 + .../extensions/mash/keep_original_keys.rb | 54 +++++++++++++++++++ lib/hashie/mash.rb | 1 + spec/hashie/dash_spec.rb | 26 +++++++++ .../mash/keep_original_keys_spec.rb | 46 ++++++++++++++++ 10 files changed, 181 insertions(+), 4 deletions(-) create mode 100644 benchmarks/keep_original_mash_keys.rb create mode 100644 lib/hashie/extensions/mash/keep_original_keys.rb create mode 100644 spec/hashie/extensions/mash/keep_original_keys_spec.rb diff --git a/.rubocop_todo.yml b/.rubocop_todo.yml index d7d21a5..06bb1e1 100644 --- a/.rubocop_todo.yml +++ b/.rubocop_todo.yml @@ -1,6 +1,6 @@ # This configuration was generated by # `rubocop --auto-gen-config` -# on 2017-02-10 18:48:03 -0600 using RuboCop version 0.34.2. +# on 2017-02-22 17:31:41 -0600 using RuboCop version 0.34.2. # The point is for the user to remove these configuration records # one by one as the offenses are removed from the code base. # Note that changes in the inspected code, or installation of new @@ -18,13 +18,13 @@ Metrics/AbcSize: # Offense count: 2 # Configuration parameters: CountComments. Metrics/ClassLength: - Max: 204 + Max: 205 # Offense count: 8 Metrics/CyclomaticComplexity: Max: 11 -# Offense count: 242 +# Offense count: 246 # Configuration parameters: AllowURI, URISchemes. Metrics/LineLength: Max: 170 @@ -43,7 +43,7 @@ Style/CaseEquality: Exclude: - 'lib/hashie/hash.rb' -# Offense count: 36 +# Offense count: 37 # Configuration parameters: Exclude. Style/Documentation: Enabled: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 926002d..faffb70 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,7 @@ scheme are considered to be bugs. ### Added +* [#326](https://github.com/intridea/hashie/pull/326): Added `Hashie::Extensions::Mash::KeepOriginalKeys` to give Mashes the ability to keep the original structure given to it - [@michaelherold](https://github.com/michaelherold). * Your contribution here. ### Changed diff --git a/Gemfile b/Gemfile index fdca87d..38f3159 100644 --- a/Gemfile +++ b/Gemfile @@ -3,6 +3,7 @@ source 'http://rubygems.org' gemspec group :development do + gem 'benchmark-ips' gem 'pry' gem 'pry-stack_explorer', platforms: [:ruby_19, :ruby_20, :ruby_21] gem 'rubocop', '0.34.2' diff --git a/README.md b/README.md index 32f2925..80d1779 100644 --- a/README.md +++ b/README.md @@ -545,6 +545,25 @@ class Response < Hashie::Mash end ``` +### 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. + +```ruby +class KeepingMash < ::Hashie::Mash + include Hashie::Extensions::Mash::KeepOriginalKeys +end + +mash = KeepingMash.new(:symbol_key => :symbol, 'string_key' => 'string') +mash.to_hash == { :symbol_key => :symbol, 'string_key' => 'string' } #=> true +mash.symbol_key #=> :symbol +mash[:symbol_key] #=> :symbol +mash['symbol_key'] #=> :symbol +mash.string_key #=> 'string' +mash['string_key'] #=> 'string' +mash[:string_key] #=> 'string' +``` + ### Mash Extension: SafeAssignment This extension can be mixed into a Mash to guard the attempted overwriting of methods by property setters. When mixed in, the Mash will raise an `ArgumentError` if you attempt to write a property with the same name as an existing method. diff --git a/benchmarks/keep_original_mash_keys.rb b/benchmarks/keep_original_mash_keys.rb new file mode 100644 index 0000000..3f20eb9 --- /dev/null +++ b/benchmarks/keep_original_mash_keys.rb @@ -0,0 +1,28 @@ +require_relative '../lib/hashie' +require 'benchmark/ips' + +class KeepingMash < Hashie::Mash + include Hashie::Extensions::Mash::KeepOriginalKeys +end + +original = { test: 'value' } +mash = Hashie::Mash.new(original) +keeping_mash = KeepingMash.new(original) + +Benchmark.ips do |x| + x.report('keep symbol') { keeping_mash.test } + x.report('normal symbol') { mash.test } + + x.compare! +end + +original = { 'test' => 'value' } +mash = Hashie::Mash.new(original) +keeping_mash = KeepingMash.new(original) + +Benchmark.ips do |x| + x.report('keep string') { keeping_mash.test } + x.report('normal string') { mash.test } + + x.compare! +end diff --git a/lib/hashie.rb b/lib/hashie.rb index 323f80f..b6e1d13 100644 --- a/lib/hashie.rb +++ b/lib/hashie.rb @@ -44,6 +44,7 @@ module Hashie end module Mash + autoload :KeepOriginalKeys, 'hashie/extensions/mash/keep_original_keys' autoload :SafeAssignment, 'hashie/extensions/mash/safe_assignment' autoload :SymbolizeKeys, 'hashie/extensions/mash/symbolize_keys' end diff --git a/lib/hashie/extensions/mash/keep_original_keys.rb b/lib/hashie/extensions/mash/keep_original_keys.rb new file mode 100644 index 0000000..1b529a7 --- /dev/null +++ b/lib/hashie/extensions/mash/keep_original_keys.rb @@ -0,0 +1,54 @@ +module Hashie + module Extensions + module Mash + # Overrides the indifferent access of a Mash to keep keys in the + # original format given to the Mash. + # + # @example + # class KeepingMash < Hashie::Mash + # include Hashie::Extensions::Mash::KeepOriginalKeys + # end + # + # mash = KeepingMash.new(:symbol_key => :symbol, 'string_key' => 'string') + # mash.to_hash #=> { :symbol_key => :symbol, 'string_key' => 'string' } + # mash['string_key'] == mash[:string_key] #=> true + # mash[:symbol_key] == mash['symbol_key'] #=> true + module KeepOriginalKeys + private + + def self.included(descendant) + unless descendant <= Hashie::Mash + fail ArgumentError, "#{descendant} is not a kind of Hashie::Mash" + end + end + + # Converts the key when necessary to access the correct Mash key. + # + # @param [Object, String, Symbol] key the key to access. + # @return [Object] the value assigned to the key. + def convert_key(key) + if regular_key?(key) + key + elsif (converted_key = __convert(key)) && regular_key?(converted_key) + converted_key + else + key + end + end + + # Converts symbol/string keys to their alternative formats, but leaves + # other keys alone. + # + # @param [Object, String, Symbol] key the key to convert. + # @return [Object, String, Symbol] the converted key. + def __convert(key) + case key + when Symbol then key.to_s + when String then key.to_sym + else key + end + end + end + end + end +end diff --git a/lib/hashie/mash.rb b/lib/hashie/mash.rb index b45ccce..cc8cb20 100644 --- a/lib/hashie/mash.rb +++ b/lib/hashie/mash.rb @@ -189,6 +189,7 @@ module Hashie self.class.new(self, default) end + alias_method :regular_key?, :key? def key?(key) super(convert_key(key)) end diff --git a/spec/hashie/dash_spec.rb b/spec/hashie/dash_spec.rb index c6e6753..87e5bc3 100644 --- a/spec/hashie/dash_spec.rb +++ b/spec/hashie/dash_spec.rb @@ -192,6 +192,32 @@ describe DashTest do end end + context 'converting from a Mash' do + class ConvertingFromMash < Hashie::Dash + property :property, required: true + end + + context 'without keeping the original keys' do + let(:mash) { Hashie::Mash.new(property: 'test') } + + it 'does not pick up the property from the stringified key' do + expect { ConvertingFromMash.new(mash) }.to raise_error(NoMethodError) + end + end + + context 'when keeping the original keys' do + class KeepingMash < Hashie::Mash + include Hashie::Extensions::Mash::KeepOriginalKeys + end + + let(:mash) { KeepingMash.new(property: 'test') } + + it 'picks up the property from the original key' do + expect { ConvertingFromMash.new(mash) }.not_to raise_error + end + end + end + describe '#new' do it 'fails with non-existent properties' do expect { described_class.new(bork: '') }.to raise_error(*no_property_error('bork')) diff --git a/spec/hashie/extensions/mash/keep_original_keys_spec.rb b/spec/hashie/extensions/mash/keep_original_keys_spec.rb new file mode 100644 index 0000000..6e3aa06 --- /dev/null +++ b/spec/hashie/extensions/mash/keep_original_keys_spec.rb @@ -0,0 +1,46 @@ +require 'spec_helper' + +RSpec.describe Hashie::Extensions::Mash::KeepOriginalKeys do + let(:keeping_mash) do + Class.new(Hashie::Mash) do + include Hashie::Extensions::Mash::KeepOriginalKeys + end + end + + it 'keeps the keys in the resulting hash identical to the original' do + original = { :a => 'apple', 'b' => 'bottle' } + mash = keeping_mash.new(original) + + expect(mash.to_hash).to eq(original) + end + + it 'indifferently responds to keys' do + original = { :a => 'apple', 'b' => 'bottle' } + mash = keeping_mash.new(original) + + expect(mash['a']).to eq(mash[:a]) + expect(mash['b']).to eq(mash[:b]) + end + + it 'responds to all method accessors like a Mash' do + original = { :a => 'apple', 'b' => 'bottle' } + mash = keeping_mash.new(original) + + expect(mash.a).to eq('apple') + expect(mash.a?).to eq(true) + expect(mash.b).to eq('bottle') + expect(mash.b?).to eq(true) + expect(mash.underbang_).to be_a(keeping_mash) + expect(mash.bang!).to be_a(keeping_mash) + expect(mash.predicate?).to eq(false) + end + + it 'keeps the keys that are directly passed without converting them' do + original = { :a => 'apple', 'b' => 'bottle' } + mash = keeping_mash.new(original) + + mash[:c] = 'cat' + mash['d'] = 'dog' + expect(mash.to_hash).to eq(:a => 'apple', 'b' => 'bottle', :c => 'cat', 'd' => 'dog') + end +end