summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorDylan Griffith <dyl.griffith@gmail.com>2019-08-13 15:18:59 +1000
committerDylan Griffith <dyl.griffith@gmail.com>2019-08-13 15:18:59 +1000
commitdcdc04ab46f4be77db52454dc634593595ae7612 (patch)
tree13931d2c770d00c4e52c7a7d9287762465c95652
parentfa70d12f0372bffff12d4385d465fe4b1c8d7a8e (diff)
downloadgitlab-ce-65963-avoid-extra-query-allowed-to-uninstall.tar.gz
Fix performance issue in Helm#can_uninstall?65963-avoid-extra-query-allowed-to-uninstall
Calling #present? was causing a DB query to happen each time around the loop. We only wanted to check for nil as it's nil in the first loop around so there is no need for #present?
-rw-r--r--app/models/clusters/applications/helm.rb2
-rw-r--r--spec/models/clusters/applications/helm_spec.rb10
2 files changed, 10 insertions, 2 deletions
diff --git a/app/models/clusters/applications/helm.rb b/app/models/clusters/applications/helm.rb
index 3a175fec148..455cf200fbc 100644
--- a/app/models/clusters/applications/helm.rb
+++ b/app/models/clusters/applications/helm.rb
@@ -41,7 +41,7 @@ module Clusters
extra_apps = Clusters::Applications::Helm.where('EXISTS (?)', klass.select(1).where(cluster_id: cluster_id))
- applications = applications.present? ? applications.or(extra_apps) : extra_apps
+ applications = applications ? applications.or(extra_apps) : extra_apps
end
!applications.exists?
diff --git a/spec/models/clusters/applications/helm_spec.rb b/spec/models/clusters/applications/helm_spec.rb
index d4f8b552088..00b5c72a3d3 100644
--- a/spec/models/clusters/applications/helm_spec.rb
+++ b/spec/models/clusters/applications/helm_spec.rb
@@ -23,7 +23,7 @@ describe Clusters::Applications::Helm do
Clusters::Cluster::APPLICATIONS.keys.each do |application_name|
next if application_name == 'helm'
- it do
+ it "is false when #{application_name} is installed" do
cluster_application = create("clusters_applications_#{application_name}".to_sym)
helm = cluster_application.cluster.application_helm
@@ -31,6 +31,14 @@ describe Clusters::Applications::Helm do
expect(helm.allowed_to_uninstall?).to be_falsy
end
end
+
+ it 'executes a single query only' do
+ cluster_application = create(:clusters_applications_ingress)
+ helm = cluster_application.cluster.application_helm
+
+ query_count = ActiveRecord::QueryRecorder.new { helm.allowed_to_uninstall? }.count
+ expect(query_count).to eq(1)
+ end
end
context "without other existing applications" do