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 fixed fields #431

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .rubocop_todo.yml
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ Metrics/AbcSize:
# Offense count: 3
# Configuration parameters: CountComments.
Metrics/ClassLength:
Max: 201
Max: 210

# Offense count: 10
Metrics/CyclomaticComplexity:
Expand Down
4 changes: 1 addition & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,10 @@

#### Features

* Your contribution here.
* [#431](https://github.com/ruby-grape/grape-swagger/pull/431): Add summary and deprecated to the operation object generator to be more compliant with [OpenAPI v2](https://github.com/OAI/OpenAPI-Specification/blob/master/versions/2.0.md#operation-object) - [@aschuster3](https://github.com/aschuster3).

#### Fixes

* Your contribution here.

### 0.20.4 (May 16, 2016)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Put this back please.


#### Features
Expand Down
46 changes: 46 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -442,6 +442,52 @@ end
```


#### Overriding the route summary

By default, the route summary is filled with the value supplied to `desc`.

```ruby
namespace 'order' do
desc 'This will be your summary'
get :order_id do
...
end
end
```

To override the summary, add `summary: '[string]'` after the description.

```ruby
namespace 'order' do
desc 'This will be your summary',
summary: 'Now this is your summary!'
get :order_id do
...
end
end
```


#### Deprecate routes

For route deprecation, add `deprecated: true` after your description.

```ruby
namespace 'order' do
desc 'Old way to do things',
deprecated: true
get :old do
...
end

desc 'New way to do things'
get :new do
...
end
end
```


#### Expose nested namespace as standalone route

Use the `nested: false` property in the `swagger` option to make nested namespaces appear as standalone resources.
Expand Down
16 changes: 14 additions & 2 deletions lib/grape-swagger/endpoint.rb
Original file line number Diff line number Diff line change
Expand Up @@ -103,18 +103,28 @@ def path_item(routes, options)

def method_object(route, options, path)
method = {}
method[:summary] = summary_object(route)
method[:description] = description_object(route, options[:markdown])
method[:produces] = produces_object(route, options[:produces] || options[:format])
method[:consumes] = consumes_object(route, options[:format])
method[:parameters] = params_object(route)
method[:responses] = response_object(route, options[:markdown])
method[:tags] = tag_object(route, options[:version].to_s)
method[:deprecated] = deprecated_object(route)
method[:operationId] = GrapeSwagger::DocMethods::OperationId.build(route, path)
method.delete_if { |_, value| value.blank? }

[route.request_method.downcase.to_sym, method]
end

def summary_object(route)
summary = route.options[:desc] if route.options.key?(:desc)
summary = route.description if route.description.present?
Copy link
Member

@LeFnord LeFnord May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this assumes, that, if summary not given it is the same as description,
I ask me if it is intended by the spec …
or should it be set only if given?

Copy link
Contributor Author

@aschuster3 aschuster3 May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This part was mostly interpretation, but when comparing Open API v1.2 Operation Object and Open API v2 Operation Object specs, summary and notes are the equivalent to summary and description, so when upgrading from v1.2 to v2 I would expect:

desc 'Add a new pet to the store' do
    detail 'More details'
end

To produce:

{
  ...
  "summary": "Add a new pet to the store",
  "description": "Add a new pet to the store\n More details",
  ...
}

More importantly, I base on what appears in SwaggerUI so that the route summary appears next to the route name, as pictured:

Pet Store Summary

This was the default behavior for when I used grape-swagger version <= 0.11 (something about SwaggerUI may have changed as well), but I'm open to discussion.

Copy link
Member

@LeFnord LeFnord May 20, 2016

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm also open for proposals 😉

want to say I'm fine with your suggestion above,
maybe you could split of the PR into the deprecation stuff and summary/description

I want only one point mention, yeap swagger-ui is changing a lot at the moment,
that's why it should be used for view stuff, but not for checking of validity of the generated swagger,
in normal cases, one should prefer http://bigstickcarpet.com/swagger-parser/www/index.html, over it, or one should use ngrok (or similar), then this would also be validated

cause IMO a valid swagger is much more important then a nice looking one

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Alright, I will split this PR into two separate ones to keep the changes atomic. I will close this one and open another.

summary = route.options[:summary] if route.options.key?(:summary)

summary
end

def description_object(route, markdown)
description = route.options[:desc] if route.options.key?(:desc)
description = route.description if route.description.present?
Expand Down Expand Up @@ -189,6 +199,10 @@ def tag_object(route, version)
Array(route.path.split('{')[0].split('/').reject(&:empty?).delete_if { |i| ((i == route.prefix.to_s) || (i == version)) }.first)
end

def deprecated_object(route)
route.options.key?(:deprecated) && route.options[:deprecated]
end

private

def partition_params(route)
Expand Down Expand Up @@ -237,9 +251,7 @@ def expose_params_from_model(model)

GrapeSwagger.model_parsers.each do |klass, ancestor|
next unless model.ancestors.map(&:to_s).include?(ancestor)

parser = klass.new(model, self)

break
end

Expand Down
4 changes: 4 additions & 0 deletions spec/issues/403_versions_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@ def app
paths: {
:'/nothings' => {
get: {
summary: 'no versions given',
description: 'no versions given',
produces: ['application/json'],
responses: {
Expand Down Expand Up @@ -71,6 +72,7 @@ def app
paths: {
:'/v2/api_version' => {
get: {
summary: 'api versions given',
description: 'api versions given',
produces: ['application/json'],
responses: {
Expand Down Expand Up @@ -112,6 +114,7 @@ def app
paths: {
:'/doc_version' => {
get: {
summary: 'doc versions given',
description: 'doc versions given',
produces: ['application/json'],
responses: {
Expand Down Expand Up @@ -154,6 +157,7 @@ def app
paths: {
:'/v2/both_versions' => {
get: {
summary: 'both versions given',
description: 'both versions given',
produces: ['application/json'],
responses: {
Expand Down
8 changes: 8 additions & 0 deletions spec/support/model_parsers/entity_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,7 @@ class RecursiveModel < Grape::Entity
'paths' => {
'/v3/other_thing/{elements}' => {
'get' => {
'summary' => 'nested route inside namespace',
'description' => 'nested route inside namespace',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'body', 'name' => 'elements', 'description' => 'Set of configuration', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true }],
Expand All @@ -204,6 +205,7 @@ class RecursiveModel < Grape::Entity
},
'/thing' => {
'get' => {
'summary' => 'This gets Things.',
'description' => 'This gets Things.',
'produces' => ['application/json'],
'parameters' => [
Expand All @@ -217,6 +219,7 @@ class RecursiveModel < Grape::Entity
'operationId' => 'getThing'
},
'post' => {
'summary' => 'This creates Thing.',
'description' => 'This creates Thing.',
'produces' => ['application/json'],
'consumes' => ['application/json'],
Expand All @@ -231,6 +234,7 @@ class RecursiveModel < Grape::Entity
},
'/thing/{id}' => {
'get' => {
'summary' => 'This gets Thing.',
'description' => 'This gets Thing.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand All @@ -239,6 +243,7 @@ class RecursiveModel < Grape::Entity
'operationId' => 'getThingId'
},
'put' => {
'summary' => 'This updates Thing.',
'description' => 'This updates Thing.',
'produces' => ['application/json'],
'consumes' => ['application/json'],
Expand All @@ -252,6 +257,7 @@ class RecursiveModel < Grape::Entity
'operationId' => 'putThingId'
},
'delete' => {
'summary' => 'This deletes Thing.',
'description' => 'This deletes Thing.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand All @@ -262,6 +268,7 @@ class RecursiveModel < Grape::Entity
},
'/thing2' => {
'get' => {
'summary' => 'This gets Things.',
'description' => 'This gets Things.',
'produces' => ['application/json'],
'responses' => { '200' => { 'description' => 'get Horses', 'schema' => { '$ref' => '#/definitions/Something' } }, '401' => { 'description' => 'HorsesOutError', 'schema' => { '$ref' => '#/definitions/ApiError' } } },
Expand All @@ -271,6 +278,7 @@ class RecursiveModel < Grape::Entity
},
'/dummy/{id}' => {
'delete' => {
'summary' => 'dummy route.',
'description' => 'dummy route.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand Down
8 changes: 8 additions & 0 deletions spec/support/model_parsers/mock_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -184,6 +184,7 @@ class RecursiveModel < OpenStruct; end
'paths' => {
'/v3/other_thing/{elements}' => {
'get' => {
'summary' => 'nested route inside namespace',
'description' => 'nested route inside namespace',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'body', 'name' => 'elements', 'description' => 'Set of configuration', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true }],
Expand All @@ -196,6 +197,7 @@ class RecursiveModel < OpenStruct; end
},
'/thing' => {
'get' => {
'summary' => 'This gets Things.',
'description' => 'This gets Things.',
'produces' => ['application/json'],
'parameters' => [
Expand All @@ -209,6 +211,7 @@ class RecursiveModel < OpenStruct; end
'operationId' => 'getThing'
},
'post' => {
'summary' => 'This creates Thing.',
'description' => 'This creates Thing.',
'produces' => ['application/json'],
'consumes' => ['application/json'],
Expand All @@ -223,6 +226,7 @@ class RecursiveModel < OpenStruct; end
},
'/thing/{id}' => {
'get' => {
'summary' => 'This gets Thing.',
'description' => 'This gets Thing.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand All @@ -231,6 +235,7 @@ class RecursiveModel < OpenStruct; end
'operationId' => 'getThingId'
},
'put' => {
'summary' => 'This updates Thing.',
'description' => 'This updates Thing.',
'produces' => ['application/json'],
'consumes' => ['application/json'],
Expand All @@ -244,6 +249,7 @@ class RecursiveModel < OpenStruct; end
'operationId' => 'putThingId'
},
'delete' => {
'summary' => 'This deletes Thing.',
'description' => 'This deletes Thing.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand All @@ -254,6 +260,7 @@ class RecursiveModel < OpenStruct; end
},
'/thing2' => {
'get' => {
'summary' => 'This gets Things.',
'description' => 'This gets Things.',
'produces' => ['application/json'],
'responses' => { '200' => { 'description' => 'get Horses', 'schema' => { '$ref' => '#/definitions/Something' } }, '401' => { 'description' => 'HorsesOutError', 'schema' => { '$ref' => '#/definitions/ApiError' } } },
Expand All @@ -263,6 +270,7 @@ class RecursiveModel < OpenStruct; end
},
'/dummy/{id}' => {
'delete' => {
'summary' => 'dummy route.',
'description' => 'dummy route.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand Down
8 changes: 8 additions & 0 deletions spec/support/model_parsers/representable_parser.rb
Original file line number Diff line number Diff line change
Expand Up @@ -261,6 +261,7 @@ class RecursiveModel < Representable::Decorator
'paths' => {
'/v3/other_thing/{elements}' => {
'get' => {
'summary' => 'nested route inside namespace',
'description' => 'nested route inside namespace',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'body', 'name' => 'elements', 'description' => 'Set of configuration', 'type' => 'array', 'items' => { 'type' => 'string' }, 'required' => true }],
Expand All @@ -273,6 +274,7 @@ class RecursiveModel < Representable::Decorator
},
'/thing' => {
'get' => {
'summary' => 'This gets Things.',
'description' => 'This gets Things.',
'produces' => ['application/json'],
'parameters' => [
Expand All @@ -286,6 +288,7 @@ class RecursiveModel < Representable::Decorator
'operationId' => 'getThing'
},
'post' => {
'summary' => 'This creates Thing.',
'description' => 'This creates Thing.',
'produces' => ['application/json'],
'consumes' => ['application/json'],
Expand All @@ -300,6 +303,7 @@ class RecursiveModel < Representable::Decorator
},
'/thing/{id}' => {
'get' => {
'summary' => 'This gets Thing.',
'description' => 'This gets Thing.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand All @@ -308,6 +312,7 @@ class RecursiveModel < Representable::Decorator
'operationId' => 'getThingId'
},
'put' => {
'summary' => 'This updates Thing.',
'description' => 'This updates Thing.',
'produces' => ['application/json'],
'consumes' => ['application/json'],
Expand All @@ -321,6 +326,7 @@ class RecursiveModel < Representable::Decorator
'operationId' => 'putThingId'
},
'delete' => {
'summary' => 'This deletes Thing.',
'description' => 'This deletes Thing.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand All @@ -331,6 +337,7 @@ class RecursiveModel < Representable::Decorator
},
'/thing2' => {
'get' => {
'summary' => 'This gets Things.',
'description' => 'This gets Things.',
'produces' => ['application/json'],
'responses' => { '200' => { 'description' => 'get Horses', 'schema' => { '$ref' => '#/definitions/Something' } }, '401' => { 'description' => 'HorsesOutError', 'schema' => { '$ref' => '#/definitions/ApiError' } } },
Expand All @@ -340,6 +347,7 @@ class RecursiveModel < Representable::Decorator
},
'/dummy/{id}' => {
'delete' => {
'summary' => 'dummy route.',
'description' => 'dummy route.',
'produces' => ['application/json'],
'parameters' => [{ 'in' => 'path', 'name' => 'id', 'type' => 'integer', 'format' => 'int32', 'required' => true }],
Expand Down
46 changes: 46 additions & 0 deletions spec/swagger_v2/api_swagger_v2_deprecation_spec.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
require 'spec_helper'

describe 'swagger spec v2.0 deprecation' do
include_context "#{MODEL_PARSER} swagger example"

def app
Class.new(Grape::API) do
format :json

desc 'This is a test sample', deprecated: true
get '/old' do
present true
end

desc 'This is another test sample', deprecated: false
get '/new' do
present true
end

version 'v1', using: :path
add_swagger_documentation api_version: 'v1',
base_path: '/api',
info: {
title: 'The API title to be displayed on the API homepage.',
description: 'A description of the API.',
contact_name: 'Contact name',
contact_email: 'Contact@email.com',
contact_url: 'Contact URL',
license: 'The name of the license.',
license_url: 'www.The-URL-of-the-license.org',
terms_of_service_url: 'www.The-URL-of-the-terms-and-service.com'
}
end
end

before do
get '/v1/swagger_doc'
end

let(:json) { JSON.parse(last_response.body) }

describe 'deprecation' do
it { expect(json['paths']['/old']['get']['deprecated']).to eql true }
it { expect(json['paths']['/new']['get']).to_not include 'deprecated' }
end
end
Loading