Improve consistency in the way we retrieve project & group in API endpoints
Currently we have #user_project
, which calls #find_project
internally and memoize the project in @project
. Note that this method acts as a finder but also as a permission checker, returning an early 404 if the project is not found or the user cannot see it.
Then we have #find_group
which try to find a group, and return an early 404 if the group is not found or the user cannot see it (no memoization here).
I suggested the following change in https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/4833#note_13572590:
I think we should be explicit and coherent, and always use
find_project
, as we do withfind_group
. I think#user_project
is too magical, and it sets@project
which is unexpected to me.
To further explain my comment:
- I don't think the memoization is really necessary since we pass explicit arguments to API entities (in contrary to the implicit controller instance arguments for views). That said, I don't mind that much the memoization.
-
#user_project
is a weird naming since it's doesn't necessarily return a project that belongs to the current user. In that sense#find_project
is clearer IMO. - I like the explicitness of calling
project = find_project
in the API methods that expect a project, I'd even argue we should rename it to#find_project!
since it acts as a bang finder + permission checker.
Another alternative would be to change #find_group
to be consistent with the current #user_project
, but in that case, I think we should at least rename these methods, but to what? #project
& #group
seem a bit too generic...