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

Swaggerfile support for array parameters, object, files #532

Closed
wants to merge 42 commits into from

Conversation

meztez
Copy link
Collaborator

@meztez meztez commented Apr 27, 2020

Fixes #187

Add swagger support for array parameters per https://swagger.io/docs/specification/serialization/ using square brackets around typed parameters.

Add swagger support for openapi object type to be listed under requestBody using plumber types list, df and data.frame.

Add swagger/plumber support for files upload, multipart content-type

Add support for detection of endpoint plumberExpression parameters.

Add support for custom body parsers, like serializers, more raw.

Around 30% overheads reduction for POST request with json body.

Syntax support for parameters would become :

  • in: query:
    • name
    • name:type
    • name:type*
    • new name:[type]*
  • in: path
    • <name>
    • <name:type>
    • new <name:[type]>

So something like below works with a matching openapi file to go with it.

library(plumber)

#* Sum numbers
#* @get /names3/<number:[integer]>/path
function(number) {
  sum(c(number, 5L))
}

#* Do something
#* @param iris2:df* Use something like jsonlite::toJSON(list(iris2=iris), auto_unbox = TRUE)
#* @png
#* @post /smoore
function(iris2) {
  iris2$Species <- as.factor(iris2$Species)
  plot(iris)
}

#* Do some more
#* @param owners:[chr]
#* @param dogId:[int]
#* @get /whosagoodboy
function(owners, dogId) {
  #query string is still parsed as character
  dogId <- as.integer(dogId)
  list(sample(owners, 1), sample(dogId , 1))
}

#* Will that work? Test detection, examples and object type
#* @post /risky
function(owners = list(name = c("Bob", "Larry"),
                       cars = c("Corvette", "Tesla")),
         dogId = 1L:2L,
         moredata = women) {
  list(owners_equal = all.equal(owners, list(name = c("Bob", "Larry"), cars = c("Corvette", "Tesla"))),
       dogId_equal = all.equal(as.integer(dogId), 1L:2L),
       moredata_equal = all.equal(moredata, women))
}

#* @param pdf:file
#* @post /upload
function(pdf) {
  f <- tempfile()
  on.exit(unlink(f), add = TRUE)
  writeBin(as.raw(pdf), f, useBytes = TRUE)
  finfo <- file.info(f)
  finfo$mode <- as.character(finfo$mode)
  finfo$real_filename <- attr(pdf, "filename")
  finfo
}

Notes:
object parameters would not be supported in path parameters.
Default upload location set to getOption("plumber.upload.dir", "."). Use multipart parse from mime package. (copied with very slight change to buffer size, filename decoding and file args response)
openapi field example for object feeded from plumberExpression arguments.

Fixes:

  • Swagger definition absent when defining a parameter in path that is not on a @param line.
  • Prevent example from being removed by swagger remove null
  • Swagger parameters now show in the order they were defined

Breaking:
req$postBody is raw since it can contains file and other breaking havoc on string elements.

Related:
#516, #512, #469, #509, #417, #523, #502, #463, #320

At this point I would need help to do more testing across different platforms.

Thank you, comments are welcome.

PR task list:

  • Update NEWS
  • Add tests
  • Update documentation with devtools::document()
    See other PR for documentation, will update there if this one pass.

@CLAassistant
Copy link

@CLAassistant CLAassistant commented Apr 29, 2020

CLA assistant check
All committers have signed the CLA.

@meztez meztez changed the title Swaggerfile support for array parameters and object Swaggerfile support for array parameters, object, files... Apr 29, 2020
@meztez
Copy link
Collaborator Author

@meztez meztez commented May 6, 2020

Performance testing on a puny old laptop from the ages with library loadtest, simple enough.
TLDR : it is faster.

library(plumber)
tmpf <- tempfile()
writeLines('
#* Echo back the input
#* @param msg:character The message to echo
#* @post /test8
function(msg = list(part1 = "a", part1 = "b", part1 = "c")) {
  do.call(paste, msg)
}
', tmpf)
pr <- plumb(tmpf)
pr$run(port = 8477, swagger = FALSE)

plumber_0.4.6.pdf
plumber_0.4.7.9000.pdf
plumber_PR532.pdf

@schloerke schloerke added this to the v0.5.0 - Next CRAN release milestone Jun 8, 2020
@schloerke schloerke added the theme: OpenAPI label Jun 8, 2020
R/parse-block.R Show resolved Hide resolved
@schloerke schloerke self-requested a review Jun 8, 2020
Copy link
Collaborator

@schloerke schloerke left a comment

Good PR to code review tomorrow

R/parse-block.R Outdated Show resolved Hide resolved
R/post-body.R Show resolved Hide resolved
R/post-body.R Outdated Show resolved Hide resolved
R/post-parsers.R Outdated Show resolved Hide resolved
R/post-parsers.R Show resolved Hide resolved
meztez added 4 commits Jun 11, 2020
Merge remote-tracking branch 'upstream/master' into list_plumber_type

# Conflicts:
#	NAMESPACE
#	R/plumber-step.R
R/swagger.R Show resolved Hide resolved
R/swagger.R Show resolved Hide resolved
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
tests/testthat/test-path-subst.R Outdated Show resolved Hide resolved
@cpsievert cpsievert self-requested a review Jun 12, 2020
Copy link
Contributor

@cpsievert cpsievert left a comment

Looking pretty good to me, pending the suggestions. @schloerke is to:

  • Make sure that the unit (or other) tests cover the important code paths.
  • Look into and possibly resolve #532 (comment)
  • In a different PR, leverage webutils::parse_query() inside parseQS()

@schloerke
Copy link
Collaborator

@schloerke schloerke commented Jun 12, 2020

I'm off for the weekend. I'll look into it on Monday. Enjoy your weekend!

@schloerke schloerke changed the title Swaggerfile support for array parameters, object, files... Swaggerfile support for array parameters, object, files Jun 16, 2020
schloerke added a commit that referenced this issue Jun 16, 2020
…532 (#550)

Co-authored-by: Barret Schloerke <barret@rstudio.com>
Co-authored-by: Carson Sievert <cpsievert1@gmail.com>
Co-authored-by: Bruno Tremblay <bruno.tremblay@lacapitale.com>
Co-authored-by: Bruno Tremblay <meztez@neoxone.com>
@meztez meztez mentioned this pull request Jun 18, 2020
30 tasks
@meztez meztez deleted the list_plumber_type branch Jun 24, 2020
@JosiahParry
Copy link
Contributor

@JosiahParry JosiahParry commented Jun 26, 2020

Super excited about multipart file upload! Is this documented? I cannot find it in rplumber.io.

@meztez
Copy link
Collaborator Author

@meztez meztez commented Jun 26, 2020

No documented yet on rplumber.io. I believe the goal is to use pkgdown to build plumber documentation. There is another branch in development for that.

I'm super pump too.

@ThHarbig
Copy link

@ThHarbig ThHarbig commented Jun 30, 2020

I tried it, works great! Is there a way to upload an arbitrary number of files at the same time?

@meztez
Copy link
Collaborator Author

@meztez meztez commented Jun 30, 2020

@ThHarbig It would probably work via curl, I would mostly have to adapt the openapi spec generation. Please open a different issue so I can reference it in a PR.

@chrischung28
Copy link

@chrischung28 chrischung28 commented Jul 2, 2020

This is a great addition! Thank you very much. Just wondering if it would be possible to get an example of the file upload feature? I'm having some trouble getting it to work with the example above when i try to upload some pdfs.

@meztez
Copy link
Collaborator Author

@meztez meztez commented Jul 2, 2020

In the implementation that made it to master, you get a vector of raw bytes to play with. Here is a working example with the current master.

#* @apiTitle Upload test

#* @param pdf:file
#* @post /upload
function(pdf) {
  f <- tempfile()
  on.exit(unlink(f), add = TRUE)
  writeBin(as.raw(pdf), f, useBytes = TRUE)
  finfo <- file.info(f)
  finfo$mode <- as.character(finfo$mode)
  finfo$real_filename <- attr(pdf, "filename")
  finfo
}

@chrischung28
Copy link

@chrischung28 chrischung28 commented Jul 2, 2020

@meztez Thank you very much! This works great.

@s-fleck
Copy link
Contributor

@s-fleck s-fleck commented Oct 21, 2020

@meztez sorry to bug you, but could you provide an example how to use this with httr::POST()? It is not clear to me where the "filename" attributes comes from. Is it possible with this approach to have an endpoint where I can upload with curl -F and httr::POST() without jumping through too many hoops and have the filename available in plumber?

@meztez
Copy link
Collaborator Author

@meztez meztez commented Oct 21, 2020

yeah, the filename comes from the multipart form-data.

plumber::options_plumber(port = 8004)

#* @param a_file:file
#* @post /upload
function(a_file) {
  # Get a named list. The names correspond
  # to the files names. The content of the list
  # will depend on the content-type set
  # for each files.
  return(name_of_file = list(names(a_file), file_content_length = nchar(a_file[[1]])))
}

with httr::POST

f <- tempfile("stempy")
writeLines(stringi::stri_rand_strings(1, 100), f)
res <- httr::POST("http://127.0.0.1:8004/upload", 
                  body = list(a_file = httr::upload_file(f)),
                  encode = "multipart")
httr::content(res)

Should be similar with curl, files is specific to my system

curl -F a_file=@bobettebo.txt http://127.0.0.1:8004/upload

@s-fleck
Copy link
Contributor

@s-fleck s-fleck commented Oct 21, 2020

thanks! i didn't know about httr::upload_file(), i was always sending raw binary data

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

Successfully merging this pull request may close these issues.

8 participants