Skip to content

Commit 810fc5f

Browse files
committed
Make the generated API docs valid Swagger 2.0
The apidocs spec has been `pending 'Is not valid swagger at the moment'` for as long as anyone remembers, so the document /api/v8/apidocs serves — the input to swagger-ui and to any client generator pointed at it — has been free to drift. Validating it against spec/resources/swagger-schema.json reported exactly three errors, all of them malformed swagger metadata in controllers, none of them a problem with the routes they describe. Two path keys were missing their leading slash: api/v8/exercises/{exercise_id}/users/{user_id}/submissions api/v8/exercises/{exercise_id}/users/current/submissions A Swagger 2.0 path key must begin with `/`; these two were the only offenders and they were being dropped by tooling rather than merged with their siblings. /api/v8/core/exercises/details declared its `ids` query parameter with a nested `schema`. Only a body parameter may carry `schema`; every other location describes its type inline, and an array must supply `items`. The parameter already had a redundant `type: :array` next to the schema, so the fix is to keep that and move `items` up beside it. `collectionFormat: csv` is now explicit because the action reads the value with `split(',')`. /api/v8/users/get_user_with_email had a query string embedded in its path key, `/api/v8/users/get_user_with_email?email={email}`, and a `content` key in its 200 response, which is OpenAPI 3 and is not allowed in a Swagger 2.0 response object. The route itself is fine — `get :get_user_with_email, on: :collection`, and the action reads `params[:email]` — so the endpoint is unchanged and only its description is corrected: the path key loses the query string and `email` becomes a query parameter. That also fixes two dangling `$ref`s found while doing it. The referenced `#/parameters/user_email` was never defined (the definition was named `path_user_email`), so the parameter was silently absent from the rendered docs altogether; it is now `query_user_email`, `in: query`, named `email` to match the action. set_password_managed_by_courses_mooc_fi referenced a non-existent `#/parameters/user_id` the same way and now points at `path_user_id`. With those fixed the document validates, so the spec becomes a real test instead of a pending one — the point of keeping it is that it now stops this from regressing. It reports through `fully_validate` rather than the boolean `validate` so a future failure names the offending property instead of just saying `false` is not truthy. A second example asserts no path key contains a `?`. The Swagger 2.0 JSON schema matches path keys with `^/` only, so it accepted the get_user_with_email key and flagged just the `content` alongside it; without this assertion the exact defect that was fixed above could come back unnoticed.
1 parent b9e3c5c commit 810fc5f

5 files changed

Lines changed: 25 additions & 19 deletions

File tree

‎app/controllers/api/v8/apidocs_controller.rb‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -62,9 +62,9 @@ class ApidocsController < ActionController::Base
6262
key :required, true
6363
key :type, :integer
6464
end
65-
parameter :path_user_email do
66-
key :name, :user_email
67-
key :in, :path
65+
parameter :query_user_email do
66+
key :name, :email
67+
key :in, :query
6868
key :description, "User's email"
6969
key :required, true
7070
key :type, :string

‎app/controllers/api/v8/core/exercises/details_controller.rb‎

Lines changed: 5 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,14 +16,12 @@ class DetailsController < Api::V8::BaseController
1616
parameter do
1717
key :in, 'query'
1818
key :name, 'ids'
19-
schema do
20-
key :type, :array
21-
items do
22-
key :type, :integer
23-
end
24-
end
25-
key :type, :array
2619
key :description, 'Exercise Ids'
20+
key :type, :array
21+
key :collectionFormat, 'csv'
22+
items do
23+
key :type, :integer
24+
end
2725
end
2826
response 200 do
2927
key :description, 'Exercises in json'

‎app/controllers/api/v8/exercises/users/submissions_controller.rb‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@ module Users
77
class SubmissionsController < Api::V8::BaseController
88
include Swagger::Blocks
99

10-
swagger_path 'api/v8/exercises/{exercise_id}/users/{user_id}/submissions' do
10+
swagger_path '/api/v8/exercises/{exercise_id}/users/{user_id}/submissions' do
1111
operation :get do
1212
key :description, 'Returns the submissions visible to the user in a json format'
1313
key :operationId, 'findUsersSubmissionsForExerciseById'
@@ -33,7 +33,7 @@ class SubmissionsController < Api::V8::BaseController
3333
end
3434
end
3535

36-
swagger_path 'api/v8/exercises/{exercise_id}/users/current/submissions' do
36+
swagger_path '/api/v8/exercises/{exercise_id}/users/current/submissions' do
3737
operation :get do
3838
key :description, "Returns the current user's submissions for the exercise in a json format. The exercise is searched by id."
3939
key :operationId, 'findUsersOwnSubmissionsForExerciseById'

‎app/controllers/api/v8/users_controller.rb‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,7 @@ class UsersController < Api::V8::BaseController
6262
key :operationId, 'setPasswordManagedByCoursesMoocFi'
6363
key :produces, ['application/json']
6464
key :tags, ['user']
65-
parameter '$ref': '#/parameters/user_id'
65+
parameter '$ref': '#/parameters/path_user_id'
6666
response 403, '$ref': '#/responses/error'
6767
response 404, '$ref': '#/responses/error'
6868
response 200 do
@@ -76,18 +76,17 @@ class UsersController < Api::V8::BaseController
7676
end
7777
end
7878

79-
swagger_path '/api/v8/users/get_user_with_email?email={email}' do
79+
swagger_path '/api/v8/users/get_user_with_email' do
8080
operation :get do
8181
key :description, "Returns the user's id as upstream_id, user's courses.mooc.fi-id as id, email, first name and last name by user email"
8282
key :operationId, 'getUserInformationByEmail'
8383
key :produces, ['application/json']
8484
key :tags, ['user']
85-
parameter '$ref': '#/parameters/user_email'
85+
parameter '$ref': '#/parameters/query_user_email'
8686
response 403, '$ref': '#/responses/error'
8787
response 404, '$ref': '#/responses/error'
8888
response 200 do
8989
key :description, "User's courses.mooc.fi-id as id, email, first name, last name and id as upstream_id as json"
90-
key :content, 'application/json'
9190
schema do
9291
key :title, :user
9392
key :required, [:user]

‎spec/controllers/api/v8/apidocs_controller_spec.rb‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,12 +5,21 @@
55

66
describe Api::V8::ApidocsController, type: :controller do
77
it 'json provided by controller should be valid swagger' do
8-
pending 'Is not valid swagger at the moment'
98
get :index
109
json = response.body
1110

1211
schema = File.join(Rails.root, 'spec', 'resources', 'swagger-schema.json')
13-
validation = JSON::Validator.validate(schema, json)
14-
expect(validation).to be_truthy
12+
errors = JSON::Validator.fully_validate(schema, json)
13+
expect(errors).to be_empty, -> { "Generated apidocs are not valid Swagger 2.0:\n#{errors.join("\n")}" }
14+
end
15+
16+
# The Swagger 2.0 JSON schema only requires a path key to start with a slash, so it
17+
# cannot catch a query string smuggled into the key. Query parameters belong in
18+
# `parameters`, with `in: query`.
19+
it 'declares no path containing a query string' do
20+
get :index
21+
22+
paths = JSON.parse(response.body)['paths'].keys
23+
expect(paths.grep(/\?/)).to be_empty
1524
end
1625
end

0 commit comments

Comments
 (0)