chore: added API validation for numeric values#826
chore: added API validation for numeric values#826Stellatsuu wants to merge 7 commits intoDIRACGrid:mainfrom
Conversation
aldbr
left a comment
There was a problem hiding this comment.
Thanks @Stellatsuu!
About the client generation, are you using the same version locally than the one in the CI? (normally yes, but worth checking)
| # Set the per_page parameter to 0 | ||
| r = normal_user_client.post("/api/jobs/search", params={"page": 1, "per_page": 0}) | ||
| assert r.status_code == 400, r.json() | ||
| assert r.json()["detail"][0]["msg"] == "Input should be greater than or equal to 1" |
There was a problem hiding this comment.
Any reason for not testing the status code (less fragile than testing the content of the message)
| assert r.json()["detail"][0]["msg"] == "Input should be greater than or equal to 1" | |
| assert r.status_code == 422, r.json() |
| raise HTTPException( | ||
| status_code=HTTPStatus.REQUESTED_RANGE_NOT_SATISFIABLE, | ||
| detail=HTTPStatus.REQUESTED_RANGE_NOT_SATISFIABLE.phrase, | ||
| headers={"Content-Range": f"jobs */{total}"}, | ||
| ) |
There was a problem hiding this comment.
You are changing the behavior of the API here:
- it's a breaking change
- the reason it was made like this are explained in the RFC 7233 linked above:
Note: Because servers are free to ignore Range, many
implementations will simply respond with the entire selected
representation in a 200 (OK) response. That is partly because
most clients are prepared to receive a 200 (OK) to complete the
task (albeit less efficiently) and partly because clients might
not stop making an invalid partial request until they have
received a complete representation. Thus, clients cannot depend
on receiving a 416 (Range Not Satisfiable) response even when it
is most appropriate.
Closes: #506
Changes:
per_pagequery value/jobs/searchpagequery value of/jobs/searchComments:
I can't reproduce this error from the issue, I think it has been fixed somewhere else:
I don't really see other cases here for numeric values. Most numeric values are
job_idsthat are stored in DB and can't be validated from the FastAPI schemas, same for thepagemax value that can't be validated since it's based on DB, anyways, apagevalue out of bound will return a 416 error = no jobs found at thispagevalue (might be why I can't reproduce the error above).As for strings values, I did not find any relevant cases, most relevant ones does already have a validation (such as
max_lengthandregex)