Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

per-request headers discard default headers #255

Closed
amirabiri opened this issue Nov 14, 2013 · 2 comments · Fixed by #270
Closed

per-request headers discard default headers #255

amirabiri opened this issue Nov 14, 2013 · 2 comments · Fixed by #270

Comments

@amirabiri
Copy link

In the following example:

class Lala
    include HTTParty
    headers 'Accept' => 'application/json'
end

Lala.post "/lala", :body => ..., :headers => { 'Content-Type' => 'application/special' }

The expected behavior is that the specific headers given for this particular request be merged with the default ones, so the final headers should be:

Accept:       application/json
Content-Type: application/special

However because of the first line in HTTParty.perform_request:

def perform_request(http_method, path, options, &block) #:nodoc:
    options = default_options.dup.merge(options)
    ...

The default headers are overwritten. This is due to the fact that merge isn't recursive - if the same key exists in both hashes it is overwritten. To merge in a different manner one must supply a block to handle conflicting keys.

I think in the specific case of :headers HTTParty should perform a deep merge.

@jnunemaker
Copy link
Owner

Yeah that makes sense. Happy to pull anything with good tests.

@amirabiri
Copy link
Author

Sorry - I won't have time for that. I'm primarily a Java & C# developer and Rails is a secondary skill. I'm not familiar enough with Cucumber. It would have been a good learning experience for me, but I'm a bit swamped at the moment!

I can give you the patch for the line itself if that helps, but that a simpler one-liner which I'm sure you can do better than me anyway... :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants