Extract variables collection item to a separate class

6 files changed, 103 insertions, 36 deletions

diff --git a/app/models/ci/variable.rb b/app/models/ci/variable.rb
index 7c71291de84..51389fb82bf 100644
--- a/app/models/ci/variable.rb
+++ b/app/models/ci/variable.rb
@@ -12,5 +12,9 @@ module Ci
     }
 
     scope :unprotected, -> { where(protected: false) }
+
+    def to_hash
+      { key: key, value: value, public: false }
+    end
   end
 end
diff --git a/app/models/project_services/kubernetes_service.rb b/app/models/project_services/kubernetes_service.rb
index edf80bbd509..347c9f34033 100644
--- a/app/models/project_services/kubernetes_service.rb
+++ b/app/models/project_services/kubernetes_service.rb
@@ -105,7 +105,7 @@ class KubernetesService < DeploymentService
   def predefined_variables
     config = YAML.dump(kubeconfig)
 
-    variables = Gitlab::Ci::Variables::Collection.new.tap do |collection|
+    Gitlab::Ci::Variables::Collection.new.tap do |collection|
       collection.append(key: 'KUBE_URL', value: api_url, public: true)
       collection.append(key: 'KUBE_TOKEN', value: token, public: false)
       collection.append(key: 'KUBE_NAMESPACE', value: actual_namespace, public: true)
@@ -114,8 +114,6 @@ class KubernetesService < DeploymentService
       collection.append(key: 'KUBE_CA_PEM', value: ca_pem, public: true)
       collection.append(key: 'KUBE_CA_PEM_FILE', value: ca_pem, public: true, file: true)
     end
-
-    variables.to_runner_variables
   end
 
   # Constructs a list of terminals from the reactive cache
diff --git a/lib/gitlab/ci/variables/collection.rb b/lib/gitlab/ci/variables/collection.rb
index 2f5987f2846..83db5492d43 100644
--- a/lib/gitlab/ci/variables/collection.rb
+++ b/lib/gitlab/ci/variables/collection.rb
@@ -4,8 +4,6 @@ module Gitlab
       class Collection
         include Enumerable
 
-        Variable = Struct.new(:key, :value, :public, :file)
-
         def initialize(variables = [])
           @variables = []
 
@@ -13,7 +11,7 @@ module Gitlab
         end
 
         def append(resource)
-          @variables.append(fabricate(resource))
+          @variables.append(Collection::Item.fabricate(resource))
         end
 
         def each
@@ -27,35 +25,8 @@ module Gitlab
           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
-        # expects).
-        #
         def to_runner_variables
-          self.map do |variable|
-            variable.to_h.reject do |component, value|
-              component == :file && value == false
-            end
-          end
-        end
-
-        private
-
-        def fabricate(resource)
-          case resource
-          when Hash
-            Collection::Variable.new(resource.fetch(:key),
-                                     resource.fetch(:value),
-                                     resource.fetch(:public, false),
-                                     resource.fetch(:file, false))
-          when ::Ci::Variable
-            Variable.new(resource.key, resource.value, false, false)
-          when Collection::Variable
-            resource.dup
-          else
-            raise ArgumentError, 'Unknown CI/CD variable resource!'
-          end
+          self.map(&:to_hash)
         end
       end
     end
diff --git a/lib/gitlab/ci/variables/collection/item.rb b/lib/gitlab/ci/variables/collection/item.rb
new file mode 100644
index 00000000000..4de96e97417
--- /dev/null
+++ b/lib/gitlab/ci/variables/collection/item.rb
@@ -0,0 +1,46 @@
+module Gitlab
+  module Ci
+    module Variables
+      class Collection
+        class Item
+          def initialize(**options)
+            @variable = {
+              key: options.fetch(:key),
+              value: options.fetch(:value),
+              public: options.fetch(:public, false),
+              file: options.fetch(:files, false)
+            }
+          end
+
+          def ==(other)
+            to_hash == self.class.fabricate(other).to_hash
+          end
+
+          ##
+          # If `file: true` has been provided we expose it, otherwise we
+          # don't expose `file` attribute at all (stems from what the runner
+          # expects).
+          #
+          def to_hash
+            @variable.reject do |hash_key, hash_value|
+              hash_key == :file && hash_value == false
+            end
+          end
+
+          def self.fabricate(resource)
+            case resource
+            when Hash
+              self.new(resource)
+            when ::Ci::Variable
+              self.new(resource.to_hash)
+            when self
+              resource.dup
+            else
+              raise ArgumentError, 'Unknown CI/CD variable resource!'
+            end
+          end
+        end
+      end
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/variables/collection/item_spec.rb b/spec/lib/gitlab/ci/variables/collection/item_spec.rb
new file mode 100644
index 00000000000..6f86d658f52
--- /dev/null
+++ b/spec/lib/gitlab/ci/variables/collection/item_spec.rb
@@ -0,0 +1,47 @@
+require 'spec_helper'
+
+describe Gitlab::Ci::Variables::Collection::Item do
+  let(:variable) do
+    { key: 'VAR', value: 'something', public: true }
+  end
+
+  describe '.fabricate' do
+    it 'supports using a hash' do
+      resource = described_class.fabricate(variable)
+
+      expect(resource).to be_a(described_class)
+      expect(resource).to eq variable
+    end
+
+    it 'supports using an active record resource' do
+      resource = described_class.fabricate(create(:ci_variable))
+
+      expect(resource).to be_a(described_class)
+      expect(resource).to eq(key: 'VARIABLE_1',
+                             value: 'VARIABLE_VALUE',
+                             public: false)
+    end
+
+    it 'supports using another collection item' do
+      item = described_class.new(**variable)
+
+      resource = described_class.fabricate(item)
+
+      expect(resource).to be_a(described_class)
+      expect(resource).to eq variable
+      expect(resource.object_id).not_to eq item.object_id
+    end
+  end
+
+  describe '#==' do
+    it 'compares a hash representation of a variable' do
+      expect(described_class.new(**variable) == variable).to be true
+    end
+  end
+
+  describe '#to_hash' do
+    it 'returns a hash representation of a collection item' do
+      expect(described_class.new(**variable).to_hash).to eq variable
+    end
+  end
+end
diff --git a/spec/lib/gitlab/ci/variables/collection_spec.rb b/spec/lib/gitlab/ci/variables/collection_spec.rb
index 16c23ae450a..c4b2df7dede 100644
--- a/spec/lib/gitlab/ci/variables/collection_spec.rb
+++ b/spec/lib/gitlab/ci/variables/collection_spec.rb
@@ -3,10 +3,11 @@ require 'spec_helper'
 describe Gitlab::Ci::Variables::Collection do
   describe '.new' do
     it 'can be initialized with an array' do
-      variable = { key: 'SOME_VAR', value: 'Some Value' }
+      variable = { key: 'VAR', value: 'value', public: true }
+
       collection = described_class.new([variable])
 
-      expect(collection.first.to_h).to include variable
+      expect(collection.first.to_hash).to eq variable
     end
 
     it 'can be initialized without an argument' do