Commit 465a852a authored by Alex Kalderimis's avatar Alex Kalderimis Committed by Jan Provaznik

Add reference patterns and pre-fetching scope

This supports reference filters for designs.

Add new query scopes for use in reference filter
parent c9c86caa
...@@ -407,6 +407,7 @@ GFM will recognize the following: ...@@ -407,6 +407,7 @@ GFM will recognize the following:
| merge request | `!123` | `namespace/project!123` | `project!123` | | merge request | `!123` | `namespace/project!123` | `project!123` |
| snippet | `$123` | `namespace/project$123` | `project$123` | | snippet | `$123` | `namespace/project$123` | `project$123` |
| epic **(ULTIMATE)** | `&123` | `group1/subgroup&123` | | | epic **(ULTIMATE)** | `&123` | `group1/subgroup&123` | |
| design **(PREMIUM)** | `#123[file.jpg]` or `#123["file.png"]` | `group1/subgroup#123[file.png]` | `project#123[file.png]` |
| label by ID | `~123` | `namespace/project~123` | `project~123` | | label by ID | `~123` | `namespace/project~123` | `project~123` |
| one-word label by name | `~bug` | `namespace/project~bug` | `project~bug` | | one-word label by name | `~bug` | `namespace/project~bug` | `project~bug` |
| multi-word label by name | `~"feature request"` | `namespace/project~"feature request"` | `project~"feature request"` | | multi-word label by name | `~"feature request"` | `namespace/project~"feature request"` | `project~"feature request"` |
......
...@@ -37,6 +37,13 @@ Design Management requires that projects are using ...@@ -37,6 +37,13 @@ Design Management requires that projects are using
[hashed storage](../../../administration/repository_storage_types.html#hashed-storage) [hashed storage](../../../administration/repository_storage_types.html#hashed-storage)
(the default storage type since v10.0). (the default storage type since v10.0).
### Feature Flags
- Reference Parsing
Designs support short references in Markdown, but this needs to be enabled by setting
the `:design_management_reference_filter_gfm_pipeline` feature flag.
## Limitations ## Limitations
- Files uploaded must have a file extension of either `png`, `jpg`, `jpeg`, `gif`, `bmp`, `tiff` or `ico`. - Files uploaded must have a file extension of either `png`, `jpg`, `jpeg`, `gif`, `bmp`, `tiff` or `ico`.
...@@ -137,3 +144,32 @@ Different discussions have different badge numbers: ...@@ -137,3 +144,32 @@ Different discussions have different badge numbers:
From GitLab 12.5 on, new annotations will be outputted to the issue activity, From GitLab 12.5 on, new annotations will be outputted to the issue activity,
so that everyone involved can participate in the discussion. so that everyone involved can participate in the discussion.
## References
GitLab Flavored Markdown supports references to designs. The syntax for this is:
`#123[file.jpg]` - the issue reference, with the filename in square braces
File names may contain a variety of odd characters, so two escaping mechanisms are supported:
### Quoting
File names may be quoted with double quotation marks, eg:
`#123["file.jpg"]`
This is useful if, for instance, your filename has square braces in its name. In this scheme, all
double quotation marks in the file name need to be escaped with backslashes, and backslashes need
to be escaped likewise:
`#123["with with \"quote\" marks and a backslash \\.png"]`
### Base64 Encoding
In the case of file names that include HTML elements, you will need to escape these names to avoid
them being processed as HTML literals. To do this, we support base64 encoding, eg.
The file `<a>.jpg` can be referenced as `#123[base64:PGE+LmpwZwo=]`
Obviously we would advise against using such filenames.
...@@ -26,6 +26,10 @@ module DesignManagement ...@@ -26,6 +26,10 @@ module DesignManagement
alias_attribute :title, :filename alias_attribute :title, :filename
# Pre-fetching scope to include the data necessary to construct a
# reference using `to_reference`.
scope :for_reference, -> { includes(issue: [{ project: [:route, :namespace] }]) }
# Find designs visible at the given version # Find designs visible at the given version
# #
# @param version [nil, DesignManagement::Version]: # @param version [nil, DesignManagement::Version]:
...@@ -52,6 +56,7 @@ module DesignManagement ...@@ -52,6 +56,7 @@ module DesignManagement
end end
scope :with_filename, -> (filenames) { where(filename: filenames) } scope :with_filename, -> (filenames) { where(filename: filenames) }
scope :on_issue, ->(issue) { where(issue_id: issue) }
# Scope called by our REST API to avoid N+1 problems # Scope called by our REST API to avoid N+1 problems
scope :with_api_entity_associations, -> { preload(:issue) } scope :with_api_entity_associations, -> { preload(:issue) }
...@@ -84,10 +89,68 @@ module DesignManagement ...@@ -84,10 +89,68 @@ module DesignManagement
# #123[homescreen.png] # #123[homescreen.png]
# other-project#72[sidebar.jpg] # other-project#72[sidebar.jpg]
# #38/designs[transition.gif] # #38/designs[transition.gif]
# #12["filename with [] in it.jpg"]
def to_reference(from = nil, full: false) def to_reference(from = nil, full: false)
infix = full ? '/designs' : '' infix = full ? '/designs' : ''
totally_simple = %r{ \A #{self.class.simple_file_name} \z }x
safe_name = if totally_simple.match?(filename)
filename
elsif filename =~ /[<>]/
%Q{base64:#{Base64.strict_encode64(filename)}}
else
escaped = filename.gsub(%r{[\\"]}) { |x| "\\#{x}" }
%Q{"#{escaped}"}
end
"#{issue.to_reference(from, full: full)}#{infix}[#{safe_name}]"
end
def self.reference_pattern
@reference_pattern ||= begin
# Filenames can be escaped with double quotes to name filenames
# that include square brackets, or other special characters
%r{
#{Issue.reference_pattern}
(\/designs)?
\[
(?<design> #{simple_file_name} | #{quoted_file_name} | #{base_64_encoded_name})
\]
}x
end
end
def self.simple_file_name
%r{
(?<simple_file_name>
( \w | [_:,'-] | \. | \s )+
\.
\w+
)
}x
end
def self.base_64_encoded_name
%r{
base64:
(?<base_64_encoded_name>
[A-Za-z0-9+\n]+
=?
)
}x
end
def self.quoted_file_name
%r{
"
(?<escaped_filename>
(\\ \\ | \\ " | [^"\\])+
)
"
}x
end
"%s%s[%s]" % [issue.to_reference(from, full: full), infix, filename] def self.link_reference_pattern
nil
end end
def to_ability_name def to_ability_name
......
# frozen_string_literal: true
module Banzai
module Filter
class DesignReferenceFilter < AbstractReferenceFilter
include Gitlab::Allowable
Identifier = Struct.new(:issue_iid, :filename, keyword_init: true)
# This filter must be enabled by setting the following flags:
# - design_management
# - design_management_reference_filter_gfm_pipeline
def call
return doc unless enabled?
super
end
def find_object(project, identifier)
records_per_parent[project][identifier]
end
def parent_records(project, identifiers)
return [] unless can_read_designs?(project)
iids = identifiers.map(&:issue_iid).to_set
filenames = identifiers.map(&:filename).to_set
issues = project.issues.where(iid: iids)
issue_map = issues.index_by(&:id)
designs(issues.to_a, filenames).select do |d|
issue = issue_map[d.issue_id]
# assign values we have already fetched
d.project = project
d.issue = issue
identifiers.include?(Identifier.new(filename: d.filename, issue_iid: issue.iid))
end
end
def relation_for_paths(paths)
super.includes(:route, :namespace, :group)
end
def parent_type
:project
end
# optimisation to reuse the parent_per_reference query information
def parent_from_ref(ref)
parent_per_reference[ref || current_parent_path]
end
def url_for_object(design, project)
path_options = { vueroute: design.filename }
Gitlab::Routing.url_helpers.designs_project_issue_path(project, design.issue, path_options)
end
def data_attributes_for(_text, _project, design, **_kwargs)
super.merge(issue: design.issue_id)
end
def self.object_class
::DesignManagement::Design
end
def self.object_sym
:design
end
def self.parse_symbol(raw, match_data)
filename = parse_filename(raw, match_data)
Identifier.new(filename: filename, issue_iid: match_data[:issue].to_i)
end
def self.parse_filename(raw, match_data)
if efn = match_data[:escaped_filename]
efn.gsub(/(\\ \\ | \\ ")/x) { |x| x[1] }
elsif b64_name = match_data[:base_64_encoded_name]
Base64.decode64(b64_name)
elsif name = match_data[:simple_file_name]
name
else
raise "Unexpected name format: #{raw}"
end
end
def record_identifier(design)
Identifier.new(filename: design.filename, issue_iid: design.issue.iid)
end
private
def can_read_designs?(project)
DeclarativePolicy.user_scope { can?(current_user, :read_design, project) }
end
def designs(issues, filenames)
DesignManagement::Design.on_issue(issues).with_filename(filenames)
end
def enabled?
Feature.enabled?(:design_management_reference_filter_gfm_pipeline)
end
end
end
end
...@@ -10,6 +10,7 @@ module EE ...@@ -10,6 +10,7 @@ module EE
def reference_filters def reference_filters
[ [
::Banzai::Filter::EpicReferenceFilter, ::Banzai::Filter::EpicReferenceFilter,
::Banzai::Filter::DesignReferenceFilter,
*super *super
] ]
end end
......
# frozen_string_literal: true
require 'spec_helper'
describe Banzai::Filter::DesignReferenceFilter do
include FilterSpecHelper
include DesignManagementTestHelpers
# Persistent stuff we only want to create once
let_it_be(:issue) { create(:issue) }
let_it_be(:issue_b) { create(:issue, project: issue.project) }
let_it_be(:design_a) { create(:design, :with_versions, issue: issue) }
let_it_be(:design_b) { create(:design, :with_versions, issue: issue_b) }
let_it_be(:developer) { create(:user).tap { |u| issue.project.add_developer(u) } }
let_it_be(:project2) { create(:project).tap { |p| p.add_developer(developer) } }
let_it_be(:issue2) { create(:issue, project: project2) }
let_it_be(:x_project_design) { create(:design, :with_versions, issue: issue2) }
# Transitory stuff we can compute cheaply from other things
let(:design) { design_a }
let(:project) { issue.project }
let(:reference) { design.to_reference }
let(:input_text) { "Added #{reference}" }
let(:doc) { process_doc(input_text) }
let(:current_user) { developer }
def process_doc(text)
reference_filter(text, project: project, current_user: current_user)
end
def process(text)
process_doc(text).to_html
end
before do
enable_design_management
end
shared_examples 'a no-op filter' do
it 'does nothing' do
expect(process(input_text)).to eq(input_text)
end
end
describe '.call' do
it 'requires project context' do
expect { described_class.call('') }.to raise_error(ArgumentError, /:project/)
end
end
describe '#call' do
describe 'feature flags' do
context 'design management is not enabled' do
before do
enable_design_management(false, true)
end
it_behaves_like 'a no-op filter'
end
context 'design reference filter is not enabled' do
before do
enable_design_management(true, false)
end
it_behaves_like 'a no-op filter'
end
end
end
%w(pre code a style).each do |elem|
context "wrapped in a <#{elem}/>" do
let(:input_text) { "<#{elem}>Design #{design.to_reference}</#{elem}>" }
it_behaves_like 'a no-op filter'
end
end
describe '.parse_symbol' do
where(:filename) do
[
['simple.png'],
['SIMPLE.PNG'],
['has spaces.png'],
['has-hyphen.jpg'],
['snake_case.svg'],
['has ] right bracket.gif'],
[%q{has slashes \o/.png}],
[%q{has "quote" 'marks'.gif}],
[%q{<a href="has">html elements</a>.gif}]
]
end
with_them do
where(:fullness) do
[
[true],
[false]
]
end
with_them do
let(:design) { build(:design, issue: issue, filename: filename) }
let(:reference) { design.to_reference(full: fullness) }
let(:parsed) do
m = parse(reference)
described_class.parse_symbol(m[described_class.object_sym], m) if m
end
def parse(ref)
described_class.object_class.reference_pattern.match(ref)
end
it 'can parse the reference' do
expect(parsed).to have_attributes(
filename: filename,
issue_iid: issue.iid
)
end
end
end
end
describe '.object_sym' do
let(:subject) { described_class.object_sym }
it { is_expected.to eq(:design) }
end
describe '.object_class' do
let(:subject) { described_class.object_class }
it { is_expected.to eq(::DesignManagement::Design) }
end
describe '#data_attributes_for' do
let(:subject) { filter_instance.data_attributes_for(input_text, project, design) }
it do
is_expected.to include(issue: design.issue_id,
original: input_text,
project: project.id,
design: design.id)
end
end
context 'a design with a quoted filename' do
let(:filename) { %q{A "very" good file.png} }
let(:design) { create(:design, :with_versions, issue: issue, filename: filename) }
it 'links to the design' do
expect(doc.css('a').first.attr('href'))
.to eq url_for_design(design)
end
end
context 'internal reference' do
it_behaves_like 'a reference containing an element node'
context "the reference is valid" do
context 'the user does not have permission to read this project' do
let(:current_user) { build_stubbed(:user) }
it_behaves_like 'a no-op filter'
end
context 'the user has permission' do
it 'produces a good link', :aggregate_failures do
link = doc.css('a').first
expect(link.attr('href')).to eq(url_for_design(design))
expect(link.attr('title')).to eq(design.filename)
expect(link.attr('class')).to eq('gfm gfm-design has-tooltip')
expect(link.attr('data-project')).to eq(project.id.to_s)
expect(link.attr('data-issue')).to eq(issue.id.to_s)
expect(link.attr('data-original')).to eq(reference)
end
end
context 'the filename needs to be escaped' do
let(:xss) do
<<~END
<script type="application/javascript">
alert('xss')
</script>
END
end
let(:filename) { %Q{#{xss}.png} }
let(:design) { create(:design, :with_versions, filename: filename, issue: issue) }
it 'leaves the text as is, but escapes the title', :aggregate_failures do
expect(doc.text).to eq(input_text)
expect(doc.css('a').first.attr('title')).to eq(design.filename)
end
end
end
context "the reference is to a non-existant design" do
let(:reference) { build(:design).to_reference }
it 'ignores it' do
expect(process(reference)).to eq(reference)
end
end
end
context 'cross-project / cross-namespace complete reference' do
let(:reference) { x_project_design.to_reference(project) }
it_behaves_like 'a reference containing an element node'
it 'links to a valid reference' do
expect(doc.css('a').first.attr('href'))
.to eq url_for_design(x_project_design)
end
context 'the current user does not have access to that project' do
let(:current_user) { build_stubbed(:user) }
it_behaves_like 'a no-op filter'
end
it 'link has valid text' do
expect(doc.css('a').first.text).to eql("#{project2.full_path}##{issue.iid}[#{x_project_design.filename}]")
end
it 'includes default classes' do
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-design has-tooltip'
end
context 'the reference is invalid' do
let(:reference) { x_project_design.to_reference(project).gsub(/jpg/, 'gif') }
it_behaves_like 'a no-op filter'
end
end
describe 'performance' do
let!(:design_c) { create(:design, :with_versions, issue: issue) }
let!(:design_d) { create(:design, :with_versions, issue: issue_b) }
let!(:design_e) { create(:design, :with_versions, issue: create(:issue, project: x_project_design.project)) }
it 'is linear in the number of projects each design refers to' do
one_ref_per_project = <<~MD
Design #{design_a.to_reference}, #{x_project_design.to_reference(project)}
MD
multiple_references = <<~MD
Designs:
* #{design_a.to_reference}
* #{design_b.to_reference}
* #{design_c.to_reference}
* #{design_d.to_reference}
* #{x_project_design.to_reference(project)}
* #{design_e.to_reference(project)}
* #1[not a valid reference.gif]
MD
baseline = ActiveRecord::QueryRecorder.new { process(one_ref_per_project) }
# each project where the current_user has :read_design permission requires 5 queries:
# * SELECT "project_features".* FROM "project_features"
# :in `feature_available?'*/
# * SELECT MAX("project_authorizations"."access_level") ...
# :in `block in max_member_access_for_user_ids'
# * SELECT "issues".* FROM "issues" WHERE "issues"."project_id" = 1 AND ...
# :in `parent_records'*/
# * SELECT "_designs".* FROM "_designs"
# WHERE "issue_id" IN (..) AND "filename" IN ('homescreen-1.jpg', ...)
# :in `parent_records'*/
# * SELECT "routes".* FROM "routes" WHERE "routes"."source_id" = 1 AND "routes"."source_type" = 'Namespace' LIMIT 1
# :in `full_path'*/
# nb: routes is a polymorphic association, and cannot be optimised by the
# use of `Project.includes(:route)
# and 3 if not:
# * SELECT "project_features".* FROM "project_features"
# :in `feature_available?'*/
# * SELECT MAX("project_authorizations"."access_level") ...
# :in `block in max_member_access_for_user_ids'
# * SELECT "users".* FROM "users" WHERE "users"."id" = 5 LIMIT 1 :in `owner'*/
# At this point we know that the current_user is not authorised.
#
# In addition there is a 1 query overhead for all the projects at the
# start. Currently, the baseline for 2 projects is `2 * 5 + 1 = 11` queries
#
expect { process(multiple_references) }.not_to exceed_query_limit(baseline.count)
end
end
end
...@@ -5,11 +5,11 @@ require 'spec_helper' ...@@ -5,11 +5,11 @@ require 'spec_helper'
describe DesignManagement::Design do describe DesignManagement::Design do
include DesignManagementTestHelpers include DesignManagementTestHelpers
set(:issue) { create(:issue) } let_it_be(:issue) { create(:issue) }
set(:design1) { create(:design, :with_versions, issue: issue, versions_count: 1) } let_it_be(:design1) { create(:design, :with_versions, issue: issue, versions_count: 1) }
set(:design2) { create(:design, :with_versions, issue: issue, versions_count: 1) } let_it_be(:design2) { create(:design, :with_versions, issue: issue, versions_count: 1) }
set(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) } let_it_be(:design3) { create(:design, :with_versions, issue: issue, versions_count: 1) }
set(:deleted_design) { create(:design, :with_versions, deleted: true) } let_it_be(:deleted_design) { create(:design, :with_versions, deleted: true) }
describe 'relations' do describe 'relations' do
it { is_expected.to belong_to(:project) } it { is_expected.to belong_to(:project) }
...@@ -64,12 +64,13 @@ describe DesignManagement::Design do ...@@ -64,12 +64,13 @@ describe DesignManagement::Design do
describe 'scopes' do describe 'scopes' do
describe '.visible_at_version' do describe '.visible_at_version' do
let(:versions) { DesignManagement::Version.where(issue: issue).ordered } let(:versions) { DesignManagement::Version.where(issue: issue).ordered }
let(:found) { described_class.visible_at_version(version) }
context 'at oldest version' do context 'at oldest version' do
let(:version) { versions.last } let(:version) { versions.last }
it 'finds the first design only' do it 'finds the first design only' do
expect(described_class.visible_at_version(version)).to contain_exactly(design1) expect(found).to contain_exactly(design1)
end end
end end
...@@ -77,7 +78,7 @@ describe DesignManagement::Design do ...@@ -77,7 +78,7 @@ describe DesignManagement::Design do
let(:version) { versions.second } let(:version) { versions.second }
it 'finds the first and second designs' do it 'finds the first and second designs' do
expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2) expect(found).to contain_exactly(design1, design2)
end end
end end
...@@ -85,7 +86,7 @@ describe DesignManagement::Design do ...@@ -85,7 +86,7 @@ describe DesignManagement::Design do
let(:version) { versions.first } let(:version) { versions.first }
it 'finds designs' do it 'finds designs' do
expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2, design3) expect(found).to contain_exactly(design1, design2, design3)
end end
end end
...@@ -93,7 +94,7 @@ describe DesignManagement::Design do ...@@ -93,7 +94,7 @@ describe DesignManagement::Design do
let(:version) { nil } let(:version) { nil }
it 'finds all undeleted designs' do it 'finds all undeleted designs' do
expect(described_class.visible_at_version(version)).to contain_exactly(design1, design2, design3) expect(found).to contain_exactly(design1, design2, design3)
end end
end end
...@@ -157,6 +158,18 @@ describe DesignManagement::Design do ...@@ -157,6 +158,18 @@ describe DesignManagement::Design do
end end
end end
describe '.on_issue' do
it 'returns correct designs when passed a single issue' do
expect(described_class.on_issue(issue)).to match_array(issue.designs)
end
it 'returns correct designs when passed an Array of issues' do
expect(
described_class.on_issue([issue, deleted_design.issue])
).to contain_exactly(design1, design2, design3, deleted_design)
end
end
describe '.current' do describe '.current' do
it 'returns just the undeleted designs' do it 'returns just the undeleted designs' do
delete_designs(design3) delete_designs(design3)
...@@ -370,4 +383,123 @@ describe DesignManagement::Design do ...@@ -370,4 +383,123 @@ describe DesignManagement::Design do
subject.after_note_changed(build(:note, :system)) subject.after_note_changed(build(:note, :system))
end end
end end
describe '.for_reference' do
let_it_be(:design_a) { create(:design) }
let_it_be(:design_b) { create(:design) }
it 'avoids extra queries when calling to_reference' do
designs = described_class.for_reference.where(id: [design_a.id, design_b.id]).to_a
expect { designs.map(&:to_reference) }.not_to exceed_query_limit(0)
end
end
describe '#to_reference' do
let(:namespace) { build(:namespace, path: 'sample-namespace') }
let(:project) { build(:project, name: 'sample-project', namespace: namespace) }
let(:group) { create(:group, name: 'Group', path: 'sample-group') }
let(:issue) { build(:issue, iid: 1, project: project) }
let(:filename) { 'homescreen.jpg' }
let(:design) { build(:design, filename: filename, issue: issue, project: project) }
context 'when nil argument' do
let(:reference) { design.to_reference }
it 'uses the simple format' do
expect(reference).to eq "#1[homescreen.jpg]"
end
context 'when the filename contains spaces, hyphens, periods, single-quotes, underscores and colons' do
let(:filename) { %q{a complex filename: containing - _ : etc., but still 'simple'.gif} }
it 'uses the simple format' do
expect(reference).to eq "#1[#{filename}]"
end
end
context 'when the filename contains HTML angle brackets' do
let(:filename) { 'a <em>great</em> filename.jpg' }
it 'uses Base64 encoding' do
expect(reference).to eq "#1[base64:#{Base64.strict_encode64(filename)}]"
end
end
context 'when the filename contains quotation marks' do
let(:filename) { %q{a "great" filename.jpg} }
it 'uses enclosing quotes, with backslash encoding' do
expect(reference).to eq %q{#1["a \"great\" filename.jpg"]}
end
end
context 'when the filename contains square brackets' do
let(:filename) { %q{a [great] filename.jpg} }
it 'uses enclosing quotes' do
expect(reference).to eq %q{#1["a [great] filename.jpg"]}
end
end
end
context 'when full is true' do
it 'returns complete path to the issue' do
refs = [
design.to_reference(full: true),
design.to_reference(project, full: true),
design.to_reference(group, full: true)
]
expect(refs).to all(eq 'sample-namespace/sample-project#1/designs[homescreen.jpg]')
end
end
context 'when full is false' do
it 'returns complete path to the issue' do
refs = [
design.to_reference(build(:project), full: false),
design.to_reference(group, full: false)
]
expect(refs).to all(eq 'sample-namespace/sample-project#1[homescreen.jpg]')
end
end
context 'when same project argument' do
it 'returns bare reference' do
expect(design.to_reference(project)).to eq("#1[homescreen.jpg]")
end
end
end
describe 'reference_pattern' do
let(:match) { described_class.reference_pattern.match(ref) }
let(:ref) { design.to_reference }
let(:design) { build(:design, filename: filename) }
context 'simple_file_name' do
let(:filename) { 'simple-file-name.jpg' }
it 'matches :simple_file_name' do
expect(match[:simple_file_name]).to eq(filename)
end
end
context 'quoted_file_name' do
let(:filename) { 'simple "file" name.jpg' }
it 'matches :simple_file_name' do
expect(match[:escaped_filename].gsub(/\\"/, '"')).to eq(filename)
end
end
context 'Base64 name' do
let(:filename) { '<>.png' }
it 'matches base_64_encoded_name' do
expect(Base64.decode64(match[:base_64_encoded_name])).to eq(filename)
end
end
end
end end
# frozen_string_literal: true # frozen_string_literal: true
module DesignManagementTestHelpers module DesignManagementTestHelpers
def enable_design_management(enabled = true) def enable_design_management(enabled = true, ref_filter = true)
stub_licensed_features(design_management: enabled) stub_licensed_features(design_management: enabled)
stub_lfs_setting(enabled: enabled) stub_lfs_setting(enabled: enabled)
stub_feature_flags(design_management_reference_filter_gfm_pipeline: ref_filter)
end end
def delete_designs(*designs) def delete_designs(*designs)
...@@ -18,6 +19,11 @@ module DesignManagementTestHelpers ...@@ -18,6 +19,11 @@ module DesignManagementTestHelpers
act_on_designs(designs) { ::DesignManagement::Action.modification } act_on_designs(designs) { ::DesignManagement::Action.modification }
end end
def url_for_design(design)
path_options = { vueroute: design.filename }
Gitlab::Routing.url_helpers.designs_project_issue_path(design.project, design.issue, path_options)
end
private private
def act_on_designs(designs, &block) def act_on_designs(designs, &block)
......
...@@ -43,15 +43,46 @@ module Banzai ...@@ -43,15 +43,46 @@ module Banzai
# Returns a String replaced with the return of the block. # Returns a String replaced with the return of the block.
def self.references_in(text, pattern = object_class.reference_pattern) def self.references_in(text, pattern = object_class.reference_pattern)
text.gsub(pattern) do |match| text.gsub(pattern) do |match|
symbol = $~[object_sym] if ident = identifier($~)
if object_class.reference_valid?(symbol) yield match, ident, $~[:project], $~[:namespace], $~
yield match, symbol.to_i, $~[:project], $~[:namespace], $~
else else
match match
end end
end end
end end
def self.identifier(match_data)
symbol = symbol_from_match(match_data)
parse_symbol(symbol, match_data) if object_class.reference_valid?(symbol)
end
def identifier(match_data)
self.class.identifier(match_data)
end
def self.symbol_from_match(match)
key = object_sym
match[key] if match.names.include?(key.to_s)
end
# Transform a symbol extracted from the text to a meaningful value
# In most cases these will be integers, so we call #to_i by default
#
# This method has the contract that if a string `ref` refers to a
# record `record`, then `parse_symbol(ref) == record_identifier(record)`.
def self.parse_symbol(symbol, match_data)
symbol.to_i
end
# We assume that most classes are identifying records by ID.
#
# This method has the contract that if a string `ref` refers to a
# record `record`, then `class.parse_symbol(ref) == record_identifier(record)`.
def record_identifier(record)
record.id
end
def object_class def object_class
self.class.object_class self.class.object_class
end end
...@@ -265,8 +296,10 @@ module Banzai ...@@ -265,8 +296,10 @@ module Banzai
@references_per[parent_type] ||= begin @references_per[parent_type] ||= begin
refs = Hash.new { |hash, key| hash[key] = Set.new } refs = Hash.new { |hash, key| hash[key] = Set.new }
regex = [
regex = Regexp.union(object_class.reference_pattern, object_class.link_reference_pattern) object_class.reference_pattern,
object_class.link_reference_pattern
].compact.reduce { |a, b| Regexp.union(a, b) }
nodes.each do |node| nodes.each do |node|
node.to_html.scan(regex) do node.to_html.scan(regex) do
...@@ -276,8 +309,9 @@ module Banzai ...@@ -276,8 +309,9 @@ module Banzai
full_group_path($~[:group]) full_group_path($~[:group])
end end
symbol = $~[object_sym] if ident = identifier($~)
refs[path] << symbol if object_class.reference_valid?(symbol) refs[path] << ident
end
end end
end end
......
...@@ -37,6 +37,11 @@ module Banzai ...@@ -37,6 +37,11 @@ module Banzai
end end
end end
# The default behaviour is `#to_i` - we just pass the hash through.
def self.parse_symbol(sha_hash, _match)
sha_hash
end
def url_for_object(commit, project) def url_for_object(commit, project)
h = Gitlab::Routing.url_helpers h = Gitlab::Routing.url_helpers
...@@ -65,10 +70,6 @@ module Banzai ...@@ -65,10 +70,6 @@ module Banzai
private private
def record_identifier(record)
record.id
end
def parent_records(parent, ids) def parent_records(parent, ids)
parent.commits_by(oids: ids.to_a) parent.commits_by(oids: ids.to_a)
end end
......
...@@ -3,30 +3,27 @@ ...@@ -3,30 +3,27 @@
require 'spec_helper' require 'spec_helper'
describe Banzai::Filter::AbstractReferenceFilter do describe Banzai::Filter::AbstractReferenceFilter do
let(:project) { create(:project) } let_it_be(:project) { create(:project) }
let(:doc) { Nokogiri::HTML.fragment('') }
let(:filter) { described_class.new(doc, project: project) }
describe '#references_per_parent' do describe '#references_per_parent' do
it 'returns a Hash containing references grouped per parent paths' do let(:doc) { Nokogiri::HTML.fragment("#1 #{project.full_path}#2 #2") }
doc = Nokogiri::HTML.fragment("#1 #{project.full_path}#2")
filter = described_class.new(doc, project: project)
expect(filter).to receive(:object_class).exactly(4).times.and_return(Issue) it 'returns a Hash containing references grouped per parent paths' do
expect(filter).to receive(:object_sym).twice.and_return(:issue) expect(described_class).to receive(:object_class).exactly(6).times.and_return(Issue)
refs = filter.references_per_parent refs = filter.references_per_parent
expect(refs).to be_an_instance_of(Hash) expect(refs).to match(a_hash_including(project.full_path => contain_exactly(1, 2)))
expect(refs[project.full_path]).to eq(Set.new(%w[1 2]))
end end
end end
describe '#parent_per_reference' do describe '#parent_per_reference' do
it 'returns a Hash containing projects grouped per parent paths' do it 'returns a Hash containing projects grouped per parent paths' do
doc = Nokogiri::HTML.fragment('')
filter = described_class.new(doc, project: project)
expect(filter).to receive(:references_per_parent) expect(filter).to receive(:references_per_parent)
.and_return({ project.full_path => Set.new(%w[1]) }) .and_return({ project.full_path => Set.new([1]) })
expect(filter.parent_per_reference) expect(filter.parent_per_reference)
.to eq({ project.full_path => project }) .to eq({ project.full_path => project })
...@@ -34,9 +31,6 @@ describe Banzai::Filter::AbstractReferenceFilter do ...@@ -34,9 +31,6 @@ describe Banzai::Filter::AbstractReferenceFilter do
end end
describe '#find_for_paths' do describe '#find_for_paths' do
let(:doc) { Nokogiri::HTML.fragment('') }
let(:filter) { described_class.new(doc, project: project) }
context 'with RequestStore disabled' do context 'with RequestStore disabled' do
it 'returns a list of Projects for a list of paths' do it 'returns a list of Projects for a list of paths' do
expect(filter.find_for_paths([project.full_path])) expect(filter.find_for_paths([project.full_path]))
......
...@@ -28,6 +28,17 @@ module FilterSpecHelper ...@@ -28,6 +28,17 @@ module FilterSpecHelper
described_class.call(html, context) described_class.call(html, context)
end end
# Get an instance of the Filter class
#
# Use this for testing instance methods, but remember to test the result of
# the full pipeline by calling #call using the other methods in this helper.
def filter_instance
render_context = Banzai::RenderContext.new(project, current_user)
context = { project: project, current_user: current_user, render_context: render_context }
described_class.new(input_text, context)
end
# Run text through HTML::Pipeline with the current filter and return the # Run text through HTML::Pipeline with the current filter and return the
# result Hash # result Hash
# #
......
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