diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-04-20 18:38:24 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-04-20 18:38:24 +0000 |
commit | 983a0bba5d2a042c4a3bbb22432ec192c7501d82 (patch) | |
tree | b153cd387c14ba23bd5a07514c7c01fddf6a78a0 /doc/development/api_styleguide.md | |
parent | a2bddee2cdb38673df0e004d5b32d9f77797de64 (diff) | |
download | gitlab-ce-983a0bba5d2a042c4a3bbb22432ec192c7501d82.tar.gz |
Add latest changes from gitlab-org/gitlab@12-10-stable-ee
Diffstat (limited to 'doc/development/api_styleguide.md')
-rw-r--r-- | doc/development/api_styleguide.md | 56 |
1 files changed, 55 insertions, 1 deletions
diff --git a/doc/development/api_styleguide.md b/doc/development/api_styleguide.md index 37d8a677389..78e35023766 100644 --- a/doc/development/api_styleguide.md +++ b/doc/development/api_styleguide.md @@ -15,7 +15,7 @@ Always use an [Entity] to present the endpoint's payload. 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 @@ -125,6 +125,58 @@ different components are making use of. [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 When writing tests for new API endpoints, consider using a schema [fixture](./testing_guide/best_practices.md#fixtures) located in `/spec/fixtures/api/schemas`. You can `expect` a response to match a given schema: @@ -132,3 +184,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. |