summaryrefslogtreecommitdiff
path: root/doc/development/api_styleguide.md
blob: e9a41e7ec58dfbf22012c4b750e2aa940d2db4a4 (plain)
1
2
3
4
5
6
7
8
9
10
11
12
13
14
15
16
17
18
19
20
21
22
23
24
25
26
27
28
29
30
31
32
33
34
35
36
37
38
39
40
41
42
43
44
45
46
47
48
49
50
51
52
53
54
55
56
57
58
59
60
61
62
63
64
65
66
67
68
69
70
71
72
73
74
75
76
77
78
79
80
81
82
83
84
85
86
87
88
89
90
91
92
93
94
95
96
97
98
99
100
101
102
103
104
105
106
107
108
109
110
111
112
113
114
115
116
117
118
119
120
121
122
123
124
125
126
127
128
129
130
131
132
133
134
135
136
137
138
139
140
141
142
143
144
145
146
147
148
149
150
151
152
153
154
155
156
157
158
159
160
161
162
163
164
165
166
167
168
169
170
171
172
173
174
175
176
177
178
179
180
181
182
183
184
# API style guide

This style guide recommends best practices for API development.

## Instance variables

Please do not use instance variables, there is no need for them (we don't need
to access them as we do in Rails views), local variables are fine.

## Entities

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.

## Methods and parameters description

Every method must be described using the [Grape DSL](https://github.com/ruby-grape/grape#describing-methods)
(see <https://gitlab.com/gitlab-org/gitlab/blob/master/lib/api/environments.rb>
for a good example):

- `desc` for the method summary. You should pass it a block for additional
  details such as:
  - The GitLab version when the endpoint was added. If it is behind a feature flag, mention that instead: _This feature is gated by the :feature\_flag\_symbol feature flag._
  - 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](https://github.com/ruby-grape/grape#parameter-validation-and-coercion)

A good example is as follows:

```ruby
desc 'Get all broadcast messages' do
  detail 'This feature was introduced in GitLab 8.12.'
  success Entities::BroadcastMessage
end
params do
  optional :page,     type: Integer, desc: 'Current page number'
  optional :per_page, type: Integer, desc: 'Number of messages per page'
end
get do
  messages = BroadcastMessage.all

  present paginate(messages), with: Entities::BroadcastMessage
end
```

## Declared parameters

> Grape allows you to access only the parameters that have been declared by your
`params` block. It filters out the parameters that have been passed, but are not
allowed.

– <https://github.com/ruby-grape/grape#declared>

### Exclude parameters from parent namespaces

> By default `declared(params)`includes parameters that were defined in all
parent namespaces.

– <https://github.com/ruby-grape/grape#include-parent-namespaces>

In most cases you will want to exclude parameters from the parent namespaces:

```ruby
declared(params, include_parent_namespaces: false)
```

### When to use `declared(params)`

You should always use `declared(params)` when you pass the parameters hash as
arguments to a method call.

For instance:

```ruby
# bad
User.create(params) # imagine the user submitted `admin=1`... :)

# good
User.create(declared(params, include_parent_namespaces: false).to_h)
```

>**Note:**
`declared(params)` return a `Hashie::Mash` object, on which you will have to
call `.to_h`.

But we can use `params[key]` directly when we access single elements.

For instance:

```ruby
# good
Model.create(foo: params[:foo])
```

## Using HTTP status helpers

For non-200 HTTP responses, use the provided helpers in `lib/api/helpers.rb` to ensure correct behavior (`not_found!`, `no_content!` etc.). These will `throw` inside Grape and abort the execution of your endpoint.

For `DELETE` requests, you should also generally use the `destroy_conditionally!` helper which by default returns a `204 No Content` response on success, or a `412 Precondition Failed` response if the given `If-Unmodified-Since` header is out of range. This helper calls `#destroy` on the passed resource, but you can also implement a custom deletion method by passing a block.

## Using API path helpers in GitLab Rails codebase

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.

For instance:

```haml
- endpoint = expose_path(api_v4_projects_issues_related_merge_requests_path(id: @project.id, issue_iid: @issue.iid))
```

## Internal API

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.

## 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:

```ruby
expect(response).to match_response_schema('merge_requests')
```

Also see [verifying N+1 performance](#verifying-with-tests) in tests.