Skip to content

Draft: PoC - Move Graphql to the WebEngine

Nikola Milojevic requested to merge remove-graphql into master

What does this MR do?

This MR represents PoC for a mechanism that will allow us to load GraphQl with all dependencies only when needed.

Based on the proposed solution, we use Rails engines, in order to split the application into the smaller functional parts that will allow as that only needed code is loaded with all dependencies.

This MR moves all GraphQL application code, required gems, routes, and initializers to the engines/web_engine folder.

image

All required Graphql gems are removed from the application's gemfile, and they are added as dependencies to the web_engine.gemspec file

  spec.add_dependency 'apollo_upload_server', '~> 2.0.2'
  spec.add_dependency 'graphql', '~> 1.11.4'
  # NOTE: graphiql-rails v1.5+ doesn't work: https://gitlab.com/gitlab-org/gitlab/issues/31771
  # TODO: remove app/views/graphiql/rails/editors/show.html.erb when https://github.com/rmosolgo/graphiql-rails/pull/71 is released:
  # https://gitlab.com/gitlab-org/gitlab/issues/31747
  spec.add_dependency 'graphiql-rails', '~> 1.4.10'

  spec.add_dependency 'graphql-docs', '~> 1.6.0'

Original Gemfile now includes web_engine, but the web_engine with all it's dependencies will not be automatically required. The web_engine will be loaded only in case we run web_server(puma or unicorn), the console, or in case we need to test web_engine.

# config/engines.rb
# Load only in case we are running web_server or rails console
if Gitlab::Runtime.web_server? || Gitlab::Runtime.console? || Gitlab::Utils.to_boolean(ENV['TEST_WEB_ENGINE'])
  require 'web_engine'
end

Does this MR meet the acceptance criteria?

Conformity

Availability and Testing

Security

If this MR contains changes to processing or storing of credentials or tokens, authorization and authentication methods and other items described in the security review guidelines:

  • Label as security and @ mention @gitlab-com/gl-security/appsec
  • The MR includes necessary changes to maintain consistency between UI, API, email, or other methods
  • Security reports checked/validated by a reviewer from the AppSec team

Related to: #288044 (closed)

Edited by Nikola Milojevic

Merge request reports