diff options
author | Douwe Maan <douwe@gitlab.com> | 2018-06-06 10:03:34 +0000 |
---|---|---|
committer | Douwe Maan <douwe@gitlab.com> | 2018-06-06 10:03:34 +0000 |
commit | af9cc234f2bf854de38e9730266a411f261918da (patch) | |
tree | bf25d7590d728ee1daa6a179ad43db87301475b8 | |
parent | 7334b8556ba2ee06397293fd90ea04be8f3fccd2 (diff) | |
parent | 9b65d4bb417fb4939289eab94487c894f0a62db6 (diff) | |
download | gitlab-ce-af9cc234f2bf854de38e9730266a411f261918da.tar.gz |
Merge branch 'bvl-graphql-start-34754' into 'master'
GraphQL setup: Basic Project and Merge request endpoint
Closes #34754
See merge request gitlab-org/gitlab-ce!19008
50 files changed, 1159 insertions, 5 deletions
@@ -93,6 +93,10 @@ gem 'grape', '~> 1.0' gem 'grape-entity', '~> 0.7.1' gem 'rack-cors', '~> 1.0.0', require: 'rack/cors' +# GraphQL API +gem 'graphql', '~> 1.8.0' +gem 'graphiql-rails', '~> 1.4.10' + # Disable strong_params so that Mash does not respond to :permitted? gem 'hashie-forbidden_attributes' diff --git a/Gemfile.lock b/Gemfile.lock index 4b57857bde7..334895351ac 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -365,6 +365,10 @@ GEM rake (~> 12) grape_logging (1.7.0) grape + graphiql-rails (1.4.10) + railties + sprockets-rails + graphql (1.8.1) grpc (1.11.0) google-protobuf (~> 3.1) googleapis-common-protos-types (~> 1.0.0) @@ -1053,6 +1057,8 @@ DEPENDENCIES grape-entity (~> 0.7.1) grape-path-helpers (~> 1.0) grape_logging (~> 1.7) + graphiql-rails (~> 1.4.10) + graphql (~> 1.8.0) grpc (~> 1.11.0) haml_lint (~> 0.26.0) hamlit (~> 2.6.1) diff --git a/app/controllers/graphql_controller.rb b/app/controllers/graphql_controller.rb new file mode 100644 index 00000000000..0a1cf169aca --- /dev/null +++ b/app/controllers/graphql_controller.rb @@ -0,0 +1,45 @@ +class GraphqlController < ApplicationController + # Unauthenticated users have access to the API for public data + skip_before_action :authenticate_user! + + before_action :check_graphql_feature_flag! + + def execute + variables = Gitlab::Graphql::Variables.new(params[:variables]).to_h + query = params[:query] + operation_name = params[:operationName] + context = { + current_user: current_user + } + result = GitlabSchema.execute(query, variables: variables, context: context, operation_name: operation_name) + render json: result + end + + rescue_from StandardError do |exception| + log_exception(exception) + + render_error("Internal server error") + end + + rescue_from Gitlab::Graphql::Variables::Invalid do |exception| + render_error(exception.message, status: :unprocessable_entity) + end + + private + + # Overridden from the ApplicationController to make the response look like + # a GraphQL response. That is nicely picked up in Graphiql. + def render_404 + render_error("Not found!", status: :not_found) + end + + def render_error(message, status: 500) + error = { errors: [message: message] } + + render json: error, status: status + end + + def check_graphql_feature_flag! + render_404 unless Feature.enabled?(:graphql) + end +end 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 new file mode 100644 index 00000000000..de4fc1d8e32 --- /dev/null +++ b/app/graphql/gitlab_schema.rb @@ -0,0 +1,8 @@ +class GitlabSchema < GraphQL::Schema + use BatchLoader::GraphQL + use Gitlab::Graphql::Authorize + use Gitlab::Graphql::Present + + query(Types::QueryType) + # mutation(Types::MutationType) +end diff --git a/app/graphql/mutations/.keep b/app/graphql/mutations/.keep new file mode 100644 index 00000000000..e69de29bb2d --- /dev/null +++ b/app/graphql/mutations/.keep 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/resolvers/full_path_resolver.rb b/app/graphql/resolvers/full_path_resolver.rb new file mode 100644 index 00000000000..4eb28aaed6c --- /dev/null +++ b/app/graphql/resolvers/full_path_resolver.rb @@ -0,0 +1,19 @@ +module Resolvers + module FullPathResolver + extend ActiveSupport::Concern + + 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) + BatchLoader.for(full_path).batch(key: "#{model.model_name.param_key}:full_path") do |full_paths, loader| + # `with_route` avoids an N+1 calculating full_path + results = model.where_full_path_in(full_paths).with_route + results.each { |project| loader.call(project.full_path, project) } + end + end + end +end 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 new file mode 100644 index 00000000000..d5d24952984 --- /dev/null +++ b/app/graphql/types/merge_request_type.rb @@ -0,0 +1,47 @@ +module Types + class MergeRequestType < BaseObject + present_using MergeRequestPresenter + + graphql_name 'MergeRequest' + + 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 new file mode 100644 index 00000000000..06ed91c1658 --- /dev/null +++ b/app/graphql/types/mutation_type.rb @@ -0,0 +1,7 @@ +module Types + class MutationType < BaseObject + graphql_name "Mutation" + + # TODO: Add Mutations as fields + end +end diff --git a/app/graphql/types/project_type.rb b/app/graphql/types/project_type.rb new file mode 100644 index 00000000000..9e885d5845a --- /dev/null +++ b/app/graphql/types/project_type.rb @@ -0,0 +1,65 @@ +module Types + class ProjectType < BaseObject + graphql_name 'Project' + + field :id, GraphQL::ID_TYPE, null: false + + field :full_path, GraphQL::ID_TYPE, null: false + field :path, GraphQL::STRING_TYPE, null: false + + field :name_with_namespace, GraphQL::STRING_TYPE, null: false + field :name, GraphQL::STRING_TYPE, null: false + + field :description, GraphQL::STRING_TYPE, null: true + + field :default_branch, GraphQL::STRING_TYPE, null: true + field :tag_list, GraphQL::STRING_TYPE, null: true + + 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, GraphQL::INT_TYPE, null: false + field :forks_count, GraphQL::INT_TYPE, null: false + + field :created_at, Types::TimeType, null: true + field :last_activity_at, Types::TimeType, null: true + + field :archived, GraphQL::BOOLEAN_TYPE, null: true + + field :visibility, GraphQL::STRING_TYPE, null: true + + 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, 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 +end diff --git a/app/graphql/types/query_type.rb b/app/graphql/types/query_type.rb new file mode 100644 index 00000000000..be79c78bf67 --- /dev/null +++ b/app/graphql/types/query_type.rb @@ -0,0 +1,21 @@ +module Types + class QueryType < BaseObject + graphql_name 'Query' + + field :project, Types::ProjectType, + null: true, + resolver: Resolvers::ProjectResolver, + description: "Find a project" do + authorize :read_project + end + + field :merge_request, Types::MergeRequestType, + null: true, + resolver: Resolvers::MergeRequestResolver, + description: "Find a merge request" do + authorize :read_merge_request + 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 new file mode 100644 index 00000000000..2333d82ad1e --- /dev/null +++ b/app/graphql/types/time_type.rb @@ -0,0 +1,14 @@ +module Types + class TimeType < BaseScalar + graphql_name 'Time' + description 'Time represented in ISO 8601' + + def self.coerce_input(value, ctx) + Time.parse(value) + end + + def self.coerce_result(value, ctx) + value.iso8601 + end + end +end diff --git a/app/presenters/merge_request_presenter.rb b/app/presenters/merge_request_presenter.rb index ad839d9840a..8d466c33510 100644 --- a/app/presenters/merge_request_presenter.rb +++ b/app/presenters/merge_request_presenter.rb @@ -179,6 +179,25 @@ class MergeRequestPresenter < Gitlab::View::Presenter::Delegated .can_push_to_branch?(source_branch) end + def mergeable_discussions_state + # This avoids calling MergeRequest#mergeable_discussions_state without + # considering the state of the MR first. If a MR isn't mergeable, we can + # safely short-circuit it. + if merge_request.mergeable_state?(skip_ci_check: true, skip_discussions_check: true) + merge_request.mergeable_discussions_state? + else + false + end + end + + def web_url + Gitlab::UrlBuilder.build(merge_request) + end + + def subscribed? + merge_request.subscribed?(current_user, merge_request.target_project) + end + private def cached_can_be_reverted? diff --git a/changelogs/unreleased/bvl-graphql-start-34754.yml b/changelogs/unreleased/bvl-graphql-start-34754.yml new file mode 100644 index 00000000000..a31f46d3a61 --- /dev/null +++ b/changelogs/unreleased/bvl-graphql-start-34754.yml @@ -0,0 +1,5 @@ +--- +title: Setup graphql with initial project & merge request query +merge_request: 19008 +author: +type: added diff --git a/config/routes/api.rb b/config/routes/api.rb index ce7a7c88900..b1aebf4d606 100644 --- a/config/routes/api.rb +++ b/config/routes/api.rb @@ -1,2 +1,7 @@ +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 new file mode 100644 index 00000000000..dcd5377284c --- /dev/null +++ b/doc/api/graphql/index.md @@ -0,0 +1,42 @@ +# GraphQL API (Beta) + +> [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 Alpha, and therefore hidden behind a +feature flag. You can enable the feature using the [features api][features-api] on a self-hosted instance. + +For example: + +```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 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/-/graphql-explorer`. + +[ce-19008]: https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/19008 +[features-api]: ../features.md diff --git a/doc/development/README.md b/doc/development/README.md index 898c60e96c0..78c1b6bc6e3 100644 --- a/doc/development/README.md +++ b/doc/development/README.md @@ -32,6 +32,8 @@ description: 'Learn how to contribute to GitLab.' - [GitLab utilities](utilities.md) - [API styleguide](api_styleguide.md) Use this styleguide if you are contributing to the API. +- [GrapQL API styleguide](api_graphql_styleguide.md) Use this + styleguide if you are contribution to the [GraphQL API](../api/graphql/index.md) - [Sidekiq guidelines](sidekiq_style_guide.md) for working with Sidekiq workers - [Working with Gitaly](gitaly.md) - [Manage feature flags](feature_flags.md) diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md new file mode 100644 index 00000000000..f74e4f0bd7e --- /dev/null +++ b/doc/development/api_graphql_styleguide.md @@ -0,0 +1,81 @@ +# GraphQL API + +## Authentication + +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 +add a `HTTP_PRIVATE_TOKEN` header. + +### Authorization + +Fields can be authorized using the same abilities used in the Rails +app. This can be done using the `authorize` helper: + +```ruby +module Types + class QueryType < BaseObject + graphql_name 'Query' + + field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver do + authorize :read_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 + +When exposing a model through the GraphQL API, we do so by creating a +new type in `app/graphql/types`. + +When exposing properties in a type, make sure to keep the logic inside +the definition as minimal as possible. Instead, consider moving any +logic into a presenter: + +```ruby +class Types::MergeRequestType < BaseObject + present_using MergeRequestPresenter + + name 'MergeRequest' +end +``` + +An existing presenter could be used, but it is also possible to create +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/api/graphql`. + +When adding a query, the `a working graphql query` shared example can +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.rb b/lib/gitlab/graphql.rb new file mode 100644 index 00000000000..04a89432230 --- /dev/null +++ b/lib/gitlab/graphql.rb @@ -0,0 +1,5 @@ +module Gitlab + module Graphql + StandardGraphqlError = Class.new(StandardError) + end +end diff --git a/lib/gitlab/graphql/authorize.rb b/lib/gitlab/graphql/authorize.rb new file mode 100644 index 00000000000..04f25c53e49 --- /dev/null +++ b/lib/gitlab/graphql/authorize.rb @@ -0,0 +1,21 @@ +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. + module Authorize + extend ActiveSupport::Concern + + def self.use(schema_definition) + schema_definition.instrument(:field, Instrumentation.new) + end + + def required_permissions + @required_permissions ||= [] + end + + def authorize(*permissions) + required_permissions.concat(permissions) + end + 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 new file mode 100644 index 00000000000..2c7b64f1be9 --- /dev/null +++ b/lib/gitlab/graphql/present.rb @@ -0,0 +1,20 @@ +module Gitlab + module Graphql + module Present + extend ActiveSupport::Concern + prepended do + def self.present_using(kls) + @presenter_class = kls + end + + 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/lib/gitlab/graphql/variables.rb b/lib/gitlab/graphql/variables.rb new file mode 100644 index 00000000000..ffbaf65b512 --- /dev/null +++ b/lib/gitlab/graphql/variables.rb @@ -0,0 +1,37 @@ +module Gitlab + module Graphql + class Variables + Invalid = Class.new(Gitlab::Graphql::StandardGraphqlError) + + def initialize(param) + @param = param + end + + def to_h + ensure_hash(@param) + end + + private + + # Handle form data, JSON body, or a blank value + def ensure_hash(ambiguous_param) + case ambiguous_param + when String + if ambiguous_param.present? + ensure_hash(JSON.parse(ambiguous_param)) + else + {} + end + when Hash, ActionController::Parameters + ambiguous_param + when nil + {} + else + raise Invalid, "Unexpected parameter: #{ambiguous_param}" + end + rescue JSON::ParserError => e + raise Invalid.new(e) + end + end + end +end diff --git a/spec/controllers/graphql_controller_spec.rb b/spec/controllers/graphql_controller_spec.rb new file mode 100644 index 00000000000..1449036e148 --- /dev/null +++ b/spec/controllers/graphql_controller_spec.rb @@ -0,0 +1,69 @@ +require 'spec_helper' + +describe GraphqlController do + describe 'execute' do + let(:user) { nil } + + before do + sign_in(user) if user + + run_test_query! + end + + subject { query_response } + + context 'graphql is disabled by feature flag' do + let(:user) { nil } + + before do + stub_feature_flags(graphql: false) + end + + it 'returns 404' do + run_test_query! + + expect(response).to have_gitlab_http_status(404) + end + end + + context 'signed out' do + let(:user) { nil } + + it 'runs the query with current_user: nil' do + is_expected.to eq('echo' => 'nil says: test success') + end + end + + context 'signed in' do + let(:user) { create(:user, username: 'Simon') } + + it 'runs the query with current_user set' do + is_expected.to eq('echo' => '"Simon" says: test success') + end + end + + context 'invalid variables' do + it 'returns an error' do + run_test_query!(variables: "This is not JSON") + + expect(response).to have_gitlab_http_status(422) + expect(json_response['errors'].first['message']).not_to be_nil + end + end + end + + # Chosen to exercise all the moving parts in GraphqlController#execute + def run_test_query!(variables: { 'text' => 'test success' }) + query = <<~QUERY + query Echo($text: String) { + echo(text: $text) + } + QUERY + + post :execute, query: query, operationName: 'Echo', variables: variables + end + + def query_response + json_response['data'] + end +end diff --git a/spec/graphql/gitlab_schema_spec.rb b/spec/graphql/gitlab_schema_spec.rb new file mode 100644 index 00000000000..b892f6b44ed --- /dev/null +++ b/spec/graphql/gitlab_schema_spec.rb @@ -0,0 +1,33 @@ +require 'spec_helper' + +describe GitlabSchema do + it 'uses batch loading' do + expect(field_instrumenters).to include(BatchLoader::GraphQL) + end + + it 'enables the preload instrumenter' do + expect(field_instrumenters).to include(BatchLoader::GraphQL) + end + + it 'enables the authorization instrumenter' do + 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::Instrumentation)) + end + + it 'has the base mutation' do + 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.to_graphql) + end + + def field_instrumenters + described_class.instrumenters[:field] + end +end diff --git a/spec/graphql/resolvers/merge_request_resolver_spec.rb b/spec/graphql/resolvers/merge_request_resolver_spec.rb new file mode 100644 index 00000000000..af015533209 --- /dev/null +++ b/spec/graphql/resolvers/merge_request_resolver_spec.rb @@ -0,0 +1,58 @@ +require 'spec_helper' + +describe Resolvers::MergeRequestResolver do + include GraphqlHelpers + + set(:project) { create(:project, :repository) } + set(:merge_request_1) { create(:merge_request, :simple, source_project: project, target_project: project) } + set(:merge_request_2) { create(:merge_request, :rebased, source_project: project, target_project: project) } + + set(:other_project) { create(:project, :repository) } + set(:other_merge_request) { create(:merge_request, source_project: other_project, target_project: other_project) } + + let(:full_path) { project.full_path } + let(:iid_1) { merge_request_1.iid } + let(:iid_2) { merge_request_2.iid } + + let(:other_full_path) { other_project.full_path } + let(:other_iid) { other_merge_request.iid } + + describe '#resolve' do + it 'batch-resolves merge requests by target project full path and IID' do + path = full_path # avoid database query + + result = batch(max_queries: 2) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2) + end + + it 'can batch-resolve merge requests from different projects' do + path = project.full_path # avoid database queries + other_path = other_full_path + + result = batch(max_queries: 3) do + [resolve_mr(path, iid_1), resolve_mr(path, iid_2), resolve_mr(other_path, other_iid)] + end + + expect(result).to contain_exactly(merge_request_1, merge_request_2, other_merge_request) + end + + it 'resolves an unknown iid to nil' do + result = batch { resolve_mr(full_path, -1) } + + expect(result).to be_nil + end + + it 'resolves a known iid for an unknown full_path to nil' do + result = batch { resolve_mr('unknown/project', iid_1) } + + expect(result).to be_nil + end + end + + def resolve_mr(full_path, iid) + resolve(described_class, args: { full_path: full_path, iid: iid }) + end +end diff --git a/spec/graphql/resolvers/project_resolver_spec.rb b/spec/graphql/resolvers/project_resolver_spec.rb new file mode 100644 index 00000000000..d4990c6492c --- /dev/null +++ b/spec/graphql/resolvers/project_resolver_spec.rb @@ -0,0 +1,32 @@ +require 'spec_helper' + +describe Resolvers::ProjectResolver do + include GraphqlHelpers + + set(:project1) { create(:project) } + set(:project2) { create(:project) } + + set(:other_project) { create(:project) } + + describe '#resolve' do + it 'batch-resolves projects by full path' do + paths = [project1.full_path, project2.full_path] + + result = batch(max_queries: 1) do + paths.map { |path| resolve_project(path) } + end + + expect(result).to contain_exactly(project1, project2) + end + + it 'resolves an unknown full_path to nil' do + result = batch { resolve_project('unknown/project') } + + expect(result).to be_nil + end + end + + def resolve_project(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 new file mode 100644 index 00000000000..8488252fd59 --- /dev/null +++ b/spec/graphql/types/query_type_spec.rb @@ -0,0 +1,37 @@ +require 'spec_helper' + +describe GitlabSchema.types['Query'] do + it 'is called Query' do + expect(described_class.graphql_name).to eq('Query') + end + + it { is_expected.to have_graphql_fields(:project, :merge_request, :echo) } + + describe 'project field' do + subject { described_class.fields['project'] } + + 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(Resolvers::ProjectResolver) + end + + it 'authorizes with read_project' do + is_expected.to require_graphql_authorizations(:read_project) + end + end + + describe 'merge_request field' do + subject { described_class.fields['mergeRequest'] } + + it 'finds MRs by project and IID' do + is_expected.to have_graphql_arguments(:full_path, :iid) + is_expected.to have_graphql_type(Types::MergeRequestType) + is_expected.to have_graphql_resolver(Resolvers::MergeRequestResolver) + end + + it 'authorizes with read_merge_request' do + is_expected.to require_graphql_authorizations(:read_merge_request) + end + end +end diff --git a/spec/graphql/types/time_type_spec.rb b/spec/graphql/types/time_type_spec.rb new file mode 100644 index 00000000000..4196d9d27d4 --- /dev/null +++ b/spec/graphql/types/time_type_spec.rb @@ -0,0 +1,16 @@ +require 'spec_helper' + +describe GitlabSchema.types['Time'] do + let(:iso) { "2018-06-04T15:23:50+02:00" } + let(:time) { Time.parse(iso) } + + it { expect(described_class.graphql_name).to eq('Time') } + + it 'coerces Time object into ISO 8601' do + expect(described_class.coerce_isolated_result(time)).to eq(iso) + end + + 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/lib/gitlab/path_regex_spec.rb b/spec/lib/gitlab/path_regex_spec.rb index a40330d853f..e90e0aba0a4 100644 --- a/spec/lib/gitlab/path_regex_spec.rb +++ b/spec/lib/gitlab/path_regex_spec.rb @@ -90,11 +90,13 @@ describe Gitlab::PathRegex do let(:routes_not_starting_in_wildcard) { routes_without_format.select { |p| p !~ %r{^/[:*]} } } let(:top_level_words) do - words = routes_not_starting_in_wildcard.map do |route| - route.split('/')[1] - end.compact - - (words + ee_top_level_words + files_in_public + Array(API::API.prefix.to_s)).uniq + routes_not_starting_in_wildcard + .map { |route| route.split('/')[1] } + .concat(ee_top_level_words) + .concat(files_in_public) + .concat(Array(API::API.prefix.to_s)) + .compact + .uniq end let(:ee_top_level_words) do 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/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 new file mode 100644 index 00000000000..30ff9a1196a --- /dev/null +++ b/spec/support/helpers/graphql_helpers.rb @@ -0,0 +1,90 @@ +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(resolver_class, obj: nil, args: {}, ctx: {}) + resolver_class.new(object: obj, context: ctx).resolve(args) + end + + # Runs a block inside a BatchLoader::Executor wrapper + def batch(max_queries: nil, &blk) + wrapper = proc do + begin + BatchLoader::Executor.ensure_current + yield + ensure + BatchLoader::Executor.clear_current + end + end + + if max_queries + result = nil + expect { result = wrapper.call }.not_to exceed_query_limit(max_queries) + result + else + wrapper.call + end + end + + 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| + if scalar?(field) + name + else + "#{name} { #{all_graphql_fields_for(field_type(field))} }" + end + end.join("\n") + end + + 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) + field_type(field).kind.scalar? + end + + def field_type(field) + if field.type.respond_to?(:of_type) + field.type.of_type + else + field.type + end + end +end diff --git a/spec/support/matchers/graphql_matchers.rb b/spec/support/matchers/graphql_matchers.rb new file mode 100644 index 00000000000..ba7a1c8cde0 --- /dev/null +++ b/spec/support/matchers/graphql_matchers.rb @@ -0,0 +1,40 @@ +RSpec::Matchers.define :require_graphql_authorizations do |*expected| + match do |field| + 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| + 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| + 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.to_graphql) + end +end + +RSpec::Matchers.define :have_graphql_resolver do |expected| + match do |field| + 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 new file mode 100644 index 00000000000..9b2b74593a5 --- /dev/null +++ b/spec/support/shared_examples/requests/graphql_shared_examples.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +shared_examples 'a working graphql query' do + include GraphqlHelpers + + it 'is returns a successfull response', :aggregate_failures do + expect(response).to be_success + expect(graphql_errors['errors']).to be_nil + expect(json_response.keys).to include('data') + end +end |