rexyai / RestRserve

R web API framework for building high-performance microservices and app backends
https://restrserve.org
271 stars 31 forks source link

Fix content-type "application/x-www-form-urlencoded" failed #184

Closed jonekeat closed 2 years ago

jonekeat commented 2 years ago

fixes #183

artemklevtsov commented 2 years ago

Thank you for the request. Can you add test for the application/x-www-form-urlencoded case?

jonekeat commented 2 years ago

@artemklevtsov I have added a test case for POST method with content_type = "application/x-www-form-urlencoded", but there is 1 thing I quite confused:

I dont know why the handler cant extract the params from POST body in .req$parameters_body?

Inside the handler, the body is correct:

> request$body
[1] "foo=bar&a=b"

but the parameters_body is empty

> request$parameters_body
list()
artemklevtsov commented 2 years ago

See example https://github.com/rexyai/RestRserve/blob/master/inst/tinytest/test-cl-request.R#L91-L103 Try request with c("foo" = "bar", "a" = "b") body.

jonekeat commented 2 years ago

See example https://github.com/rexyai/RestRserve/blob/master/inst/tinytest/test-cl-request.R#L91-L103 Try request with c("foo" = "bar", "a" = "b") body.

Weird, I still did not get the request$parameters_body, is the request object must passed to backend$set_request(r, headers = h, body = b) to run something like parse_params_body?

artemklevtsov commented 2 years ago

Parsing an url-encoded body defined here: https://github.com/rexyai/RestRserve/blob/96513b969e1dbcf8e2d14732e0618e607945112e/R/BackendRserve.R#L232-L246 So we simply convert named vector to list, encode it and convert to raw-vector.

jonekeat commented 2 years ago

I see, that makes sense now. because Application$process_request() never pass the request to BackendRserve (so far I didnt saw that, let me know if i am wrong), so when use Application$process_request() the request$parameters_body return list()

  1. When we start the backend, and user send API request, do it pass the request directly to BackendRserve? or it will construct a RestRserve::Request object first then pass to backend?

  2. if the body is parsed correctly in the production setting (with backend serving), do we still need to include these parse_form_urlencoded & other parse functions in Application, just for the sake of testing?

codecov[bot] commented 2 years ago

Codecov Report

Merging #184 (7553d7a) into dev (967c48c) will increase coverage by 0.07%. The diff coverage is 100.00%.

@@            Coverage Diff             @@
##              dev     #184      +/-   ##
==========================================
+ Coverage   95.01%   95.09%   +0.07%     
==========================================
  Files          27       27              
  Lines        1305     1305              
==========================================
+ Hits         1240     1241       +1     
+ Misses         65       64       -1     
Impacted Files Coverage Δ
R/ContentHandlersFactory.R 97.05% <100.00%> (+1.47%) :arrow_up:

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 967c48c...7553d7a. Read the comment docs.

dselivanov commented 2 years ago
  1. When we start the backend, and user send API request, do it pass the request directly to BackendRserve? or it will construct a RestRserve::Request object first then pass to backend?

HTTP request is initially processed by backend (Rserve) and transformed into R object (which is specific per backend). Than this R object is transformed into standard RestRserve::Request and then all operations happen with Request object. And on the way back RestRserve::Response is converted to back-end-specific response.

if the body is parsed correctly in the production setting (with backend serving), do we still need to include these parse_form_urlencoded & other parse functions in Application, just for the sake of testing?

parse_form_urlencoded used internally, no need to call it the application