diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-16 18:18:33 +0000 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2021-03-16 18:18:33 +0000 |
commit | f64a639bcfa1fc2bc89ca7db268f594306edfd7c (patch) | |
tree | a2c3c2ebcc3b45e596949db485d6ed18ffaacfa1 /spec/lib/gitlab/graphql | |
parent | bfbc3e0d6583ea1a91f627528bedc3d65ba4b10f (diff) | |
download | gitlab-ce-f64a639bcfa1fc2bc89ca7db268f594306edfd7c.tar.gz |
Add latest changes from gitlab-org/gitlab@13-10-stable-eev13.10.0-rc40
Diffstat (limited to 'spec/lib/gitlab/graphql')
8 files changed, 461 insertions, 134 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..5afed8c3390 100644 --- a/spec/lib/gitlab/graphql/docs/renderer_spec.rb +++ b/spec/lib/gitlab/graphql/docs/renderer_spec.rb @@ -5,27 +5,50 @@ 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) + GraphQL::Schema.define( + query: query_type, + resolve_type: ->(obj, ctx) { raise 'Not a real schema' } + ) end - let_it_be(:template) { Rails.root.join('lib/gitlab/graphql/docs/templates/', 'default.md.haml') } + 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 end - context 'A type with a field with a [Array] return type' do + describe 'headings' do + let(:type) { ::GraphQL::INT_TYPE } + + it 'contains the expected sections' do + expect(contents.lines.map(&:chomp)).to include( + '## `Query` type', + '## Object types', + '## Enumeration types', + '## Scalar types', + '## Abstract types', + '### Unions', + '### Interfaces' + ) + end + end + + context 'when a field has a list type' do let(:type) do Class.new(Types::BaseObject) do graphql_name 'ArrayTest' @@ -35,19 +58,51 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do end specify do + type_name = '[String!]!' + inner_type = 'string' expectation = <<~DOC - ### ArrayTest + ### `ArrayTest` | Field | Type | Description | | ----- | ---- | ----------- | - | `foo` | String! => Array | A description. | + | `foo` | [`#{type_name}`](##{inner_type}) | A description. | DOC is_expected.to include(expectation) end + + describe 'a top level query field' do + let(:expectation) do + <<~DOC + ### `foo` + + List of objects. + + Returns [`ArrayTest`](#arraytest). + + #### Arguments + + | Name | Type | Description | + | ---- | ---- | ----------- | + | `id` | [`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 + describe 'when fields are not defined in alphabetical order' do let(:type) do Class.new(Types::BaseObject) do graphql_name 'OrderingTest' @@ -57,49 +112,56 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do end end - specify do + it 'lists the fields in alphabetical order' do expectation = <<~DOC - ### OrderingTest + ### `OrderingTest` | Field | Type | Description | | ----- | ---- | ----------- | - | `bar` | String! | A description of bar field. | - | `foo` | String! | A description of foo field. | + | `bar` | [`String!`](#string) | A description of bar field. | + | `foo` | [`String!`](#string) | A description of foo field. | DOC is_expected.to include(expectation) end end - context 'A type with a deprecated field' do + context 'when a field is deprecated' do let(:type) do Class.new(Types::BaseObject) do graphql_name 'DeprecatedTest' - field :foo, GraphQL::STRING_TYPE, null: false, deprecated: { reason: 'This is deprecated', milestone: '1.10' }, description: 'A description.' + field :foo, + type: GraphQL::STRING_TYPE, + null: false, + deprecated: { reason: 'This is deprecated', milestone: '1.10' }, + description: 'A description.' end end - specify do + it 'includes the deprecation' do expectation = <<~DOC - ### DeprecatedTest + ### `DeprecatedTest` | Field | Type | Description | | ----- | ---- | ----------- | - | `foo` **{warning-solid}** | String! | **Deprecated:** This is deprecated. Deprecated in 1.10. | + | `foo` **{warning-solid}** | [`String!`](#string) | **Deprecated:** This is deprecated. Deprecated in 1.10. | DOC is_expected.to include(expectation) end end - context 'A type with an emum field' do + context 'when a field has an Enumeration type' do let(:type) do enum_type = Class.new(Types::BaseEnum) do graphql_name 'MyEnum' - value 'BAZ', description: 'A description of BAZ.' - value 'BAR', description: 'A description of BAR.', deprecated: { reason: 'This is deprecated', milestone: '1.10' } + value 'BAZ', + description: 'A description of BAZ.' + value 'BAR', + description: 'A description of BAR.', + deprecated: { reason: 'This is deprecated', milestone: '1.10' } end Class.new(Types::BaseObject) do @@ -109,9 +171,9 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do end end - specify do + it 'includes the description of the Enumeration' do expectation = <<~DOC - ### MyEnum + ### `MyEnum` | Value | Description | | ----- | ----------- | @@ -122,5 +184,129 @@ RSpec.describe Gitlab::Graphql::Docs::Renderer do is_expected.to include(expectation) end end + + context 'when a field has a global ID type' do + let(:type) do + Class.new(Types::BaseObject) do + graphql_name 'IDTest' + description 'A test for rendering IDs.' + + field :foo, ::Types::GlobalIDType[::User], null: true, description: 'A user foo.' + end + end + + it 'includes the field and the description of the ID, so we can link to it' do + type_section = <<~DOC + ### `IDTest` + + A test for rendering IDs. + + | Field | Type | Description | + | ----- | ---- | ----------- | + | `foo` | [`UserID`](#userid) | A user foo. | + DOC + + id_section = <<~DOC + ### `UserID` + + A `UserID` is a global ID. It is encoded as a string. + + An example `UserID` is: `"gid://gitlab/User/1"`. + DOC + + is_expected.to include(type_section, id_section) + end + end + + context 'when there is an interface and a union' do + let(:type) do + user = Class.new(::Types::BaseObject) + user.graphql_name 'User' + user.field :user_field, ::GraphQL::STRING_TYPE, null: true + group = Class.new(::Types::BaseObject) + group.graphql_name 'Group' + group.field :group_field, ::GraphQL::STRING_TYPE, null: true + + union = Class.new(::Types::BaseUnion) + union.graphql_name 'UserOrGroup' + union.description 'Either a user or a group.' + union.possible_types user, group + + interface = Module.new + interface.include(::Types::BaseInterface) + interface.graphql_name 'Flying' + interface.description 'Something that can fly.' + interface.field :flight_speed, GraphQL::INT_TYPE, null: true, description: 'Speed in mph.' + + african_swallow = Class.new(::Types::BaseObject) + african_swallow.graphql_name 'AfricanSwallow' + african_swallow.description 'A swallow from Africa.' + african_swallow.implements interface + interface.orphan_types african_swallow + + Class.new(::Types::BaseObject) do + graphql_name 'AbstactTypeTest' + description 'A test for abstract types.' + + field :foo, union, null: true, description: 'The foo.' + field :flying, interface, null: true, description: 'A flying thing.' + end + end + + it 'lists the fields correctly, and includes descriptions of all the types' do + type_section = <<~DOC + ### `AbstactTypeTest` + + A test for abstract types. + + | Field | Type | Description | + | ----- | ---- | ----------- | + | `flying` | [`Flying`](#flying) | A flying thing. | + | `foo` | [`UserOrGroup`](#userorgroup) | The foo. | + DOC + + union_section = <<~DOC + #### `UserOrGroup` + + Either a user or a group. + + One of: + + - [`Group`](#group) + - [`User`](#user) + DOC + + interface_section = <<~DOC + #### `Flying` + + Something that can fly. + + Implementations: + + - [`AfricanSwallow`](#africanswallow) + + | Field | Type | Description | + | ----- | ---- | ----------- | + | `flightSpeed` | [`Int`](#int) | Speed in mph. | + DOC + + implementation_section = <<~DOC + ### `AfricanSwallow` + + A swallow from Africa. + + | Field | Type | Description | + | ----- | ---- | ----------- | + | `flightSpeed` | [`Int`](#int) | Speed in mph. | + DOC + + is_expected.to include( + type_section, + union_section, + interface_section, + implementation_section + ) + end + end end end 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 |