diff options
author | Nick Thomas <nick@gitlab.com> | 2019-08-13 10:05:37 +0000 |
---|---|---|
committer | Nick Thomas <nick@gitlab.com> | 2019-08-13 10:05:37 +0000 |
commit | 35994fe091b65c902c465f3159a3efe881e1df77 (patch) | |
tree | faa5725d18436209756c3a55a7a0ca72e97a6a26 | |
parent | 266211b9c4edc308d7c99ed6b80b3a1bd01ff1f3 (diff) | |
parent | dcdc04ab46f4be77db52454dc634593595ae7612 (diff) | |
download | gitlab-ce-35994fe091b65c902c465f3159a3efe881e1df77.tar.gz |
Merge branch '65963-avoid-extra-query-allowed-to-uninstall' into 'master'
Fix a performance issue in Helm#can_uninstall?
Closes #65963
See merge request gitlab-org/gitlab-ce!31751
-rw-r--r-- | app/models/clusters/applications/helm.rb | 2 | ||||
-rw-r--r-- | spec/models/clusters/applications/helm_spec.rb | 10 |
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 |