CodementorIO / rest-firebase

Ruby Firebase REST API client built on top of rest-core
Apache License 2.0
107 stars 22 forks source link

Something's wrong with nil #5

Closed wyattisimo closed 8 years ago

wyattisimo commented 9 years ago

Given a hash at /path/to/something and no value at /path/to/nothing, consider the following:

firebase  = RestFirebase.new(secret: ENV['FIREBASE_SECRET'], d: { consumer: ENV['FIREBASE_CONSUMER_AUTH_VALUE'] })
something = firebase.get("#{ENV['FIREBASE_URL']}/path/to/something")
nothing   = firebase.get("#{ENV['FIREBASE_URL']}/path/to/nothing")

Then:

something.class
# => Hash
nothing.class
# => NilClass

something.nil?
# => false
nothing.nil?
# => true

something == nil
# => false
nothing == nil
# => false

something === nil
# => false
nothing === nil
# => true

It appears that nothing is appropriately nil unless you try the comparison nothing == nil, which erroneously returns false.

Why is this the case? Is NilClass#== being overridden somewhere?

godfat commented 9 years ago

This is because the response object is a proxy to the real object, not the real object itself. It's implemented this way: https://github.com/godfat/rest-core/blob/rest-core-3.5.6/lib/rest-core/promise.rb#L8-L16

class Future < BasicObject
  def initialize promise, target
    @promise, @target = promise, target
  end

  def method_missing msg, *args, &block
    @promise.yield[@target].__send__(msg, *args, &block)
  end
end

And BasicObject didn't implement .class, .nil? nor .=== but it did implement .== according to the doc: http://ruby-doc.org/core-2.2.3/BasicObject.html Everything implemented for BasicObject won't be passed through the real object.

You could also check this related ticket on rest-core: https://github.com/godfat/rest-core/issues/9

This could probably be an unfortunate design, but I am not sure if I should introduce such incompatibility. To completely avoid this issue, we need to stop using proxies with method_missing. So the API would be changed to something like: (in your example)

firebase  = RestFirebase.new(secret: ENV['FIREBASE_SECRET'], d: { consumer: ENV['FIREBASE_CONSUMER_AUTH_VALUE'] })
something = firebase.get("#{ENV['FIREBASE_URL']}/path/to/something").load
nothing   = firebase.get("#{ENV['FIREBASE_URL']}/path/to/nothing").load

Note the extra .load in the end to get the real object. For now there's a workaround with tap{}. You can do the same thing currently with it. Example:

firebase  = RestFirebase.new(secret: ENV['FIREBASE_SECRET'], d: { consumer: ENV['FIREBASE_CONSUMER_AUTH_VALUE'] })
something = firebase.get("#{ENV['FIREBASE_URL']}/path/to/something").tap{}
nothing   = firebase.get("#{ENV['FIREBASE_URL']}/path/to/nothing").tap{}

This has the same effect to make it load and return the real object. You can check the section in rest-core: Exception Handling for Futures

.tap is provided by Ruby: http://ruby-doc.org/core-2.2.3/Object.html#method-i-tap Now there's also .itself: http://ruby-doc.org/core-2.2.3/Object.html#method-i-itself But I haven't verified that if that would work. I think since BasicObject didn't implement it, it should actually work. So you can also do this:

firebase  = RestFirebase.new(secret: ENV['FIREBASE_SECRET'], d: { consumer: ENV['FIREBASE_CONSUMER_AUTH_VALUE'] })
something = firebase.get("#{ENV['FIREBASE_URL']}/path/to/something").itself
nothing   = firebase.get("#{ENV['FIREBASE_URL']}/path/to/nothing").itself
godfat commented 8 years ago

Let's move the discussion over to https://github.com/godfat/rest-core/issues/9