Merge branch 'ajk-globalid-design-management' into 'master'

[GraphQL] Design Management: use Global-ID scalar

See merge request gitlab-org/gitlab!36115

app/assets/javascripts/design_management/graphql/queries/get_design.query.graphql

 #import "../fragments/design.fragment.graphql"
 #import "~/graphql_shared/fragments/author.fragment.graphql"
 
-query getDesign($fullPath: ID!, $iid: String!, $atVersion: ID, $filenames: [String!]) {
+query getDesign(
+  $fullPath: ID!
+  $iid: String!
+  $atVersion: DesignManagementVersionID
+  $filenames: [String!]
+) {
   project(fullPath: $fullPath) {
     id
     issue(iid: $iid) {

app/graphql/resolvers/design_management/design_at_version_resolver.rb

@@ -9,7 +9,7 @@ module Resolvers
 
       authorize :read_design
 
-      argument :id, GraphQL::ID_TYPE,
+      argument :id, ::Types::GlobalIDType[::DesignManagement::DesignAtVersion],
                required: true,
                description: 'The Global ID of the design at this version'
 
@@ -18,7 +18,10 @@ module Resolvers
       end
 
       def find_object(id:)
-        dav = GitlabSchema.object_from_id(id, expected_type: ::DesignManagement::DesignAtVersion)
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        id = ::Types::GlobalIDType[::DesignManagement::DesignAtVersion].coerce_isolated_input(id)
+        dav = GitlabSchema.find_by_gid(id)
         return unless consistent?(dav)
 
         dav

app/graphql/resolvers/design_management/design_resolver.rb

@@ -3,7 +3,7 @@
 module Resolvers
   module DesignManagement
     class DesignResolver < BaseResolver
-      argument :id, GraphQL::ID_TYPE,
+      argument :id, ::Types::GlobalIDType[::DesignManagement::Design],
                required: false,
                description: 'Find a design by its ID'
 
@@ -50,7 +50,11 @@ module Resolvers
       end
 
       def parse_gid(gid)
-        GitlabSchema.parse_gid(gid, expected_type: ::DesignManagement::Design).model_id
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        gid = ::Types::GlobalIDType[::DesignManagement::Design].coerce_isolated_input(gid)
+
+        gid.model_id
       end
     end
   end

app/graphql/resolvers/design_management/designs_resolver.rb

@@ -3,16 +3,16 @@
 module Resolvers
   module DesignManagement
     class DesignsResolver < BaseResolver
-      argument :ids,
-               [GraphQL::ID_TYPE],
+      DesignID = ::Types::GlobalIDType[::DesignManagement::Design]
+      VersionID = ::Types::GlobalIDType[::DesignManagement::Version]
+
+      argument :ids, [DesignID],
                required: false,
                description: 'Filters designs by their ID'
-      argument :filenames,
-               [GraphQL::STRING_TYPE],
+      argument :filenames, [GraphQL::STRING_TYPE],
                required: false,
                description: 'Filters designs by their filename'
-      argument :at_version,
-               GraphQL::ID_TYPE,
+      argument :at_version, VersionID,
                required: false,
                description: 'Filters designs to only those that existed at the version. ' \
                             'If argument is omitted or nil then all designs will reflect the latest version'
@@ -36,11 +36,20 @@ module Resolvers
       def version(at_version)
         return unless at_version
 
-        GitlabSchema.object_from_id(at_version, expected_type: ::DesignManagement::Version)&.sync
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        at_version = VersionID.coerce_isolated_input(at_version)
+        # TODO: when we get promises use this to make resolve lazy
+        Gitlab::Graphql::Lazy.force(GitlabSchema.find_by_gid(at_version))
       end
 
-      def design_ids(ids)
-        ids&.map { |id| GlobalID.parse(id, expected_type: ::DesignManagement::Design).model_id }
+      def design_ids(gids)
+        return if gids.nil?
+
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        gids = gids.map { |id| DesignID.coerce_isolated_input(id) }
+        gids.map(&:model_id)
       end
 
       def issue

app/graphql/resolvers/design_management/version/design_at_version_resolver.rb

@@ -5,17 +5,20 @@ module Resolvers
     module Version
       # Resolver for a DesignAtVersion object given an implicit version context
       class DesignAtVersionResolver < BaseResolver
+        DesignAtVersionID = ::Types::GlobalIDType[::DesignManagement::DesignAtVersion]
+        DesignID = ::Types::GlobalIDType[::DesignManagement::Design]
+
         include Gitlab::Graphql::Authorize::AuthorizeResource
 
         type Types::DesignManagement::DesignAtVersionType, null: true
 
         authorize :read_design
 
-        argument :id, GraphQL::ID_TYPE,
+        argument :id, DesignAtVersionID,
                  required: false,
                  as: :design_at_version_id,
                  description: 'The ID of the DesignAtVersion'
-        argument :design_id, GraphQL::ID_TYPE,
+        argument :design_id, DesignID,
                  required: false,
                  description: 'The ID of a specific design'
         argument :filename, GraphQL::STRING_TYPE,
@@ -29,6 +32,11 @@ module Resolvers
         def resolve(design_id: nil, filename: nil, design_at_version_id: nil)
           validate_arguments(design_id, filename, design_at_version_id)
 
+          # TODO: remove this when the compatibility layer is removed
+          # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+          design_id &&= DesignID.coerce_isolated_input(design_id)
+          design_at_version_id &&= DesignAtVersionID.coerce_isolated_input(design_at_version_id)
+
           return unless Ability.allowed?(current_user, :read_design, issue)
           return specific_design_at_version(design_at_version_id) if design_at_version_id
 
@@ -49,7 +57,7 @@ module Resolvers
         end
 
         def specific_design_at_version(id)
-          dav = GitlabSchema.object_from_id(id, expected_type: ::DesignManagement::DesignAtVersion)
+          dav = GitlabSchema.find_by_gid(id)
           return unless consistent?(dav)
 
           dav
@@ -65,8 +73,8 @@ module Resolvers
             dav.design.visible_in?(version)
         end
 
-        def find(id, filename)
-          ids = [parse_design_id(id).model_id] if id
+        def find(gid, filename)
+          ids = [gid.model_id] if gid
           filenames = [filename] if filename
 
           ::DesignManagement::DesignsFinder
@@ -74,10 +82,6 @@ module Resolvers
             .execute
         end
 
-        def parse_design_id(id)
-          GitlabSchema.parse_gid(id, expected_type: ::DesignManagement::Design)
-        end
-
         def issue
           version.issue
         end

app/graphql/resolvers/design_management/version/designs_at_version_resolver.rb

@@ -11,8 +11,9 @@ module Resolvers
 
         authorize :read_design
 
-        argument :ids,
-                 [GraphQL::ID_TYPE],
+        DesignID = ::Types::GlobalIDType[::DesignManagement::Design]
+
+        argument :ids, [DesignID],
                  required: false,
                  description: 'Filters designs by their ID'
         argument :filenames,
@@ -31,16 +32,19 @@ module Resolvers
         private
 
         def find(ids, filenames)
-          ids = ids&.map { |id| parse_design_id(id).model_id }
-
           ::DesignManagement::DesignsFinder.new(issue, current_user,
-                                                ids: ids,
+                                                ids: design_ids(ids),
                                                 filenames: filenames,
                                                 visible_at_version: version)
         end
 
-        def parse_design_id(id)
-          GitlabSchema.parse_gid(id, expected_type: ::DesignManagement::Design)
+        def design_ids(gids)
+          return if gids.nil?
+
+          # TODO: remove this line when the compatibility layer is removed
+          # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+          gids = gids.map { |id| DesignID.coerce_isolated_input(id) }
+          gids.map(&:model_id)
         end
 
         def issue

app/graphql/resolvers/design_management/version_in_collection_resolver.rb

@@ -11,20 +11,25 @@ module Resolvers
 
       alias_method :collection, :object
 
+      VersionID = ::Types::GlobalIDType[::DesignManagement::Version]
+
       argument :sha, GraphQL::STRING_TYPE,
                required: false,
                description: "The SHA256 of a specific version"
-      argument :id, GraphQL::ID_TYPE,
+      argument :id, VersionID,
+               as: :version_id,
                required: false,
                description: 'The Global ID of the version'
 
-      def resolve(id: nil, sha: nil)
-        check_args(id, sha)
+      def resolve(version_id: nil, sha: nil)
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        version_id &&= VersionID.coerce_isolated_input(version_id)
 
-        gid = GitlabSchema.parse_gid(id, expected_type: ::DesignManagement::Version) if id
+        check_args(version_id, sha)
 
         ::DesignManagement::VersionsFinder
-          .new(collection, current_user, sha: sha, version_id: gid&.model_id)
+          .new(collection, current_user, sha: sha, version_id: version_id&.model_id)
           .execute
           .first
       end

app/graphql/resolvers/design_management/version_resolver.rb

@@ -9,7 +9,7 @@ module Resolvers
 
       authorize :read_design
 
-      argument :id, GraphQL::ID_TYPE,
+      argument :id, ::Types::GlobalIDType[::DesignManagement::Version],
                required: true,
                description: 'The Global ID of the version'
 
@@ -18,7 +18,11 @@ module Resolvers
       end
 
       def find_object(id:)
-        GitlabSchema.object_from_id(id, expected_type: ::DesignManagement::Version)
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        id = ::Types::GlobalIDType[::DesignManagement::Version].coerce_isolated_input(id)
+
+        GitlabSchema.find_by_gid(id)
       end
     end
   end

app/graphql/resolvers/design_management/versions_resolver.rb

@@ -7,12 +7,14 @@ module Resolvers
 
       alias_method :design_or_collection, :object
 
+      VersionID = ::Types::GlobalIDType[::DesignManagement::Version]
+
       argument :earlier_or_equal_to_sha, GraphQL::STRING_TYPE,
                as: :sha,
                required: false,
                description: 'The SHA256 of the most recent acceptable version'
 
-      argument :earlier_or_equal_to_id, GraphQL::ID_TYPE,
+      argument :earlier_or_equal_to_id, VersionID,
                as: :id,
                required: false,
                description: 'The Global ID of the most recent acceptable version'
@@ -23,6 +25,9 @@ module Resolvers
       end
 
       def resolve(parent: nil, id: nil, sha: nil)
+        # TODO: remove this line when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        id &&= VersionID.coerce_isolated_input(id)
         version = cutoff(parent, id, sha)
 
         raise ::Gitlab::Graphql::Errors::ResourceNotAvailable, 'cutoff not found' unless version.present?
@@ -47,8 +52,7 @@ module Resolvers
         end
       end
 
-      def specific_version(id, sha)
-        gid = GitlabSchema.parse_gid(id, expected_type: ::DesignManagement::Version) if id
+      def specific_version(gid, sha)
         find(sha: sha, version_id: gid&.model_id).first
       end
 
@@ -58,8 +62,8 @@ module Resolvers
           .execute
       end
 
-      def by_id(id)
-        GitlabSchema.object_from_id(id, expected_type: ::DesignManagement::Version).sync
+      def by_id(gid)
+        ::Gitlab::Graphql::Lazy.force(GitlabSchema.find_by_gid(gid))
       end
 
       # Find an `at_version` argument passed to a parent node.
@@ -69,7 +73,11 @@ module Resolvers
       # for consistency we should only present versions up to the given
       # version here.
       def at_version_arg(parent)
-        ::Gitlab::Graphql::FindArgumentInParent.find(parent, :at_version, limit_depth: 4)
+        # TODO: remove coercion when the compatibility layer is removed
+        # See: https://gitlab.com/gitlab-org/gitlab/-/issues/257883
+        version_id = ::Gitlab::Graphql::FindArgumentInParent.find(parent, :at_version, limit_depth: 4)
+        version_id &&= VersionID.coerce_isolated_input(version_id)
+        version_id
       end
     end
   end

doc/api/graphql/reference/gitlab_schema.graphql

@@ -5139,7 +5139,7 @@ type Design implements CurrentUserTodos & DesignFields & Noteable {
     """
     The Global ID of the most recent acceptable version
     """
-    earlierOrEqualToId: ID
+    earlierOrEqualToId: DesignManagementVersionID
 
     """
     The SHA256 of the most recent acceptable version
@@ -5279,7 +5279,7 @@ type DesignCollection {
     """
     Find a design by its ID
     """
-    id: ID
+    id: DesignManagementDesignID
   ): Design
 
   """
@@ -5289,7 +5289,7 @@ type DesignCollection {
     """
     The Global ID of the design at this version
     """
-    id: ID!
+    id: DesignManagementDesignAtVersionID!
   ): DesignAtVersion
 
   """
@@ -5305,7 +5305,7 @@ type DesignCollection {
     Filters designs to only those that existed at the version. If argument is
     omitted or nil then all designs will reflect the latest version
     """
-    atVersion: ID
+    atVersion: DesignManagementVersionID
 
     """
     Returns the elements in the list that come before the specified cursor.
@@ -5325,7 +5325,7 @@ type DesignCollection {
     """
     Filters designs by their ID
     """
-    ids: [ID!]
+    ids: [DesignManagementDesignID!]
 
     """
     Returns the last _n_ elements from the list.
@@ -5350,7 +5350,7 @@ type DesignCollection {
     """
     The Global ID of the version
     """
-    id: ID
+    id: DesignManagementVersionID
 
     """
     The SHA256 of a specific version
@@ -5375,7 +5375,7 @@ type DesignCollection {
     """
     The Global ID of the most recent acceptable version
     """
-    earlierOrEqualToId: ID
+    earlierOrEqualToId: DesignManagementVersionID
 
     """
     The SHA256 of the most recent acceptable version
@@ -5509,7 +5509,7 @@ type DesignManagement {
     """
     The Global ID of the design at this version
     """
-    id: ID!
+    id: DesignManagementDesignAtVersionID!
   ): DesignAtVersion
 
   """
@@ -5519,7 +5519,7 @@ type DesignManagement {
     """
     The Global ID of the version
     """
-    id: ID!
+    id: DesignManagementVersionID!
   ): DesignVersion
 }
 
@@ -5568,6 +5568,11 @@ type DesignManagementDeletePayload {
   version: DesignVersion
 }
 
+"""
+Identifier of DesignManagement::DesignAtVersion
+"""
+scalar DesignManagementDesignAtVersionID
+
 """
 Identifier of DesignManagement::Design
 """
@@ -5668,6 +5673,11 @@ type DesignManagementUploadPayload {
   skippedDesigns: [Design!]!
 }
 
+"""
+Identifier of DesignManagement::Version
+"""
+scalar DesignManagementVersionID
+
 """
 A specific version in which designs were added, modified or deleted
 """
@@ -5679,7 +5689,7 @@ type DesignVersion {
     """
     The ID of a specific design
     """
-    designId: ID
+    designId: DesignManagementDesignID
 
     """
     The filename of a specific design
@@ -5689,7 +5699,7 @@ type DesignVersion {
     """
     The ID of the DesignAtVersion
     """
-    id: ID
+    id: DesignManagementDesignAtVersionID
   ): DesignAtVersion!
 
   """
@@ -5744,7 +5754,7 @@ type DesignVersion {
     """
     Filters designs by their ID
     """
-    ids: [ID!]
+    ids: [DesignManagementDesignID!]
 
     """
     Returns the last _n_ elements from the list.

doc/api/graphql/reference/gitlab_schema.json

@@ -13948,7 +13948,7 @@
                   "description": "The Global ID of the most recent acceptable version",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementVersionID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -14397,7 +14397,7 @@
                   "description": "Find a design by its ID",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementDesignID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -14433,7 +14433,7 @@
                     "name": null,
                     "ofType": {
                       "kind": "SCALAR",
-                      "name": "ID",
+                      "name": "DesignManagementDesignAtVersionID",
                       "ofType": null
                     }
                   },
@@ -14463,7 +14463,7 @@
                       "name": null,
                       "ofType": {
                         "kind": "SCALAR",
-                        "name": "ID",
+                        "name": "DesignManagementDesignID",
                         "ofType": null
                       }
                     }
@@ -14493,7 +14493,7 @@
                   "description": "Filters designs to only those that existed at the version. If argument is omitted or nil then all designs will reflect the latest version",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementVersionID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -14606,7 +14606,7 @@
                   "description": "The Global ID of the version",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementVersionID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -14639,7 +14639,7 @@
                   "description": "The Global ID of the most recent acceptable version",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementVersionID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -15061,7 +15061,7 @@
                     "name": null,
                     "ofType": {
                       "kind": "SCALAR",
-                      "name": "ID",
+                      "name": "DesignManagementDesignAtVersionID",
                       "ofType": null
                     }
                   },
@@ -15088,7 +15088,7 @@
                     "name": null,
                     "ofType": {
                       "kind": "SCALAR",
-                      "name": "ID",
+                      "name": "DesignManagementVersionID",
                       "ofType": null
                     }
                   },
@@ -15249,6 +15249,16 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "SCALAR",
+          "name": "DesignManagementDesignAtVersionID",
+          "description": "Identifier of DesignManagement::DesignAtVersion",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "SCALAR",
           "name": "DesignManagementDesignID",
@@ -15557,6 +15567,16 @@
           "enumValues": null,
           "possibleTypes": null
         },
+        {
+          "kind": "SCALAR",
+          "name": "DesignManagementVersionID",
+          "description": "Identifier of DesignManagement::Version",
+          "fields": null,
+          "inputFields": null,
+          "interfaces": null,
+          "enumValues": null,
+          "possibleTypes": null
+        },
         {
           "kind": "OBJECT",
           "name": "DesignVersion",
@@ -15571,7 +15591,7 @@
                   "description": "The ID of the DesignAtVersion",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementDesignAtVersionID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -15581,7 +15601,7 @@
                   "description": "The ID of a specific design",
                   "type": {
                     "kind": "SCALAR",
-                    "name": "ID",
+                    "name": "DesignManagementDesignID",
                     "ofType": null
                   },
                   "defaultValue": null
@@ -15681,7 +15701,7 @@
                       "name": null,
                       "ofType": {
                         "kind": "SCALAR",
-                        "name": "ID",
+                        "name": "DesignManagementDesignID",
                         "ofType": null
                       }
                     }

spec/graphql/resolvers/design_management/design_resolver_spec.rb

@@ -57,12 +57,21 @@ RSpec.describe Resolvers::DesignManagement::DesignResolver do
       end
 
       context 'the ID belongs to a design on another issue' do
-        let(:args) { { id: GitlabSchema.id_from_object(design_on_other_issue).to_s } }
+        let(:args) { { id: global_id_of(design_on_other_issue) } }
 
         it 'returns nothing' do
           expect(resolve_design).to be_nil
         end
       end
+
+      context 'the ID does not belong to a design at all' do
+        let(:args) { { id: global_id_of(issue) } }
+        let(:msg) { /does not represent an instance of DesignManagement::Design/ }
+
+        it 'complains meaningfully' do
+          expect { resolve_design }.to raise_error(msg)
+        end
+      end
     end
 
     context 'by filename' do

spec/graphql/resolvers/design_management/designs_resolver_spec.rb

@@ -65,8 +65,24 @@ RSpec.describe Resolvers::DesignManagement::DesignsResolver do
         let(:second_version) { create(:design_version) }
         let(:second_design) { create(:design, issue: issue, versions: [second_version]) }
 
+        context 'ids is provided but null' do
+          let(:args) { { ids: nil } }
+
+          it 'behaves as if unfiltered' do
+            expect(resolve_designs).to contain_exactly(first_design, second_design)
+          end
+        end
+
+        context 'ids is provided but empty' do
+          let(:args) { { ids: [] } }
+
+          it 'eliminates all values' do
+            expect(resolve_designs).to be_empty
+          end
+        end
+
         context 'the ID is on the current issue' do
-          let(:args) { { ids: [GitlabSchema.id_from_object(second_design).to_s] } }
+          let(:args) { { ids: [GitlabSchema.id_from_object(second_design)] } }
 
           it 'resolves to just the relevant design' do
             expect(resolve_designs).to contain_exactly(second_design)
@@ -77,7 +93,7 @@ RSpec.describe Resolvers::DesignManagement::DesignsResolver do
           let(:third_version) { create(:design_version) }
           let(:third_design) { create(:design, issue: create(:issue, project: project), versions: [third_version]) }
 
-          let(:args) { { ids: [GitlabSchema.id_from_object(third_design).to_s] } }
+          let(:args) { { ids: [GitlabSchema.id_from_object(third_design)] } }
 
           it 'ignores it' do
             expect(resolve_designs).to be_empty

spec/graphql/resolvers/design_management/version_in_collection_resolver_spec.rb

@@ -32,7 +32,7 @@ RSpec.describe Resolvers::DesignManagement::VersionInCollectionResolver do
     end
 
     context 'we pass an id' do
-      let(:params) { { id: global_id_of(first_version) } }
+      let(:params) { { version_id: global_id_of(first_version) } }
 
       it { is_expected.to eq(first_version) }
     end
@@ -44,13 +44,14 @@ RSpec.describe Resolvers::DesignManagement::VersionInCollectionResolver do
     end
 
     context 'we pass an inconsistent mixture of sha and version id' do
-      let(:params) { { sha: first_version.sha, id: global_id_of(create(:design_version)) } }
+      let(:params) { { sha: first_version.sha, version_id: global_id_of(create(:design_version)) } }
 
       it { is_expected.to be_nil }
     end
 
     context 'we pass the id of something that is not a design_version' do
-      let(:params) { { id: global_id_of(project) } }
+      let(:params) { { version_id: global_id_of(project) } }
+      let(:appropriate_error) { ::GraphQL::CoercionError }
 
       it 'raises an appropriate error' do
         expect { result }.to raise_error(appropriate_error)