HTTParty::Request::Body#multipart? fixes

[EmilioCristalli]

This PR intends to fix a few issues with the multipart? method:

1. It wasn't working for params that respond to #to_hash but not to #detect (like ActionDispatch::Request::Session). Same for params that respond to #to_ary but not to #detect

class HashLike; def to_hash; { a: 3 }; end; end
HTTParty::Request::Body.new(a: HashLike.new).multipart? # NoMethodError: undefined method `detect' for #<HashLike:0x00007fbf8971ae08>

2. Wasn't detecting files inside arrays
   (includes_hash?(value) was always returning true for not empty arrays, because all objects respond to #hash, and then it was doing hash.detect do |key, value| with an array which actually yields only one arg)

HTTParty::Request::Body.new(a: [Tempfile.new(['some_temp_file','.gif'])]).multipart? # false

3. looked like it was intended to be recursive and look for a file inside all hash values or array elements recursively but I think it wasn't doing it (if the method was not supposed to be recursive please let me know because this might impact performance)

[FLarra]

@TheSmartnik It'd be great to have this PR merged as soon as you feel confident with it.

I'm maintaining the gem https://github.com/smartinez87/exception_notification which relies on HTTParty in several notifiers and they are not working if updating HTTParty version. The last working version is on tag: https://github.com/jnunemaker/httparty/releases/tag/v0.15.7

It'd be great to merge this #multipart? fix so we don't need to lock our users to versions prior to the multipart refactor.

Cheers

[TheSmartnik]

Thanks a lot for the PR, sorry it took a long time to respond. It looks good, a few minor comments about naming and we are good to go.

[TheSmartnik]

I feel like #has_file? is better a naming for this method, anyway.
You still ask whether the params have a file or not. You're right that it can be file, but it can also be hash, array and #has_file? is a more inclusive naming

[EmilioCristalli]

yeah, you are right. I changed here 5a778ed

[TheSmartnik]

Previous use of a method was more readable and understandable , imho.
It showed that if value responds to path and read it is a file, which is exactly what we are looking for.

[EmilioCristalli]

you mean this method?

      def file?(object)
        object.respond_to?(:path) && object.respond_to?(:read)
      end

if that's what you meant, i think it makes sense, and also would help for the generate_multipart which should only check if the value is a file (not recursive) if i understand it correctly.

I changed it here as well: 5a778ed

[EmilioCristalli]

@TheSmartnik thanks for the review!
I think i addressed your comments, but let me know if i misunderstood them or if you have any other comments!

[TheSmartnik]

Thanks! Look good. I'll run a few more tests later today and bump the version