Initial setup GraphQL using graphql-ruby 1.8

- All definitions have been replaced by classes:
  http://graphql-ruby.org/schema/class_based_api.html
- Authorization & Presentation have been refactored to work in the
  class based system
- Loaders have been replaced by resolvers
- Times are now coersed as ISO 8601

.rubocop.yml

@@ -43,7 +43,6 @@ Naming/FileName:
     - 'config/**/*'
     - 'lib/generators/**/*'
     - 'ee/lib/generators/**/*'
-    - 'app/graphql/**/*'
   IgnoreExecutableScripts: true
   AllowedAcronyms:
     - EE

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?

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)

app/graphql/functions/base_function.rb

+module Functions
+  class BaseFunction < GraphQL::Function
+  end
+end

app/graphql/functions/echo.rb

+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

app/graphql/gitlab_schema.rb

-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

app/graphql/loaders/base_loader.rb

-# 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

app/graphql/loaders/iid_loader.rb

-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

app/graphql/resolvers/base_resolver.rb

+module Resolvers
+  class BaseResolver < GraphQL::Schema::Resolver
+  end
+end

app/graphql/loaders/full_path_loader.rb → app/graphql/resolvers/full_path_resolver.rb

-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)

app/graphql/resolvers/merge_request_resolver.rb

+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

app/graphql/resolvers/project_resolver.rb

+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

app/graphql/types/base_enum.rb

+module Types
+  class BaseEnum < GraphQL::Schema::Enum
+  end
+end

app/graphql/types/base_field.rb

+module Types
+  class BaseField < GraphQL::Schema::Field
+    prepend Gitlab::Graphql::Authorize
+  end
+end

app/graphql/types/base_input_object.rb

+module Types
+  class BaseInputObject < GraphQL::Schema::InputObject
+  end
+end

app/graphql/types/base_interface.rb

+module Types
+  module BaseInterface
+    include GraphQL::Schema::Interface
+  end
+end

app/graphql/types/base_object.rb

+module Types
+  class BaseObject < GraphQL::Schema::Object
+    prepend Gitlab::Graphql::Present
+
+    field_class Types::BaseField
+  end
+end

app/graphql/types/base_scalar.rb

+module Types
+  class BaseScalar < GraphQL::Schema::Scalar
+  end
+end

app/graphql/types/base_union.rb

+module Types
+  class BaseUnion < GraphQL::Schema::Union
+  end
+end

app/graphql/types/merge_request_type.rb

-Types::MergeRequestType = GraphQL::ObjectType.define do
+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
+    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
-  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 :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

app/graphql/types/mutation_type.rb

-Types::MutationType = GraphQL::ObjectType.define do
-  name "Mutation"
+module Types
+  class MutationType < BaseObject
+    graphql_name "Mutation"
 
     # TODO: Add Mutations as fields
+  end
 end

app/graphql/types/project_type.rb

-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) }
+    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", types.Boolean do
-      resolve ->(project, args, ctx) { project.feature_available?(feature, ctx[:current_user]) }
+      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, types.Boolean do
-    resolve ->(project, args, ctx) { project.feature_available?(:builds, ctx[:current_user]) }
+    field :jobs_enabled, GraphQL::BOOLEAN_TYPE, null: true, resolve: -> (project, args, ctx) do
+      project.feature_available?(:builds, ctx[:current_user])
     end
 
-  field :public_jobs, types.Boolean, property: :public_builds
+    field :public_jobs, GraphQL::BOOLEAN_TYPE, method: :public_builds, null: true
 
-  field :open_issues_count, types.Int do
-    resolve ->(project, args, ctx) { project.open_issues_count if project.feature_available?(:issues, ctx[:current_user]) }
+    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, types.String
-  field :ci_config_path, types.String
+    field :import_status, GraphQL::STRING_TYPE, null: true
+    field :ci_config_path, GraphQL::STRING_TYPE, null: true
 
-  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
+    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

app/graphql/types/query_type.rb

-Types::QueryType = GraphQL::ObjectType.define do
-  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"'
-    end
-
+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
-
-    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"'
-    end
-
-    argument :iid, !types.ID do
-      description 'The IID of the merge request, e.g., "1"'
     end
 
+    field :merge_request, Types::MergeRequestType,
+          null: true,
+          resolver: Resolvers::MergeRequestResolver,
+          description: "Find a merge request" do
       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

app/graphql/types/time_type.rb

-# 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

config/routes/api.rb

-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 => '/'

doc/api/graphql/index.md

@@ -2,33 +2,41 @@
 
 > [Introduced][ce-19008] in GitLab 11.0.
 
-## Enabling the GraphQL feature
+[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 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 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.
 
-Start the console by running
+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).
 
-```bash
-sudo gitlab-rails console
-```
+## 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.
 
-Then enable the feature by running
+For example:
 
-```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

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'
-
-  field :project, Types::ProjectType do
-    argument :full_path, !types.ID do
-      description 'The full path of the project, e.g., "gitlab-org/gitlab-ce"'
-    end
+module Types
+  class QueryType < BaseObject
+    graphql_name 'Query'
 
+    field :project, Types::ProjectType, null: true, resolver: Resolvers::ProjectResolver do
       authorize :read_project
-
-    resolve Loaders::FullPathLoader[:project]
     end
-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.

lib/constraints/feature_constrainer.rb

+module Constraints
+  class FeatureConstrainer
+    attr_reader :feature
+
+    def initialize(feature)
+      @feature = feature
+    end
+
+    def matches?(_request)
+      Feature.enabled?(feature)
+    end
+  end
+end

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)
-      end
-
-      INSTRUMENT_PROC = -> (schema) do
-        schema.instrument(:field, new)
-      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)
+    module Authorize
+      extend ActiveSupport::Concern
 
-        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
+      def self.use(schema_definition)
+        schema_definition.instrument(:field, Instrumentation.new)
       end
 
-        field.redefine do
-          resolve(new_resolver)
+      def required_permissions
+        @required_permissions ||= []
       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

lib/gitlab/graphql/authorize/instrumentation.rb

+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

lib/gitlab/graphql/present.rb

 module Gitlab
   module Graphql
-    class Present
-      PRESENT_USING = -> (type, presenter_class, *args) do
-        type.metadata[:presenter_class] = presenter_class
+    module Present
+      extend ActiveSupport::Concern
+      prepended do
+        def self.present_using(kls)
+          @presenter_class = kls
         end
 
-      INSTRUMENT_PROC = -> (schema) do
-        schema.instrument(:field, new)
+        def self.presenter_class
+          @presenter_class
         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)
-        end
-
-        field.redefine do
-          resolve(resolve_with_presenter)
-        end
+      def self.use(schema_definition)
+        schema_definition.instrument(:field, Instrumentation.new)
       end
     end
   end

lib/gitlab/graphql/present/instrumentation.rb

+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

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

spec/graphql/loaders/iid_loader_spec.rb → spec/graphql/resolvers/merge_request_resolver_spec.rb

 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

spec/graphql/loaders/full_path_loader_spec.rb → spec/graphql/resolvers/project_resolver_spec.rb

 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

spec/graphql/types/project_type_spec.rb

+require 'spec_helper'
+
+describe GitlabSchema.types['Project'] do
+  it { expect(described_class.graphql_name).to eq('Project') }
+end

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

spec/graphql/types/time_type_spec.rb

 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

spec/requests/api/graphql/merge_request_query_spec.rb

+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

spec/requests/api/graphql/project_query_spec.rb

+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

spec/requests/graphql/merge_request_query_spec.rb

-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

spec/requests/graphql/project_query_spec.rb

-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

spec/routing/api_routing_spec.rb

+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

spec/support/helpers/graphql_helpers.rb

 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)

spec/support/matchers/graphql_matchers.rb

 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

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