grape-entity icon indicating copy to clipboard operation
grape-entity copied to clipboard

NoMethodError: undefined method `[]=' for #<Grape::Entity::Options:0x007fc6a6f1a7b0>

Open mcls opened this issue 9 years ago • 7 comments

When you have an entity which exposes an ActiveRecord belongs_to without a :using option, then serializable_hash is called with a Grape::Entity::Options object.

But serializable_hash clones the object and then tries to set :except on it. (It sets default :except to an empty array see here.) This causes the exception:

NoMethodError: undefined method '[]=' for #<Grape::Entity::Options:0x007fc6a6f1a7b0>

Not too big of a problem, it was just a forgotten :using, but it was very confusing. Also depending on whether it's an object, array or hash, serializable_hash is called with or without options.

I'm talking about the code in Grape::Entity::Exposure::Base#serializable_value.

Should the options even be passed to serializable_hash at all? Or should []= be implemented? This did work up until this commit btw.

Example to reproduce:

class Author < ActiveRecord::Base
end

class Post < ActiveRecord::Base
  belongs_to :author
end

class PostEntity < Grape::Entity
  expose :author # notice missing :using
end

PostEntity.represent(Post.first).as_json # => NoMethodError

mcls avatar Feb 02 '16 12:02 mcls

I agree that this is confusing, and the error is not helpful. I am not sure what to do though, I guess the real question is what we'd like to see?

dblock avatar Feb 02 '16 17:02 dblock

Instead of relying on serializable_hash, maybe checking whether the object is a JSON primitive? Then if it isn't a primitive, you can raise an exception, saying that the :using should be specified. Maybe provide an option to fallback to serializable_hash or another method.

In multi_json they do this: https://github.com/intridea/multi_json/blob/1ba3be42600696f676d9cc1f029227b84c3c763b/lib/multi_json/convertible_hash_keys.rb#L17-L40

    def prepare_hash(hash, &key_modifier)
      return hash unless block_given?
      case hash
      when Array
        hash.map do |value|
          prepare_hash(value, &key_modifier)
        end
      when Hash
        hash.inject({}) do |result, (key, value)|
          new_key   = key_modifier.call(key)
          new_value = prepare_hash(value, &key_modifier)
          result.merge! new_key => new_value
        end
      when String, Numeric, true, false, nil
        hash
      else
        if hash.respond_to?(:to_json)
          hash
        elsif hash.respond_to?(:to_s)
          hash.to_s
        else
          hash
        end
      end

What do you think?

mcls avatar Feb 08 '16 08:02 mcls

I think it's a good strategy, try to PR?

dblock avatar Feb 08 '16 15:02 dblock

Looks like this will be hard to do without introducing any breaking changes. The code relies pretty heavily on serializable_hash.

I'll post my spike branch later, once I get a chance to clean it up a bit.

mcls avatar Feb 10 '16 09:02 mcls

I am running into this exact error with grape-entity (0.5.1). It occurs on gitlab 8.6.6 when requesting api/v3/projects and anything below. Is there anything I can do to get this functionality working? Is there possibly a version I could safely downgrade to?

thanks

bambam247 avatar Apr 30 '16 13:04 bambam247

@bambam247 Are you sure you have the same problem? In here the workaround is to use using and we're discussing producing a more helpful error.

dblock avatar May 02 '16 11:05 dblock

@dblock I'm using using and getting the same error.

dja avatar Jun 06 '16 18:06 dja