diff options
author | Thong Kuah <tkuah@gitlab.com> | 2018-12-03 14:38:20 +0300 |
---|---|---|
committer | Thong Kuah <tkuah@gitlab.com> | 2018-12-05 00:16:44 +0300 |
commit | 6c642c087bef3b7925ca5c8cf93078b58a8efe98 (patch) | |
tree | 09b57d9e7362ebf41c09961c3998d639a5b410da | |
parent | ebf87fd9c2e1c5bde72f2c08db0fff7e81882cb8 (diff) |
Eager load clusters to prevent N+1
This also means we need to apply the `current_scope` otherwise this
method will return all clusters associated with the groups regardless of
any scopes applied to this method
-rw-r--r-- | app/models/clusters/cluster.rb | 3 | ||||
-rw-r--r-- | spec/models/clusters/cluster_spec.rb | 16 |
2 files changed, 18 insertions, 1 deletions
diff --git a/app/models/clusters/cluster.rb b/app/models/clusters/cluster.rb index e7d0471813a..c9bd1728dbd 100644 --- a/app/models/clusters/cluster.rb +++ b/app/models/clusters/cluster.rb @@ -94,7 +94,8 @@ module Clusters end def self.ancestor_clusters_for_clusterable(clusterable, hierarchy_order: :asc) - hierarchy_groups = clusterable.ancestors_upto(hierarchy_order: hierarchy_order) + hierarchy_groups = clusterable.ancestors_upto(hierarchy_order: hierarchy_order).eager_load(:clusters) + hierarchy_groups = hierarchy_groups.merge(current_scope) if current_scope hierarchy_groups.flat_map(&:clusters) end diff --git a/spec/models/clusters/cluster_spec.rb b/spec/models/clusters/cluster_spec.rb index adb0ea3a444..840f74c9890 100644 --- a/spec/models/clusters/cluster_spec.rb +++ b/spec/models/clusters/cluster_spec.rb @@ -292,6 +292,22 @@ describe Clusters::Cluster do is_expected.to eq([group_cluster, sub_group_cluster]) end + it 'avoids N+1 queries' do + another_project = create(:project) + control_count = ActiveRecord::QueryRecorder.new do + described_class.ancestor_clusters_for_clusterable(another_project, hierarchy_order: hierarchy_order) + end.count + + cluster2 = create(:cluster, :provided_by_gcp, :group) + child2 = cluster2.group + child2.update!(parent: sub_group) + project = create(:project, group: child2) + + expect do + described_class.ancestor_clusters_for_clusterable(project, hierarchy_order: hierarchy_order) + end.not_to exceed_query_limit(control_count) + end + context 'for a group' do let(:clusterable) { sub_group } |