From 5f502c3a82827ebd76e9585e5caa015753016c6a Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon <grzesiek.bizon@gmail.com> Date: Tue, 16 Oct 2018 15:09:36 +0200 Subject: [PATCH] Move external CI config class into proper namespace --- lib/gitlab/ci/config.rb | 4 +- lib/gitlab/ci/config/external/file/base.rb | 32 +++++---- lib/gitlab/ci/config/external/file/local.rb | 38 ++++++----- lib/gitlab/ci/config/external/file/remote.rb | 32 +++++---- lib/gitlab/ci/config/external/mapper.rb | 37 +++++----- lib/gitlab/ci/config/external/processor.rb | 68 ++++++++++--------- .../ci/config/external/file/local_spec.rb | 2 +- .../ci/config/external/file/remote_spec.rb | 2 +- .../gitlab/ci/config/external/mapper_spec.rb | 8 ++- .../ci/config/external/processor_spec.rb | 13 ++-- spec/lib/gitlab/ci/config_spec.rb | 4 +- 11 files changed, 127 insertions(+), 113 deletions(-) diff --git a/lib/gitlab/ci/config.rb b/lib/gitlab/ci/config.rb index fe98d25af29..7b3c5c12c61 100644 --- a/lib/gitlab/ci/config.rb +++ b/lib/gitlab/ci/config.rb @@ -15,7 +15,7 @@ module Gitlab @global.compose! rescue Loader::FormatError, Extendable::ExtensionError => e raise Config::ConfigError, e.message - rescue ::Gitlab::Ci::External::Processor::FileError => e + rescue External::Processor::FileError => e raise ::Gitlab::Ci::YamlProcessor::ValidationError, e.message end @@ -81,7 +81,7 @@ module Gitlab def process_external_files(config, project, opts) sha = opts.fetch(:sha) { project.repository.root_ref_sha } - ::Gitlab::Ci::External::Processor.new(config, project, sha).perform + Config::External::Processor.new(config, project, sha).perform end end end diff --git a/lib/gitlab/ci/config/external/file/base.rb b/lib/gitlab/ci/config/external/file/base.rb index f4da07b0b02..dc96c97b129 100644 --- a/lib/gitlab/ci/config/external/file/base.rb +++ b/lib/gitlab/ci/config/external/file/base.rb @@ -2,25 +2,27 @@ module Gitlab module Ci - module External - module File - class Base - YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze + class Config + module External + module File + class Base + YAML_WHITELIST_EXTENSION = /(yml|yaml)$/i.freeze - def initialize(location, opts = {}) - @location = location - end + def initialize(location, opts = {}) + @location = location + end - def valid? - location.match(YAML_WHITELIST_EXTENSION) && content - end + def valid? + location.match(YAML_WHITELIST_EXTENSION) && content + end - def content - raise NotImplementedError, 'content must be implemented and return a string or nil' - end + def content + raise NotImplementedError, 'content must be implemented and return a string or nil' + end - def error_message - raise NotImplementedError, 'error_message must be implemented and return a string' + def error_message + raise NotImplementedError, 'error_message must be implemented and return a string' + end end end end diff --git a/lib/gitlab/ci/config/external/file/local.rb b/lib/gitlab/ci/config/external/file/local.rb index 1aa7f687507..8c0ffd36449 100644 --- a/lib/gitlab/ci/config/external/file/local.rb +++ b/lib/gitlab/ci/config/external/file/local.rb @@ -2,30 +2,32 @@ module Gitlab module Ci - module External - module File - class Local < Base - attr_reader :location, :project, :sha + class Config + module External + module File + class Local < Base + attr_reader :location, :project, :sha - def initialize(location, opts = {}) - super + def initialize(location, opts = {}) + super - @project = opts.fetch(:project) - @sha = opts.fetch(:sha) - end + @project = opts.fetch(:project) + @sha = opts.fetch(:sha) + end - def content - @content ||= fetch_local_content - end + def content + @content ||= fetch_local_content + end - def error_message - "Local file '#{location}' is not valid." - end + def error_message + "Local file '#{location}' is not valid." + end - private + private - def fetch_local_content - project.repository.blob_data_at(sha, location) + def fetch_local_content + project.repository.blob_data_at(sha, location) + end end end end diff --git a/lib/gitlab/ci/config/external/file/remote.rb b/lib/gitlab/ci/config/external/file/remote.rb index 59bb3e8999e..a19c7639b5a 100644 --- a/lib/gitlab/ci/config/external/file/remote.rb +++ b/lib/gitlab/ci/config/external/file/remote.rb @@ -2,26 +2,28 @@ module Gitlab module Ci - module External - module File - class Remote < Base - include Gitlab::Utils::StrongMemoize - attr_reader :location + class Config + module External + module File + class Remote < Base + include Gitlab::Utils::StrongMemoize + attr_reader :location - def content - return @content if defined?(@content) + def content + return @content if defined?(@content) - @content = strong_memoize(:content) do - begin - Gitlab::HTTP.get(location) - rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError - nil + @content = strong_memoize(:content) do + begin + Gitlab::HTTP.get(location) + rescue Gitlab::HTTP::Error, Timeout::Error, SocketError, Gitlab::HTTP::BlockedUrlError + nil + end end end - end - def error_message - "Remote file '#{location}' is not valid." + def error_message + "Remote file '#{location}' is not valid." + end end end end diff --git a/lib/gitlab/ci/config/external/mapper.rb b/lib/gitlab/ci/config/external/mapper.rb index 58bd6a19acf..def3563e505 100644 --- a/lib/gitlab/ci/config/external/mapper.rb +++ b/lib/gitlab/ci/config/external/mapper.rb @@ -2,28 +2,29 @@ module Gitlab module Ci - module External - class Mapper - def initialize(values, project, sha) - @locations = Array(values.fetch(:include, [])) - @project = project - @sha = sha - end + class Config + module External + class Mapper + def initialize(values, project, sha) + @locations = Array(values.fetch(:include, [])) + @project = project + @sha = sha + end - def process - locations.map { |location| build_external_file(location) } - end + def process + locations.map { |location| build_external_file(location) } + end - private + private - attr_reader :locations, :project, :sha + attr_reader :locations, :project, :sha - def build_external_file(location) - if ::Gitlab::UrlSanitizer.valid?(location) - Gitlab::Ci::External::File::Remote.new(location) - else - options = { project: project, sha: sha } - Gitlab::Ci::External::File::Local.new(location, options) + def build_external_file(location) + if ::Gitlab::UrlSanitizer.valid?(location) + External::File::Remote.new(location) + else + External::File::Local.new(location, project: project, sha: sha) + end end end end diff --git a/lib/gitlab/ci/config/external/processor.rb b/lib/gitlab/ci/config/external/processor.rb index 76cf3ce89f9..f3b20085cd6 100644 --- a/lib/gitlab/ci/config/external/processor.rb +++ b/lib/gitlab/ci/config/external/processor.rb @@ -2,49 +2,51 @@ module Gitlab module Ci - module External - class Processor - FileError = Class.new(StandardError) - - def initialize(values, project, sha) - @values = values - @external_files = Gitlab::Ci::External::Mapper.new(values, project, sha).process - @content = {} - end + class Config + module External + class Processor + FileError = Class.new(StandardError) + + def initialize(values, project, sha) + @values = values + @external_files = External::Mapper.new(values, project, sha).process + @content = {} + end - def perform - return values if external_files.empty? + def perform + return values if external_files.empty? - external_files.each do |external_file| - validate_external_file(external_file) - @content.deep_merge!(content_of(external_file)) - end + external_files.each do |external_file| + validate_external_file(external_file) + @content.deep_merge!(content_of(external_file)) + end - append_inline_content - remove_include_keyword - end + append_inline_content + remove_include_keyword + end - private + private - attr_reader :values, :external_files, :content + attr_reader :values, :external_files, :content - def validate_external_file(external_file) - unless external_file.valid? - raise FileError, external_file.error_message + def validate_external_file(external_file) + unless external_file.valid? + raise FileError, external_file.error_message + end end - end - def content_of(external_file) - Gitlab::Ci::Config::Loader.new(external_file.content).load! - end + def content_of(external_file) + Config::Loader.new(external_file.content).load! + end - def append_inline_content - @content.deep_merge!(@values) - end + def append_inline_content + @content.deep_merge!(@values) + end - def remove_include_keyword - content.delete(:include) - content + def remove_include_keyword + content.delete(:include) + content + end end end end diff --git a/spec/lib/gitlab/ci/config/external/file/local_spec.rb b/spec/lib/gitlab/ci/config/external/file/local_spec.rb index 73bb4ccf468..5c0e2eee71f 100644 --- a/spec/lib/gitlab/ci/config/external/file/local_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/local_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::File::Local do +describe Gitlab::Ci::Config::External::File::Local do let(:project) { create(:project, :repository) } let(:local_file) { described_class.new(location, { project: project, sha: '12345' }) } diff --git a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb index b1819c8960b..72853797bcd 100644 --- a/spec/lib/gitlab/ci/config/external/file/remote_spec.rb +++ b/spec/lib/gitlab/ci/config/external/file/remote_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::File::Remote do +describe Gitlab::Ci::Config::External::File::Remote do let(:remote_file) { described_class.new(location) } let(:location) { 'https://gitlab.com/gitlab-org/gitlab-ce/blob/1234/.gitlab-ci-1.yml' } let(:remote_file_content) do diff --git a/spec/lib/gitlab/ci/config/external/mapper_spec.rb b/spec/lib/gitlab/ci/config/external/mapper_spec.rb index d925d6af73d..5b236fe99f1 100644 --- a/spec/lib/gitlab/ci/config/external/mapper_spec.rb +++ b/spec/lib/gitlab/ci/config/external/mapper_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::Mapper do +describe Gitlab::Ci::Config::External::Mapper do let(:project) { create(:project, :repository) } let(:file_content) do <<~HEREDOC @@ -27,7 +27,8 @@ describe Gitlab::Ci::External::Mapper do end it 'returns File instances' do - expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Local) + expect(subject.first) + .to be_an_instance_of(Gitlab::Ci::Config::External::File::Local) end end @@ -49,7 +50,8 @@ describe Gitlab::Ci::External::Mapper do end it 'returns File instances' do - expect(subject.first).to be_an_instance_of(Gitlab::Ci::External::File::Remote) + expect(subject.first) + .to be_an_instance_of(Gitlab::Ci::Config::External::File::Remote) end end end diff --git a/spec/lib/gitlab/ci/config/external/processor_spec.rb b/spec/lib/gitlab/ci/config/external/processor_spec.rb index 3c7394f53d2..0196e11628d 100644 --- a/spec/lib/gitlab/ci/config/external/processor_spec.rb +++ b/spec/lib/gitlab/ci/config/external/processor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' -describe Gitlab::Ci::External::Processor do +describe Gitlab::Ci::Config::External::Processor do let(:project) { create(:project, :repository) } let(:processor) { described_class.new(values, project, '12345') } @@ -92,7 +92,8 @@ describe Gitlab::Ci::External::Processor do end before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) + .to receive(:fetch_local_content).and_return(local_file_content) end it 'should append the file to the values' do @@ -131,7 +132,10 @@ describe Gitlab::Ci::External::Processor do before do local_file_content = File.read(Rails.root.join('spec/fixtures/gitlab/ci/external_files/.gitlab-ci-template-1.yml')) - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + + allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) + .to receive(:fetch_local_content).and_return(local_file_content) + WebMock.stub_request(:get, remote_file).to_return(body: remote_file_content) end @@ -150,7 +154,8 @@ describe Gitlab::Ci::External::Processor do let(:local_file_content) { 'invalid content file ////' } before do - allow_any_instance_of(Gitlab::Ci::External::File::Local).to receive(:fetch_local_content).and_return(local_file_content) + allow_any_instance_of(Gitlab::Ci::Config::External::File::Local) + .to receive(:fetch_local_content).and_return(local_file_content) end it 'should raise an error' do diff --git a/spec/lib/gitlab/ci/config_spec.rb b/spec/lib/gitlab/ci/config_spec.rb index b43aca8a354..7a749a2ef6d 100644 --- a/spec/lib/gitlab/ci/config_spec.rb +++ b/spec/lib/gitlab/ci/config_spec.rb @@ -1,6 +1,4 @@ -require 'fast_spec_helper' - -require_dependency 'active_model' +require 'spec_helper' describe Gitlab::Ci::Config do let(:config) do -- 2.30.9