Commit f42cfa9e authored by Douwe Maan's avatar Douwe Maan
Browse files

Refactor reference gathering to use a dedicated filter

parent cf5cbc198a21
......@@ -19,7 +19,8 @@ def link_to_gfm(body, url, html_options = {})
escape_once(body)
end
gfm_body = Gitlab::Markdown.gfm(escaped_body, project: @project, current_user: current_user)
user = current_user if defined?(current_user)
gfm_body = Gitlab::Markdown.gfm(escaped_body, project: @project, current_user: user)
fragment = Nokogiri::HTML::DocumentFragment.parse(gfm_body)
if fragment.children.size == 1 && fragment.children[0].name == 'a'
......@@ -55,8 +56,10 @@ def markdown(text, context = {})
ref: @ref
)
user = current_user if defined?(current_user)
html = Gitlab::Markdown.render(text, context)
Gitlab::Markdown.post_process(html, pipeline: context[:pipeline], user: current_user)
Gitlab::Markdown.post_process(html, pipeline: context[:pipeline], user: user)
end
# TODO (rspeicher): Remove all usages of this helper and just call `markdown`
......@@ -72,8 +75,10 @@ def gfm(text, options = {})
ref: @ref
)
user = current_user if defined?(current_user)
html = Gitlab::Markdown.gfm(text, options)
Gitlab::Markdown.post_process(html, pipeline: options[:pipeline], user: current_user)
Gitlab::Markdown.post_process(html, pipeline: options[:pipeline], user: user)
end
def asciidoc(text)
......
......@@ -26,6 +26,18 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
project = Project.find(node.attr("data-project")) rescue nil
return unless project
id = node.attr("data-commit-range")
range = CommitRange.new(id, project)
return unless range.valid_commits?
{ commit_range: range }
end
def initialize(*args)
super
......@@ -53,13 +65,11 @@ def commit_range_link_filter(text)
range = CommitRange.new(id, project)
if range.valid_commits?
push_result(:commit_range, range)
url = url_for_commit_range(project, range)
title = range.reference_title
klass = reference_class(:commit_range)
data = data_attribute(project.id)
data = data_attribute(project: project.id, commit_range: id)
project_ref += '@' if project_ref
......
......@@ -26,6 +26,18 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
project = Project.find(node.attr("data-project")) rescue nil
return unless project
id = node.attr("data-commit")
commit = commit_from_ref(project, id)
return unless commit
{ commit: commit }
end
def call
replace_text_nodes_matching(Commit.reference_pattern) do |content|
commit_link_filter(content)
......@@ -39,17 +51,15 @@ def call
# Returns a String with commit references replaced with links. All links
# have `gfm` and `gfm-commit` class names attached for styling.
def commit_link_filter(text)
self.class.references_in(text) do |match, commit_ref, project_ref|
self.class.references_in(text) do |match, id, project_ref|
project = self.project_from_ref(project_ref)
if commit = commit_from_ref(project, commit_ref)
push_result(:commit, commit)
if commit = self.class.commit_from_ref(project, id)
url = url_for_commit(project, commit)
title = escape_once(commit.link_title)
klass = reference_class(:commit)
data = data_attribute(project.id)
data = data_attribute(project: project.id, commit: id)
project_ref += '@' if project_ref
......@@ -62,9 +72,9 @@ def commit_link_filter(text)
end
end
def commit_from_ref(project, commit_ref)
def self.commit_from_ref(project, id)
if project && project.valid_repo?
project.commit(commit_ref)
project.commit(id)
end
end
......
......@@ -47,8 +47,9 @@ def issue_link_filter(text)
title = escape_once("Issue in #{project.external_issue_tracker.title}")
klass = reference_class(:issue)
data = data_attribute(project: project.id)
%(<a href="#{url}"
%(<a href="#{url}" #{data}
title="#{title}"
class="#{klass}">#{match}</a>)
end
......
......@@ -27,6 +27,13 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
issue = Issue.find(node.attr("data-issue")) rescue nil
return unless issue
{ issue: issue }
end
def call
replace_text_nodes_matching(Issue.reference_pattern) do |content|
issue_link_filter(content)
......@@ -45,13 +52,11 @@ def issue_link_filter(text)
project = self.project_from_ref(project_ref)
if project && issue = project.get_issue(id)
push_result(:issue, issue)
url = url_for_issue(id, project, only_path: context[:only_path])
title = escape_once("Issue: #{issue.title}")
klass = reference_class(:issue)
data = data_attribute(project.id)
data = data_attribute(project: project.id, issue: issue.id)
%(<a href="#{url}" #{data}
title="#{title}"
......
......@@ -22,6 +22,13 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
label = Label.find(node.attr("data-label")) rescue nil
return unless label
{ label: label }
end
def call
replace_text_nodes_matching(Label.reference_pattern) do |content|
label_link_filter(content)
......@@ -41,11 +48,9 @@ def label_link_filter(text)
params = label_params(id, name)
if label = project.labels.find_by(params)
push_result(:label, label)
url = url_for_label(project, label)
klass = reference_class(:label)
data = data_attribute(project.id)
data = data_attribute(project: project.id, label: label.id)
%(<a href="#{url}" #{data}
class="#{klass}">#{render_colored_label(label)}</a>)
......
......@@ -27,6 +27,13 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
merge_request = MergeRequest.find(node.attr("data-merge-request")) rescue nil
return unless merge_request
{ merge_request: merge_request }
end
def call
replace_text_nodes_matching(MergeRequest.reference_pattern) do |content|
merge_request_link_filter(content)
......@@ -45,11 +52,9 @@ def merge_request_link_filter(text)
project = self.project_from_ref(project_ref)
if project && merge_request = project.merge_requests.find_by(iid: id)
push_result(:merge_request, merge_request)
title = escape_once("Merge Request: #{merge_request.title}")
klass = reference_class(:merge_request)
data = data_attribute(project.id)
data = data_attribute(project: project.id, merge_request: merge_request.id)
url = url_for_merge_request(merge_request, project)
......
......@@ -19,49 +19,19 @@ def call
doc
end
private
def user_can_reference?(node)
if node.has_attribute?('data-group-id')
user_can_reference_group?(node.attr('data-group-id'))
elsif node.has_attribute?('data-project-id')
user_can_reference_project?(node.attr('data-project-id'))
elsif node.has_attribute?('data-user-id')
user_can_reference_user?(node.attr('data-user-id'))
if node.has_attribute?('data-reference-filter')
reference_type = node.attr('data-reference-filter')
reference_filter = reference_type.constantize
reference_filter.user_can_reference?(current_user, node)
else
true
end
end
def user_can_reference_group?(id)
group = Group.find(id)
group && can?(:read_group, group)
rescue ActiveRecord::RecordNotFound
false
end
def user_can_reference_project?(id)
project = Project.find(id)
project && can?(:read_project, project)
rescue ActiveRecord::RecordNotFound
false
end
def user_can_reference_user?(id)
# Permit all user reference links
true
end
private
def abilities
Ability.abilities
end
def can?(ability, object)
abilities.allowed?(current_user, ability, object)
end
def current_user
context[:current_user]
end
......
......@@ -15,10 +15,17 @@ module Markdown
# Results:
# :references - A Hash of references that were found and replaced.
class ReferenceFilter < HTML::Pipeline::Filter
def initialize(*args)
super
def self.user_can_reference?(user, node)
if node.has_attribute?('data-project')
project = Project.find(node.attr('data-project')) rescue nil
Ability.abilities.allowed?(user, :read_project, project)
else
true
end
end
result[:references] = Hash.new { |hash, type| hash[type] = [] }
def self.referenced_by(node)
nil
end
# Returns a data attribute String to attach to a reference link
......@@ -28,13 +35,14 @@ def initialize(*args)
#
# Examples:
#
# data_attribute(1) # => "data-project-id=\"1\""
# data_attribute(2, :user) # => "data-user-id=\"2\""
# data_attribute(3, :group) # => "data-group-id=\"3\""
# data_attribute(project: 1) # => "data-reference-filter=\"SomeReferenceFilter\" data-project=\"1\""
# data_attribute(user: 2) # => "data-reference-filter=\"SomeReferenceFilter\" data-user=\"2\""
# data_attribute(group: 3) # => "data-reference-filter=\"SomeReferenceFilter\" data-group=\"3\""
#
# Returns a String
def data_attribute(id, type = :project)
%Q(data-#{type}-id="#{id}")
def data_attribute(attributes = {})
attributes[:reference_filter] = self.class.name
attributes.map { |key, value| %Q(data-#{key.to_s.dasherize}="#{value}") }.join(" ")
end
def escape_once(html)
......@@ -59,16 +67,6 @@ def project
context[:project]
end
# Add a reference to the pipeline's result Hash
#
# type - Singular Symbol reference type (e.g., :issue, :user, etc.)
# values - One or more Objects to add
def push_result(type, *values)
return if values.empty?
result[:references][type].push(*values)
end
def reference_class(type)
"gfm gfm-#{type}"
end
......
require 'gitlab/markdown'
require 'html/pipeline/filter'
module Gitlab
module Markdown
# HTML filter that removes references to records that the current user does
# not have permission to view.
#
# Expected to be run in its own post-processing pipeline.
#
class ReferenceGathererFilter < HTML::Pipeline::Filter
def initialize(*)
super
result[:references] ||= Hash.new { |hash, type| hash[type] = [] }
end
def call
doc.css('a.gfm').each do |node|
gather_references(node)
end
doc
end
private
def gather_references(node)
return unless node.has_attribute?('data-reference-filter')
reference_type = node.attr('data-reference-filter')
reference_filter = reference_type.constantize
return unless reference_filter.user_can_reference?(current_user, node)
references = reference_filter.referenced_by(node)
return unless references
references.each do |type, values|
result[:references][type].push(*values)
end
end
def current_user
context[:current_user]
end
end
end
end
......@@ -27,6 +27,13 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
snippet = Snippet.find(node.attr("data-snippet")) rescue nil
return unless snippet
{ snippet: snippet }
end
def call
replace_text_nodes_matching(Snippet.reference_pattern) do |content|
snippet_link_filter(content)
......@@ -45,11 +52,9 @@ def snippet_link_filter(text)
project = self.project_from_ref(project_ref)
if project && snippet = project.snippets.find_by(id: id)
push_result(:snippet, snippet)
title = escape_once("Snippet: #{snippet.title}")
klass = reference_class(:snippet)
data = data_attribute(project.id)
data = data_attribute(project: project.id, snippet: snippet.id)
url = url_for_snippet(snippet, project)
......
......@@ -23,6 +23,34 @@ def self.references_in(text)
end
end
def self.referenced_by(node)
if node.has_attribute?('data-group')
group = Group.find(node.attr('data-group')) rescue nil
return unless group
{ user: group.users }
elsif node.has_attribute?('data-user')
user = User.find(node.attr('data-user')) rescue nil
return unless user
{ user: user }
elsif node.has_attribute?('data-project')
project = Project.find(node.attr('data-project')) rescue nil
return unless project
{ user: project.team.members.flatten }
end
end
def self.user_can_reference?(user, node)
if node.has_attribute?('data-group')
group = Group.find(node.attr('data-group')) rescue nil
Ability.abilities.allowed?(user, :read_group, group)
else
super
end
end
def call
replace_text_nodes_matching(User.reference_pattern) do |content|
user_link_filter(content)
......@@ -61,14 +89,12 @@ def link_class
def link_to_all
project = context[:project]
# FIXME (rspeicher): Law of Demeter
push_result(:user, *project.team.members.flatten)
url = urls.namespace_project_url(project.namespace, project,
only_path: context[:only_path])
data = data_attribute(project: project.id)
text = User.reference_prefix + 'all'
%(<a href="#{url}" class="#{link_class}">#{text}</a>)
%(<a href="#{url}" #{data} class="#{link_class}">#{text}</a>)
end
def link_to_namespace(namespace)
......@@ -80,20 +106,16 @@ def link_to_namespace(namespace)
end
def link_to_group(group, namespace)
push_result(:user, *namespace.users)
url = urls.group_url(group, only_path: context[:only_path])
data = data_attribute(namespace.id, :group)
data = data_attribute(group: namespace.id)
text = Group.reference_prefix + group
%(<a href="#{url}" #{data} class="#{link_class}">#{text}</a>)
end
def link_to_user(user, namespace)
push_result(:user, namespace.owner)
url = urls.user_url(user, only_path: context[:only_path])
data = data_attribute(namespace.owner_id, :user)
data = data_attribute(user: namespace.owner_id)
text = User.reference_prefix + user
%(<a href="#{url}" #{data} class="#{link_class}">#{text}</a>)
......
......@@ -50,7 +50,7 @@ def pipeline_result(filter_type)
ignore_blockquotes: true
}
pipeline = HTML::Pipeline.new([filter], context)
pipeline = HTML::Pipeline.new([filter, Gitlab::Markdown::ReferenceGathererFilter], context)
result = pipeline.call(@text)
result[:references][filter_type]
......
......@@ -44,7 +44,7 @@
describe "override default project" do
let(:actual) { issue.to_reference }
let(:second_project) { create(:project) }
let(:second_project) { create(:project, :public) }
let(:second_issue) { create(:issue, project: second_project) }
it 'should link to the issue' do
......
......@@ -4,7 +4,7 @@ module Gitlab::Markdown
describe CommitRangeReferenceFilter do
include FilterSpecHelper
let(:project) { create(:project) }
let(:project) { create(:project, :public) }
let(:commit1) { project.commit }
let(:commit2) { project.commit("HEAD~2") }
......@@ -75,12 +75,20 @@ module Gitlab::Markdown
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit_range'
end
it 'includes a data-project-id attribute' do
it 'includes a data-project attribute' do
doc = filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-project-id')
expect(link.attr('data-project-id')).to eq project.id.to_s
expect(link).to have_attribute('data-project')
expect(link.attr('data-project')).to eq project.id.to_s
end
it 'includes a data-commit-range attribute' do
doc = filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-commit-range')
expect(link.attr('data-commit-range')).to eq range.to_reference
end
it 'supports an :only_path option' do
......@@ -92,14 +100,14 @@ module Gitlab::Markdown
end
it 'adds to the results hash' do
result = pipeline_result("See #{reference}")
result = reference_pipeline_result("See #{reference}")
expect(result[:references][:commit_range]).not_to be_empty
end
end
context 'cross-project reference' do
let(:namespace) { create(:namespace, name: 'cross-reference') }
let(:project2) { create(:project, namespace: namespace) }
let(:project2) { create(:project, :public, namespace: namespace) }
let(:reference) { range.to_reference(project) }
before do
......@@ -129,7 +137,7 @@ module Gitlab::Markdown
end
it 'adds to the results hash' do
result = pipeline_result("See #{reference}")
result = reference_pipeline_result("See #{reference}")
expect(result[:references][:commit_range]).not_to be_empty
end
end
......
......@@ -4,7 +4,7 @@ module Gitlab::Markdown
describe CommitReferenceFilter do
include FilterSpecHelper
let(:project) { create(:project) }
let(:project) { create(:project, :public) }
let(:commit) { project.commit }
it 'requires project context' do
......@@ -71,12 +71,20 @@ module Gitlab::Markdown
expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-commit'
end
it 'includes a data-project-id attribute' do
it 'includes a data-project attribute' do
doc = filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-project-id')
expect(link.attr('data-project-id')).to eq project.id.to_s
expect(link).to have_attribute('data-project')
expect(link.attr('data-project')).to eq project.id.to_s
end
it 'includes a data-commit attribute' do
doc = filter("See #{reference}")
link = doc.css('a').first
expect(link).to have_attribute('data-commit')
expect(link.attr('data-commit')).to eq commit.id
end
it 'supports an :only_path context' do
......@@ -88,14 +96,14 @@ module Gitlab::Markdown
end
it 'adds to the results hash' do
result = pipeline_result("See #{reference}")