diff options
Diffstat (limited to 'spec/lib/banzai')
8 files changed, 100 insertions, 41 deletions
diff --git a/spec/lib/banzai/filter/references/label_reference_filter_spec.rb b/spec/lib/banzai/filter/references/label_reference_filter_spec.rb index db7dda96cad..b18d68c8dd4 100644 --- a/spec/lib/banzai/filter/references/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/label_reference_filter_spec.rb @@ -702,4 +702,72 @@ RSpec.describe Banzai::Filter::References::LabelReferenceFilter do expect(result.css('a').first.text).to eq "#{label.name} in #{project.full_name}" end end + + context 'checking N+1' do + let_it_be(:group) { create(:group) } + let_it_be(:group2) { create(:group) } + let_it_be(:project) { create(:project, :public, namespace: group) } + let_it_be(:project2) { create(:project, :public, namespace: group2) } + let_it_be(:project3) { create(:project, :public) } + let_it_be(:project_label) { create(:label, project: project) } + let_it_be(:project_label2) { create(:label, project: project) } + let_it_be(:project2_label) { create(:label, project: project2) } + let_it_be(:group2_label) { create(:group_label, group: group2, color: '#00ff00') } + let_it_be(:project_reference) { "#{project_label.to_reference}" } + let_it_be(:project_reference2) { "#{project_label2.to_reference}" } + let_it_be(:project2_reference) { "#{project2_label.to_reference}" } + let_it_be(:group2_reference) { "#{project2.full_path}~#{group2_label.name}" } + + it 'does not have N+1 per multiple references per project', :use_sql_query_cache do + markdown = "#{project_reference}" + control_count = 1 + + expect do + reference_filter(markdown) + end.not_to exceed_all_query_limit(control_count) + + markdown = "#{project_reference} ~qwert ~werty ~ertyu ~rtyui #{project_reference2}" + + expect do + reference_filter(markdown) + end.not_to exceed_all_query_limit(control_count) + end + + it 'has N+1 for multiple unique project/group references', :use_sql_query_cache do + # reference to already loaded project, only one query + markdown = "#{project_reference}" + control_count = 1 + + expect do + reference_filter(markdown, project: project) + end.not_to exceed_all_query_limit(control_count) + + # Since we're not batching label queries across projects/groups, + # queries increase when a new project/group is added. + # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/330359 + # first reference to already loaded project (1), + # second reference requires project and namespace (2), and label (1) + markdown = "#{project_reference} #{group2_reference}" + max_count = control_count + 3 + + expect do + reference_filter(markdown) + end.not_to exceed_all_query_limit(max_count) + + # third reference to already queried project/namespace, nothing extra (no N+1 here) + markdown = "#{project_reference} #{group2_reference} #{project2_reference}" + + expect do + reference_filter(markdown) + end.not_to exceed_all_query_limit(max_count) + + # last reference needs another namespace and label query (2) + markdown = "#{project_reference} #{group2_reference} #{project2_reference} #{project3.full_path}~test_label" + max_count += 2 + + expect do + reference_filter(markdown) + end.not_to exceed_all_query_limit(max_count) + end + end end diff --git a/spec/lib/banzai/filter/references/reference_cache_spec.rb b/spec/lib/banzai/filter/references/reference_cache_spec.rb index 9e2a6f35910..c9404c381d3 100644 --- a/spec/lib/banzai/filter/references/reference_cache_spec.rb +++ b/spec/lib/banzai/filter/references/reference_cache_spec.rb @@ -55,11 +55,12 @@ RSpec.describe Banzai::Filter::References::ReferenceCache do cache_single.load_records_per_parent end.count + expect(control_count).to eq 1 + # Since this is an issue filter that is not batching issue queries # across projects, we have to account for that. - # 1 for both projects, 1 for issues in each project == 3 - # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/330359 - max_count = control_count + 1 + # 1 for original issue, 2 for second route/project, 1 for other issue + max_count = control_count + 3 expect do cache.load_references_per_parent(filter.nodes) diff --git a/spec/lib/banzai/filter/references/snippet_reference_filter_spec.rb b/spec/lib/banzai/filter/references/snippet_reference_filter_spec.rb index 7ab3b24b1c2..2e324669870 100644 --- a/spec/lib/banzai/filter/references/snippet_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/references/snippet_reference_filter_spec.rb @@ -233,13 +233,15 @@ RSpec.describe Banzai::Filter::References::SnippetReferenceFilter do reference_filter(markdown) end.count + expect(control_count).to eq 1 + markdown = "#{reference} $9999990 $9999991 $9999992 $9999993 #{reference2} something/cool$12" # Since we're not batching snippet queries across projects, # we have to account for that. # 1 for both projects, 1 for snippets in each project == 3 # TODO: https://gitlab.com/gitlab-org/gitlab/-/issues/330359 - max_count = control_count + 1 + max_count = control_count + 2 expect do reference_filter(markdown) diff --git a/spec/lib/banzai/pipeline/plain_markdown_pipeline_spec.rb b/spec/lib/banzai/pipeline/plain_markdown_pipeline_spec.rb index 5f31ad0c8f6..4903f624469 100644 --- a/spec/lib/banzai/pipeline/plain_markdown_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/plain_markdown_pipeline_spec.rb @@ -17,20 +17,6 @@ RSpec.describe Banzai::Pipeline::PlainMarkdownPipeline do result end - context 'when feature flag honor_escaped_markdown is disabled' do - before do - stub_feature_flags(honor_escaped_markdown: false) - end - - it 'does not escape the markdown' do - result = described_class.call(%q(\!), project: project) - output = result[:output].to_html - - expect(output).to eq('<p data-sourcepos="1:1-1:2">!</p>') - expect(result[:escaped_literals]).to be_falsey - end - end - describe 'CommonMark tests', :aggregate_failures do it 'converts all reference punctuation to literals' do reference_chars = Banzai::Filter::MarkdownPreEscapeFilter::REFERENCE_CHARACTERS diff --git a/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb b/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb index 55038d58f22..e8df395564a 100644 --- a/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb +++ b/spec/lib/banzai/pipeline/post_process_pipeline_spec.rb @@ -44,18 +44,5 @@ RSpec.describe Banzai::Pipeline::PostProcessPipeline do subject end - - context 'when "optimize_linkable_attributes" is disabled' do - before do - stub_feature_flags(optimize_linkable_attributes: false) - end - - it 'searches for attributes twice' do - expect(doc).to receive(:xpath).exactly(non_related_xpath_calls + 2).times - .and_call_original - - subject - end - end end end diff --git a/spec/lib/banzai/reference_parser/commit_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_parser_spec.rb index 612ce6b93f1..31cece108bf 100644 --- a/spec/lib/banzai/reference_parser/commit_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/commit_parser_spec.rb @@ -130,11 +130,11 @@ RSpec.describe Banzai::ReferenceParser::CommitParser do end context 'when checking commits on another projects' do - let(:control_links) do + let!(:control_links) do [commit_link] end - let(:actual_links) do + let!(:actual_links) do control_links + [commit_link, commit_link] end diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb index 76f13e7b3aa..7de78710d34 100644 --- a/spec/lib/banzai/reference_parser/issue_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -5,9 +5,11 @@ require 'spec_helper' RSpec.describe Banzai::ReferenceParser::IssueParser do include ReferenceParserHelpers - let(:project) { create(:project, :public) } - let(:user) { create(:user) } - let(:issue) { create(:issue, project: project) } + let_it_be(:group) { create(:group, :public) } + let_it_be(:project) { create(:project, :public, group: group) } + let_it_be(:user) { create(:user) } + let_it_be(:issue) { create(:issue, project: project) } + let(:link) { empty_html_link } subject { described_class.new(Banzai::RenderContext.new(project, user)) } @@ -121,7 +123,7 @@ RSpec.describe Banzai::ReferenceParser::IssueParser do end end - context 'when checking multiple merge requests on another project' do + context 'when checking multiple issues on another project' do let(:other_project) { create(:project, :public) } let(:other_issue) { create(:issue, project: other_project) } diff --git a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb index 1820141c898..04c35c8b082 100644 --- a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb @@ -5,9 +5,11 @@ require 'spec_helper' RSpec.describe Banzai::ReferenceParser::MergeRequestParser do include ReferenceParserHelpers + let(:group) { create(:group, :public) } + let(:project) { create(:project, :public, group: group) } let(:user) { create(:user) } - let(:project) { create(:project, :public) } let(:merge_request) { create(:merge_request, source_project: project) } + subject(:parser) { described_class.new(Banzai::RenderContext.new(merge_request.target_project, user)) } let(:link) { empty_html_link } @@ -16,10 +18,19 @@ RSpec.describe Banzai::ReferenceParser::MergeRequestParser do context 'when the link has a data-issue attribute' do before do project.update_attribute(:visibility_level, Gitlab::VisibilityLevel::PUBLIC) + link['data-project'] = merge_request.project_id.to_s link['data-merge-request'] = merge_request.id.to_s end it_behaves_like "referenced feature visibility", "merge_requests" + + context 'when optimize_merge_request_parser feature flag is off' do + before do + stub_feature_flags(optimize_merge_request_parser: false) + end + + it_behaves_like "referenced feature visibility", "merge_requests" + end end end @@ -27,6 +38,7 @@ RSpec.describe Banzai::ReferenceParser::MergeRequestParser do describe 'when the link has a data-merge-request attribute' do context 'using an existing merge request ID' do it 'returns an Array of merge requests' do + link['data-project'] = merge_request.project_id.to_s link['data-merge-request'] = merge_request.id.to_s expect(subject.referenced_by([link])).to eq([merge_request]) @@ -35,6 +47,7 @@ RSpec.describe Banzai::ReferenceParser::MergeRequestParser do context 'using a non-existing merge request ID' do it 'returns an empty Array' do + link['data-project'] = merge_request.project_id.to_s link['data-merge-request'] = '' expect(subject.referenced_by([link])).to eq([]) @@ -47,16 +60,16 @@ RSpec.describe Banzai::ReferenceParser::MergeRequestParser do let(:other_project) { create(:project, :public) } let(:other_merge_request) { create(:merge_request, source_project: other_project) } - let(:control_links) do + let!(:control_links) do [merge_request_link(other_merge_request)] end - let(:actual_links) do + let!(:actual_links) do control_links + [merge_request_link(create(:merge_request, :conflict, source_project: other_project))] end def merge_request_link(merge_request) - Nokogiri::HTML.fragment(%Q{<a data-merge-request="#{merge_request.id}"></a>}).children[0] + Nokogiri::HTML.fragment(%Q{<a data-project="#{merge_request.project_id}" data-merge-request="#{merge_request.id}"></a>}).children[0] end before do |