Prevent warnings and errors when setting Mash keys more than once #415
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
When we switched to using `#respond_to?` to detecting whether to log a Mash collision, we started reporting when we were overwriting keys that already exist in the Mash. This is a poor experience because it causes extra warnings (as in hashie#414) or, in the worst case, causes an "undefined method" error (as in hashie#413). This change fixes that problem and benchmarks to ensure we're not appreciably regressing performance. The results of two benchmarks are below: ``` bundle exec ruby benchmark/mash_method_access.rb: Warming up -------------------------------------- before 92.456k i/100ms Calculating ------------------------------------- before 1.290M (± 4.4%) i/s - 6.472M in 5.028183s Pausing here -- run Ruby again to measure the next benchmark... Warming up -------------------------------------- after 92.941k i/100ms Calculating ------------------------------------- after 1.326M (± 5.4%) i/s - 6.692M in 5.060756s Comparison: after: 1326239.2 i/s before: 1289624.0 i/s - same-ish: difference falls within error ``` and ``` within spec/integrations/omniauth, bundle exec rake perf:ips Warming up -------------------------------------- before 1.260k i/100ms Calculating ------------------------------------- before 13.114k (± 4.2%) i/s - 66.780k in 5.101689s Pausing here -- run Ruby again to measure the next benchmark... Warming up -------------------------------------- after 1.299k i/100ms Calculating ------------------------------------- after 13.149k (± 4.0%) i/s - 66.249k in 5.046630s Comparison: after: 13148.9 i/s before: 13113.8 i/s - same-ish: difference falls within error ``` Closes hashie#413 Closes hashie#414
michaelherold
force-pushed
the
fix-double-set
branch
from
2088079
to
55cfb8a
Compare
|
Yes, this fixes #414. Thanks. |
michaelherold
changed the title
|
You're my hero @michaelherold! |
|
Thanks for the fix @michaelherold! 💙❤️💛💜💚💙❤️💛💜💚 |
|
Awesome, I'm glad it works for both of you. I'll get a new version out today! 🏆 |
jsonn
pushed a commit
to jsonn/pkgsrc
that referenced
this pull request
Mar 20, 2017
# Change Log ## [3.5.5] - 2017-02-24 [3.5.5]: hashie/hashie@v3.5.4...v3.5.5 ### Added * [#326](hashie/hashie#326): Added `Hashie::Extensions::Mash::KeepOriginalKeys` to give Mashes the ability to keep the original structure given to it - [@michaelherold](https://github.com/michaelherold). ### Fixed * [#415](hashie/hashie#415): Fixed Mash logging keys multiple times which lead to a bad user experience or, in some cases, errors - [@michaelherold](https://github.com/michaelherold). ## [3.5.4] - 2017-02-22 [3.5.4]: hashie/hashie@v3.5.3...v3.5.4 ### Added * [#412](hashie/hashie#412): Added a Hashie::Extensions::Mash::SymbolizeKeys extension that overrides the default stringification behavior for keys - [@michaelherold](https://github.com/michaelherold). ### Fixed * [#409](hashie/hashie#409): Fixed Railtie detection for projects where Rails is defined but Railties are not availble - [@CallumD](https://github.com/callumd). * [#411](hashie/hashie#411): Fixed a performance regression from 3.4.3 that caused a 10x slowdown in OmniAuth - [@michaelherold](https://github.com/michaelherold). ## [3.5.3] - 2017-02-11 [3.5.3]: hashie/hashie@v3.5.2...v3.5.3 ### Fixed * [#402](hashie/hashie#402): Use a Railtie to set Hashie.logger on rails boot - [@matthewrudy](https://github.com/matthewrudy). * [#406](hashie/hashie#406): Ensure that subclasses that disable warnings propagate that setting to grandchild classes - [@michaelherold](https://github.com/michaelherold). * Your contribution here. ## [3.5.2] - 2017-02-10 [3.5.2]: hashie/hashie@v3.5.1...v3.5.2 ### Added * [#395](hashie/hashie#395): Add the ability to disable warnings in Mash subclasses - [@michaelherold](https://github.com/michaelherold). * [#400](hashie/hashie#400): Fix Hashie.logger load and set the Hashie logger to the Rails logger in a Rails environment - [@michaelherold](https://github.com/michaelherold). ### Fixed * [#396](hashie/hashie#396): Fix for specs in #381: Incorrect use of shared context meant example was not being run - [@biinari](https://github.com/biinari). * [#399](hashie/hashie#399): Fix passing Pathname object to Hashie::Mesh.load() - [@albb0920](https://github.com/albb0920). ### Miscellanous * [#397](hashie/hashie#397): Add the integration specs harness into the main test tasks - [@michaelherold](https://github.com/michaelherold). ## [3.5.1] - 2017-01-31 * [#392](hashie/hashie#392): Fix for #391: Require all dependencies of Hashie::Mash - [@dblock](https://github.com/dblock). [3.5.1]: hashie/hashie@v3.5.0...v3.5.1 ## [3.5.0] - 2017-01-31 * [#386](hashie/hashie#386): Fix for #385: Make `deep_merge` always `deep_dup` nested hashes before merging them in so that there are no shared references between the two hashes being merged. - [@mltsy](https://github.com/mltsy). * [#389](hashie/hashie#389): Support Ruby 2.4.0 - [@camelmasa](https://github.com/camelmasa). [3.5.0]: hashie/hashie@v3.4.6...v3.5.0 ### Added * [#381](hashie/hashie#381): Add a logging layer that lets us report potential issues to our users. As the first logged issue, report when a `Hashie::Mash` is attempting to overwrite a built-in method, since that is one of our number one questions - [@michaelherold](https://github.com/michaelherold). ### Changed * [#384](hashie/hashie#384): Updated to CodeClimate 1.x - [@boffbowsh](https://github.com/boffbowsh). ### Fixed * [#369](hashie/hashie#369): If a translation for a property exists when using IndifferentAccess and IgnoreUndeclared, use the translation to find the property - [@whitethunder](https://github.com/whitethunder). * [#376](hashie/hashie#376): Leave string index unchanged if it can't be converted to integer for Array#dig - [@sazor](https://github.com/sazor). * [#377](hashie/hashie#377): Dont use Rubygems to check ruby version - [@sazor](https://github.com/sazor). * [#378](hashie/hashie#378): Deep find all searches inside all nested hashes - [@sazor](https://github.com/sazor). * [#380](hashie/hashie#380): Evaluate procs default values of Dash in object initialization - [@sazor](https://github.com/sazor). ### Miscellanous * [#387](hashie/hashie#387): Fix builds failing due to Rake 11 having a breaking change - [@michaelherold](https://github.com/michaelherold).
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
When we switched to using
#respond_to?to detecting whether to loga Mash collision, we started reporting when we were overwriting keys
that already exist in the Mash. This is a poor experience because it
causes extra warnings (as in #414) or, in the worst case, causes an
"undefined method" error (as in #413).
This change fixes that problem and benchmarks to ensure we're not
appreciably regressing performance. The results of two benchmarks are
below:
and
Closes #413
Closes #414