From 000f9d01f7a8c6eb20c16927f95a2e3de42bc283 Mon Sep 17 00:00:00 2001 From: Grzegorz Bizon Date: Thu, 22 Mar 2018 13:57:39 +0100 Subject: [PATCH] Decouple YAML processor from pipeline objects --- app/models/ci/pipeline.rb | 8 +++- lib/gitlab/ci/yaml_processor.rb | 48 +++++++---------------- spec/lib/gitlab/ci/yaml_processor_spec.rb | 44 ++++++++++----------- 3 files changed, 43 insertions(+), 57 deletions(-) diff --git a/app/models/ci/pipeline.rb b/app/models/ci/pipeline.rb index bac5b3860e4..acc2dbb59fd 100644 --- a/app/models/ci/pipeline.rb +++ b/app/models/ci/pipeline.rb @@ -362,7 +362,13 @@ module Ci def stage_seeds return [] unless config_processor - @stage_seeds ||= config_processor.stage_seeds(self) + strong_memoize(:stage_seeds) do + seeds = config_processor.stages.map do |attributes| + Gitlab::Ci::Pipeline::Seed::Stage.new(self, attributes) + end + + seeds.select(&:included?) + end end def has_kubernetes_active? diff --git a/lib/gitlab/ci/yaml_processor.rb b/lib/gitlab/ci/yaml_processor.rb index 7519331c9de..6074829e152 100644 --- a/lib/gitlab/ci/yaml_processor.rb +++ b/lib/gitlab/ci/yaml_processor.rb @@ -53,60 +53,40 @@ module Gitlab }.compact } end - # REFACTORING, this needs improvement + # REFACTORING, this needs improvement, and specs # - def build_seed_attributes(stage) + def stage_attributes(stage) selected = @jobs.values.select do |job| job[:stage] == stage end selected.map do |job| build_attributes(job[:name]) - .merge(only: job.fetch(:only, {})) - .merge(except: job.fetch(:except, {})) end end - # REFACTORING, slated for removal + # REFACTORING, needs specs # - def pipeline_stage_builds(stage, pipeline) - selected_jobs = @jobs.select do |_, job| - next unless job[:stage] == stage - - only_specs = Gitlab::Ci::Build::Policy - .fabricate(job.fetch(:only, {})) - except_specs = Gitlab::Ci::Build::Policy - .fabricate(job.fetch(:except, {})) - - only_specs.all? { |spec| spec.satisfied_by?(pipeline) } && - except_specs.none? { |spec| spec.satisfied_by?(pipeline) } - end + def seed_attributes(stage) + seeds = stage_attributes(stage).map do |attributes| + job = @jobs.fetch(attributes[:name].to_sym) - selected_jobs.map { |_, job| build_attributes(job[:name]) } + attributes + .merge(only: job.fetch(:only, {})) + .merge(except: job.fetch(:except, {})) end - def stage_seed_attributes(stage) { name: stage, index: @stages.index(stage), - builds: build_seed_attributes(stage) } + builds: seeds } end - # REFACTORING, slated for removal - # * WARNING this method is currently evaluating only/except policies - # in two places - Seed::Build, and in pipeline_stage_builds - # * WARNING it needs to be refactored to use SSOT + # REFACTORING, needs specs # - def stage_seeds(pipeline) - seeds = @stages.uniq.map do |stage| - builds = pipeline_stage_builds(stage, pipeline) - - if builds.any? - Gitlab::Ci::Pipeline::Seed::Stage - .new(pipeline, stage_seed_attributes(stage)) - end + def stages + @stages.uniq.map do |stage| + seed_attributes(stage) end - - seeds.compact end def self.validation_message(content) diff --git a/spec/lib/gitlab/ci/yaml_processor_spec.rb b/spec/lib/gitlab/ci/yaml_processor_spec.rb index 922c93c9b2e..19dd89f0b6d 100644 --- a/spec/lib/gitlab/ci/yaml_processor_spec.rb +++ b/spec/lib/gitlab/ci/yaml_processor_spec.rb @@ -203,7 +203,7 @@ module Gitlab let(:config_data) { YAML.dump(config) } let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config_data) } - subject { config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first } + subject { config_processor.stage_attributes('test').first } describe "before_script" do context "in global context" do @@ -286,8 +286,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first).to eq({ + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -321,8 +321,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first).to eq({ + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -354,8 +354,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first).to eq({ + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -383,8 +383,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first).to eq({ + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -529,8 +529,8 @@ module Gitlab }) config_processor = Gitlab::Ci::YamlProcessor.new(config) + builds = config_processor.stage_attributes("test") - builds = config_processor.pipeline_stage_builds("test", pipeline(ref: "master")) expect(builds.size).to eq(1) expect(builds.first[:when]).to eq(when_state) end @@ -561,8 +561,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first[:options][:cache]).to eq( + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first[:options][:cache]).to eq( paths: ["logs/", "binaries/"], untracked: true, key: 'key', @@ -580,8 +580,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first[:options][:cache]).to eq( + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first[:options][:cache]).to eq( paths: ["logs/", "binaries/"], untracked: true, key: 'key', @@ -600,8 +600,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first[:options][:cache]).to eq( + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first[:options][:cache]).to eq( paths: ["test/"], untracked: false, key: 'local', @@ -629,8 +629,8 @@ module Gitlab config_processor = Gitlab::Ci::YamlProcessor.new(config) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).size).to eq(1) - expect(config_processor.pipeline_stage_builds("test", pipeline(ref: "master")).first).to eq({ + expect(config_processor.stage_attributes("test").size).to eq(1) + expect(config_processor.stage_attributes("test").first).to eq({ stage: "test", stage_idx: 1, name: "rspec", @@ -666,8 +666,8 @@ module Gitlab }) config_processor = Gitlab::Ci::YamlProcessor.new(config) + builds = config_processor.stage_attributes("test") - builds = config_processor.pipeline_stage_builds("test", pipeline(ref: "master")) expect(builds.size).to eq(1) expect(builds.first[:options][:artifacts][:when]).to eq(when_state) end @@ -682,7 +682,7 @@ module Gitlab end let(:processor) { Gitlab::Ci::YamlProcessor.new(YAML.dump(config)) } - let(:builds) { processor.pipeline_stage_builds('deploy', pipeline(ref: 'master')) } + let(:builds) { processor.stage_attributes('deploy') } context 'when a production environment is specified' do let(:environment) { 'production' } @@ -839,7 +839,7 @@ module Gitlab describe "Hidden jobs" do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } - subject { config_processor.pipeline_stage_builds("test", pipeline(ref: "master")) } + subject { config_processor.stage_attributes("test") } shared_examples 'hidden_job_handling' do it "doesn't create jobs that start with dot" do @@ -887,7 +887,7 @@ module Gitlab describe "YAML Alias/Anchor" do let(:config_processor) { Gitlab::Ci::YamlProcessor.new(config) } - subject { config_processor.pipeline_stage_builds("build", pipeline(ref: "master")) } + subject { config_processor.stage_attributes("build") } shared_examples 'job_templates_handling' do it "is correctly supported for jobs" do -- GitLab