diff options
author | Oswaldo Ferreira <oswaldo@gitlab.com> | 2018-12-04 20:17:10 -0200 |
---|---|---|
committer | Stan Hu <stanhu@gmail.com> | 2018-12-06 17:07:49 -0800 |
commit | 26b94bcc7d510482eea253b02dcc4ff22c48d68a (patch) | |
tree | 68a0f8dddb009b03af5de530fe4feb94a289dc6d /spec | |
parent | 9a78524f465488b2b5daa3fbdb2ec7c71c9641a6 (diff) | |
download | gitlab-ce-26b94bcc7d510482eea253b02dcc4ff22c48d68a.tar.gz |
Remove unused data from discussions endpoint
We don't need a series of attributes to render diff files on
discussions.json request. Therefore this MR removes lots of unnecessary
attributes from the request, mainly the highlighted diff lines, which
are pretty expensive.
Diffstat (limited to 'spec')
5 files changed, 85 insertions, 43 deletions
diff --git a/spec/javascripts/diffs/mock_data/diff_discussions.js b/spec/javascripts/diffs/mock_data/diff_discussions.js index 5ffe5a366ba..44313caba29 100644 --- a/spec/javascripts/diffs/mock_data/diff_discussions.js +++ b/spec/javascripts/diffs/mock_data/diff_discussions.js @@ -489,8 +489,6 @@ export default { diff_discussion: true, truncated_diff_lines: '<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="1">\n1\n</td>\n<td class="line_content new noteable_line"><span id="LC1" class="line" lang="plaintext"> - Bad dates</span>\n</td>\n</tr>\n<tr class="line_holder new" id="">\n<td class="diff-line-num new old_line" data-linenumber="1">\n \n</td>\n<td class="diff-line-num new new_line" data-linenumber="2">\n2\n</td>\n<td class="line_content new noteable_line"><span id="LC2" class="line" lang="plaintext"></span>\n</td>\n</tr>\n', - image_diff_html: - '<div class="image js-replaced-image" data="">\n<div class="two-up view">\n<div class="wrap">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n<div class="wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{"base_sha":"e63f41fe459e62e1228fcef60d7189127aeba95a","start_sha":"d9eaefe5a676b820c57ff18cf5b68316025f7962","head_sha":"c48ee0d1bf3b30453f5b32250ce03134beaa6d13","old_path":"CHANGELOG","new_path":"CHANGELOG","position_type":"text","old_line":null,"new_line":2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<p class="image-info hide">\n<span class="meta-filesize">22.3 KB</span>\n|\n<strong>W:</strong>\n<span class="meta-width"></span>\n|\n<strong>H:</strong>\n<span class="meta-height"></span>\n</p>\n</div>\n</div>\n<div class="swipe view hide">\n<div class="swipe-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="swipe-wrap">\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{"base_sha":"e63f41fe459e62e1228fcef60d7189127aeba95a","start_sha":"d9eaefe5a676b820c57ff18cf5b68316025f7962","head_sha":"c48ee0d1bf3b30453f5b32250ce03134beaa6d13","old_path":"CHANGELOG","new_path":"CHANGELOG","position_type":"text","old_line":null,"new_line":2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n</div>\n<span class="swipe-bar">\n<span class="top-handle"></span>\n<span class="bottom-handle"></span>\n</span>\n</div>\n</div>\n<div class="onion-skin view hide">\n<div class="onion-skin-frame">\n<div class="frame deleted">\n<img alt="CHANGELOG" src="http://localhost:3000/gitlab-org/gitlab-test/raw/e63f41fe459e62e1228fcef60d7189127aeba95a/CHANGELOG" />\n</div>\n<div class="added frame js-image-frame" data-note-type="DiffNote" data-position="{"base_sha":"e63f41fe459e62e1228fcef60d7189127aeba95a","start_sha":"d9eaefe5a676b820c57ff18cf5b68316025f7962","head_sha":"c48ee0d1bf3b30453f5b32250ce03134beaa6d13","old_path":"CHANGELOG","new_path":"CHANGELOG","position_type":"text","old_line":null,"new_line":2}">\n<img alt="CHANGELOG" draggable="false" src="http://localhost:3000/gitlab-org/gitlab-test/raw/c48ee0d1bf3b30453f5b32250ce03134beaa6d13/CHANGELOG" />\n</div>\n\n<div class="controls">\n<div class="transparent"></div>\n<div class="drag-track">\n<div class="dragger" style="left: 0px;"></div>\n</div>\n<div class="opaque"></div>\n</div>\n</div>\n</div>\n</div>\n<div class="view-modes hide">\n<ul class="view-modes-menu">\n<li class="two-up" data-mode="two-up">2-up</li>\n<li class="swipe" data-mode="swipe">Swipe</li>\n<li class="onion-skin" data-mode="onion-skin">Onion skin</li>\n</ul>\n</div>\n', }; export const imageDiffDiscussions = [ diff --git a/spec/serializers/diff_file_entity_spec.rb b/spec/serializers/diff_file_entity_spec.rb index 7497b8f27bd..073c13c2cbb 100644 --- a/spec/serializers/diff_file_entity_spec.rb +++ b/spec/serializers/diff_file_entity_spec.rb @@ -13,39 +13,6 @@ describe DiffFileEntity do subject { entity.as_json } - shared_examples 'diff file entity' do - it 'exposes correct attributes' do - expect(subject).to include( - :submodule, :submodule_link, :submodule_tree_url, :file_path, - :deleted_file, :old_path, :new_path, :mode_changed, - :a_mode, :b_mode, :text, :old_path_html, - :new_path_html, :highlighted_diff_lines, :parallel_diff_lines, - :blob, :file_hash, :added_lines, :removed_lines, :diff_refs, :content_sha, - :stored_externally, :external_storage, :too_large, :collapsed, :new_file, - :context_lines_path - ) - end - - it 'includes viewer' do - expect(subject[:viewer].with_indifferent_access) - .to match_schema('entities/diff_viewer') - end - - # Converted diff files from GitHub import does not contain blob file - # and content sha. - context 'when diff file does not have a blob and content sha' do - it 'exposes some attributes as nil' do - allow(diff_file).to receive(:content_sha).and_return(nil) - allow(diff_file).to receive(:blob).and_return(nil) - - expect(subject[:context_lines_path]).to be_nil - expect(subject[:view_path]).to be_nil - expect(subject[:highlighted_diff_lines]).to be_nil - expect(subject[:can_modify_blob]).to be_nil - end - end - end - context 'when there is no merge request' do it_behaves_like 'diff file entity' end diff --git a/spec/serializers/discussion_diff_file_entity_spec.rb b/spec/serializers/discussion_diff_file_entity_spec.rb new file mode 100644 index 00000000000..101ac918a98 --- /dev/null +++ b/spec/serializers/discussion_diff_file_entity_spec.rb @@ -0,0 +1,39 @@ +# frozen_string_literal: true + +require 'spec_helper' + +describe DiscussionDiffFileEntity do + include RepoHelpers + + let(:project) { create(:project, :repository) } + let(:repository) { project.repository } + let(:commit) { project.commit(sample_commit.id) } + let(:diff_refs) { commit.diff_refs } + let(:diff) { commit.raw_diffs.first } + let(:diff_file) { Gitlab::Diff::File.new(diff, diff_refs: diff_refs, repository: repository) } + let(:entity) { described_class.new(diff_file, request: {}) } + + subject { entity.as_json } + + context 'when there is no merge request' do + it_behaves_like 'diff file discussion entity' + end + + context 'when there is a merge request' do + let(:user) { create(:user) } + let(:request) { EntityRequest.new(project: project, current_user: user) } + let(:merge_request) { create(:merge_request, source_project: project, target_project: project) } + let(:entity) { described_class.new(diff_file, request: request, merge_request: merge_request) } + + it_behaves_like 'diff file discussion entity' + + it 'exposes additional attributes' do + expect(subject).to include(:edit_path) + end + + it 'exposes no diff lines' do + expect(subject).not_to include(:highlighted_diff_lines, + :parallel_diff_lines) + end + end +end diff --git a/spec/serializers/discussion_entity_spec.rb b/spec/serializers/discussion_entity_spec.rb index 0590304e832..138749b0fdf 100644 --- a/spec/serializers/discussion_entity_spec.rb +++ b/spec/serializers/discussion_entity_spec.rb @@ -74,13 +74,5 @@ describe DiscussionEntity do :active ) end - - context 'when diff file is a image' do - it 'exposes image attributes' do - allow(discussion).to receive(:on_image?).and_return(true) - - expect(subject.keys).to include(:image_diff_html) - end - end end end diff --git a/spec/support/shared_examples/serializers/diff_file_entity_examples.rb b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb new file mode 100644 index 00000000000..b8065886c42 --- /dev/null +++ b/spec/support/shared_examples/serializers/diff_file_entity_examples.rb @@ -0,0 +1,46 @@ +# frozen_string_literal: true + +shared_examples 'diff file base entity' do + it 'exposes essential attributes' do + expect(subject).to include(:content_sha, :submodule, :submodule_link, + :submodule_tree_url, :old_path_html, + :new_path_html, :blob, :can_modify_blob, + :file_hash, :file_path, :old_path, :new_path, + :collapsed, :text, :diff_refs, :stored_externally, + :external_storage, :renamed_file, :deleted_file, + :mode_changed, :a_mode, :b_mode, :new_file) + end + + # Converted diff files from GitHub import does not contain blob file + # and content sha. + context 'when diff file does not have a blob and content sha' do + it 'exposes some attributes as nil' do + allow(diff_file).to receive(:content_sha).and_return(nil) + allow(diff_file).to receive(:blob).and_return(nil) + + expect(subject[:context_lines_path]).to be_nil + expect(subject[:view_path]).to be_nil + expect(subject[:highlighted_diff_lines]).to be_nil + expect(subject[:can_modify_blob]).to be_nil + end + end +end + +shared_examples 'diff file entity' do + it_behaves_like 'diff file base entity' + + it 'exposes correct attributes' do + expect(subject).to include(:too_large, :added_lines, :removed_lines, + :context_lines_path, :highlighted_diff_lines, + :parallel_diff_lines) + end + + it 'includes viewer' do + expect(subject[:viewer].with_indifferent_access) + .to match_schema('entities/diff_viewer') + end +end + +shared_examples 'diff file discussion entity' do + it_behaves_like 'diff file base entity' +end |