Commit e3dbf07a authored by Alishan Ladhani's avatar Alishan Ladhani

Expose Deployment Approvals to internal Environments API

- Fix N+1 issue with Deployment#last?
- Optimize Deployment#pending_approval_count so it doesn't
run a COUNT(*) query for each deployment
- Update EnvironmentSerializer N+1 query threshold
parent 1f0de0b8
...@@ -34,8 +34,16 @@ module Preloaders ...@@ -34,8 +34,16 @@ module Preloaders
deployments_by_environment_id = deployments.index_by(&:environment_id) deployments_by_environment_id = deployments.index_by(&:environment_id)
environments.each do |environment| environments.each do |environment|
environment.association(association_name).target = deployments_by_environment_id[environment.id] associated_deployment = deployments_by_environment_id[environment.id]
environment.association(association_name).target = associated_deployment
environment.association(association_name).loaded! environment.association(association_name).loaded!
if associated_deployment
# `last?` in DeploymentEntity requires this environment to be loaded
associated_deployment.association(:environment).target = environment
associated_deployment.association(:environment).loaded!
end
end end
end end
end end
......
...@@ -34,7 +34,7 @@ module EE ...@@ -34,7 +34,7 @@ module EE
def pending_approval_count def pending_approval_count
return 0 unless blocked? return 0 unless blocked?
environment.required_approval_count - approvals.count environment.required_approval_count - approvals.length
end end
end end
end end
...@@ -6,6 +6,7 @@ module EE ...@@ -6,6 +6,7 @@ module EE
prepended do prepended do
expose :pending_approval_count expose :pending_approval_count
expose :approvals, using: ::API::Entities::Deployments::Approval
end end
end end
end end
...@@ -10,6 +10,13 @@ module EE ...@@ -10,6 +10,13 @@ module EE
super.deep_merge(latest_opened_most_severe_alert: []) super.deep_merge(latest_opened_most_severe_alert: [])
end end
override :deployment_associations
def deployment_associations
super.deep_merge(approvals: {
user: []
})
end
override :project_associations override :project_associations
def project_associations def project_associations
super.deep_merge(protected_environments: []) super.deep_merge(protected_environments: [])
......
...@@ -69,6 +69,17 @@ RSpec.describe Deployment do ...@@ -69,6 +69,17 @@ RSpec.describe Deployment do
expect(deployment.pending_approval_count).to eq(0) expect(deployment.pending_approval_count).to eq(0)
end end
end end
context 'loading approval count' do
before do
deployment.environment.required_approval_count
deployment.approvals.to_a
end
it 'does not perform an extra query when approvals are loaded', :request_store do
expect { deployment.pending_approval_count }.not_to exceed_query_limit(0)
end
end
end end
context 'when Protected Environments feature is not available' do context 'when Protected Environments feature is not available' do
......
...@@ -21,4 +21,10 @@ RSpec.describe DeploymentEntity do ...@@ -21,4 +21,10 @@ RSpec.describe DeploymentEntity do
expect(subject[:pending_approval_count]).to eq(2) expect(subject[:pending_approval_count]).to eq(2)
end end
end end
describe '#approvals' do
it 'exposes approvals' do
expect(subject[:approvals].length).to eq(1)
end
end
end end
...@@ -19,8 +19,10 @@ RSpec.describe EE::EnvironmentSerializer do ...@@ -19,8 +19,10 @@ RSpec.describe EE::EnvironmentSerializer do
def create_environment_with_associations(project) def create_environment_with_associations(project)
create(:environment, project: project).tap do |environment| create(:environment, project: project).tap do |environment|
create(:deployment, :success, environment: environment, project: project) create(:deployment, :success, environment: environment, project: project)
create(:deployment, :running, environment: environment, project: project) create(:deployment, :blocked, environment: environment, project: project) do |deployment|
create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project) create(:deployment_approval, deployment: deployment)
end
create(:protected_environment, :maintainers_can_deploy, name: environment.name, project: project, required_approval_count: 2)
prometheus_alert = create(:prometheus_alert, project: project, environment: environment) prometheus_alert = create(:prometheus_alert, project: project, environment: environment)
create(:alert_management_alert, :triggered, :prometheus, project: project, environment: environment, prometheus_alert: prometheus_alert) create(:alert_management_alert, :triggered, :prometheus, project: project, environment: environment, prometheus_alert: prometheus_alert)
end end
......
...@@ -62,4 +62,22 @@ RSpec.describe Preloaders::Environments::DeploymentPreloader do ...@@ -62,4 +62,22 @@ RSpec.describe Preloaders::Environments::DeploymentPreloader do
expect(default_preload_query).to be(false) expect(default_preload_query).to be(false)
end end
it 'sets environment on the associated deployment', :aggregate_failures do
preload_association(:last_deployment)
expect do
project.environments.each { |environment| environment.last_deployment.environment }
end.not_to exceed_query_limit(0)
project.environments.each do |environment|
expect(environment.last_deployment.environment).to eq(environment)
end
end
it 'does not attempt to set environment on a nil deployment' do
create(:environment, project: project, state: :available)
expect { preload_association(:last_deployment) }.not_to raise_error
end
end end
# frozen_string_literal: true # frozen_string_literal: true
RSpec.shared_examples 'avoid N+1 on environments serialization' do |ee: false| RSpec.shared_examples 'avoid N+1 on environments serialization' do |ee: false|
# Investigating in https://gitlab.com/gitlab-org/gitlab/-/issues/353209 # Investigating in https://gitlab.com/gitlab-org/gitlab/-/issues/353209
let(:query_threshold) { 9 + (ee ? 4 : 0) } let(:query_threshold) { 1 + (ee ? 4 : 0) }
it 'avoids N+1 database queries with grouping', :request_store do it 'avoids N+1 database queries with grouping', :request_store do
create_environment_with_associations(project) create_environment_with_associations(project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment