Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc41

8 files changed, 291 insertions, 118 deletions

diff --git a/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb b/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb
new file mode 100644
index 00000000000..1d8849f7e38
--- /dev/null
+++ b/spec/lib/gitlab/graphql/calls_gitaly/field_extension_spec.rb
@@ -0,0 +1,87 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::CallsGitaly::FieldExtension, :request_store do
+  include GraphqlHelpers
+
+  let(:field_args) { {} }
+  let(:owner) { fresh_object_type }
+  let(:field) do
+    ::Types::BaseField.new(name: 'value', type: GraphQL::STRING_TYPE, null: true, owner: owner, **field_args)
+  end
+
+  def resolve_value
+    resolve_field(field, { value: 'foo' }, object_type: owner)
+  end
+
+  context 'when the field calls gitaly' do
+    before do
+      owner.define_method :value do
+        Gitlab::SafeRequestStore['gitaly_call_actual'] = 1
+        'fresh-from-the-gitaly-mines!'
+      end
+    end
+
+    context 'when the field has a constant complexity' do
+      let(:field_args) { { complexity: 100 } }
+
+      it 'allows the call' do
+        expect { resolve_value }.not_to raise_error
+      end
+    end
+
+    context 'when the field declares that it calls gitaly' do
+      let(:field_args) { { calls_gitaly: true } }
+
+      it 'allows the call' do
+        expect { resolve_value }.not_to raise_error
+      end
+    end
+
+    context 'when the field does not have these arguments' do
+      let(:field_args) { {} }
+
+      it 'notices, and raises, mentioning the field' do
+        expect { resolve_value }.to raise_error(include('Object.value'))
+      end
+    end
+  end
+
+  context 'when it does not call gitaly' do
+    let(:field_args) { {} }
+
+    it 'does not raise' do
+      value = resolve_value
+
+      expect(value).to eq 'foo'
+    end
+  end
+
+  context 'when some field calls gitaly while we were waiting' do
+    let(:extension) { described_class.new(field: field, options: {}) }
+
+    it 'is acceptable if all are accounted for' do
+      object = :anything
+      arguments = :any_args
+
+      ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 3
+      ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 0
+
+      expect do |b|
+        extension.resolve(object: object, arguments: arguments, &b)
+      end.to yield_with_args(object, arguments, [3, 0])
+
+      ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 13
+      ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 10
+
+      expect { extension.after_resolve(value: 'foo', memo: [3, 0]) }.not_to raise_error
+    end
+
+    it 'is unacceptable if some of the calls are unaccounted for' do
+      ::Gitlab::SafeRequestStore['gitaly_call_actual'] = 10
+      ::Gitlab::SafeRequestStore['graphql_gitaly_accounted_for'] = 9
+
+      expect { extension.after_resolve(value: 'foo', memo: [0, 0]) }.to raise_error(include('Object.value'))
+    end
+  end
+end
diff --git a/spec/lib/gitlab/graphql/calls_gitaly/instrumentation_spec.rb b/spec/lib/gitlab/graphql/calls_gitaly/instrumentation_spec.rb
deleted file mode 100644
index f16767f7d14..00000000000
--- a/spec/lib/gitlab/graphql/calls_gitaly/instrumentation_spec.rb
+++ /dev/null
@@ -1,23 +0,0 @@
-# frozen_string_literal: true
-require 'spec_helper'
-
-RSpec.describe Gitlab::Graphql::CallsGitaly::Instrumentation do
-  subject { described_class.new }
-
-  describe '#calls_gitaly_check' do
-    let(:gitaly_field) { Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, null: true, calls_gitaly: true) }
-    let(:no_gitaly_field) { Types::BaseField.new(name: 'test', type: GraphQL::STRING_TYPE, null: true, calls_gitaly: false) }
-
-    context 'if there are no Gitaly calls' do
-      it 'does not raise an error if calls_gitaly is false' do
-        expect { subject.send(:calls_gitaly_check, no_gitaly_field, 0) }.not_to raise_error
-      end
-    end
-
-    context 'if there is at least 1 Gitaly call' do
-      it 'raises an error if calls_gitaly: is false or not defined' do
-        expect { subject.send(:calls_gitaly_check, no_gitaly_field, 1) }.to raise_error(/specify a constant complexity or add `calls_gitaly: true`/)
-      end
-    end
-  end
-end
diff --git a/spec/lib/gitlab/graphql/docs/renderer_spec.rb b/spec/lib/gitlab/graphql/docs/renderer_spec.rb
index 064e0c6828b..b956328e344 100644
--- a/spec/lib/gitlab/graphql/docs/renderer_spec.rb
+++ b/spec/lib/gitlab/graphql/docs/renderer_spec.rb
@@ -5,21 +5,25 @@ require 'spec_helper'
 RSpec.describe Gitlab::Graphql::Docs::Renderer do
   describe '#contents' do
     # Returns a Schema that uses the given `type`
-    def mock_schema(type)
+    def mock_schema(type, field_description)
       query_type = Class.new(Types::BaseObject) do
-        graphql_name 'QueryType'
+        graphql_name 'Query'
 
-        field :foo, type, null: true
+        field :foo, type, null: true do
+          description field_description
+          argument :id, GraphQL::ID_TYPE, required: false, description: 'ID of the object.'
+        end
       end
 
       GraphQL::Schema.define(query: query_type)
     end
 
     let_it_be(:template) { Rails.root.join('lib/gitlab/graphql/docs/templates/', 'default.md.haml') }
+    let(:field_description) { 'List of objects.' }
 
     subject(:contents) do
       described_class.new(
-        mock_schema(type).graphql_definition,
+        mock_schema(type, field_description).graphql_definition,
         output_dir: nil,
         template: template
       ).contents
@@ -36,7 +40,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
 
       specify do
         expectation = <<~DOC
-          ### ArrayTest
+          ### `ArrayTest`
 
           | Field | Type | Description |
           | ----- | ---- | ----------- |
@@ -45,6 +49,34 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
 
         is_expected.to include(expectation)
       end
+
+      context 'query generation' do
+        let(:expectation) do
+          <<~DOC
+            ### `foo`
+
+            List of objects.
+
+            #### Arguments
+
+            | Name | Type | Description |
+            | ---- | ---- | ----------- |
+            | `id` | ID | ID of the object. |
+          DOC
+        end
+
+        it 'generates the query with arguments' do
+          expect(subject).to include(expectation)
+        end
+
+        context 'when description does not end with `.`' do
+          let(:field_description) { 'List of objects' }
+
+          it 'adds the `.` to the end' do
+            expect(subject).to include(expectation)
+          end
+        end
+      end
     end
 
     context 'A type with fields defined in reverse alphabetical order' do
@@ -59,7 +91,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
 
       specify do
         expectation = <<~DOC
-          ### OrderingTest
+          ### `OrderingTest`
 
           | Field | Type | Description |
           | ----- | ---- | ----------- |
@@ -82,7 +114,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
 
       specify do
         expectation = <<~DOC
-          ### DeprecatedTest
+          ### `DeprecatedTest`
 
           | Field | Type | Description |
           | ----- | ---- | ----------- |
@@ -111,7 +143,7 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do
 
       specify do
         expectation = <<~DOC
-          ### MyEnum
+          ### `MyEnum`
 
           | Value | Description |
           | ----- | ----------- |
diff --git a/spec/lib/gitlab/graphql/pagination/keyset/last_items_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/last_items_spec.rb
index b45bb8b79d9..ec2ec4bf50d 100644
--- a/spec/lib/gitlab/graphql/pagination/keyset/last_items_spec.rb
+++ b/spec/lib/gitlab/graphql/pagination/keyset/last_items_spec.rb
@@ -4,7 +4,7 @@ require 'spec_helper'
 
 RSpec.describe Gitlab::Graphql::Pagination::Keyset::LastItems do
   let_it_be(:merge_request) { create(:merge_request) }
-  let(:scope) { MergeRequest.order_merged_at_asc.with_order_id_desc }
+  let(:scope) { MergeRequest.order_merged_at_asc }
 
   subject { described_class.take_items(*args) }
 
diff --git a/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb
index eb28e6c8c0a..40ee47ece49 100644
--- a/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb
+++ b/spec/lib/gitlab/graphql/pagination/keyset/order_info_spec.rb
@@ -52,18 +52,6 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::OrderInfo do
       end
     end
 
-    context 'when ordering by SIMILARITY' do
-      let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) }
-
-      it 'assigns the right attribute name, named function, and direction' do
-        expect(order_list.count).to eq 2
-        expect(order_list.first.attribute_name).to eq 'similarity'
-        expect(order_list.first.named_function).to be_kind_of(Arel::Nodes::Addition)
-        expect(order_list.first.named_function.to_sql).to include 'SIMILARITY('
-        expect(order_list.first.sort_direction).to eq :desc
-      end
-    end
-
     context 'when ordering by CASE', :aggregate_failuers do
       let(:relation) { Project.order(Arel::Nodes::Case.new(Project.arel_table[:pending_delete]).when(true).then(100).else(1000).asc) }
 
diff --git a/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb b/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb
index fa631aa5666..31c02fd43e8 100644
--- a/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb
+++ b/spec/lib/gitlab/graphql/pagination/keyset/query_builder_spec.rb
@@ -131,43 +131,5 @@ RSpec.describe Gitlab::Graphql::Pagination::Keyset::QueryBuilder do
         end
       end
     end
-
-    context 'when sorting using SIMILARITY' do
-      let(:relation) { Project.sorted_by_similarity_desc('test', include_in_select: true) }
-      let(:arel_table) { Project.arel_table }
-      let(:decoded_cursor) { { 'similarity' => 0.5, 'id' => 100 } }
-      let(:similarity_function_call) { Gitlab::Database::SimilarityScore::SIMILARITY_FUNCTION_CALL_WITH_ANNOTATION }
-      let(:similarity_sql) do
-        [
-          "(#{similarity_function_call}(COALESCE(\"projects\".\"path\", ''), 'test') * CAST('1' AS numeric))",
-          "(#{similarity_function_call}(COALESCE(\"projects\".\"name\", ''), 'test') * CAST('0.7' AS numeric))",
-          "(#{similarity_function_call}(COALESCE(\"projects\".\"description\", ''), 'test') * CAST('0.2' AS numeric))"
-        ].join(' + ')
-      end
-
-      context 'when no values are nil' do
-        context 'when :after' do
-          it 'generates the correct condition' do
-            conditions = builder.conditions.gsub(/\s+/, ' ')
-
-            expect(conditions).to include "(#{similarity_sql} < 0.5)"
-            expect(conditions).to include '"projects"."id" < 100'
-            expect(conditions).to include "OR (#{similarity_sql} IS NULL)"
-          end
-        end
-
-        context 'when :before' do
-          let(:before_or_after) { :before }
-
-          it 'generates the correct condition' do
-            conditions = builder.conditions.gsub(/\s+/, ' ')
-
-            expect(conditions).to include "(#{similarity_sql} > 0.5)"
-            expect(conditions).to include '"projects"."id" > 100'
-            expect(conditions).to include "OR ( #{similarity_sql} = 0.5"
-          end
-        end
-      end
-    end
   end
 end
diff --git a/spec/lib/gitlab/graphql/present/field_extension_spec.rb b/spec/lib/gitlab/graphql/present/field_extension_spec.rb
new file mode 100644
index 00000000000..5e66e16d655
--- /dev/null
+++ b/spec/lib/gitlab/graphql/present/field_extension_spec.rb
@@ -0,0 +1,143 @@
+# frozen_string_literal: true
+require 'spec_helper'
+
+RSpec.describe Gitlab::Graphql::Present::FieldExtension do
+  include GraphqlHelpers
+
+  let_it_be(:user) { create(:user) }
+
+  let(:object) { double(value: 'foo') }
+  let(:owner) { fresh_object_type }
+  let(:field_name) { 'value' }
+  let(:field) do
+    ::Types::BaseField.new(name: field_name, type: GraphQL::STRING_TYPE, null: true, owner: owner)
+  end
+
+  let(:base_presenter) do
+    Class.new(SimpleDelegator) do
+      def initialize(object, **options)
+        super(object)
+        @object = object
+        @options = options
+      end
+    end
+  end
+
+  def resolve_value
+    resolve_field(field, object, current_user: user, object_type: owner)
+  end
+
+  context 'when the object does not declare a presenter' do
+    it 'does not affect normal resolution' do
+      expect(resolve_value).to eq 'foo'
+    end
+  end
+
+  describe 'interactions with inheritance' do
+    def parent
+      type = fresh_object_type('Parent')
+      type.present_using(provide_foo)
+      type.field :foo, ::GraphQL::INT_TYPE, null: true
+      type.field :value, ::GraphQL::STRING_TYPE, null: true
+      type
+    end
+
+    def child
+      type = Class.new(parent)
+      type.graphql_name 'Child'
+      type.present_using(provide_bar)
+      type.field :bar, ::GraphQL::INT_TYPE, null: true
+      type
+    end
+
+    def provide_foo
+      Class.new(base_presenter) do
+        def foo
+          100
+        end
+      end
+    end
+
+    def provide_bar
+      Class.new(base_presenter) do
+        def bar
+          101
+        end
+      end
+    end
+
+    it 'can resolve value, foo and bar' do
+      type = child
+      value = resolve_field(:value, object, object_type: type)
+      foo = resolve_field(:foo, object, object_type: type)
+      bar = resolve_field(:bar, object, object_type: type)
+
+      expect([value, foo, bar]).to eq ['foo', 100, 101]
+    end
+  end
+
+  shared_examples 'calling the presenter method' do
+    it 'calls the presenter method' do
+      expect(resolve_value).to eq presenter.new(object, current_user: user).send(field_name)
+    end
+  end
+
+  context 'when the object declares a presenter' do
+    before do
+      owner.present_using(presenter)
+    end
+
+    context 'when the presenter overrides the original method' do
+      def twice
+        Class.new(base_presenter) do
+          def value
+            @object.value * 2
+          end
+        end
+      end
+
+      let(:presenter) { twice }
+
+      it_behaves_like 'calling the presenter method'
+    end
+
+    # This is exercised here using an explicit `resolve:` proc, but
+    # @resolver_proc values are used in field instrumentation as well.
+    context 'when the field uses a resolve proc' do
+      let(:presenter) { base_presenter }
+      let(:field) do
+        ::Types::BaseField.new(
+          name: field_name,
+          type: GraphQL::STRING_TYPE,
+          null: true,
+          owner: owner,
+          resolve: ->(obj, args, ctx) { 'Hello from a proc' }
+        )
+      end
+
+      specify { expect(resolve_value).to eq 'Hello from a proc' }
+    end
+
+    context 'when the presenter provides a new method' do
+      def presenter
+        Class.new(base_presenter) do
+          def current_username
+            "Hello #{@options[:current_user]&.username} from the presenter!"
+          end
+        end
+      end
+
+      context 'when we select the original field' do
+        it 'is unaffected' do
+          expect(resolve_value).to eq 'foo'
+        end
+      end
+
+      context 'when we select the new field' do
+        let(:field_name) { 'current_username' }
+
+        it_behaves_like 'calling the presenter method'
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb b/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb
index 138765afd8a..8450396284a 100644
--- a/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb
+++ b/spec/lib/gitlab/graphql/query_analyzers/logger_analyzer_spec.rb
@@ -5,42 +5,6 @@ require 'spec_helper'
 RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
   subject { described_class.new }
 
-  describe '#analyze?' do
-    context 'feature flag disabled' do
-      before do
-        stub_feature_flags(graphql_logging: false)
-      end
-
-      it 'disables the analyzer' do
-        expect(subject.analyze?(anything)).to be_falsey
-      end
-    end
-
-    context 'feature flag enabled by default' do
-      let(:monotonic_time_before) { 42 }
-      let(:monotonic_time_after) { 500 }
-      let(:monotonic_time_duration) { monotonic_time_after - monotonic_time_before }
-
-      it 'enables the analyzer' do
-        expect(subject.analyze?(anything)).to be_truthy
-      end
-
-      it 'returns a duration in seconds' do
-        allow(GraphQL::Analysis).to receive(:analyze_query).and_return([4, 2, [[], []]])
-        allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after)
-        allow(Gitlab::GraphqlLogger).to receive(:info)
-
-        expected_duration = monotonic_time_duration
-        memo = subject.initial_value(spy('query'))
-
-        subject.final_value(memo)
-
-        expect(memo).to have_key(:duration_s)
-        expect(memo[:duration_s]).to eq(expected_duration)
-      end
-    end
-  end
-
   describe '#initial_value' do
     it 'filters out sensitive variables' do
       doc = GraphQL.parse <<-GRAPHQL
@@ -58,4 +22,24 @@ RSpec.describe Gitlab::Graphql::QueryAnalyzers::LoggerAnalyzer do
       expect(subject.initial_value(query)[:variables]).to eq('{:body=>"[FILTERED]"}')
     end
   end
+
+  describe '#final_value' do
+    let(:monotonic_time_before) { 42 }
+    let(:monotonic_time_after) { 500 }
+    let(:monotonic_time_duration) { monotonic_time_after - monotonic_time_before }
+
+    it 'returns a duration in seconds' do
+      allow(GraphQL::Analysis).to receive(:analyze_query).and_return([4, 2, [[], []]])
+      allow(Gitlab::Metrics::System).to receive(:monotonic_time).and_return(monotonic_time_before, monotonic_time_after)
+      allow(Gitlab::GraphqlLogger).to receive(:info)
+
+      expected_duration = monotonic_time_duration
+      memo = subject.initial_value(spy('query'))
+
+      subject.final_value(memo)
+
+      expect(memo).to have_key(:duration_s)
+      expect(memo[:duration_s]).to eq(expected_duration)
+    end
+  end
 end