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

to_json fails if Dash has a property named method #222

Open
atomovski opened this issue Sep 2, 2014 · 1 comment
Open

to_json fails if Dash has a property named method #222

atomovski opened this issue Sep 2, 2014 · 1 comment
Labels

Comments

@atomovski
Copy link

require 'hashie'

class Recipient < Hashie::Dash
  include Hashie::Extensions::IgnoreUndeclared

  property :method
  property :destination
  property :contactId

end

class SendAction < Hashie::Dash
  include Hashie::Extensions::IgnoreUndeclared
  include Hashie::Extensions::Coercion

  coerce_key :recipient, Recipient

  property :recipient, default: Recipient.new
  property :messageId
end

SendAction.new.to_json

The result is:

hashie-3.2.0/lib/hashie/dash.rb:41:in `block in property': wrong number of arguments (1 for 0) (ArgumentError)
    from /Users/alext/.rvm/gems/ruby-2.1.2/gems/hashie-3.2.0/lib/hashie/hash.rb:48:in `flexibly_convert_to_hash'
    from /Users/alext/.rvm/gems/ruby-2.1.2/gems/hashie-3.2.0/lib/hashie/hash.rb:34:in `block in to_hash'
    from /Users/alext/.rvm/gems/ruby-2.1.2/gems/hashie-3.2.0/lib/hashie/hash.rb:20:in `each'
    from /Users/alext/.rvm/gems/ruby-2.1.2/gems/hashie-3.2.0/lib/hashie/hash.rb:20:in `to_hash'
    from /Users/alext/.rvm/gems/ruby-2.1.2/gems/hashie-3.2.0/lib/hashie/hash.rb:42:in `to_json'
    from ./problem.sh:24:in `<main>'

Basically the problem is that the instance method: "method" is overridden in the Recipient class. And when called in the snippet bellow it produces the error.

def flexibly_convert_to_hash(object, options = {})
  if object.method(:to_hash).arity == 0

Also I can see that the method def self.property in dash.rb contains the following code that handles a similar case:

unless instance_methods.map { |m| m.to_s }.include?("#{property_name}=")
  define_method(property_name) { |&block| self.[](property_name, &block) }
  property_assignment = property_name.to_s.concat('=').to_sym
  define_method(property_assignment) { |value| self.[]=(property_name, value) }
end

So I guess I opened this issue mostly as a discussion on how to handle this case?

@dblock dblock added the bug? label Sep 3, 2014
@michaelherold
Copy link
Member

I've been thinking about this since I saw the report. As with the related issue with Mash (see #119 and #198 for the discussion), it comes down to the issue of how much we can/want to prevent people from shooting themselves in the foot. If someone creates a property named method, what do they think is going to happen? Everyone should know that Object#method and that creating a property named method would conflict with that. It's just not something you should do.

That being said, we:

  1. definitely should note in the README that dangerous things can be done with Dash.
  2. could make an extension for Dash that prevents these dangerous actions, like I did for Mash.

As for default behavior, it would be nice if this wasn't an issue and that everyone could define whatever method they wanted. However, I'm not sure that's really feasible -- and the ways that I can think of to accomplish this would cause the same issue as with Mash in that a Dash should quack like a Hash. I think we should just note it in the README and leave it as is, or create a safe extension.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants