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 with
find_group. I think
#user_projectis too magical, and it sets
@projectwhich 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_projectis a weird naming since it's doesn't necessarily return a project that belongs to the current user. In that sense
#find_projectis clearer IMO.
- I like the explicitness of calling
project = find_projectin 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?
#group seem a bit too generic...