From 0e9798aaa3b3cd9f76e9532dcf1621506fa402f0 Mon Sep 17 00:00:00 2001 From: GitLab Bot Date: Wed, 16 Sep 2020 03:09:23 +0000 Subject: [PATCH] Add latest changes from gitlab-org/gitlab@master --- .../projects/web_ide_schemas_controller.rb | 25 +++++++ .../projects/web_ide_terminals_controller.rb | 2 +- .../ci/create_web_ide_terminal_service.rb | 2 +- .../base_config_service.rb} | 20 +++--- app/services/ide/schemas_config_service.rb | 49 +++++++++++++ app/services/ide/terminal_config_service.rb | 13 ++++ config/routes/project.rb | 5 ++ doc/development/api_graphql_styleguide.md | 2 +- doc/development/fe_guide/graphql.md | 14 ++++ lib/gitlab/web_ide/config.rb | 4 ++ lib/gitlab/web_ide/config/entry/global.rb | 12 ++-- .../web_ide_schemas_controller_spec.rb | 66 ++++++++++++++++++ .../web_ide_terminals_controller_spec.rb | 2 +- .../web_ide/config/entry/global_spec.rb | 5 +- .../base_config_service_spec.rb} | 40 +---------- .../ide/schemas_config_service_spec.rb | 53 ++++++++++++++ .../ide/terminal_config_service_spec.rb | 69 +++++++++++++++++++ 17 files changed, 325 insertions(+), 58 deletions(-) create mode 100644 app/controllers/projects/web_ide_schemas_controller.rb rename app/services/{ci/web_ide_config_service.rb => ide/base_config_service.rb} (88%) create mode 100644 app/services/ide/schemas_config_service.rb create mode 100644 app/services/ide/terminal_config_service.rb create mode 100644 spec/controllers/projects/web_ide_schemas_controller_spec.rb rename spec/services/{ci/web_ide_config_service_spec.rb => ide/base_config_service_spec.rb} (53%) create mode 100644 spec/services/ide/schemas_config_service_spec.rb create mode 100644 spec/services/ide/terminal_config_service_spec.rb diff --git a/app/controllers/projects/web_ide_schemas_controller.rb b/app/controllers/projects/web_ide_schemas_controller.rb new file mode 100644 index 00000000000..3d16a6fafd4 --- /dev/null +++ b/app/controllers/projects/web_ide_schemas_controller.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +class Projects::WebIdeSchemasController < Projects::ApplicationController + before_action :authenticate_user! + + def show + return respond_422 unless branch_sha + + result = ::Ide::SchemasConfigService.new(project, current_user, sha: branch_sha, filename: params[:filename]).execute + + if result[:status] == :success + render json: result[:schema] + else + render json: result, status: :unprocessable_entity + end + end + + private + + def branch_sha + return unless params[:branch].present? + + project.commit(params[:branch])&.id + end +end diff --git a/app/controllers/projects/web_ide_terminals_controller.rb b/app/controllers/projects/web_ide_terminals_controller.rb index 08ea5c4bca8..76bcaa9e80c 100644 --- a/app/controllers/projects/web_ide_terminals_controller.rb +++ b/app/controllers/projects/web_ide_terminals_controller.rb @@ -11,7 +11,7 @@ class Projects::WebIdeTerminalsController < Projects::ApplicationController def check_config return respond_422 unless branch_sha - result = ::Ci::WebIdeConfigService.new(project, current_user, sha: branch_sha).execute + result = ::Ide::TerminalConfigService.new(project, current_user, sha: branch_sha).execute if result[:status] == :success head :ok diff --git a/app/services/ci/create_web_ide_terminal_service.rb b/app/services/ci/create_web_ide_terminal_service.rb index 4f1bf0447d2..a78281aed16 100644 --- a/app/services/ci/create_web_ide_terminal_service.rb +++ b/app/services/ci/create_web_ide_terminal_service.rb @@ -70,7 +70,7 @@ module Ci end def load_terminal_config! - result = ::Ci::WebIdeConfigService.new(project, current_user, sha: sha).execute + result = ::Ide::TerminalConfigService.new(project, current_user, sha: sha).execute raise TerminalCreationError, result[:message] if result[:status] != :success @terminal = result[:terminal] diff --git a/app/services/ci/web_ide_config_service.rb b/app/services/ide/base_config_service.rb similarity index 88% rename from app/services/ci/web_ide_config_service.rb rename to app/services/ide/base_config_service.rb index ade9132f419..1f8d5c17584 100644 --- a/app/services/ci/web_ide_config_service.rb +++ b/app/services/ide/base_config_service.rb @@ -1,9 +1,7 @@ # frozen_string_literal: true -module Ci - class WebIdeConfigService < ::BaseService - include ::Gitlab::Utils::StrongMemoize - +module Ide + class BaseConfigService < ::BaseService ValidationError = Class.new(StandardError) WEBIDE_CONFIG_FILE = '.gitlab/.gitlab-webide.yml'.freeze @@ -11,13 +9,19 @@ module Ci attr_reader :config, :config_content def execute + check_access_and_load_config! + + success + rescue ValidationError => e + error(e.message) + end + + protected + + def check_access_and_load_config! check_access! load_config_content! load_config! - - success(terminal: config.terminal_value) - rescue ValidationError => e - error(e.message) end private diff --git a/app/services/ide/schemas_config_service.rb b/app/services/ide/schemas_config_service.rb new file mode 100644 index 00000000000..8d2ce97103d --- /dev/null +++ b/app/services/ide/schemas_config_service.rb @@ -0,0 +1,49 @@ +# frozen_string_literal: true + +module Ide + class SchemasConfigService < ::Ide::BaseConfigService + PREDEFINED_SCHEMAS = [{ + uri: 'https://json.schemastore.org/gitlab-ci', + match: ['*.gitlab-ci.yml'] + }].freeze + + def execute + schema = predefined_schema_for(params[:filename]) || {} + success(schema: schema) + rescue => e + error(e.message) + end + + private + + def find_schema(filename, schemas) + match_flags = ::File::FNM_DOTMATCH | ::File::FNM_PATHNAME + + schemas.each do |schema| + match = schema[:match].any? { |pattern| ::File.fnmatch?(pattern, filename, match_flags) } + + return Gitlab::Json.parse(get_cached(schema[:uri])) if match + end + + nil + end + + def predefined_schema_for(filename) + find_schema(filename, predefined_schemas) + end + + def predefined_schemas + return PREDEFINED_SCHEMAS if Feature.enabled?(:schema_linting) + + [] + end + + def get_cached(url) + Rails.cache.fetch("services:ide:schema:#{url}", expires_in: 1.day) do + Gitlab::HTTP.get(url).body + end + end + end +end + +Ide::SchemasConfigService.prepend_if_ee('::EE::Ide::SchemasConfigService') diff --git a/app/services/ide/terminal_config_service.rb b/app/services/ide/terminal_config_service.rb new file mode 100644 index 00000000000..318df3436c4 --- /dev/null +++ b/app/services/ide/terminal_config_service.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +module Ide + class TerminalConfigService < ::Ide::BaseConfigService + private + + def success(pass_back = {}) + result = super(pass_back) + result[:terminal] = config.terminal_value + result + end + end +end diff --git a/config/routes/project.rb b/config/routes/project.rb index 3361193581e..24b44646d95 100644 --- a/config/routes/project.rb +++ b/config/routes/project.rb @@ -378,6 +378,11 @@ constraints(::Constraints::ProjectUrlConstrainer.new) do post :reset_token end resources :feature_flags_user_lists, param: :iid, only: [:new, :edit, :show] + + get '/schema/:branch/*filename', + to: 'web_ide_schemas#show', + format: false, + as: :schema end # End of the /-/ scope. diff --git a/doc/development/api_graphql_styleguide.md b/doc/development/api_graphql_styleguide.md index 76fb895cc5e..18fc0fb7d33 100644 --- a/doc/development/api_graphql_styleguide.md +++ b/doc/development/api_graphql_styleguide.md @@ -41,7 +41,7 @@ add a `HTTP_PRIVATE_TOKEN` header. GitLab's GraphQL API uses Global IDs (i.e: `"gid://gitlab/MyObject/123"`) and never database primary key IDs. -Global ID is [a standard](https://graphql.org/learn/global-object-identification/) +Global ID is [a convention](https://graphql.org/learn/global-object-identification/) used for caching and fetching in client-side libraries. See also: diff --git a/doc/development/fe_guide/graphql.md b/doc/development/fe_guide/graphql.md index 1bbefc1370b..82cd19dce4f 100644 --- a/doc/development/fe_guide/graphql.md +++ b/doc/development/fe_guide/graphql.md @@ -141,6 +141,20 @@ fragment DesignItem on Design { More about fragments: [GraphQL Docs](https://graphql.org/learn/queries/#fragments) +## Global IDs + +GitLab's GraphQL API expresses `id` fields as Global IDs rather than the PostgreSQL +primary key `id`. Global ID is [a convention](https://graphql.org/learn/global-object-identification/) +used for caching and fetching in client-side libraries. + +To convert a Global ID to the primary key `id`, you can use `getIdFromGraphQLId`: + +```javascript +import { getIdFromGraphQLId } from '~/graphql_shared/utils'; + +const primaryKeyId = getIdFromGraphQLId(data.id); +``` + ## Immutability and cache updates From Apollo version 3.0.0 all the cache updates need to be immutable; it needs to be replaced entirely diff --git a/lib/gitlab/web_ide/config.rb b/lib/gitlab/web_ide/config.rb index 3b1fa162b53..b2ab5c0b6e3 100644 --- a/lib/gitlab/web_ide/config.rb +++ b/lib/gitlab/web_ide/config.rb @@ -34,6 +34,10 @@ module Gitlab @global.terminal_value end + def schemas_value + @global.schemas_value + end + private def build_config(config, opts = {}) diff --git a/lib/gitlab/web_ide/config/entry/global.rb b/lib/gitlab/web_ide/config/entry/global.rb index 50c3f2d294f..2c67c7d02d4 100644 --- a/lib/gitlab/web_ide/config/entry/global.rb +++ b/lib/gitlab/web_ide/config/entry/global.rb @@ -12,18 +12,22 @@ module Gitlab include ::Gitlab::Config::Entry::Configurable include ::Gitlab::Config::Entry::Attributable - ALLOWED_KEYS = %i[terminal].freeze + def self.allowed_keys + %i[terminal].freeze + end validations do - validates :config, allowed_keys: ALLOWED_KEYS + validates :config, allowed_keys: Global.allowed_keys end + attributes allowed_keys + entry :terminal, Entry::Terminal, description: 'Configuration of the webide terminal.' - - attributes :terminal end end end end end + +::Gitlab::WebIde::Config::Entry::Global.prepend_if_ee('EE::Gitlab::WebIde::Config::Entry::Global') diff --git a/spec/controllers/projects/web_ide_schemas_controller_spec.rb b/spec/controllers/projects/web_ide_schemas_controller_spec.rb new file mode 100644 index 00000000000..fbec941aecc --- /dev/null +++ b/spec/controllers/projects/web_ide_schemas_controller_spec.rb @@ -0,0 +1,66 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Projects::WebIdeSchemasController do + let_it_be(:developer) { create(:user) } + let_it_be(:project) { create(:project, :private, :repository, namespace: developer.namespace) } + + before do + project.add_developer(developer) + + sign_in(user) + end + + describe 'GET show' do + let(:user) { developer } + let(:branch) { 'master' } + + subject do + get :show, params: { + namespace_id: project.namespace.to_param, + project_id: project, + branch: branch, + filename: 'package.json' + } + end + + before do + allow_next_instance_of(::Ide::SchemasConfigService) do |instance| + allow(instance).to receive(:execute).and_return(result) + end + end + + context 'when branch is invalid' do + let(:branch) { 'non-existent' } + + it 'returns 422' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + end + end + + context 'when a valid schema exists' do + let(:result) { { status: :success, schema: { schema: 'Sample Schema' } } } + + it 'returns the schema' do + subject + + expect(response).to have_gitlab_http_status(:ok) + expect(response.body).to eq('{"schema":"Sample Schema"}') + end + end + + context 'when an error occurs parsing the schema' do + let(:result) { { status: :error, message: 'Some error occured' } } + + it 'returns 422 with the error' do + subject + + expect(response).to have_gitlab_http_status(:unprocessable_entity) + expect(response.body).to eq('{"status":"error","message":"Some error occured"}') + end + end + end +end diff --git a/spec/controllers/projects/web_ide_terminals_controller_spec.rb b/spec/controllers/projects/web_ide_terminals_controller_spec.rb index 2ae5899c258..3eb3d5da351 100644 --- a/spec/controllers/projects/web_ide_terminals_controller_spec.rb +++ b/spec/controllers/projects/web_ide_terminals_controller_spec.rb @@ -113,7 +113,7 @@ RSpec.describe Projects::WebIdeTerminalsController do let(:result) { { status: :success } } before do - allow_next_instance_of(::Ci::WebIdeConfigService) do |instance| + allow_next_instance_of(::Ide::TerminalConfigService) do |instance| allow(instance).to receive(:execute).and_return(result) end diff --git a/spec/lib/gitlab/web_ide/config/entry/global_spec.rb b/spec/lib/gitlab/web_ide/config/entry/global_spec.rb index 3a50667163b..3e29bf89785 100644 --- a/spec/lib/gitlab/web_ide/config/entry/global_spec.rb +++ b/spec/lib/gitlab/web_ide/config/entry/global_spec.rb @@ -12,8 +12,7 @@ RSpec.describe Gitlab::WebIde::Config::Entry::Global do context 'when filtering all the entry/node names' do it 'contains the expected node names' do - expect(described_class.nodes.keys) - .to match_array(%i[terminal]) + expect(described_class.nodes.keys).to match_array(described_class.allowed_keys) end end end @@ -34,7 +33,7 @@ RSpec.describe Gitlab::WebIde::Config::Entry::Global do end it 'creates node object for each entry' do - expect(global.descendants.count).to eq 1 + expect(global.descendants.count).to eq described_class.allowed_keys.length end it 'creates node object using valid class' do diff --git a/spec/services/ci/web_ide_config_service_spec.rb b/spec/services/ide/base_config_service_spec.rb similarity index 53% rename from spec/services/ci/web_ide_config_service_spec.rb rename to spec/services/ide/base_config_service_spec.rb index 437b468cec8..debdc6e5809 100644 --- a/spec/services/ci/web_ide_config_service_spec.rb +++ b/spec/services/ide/base_config_service_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -RSpec.describe Ci::WebIdeConfigService do +RSpec.describe Ide::BaseConfigService do let_it_be(:project) { create(:project, :repository) } let_it_be(:user) { create(:user) } let(:sha) { 'sha' } @@ -47,44 +47,6 @@ RSpec.describe Ci::WebIdeConfigService do message: "Invalid configuration format") end end - - context 'content is valid, but terminal not defined' do - let(:config_content) { '{}' } - - it 'returns success' do - is_expected.to include( - status: :success, - terminal: nil) - end - end - - context 'content is valid, with enabled terminal' do - let(:config_content) { 'terminal: {}' } - - it 'returns success' do - is_expected.to include( - status: :success, - terminal: { - tag_list: [], - yaml_variables: [], - options: { script: ["sleep 60"] } - }) - end - end - - context 'content is valid, with custom terminal' do - let(:config_content) { 'terminal: { before_script: [ls] }' } - - it 'returns success' do - is_expected.to include( - status: :success, - terminal: { - tag_list: [], - yaml_variables: [], - options: { before_script: ["ls"], script: ["sleep 60"] } - }) - end - end end end end diff --git a/spec/services/ide/schemas_config_service_spec.rb b/spec/services/ide/schemas_config_service_spec.rb new file mode 100644 index 00000000000..19e5ca9e87d --- /dev/null +++ b/spec/services/ide/schemas_config_service_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ide::SchemasConfigService do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:filename) { 'sample.yml' } + let(:schema_content) { double(body: '{"title":"Sample schema"}') } + + describe '#execute' do + before do + project.add_developer(user) + + allow(Gitlab::HTTP).to receive(:get).with(anything) do + schema_content + end + end + + subject { described_class.new(project, user, filename: filename).execute } + + context 'feature flag schema_linting is enabled', unless: Gitlab.ee? do + before do + stub_feature_flags(schema_linting: true) + end + + context 'when no predefined schema exists for the given filename' do + it 'returns an empty object' do + is_expected.to include( + status: :success, + schema: {}) + end + end + + context 'when a predefined schema exists for the given filename' do + let(:filename) { '.gitlab-ci.yml' } + + it 'uses predefined schema matches' do + expect(Gitlab::HTTP).to receive(:get).with('https://json.schemastore.org/gitlab-ci') + expect(subject[:schema]['title']).to eq "Sample schema" + end + end + end + + context 'feature flag schema_linting is disabled', unless: Gitlab.ee? do + it 'returns an empty object' do + is_expected.to include( + status: :success, + schema: {}) + end + end + end +end diff --git a/spec/services/ide/terminal_config_service_spec.rb b/spec/services/ide/terminal_config_service_spec.rb new file mode 100644 index 00000000000..d6c4f7a2a69 --- /dev/null +++ b/spec/services/ide/terminal_config_service_spec.rb @@ -0,0 +1,69 @@ +# frozen_string_literal: true + +require 'spec_helper' + +RSpec.describe Ide::TerminalConfigService do + let_it_be(:project) { create(:project, :repository) } + let_it_be(:user) { create(:user) } + let(:sha) { 'sha' } + + describe '#execute' do + subject { described_class.new(project, user, sha: sha).execute } + + before do + project.add_developer(user) + + allow(project.repository).to receive(:blob_data_at).with('sha', anything) do + config_content + end + end + + context 'content is not valid' do + let(:config_content) { 'invalid content' } + + it 'returns an error' do + is_expected.to include( + status: :error, + message: "Invalid configuration format") + end + end + + context 'terminal not defined' do + let(:config_content) { '{}' } + + it 'returns success' do + is_expected.to include( + status: :success, + terminal: nil) + end + end + + context 'terminal enabled' do + let(:config_content) { 'terminal: {}' } + + it 'returns success' do + is_expected.to include( + status: :success, + terminal: { + tag_list: [], + yaml_variables: [], + options: { script: ["sleep 60"] } + }) + end + end + + context 'custom terminal enabled' do + let(:config_content) { 'terminal: { before_script: [ls] }' } + + it 'returns success' do + is_expected.to include( + status: :success, + terminal: { + tag_list: [], + yaml_variables: [], + options: { before_script: ["ls"], script: ["sleep 60"] } + }) + end + end + end +end