Commit 7245e11c authored by Alex Kalderimis's avatar Alex Kalderimis Committed by Robert Speicher

Snippets: use GlobalID scalar

This uses the new GlobalID scalar types in the Snippets code. The
benefits of this are:

- More informative types. A reader of our schema, or our queries can see
  what a valid value will be.
- Safer resolution and mutation. When using this type we guarantee that
  our code will only encounter valid values, from our own namespace, and
  when using the parameterized IDs, that they cannot refer to other
  kinds of object.
parent b40e8c75
...@@ -7,8 +7,7 @@ module Mutations ...@@ -7,8 +7,7 @@ module Mutations
ERROR_MSG = 'Error deleting the snippet' ERROR_MSG = 'Error deleting the snippet'
argument :id, argument :id, ::Types::GlobalIDType[::Snippet],
GraphQL::ID_TYPE,
required: true, required: true,
description: 'The global id of the snippet to destroy' description: 'The global id of the snippet to destroy'
......
...@@ -5,8 +5,7 @@ module Mutations ...@@ -5,8 +5,7 @@ module Mutations
class MarkAsSpam < Base class MarkAsSpam < Base
graphql_name 'MarkAsSpamSnippet' graphql_name 'MarkAsSpamSnippet'
argument :id, argument :id, ::Types::GlobalIDType[::Snippet],
GraphQL::ID_TYPE,
required: true, required: true,
description: 'The global id of the snippet to update' description: 'The global id of the snippet to update'
......
...@@ -7,8 +7,7 @@ module Mutations ...@@ -7,8 +7,7 @@ module Mutations
graphql_name 'UpdateSnippet' graphql_name 'UpdateSnippet'
argument :id, argument :id, ::Types::GlobalIDType[::Snippet],
GraphQL::ID_TYPE,
required: true, required: true,
description: 'The global id of the snippet to update' description: 'The global id of the snippet to update'
......
query GetSnippetQuery($ids: [ID!]) { query GetSnippetQuery($ids: [SnippetID!]) {
snippets(ids: $ids) { snippets(ids: $ids) {
__typename __typename
nodes { nodes {
......
...@@ -6,7 +6,7 @@ module ResolvesSnippets ...@@ -6,7 +6,7 @@ module ResolvesSnippets
included do included do
type Types::SnippetType, null: false type Types::SnippetType, null: false
argument :ids, [GraphQL::ID_TYPE], argument :ids, [::Types::GlobalIDType[::Snippet]],
required: false, required: false,
description: 'Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"' description: 'Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"'
...@@ -32,16 +32,15 @@ module ResolvesSnippets ...@@ -32,16 +32,15 @@ module ResolvesSnippets
}.merge(options_by_type(args[:type])) }.merge(options_by_type(args[:type]))
end end
def resolve_ids(ids) def resolve_ids(ids, type = ::Types::GlobalIDType[::Snippet])
Array.wrap(ids).map { |id| resolve_gid(id, :id) } Array.wrap(ids).map do |id|
end next unless id.present?
def resolve_gid(gid, argument)
return unless gid.present?
GlobalID.parse(gid)&.model_id.tap do |id| # TODO: remove this line when the compatibility layer is removed
raise Gitlab::Graphql::Errors::ArgumentError, "Invalid global id format for param #{argument}" if id.nil? # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
end id = type.coerce_isolated_input(id)
id.model_id
end.compact
end end
def options_by_type(type) def options_by_type(type)
......
...@@ -8,11 +8,11 @@ module Resolvers ...@@ -8,11 +8,11 @@ module Resolvers
alias_method :user, :object alias_method :user, :object
argument :author_id, GraphQL::ID_TYPE, argument :author_id, ::Types::GlobalIDType[::User],
required: false, required: false,
description: 'The ID of an author' description: 'The ID of an author'
argument :project_id, GraphQL::ID_TYPE, argument :project_id, ::Types::GlobalIDType[::Project],
required: false, required: false,
description: 'The ID of a project' description: 'The ID of a project'
...@@ -36,9 +36,11 @@ module Resolvers ...@@ -36,9 +36,11 @@ module Resolvers
private private
def snippet_finder_params(args) def snippet_finder_params(args)
# TODO: remove the type arguments when the compatibility layer is removed
# See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
super super
.merge(author: resolve_gid(args[:author_id], :author), .merge(author: resolve_ids(args[:author_id], ::Types::GlobalIDType[::User]),
project: resolve_gid(args[:project_id], :project), project: resolve_ids(args[:project_id], ::Types::GlobalIDType[::Project]),
explore: args[:explore]) explore: args[:explore])
end end
end end
......
...@@ -13,7 +13,7 @@ module Types ...@@ -13,7 +13,7 @@ module Types
expose_permissions Types::PermissionTypes::Snippet expose_permissions Types::PermissionTypes::Snippet
field :id, GraphQL::ID_TYPE, field :id, Types::GlobalIDType[::Snippet],
description: 'ID of the snippet', description: 'ID of the snippet',
null: false null: false
......
---
title: 'GraphQL Snippets: use Global-ID scalar'
merge_request: 36117
author:
type: changed
...@@ -5955,7 +5955,7 @@ input DestroySnippetInput { ...@@ -5955,7 +5955,7 @@ input DestroySnippetInput {
""" """
The global id of the snippet to destroy The global id of the snippet to destroy
""" """
id: ID! id: SnippetID!
} }
""" """
...@@ -11576,7 +11576,7 @@ input MarkAsSpamSnippetInput { ...@@ -11576,7 +11576,7 @@ input MarkAsSpamSnippetInput {
""" """
The global id of the snippet to update The global id of the snippet to update
""" """
id: ID! id: SnippetID!
} }
""" """
...@@ -15990,7 +15990,7 @@ type Project { ...@@ -15990,7 +15990,7 @@ type Project {
""" """
Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1" Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"
""" """
ids: [ID!] ids: [SnippetID!]
""" """
Returns the last _n_ elements from the list. Returns the last _n_ elements from the list.
...@@ -17036,7 +17036,7 @@ type Query { ...@@ -17036,7 +17036,7 @@ type Query {
""" """
The ID of an author The ID of an author
""" """
authorId: ID authorId: UserID
""" """
Returns the elements in the list that come before the specified cursor. Returns the elements in the list that come before the specified cursor.
...@@ -17056,7 +17056,7 @@ type Query { ...@@ -17056,7 +17056,7 @@ type Query {
""" """
Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1" Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"
""" """
ids: [ID!] ids: [SnippetID!]
""" """
Returns the last _n_ elements from the list. Returns the last _n_ elements from the list.
...@@ -17066,7 +17066,7 @@ type Query { ...@@ -17066,7 +17066,7 @@ type Query {
""" """
The ID of a project The ID of a project
""" """
projectId: ID projectId: ProjectID
""" """
The type of snippet The type of snippet
...@@ -19638,7 +19638,7 @@ type Snippet implements Noteable { ...@@ -19638,7 +19638,7 @@ type Snippet implements Noteable {
""" """
ID of the snippet ID of the snippet
""" """
id: ID! id: SnippetID!
""" """
All notes on this noteable All notes on this noteable
...@@ -19916,6 +19916,11 @@ type SnippetEdge { ...@@ -19916,6 +19916,11 @@ type SnippetEdge {
node: Snippet node: Snippet
} }
"""
Identifier of Snippet
"""
scalar SnippetID
type SnippetPermissions { type SnippetPermissions {
""" """
Indicates the user can perform `admin_snippet` on this resource Indicates the user can perform `admin_snippet` on this resource
...@@ -21831,7 +21836,7 @@ input UpdateSnippetInput { ...@@ -21831,7 +21836,7 @@ input UpdateSnippetInput {
""" """
The global id of the snippet to update The global id of the snippet to update
""" """
id: ID! id: SnippetID!
""" """
Title of the snippet Title of the snippet
...@@ -22139,7 +22144,7 @@ type User { ...@@ -22139,7 +22144,7 @@ type User {
""" """
Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1" Array of global snippet ids, e.g., "gid://gitlab/ProjectSnippet/1"
""" """
ids: [ID!] ids: [SnippetID!]
""" """
Returns the last _n_ elements from the list. Returns the last _n_ elements from the list.
......
...@@ -16300,7 +16300,7 @@ ...@@ -16300,7 +16300,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
}, },
...@@ -31712,7 +31712,7 @@ ...@@ -31712,7 +31712,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
}, },
...@@ -46307,7 +46307,7 @@ ...@@ -46307,7 +46307,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
} }
...@@ -49373,7 +49373,7 @@ ...@@ -49373,7 +49373,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
} }
...@@ -49395,7 +49395,7 @@ ...@@ -49395,7 +49395,7 @@
"description": "The ID of an author", "description": "The ID of an author",
"type": { "type": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "UserID",
"ofType": null "ofType": null
}, },
"defaultValue": null "defaultValue": null
...@@ -49405,7 +49405,7 @@ ...@@ -49405,7 +49405,7 @@
"description": "The ID of a project", "description": "The ID of a project",
"type": { "type": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "ProjectID",
"ofType": null "ofType": null
}, },
"defaultValue": null "defaultValue": null
...@@ -56879,7 +56879,7 @@ ...@@ -56879,7 +56879,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
}, },
...@@ -57745,6 +57745,16 @@ ...@@ -57745,6 +57745,16 @@
"enumValues": null, "enumValues": null,
"possibleTypes": null "possibleTypes": null
}, },
{
"kind": "SCALAR",
"name": "SnippetID",
"description": "Identifier of Snippet",
"fields": null,
"inputFields": null,
"interfaces": null,
"enumValues": null,
"possibleTypes": null
},
{ {
"kind": "OBJECT", "kind": "OBJECT",
"name": "SnippetPermissions", "name": "SnippetPermissions",
...@@ -63283,7 +63293,7 @@ ...@@ -63283,7 +63293,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
}, },
...@@ -64057,7 +64067,7 @@ ...@@ -64057,7 +64067,7 @@
"name": null, "name": null,
"ofType": { "ofType": {
"kind": "SCALAR", "kind": "SCALAR",
"name": "ID", "name": "SnippetID",
"ofType": null "ofType": null
} }
} }
...@@ -2841,7 +2841,7 @@ Represents a snippet entry. ...@@ -2841,7 +2841,7 @@ Represents a snippet entry.
| `discussions` | DiscussionConnection! | All discussions on this noteable | | `discussions` | DiscussionConnection! | All discussions on this noteable |
| `fileName` | String | File Name of the snippet | | `fileName` | String | File Name of the snippet |
| `httpUrlToRepo` | String | HTTP URL to the snippet repository | | `httpUrlToRepo` | String | HTTP URL to the snippet repository |
| `id` | ID! | ID of the snippet | | `id` | SnippetID! | ID of the snippet |
| `notes` | NoteConnection! | All notes on this noteable | | `notes` | NoteConnection! | All notes on this noteable |
| `project` | Project | The project the snippet is associated with | | `project` | Project | The project the snippet is associated with |
| `rawUrl` | String! | Raw URL of the snippet | | `rawUrl` | String! | Raw URL of the snippet |
......
...@@ -56,12 +56,6 @@ RSpec.describe Resolvers::Projects::SnippetsResolver do ...@@ -56,12 +56,6 @@ RSpec.describe Resolvers::Projects::SnippetsResolver do
expect(snippets).to contain_exactly(project_snippet, other_project_snippet) expect(snippets).to contain_exactly(project_snippet, other_project_snippet)
end end
it 'returns an error if the gid is invalid' do
expect do
resolve_snippets(args: { ids: 'foo' })
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError)
end
end end
context 'when no project is provided' do context 'when no project is provided' do
......
...@@ -36,7 +36,7 @@ RSpec.describe Resolvers::SnippetsResolver do ...@@ -36,7 +36,7 @@ RSpec.describe Resolvers::SnippetsResolver do
context 'when using filters' do context 'when using filters' do
context 'by author id' do context 'by author id' do
it 'returns the snippets' do it 'returns the snippets' do
snippets = resolve_snippets(args: { author_id: current_user.to_global_id }) snippets = resolve_snippets(args: { author_id: global_id_of(current_user) })
expect(snippets).to contain_exactly(personal_snippet, project_snippet) expect(snippets).to contain_exactly(personal_snippet, project_snippet)
end end
...@@ -44,7 +44,7 @@ RSpec.describe Resolvers::SnippetsResolver do ...@@ -44,7 +44,7 @@ RSpec.describe Resolvers::SnippetsResolver do
it 'returns an error if the param id is invalid' do it 'returns an error if the param id is invalid' do
expect do expect do
resolve_snippets(args: { author_id: 'foo' }) resolve_snippets(args: { author_id: 'foo' })
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end.to raise_error(GraphQL::CoercionError)
end end
end end
...@@ -65,7 +65,7 @@ RSpec.describe Resolvers::SnippetsResolver do ...@@ -65,7 +65,7 @@ RSpec.describe Resolvers::SnippetsResolver do
it 'returns an error if the param id is invalid' do it 'returns an error if the param id is invalid' do
expect do expect do
resolve_snippets(args: { project_id: 'foo' }) resolve_snippets(args: { project_id: 'foo' })
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end.to raise_error(GraphQL::CoercionError)
end end
end end
...@@ -99,14 +99,14 @@ RSpec.describe Resolvers::SnippetsResolver do ...@@ -99,14 +99,14 @@ RSpec.describe Resolvers::SnippetsResolver do
expect(snippets).to contain_exactly(personal_snippet, project_snippet) expect(snippets).to contain_exactly(personal_snippet, project_snippet)
end end
it 'returns an error if the gid is invalid' do it 'returns an error if the id cannot be coerced' do
args = { args = {
ids: [personal_snippet.to_global_id, 'foo'] ids: [personal_snippet.to_global_id, 'foo']
} }
expect do expect do
resolve_snippets(args: args) resolve_snippets(args: args)
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end.to raise_error(GraphQL::CoercionError, '"foo" is not a valid Global ID')
end end
it 'returns an error if both project and author are provided' do it 'returns an error if both project and author are provided' do
......
...@@ -73,7 +73,7 @@ RSpec.describe Resolvers::Users::SnippetsResolver do ...@@ -73,7 +73,7 @@ RSpec.describe Resolvers::Users::SnippetsResolver do
expect do expect do
resolve_snippets(args: args) resolve_snippets(args: args)
end.to raise_error(Gitlab::Graphql::Errors::ArgumentError) end.to raise_error(GraphQL::CoercionError)
end end
end end
end end
......
...@@ -53,10 +53,11 @@ RSpec.describe 'Destroying a Snippet' do ...@@ -53,10 +53,11 @@ RSpec.describe 'Destroying a Snippet' do
let!(:snippet_gid) { project.to_gid.to_s } let!(:snippet_gid) { project.to_gid.to_s }
it 'returns an error' do it 'returns an error' do
err_message = %Q["#{snippet_gid}" does not represent an instance of Snippet]
post_graphql_mutation(mutation, current_user: current_user) post_graphql_mutation(mutation, current_user: current_user)
expect(graphql_errors) expect(graphql_errors).to include(a_hash_including('message' => a_string_including(err_message)))
.to include(a_hash_including('message' => "#{snippet_gid} is not a valid ID for Snippet."))
end end
it 'does not destroy the Snippet' do it 'does not destroy the Snippet' do
......
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