Merge branch 'pedropombeiro/26345/2-add-sorted-collection-method' into 'master'

Add #sorted_collection in Variables::Collection

See merge request gitlab-org/gitlab!54502

lib/gitlab/ci/variables/collection.rb

@@ -6,8 +6,11 @@ module Gitlab
       class Collection
         include Enumerable
 
-        def initialize(variables = [])
+        attr_reader :errors
+
+        def initialize(variables = [], errors = nil)
           @variables = []
+          @errors = errors
 
           variables.each { |variable| self.append(variable) }
         end
@@ -42,6 +45,11 @@ module Gitlab
             .map { |env| [env.fetch(:key), env.fetch(:value)] }
             .to_h.with_indifferent_access
         end
+
+        # Returns a sorted Collection object, and sets errors property in case of an error
+        def sorted_collection(project)
+          Sort.new(self, project).collection
+        end
       end
     end
   end

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

@@ -4,7 +4,7 @@ module Gitlab
   module Ci
     module Variables
       class Collection
-        class Sorted
+        class Sort
           include TSort
           include Gitlab::Utils::StrongMemoize
 
@@ -35,11 +35,12 @@ module Gitlab
             end
           end
 
-          # sort sorts an array of variables, ignoring unknown variable references.
-          # If a circular variable reference is found, the original array is returned
-          def sort
+          # collection sorts a collection of variables, ignoring unknown variable references.
+          # If a circular variable reference is found, a new collection with the original array and an error is returned
+          def collection
             return @collection if Feature.disabled?(:variable_inside_variable, @project)
-            return @collection if errors
+
+            return Gitlab::Ci::Variables::Collection.new(@collection, errors) if errors
 
             Gitlab::Ci::Variables::Collection.new(tsort)
           end

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

@@ -2,7 +2,7 @@
 
 require 'spec_helper'
 
-RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
+RSpec.describe Gitlab::Ci::Variables::Collection::Sort 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) }
@@ -12,7 +12,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
     end
 
     context 'when FF :variable_inside_variable is disabled' do
-      subject { Gitlab::Ci::Variables::Collection::Sorted.new([], project_with_flag_disabled) }
+      subject { Gitlab::Ci::Variables::Collection::Sort.new([], project_with_flag_disabled) }
 
       it 'raises ArgumentError' do
         expect { subject }.to raise_error(ArgumentError, /Collection object was expected/)
@@ -20,7 +20,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
     end
 
     context 'when FF :variable_inside_variable is enabled' do
-      subject { Gitlab::Ci::Variables::Collection::Sorted.new([], project_with_flag_enabled) }
+      subject { Gitlab::Ci::Variables::Collection::Sort.new([], project_with_flag_enabled) }
 
       it 'raises ArgumentError' do
         expect { subject }.to raise_error(ArgumentError, /Collection object was expected/)
@@ -83,7 +83,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
         with_them do
           let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
 
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project_with_flag_disabled) }
+          subject { Gitlab::Ci::Variables::Collection::Sort.new(collection, project_with_flag_disabled) }
 
           it 'does not report error' do
             expect(subject.errors).to eq(nil)
@@ -135,7 +135,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
         with_them do
           let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
 
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project_with_flag_enabled) }
+          subject { Gitlab::Ci::Variables::Collection::Sort.new(collection, project_with_flag_enabled) }
 
           it 'errors matches expected validation result' do
             expect(subject.errors).to eq(validation_result)
@@ -149,7 +149,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
     end
   end
 
-  describe '#sort' do
+  describe '#collection' do
     context 'when FF :variable_inside_variable is disabled' do
       before do
         stub_feature_flags(variable_inside_variable: false)
@@ -202,7 +202,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
           let_it_be(:project) { create(:project) }
           let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
 
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project).sort }
+          subject { Gitlab::Ci::Variables::Collection::Sort.new(collection, project).collection }
 
           it 'does not expand variables' do
             is_expected.to be(collection)
@@ -280,7 +280,7 @@ RSpec.describe Gitlab::Ci::Variables::Collection::Sorted do
           let_it_be(:project) { create(:project) }
           let(:collection) { Gitlab::Ci::Variables::Collection.new(variables) }
 
-          subject { Gitlab::Ci::Variables::Collection::Sorted.new(collection, project).sort }
+          subject { Gitlab::Ci::Variables::Collection::Sort.new(collection, project).collection }
 
           it 'returns correctly sorted variables' do
             expect(subject.map { |var| var[:key] }).to eq(result)

spec/lib/gitlab/ci/variables/collection_spec.rb

@@ -121,4 +121,49 @@ RSpec.describe Gitlab::Ci::Variables::Collection do
       expect(collection.to_hash).not_to include(TEST1: 'test-1')
     end
   end
+
+  describe '#sorted_collection' do
+    let!(:project) { create(:project) }
+
+    subject { collection.sorted_collection(project) }
+
+    context 'when FF :variable_inside_variable is disabled' do
+      before do
+        stub_feature_flags(variable_inside_variable: false)
+      end
+
+      let(:collection) do
+        described_class.new
+          .append(key: 'A', value: 'test-$B')
+          .append(key: 'B', value: 'test-$C')
+          .append(key: 'C', value: 'test')
+      end
+
+      it { is_expected.to be(collection) }
+    end
+
+    context 'when FF :variable_inside_variable is enabled' do
+      before do
+        stub_feature_flags(variable_inside_variable: [project])
+      end
+
+      let(:collection) do
+        described_class.new
+          .append(key: 'A', value: 'test-$B')
+          .append(key: 'B', value: 'test-$C')
+          .append(key: 'C', value: 'test')
+      end
+
+      it { is_expected.to be_a(Gitlab::Ci::Variables::Collection) }
+
+      it 'returns sorted collection' do
+        expect(subject.to_a).to eq(
+          [
+            { key: 'C', value: 'test', masked: false, public: true },
+            { key: 'B', value: 'test-$C', masked: false, public: true },
+            { key: 'A', value: 'test-$B', masked: false, public: true }
+          ])
+      end
+    end
+  end
 end