diff options
| -rw-r--r-- | app/controllers/projects/issues_controller.rb | 6 | ||||
| -rw-r--r-- | app/controllers/projects/merge_requests_controller.rb | 24 | ||||
| -rw-r--r-- | app/controllers/projects/notes_controller.rb | 10 | ||||
| -rw-r--r-- | app/models/note.rb | 4 | ||||
| -rw-r--r-- | app/views/projects/notes/_note.html.haml | 2 | ||||
| -rw-r--r-- | config/initializers/metrics.rb | 4 | ||||
| -rw-r--r-- | lib/banzai/filter/redactor_filter.rb | 29 | ||||
| -rw-r--r-- | lib/banzai/note_renderer.rb | 22 | ||||
| -rw-r--r-- | lib/banzai/object_renderer.rb | 85 | ||||
| -rw-r--r-- | lib/banzai/pipeline/relative_link_pipeline.rb | 11 | ||||
| -rw-r--r-- | lib/banzai/redactor.rb | 69 | ||||
| -rw-r--r-- | spec/lib/banzai/note_renderer_spec.rb | 25 | ||||
| -rw-r--r-- | spec/lib/banzai/object_renderer_spec.rb | 120 | ||||
| -rw-r--r-- | spec/lib/banzai/redactor_spec.rb | 53 |
14 files changed, 431 insertions, 33 deletions
diff --git a/app/controllers/projects/issues_controller.rb b/app/controllers/projects/issues_controller.rb index 4e2d3bebb2e..fbf8b01b7c2 100644 --- a/app/controllers/projects/issues_controller.rb +++ b/app/controllers/projects/issues_controller.rb @@ -62,8 +62,12 @@ class Projects::IssuesController < Projects::ApplicationController end def show + raw_notes = @issue.notes_with_associations.fresh + + @notes = Banzai::NoteRenderer. + render(raw_notes, @project, current_user, @path, @project_wiki, @ref) + @note = @project.notes.new(noteable: @issue) - @notes = @issue.notes.with_associations.fresh @noteable = @issue respond_to do |format| diff --git a/app/controllers/projects/merge_requests_controller.rb b/app/controllers/projects/merge_requests_controller.rb index 851822d805a..a4974aac5b7 100644 --- a/app/controllers/projects/merge_requests_controller.rb +++ b/app/controllers/projects/merge_requests_controller.rb @@ -85,6 +85,15 @@ class Projects::MergeRequestsController < Projects::ApplicationController @grouped_diff_notes = @merge_request.notes.grouped_diff_notes + Banzai::NoteRenderer.render( + @grouped_diff_notes.values.flatten, + @project, + current_user, + @path, + @project_wiki, + @ref + ) + respond_to do |format| format.html format.json { render json: { html: view_to_html_string("projects/merge_requests/show/_diffs") } } @@ -204,7 +213,7 @@ class Projects::MergeRequestsController < Projects::ApplicationController @merge_request.update(merge_error: nil) - if params[:merge_when_build_succeeds].present? + if params[:merge_when_build_succeeds].present? if @merge_request.pipeline && @merge_request.pipeline.active? MergeRequests::MergeWhenBuildSucceedsService.new(@project, current_user, merge_params) .execute(@merge_request) @@ -320,8 +329,17 @@ class Projects::MergeRequestsController < Projects::ApplicationController def define_show_vars # Build a note object for comment form @note = @project.notes.new(noteable: @merge_request) - @notes = @merge_request.mr_and_commit_notes.inc_author.fresh - @discussions = @notes.discussions + + @discussions = @merge_request.mr_and_commit_notes.inc_author.fresh. + discussions + + @notes = Banzai::NoteRenderer.render(@discussions.flatten, + @project, + current_user, + @path, + @project_wiki, + @ref) + @noteable = @merge_request # Get commits from repository diff --git a/app/controllers/projects/notes_controller.rb b/app/controllers/projects/notes_controller.rb index 836f79ff080..e14fe26dde7 100644 --- a/app/controllers/projects/notes_controller.rb +++ b/app/controllers/projects/notes_controller.rb @@ -24,6 +24,10 @@ class Projects::NotesController < Projects::ApplicationController def create @note = Notes::CreateService.new(project, current_user, note_params).execute + if @note.is_a?(Note) + Banzai::NoteRenderer.render([@note], @project, current_user) + end + respond_to do |format| format.json { render json: note_json(@note) } format.html { redirect_back_or_default } @@ -33,6 +37,10 @@ class Projects::NotesController < Projects::ApplicationController def update @note = Notes::UpdateService.new(project, current_user, note_params).execute(note) + if @note.is_a?(Note) + Banzai::NoteRenderer.render([@note], @project, current_user) + end + respond_to do |format| format.json { render json: note_json(@note) } format.html { redirect_back_or_default } @@ -118,6 +126,8 @@ class Projects::NotesController < Projects::ApplicationController name: note.name } elsif note.valid? + Banzai::NoteRenderer.render([note], @project, current_user) + { valid: true, id: note.id, diff --git a/app/models/note.rb b/app/models/note.rb index 8d164647550..656c65cf564 100644 --- a/app/models/note.rb +++ b/app/models/note.rb @@ -6,6 +6,10 @@ class Note < ActiveRecord::Base include Awardable include Importable + # Attribute containing rendered and redacted Markdown as generated by + # Banzai::ObjectRenderer. + attr_accessor :note_html + default_value_for :system, false attr_mentionable :note, pipeline: :note diff --git a/app/views/projects/notes/_note.html.haml b/app/views/projects/notes/_note.html.haml index c04d291412c..a5e163b91e9 100644 --- a/app/views/projects/notes/_note.html.haml +++ b/app/views/projects/notes/_note.html.haml @@ -32,7 +32,7 @@ .note-body{class: note_editable ? 'js-task-list-container' : ''} .note-text = preserve do - = markdown(note.note, pipeline: :note, cache_key: [note, "note"], author: note.author) + = note.note_html = edited_time_ago_with_tooltip(note, placement: 'bottom', html_class: 'note_edited_ago', include_author: true) - if note_editable = render 'projects/notes/edit_form', note: note diff --git a/config/initializers/metrics.rb b/config/initializers/metrics.rb index d159f4eded2..75f89d524e7 100644 --- a/config/initializers/metrics.rb +++ b/config/initializers/metrics.rb @@ -113,6 +113,10 @@ if Gitlab::Metrics.enabled? config.instrument_methods(Banzai::Renderer) config.instrument_methods(Banzai::Querying) + config.instrument_instance_methods(Banzai::ObjectRenderer) + config.instrument_instance_methods(Banzai::Redactor) + config.instrument_methods(Banzai::NoteRenderer) + [Issuable, Mentionable, Participable].each do |klass| config.instrument_instance_methods(klass) config.instrument_instance_methods(klass::ClassMethods) diff --git a/lib/banzai/filter/redactor_filter.rb b/lib/banzai/filter/redactor_filter.rb index c753a84a20d..c59a80dd1c7 100644 --- a/lib/banzai/filter/redactor_filter.rb +++ b/lib/banzai/filter/redactor_filter.rb @@ -7,40 +7,13 @@ module Banzai # class RedactorFilter < HTML::Pipeline::Filter def call - 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 - node.replace(text) - end - end + Redactor.new(project, current_user).redact([doc]) doc end private - 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) - - visible.merge(parser.nodes_visible_to_user(current_user, nodes)) - end - - visible - end - def current_user context[:current_user] end diff --git a/lib/banzai/note_renderer.rb b/lib/banzai/note_renderer.rb new file mode 100644 index 00000000000..bab6a9934d1 --- /dev/null +++ b/lib/banzai/note_renderer.rb @@ -0,0 +1,22 @@ +module Banzai + module NoteRenderer + # Renders a collection of Note instances. + # + # notes - The notes to render. + # project - The project to use for rendering/redacting. + # user - The user viewing the notes. + # path - The request path. + # wiki - The project's wiki. + # git_ref - The current Git reference. + def self.render(notes, project, user = nil, path = nil, wiki = nil, git_ref = nil) + renderer = ObjectRenderer.new(project, + user, + requested_path: path, + project_wiki: wiki, + ref: git_ref, + pipeline: :note) + + renderer.render(notes, :note) + end + end +end diff --git a/lib/banzai/object_renderer.rb b/lib/banzai/object_renderer.rb new file mode 100644 index 00000000000..f0e4f28bf12 --- /dev/null +++ b/lib/banzai/object_renderer.rb @@ -0,0 +1,85 @@ +module Banzai + # Class for rendering multiple objects (e.g. Note instances) in a single pass. + # + # Rendered Markdown is stored in an attribute in every object based on the + # name of the attribute containing the Markdown. For example, when the + # attribute `note` is rendered the HTML is stored in `note_html`. + class ObjectRenderer + attr_reader :project, :user + + # Make sure to set the appropriate pipeline in the `raw_context` attribute + # (e.g. `:note` for Note instances). + # + # project - A Project to use for rendering and redacting Markdown. + # user - The user viewing the Markdown/HTML documents, if any. + # context - A Hash containing extra attributes to use in the rendering + # pipeline. + def initialize(project, user = nil, raw_context = {}) + @project = project + @user = user + @raw_context = raw_context + end + + # Renders and redacts an Array of objects. + # + # objects - The objects to render + # attribute - The attribute containing the raw Markdown to render. + # + # Returns the same input objects. + def render(objects, attribute) + documents = render_objects(objects, attribute) + redacted = redact_documents(documents) + + objects.each_with_index do |object, index| + object.__send__("#{attribute}_html=", redacted.fetch(index)) + end + + objects + end + + # Renders the attribute of every given object. + def render_objects(objects, attribute) + objects.map do |object| + render_attribute(object, attribute) + end + end + + # Redacts the list of documents. + # + # Returns an Array containing the redacted documents. + def redact_documents(documents) + redactor = Redactor.new(project, user) + + redactor.redact(documents).map do |document| + document.to_html.html_safe + end + end + + # Returns a Banzai context for the given object and attribute. + def context_for(object, attribute) + context = base_context.merge(cache_key: [object, attribute]) + + if object.respond_to?(:author) + context[:author] = object.author + end + + context + end + + # Renders the attribute of an object. + # + # Returns a `Nokogiri::HTML::Document`. + def render_attribute(object, attribute) + context = context_for(object, attribute) + + string = object.__send__(attribute) + html = Banzai.render(string, context) + + Banzai::Pipeline[:relative_link].to_document(html, context) + end + + def base_context + @base_context ||= @raw_context.merge(current_user: user, project: project) + end + end +end diff --git a/lib/banzai/pipeline/relative_link_pipeline.rb b/lib/banzai/pipeline/relative_link_pipeline.rb new file mode 100644 index 00000000000..270990e7ab4 --- /dev/null +++ b/lib/banzai/pipeline/relative_link_pipeline.rb @@ -0,0 +1,11 @@ +module Banzai + module Pipeline + class RelativeLinkPipeline < BasePipeline + def self.filters + FilterArray[ + Filter::RelativeLinkFilter + ] + end + end + end +end diff --git a/lib/banzai/redactor.rb b/lib/banzai/redactor.rb new file mode 100644 index 00000000000..ffd267d5e9a --- /dev/null +++ b/lib/banzai/redactor.rb @@ -0,0 +1,69 @@ +module Banzai + # Class for removing Markdown references a certain user is not allowed to + # view. + class Redactor + attr_reader :user, :project + + # project - A Project to use for redacting links. + # user - The currently logged in user (if any). + def initialize(project, user = nil) + @project = project + @user = user + end + + # Redacts the references in the given Array of documents. + # + # This method modifies the given documents in-place. + # + # documents - A list of HTML documents containing references to redact. + # + # Returns the documents passed as the first argument. + def redact(documents) + nodes = documents.flat_map do |document| + Querying.css(document, 'a.gfm[data-reference-type]') + end + + redact_nodes(nodes) + + documents + end + + # Redacts the given nodes + # + # nodes - An Array of HTML nodes to redact. + def redact_nodes(nodes) + 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 + node.replace(text) + end + end + end + + # Returns the nodes visible to the current user. + # + # nodes - The input nodes to check. + # + # Returns a new Array containing the visible nodes. + 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, user) + + visible.merge(parser.nodes_visible_to_user(user, nodes)) + end + + visible + end + end +end diff --git a/spec/lib/banzai/note_renderer_spec.rb b/spec/lib/banzai/note_renderer_spec.rb new file mode 100644 index 00000000000..98f76f36fd5 --- /dev/null +++ b/spec/lib/banzai/note_renderer_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +describe Banzai::NoteRenderer do + describe '.render' do + it 'renders a Note' do + note = double(:note) + project = double(:project) + wiki = double(:wiki) + user = double(:user) + + expect(Banzai::ObjectRenderer).to receive(:new). + with(project, user, + requested_path: 'foo', + project_wiki: wiki, + ref: 'bar', + pipeline: :note). + and_call_original + + expect_any_instance_of(Banzai::ObjectRenderer). + to receive(:render).with([note], :note) + + described_class.render([note], project, user, 'foo', wiki, 'bar') + end + end +end diff --git a/spec/lib/banzai/object_renderer_spec.rb b/spec/lib/banzai/object_renderer_spec.rb new file mode 100644 index 00000000000..44256b32bdc --- /dev/null +++ b/spec/lib/banzai/object_renderer_spec.rb @@ -0,0 +1,120 @@ +require 'spec_helper' + +describe Banzai::ObjectRenderer do + let(:project) { create(:empty_project) } + let(:user) { project.owner } + + describe '#render' do + it 'renders and redacts an Array of objects' do + renderer = described_class.new(project, user) + object = double(:object, note: 'hello', note_html: nil) + + expect(renderer).to receive(:render_objects).with([object], :note). + and_call_original + + expect(renderer).to receive(:redact_documents). + with(an_instance_of(Array)). + and_call_original + + expect(object).to receive(:note_html=).with('<p>hello</p>') + + renderer.render([object], :note) + end + end + + describe '#render_objects' do + it 'renders an Array of objects' do + object = double(:object, note: 'hello') + renderer = described_class.new(project, user) + + expect(renderer).to receive(:render_attribute).with(object, :note). + and_call_original + + rendered = renderer.render_objects([object], :note) + + expect(rendered).to be_an_instance_of(Array) + expect(rendered[0]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) + end + end + + describe '#redact_documents' do + it 'redacts a set of documents and returns them as an Array of Strings' do + doc = Nokogiri::HTML.fragment('<p>hello</p>') + renderer = described_class.new(project, user) + + expect_any_instance_of(Banzai::Redactor).to receive(:redact). + with([doc]). + and_call_original + + redacted = renderer.redact_documents([doc]) + + expect(redacted).to eq(['<p>hello</p>']) + end + end + + describe '#context_for' do + let(:object) { double(:object, note: 'hello') } + let(:renderer) { described_class.new(project, user) } + + it 'returns a Hash' do + expect(renderer.context_for(object, :note)).to be_an_instance_of(Hash) + end + + it 'includes the cache key' do + context = renderer.context_for(object, :note) + + expect(context[:cache_key]).to eq([object, :note]) + end + + context 'when the object responds to "author"' do + it 'includes the author in the context' do + expect(object).to receive(:author).and_return('Alice') + + context = renderer.context_for(object, :note) + + expect(context[:author]).to eq('Alice') + end + end + + context 'when the object does not respond to "author"' do + it 'does not include the author in the context' do + context = renderer.context_for(object, :note) + + expect(context.key?(:author)).to eq(false) + end + end + end + + describe '#render_attribute' do + it 'renders the attribute of an object' do + object = double(:doc, note: 'hello') + renderer = described_class.new(project, user, pipeline: :note) + doc = renderer.render_attribute(object, :note) + + expect(doc).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) + expect(doc.to_html).to eq('<p>hello</p>') + end + end + + describe '#base_context' do + let(:context) do + described_class.new(project, user, pipeline: :note).base_context + end + + it 'returns a Hash' do + expect(context).to be_an_instance_of(Hash) + end + + it 'includes the custom attributes' do + expect(context[:pipeline]).to eq(:note) + end + + it 'includes the current user' do + expect(context[:current_user]).to eq(user) + end + + it 'includes the current project' do + expect(context[:project]).to eq(project) + end + end +end diff --git a/spec/lib/banzai/redactor_spec.rb b/spec/lib/banzai/redactor_spec.rb new file mode 100644 index 00000000000..488f465bcda --- /dev/null +++ b/spec/lib/banzai/redactor_spec.rb @@ -0,0 +1,53 @@ +require 'spec_helper' + +describe Banzai::Redactor do + let(:user) { build(:user) } + let(:project) { build(:empty_project) } + let(:redactor) { described_class.new(project, user) } + + describe '#redact' do + it 'redacts an Array of documents' do + doc1 = Nokogiri::HTML. + fragment('<a class="gfm" data-reference-type="issue">foo</a>') + + doc2 = Nokogiri::HTML. + fragment('<a class="gfm" data-reference-type="issue">bar</a>') + + expect(redactor).to receive(:nodes_visible_to_user).and_return([]) + + expect(redactor.redact([doc1, doc2])).to eq([doc1, doc2]) + + expect(doc1.to_html).to eq('foo') + expect(doc2.to_html).to eq('bar') + end + end + + describe '#redact_nodes' do + it 'redacts an Array of nodes' do + doc = Nokogiri::HTML.fragment('<a href="foo">foo</a>') + node = doc.children[0] + + expect(redactor).to receive(:nodes_visible_to_user). + with([node]). + and_return(Set.new) + + redactor.redact_nodes([node]) + + expect(doc.to_html).to eq('foo') + end + end + + describe '#nodes_visible_to_user' do + it 'returns a Set containing the visible nodes' do + doc = Nokogiri::HTML.fragment('<a data-reference-type="issue"></a>') + node = doc.children[0] + + expect_any_instance_of(Banzai::ReferenceParser::IssueParser). + to receive(:nodes_visible_to_user). + with(user, [node]). + and_return([node]) + + expect(redactor.nodes_visible_to_user([node])).to eq(Set.new([node])) + end + end +end |