Commit e373235f authored by Pedro Pombeiro's avatar Pedro Pombeiro Committed by Kamil Trzciński

Support `:depends_on` attribute in `Collection::Item`

Allows for simpler logic in `Sorted` class
parent 4c9f18d1
...@@ -5,13 +5,13 @@ module Gitlab ...@@ -5,13 +5,13 @@ module Gitlab
module Variables module Variables
class Collection class Collection
class Item class Item
include Gitlab::Utils::StrongMemoize
def initialize(key:, value:, public: true, file: false, masked: false) def initialize(key:, value:, public: true, file: false, masked: false)
raise ArgumentError, "`#{key}` must be of type String or nil value, while it was: #{value.class}" unless raise ArgumentError, "`#{key}` must be of type String or nil value, while it was: #{value.class}" unless
value.is_a?(String) || value.nil? value.is_a?(String) || value.nil?
@variable = { @variable = { key: key, value: value, public: public, file: file, masked: masked }
key: key, value: value, public: public, file: file, masked: masked
}
end end
def value def value
...@@ -26,6 +26,14 @@ module Gitlab ...@@ -26,6 +26,14 @@ module Gitlab
to_runner_variable == self.class.fabricate(other).to_runner_variable to_runner_variable == self.class.fabricate(other).to_runner_variable
end end
def depends_on
strong_memoize(:depends_on) do
next unless ExpandVariables.possible_var_reference?(value)
value.scan(ExpandVariables::VARIABLES_REGEXP).map(&:first)
end
end
## ##
# If `file: true` has been provided we expose it, otherwise we # If `file: true` has been provided we expose it, otherwise we
# don't expose `file` attribute at all (stems from what the runner # don't expose `file` attribute at all (stems from what the runner
......
...@@ -27,11 +27,11 @@ module Gitlab ...@@ -27,11 +27,11 @@ module Gitlab
strong_memoize(:errors) do strong_memoize(:errors) do
# Check for cyclic dependencies and build error message in that case # Check for cyclic dependencies and build error message in that case
errors = each_strongly_connected_component.filter_map do |component| cyclic_vars = each_strongly_connected_component.filter_map do |component|
component.map { |v| v[:key] }.inspect if component.size > 1 component.map { |v| v[:key] }.inspect if component.size > 1
end end
"circular variable reference detected: #{errors.join(', ')}" if errors.any? "circular variable reference detected: #{cyclic_vars.join(', ')}" if cyclic_vars.any?
end end
end end
...@@ -51,29 +51,11 @@ module Gitlab ...@@ -51,29 +51,11 @@ module Gitlab
@collection.each(&block) @collection.each(&block)
end end
def tsort_each_child(variable, &block) def tsort_each_child(var_item, &block)
each_variable_reference(variable.value, &block) depends_on = var_item.depends_on
end return unless depends_on
def input_vars
strong_memoize(:input_vars) do
@collection.index_by { |env| env[:key] }
end
end
def walk_references(value)
return unless ExpandVariables.possible_var_reference?(value)
value.scan(ExpandVariables::VARIABLES_REGEXP) do |var_ref| depends_on.filter_map { |var_ref_name| @collection[var_ref_name] }.each(&block)
yield(input_vars, var_ref.first)
end
end
def each_variable_reference(value)
walk_references(value) do |vars_hash, ref_var_name|
variable = vars_hash.dig(ref_var_name)
yield variable if variable
end
end end
end end
end end
......
...@@ -70,6 +70,43 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Item do ...@@ -70,6 +70,43 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Item do
end end
end end
describe '#depends_on' do
let(:item) { Gitlab::Ci::Variables::Collection::Item.new(**variable) }
subject { item.depends_on }
context 'table tests' do
using RSpec::Parameterized::TableSyntax
where do
{
"no variable references": {
variable: { key: 'VAR', value: 'something' },
expected_depends_on: nil
},
"simple variable reference": {
variable: { key: 'VAR', value: 'something_$VAR2' },
expected_depends_on: %w(VAR2)
},
"complex expansion": {
variable: { key: 'VAR', value: 'something_${VAR2}_$VAR3' },
expected_depends_on: %w(VAR2 VAR3)
},
"complex expansions for Windows": {
variable: { key: 'variable3', value: 'key%variable%%variable2%' },
expected_depends_on: %w(variable variable2)
}
}
end
with_them do
it 'contains referenced variable names' do
is_expected.to eq(expected_depends_on)
end
end
end
end
describe '.fabricate' do describe '.fabricate' do
it 'supports using a hash' do it 'supports using a hash' do
resource = described_class.fabricate(variable) resource = described_class.fabricate(variable)
...@@ -140,5 +177,13 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Item do ...@@ -140,5 +177,13 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Item do
.to eq(key: 'VAR', value: 'value', public: true, file: true, masked: false) .to eq(key: 'VAR', value: 'value', public: true, file: true, masked: false)
end end
end end
context 'when referencing a variable' do
it '#depends_on contains names of dependencies' do
runner_variable = described_class.new(key: 'CI_VAR', value: '${CI_VAR_2}-123-$CI_VAR_3')
expect(runner_variable.depends_on).to eq(%w(CI_VAR_2 CI_VAR_3))
end
end
end end
end end
...@@ -2584,14 +2584,14 @@ RSpec.describe Ci::Build do ...@@ -2584,14 +2584,14 @@ RSpec.describe Ci::Build do
end end
shared_examples 'containing environment variables' do shared_examples 'containing environment variables' do
it { environment_variables.each { |v| is_expected.to include(v) } } it { is_expected.to include(*environment_variables) }
end end
context 'when no URL was set' do context 'when no URL was set' do
it_behaves_like 'containing environment variables' it_behaves_like 'containing environment variables'
it 'does not have CI_ENVIRONMENT_URL' do it 'does not have CI_ENVIRONMENT_URL' do
keys = subject.map { |var| var[:key] } keys = subject.pluck(:key)
expect(keys).not_to include('CI_ENVIRONMENT_URL') expect(keys).not_to include('CI_ENVIRONMENT_URL')
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