expose_nil delegates to object, ignores methods defined in Entities

Open
#305 4 comments 9 reactions 0 assignees View on GitHub

Nobody has claimed this yet.

Assessment

Difficulty
3/5
Estimated time
1-2 days
Newbie friendliness
45/100
Issue type
Bug
Clarity
Mostly clear
Activity status
Stale
Tech stack
ruby
Domain
api

Research direction

Start with lib/grape_entity/exposure.rb at the linked expose_nil delegation code, then inspect how delegatable? handles methods defined on Entity classes. Reproduce the three MyEntity examples from the issue and verify that inherited Entity methods are respected without calling missing methods on the represented object.

Written by the indexing model from the issue text.

Description

expose_nil delegates to the underlying object and doesn't test whether the method is delegatable? This can lead to odd behaviour and errors.

You'd probably have to be doing inheritance with your Entities in order to encounter it in the wild, but as a contrived example:

class MyEntity < ::Grape::Entity
  expose :foo, expose_nil: false

  def foo
    'forty-foo'
  end
end

class FooableClass
  def initialize(foo = nil)
    @foo = foo
  end

  def foo
    @foo
  end
end

class NoFooFoYou
end

# In all 3 of the following, I'd argue that the expected result is {:foo=>'forty-foo'}

MyEntity.represent(FooableClass.new(42)).as_json
# => {:foo=>'forty-foo'}

MyEntity.represent(FooableClass.new).as_json
# => {}

MyEntity.represent(NoFooFoYou.new).as_json
NoMethodError: undefined method `foo' for #<NoFooFoYou:>

Would you agree this is an issue?

Dominant language
Ruby
Stars
729
Forks
154
No merged PRs in 30d

Contributor guide

Open the contributing guide

First steps

  1. Read the whole issue, then the project's contributing guide.
  2. Comment on the issue to say you are picking it up — it saves two people doing the same work.
  3. Fork the repository and make your change on a branch.
  4. Open a pull request that references the issue number.

More from ruby-grape/grape-entity

All issues in ruby-grape/grape-entity

Similar issues

More Ruby issues

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.