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

Allow passing HTTP::FormData object to HTTP::Client directly #599

Closed

Conversation

summera
Copy link

@summera summera commented Mar 10, 2020

As discussed with @ixti in httprb/form_data#29, this allows you to pass an HTTP::FormData object directly instead of a Hash for more customization such as using a custom encoder. Example:

custom_encoder = proc { |data| ::JSON.dump(data) }
data = { "foo[bar]" => "test" }
form_data = HTTP::FormData.create(data, encoder: custom_encoder)
HTTP.post("https://some-url.com", form: form_data)

Note that I decided to use the following logic to determine whether an HTTP::FormData object was being passed in directly:

    def form_data(form)
      (form || {}).respond_to?(:to_h) ? HTTP::FormData.create(form) : form
    end

The other option would be to check if form is_a? HTTP::FormData::Urlencoded or a HTTP::FormData::Multipart but this seemed less flexible since it creates a coupling with what's returned from HTTP::FormData.create. However, I don't have a strong opinion here and am happy to change if another method is more desirable.

@summera
Copy link
Author

summera commented Mar 10, 2020

@ixti provided everything here is all good, can we also backport this to v4.x?

@summera summera force-pushed the allow-passing-form-data-directly branch from 85da8fd to 8bcb472 Compare March 10, 2020 03:50
@summera
Copy link
Author

summera commented Mar 12, 2020

@ixti just wanted to check back with you on this. Let me know what you think.

@ixti
Copy link
Member

ixti commented Mar 16, 2020

Honestly I think it's better to check type. But I agree that checking multiple types in here is kinda ugly. So I tend to think that we can add coerce to FormData instead and use it here instead of create. WDYT?

@ixti ixti closed this in 4ec7d71 Mar 23, 2020
ixti added a commit that referenced this pull request Mar 23, 2020
This patch is a squash (with some refactoring) if following commits:

    commit 8bcb472
    Author: Ari Summer <aribsummer@gmail.com>
    Date:   Mon Mar 9 21:28:29 2020 -0600

        Remove space inside hash

    commit 08d19d8
    Author: Ari Summer <aribsummer@gmail.com>
    Date:   Mon Mar 9 18:31:22 2020 -0600

        Allow passing HTTP::FormData object directly

I've decided to go with tye checking after all to keep method semantics
as they were, as `options.form` shoudl be either form data or anything
supported by FormData\.create at least for now.

Closes: #599
@summera
Copy link
Author

summera commented Mar 23, 2020

@ixti thanks for the merge and apologies for not getting back to you earlier with my thoughts! Any chance of backporting this update to 4.x?

@summera
Copy link
Author

summera commented Mar 23, 2020

Sorry, just realized it already was! Great 👍 . Please let me know when a new release is out with the update 😄

@ixti
Copy link
Member

ixti commented Mar 25, 2020

Released v4.4.0

netbsd-srcmastr pushed a commit to NetBSD/pkgsrc that referenced this pull request Jun 7, 2020
Update ruby-http to 4.4.1.


## 4.4.1 (2020-03-29)

* Backport [#590](httprb/http#590)
  Fix parser failing on some edge cases.
  ([@ixti])

## 4.4.0 (2020-03-25)

* Backport [#587](httprb/http#587)
  Fix redirections when server responds with multiple Location headers.
  ([@ixti])

* Backport [#599](httprb/http#599)
  Allow passing HTTP::FormData::{Multipart,UrlEncoded} object directly.
  ([@ixti])

## 4.3.0 (2020-01-09)

* Backport [#581](httprb/http#581)
  Add Ruby-2.7 compatibility.
  ([@ixti], [@janko])


## 4.2.0 (2019-10-22)

* Backport [#489](httprb/http#489)
  Fix HTTP parser.
  ([@ixti], [@fxposter])


## 4.1.1 (2019-03-12)

* Add `HTTP::Headers::ACCEPT_ENCODING` constant.
  ([@ixti])


## 4.1.0 (2019-03-11)

* [#533](httprb/http#533)
  Add URI normalizer feature that allows to swap default URI normalizer.
  ([@mamoonraja])


## 4.0.5 (2019-02-15)

* Backport [#532](httprb/http#532) from master.
  Fix pipes support in request bodies.
  ([@ixti])


## 4.0.4 (2019-02-12)

* Backport [#506](httprb/http#506) from master.
  Skip auto-deflate when there is no body.
  ([@Bonias])


## 4.0.3 (2019-01-18)

* Fix missing URL in response wrapped by auto inflate.
  ([@ixti])

* Provide `HTTP::Request#inspect` method for debugging purposes.
  ([@ixti])


## 4.0.2 (2019-01-15)

* [#506](httprb/http#506)
  Fix instrumentation feature.
  ([@paul])


## 4.0.1 (2019-01-14)

* [#515](httprb/http#515)
  Fix `#build_request` and `#request` to respect default options.
  ([@RickCSong])


## 4.0.0 (2018-10-15)

* [#482](httprb/http#482)
  [#499](httprb/http#499)
  Introduce new features injection API with 2 new feaures: instrumentation
  (compatible with ActiveSupport::Notification) and logging.
  ([@paul])

* [#473](httprb/http#473)
  Handle early responses.
  ([@janko-m])

* [#468](httprb/http#468)
  Rewind `HTTP::Request::Body#source` once `#each` is complete.
  ([@ixti])

* [#467](httprb/http#467)
  Drop Ruby 2.2 support.
  ([@ixti])

* [#436](httprb/http#436)
  Raise ConnectionError when writing to socket fails.
  ([@janko-m])

* [#438](httprb/http#438)
  Expose `HTTP::Request::Body#source`.
  ([@janko-m])

* [#446](httprb/http#446)
  Simplify setting a timeout.
  ([@mikegee])

* [#451](httprb/http#451)
  Reduce memory usage when reading response body.
  ([@janko-m])

* [#458](httprb/http#458)
  Extract HTTP::Client#build_request method.
  ([@tycoon])

* [#462](httprb/http#462)
  Fix HTTP::Request#headline to allow two leading slashes in path.
  ([@scarfacedeb])

* [#454](httprb/http#454)
  [#464](httprb/http#464)
  [#384](httprb/http#384)
  Fix #readpartial not respecting max length argument.
  ([@janko-m], [@marshall-lee])
@tarcieri tarcieri mentioned this pull request May 13, 2021
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 this pull request may close these issues.

None yet

2 participants