Merge branch 'remove_feature_flags_for_diff_controller_performance' into 'master'

Remove the feature flags around improving the performance for diff_batches and metadata endpoints

See merge request gitlab-org/gitlab!36551

app/models/merge_request.rb

@@ -1117,14 +1117,8 @@ class MergeRequest < ApplicationRecord
   end
 
   def source_branch_exists?
-    if Feature.enabled?(:memoize_source_branch_merge_request, project)
-      strong_memoize(:source_branch_exists) do
-        next false unless self.source_project
-
-        self.source_project.repository.branch_exists?(self.source_branch)
-      end
-    else
-      return false unless self.source_project
+    strong_memoize(:source_branch_exists) do
+      next false unless self.source_project
 
       self.source_project.repository.branch_exists?(self.source_branch)
     end

lib/gitlab/diff/file_collection/merge_request_diff_base.rb

@@ -29,13 +29,13 @@ module Gitlab
         override :write_cache
         def write_cache
           highlight_cache.write_if_empty
-          diff_stats_cache&.write_if_empty(diff_stats_collection)
+          diff_stats_cache.write_if_empty(diff_stats_collection)
         end
 
         override :clear_cache
         def clear_cache
           highlight_cache.clear
-          diff_stats_cache&.clear
+          diff_stats_cache.clear
         end
 
         def real_size
@@ -52,9 +52,7 @@ module Gitlab
 
         def diff_stats_cache
           strong_memoize(:diff_stats_cache) do
-            if Feature.enabled?(:cache_diff_stats_merge_request, project)
-              Gitlab::Diff::StatsCache.new(cachable_key: @merge_request_diff.cache_key)
-            end
+            Gitlab::Diff::StatsCache.new(cachable_key: @merge_request_diff.cache_key)
           end
         end
 
@@ -63,7 +61,7 @@ module Gitlab
           strong_memoize(:diff_stats) do
             next unless fetch_diff_stats?
 
-            diff_stats_cache&.read || super
+            diff_stats_cache.read || super
           end
         end
       end

spec/models/merge_request_spec.rb

@@ -1248,51 +1248,21 @@ RSpec.describe MergeRequest do
     let(:merge_request) { subject }
     let(:repository) { merge_request.source_project.repository }
 
-    context 'when memoize_source_branch_merge_request feature is enabled' do
-      before do
-        stub_feature_flags(memoize_source_branch_merge_request: true)
-      end
-
-      context 'when the source project is set' do
-        it 'memoizes the value and returns the result' do
-          expect(repository).to receive(:branch_exists?).once.with(merge_request.source_branch).and_return(true)
-
-          2.times { expect(merge_request.source_branch_exists?).to eq(true) }
-        end
-      end
+    context 'when the source project is set' do
+      it 'memoizes the value and returns the result' do
+        expect(repository).to receive(:branch_exists?).once.with(merge_request.source_branch).and_return(true)
 
-      context 'when the source project is not set' do
-        before do
-          merge_request.source_project = nil
-        end
-
-        it 'returns false' do
-          expect(merge_request.source_branch_exists?).to eq(false)
-        end
+        2.times { expect(merge_request.source_branch_exists?).to eq(true) }
       end
     end
 
-    context 'when memoize_source_branch_merge_request feature is disabled' do
+    context 'when the source project is not set' do
       before do
-        stub_feature_flags(memoize_source_branch_merge_request: false)
+        merge_request.source_project = nil
       end
 
-      context 'when the source project is set' do
-        it 'does not memoize the value and returns the result' do
-          expect(repository).to receive(:branch_exists?).twice.with(merge_request.source_branch).and_return(true)
-
-          2.times { expect(merge_request.source_branch_exists?).to eq(true) }
-        end
-      end
-
-      context 'when the source project is not set' do
-        before do
-          merge_request.source_project = nil
-        end
-
-        it 'returns false' do
-          expect(merge_request.source_branch_exists?).to eq(false)
-        end
+      it 'returns false' do
+        expect(merge_request.source_branch_exists?).to eq(false)
       end
     end
   end

spec/support/shared_examples/lib/gitlab/diff_file_collections_shared_examples.rb

@@ -67,77 +67,51 @@ RSpec.shared_examples 'cacheable diff collection' do
   end
 
   describe '#write_cache' do
+    before do
+      expect(Gitlab::Diff::StatsCache).to receive(:new).with(cachable_key: diffable.cache_key) { stats_cache }
+    end
+
     it 'calls Gitlab::Diff::HighlightCache#write_if_empty' do
       expect(highlight_cache).to receive(:write_if_empty).once
 
       subject.write_cache
     end
 
-    context 'when the feature flag is enabled' do
-      before do
-        stub_feature_flags(cache_diff_stats_merge_request: true)
-        expect(Gitlab::Diff::StatsCache).to receive(:new).with(cachable_key: diffable.cache_key) { stats_cache }
-      end
-
-      it 'calls Gitlab::Diff::StatsCache#write_if_empty with diff stats' do
-        diff_stats = Gitlab::Git::DiffStatsCollection.new([])
-
-        expect(diffable.project.repository)
-          .to receive(:diff_stats).and_return(diff_stats)
-
-        expect(stats_cache).to receive(:write_if_empty).once.with(diff_stats)
-
-        subject.write_cache
-      end
-    end
+    it 'calls Gitlab::Diff::StatsCache#write_if_empty with diff stats' do
+      diff_stats = Gitlab::Git::DiffStatsCollection.new([])
 
-    context 'when the feature flag is disabled' do
-      before do
-        stub_feature_flags(cache_diff_stats_merge_request: false)
-      end
+      expect(diffable.project.repository)
+        .to receive(:diff_stats).and_return(diff_stats)
 
-      it 'does not call Gitlab::Diff::StatsCache#write_if_empty' do
-        expect(stats_cache).not_to receive(:write_if_empty)
+      expect(stats_cache).to receive(:write_if_empty).once.with(diff_stats)
 
-        subject.write_cache
-      end
+      subject.write_cache
     end
   end
 
   describe '#clear_cache' do
+    before do
+      expect(Gitlab::Diff::StatsCache).to receive(:new).with(cachable_key: diffable.cache_key) { stats_cache }
+    end
+
     it 'calls Gitlab::Diff::HighlightCache#clear' do
       expect(highlight_cache).to receive(:clear).once
 
       subject.clear_cache
     end
 
-    context 'when the feature flag is enabled' do
-      before do
-        stub_feature_flags(cache_diff_stats_merge_request: true)
-        expect(Gitlab::Diff::StatsCache).to receive(:new).with(cachable_key: diffable.cache_key) { stats_cache }
-      end
-
-      it 'calls Gitlab::Diff::StatsCache#clear' do
-        expect(stats_cache).to receive(:clear).once
-
-        subject.clear_cache
-      end
-    end
-
-    context 'when the feature flag is disabled' do
-      before do
-        stub_feature_flags(cache_diff_stats_merge_request: false)
-      end
-
-      it 'does not calls Gitlab::Diff::StatsCache#clear' do
-        expect(stats_cache).not_to receive(:clear)
+    it 'calls Gitlab::Diff::StatsCache#clear' do
+      expect(stats_cache).to receive(:clear).once
 
-        subject.clear_cache
-      end
+      subject.clear_cache
     end
   end
 
   describe '#diff_files' do
+    before do
+      expect(Gitlab::Diff::StatsCache).to receive(:new).with(cachable_key: diffable.cache_key) { stats_cache }
+    end
+
     it 'calls Gitlab::Diff::HighlightCache#decorate' do
       expect(highlight_cache).to receive(:decorate)
         .with(instance_of(Gitlab::Diff::File))
@@ -146,40 +120,19 @@ RSpec.shared_examples 'cacheable diff collection' do
       subject.diff_files
     end
 
-    context 'when the feature swtich is enabled' do
+    context 'when there are stats cached' do
       before do
-        stub_feature_flags(cache_diff_stats_merge_request: true)
-        expect(Gitlab::Diff::StatsCache).to receive(:new).with(cachable_key: diffable.cache_key) { stats_cache }
-      end
-
-      context 'when there are stats cached' do
-        before do
-          allow(stats_cache).to receive(:read).and_return(Gitlab::Git::DiffStatsCollection.new([]))
-        end
-
-        it 'does not make a diff stats rpc call' do
-          expect(diffable.project.repository).not_to receive(:diff_stats)
-
-          subject.diff_files
-        end
+        allow(stats_cache).to receive(:read).and_return(Gitlab::Git::DiffStatsCollection.new([]))
       end
 
-      context 'when there are no stats cached' do
-        it 'makes a diff stats rpc call' do
-          expect(diffable.project.repository)
-            .to receive(:diff_stats)
-            .with(diffable.diff_refs.base_sha, diffable.diff_refs.head_sha)
+      it 'does not make a diff stats rpc call' do
+        expect(diffable.project.repository).not_to receive(:diff_stats)
 
-          subject.diff_files
-        end
+        subject.diff_files
       end
     end
 
-    context 'when the feature switch is disabled' do
-      before do
-        stub_feature_flags(cache_diff_stats_merge_request: false)
-      end
-
+    context 'when there are no stats cached' do
       it 'makes a diff stats rpc call' do
         expect(diffable.project.repository)
           .to receive(:diff_stats)