Skip to content

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_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...