Merge branch 'ajk-full-path-resolver' into 'master'

Add full-path loader

See merge request gitlab-org/gitlab!33299

app/finders/issuable_finder/params.rb

@@ -105,7 +105,7 @@ class IssuableFinder
     end
 
     def project?
-      params[:project_id].present?
+      project_id.present?
     end
 
     def group
@@ -132,15 +132,19 @@ class IssuableFinder
 
     def project
       strong_memoize(:project) do
-        next nil unless params[:project_id].present?
+        next nil unless project?
 
-        project = Project.find(params[:project_id])
+        project = project_id.is_a?(Project) ? project_id : Project.find(project_id)
         project = nil unless Ability.allowed?(current_user, :"read_#{klass.to_ability_name}", project)
 
         project
       end
     end
 
+    def project_id
+      params[:project_id]
+    end
+
     def projects
       strong_memoize(:projects) do
         next [project] if project?

app/graphql/mutations/alert_management/base.rb

@@ -3,7 +3,7 @@
 module Mutations
   module AlertManagement
     class Base < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       argument :project_path, GraphQL::ID_TYPE,
                required: true,

app/graphql/mutations/branches/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module Branches
     class Create < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'CreateBranch'
 

app/graphql/mutations/commits/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module Commits
     class Create < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'CommitCreate'
 

app/graphql/mutations/concerns/mutations/resolves_issuable.rb

@@ -3,7 +3,10 @@
 module Mutations
   module ResolvesIssuable
     extend ActiveSupport::Concern
-    include Mutations::ResolvesProject
+
+    included do
+      include ResolvesProject
+    end
 
     def resolve_issuable(type:, parent_path:, iid:)
       parent = resolve_issuable_parent(type, parent_path)
@@ -29,7 +32,7 @@ module Mutations
     def resolve_issuable_parent(type, parent_path)
       return unless type == :issue || type == :merge_request
 
-      resolve_project(full_path: parent_path)
+      resolve_project(full_path: parent_path) if parent_path.present?
     end
   end
 end

app/graphql/mutations/concerns/mutations/resolves_project.rb

-# frozen_string_literal: true
-
-module Mutations
-  module ResolvesProject
-    extend ActiveSupport::Concern
-
-    def resolve_project(full_path:)
-      project_resolver.resolve(full_path: full_path)
-    end
-
-    def project_resolver
-      Resolvers::ProjectResolver.new(object: nil, context: context, field: nil)
-    end
-  end
-end

app/graphql/mutations/jira_import/start.rb

@@ -3,7 +3,7 @@
 module Mutations
   module JiraImport
     class Start < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'JiraImportStart'
 

app/graphql/mutations/merge_requests/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module MergeRequests
     class Create < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'MergeRequestCreate'
 

app/graphql/mutations/snippets/create.rb

@@ -3,7 +3,7 @@
 module Mutations
   module Snippets
     class Create < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'CreateSnippet'
 

app/graphql/resolvers/concerns/resolves_project.rb

+# frozen_string_literal: true
+
+module ResolvesProject
+  def resolve_project(full_path: nil, project_id: nil)
+    unless full_path.present? ^ project_id.present?
+      raise ::Gitlab::Graphql::Errors::ArgumentError, 'Incompatible arguments: projectId, projectPath.'
+    end
+
+    if full_path.present?
+      ::Gitlab::Graphql::Loaders::FullPathModelLoader.new(Project, full_path).find
+    else
+      ::GitlabSchema.object_from_id(project_id, expected_type: Project)
+    end
+  end
+end

app/graphql/resolvers/full_path_resolver.rb

@@ -11,12 +11,7 @@ module Resolvers
     end
 
     def model_by_full_path(model, full_path)
-      BatchLoader::GraphQL.for(full_path).batch(key: model) do |full_paths, loader, args|
-        # `with_route` avoids an N+1 calculating full_path
-        args[:key].where_full_path_in(full_paths).with_route.each do |model_instance|
-          loader.call(model_instance.full_path, model_instance)
-        end
-      end
+      ::Gitlab::Graphql::Loaders::FullPathModelLoader.new(model, full_path).find
     end
   end
 end

ee/app/graphql/mutations/requirements_management/create_requirement.rb

@@ -3,7 +3,7 @@
 module Mutations
   module RequirementsManagement
     class CreateRequirement < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'CreateRequirement'
 

ee/app/graphql/mutations/requirements_management/update_requirement.rb

@@ -3,7 +3,7 @@
 module Mutations
   module RequirementsManagement
     class UpdateRequirement < BaseMutation
-      include Mutations::ResolvesProject
+      include ResolvesProject
 
       graphql_name 'UpdateRequirement'
 

lib/gitlab/graphql/loaders/full_path_model_loader.rb

+# frozen_string_literal: true
+
+module Gitlab
+  module Graphql
+    module Loaders
+      # Suitable for use to find resources that expose `where_full_path_in`,
+      # such as Project, Group, Namespace
+      class FullPathModelLoader
+        attr_reader :model_class, :full_path
+
+        def initialize(model_class, full_path)
+          @model_class, @full_path = model_class, full_path
+        end
+
+        def find
+          BatchLoader::GraphQL.for(full_path).batch(key: model_class) do |full_paths, loader, args|
+            # `with_route` avoids an N+1 calculating full_path
+            args[:key].where_full_path_in(full_paths).with_route.each do |model_instance|
+              loader.call(model_instance.full_path, model_instance)
+            end
+          end
+        end
+      end
+    end
+  end
+end

spec/graphql/mutations/concerns/mutations/resolves_project_spec.rb

-# frozen_string_literal: true
-
-require 'spec_helper'
-
-describe Mutations::ResolvesProject do
-  let(:mutation_class) do
-    Class.new(Mutations::BaseMutation) do
-      include Mutations::ResolvesProject
-    end
-  end
-
-  let(:context) { double }
-
-  subject(:mutation) { mutation_class.new(object: nil, context: context, field: nil) }
-
-  it 'uses the ProjectsResolver to resolve projects by path' do
-    project = create(:project)
-
-    expect(Resolvers::ProjectResolver).to receive(:new).with(object: nil, context: context, field: nil).and_call_original
-    expect(mutation.resolve_project(full_path: project.full_path).sync).to eq(project)
-  end
-end

spec/graphql/resolvers/concerns/resolves_project_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe ResolvesProject do
+  include GraphqlHelpers
+
+  let(:implementing_class) do
+    Class.new do
+      include ResolvesProject
+    end
+  end
+
+  subject(:instance) { implementing_class.new }
+
+  let_it_be(:project) { create(:project) }
+
+  it 'can resolve projects by path' do
+    expect(sync(instance.resolve_project(full_path: project.full_path))).to eq(project)
+  end
+
+  it 'can resolve projects by id' do
+    expect(sync(instance.resolve_project(project_id: global_id_of(project)))).to eq(project)
+  end
+
+  it 'complains when both are present' do
+    expect do
+      instance.resolve_project(full_path: project.full_path, project_id: global_id_of(project))
+    end.to raise_error(::Gitlab::Graphql::Errors::ArgumentError)
+  end
+
+  it 'complains when neither is present' do
+    expect do
+      instance.resolve_project(full_path: nil, project_id: nil)
+    end.to raise_error(::Gitlab::Graphql::Errors::ArgumentError)
+  end
+end

spec/support/shared_examples/graphql/resolves_issuable_shared_examples.rb

@@ -29,16 +29,6 @@ RSpec.shared_examples 'resolving an issuable in GraphQL' do |type|
       subject
     end
 
-    it 'uses correct Resolver to resolve issuable parent' do
-      resolver_class = type == :epic ? 'Resolvers::GroupResolver' : 'Resolvers::ProjectResolver'
-
-      expect(resolver_class.constantize).to receive(:new)
-        .with(object: nil, context: context, field: nil)
-        .and_call_original
-
-      subject
-    end
-
     it 'returns nil if issuable is not found' do
       result = mutation.resolve_issuable(type: type, parent_path: parent.full_path, iid: "100")
       result = result.respond_to?(:sync) ? result.sync : result