Remove preload_max_access_levels_for_labels_... FF

This change removes the preload_max_access_levels_for_labels_finder
feature flag.

Changelog: removed

app/finders/concerns/finder_with_group_hierarchy.rb

@@ -27,8 +27,7 @@ def group_ids_for(group)
       # we can preset root group for all of them to optimize permission checks
       Group.preset_root_ancestor_for(groups)
 
-      preload_associations(groups) if !skip_authorization && current_user && Feature.enabled?(
-        :preload_max_access_levels_for_labels_finder, group)
+      preload_associations(groups) if !skip_authorization && current_user
 
       groups_user_can_read_items(groups).map(&:id)
     end

app/graphql/resolvers/group_labels_resolver.rb

@@ -15,9 +15,7 @@ class GroupLabelsResolver < LabelsResolver
              default_value: false
 
     before_connection_authorization do |nodes, current_user|
-      if Feature.enabled?(:preload_max_access_levels_for_labels_finder)
-        Preloaders::LabelsPreloader.new(nodes, current_user).preload_all
-      end
+      Preloaders::LabelsPreloader.new(nodes, current_user).preload_all
     end
   end
 end

app/graphql/resolvers/labels_resolver.rb

@@ -18,9 +18,7 @@ class LabelsResolver < BaseResolver
              default_value: false
 
     before_connection_authorization do |nodes, current_user|
-      if Feature.enabled?(:preload_max_access_levels_for_labels_finder)
-        Preloaders::LabelsPreloader.new(nodes, current_user).preload_all
-      end
+      Preloaders::LabelsPreloader.new(nodes, current_user).preload_all
     end
 
     def resolve(**args)
@@ -35,8 +33,6 @@ def resolve(**args)
       # Rely on the LabelsPreloader rather than the default parent record preloading in the
       # finder because LabelsPreloader preloads more associations which are required for the
       # permission check.
-      args[:preload_parent_association] = false if Feature.disabled?(:preload_max_access_levels_for_labels_finder)
-
       LabelsFinder.new(current_user, parent_param.merge(args)).execute
     end
 

config/feature_flags/development/preload_max_access_levels_for_labels_finder.yml

----
-name: preload_max_access_levels_for_labels_finder
-introduced_by_url: https://gitlab.com/gitlab-org/gitlab/-/merge_requests/111313
-rollout_issue_url: https://gitlab.com/gitlab-org/gitlab/-/issues/390664
-milestone: '15.9'
-type: development
-group: group::optimize
-default_enabled: true

ee/spec/requests/api/graphql/group/labels_query_spec.rb

@@ -80,23 +80,5 @@ def run_query
 
       expect(titles).to eq(expected_titles)
     end
-
-    it 'does not generate N+1 queries' do
-      stub_feature_flags(preload_max_access_levels_for_labels_finder: false)
-
-      run_query # warmup
-      recorder_without_flag = Gitlab::WithRequestStore.with_request_store do
-        ActiveRecord::QueryRecorder.new(skip_cached: false) { run_query }.count
-      end
-
-      stub_feature_flags(preload_max_access_levels_for_labels_finder: true)
-
-      run_query # warmup
-      recorder_with_flag = Gitlab::WithRequestStore.with_request_store do
-        ActiveRecord::QueryRecorder.new(skip_cached: false) { run_query }
-      end
-
-      expect(recorder_with_flag).not_to exceed_query_limit(recorder_without_flag)
-    end
   end
 end

spec/finders/concerns/finder_with_group_hierarchy_spec.rb

@@ -129,80 +129,4 @@ def run_query(group)
       expect { run_query(private_group) }.not_to exceed_query_limit(control)
     end
   end
-
-  context 'when preload_max_access_levels_for_labels_finder is disabled' do
-    # All test cases were copied from above, these will be removed once the FF is removed.
-
-    before do
-      stub_feature_flags(preload_max_access_levels_for_labels_finder: false)
-    end
-
-    context 'when specifying group' do
-      it 'returns only the group by default' do
-        finder = finder_class.new(user, group: group)
-
-        expect(finder.execute).to match_array([group.id])
-      end
-    end
-
-    context 'when specifying group_id' do
-      it 'returns only the group by default' do
-        finder = finder_class.new(user, group_id: group.id)
-
-        expect(finder.execute).to match_array([group.id])
-      end
-    end
-
-    context 'when including items from group ancestors' do
-      before do
-        private_subgroup.add_developer(user)
-      end
-
-      it 'returns group and its ancestors' do
-        private_group.add_developer(user)
-
-        finder = finder_class.new(user, group: private_subgroup, include_ancestor_groups: true)
-
-        expect(finder.execute).to match_array([private_group.id, private_subgroup.id])
-      end
-
-      it 'ignores groups which user can not read' do
-        finder = finder_class.new(user, group: private_subgroup, include_ancestor_groups: true)
-
-        expect(finder.execute).to match_array([private_subgroup.id])
-      end
-
-      it 'returns them all when skip_authorization is true' do
-        finder = finder_class.new(user, group: private_subgroup, include_ancestor_groups: true)
-
-        expect(finder.execute(skip_authorization: true)).to match_array([private_group.id, private_subgroup.id])
-      end
-    end
-
-    context 'when including items from group descendants' do
-      before do
-        private_subgroup.add_developer(user)
-      end
-
-      it 'returns items from group and its descendants' do
-        private_group.add_developer(user)
-
-        finder = finder_class.new(user, group: private_group, include_descendant_groups: true)
-
-        expect(finder.execute).to match_array([private_group.id, private_subgroup.id])
-      end
-
-      it 'ignores items from groups which user can not read' do
-        finder = finder_class.new(user, group: private_group, include_descendant_groups: true)
-
-        expect(finder.execute).to match_array([private_subgroup.id])
-      end
-
-      it 'returns them all when skip_authorization is true' do
-        finder = finder_class.new(user, group: private_group, include_descendant_groups: true)
-
-        expect(finder.execute(skip_authorization: true)).to match_array([private_group.id, private_subgroup.id])
-      end
-    end
-  end
 end

spec/graphql/resolvers/group_labels_resolver_spec.rb

@@ -60,52 +60,25 @@
       before do
         group.add_developer(current_user)
 
-        stub_feature_flags(preload_max_access_levels_for_labels_finder: flag_enabled)
-
         # warmup
         resolve_labels(group, params).to_a
       end
 
-      context 'when the preload_max_access_levels_for_labels_finder FF is on' do
-        let(:flag_enabled) { true }
-
-        it 'prevents N+1 queries' do
-          control = Gitlab::WithRequestStore.with_request_store do
-            ActiveRecord::QueryRecorder.new { resolve_labels(group, params).to_a }
-          end
-
-          another_project = create(:project, :private, group: sub_subgroup)
-          another_subgroup = create(:group, :private, parent: group)
-          create(:label, project: another_project, name: 'another project feature')
-          create(:group_label, group: another_subgroup, name: 'another group feature')
-
-          expect do
-            Gitlab::WithRequestStore.with_request_store do
-              resolve_labels(group, params).to_a
-            end
-          end.not_to exceed_query_limit(control.count)
+      it 'prevents N+1 queries' do
+        control = Gitlab::WithRequestStore.with_request_store do
+          ActiveRecord::QueryRecorder.new { resolve_labels(group, params).to_a }
         end
-      end
 
-      context 'when the preload_max_access_levels_for_labels_finder FF is off' do
-        let(:flag_enabled) { false }
+        another_project = create(:project, :private, group: sub_subgroup)
+        another_subgroup = create(:group, :private, parent: group)
+        create(:label, project: another_project, name: 'another project feature')
+        create(:group_label, group: another_subgroup, name: 'another group feature')
 
-        it 'creates N+1 queries' do
-          control = Gitlab::WithRequestStore.with_request_store do
-            ActiveRecord::QueryRecorder.new { resolve_labels(group, params).to_a }
+        expect do
+          Gitlab::WithRequestStore.with_request_store do
+            resolve_labels(group, params).to_a
           end
-
-          another_project = create(:project, :private, group: sub_subgroup)
-          another_subgroup = create(:group, :private, parent: group)
-          create(:label, project: another_project, name: 'another project feature')
-          create(:group_label, group: another_subgroup, name: 'another group feature')
-
-          expect do
-            Gitlab::WithRequestStore.with_request_store do
-              resolve_labels(group, params).to_a
-            end
-          end.to exceed_query_limit(control.count)
-        end
+        end.not_to exceed_query_limit(control.count)
       end
     end
 

spec/graphql/resolvers/labels_resolver_spec.rb

@@ -60,51 +60,25 @@
       before do
         group.add_developer(current_user)
 
-        stub_feature_flags(preload_max_access_levels_for_labels_finder: flag_enabled)
-
         # warmup
         resolve_labels(project, params).to_a
       end
 
-      context 'when the preload_max_access_levels_for_labels_finder FF is on' do
-        let(:flag_enabled) { true }
-
-        it 'prevents N+1 queries' do
-          control = Gitlab::WithRequestStore.with_request_store do
-            ActiveRecord::QueryRecorder.new { resolve_labels(project, params).to_a }
-          end
-
-          another_project = create(:project, :private, group: subgroup)
-          another_subgroup = create(:group, :private, parent: group)
-          create(:label, project: another_project, name: 'another project feature')
-          create(:group_label, group: another_subgroup, name: 'another group feature')
-
-          expect do
-            Gitlab::WithRequestStore.with_request_store do
-              resolve_labels(project, params).to_a
-            end
-          end.not_to exceed_query_limit(control.count)
+      it 'prevents N+1 queries' do
+        control = Gitlab::WithRequestStore.with_request_store do
+          ActiveRecord::QueryRecorder.new { resolve_labels(project, params).to_a }
         end
-      end
 
-      context 'when the preload_max_access_levels_for_labels_finder FF is off' do
-        let(:flag_enabled) { false }
+        another_project = create(:project, :private, group: subgroup)
+        another_subgroup = create(:group, :private, parent: group)
+        create(:label, project: another_project, name: 'another project feature')
+        create(:group_label, group: another_subgroup, name: 'another group feature')
 
-        it 'creates N+1 queries' do
-          control = Gitlab::WithRequestStore.with_request_store do
-            ActiveRecord::QueryRecorder.new { resolve_labels(project, params).to_a }
+        expect do
+          Gitlab::WithRequestStore.with_request_store do
+            resolve_labels(project, params).to_a
           end
-
-          another_project = create(:project, :private, group: subgroup)
-          create(:label, project: another_project, name: 'another project feature')
-          create(:group_label, group: subgroup, name: 'another group feature')
-
-          expect do
-            Gitlab::WithRequestStore.with_request_store do
-              resolve_labels(project, params).to_a
-            end
-          end.to exceed_query_limit(control.count)
-        end
+        end.not_to exceed_query_limit(control.count)
       end
     end