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

Return body from PUT or POST #26

Closed
stijnopheide opened this issue Nov 2, 2015 · 9 comments
Closed

Return body from PUT or POST #26

stijnopheide opened this issue Nov 2, 2015 · 9 comments

Comments

@stijnopheide
Copy link
Contributor

Currently it's impossible to return a body that contains e.g. a map of data from a PUT or POST method.

SEVERE: error sending body of type  clojure.lang.PersistentArrayMap
java.lang.IllegalArgumentException: Don't know how to convert class clojure.lang.PersistentArrayMap into (stream-of io.netty.buffer.ByteBuf)
    at byte_streams$convert.invoke(byte_streams.clj:187)
    at aleph.netty$to_byte_buf_stream.invoke(netty.clj:170)
    at aleph.http.core$send_streaming_body.invoke(core.clj:277)
    at aleph.http.core$eval24414$send_message__24421$fn__24422.invoke(core.clj:353)
    at aleph.http.core$eval24414$send_message__24421.invoke(core.clj:352)
    at aleph.http.server$eval24451$send_response__24456$f__24005__auto____24460.invoke(server.clj:124)
    at aleph.http.server$eval24451$send_response__24456$fn__24463.invoke(server.clj:117)
    at clojure.lang.AFn.run(AFn.java:22)
    at io.netty.util.concurrent.SingleThreadEventExecutor.runAllTasks(SingleThreadEventExecutor.java:339)
    at io.netty.channel.nio.NioEventLoop.run(NioEventLoop.java:356)
    at io.netty.util.concurrent.SingleThreadEventExecutor$5.run(SingleThreadEventExecutor.java:742)
    at io.netty.util.concurrent.DefaultThreadFactory$DefaultRunnableDecorator.run(DefaultThreadFactory.java:137)
    at java.lang.Thread.run(Thread.java:745)

The reason is that there is no body conversion in PUT/POST as it is done in GetMethod/request.

Is this something yada intends to support? The HTTP spec allows for either referencing the newly created/updated entity or actually specifying it.

http://www.w3.org/Protocols/rfc2616/rfc2616-sec10.html
Section 10.2.1 200 OK
POST an entity describing or containing the result of the action;

There's no mention about PUT 200 or 201 other than The newly created resource can be referenced by the URI(s) returned in the entity of the response, with the most specific URI for the resource given by a Location header field. The response SHOULD include an entity containing a list of resource characteristics and location(s) from which the user or user agent can choose the one most appropriate.

As this impacts all methods that can return a body, I'm not sure how you'd like to pull that out of the GetMethod.

@malcolmsparks
Copy link
Contributor

Yes, after consideration I think yada should support returning a body in PUT/POST as per the lines of the spec. Let's keep this issue open.

@tangrammer
Copy link
Contributor

Hi guys!
Do we have some shortcut for having body in PUT until proper impl and release?
I'm trying this with no lucky

(defrecord MyRecordType [d])

(extend-protocol PutResult
  MyRecordType
  (interpret-put-result [o ctx]
    (assoc-in ctx [:response :body] (:d o))))

@malcolmsparks
Copy link
Contributor

PUT bodies should work - see the phonebook example. Or do you mean
response bodies?

On 8 March 2016 at 10:35, tangrammer notifications@github.com wrote:

Hi guys!
Do we have some shortcut for having body in PUT until proper impl and
release?
I'm trying this with no lucky


Reply to this email directly or view it on GitHub
#26 (comment).

@tangrammer
Copy link
Contributor

sorry, I meant response bodies :)

@tangrammer
Copy link
Contributor

Thanks Malcolm!
could i ask you to do a release including this commit?
BTW: I tried to find tag xxxx-32 to work on top of it but i couldn't find it

@malcolmsparks
Copy link
Contributor

Yes... but my tests fail :(

But give me a couple of hours...
On 8 Mar 2016 13:57, "tangrammer" notifications@github.com wrote:

Thanks Malcolm!
could i ask you to do a release including this commit?
BTW: I tried to find tag xxxx-32 to work on top of it but i couldn't find
it


Reply to this email directly or view it on GitHub
#26 (comment).

@tangrammer
Copy link
Contributor

👍 👍

@malcolmsparks
Copy link
Contributor

Hi,

You can do this by returning a modified response, which yada's method proxy
interprets that you want to control what is returned.

(testing "return response"
(let [h (yada (resource {:methods {:put {:response (fn [ctx](assoc
%28:response ctx%29 :body))}}}))
response (h (request :put "/"))](is %28= "BODY" %28b/to-string %28:body @response))))))

I've had to back out the last commit because it broke some test, but I've
added the above test which passes.

Regards,

Malcolm

JUXT LTD.
Software Consulting, Delivery, Training

On 8 March 2016 at 14:59, tangrammer notifications@github.com wrote:

[image: 👍] [image: 👍]


Reply to this email directly or view it on GitHub
#26 (comment).

@stanislas
Copy link

Thanks for the solution :)

I had to change the status to 200 in order to have aleph (or the yada handler?) actually serving the body. (with status 204, the body is not send, although the header contains a correct content length). To reflect that, I prepared a PR with the corresponding change in the test.

Pull Request: #72

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

No branches or pull requests

4 participants