diff --git a/.rubocop.yml b/.rubocop.yml index 2639a33f363..0582bfe8d70 100644 --- a/.rubocop.yml +++ b/.rubocop.yml @@ -43,7 +43,6 @@ Naming/FileName: - 'config/**/*' - 'lib/generators/**/*' - 'ee/lib/generators/**/*' - - 'app/graphql/**/*' IgnoreExecutableScripts: true AllowedAcronyms: - EE diff --git a/Gemfile b/Gemfile index 8f1d8f66f10..4c63f4c10b8 100644 --- a/Gemfile +++ b/Gemfile @@ -94,8 +94,7 @@ gem 'grape-entity', '~> 0.7.1' gem 'rack-cors', '~> 1.0.0', require: 'rack/cors' # GraphQL API -gem 'graphql', '~> 1.7.14' -gem 'graphql-preload', '~> 2.0.0' +gem 'graphql', '~> 1.8.0' gem 'graphiql-rails', '~> 1.4.10' # Disable strong_params so that Mash does not respond to :permitted? diff --git a/Gemfile.lock b/Gemfile.lock index 8fe2f5f06f8..761dc3c1804 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -368,15 +368,7 @@ GEM graphiql-rails (1.4.10) railties sprockets-rails - graphql (1.7.14) - graphql-batch (0.3.9) - graphql (>= 0.8, < 2) - promise.rb (~> 0.7.2) - graphql-preload (2.0.1) - activerecord (>= 4.1, < 6) - graphql (>= 1.5, < 2) - graphql-batch (~> 0.3) - promise.rb (~> 0.7) + graphql (1.8.1) grpc (1.11.0) google-protobuf (~> 3.1) googleapis-common-protos-types (~> 1.0.0) @@ -639,7 +631,6 @@ GEM unparser procto (0.0.3) prometheus-client-mmap (0.9.3) - promise.rb (0.7.4) pry (0.10.4) coderay (~> 1.1.0) method_source (~> 0.8.1) @@ -1067,8 +1058,7 @@ DEPENDENCIES grape-path-helpers (~> 1.0) grape_logging (~> 1.7) graphiql-rails (~> 1.4.10) - graphql (~> 1.7.14) - graphql-preload (~> 2.0.0) + graphql (~> 1.8.0) grpc (~> 1.11.0) haml_lint (~> 0.26.0) hamlit (~> 2.6.1) diff --git a/app/graphql/functions/base_function.rb b/app/graphql/functions/base_function.rb new file mode 100644 index 00000000000..42fb8f99acc --- /dev/null +++ b/app/graphql/functions/base_function.rb @@ -0,0 +1,4 @@ +module Functions + class BaseFunction < GraphQL::Function + end +end diff --git a/app/graphql/functions/echo.rb b/app/graphql/functions/echo.rb new file mode 100644 index 00000000000..e5bf109b8d7 --- /dev/null +++ b/app/graphql/functions/echo.rb @@ -0,0 +1,13 @@ +module Functions + class Echo < BaseFunction + argument :text, GraphQL::STRING_TYPE + + description "Testing endpoint to validate the API with" + + def call(obj, args, ctx) + username = ctx[:current_user]&.username + + "#{username.inspect} says: #{args[:text]}" + end + end +end diff --git a/app/graphql/gitlab_schema.rb b/app/graphql/gitlab_schema.rb index ac75c4bf2e3..de4fc1d8e32 100644 --- a/app/graphql/gitlab_schema.rb +++ b/app/graphql/gitlab_schema.rb @@ -1,12 +1,8 @@ -Gitlab::Graphql::Authorize.register! -Gitlab::Graphql::Present.register! - -GitlabSchema = GraphQL::Schema.define do +class GitlabSchema < GraphQL::Schema use BatchLoader::GraphQL - - enable_preloading - enable_authorization - enable_presenting + use Gitlab::Graphql::Authorize + use Gitlab::Graphql::Present query(Types::QueryType) + # mutation(Types::MutationType) end diff --git a/app/graphql/loaders/base_loader.rb b/app/graphql/loaders/base_loader.rb deleted file mode 100644 index aad435ea09b..00000000000 --- a/app/graphql/loaders/base_loader.rb +++ /dev/null @@ -1,15 +0,0 @@ -# Helper methods for all loaders -module Loaders::BaseLoader - extend ActiveSupport::Concern - - class_methods do - # Convert a class method into a resolver proc. The method should follow the - # (obj, args, ctx) calling convention - def [](sym) - resolver = method(sym) - raise ArgumentError.new("#{self}.#{sym} is not a resolver") unless resolver.arity == 3 - - resolver - end - end -end diff --git a/app/graphql/loaders/iid_loader.rb b/app/graphql/loaders/iid_loader.rb deleted file mode 100644 index f9827765a92..00000000000 --- a/app/graphql/loaders/iid_loader.rb +++ /dev/null @@ -1,23 +0,0 @@ -class Loaders::IidLoader - include Loaders::BaseLoader - - class << self - def merge_request(obj, args, ctx) - iid = args[:iid] - project = Loaders::FullPathLoader.project_by_full_path(args[:project]) - merge_request_by_project_and_iid(project, iid) - end - - def merge_request_by_project_and_iid(project_loader, iid) - project_id = project_loader.__sync&.id - - # IIDs are represented as the GraphQL `id` type, which is a string - BatchLoader.for(iid.to_s).batch(key: "merge_request:target_project:#{project_id}:iid") do |iids, loader| - if project_id - results = MergeRequest.where(target_project_id: project_id, iid: iids) - results.each { |mr| loader.call(mr.iid.to_s, mr) } - end - end - end - end -end diff --git a/app/graphql/resolvers/base_resolver.rb b/app/graphql/resolvers/base_resolver.rb new file mode 100644 index 00000000000..89b7f9dad6f --- /dev/null +++ b/app/graphql/resolvers/base_resolver.rb @@ -0,0 +1,4 @@ +module Resolvers + class BaseResolver < GraphQL::Schema::Resolver + end +end diff --git a/app/graphql/loaders/full_path_loader.rb b/app/graphql/resolvers/full_path_resolver.rb similarity index 59% rename from app/graphql/loaders/full_path_loader.rb rename to app/graphql/resolvers/full_path_resolver.rb index 27ada90b82a..4eb28aaed6c 100644 --- a/app/graphql/loaders/full_path_loader.rb +++ b/app/graphql/resolvers/full_path_resolver.rb @@ -1,13 +1,11 @@ -module Loaders::FullPathLoader - include Loaders::BaseLoader +module Resolvers + module FullPathResolver + extend ActiveSupport::Concern - class << self - def project(obj, args, ctx) - project_by_full_path(args[:full_path]) - end - - def project_by_full_path(full_path) - model_by_full_path(Project, full_path) + prepended do + argument :full_path, GraphQL::ID_TYPE, + required: true, + description: 'The full path of the project or namespace, e.g., "gitlab-org/gitlab-ce"' end def model_by_full_path(model, full_path) diff --git a/app/graphql/resolvers/merge_request_resolver.rb b/app/graphql/resolvers/merge_request_resolver.rb new file mode 100644 index 00000000000..b1857ab09f7 --- /dev/null +++ b/app/graphql/resolvers/merge_request_resolver.rb @@ -0,0 +1,21 @@ +module Resolvers + class MergeRequestResolver < BaseResolver + prepend FullPathResolver + + type Types::ProjectType, null: true + + argument :iid, GraphQL::ID_TYPE, + required: true, + description: 'The IID of the merge request, e.g., "1"' + + def resolve(full_path:, iid:) + project = model_by_full_path(Project, full_path) + return unless project.present? + + BatchLoader.for(iid.to_s).batch(key: project.id) do |iids, loader| + results = project.merge_requests.where(iid: iids) + results.each { |mr| loader.call(mr.iid.to_s, mr) } + end + end + end +end diff --git a/app/graphql/resolvers/project_resolver.rb b/app/graphql/resolvers/project_resolver.rb new file mode 100644 index 00000000000..ec115bad896 --- /dev/null +++ b/app/graphql/resolvers/project_resolver.rb @@ -0,0 +1,11 @@ +module Resolvers + class ProjectResolver < BaseResolver + prepend FullPathResolver + + type Types::ProjectType, null: true + + def resolve(full_path:) + model_by_full_path(Project, full_path) + end + end +end diff --git a/app/graphql/types/base_enum.rb b/app/graphql/types/base_enum.rb new file mode 100644 index 00000000000..b45a845f74f --- /dev/null +++ b/app/graphql/types/base_enum.rb @@ -0,0 +1,4 @@ +module Types + class BaseEnum < GraphQL::Schema::Enum + end +end diff --git a/app/graphql/types/base_field.rb b/app/graphql/types/base_field.rb new file mode 100644 index 00000000000..c5740a334d7 --- /dev/null +++ b/app/graphql/types/base_field.rb @@ -0,0 +1,5 @@ +module Types + class BaseField < GraphQL::Schema::Field + prepend Gitlab::Graphql::Authorize + end +end diff --git a/app/graphql/types/base_input_object.rb b/app/graphql/types/base_input_object.rb new file mode 100644 index 00000000000..309e336e6c8 --- /dev/null +++ b/app/graphql/types/base_input_object.rb @@ -0,0 +1,4 @@ +module Types + class BaseInputObject < GraphQL::Schema::InputObject + end +end diff --git a/app/graphql/types/base_interface.rb b/app/graphql/types/base_interface.rb new file mode 100644 index 00000000000..69e72dc5808 --- /dev/null +++ b/app/graphql/types/base_interface.rb @@ -0,0 +1,5 @@ +module Types + module BaseInterface + include GraphQL::Schema::Interface + end +end diff --git a/app/graphql/types/base_object.rb b/app/graphql/types/base_object.rb new file mode 100644 index 00000000000..e033ef96ce9 --- /dev/null +++ b/app/graphql/types/base_object.rb @@ -0,0 +1,7 @@ +module Types + class BaseObject < GraphQL::Schema::Object + prepend Gitlab::Graphql::Present + + field_class Types::BaseField + end +end diff --git a/app/graphql/types/base_scalar.rb b/app/graphql/types/base_scalar.rb new file mode 100644 index 00000000000..c0aa38be239 --- /dev/null +++ b/app/graphql/types/base_scalar.rb @@ -0,0 +1,4 @@ +module Types + class BaseScalar < GraphQL::Schema::Scalar + end +end diff --git a/app/graphql/types/base_union.rb b/app/graphql/types/base_union.rb new file mode 100644 index 00000000000..36337fc6ee5 --- /dev/null +++ b/app/graphql/types/base_union.rb @@ -0,0 +1,4 @@ +module Types + class BaseUnion < GraphQL::Schema::Union + end +end diff --git a/app/graphql/types/merge_request_type.rb b/app/graphql/types/merge_request_type.rb index fc430ca03d5..d5d24952984 100644 --- a/app/graphql/types/merge_request_type.rb +++ b/app/graphql/types/merge_request_type.rb @@ -1,46 +1,47 @@ -Types::MergeRequestType = GraphQL::ObjectType.define do - present_using MergeRequestPresenter +module Types + class MergeRequestType < BaseObject + present_using MergeRequestPresenter - name 'MergeRequest' + graphql_name 'MergeRequest' - field :id, !types.ID - field :iid, !types.ID - field :title, !types.String - field :description, types.String - field :state, types.String - field :created_at, !Types::TimeType - field :updated_at, !Types::TimeType - field :source_project, Types::ProjectType - field :target_project, !Types::ProjectType - # Alias for target_project - field :project, !Types::ProjectType - field :project_id, !types.Int, property: :target_project_id - field :source_project_id, types.Int - field :target_project_id, !types.Int - field :source_branch, !types.String - field :target_branch, !types.String - field :work_in_progress, types.Boolean, property: :work_in_progress? - field :merge_when_pipeline_succeeds, types.Boolean - field :sha, types.String, property: :diff_head_sha - field :merge_commit_sha, types.String - field :user_notes_count, types.Int - field :should_remove_source_branch, types.Boolean, property: :should_remove_source_branch? - field :force_remove_source_branch, types.Boolean, property: :force_remove_source_branch? - field :merge_status, types.String - field :in_progress_merge_commit_sha, types.String - field :merge_error, types.String - field :allow_maintainer_to_push, types.Boolean - field :should_be_rebased, types.Boolean, property: :should_be_rebased? - field :rebase_commit_sha, types.String - field :rebase_in_progress, types.Boolean, property: :rebase_in_progress? - field :diff_head_sha, types.String - field :merge_commit_message, types.String - field :merge_ongoing, types.Boolean, property: :merge_ongoing? - field :work_in_progress, types.Boolean, property: :work_in_progress? - field :source_branch_exists, types.Boolean, property: :source_branch_exists? - field :mergeable_discussions_state, types.Boolean - field :web_url, types.String, property: :web_url - field :upvotes, types.Int - field :downvotes, types.Int - field :subscribed, types.Boolean, property: :subscribed? + field :id, GraphQL::ID_TYPE, null: false + field :iid, GraphQL::ID_TYPE, null: false + field :title, GraphQL::STRING_TYPE, null: false + field :description, GraphQL::STRING_TYPE, null: true + field :state, GraphQL::STRING_TYPE, null: true + field :created_at, Types::TimeType, null: false + field :updated_at, Types::TimeType, null: false + field :source_project, Types::ProjectType, null: true + field :target_project, Types::ProjectType, null: false + # Alias for target_project + field :project, Types::ProjectType, null: false + field :project_id, GraphQL::INT_TYPE, null: false, method: :target_project_id + field :source_project_id, GraphQL::INT_TYPE, null: true + field :target_project_id, GraphQL::INT_TYPE, null: false + field :source_branch, GraphQL::STRING_TYPE, null: false + field :target_branch, GraphQL::STRING_TYPE, null: false + field :work_in_progress, GraphQL::BOOLEAN_TYPE, method: :work_in_progress?, null: false + field :merge_when_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true + field :diff_head_sha, GraphQL::STRING_TYPE, null: true + field :merge_commit_sha, GraphQL::STRING_TYPE, null: true + field :user_notes_count, GraphQL::INT_TYPE, null: true + field :should_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :should_remove_source_branch?, null: true + field :force_remove_source_branch, GraphQL::BOOLEAN_TYPE, method: :force_remove_source_branch?, null: true + field :merge_status, GraphQL::STRING_TYPE, null: true + field :in_progress_merge_commit_sha, GraphQL::STRING_TYPE, null: true + field :merge_error, GraphQL::STRING_TYPE, null: true + field :allow_collaboration, GraphQL::BOOLEAN_TYPE, null: true + field :should_be_rebased, GraphQL::BOOLEAN_TYPE, method: :should_be_rebased?, null: false + field :rebase_commit_sha, GraphQL::STRING_TYPE, null: true + field :rebase_in_progress, GraphQL::BOOLEAN_TYPE, method: :rebase_in_progress?, null: false + field :diff_head_sha, GraphQL::STRING_TYPE, null: true + field :merge_commit_message, GraphQL::STRING_TYPE, null: true + field :merge_ongoing, GraphQL::BOOLEAN_TYPE, method: :merge_ongoing?, null: false + field :source_branch_exists, GraphQL::BOOLEAN_TYPE, method: :source_branch_exists?, null: false + field :mergeable_discussions_state, GraphQL::BOOLEAN_TYPE, null: true + field :web_url, GraphQL::STRING_TYPE, null: true + field :upvotes, GraphQL::INT_TYPE, null: false + field :downvotes, GraphQL::INT_TYPE, null: false + field :subscribed, GraphQL::BOOLEAN_TYPE, method: :subscribed?, null: false + end end diff --git a/app/graphql/types/mutation_type.rb b/app/graphql/types/mutation_type.rb index c5061f10239..06ed91c1658 100644 --- a/app/graphql/types/mutation_type.rb +++ b/app/graphql/types/mutation_type.rb @@ -1,5 +1,7 @@ -Types::MutationType = GraphQL::ObjectType.define do - name "Mutation" +module Types + class MutationType < BaseObject + graphql_name "Mutation" - # TODO: Add Mutations as fields + # TODO: Add Mutations as fields + end end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb index 593c1b67830..9e885d5845a 100644 --- a/app/graphql/types/project_type.rb +++ b/app/graphql/types/project_type.rb @@ -1,63 +1,65 @@ -Types::ProjectType = GraphQL::ObjectType.define do - name 'Project' +module Types + class ProjectType < BaseObject + graphql_name 'Project' - field :id, !types.ID + field :id, GraphQL::ID_TYPE, null: false - field :full_path, !types.ID - field :path, !types.String + field :full_path, GraphQL::ID_TYPE, null: false + field :path, GraphQL::STRING_TYPE, null: false - field :name_with_namespace, !types.String - field :name, !types.String + field :name_with_namespace, GraphQL::STRING_TYPE, null: false + field :name, GraphQL::STRING_TYPE, null: false - field :description, types.String + field :description, GraphQL::STRING_TYPE, null: true - field :default_branch, types.String - field :tag_list, types.String + field :default_branch, GraphQL::STRING_TYPE, null: true + field :tag_list, GraphQL::STRING_TYPE, null: true - field :ssh_url_to_repo, types.String - field :http_url_to_repo, types.String - field :web_url, types.String + field :ssh_url_to_repo, GraphQL::STRING_TYPE, null: true + field :http_url_to_repo, GraphQL::STRING_TYPE, null: true + field :web_url, GraphQL::STRING_TYPE, null: true - field :star_count, !types.Int - field :forks_count, !types.Int + field :star_count, GraphQL::INT_TYPE, null: false + field :forks_count, GraphQL::INT_TYPE, null: false - field :created_at, Types::TimeType - field :last_activity_at, Types::TimeType + field :created_at, Types::TimeType, null: true + field :last_activity_at, Types::TimeType, null: true - field :archived, types.Boolean + field :archived, GraphQL::BOOLEAN_TYPE, null: true - field :visibility, types.String + field :visibility, GraphQL::STRING_TYPE, null: true - field :container_registry_enabled, types.Boolean - field :shared_runners_enabled, types.Boolean - field :lfs_enabled, types.Boolean - field :ff_only_enabled, types.Boolean, property: :merge_requests_ff_only_enabled + field :container_registry_enabled, GraphQL::BOOLEAN_TYPE, null: true + field :shared_runners_enabled, GraphQL::BOOLEAN_TYPE, null: true + field :lfs_enabled, GraphQL::BOOLEAN_TYPE, null: true + field :merge_requests_ff_only_enabled, GraphQL::BOOLEAN_TYPE, null: true - field :avatar_url, types.String do - resolve ->(project, args, ctx) { project.avatar_url(only_path: false) } - end - - %i[issues merge_requests wiki snippets].each do |feature| - field "#{feature}_enabled", types.Boolean do - resolve ->(project, args, ctx) { project.feature_available?(feature, ctx[:current_user]) } + field :avatar_url, GraphQL::STRING_TYPE, null: true, resolve: -> (project, args, ctx) do + project.avatar_url(only_path: false) end + + %i[issues merge_requests wiki snippets].each do |feature| + field "#{feature}_enabled", GraphQL::BOOLEAN_TYPE, null: true, resolve: -> (project, args, ctx) do + project.feature_available?(feature, ctx[:current_user]) + end + end + + field :jobs_enabled, GraphQL::BOOLEAN_TYPE, null: true, resolve: -> (project, args, ctx) do + project.feature_available?(:builds, ctx[:current_user]) + end + + field :public_jobs, GraphQL::BOOLEAN_TYPE, method: :public_builds, null: true + + field :open_issues_count, GraphQL::INT_TYPE, null: true, resolve: -> (project, args, ctx) do + project.open_issues_count if project.feature_available?(:issues, ctx[:current_user]) + end + + field :import_status, GraphQL::STRING_TYPE, null: true + field :ci_config_path, GraphQL::STRING_TYPE, null: true + + field :only_allow_merge_if_pipeline_succeeds, GraphQL::BOOLEAN_TYPE, null: true + field :request_access_enabled, GraphQL::BOOLEAN_TYPE, null: true + field :only_allow_merge_if_all_discussions_are_resolved, GraphQL::BOOLEAN_TYPE, null: true + field :printing_merge_request_link_enabled, GraphQL::BOOLEAN_TYPE, null: true end - - field :jobs_enabled, types.Boolean do - resolve ->(project, args, ctx) { project.feature_available?(:builds, ctx[:current_user]) } - end - - field :public_jobs, types.Boolean, property: :public_builds - - field :open_issues_count, types.Int do - resolve ->(project, args, ctx) { project.open_issues_count if project.feature_available?(:issues, ctx[:current_user]) } - end - - field :import_status, types.String - field :ci_config_path, types.String - - field :only_allow_merge_if_pipeline_succeeds, types.Boolean - field :request_access_enabled, types.Boolean - field :only_allow_merge_if_all_discussions_are_resolved, types.Boolean - field :printing_merge_request_link_enabled, types.Boolean end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb index 029bbd098ad..be79c78bf67 100644 --- a/app/graphql/types/query_type.rb +++ b/app/graphql/types/query_type.rb @@ -1,38 +1,21 @@ -Types::QueryType = GraphQL::ObjectType.define do - name 'Query' +module Types + class QueryType < BaseObject + graphql_name 'Query' - field :project, Types::ProjectType do - argument :full_path, !types.ID do - description 'The full path of the project, e.g., "gitlab-org/gitlab-ce"' + field :project, Types::ProjectType, + null: true, + resolver: Resolvers::ProjectResolver, + description: "Find a project" do + authorize :read_project end - authorize :read_project - - resolve Loaders::FullPathLoader[:project] - end - - field :merge_request, Types::MergeRequestType do - argument :project, !types.ID do - description 'The full path of the target project, e.g., "gitlab-org/gitlab-ce"' + field :merge_request, Types::MergeRequestType, + null: true, + resolver: Resolvers::MergeRequestResolver, + description: "Find a merge request" do + authorize :read_merge_request end - argument :iid, !types.ID do - description 'The IID of the merge request, e.g., "1"' - end - - authorize :read_merge_request - - resolve Loaders::IidLoader[:merge_request] - end - - # Testing endpoint to validate the API with - field :echo, types.String do - argument :text, types.String - - resolve -> (obj, args, ctx) do - username = ctx[:current_user]&.username - - "#{username.inspect} says: #{args[:text]}" - end + field :echo, GraphQL::STRING_TYPE, null: false, function: Functions::Echo.new end end diff --git a/app/graphql/types/time_type.rb b/app/graphql/types/time_type.rb index fb717eb3dc7..2333d82ad1e 100644 --- a/app/graphql/types/time_type.rb +++ b/app/graphql/types/time_type.rb @@ -1,8 +1,14 @@ -# Taken from http://www.rubydoc.info/github/rmosolgo/graphql-ruby/GraphQL/ScalarType -Types::TimeType = GraphQL::ScalarType.define do - name 'Time' - description 'Time since epoch in fractional seconds' +module Types + class TimeType < BaseScalar + graphql_name 'Time' + description 'Time represented in ISO 8601' - coerce_input ->(value, ctx) { Time.at(Float(value)) } - coerce_result ->(value, ctx) { value.to_f } + def self.coerce_input(value, ctx) + Time.parse(value) + end + + def self.coerce_result(value, ctx) + value.iso8601 + end + end end diff --git a/config/routes/api.rb b/config/routes/api.rb index 54ce6d01df0..b1aebf4d606 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -1,5 +1,7 @@ -post '/api/graphql', to: 'graphql#execute' -mount GraphiQL::Rails::Engine, at: '/api/graphiql', graphql_path: '/api/graphql' +constraints(::Constraints::FeatureConstrainer.new(:graphql)) do + post '/api/graphql', to: 'graphql#execute' + mount GraphiQL::Rails::Engine, at: '/-/graphql-explorer', graphql_path: '/api/graphql' +end API::API.logger Rails.logger mount API::API => '/' diff --git a/doc/api/graphql/index.md b/doc/api/graphql/index.md index 78e634df347..dcd5377284c 100644 --- a/doc/api/graphql/index.md +++ b/doc/api/graphql/index.md @@ -2,33 +2,41 @@ > [Introduced][ce-19008] in GitLab 11.0. +[GraphQL](https://graphql.org/) is a query language for APIs that +allows clients to request exactly the data they need, making it +possible to get all required data in a limited number of requests. + +The GraphQL data (fields) can be described in the form of types, +allowing clients to use [clientside GraphQL +libraries](https://graphql.org/code/#graphql-clients) to consume the +API and avoid manual parsing. + +Since there's no fixed endpoints and datamodel, new abilities can be +added to the API without creating breaking changes. This allows us to +have a versionless API as described in [the GraphQL +documentation](https://graphql.org/learn/best-practices/#versioning). + ## Enabling the GraphQL feature -The GraphQL API itself is currently in Beta, and therefore hidden behind a -feature flag. To enable it on your selfhosted instance, run -`Feature.enable(:graphql)`. +The GraphQL API itself is currently in Alpha, and therefore hidden behind a +feature flag. You can enable the feature using the [features api][features-api] on a self-hosted instance. -Start the console by running +For example: -```bash -sudo gitlab-rails console -``` - -Then enable the feature by running - -```ruby -Feature.enable(:graphql) +```shell +curl --data "value=100" --header "PRIVATE-TOKEN: 9koXpg98eAheJpvBs5tK" https://gitlab.example.com/api/v4/features/graphql ``` ## Available queries -A first iteration of a GraphQL API inlcudes only 2 queries: `project` and +A first iteration of a GraphQL API includes only 2 queries: `project` and `merge_request` and only returns scalar fields, or fields of the type `Project` or `MergeRequest`. ## GraphiQL The API can be explored by using the GraphiQL IDE, it is available on your -instance on `gitlab.example.com/api/graphiql`. +instance on `gitlab.example.com/-/graphql-explorer`. [ce-19008]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19008 +[features-api]: ../features.md diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 4d73efa3f71..f74e4f0bd7e 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -2,8 +2,8 @@ ## Authentication -Authentication happens through the `GrapqlController`, right now this -uses the same authentication as the rails application. So the session +Authentication happens through the `GraphqlController`, right now this +uses the same authentication as the Rails application. So the session can be shared. It is also possible to add a `private_token` to the querystring, or @@ -11,27 +11,25 @@ add a `HTTP_PRIVATE_TOKEN` header. ### Authorization -Fields can be authorized using the same abilities used in the rails +Fields can be authorized using the same abilities used in the Rails app. This can be done using the `authorize` helper: ```ruby -Types::QueryType = GraphQL::ObjectType.define do - name 'Query' +module Types + class QueryType < BaseObject + graphql_name 'Query' - field :project, Types::ProjectType do - argument :full_path, !types.ID do - description 'The full path of the project, e.g., "gitlab-org/gitlab-ce"' + field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver do + authorize :read_project end - - authorize :read_project - - resolve Loaders::FullPathLoader[:project] end -end ``` The object found by the resolve call is used for authorization. +This works for authorizing a single record, for authorizing +collections, we should only load what the currently authenticated user +is allowed to view. Preferably we use our existing finders for that. ## Types @@ -43,7 +41,7 @@ the definition as minimal as possible. Instead, consider moving any logic into a presenter: ```ruby -Types::MergeRequestType = GraphQL::ObjectType.define do +class Types::MergeRequestType < BaseObject present_using MergeRequestPresenter name 'MergeRequest' @@ -56,11 +54,28 @@ a new presenter specifically for GraphQL. The presenter is initialized using the object resolved by a field, and the context. +## Resolvers + +To find objects to display in a field, we can add resolvers to +`app/graphql/resolvers`. + +Arguments can be defined within the resolver, those arguments will be +made available to the fields using the resolver. + +We already have a `FullPathLoader` that can be included in other +resolvers to quickly find Projects and Namespaces which will have a +lot of dependant objects. + +To limit the amount of queries performed, we can use `BatchLoader`. + ## Testing _full stack_ tests for a graphql query or mutation live in -`spec/requests/graphql`. +`spec/requests/api/graphql`. When adding a query, the `a working graphql query` shared example can -be used to test the query, it expects a valid `query` to be available -in the spec. +be used to test if the query renders valid results. + +Using the `GraphqlHelpers#all_graphql_fields_for`-helper, a query +including all available fields can be constructed. This makes it easy +to add a test rendering all possible fields for a query. diff --git a/lib/constraints/feature_constrainer.rb b/lib/constraints/feature_constrainer.rb new file mode 100644 index 00000000000..05d48b0f25a --- /dev/null +++ b/lib/constraints/feature_constrainer.rb @@ -0,0 +1,13 @@ +module Constraints + class FeatureConstrainer + attr_reader :feature + + def initialize(feature) + @feature = feature + end + + def matches?(_request) + Feature.enabled?(feature) + end + end +end diff --git a/lib/gitlab/graphql/authorize.rb b/lib/gitlab/graphql/authorize.rb index 6f9de8c38b7..04f25c53e49 100644 --- a/lib/gitlab/graphql/authorize.rb +++ b/lib/gitlab/graphql/authorize.rb @@ -2,55 +2,19 @@ module Gitlab module Graphql # Allow fields to declare permissions their objects must have. The field # will be set to nil unless all required permissions are present. - class Authorize - SETUP_PROC = -> (type, *args) do - type.metadata[:authorize] ||= [] - type.metadata[:authorize].concat(args) + module Authorize + extend ActiveSupport::Concern + + def self.use(schema_definition) + schema_definition.instrument(:field, Instrumentation.new) end - INSTRUMENT_PROC = -> (schema) do - schema.instrument(:field, new) + def required_permissions + @required_permissions ||= [] end - def self.register! - GraphQL::Schema.accepts_definitions(enable_authorization: INSTRUMENT_PROC) - GraphQL::Field.accepts_definitions(authorize: SETUP_PROC) - end - - # Replace the resolver for the field with one that will only return the - # resolved object if the permissions check is successful. - # - # Collections are not supported. Apply permissions checks for those at the - # database level instead, to avoid loading superfluous data from the DB - def instrument(_type, field) - return field unless field.metadata.include?(:authorize) - - old_resolver = field.resolve_proc - - new_resolver = -> (obj, args, ctx) do - resolved_obj = old_resolver.call(obj, args, ctx) - checker = build_checker(ctx[:current_user], field.metadata[:authorize]) - - if resolved_obj.respond_to?(:then) - resolved_obj.then(&checker) - else - checker.call(resolved_obj) - end - end - - field.redefine do - resolve(new_resolver) - end - end - - private - - def build_checker(current_user, abilities) - proc do |obj| - # Load the elements if they weren't loaded by BatchLoader yet - obj = obj.sync if obj.respond_to?(:sync) - obj if abilities.all? { |ability| Ability.allowed?(current_user, ability, obj) } - end + def authorize(*permissions) + required_permissions.concat(permissions) end end end diff --git a/lib/gitlab/graphql/authorize/instrumentation.rb b/lib/gitlab/graphql/authorize/instrumentation.rb new file mode 100644 index 00000000000..6cb8e617f62 --- /dev/null +++ b/lib/gitlab/graphql/authorize/instrumentation.rb @@ -0,0 +1,45 @@ +module Gitlab + module Graphql + module Authorize + class Instrumentation + # Replace the resolver for the field with one that will only return the + # resolved object if the permissions check is successful. + # + # Collections are not supported. Apply permissions checks for those at the + # database level instead, to avoid loading superfluous data from the DB + def instrument(_type, field) + field_definition = field.metadata[:type_class] + return field unless field_definition.respond_to?(:required_permissions) + return field if field_definition.required_permissions.empty? + + old_resolver = field.resolve_proc + + new_resolver = -> (obj, args, ctx) do + resolved_obj = old_resolver.call(obj, args, ctx) + checker = build_checker(ctx[:current_user], field_definition.required_permissions) + + if resolved_obj.respond_to?(:then) + resolved_obj.then(&checker) + else + checker.call(resolved_obj) + end + end + + field.redefine do + resolve(new_resolver) + end + end + + private + + def build_checker(current_user, abilities) + proc do |obj| + # Load the elements if they weren't loaded by BatchLoader yet + obj = obj.sync if obj.respond_to?(:sync) + obj if abilities.all? { |ability| Ability.allowed?(current_user, ability, obj) } + end + end + end + end + end +end diff --git a/lib/gitlab/graphql/present.rb b/lib/gitlab/graphql/present.rb index b060692b334..2c7b64f1be9 100644 --- a/lib/gitlab/graphql/present.rb +++ b/lib/gitlab/graphql/present.rb @@ -1,34 +1,20 @@ module Gitlab module Graphql - class Present - PRESENT_USING = -> (type, presenter_class, *args) do - type.metadata[:presenter_class] = presenter_class - end - - INSTRUMENT_PROC = -> (schema) do - schema.instrument(:field, new) - end - - def self.register! - GraphQL::Schema.accepts_definitions(enable_presenting: INSTRUMENT_PROC) - GraphQL::ObjectType.accepts_definitions(present_using: PRESENT_USING) - end - - def instrument(type, field) - return field unless type.metadata[:presenter_class] - - old_resolver = field.resolve_proc - - resolve_with_presenter = -> (obj, args, context) do - presenter = type.metadata[:presenter_class].new(obj, **context.to_h) - - old_resolver.call(presenter, args, context) + module Present + extend ActiveSupport::Concern + prepended do + def self.present_using(kls) + @presenter_class = kls end - field.redefine do - resolve(resolve_with_presenter) + def self.presenter_class + @presenter_class end end + + def self.use(schema_definition) + schema_definition.instrument(:field, Instrumentation.new) + end end end end diff --git a/lib/gitlab/graphql/present/instrumentation.rb b/lib/gitlab/graphql/present/instrumentation.rb new file mode 100644 index 00000000000..1688262974b --- /dev/null +++ b/lib/gitlab/graphql/present/instrumentation.rb @@ -0,0 +1,25 @@ +module Gitlab + module Graphql + module Present + class Instrumentation + def instrument(type, field) + presented_in = field.metadata[:type_class].owner + return field unless presented_in.respond_to?(:presenter_class) + return field unless presented_in.presenter_class + + old_resolver = field.resolve_proc + + resolve_with_presenter = -> (presented_type, args, context) do + object = presented_type.object + presenter = presented_in.presenter_class.new(object, **context.to_h) + old_resolver.call(presenter, args, context) + end + + field.redefine do + resolve(resolve_with_presenter) + end + end + end + end + end +end 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 similarity index 92% rename from spec/graphql/loaders/iid_loader_spec.rb rename to 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 similarity index 83% rename from spec/graphql/loaders/full_path_loader_spec.rb rename to 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