Merge branch 'pedropombeiro/26345/1-make-sorted-take-collection' into 'master'

Make Collection::Sorted class input/output a Collection

See merge request gitlab-org/gitlab!54500

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

@@ -8,8 +8,11 @@ module Gitlab
           include TSort
           include Gitlab::Utils::StrongMemoize
 
-          def initialize(variables, project)
-            @variables = variables
+          def initialize(collection, project)
+            raise(ArgumentError, "A Gitlab::Ci::Variables::Collection object was expected") unless
+              collection.is_a?(Collection)
+
+            @collection = collection
             @project = project
           end
 
@@ -35,16 +38,16 @@ module Gitlab
           # sort sorts an array of variables, ignoring unknown variable references.
           # If a circular variable reference is found, the original array is returned
           def sort
-            return @variables if Feature.disabled?(:variable_inside_variable, @project)
-            return @variables if errors
+            return @collection if Feature.disabled?(:variable_inside_variable, @project)
+            return @collection if errors
 
-            tsort
+            Gitlab::Ci::Variables::Collection.new(tsort)
           end
 
           private
 
           def tsort_each_node(&block)
-            @variables.each(&block)
+            @collection.each(&block)
           end
 
           def tsort_each_child(variable, &block)
@@ -53,7 +56,7 @@ module Gitlab
 
           def input_vars
             strong_memoize(:input_vars) do
-              @variables.index_by { |env| env.fetch(:key) }
+              @collection.index_by { |env| env[:key] }
             end
           end
 

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

@@ -3,6 +3,31 @@
 require 'spec_helper'
 
 RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
+  describe '#initialize with non-Collection value' do
+    let_it_be(:project_with_flag_disabled) { create(:project) }
+    let_it_be(:project_with_flag_enabled) { create(:project) }
+
+    before do
+      stub_feature_flags(variable_inside_variable: [project_with_flag_enabled])
+    end
+
+    context 'when FF :variable_inside_variable is disabled' do
+      subject { Gitlab::Ci::Variables::Collection::Sorted.new([], project_with_flag_disabled) }
+
+      it 'raises ArgumentError' do
+        expect { subject }.to raise_error(ArgumentError, /Collection object was expected/)
+      end
+    end
+
+    context 'when FF :variable_inside_variable is enabled' do
+      subject { Gitlab::Ci::Variables::Collection::Sorted.new([], project_with_flag_enabled) }
+
+      it 'raises ArgumentError' do
+        expect { subject }.to raise_error(ArgumentError, /Collection object was expected/)
+      end
+    end
+  end
+
   describe '#errors' do
     context 'when FF :variable_inside_variable is disabled' do
       let_it_be(:project_with_flag_disabled) { create(:project) }
@@ -56,7 +81,9 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
         end
 
         with_them do
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(variables, project_with_flag_disabled) }
+          let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
+
+          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project_with_flag_disabled) }
 
           it 'does not report error' do
             expect(subject.errors).to eq(nil)
@@ -106,7 +133,9 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
         end
 
         with_them do
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(variables, project_with_flag_enabled) }
+          let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
+
+          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project_with_flag_enabled) }
 
           it 'errors matches expected validation result' do
             expect(subject.errors).to eq(validation_result)
@@ -171,10 +200,12 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
 
         with_them do
           let_it_be(:project) { create(:project) }
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(variables, project) }
+          let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
+
+          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project).sort }
 
           it 'does not expand variables' do
-            expect(subject.sort).to eq(variables)
+            is_expected.to be(collection)
           end
         end
       end
@@ -247,10 +278,12 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
 
         with_them do
           let_it_be(:project) { create(:project) }
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(variables, project) }
+          let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
+
+          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project).sort }
 
-          it 'sort returns correctly sorted variables' do
-            expect(subject.sort.map { |var| var[:key] }).to eq(result)
+          it 'returns correctly sorted variables' do
+            expect(subject.map { |var| var[:key] }).to eq(result)
           end
         end
       end