From 1b5b0dea5228ae7fd520c8bca3f03c4799a4d31d Mon Sep 17 00:00:00 2001 From: Thong Kuah Date: Mon, 1 Jul 2019 20:27:58 +1200 Subject: [PATCH] Share project object in EnvironmentStatus Otherwise, each EnvironmentStatus object instantiates its own project when really they are the same. Improves query count performance --- app/models/environment_status.rb | 8 ++++---- spec/models/environment_status_spec.rb | 3 +-- spec/serializers/environment_status_entity_spec.rb | 10 ++++++++-- 3 files changed, 13 insertions(+), 8 deletions(-) diff --git a/app/models/environment_status.rb b/app/models/environment_status.rb index 2fb6cadc8cd..2c71520cec1 100644 --- a/app/models/environment_status.rb +++ b/app/models/environment_status.rb @@ -3,11 +3,10 @@ class EnvironmentStatus include Gitlab::Utils::StrongMemoize - attr_reader :environment, :merge_request, :sha + attr_reader :project, :environment, :merge_request, :sha delegate :id, to: :environment delegate :name, to: :environment - delegate :project, to: :environment delegate :status, to: :deployment, allow_nil: true delegate :deployed_at, to: :deployment, allow_nil: true @@ -21,7 +20,8 @@ class EnvironmentStatus build_environments_status(mr, user, mr.merge_pipeline) end - def initialize(environment, merge_request, sha) + def initialize(project, environment, merge_request, sha) + @project = project @environment = environment @merge_request = merge_request @sha = sha @@ -67,7 +67,7 @@ class EnvironmentStatus pipeline.environments.available.map do |environment| next unless Ability.allowed?(user, :read_environment, environment) - EnvironmentStatus.new(environment, mr, pipeline.sha) + EnvironmentStatus.new(pipeline.project, environment, mr, pipeline.sha) end.compact end private_class_method :build_environments_status diff --git a/spec/models/environment_status_spec.rb b/spec/models/environment_status_spec.rb index c503c35305f..e2836420df9 100644 --- a/spec/models/environment_status_spec.rb +++ b/spec/models/environment_status_spec.rb @@ -11,11 +11,10 @@ describe EnvironmentStatus do let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } let(:sha) { deployment.sha } - subject(:environment_status) { described_class.new(environment, merge_request, sha) } + subject(:environment_status) { described_class.new(project, environment, merge_request, sha) } it { is_expected.to delegate_method(:id).to(:environment) } it { is_expected.to delegate_method(:name).to(:environment) } - it { is_expected.to delegate_method(:project).to(:environment) } it { is_expected.to delegate_method(:deployed_at).to(:deployment) } it { is_expected.to delegate_method(:status).to(:deployment) } diff --git a/spec/serializers/environment_status_entity_spec.rb b/spec/serializers/environment_status_entity_spec.rb index 8a6a38fe5f8..f421432e8d6 100644 --- a/spec/serializers/environment_status_entity_spec.rb +++ b/spec/serializers/environment_status_entity_spec.rb @@ -9,7 +9,7 @@ describe EnvironmentStatusEntity do let(:project) { deployment.project } let(:merge_request) { create(:merge_request, :deployed_review_app, deployment: deployment) } - let(:environment_status) { EnvironmentStatus.new(environment, merge_request, merge_request.diff_head_sha) } + let(:environment_status) { EnvironmentStatus.new(project, environment, merge_request, merge_request.diff_head_sha) } let(:entity) { described_class.new(environment_status, request: request) } subject { entity.as_json } @@ -55,8 +55,14 @@ describe EnvironmentStatusEntity do before do project.add_maintainer(user) allow(deployment).to receive(:prometheus_adapter).and_return(prometheus_adapter) - allow(prometheus_adapter).to receive(:query).with(:deployment, deployment).and_return(simple_metrics) allow(entity).to receive(:deployment).and_return(deployment) + + expect_next_instance_of(DeploymentMetrics) do |deployment_metrics| + allow(deployment_metrics).to receive(:prometheus_adapter).and_return(prometheus_adapter) + + allow(prometheus_adapter).to receive(:query) + .with(:deployment, deployment).and_return(simple_metrics) + end end context 'when deployment succeeded' do