diff --git a/app/models/commit_range.rb b/app/models/commit_range.rb new file mode 100644 index 0000000000000000000000000000000000000000..e64561982645ca4fc8ca0c7a757570a0b5359b5b --- /dev/null +++ b/app/models/commit_range.rb @@ -0,0 +1,106 @@ +# CommitRange makes it easier to work with commit ranges +# +# Examples: +# +# range = CommitRange.new('f3f85602...e86e1013') +# range.exclude_start? # => false +# range.reference_title # => "Commits f3f85602 through e86e1013" +# range.to_s # => "f3f85602...e86e1013" +# +# range = CommitRange.new('f3f856029bc5f966c5a7ee24cf7efefdd20e6019..e86e1013709735be5bb767e2b228930c543f25ae') +# range.exclude_start? # => true +# range.reference_title # => "Commits f3f85602^ through e86e1013" +# range.to_param # => {from: "f3f856029bc5f966c5a7ee24cf7efefdd20e6019^", to: "e86e1013709735be5bb767e2b228930c543f25ae"} +# range.to_s # => "f3f85602..e86e1013" +# +# # Assuming `project` is a Project with a repository containing both commits: +# range.project = project +# range.valid_commits? # => true +# +class CommitRange + include ActiveModel::Conversion + + attr_reader :sha_from, :notation, :sha_to + + # Optional Project model + attr_accessor :project + + # See `exclude_start?` + attr_reader :exclude_start + + # The beginning and ending SHA sums can be between 6 and 40 hex characters, + # and the range selection can be double- or triple-dot. + PATTERN = /\h{6,40}\.{2,3}\h{6,40}/ + + # Initialize a CommitRange + # + # range_string - The String commit range. + # project - An optional Project model. + # + # Raises ArgumentError if `range_string` does not match `PATTERN`. + def initialize(range_string, project = nil) + range_string.strip! + + unless range_string.match(/\A#{PATTERN}\z/) + raise ArgumentError, "invalid CommitRange string format: #{range_string}" + end + + @exclude_start = !range_string.include?('...') + @sha_from, @notation, @sha_to = range_string.split(/(\.{2,3})/, 2) + + @project = project + end + + def inspect + %(#<#{self.class}:#{object_id} #{to_s}>) + end + + def to_s + "#{sha_from[0..7]}#{notation}#{sha_to[0..7]}" + end + + # Returns a String for use in a link's title attribute + def reference_title + "Commits #{suffixed_sha_from} through #{sha_to}" + end + + # Return a Hash of parameters for passing to a URL helper + # + # See `namespace_project_compare_url` + def to_param + { from: suffixed_sha_from, to: sha_to } + end + + def exclude_start? + exclude_start + end + + # Check if both the starting and ending commit IDs exist in a project's + # repository + # + # project - An optional Project to check (default: `project`) + def valid_commits?(project = project) + return nil unless project.present? + return false unless project.valid_repo? + + commit_from.present? && commit_to.present? + end + + def persisted? + true + end + + def commit_from + @commit_from ||= project.repository.commit(suffixed_sha_from) + end + + def commit_to + @commit_to ||= project.repository.commit(sha_to) + end + + private + + def suffixed_sha_from + sha_from + (exclude_start? ? '^' : '') + end +end diff --git a/lib/gitlab/markdown/commit_range_reference_filter.rb b/lib/gitlab/markdown/commit_range_reference_filter.rb index baa97bee9bfd468187836254cb1df43e85564a6f..8764f7e474f4111730b227172c6b06f69c0aea29 100644 --- a/lib/gitlab/markdown/commit_range_reference_filter.rb +++ b/lib/gitlab/markdown/commit_range_reference_filter.rb @@ -32,11 +32,8 @@ module Gitlab # Pattern used to extract commit range references from text # - # The beginning and ending SHA1 sums can be between 6 and 40 hex - # characters, and the range selection can be double- or triple-dot. - # # This pattern supports cross-project references. - COMMIT_RANGE_PATTERN = /(#{PROJECT_PATTERN}@)?(?<commit_range>\h{6,40}\.{2,3}\h{6,40})/ + COMMIT_RANGE_PATTERN = /(#{PROJECT_PATTERN}@)?(?<commit_range>#{CommitRange::PATTERN})/ def call replace_text_nodes_matching(COMMIT_RANGE_PATTERN) do |content| @@ -53,52 +50,34 @@ module Gitlab # links have `gfm` and `gfm-commit_range` class names attached for # styling. def commit_range_link_filter(text) - self.class.references_in(text) do |match, commit_range, project_ref| + self.class.references_in(text) do |match, id, project_ref| project = self.project_from_ref(project_ref) - from_id, to_id = split_commit_range(commit_range) + range = CommitRange.new(id, project) + + if range.valid_commits? + push_result(:commit_range, range) - if valid_range?(project, from_id, to_id) - url = url_for_commit_range(project, from_id, to_id) + url = url_for_commit_range(project, range) - title = "Commits #{from_id} through #{to_id}" + title = range.reference_title klass = reference_class(:commit_range) project_ref += '@' if project_ref %(<a href="#{url}" title="#{title}" - class="#{klass}">#{project_ref}#{commit_range}</a>) + class="#{klass}">#{project_ref}#{range}</a>) else match end end end - def split_commit_range(range) - from_id, to_id = range.split(/\.{2,3}/, 2) - from_id << "^" if range !~ /\.{3}/ - - [from_id, to_id] - end - - def commit(id) - unless @commit_map[id] - @commit_map[id] = project.commit(id) - end - - @commit_map[id] - end - - def valid_range?(project, from_id, to_id) - project && project.valid_repo? && commit(from_id) && commit(to_id) - end - - def url_for_commit_range(project, from_id, to_id) + def url_for_commit_range(project, range) h = Rails.application.routes.url_helpers h.namespace_project_compare_url(project.namespace, project, - from: from_id, to: to_id, - only_path: context[:only_path]) + range.to_param.merge(only_path: context[:only_path])) end end end diff --git a/lib/gitlab/markdown/commit_reference_filter.rb b/lib/gitlab/markdown/commit_reference_filter.rb index 66598127f6e0ec78601433909d310711b000d954..b20b29f5d0ca07b7736242cf8659d926c9c16b00 100644 --- a/lib/gitlab/markdown/commit_reference_filter.rb +++ b/lib/gitlab/markdown/commit_reference_filter.rb @@ -48,6 +48,8 @@ module Gitlab project = self.project_from_ref(project_ref) if commit = commit_from_ref(project, commit_ref) + push_result(:commit, commit) + url = url_for_commit(project, commit) title = escape_once(commit.link_title) @@ -57,7 +59,7 @@ module Gitlab %(<a href="#{url}" title="#{title}" - class="#{klass}">#{project_ref}#{commit_ref}</a>) + class="#{klass}">#{project_ref}#{commit.short_id}</a>) else match end diff --git a/lib/gitlab/markdown/issue_reference_filter.rb b/lib/gitlab/markdown/issue_reference_filter.rb index c9267cc3e9d90c46bd41df347d5fa3f8967d70f4..4b360369d3768f5905c05fd41aa9b275e9706f6f 100644 --- a/lib/gitlab/markdown/issue_reference_filter.rb +++ b/lib/gitlab/markdown/issue_reference_filter.rb @@ -48,6 +48,9 @@ module Gitlab project = self.project_from_ref(project_ref) if project && project.issue_exists?(issue) + # FIXME (rspeicher): Law of Demeter + push_result(:issue, project.issues.where(iid: issue).first) + url = url_for_issue(issue, project, only_path: context[:only_path]) title = escape_once("Issue: #{title_for_issue(issue, project)}") diff --git a/lib/gitlab/markdown/label_reference_filter.rb b/lib/gitlab/markdown/label_reference_filter.rb index 4c21192c0d3cb4ba2cacd4228fc9084e10f97606..a357f28458d5269a0bf8ef6061f004137a69d798 100644 --- a/lib/gitlab/markdown/label_reference_filter.rb +++ b/lib/gitlab/markdown/label_reference_filter.rb @@ -52,11 +52,13 @@ module Gitlab params = label_params(id, name) if label = project.labels.find_by(params) - url = url_for_label(project, label) + push_result(:label, label) + url = url_for_label(project, label) klass = reference_class(:label) - %(<a href="#{url}" class="#{klass}">#{render_colored_label(label)}</a>) + %(<a href="#{url}" + class="#{klass}">#{render_colored_label(label)}</a>) else match end diff --git a/lib/gitlab/markdown/merge_request_reference_filter.rb b/lib/gitlab/markdown/merge_request_reference_filter.rb index 40239523cdaa33188a4c436b36b84a7ab0d0b2f3..7c28fe112efcd0b3a8cb3bf9c811b7bb6e1ec50a 100644 --- a/lib/gitlab/markdown/merge_request_reference_filter.rb +++ b/lib/gitlab/markdown/merge_request_reference_filter.rb @@ -48,6 +48,8 @@ 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) diff --git a/lib/gitlab/markdown/reference_filter.rb b/lib/gitlab/markdown/reference_filter.rb index ef4aa408a7e57b8ddd1c7423ca1718538f148506..42eadf450c7c5510cb325aecdae876148c9fd7ec 100644 --- a/lib/gitlab/markdown/reference_filter.rb +++ b/lib/gitlab/markdown/reference_filter.rb @@ -12,7 +12,15 @@ module Gitlab # :reference_class - Custom CSS class added to reference links. # :only_path - Generate path-only links. # + # Results: + # :references - A Hash of references that were found and replaced. class ReferenceFilter < HTML::Pipeline::Filter + def initialize(*args) + super + + result[:references] = Hash.new { |hash, type| hash[type] = [] } + end + def escape_once(html) ERB::Util.html_escape_once(html) end @@ -29,6 +37,16 @@ module Gitlab context[:project] end + # Add a reference to the pipeline's result Hash + # + # type - Singular Symbol reference type (e.g., :issue, :user, etc.) + # value - Object to add + def push_result(type, *values) + return if values.empty? + + result[:references][type].push(*values) + end + def reference_class(type) "gfm gfm-#{type} #{context[:reference_class]}".strip end diff --git a/lib/gitlab/markdown/snippet_reference_filter.rb b/lib/gitlab/markdown/snippet_reference_filter.rb index ada67de992b575a6c543f497ffcf3bd53b7ac56b..64a0a2696f7587867fc4d06024695d64223d252e 100644 --- a/lib/gitlab/markdown/snippet_reference_filter.rb +++ b/lib/gitlab/markdown/snippet_reference_filter.rb @@ -48,6 +48,8 @@ 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) diff --git a/lib/gitlab/markdown/user_reference_filter.rb b/lib/gitlab/markdown/user_reference_filter.rb index 5fc8ed55fe26f114459f52e1ce95f3746b9a1190..b4c48d29684a08b18cc366fa92f75b793f348009 100644 --- a/lib/gitlab/markdown/user_reference_filter.rb +++ b/lib/gitlab/markdown/user_reference_filter.rb @@ -44,18 +44,25 @@ module Gitlab klass = reference_class(:project_member) if user == 'all' + # FIXME (rspeicher): Law of Demeter + push_result(:user, *project.team.members.flatten) + url = link_to_all(project) %(<a href="#{url}" class="#{klass}">@#{user}</a>) elsif namespace = Namespace.find_by(path: user) if namespace.is_a?(Group) if user_can_reference_group?(namespace) + push_result(:user, *namespace.users) + url = group_url(user, only_path: context[:only_path]) %(<a href="#{url}" class="#{klass}">@#{user}</a>) else match end else + push_result(:user, namespace.owner) + url = user_url(user, only_path: context[:only_path]) %(<a href="#{url}" class="#{klass}">@#{user}</a>) end diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 949dd5d26b10cdfef0cd58b2a0ed96a502b1a984..e35f848fa6e5f60701b4427a7d40ac97bd6d732f 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -8,151 +8,70 @@ module Gitlab @current_user = current_user end - def can?(user, action, subject) - Ability.abilities.allowed?(user, action, subject) - end - def analyze(text) - text = text.dup - - # Remove preformatted/code blocks so that references are not included - text.gsub!(/^```.*?^```/m, '') - text.gsub!(/[^`]`[^`]*?`[^`]/, '') - - @references = Hash.new { |hash, type| hash[type] = [] } - parse_references(text) + @_text = text.dup end - # Given a valid project, resolve the extracted identifiers of the requested type to - # model objects. - def users - references[:user].uniq.map do |project, identifier| - if identifier == "all" - project.team.members.flatten - elsif namespace = Namespace.find_by(path: identifier) - if namespace.is_a?(Group) - namespace.users if can?(current_user, :read_group, namespace) - else - namespace.owner - end - end - end.flatten.compact.uniq + result = pipeline_result(:user) + result.uniq end def labels - references[:label].uniq.map do |project, identifier| - project.labels.where(id: identifier).first - end.compact.uniq + result = pipeline_result(:label) + result.uniq end def issues - references[:issue].uniq.map do |project, identifier| - if project.default_issues_tracker? - project.issues.where(iid: identifier).first - end - end.compact.uniq + # TODO (rspeicher): What about external issues? + + result = pipeline_result(:issue) + result.uniq end def merge_requests - references[:merge_request].uniq.map do |project, identifier| - project.merge_requests.where(iid: identifier).first - end.compact.uniq + result = pipeline_result(:merge_request) + result.uniq end def snippets - references[:snippet].uniq.map do |project, identifier| - project.snippets.where(id: identifier).first - end.compact.uniq + result = pipeline_result(:snippet) + result.uniq end def commits - references[:commit].uniq.map do |project, identifier| - repo = project.repository - repo.commit(identifier) if repo - end.compact.uniq + result = pipeline_result(:commit) + result.uniq end def commit_ranges - references[:commit_range].uniq.map do |project, identifier| - repo = project.repository - if repo - from_id, to_id = identifier.split(/\.{2,3}/, 2) - [repo.commit(from_id), repo.commit(to_id)] - end - end.compact.uniq + result = pipeline_result(:commit_range) + result.uniq end private - NAME_STR = Gitlab::Regex::NAMESPACE_REGEX_STR - PROJ_STR = "(?<project>#{NAME_STR}/#{NAME_STR})" - - REFERENCE_PATTERN = %r{ - (?<prefix>\W)? # Prefix - ( # Reference - @(?<user>#{NAME_STR}) # User name - |~(?<label>\d+) # Label ID - |(?<issue>([A-Z\-]+-)\d+) # JIRA Issue ID - |#{PROJ_STR}?\#(?<issue>([a-zA-Z\-]+-)?\d+) # Issue ID - |#{PROJ_STR}?!(?<merge_request>\d+) # MR ID - |\$(?<snippet>\d+) # Snippet ID - |(#{PROJ_STR}@)?(?<commit_range>[\h]{6,40}\.{2,3}[\h]{6,40}) # Commit range - |(#{PROJ_STR}@)?(?<commit>[\h]{6,40}) # Commit ID - ) - (?<suffix>\W)? # Suffix - }x.freeze - - TYPES = %i(user issue label merge_request snippet commit commit_range).freeze - - def parse_references(text, project = @project) - # parse reference links - text.gsub!(REFERENCE_PATTERN) do |match| - type = TYPES.detect { |t| $~[t].present? } - - actual_project = project - project_prefix = nil - project_path = $LAST_MATCH_INFO[:project] - if project_path - actual_project = ::Project.find_with_namespace(project_path) - actual_project = nil unless can?(current_user, :read_project, actual_project) - project_prefix = project_path - end - - parse_result($LAST_MATCH_INFO, type, - actual_project, project_prefix) || match - end - end - - # Called from #parse_references. Attempts to build a gitlab reference - # link. Returns nil if +type+ is nil, if the match string is an HTML - # entity, if the reference is invalid, or if the matched text includes an - # invalid project path. - def parse_result(match_info, type, project, project_prefix) - prefix = match_info[:prefix] - suffix = match_info[:suffix] - - return nil if html_entity?(prefix, suffix) || type.nil? - return nil if project.nil? && !project_prefix.nil? - - identifier = match_info[type] - ref_link = reference_link(type, identifier, project, project_prefix) - - if ref_link - "#{prefix}#{ref_link}#{suffix}" - else - nil - end - end - - # Return true if the +prefix+ and +suffix+ indicate that the matched string - # is an HTML entity like & - def html_entity?(prefix, suffix) - prefix && suffix && prefix[0] == '&' && suffix[-1] == ';' - end - - def reference_link(type, identifier, project, _) - references[type] << [project, identifier] + # Instantiate and call HTML::Pipeline with a single reference filter type, + # returning the result + # + # filter_type - Symbol reference type (e.g., :commit, :issue, etc.) + # + # Returns the results Array for the requested filter type + def pipeline_result(filter_type) + klass = filter_type.to_s.camelize + 'ReferenceFilter' + filter = "Gitlab::Markdown::#{klass}".constantize + + context = { + project: project, + current_user: current_user, + # We don't actually care about the links generated + only_path: true + } + + pipeline = HTML::Pipeline.new([filter], context) + result = pipeline.call(@_text) + + result[:references][filter_type] end end end 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 f0804ce0056de50e913a209b151f84a03303db02..7274cb309a0a5fb9cced19f7d463cd7de3df5b4a 100644 --- a/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_range_reference_filter_spec.rb @@ -42,13 +42,17 @@ module Gitlab::Markdown reference = "#{commit1.short_id}...#{commit2.id}" reference2 = "#{commit1.id}...#{commit2.short_id}" - expect(filter("See #{reference}").css('a').first.text).to eq reference - expect(filter("See #{reference2}").css('a').first.text).to eq reference2 + exp = commit1.short_id + '...' + commit2.short_id + + expect(filter("See #{reference}").css('a').first.text).to eq exp + expect(filter("See #{reference2}").css('a').first.text).to eq exp end it 'links with adjacent text' do doc = filter("See (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) + + exp = Regexp.escape("#{commit1.short_id}...#{commit2.short_id}") + expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/) end it 'ignores invalid commit IDs' do @@ -81,6 +85,11 @@ module Gitlab::Markdown expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_compare_url(project.namespace, project, from: commit1.id, to: commit2.id, only_path: true) end + + it 'adds to the results hash' do + result = pipeline_result("See #{reference}") + expect(result[:references][:commit_range]).not_to be_empty + end end context 'cross-project reference' do @@ -102,7 +111,9 @@ module Gitlab::Markdown it 'links with adjacent text' do doc = filter("Fixed (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) + + exp = Regexp.escape("#{project2.path_with_namespace}@#{commit1.short_id}...#{commit2.short_id}") + expect(doc.to_html).to match(/\(<a.+>#{exp}<\/a>\.\)/) end it 'ignores invalid commit IDs on the referenced project' do @@ -112,6 +123,11 @@ module Gitlab::Markdown exp = act = "Fixed #{project2.path_with_namespace}##{commit1.id}...#{commit2.id.reverse}" expect(filter(act).to_html).to eq exp end + + it 'adds to the results hash' do + result = pipeline_result("See #{reference}") + expect(result[:references][:commit_range]).not_to be_empty + end end context 'when user cannot access reference' do diff --git a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb index f792d7f696e07beaaa769100d20ba4ada4dc0ead..cc32a4fcf030f6fb6bef269d7587a3be150bd998 100644 --- a/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/commit_reference_filter_spec.rb @@ -27,15 +27,23 @@ module Gitlab::Markdown it "links to a valid reference of #{size} characters" do doc = filter("See #{reference[0...size]}") - expect(doc.css('a').first.text).to eq reference[0...size] + expect(doc.css('a').first.text).to eq commit.short_id expect(doc.css('a').first.attr('href')). to eq urls.namespace_project_commit_url(project.namespace, project, reference) end end + it 'always uses the short ID as the link text' do + doc = filter("See #{commit.id}") + expect(doc.text).to eq "See #{commit.short_id}" + + doc = filter("See #{commit.id[0...6]}") + expect(doc.text).to eq "See #{commit.short_id}" + end + it 'links with adjacent text' do doc = filter("See (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) + expect(doc.to_html).to match(/\(<a.+>#{commit.short_id}<\/a>\.\)/) end it 'ignores invalid commit IDs' do @@ -55,7 +63,7 @@ module Gitlab::Markdown allow_any_instance_of(Commit).to receive(:title).and_return(%{"></a>whatever<a title="}) doc = filter("See #{reference}") - expect(doc.text).to eq "See #{commit.id}" + expect(doc.text).to eq "See #{commit.short_id}" end it 'includes default classes' do @@ -75,6 +83,11 @@ module Gitlab::Markdown expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_commit_url(project.namespace, project, reference, only_path: true) end + + it 'adds to the results hash' do + result = pipeline_result("See #{reference}") + expect(result[:references][:commit]).not_to be_empty + end end context 'cross-project reference' do @@ -95,13 +108,20 @@ module Gitlab::Markdown it 'links with adjacent text' do doc = filter("Fixed (#{reference}.)") - expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(reference)}<\/a>\.\)/) + + exp = Regexp.escape(project2.path_with_namespace) + expect(doc.to_html).to match(/\(<a.+>#{exp}@#{commit.short_id}<\/a>\.\)/) end it 'ignores invalid commit IDs on the referenced project' do exp = act = "Committed #{project2.path_with_namespace}##{commit.id.reverse}" expect(filter(act).to_html).to eq exp end + + it 'adds to the results hash' do + result = pipeline_result("See #{reference}") + expect(result[:references][:commit]).not_to be_empty + end end context 'when user cannot access reference' do diff --git a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb index f95b37d69544422117aaadc35e2b50111d1ca6be..393bf32e19610c632fd310b53af2e8f4e7920902 100644 --- a/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/issue_reference_filter_spec.rb @@ -34,7 +34,7 @@ module Gitlab::Markdown end it 'links to a valid reference' do - doc = filter("See #{reference}") + doc = filter("Fixed #{reference}") expect(doc.css('a').first.attr('href')). to eq helper.url_for_issue(issue.iid, project) @@ -81,6 +81,11 @@ module Gitlab::Markdown expect(link).not_to match %r(https?://) expect(link).to eq helper.url_for_issue(issue.iid, project, only_path: true) end + + it 'adds to the results hash' do + result = pipeline_result("Fixed #{reference}") + expect(result[:references][:issue]).to eq [issue] + end end context 'cross-project reference' do @@ -117,6 +122,11 @@ module Gitlab::Markdown expect(filter(act).to_html).to eq exp end + + it 'adds to the results hash' do + result = pipeline_result("Fixed #{reference}") + expect(result[:references][:issue]).to eq [issue] + end end context 'when user cannot access reference' do diff --git a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb index c84e568e17269417d42fa28421c0f1f368f7934c..9f898837466c88cc7e7bff38c2fb23e563d2a289 100644 --- a/spec/lib/gitlab/markdown/label_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/label_reference_filter_spec.rb @@ -39,6 +39,11 @@ module Gitlab::Markdown expect(link).to eq urls.namespace_project_issues_url(project.namespace, project, label_name: label.name, only_path: true) end + it 'adds to the results hash' do + result = pipeline_result("Label #{reference}") + expect(result[:references][:label]).to eq [label] + end + describe 'label span element' do it 'includes default classes' do doc = filter("Label #{reference}") 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 0f66442269b7dba839a6f0add9005ababbb5981d..d6e745114f288aa63f5dad883a74924286014b8b 100644 --- a/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/merge_request_reference_filter_spec.rb @@ -69,6 +69,11 @@ module Gitlab::Markdown expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_merge_request_url(project.namespace, project, merge, only_path: true) end + + it 'adds to the results hash' do + result = pipeline_result("Merge #{reference}") + expect(result[:references][:merge_request]).to eq [merge] + end end context 'cross-project reference' do @@ -98,6 +103,11 @@ module Gitlab::Markdown expect(filter(act).to_html).to eq exp end + + it 'adds to the results hash' do + result = pipeline_result("Merge #{reference}") + expect(result[:references][:merge_request]).to eq [merge] + end end context 'when user cannot access reference' do diff --git a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb index 79533a90b558d2e8f7797252d632e61530d8e98f..a4b331157afe5a8d9ef229eb5ad60d8cbd3bebbf 100644 --- a/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/snippet_reference_filter_spec.rb @@ -68,6 +68,11 @@ module Gitlab::Markdown expect(link).not_to match %r(https?://) expect(link).to eq urls.namespace_project_snippet_url(project.namespace, project, snippet, only_path: true) end + + it 'adds to the results hash' do + result = pipeline_result("Snippet #{reference}") + expect(result[:references][:snippet]).to eq [snippet] + end end context 'cross-project reference' do @@ -96,6 +101,11 @@ module Gitlab::Markdown expect(filter(act).to_html).to eq exp end + + it 'adds to the results hash' do + result = pipeline_result("Snippet #{reference}") + expect(result[:references][:snippet]).to eq [snippet] + end end context 'when user cannot access reference' do diff --git a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb index a5eb927072e5f5a79baf989efdfaf7455f7eb400..922502ada334f92f75c43b4ad3499445a633c625 100644 --- a/spec/lib/gitlab/markdown/user_reference_filter_spec.rb +++ b/spec/lib/gitlab/markdown/user_reference_filter_spec.rb @@ -24,9 +24,29 @@ module Gitlab::Markdown end end + context 'mentioning @all' do + before do + project.team << [project.creator, :developer] + end + + it 'supports a special @all mention' do + doc = filter("Hey @all") + expect(doc.css('a').length).to eq 1 + expect(doc.css('a').first.attr('href')) + .to eq urls.namespace_project_url(project.namespace, project) + end + + it 'adds to the results hash' do + result = pipeline_result('Hey @all') + expect(result[:references][:user]).to eq [project.creator] + end + end + context 'mentioning a user' do + let(:reference) { "@#{user.username}" } + it 'links to a User' do - doc = filter("Hey @#{user.username}") + doc = filter("Hey #{reference}") expect(doc.css('a').first.attr('href')).to eq urls.user_url(user) end @@ -45,22 +65,45 @@ module Gitlab::Markdown doc = filter("Hey @#{user.username}") expect(doc.css('a').length).to eq 1 end + + it 'adds to the results hash' do + result = pipeline_result("Hey #{reference}") + expect(result[:references][:user]).to eq [user] + end end context 'mentioning a group' do let(:group) { create(:group) } let(:user) { create(:user) } - it 'links to a Group that the current user can read' do - group.add_user(user, Gitlab::Access::DEVELOPER) + let(:reference) { "@#{group.name}" } + + context 'that the current user can read' do + before do + group.add_user(user, Gitlab::Access::DEVELOPER) + end - doc = filter("Hey @#{group.name}", current_user: user) - expect(doc.css('a').first.attr('href')).to eq urls.group_url(group) + it 'links to the Group' do + doc = filter("Hey #{reference}", current_user: user) + expect(doc.css('a').first.attr('href')).to eq urls.group_url(group) + end + + it 'adds to the results hash' do + result = pipeline_result("Hey #{reference}", current_user: user) + expect(result[:references][:user]).to eq group.users + end end - it 'ignores references to a Group that the current user cannot read' do - doc = filter("Hey @#{group.name}", current_user: user) - expect(doc.to_html).to eq "Hey @#{group.name}" + context 'that the current user cannot read' do + it 'ignores references to the Group' do + doc = filter("Hey #{reference}", current_user: user) + expect(doc.to_html).to eq "Hey #{reference}" + end + + it 'does not add to the results hash' do + result = pipeline_result("Hey #{reference}", current_user: user) + expect(result[:references][:user]).to eq [] + end end end @@ -70,13 +113,6 @@ module Gitlab::Markdown expect(doc.to_html).to match(/\(<a.+>@#{user.username}<\/a>\.\)/) end - it 'supports a special @all mention' do - doc = filter("Hey @all") - expect(doc.css('a').length).to eq 1 - expect(doc.css('a').first.attr('href')) - .to eq urls.namespace_project_url(project.namespace, project) - end - it 'includes default classes' do doc = filter("Hey @#{user.username}") expect(doc.css('a').first.attr('class')).to eq 'gfm gfm-project_member' diff --git a/spec/lib/gitlab/reference_extractor_spec.rb b/spec/lib/gitlab/reference_extractor_spec.rb index 0f4ea2a24eddbb4e747639ec27d34bfb1ac981ea..9801dc16554112aa3138347c6b3b39fa6111e9a4 100644 --- a/spec/lib/gitlab/reference_extractor_spec.rb +++ b/spec/lib/gitlab/reference_extractor_spec.rb @@ -4,80 +4,6 @@ describe Gitlab::ReferenceExtractor do let(:project) { create(:project) } subject { Gitlab::ReferenceExtractor.new(project, project.creator) } - it 'extracts username references' do - subject.analyze('this contains a @user reference') - expect(subject.references[:user]).to eq([[project, 'user']]) - end - - it 'extracts issue references' do - subject.analyze('this one talks about issue #1234') - expect(subject.references[:issue]).to eq([[project, '1234']]) - end - - it 'extracts JIRA issue references' do - subject.analyze('this one talks about issue JIRA-1234') - expect(subject.references[:issue]).to eq([[project, 'JIRA-1234']]) - end - - it 'extracts merge request references' do - subject.analyze("and here's !43, a merge request") - expect(subject.references[:merge_request]).to eq([[project, '43']]) - end - - it 'extracts snippet ids' do - subject.analyze('snippets like $12 get extracted as well') - expect(subject.references[:snippet]).to eq([[project, '12']]) - end - - it 'extracts commit shas' do - subject.analyze('commit shas 98cf0ae3 are pulled out as Strings') - expect(subject.references[:commit]).to eq([[project, '98cf0ae3']]) - end - - it 'extracts commit ranges' do - subject.analyze('here you go, a commit range: 98cf0ae3...98cf0ae4') - expect(subject.references[:commit_range]).to eq([[project, '98cf0ae3...98cf0ae4']]) - end - - it 'extracts multiple references and preserves their order' do - subject.analyze('@me and @you both care about this') - expect(subject.references[:user]).to eq([ - [project, 'me'], - [project, 'you'] - ]) - end - - it 'leaves the original note unmodified' do - text = 'issue #123 is just the worst, @user' - subject.analyze(text) - expect(text).to eq('issue #123 is just the worst, @user') - end - - it 'extracts no references for <pre>..</pre> blocks' do - subject.analyze("<pre>def puts '#1 issue'\nend\n</pre>```") - expect(subject.issues).to be_blank - end - - it 'extracts no references for <code>..</code> blocks' do - subject.analyze("<code>def puts '!1 request'\nend\n</code>```") - expect(subject.merge_requests).to be_blank - end - - it 'extracts no references for code blocks with language' do - subject.analyze("this code:\n```ruby\ndef puts '#1 issue'\nend\n```") - expect(subject.issues).to be_blank - end - - it 'extracts issue references for invalid code blocks' do - subject.analyze('test: ```this one talks about issue #1234```') - expect(subject.references[:issue]).to eq([[project, '1234']]) - end - - it 'handles all possible kinds of references' do - accessors = described_class::TYPES.map { |t| "#{t}s".to_sym } - expect(subject).to respond_to(*accessors) - end - it 'accesses valid user objects' do @u_foo = create(:user, username: 'foo') @u_bar = create(:user, username: 'bar') @@ -139,12 +65,12 @@ describe Gitlab::ReferenceExtractor do earlier_commit = project.commit('master~2') subject.analyze("this references commits #{earlier_commit.sha[0..6]}...#{commit.sha[0..6]}") + extracted = subject.commit_ranges expect(extracted.size).to eq(1) - expect(extracted[0][0].sha).to eq(earlier_commit.sha) - expect(extracted[0][0].message).to eq(earlier_commit.message) - expect(extracted[0][1].sha).to eq(commit.sha) - expect(extracted[0][1].message).to eq(commit.message) + expect(extracted.first).to be_kind_of(CommitRange) + expect(extracted.first.commit_from).to eq earlier_commit + expect(extracted.first.commit_to).to eq commit end context 'with a project with an underscore' do diff --git a/spec/models/commit_range_spec.rb b/spec/models/commit_range_spec.rb new file mode 100644 index 0000000000000000000000000000000000000000..31ee3e99cad6eccbecb79e94406571944ffcf6b0 --- /dev/null +++ b/spec/models/commit_range_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe CommitRange do + let(:sha_from) { 'f3f85602' } + let(:sha_to) { 'e86e1013' } + + let(:range) { described_class.new("#{sha_from}...#{sha_to}") } + let(:range2) { described_class.new("#{sha_from}..#{sha_to}") } + + it 'raises ArgumentError when given an invalid range string' do + expect { described_class.new("Foo") }.to raise_error + end + + describe '#to_s' do + it 'is correct for three-dot syntax' do + expect(range.to_s).to eq "#{sha_from[0..7]}...#{sha_to[0..7]}" + end + + it 'is correct for two-dot syntax' do + expect(range2.to_s).to eq "#{sha_from[0..7]}..#{sha_to[0..7]}" + end + end + + describe '#reference_title' do + it 'returns the correct String for three-dot ranges' do + expect(range.reference_title).to eq "Commits #{sha_from} through #{sha_to}" + end + + it 'returns the correct String for two-dot ranges' do + expect(range2.reference_title).to eq "Commits #{sha_from}^ through #{sha_to}" + end + end + + describe '#to_param' do + it 'includes the correct keys' do + expect(range.to_param.keys).to eq %i(from to) + end + + it 'includes the correct values for a three-dot range' do + expect(range.to_param).to eq({from: sha_from, to: sha_to}) + end + + it 'includes the correct values for a two-dot range' do + expect(range2.to_param).to eq({from: sha_from + '^', to: sha_to}) + end + end + + describe '#exclude_start?' do + it 'is false for three-dot ranges' do + expect(range.exclude_start?).to eq false + end + + it 'is true for two-dot ranges' do + expect(range2.exclude_start?).to eq true + end + end + + describe '#valid_commits?' do + context 'without a project' do + it 'returns nil' do + expect(range.valid_commits?).to be_nil + end + end + + it 'accepts an optional project argument' do + project1 = double('project1').as_null_object + project2 = double('project2').as_null_object + + # project1 gets assigned through the accessor, but ignored when not given + # as an argument to `valid_commits?` + expect(project1).not_to receive(:present?) + range.project = project1 + + # project2 gets passed to `valid_commits?` + expect(project2).to receive(:present?).and_return(false) + + range.valid_commits?(project2) + end + + context 'with a project' do + let(:project) { double('project', repository: double('repository')) } + + context 'with a valid repo' do + before do + expect(project).to receive(:valid_repo?).and_return(true) + range.project = project + end + + it 'is false when `sha_from` is invalid' do + expect(project.repository).to receive(:commit).with(sha_from).and_return(false) + expect(project.repository).not_to receive(:commit).with(sha_to) + expect(range).not_to be_valid_commits + end + + it 'is false when `sha_to` is invalid' do + expect(project.repository).to receive(:commit).with(sha_from).and_return(true) + expect(project.repository).to receive(:commit).with(sha_to).and_return(false) + expect(range).not_to be_valid_commits + end + + it 'is true when both `sha_from` and `sha_to` are valid' do + expect(project.repository).to receive(:commit).with(sha_from).and_return(true) + expect(project.repository).to receive(:commit).with(sha_to).and_return(true) + expect(range).to be_valid_commits + end + end + + context 'without a valid repo' do + before do + expect(project).to receive(:valid_repo?).and_return(false) + range.project = project + end + + it 'returns false' do + expect(range).not_to be_valid_commits + end + end + end + end +end diff --git a/spec/support/mentionable_shared_examples.rb b/spec/support/mentionable_shared_examples.rb index 9d0af29ff9927121eb314d8f86c4f0b8e07d73c7..53fb654555397953f1907fbf510207a31bbf9161 100644 --- a/spec/support/mentionable_shared_examples.rb +++ b/spec/support/mentionable_shared_examples.rb @@ -53,7 +53,7 @@ def common_mentionable_setup extra_commits.each { |c| commitmap[c.short_id] = c } allow(project.repository).to receive(:commit) { |sha| commitmap[sha] } - + set_mentionable_text.call(ref_string) end end diff --git a/spec/support/reference_filter_spec_helper.rb b/spec/support/reference_filter_spec_helper.rb index bcee5715cad051658e518e7ff590799ee5598a50..41253811490e0872423b0c91bd3071d7736342f4 100644 --- a/spec/support/reference_filter_spec_helper.rb +++ b/spec/support/reference_filter_spec_helper.rb @@ -35,6 +35,20 @@ module ReferenceFilterSpecHelper described_class.call(html, contexts) end + # Run text through HTML::Pipeline with the current filter and return the + # result Hash + # + # body - String text to run through the pipeline + # contexts - Hash context for the filter. (default: {project: project}) + # + # Returns the Hash of the pipeline result + def pipeline_result(body, contexts = {}) + contexts.reverse_merge!(project: project) + + pipeline = HTML::Pipeline.new([described_class], contexts) + pipeline.call(body) + end + def allow_cross_reference! allow_any_instance_of(described_class). to receive(:user_can_reference_project?).and_return(true)