Commit 149b42ed authored by Sean McGivern's avatar Sean McGivern

Merge branch 'docs-n-plus-one-rest-api' into 'master'

Add docs for how to avoid N+1 problems in the REST API

See merge request gitlab-org/gitlab!17475
parents 603ac77b ef49d773
...@@ -15,7 +15,7 @@ Always use an [Entity] to present the endpoint's payload. ...@@ -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. 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, 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 ## Methods and parameters description
...@@ -125,6 +125,58 @@ different components are making use of. ...@@ -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 [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 [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 ## 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: 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 ...@@ -132,3 +184,5 @@ When writing tests for new API endpoints, consider using a schema [fixture](./te
```ruby ```ruby
expect(response).to match_response_schema('merge_requests') expect(response).to match_response_schema('merge_requests')
``` ```
Also see [verifying N+1 performance](#verifying-with-tests) in tests.
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment