From 3e831453d606a3d2eb82fddaa142f9ce52f0ac76 Mon Sep 17 00:00:00 2001 From: Jan Provaznik Date: Thu, 12 Apr 2018 11:37:01 +0200 Subject: [PATCH] Fix reference filters in group context Since !18150 abstract reference filter supports also `group` as a resource parent (not only `project`). Some reference filters, which inherit from abstract filter, depend on parent being `project` when checking a reference. This patch adds explicit check of parent class to the given filters. --- .../fix-references-in-group-context.yml | 5 ++++ .../filter/abstract_reference_filter.rb | 26 +++++++++---------- .../filter/commit_range_reference_filter.rb | 2 ++ lib/banzai/filter/commit_reference_filter.rb | 2 ++ lib/banzai/filter/label_reference_filter.rb | 4 +-- .../filter/milestone_reference_filter.rb | 6 ++++- lib/banzai/filter/snippet_reference_filter.rb | 2 ++ .../reference_parser/commit_range_parser.rb | 2 ++ .../commit_range_reference_filter_spec.rb | 16 ++++++++++++ .../filter/commit_reference_filter_spec.rb | 16 ++++++++++++ .../filter/milestone_reference_filter_spec.rb | 12 +++++++-- .../filter/snippet_reference_filter_spec.rb | 6 +++++ .../commit_range_parser_spec.rb | 14 +++++++--- 13 files changed, 91 insertions(+), 22 deletions(-) create mode 100644 changelogs/unreleased/fix-references-in-group-context.yml diff --git a/changelogs/unreleased/fix-references-in-group-context.yml b/changelogs/unreleased/fix-references-in-group-context.yml new file mode 100644 index 00000000000..b436c2089ed --- /dev/null +++ b/changelogs/unreleased/fix-references-in-group-context.yml @@ -0,0 +1,5 @@ +--- +title: Ignore project internal references in group context +merge_request: +author: +type: fixed diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index a848154b2d4..60a12dca9d3 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -56,29 +56,29 @@ module Banzai # Implement in child class # Example: project.merge_requests.find - def find_object(project, id) + def find_object(parent_object, id) end # Override if the link reference pattern produces a different ID (global # ID vs internal ID, for instance) to the regular reference pattern. - def find_object_from_link(project, id) - find_object(project, id) + def find_object_from_link(parent_object, id) + find_object(parent_object, id) end # Implement in child class # Example: project_merge_request_url - def url_for_object(object, project) + def url_for_object(object, parent_object) end - def find_object_cached(project, id) - cached_call(:banzai_find_object, id, path: [object_class, project.id]) do - find_object(project, id) + def find_object_cached(parent_object, id) + cached_call(:banzai_find_object, id, path: [object_class, parent_object.id]) do + find_object(parent_object, id) end end - def find_object_from_link_cached(project, id) - cached_call(:banzai_find_object_from_link, id, path: [object_class, project.id]) do - find_object_from_link(project, id) + def find_object_from_link_cached(parent_object, id) + cached_call(:banzai_find_object_from_link, id, path: [object_class, parent_object.id]) do + find_object_from_link(parent_object, id) end end @@ -88,9 +88,9 @@ module Banzai end end - def url_for_object_cached(object, project) - cached_call(:banzai_url_for_object, object, path: [object_class, project.id]) do - url_for_object(object, project) + def url_for_object_cached(object, parent_object) + cached_call(:banzai_url_for_object, object, path: [object_class, parent_object.id]) do + url_for_object(object, parent_object) end end diff --git a/lib/banzai/filter/commit_range_reference_filter.rb b/lib/banzai/filter/commit_range_reference_filter.rb index 99fa2d9d8fb..01b3b0dafb9 100644 --- a/lib/banzai/filter/commit_range_reference_filter.rb +++ b/lib/banzai/filter/commit_range_reference_filter.rb @@ -23,6 +23,8 @@ module Banzai end def find_object(project, id) + return unless project.is_a?(Project) + range = CommitRange.new(id, project) range.valid_commits? ? range : nil diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index 43bf4fc6565..8cd92a1adba 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -17,6 +17,8 @@ module Banzai end def find_object(project, id) + return unless project.is_a?(Project) + if project && project.valid_repo? # n+1: https://gitlab.com/gitlab-org/gitlab-ce/issues/43894 Gitlab::GitalyClient.allow_n_plus_1_calls { project.commit(id) } diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 1cbada818fb..a5f38046a43 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -8,8 +8,8 @@ module Banzai Label end - def find_object(project, id) - find_labels(project).find(id) + def find_object(parent_object, id) + find_labels(parent_object).find(id) end def self.references_in(text, pattern = Label.reference_pattern) diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index 1a1d7dbeb3d..b144bd8cf54 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -12,10 +12,14 @@ module Banzai # 'regular' references, we need to use the global ID to disambiguate # between group and project milestones. def find_object(project, id) + return unless project.is_a?(Project) + find_milestone_with_finder(project, id: id) end def find_object_from_link(project, iid) + return unless project.is_a?(Project) + find_milestone_with_finder(project, iid: iid) end @@ -40,7 +44,7 @@ module Banzai project_path = full_project_path(namespace_ref, project_ref) project = parent_from_ref(project_path) - return unless project + return unless project && project.is_a?(Project) milestone_params = milestone_params(milestone_id, milestone_name) diff --git a/lib/banzai/filter/snippet_reference_filter.rb b/lib/banzai/filter/snippet_reference_filter.rb index 134a192c22b..881e10afb9f 100644 --- a/lib/banzai/filter/snippet_reference_filter.rb +++ b/lib/banzai/filter/snippet_reference_filter.rb @@ -12,6 +12,8 @@ module Banzai end def find_object(project, id) + return unless project.is_a?(Project) + project.snippets.find_by(id: id) end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb index a50e6f8ef8f..2920e886938 100644 --- a/lib/banzai/reference_parser/commit_range_parser.rb +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -29,6 +29,8 @@ module Banzai end def find_object(project, id) + return unless project.is_a?(Project) + range = CommitRange.new(id, project) range.valid_commits? ? range : nil diff --git a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb index a41a28a56f1..e1af5a15371 100644 --- a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb @@ -233,4 +233,20 @@ describe Banzai::Filter::CommitRangeReferenceFilter do expect(reference_filter(act).to_html).to eq exp end end + + context 'group context' do + let(:context) { { project: nil, group: create(:group) } } + + it 'ignores internal references' do + exp = act = "See #{range.to_reference}" + + expect(reference_filter(act, context).to_html).to eq exp + end + + it 'links to a full-path reference' do + reference = "#{project.full_path}@#{commit1.short_id}...#{commit2.short_id}" + + expect(reference_filter("See #{reference}", context).css('a').first.text).to eql(reference) + end + end end diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index b18af806118..d6c9e9e4b19 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -238,4 +238,20 @@ describe Banzai::Filter::CommitReferenceFilter do expect(doc.text).to eq("See (#{commit.reference_link_text(project)} (builds).patch)") end end + + context 'group context' do + let(:context) { { project: nil, group: create(:group) } } + + it 'ignores internal references' do + exp = act = "See #{commit.id}" + + expect(reference_filter(act, context).to_html).to eq exp + end + + it 'links to a valid reference' do + act = "See #{project.full_path}@#{commit.id}" + + expect(reference_filter(act, context).css('a').first.text).to eql("#{project.full_path}@#{commit.short_id}") + end + end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index 6a9087d2e59..f8fa9b2d13d 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -343,14 +343,22 @@ describe Banzai::Filter::MilestoneReferenceFilter do end context 'group context' do + let(:context) { { project: nil, group: create(:group) } } + let(:milestone) { create(:milestone, project: project) } + it 'links to a valid reference' do - milestone = create(:milestone, project: project) reference = "#{project.full_path}%#{milestone.iid}" - result = reference_filter("See #{reference}", { project: nil, group: create(:group) } ) + result = reference_filter("See #{reference}", context) expect(result.css('a').first.attr('href')).to eq(urls.milestone_url(milestone)) end + + it 'ignores internal references' do + exp = act = "See %#{milestone.iid}" + + expect(reference_filter(act, context).to_html).to eq exp + end end context 'when milestone is open' do diff --git a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb index e068e02d4fc..21cf092428d 100644 --- a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb @@ -210,5 +210,11 @@ describe Banzai::Filter::SnippetReferenceFilter do expect(result.css('a').first.attr('href')).to eq(urls.project_snippet_url(project, snippet)) end + + it 'ignores internal references' do + exp = act = "See $#{snippet.id}" + + expect(reference_filter(act, project: nil, group: create(:group)).to_html).to eq exp + end end end diff --git a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb index 23e16fe0213..ff3b82cc482 100644 --- a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb +++ b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb @@ -107,12 +107,9 @@ describe Banzai::ReferenceParser::CommitRangeParser do describe '#find_object' do let(:range) { double(:range) } - before do - expect(CommitRange).to receive(:new).and_return(range) - end - context 'when the range has valid commits' do it 'returns the commit range' do + expect(CommitRange).to receive(:new).and_return(range) expect(range).to receive(:valid_commits?).and_return(true) expect(subject.find_object(project, '123..456')).to eq(range) @@ -121,10 +118,19 @@ describe Banzai::ReferenceParser::CommitRangeParser do context 'when the range does not have any valid commits' do it 'returns nil' do + expect(CommitRange).to receive(:new).and_return(range) expect(range).to receive(:valid_commits?).and_return(false) expect(subject.find_object(project, '123..456')).to be_nil end end + + context 'group context' do + it 'returns nil' do + group = create(:group) + + expect(subject.find_object(group, '123..456')).to be_nil + end + end end end -- GitLab