RUBY-3664, RUBY-3663 - BSON::Document fix type confusion with Hash #348
Conversation
This file contains hidden or 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
…rent class Hash: 1. #to_h does not convert nested hashes. (See RUBY-3663) 2. When we call many of the Hash methods which it overrides (especially non-bang methods), they return Hash instances rather than BSON::Document. (Examples: .try_convert, #select, #reject, #invert) 3. Some of the bang (!) methods `#transform_keys!` and `#transform_values!` can be used to transform BSON::Documents into a disallowed, state, e.g. with Symbol keys or Hash values (nested values should always be BSON document). 4. `#deep_symbolize_keys` (non-bang) plainly doesn't work; it should return a Hash with deeply symbolized keys, so all nested BSON::Documents need to be converted to Hash. In a similar vein, #symbolize_keys should return a deep-converted Hash, but with only the first-level Hash's keys symbolized. (It does not make sense to mix Hash with nested BSON::Document, you're gonna have a bad time.) 5. For methods that select keys, such as #key?, #delete, #slice etc. should always consistently be calling #convert_key on the key args. Today this is done inconsistently, for example, #key?, #delete, #dig all convert the keys, but #values_at, #fetch_values, #store, #without, #assoc do not. 6. Some needed aliases are missing. Also #value? is incorrectly aliases as #value (without question mark) which doesn't exist on Hash (coding mistake) 7. Minor point, but in the code, the method definiton of #dig is wrapped in instance_methods.include?(:dig). This is no longer needed, since dig was added in Ruby 2.5 and the minimum Ruby version of BSON Ruby is 2.6.
johnnyshields
changed the title
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.
Fixes RUBY-3664
Fixes RUBY-3663
Fix various type-confusion problems between BSON::Document and its parent class Hash:
#to_h does not convert nested hashes. (See RUBY-3663)
When we call many of the Hash methods which it overrides (especially non-bang methods), they return Hash instances rather than BSON::Document. (Examples: .try_convert, #select, #reject, #invert)
Some of the bang (!) methods
#transform_keys!and#transform_values!can be used to transform BSON::Documents into a disallowed, state, e.g. with Symbol keys or Hash values (nested values should always be BSON document).#deep_symbolize_keys(non-bang) plainly doesn't work; it should return a Hash with deeply symbolized keys, so all nested BSON::Documents need to be converted to Hash. In a similar vein, #symbolize_keys should return a deep-converted Hash, but with only the first-level Hash's keys symbolized. (It does not make sense to mix Hash with nested BSON::Document, you're gonna have a bad time.)For methods that select keys, such as #key?, #delete, #slice etc. should always consistently be calling #convert_key on the key args. Today this is done inconsistently, for example, #key?, #delete, #dig all convert the keys, but #values_at, #fetch_values, #store, #without, #assoc do not.
Some needed aliases are missing. Also #value? is incorrectly aliases as #value (without question mark) which doesn't exist on Hash (coding mistake)
Minor point, but in the code, the method definiton of #dig is wrapped in instance_methods.include?(:dig). This is no longer needed, since dig was added in Ruby 2.5 and the minimum Ruby version of BSON Ruby is 2.6.
I've added Ruby 3.4 to Github Actions to make sure it passes as well.