Commit abb20b6f authored by Alex Kalderimis's avatar Alex Kalderimis

Add explicit coercions for all existing uses of the GlobalIDType

This mainly adds explicit coercions of the form
`ThingID.coerce_isolated_input(id)`, but an abstraction is added to do
this generically in global_id_compatibility, which is included for
mutations and resolvers.
parent ac235c10
...@@ -4,6 +4,7 @@ module Mutations ...@@ -4,6 +4,7 @@ module Mutations
class BaseMutation < GraphQL::Schema::RelayClassicMutation class BaseMutation < GraphQL::Schema::RelayClassicMutation
prepend Gitlab::Graphql::Authorize::AuthorizeResource prepend Gitlab::Graphql::Authorize::AuthorizeResource
prepend Gitlab::Graphql::CopyFieldDescription prepend Gitlab::Graphql::CopyFieldDescription
prepend ::Gitlab::Graphql::GlobalIDCompatibility
ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance' ERROR_MESSAGE = 'You cannot perform write operations on a read-only instance'
......
...@@ -3,13 +3,18 @@ ...@@ -3,13 +3,18 @@
module Mutations module Mutations
module Ci module Ci
class Base < BaseMutation class Base < BaseMutation
argument :id, ::Types::GlobalIDType[::Ci::Pipeline], PipelineID = ::Types::GlobalIDType[::Ci::Pipeline]
argument :id, PipelineID,
required: true, required: true,
description: 'The id of the pipeline to mutate' description: 'The id of the pipeline to mutate'
private private
def find_object(id:) def find_object(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = PipelineID.coerce_isolated_input(id)
GlobalID::Locator.locate(id) GlobalID::Locator.locate(id)
end end
end end
......
...@@ -35,7 +35,10 @@ module Mutations ...@@ -35,7 +35,10 @@ module Mutations
end end
def find_design(id) def find_design(id)
GitlabSchema.object_from_id(DesignID.coerce_isolated_input(id)) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = DesignID.coerce_isolated_input(id)
GitlabSchema.object_from_id(id)
end end
def not_found(gid) def not_found(gid)
......
...@@ -4,6 +4,7 @@ module Resolvers ...@@ -4,6 +4,7 @@ module Resolvers
class BaseResolver < GraphQL::Schema::Resolver class BaseResolver < GraphQL::Schema::Resolver
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
include ::Gitlab::Utils::StrongMemoize include ::Gitlab::Utils::StrongMemoize
include ::Gitlab::Graphql::GlobalIDCompatibility
def self.single def self.single
@single ||= Class.new(self) do @single ||= Class.new(self) do
......
...@@ -85,11 +85,15 @@ module Types ...@@ -85,11 +85,15 @@ module Types
end end
def issue(id:) def issue(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Issue].coerce_isolated_input(id) id = ::Types::GlobalIDType[::Issue].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
def milestone(id:) def milestone(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[Milestone].coerce_isolated_input(id) id = ::Types::GlobalIDType[Milestone].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
......
...@@ -8,8 +8,10 @@ module EE ...@@ -8,8 +8,10 @@ module EE
extend ActiveSupport::Concern extend ActiveSupport::Concern
extend ::Gitlab::Utils::Override extend ::Gitlab::Utils::Override
EpicID = ::Types::GlobalIDType[::Epic]
prepended do prepended do
argument :epic_id, ::Types::GlobalIDType[::Epic], argument :epic_id, EpicID,
required: false, required: false,
description: 'The ID of the parent epic. NULL when removing the association' description: 'The ID of the parent epic. NULL when removing the association'
end end
...@@ -28,10 +30,12 @@ module EE ...@@ -28,10 +30,12 @@ module EE
override :move_arguments override :move_arguments
def move_arguments(args) def move_arguments(args)
allowed_args = super # TODO: remove this line once the compatibility layer is removed
allowed_args[:epic_id] = args[:epic_id]&.model_id if args.has_key?(:epic_id) # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
coerce_global_id_arguments!(args)
epic_arguments = args.slice(:epic_id).transform_values { |id| id&.model_id }
allowed_args super.merge!(epic_arguments)
end end
end end
end end
......
...@@ -87,7 +87,10 @@ module EE ...@@ -87,7 +87,10 @@ module EE
::Types::DastSiteProfileType, ::Types::DastSiteProfileType,
null: true, null: true,
resolve: -> (obj, args, _ctx) do resolve: -> (obj, args, _ctx) do
DastSiteProfilesFinder.new(project_id: obj.id, id: args[:id].model_id).execute.first # TODO: remove this coercion when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
gid = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(args[:id])
DastSiteProfilesFinder.new(project_id: obj.id, id: gid.model_id).execute.first
end, end,
description: 'DAST Site Profile associated with the project' do description: 'DAST Site Profile associated with the project' do
argument :id, ::Types::GlobalIDType[::DastSiteProfile], required: true, description: 'ID of the site profile' argument :id, ::Types::GlobalIDType[::DastSiteProfile], required: true, description: 'ID of the site profile'
......
...@@ -23,8 +23,7 @@ module EE ...@@ -23,8 +23,7 @@ module EE
field :vulnerability, field :vulnerability,
::Types::VulnerabilityType, ::Types::VulnerabilityType,
null: true, null: true,
description: "Find a vulnerability", description: "Find a vulnerability" do
resolve: -> (_obj, args, _ctx) { ::GitlabSchema.find_by_gid(args[:id]) } do
argument :id, ::Types::GlobalIDType[::Vulnerability], argument :id, ::Types::GlobalIDType[::Vulnerability],
required: true, required: true,
description: 'The Global ID of the Vulnerability' description: 'The Global ID of the Vulnerability'
...@@ -54,8 +53,18 @@ module EE ...@@ -54,8 +53,18 @@ module EE
description: 'Fields related to Instance Security Dashboard' description: 'Fields related to Instance Security Dashboard'
end end
def vulnerability(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Vulnerability].coerce_isolated_input(id)
::GitlabSchema.find_by_gid(id)
end
def iteration(id:) def iteration(id:)
::GitlabSchema.find_by_gid(::Types::GlobalIDType[Iteration].coerce_isolated_input(id)) # TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[Iteration].coerce_isolated_input(id)
::GitlabSchema.find_by_gid(id)
end end
end end
end end
......
...@@ -8,8 +8,9 @@ module Mutations ...@@ -8,8 +8,9 @@ module Mutations
authorize :create_cluster authorize :create_cluster
argument :cluster_agent_id, ClusterAgentID = ::Types::GlobalIDType[::Clusters::Agent]
::Types::GlobalIDType[::Clusters::Agent],
argument :cluster_agent_id, ClusterAgentID,
required: true, required: true,
description: 'Global ID of the cluster agent that will be associated with the new token' description: 'Global ID of the cluster agent that will be associated with the new token'
...@@ -42,6 +43,9 @@ module Mutations ...@@ -42,6 +43,9 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ClusterAgentID.coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
end end
......
...@@ -8,8 +8,9 @@ module Mutations ...@@ -8,8 +8,9 @@ module Mutations
authorize :admin_cluster authorize :admin_cluster
argument :id, TokenID = ::Types::GlobalIDType[::Clusters::AgentToken]
::Types::GlobalIDType[::Clusters::AgentToken],
argument :id, TokenID,
required: true, required: true,
description: 'Global ID of the cluster agent token that will be deleted' description: 'Global ID of the cluster agent token that will be deleted'
...@@ -23,6 +24,9 @@ module Mutations ...@@ -23,6 +24,9 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = TokenID.coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
end end
......
...@@ -8,8 +8,9 @@ module Mutations ...@@ -8,8 +8,9 @@ module Mutations
authorize :admin_cluster authorize :admin_cluster
argument :id, AgentID = ::Types::GlobalIDType[::Clusters::Agent]
::Types::GlobalIDType[::Clusters::Agent],
argument :id, AgentID,
required: true, required: true,
description: 'Global id of the cluster agent that will be deleted' description: 'Global id of the cluster agent that will be deleted'
...@@ -27,6 +28,9 @@ module Mutations ...@@ -27,6 +28,9 @@ module Mutations
private private
def find_object(id:) def find_object(id:)
# TODO: remove this line when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = AgentID.coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
end end
......
...@@ -31,7 +31,9 @@ module Mutations ...@@ -31,7 +31,9 @@ module Mutations
project = authorized_find_project!(full_path: full_path) project = authorized_find_project!(full_path: full_path)
# TODO: remove explicit coercion once compatibility layer is removed # TODO: remove explicit coercion once compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
dast_site_profile_id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(dast_site_profile_id) dast_site_profile_id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(dast_site_profile_id)
dast_site_profile = find_dast_site_profile(project: project, dast_site_profile_id: dast_site_profile_id) dast_site_profile = find_dast_site_profile(project: project, dast_site_profile_id: dast_site_profile_id)
dast_site = dast_site_profile.dast_site dast_site = dast_site_profile.dast_site
dast_scanner_profile = find_dast_scanner_profile(project: project, dast_scanner_profile_id: args[:dast_scanner_profile_id]) dast_scanner_profile = find_dast_scanner_profile(project: project, dast_scanner_profile_id: args[:dast_scanner_profile_id])
...@@ -65,6 +67,11 @@ module Mutations ...@@ -65,6 +67,11 @@ module Mutations
def find_dast_scanner_profile(project:, dast_scanner_profile_id:) def find_dast_scanner_profile(project:, dast_scanner_profile_id:)
return unless dast_scanner_profile_id return unless dast_scanner_profile_id
# TODO: remove explicit coercion once compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
dast_scanner_profile_id = ::Types::GlobalIDType[::DastScannerProfile]
.coerce_isolated_input(dast_scanner_profile_id)
project project
.dast_scanner_profiles .dast_scanner_profiles
.find(dast_scanner_profile_id.model_id) .find(dast_scanner_profile_id.model_id)
......
...@@ -7,17 +7,23 @@ module Mutations ...@@ -7,17 +7,23 @@ module Mutations
graphql_name 'DastScannerProfileDelete' graphql_name 'DastScannerProfileDelete'
ScannerProfileID = ::Types::GlobalIDType[::DastScannerProfile]
argument :full_path, GraphQL::ID_TYPE, argument :full_path, GraphQL::ID_TYPE,
required: true, required: true,
description: 'Full path for the project the scanner profile belongs to.' description: 'Full path for the project the scanner profile belongs to.'
argument :id, ::Types::GlobalIDType[::DastScannerProfile], argument :id, ScannerProfileID,
required: true, required: true,
description: 'ID of the scanner profile to be deleted.' description: 'ID of the scanner profile to be deleted.'
authorize :create_on_demand_dast_scan authorize :create_on_demand_dast_scan
def resolve(full_path:, id:) def resolve(full_path:, id:)
# TODO: remove this line once the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ScannerProfileID.coerce_isolated_input(id)
project = authorized_find_project!(full_path: full_path) project = authorized_find_project!(full_path: full_path)
service = ::DastScannerProfiles::DestroyService.new(project, current_user) service = ::DastScannerProfiles::DestroyService.new(project, current_user)
......
...@@ -34,10 +34,14 @@ module Mutations ...@@ -34,10 +34,14 @@ module Mutations
authorize :create_on_demand_dast_scan authorize :create_on_demand_dast_scan
def resolve(full_path:, **service_args) def resolve(full_path:, **service_args)
# TODO: remove this explicit coercion once the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
gid = ::Types::GlobalIDType[::DastScannerProfile].coerce_isolated_input(service_args[:id])
project = authorized_find!(full_path: full_path) project = authorized_find!(full_path: full_path)
service = ::DastScannerProfiles::UpdateService.new(project, current_user) service = ::DastScannerProfiles::UpdateService.new(project, current_user)
result = service.execute({ **service_args, id: service_args[:id].model_id }) result = service.execute({ **service_args, id: gid.model_id })
if result.success? if result.success?
{ id: result.payload.to_global_id, errors: [] } { id: result.payload.to_global_id, errors: [] }
......
...@@ -20,6 +20,7 @@ module Mutations ...@@ -20,6 +20,7 @@ module Mutations
def resolve(full_path:, id:) def resolve(full_path:, id:)
project = authorized_find_project!(full_path: full_path) project = authorized_find_project!(full_path: full_path)
# TODO: remove explicit coercion once compatibility layer is removed # TODO: remove explicit coercion once compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id) id = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id)
dast_site_profile = find_dast_site_profile(project: project, global_id: id) dast_site_profile = find_dast_site_profile(project: project, global_id: id)
......
...@@ -31,6 +31,7 @@ module Mutations ...@@ -31,6 +31,7 @@ module Mutations
def resolve(full_path:, id:, **service_args) def resolve(full_path:, id:, **service_args)
# TODO: remove explicit coercion once compatibility layer has been removed # TODO: remove explicit coercion once compatibility layer has been removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
service_args[:id] = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id).model_id service_args[:id] = ::Types::GlobalIDType[::DastSiteProfile].coerce_isolated_input(id).model_id
project = authorized_find_project!(full_path: full_path) project = authorized_find_project!(full_path: full_path)
......
...@@ -33,6 +33,9 @@ module Mutations ...@@ -33,6 +33,9 @@ module Mutations
end end
def find_object(id:) def find_object(id:)
# TODO: remove this line once the compatibility layer is removed.
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Vulnerability].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
end end
......
...@@ -33,6 +33,9 @@ module Mutations ...@@ -33,6 +33,9 @@ module Mutations
end end
def find_object(id:) def find_object(id:)
# TODO: remove this line once the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
id = ::Types::GlobalIDType[::Vulnerability].coerce_isolated_input(id)
GitlabSchema.find_by_gid(id) GitlabSchema.find_by_gid(id)
end end
end end
......
...@@ -52,7 +52,7 @@ RSpec.describe Mutations::Clusters::AgentTokens::Create do ...@@ -52,7 +52,7 @@ RSpec.describe Mutations::Clusters::AgentTokens::Create do
subject { mutation.resolve(cluster_agent_id: cluster_agent.id) } subject { mutation.resolve(cluster_agent_id: cluster_agent.id) }
it 'generates an error message when id invalid', :aggregate_failures do it 'generates an error message when id invalid', :aggregate_failures do
expect { subject }.to raise_error(NoMethodError) expect { subject }.to raise_error(::GraphQL::CoercionError)
end end
end end
end end
......
...@@ -44,7 +44,7 @@ RSpec.describe Mutations::Clusters::AgentTokens::Delete do ...@@ -44,7 +44,7 @@ RSpec.describe Mutations::Clusters::AgentTokens::Delete do
let(:global_id) { token.id } let(:global_id) { token.id }
it 'raises an error if the cluster agent id is invalid', :aggregate_failures do it 'raises an error if the cluster agent id is invalid', :aggregate_failures do
expect { subject }.to raise_error(NoMethodError) expect { subject }.to raise_error(::GraphQL::CoercionError)
expect { token.reload }.not_to raise_error expect { token.reload }.not_to raise_error
end end
end end
......
...@@ -24,7 +24,7 @@ RSpec.describe Mutations::Clusters::Agents::Delete do ...@@ -24,7 +24,7 @@ RSpec.describe Mutations::Clusters::Agents::Delete do
context 'without user permissions' do context 'without user permissions' do
it 'fails to delete the cluster agent', :aggregate_failures do it 'fails to delete the cluster agent', :aggregate_failures do
expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable) expect { subject }.to raise_error(Gitlab::Graphql::Errors::ResourceNotAvailable)
expect { cluster_agent.reload }.not_to raise_error(ActiveRecord::RecordNotFound) expect { cluster_agent.reload }.not_to raise_error
end end
end end
...@@ -43,8 +43,8 @@ RSpec.describe Mutations::Clusters::Agents::Delete do ...@@ -43,8 +43,8 @@ RSpec.describe Mutations::Clusters::Agents::Delete do
subject { mutation.resolve(id: cluster_agent.id) } subject { mutation.resolve(id: cluster_agent.id) }
it 'raises an error if the cluster agent id is invalid', :aggregate_failures do it 'raises an error if the cluster agent id is invalid', :aggregate_failures do
expect { subject }.to raise_error(NoMethodError) expect { subject }.to raise_error(::GraphQL::CoercionError)
expect { cluster_agent.reload }.not_to raise_error(ActiveRecord::RecordNotFound) expect { cluster_agent.reload }.not_to raise_error
end end
end end
end end
......
# frozen_string_literal: true
module Gitlab
module Graphql
module GlobalIDCompatibility
# TODO: remove this module once the compatibility layer is no longer needed.
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
def coerce_global_id_arguments!(args)
global_id_arguments = self.class.arguments.values.select do |arg|
arg.type.is_a?(Class) && arg.type <= ::Types::GlobalIDType
end
global_id_arguments.each do |arg|
k = arg.keyword
args[k] &&= arg.type.coerce_isolated_input(args[k])
end
end
end
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment