Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

metadata HashWithIndifferentAccess incompatible with rails4 #44

Open
jonnymacs opened this issue Jul 29, 2014 · 1 comment
Open

metadata HashWithIndifferentAccess incompatible with rails4 #44

jonnymacs opened this issue Jul 29, 2014 · 1 comment

Comments

@jonnymacs
Copy link

The issue I came across has to do with the way rails4 casts parameters to ActionController::Parameters objects. There are some methods rails expects to exist on these objects, but the are missing if the tpkg gem is loaded

Easiest way to repro this would be to run a rails4 app that has tpkg in the Gemfile default group, then execute these steps from the rails console

$raw_parameters = { :teams => { :number => "37" } }
$parameters = ActionController::Parameters.new(raw_parameters)
$parameters.require(:teams).permit!

NoMethodError: undefined method `permitted=' for {"number"=>"37"}:ActiveSupport::HashWithIndifferentAccess
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/actionpack-4.1.4/lib/action_controller/metal/strong_parameters.rb:325:in `block in dup'
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/actionpack-4.1.4/lib/action_controller/metal/strong_parameters.rb:324:in `tap'
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/actionpack-4.1.4/lib/action_controller/metal/strong_parameters.rb:324:in `dup'
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/activesupport-4.1.4/lib/active_support/hash_with_indifferent_access.rb:50:in `with_indifferent_access'

from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/tpkg-2.3.3/lib/tpkg/metadata.rb:135:in `convert_value'
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/tpkg-2.3.3/lib/tpkg/metadata.rb:51:in `[]='

    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/actionpack-4.1.4/lib/action_controller/metal/strong_parameters.rb:337:in `convert_hashes_to_parameters'
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/actionpack-4.1.4/lib/action_controller/metal/strong_parameters.rb:282:in `[]'
    from /Users/jmcallister/.rvm/gems/ruby-2.1.2@ypleads-sales-tool/gems/tpkg-2.3.3/lib/tpkg/metadata.rb:36:in `default'

The output should be

=> {"number"=>"37"}

Now the tpkg gem should not get loaded with rails. It is / was getting loaded b/c it is / was part of the default group in bundler in our application. We no longer include tpkg in the default bundler group, which resolves the issue.

But it still seems risky to define HashWithIndifferentAccess class in tpkg in the event that it's already defined by someone else.

Perhaps there is a way to conditionally create HashWithIndifferentAccess if it's not already defined by someone else (like ActiveSupport)?

some references
https://github.com/rails/strong_parameters
rails/strong_parameters#140

@tdombrowski
Copy link

Conditional definition of HashWithIndifferentAccess can also be risky, if the active_support version does exist, but is or becomes incompatible with the way tpkg uses it.

Might be safer to rename the class entirely, or put it into a module to namespace it (along with other classes that may be borrowed from active_support)

bhenderson pushed a commit to yp-engineering/client that referenced this issue Feb 29, 2016
ActiveSupport actually has a line like

> HashWithIndifferentAccess = ActiveSupport::HashWithIndifferentAccess

Which means that we're actually reopening the class, rather than
defining our own, which means we're mixing old/new functionality.

This was reported as breaking Rails 4: tpkg#44, tpkg#50

I tool tdombrowski's suggestion to keep the functionality vendored.
bhenderson pushed a commit to yp-engineering/client that referenced this issue Feb 29, 2016
ActiveSupport actually has a line like

> HashWithIndifferentAccess = ActiveSupport::HashWithIndifferentAccess

Which means that we're actually reopening the class, rather than
defining our own, which means we're mixing old/new functionality.

This was reported as breaking Rails 4: tpkg#44, tpkg#50

I took tdombrowski's suggestion to keep the functionality vendored.
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

No branches or pull requests

2 participants