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

Add tags to swagger specification #226

Closed
svdwoude opened this issue Jan 4, 2018 · 7 comments
Closed

Add tags to swagger specification #226

svdwoude opened this issue Jan 4, 2018 · 7 comments

Comments

@svdwoude
Copy link
Contributor

svdwoude commented Jan 4, 2018

I would like to make use of the tags to group operations as described in the swagger docs here.

In plumber I'd like to do something like this:

#* @apiTitle Auto Inventory Manager
#* @apiDescription Manage the inventory of an automobile
#*   store using an API.
#* @apiTag section1 description to section 1
#* @apiTag section2 description to section 2
...
#* Lookup a car by ID
#* @param id The ID of the car to get
#* @tags section1
#* @get /car/<id:int>
#* @response 404 No car with the given ID was found in the inventory.
getCar <- function(id, res){
  car <- inventory[inventory$id == id,]
  if (nrow(car) == 0){
    res$status <- 404
  }
  car
}
...
#* Delete a car from the inventory
#* @param id:int The ID of the car to delete
#* @tags section2
#* @delete /car/<id:int>
deleteCar <- function(id, res){
  if (!(id %in% inventory$id)){
    res$status <- 400
    return(list(errors=paste0("No such ID: ", id)))
  }
  inventory <<- inventory[inventory$id != id,]
}

Looking into the codebase I believe this would require two changes:

  • prepareSwaggerEndpoints in swagger.R to extract the @tags decorator on the endpoints
  • parseOneGlobal in parse-globals.R to extract the @apiTag decorators that hold the name and description of each tag similiar to how @param gets parsed on endpoints

Would you accept a pull request to add this functionality?

@svdwoude svdwoude changed the title add tags to swagger specification Add tags to swagger specification Jan 4, 2018
@trestletech
Copy link
Contributor

That sounds great! If the PR had tests to cover the change, I'd definitely welcome it!

The only other thing I'd want to investigate is whether or not an endpoint can/should support multiple tags. And how we resolve errors if the referenced @tag hasn't been defined as an @apiTag.

@svdwoude
Copy link
Contributor Author

Perfect! I'll make sure to add proper testing.

For the @tags semantics, I'll explore how swagger deals with multiple tags and tags that haven't been defined and try to leverage as much of it's built-in fallbacks/behaviour. If things wouldn't break on the swagger side, I'd propose to throw warnings rather than errors.

I'll come back with a concrete proposition on how to handle it.

@trestletech
Copy link
Contributor

Yup! That sounds right to me. I'd hope to follow Swagger's lead on this one and support whatever they support.

@svdwoude
Copy link
Contributor Author

svdwoude commented Jan 16, 2018

I implemented the functionality, still need to add some proper tests.

Current implementation works like this

@apiTitle Tags Example
@apiTag name1 description1
@apiTag name2 description2


#* Example endpoint with one tag
#* @tag name1
function() {}

#* Example endpoint with multiple tags
#* @tag name1
#* @tag name2
function() {}

#* Example endpoint with unmatched tags
#* @tag randomName
function() {}

Couple of notes:

  • Unmatched tags are allowed by swagger and will be showed. You just won't find a description added to the randomName tag in your swagger-ui. This reduces the global @apiTag just to provide a description.
  • I did not add functionality to parse externalDocs from @apiTag since I can't come up with a feasible consistent syntax.
  • Important: Tags defined in @apiTag can only consist of one word (to mimic @param behaviour) the rest is parsed as description.

Any feedback before I writing some tests?

@trestletech
Copy link
Contributor

Looks like a good start to me!

  1. Is it common to have whitespace in tag names? Looks like the de facto example only has single-word tags. If so we could look at supporting quoting for the tag name (@apiTag "multi word name" description here), though it's a little weird that the name is quoted but the description isn't. So I don't really love that. Maybe we start with single-word tags.
  2. It looks like @tag isn't used by roxygen right now, but I'm slightly worried about collision. I can't think of a great alternative, though... So I guess I vote to leave it as is.

Tl;dr: looks good!

@svdwoude
Copy link
Contributor Author

Cool!

I've added a commit (e1c1f42) that would allow for tag names with whitespaces. You would just use 'underscores ' to indicate spaces. During parsing I gsub all underscores into spaces in @apiTag names and @tag references. If you believe this is not consistent or possibly confusing, feel free to exclude it form the PR, but it seemed like an easy solution to allow for move 'verbose' tags.

Minimal example:

#* @apiTitle Tags Example
#* @apiTag name_with_whitespace description1

#* Example endpoint with one tag
#* @tag name_with_whitespace
function() {}

which would be displayed in swagger-ui as "name with whitespace"

@trestletech
Copy link
Contributor

Closed by #230

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

2 participants