Welcome to mirror list, hosted at ThFree Co, Russian Federation.

gitlab.com/gitlab-org/gitlab-foss.git - Unnamed repository; edit this file 'description' to name the repository.
summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorStan Hu <stanhu@gmail.com>2019-06-21 01:18:51 +0300
committerStan Hu <stanhu@gmail.com>2019-06-21 01:59:41 +0300
commit1b7ab11f9489690149f7ff1b78a927e5b3bbcfe0 (patch)
tree21d761c3baf2c37021c67b753e475be266b9aae8
parent61a2ed5d0ec945e17f10b01c3eb1fc9e22c87bb1 (diff)
Omit issues links in merge request entity API response
The merge request widget has a section that includes which issues may be closed or mentioned based on the merge request description. The problem is that rendering and redacting Markdown can be expensive, especially since the browser polls for the data every 10 seconds. Since these links don't change much and are just nice to have, we only load them on first page load. The frontend will use the existing data if the data doesn't appear on subsequent requests. This saves about 30% of the rendering time of this endpoint, which adds up to significant savings considering that `MergeRequestsController#show.json` is called over a million times a day on GitLab.com. Closes https://gitlab.com/gitlab-org/gitlab-ce/issues/63546
-rw-r--r--app/helpers/issuables_helper.rb4
-rw-r--r--app/serializers/merge_request_widget_entity.rb5
-rw-r--r--app/views/projects/merge_requests/show.html.haml2
-rw-r--r--changelogs/unreleased/sh-omit-issues-links-on-poll.yml5
-rw-r--r--spec/serializers/merge_request_widget_entity_spec.rb13
5 files changed, 25 insertions, 4 deletions
diff --git a/app/helpers/issuables_helper.rb b/app/helpers/issuables_helper.rb
index 9a12db258d5..150f24a5d5b 100644
--- a/app/helpers/issuables_helper.rb
+++ b/app/helpers/issuables_helper.rb
@@ -74,7 +74,7 @@ module IssuablesHelper
end
end
- def serialize_issuable(issuable, serializer: nil)
+ def serialize_issuable(issuable, opts = {})
serializer_klass = case issuable
when Issue
IssueSerializer
@@ -84,7 +84,7 @@ module IssuablesHelper
serializer_klass
.new(current_user: current_user, project: issuable.project)
- .represent(issuable, serializer: serializer)
+ .represent(issuable, opts)
.to_json
end
diff --git a/app/serializers/merge_request_widget_entity.rb b/app/serializers/merge_request_widget_entity.rb
index a428930dbbf..43aced598a9 100644
--- a/app/serializers/merge_request_widget_entity.rb
+++ b/app/serializers/merge_request_widget_entity.rb
@@ -114,7 +114,10 @@ class MergeRequestWidgetEntity < IssuableEntity
presenter(merge_request).ci_status
end
- expose :issues_links do
+ # Rendering and redacting Markdown can be expensive. These links are
+ # just nice to have in the merge request widget, so only
+ # include them if they are explicitly requested on first load.
+ expose :issues_links, if: -> (_, opts) { opts[:issues_links] } do
expose :assign_to_closing do |merge_request|
presenter(merge_request).assign_to_closing_issues_link
end
diff --git a/app/views/projects/merge_requests/show.html.haml b/app/views/projects/merge_requests/show.html.haml
index f593f4e049e..2084ca6f905 100644
--- a/app/views/projects/merge_requests/show.html.haml
+++ b/app/views/projects/merge_requests/show.html.haml
@@ -19,7 +19,7 @@
-# haml-lint:disable InlineJavaScript
:javascript
window.gl = window.gl || {};
- window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget')}
+ window.gl.mrWidgetData = #{serialize_issuable(@merge_request, serializer: 'widget', issues_links: true)}
window.gl.mrWidgetData.squash_before_merge_help_path = '#{help_page_path("user/project/merge_requests/squash_and_merge")}';
window.gl.mrWidgetData.troubleshooting_docs_path = '#{help_page_path('user/project/merge_requests/index.md', anchor: 'troubleshooting')}';
diff --git a/changelogs/unreleased/sh-omit-issues-links-on-poll.yml b/changelogs/unreleased/sh-omit-issues-links-on-poll.yml
new file mode 100644
index 00000000000..21e51d3534f
--- /dev/null
+++ b/changelogs/unreleased/sh-omit-issues-links-on-poll.yml
@@ -0,0 +1,5 @@
+---
+title: Omit issues links in merge request entity API response
+merge_request: 29917
+author:
+type: performance
diff --git a/spec/serializers/merge_request_widget_entity_spec.rb b/spec/serializers/merge_request_widget_entity_spec.rb
index a27c22191f4..ffbfac9b326 100644
--- a/spec/serializers/merge_request_widget_entity_spec.rb
+++ b/spec/serializers/merge_request_widget_entity_spec.rb
@@ -32,6 +32,19 @@ describe MergeRequestWidgetEntity do
end
end
+ describe 'issues links' do
+ it 'includes issues links when requested' do
+ data = described_class.new(resource, request: request, issues_links: true).as_json
+
+ expect(data).to include(:issues_links)
+ expect(data[:issues_links]).to include(:assign_to_closing, :closing, :mentioned_but_not_closing)
+ end
+
+ it 'omits issue links by default' do
+ expect(subject).not_to include(:issues_links)
+ end
+ end
+
describe 'pipeline' do
let(:pipeline) { create(:ci_empty_pipeline, project: project, ref: resource.source_branch, sha: resource.source_branch_sha, head_pipeline_of: resource) }