Adding put_form to HTTP::Client. Fixes #4181 #4190

jwoertink · 2017-03-24T19:23:55Z

This commit adds in a helper put_form method for HTTP::Client. The only real change is that it submits a "PUT" request instead of "POST". Since the code seems to be almost identical to the post_form methods, I put it in a macro which would allow for a patch_form later if that's wanted.

client = HTTP::Client.new "www.example.com"
response = client.put_form "/", "foo=bar"

Sija · 2017-06-02T16:27:47Z

src/http/client.cr

-    request.headers["Content-type"] = "application/x-www-form-urlencoded"
-    exec request
-  end
+  {% for http_method in ["post", "put"] %}


You could also add patch to the list. (%w(post put patch))

Ok, I wasn't sure if that would be a wanted change or not so I left it off. I'll add it in though.

bararchy · 2017-07-31T20:12:34Z

@jwoertink if you rebase , maybe @RX14 can take a look ?

jwoertink · 2017-07-31T21:15:52Z

yikes! I think I did that wrong... I did a fetch and rebase. Looks like it pulled in all the commits since I originally did this back in march 😝 How I do I squish all those?

bararchy · 2017-07-31T21:21:32Z

@RX14 or some other maintainer can squash them in the git PR window , or you can do it (Goole is your best friend )

straight-shoota · 2017-07-31T22:51:06Z

Woah, there went something terribly wrong. It seems you've rebased on a wrong commit and now all commits to master have been rebased on that. Why is the commit Adding put_form to HTTP::Client. Fixes #4181 from 24 March even twice in the history?
This doesn't need a squash but a proper clean up.
A simple solution might be to check out origin/master in a new branch, cherry-pick c40a51a and dd37a5c from the old branch (there you'll need to resolve the merge conflict) and then force push the new branch with these two commits to jwoertink:feature/http_client_put_form. That should bring things in order.

jwoertink · 2017-08-01T00:07:29Z

ok, much better. Thanks @straight-shoota 😄

straight-shoota · 2017-08-01T00:14:09Z

src/http/client.cr

+    # ```
+    def {{http_method.id}}_form(path, form : String | IO, headers : HTTP::Headers? = nil) : HTTP::Client::Response
+      request = new_request({{http_method.upcase}}, path, headers, form)
+      request.headers["Content-type"] = "application/x-www-form-urlencoded"


While you're at it, could you please change all Content-type to Content-Type with capital T? Apart from this file we have it this way everywhere and that's the spelling recommended by RFC 7231.

straight-shoota · 2017-08-01T00:16:35Z

src/http/client.cr

-    exec request
-  end
+  {% for http_method in %w(post put patch) %}
+    # Executes a {{http_method.id.upcase}} with form data. The "Content-type" header is set


A) Maybe add "request" after the method type? => Executes a POST request with form data...

straight-shoota · 2017-08-01T00:18:00Z

src/http/client.cr

-    request.headers["Content-type"] = "application/x-www-form-urlencoded"
-    exec(request) do |response|
-      yield response
+    # Executes a {{http_method.id.upcase}} with form data and yields the response to the block.


Thank you! Could you put this in, too?

oops! Missed one. Sure thing

straight-shoota · 2017-08-01T00:20:10Z

src/http/client.cr

-    exec request
-  end
+  {% for http_method in %w(post put patch) %}
+    # Executes a {{http_method.id.upcase}} with form data. The "Content-type" header is set


B) Maybe add "and returns a Response." =>... with form data and returns a `Response`.

straight-shoota · 2017-08-01T00:20:50Z

src/http/client.cr

-    body = HTTP::Params.encode(form)
-    post_form path, body, headers
-  end
+    # Executes a {{http_method.id.upcase}} with form data. The "Content-type" header is set


straight-shoota · 2017-08-01T00:25:38Z

src/http/client.cr

-    body = HTTP::Params.encode(form)
-    post_form(path, body, headers) do |response|
-      yield response
+    # Executes a {{http_method.id.upcase}} with form data and yields the response to the block.


straight-shoota · 2017-08-01T00:25:58Z

src/http/client.cr

+    # end
+    # ```
+    def {{http_method.id}}_form(path, form : Hash(String, _) | NamedTuple, headers : HTTP::Headers? = nil)
+      body = HTTP::Params.from_hash(form)


straight-shoota · 2017-08-01T00:26:41Z

src/http/client.cr

-  def self.post_form(url, form : String | IO | Hash, headers : HTTP::Headers? = nil, tls = nil) : HTTP::Client::Response
-    exec(url, tls) do |client, path|
-      client.post_form(path, form, headers)
+    # Executes a {{http_method.id.upcase}} with form data. The "Content-type" header is set


straight-shoota · 2017-08-01T00:26:50Z

src/http/client.cr

-    exec(url, tls) do |client, path|
-      client.post_form(path, form, headers) do |response|
-        yield response
+    # Executes a {{http_method.id.upcase}} with form data and yields the response to the block.


straight-shoota · 2017-08-01T00:28:57Z

src/http/client.cr

+    # response = client.{{http_method.id}}_form "/", {"foo" => "bar"}
+    # ```
+    def {{http_method.id}}_form(path, form : Hash(String, _) | NamedTuple, headers : HTTP::Headers? = nil) : HTTP::Client::Response
+      body = HTTP::Params.from_hash(form)


C) HTTP::Params.from_hash has been renamed to HTTP::Params.encode (#4205). That's why the specs are failing.

…FC 7231 spec.

…s renamed to encode in crystal-lang#4205

straight-shoota

Great!

jwoertink · 2017-08-21T18:53:03Z

So... Who do I have to buy a drink for to get this merged? 😂

jwoertink · 2017-08-22T15:21:23Z

yay! Thanks guys 😂

Sija mentioned this pull request Mar 29, 2017

Add HTTP::Client.put_form, same as post_form. #4210

Closed

molovo mentioned this pull request Jun 2, 2017

Need HTTP::Client.put_form #4181

Closed

Sija reviewed Jun 2, 2017

View reviewed changes

jwoertink mentioned this pull request Jul 31, 2017

I <3 Crystal #4757

Closed

jwoertink added 2 commits July 31, 2017 16:50

Adding put_form to HTTP::Client. Fixes crystal-lang#4181

02ffc8a

added patch_form to HTTP::Client to match post_form and put_form

ec049a0

jwoertink force-pushed the feature/http_client_put_form branch from 481ea67 to ec049a0 Compare August 1, 2017 00:06

straight-shoota requested changes Aug 1, 2017

View reviewed changes

jwoertink added 3 commits August 1, 2017 09:07

fixing some recommended typos for documentation clarity and to meet R…

3fc22b3

…FC 7231 spec.

fixing method call on HTTP::Params to fix failing specs. from_hash wa…

48e7a8b

…s renamed to encode in crystal-lang#4205

missed a word in the http client docs

caa57ef

straight-shoota approved these changes Aug 1, 2017

View reviewed changes

Sija approved these changes Aug 5, 2017

View reviewed changes

asterite approved these changes Aug 22, 2017

View reviewed changes

RX14 approved these changes Aug 22, 2017

View reviewed changes

asterite merged commit ef17d5d into crystal-lang:master Aug 22, 2017

asterite added this to the Next milestone Aug 22, 2017

RX14 added kind:feature topic:stdlib labels Aug 22, 2017

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adding put_form to HTTP::Client. Fixes #4181 #4190

Adding put_form to HTTP::Client. Fixes #4181 #4190

jwoertink commented Mar 24, 2017

Sija Jun 2, 2017

jwoertink Jun 2, 2017

bararchy commented Jul 31, 2017

jwoertink commented Jul 31, 2017

bararchy commented Jul 31, 2017

straight-shoota commented Jul 31, 2017 •

edited

Loading

jwoertink commented Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

jwoertink Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota Aug 1, 2017

straight-shoota left a comment

jwoertink commented Aug 21, 2017

jwoertink commented Aug 22, 2017

Adding put_form to HTTP::Client. Fixes #4181 #4190

Adding put_form to HTTP::Client. Fixes #4181 #4190

Conversation

jwoertink commented Mar 24, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

bararchy commented Jul 31, 2017

jwoertink commented Jul 31, 2017

bararchy commented Jul 31, 2017

straight-shoota commented Jul 31, 2017 • edited Loading

jwoertink commented Aug 1, 2017

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

straight-shoota left a comment

Choose a reason for hiding this comment

jwoertink commented Aug 21, 2017

jwoertink commented Aug 22, 2017

straight-shoota commented Jul 31, 2017 •

edited

Loading