summaryrefslogtreecommitdiff
path: root/doc/development/api_styleguide.md
diff options
context:
space:
mode:
Diffstat (limited to 'doc/development/api_styleguide.md')
-rw-r--r--doc/development/api_styleguide.md56
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.