swagger function does not handle tags correctly

[eduardszoecs]

plumber with new swagger functionality does not handle tags of lenght one correctly.

Here is a reprex.

With only one tag in /users this produces:

http://localhost:1234/openapi.json:

{"openapi":"3.0.3","info":{"description":"An API","version":"0.1.1.9000","title":"An API"},"tags":[{"name":"tag1","description":"tag1"}],"paths":{"/users":{"get":{"summary":"get endpoint","tags":"tag1","description":"an endpoint","parameters":[{"name":"parameter","in":"query","description":"a parameters","required":false,"schema":{"type":"string"}}],"responses":{"204":{"description":"No match not found."}}}}}}

With two tags this works as expected:

http://localhost:1234/openapi.json:

{"openapi":"3.0.3","info":{"description":"An API","version":"0.1.1.9000","title":"An API"},"tags":[{"name":"tag1","description":"tag1"}],"paths":{"/users":{"get":{"summary":"get endpoint","tags":["tag1","tags2"],"description":"an endpoint","parameters":[{"name":"parameter","in":"query","description":"a parameters","required":false,"schema":{"type":"string"}}],"responses":{"204":{"description":"No match not found."}}}}}}

Digging into plumbers, the problems is here which auto_unboxes all atomic vectors of length 1, which should not be done with tags.

[schloerke]

I believe this is a bug within yaml::read_yaml. The tags supplied in spec for the swagger function have already been wrapped with I(tags) to prevent unboxing. The read_yaml function does not know if it should be an atomic value or a list when reading, so it chooses string value.

[eduardszoecs]

@schloerke That's true for the decorators (see https://github.com/trestletech/plumber/blob/e3610056929753fb009bfad9fe34620eab87d36a/R/plumber-step.R#L170), but not if we supply a custom swagger function? Or is this in the swagger package?

[eduardszoecs]

@schloerke You have a PR ;)

[schloerke]

The spec supplied to the swagger function will work for swagger UI in the swagger package.

With how complicated the OpenAPI specification object can get, I am not doing any validation or verification on the object returned from the swagger function, only passing it directly to Swagger UI by way of serializer_unboxed_json().

In your reprex, spec is being completely overwritten by the output from yaml::read_yaml. I strongly believe this is where the error is being introduced.

I would upgrade your function to use your box_tags function.

pr <- plumber::plumb('~/tmp/tags.R')
pr$run(port = 1234, swagger = function(pr, spec, ...) {
  spec <- yaml::read_yaml('~/tmp/swagger.yml')
  box_tags(spec)
})

[eduardszoecs]

@schloerke Thanks for your input. I'll check where the error originates.

[eduardszoecs]

@schloerke Here is an example without yaml::read_yaml:

pr <- plumber::plumber$new()
pr$handle("GET", "/users", function(){})
pr$run(port = 1234, swagger = function(pr_, spec, ...) {
  spec$paths$`/users`$get$tags <- 'tag1'
  spec
  }
)

With two tags it works (because it will not be auto_unboxed):

pr <- plumber::plumber$new()
pr$handle("GET", "/users", function(){})
pr$run(port = 1234, swagger = function(pr_, spec, ...) {
  spec$paths$`/users`$get$tags <- c('tag1', 'tag2')
  spec
  }
)

My PR fixes this.

[schloerke]

With how complicated the OpenAPI specification object can get, I am not doing any validation or verification on the object returned from the swagger function, only passing it directly to Swagger UI by way of serializer_unboxed_json().

This feature was originally added to allow users to upgrade their swagger spec without having to hack the R6 plumber object. If users are using this function, I am under the impression that they will produce a valid specification object.

I'm still wanting to take the stance that plumber not alter how the upgraded specification is handled until we can officially validate that the full specification being returned is valid.

---

Yes, your example above displays how users will/can run into issues. I'm going to keep your PR open as a reminder to implement a way validate the full swagger spec.

I have hooks to officially validate swagger specifications within the testing suite in https://github.com/trestletech/plumber/blob/master/tests/testthat/test-swagger.R#L178-L246 . I don't know if the validation belongs as a separate R pkg or as a method with the plumber R pkg or swagger R pkg . I'd rather it be using the v8 R package with a bundled javascript file. v8 unfortunately makes things harder to install, making it an opt in situation.

[eduardszoecs]

I'm still wanting to take the stance that plumber not alter how the upgraded specification is handled until we can officially validate that the full specification being returned is valid.

But doesn't plumber already alter the upgraded specification by auto_unboxing length-1 tags?
Because by doing so it currently produces an invalid specification ("tags should be an array").

On the other hand:

pr <- plumber::plumber$new()
pr$handle("GET", "/users", function(){})
pr$run(port = 1234, swagger = function(pr_, spec, ...) {
  spec$paths$`/users`$get$tags <- list('tag1')
  spec
  }
)

works as expected (avoiding auto_unboxing). So one could argue, users should always pass tags as a list.

I don't know who is in charge here:

1. users making sure that length-1 tags are always lists or taking other means to avoid auto_unboxing
2. plumber making sure that it produces valid specs by not auto_unboxing length-1 tags

?

[eduardszoecs]

After rethinking this, I think that users should just ensure that tags are always lists when modifying the spec.

[schloerke]

Closing this issue in favor of wanting to validate the whole swagger spec in issue #397

[eduardszoecs]

Just as an addendum: This can be fixed when using yaml::read_yaml like this:

library("plumber")
plumb("api.R")$run(
  host = host,
  port = port, 
  swagger = function(pr, spec, ...) {
    spec <- yaml::read_yaml("swagger.yml", handlers = list(seq = function(x) x))
    spec
  })

[s-fleck]

Is there a similar way to use custom swagger files when deploying the API to rsconnect and co?

[schloerke]

@s-fleck Currently, if you have the github version, you can do this with any plumber deployment.

(For new questions, please make a new issue referencing any other issues to keep conversations/ideas separate. Thank you!)