diff --git a/app/models/deployment.rb b/app/models/deployment.rb
index 68aadf577a0af4ae5acc96f5db5cbef0064cd067..a3213a59beda95c29444be3838de352e32ea1830 100644
--- a/app/models/deployment.rb
+++ b/app/models/deployment.rb
@@ -206,11 +206,6 @@ def self.last_deployment_group_for_environment(env)
end
end
- def self.distinct_on_environment
- order('environment_id, deployments.id DESC')
- .select('DISTINCT ON (environment_id) deployments.*')
- end
-
def self.find_successful_deployment!(iid)
success.find_by!(iid: iid)
end
diff --git a/app/models/environment.rb b/app/models/environment.rb
index 02bdbd86cd13c7daf648703d326cd0d82c050570..1950431446b5b7ffc66398db3f351b601e2157d9 100644
--- a/app/models/environment.rb
+++ b/app/models/environment.rb
@@ -26,12 +26,11 @@ class Environment < ApplicationRecord
has_many :self_managed_prometheus_alert_events, inverse_of: :environment
has_many :alert_management_alerts, class_name: 'AlertManagement::Alert', inverse_of: :environment
+ # NOTE: If you preload multiple last deployments of environments, use Preloaders::Environments::DeploymentPreloader.
has_one :last_deployment, -> { success.ordered }, class_name: 'Deployment', inverse_of: :environment
- has_one :last_visible_deployment, -> { visible.distinct_on_environment }, inverse_of: :environment, class_name: 'Deployment'
- has_one :last_visible_deployable, through: :last_visible_deployment, source: 'deployable', source_type: 'CommitStatus', disable_joins: true
- has_one :last_visible_pipeline, through: :last_visible_deployable, source: 'pipeline', disable_joins: true
+ has_one :last_visible_deployment, -> { visible.order(id: :desc) }, inverse_of: :environment, class_name: 'Deployment'
- has_one :upcoming_deployment, -> { upcoming.distinct_on_environment }, class_name: 'Deployment', inverse_of: :environment
+ has_one :upcoming_deployment, -> { upcoming.order(id: :desc) }, class_name: 'Deployment', inverse_of: :environment
has_one :latest_opened_most_severe_alert, -> { order_severity_with_open_prometheus_alert }, class_name: 'AlertManagement::Alert', inverse_of: :environment
before_validation :generate_slug, if: ->(env) { env.slug.blank? }
@@ -216,28 +215,11 @@ def legacy_last_deployment_group
deployable_id: last_deployment_pipeline.latest_builds.pluck(:id))
end
- # NOTE: Below assocation overrides is a workaround for issue https://gitlab.com/gitlab-org/gitlab/-/issues/339908
- # It helps to avoid cross joins with the CI database.
- # Caveat: It also overrides and losses the default AR caching mechanism.
- # Read - https://gitlab.com/gitlab-org/gitlab/-/merge_requests/68870#note_677227727
-
- # NOTE: Association Preloads does not use the overriden definitions below.
- # Association Preloads when preloading uses the original definitions from the relationships above.
- # https://github.com/rails/rails/blob/75ac626c4e21129d8296d4206a1960563cc3d4aa/activerecord/lib/active_record/associations/preloader.rb#L158
- # But after preloading, when they are called it is using the overriden methods below.
- # So we are checking for `association_cached?(:association_name)` in the overridden methods and calling `super` which inturn fetches the preloaded values.
-
- # Overriding association
def last_visible_deployable
- return super if association_cached?(:last_visible_deployable)
-
last_visible_deployment&.deployable
end
- # Overriding association
def last_visible_pipeline
- return super if association_cached?(:last_visible_pipeline)
-
last_visible_deployable&.pipeline
end
diff --git a/ee/app/serializers/dashboard_environments_serializer.rb b/ee/app/serializers/dashboard_environments_serializer.rb
index b101af4965548a18ac9351e2e1c2765e4c035eaf..843d92582d01a7b91f62532cbbe290abfe8ea749 100644
--- a/ee/app/serializers/dashboard_environments_serializer.rb
+++ b/ee/app/serializers/dashboard_environments_serializer.rb
@@ -18,24 +18,43 @@ def batch_load(projects)
ActiveRecord::Associations::Preloader.new.preload(projects, [
:route,
environments_for_dashboard: [
- last_visible_pipeline: [
- :user,
- project: [:route, :group, :project_feature, namespace: :route]
- ],
- last_visible_deployment: [
- deployable: [
- :metadata,
- :pipeline,
- project: [:project_feature, :group, :route, namespace: :route]
- ],
- project: [:route, namespace: :route]
- ],
project: [:project_feature, :group, namespace: :route]
],
namespace: [:route, :owner]
])
+ environments = projects.map(&:environments_for_dashboard).flatten
+
+ Preloaders::Environments::DeploymentPreloader.new(environments)
+ .execute_with_union(:last_visible_deployment, deployment_associations)
+
projects
end
# rubocop: enable CodeReuse/ActiveRecord
+
+ def deployment_associations
+ {
+ deployable: {
+ metadata: nil,
+ pipeline: {
+ user: nil,
+ project: project_associations
+ },
+ project: project_associations
+ },
+ project: {
+ route: nil,
+ namespace: :route
+ }
+ }
+ end
+
+ def project_associations
+ {
+ project_feature: nil,
+ group: nil,
+ route: nil,
+ namespace: :route
+ }
+ end
end
diff --git a/spec/models/environment_spec.rb b/spec/models/environment_spec.rb
index 573e69799246020dc5e15fd7908def73fdb5f8e9..3f4372dafd0ec0f846d85550d66d560e199c9488 100644
--- a/spec/models/environment_spec.rb
+++ b/spec/models/environment_spec.rb
@@ -1025,178 +1025,29 @@
describe '#last_visible_deployable' do
subject { environment.last_visible_deployable }
- context 'does not join across databases' do
- let(:pipeline_a) { create(:ci_pipeline, project: project) }
- let(:pipeline_b) { create(:ci_pipeline, project: project) }
- let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) }
- let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) }
-
- before do
- create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b)
- end
-
- it 'for direct call' do
- with_cross_joins_prevented do
- expect(subject.id).to eq(ci_build_b.id)
- end
- end
-
- it 'for preload' do
- environment.reload
-
- with_cross_joins_prevented do
- ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_deployable: []])
- expect(subject.id).to eq(ci_build_b.id)
- end
- end
+ let!(:deployment) do
+ create(:deployment, :success, project: project, environment: environment, deployable: deployable)
end
- context 'call after preload' do
- it 'fetches from association cache' do
- pipeline = create(:ci_pipeline, project: project)
- ci_build = create(:ci_build, project: project, pipeline: pipeline)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build)
-
- environment.reload
- ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_deployable: []])
+ let!(:deployable) { create(:ci_build, :success, project: project) }
- query_count = ActiveRecord::QueryRecorder.new do
- expect(subject.id).to eq(ci_build.id)
- end.count
-
- expect(query_count).to eq(0)
- end
+ it 'fetches the deployable through the last visible deployment' do
+ is_expected.to eq(deployable)
end
end
describe '#last_visible_pipeline' do
- let(:user) { create(:user) }
- let_it_be(:project) { create(:project, :repository) }
-
- let(:environment) { create(:environment, project: project) }
- let(:commit) { project.commit }
-
- let(:success_pipeline) do
- create(:ci_pipeline, :success, project: project, user: user, sha: commit.sha)
- end
-
- let(:failed_pipeline) do
- create(:ci_pipeline, :failed, project: project, user: user, sha: commit.sha)
- end
+ subject { environment.last_visible_pipeline }
- it 'uses the last deployment even if it failed' do
- pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
- ci_build = create(:ci_build, project: project, pipeline: pipeline)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
-
- last_pipeline = environment.last_visible_pipeline
-
- expect(last_pipeline).to eq(pipeline)
- end
-
- it 'returns nil if there is no deployment' do
- create(:ci_build, project: project, pipeline: success_pipeline)
-
- expect(environment.last_visible_pipeline).to be_nil
+ let!(:deployment) do
+ create(:deployment, :success, project: project, environment: environment, deployable: deployable)
end
- it 'does not return an invisible pipeline' do
- failed_pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
- ci_build_a = create(:ci_build, project: project, pipeline: failed_pipeline)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_a, sha: commit.sha)
- pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
- ci_build_b = create(:ci_build, project: project, pipeline: pipeline)
- create(:deployment, :created, project: project, environment: environment, deployable: ci_build_b, sha: commit.sha)
+ let!(:deployable) { create(:ci_build, :success, project: project, pipeline: pipeline) }
+ let!(:pipeline) { create(:ci_pipeline, :success, project: project) }
- last_pipeline = environment.last_visible_pipeline
-
- expect(last_pipeline).to eq(failed_pipeline)
- end
-
- context 'does not join across databases' do
- let(:pipeline_a) { create(:ci_pipeline, project: project) }
- let(:pipeline_b) { create(:ci_pipeline, project: project) }
- let(:ci_build_a) { create(:ci_build, project: project, pipeline: pipeline_a) }
- let(:ci_build_b) { create(:ci_build, project: project, pipeline: pipeline_b) }
-
- before do
- create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b)
- end
-
- subject { environment.last_visible_pipeline }
-
- it 'for direct call' do
- with_cross_joins_prevented do
- expect(subject.id).to eq(pipeline_b.id)
- end
- end
-
- it 'for preload' do
- environment.reload
-
- with_cross_joins_prevented do
- ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_pipeline: []])
- expect(subject.id).to eq(pipeline_b.id)
- end
- end
- end
-
- context 'for the environment' do
- it 'returns the last pipeline' do
- pipeline = create(:ci_pipeline, project: project, user: user, sha: commit.sha)
- ci_build = create(:ci_build, project: project, pipeline: pipeline)
- create(:deployment, :success, project: project, environment: environment, deployable: ci_build, sha: commit.sha)
-
- last_pipeline = environment.last_visible_pipeline
-
- expect(last_pipeline).to eq(pipeline)
- end
-
- context 'with multiple deployments' do
- it 'returns the last pipeline' do
- pipeline_a = create(:ci_pipeline, project: project, user: user)
- pipeline_b = create(:ci_pipeline, project: project, user: user)
- ci_build_a = create(:ci_build, project: project, pipeline: pipeline_a)
- ci_build_b = create(:ci_build, project: project, pipeline: pipeline_b)
- create(:deployment, :success, project: project, environment: environment, deployable: ci_build_a)
- create(:deployment, :success, project: project, environment: environment, deployable: ci_build_b)
-
- last_pipeline = environment.last_visible_pipeline
-
- expect(last_pipeline).to eq(pipeline_b)
- end
- end
-
- context 'with multiple pipelines' do
- it 'returns the last pipeline' do
- create(:ci_build, project: project, pipeline: success_pipeline)
- ci_build_b = create(:ci_build, project: project, pipeline: failed_pipeline)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build_b, sha: commit.sha)
-
- last_pipeline = environment.last_visible_pipeline
-
- expect(last_pipeline).to eq(failed_pipeline)
- end
- end
- end
-
- context 'call after preload' do
- it 'fetches from association cache' do
- pipeline = create(:ci_pipeline, project: project)
- ci_build = create(:ci_build, project: project, pipeline: pipeline)
- create(:deployment, :failed, project: project, environment: environment, deployable: ci_build)
-
- environment.reload
- ActiveRecord::Associations::Preloader.new.preload(environment, [last_visible_pipeline: []])
-
- query_count = ActiveRecord::QueryRecorder.new do
- expect(environment.last_visible_pipeline.id).to eq(pipeline.id)
- end.count
-
- expect(query_count).to eq(0)
- end
+ it 'fetches the pipeline through the last visible deployment' do
+ is_expected.to eq(pipeline)
end
end