diff options
author | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-05-23 09:55:14 +0200 |
---|---|---|
committer | Bob Van Landuyt <bob@vanlanduyt.co> | 2018-06-06 10:58:54 +0200 |
commit | 9b65d4bb417fb4939289eab94487c894f0a62db6 (patch) | |
tree | 1f97b9a1bd0d722a3c3ff4e89ec13bdb7a3aec00 /spec | |
parent | c443133e779c4c508b9c6429dd4ba623d64f03f1 (diff) | |
download | gitlab-ce-9b65d4bb417fb4939289eab94487c894f0a62db6.tar.gz |
Initial setup GraphQL using graphql-ruby 1.8
- All definitions have been replaced by classes:
http://graphql-ruby.org/schema/class_based_api.html
- Authorization & Presentation have been refactored to work in the
class based system
- Loaders have been replaced by resolvers
- Times are now coersed as ISO 8601
Diffstat (limited to 'spec')
-rw-r--r-- | spec/graphql/gitlab_schema_spec.rb | 15 | ||||
-rw-r--r-- | spec/graphql/resolvers/merge_request_resolver_spec.rb (renamed from spec/graphql/loaders/iid_loader_spec.rb) | 6 | ||||
-rw-r--r-- | spec/graphql/resolvers/project_resolver_spec.rb (renamed from spec/graphql/loaders/full_path_loader_spec.rb) | 6 | ||||
-rw-r--r-- | spec/graphql/types/project_type_spec.rb | 5 | ||||
-rw-r--r-- | spec/graphql/types/query_type_spec.rb | 10 | ||||
-rw-r--r-- | spec/graphql/types/time_type_spec.rb | 14 | ||||
-rw-r--r-- | spec/requests/api/graphql/merge_request_query_spec.rb | 49 | ||||
-rw-r--r-- | spec/requests/api/graphql/project_query_spec.rb | 39 | ||||
-rw-r--r-- | spec/requests/graphql/merge_request_query_spec.rb | 24 | ||||
-rw-r--r-- | spec/requests/graphql/project_query_spec.rb | 23 | ||||
-rw-r--r-- | spec/routing/api_routing_spec.rb | 31 | ||||
-rw-r--r-- | spec/support/helpers/graphql_helpers.rb | 47 | ||||
-rw-r--r-- | spec/support/matchers/graphql_matchers.rb | 23 | ||||
-rw-r--r-- | spec/support/shared_examples/requests/graphql_shared_examples.rb | 11 |
14 files changed, 208 insertions, 95 deletions
diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb index f25cc2fd6c9..b892f6b44ed 100644 --- a/spec/graphql/gitlab_schema_spec.rb +++ b/spec/graphql/gitlab_schema_spec.rb @@ -6,26 +6,25 @@ describe GitlabSchema do end it 'enables the preload instrumenter' do - expect(field_instrumenters).to include(instance_of(::GraphQL::Preload::Instrument)) + expect(field_instrumenters).to include(BatchLoader::GraphQL) end it 'enables the authorization instrumenter' do - expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize)) + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Authorize::Instrumentation)) end it 'enables using presenters' do - expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present)) + expect(field_instrumenters).to include(instance_of(::Gitlab::Graphql::Present::Instrumentation)) end it 'has the base mutation' do - pending <<~REASON - Having empty mutations breaks the automatic documentation in Graphiql, so removed for now." - REASON - expect(described_class.mutation).to eq(::Types::MutationType) + pending('Adding an empty mutation breaks the documentation explorer') + + expect(described_class.mutation).to eq(::Types::MutationType.to_graphql) end it 'has the base query' do - expect(described_class.query).to eq(::Types::QueryType) + expect(described_class.query).to eq(::Types::QueryType.to_graphql) end def field_instrumenters diff --git a/spec/graphql/loaders/iid_loader_spec.rb b/spec/graphql/resolvers/merge_request_resolver_spec.rb index 0a57d7c4ed4..af015533209 100644 --- a/spec/graphql/loaders/iid_loader_spec.rb +++ b/spec/graphql/resolvers/merge_request_resolver_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Loaders::IidLoader do +describe Resolvers::MergeRequestResolver do include GraphqlHelpers set(:project) { create(:project, :repository) } @@ -17,7 +17,7 @@ describe Loaders::IidLoader do let(:other_full_path) { other_project.full_path } let(:other_iid) { other_merge_request.iid } - describe '.merge_request' do + describe '#resolve' do it 'batch-resolves merge requests by target project full path and IID' do path = full_path # avoid database query @@ -53,6 +53,6 @@ describe Loaders::IidLoader do end def resolve_mr(full_path, iid) - resolve(described_class, :merge_request, args: { project: full_path, iid: iid }) + resolve(described_class, args: { full_path: full_path, iid: iid }) end end diff --git a/spec/graphql/loaders/full_path_loader_spec.rb b/spec/graphql/resolvers/project_resolver_spec.rb index 2732dd8c9da..d4990c6492c 100644 --- a/spec/graphql/loaders/full_path_loader_spec.rb +++ b/spec/graphql/resolvers/project_resolver_spec.rb @@ -1,6 +1,6 @@ require 'spec_helper' -describe Loaders::FullPathLoader do +describe Resolvers::ProjectResolver do include GraphqlHelpers set(:project1) { create(:project) } @@ -8,7 +8,7 @@ describe Loaders::FullPathLoader do set(:other_project) { create(:project) } - describe '.project' do + describe '#resolve' do it 'batch-resolves projects by full path' do paths = [project1.full_path, project2.full_path] @@ -27,6 +27,6 @@ describe Loaders::FullPathLoader do end def resolve_project(full_path) - resolve(described_class, :project, args: { full_path: full_path }) + resolve(described_class, args: { full_path: full_path }) end end diff --git a/spec/graphql/types/project_type_spec.rb b/spec/graphql/types/project_type_spec.rb new file mode 100644 index 00000000000..e0f89105b86 --- /dev/null +++ b/spec/graphql/types/project_type_spec.rb @@ -0,0 +1,5 @@ +require 'spec_helper' + +describe GitlabSchema.types['Project'] do + it { expect(described_class.graphql_name).to eq('Project') } +end diff --git a/spec/graphql/types/query_type_spec.rb b/spec/graphql/types/query_type_spec.rb index 17d9395504c..8488252fd59 100644 --- a/spec/graphql/types/query_type_spec.rb +++ b/spec/graphql/types/query_type_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe GitlabSchema.types['Query'] do it 'is called Query' do - expect(described_class.name).to eq('Query') + expect(described_class.graphql_name).to eq('Query') end it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) } @@ -13,7 +13,7 @@ describe GitlabSchema.types['Query'] do it 'finds projects by full path' do is_expected.to have_graphql_arguments(:full_path) is_expected.to have_graphql_type(Types::ProjectType) - is_expected.to have_graphql_resolver(Loaders::FullPathLoader[:project]) + is_expected.to have_graphql_resolver(Resolvers::ProjectResolver) end it 'authorizes with read_project' do @@ -22,12 +22,12 @@ describe GitlabSchema.types['Query'] do end describe 'merge_request field' do - subject { described_class.fields['merge_request'] } + subject { described_class.fields['mergeRequest'] } it 'finds MRs by project and IID' do - is_expected.to have_graphql_arguments(:project, :iid) + is_expected.to have_graphql_arguments(:full_path, :iid) is_expected.to have_graphql_type(Types::MergeRequestType) - is_expected.to have_graphql_resolver(Loaders::IidLoader[:merge_request]) + is_expected.to have_graphql_resolver(Resolvers::MergeRequestResolver) end it 'authorizes with read_merge_request' do diff --git a/spec/graphql/types/time_type_spec.rb b/spec/graphql/types/time_type_spec.rb index 087655cc67d..4196d9d27d4 100644 --- a/spec/graphql/types/time_type_spec.rb +++ b/spec/graphql/types/time_type_spec.rb @@ -1,16 +1,16 @@ require 'spec_helper' describe GitlabSchema.types['Time'] do - let(:float) { 1504630455.96215 } - let(:time) { Time.at(float) } + let(:iso) { "2018-06-04T15:23:50+02:00" } + let(:time) { Time.parse(iso) } - it { expect(described_class.name).to eq('Time') } + it { expect(described_class.graphql_name).to eq('Time') } - it 'coerces Time into fractional seconds since epoch' do - expect(described_class.coerce_isolated_result(time)).to eq(float) + it 'coerces Time object into ISO 8601' do + expect(described_class.coerce_isolated_result(time)).to eq(iso) end - it 'coerces fractional seconds since epoch into Time' do - expect(described_class.coerce_isolated_input(float)).to eq(time) + it 'coerces an ISO-time into Time object' do + expect(described_class.coerce_isolated_input(iso)).to eq(time) end end diff --git a/spec/requests/api/graphql/merge_request_query_spec.rb b/spec/requests/api/graphql/merge_request_query_spec.rb new file mode 100644 index 00000000000..12b1d5d18a2 --- /dev/null +++ b/spec/requests/api/graphql/merge_request_query_spec.rb @@ -0,0 +1,49 @@ +require 'spec_helper' + +describe 'getting merge request information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository) } + let(:merge_request) { create(:merge_request, source_project: project) } + let(:current_user) { create(:user) } + + let(:query) do + attributes = { + 'fullPath' => merge_request.project.full_path, + 'iid' => merge_request.iid + } + graphql_query_for('mergeRequest', attributes) + end + + context 'when the user has access to the merge request' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it 'returns the merge request' do + expect(graphql_data['mergeRequest']).not_to be_nil + end + + # This is a field coming from the `MergeRequestPresenter` + it 'includes a web_url' do + expect(graphql_data['mergeRequest']['webUrl']).to be_present + end + + it_behaves_like 'a working graphql query' + end + + context 'when the user does not have access to the merge request' do + before do + post_graphql(query, current_user: current_user) + end + + it 'returns an empty field' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['mergeRequest']).to be_nil + end + + it_behaves_like 'a working graphql query' + end +end diff --git a/spec/requests/api/graphql/project_query_spec.rb b/spec/requests/api/graphql/project_query_spec.rb new file mode 100644 index 00000000000..8196bcfa87c --- /dev/null +++ b/spec/requests/api/graphql/project_query_spec.rb @@ -0,0 +1,39 @@ +require 'spec_helper' + +describe 'getting project information' do + include GraphqlHelpers + + let(:project) { create(:project, :repository) } + let(:current_user) { create(:user) } + + let(:query) do + graphql_query_for('project', 'fullPath' => project.full_path) + end + + context 'when the user has access to the project' do + before do + project.add_developer(current_user) + post_graphql(query, current_user: current_user) + end + + it 'includes the project' do + expect(graphql_data['project']).not_to be_nil + end + + it_behaves_like 'a working graphql query' + end + + context 'when the user does not have access to the project' do + before do + post_graphql(query, current_user: current_user) + end + + it 'returns an empty field' do + post_graphql(query, current_user: current_user) + + expect(graphql_data['project']).to be_nil + end + + it_behaves_like 'a working graphql query' + end +end diff --git a/spec/requests/graphql/merge_request_query_spec.rb b/spec/requests/graphql/merge_request_query_spec.rb deleted file mode 100644 index cbc19782e2f..00000000000 --- a/spec/requests/graphql/merge_request_query_spec.rb +++ /dev/null @@ -1,24 +0,0 @@ -require 'spec_helper' - -describe 'getting merge request information' do - include GraphqlHelpers - - let(:project) { create(:project, :repository, :public) } - let(:merge_request) { create(:merge_request, source_project: project) } - - let(:query) do - <<~QUERY - { - merge_request(project: "#{merge_request.project.full_path}", iid: "#{merge_request.iid}") { - #{all_graphql_fields_for(MergeRequest)} - } - } - QUERY - end - - it_behaves_like 'a working graphql query' do - it 'renders a merge request with all fields' do - expect(response_data['merge_request']).not_to be_nil - end - end -end diff --git a/spec/requests/graphql/project_query_spec.rb b/spec/requests/graphql/project_query_spec.rb deleted file mode 100644 index 110ed433e03..00000000000 --- a/spec/requests/graphql/project_query_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -require 'spec_helper' - -describe 'getting project information' do - include GraphqlHelpers - - let(:project) { create(:project, :repository, :public) } - - let(:query) do - <<~QUERY - { - project(full_path: "#{project.full_path}") { - #{all_graphql_fields_for(Project)} - } - } - QUERY - end - - it_behaves_like 'a working graphql query' do - it 'renders a project with all fields' do - expect(response_data['project']).not_to be_nil - end - end -end diff --git a/spec/routing/api_routing_spec.rb b/spec/routing/api_routing_spec.rb new file mode 100644 index 00000000000..5fde4bd885b --- /dev/null +++ b/spec/routing/api_routing_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe 'api', 'routing' do + context 'when graphql is disabled' do + before do + stub_feature_flags(graphql: false) + end + + it 'does not route to the GraphqlController' do + expect(get('/api/graphql')).not_to route_to('graphql#execute') + end + + it 'does not expose graphiql' do + expect(get('/-/graphql-explorer')).not_to route_to('graphiql/rails/editors#show') + end + end + + context 'when graphql is disabled' do + before do + stub_feature_flags(graphql: true) + end + + it 'routes to the GraphqlController' do + expect(get('/api/graphql')).not_to route_to('graphql#execute') + end + + it 'exposes graphiql' do + expect(get('/-/graphql-explorer')).not_to route_to('graphiql/rails/editors#show') + end + end +end diff --git a/spec/support/helpers/graphql_helpers.rb b/spec/support/helpers/graphql_helpers.rb index ae29b16e32c..30ff9a1196a 100644 --- a/spec/support/helpers/graphql_helpers.rb +++ b/spec/support/helpers/graphql_helpers.rb @@ -1,7 +1,16 @@ module GraphqlHelpers + # makes an underscored string look like a fieldname + # "merge_request" => "mergeRequest" + def self.fieldnamerize(underscored_field_name) + graphql_field_name = underscored_field_name.to_s.camelize + graphql_field_name[0] = graphql_field_name[0].downcase + + graphql_field_name + end + # Run a loader's named resolver - def resolve(kls, name, obj: nil, args: {}, ctx: {}) - kls[name].call(obj, args, ctx) + def resolve(resolver_class, obj: nil, args: {}, ctx: {}) + resolver_class.new(object: obj, context: ctx).resolve(args) end # Runs a block inside a BatchLoader::Executor wrapper @@ -24,8 +33,20 @@ module GraphqlHelpers end end - def all_graphql_fields_for(klass) - type = GitlabSchema.types[klass.name] + def graphql_query_for(name, attributes = {}, fields = nil) + fields ||= all_graphql_fields_for(name.classify) + attributes = attributes_to_graphql(attributes) + <<~QUERY + { + #{name}(#{attributes}) { + #{fields} + } + } + QUERY + end + + def all_graphql_fields_for(class_name) + type = GitlabSchema.types[class_name.to_s] return "" unless type type.fields.map do |name, field| @@ -37,8 +58,22 @@ module GraphqlHelpers end.join("\n") end - def post_graphql(query) - post '/api/graphql', query: query + def attributes_to_graphql(attributes) + attributes.map do |name, value| + "#{GraphqlHelpers.fieldnamerize(name.to_s)}: \"#{value}\"" + end.join(", ") + end + + def post_graphql(query, current_user: nil) + post api('/', current_user, version: 'graphql'), query: query + end + + def graphql_data + json_response['data'] + end + + def graphql_errors + json_response['data'] end def scalar?(field) diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb index c0ed16ecaba..ba7a1c8cde0 100644 --- a/spec/support/matchers/graphql_matchers.rb +++ b/spec/support/matchers/graphql_matchers.rb @@ -1,31 +1,40 @@ RSpec::Matchers.define :require_graphql_authorizations do |*expected| match do |field| - authorizations = field.metadata[:authorize] - - expect(authorizations).to contain_exactly(*expected) + field_definition = field.metadata[:type_class] + expect(field_definition).to respond_to(:required_permissions) + expect(field_definition.required_permissions).to contain_exactly(*expected) end end RSpec::Matchers.define :have_graphql_fields do |*expected| match do |kls| - expect(kls.fields.keys).to contain_exactly(*expected.map(&:to_s)) + field_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + expect(kls.fields.keys).to contain_exactly(*field_names) end end RSpec::Matchers.define :have_graphql_arguments do |*expected| + include GraphqlHelpers + match do |field| - expect(field.arguments.keys).to contain_exactly(*expected.map(&:to_s)) + argument_names = expected.map { |name| GraphqlHelpers.fieldnamerize(name) } + expect(field.arguments.keys).to contain_exactly(*argument_names) end end RSpec::Matchers.define :have_graphql_type do |expected| match do |field| - expect(field.type).to eq(expected) + expect(field.type).to eq(expected.to_graphql) end end RSpec::Matchers.define :have_graphql_resolver do |expected| match do |field| - expect(field.resolve_proc).to eq(expected) + case expected + when Method + expect(field.metadata[:type_class].resolve_proc).to eq(expected) + else + expect(field.metadata[:type_class].resolver).to eq(expected) + end end end diff --git a/spec/support/shared_examples/requests/graphql_shared_examples.rb b/spec/support/shared_examples/requests/graphql_shared_examples.rb index c143b83696e..9b2b74593a5 100644 --- a/spec/support/shared_examples/requests/graphql_shared_examples.rb +++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb @@ -3,16 +3,9 @@ require 'spec_helper' shared_examples 'a working graphql query' do include GraphqlHelpers - let(:parsed_response) { JSON.parse(response.body) } - let(:response_data) { parsed_response['data'] } - - before do - post_graphql(query) - end - it 'is returns a successfull response', :aggregate_failures do expect(response).to be_success - expect(parsed_response['errors']).to be_nil - expect(response_data).not_to be_empty + expect(graphql_errors['errors']).to be_nil + expect(json_response.keys).to include('data') end end |