Document a failure entry's entity as its response content

What does this MR do and why?

Closes #17

A failure (or http_codes) entry given as an Array can name the entity that describes its error body as its third element:

desc 'Create a tag' do
  success Entities::Tag
  failure [[400, 'Bad request', Entities::ApiError]]
end

That is also the entity Grape presents the error body with: when an endpoint calls error! with a code, Grape::ErrorFormatter::Base looks for an Array entry with that code and a model (http_code[0] and http_code[2]).

The response converter read only the code and the message of each entry, so an error response never had a schema, and an entity named only in failure entries was never emitted under components.schemas. With this MR an Array entry that names a Grape::Entity class gets that entity's $ref as its application/json content, built the same way as a success entity's, and the entity is emitted beside the success entities. The content describes one object, since the standalone is_array of the desc block applies to the success declaration only. When a Hash entry and an Array entry declare the same code, the response takes the Array entry's entity, the one Grape uses.

What stays as it was, each pinned by a spec:

  • A Hash entry keeps its description-only response whatever its model:. Grape never reads the model of a Hash entry, so documenting it would describe a body Grape does not send.
  • A 204 or 304 response gets no content and registers no entity, since neither has a body.
  • An entry that repeats a code whose success response already has content keeps that content (the existing "keeps the success content" requirement), and its own entity is then not emitted on its account, since that response does not reference it.
  • An entry whose model is not a Grape::Entity class keeps its description-only response. That includes a String class name, the form Grape's README shows in its desc example; this MR does not resolve String names.
  • An entity named only by a failure entry on a hidden true route is not emitted, like a success entity on such a route.

The entity is registered in the response converter, where the response takes it, rather than with the success entities in PathConverter: only there is it known whether a response references it. The comment on PathConverter#register_route_entities now says so.

The project does not squash by default, so the three commits land as they are. Each passes rspec and rubocop on Ruby 3.2, 3.3 and 3.4 on its own:

  1. Pin failure entries whose model does not become content adds TestEntities::ErrorEntity and the specs for the five kept behaviours above (a Hash entry, a 204 or 304 code, a repeated success code, a model that is not an entity, a hidden route). They pass on main.
  2. Read failure entries through one shape turns an Array entry into the Hash form before anything reads it, so the fix has one place to read the model, the way grape-swagger reads failure entries. The output does not change, with one exception: a nil entry used to raise NoMethodError while the converter looked up the declared codes, and is now skipped like any other entry that is neither an Array nor a Hash. A new spec pins that, and fails on main with that NoMethodError.
  3. Document a failure entry's entity as its response content is the change, with nine examples in response_converter_spec.rb and generator_spec.rb; eight of them fail on the previous commit, and the ninth pins the Hash route of the new fixture staying description-only, which it already is there. It also updates the requirements on failure entries in openspec/specs/responses/spec.md and the entity-emission requirement in openspec/specs/entity-schemas/spec.md, and adds a paragraph to the README's "Response media types" section. It carries the Changelog: fixed trailer.

As the issue notes, no failure declaration in gitlab-org/gitlab names a model today: every model: under lib/api and ee/lib/api sits in a success declaration, and no Array entry carries a third element (checked by grep on master at 55bfaf7fe32). I did not regenerate openapi_v3.yaml, but I expect it not to change.

How to set up and validate locally

bundle exec rspec spec/gitlab/grape_openapi/converters/response_converter_spec.rb spec/gitlab/grape_openapi/generator_spec.rb
bundle exec rspec
bundle exec rubocop
The generator and Grape before and after, on a probe API

ProbeEntities::ApiError exposes only error. The probe API has six routes: an Array failure entry naming it beside success ProbeEntities::Thing (array), a Hash entry naming it (hash), a Hash entry naming ProbeEntities::Other followed by an Array entry naming it for the same code (mixed), a 304 entry naming it (not_modified), a 204 entry naming it on a DELETE, and the hash-form desc option http_codes: [[200, 'OK', ProbeEntities::Thing]] (listed). The first three call error!({ error: 'x', extra: 1 }, 409).

What Grape 3.2.1 sends for the first three, the same on main and on this branch:

GET /api/v1/things/array Grape sends 409 {"error":"x"}
GET /api/v1/things/hash Grape sends 409 {"error":"x","extra":1}
GET /api/v1/things/mixed Grape sends 409 {"error":"x"}

On main (f1d2af76):

GET /api/v1/things/array responses: {"200":{"description":"OK","content":{"application/json":{"schema":{"$ref":"#/components/schemas/ProbeEntitiesThing"}}}},"409":{"description":"Conflict"}}
GET /api/v1/things/hash responses: {"200":{"description":"OK"},"409":{"description":"Conflict"}}
GET /api/v1/things/mixed responses: {"200":{"description":"OK"},"409":{"description":"Conflict"}}
GET /api/v1/things/not_modified responses: {"200":{"description":"OK"},"304":{"description":"Not modified"}}
DELETE /api/v1/things/{id} responses: {"204":{"description":"Deleted"},"404":{"description":"Not Found"}}
GET /api/v1/things/listed responses: {"200":{"description":"OK"}}
components.schemas: ["ProbeEntitiesThing"]

On this branch:

GET /api/v1/things/array responses: {"200":{"description":"OK","content":{"application/json":{"schema":{"$ref":"#/components/schemas/ProbeEntitiesThing"}}}},"409":{"description":"Conflict","content":{"application/json":{"schema":{"$ref":"#/components/schemas/ProbeEntitiesApiError"}}}}}
GET /api/v1/things/hash responses: {"200":{"description":"OK"},"409":{"description":"Conflict"}}
GET /api/v1/things/mixed responses: {"200":{"description":"OK"},"409":{"description":"Conflict","content":{"application/json":{"schema":{"$ref":"#/components/schemas/ProbeEntitiesApiError"}}}}}
GET /api/v1/things/not_modified responses: {"200":{"description":"OK"},"304":{"description":"Not modified"}}
DELETE /api/v1/things/{id} responses: {"204":{"description":"Deleted"},"404":{"description":"Not Found"}}
GET /api/v1/things/listed responses: {"200":{"description":"OK","content":{"application/json":{"schema":{"$ref":"#/components/schemas/ProbeEntitiesThing"}}}}}
components.schemas: ["ProbeEntitiesThing", "ProbeEntitiesApiError"]

The same API with the model given as the String 'ProbeEntities::ApiError' produces description-only responses on this branch, as on main.

The new examples on the previous commit

The specs of commit 3 run against the lib/ of commit 2 (Ruby 3.2):

1085 examples, 8 failures
rspec ./spec/gitlab/grape_openapi/converters/response_converter_spec.rb:656
rspec ./spec/gitlab/grape_openapi/converters/response_converter_spec.rb:660
rspec ./spec/gitlab/grape_openapi/converters/response_converter_spec.rb:675
rspec ./spec/gitlab/grape_openapi/converters/response_converter_spec.rb:684
rspec ./spec/gitlab/grape_openapi/converters/response_converter_spec.rb:692
rspec ./spec/gitlab/grape_openapi/converters/response_converter_spec.rb:704
rspec ./spec/gitlab/grape_openapi/generator_spec.rb:340
rspec ./spec/gitlab/grape_openapi/generator_spec.rb:348
Checks I ran locally

In the official ruby:3.2, ruby:3.3 and ruby:3.4 images (3.2.11, 3.3.12, 3.4.11), with grape 3.2.1, grape-entity 1.1.0 and rubocop 1.71.1, on each commit:

Commit bundle exec rspec bundle exec rubocop
main (f1d2af76) 1071 examples, 0 failures 108 files, no offenses
1 1075 examples, 0 failures 109 files, no offenses
2 1076 examples, 0 failures 109 files, no offenses
3 1085 examples, 0 failures 110 files, no offenses

Also:

  • gem build gitlab-grape-openapi.gemspec succeeds on each commit.
  • npx @fission-ai/openspec@1.13.2 validate --all --strict: 14 passed, 0 failed, on each commit.
  • !70 and !76 change README.md, spec/spec_helper.rb or openspec/specs/entity-schemas/spec.md too. git merge-tree against the head of each finds no conflict this branch adds: !76 merges cleanly with it, and !70 conflicts with this branch in the same three files it already conflicts with main in.
Edited by José M. Requena Plens

Merge request reports

Loading
Loading