hoodoo icon indicating copy to clipboard operation
hoodoo copied to clipboard

Response#set_resources and #set_estimated_resources do not check type

Open pond opened this issue 8 years ago • 0 comments

In Hoodoo::Services::Response:

  • https://github.com/LoyaltyNZ/hoodoo/blob/master/lib/hoodoo/services/services/response.rb

...there was originally just a #body accessor used to set response data, but this was later improved with some higher-level names of #set_resource and #set_resources. These were originally implemented just as pure aliases (and #set_resource still is), but the introduction of "estimated dataset size" meant that #set_resources and #set_estimated_resources ended up implemented as small, bespoke methods.

  • https://github.com/LoyaltyNZ/hoodoo/blob/master/lib/hoodoo/services/services/response.rb#L143
  • https://github.com/LoyaltyNZ/hoodoo/blob/master/lib/hoodoo/services/services/response.rb#L180

This is handy because Hoodoo really ought to check that an Array-like object is passed here. The rendering behaviour in terms of the top-level _data key and Array value (or not) is switched not based on the inbound API request method, of which the Response class is unaware, but simply on the data type passed. This makes it easy for a service author to accidentally pass a "singular" resource (e.g. a Hash) in a #list implementation under certain circumstances which might lead to an implementation that "knows" it'll only return a single instance.

A simple Type check would solve the problem. Since the switching behaviour is here:

  • https://github.com/LoyaltyNZ/hoodoo/blob/master/lib/hoodoo/services/services/response.rb#L340

...then the same type check semantics ought to be used in #set_resources and #set_estimated_resources, so it makes sense to extract the check into a private method and call from all three places.

  • [ ] Extract Array check from #for_rack into private method
  • [ ] Add call to this into #set_resources and #set_estimated_resources raising an exception if a bad type is encountered
  • [ ] Add tests to ensure 100% non-trivial coverage on updates
  • [ ] Update documentation if necessary, including checking Hoodoo Guides

pond avatar May 18 '17 23:05 pond