Merge branch 'pedropombeiro/26345/4-add-depends_on-attribute' into 'master'

Support :depends_on attribute in Collection::Item [RUN ALL RSPEC] [RUN AS-IF-FOSS]

See merge request gitlab-org/gitlab!54504

lib/gitlab/ci/variables/collection/item.rb

@@ -5,13 +5,13 @@ module Gitlab
     module Variables
       class Collection
         class Item
+          include Gitlab::Utils::StrongMemoize
+
           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
               value.is_a?(String) || value.nil?
 
-            @variable = {
-              key: key, value: value, public: public, file: file, masked: masked
-            }
+            @variable = { key: key, value: value, public: public, file: file, masked: masked }
           end
 
           def value
@@ -26,6 +26,14 @@ module Gitlab
             to_runner_variable == self.class.fabricate(other).to_runner_variable
           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
           # don't expose `file` attribute at all (stems from what the runner

lib/gitlab/ci/variables/collection/sort.rb

@@ -27,11 +27,11 @@ module Gitlab
 
             strong_memoize(:errors) do
               # 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
               end
 
-              "circular variable reference detected: #{errors.join(', ')}" if errors.any?
+              "circular variable reference detected: #{cyclic_vars.join(', ')}" if cyclic_vars.any?
             end
           end
 
@@ -51,29 +51,11 @@ module Gitlab
             @collection.each(&block)
           end
 
-          def tsort_each_child(variable, &block)
-            each_variable_reference(variable.value, &block)
-          end
-
-          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)
+          def tsort_each_child(var_item, &block)
+            depends_on = var_item.depends_on
+            return unless depends_on
 
-            value.scan(ExpandVariables::VARIABLES_REGEXP) do |var_ref|
-              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
+            depends_on.filter_map { |var_ref_name| @collection[var_ref_name] }.each(&block)
           end
         end
       end

spec/lib/gitlab/ci/variables/collection/item_spec.rb

@@ -70,6 +70,43 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Item do
     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
     it 'supports using a hash' do
       resource = described_class.fabricate(variable)
@@ -140,5 +177,13 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Item do
           .to eq(key: 'VAR', value: 'value', public: true, file: true, masked: false)
       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

spec/models/ci/build_spec.rb

@@ -2584,14 +2584,14 @@ RSpec.describe Ci::Build do
       end
 
       shared_examples 'containing environment variables' do
-        it { environment_variables.each { |v| is_expected.to include(v) } }
+        it { is_expected.to include(*environment_variables) }
       end
 
       context 'when no URL was set' do
         it_behaves_like 'containing environment variables'
 
         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')
         end