gamache / hyperresource

A self-inflating Ruby client for hypermedia APIs. Not under active development.
http://hyperresource.com/doc
MIT License
304 stars 29 forks source link

fixed "warning: toplevel constant String referenced by Foo::String" when using hyperresource in ActiveResource-style #39

Closed pvmeerbe closed 9 years ago

pvmeerbe commented 9 years ago

summary:

see http://stem.ps/rails/2015/01/25/ruby-gotcha-toplevel-constant-referenced-by.html for a detailed explanation on the issue in Rails

New classes are automatically created based on return value of get_data_type call. Previously 'eval' was used to detect whether the class already exists or still needs to be created. However these classes are nested within the namespaced class of the initialised hyperresource object. As a result (see related article) this eval call will also succeed if the class already exists as a toplevel constant, hence will not create and use the intended class

Solution is to check whether the class constant is defined instead of using eval

Detailed example:

Given:

class MyApi < HyperResource end

Assume the following:

class Bananas

special business code on selling/eating bananas

def self.types end end

When:

Bananas toplevel class is already loaded and API get is done:

my_types = Bananas.types res = Myapi.new(:root => 'http://whatever.com').bananas.get

Then:

HyperResource will ensure:

As explained in the related article

eval(MyApi::Bananas)

will search for 'Bananas' both in the MyApi namespace and in upper namespaces as MyApi is a class and not a module The toplevel Bananas class is found, an the MyApi::Bananas class will not be created dynamically as expected

As a result an attempt will be made to create a new HyperResource resource using the toplevel Bananas class which will fail

As a workaround one could declare all known response data types in the API, hence bypassing the dynamic creation of required classes

class MyApi < HyperResource

class Bananas end end

However this is error-prone as somebody will certainly forget to add a newly created return type to the list.

Solution:

solution is to check whether the class constant already exists instead of using the eval construct

gamache commented 9 years ago

Thanks for the patch! Rolling it in. :+1: