From f44eba8c0e537362ccf362480517f12ee7c73130 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Sun, 2 Jun 2019 10:23:53 -0700 Subject: [PATCH 1/5] Lock suggestions_filter_enabled as true Most/all of the work we're doing in this method is done at creation/edit time, so do we need to also check at render? Assume if you've gotten to this point, it's ok to enable the suggestion filter. --- app/models/diff_note.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index 8221b7de2b6..f75c32633b1 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -90,7 +90,7 @@ class DiffNote < Note end def banzai_render_context(field) - super.merge(project: project, suggestions_filter_enabled: supports_suggestion?) + super.merge(suggestions_filter_enabled: true) end private From fb3d40cb61eaff46bc10d3d49d499bd5d34a6be1 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Mon, 3 Jun 2019 14:52:58 -0700 Subject: [PATCH 2/5] Extend #parse to accept a suggestions_filter_enabled param This will allow the front end to specify the behavior as needed. --- app/services/preview_markdown_service.rb | 4 +++- lib/gitlab/diff/suggestions_parser.rb | 6 ++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/app/services/preview_markdown_service.rb b/app/services/preview_markdown_service.rb index 7386530f45f..2b4c4ae68e2 100644 --- a/app/services/preview_markdown_service.rb +++ b/app/services/preview_markdown_service.rb @@ -38,7 +38,9 @@ class PreviewMarkdownService < BaseService head_sha: params[:head_sha], start_sha: params[:start_sha]) - Gitlab::Diff::SuggestionsParser.parse(text, position: position, project: project) + Gitlab::Diff::SuggestionsParser.parse(text, position: position, + project: project, + supports_suggestion: params[:preview_suggestions]) end def preview_sugestions? diff --git a/lib/gitlab/diff/suggestions_parser.rb b/lib/gitlab/diff/suggestions_parser.rb index c8c03d5d001..6e17ffaf6ff 100644 --- a/lib/gitlab/diff/suggestions_parser.rb +++ b/lib/gitlab/diff/suggestions_parser.rb @@ -10,10 +10,12 @@ module Gitlab # Returns an array of Gitlab::Diff::Suggestion which represents each # suggestion in the given text. # - def parse(text, position:, project:) + def parse(text, position:, project:, supports_suggestion: true) return [] unless position.complete? - html = Banzai.render(text, project: nil, no_original_data: true) + html = Banzai.render(text, project: nil, + no_original_data: true, + suggestions_filter_enabled: supports_suggestion) doc = Nokogiri::HTML(html) suggestion_nodes = doc.search('pre.suggestion') From bd624741a170a30c965ff1a2bcf9a0ae1bc763e3 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Tue, 4 Jun 2019 07:50:41 -0700 Subject: [PATCH 3/5] Remove 2nd stub expectation of #last_diff_file It looks to be a stub/mock rather than strictly an expectation of the system, so dropping this to only a single invocation expected, as we've removed one of the two places #last_diff_file would be invoked. --- spec/services/suggestions/create_service_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/spec/services/suggestions/create_service_spec.rb b/spec/services/suggestions/create_service_spec.rb index ccd44e615a8..d95f9e3349b 100644 --- a/spec/services/suggestions/create_service_spec.rb +++ b/spec/services/suggestions/create_service_spec.rb @@ -96,7 +96,7 @@ describe Suggestions::CreateService do it 'creates no suggestion when diff file is not found' do expect_next_instance_of(DiffNote) do |diff_note| - expect(diff_note).to receive(:latest_diff_file).twice { nil } + expect(diff_note).to receive(:latest_diff_file).once { nil } end expect { subject.execute }.not_to change(Suggestion, :count) From 732a89ce239a2bb267aec4af222cf54d201d8e27 Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Tue, 4 Jun 2019 08:43:29 -0700 Subject: [PATCH 4/5] Update spec param expectations --- spec/services/preview_markdown_service_spec.rb | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/spec/services/preview_markdown_service_spec.rb b/spec/services/preview_markdown_service_spec.rb index f7261cd7125..d25e9958831 100644 --- a/spec/services/preview_markdown_service_spec.rb +++ b/spec/services/preview_markdown_service_spec.rb @@ -56,7 +56,9 @@ describe PreviewMarkdownService do expect(Gitlab::Diff::SuggestionsParser) .to receive(:parse) - .with(text, position: position, project: merge_request.project) + .with(text, position: position, + project: merge_request.project, + supports_suggestion: true) .and_call_original result = service.execute From 9994a5c12d6d68ffbfbd70fe210b53422dd78f4a Mon Sep 17 00:00:00 2001 From: Kerri Miller Date: Wed, 5 Jun 2019 11:12:27 -0700 Subject: [PATCH 5/5] Add changelog entry --- ...8297-remove-extraneous-gitaly-calls-from-md-rendering.yml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 changelogs/unreleased/58297-remove-extraneous-gitaly-calls-from-md-rendering.yml diff --git a/changelogs/unreleased/58297-remove-extraneous-gitaly-calls-from-md-rendering.yml b/changelogs/unreleased/58297-remove-extraneous-gitaly-calls-from-md-rendering.yml new file mode 100644 index 00000000000..25cc973159f --- /dev/null +++ b/changelogs/unreleased/58297-remove-extraneous-gitaly-calls-from-md-rendering.yml @@ -0,0 +1,5 @@ +--- +title: Reduce Gitaly calls to improve performance when rendering suggestions +merge_request: 29027 +author: +type: performance