Merge branch 'expand-variables-only-when-needed' into 'master'

Expand variables only when needed

See merge request gitlab-org/gitlab-ce!31772

app/models/ci/build.rb

@@ -384,7 +384,7 @@ module Ci
       return unless has_environment?
 
       strong_memoize(:expanded_environment_name) do
-        ExpandVariables.expand(environment, simple_variables)
+        ExpandVariables.expand(environment, -> { simple_variables })
       end
     end
 

app/services/update_deployment_service.rb

@@ -42,7 +42,7 @@ class UpdateDeploymentService
     return unless environment_url
 
     @expanded_environment_url =
-      ExpandVariables.expand(environment_url, variables)
+      ExpandVariables.expand(environment_url, -> { variables })
   end
 
   def environment_url

lib/expand_variables.rb

@@ -3,6 +3,20 @@
 module ExpandVariables
   class << self
     def expand(value, variables)
+      variables_hash = nil
+
+      value.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do
+        variables_hash ||= transform_variables(variables)
+        variables_hash[$1 || $2]
+      end
+    end
+
+    private
+
+    def transform_variables(variables)
+      # Lazily initialise variables
+      variables = variables.call if variables.is_a?(Proc)
+
       # Convert hash array to variables
       if variables.is_a?(Array)
         variables = variables.reduce({}) do |hash, variable|
@@ -11,9 +25,7 @@ module ExpandVariables
         end
       end
 
-      value.gsub(/\$([a-zA-Z_][a-zA-Z0-9_]*)|\${\g<1>}|%\g<1>%/) do
-        variables[$1 || $2]
-      end
+      variables
     end
   end
 end

spec/lib/expand_variables_spec.rb

@@ -4,62 +4,131 @@ require 'spec_helper'
 
 describe ExpandVariables do
   describe '#expand' do
-    subject { described_class.expand(value, variables) }
+    context 'table tests' do
+      using RSpec::Parameterized::TableSyntax
 
-    tests = [
-      { value: 'key',
-        result: 'key',
-        variables: [] },
-      { value: 'key$variable',
-        result: 'key',
-        variables: [] },
-      { value: 'key$variable',
-        result: 'keyvalue',
-        variables: [
-          { key: 'variable', value: 'value' }
-        ] },
-      { value: 'key${variable}',
-        result: 'keyvalue',
-        variables: [
-          { key: 'variable', value: 'value' }
-        ] },
-      { value: 'key$variable$variable2',
-        result: 'keyvalueresult',
-        variables: [
-          { key: 'variable', value: 'value' },
-          { key: 'variable2', value: 'result' }
-        ] },
-      { value: 'key${variable}${variable2}',
-        result: 'keyvalueresult',
-        variables: [
-          { key: 'variable', value: 'value' },
-          { key: 'variable2', value: 'result' }
-        ] },
-      { value: 'key$variable2$variable',
-        result: 'keyresultvalue',
-        variables: [
-          { key: 'variable', value: 'value' },
-          { key: 'variable2', value: 'result' }
-        ] },
-      { value: 'key${variable2}${variable}',
-        result: 'keyresultvalue',
-        variables: [
-          { key: 'variable', value: 'value' },
-          { key: 'variable2', value: 'result' }
-        ] },
-      { value: 'review/$CI_COMMIT_REF_NAME',
-        result: 'review/feature/add-review-apps',
-        variables: [
-          { key: 'CI_COMMIT_REF_NAME', value: 'feature/add-review-apps' }
-        ] }
-    ]
+      where do
+        {
+          "no expansion": {
+            value: 'key',
+            result: 'key',
+            variables: []
+          },
+          "missing variable": {
+            value: 'key$variable',
+            result: 'key',
+            variables: []
+          },
+          "simple expansion": {
+            value: 'key$variable',
+            result: 'keyvalue',
+            variables: [
+              { key: 'variable', value: 'value' }
+            ]
+          },
+          "simple with hash of variables": {
+            value: 'key$variable',
+            result: 'keyvalue',
+            variables: {
+              'variable' => 'value'
+            }
+          },
+          "complex expansion": {
+            value: 'key${variable}',
+            result: 'keyvalue',
+            variables: [
+              { key: 'variable', value: 'value' }
+            ]
+          },
+          "simple expansions": {
+            value: 'key$variable$variable2',
+            result: 'keyvalueresult',
+            variables: [
+              { key: 'variable', value: 'value' },
+              { key: 'variable2', value: 'result' }
+            ]
+          },
+          "complex expansions": {
+            value: 'key${variable}${variable2}',
+            result: 'keyvalueresult',
+            variables: [
+              { key: 'variable', value: 'value' },
+              { key: 'variable2', value: 'result' }
+            ]
+          },
+          "complex expansions with missing variable": {
+            value: 'key${variable}${variable2}',
+            result: 'keyvalue',
+            variables: [
+              { key: 'variable', value: 'value' }
+            ]
+          },
+          "out-of-order expansion": {
+            value: 'key$variable2$variable',
+            result: 'keyresultvalue',
+            variables: [
+              { key: 'variable', value: 'value' },
+              { key: 'variable2', value: 'result' }
+            ]
+          },
+          "out-of-order complex expansion": {
+            value: 'key${variable2}${variable}',
+            result: 'keyresultvalue',
+            variables: [
+              { key: 'variable', value: 'value' },
+              { key: 'variable2', value: 'result' }
+            ]
+          },
+          "review-apps expansion": {
+            value: 'review/$CI_COMMIT_REF_NAME',
+            result: 'review/feature/add-review-apps',
+            variables: [
+              { key: 'CI_COMMIT_REF_NAME', value: 'feature/add-review-apps' }
+            ]
+          },
+          "do not lazily access variables when no expansion": {
+            value: 'key',
+            result: 'key',
+            variables: -> { raise NotImplementedError }
+          },
+          "lazily access variables": {
+            value: 'key$variable',
+            result: 'keyvalue',
+            variables: -> { [{ key: 'variable', value: 'value' }] }
+          }
+        }
+      end
+
+      with_them do
+        subject { ExpandVariables.expand(value, variables) } # rubocop:disable RSpec/DescribedClass
+
+        it { is_expected.to eq(result) }
+      end
+    end
+
+    context 'lazily inits variables' do
+      let(:variables) { -> { [{ key: 'variable', value: 'result' }] } }
+
+      subject { described_class.expand(value, variables) }
+
+      context 'when expanding variable' do
+        let(:value) { 'key$variable$variable2' }
+
+        it 'calls block at most once' do
+          expect(variables).to receive(:call).once.and_call_original
+
+          is_expected.to eq('keyresult')
+        end
+      end
+
+      context 'when no expansion is needed' do
+        let(:value) { 'key' }
 
-    tests.each do |test|
-      context "#{test[:value]} resolves to #{test[:result]}" do
-        let(:value) { test[:value] }
-        let(:variables) { test[:variables] }
+        it 'does not call block' do
+          expect(variables).not_to receive(:call)
 
-        it { is_expected.to eq(test[:result]) }
+          is_expected.to eq('key')
+        end
       end
     end
   end