Merge branch '230689-remove-feature-flag-for-reference-filter' into 'master'

Remove feature flag for banzai reference filter optimization

See merge request gitlab-org/gitlab!38290

changelogs/unreleased/230689-remove-feature-flag-for-reference-filter.yml

+---
+title: Improve performance of Banzai reference filters
+merge_request: 38290
+author:
+type: performance

lib/banzai/filter/reference_filter.rb

@@ -25,14 +25,12 @@ module Banzai
       def initialize(doc, context = nil, result = nil)
         super
 
-        if update_nodes_enabled?
-          @new_nodes = {}
-          @nodes = self.result[:reference_filter_nodes]
-        end
+        @new_nodes = {}
+        @nodes = self.result[:reference_filter_nodes]
       end
 
       def call_and_update_nodes
-        update_nodes_enabled? ? with_update_nodes { call } : call
+        with_update_nodes { call }
       end
 
       # Returns a data attribute String to attach to a reference link
@@ -165,11 +163,7 @@ module Banzai
       end
 
       def replace_text_with_html(node, index, html)
-        if update_nodes_enabled?
-          replace_and_update_new_nodes(node, index, html)
-        else
-          node.replace(html)
-        end
+        replace_and_update_new_nodes(node, index, html)
       end
 
       def replace_and_update_new_nodes(node, index, html)
@@ -209,10 +203,6 @@ module Banzai
         end
         result[:reference_filter_nodes] = nodes
       end
-
-      def update_nodes_enabled?
-        Feature.enabled?(:update_nodes_for_banzai_reference_filter, project)
-      end
     end
   end
 end

spec/lib/banzai/filter/reference_filter_spec.rb

@@ -110,20 +110,6 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
 
         expect(filter.instance_variable_get(:@new_nodes)).to eq({ index => [filter.each_node.to_a[index]] })
       end
-
-      context "with update_nodes_for_banzai_reference_filter feature flag disabled" do
-        before do
-          stub_feature_flags(update_nodes_for_banzai_reference_filter: false)
-        end
-
-        it 'does not call replace_and_update_new_nodes' do
-          expect(filter).not_to receive(:replace_and_update_new_nodes).with(filter.nodes[index], index, html)
-
-          filter.send(method_name, *args) do
-            html
-          end
-        end
-      end
     end
   end
 
@@ -198,49 +184,20 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
   end
 
   describe "#call_and_update_nodes" do
-    context "with update_nodes_for_banzai_reference_filter feature flag enabled" do
-      include_context 'new nodes'
-      let(:document) { Nokogiri::HTML.fragment('<a href="foo">foo</a>') }
-      let(:filter) { described_class.new(document, project: project) }
-
-      before do
-        stub_feature_flags(update_nodes_for_banzai_reference_filter: true)
-      end
-
-      it "updates all new nodes", :aggregate_failures do
-        filter.instance_variable_set('@nodes', nodes)
-
-        expect(filter).to receive(:call) { filter.instance_variable_set('@new_nodes', new_nodes) }
-        expect(filter).to receive(:with_update_nodes).and_call_original
-        expect(filter).to receive(:update_nodes!).and_call_original
-
-        filter.call_and_update_nodes
-
-        expect(filter.result[:reference_filter_nodes]).to eq(expected_nodes)
-      end
-    end
-
-    context "with update_nodes_for_banzai_reference_filter feature flag disabled" do
-      include_context 'new nodes'
-
-      before do
-        stub_feature_flags(update_nodes_for_banzai_reference_filter: false)
-      end
+    include_context 'new nodes'
+    let(:document) { Nokogiri::HTML.fragment('<a href="foo">foo</a>') }
+    let(:filter) { described_class.new(document, project: project) }
 
-      it "does not change nodes", :aggregate_failures do
-        document = Nokogiri::HTML.fragment('<a href="foo">foo</a>')
-        filter = described_class.new(document, project: project)
-        filter.instance_variable_set('@nodes', nodes)
+    it "updates all new nodes", :aggregate_failures do
+      filter.instance_variable_set('@nodes', nodes)
 
-        expect(filter).to receive(:call) { filter.instance_variable_set('@new_nodes', new_nodes) }
-        expect(filter).not_to receive(:with_update_nodes)
-        expect(filter).not_to receive(:update_nodes!)
+      expect(filter).to receive(:call) { filter.instance_variable_set('@new_nodes', new_nodes) }
+      expect(filter).to receive(:with_update_nodes).and_call_original
+      expect(filter).to receive(:update_nodes!).and_call_original
 
-        filter.call_and_update_nodes
+      filter.call_and_update_nodes
 
-        expect(filter.nodes).to eq(nodes)
-        expect(filter.result[:reference_filter_nodes]).to be nil
-      end
+      expect(filter.result[:reference_filter_nodes]).to eq(expected_nodes)
     end
   end
 
@@ -251,10 +208,6 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
 
     let(:result) { { reference_filter_nodes: nodes } }
 
-    before do
-      stub_feature_flags(update_nodes_for_banzai_reference_filter: true)
-    end
-
     it "updates all nodes", :aggregate_failures do
       expect_next_instance_of(described_class) do |filter|
         expect(filter).to receive(:call_and_update_nodes).and_call_original
@@ -267,26 +220,5 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
 
       expect(result[:reference_filter_nodes]).to eq(expected_nodes)
     end
-
-    context "with update_nodes_for_banzai_reference_filter feature flag disabled" do
-      let(:result) { {} }
-
-      before do
-        stub_feature_flags(update_nodes_for_banzai_reference_filter: false)
-      end
-
-      it "updates all nodes", :aggregate_failures do
-        expect_next_instance_of(described_class) do |filter|
-          expect(filter).to receive(:call_and_update_nodes).and_call_original
-          expect(filter).not_to receive(:with_update_nodes)
-          expect(filter).to receive(:call) { filter.instance_variable_set('@new_nodes', new_nodes) }
-          expect(filter).not_to receive(:update_nodes!)
-        end
-
-        described_class.call(document, { project: project }, result)
-
-        expect(result[:reference_filter_nodes]).to be nil
-      end
-    end
   end
 end

spec/lib/banzai/pipeline/gfm_pipeline_spec.rb

@@ -30,34 +30,6 @@ RSpec.describe Banzai::Pipeline::GfmPipeline do
         described_class.call(markdown, project: project)
       end
 
-      context "with update_nodes_for_banzai_reference_filter feature flag disabled" do
-        before do
-          stub_feature_flags(update_nodes_for_banzai_reference_filter: false)
-        end
-
-        context 'when shorthand pattern #ISSUE_ID is used' do
-          it 'links an internal issues and doesnt store nodes in result[:reference_filter_nodes]', :aggregate_failures do
-            issue = create(:issue, project: project)
-            markdown = "text #{issue.to_reference(project, full: true)}"
-            result = described_class.call(markdown, project: project)
-            link = result[:output].css('a').first
-
-            expect(link['href']).to eq(Gitlab::Routing.url_helpers.project_issue_path(project, issue))
-            expect(result[:reference_filter_nodes]).to eq nil
-          end
-        end
-
-        it 'execute :each_node for each reference_filter', :aggregate_failures do
-          issue = create(:issue, project: project)
-          markdown = "text #{issue.to_reference(project, full: true)}"
-          described_class.reference_filters do |reference_filter|
-            expect_any_instance_of(reference_filter).to receive(:each_node).once
-          end
-
-          described_class.call(markdown, project: project)
-        end
-      end
-
       context 'when shorthand pattern #ISSUE_ID is used' do
         it 'links an internal issue  if it exists' do
           issue = create(:issue, project: project)