Merge branch 'use-deployment-relation-to-fetch-environment' into 'master'

Optimize slow pipelines.js response

Closes gitlab-ce#53146 and #9549

See merge request gitlab-org/gitlab-ee!9387

app/models/ci/build.rb

@@ -48,13 +48,23 @@ module Ci
     delegate :trigger_short_token, to: :trigger_request, allow_nil: true
 
     ##
-    # The "environment" field for builds is a String, and is the unexpanded name!
+    # Since Gitlab 11.5, deployments records started being created right after
+    # `ci_builds` creation. We can look up a relevant `environment` through
+    # `deployment` relation today. This is much more efficient than expanding
+    # environment name with variables.
+    # (See more https://gitlab.com/gitlab-org/gitlab-ce/merge_requests/22380)
     #
+    # However, we have to still expand environment name if it's a stop action,
+    # because `deployment` persists information for start action only.
+    #
+    # We will follow up this by persisting expanded name in build metadata or
+    # persisting stop action in database.
     def persisted_environment
       return unless has_environment?
 
       strong_memoize(:persisted_environment) do
-        Environment.find_by(name: expanded_environment_name, project: project)
+        deployment&.environment ||
+          Environment.find_by(name: expanded_environment_name, project: project)
       end
     end
 

changelogs/unreleased/use-deployment-relation-to-fetch-environment-ce.yml

+---
+title: Use deployment relation to get an environment name
+merge_request: 24890
+author:
+type: performance

ee/app/policies/ee/ci/build_policy.rb

@@ -38,7 +38,7 @@ module EE
           # as evaluating `build.expanded_environment_name` is expensive.
           return true unless build.project.protected_environments_feature_available?
 
-          build.project.protected_environment_accessible_to?(build.expanded_environment_name, user)
+          build.project.protected_environment_accessible_to?(build.persisted_environment&.name, user)
         end
       end
     end

ee/changelogs/unreleased/use-deployment-relation-to-fetch-environment.yml

+---
+title: Optimize slow pipelines.js response
+merge_request: 9387
+author:
+type: performance

ee/spec/policies/ci/build_policy_spec.rb

@@ -15,6 +15,22 @@ describe Ci::BuildPolicy do
     subject { user.can?(:update_build, build) }
 
     it_behaves_like 'protected environments access'
+
+    context 'when a pipeline has manual deployment job' do
+      let!(:build) { create(:ee_ci_build, :manual, :deploy_to_production, pipeline: pipeline) }
+
+      before do
+        project.add_developer(user)
+      end
+
+      it 'does not expand environment name' do
+        allow(build.project).to receive(:protected_environments_feature_available?) { true }
+        expect(build.project).to receive(:protected_environment_accessible_to?)
+        expect(build).not_to receive(:expanded_environment_name)
+
+        subject
+      end
+    end
   end
 
   describe 'manage a web ide terminal' do

spec/models/ci/build_spec.rb

@@ -1845,6 +1845,26 @@ describe Ci::Build do
     context 'when there is no environment' do
       it { is_expected.to be_nil }
     end
+
+    context 'when build has a start environment' do
+      let(:build) { create(:ci_build, :deploy_to_production, pipeline: pipeline) }
+
+      it 'does not expand environment name' do
+        expect(build).not_to receive(:expanded_environment_name)
+
+        subject
+      end
+    end
+
+    context 'when build has a stop environment' do
+      let(:build) { create(:ci_build, :stop_review_app, pipeline: pipeline) }
+
+      it 'expands environment name' do
+        expect(build).to receive(:expanded_environment_name)
+
+        subject
+      end
+    end
   end
 
   describe '#play' do