From 0d3631acc16b5046ef295bf09204a565cb8580ea Mon Sep 17 00:00:00 2001 From: Lin Jen-Shin Date: Wed, 21 Jun 2017 19:53:19 +0800 Subject: Move expanded_environment_url to CreateDeploymentService Because that's the only place we need it. --- app/models/ci/build.rb | 30 +++++++-------------- app/services/create_deployment_service.rb | 13 ++++++++- spec/models/ci/build_spec.rb | 36 ------------------------- spec/services/create_deployment_service_spec.rb | 36 +++++++++++++++++++++++++ 4 files changed, 57 insertions(+), 58 deletions(-) diff --git a/app/models/ci/build.rb b/app/models/ci/build.rb index 9e38a916a2e..4454057e418 100644 --- a/app/models/ci/build.rb +++ b/app/models/ci/build.rb @@ -138,17 +138,6 @@ module Ci ExpandVariables.expand(environment, simple_variables) if environment end - def expanded_environment_url - return @expanded_environment_url if defined?(@expanded_environment_url) - - @expanded_environment_url = - if unexpanded_url = environment_url - ExpandVariables.expand(unexpanded_url, simple_variables) - else - persisted_environment&.external_url - end - end - def has_environment? environment.present? end @@ -482,15 +471,10 @@ module Ci variables = persisted_environment.predefined_variables - if url = environment_url - # Note that CI_ENVIRONMENT_URL should be the last variable, because - # here we're passing unexpanded environment_url for runner to expand, - # and the runner would expand in order. In order to make sure that - # CI_ENVIRONMENT_URL has everything available, such as variables - # from Environment#predefined_variables, we need to make sure it's - # the last variable. - variables << { key: 'CI_ENVIRONMENT_URL', value: url, public: true } - end + # Here we're passing unexpanded environment_url for runner to expand, + # and we need to make sure that CI_ENVIRONMENT_NAME and + # CI_ENVIRONMENT_SLUG so on are available for the URL be expanded. + variables << { key: 'CI_ENVIRONMENT_URL', value: environment_url, public: true } if environment_url variables end @@ -514,7 +498,11 @@ module Ci end def environment_url - options&.dig(:environment, :url) + return @environment_url if defined?(@environment_url) + + @environment_url = + options&.dig(:environment, :url) || + persisted_environment&.external_url end def build_attributes_from_config diff --git a/app/services/create_deployment_service.rb b/app/services/create_deployment_service.rb index fd8781aa48f..85500d6c9c9 100644 --- a/app/services/create_deployment_service.rb +++ b/app/services/create_deployment_service.rb @@ -2,7 +2,7 @@ class CreateDeploymentService attr_reader :job delegate :expanded_environment_name, - :expanded_environment_url, + :simple_variables, :project, to: :job @@ -50,6 +50,17 @@ class CreateDeploymentService @environment_options ||= job.options&.dig(:environment) || {} end + def expanded_environment_url + return @expanded_environment_url if defined?(@expanded_environment_url) + + @expanded_environment_url = + if unexpanded_url = environment_options[:url] + ExpandVariables.expand(unexpanded_url, simple_variables) + else + environment&.external_url + end + end + def on_stop environment_options[:on_stop] end diff --git a/spec/models/ci/build_spec.rb b/spec/models/ci/build_spec.rb index ea6a5fc48c5..77ab9a5c5ab 100644 --- a/spec/models/ci/build_spec.rb +++ b/spec/models/ci/build_spec.rb @@ -451,42 +451,6 @@ describe Ci::Build, :models do end end - describe '#expanded_environment_url' do - subject { job.expanded_environment_url } - - context 'when yaml environment uses $CI_COMMIT_REF_NAME' do - let(:job) do - create(:ci_build, - ref: 'master', - options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) - end - - it { is_expected.to eq('http://review/master') } - end - - context 'when yaml environment uses yaml_variables containing symbol keys' do - let(:job) do - create(:ci_build, - yaml_variables: [{ key: :APP_HOST, value: 'host' }], - options: { environment: { url: 'http://review/$APP_HOST' } }) - end - - it { is_expected.to eq('http://review/host') } - end - - context 'when yaml environment does not have url' do - let(:job) { create(:ci_build, environment: 'staging') } - - let!(:environment) do - create(:environment, project: job.project, name: job.environment) - end - - it 'returns the external_url from persisted environment' do - is_expected.to eq(environment.external_url) - end - end - end - describe '#starts_environment?' do subject { build.starts_environment? } diff --git a/spec/services/create_deployment_service_spec.rb b/spec/services/create_deployment_service_spec.rb index 6cf4342ad4c..790713cf2e4 100644 --- a/spec/services/create_deployment_service_spec.rb +++ b/spec/services/create_deployment_service_spec.rb @@ -122,6 +122,42 @@ describe CreateDeploymentService, services: true do end end + describe '#expanded_environment_url' do + subject { service.send(:expanded_environment_url) } + + context 'when yaml environment uses $CI_COMMIT_REF_NAME' do + let(:job) do + create(:ci_build, + ref: 'master', + options: { environment: { url: 'http://review/$CI_COMMIT_REF_NAME' } }) + end + + it { is_expected.to eq('http://review/master') } + end + + context 'when yaml environment uses yaml_variables containing symbol keys' do + let(:job) do + create(:ci_build, + yaml_variables: [{ key: :APP_HOST, value: 'host' }], + options: { environment: { url: 'http://review/$APP_HOST' } }) + end + + it { is_expected.to eq('http://review/host') } + end + + context 'when yaml environment does not have url' do + let(:job) { create(:ci_build, environment: 'staging') } + + let!(:environment) do + create(:environment, project: job.project, name: job.environment) + end + + it 'returns the external_url from persisted environment' do + is_expected.to eq(environment.external_url) + end + end + end + describe 'processing of builds' do shared_examples 'does not create deployment' do it 'does not create a new deployment' do -- cgit v1.2.1