Skip to content

Link method_missing delegation wrongly returns nil when field value is false #126

@ivoanjo

Description

@ivoanjo

When a resource includes some field that is false, trying to read it through the Link class will result in the value being returned as nil instead of false.

Test case:

diff --git a/test/hyperclient/link_test.rb b/test/hyperclient/link_test.rb
index 823efa6..20e8872 100644
--- a/test/hyperclient/link_test.rb
+++ b/test/hyperclient/link_test.rb
@@ -479,6 +479,16 @@ module Hyperclient
           resource.orders.first.id.must_equal 1
         end
 
+        it 'can handle false values in the response' do
+          resource = Resource.new({ '_links' => { 'orders' => { 'href' => '/orders' } } }, entry_point)
+
+          stub_request(entry_point.connection) do |stub|
+            stub.get('http://api.example.org/orders') { [200, {}, { 'any' => false }] }
+          end
+
+          resource.orders.any.must_equal false
+        end
+
         it "doesn't delegate when link key doesn't match" do
           resource = Resource.new({ '_links' => { 'foos' => { 'href' => '/orders' } } }, entry_point)

Current result:

Hyperclient::Link::method_missing::delegation
     FAIL (0:00:00.145) test_0002_can handle false values in the response
          Expected: false
            Actual: nil
        @ (eval):8:in `must_equal'
          test/hyperclient/link_test.rb:489:in `block (4 levels) in <module:Hyperclient>'

This seems to me to be due to a bad interaction in Link#method_missing (https://github.com/codegram/hyperclient/blob/master/lib/hyperclient/link.rb#L129):

    # Internal: Delegate the method further down the API if the resource cannot serve it.
    def method_missing(method, *args, &block)
      if _resource.respond_to?(method.to_s)
        _resource.send(method, *args, &block) || delegate_method(method, *args, &block)
      else
        super
      end
    end

In this case, _resource.send(method, *args, &block) is returning false, which we are mistaking for the case where we're trying to call an array method on the resource, and thus instead of returning false we are calling delegate_method (which will then return the nil).

I was thinking about it and it seems to me that changing it to treat false differently would fix the issue:

diff --git a/lib/hyperclient/link.rb b/lib/hyperclient/link.rb
index 1fd1169..cc5b635 100644
--- a/lib/hyperclient/link.rb
+++ b/lib/hyperclient/link.rb
@@ -126,7 +126,8 @@ module Hyperclient
     # Internal: Delegate the method further down the API if the resource cannot serve it.
     def method_missing(method, *args, &block)
       if _resource.respond_to?(method.to_s)
-        _resource.send(method, *args, &block) || delegate_method(method, *args, &block)
+        result = _resource.send(method, *args, &block)
+        result.nil? ? delegate_method(method, *args, &block) : result
       else
         super
       end

But I was not entirely sure, hence I decided to open an issue first, instead of jumping straight to a PR. Any thoughts on this approach?

Metadata

Metadata

Assignees

No one assigned

    Labels

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions