diff options
| author | Yorick Peterse <yorickpeterse@gmail.com> | 2016-05-26 13:16:43 +0200 |
|---|---|---|
| committer | Yorick Peterse <yorickpeterse@gmail.com> | 2016-05-26 17:14:00 +0200 |
| commit | 86166d28029d5fcc729f7b7f5a41635c2e783a9e (patch) | |
| tree | d4e9354a4daafc7b298bc7a73980166e41d55bf7 | |
| parent | 94d5416db6415b06706204fb4a4df0100bcab7be (diff) | |
| download | gitlab-ce-86166d28029d5fcc729f7b7f5a41635c2e783a9e.tar.gz | |
Split Markdown rendering & reference gathering
This splits the Markdown rendering and reference extraction phases into
two distinct code bases. The reference extraction phase no longer relies
on the html-pipeline Gem (and any related code) and allows for
extracting of references from multiple HTML nodes in a single pass. This
means that if you want to extract user references from 200 comments you
no longer need to run 200 times N number of queries, instead only a
handful of queries may be needed.
52 files changed, 1527 insertions, 515 deletions
diff --git a/app/controllers/projects/wikis_controller.rb b/app/controllers/projects/wikis_controller.rb index 0d6c32fabd2..4b404eb03fa 100644 --- a/app/controllers/projects/wikis_controller.rb +++ b/app/controllers/projects/wikis_controller.rb @@ -91,8 +91,8 @@ class Projects::WikisController < Projects::ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) - ext.analyze(text) + ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext.analyze(text, author: current_user) render json: { body: view_context.markdown(text, pipeline: :wiki, project_wiki: @project_wiki), diff --git a/app/controllers/projects_controller.rb b/app/controllers/projects_controller.rb index 9697b88c032..f94e2a84fa2 100644 --- a/app/controllers/projects_controller.rb +++ b/app/controllers/projects_controller.rb @@ -197,8 +197,8 @@ class ProjectsController < Projects::ApplicationController def markdown_preview text = params[:text] - ext = Gitlab::ReferenceExtractor.new(@project, current_user, current_user) - ext.analyze(text) + ext = Gitlab::ReferenceExtractor.new(@project, current_user) + ext.analyze(text, author: current_user) render json: { body: view_context.markdown(text), diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb index b8962379cb5..298a88ba08a 100644 --- a/lib/banzai/filter/abstract_reference_filter.rb +++ b/lib/banzai/filter/abstract_reference_filter.rb @@ -18,10 +18,6 @@ module Banzai @object_sym ||= object_name.to_sym end - def self.data_reference - @data_reference ||= "data-#{object_name.dasherize}" - end - def self.object_class_title @object_title ||= object_class.name.titleize end @@ -45,10 +41,6 @@ module Banzai end end - def self.referenced_by(node) - { object_sym => LazyReference.new(object_class, node.attr(data_reference)) } - end - def object_class self.class.object_class end diff --git a/lib/banzai/filter/commit_range_reference_filter.rb b/lib/banzai/filter/commit_range_reference_filter.rb index b469ea0f626..bbb88c979cc 100644 --- a/lib/banzai/filter/commit_range_reference_filter.rb +++ b/lib/banzai/filter/commit_range_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # This filter supports cross-project references. class CommitRangeReferenceFilter < AbstractReferenceFilter + self.reference_type = :commit_range + def self.object_class CommitRange end @@ -14,34 +16,18 @@ module Banzai 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 = find_object(project, id) - - return unless range - - { commit_range: range } - end - def initialize(*args) super @commit_map = {} end - def self.find_object(project, id) + def find_object(project, id) range = CommitRange.new(id, project) range.valid_commits? ? range : nil end - def find_object(*args) - self.class.find_object(*args) - end - def url_for_object(range, project) h = Gitlab::Routing.url_helpers h.namespace_project_compare_url(project.namespace, project, diff --git a/lib/banzai/filter/commit_reference_filter.rb b/lib/banzai/filter/commit_reference_filter.rb index bd88207326c..2ce1816672b 100644 --- a/lib/banzai/filter/commit_reference_filter.rb +++ b/lib/banzai/filter/commit_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # This filter supports cross-project references. class CommitReferenceFilter < AbstractReferenceFilter + self.reference_type = :commit + def self.object_class Commit end @@ -14,28 +16,12 @@ module Banzai 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 = find_object(project, id) - - return unless commit - - { commit: commit } - end - - def self.find_object(project, id) + def find_object(project, id) if project && project.valid_repo? project.commit(id) end end - def find_object(*args) - self.class.find_object(*args) - end - def url_for_object(commit, project) h = Gitlab::Routing.url_helpers h.namespace_project_commit_url(project.namespace, project, commit, diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb index 37344b90576..eaa702952cc 100644 --- a/lib/banzai/filter/external_issue_reference_filter.rb +++ b/lib/banzai/filter/external_issue_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # References are ignored if the project doesn't use an external issue # tracker. class ExternalIssueReferenceFilter < ReferenceFilter + self.reference_type = :external_issue + # Public: Find `JIRA-123` issue references in text # # ExternalIssueReferenceFilter.references_in(text) do |match, issue| @@ -21,18 +23,6 @@ module Banzai end end - def self.referenced_by(node) - project = Project.find(node.attr("data-project")) rescue nil - return unless project - - id = node.attr("data-external-issue") - external_issue = ExternalIssue.new(id, project) - - return unless external_issue - - { external_issue: external_issue } - end - def call # Early return if the project isn't using an external tracker return doc if project.nil? || default_issues_tracker? diff --git a/lib/banzai/filter/issue_reference_filter.rb b/lib/banzai/filter/issue_reference_filter.rb index 59c5e89c546..2496e704002 100644 --- a/lib/banzai/filter/issue_reference_filter.rb +++ b/lib/banzai/filter/issue_reference_filter.rb @@ -5,18 +5,12 @@ module Banzai # # This filter supports cross-project references. class IssueReferenceFilter < AbstractReferenceFilter + self.reference_type = :issue + def self.object_class Issue end - def self.user_can_see_reference?(user, node, context) - # It is not possible to check access rights for external issue trackers - return true if context[:project].try(:external_issue_tracker) - - issue = Issue.find(node.attr('data-issue')) rescue nil - Ability.abilities.allowed?(user, :read_issue, issue) - end - def find_object(project, id) project.get_issue(id) end diff --git a/lib/banzai/filter/label_reference_filter.rb b/lib/banzai/filter/label_reference_filter.rb index 8488a493b55..e4d3f87d0aa 100644 --- a/lib/banzai/filter/label_reference_filter.rb +++ b/lib/banzai/filter/label_reference_filter.rb @@ -2,6 +2,8 @@ module Banzai module Filter # HTML filter that replaces label references with links. class LabelReferenceFilter < AbstractReferenceFilter + self.reference_type = :label + def self.object_class Label end diff --git a/lib/banzai/filter/merge_request_reference_filter.rb b/lib/banzai/filter/merge_request_reference_filter.rb index cad38a51851..ac5216d9cfb 100644 --- a/lib/banzai/filter/merge_request_reference_filter.rb +++ b/lib/banzai/filter/merge_request_reference_filter.rb @@ -5,6 +5,8 @@ module Banzai # # This filter supports cross-project references. class MergeRequestReferenceFilter < AbstractReferenceFilter + self.reference_type = :merge_request + def self.object_class MergeRequest end diff --git a/lib/banzai/filter/milestone_reference_filter.rb b/lib/banzai/filter/milestone_reference_filter.rb index dad0768f51b..ca686c87d97 100644 --- a/lib/banzai/filter/milestone_reference_filter.rb +++ b/lib/banzai/filter/milestone_reference_filter.rb @@ -2,6 +2,8 @@ module Banzai module Filter # HTML filter that replaces milestone references with links. class MilestoneReferenceFilter < AbstractReferenceFilter + self.reference_type = :milestone + def self.object_class Milestone end diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb index e589b5df6ec..c753a84a20d 100644 --- a/lib/banzai/filter/redactor_filter.rb +++ b/lib/banzai/filter/redactor_filter.rb @@ -7,8 +7,11 @@ module Banzai # class RedactorFilter < HTML::Pipeline::Filter def call - Querying.css(doc, 'a.gfm').each do |node| - unless user_can_see_reference?(node) + nodes = Querying.css(doc, 'a.gfm[data-reference-type]') + visible = nodes_visible_to_user(nodes) + + nodes.each do |node| + unless visible.include?(node) # The reference should be replaced by the original text, # which is not always the same as the rendered text. text = node.attr('data-original') || node.text @@ -21,20 +24,30 @@ module Banzai private - def user_can_see_reference?(node) - if node.has_attribute?('data-reference-filter') - reference_type = node.attr('data-reference-filter') - reference_filter = Banzai::Filter.const_get(reference_type) + def nodes_visible_to_user(nodes) + per_type = Hash.new { |h, k| h[k] = [] } + visible = Set.new + + nodes.each do |node| + per_type[node.attr('data-reference-type')] << node + end + + per_type.each do |type, nodes| + parser = Banzai::ReferenceParser[type].new(project, current_user) - reference_filter.user_can_see_reference?(current_user, node, context) - else - true + visible.merge(parser.nodes_visible_to_user(current_user, nodes)) end + + visible end def current_user context[:current_user] end + + def project + context[:project] + end end end end diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb index 31386cf851c..41ae0e1f9cc 100644 --- a/lib/banzai/filter/reference_filter.rb +++ b/lib/banzai/filter/reference_filter.rb @@ -8,24 +8,8 @@ module Banzai # :project (required) - Current project, ignored if reference is cross-project. # :only_path - Generate path-only links. class ReferenceFilter < HTML::Pipeline::Filter - def self.user_can_see_reference?(user, node, context) - if node.has_attribute?('data-project') - project_id = node.attr('data-project').to_i - return true if project_id == context[:project].try(:id) - - project = Project.find(project_id) rescue nil - Ability.abilities.allowed?(user, :read_project, project) - else - true - end - end - - def self.user_can_reference?(user, node, context) - true - end - - def self.referenced_by(node) - raise NotImplementedError, "#{self} does not implement #{__method__}" + class << self + attr_accessor :reference_type end # Returns a data attribute String to attach to a reference link @@ -43,7 +27,9 @@ module Banzai # # Returns a String def data_attribute(attributes = {}) - attributes[:reference_filter] = self.class.name.demodulize + attributes = attributes.reject { |_, v| v.nil? } + + attributes[:reference_type] = self.class.reference_type attributes.delete(:original) if context[:no_original_data] attributes.map { |key, value| %Q(data-#{key.to_s.dasherize}="#{escape_once(value)}") }.join(" ") end diff --git a/lib/banzai/filter/reference_gatherer_filter.rb b/lib/banzai/filter/reference_gatherer_filter.rb deleted file mode 100644 index 96fdb06304e..00000000000 --- a/lib/banzai/filter/reference_gatherer_filter.rb +++ /dev/null @@ -1,65 +0,0 @@ -module Banzai - module Filter - # HTML filter that gathers all referenced records that the current user has - # 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 - Querying.css(doc, 'a.gfm').each do |node| - gather_references(node) - end - - load_lazy_references unless ReferenceExtractor.lazy? - - doc - end - - private - - def gather_references(node) - return unless node.has_attribute?('data-reference-filter') - - reference_type = node.attr('data-reference-filter') - reference_filter = Banzai::Filter.const_get(reference_type) - - return if context[:reference_filter] && reference_filter != context[:reference_filter] - - return if author && !reference_filter.user_can_reference?(author, node, context) - - return unless reference_filter.user_can_see_reference?(current_user, node, context) - - references = reference_filter.referenced_by(node) - return unless references - - references.each do |type, values| - Array.wrap(values).each do |value| - result[:references][type] << value - end - end - end - - def load_lazy_references - refs = result[:references] - refs.each do |type, values| - refs[type] = ReferenceExtractor.lazily(values) - end - end - - def current_user - context[:current_user] - end - - def author - context[:author] - end - end - end -end diff --git a/lib/banzai/filter/snippet_reference_filter.rb b/lib/banzai/filter/snippet_reference_filter.rb index d507eb5ebe1..212a0bbf2a0 100644 --- a/lib/banzai/filter/snippet_reference_filter.rb +++ b/lib/banzai/filter/snippet_reference_filter.rb @@ -5,6 +5,8 @@ module Banzai # # This filter supports cross-project references. class SnippetReferenceFilter < AbstractReferenceFilter + self.reference_type = :snippet + def self.object_class Snippet end diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb index eea3af842b6..331d8007257 100644 --- a/lib/banzai/filter/user_reference_filter.rb +++ b/lib/banzai/filter/user_reference_filter.rb @@ -4,6 +4,8 @@ module Banzai # # A special `@all` reference is also supported. class UserReferenceFilter < ReferenceFilter + self.reference_type = :user + # Public: Find `@user` user references in text # # UserReferenceFilter.references_in(text) do |match, username| @@ -21,43 +23,6 @@ module Banzai 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: LazyReference.new(User, node.attr('data-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_see_reference?(user, node, context) - 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 self.user_can_reference?(user, node, context) - # Only team members can reference `@all` - if node.has_attribute?('data-project') - project = Project.find(node.attr('data-project')) rescue nil - return false unless project - - user && project.team.member?(user) - else - super - end - end - def call return doc if project.nil? @@ -114,9 +79,12 @@ module Banzai def link_to_all(link_text: nil) project = context[:project] + author = context[:author] + url = urls.namespace_project_url(project.namespace, project, only_path: context[:only_path]) - data = data_attribute(project: project.id) + + data = data_attribute(project: project.id, author: author.try(:id)) text = link_text || User.reference_prefix + 'all' link_tag(url, data, text) diff --git a/lib/banzai/lazy_reference.rb b/lib/banzai/lazy_reference.rb deleted file mode 100644 index 1095b4debc7..00000000000 --- a/lib/banzai/lazy_reference.rb +++ /dev/null @@ -1,25 +0,0 @@ -module Banzai - class LazyReference - def self.load(refs) - lazy_references, values = refs.partition { |ref| ref.is_a?(self) } - - lazy_values = lazy_references.group_by(&:klass).flat_map do |klass, refs| - ids = refs.flat_map(&:ids) - klass.where(id: ids) - end - - values + lazy_values - end - - attr_reader :klass, :ids - - def initialize(klass, ids) - @klass = klass - @ids = Array.wrap(ids).map(&:to_i) - end - - def load - self.klass.where(id: self.ids) - end - end -end diff --git a/lib/banzai/pipeline/reference_extraction_pipeline.rb b/lib/banzai/pipeline/reference_extraction_pipeline.rb deleted file mode 100644 index 919998380e4..00000000000 --- a/lib/banzai/pipeline/reference_extraction_pipeline.rb +++ /dev/null @@ -1,11 +0,0 @@ -module Banzai - module Pipeline - class ReferenceExtractionPipeline < BasePipeline - def self.filters - FilterArray[ - Filter::ReferenceGathererFilter - ] - end - end - end -end diff --git a/lib/banzai/reference_extractor.rb b/lib/banzai/reference_extractor.rb index f4079538ec5..bf366962aef 100644 --- a/lib/banzai/reference_extractor.rb +++ b/lib/banzai/reference_extractor.rb @@ -1,28 +1,6 @@ module Banzai # Extract possible GFM references from an arbitrary String for further processing. class ReferenceExtractor - class << self - LAZY_KEY = :banzai_reference_extractor_lazy - - def lazy? - Thread.current[LAZY_KEY] - end - - def lazily(values = nil, &block) - return (values || block.call).uniq if lazy? - - begin - Thread.current[LAZY_KEY] = true - - values ||= block.call - - Banzai::LazyReference.load(values.uniq).uniq - ensure - Thread.current[LAZY_KEY] = false - end - end - end - def initialize @texts = [] end @@ -31,23 +9,21 @@ module Banzai @texts << Renderer.render(text, context) end - def references(type, context = {}) - filter = Banzai::Filter["#{type}_reference"] + def references(type, project, current_user = nil) + processor = Banzai::ReferenceParser[type]. + new(project, current_user) + + processor.process(html_documents) + end - context.merge!( - pipeline: :reference_extraction, + private - # ReferenceGathererFilter - reference_filter: filter - ) + def html_documents + # This ensures that we don't memoize anything until we have a number of + # text blobs to parse. + return [] if @texts.empty? - self.class.lazily do - @texts.flat_map do |html| - text_context = context.dup - result = Renderer.render_result(html, text_context) - result[:references][type] - end.uniq - end + @html_documents ||= @texts.map { |html| Nokogiri::HTML.fragment(html) } end end end diff --git a/lib/banzai/reference_parser.rb b/lib/banzai/reference_parser.rb new file mode 100644 index 00000000000..557bec4316e --- /dev/null +++ b/lib/banzai/reference_parser.rb @@ -0,0 +1,14 @@ +module Banzai + module ReferenceParser + # Returns the reference parser class for the given type + # + # Example: + # + # Banzai::ReferenceParser['issue'] + # + # This would return the `Banzai::ReferenceParser::IssueParser` class. + def self.[](name) + const_get("#{name.to_s.camelize}Parser") + end + end +end diff --git a/lib/banzai/reference_parser/base_parser.rb b/lib/banzai/reference_parser/base_parser.rb new file mode 100644 index 00000000000..3d7b9c4a024 --- /dev/null +++ b/lib/banzai/reference_parser/base_parser.rb @@ -0,0 +1,204 @@ +module Banzai + module ReferenceParser + # Base class for reference parsing classes. + # + # Each parser should also specify its reference type by calling + # `self.reference_type = ...` in the body of the class. The value of this + # method should be a symbol such as `:issue` or `:merge_request`. For + # example: + # + # class IssueParser < BaseParser + # self.reference_type = :issue + # end + # + # The reference type is used to determine what nodes to pass to the + # `referenced_by` method. + # + # Parser classes should either implement the instance method + # `references_relation` or overwrite `referenced_by`. The + # `references_relation` method is supposed to return an + # ActiveRecord::Relation used as a base relation for retrieving the objects + # referenced in a set of HTML nodes. + # + # Each class can implement two additional methods: + # + # * `nodes_user_can_reference`: returns an Array of nodes the given user can + # refer to. + # * `nodes_visible_to_user`: returns an Array of nodes that are visible to + # the given user. + # + # You only need to overwrite these methods if you want to tweak who can see + # which references. For example, the IssueParser class defines its own + # `nodes_visible_to_user` method so it can ensure users can only see issues + # they have access to. + class BaseParser + class << self + attr_accessor :reference_type + end + + # Returns the attribute name containing the value for every object to be + # parsed by the current parser. + # + # For example, for a parser class that returns "Animal" objects this + # attribute would be "data-animal". + def self.data_attribute + @data_attribute ||= "data-#{reference_type.to_s.dasherize}" + end + + def initialize(project = nil, current_user = nil) + @project = project + @current_user = current_user + end + + # Returns all the nodes containing references that the user can refer to. + def nodes_user_can_reference(user, nodes) + nodes + end + + # Returns all the nodes that are visible to the given user. + def nodes_visible_to_user(user, nodes) + projects = lazy { projects_for_nodes(nodes) } + project_attr = 'data-project' + + nodes.select do |node| + if node.has_attribute?(project_attr) + node_id = node.attr(project_attr).to_i + + if project && project.id == node_id + true + else + can?(user, :read_project, projects[node_id]) + end + else + true + end + end + end + + # Returns an Array of objects referenced by any of the given HTML nodes. + def referenced_by(nodes) + ids = unique_attribute_values(nodes, self.class.data_attribute) + + references_relation.where(id: ids) + end + + # Returns the ActiveRecord::Relation to use for querying references in the + # DB. + def references_relation + raise NotImplementedError, + "#{self.class} does not implement #{__method__}" + end + + # Returns a Hash containing attribute values per project ID. + # + # The returned Hash uses the following format: + # + # { project id => [value1, value2, ...] } + # + # nodes - An Array of HTML nodes to process. + # attribute - The name of the attribute (as a String) for which to gather + # values. + # + # Returns a Hash. + def gather_attributes_per_project(nodes, attribute) + per_project = Hash.new { |hash, key| hash[key] = Set.new } + + nodes.each do |node| + project_id = node.attr('data-project').to_i + id = node.attr(attribute) + + per_project[project_id] << id if id + end + + per_project + end + + # Returns a Hash containing objects for an attribute grouped per their + # IDs. + # + # The returned Hash uses the following format: + # + # { id value => row } + # + # nodes - An Array of HTML nodes to process. + # + # collection - The model or ActiveRecord relation to use for retrieving + # rows from the database. + # + # attribute - The name of the attribute containing the primary key values + # for every row. + # + # Returns a Hash. + def grouped_objects_for_nodes(nodes, collection, attribute) + return {} if nodes.empty? + + ids = unique_attribute_values(nodes, attribute) + + collection.where(id: ids).each_with_object({}) do |row, hash| + hash[row.id] = row + end + end + + # Returns an Array containing all unique values of an attribute of the + # given nodes. + def unique_attribute_values(nodes, attribute) + values = Set.new + + nodes.each do |node| + if node.has_attribute?(attribute) + values << node.attr(attribute) + end + end + + values.to_a + end + + # Processes the list of HTML documents and returns an Array containing all + # the references. + def process(documents) + type = self.class.reference_type + + nodes = documents.flat_map do |document| + Querying.css(document, "a[data-reference-type='#{type}'].gfm").to_a + end + + gather_references(nodes) + end + + # Gathers the references for the given HTML nodes. + def gather_references(nodes) + nodes = nodes_user_can_reference(current_user, nodes) + nodes = nodes_visible_to_user(current_user, nodes) + + referenced_by(nodes) + end + + # Returns a Hash containing the projects for a given list of HTML nodes. + # + # The returned Hash uses the following format: + # + # { project ID => project } + # + def projects_for_nodes(nodes) + @projects_for_nodes ||= + grouped_objects_for_nodes(nodes, Project, 'data-project') + end + + def can?(user, permission, subject) + Ability.abilities.allowed?(user, permission, subject) + end + + def find_projects_for_hash_keys(hash) + Project.where(id: hash.keys) + end + + private + + attr_reader :current_user, :project + + def lazy(&block) + Gitlab::Lazy.new(&block) + end + end + end +end diff --git a/lib/banzai/reference_parser/commit_parser.rb b/lib/banzai/reference_parser/commit_parser.rb new file mode 100644 index 00000000000..0fee9d267de --- /dev/null +++ b/lib/banzai/reference_parser/commit_parser.rb @@ -0,0 +1,34 @@ +module Banzai + module ReferenceParser + class CommitParser < BaseParser + self.reference_type = :commit + + def referenced_by(nodes) + commit_ids = commit_ids_per_project(nodes) + projects = find_projects_for_hash_keys(commit_ids) + + projects.flat_map do |project| + find_commits(project, commit_ids[project.id]) + end + end + + def commit_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + + def find_commits(project, ids) + commits = [] + + return commits unless project.valid_repo? + + ids.each do |id| + commit = project.commit(id) + + commits << commit if commit + end + + commits + end + end + end +end diff --git a/lib/banzai/reference_parser/commit_range_parser.rb b/lib/banzai/reference_parser/commit_range_parser.rb new file mode 100644 index 00000000000..69d01f8db15 --- /dev/null +++ b/lib/banzai/reference_parser/commit_range_parser.rb @@ -0,0 +1,38 @@ +module Banzai + module ReferenceParser + class CommitRangeParser < BaseParser + self.reference_type = :commit_range + + def referenced_by(nodes) + range_ids = commit_range_ids_per_project(nodes) + projects = find_projects_for_hash_keys(range_ids) + + projects.flat_map do |project| + find_ranges(project, range_ids[project.id]) + end + end + + def commit_range_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + + def find_ranges(project, range_ids) + ranges = [] + + range_ids.each do |id| + range = find_object(project, id) + + ranges << range if range + end + + ranges + end + + def find_object(project, id) + range = CommitRange.new(id, project) + + range.valid_commits? ? range : nil + end + end + end +end diff --git a/lib/banzai/reference_parser/external_issue_parser.rb b/lib/banzai/reference_parser/external_issue_parser.rb new file mode 100644 index 00000000000..a1264db2111 --- /dev/null +++ b/lib/banzai/reference_parser/external_issue_parser.rb @@ -0,0 +1,25 @@ +module Banzai + module ReferenceParser + class ExternalIssueParser < BaseParser + self.reference_type = :external_issue + + def referenced_by(nodes) + issue_ids = issue_ids_per_project(nodes) + projects = find_projects_for_hash_keys(issue_ids) + issues = [] + + projects.each do |project| + issue_ids[project.id].each do |id| + issues << ExternalIssue.new(id, project) + end + end + + issues + end + + def issue_ids_per_project(nodes) + gather_attributes_per_project(nodes, self.class.data_attribute) + end + end + end +end diff --git a/lib/banzai/reference_parser/issue_parser.rb b/lib/banzai/reference_parser/issue_parser.rb new file mode 100644 index 00000000000..24076e3d9ec --- /dev/null +++ b/lib/banzai/reference_parser/issue_parser.rb @@ -0,0 +1,40 @@ +module Banzai + module ReferenceParser + class IssueParser < BaseParser + self.reference_type = :issue + + def nodes_visible_to_user(user, nodes) + # It is not possible to check access rights for external issue trackers + return nodes if project && project.external_issue_tracker + + issues = issues_for_nodes(nodes) + + nodes.select do |node| + issue = issue_for_node(issues, node) + + issue ? can?(user, :read_issue, issue) : false + end + end + + def referenced_by(nodes) + issues = issues_for_nodes(nodes) + + nodes.map { |node| issue_for_node(issues, node) }.uniq + end + + def issues_for_nodes(nodes) + @issues_for_nodes ||= grouped_objects_for_nodes( + nodes, + Issue.all.includes(:author, :assignee, :project), + self.class.data_attribute + ) + end + + private + + def issue_for_node(issues, node) + issues[node.attr(self.class.data_attribute).to_i] + end + end + end +end diff --git a/lib/banzai/reference_parser/label_parser.rb b/lib/banzai/reference_parser/label_parser.rb new file mode 100644 index 00000000000..e5d1eb11d7f --- /dev/null +++ b/lib/banzai/reference_parser/label_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class LabelParser < BaseParser + self.reference_type = :label + + def references_relation + Label + end + end + end +end diff --git a/lib/banzai/reference_parser/merge_request_parser.rb b/lib/banzai/reference_parser/merge_request_parser.rb new file mode 100644 index 00000000000..c9a9ca79c09 --- /dev/null +++ b/lib/banzai/reference_parser/merge_request_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class MergeRequestParser < BaseParser + self.reference_type = :merge_request + + def references_relation + MergeRequest.includes(:author, :assignee, :target_project) + end + end + end +end diff --git a/lib/banzai/reference_parser/milestone_parser.rb b/lib/banzai/reference_parser/milestone_parser.rb new file mode 100644 index 00000000000..a000ac61e5c --- /dev/null +++ b/lib/banzai/reference_parser/milestone_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class MilestoneParser < BaseParser + self.reference_type = :milestone + + def references_relation + Milestone + end + end + end +end diff --git a/lib/banzai/reference_parser/snippet_parser.rb b/lib/banzai/reference_parser/snippet_parser.rb new file mode 100644 index 00000000000..fa71b3c952a --- /dev/null +++ b/lib/banzai/reference_parser/snippet_parser.rb @@ -0,0 +1,11 @@ +module Banzai + module ReferenceParser + class SnippetParser < BaseParser + self.reference_type = :snippet + + def references_relation + Snippet + end + end + end +end diff --git a/lib/banzai/reference_parser/user_parser.rb b/lib/banzai/reference_parser/user_parser.rb new file mode 100644 index 00000000000..a12b0d19560 --- /dev/null +++ b/lib/banzai/reference_parser/user_parser.rb @@ -0,0 +1,92 @@ +module Banzai + module ReferenceParser + class UserParser < BaseParser + self.reference_type = :user + + def referenced_by(nodes) + group_ids = [] + user_ids = [] + project_ids = [] + + nodes.each do |node| + if node.has_attribute?('data-group') + group_ids << node.attr('data-group').to_i + elsif node.has_attribute?(self.class.data_attribute) + user_ids << node.attr(self.class.data_attribute).to_i + elsif node.has_attribute?('data-project') + project_ids << node.attr('data-project').to_i + end + end + + find_users_for_groups(group_ids) | find_users(user_ids) | + find_users_for_projects(project_ids) + end + + def nodes_visible_to_user(user, nodes) + group_attr = 'data-group' + groups = lazy { grouped_objects_for_nodes(nodes, Group, group_attr) } + visible = [] + remaining = [] + + nodes.each do |node| + if node.has_attribute?(group_attr) + node_group = groups[node.attr(group_attr).to_i] + + if node_group && + can?(user, :read_group, node_group) + visible << node + end + # Remaining nodes will be processed by the parent class' + # implementation of this method. + else + remaining << node + end + end + + visible + super(current_user, remaining) + end + + def nodes_user_can_reference(current_user, nodes) + project_attr = 'data-project' + author_attr = 'data-author' + + projects = lazy { projects_for_nodes(nodes) } + users = lazy { grouped_objects_for_nodes(nodes, User, author_attr) } + + nodes.select do |node| + project_id = node.attr(project_attr) + user_id = node.attr(author_attr) + + if project && project_id && project.id == project_id.to_i + true + elsif project_id && user_id + project = projects[project_id.to_i] + user = users[user_id.to_i] + + project && user ? project.team.member?(user) : false + else + true + end + end + end + + def find_users(ids) + return [] if ids.empty? + + User.where(id: ids).to_a + end + + def find_users_for_groups(ids) + return [] if ids.empty? + + User.joins(:group_members).where(members: { source_id: ids }).to_a + end + + def find_users_for_projects(ids) + return [] if ids.empty? + + Project.where(id: ids).flat_map { |p| p.team.members.to_a } + end + end + end +end diff --git a/lib/gitlab/reference_extractor.rb b/lib/gitlab/reference_extractor.rb index 13c4d64c99b..11c0b01f0dc 100644 --- a/lib/gitlab/reference_extractor.rb +++ b/lib/gitlab/reference_extractor.rb @@ -4,10 +4,9 @@ module Gitlab REFERABLES = %i(user issue label milestone merge_request snippet commit commit_range) attr_accessor :project, :current_user, :author - def initialize(project, current_user = nil, author = nil) + def initialize(project, current_user = nil) @project = project @current_user = current_user - @author = author @references = {} @@ -18,17 +17,21 @@ module Gitlab super(text, context.merge(project: project)) end + def references(type) + super(type, project, current_user) + end + REFERABLES.each do |type| define_method("#{type}s") do - @references[type] ||= references(type, reference_context) + @references[type] ||= references(type) end end def issues if project && project.jira_tracker? - @references[:external_issue] ||= references(:external_issue, reference_context) + @references[:external_issue] ||= references(:external_issue) else - @references[:issue] ||= references(:issue, reference_context) + @references[:issue] ||= references(:issue) end end @@ -46,11 +49,5 @@ module Gitlab @pattern = Regexp.union(patterns.compact) end - - private - - def reference_context - { project: project, current_user: current_user, author: author } - end end end diff --git a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb index c2a8ad36c30..593bd6d5cac 100644 --- a/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_range_reference_filter_spec.rb @@ -98,11 +98,6 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do 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 = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end context 'cross-project reference' do @@ -135,11 +130,6 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end context 'cross-project URL reference' do @@ -173,10 +163,5 @@ describe Banzai::Filter::CommitRangeReferenceFilter, lib: true do exp = act = "Fixed #{project2.to_reference}@#{commit1.id}...#{commit2.id.reverse}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit_range]).not_to be_empty - end end end diff --git a/spec/lib/banzai/filter/commit_reference_filter_spec.rb b/spec/lib/banzai/filter/commit_reference_filter_spec.rb index 63a32d9d455..d46d3f1489e 100644 --- a/spec/lib/banzai/filter/commit_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/commit_reference_filter_spec.rb @@ -93,11 +93,6 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do 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 = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end context 'cross-project reference' do @@ -124,11 +119,6 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do exp = act = "Committed #{invalidate_reference(reference)}" expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end context 'cross-project URL reference' do @@ -154,10 +144,5 @@ describe Banzai::Filter::CommitReferenceFilter, lib: true do act = "Committed #{invalidate_reference(reference)}" expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("See #{reference}") - expect(result[:references][:commit]).not_to be_empty - end end end diff --git a/spec/lib/banzai/filter/issue_reference_filter_spec.rb b/spec/lib/banzai/filter/issue_reference_filter_spec.rb index 266ebef33d6..8e6a264970d 100644 --- a/spec/lib/banzai/filter/issue_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/issue_reference_filter_spec.rb @@ -91,11 +91,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do expect(link).to eq helper.url_for_issue(issue.iid, project, only_path: true) end - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end - it 'does not process links containing issue numbers followed by text' do href = "#{reference}st" doc = reference_filter("<a href='#{href}'></a>") @@ -136,11 +131,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project URL reference' do @@ -160,11 +150,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(issue.to_reference(project))} \(comment 123\)<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project reference in link href' do @@ -184,11 +169,6 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end context 'cross-project URL in link href' do @@ -208,10 +188,5 @@ describe Banzai::Filter::IssueReferenceFilter, lib: true do doc = reference_filter("Fixed (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>Reference<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Fixed #{reference}") - expect(result[:references][:issue]).to eq [issue] - end end end diff --git a/spec/lib/banzai/filter/label_reference_filter_spec.rb b/spec/lib/banzai/filter/label_reference_filter_spec.rb index b0a38e7c251..f1064a701d8 100644 --- a/spec/lib/banzai/filter/label_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/label_reference_filter_spec.rb @@ -48,11 +48,6 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do expect(link).to eq urls.namespace_project_issues_path(project.namespace, project, label_name: label.name) end - it 'adds to the results hash' do - result = reference_pipeline_result("Label #{reference}") - expect(result[:references][:label]).to eq [label] - end - describe 'label span element' do it 'includes default classes' do doc = reference_filter("Label #{reference}") @@ -170,11 +165,6 @@ describe Banzai::Filter::LabelReferenceFilter, lib: true do expect(link).to have_attribute('data-label') expect(link.attr('data-label')).to eq label.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Label #{reference}") - expect(result[:references][:label]).to eq [label] - end end describe 'cross project label references' do diff --git a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb index 352710df307..3185e41fe5c 100644 --- a/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/merge_request_reference_filter_spec.rb @@ -78,11 +78,6 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do 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 = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end context 'cross-project reference' do @@ -109,11 +104,6 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end context 'cross-project URL reference' do @@ -133,10 +123,5 @@ describe Banzai::Filter::MergeRequestReferenceFilter, lib: true do doc = reference_filter("Merge (#{reference}.)") expect(doc.to_html).to match(/\(<a.+>#{Regexp.escape(merge.to_reference(project))} \(diffs, comment 123\)<\/a>\.\)/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Merge #{reference}") - expect(result[:references][:merge_request]).to eq [merge] - end end end diff --git a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb index bdf48eabb0e..9424f2363e1 100644 --- a/spec/lib/banzai/filter/milestone_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/milestone_reference_filter_spec.rb @@ -48,11 +48,6 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do namespace_project_milestone_path(project.namespace, project, milestone) end - it 'adds to the results hash' do - result = reference_pipeline_result("Milestone #{reference}") - expect(result[:references][:milestone]).to eq [milestone] - end - context 'Integer-based references' do it 'links to a valid reference' do doc = reference_filter("See #{reference}") @@ -151,11 +146,6 @@ describe Banzai::Filter::MilestoneReferenceFilter, lib: true do expect(link).to have_attribute('data-milestone') expect(link.attr('data-milestone')).to eq milestone.id.to_s end - - it 'adds to the results hash' do - result = reference_pipeline_result("Milestone #{reference}") - expect(result[:references][:milestone]).to eq [milestone] - end end describe 'cross project milestone references' do diff --git a/spec/lib/banzai/filter/redactor_filter_spec.rb b/spec/lib/banzai/filter/redactor_filter_spec.rb index c2c2fd0eb6a..697d10bbf70 100644 --- a/spec/lib/banzai/filter/redactor_filter_spec.rb +++ b/spec/lib/banzai/filter/redactor_filter_spec.rb @@ -16,11 +16,23 @@ describe Banzai::Filter::RedactorFilter, lib: true do end context 'with data-project' do + let(:parser_class) do + Class.new(Banzai::ReferenceParser::BaseParser) do + self.reference_type = :test + end + end + + before do + allow(Banzai::ReferenceParser).to receive(:[]). + with('test'). + and_return(parser_class) + end + it 'removes unpermitted Project references' do user = create(:user) project = create(:empty_project) - link = reference_link(project: project.id, reference_filter: 'ReferenceFilter') + link = reference_link(project: project.id, reference_type: 'test') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -31,14 +43,14 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project) project.team << [user, :master] - link = reference_link(project: project.id, reference_filter: 'ReferenceFilter') + link = reference_link(project: project.id, reference_type: 'test') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 end it 'handles invalid Project references' do - link = reference_link(project: 12345, reference_filter: 'ReferenceFilter') + link = reference_link(project: 12345, reference_type: 'test') expect { filter(link) }.not_to raise_error end @@ -51,7 +63,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: non_member) expect(doc.css('a').length).to eq 0 @@ -62,7 +74,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project, author: author) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: author) expect(doc.css('a').length).to eq 1 @@ -73,7 +85,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project, assignee: assignee) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: assignee) expect(doc.css('a').length).to eq 1 @@ -85,7 +97,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project.team << [member, :developer] issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: member) expect(doc.css('a').length).to eq 1 @@ -96,7 +108,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, :confidential, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: admin) expect(doc.css('a').length).to eq 1 @@ -108,7 +120,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do project = create(:empty_project, :public) issue = create(:issue, project: project) - link = reference_link(project: project.id, issue: issue.id, reference_filter: 'IssueReferenceFilter') + link = reference_link(project: project.id, issue: issue.id, reference_type: 'issue') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 1 @@ -121,7 +133,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do user = create(:user) group = create(:group, :private) - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') + link = reference_link(group: group.id, reference_type: 'user') doc = filter(link, current_user: user) expect(doc.css('a').length).to eq 0 @@ -132,14 +144,14 @@ describe Banzai::Filter::RedactorFilter, lib: true do group = create(:group, :private) group.add_developer(user) - link = reference_link(group: group.id, reference_filter: 'UserReferenceFilter') + link = reference_link(group: group.id, reference_type: 'user') 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: 12345, reference_filter: 'UserReferenceFilter') + link = reference_link(group: 12345, reference_type: 'user') expect { filter(link) }.not_to raise_error end @@ -149,7 +161,7 @@ describe Banzai::Filter::RedactorFilter, lib: true do it 'allows any User reference' do user = create(:user) - link = reference_link(user: user.id, reference_filter: 'UserReferenceFilter') + link = reference_link(user: user.id, reference_type: 'user') doc = filter(link) expect(doc.css('a').length).to eq 1 diff --git a/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb b/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb deleted file mode 100644 index c8b1dfdf944..00000000000 --- a/spec/lib/banzai/filter/reference_gatherer_filter_spec.rb +++ /dev/null @@ -1,87 +0,0 @@ -require 'spec_helper' - -describe Banzai::Filter::ReferenceGathererFilter, lib: true 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: 'IssueReferenceFilter') - 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: 'IssueReferenceFilter') - 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: 'IssueReferenceFilter') - - 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: 'UserReferenceFilter') - 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: 'UserReferenceFilter') - 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: 'UserReferenceFilter') - - 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: 'UserReferenceFilter') - result = pipeline_result(link) - - expect(result[:references][:user]).to eq([user]) - end - end - end -end diff --git a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb index 26466fbb180..5068ddd7faa 100644 --- a/spec/lib/banzai/filter/snippet_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/snippet_reference_filter_spec.rb @@ -77,11 +77,6 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do 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 = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end context 'cross-project reference' do @@ -107,11 +102,6 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(reference_filter(act).to_html).to eq exp end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end context 'cross-project URL reference' do @@ -137,10 +127,5 @@ describe Banzai::Filter::SnippetReferenceFilter, lib: true do expect(reference_filter(act).to_html).to match(/<a.+>#{Regexp.escape(invalidate_reference(reference))}<\/a>/) end - - it 'adds to the results hash' do - result = reference_pipeline_result("Snippet #{reference}") - expect(result[:references][:snippet]).to eq [snippet] - end end end diff --git a/spec/lib/banzai/filter/user_reference_filter_spec.rb b/spec/lib/banzai/filter/user_reference_filter_spec.rb index 8bdebae1841..d7dfd6699ef 100644 --- a/spec/lib/banzai/filter/user_reference_filter_spec.rb +++ b/spec/lib/banzai/filter/user_reference_filter_spec.rb @@ -31,28 +31,22 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do end it 'supports a special @all mention' do - doc = reference_filter("Hey #{reference}") + doc = reference_filter("Hey #{reference}", author: user) 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 - context "when the author is a member of the project" do + it 'includes a data-author attribute when there is an author' do + doc = reference_filter(reference, author: user) - it 'adds to the results hash' do - result = reference_pipeline_result("Hey #{reference}", author: project.creator) - expect(result[:references][:user]).to eq [project.creator] - end + expect(doc.css('a').first.attr('data-author')).to eq(user.id.to_s) end - context "when the author is not a member of the project" do - - let(:other_user) { create(:user) } + it 'does not include a data-author attribute when there is no author' do + doc = reference_filter(reference) - it "doesn't add to the results hash" do - result = reference_pipeline_result("Hey #{reference}", author: other_user) - expect(result[:references][:user]).to eq [] - end + expect(doc.css('a').first.has_attribute?('data-author')).to eq(false) end end @@ -83,11 +77,6 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do 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 = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [user] - end end context 'mentioning a group' do @@ -106,11 +95,6 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do 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 = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq group.users - end end it 'links with adjacent text' do @@ -151,10 +135,5 @@ describe Banzai::Filter::UserReferenceFilter, lib: true do 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 = reference_pipeline_result("Hey #{reference}") - expect(result[:references][:user]).to eq [user] - end end end diff --git a/spec/lib/banzai/reference_parser/base_parser_spec.rb b/spec/lib/banzai/reference_parser/base_parser_spec.rb new file mode 100644 index 00000000000..543b4786d84 --- /dev/null +++ b/spec/lib/banzai/reference_parser/base_parser_spec.rb @@ -0,0 +1,237 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::BaseParser, lib: true do + include ReferenceParserHelpers + + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public) } + + subject do + klass = Class.new(described_class) do + self.reference_type = :foo + end + + klass.new(project, user) + end + + describe '.reference_type=' do + it 'sets the reference type' do + dummy = Class.new(described_class) + dummy.reference_type = :foo + + expect(dummy.reference_type).to eq(:foo) + end + end + + describe '#nodes_visible_to_user' do + let(:link) { empty_html_link } + + context 'when the link has a data-project attribute' do + it 'returns the nodes if the attribute value equals the current project ID' do + link['data-project'] = project.id.to_s + + expect(Ability.abilities).not_to receive(:allowed?) + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns the nodes if the user can read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the attribute value is empty' do + link['data-project'] = '' + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user can not read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns the nodes' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + + describe '#nodes_user_can_reference' do + it 'returns the nodes' do + link = double(:link) + + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + end + + describe '#referenced_by' do + context 'when references_relation is implemented' do + it 'returns a collection of objects' do + links = Nokogiri::HTML.fragment("<a data-foo='#{user.id}'></a>"). + children + + expect(subject).to receive(:references_relation).and_return(User) + expect(subject.referenced_by(links)).to eq([user]) + end + end + + context 'when references_relation is not implemented' do + it 'raises NotImplementedError' do + links = Nokogiri::HTML.fragment('<a data-foo="1"></a>').children + + expect { subject.referenced_by(links) }. + to raise_error(NotImplementedError) + end + end + end + + describe '#references_relation' do + it 'raises NotImplementedError' do + expect { subject.references_relation }.to raise_error(NotImplementedError) + end + end + + describe '#gather_attributes_per_project' do + it 'returns a Hash containing attribute values per project' do + link = Nokogiri::HTML.fragment('<a data-project="1" data-foo="2"></a>'). + children[0] + + hash = subject.gather_attributes_per_project([link], 'data-foo') + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[1].to_a).to eq(['2']) + end + end + + describe '#grouped_objects_for_nodes' do + it 'returns a Hash grouping objects per ID' do + nodes = [double(:node)] + + expect(subject).to receive(:unique_attribute_values). + with(nodes, 'data-user'). + and_return([user.id]) + + hash = subject.grouped_objects_for_nodes(nodes, User, 'data-user') + + expect(hash).to eq({ user.id => user }) + end + + it 'returns an empty Hash when the list of nodes is empty' do + expect(subject.grouped_objects_for_nodes([], User, 'data-user')).to eq({}) + end + end + + describe '#unique_attribute_values' do + it 'returns an Array of unique values' do + link = double(:link) + + expect(link).to receive(:has_attribute?). + with('data-foo'). + twice. + and_return(true) + + expect(link).to receive(:attr). + with('data-foo'). + twice. + and_return('1') + + nodes = [link, link] + + expect(subject.unique_attribute_values(nodes, 'data-foo')).to eq(['1']) + end + end + + describe '#process' do + it 'gathers the references for every node matching the reference type' do + dummy = Class.new(described_class) do + self.reference_type = :test + end + + instance = dummy.new(project, user) + document = Nokogiri::HTML.fragment('<a class="gfm"></a><a class="gfm" data-reference-type="test"></a>') + + expect(instance).to receive(:gather_references). + with([document.children[1]]). + and_return([user]) + + expect(instance.process([document])).to eq([user]) + end + end + + describe '#gather_references' do + let(:link) { double(:link) } + + it 'does not process links a user can not reference' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([]) + + expect(subject).to receive(:referenced_by).with([]) + + subject.gather_references([link]) + end + + it 'does not process links a user can not see' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:nodes_visible_to_user). + with(user, [link]). + and_return([]) + + expect(subject).to receive(:referenced_by).with([]) + + subject.gather_references([link]) + end + + it 'returns the references if a user can reference and see a link' do + expect(subject).to receive(:nodes_user_can_reference). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:nodes_visible_to_user). + with(user, [link]). + and_return([link]) + + expect(subject).to receive(:referenced_by).with([link]) + + subject.gather_references([link]) + end + end + + describe '#can?' do + it 'delegates the permissions check to the Ability class' do + user = double(:user) + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, project) + + subject.can?(user, :read_project, project) + end + end + + describe '#find_projects_for_hash_keys' do + it 'returns a list of Projects' do + expect(subject.find_projects_for_hash_keys(project.id => project)). + to eq([project]) + end + end +end diff --git a/spec/lib/banzai/reference_parser/commit_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_parser_spec.rb new file mode 100644 index 00000000000..0b76d29fce0 --- /dev/null +++ b/spec/lib/banzai/reference_parser/commit_parser_spec.rb @@ -0,0 +1,113 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::CommitParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link has a data-commit attribute' do + before do + link['data-commit'] = '123' + end + + it 'returns an Array of commits' do + commit = double(:commit) + + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject).to receive(:find_commits). + with(project, ['123']). + and_return([commit]) + + expect(subject.referenced_by([link])).to eq([commit]) + end + + it 'returns an empty Array when the commit could not be found' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject).to receive(:find_commits). + with(project, ['123']). + and_return([]) + + expect(subject.referenced_by([link])).to eq([]) + end + + it 'skips projects without valid repositories' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(false) + + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'when the link does not have a data-commit attribute' do + it 'returns an empty Array' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + allow_any_instance_of(Project).to receive(:valid_repo?). + and_return(true) + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#commit_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing commit IDs per project' do + link['data-commit'] = '123' + + hash = subject.commit_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123']) + end + + it 'does not add a project when the data-commit attribute is empty' do + hash = subject.commit_ids_per_project([link]) + + expect(hash).to be_empty + end + end + + describe '#find_commits' do + it 'returns an Array of commit objects' do + commit = double(:commit) + + expect(project).to receive(:commit).with('123').and_return(commit) + expect(project).to receive(:valid_repo?).and_return(true) + + expect(subject.find_commits(project, %w{123})).to eq([commit]) + end + + it 'skips commit IDs for which no commit could be found' do + expect(project).to receive(:commit).with('123').and_return(nil) + expect(project).to receive(:valid_repo?).and_return(true) + + expect(subject.find_commits(project, %w{123})).to eq([]) + end + end +end diff --git a/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb new file mode 100644 index 00000000000..ba982f38542 --- /dev/null +++ b/spec/lib/banzai/reference_parser/commit_range_parser_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::CommitRangeParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link as a data-commit-range attribute' do + before do + link['data-commit-range'] = '123..456' + end + + it 'returns an Array of commit ranges' do + range = double(:range) + + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(range) + + expect(subject.referenced_by([link])).to eq([range]) + end + + it 'returns an empty Array when the commit range could not be found' do + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(nil) + + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'when the link does not have a data-commit-range attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#commit_range_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing range IDs per project' do + link['data-commit-range'] = '123..456' + + hash = subject.commit_range_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123..456']) + end + + it 'does not add a project when the data-commit-range attribute is empty' do + hash = subject.commit_range_ids_per_project([link]) + + expect(hash).to be_empty + end + end + + describe '#find_ranges' do + it 'returns an Array of range objects' do + range = double(:commit) + + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(range) + + expect(subject.find_ranges(project, ['123..456'])).to eq([range]) + end + + it 'skips ranges that could not be found' do + expect(subject).to receive(:find_object). + with(project, '123..456'). + and_return(nil) + + expect(subject.find_ranges(project, ['123..456'])).to eq([]) + end + end + + describe '#find_object' do + let(:range) { double(:range) } + + before do + expect(CommitRange).to receive(:new).and_return(range) + end + + context 'when the range has valid commits' do + it 'returns the commit range' do + expect(range).to receive(:valid_commits?).and_return(true) + + expect(subject.find_object(project, '123..456')).to eq(range) + end + end + + context 'when the range does not have any valid commits' do + it 'returns nil' do + expect(range).to receive(:valid_commits?).and_return(false) + + expect(subject.find_object(project, '123..456')).to be_nil + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb new file mode 100644 index 00000000000..a6ef8394fe7 --- /dev/null +++ b/spec/lib/banzai/reference_parser/external_issue_parser_spec.rb @@ -0,0 +1,62 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::ExternalIssueParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-project attribute' do + before do + link['data-project'] = project.id.to_s + end + + context 'when the link has a data-external-issue attribute' do + it 'returns an Array of ExternalIssue instances' do + link['data-external-issue'] = '123' + + refs = subject.referenced_by([link]) + + expect(refs).to eq([ExternalIssue.new('123', project)]) + end + end + + context 'when the link does not have a data-external-issue attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link does not have a data-project attribute' do + it 'returns an empty Array' do + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + describe '#issue_ids_per_project' do + before do + link['data-project'] = project.id.to_s + end + + it 'returns a Hash containing range IDs per project' do + link['data-external-issue'] = '123' + + hash = subject.issue_ids_per_project([link]) + + expect(hash).to be_an_instance_of(Hash) + + expect(hash[project.id].to_a).to eq(['123']) + end + + it 'does not add a project when the data-external-issue attribute is empty' do + hash = subject.issue_ids_per_project([link]) + + expect(hash).to be_empty + end + end +end diff --git a/spec/lib/banzai/reference_parser/issue_parser_spec.rb b/spec/lib/banzai/reference_parser/issue_parser_spec.rb new file mode 100644 index 00000000000..514c752546d --- /dev/null +++ b/spec/lib/banzai/reference_parser/issue_parser_spec.rb @@ -0,0 +1,79 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::IssueParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:issue) { create(:issue, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#nodes_visible_to_user' do + context 'when the link has a data-issue attribute' do + before do + link['data-issue'] = issue.id.to_s + end + + it 'returns the nodes when the user can read the issue' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_issue, issue). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the user can not read the issue' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_issue, issue). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-issue attribute' do + it 'returns an empty Array' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the project uses an external issue tracker' do + it 'returns all nodes' do + link = double(:link) + + expect(project).to receive(:external_issue_tracker).and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + + describe '#referenced_by' do + context 'when the link has a data-issue attribute' do + context 'using an existing issue ID' do + before do + link['data-issue'] = issue.id.to_s + end + + it 'returns an Array of issues' do + expect(subject.referenced_by([link])).to eq([issue]) + end + + it 'returns an empty Array when the list of nodes is empty' do + expect(subject.referenced_by([link])).to eq([issue]) + expect(subject.referenced_by([])).to eq([]) + end + end + end + end + + describe '#issues_for_nodes' do + it 'returns a Hash containing the issues for a list of nodes' do + link['data-issue'] = issue.id.to_s + nodes = [link] + + expect(subject.issues_for_nodes(nodes)).to eq({ issue.id => issue }) + end + end +end diff --git a/spec/lib/banzai/reference_parser/label_parser_spec.rb b/spec/lib/banzai/reference_parser/label_parser_spec.rb new file mode 100644 index 00000000000..77fda47f0e7 --- /dev/null +++ b/spec/lib/banzai/reference_parser/label_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::LabelParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:label) { create(:label, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-label attribute' do + context 'using an existing label ID' do + it 'returns an Array of labels' do + link['data-label'] = label.id.to_s + + expect(subject.referenced_by([link])).to eq([label]) + end + end + + context 'using a non-existing label ID' do + it 'returns an empty Array' do + link['data-label'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb new file mode 100644 index 00000000000..cf89ad598ea --- /dev/null +++ b/spec/lib/banzai/reference_parser/merge_request_parser_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::MergeRequestParser, lib: true do + include ReferenceParserHelpers + + let(:user) { create(:user) } + let(:merge_request) { create(:merge_request) } + subject { described_class.new(merge_request.target_project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-merge-request attribute' do + context 'using an existing merge request ID' do + it 'returns an Array of merge requests' do + link['data-merge-request'] = merge_request.id.to_s + + expect(subject.referenced_by([link])).to eq([merge_request]) + end + end + + context 'using a non-existing merge request ID' do + it 'returns an empty Array' do + link['data-merge-request'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/milestone_parser_spec.rb b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb new file mode 100644 index 00000000000..6aa45a22cc4 --- /dev/null +++ b/spec/lib/banzai/reference_parser/milestone_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::MilestoneParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:milestone) { create(:milestone, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-milestone attribute' do + context 'using an existing milestone ID' do + it 'returns an Array of milestones' do + link['data-milestone'] = milestone.id.to_s + + expect(subject.referenced_by([link])).to eq([milestone]) + end + end + + context 'using a non-existing milestone ID' do + it 'returns an empty Array' do + link['data-milestone'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/snippet_parser_spec.rb b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb new file mode 100644 index 00000000000..59127b7c5d1 --- /dev/null +++ b/spec/lib/banzai/reference_parser/snippet_parser_spec.rb @@ -0,0 +1,31 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::SnippetParser, lib: true do + include ReferenceParserHelpers + + let(:project) { create(:empty_project, :public) } + let(:user) { create(:user) } + let(:snippet) { create(:snippet, project: project) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + describe 'when the link has a data-snippet attribute' do + context 'using an existing snippet ID' do + it 'returns an Array of snippets' do + link['data-snippet'] = snippet.id.to_s + + expect(subject.referenced_by([link])).to eq([snippet]) + end + end + + context 'using a non-existing snippet ID' do + it 'returns an empty Array' do + link['data-snippet'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end +end diff --git a/spec/lib/banzai/reference_parser/user_parser_spec.rb b/spec/lib/banzai/reference_parser/user_parser_spec.rb new file mode 100644 index 00000000000..9a82891297d --- /dev/null +++ b/spec/lib/banzai/reference_parser/user_parser_spec.rb @@ -0,0 +1,189 @@ +require 'spec_helper' + +describe Banzai::ReferenceParser::UserParser, lib: true do + include ReferenceParserHelpers + + let(:group) { create(:group) } + let(:user) { create(:user) } + let(:project) { create(:empty_project, :public, group: group, creator: user) } + subject { described_class.new(project, user) } + let(:link) { empty_html_link } + + describe '#referenced_by' do + context 'when the link has a data-group attribute' do + context 'using an existing group ID' do + before do + link['data-group'] = project.group.id.to_s + end + + it 'returns the users of the group' do + create(:group_member, group: group, user: user) + + expect(subject.referenced_by([link])).to eq([user]) + end + + it 'returns an empty Array when the group has no users' do + expect(subject.referenced_by([link])).to eq([]) + end + end + + context 'using a non-existing group ID' do + it 'returns an empty Array' do + link['data-group'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + + context 'when the link has a data-user attribute' do + it 'returns an Array of users' do + link['data-user'] = user.id.to_s + + expect(subject.referenced_by([link])).to eq([user]) + end + end + + context 'when the link has a data-project attribute' do + context 'using an existing project ID' do + let(:contributor) { create(:user) } + + before do + project.team << [user, :developer] + project.team << [contributor, :developer] + end + + it 'returns the members of a project' do + link['data-project'] = project.id.to_s + + # This uses an explicit sort to make sure this spec doesn't randomly + # fail when objects are returned in a different order. + refs = subject.referenced_by([link]).sort_by(&:id) + + expect(refs).to eq([user, contributor]) + end + end + + context 'using a non-existing project ID' do + it 'returns an empty Array' do + link['data-project'] = '' + + expect(subject.referenced_by([link])).to eq([]) + end + end + end + end + + describe '#nodes_visible_to_use?' do + context 'when the link has a data-group attribute' do + context 'using an existing group ID' do + before do + link['data-group'] = group.id.to_s + end + + it 'returns the nodes if the user can read the group' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_group, group). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array if the user can not read the group' do + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_group, group). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-group attribute' do + context 'with a data-project attribute' do + it 'returns the nodes if the attribute value equals the current project ID' do + link['data-project'] = project.id.to_s + + expect(Ability.abilities).not_to receive(:allowed?) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns the nodes if the user can read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(true) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + + it 'returns an empty Array if the user can not read the project' do + other_project = create(:empty_project, :public) + + link['data-project'] = other_project.id.to_s + + expect(Ability.abilities).to receive(:allowed?). + with(user, :read_project, other_project). + and_return(false) + + expect(subject.nodes_visible_to_user(user, [link])).to eq([]) + end + end + + context 'without a data-project attribute' do + it 'returns the nodes' do + expect(subject.nodes_visible_to_user(user, [link])).to eq([link]) + end + end + end + end + end + + describe '#nodes_user_can_reference' do + context 'when the link has a data-author attribute' do + it 'returns the nodes when the user is a member of the project' do + other_project = create(:project) + other_project.team << [user, :developer] + + link['data-project'] = other_project.id.to_s + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + + it 'returns an empty Array when the project could not be found' do + link['data-project'] = '' + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user could not be found' do + other_project = create(:project) + + link['data-project'] = other_project.id.to_s + link['data-author'] = '' + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + + it 'returns an empty Array when the user is not a team member' do + other_project = create(:project) + + link['data-project'] = other_project.id.to_s + link['data-author'] = user.id.to_s + + expect(subject.nodes_user_can_reference(user, [link])).to eq([]) + end + end + + context 'when the link does not have a data-author attribute' do + it 'returns the nodes' do + expect(subject.nodes_user_can_reference(user, [link])).to eq([link]) + end + end + end +end diff --git a/spec/support/filter_spec_helper.rb b/spec/support/filter_spec_helper.rb index e849a9633b9..a8e454eb09e 100644 --- a/spec/support/filter_spec_helper.rb +++ b/spec/support/filter_spec_helper.rb @@ -40,8 +40,7 @@ module FilterSpecHelper filters = [ Banzai::Filter::AutolinkFilter, - described_class, - Banzai::Filter::ReferenceGathererFilter + described_class ] HTML::Pipeline.new(filters, context) diff --git a/spec/support/reference_parser_helpers.rb b/spec/support/reference_parser_helpers.rb new file mode 100644 index 00000000000..01689194eac --- /dev/null +++ b/spec/support/reference_parser_helpers.rb @@ -0,0 +1,5 @@ +module ReferenceParserHelpers + def empty_html_link + Nokogiri::HTML.fragment('<a></a>').children[0] + end +end |