diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-04-21 18:21:10 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-04-21 18:21:10 +0300 |
commit | e33f87ac0fabaab468ce4b457996cc0f1b1bb648 (patch) | |
tree | 8bf0de72a9acac014cfdaddab7d463b208294af2 /doc/development/api_styleguide.md | |
parent | 5baf990db20a75078684702782c24399ef9eb0fa (diff) |
Add latest changes from gitlab-org/gitlab@master
Diffstat (limited to 'doc/development/api_styleguide.md')
-rw-r--r-- | doc/development/api_styleguide.md | 64 |
1 files changed, 57 insertions, 7 deletions
diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index 37d8a677389..77e44760ef2 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -9,13 +9,13 @@ to access them as we do in Rails views), local variables are fine. ## Entities -Always use an [Entity] to present the endpoint's payload. +Always use an [Entity](https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/entities) to present the endpoint's payload. ## Documentation API endpoints must come with [documentation](documentation/styleguide.md#api), unless it is internal or behind a feature flag. The docs should be in the same merge request, or, if strictly necessary, -in a follow-up with the same milestone as the original merge request. +in a follow-up with the same milestone as the original merge request. ## Methods and parameters description @@ -29,7 +29,7 @@ for a good example): - If the endpoint is deprecated, and if so, when will it be removed - `params` for the method parameters. This acts as description, - [validation, and coercion of the parameters] + [validation, and coercion of the parameters](https://github.com/ruby-grape/grape#parameter-validation-and-coercion) A good example is as follows: @@ -106,7 +106,7 @@ For `DELETE` requests, you should also generally use the `destroy_conditionally! ## Using API path helpers in GitLab Rails codebase -Because we support [installing GitLab under a relative URL], one must take this +Because we support [installing GitLab under a relative URL](../install/relative_url.md), one must take this into account when using API path helpers generated by Grape. Any such API path helper usage must be in wrapped into the `expose_path` helper call. @@ -121,9 +121,57 @@ For instance: The [internal API](./internal_api.md) is documented for internal use. Please keep it up to date so we know what endpoints different components are making use of. -[Entity]: https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/entities -[validation, and coercion of the parameters]: https://github.com/ruby-grape/grape#parameter-validation-and-coercion -[installing GitLab under a relative URL]: https://docs.gitlab.com/ee/install/relative_url.html +## Avoiding N+1 problems + +In order to avoid N+1 problems that are common when returning collections +of records in an API endpoint, we should use eager loading. + +A standard way to do this within the API is for models to implement a +scope called `with_api_entity_associations` that will preload the +associations and data returned in the API. An example of this scope can +be seen in +[the `Issue` model](https://gitlab.com/gitlab-org/gitlab/blob/2fedc47b97837ea08c3016cf2fb773a0300a4a25/app%2Fmodels%2Fissue.rb#L62). + +In situations where the same model has multiple entities in the API +(for instance, `UserBasic`, `User` and `UserPublic`) you should use your +discretion with applying this scope. It may be that you optimize for the +most basic entity, with successive entities building upon that scope. + +The `with_api_entity_associations` scope will also [automatically preload +data](https://gitlab.com/gitlab-org/gitlab/blob/19f74903240e209736c7668132e6a5a735954e7c/app%2Fmodels%2Ftodo.rb#L34) +for `Todo` _targets_ when returned in the Todos API. + +For more context and discussion about preloading see +[this merge request](https://gitlab.com/gitlab-org/gitlab-foss/merge_requests/25711) +which introduced the scope. + +### Verifying with tests + +When an API endpoint returns collections, always add a test to verify +that the API endpoint does not have an N+1 problem, now and in the future. +We can do this using [`ActiveRecord::QueryRecorder`](query_recorder.md). + +Example: + +```ruby +def make_api_request + get api('/foo', personal_access_token: pat) +end + +it 'avoids N+1 queries', :request_store do + # Firstly, record how many PostgreSQL queries the endpoint will make + # when it returns a single record + create_record + + control = ActiveRecord::QueryRecorder.new { make_api_request } + + # Now create a second record and ensure that the API does not execute + # any more queries than before + create_record + + expect { make_api_request }.not_to exceed_query_limit(control) +end +``` ## Testing @@ -132,3 +180,5 @@ When writing tests for new API endpoints, consider using a schema [fixture](./te ```ruby expect(response).to match_response_schema('merge_requests') ``` + +Also see [verifying N+1 performance](#verifying-with-tests) in tests. |