Commit b58707b3 authored by Douglas Barbosa Alexandre's avatar Douglas Barbosa Alexandre

Merge branch 'revert-62343ffa' into 'master'

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

See merge request gitlab-org/gitlab!38004
parents 13fefb7f 095dd343
---
title: Improve performance of Banzai reference filters
merge_request: 37465
author:
type: performance
...@@ -25,12 +25,14 @@ module Banzai ...@@ -25,12 +25,14 @@ module Banzai
def initialize(doc, context = nil, result = nil) def initialize(doc, context = nil, result = nil)
super super
@new_nodes = {} if update_nodes_enabled?
@nodes = self.result[:reference_filter_nodes] @new_nodes = {}
@nodes = self.result[:reference_filter_nodes]
end
end end
def call_and_update_nodes def call_and_update_nodes
with_update_nodes { call } update_nodes_enabled? ? with_update_nodes { call } : call
end end
# Returns a data attribute String to attach to a reference link # Returns a data attribute String to attach to a reference link
...@@ -163,7 +165,11 @@ module Banzai ...@@ -163,7 +165,11 @@ module Banzai
end end
def replace_text_with_html(node, index, html) def replace_text_with_html(node, index, html)
replace_and_update_new_nodes(node, index, html) if update_nodes_enabled?
replace_and_update_new_nodes(node, index, html)
else
node.replace(html)
end
end end
def replace_and_update_new_nodes(node, index, html) def replace_and_update_new_nodes(node, index, html)
...@@ -203,6 +209,10 @@ module Banzai ...@@ -203,6 +209,10 @@ module Banzai
end end
result[:reference_filter_nodes] = nodes result[:reference_filter_nodes] = nodes
end end
def update_nodes_enabled?
Feature.enabled?(:update_nodes_for_banzai_reference_filter, project)
end
end end
end end
end end
...@@ -110,6 +110,20 @@ RSpec.describe Banzai::Filter::ReferenceFilter do ...@@ -110,6 +110,20 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
expect(filter.instance_variable_get(:@new_nodes)).to eq({ index => [filter.each_node.to_a[index]] }) expect(filter.instance_variable_get(:@new_nodes)).to eq({ index => [filter.each_node.to_a[index]] })
end 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
end end
...@@ -184,20 +198,49 @@ RSpec.describe Banzai::Filter::ReferenceFilter do ...@@ -184,20 +198,49 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
end end
describe "#call_and_update_nodes" do describe "#call_and_update_nodes" do
include_context 'new nodes' context "with update_nodes_for_banzai_reference_filter feature flag enabled" do
let(:document) { Nokogiri::HTML.fragment('<a href="foo">foo</a>') } include_context 'new nodes'
let(:filter) { described_class.new(document, project: project) } let(:document) { Nokogiri::HTML.fragment('<a href="foo">foo</a>') }
let(:filter) { described_class.new(document, project: project) }
it "updates all new nodes", :aggregate_failures do before do
filter.instance_variable_set('@nodes', nodes) stub_feature_flags(update_nodes_for_banzai_reference_filter: true)
end
expect(filter).to receive(:call) { filter.instance_variable_set('@new_nodes', new_nodes) } it "updates all new nodes", :aggregate_failures do
expect(filter).to receive(:with_update_nodes).and_call_original filter.instance_variable_set('@nodes', nodes)
expect(filter).to receive(:update_nodes!).and_call_original
filter.call_and_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
expect(filter.result[:reference_filter_nodes]).to eq(expected_nodes) 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
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)
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!)
filter.call_and_update_nodes
expect(filter.nodes).to eq(nodes)
expect(filter.result[:reference_filter_nodes]).to be nil
end
end end
end end
...@@ -208,6 +251,10 @@ RSpec.describe Banzai::Filter::ReferenceFilter do ...@@ -208,6 +251,10 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
let(:result) { { reference_filter_nodes: nodes } } 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 it "updates all nodes", :aggregate_failures do
expect_next_instance_of(described_class) do |filter| expect_next_instance_of(described_class) do |filter|
expect(filter).to receive(:call_and_update_nodes).and_call_original expect(filter).to receive(:call_and_update_nodes).and_call_original
...@@ -220,5 +267,26 @@ RSpec.describe Banzai::Filter::ReferenceFilter do ...@@ -220,5 +267,26 @@ RSpec.describe Banzai::Filter::ReferenceFilter do
expect(result[:reference_filter_nodes]).to eq(expected_nodes) expect(result[:reference_filter_nodes]).to eq(expected_nodes)
end 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
end end
...@@ -30,6 +30,34 @@ RSpec.describe Banzai::Pipeline::GfmPipeline do ...@@ -30,6 +30,34 @@ RSpec.describe Banzai::Pipeline::GfmPipeline do
described_class.call(markdown, project: project) described_class.call(markdown, project: project)
end 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 context 'when shorthand pattern #ISSUE_ID is used' do
it 'links an internal issue if it exists' do it 'links an internal issue if it exists' do
issue = create(:issue, project: project) issue = create(:issue, project: project)
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment