Remove unneeded overrides of find_object
What does this MR do and why?
This MR removes around 30 unneeded overrides of the find_object
method in GraphQL mutations.
This is a second pass at cleaning up duplicate instances of this method, see !114555 (merged) for details of the first pass.
Many of the removed methods make use of an :expected_type
check.
This is no longer required in most cases (i.e. those removed here)
so these mutations do not need to override the base class implementation of find_object
.
The :expected_type
checks date back to when ids were all represented
by a single generic ID
type. This has been replaced with specific
types of the form GlobalIdType[::Project]
, GlobalIdType[::Issue]
,
etc.
These specific types already handle the type checking when the GID argument is
parsed.
This happens before resolve
is called on the mutation.
So when an argument is defined as GlobalIdType[::Project]
passing
a GID of the wrong type (like gid://gitlab/Issue/1
) will fail before the :expected_type
check is ever performed.
See #257883 (closed) for more context of the history of this.
Note on behaviour changes
Many of the find_object
implementations that are being removed here are slightly different
to the default base class method.
The removed methods are of the form:
def find_object(id: id)
::GitlabSchema.object_from_id(id, expected_type: ::SomeModel)
end
whereas the base implementation looks like this:
def find_object(id:)
GitlabSchema.find_by_gid(id)
end
The object_from_id
version performs 2 actions, it parses the string to a Gitlab::GlobalId
and then calls find_by_gid
.
So with this MR, all of the mutations with object_from_id
change behaviour from:
gid = parse_gid(id)
find_by_gid(gid)
to
find_by_gid(id)
This is ok as the first step is not required because the object referenced by id
will already be a parsed GID instance.
In the methods being removed we were calling a redundant second parse, which is effectively a noop.
[2] pry(main)> gid = GitlabSchema.parse_gid('gid://gitlab/Project/1')
=> #<GlobalID:0x00007f9906603f50 @uri=#<URI::GID gid://gitlab/Project/1>>
[3] pry(main)> GitlabSchema.parse_gid(gid)
=> #<GlobalID:0x00007f9906603f50 @uri=#<URI::GID gid://gitlab/Project/1>>
Summary
Broadly there are 3 types of changes in this MR split into 3 commits:
- Removal of
find_object
overrides without extra modification. These will fall back to the implementation fromGitlab::Graphql::Authorize::AuthorizeResource
- Update of calls from
authorized_find!(id)
toauthorized_find!(id: id)
and subsequent removal of redundantfind_object
overrides. - Modification of mutation unit specs to use a GID instance rather than a string. The latter will never be passed to these mutations when executed in a full stack GraphQL call.