Skip to content

Conversation

@JamesChevalier
Copy link

@JamesChevalier JamesChevalier commented Aug 23, 2022

Closes #271 by reverting the transform_keys change

omniauth ultimately defines options as a Hashie::Mash not a Hash:
https://github.com/omniauth/omniauth/blob/master/lib/omniauth/strategy.rb#L547
https://github.com/omniauth/omniauth/blob/master/lib/omniauth/key_store.rb#L5

While Hashie::Mash does implement transform_keys, it does not behave the same way as the Hash implementation does:

Hashie::Mash.new(one: "1", two: "2").transform_keys(&:to_sym)
{"one"=>"1", "two"=>"2"}

Hashie::Mash.new(one: "1", two: "2").to_hash.transform_keys(&:to_sym)
=> {:one=>"1", :two=>"2"}

🤔 An alternative could be to rewrite the existing line to:

@options = @@default_options.merge(options.to_hash.transform_keys(&:to_sym)) 
Never mind this idea 😬 Ah! Another option is to add `.with_indifferent_access` when `@@default_options` is created: https://github.com/oauth-xx/oauth-ruby/blob/62d3f3e52daa565a04e508df915456a21a7faf37/lib/oauth/consumer.rb#L33-L73
irb(main):001:0> hh = {one: '1', two: '2'}
=> {:one=>"1", :two=>"2"}
irb(main):002:0> hm = Hashie::Mash.new(one: "one", two: "two")
=> {"one"=>"1", "two"=>"2"}
irb(main):003:0> mm = hh.merge(hm)
=> {:one=>"1", :two=>"2", "one"=>"one", "two"=>"two"}
irb(main):004:0> hh = {one: '1', two: '2'}.with_indifferent_access
=> {"one"=>"1", "two"=>"2"}
irb(main):005:0> mm = hh.merge(hm)
=> {"one"=>"one", "two"=>"two"}
irb(main):006:0> mm[:one]
=> "one"

I don't expect the removal of Hashie from omniauth to be possible, based on how purposeful they were in switching to it:
https://github.com/omniauth/omniauth/wiki/OmniAuth-1.0#changes-to-the-auth-hash

@JamesChevalier
Copy link
Author

JamesChevalier commented Aug 23, 2022

Closing in favor of #273
Sorry for the noise

Update: Re-opening because 273 inadvertently added a dependency on ActiveSupport

@JamesChevalier JamesChevalier deleted the patch-1 branch August 23, 2022 16:35
@JamesChevalier JamesChevalier restored the patch-1 branch August 24, 2022 01:06
@pboling
Copy link
Member

pboling commented Aug 24, 2022

In the OAuth2 gem we actually decided to solve this by using the rash_alt gem, which is a subclass of Hashie::Mash, and it is working great there. I think perhaps we could do the same thing here.

Note: faraday had the same idea!

@JamesChevalier
Copy link
Author

maintainers went with #276

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

v0.6.0 breaks omniauth due to transform_keys change

2 participants