:safe option

[marshall-lee]

Hello!

Here's the excerpt from README:

Don't raise an exception and expose as nil, even if the :x cannot be evaluated.
expose :ip, safe: true

However, such fields are not exposed as nil — they're simply not exposed. Moreover this is written in specs:

сontext 'with safe option' do
  it 'does not throw an exception when an attribute is not found on the object' do
    fresh_class.expose :name, :nonexistent_attribute, safe: true
    expect { fresh_class.new(model).serializable_hash }.not_to raise_error
  end

  it "does not expose attributes that don't exist on the object" do
    fresh_class.expose :email, :nonexistent_attribute, :name, safe: true

    res = fresh_class.new(model).serializable_hash
    expect(res).to have_key :email
    expect(res).not_to have_key :nonexistent_attribute
    expect(res).to have_key :name
  end
  # ...
end

Who's right — the README or the specs?
My opinion that this is a bug. If we say expose then it should be somehow exposed. Not exposing it is just surprising.

@dblock
I want to fix it because this :safe thing stops me from fixing another bugs and it stops me even from reasoning about that bugs 😄 But what to fix — implementation+broken specs or just README ?

[dblock]

Obviously README and implementation should match. I think attribute that doesn't exist vs. :x cannot be evaluated are different things though, and I would definitely welcome a README update on what happens when the attribute doesn't exist vs. cannot be evaluated.

[marshall-lee]

@dblock

attribute that doesn't exist vs. :x cannot be evaluated are different things

What is "cannot be evaluated" then? When it raises exception? It seems like this option wasn't intented for such things. I think it's implemented for exposing mixed collections of objects — some of them may have some attribute and some of them may not.

If I want expose something optionally I would use :if or :unless. If exposure is not conditional then it should be exposed anyway and if it cannot (respond_to? if false or NoMethodError is raised) then raise an error (if unsafe) or expose nil (if safe).

For example:

class E < Grape::Entity
  expose :a, if: -> (obj,opts) { obj.respond_to? :a }
  expose :b, safe: true
end

# ...

X = Struct.new(:a, :b)
Y = Struct.new(:a)
Z = Struct.new(:b)

# ...

E.represent(X.new(1,2), serializable: true) # should evaluate to { a: 1, b: 2 }
E.represent(Y.new(1), serializable: true)   # should evaluate to { a: 1, b: nil }
E.represent(Z.new(2), serializable: true)   # should evaluate to { b: 2 } because :a is conditional
E.represent(Object.new, serializable: true) # should evaluate to { b: nil }

So I think :if and :safe are different things. If there's no :if then I expect to see the attribute in response.

[dblock]

Ok, makes sense, other than your fix in #142 what do you propose we do about this issue?