From f42cfa9e8757161414f88e50d23b1d618bffad1f Mon Sep 17 00:00:00 2001 From: Douwe Maan <douwe@gitlab.com> Date: Wed, 7 Oct 2015 17:00:48 +0200 Subject: [PATCH] Refactor reference gathering to use a dedicated filter --- app/helpers/gitlab_markdown_helper.rb | 11 ++- .../markdown/commit_range_reference_filter.rb | 16 +++- .../markdown/commit_reference_filter.rb | 24 +++-- .../external_issue_reference_filter.rb | 3 +- lib/gitlab/markdown/issue_reference_filter.rb | 11 ++- lib/gitlab/markdown/label_reference_filter.rb | 11 ++- .../merge_request_reference_filter.rb | 11 ++- lib/gitlab/markdown/redactor_filter.rb | 44 ++------- lib/gitlab/markdown/reference_filter.rb | 34 ++++--- .../markdown/reference_gatherer_filter.rb | 49 ++++++++++ .../markdown/snippet_reference_filter.rb | 11 ++- lib/gitlab/markdown/user_reference_filter.rb | 42 ++++++--- lib/gitlab/reference_extractor.rb | 2 +- spec/helpers/gitlab_markdown_helper_spec.rb | 2 +- .../commit_range_reference_filter_spec.rb | 22 +++-- .../markdown/commit_reference_filter_spec.rb | 22 +++-- .../markdown/issue_reference_filter_spec.rb | 22 +++-- .../markdown/label_reference_filter_spec.rb | 18 ++-- .../merge_request_reference_filter_spec.rb | 22 +++-- .../gitlab/markdown/redactor_filter_spec.rb | 75 ++++++++-------- .../reference_gatherer_filter_spec.rb | 89 +++++++++++++++++++ .../markdown/snippet_reference_filter_spec.rb | 22 +++-- .../markdown/user_reference_filter_spec.rb | 20 ++--- spec/lib/gitlab/reference_extractor_spec.rb | 4 +- spec/support/filter_spec_helper.rb | 9 +- 25 files changed, 414 insertions(+), 182 deletions(-) create mode 100644 lib/gitlab/markdown/reference_gatherer_filter.rb create mode 100644 spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb diff --git a/app/helpers/gitlab_markdown_helper.rb b/app/helpers/gitlab_markdown_helper.rb index 6264b7f82ef..1b8bb46d25e 100644 --- a/app/helpers/gitlab_markdown_helper.rb +++ b/app/helpers/gitlab_markdown_helper.rb @@ -19,7 +19,8 @@ module GitlabMarkdownHelper 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 @@ module GitlabMarkdownHelper 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 @@ module GitlabMarkdownHelper 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) diff --git a/lib/gitlab/markdown/commit_range_reference_filter.rb b/lib/gitlab/markdown/commit_range_reference_filter.rb index bb496135d92..e070edae0a4 100644 --- a/lib/gitlab/markdown/commit_range_reference_filter.rb +++ b/lib/gitlab/markdown/commit_range_reference_filter.rb @@ -26,6 +26,18 @@ module Gitlab 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 @@ module Gitlab 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 diff --git a/lib/gitlab/markdown/commit_reference_filter.rb b/lib/gitlab/markdown/commit_reference_filter.rb index fcbb2e936a5..8cdbeb1f9cf 100644 --- a/lib/gitlab/markdown/commit_reference_filter.rb +++ b/lib/gitlab/markdown/commit_reference_filter.rb @@ -26,6 +26,18 @@ module Gitlab 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 @@ module Gitlab # 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 @@ module Gitlab 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 diff --git a/lib/gitlab/markdown/external_issue_reference_filter.rb b/lib/gitlab/markdown/external_issue_reference_filter.rb index f7c43e1ca89..8f86f13976a 100644 --- a/lib/gitlab/markdown/external_issue_reference_filter.rb +++ b/lib/gitlab/markdown/external_issue_reference_filter.rb @@ -47,8 +47,9 @@ module Gitlab 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 diff --git a/lib/gitlab/markdown/issue_reference_filter.rb b/lib/gitlab/markdown/issue_reference_filter.rb index 01320f80796..cd2a9b75680 100644 --- a/lib/gitlab/markdown/issue_reference_filter.rb +++ b/lib/gitlab/markdown/issue_reference_filter.rb @@ -27,6 +27,13 @@ module Gitlab 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 @@ module Gitlab 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}" diff --git a/lib/gitlab/markdown/label_reference_filter.rb b/lib/gitlab/markdown/label_reference_filter.rb index 1e5cb12071e..59568ab531f 100644 --- a/lib/gitlab/markdown/label_reference_filter.rb +++ b/lib/gitlab/markdown/label_reference_filter.rb @@ -22,6 +22,13 @@ module Gitlab 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 @@ module Gitlab 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>) diff --git a/lib/gitlab/markdown/merge_request_reference_filter.rb b/lib/gitlab/markdown/merge_request_reference_filter.rb index ecbd263d0e0..440574e574b 100644 --- a/lib/gitlab/markdown/merge_request_reference_filter.rb +++ b/lib/gitlab/markdown/merge_request_reference_filter.rb @@ -27,6 +27,13 @@ module Gitlab 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 @@ module Gitlab 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) diff --git a/lib/gitlab/markdown/redactor_filter.rb b/lib/gitlab/markdown/redactor_filter.rb index ae1c3c365bd..07ea6207d22 100644 --- a/lib/gitlab/markdown/redactor_filter.rb +++ b/lib/gitlab/markdown/redactor_filter.rb @@ -19,49 +19,19 @@ module Gitlab 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 diff --git a/lib/gitlab/markdown/reference_filter.rb b/lib/gitlab/markdown/reference_filter.rb index 9b293c957d6..ea6136b3303 100644 --- a/lib/gitlab/markdown/reference_filter.rb +++ b/lib/gitlab/markdown/reference_filter.rb @@ -15,10 +15,17 @@ module Gitlab # 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 @@ module Gitlab # # 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 @@ module Gitlab 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 diff --git a/lib/gitlab/markdown/reference_gatherer_filter.rb b/lib/gitlab/markdown/reference_gatherer_filter.rb new file mode 100644 index 00000000000..d64671e9550 --- /dev/null +++ b/lib/gitlab/markdown/reference_gatherer_filter.rb @@ -0,0 +1,49 @@ +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 diff --git a/lib/gitlab/markdown/snippet_reference_filter.rb b/lib/gitlab/markdown/snippet_reference_filter.rb index e2cf89cb1d8..a7396e96529 100644 --- a/lib/gitlab/markdown/snippet_reference_filter.rb +++ b/lib/gitlab/markdown/snippet_reference_filter.rb @@ -27,6 +27,13 @@ module Gitlab 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 @@ module Gitlab 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) diff --git a/lib/gitlab/markdown/user_reference_filter.rb b/lib/gitlab/markdown/user_reference_filter.rb index c08811effb2..0d2be7499b7 100644 --- a/lib/gitlab/markdown/user_reference_filter.rb +++ b/lib/gitlab/markdown/user_reference_filter.rb @@ -23,6 +23,34 @@ module Gitlab 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 @@ module Gitlab 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 @@ module Gitlab 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>) diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 0961bd80421..2e546ef0d54 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -50,7 +50,7 @@ module Gitlab 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] diff --git a/spec/helpers/gitlab_markdown_helper_spec.rb b/spec/helpers/gitlab_markdown_helper_spec.rb index 5d471f2f6ee..762ec25c4f5 100644 --- a/spec/helpers/gitlab_markdown_helper_spec.rb +++ b/spec/helpers/gitlab_markdown_helper_spec.rb @@ -44,7 +44,7 @@ describe GitlabMarkdownHelper do 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 diff --git a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb index 6813d6db14c..e5b8d723fe5 100644 --- a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb @@ -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 diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb index f937b4f50ee..d080efbf3d4 100644 --- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb @@ -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}") + result = reference_pipeline_result("See #{reference}") expect(result[:references][:commit]).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(:commit) { project2.commit } let(:reference) { commit.to_reference(project) } @@ -119,7 +127,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]).not_to be_empty end end diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb index 96787954516..94c80ae6611 100644 --- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb @@ -8,7 +8,7 @@ module Gitlab::Markdown IssuesHelper end - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:issue) { create(:issue, project: project) } it 'requires project context' do @@ -68,12 +68,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-issue' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Issue #{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-issue attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-issue') + expect(link.attr('data-issue')).to eq issue.id.to_s end it 'supports an :only_path context' do @@ -85,14 +93,14 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Fixed #{reference}") + result = reference_pipeline_result("Fixed #{reference}") expect(result[:references][:issue]).to eq [issue] end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:empty_project, namespace: namespace) } + let(:project2) { create(:empty_project, :public, namespace: namespace) } let(:issue) { create(:issue, project: project2) } let(:reference) { issue.to_reference(project) } @@ -123,7 +131,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Fixed #{reference}") + result = reference_pipeline_result("Fixed #{reference}") expect(result[:references][:issue]).to eq [issue] end end diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb index e32089de376..fc21b65a843 100644 --- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb @@ -5,7 +5,7 @@ module Gitlab::Markdown describe LabelReferenceFilter do include FilterSpecHelper - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:label) { create(:label, project: project) } let(:reference) { label.to_reference } @@ -25,12 +25,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-label' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Label #{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-label attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-label') + expect(link.attr('data-label')).to eq label.id.to_s end it 'supports an :only_path context' do @@ -42,7 +50,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Label #{reference}") + result = reference_pipeline_result("Label #{reference}") expect(result[:references][:label]).to eq [label] end diff --git a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb index feba08f7200..3ef6cdfff33 100644 --- a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe MergeRequestReferenceFilter do include FilterSpecHelper - let(:project) { create(:project) } + let(:project) { create(:project, :public) } let(:merge) { create(:merge_request, source_project: project) } it 'requires project context' do @@ -56,12 +56,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-merge_request' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Merge #{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-merge-request attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-merge-request') + expect(link.attr('data-merge-request')).to eq merge.id.to_s end it 'supports an :only_path context' do @@ -73,14 +81,14 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Merge #{reference}") + result = reference_pipeline_result("Merge #{reference}") expect(result[:references][:merge_request]).to eq [merge] 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(:merge) { create(:merge_request, source_project: project2) } let(:reference) { merge.to_reference(project) } @@ -104,7 +112,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Merge #{reference}") + result = reference_pipeline_result("Merge #{reference}") expect(result[:references][:merge_request]).to eq [merge] end end diff --git a/spec/lib/gitlab/markdown/redactor_filter_spec.rb b/spec/lib/gitlab/markdown/redactor_filter_spec.rb index 4ffba9ac7b1..eea3f1cf370 100644 --- a/spec/lib/gitlab/markdown/redactor_filter_spec.rb +++ b/spec/lib/gitlab/markdown/redactor_filter_spec.rb @@ -16,72 +16,75 @@ module Gitlab::Markdown link_to('text', '', class: 'gfm', data: data) end - context 'with data-group-id' do - it 'removes unpermitted Group references' do + context 'with data-project' do + it 'removes unpermitted Project references' do user = create(:user) - group = create(:group) + project = create(:empty_project) - link = reference_link(group_id: group.id) + link = reference_link(project: project.id, reference_filter: Gitlab::Markdown::ReferenceFilter.name) doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 end - it 'allows permitted Group references' do + it 'allows permitted Project references' do user = create(:user) - group = create(:group) - group.add_developer(user) + project = create(:empty_project) + project.team << [user, :master] - link = reference_link(group_id: group.id) + link = reference_link(project: project.id, reference_filter: Gitlab::Markdown::ReferenceFilter.name) doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 end - it 'handles invalid Group references' do - link = reference_link(group_id: 12345) + it 'handles invalid Project references' do + link = reference_link(project: 12345, reference_filter: Gitlab::Markdown::ReferenceFilter.name) expect { filter(link) }.not_to raise_error end end - context 'with data-project-id' do - it 'removes unpermitted Project references' do - user = create(:user) - project = create(:empty_project) + context "for user references" do - link = reference_link(project_id: project.id) - doc = filter(link, current_user: user) + context 'with data-group' do + it 'removes unpermitted Group references' do + user = create(:user) + group = create(:group) - expect(doc.css('a').length).to eq 0 - end + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + doc = filter(link, current_user: user) - it 'allows permitted Project references' do - user = create(:user) - project = create(:empty_project) - project.team << [user, :master] + expect(doc.css('a').length).to eq 0 + end - link = reference_link(project_id: project.id) - doc = filter(link, current_user: user) + it 'allows permitted Group references' do + user = create(:user) + group = create(:group) + group.add_developer(user) - expect(doc.css('a').length).to eq 1 - end + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + doc = filter(link, current_user: user) - it 'handles invalid Project references' do - link = reference_link(project_id: 12345) + expect(doc.css('a').length).to eq 1 + end - expect { filter(link) }.not_to raise_error + it 'handles invalid Group references' do + link = reference_link(group: 12345, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + + expect { filter(link) }.not_to raise_error + end end - end - context 'with data-user-id' do - it 'allows any User reference' do - user = create(:user) + context 'with data-user' do + it 'allows any User reference' do + user = create(:user) - link = reference_link(user_id: user.id) - doc = filter(link) + link = reference_link(user: user.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + doc = filter(link) - expect(doc.css('a').length).to eq 1 + expect(doc.css('a').length).to eq 1 + end end end end diff --git a/spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb b/spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb new file mode 100644 index 00000000000..4fa473ad191 --- /dev/null +++ b/spec/lib/gitlab/markdown/reference_gatherer_filter_spec.rb @@ -0,0 +1,89 @@ +require 'spec_helper' + +module Gitlab::Markdown + describe ReferenceGathererFilter do + include ActionView::Helpers::UrlHelper + include FilterSpecHelper + + def reference_link(data) + link_to('text', '', class: 'gfm', data: data) + end + + context "for issue references" do + + context 'with data-project' do + it 'removes unpermitted Project references' do + user = create(:user) + project = create(:empty_project) + issue = create(:issue, project: project) + + link = reference_link(project: project.id, issue: issue.id, reference_filter: Gitlab::Markdown::IssueReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:issue]).to be_empty + end + + it 'allows permitted Project references' do + user = create(:user) + project = create(:empty_project) + issue = create(:issue, project: project) + project.team << [user, :master] + + link = reference_link(project: project.id, issue: issue.id, reference_filter: Gitlab::Markdown::IssueReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:issue]).to eq([issue]) + end + + it 'handles invalid Project references' do + link = reference_link(project: 12345, issue: 12345, reference_filter: Gitlab::Markdown::IssueReferenceFilter.name) + + expect { pipeline_result(link) }.not_to raise_error + end + end + end + + context "for user references" do + + context 'with data-group' do + it 'removes unpermitted Group references' do + user = create(:user) + group = create(:group) + + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:user]).to be_empty + end + + it 'allows permitted Group references' do + user = create(:user) + group = create(:group) + group.add_developer(user) + + link = reference_link(group: group.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + result = pipeline_result(link, current_user: user) + + expect(result[:references][:user]).to eq([user]) + end + + it 'handles invalid Group references' do + link = reference_link(group: 12345, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + + expect { pipeline_result(link) }.not_to raise_error + end + end + + context 'with data-user' do + it 'allows any User reference' do + user = create(:user) + + link = reference_link(user: user.id, reference_filter: Gitlab::Markdown::UserReferenceFilter.name) + result = pipeline_result(link) + + expect(result[:references][:user]).to eq([user]) + end + end + end + end +end diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb index 02d581a7c46..9d9652dba46 100644 --- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe SnippetReferenceFilter do include FilterSpecHelper - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:snippet) { create(:project_snippet, project: project) } let(:reference) { snippet.to_reference } @@ -55,12 +55,20 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-snippet' end - it 'includes a data-project-id attribute' do + it 'includes a data-project attribute' do doc = filter("Snippet #{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-snippet attribute' do + doc = filter("See #{reference}") + link = doc.css('a').first + + expect(link).to have_attribute('data-snippet') + expect(link.attr('data-snippet')).to eq snippet.id.to_s end it 'supports an :only_path context' do @@ -72,14 +80,14 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Snippet #{reference}") + result = reference_pipeline_result("Snippet #{reference}") expect(result[:references][:snippet]).to eq [snippet] end end context 'cross-project reference' do let(:namespace) { create(:namespace, name: 'cross-reference') } - let(:project2) { create(:empty_project, namespace: namespace) } + let(:project2) { create(:empty_project, :public, namespace: namespace) } let(:snippet) { create(:project_snippet, project: project2) } let(:reference) { snippet.to_reference(project) } @@ -102,7 +110,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Snippet #{reference}") + result = reference_pipeline_result("Snippet #{reference}") expect(result[:references][:snippet]).to eq [snippet] end end diff --git a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb index c206cf4b745..d9e0d7c42db 100644 --- a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb @@ -4,7 +4,7 @@ module Gitlab::Markdown describe UserReferenceFilter do include FilterSpecHelper - let(:project) { create(:empty_project) } + let(:project) { create(:empty_project, :public) } let(:user) { create(:user) } let(:reference) { user.to_reference } @@ -39,7 +39,7 @@ module Gitlab::Markdown end it 'adds to the results hash' do - result = pipeline_result("Hey #{reference}") + result = reference_pipeline_result("Hey #{reference}") expect(result[:references][:user]).to eq [project.creator] end end @@ -64,16 +64,16 @@ module Gitlab::Markdown expect(doc.css('a').length).to eq 1 end - it 'includes a data-user-id attribute' do + it 'includes a data-user attribute' do doc = filter("Hey #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-user-id') - expect(link.attr('data-user-id')).to eq user.namespace.owner_id.to_s + expect(link).to have_attribute('data-user') + expect(link.attr('data-user')).to eq user.namespace.owner_id.to_s end it 'adds to the results hash' do - result = pipeline_result("Hey #{reference}") + result = reference_pipeline_result("Hey #{reference}") expect(result[:references][:user]).to eq [user] end end @@ -87,16 +87,16 @@ module Gitlab::Markdown expect(doc.css('a').first.attr('href')).to eq urls.group_url(group) end - it 'includes a data-group-id attribute' do + it 'includes a data-group attribute' do doc = filter("Hey #{reference}") link = doc.css('a').first - expect(link).to have_attribute('data-group-id') - expect(link.attr('data-group-id')).to eq group.id.to_s + expect(link).to have_attribute('data-group') + expect(link.attr('data-group')).to eq group.id.to_s end it 'adds to the results hash' do - result = pipeline_result("Hey #{reference}") + result = reference_pipeline_result("Hey #{reference}") expect(result[:references][:user]).to eq group.users end end diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 088e34f050c..6d7a067e4e0 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -2,7 +2,7 @@ require 'spec_helper' describe Gitlab::ReferenceExtractor do let(:project) { create(:project) } - subject { Gitlab::ReferenceExtractor.new(project, project.creator) } + subject { Gitlab::ReferenceExtractor.new(project, project.owner) } it 'accesses valid user objects' do @u_foo = create(:user, username: 'foo') @@ -102,7 +102,7 @@ describe Gitlab::ReferenceExtractor do let(:issue) { create(:issue, project: other_project) } before do - other_project.team << [project.creator, :developer] + other_project.team << [project.owner, :developer] end it 'handles project issue references' do diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index 56ce88abb48..97e5c270a59 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -29,12 +29,19 @@ module FilterSpecHelper # # Returns the Hash def pipeline_result(body, contexts = {}) - contexts.reverse_merge!(project: project) + contexts.reverse_merge!(project: project) if defined?(project) pipeline = HTML::Pipeline.new([described_class], contexts) pipeline.call(body) end + def reference_pipeline_result(body, contexts = {}) + contexts.reverse_merge!(project: project) if defined?(project) + + pipeline = HTML::Pipeline.new([described_class, Gitlab::Markdown::ReferenceGathererFilter], contexts) + pipeline.call(body) + end + # Modify a String reference to make it invalid # # Commit SHAs get reversed, IDs get incremented by 1, all other Strings get -- GitLab