Commit a85f2d9d authored by Chad Woolley's avatar Chad Woolley Committed by Thong Kuah

Fully implement SSE config file handling

* Third iteration towards static site editor config file.
* Replaces stub FileConfig with full implementation based on
  internal ::Gitlab::Config config-file API.
parent ef9eae98
...@@ -27,7 +27,12 @@ class Projects::StaticSiteEditorController < Projects::ApplicationController ...@@ -27,7 +27,12 @@ class Projects::StaticSiteEditorController < Projects::ApplicationController
if service_response.success? if service_response.success?
@data = service_response.payload @data = service_response.payload
else else
respond_422 # TODO: For now, if the service returns any error, the user is redirected
# to the root project page with the error message displayed as an alert.
# See https://gitlab.com/gitlab-org/gitlab/-/issues/213285#note_414808004
# for discussion of plans to handle this via a page owned by the Static Site Editor.
flash[:alert] = service_response.message
redirect_to project_path(project)
end end
end end
......
...@@ -4,18 +4,38 @@ module StaticSiteEditor ...@@ -4,18 +4,38 @@ module StaticSiteEditor
class ConfigService < ::BaseContainerService class ConfigService < ::BaseContainerService
ValidationError = Class.new(StandardError) ValidationError = Class.new(StandardError)
def execute def initialize(container:, current_user: nil, params: {})
super
@project = container @project = container
@repository = project.repository
@ref = params.fetch(:ref)
end
def execute
check_access! check_access!
file_config = load_file_config!
file_data = file_config.to_hash_with_defaults
generated_data = load_generated_config.data
check_for_duplicate_keys!(generated_data, file_data)
data = merged_data(generated_data, file_data)
ServiceResponse.success(payload: data) ServiceResponse.success(payload: data)
rescue ValidationError => e rescue ValidationError => e
ServiceResponse.error(message: e.message) ServiceResponse.error(message: e.message)
rescue => e
Gitlab::ErrorTracking.track_and_raise_exception(e)
end end
private private
attr_reader :project attr_reader :project, :repository, :ref
def static_site_editor_config_file
'.gitlab/static-site-editor.yml'
end
def check_access! def check_access!
unless can?(current_user, :download_code, project) unless can?(current_user, :download_code, project)
...@@ -23,27 +43,43 @@ module StaticSiteEditor ...@@ -23,27 +43,43 @@ module StaticSiteEditor
end end
end end
def data def load_file_config!
check_for_duplicate_keys! yaml = yaml_from_repo.presence || '{}'
generated_data.merge(file_data) file_config = Gitlab::StaticSiteEditor::Config::FileConfig.new(yaml)
unless file_config.valid?
raise ValidationError, file_config.errors.first
end
file_config
rescue Gitlab::StaticSiteEditor::Config::FileConfig::ConfigError => e
raise ValidationError, e.message
end end
def generated_data def load_generated_config
@generated_data ||= Gitlab::StaticSiteEditor::Config::GeneratedConfig.new( Gitlab::StaticSiteEditor::Config::GeneratedConfig.new(
project.repository, project.repository,
params.fetch(:ref), ref,
params.fetch(:path), params.fetch(:path),
params[:return_url] params[:return_url]
).data )
end
def file_data
@file_data ||= Gitlab::StaticSiteEditor::Config::FileConfig.new.data
end end
def check_for_duplicate_keys! def check_for_duplicate_keys!(generated_data, file_data)
duplicate_keys = generated_data.keys & file_data.keys duplicate_keys = generated_data.keys & file_data.keys
raise ValidationError.new("Duplicate key(s) '#{duplicate_keys}' found.") if duplicate_keys.present? raise ValidationError.new("Duplicate key(s) '#{duplicate_keys}' found.") if duplicate_keys.present?
end end
def merged_data(generated_data, file_data)
generated_data.merge(file_data)
end
def yaml_from_repo
project.repository.blob_data_at(ref, static_site_editor_config_file)
rescue GRPC::NotFound
# Return nil in the case of a GRPC::NotFound exception, so the default config will be used.
# Allow any other unexpected exception will be tracked and re-raised.
nil
end
end end
end end
---
title: Introduce '.gitlab/static-site-editor.yml' config file, with support for 'static_site_generator'
entry.
merge_request: 41957
author:
type: added
...@@ -3,11 +3,38 @@ ...@@ -3,11 +3,38 @@
module Gitlab module Gitlab
module StaticSiteEditor module StaticSiteEditor
module Config module Config
#
# Base GitLab Static Site Editor Configuration facade
#
class FileConfig class FileConfig
def data ConfigError = Class.new(StandardError)
{
static_site_generator: 'middleman' def initialize(yaml)
} content_hash = content_hash(yaml)
@global = Entry::Global.new(content_hash)
@global.compose!
rescue Gitlab::Config::Loader::FormatError => e
raise FileConfig::ConfigError, e.message
end
def valid?
@global.valid?
end
def errors
@global.errors
end
def to_hash_with_defaults
# NOTE: The current approach of simply mapping all the descendents' keys and values ('config')
# into a flat hash may need to be enhanced as we add more complex, non-scalar entries.
@global.descendants.map { |descendant| [descendant.key, descendant.config] }.to_h
end
private
def content_hash(yaml)
Gitlab::Config::Loader::Yaml.new(yaml).load!
end end
end end
end end
......
# frozen_string_literal: true
module Gitlab
module StaticSiteEditor
module Config
class FileConfig
module Entry
##
# This class represents a global entry - root Entry for entire
# GitLab StaticSiteEditor Configuration file.
#
class Global < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Configurable
include ::Gitlab::Config::Entry::Attributable
ALLOWED_KEYS = %i[static_site_generator].freeze
attributes ALLOWED_KEYS
validations do
validates :config, allowed_keys: ALLOWED_KEYS
end
entry :static_site_generator, Entry::StaticSiteGenerator,
description: 'Configuration of the Static Site Editor static site generator.'
end
end
end
end
end
end
# frozen_string_literal: true
module Gitlab
module StaticSiteEditor
module Config
class FileConfig
module Entry
##
# Entry that represents the static site generator tool/framework.
#
class StaticSiteGenerator < ::Gitlab::Config::Entry::Node
include ::Gitlab::Config::Entry::Validatable
validations do
validates :config, type: String, inclusion: { in: %w[middleman], message: "should be 'middleman'" }
end
def self.default
'middleman'
end
end
end
end
end
end
end
...@@ -8,6 +8,8 @@ RSpec.describe Projects::StaticSiteEditorController do ...@@ -8,6 +8,8 @@ RSpec.describe Projects::StaticSiteEditorController do
let(:data) { instance_double(Hash) } let(:data) { instance_double(Hash) }
describe 'GET show' do describe 'GET show' do
render_views
let(:default_params) do let(:default_params) do
{ {
namespace_id: project.namespace, namespace_id: project.namespace,
...@@ -82,8 +84,9 @@ RSpec.describe Projects::StaticSiteEditorController do ...@@ -82,8 +84,9 @@ RSpec.describe Projects::StaticSiteEditorController do
context 'when invalid config file' do context 'when invalid config file' do
let(:service_response) { ServiceResponse.error(message: 'invalid') } let(:service_response) { ServiceResponse.error(message: 'invalid') }
it 'returns 422' do it 'redirects to project page and flashes error message' do
expect(response).to have_gitlab_http_status(:unprocessable_entity) expect(response).to redirect_to(project_path(project))
expect(response).to set_flash[:alert].to('invalid')
end end
end end
end end
......
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::Global do
let(:global) { described_class.new(hash) }
let(:default_static_site_generator_value) { 'middleman' }
shared_examples_for 'valid default configuration' do
describe '#compose!' do
before do
global.compose!
end
it 'creates nodes hash' do
expect(global.descendants).to be_an Array
end
it 'creates node object for each entry' do
expect(global.descendants.count).to eq 1
end
it 'creates node object using valid class' do
expect(global.descendants.first)
.to be_an_instance_of expected_node_object_class
end
it 'sets correct description for nodes' do
expect(global.descendants.first.description)
.to eq 'Configuration of the Static Site Editor static site generator.'
end
describe '#leaf?' do
it 'is not leaf' do
expect(global).not_to be_leaf
end
end
end
context 'when not composed' do
describe '#static_site_generator_value' do
it 'returns nil' do
expect(global.static_site_generator_value).to be nil
end
end
describe '#leaf?' do
it 'is leaf' do
expect(global).to be_leaf
end
end
end
context 'when composed' do
before do
global.compose!
end
describe '#errors' do
it 'has no errors' do
expect(global.errors).to be_empty
end
end
describe '#static_site_generator_value' do
it 'returns correct values' do
expect(global.static_site_generator_value).to eq(default_static_site_generator_value)
end
end
end
end
describe '.nodes' do
it 'returns a hash' do
expect(described_class.nodes).to be_a(Hash)
end
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[static_site_generator])
end
end
end
context 'when configuration is valid' do
context 'when some entries defined' do
let(:expected_node_object_class) { Gitlab::StaticSiteEditor::Config::FileConfig::Entry::StaticSiteGenerator }
let(:hash) do
{ static_site_generator: default_static_site_generator_value }
end
it_behaves_like 'valid default configuration'
end
end
context 'when value is an empty hash' do
let(:expected_node_object_class) { Gitlab::Config::Entry::Unspecified }
let(:hash) { {} }
it_behaves_like 'valid default configuration'
end
context 'when configuration is not valid' do
before do
global.compose!
end
context 'when static_site_generator is invalid' do
let(:hash) do
{ static_site_generator: { not_a_string: true } }
end
describe '#errors' do
it 'reports errors' do
expect(global.errors)
.to include 'static_site_generator config should be a string'
end
end
end
context 'when there is an invalid key' do
let(:hash) do
{ invalid_key: true }
end
describe '#errors' do
it 'reports errors' do
expect(global.errors)
.to include 'global config contains unknown keys: invalid_key'
end
end
end
end
context 'when value is not a hash' do
let(:hash) { [] }
describe '#valid?' do
it 'is not valid' do
expect(global).not_to be_valid
end
end
describe '#errors' do
it 'returns error about invalid type' do
expect(global.errors.first).to match /should be a hash/
end
end
end
describe '#specified?' do
it 'is concrete entry that is defined' do
expect(global.specified?).to be true
end
end
describe '#[]' do
before do
global.compose!
end
let(:hash) do
{ static_site_generator: default_static_site_generator_value }
end
context 'when entry exists' do
it 'returns correct entry' do
expect(global[:static_site_generator])
.to be_an_instance_of Gitlab::StaticSiteEditor::Config::FileConfig::Entry::StaticSiteGenerator
expect(global[:static_site_generator].value).to eq default_static_site_generator_value
end
end
context 'when entry does not exist' do
it 'always return unspecified node' do
expect(global[:some][:unknown][:node])
.not_to be_specified
end
end
end
end
# frozen_string_literal: true
require 'spec_helper'
RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig::Entry::StaticSiteGenerator do
let(:static_site_generator) { described_class.new(config) }
describe 'validations' do
context 'when value is valid' do
let(:config) { 'middleman' }
describe '#value' do
it 'returns a static_site_generator key' do
expect(static_site_generator.value).to eq config
end
end
describe '#valid?' do
it 'is valid' do
expect(static_site_generator).to be_valid
end
end
end
context 'when value is invalid' do
let(:config) { 'not-a-valid-generator' }
describe '#valid?' do
it 'is not valid' do
expect(static_site_generator).not_to be_valid
end
end
end
context 'when value has a wrong type' do
let(:config) { { not_a_string: true } }
it 'reports errors about wrong type' do
expect(static_site_generator.errors)
.to include 'static site generator config should be a string'
end
end
end
describe '.default' do
it 'returns default static_site_generator' do
expect(described_class.default).to eq 'middleman'
end
end
end
...@@ -3,13 +3,93 @@ ...@@ -3,13 +3,93 @@
require 'spec_helper' require 'spec_helper'
RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig do RSpec.describe Gitlab::StaticSiteEditor::Config::FileConfig do
subject(:config) { described_class.new } let(:config) do
described_class.new(yml)
end
context 'when config is valid' do
context 'when config has valid values' do
let(:yml) do
<<-EOS
static_site_generator: middleman
EOS
end
describe '#to_hash_with_defaults' do
it 'returns hash created from string' do
hash = {
static_site_generator: 'middleman'
}
expect(config.to_hash_with_defaults).to eq hash
end
end
describe '#valid?' do
it 'is valid' do
expect(config).to be_valid
end
it 'has no errors' do
expect(config.errors).to be_empty
end
end
end
end
context 'when a config entry has an empty value' do
let(:yml) { 'static_site_generator: ' }
describe '#to_hash_with_defaults' do
it 'returns default values' do
hash = {
static_site_generator: 'middleman'
}
expect(config.to_hash_with_defaults).to eq hash
end
end
describe '#valid?' do
it 'is valid' do
expect(config).to be_valid
end
it 'has no errors' do
expect(config.errors).to be_empty
end
end
end
context 'when config is invalid' do
context 'when yml is incorrect' do
let(:yml) { '// invalid' }
describe '.new' do
it 'raises error' do
expect { config }.to raise_error(described_class::ConfigError, /Invalid configuration format/)
end
end
end
context 'when config value exists but is not a valid value' do
let(:yml) { 'static_site_generator: "unsupported-generator"' }
describe '#valid?' do
it 'is not valid' do
expect(config).not_to be_valid
end
describe '#data' do it 'has errors' do
subject { config.data } expect(config.errors).not_to be_empty
end
end
it 'returns hardcoded data for now' do describe '#errors' do
is_expected.to match(static_site_generator: 'middleman') it 'returns an array of strings' do
expect(config.errors).to all(be_an_instance_of(String))
end
end
end end
end end
end end
...@@ -7,8 +7,8 @@ RSpec.describe StaticSiteEditor::ConfigService do ...@@ -7,8 +7,8 @@ RSpec.describe StaticSiteEditor::ConfigService do
let_it_be(:user) { create(:user) } let_it_be(:user) { create(:user) }
# params # params
let(:ref) { double(:ref) } let(:ref) { 'master' }
let(:path) { double(:path) } let(:path) { 'README.md' }
let(:return_url) { double(:return_url) } let(:return_url) { double(:return_url) }
# stub data # stub data
...@@ -42,22 +42,84 @@ RSpec.describe StaticSiteEditor::ConfigService do ...@@ -42,22 +42,84 @@ RSpec.describe StaticSiteEditor::ConfigService do
allow_next_instance_of(Gitlab::StaticSiteEditor::Config::GeneratedConfig) do |config| allow_next_instance_of(Gitlab::StaticSiteEditor::Config::GeneratedConfig) do |config|
allow(config).to receive(:data) { generated_data } allow(config).to receive(:data) { generated_data }
end end
end
context 'when reading file from repo fails with an unexpected error' do
let(:unexpected_error) { RuntimeError.new('some unexpected error') }
allow_next_instance_of(Gitlab::StaticSiteEditor::Config::FileConfig) do |config| before do
allow(config).to receive(:data) { file_data } allow(project.repository).to receive(:blob_data_at).and_raise(unexpected_error)
end
it 'returns an error response' do
expect(Gitlab::ErrorTracking).to receive(:track_and_raise_exception).with(unexpected_error).and_call_original
expect { execute }.to raise_error(unexpected_error)
end end
end end
it 'returns merged generated data and config file data' do context 'when file is missing' do
expect(execute).to be_success before do
expect(execute.payload).to eq(generated: true, file: true) allow(project.repository).to receive(:blob_data_at).and_raise(GRPC::NotFound)
expect_next_instance_of(Gitlab::StaticSiteEditor::Config::FileConfig, '{}') do |config|
allow(config).to receive(:valid?) { true }
allow(config).to receive(:to_hash_with_defaults) { file_data }
end
end
it 'returns default config' do
expect(execute).to be_success
expect(execute.payload).to eq(generated: true, file: true)
end
end end
it 'returns an error if any keys would be overwritten by the merge' do context 'when file is present' do
generated_data[:duplicate_key] = true before do
file_data[:duplicate_key] = true allow(project.repository).to receive(:blob_data_at).with(ref, anything) do
expect(execute).to be_error config_content
expect(execute.message).to match(/duplicate key.*duplicate_key.*found/i) end
end
context 'and configuration is not valid' do
let(:config_content) { 'invalid content' }
before do
expect_next_instance_of(Gitlab::StaticSiteEditor::Config::FileConfig, config_content) do |config|
error = 'error'
allow(config).to receive_message_chain('errors.first') { error }
allow(config).to receive(:valid?) { false }
end
end
it 'returns an error' do
expect(execute).to be_error
expect(execute.message).to eq('Invalid configuration format')
end
end
context 'and configuration is valid' do
# NOTE: This has to be a valid config, even though it is mocked, because
# `expect_next_instance_of` executes the constructor logic.
let(:config_content) { 'static_site_generator: middleman' }
before do
expect_next_instance_of(Gitlab::StaticSiteEditor::Config::FileConfig, config_content) do |config|
allow(config).to receive(:valid?) { true }
allow(config).to receive(:to_hash_with_defaults) { file_data }
end
end
it 'returns merged generated data and config file data' do
expect(execute).to be_success
expect(execute.payload).to eq(generated: true, file: true)
end
it 'returns an error if any keys would be overwritten by the merge' do
generated_data[:duplicate_key] = true
file_data[:duplicate_key] = true
expect(execute).to be_error
expect(execute.message).to match(/duplicate key.*duplicate_key.*found/i)
end
end
end end
end end
end end
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment