feat: add MAX_ITEMS_PER_QUERY #175
Conversation
| 'MAX_BATCH_MUTATION', | ||
| 'LOGGER_.+', | ||
| 'ROBOTS_TXT', | ||
| 'MAX_RELATIONAL_DEPTH', |
There was a problem hiding this comment.
It's not present in the rest of the code ?
There was a problem hiding this comment.
It already existed; it just wasn’t present here like all environment variables. Here’s an example of a call:
const maxRelationalDepth = Number(env['MAX_RELATIONAL_DEPTH']) > 2 ? Number(env['MAX_RELATIONAL_DEPTH']) : 2;
api/src/utils/sanitize-query.ts
Outdated
| let limit = sanitizeLimit(rawQuery['limit']); | ||
|
|
||
| if (typeof limit === 'number') { | ||
| const maxItemsPerQuery = Number(env['MAX_ITEMS_PER_QUERY']); |
There was a problem hiding this comment.
env already has a functionality to format env value.
It uses either typeMap or a type prefix in the env variable.
Would it be possible to use this instead ?
There was a problem hiding this comment.
Alright, I will look into that. I based myself on the existing practices on the following line:
const maxRelationalDepth = Number(env['MAX_RELATIONAL_DEPTH']) > 2 ? Number(env['MAX_RELATIONAL_DEPTH']) : 2;
There was a problem hiding this comment.
It seems to be automatically converted with:
if (
String(value).startsWith('0') === false &&
isNaN(value) === false &&
value.length > 0 &&
value <= Number.MAX_SAFE_INTEGER
) {
env[key] = Number(value);
continue;
}
It could be enforced by adding it in typeMap.
There was a problem hiding this comment.
then we can remove the Number(...) constructor ?
api/src/utils/sanitize-query.ts
Outdated
| if (typeof limit === 'number') { | ||
| const maxItemsPerQuery = Number(env['MAX_ITEMS_PER_QUERY']); | ||
|
|
||
| if (maxItemsPerQuery !== -1) { |
There was a problem hiding this comment.
maxItemsPerQuery !== -1 && limit === -1 ?
api/src/utils/validate-query.ts
Outdated
| } | ||
|
|
||
| function validateLimit(limit: any) { | ||
| const maxItemsPerQuery = Number(env['MAX_ITEMS_PER_QUERY']) || 1000; |
There was a problem hiding this comment.
Can be manage by the getEnv and env definition.
There was a problem hiding this comment.
Indeed, moreover, I initialized it in defaults.
api/src/utils/validate-query.ts
Outdated
| function validateLimit(limit: any) { | ||
| const maxItemsPerQuery = Number(env['MAX_ITEMS_PER_QUERY']) || 1000; | ||
|
|
||
| if (limit !== -1 && maxItemsPerQuery !== -1) { |
There was a problem hiding this comment.
maxItemsPerQuery !== -1 is sufficient here ?
There was a problem hiding this comment.
Indeed, that was the initial test before adding the sanitize.
Change description
Type of change
Related issues
Checklists
Development
Security
Network
Code review