diff options
26 files changed, 89 insertions, 1234 deletions
diff --git a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue index e31a3546b69..f4a9be19496 100644 --- a/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue +++ b/app/assets/javascripts/diffs/components/diff_line_gutter_content.vue @@ -55,6 +55,11 @@ export default { required: false, default: false, }, + isContextLine: { + type: Boolean, + required: false, + default: false, + }, isHover: { type: Boolean, required: false, @@ -76,6 +81,7 @@ export default { this.showCommentButton && this.isHover && !this.isMatchLine && + !this.isContextLine && !this.isMetaLine && !this.hasDiscussions ); diff --git a/app/assets/javascripts/diffs/components/diff_table_cell.vue b/app/assets/javascripts/diffs/components/diff_table_cell.vue index e26aa9c9b00..5d9a0b123fe 100644 --- a/app/assets/javascripts/diffs/components/diff_table_cell.vue +++ b/app/assets/javascripts/diffs/components/diff_table_cell.vue @@ -3,6 +3,7 @@ import { mapGetters } from 'vuex'; import DiffLineGutterContent from './diff_line_gutter_content.vue'; import { MATCH_LINE_TYPE, + CONTEXT_LINE_TYPE, EMPTY_CELL_TYPE, OLD_LINE_TYPE, OLD_NO_NEW_LINE_TYPE, @@ -70,6 +71,9 @@ export default { isMatchLine() { return this.line.type === MATCH_LINE_TYPE; }, + isContextLine() { + return this.line.type === CONTEXT_LINE_TYPE; + }, isMetaLine() { const { type } = this.line; @@ -84,7 +88,11 @@ export default { [type]: type, [LINE_UNFOLD_CLASS_NAME]: this.isMatchLine, [LINE_HOVER_CLASS_NAME]: - this.isLoggedIn && this.isHover && !this.isMatchLine && !this.isMetaLine, + this.isLoggedIn && + this.isHover && + !this.isMatchLine && + !this.isContextLine && + !this.isMetaLine, }; }, lineNumber() { diff --git a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue index 44c05e4b634..542acd3d930 100644 --- a/app/assets/javascripts/diffs/components/inline_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/inline_diff_table_row.vue @@ -4,6 +4,8 @@ import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, OLD_LINE_TYPE, + CONTEXT_LINE_TYPE, + CONTEXT_LINE_CLASS_NAME, PARALLEL_DIFF_VIEW_TYPE, LINE_POSITION_LEFT, LINE_POSITION_RIGHT, @@ -39,9 +41,13 @@ export default { }, computed: { ...mapGetters('diffs', ['isInlineView']), + isContextLine() { + return this.line.type === CONTEXT_LINE_TYPE; + }, classNameMap() { return { [this.line.type]: this.line.type, + [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, [PARALLEL_DIFF_VIEW_TYPE]: this.isParallelView, }; }, diff --git a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue index 39312cddfce..fcc3b3e9117 100644 --- a/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue +++ b/app/assets/javascripts/diffs/components/parallel_diff_table_row.vue @@ -5,6 +5,8 @@ import DiffTableCell from './diff_table_cell.vue'; import { NEW_LINE_TYPE, OLD_LINE_TYPE, + CONTEXT_LINE_TYPE, + CONTEXT_LINE_CLASS_NAME, OLD_NO_NEW_LINE_TYPE, PARALLEL_DIFF_VIEW_TYPE, NEW_NO_NEW_LINE_TYPE, @@ -41,8 +43,12 @@ export default { }; }, computed: { + isContextLine() { + return this.line.left && this.line.left.type === CONTEXT_LINE_TYPE; + }, classNameMap() { return { + [CONTEXT_LINE_CLASS_NAME]: this.isContextLine, [PARALLEL_DIFF_VIEW_TYPE]: true, }; }, diff --git a/app/assets/javascripts/diffs/constants.js b/app/assets/javascripts/diffs/constants.js index f5f5c0ffc29..78a39baa4cb 100644 --- a/app/assets/javascripts/diffs/constants.js +++ b/app/assets/javascripts/diffs/constants.js @@ -3,6 +3,7 @@ export const PARALLEL_DIFF_VIEW_TYPE = 'parallel'; export const MATCH_LINE_TYPE = 'match'; export const OLD_NO_NEW_LINE_TYPE = 'old-nonewline'; export const NEW_NO_NEW_LINE_TYPE = 'new-nonewline'; +export const CONTEXT_LINE_TYPE = 'context'; export const EMPTY_CELL_TYPE = 'empty-cell'; export const COMMENT_FORM_TYPE = 'commentForm'; export const DIFF_NOTE_TYPE = 'DiffNote'; @@ -21,6 +22,7 @@ export const LINE_SIDE_RIGHT = 'right-side'; export const DIFF_VIEW_COOKIE_NAME = 'diff_view'; export const LINE_HOVER_CLASS_NAME = 'is-over'; export const LINE_UNFOLD_CLASS_NAME = 'unfold js-unfold'; +export const CONTEXT_LINE_CLASS_NAME = 'diff-expanded'; export const UNFOLD_COUNT = 20; export const COUNT_OF_AVATARS_IN_GUTTER = 3; diff --git a/app/assets/javascripts/diffs/store/mutations.js b/app/assets/javascripts/diffs/store/mutations.js index e651c197968..a7eea2c1449 100644 --- a/app/assets/javascripts/diffs/store/mutations.js +++ b/app/assets/javascripts/diffs/store/mutations.js @@ -65,13 +65,7 @@ export default { const { highlightedDiffLines, parallelDiffLines } = diffFile; removeMatchLine(diffFile, lineNumbers, bottom); - - const lines = addLineReferences(contextLines, lineNumbers, bottom).map(line => ({ - ...line, - lineCode: line.lineCode || `${fileHash}_${line.oldLine}_${line.newLine}`, - discussions: line.discussions || [], - })); - + const lines = addLineReferences(contextLines, lineNumbers, bottom); addContextLines({ inlineLines: highlightedDiffLines, parallelLines: parallelDiffLines, diff --git a/app/controllers/projects/blob_controller.rb b/app/controllers/projects/blob_controller.rb index 2a6fe3b9c97..c02ec407262 100644 --- a/app/controllers/projects/blob_controller.rb +++ b/app/controllers/projects/blob_controller.rb @@ -122,7 +122,7 @@ class Projects::BlobController < Projects::ApplicationController @lines.map! do |line| # These are marked as context lines but are loaded from blobs. # We also have context lines loaded from diffs in other places. - diff_line = Gitlab::Diff::Line.new(line, nil, nil, nil, nil) + diff_line = Gitlab::Diff::Line.new(line, 'context', nil, nil, nil) diff_line.rich_text = line diff_line end diff --git a/app/controllers/projects/merge_requests/diffs_controller.rb b/app/controllers/projects/merge_requests/diffs_controller.rb index 740f41c0642..5307cd0720a 100644 --- a/app/controllers/projects/merge_requests/diffs_controller.rb +++ b/app/controllers/projects/merge_requests/diffs_controller.rb @@ -22,12 +22,6 @@ class Projects::MergeRequests::DiffsController < Projects::MergeRequests::Applic def render_diffs @environment = @merge_request.environments_for(current_user).last - notes_grouped_by_path = @notes.group_by { |note| note.position.file_path } - - @diffs.diff_files.each do |diff_file| - notes = notes_grouped_by_path.fetch(diff_file.file_path, []) - notes.each { |note| diff_file.unfold_diff_lines(note.position) } - end @diffs.write_cache diff --git a/app/models/diff_note.rb b/app/models/diff_note.rb index c32008aa9c7..5f59e4832db 100644 --- a/app/models/diff_note.rb +++ b/app/models/diff_note.rb @@ -66,10 +66,6 @@ class DiffNote < Note self.original_position.diff_refs == diff_refs end - def discussion_first_note? - self == discussion.first_note - end - private def enqueue_diff_file_creation_job @@ -82,33 +78,26 @@ class DiffNote < Note end def should_create_diff_file? - on_text? && note_diff_file.nil? && discussion_first_note? + on_text? && note_diff_file.nil? && self == discussion.first_note end def fetch_diff_file - file = - if note_diff_file - diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) - Gitlab::Diff::File.new(diff, - repository: project.repository, - diff_refs: original_position.diff_refs) - elsif created_at_diff?(noteable.diff_refs) - # We're able to use the already persisted diffs (Postgres) if we're - # presenting a "current version" of the MR discussion diff. - # So no need to make an extra Gitaly diff request for it. - # As an extra benefit, the returned `diff_file` already - # has `highlighted_diff_lines` data set from Redis on - # `Diff::FileCollection::MergeRequestDiff`. - noteable.diffs(original_position.diff_options).diff_files.first - else - original_position.diff_file(self.project.repository) - end - - # Since persisted diff files already have its content "unfolded" - # there's no need to make it pass through the unfolding process. - file&.unfold_diff_lines(position) unless note_diff_file - - file + if note_diff_file + diff = Gitlab::Git::Diff.new(note_diff_file.to_hash) + Gitlab::Diff::File.new(diff, + repository: project.repository, + diff_refs: original_position.diff_refs) + elsif created_at_diff?(noteable.diff_refs) + # We're able to use the already persisted diffs (Postgres) if we're + # presenting a "current version" of the MR discussion diff. + # So no need to make an extra Gitaly diff request for it. + # As an extra benefit, the returned `diff_file` already + # has `highlighted_diff_lines` data set from Redis on + # `Diff::FileCollection::MergeRequestDiff`. + noteable.diffs(original_position.diff_options).diff_files.first + else + original_position.diff_file(self.project.repository) + end end def supported? diff --git a/app/services/merge_requests/reload_diffs_service.rb b/app/services/merge_requests/reload_diffs_service.rb index c64b2e99b52..b47d8f3f63a 100644 --- a/app/services/merge_requests/reload_diffs_service.rb +++ b/app/services/merge_requests/reload_diffs_service.rb @@ -29,6 +29,10 @@ module MergeRequests # rubocop: disable CodeReuse/ActiveRecord def clear_cache(new_diff) + # Executing the iteration we cache highlighted diffs for each diff file of + # MergeRequestDiff. + cacheable_collection(new_diff).write_cache + # Remove cache for all diffs on this MR. Do not use the association on the # model, as that will interfere with other actions happening when # reloading the diff. diff --git a/app/services/notes/base_service.rb b/app/services/notes/base_service.rb deleted file mode 100644 index 431ff6c11c4..00000000000 --- a/app/services/notes/base_service.rb +++ /dev/null @@ -1,15 +0,0 @@ -# frozen_string_literal: true - -module Notes - class BaseService < ::BaseService - def clear_noteable_diffs_cache(note) - noteable = note.noteable - - if note.is_a?(DiffNote) && - note.discussion_first_note? && - note.position.unfolded_diff?(project.repository) - noteable.diffs.clear_cache - end - end - end -end diff --git a/app/services/notes/create_service.rb b/app/services/notes/create_service.rb index e03789e3ca9..049e6c5a871 100644 --- a/app/services/notes/create_service.rb +++ b/app/services/notes/create_service.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true module Notes - class CreateService < ::Notes::BaseService + class CreateService < ::BaseService def execute merge_request_diff_head_sha = params.delete(:merge_request_diff_head_sha) @@ -35,7 +35,6 @@ module Notes if !only_commands && note.save todo_service.new_note(note, current_user) - clear_noteable_diffs_cache(note) end if command_params.present? diff --git a/app/services/notes/destroy_service.rb b/app/services/notes/destroy_service.rb index fa0c2c5c86b..64e9accd97f 100644 --- a/app/services/notes/destroy_service.rb +++ b/app/services/notes/destroy_service.rb @@ -1,13 +1,11 @@ # frozen_string_literal: true module Notes - class DestroyService < ::Notes::BaseService + class DestroyService < BaseService def execute(note) TodoService.new.destroy_target(note) do |note| note.destroy end - - clear_noteable_diffs_cache(note) end end end diff --git a/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml b/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml deleted file mode 100644 index 7b48a94a993..00000000000 --- a/changelogs/unreleased/osw-comment-on-any-line-on-diffs.yml +++ /dev/null @@ -1,5 +0,0 @@ ---- -title: Allow commenting on any diff line in Merge Requests -merge_request: 22398 -author: -type: added diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 84595f8afd7..fb117baca9e 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -26,7 +26,6 @@ module Gitlab @repository = repository @diff_refs = diff_refs @fallback_diff_refs = fallback_diff_refs - @unfolded = false # Ensure items are collected in the the batch new_blob_lazy @@ -136,24 +135,6 @@ module Gitlab Gitlab::Diff::Parser.new.parse(raw_diff.each_line, diff_file: self).to_a end - # Changes diff_lines according to the given position. That is, - # it checks whether the position requires blob lines into the diff - # in order to be presented. - def unfold_diff_lines(position) - return unless position - - unfolder = Gitlab::Diff::LinesUnfolder.new(self, position) - - if unfolder.unfold_required? - @diff_lines = unfolder.unfolded_diff_lines - @unfolded = true - end - end - - def unfolded? - @unfolded - end - def highlighted_diff_lines @highlighted_diff_lines ||= Gitlab::Diff::Highlight.new(self, repository: self.repository).highlight diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 70063071ee7..5b67cd46c48 100644 --- a/lib/gitlab/diff/line.rb +++ b/lib/gitlab/diff/line.rb @@ -3,9 +3,9 @@ module Gitlab class Line SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze - attr_reader :line_code, :type, :old_pos, :new_pos + attr_reader :line_code, :type, :index, :old_pos, :new_pos attr_writer :rich_text - attr_accessor :text, :index + attr_accessor :text def initialize(text, type, index, old_pos, new_pos, parent_file: nil, line_code: nil, rich_text: nil) @text, @type, @index = text, type, index @@ -19,14 +19,7 @@ module Gitlab end def self.init_from_hash(hash) - new(hash[:text], - hash[:type], - hash[:index], - hash[:old_pos], - hash[:new_pos], - parent_file: hash[:parent_file], - line_code: hash[:line_code], - rich_text: hash[:rich_text]) + new(hash[:text], hash[:type], hash[:index], hash[:old_pos], hash[:new_pos], line_code: hash[:line_code], rich_text: hash[:rich_text]) end def to_hash diff --git a/lib/gitlab/diff/lines_unfolder.rb b/lib/gitlab/diff/lines_unfolder.rb deleted file mode 100644 index 9306b7e16a2..00000000000 --- a/lib/gitlab/diff/lines_unfolder.rb +++ /dev/null @@ -1,235 +0,0 @@ -# frozen_string_literal: true - -# Given a position, calculates which Blob lines should be extracted, treated and -# injected in the current diff file lines in order to present a "unfolded" diff. -module Gitlab - module Diff - class LinesUnfolder - include Gitlab::Utils::StrongMemoize - - UNFOLD_CONTEXT_SIZE = 3 - - def initialize(diff_file, position) - @diff_file = diff_file - @blob = diff_file.old_blob - @position = position - @generate_top_match_line = true - @generate_bottom_match_line = true - - # These methods update `@generate_top_match_line` and - # `@generate_bottom_match_line`. - @from_blob_line = calculate_from_blob_line! - @to_blob_line = calculate_to_blob_line! - end - - # Returns merged diff lines with required blob lines with correct - # positions. - def unfolded_diff_lines - strong_memoize(:unfolded_diff_lines) do - next unless unfold_required? - - merged_diff_with_blob_lines - end - end - - # Returns the extracted lines from the old blob which should be merged - # with the current diff lines. - def blob_lines - strong_memoize(:blob_lines) do - # Blob lines, unlike diffs, doesn't start with an empty space for - # unchanged line, so the parsing and highlighting step can get fuzzy - # without the following change. - line_prefix = ' ' - blob_as_diff_lines = @blob.data.each_line.map { |line| "#{line_prefix}#{line}" } - - lines = Gitlab::Diff::Parser.new.parse(blob_as_diff_lines, diff_file: @diff_file).to_a - - from = from_blob_line - 1 - to = to_blob_line - 1 - - lines[from..to] - end - end - - def unfold_required? - strong_memoize(:unfold_required) do - next false unless @diff_file.text? - next false unless @position.unchanged? - next false if @diff_file.new_file? || @diff_file.deleted_file? - next false unless @position.old_line - # Invalid position (MR import scenario) - next false if @position.old_line > @blob.lines.size - next false if @diff_file.diff_lines.empty? - next false if @diff_file.line_for_position(@position) - next false unless unfold_line - - true - end - end - - private - - attr_reader :from_blob_line, :to_blob_line - - def merged_diff_with_blob_lines - lines = @diff_file.diff_lines - match_line = unfold_line - insert_index = bottom? ? -1 : match_line.index - - lines -= [match_line] unless bottom? - - lines.insert(insert_index, *blob_lines_with_matches) - - # The inserted blob lines have invalid indexes, so we need - # to reindex them. - reindex(lines) - - lines - end - - # Returns 'unchanged' blob lines with recalculated `old_pos` and - # `new_pos` and the recalculated new match line (needed if we for instance - # we unfolded once, but there are still folded lines). - def blob_lines_with_matches - old_pos = from_blob_line - new_pos = from_blob_line + offset - - new_blob_lines = [] - - new_blob_lines.push(top_blob_match_line) if top_blob_match_line - - blob_lines.each do |line| - new_blob_lines << Gitlab::Diff::Line.new(line.text, line.type, nil, old_pos, new_pos, - parent_file: @diff_file) - - old_pos += 1 - new_pos += 1 - end - - new_blob_lines.push(bottom_blob_match_line) if bottom_blob_match_line - - new_blob_lines - end - - def reindex(lines) - lines.each_with_index { |line, i| line.index = i } - end - - def top_blob_match_line - strong_memoize(:top_blob_match_line) do - next unless @generate_top_match_line - - old_pos = from_blob_line - new_pos = from_blob_line + offset - - build_match_line(old_pos, new_pos) - end - end - - def bottom_blob_match_line - strong_memoize(:bottom_blob_match_line) do - # The bottom line match addition is already handled on - # Diff::File#diff_lines_for_serializer - next if bottom? - next unless @generate_bottom_match_line - - position = line_after_unfold_position.old_pos - - old_pos = position - new_pos = position + offset - - build_match_line(old_pos, new_pos) - end - end - - def build_match_line(old_pos, new_pos) - blob_lines_length = blob_lines.length - old_line_ref = [old_pos, blob_lines_length].join(',') - new_line_ref = [new_pos, blob_lines_length].join(',') - new_match_line_str = "@@ -#{old_line_ref}+#{new_line_ref} @@" - - Gitlab::Diff::Line.new(new_match_line_str, 'match', nil, old_pos, new_pos) - end - - # Returns the first line position that should be extracted - # from `blob_lines`. - def calculate_from_blob_line! - return unless unfold_required? - - from = comment_position - UNFOLD_CONTEXT_SIZE - - # There's no line before the match if it's in the top-most - # position. - prev_line_number = line_before_unfold_position&.old_pos || 0 - - if from <= prev_line_number + 1 - @generate_top_match_line = false - from = prev_line_number + 1 - end - - from - end - - # Returns the last line position that should be extracted - # from `blob_lines`. - def calculate_to_blob_line! - return unless unfold_required? - - to = comment_position + UNFOLD_CONTEXT_SIZE - - return to if bottom? - - next_line_number = line_after_unfold_position.old_pos - - if to >= next_line_number - 1 - @generate_bottom_match_line = false - to = next_line_number - 1 - end - - to - end - - def offset - unfold_line.new_pos - unfold_line.old_pos - end - - def line_before_unfold_position - return unless index = unfold_line&.index - - @diff_file.diff_lines[index - 1] if index > 0 - end - - def line_after_unfold_position - return unless index = unfold_line&.index - - @diff_file.diff_lines[index + 1] if index >= 0 - end - - def bottom? - strong_memoize(:bottom) do - @position.old_line > last_line.old_pos - end - end - - # Returns the line which needed to be expanded in order to send a comment - # in `@position`. - def unfold_line - strong_memoize(:unfold_line) do - next last_line if bottom? - - @diff_file.diff_lines.find do |line| - line.old_pos > comment_position && line.type == 'match' - end - end - end - - def comment_position - @position.old_line - end - - def last_line - @diff_file.diff_lines.last - end - end - end -end diff --git a/lib/gitlab/diff/position.rb b/lib/gitlab/diff/position.rb index 7bfab2d808f..f967494199e 100644 --- a/lib/gitlab/diff/position.rb +++ b/lib/gitlab/diff/position.rb @@ -101,10 +101,6 @@ module Gitlab @diff_refs ||= DiffRefs.new(base_sha: base_sha, start_sha: start_sha, head_sha: head_sha) end - def unfolded_diff?(repository) - diff_file(repository)&.unfolded? - end - def diff_file(repository) return @diff_file if defined?(@diff_file) @@ -138,13 +134,7 @@ module Gitlab return unless diff_refs.complete? return unless comparison = diff_refs.compare_in(repository.project) - file = comparison.diffs(diff_options).diff_files.first - - # We need to unfold diff lines according to the position in order - # to correctly calculate the line code and trace position changes. - file&.unfold_diff_lines(self) - - file + comparison.diffs(diff_options).diff_files.first end def get_formatter_class(type) diff --git a/spec/controllers/projects/blob_controller_spec.rb b/spec/controllers/projects/blob_controller_spec.rb index 5fdf7f1229d..28f7e4634a5 100644 --- a/spec/controllers/projects/blob_controller_spec.rb +++ b/spec/controllers/projects/blob_controller_spec.rb @@ -157,7 +157,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).first - expect(match_line['type']).to be_nil + expect(match_line['type']).to eq('context') end it 'adds bottom match line when "t"o is less than blob size' do @@ -177,7 +177,7 @@ describe Projects::BlobController do match_line = JSON.parse(response.body).last - expect(match_line['type']).to be_nil + expect(match_line['type']).to eq('context') end end end diff --git a/spec/features/merge_request/user_posts_diff_notes_spec.rb b/spec/features/merge_request/user_posts_diff_notes_spec.rb index 51b78d3e7d1..fa148715855 100644 --- a/spec/features/merge_request/user_posts_diff_notes_spec.rb +++ b/spec/features/merge_request/user_posts_diff_notes_spec.rb @@ -85,13 +85,12 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('#a5cc2925ca8258af241be7e5b0381edf30266302 .line_holder') } - it 'allows commenting on the left side' do - should_allow_commenting(line_holder, 'left') + it 'does not allow commenting on the left side' do + should_not_allow_commenting(line_holder, 'left') end - it 'allows commenting on the right side' do - # Automatically shifts comment box to left side. - should_allow_commenting(line_holder, 'right') + it 'does not allow commenting on the right side' do + should_not_allow_commenting(line_holder, 'right') end end end @@ -148,8 +147,8 @@ describe 'Merge request > User posts diff notes', :js do # `.line_holder` will be an unfolded line. let(:line_holder) { first('.line_holder[id="a5cc2925ca8258af241be7e5b0381edf30266302_1_1"]') } - it 'allows commenting' do - should_allow_commenting line_holder + it 'does not allow commenting' do + should_not_allow_commenting line_holder end end diff --git a/spec/javascripts/diffs/store/mutations_spec.js b/spec/javascripts/diffs/store/mutations_spec.js index 8821cde76f4..fed04cbaed8 100644 --- a/spec/javascripts/diffs/store/mutations_spec.js +++ b/spec/javascripts/diffs/store/mutations_spec.js @@ -98,7 +98,7 @@ describe('DiffsStoreMutations', () => { it('should call utils.addContextLines with proper params', () => { const options = { lineNumbers: { oldLineNumber: 1, newLineNumber: 2 }, - contextLines: [{ oldLine: 1, newLine: 1, lineCode: 'ff9200_1_1', discussions: [] }], + contextLines: [{ oldLine: 1 }], fileHash: 'ff9200', params: { bottom: true, @@ -110,7 +110,7 @@ describe('DiffsStoreMutations', () => { parallelDiffLines: [], }; const state = { diffFiles: [diffFile] }; - const lines = [{ oldLine: 1, newLine: 1 }]; + const lines = [{ oldLine: 1 }]; const findDiffFileSpy = spyOnDependency(mutations, 'findDiffFile').and.returnValue(diffFile); const removeMatchLineSpy = spyOnDependency(mutations, 'removeMatchLine'); diff --git a/spec/lib/gitlab/diff/file_spec.rb b/spec/lib/gitlab/diff/file_spec.rb index 3417896e259..2f51642b58e 100644 --- a/spec/lib/gitlab/diff/file_spec.rb +++ b/spec/lib/gitlab/diff/file_spec.rb @@ -41,52 +41,6 @@ describe Gitlab::Diff::File do end end - describe '#unfold_diff_lines' do - let(:unfolded_lines) { double('expanded-lines') } - let(:unfolder) { instance_double(Gitlab::Diff::LinesUnfolder) } - let(:position) { instance_double(Gitlab::Diff::Position, old_line: 10) } - - before do - allow(Gitlab::Diff::LinesUnfolder).to receive(:new) { unfolder } - end - - context 'when unfold required' do - before do - allow(unfolder).to receive(:unfold_required?) { true } - allow(unfolder).to receive(:unfolded_diff_lines) { unfolded_lines } - end - - it 'changes @unfolded to true' do - diff_file.unfold_diff_lines(position) - - expect(diff_file).to be_unfolded - end - - it 'updates @diff_lines' do - diff_file.unfold_diff_lines(position) - - expect(diff_file.diff_lines).to eq(unfolded_lines) - end - end - - context 'when unfold not required' do - before do - allow(unfolder).to receive(:unfold_required?) { false } - end - - it 'keeps @unfolded false' do - diff_file.unfold_diff_lines(position) - - expect(diff_file).not_to be_unfolded - end - - it 'does not update @diff_lines' do - expect { diff_file.unfold_diff_lines(position) } - .not_to change(diff_file, :diff_lines) - end - end - end - describe '#mode_changed?' do it { expect(diff_file.mode_changed?).to be_falsey } end diff --git a/spec/lib/gitlab/diff/lines_unfolder_spec.rb b/spec/lib/gitlab/diff/lines_unfolder_spec.rb deleted file mode 100644 index 8e00c8e0e30..00000000000 --- a/spec/lib/gitlab/diff/lines_unfolder_spec.rb +++ /dev/null @@ -1,750 +0,0 @@ -# frozen_string_literal: true - -require 'spec_helper' - -describe Gitlab::Diff::LinesUnfolder do - let(:raw_diff) do - <<-DIFF.strip_heredoc - @@ -7,9 +7,6 @@ - "tags": ["devel", "development", "nightly"], - "desktop-file-name-prefix": "(Development) ", - "finish-args": [ - - "--share=ipc", "--socket=x11", - - "--socket=wayland", - - "--talk-name=org.gnome.OnlineAccounts", - "--talk-name=org.freedesktop.Tracker1", - "--filesystem=home", - "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", - @@ -62,7 +59,7 @@ - }, - { - "name": "gnome-desktop", - - "config-opts": ["--disable-debug-tools", "--disable-udev"], - + "config-opts": ["--disable-debug-tools", "--disable-"], - "sources": [ - { - "type": "git", - @@ -83,11 +80,6 @@ - "buildsystem": "meson", - "builddir": true, - "name": "nautilus", - - "config-opts": [ - - "-Denable-desktop=false", - - "-Denable-selinux=false", - - "--libdir=/app/lib" - - ], - "sources": [ - { - "type": "git", - DIFF - end - - let(:raw_old_blob) do - <<-BLOB.strip_heredoc - { - "app-id": "org.gnome.Nautilus", - "runtime": "org.gnome.Platform", - "runtime-version": "master", - "sdk": "org.gnome.Sdk", - "command": "nautilus", - "tags": ["devel", "development", "nightly"], - "desktop-file-name-prefix": "(Development) ", - "finish-args": [ - "--share=ipc", "--socket=x11", - "--socket=wayland", - "--talk-name=org.gnome.OnlineAccounts", - "--talk-name=org.freedesktop.Tracker1", - "--filesystem=home", - "--talk-name=org.gtk.vfs", "--talk-name=org.gtk.vfs.*", - "--filesystem=xdg-run/dconf", "--filesystem=~/.config/dconf:ro", - "--talk-name=ca.desrt.dconf", "--env=DCONF_USER_CONFIG_DIR=.config/dconf" - ], - "cleanup": [ "/include", "/share/bash-completion" ], - "modules": [ - { - "name": "exiv2", - "sources": [ - { - "type": "archive", - "url": "http://exiv2.org/builds/exiv2-0.26-trunk.tar.gz", - "sha256": "c75e3c4a0811bf700d92c82319373b7a825a2331c12b8b37d41eb58e4f18eafb" - }, - { - "type": "shell", - "commands": [ - "cp -f /usr/share/gnu-config/config.sub ./config/", - "cp -f /usr/share/gnu-config/config.guess ./config/" - ] - } - ] - }, - { - "name": "gexiv2", - "config-opts": [ "--disable-introspection" ], - "sources": [ - { - "type": "git", - "url": "https://git.gnome.org/browse/gexiv2" - } - ] - }, - { - "name": "tracker", - "cleanup": [ "/bin", "/etc", "/libexec" ], - "config-opts": [ "--disable-miner-apps", "--disable-static", - "--disable-tracker-extract", "--disable-tracker-needle", - "--disable-tracker-preferences", "--disable-artwork", - "--disable-tracker-writeback", "--disable-miner-user-guides", - "--with-bash-completion-dir=no" ], - "sources": [ - { - "type": "git", - "url": "https://git.gnome.org/browse/tracker" - } - ] - }, - { - "name": "gnome-desktop", - "config-opts": ["--disable-debug-tools", "--disable-udev"], - "sources": [ - { - "type": "git", - "url": "https://git.gnome.org/browse/gnome-desktop" - } - ] - }, - { - "name": "gnome-autoar", - "sources": [ - { - "type": "git", - "url": "https://git.gnome.org/browse/gnome-autoar" - } - ] - }, - { - "buildsystem": "meson", - "builddir": true, - "name": "nautilus", - "config-opts": [ - "-Denable-desktop=false", - "-Denable-selinux=false", - "--libdir=/app/lib" - ], - "sources": [ - { - "type": "git", - "url": "https://gitlab.gnome.org/GNOME/nautilus.git" - } - ] - } - ] - }, - { - "app-id": "foo", - "runtime": "foo", - "runtime-version": "foo", - "sdk": "foo", - "command": "foo", - "tags": ["foo", "bar", "kux"], - "desktop-file-name-prefix": "(Foo) ", - { - "buildsystem": "meson", - "builddir": true, - "name": "nautilus", - "sources": [ - { - "type": "git", - "url": "https://gitlab.gnome.org/GNOME/nautilus.git" - } - ] - } - }, - { - "app-id": "foo", - "runtime": "foo", - "runtime-version": "foo", - "sdk": "foo", - "command": "foo", - "tags": ["foo", "bar", "kux"], - "desktop-file-name-prefix": "(Foo) ", - { - "buildsystem": "meson", - "builddir": true, - "name": "nautilus", - "sources": [ - { - "type": "git", - "url": "https://gitlab.gnome.org/GNOME/nautilus.git" - } - ] - } - } - BLOB - end - - let(:project) { create(:project) } - - let(:old_blob) { Gitlab::Git::Blob.new(data: raw_old_blob) } - - let(:diff) do - Gitlab::Git::Diff.new(diff: raw_diff, - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - a_mode: "100644", - b_mode: "100644", - new_file: false, - renamed_file: false, - deleted_file: false, - too_large: false) - end - - let(:diff_file) do - Gitlab::Diff::File.new(diff, repository: project.repository) - end - - before do - allow(old_blob).to receive(:load_all_data!) - allow(diff_file).to receive(:old_blob) { old_blob } - end - - subject { described_class.new(diff_file, position) } - - context 'position requires a middle expansion and new match lines' do - let(:position) do - Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - head_sha: "1487062132228de836236c522fe52fed4980a46c", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - position_type: "text", - old_line: 43, - new_line: 40) - end - - context 'blob lines' do - let(:expected_blob_lines) do - [[40, 40, " \"config-opts\": [ \"--disable-introspection\" ],"], - [41, 41, " \"sources\": ["], - [42, 42, " {"], - [43, 43, " \"type\": \"git\","], - [44, 44, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], - [45, 45, " }"], - [46, 46, " ]"]] - end - - it 'returns the extracted blob lines correctly' do - extracted_lines = subject.blob_lines - - expect(extracted_lines.size).to eq(7) - - extracted_lines.each_with_index do |line, i| - expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) - end - end - end - - context 'diff lines' do - let(:expected_diff_lines) do - [[7, 7, "@@ -7,9 +7,6 @@"], - [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], - [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], - [9, 9, " \"finish-args\": ["], - [10, 10, "- \"--share=ipc\", \"--socket=x11\","], - [11, 10, "- \"--socket=wayland\","], - [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], - [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], - [14, 11, " \"--filesystem=home\","], - [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], - - # New match line - [40, 37, "@@ -40,7+37,7 @@"], - - # Injected blob lines - [40, 37, " \"config-opts\": [ \"--disable-introspection\" ],"], - [41, 38, " \"sources\": ["], - [42, 39, " {"], - [43, 40, " \"type\": \"git\","], # comment - [44, 41, " \"url\": \"https://git.gnome.org/browse/gexiv2\""], - [45, 42, " }"], - [46, 43, " ]"], - # end - - # Second match line - [62, 59, "@@ -62,7+59,7 @@"], - - [62, 59, " },"], - [63, 60, " {"], - [64, 61, " \"name\": \"gnome-desktop\","], - [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], - [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], - [66, 63, " \"sources\": ["], - [67, 64, " {"], - [68, 65, " \"type\": \"git\","], - [83, 80, "@@ -83,11 +80,6 @@"], - [83, 80, " \"buildsystem\": \"meson\","], - [84, 81, " \"builddir\": true,"], - [85, 82, " \"name\": \"nautilus\","], - [86, 83, "- \"config-opts\": ["], - [87, 83, "- \"-Denable-desktop=false\","], - [88, 83, "- \"-Denable-selinux=false\","], - [89, 83, "- \"--libdir=/app/lib\""], - [90, 83, "- ],"], - [91, 83, " \"sources\": ["], - [92, 84, " {"], - [93, 85, " \"type\": \"git\","]] - end - - it 'return merge of blob lines with diff lines correctly' do - new_diff_lines = subject.unfolded_diff_lines - - expected_diff_lines.each_with_index do |expected_line, i| - line = new_diff_lines[i] - - expect([line.old_pos, line.new_pos, line.text]) - .to eq([expected_line[0], expected_line[1], expected_line[2]]) - end - end - - it 'merged lines have correct line codes' do - new_diff_lines = subject.unfolded_diff_lines - - new_diff_lines.each_with_index do |line, i| - old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] - - unless line.type == 'match' - expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) - end - end - end - end - end - - context 'position requires a middle expansion and no top match line' do - let(:position) do - Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - head_sha: "1487062132228de836236c522fe52fed4980a46c", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - position_type: "text", - old_line: 16, - new_line: 17) - end - - context 'blob lines' do - let(:expected_blob_lines) do - [[16, 16, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], - [17, 17, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], - [18, 18, " ],"], - [19, 19, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"]] - end - - it 'returns the extracted blob lines correctly' do - extracted_lines = subject.blob_lines - - expect(extracted_lines.size).to eq(4) - - extracted_lines.each_with_index do |line, i| - expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) - end - end - end - - context 'diff lines' do - let(:expected_diff_lines) do - [[7, 7, "@@ -7,9 +7,6 @@"], - [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], - [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], - [9, 9, " \"finish-args\": ["], - [10, 10, "- \"--share=ipc\", \"--socket=x11\","], - [11, 10, "- \"--socket=wayland\","], - [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], - [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], - [14, 11, " \"--filesystem=home\","], - [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], - # No new match needed - - # Injected blob lines - [16, 13, " \"--filesystem=xdg-run/dconf\", \"--filesystem=~/.config/dconf:ro\","], - [17, 14, " \"--talk-name=ca.desrt.dconf\", \"--env=DCONF_USER_CONFIG_DIR=.config/dconf\""], - [18, 15, " ],"], - [19, 16, " \"cleanup\": [ \"/include\", \"/share/bash-completion\" ],"], - # end - - # Second match line - [62, 59, "@@ -62,4+59,4 @@"], - - [62, 59, " },"], - [63, 60, " {"], - [64, 61, " \"name\": \"gnome-desktop\","], - [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], - [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], - [66, 63, " \"sources\": ["], - [67, 64, " {"], - [68, 65, " \"type\": \"git\","], - [83, 80, "@@ -83,11 +80,6 @@"], - [83, 80, " \"buildsystem\": \"meson\","], - [84, 81, " \"builddir\": true,"], - [85, 82, " \"name\": \"nautilus\","], - [86, 83, "- \"config-opts\": ["], - [87, 83, "- \"-Denable-desktop=false\","], - [88, 83, "- \"-Denable-selinux=false\","], - [89, 83, "- \"--libdir=/app/lib\""], - [90, 83, "- ],"], - [91, 83, " \"sources\": ["], - [92, 84, " {"], - [93, 85, " \"type\": \"git\","]] - end - - it 'return merge of blob lines with diff lines correctly' do - new_diff_lines = subject.unfolded_diff_lines - - expected_diff_lines.each_with_index do |expected_line, i| - line = new_diff_lines[i] - - expect([line.old_pos, line.new_pos, line.text]) - .to eq([expected_line[0], expected_line[1], expected_line[2]]) - end - end - - it 'merged lines have correct line codes' do - new_diff_lines = subject.unfolded_diff_lines - - new_diff_lines.each_with_index do |line, i| - old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] - - unless line.type == 'match' - expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) - end - end - end - end - end - - context 'position requires a middle expansion and no bottom match line' do - let(:position) do - Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - head_sha: "1487062132228de836236c522fe52fed4980a46c", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - position_type: "text", - old_line: 82, - new_line: 79) - end - - context 'blob lines' do - let(:expected_blob_lines) do - [[79, 79, " }"], - [80, 80, " ]"], - [81, 81, " },"], - [82, 82, " {"]] - end - - it 'returns the extracted blob lines correctly' do - extracted_lines = subject.blob_lines - - expect(extracted_lines.size).to eq(4) - - extracted_lines.each_with_index do |line, i| - expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) - end - end - end - - context 'diff lines' do - let(:expected_diff_lines) do - [[7, 7, "@@ -7,9 +7,6 @@"], - [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], - [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], - [9, 9, " \"finish-args\": ["], - [10, 10, "- \"--share=ipc\", \"--socket=x11\","], - [11, 10, "- \"--socket=wayland\","], - [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], - [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], - [14, 11, " \"--filesystem=home\","], - [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], - [62, 59, "@@ -62,7 +59,7 @@"], - [62, 59, " },"], - [63, 60, " {"], - [64, 61, " \"name\": \"gnome-desktop\","], - [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], - [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], - [66, 63, " \"sources\": ["], - [67, 64, " {"], - [68, 65, " \"type\": \"git\","], - - # New top match line - [79, 76, "@@ -79,4+76,4 @@"], - - # Injected blob lines - [79, 76, " }"], - [80, 77, " ]"], - [81, 78, " },"], - [82, 79, " {"], - # end - - # No new second match line - [83, 80, " \"buildsystem\": \"meson\","], - [84, 81, " \"builddir\": true,"], - [85, 82, " \"name\": \"nautilus\","], - [86, 83, "- \"config-opts\": ["], - [87, 83, "- \"-Denable-desktop=false\","], - [88, 83, "- \"-Denable-selinux=false\","], - [89, 83, "- \"--libdir=/app/lib\""], - [90, 83, "- ],"], - [91, 83, " \"sources\": ["], - [92, 84, " {"], - [93, 85, " \"type\": \"git\","]] - end - - it 'return merge of blob lines with diff lines correctly' do - new_diff_lines = subject.unfolded_diff_lines - - expected_diff_lines.each_with_index do |expected_line, i| - line = new_diff_lines[i] - - expect([line.old_pos, line.new_pos, line.text]) - .to eq([expected_line[0], expected_line[1], expected_line[2]]) - end - end - - it 'merged lines have correct line codes' do - new_diff_lines = subject.unfolded_diff_lines - - new_diff_lines.each_with_index do |line, i| - old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] - - unless line.type == 'match' - expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) - end - end - end - end - end - - context 'position requires a short top expansion' do - let(:position) do - Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - head_sha: "1487062132228de836236c522fe52fed4980a46c", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - position_type: "text", - old_line: 6, - new_line: 6) - end - - context 'blob lines' do - let(:expected_blob_lines) do - [[3, 3, " \"runtime\": \"org.gnome.Platform\","], - [4, 4, " \"runtime-version\": \"master\","], - [5, 5, " \"sdk\": \"org.gnome.Sdk\","], - [6, 6, " \"command\": \"nautilus\","]] - end - - it 'returns the extracted blob lines correctly' do - extracted_lines = subject.blob_lines - - expect(extracted_lines.size).to eq(4) - - extracted_lines.each_with_index do |line, i| - expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) - end - end - end - - context 'diff lines' do - let(:expected_diff_lines) do - # New match line - [[3, 3, "@@ -3,4+3,4 @@"], - - # Injected blob lines - [3, 3, " \"runtime\": \"org.gnome.Platform\","], - [4, 4, " \"runtime-version\": \"master\","], - [5, 5, " \"sdk\": \"org.gnome.Sdk\","], - [6, 6, " \"command\": \"nautilus\","], - # end - [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], - [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], - [9, 9, " \"finish-args\": ["], - [10, 10, "- \"--share=ipc\", \"--socket=x11\","], - [11, 10, "- \"--socket=wayland\","], - [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], - [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], - [14, 11, " \"--filesystem=home\","], - [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], - [62, 59, "@@ -62,7 +59,7 @@"], - [62, 59, " },"], - [63, 60, " {"], - [64, 61, " \"name\": \"gnome-desktop\","], - [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], - [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], - [66, 63, " \"sources\": ["], - [67, 64, " {"], - [68, 65, " \"type\": \"git\","], - [83, 80, "@@ -83,11 +80,6 @@"], - [83, 80, " \"buildsystem\": \"meson\","], - [84, 81, " \"builddir\": true,"], - [85, 82, " \"name\": \"nautilus\","], - [86, 83, "- \"config-opts\": ["], - [87, 83, "- \"-Denable-desktop=false\","], - [88, 83, "- \"-Denable-selinux=false\","], - [89, 83, "- \"--libdir=/app/lib\""], - [90, 83, "- ],"], - [91, 83, " \"sources\": ["], - [92, 84, " {"], - [93, 85, " \"type\": \"git\","]] - end - - it 'return merge of blob lines with diff lines correctly' do - new_diff_lines = subject.unfolded_diff_lines - - expected_diff_lines.each_with_index do |expected_line, i| - line = new_diff_lines[i] - - expect([line.old_pos, line.new_pos, line.text]) - .to eq([expected_line[0], expected_line[1], expected_line[2]]) - end - end - - it 'merged lines have correct line codes' do - new_diff_lines = subject.unfolded_diff_lines - - new_diff_lines.each_with_index do |line, i| - old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] - - unless line.type == 'match' - expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) - end - end - end - end - end - - context 'position sits between two match lines (no expasion needed)' do - let(:position) do - Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - head_sha: "1487062132228de836236c522fe52fed4980a46c", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - position_type: "text", - old_line: 64, - new_line: 61) - end - - context 'diff lines' do - it 'returns nil' do - expect(subject.unfolded_diff_lines).to be_nil - end - end - end - - context 'position requires bottom expansion and new match lines' do - let(:position) do - Gitlab::Diff::Position.new(base_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - start_sha: "1c59dfa64afbea8c721bb09a06a9d326c952ea19", - head_sha: "1487062132228de836236c522fe52fed4980a46c", - old_path: "build-aux/flatpak/org.gnome.Nautilus.json", - new_path: "build-aux/flatpak/org.gnome.Nautilus.json", - position_type: "text", - old_line: 107, - new_line: 99) - end - - context 'blob lines' do - let(:expected_blob_lines) do - [[104, 104, " \"sdk\": \"foo\","], - [105, 105, " \"command\": \"foo\","], - [106, 106, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], - [107, 107, " \"desktop-file-name-prefix\": \"(Foo) \","], - [108, 108, " {"], - [109, 109, " \"buildsystem\": \"meson\","], - [110, 110, " \"builddir\": true,"]] - end - - it 'returns the extracted blob lines correctly' do - extracted_lines = subject.blob_lines - - expect(extracted_lines.size).to eq(7) - - extracted_lines.each_with_index do |line, i| - expect([line.old_line, line.new_line, line.text]).to eq(expected_blob_lines[i]) - end - end - end - - context 'diff lines' do - let(:expected_diff_lines) do - [[7, 7, "@@ -7,9 +7,6 @@"], - [7, 7, " \"tags\": [\"devel\", \"development\", \"nightly\"],"], - [8, 8, " \"desktop-file-name-prefix\": \"(Development) \","], - [9, 9, " \"finish-args\": ["], - [10, 10, "- \"--share=ipc\", \"--socket=x11\","], - [11, 10, "- \"--socket=wayland\","], - [12, 10, "- \"--talk-name=org.gnome.OnlineAccounts\","], - [13, 10, " \"--talk-name=org.freedesktop.Tracker1\","], - [14, 11, " \"--filesystem=home\","], - [15, 12, " \"--talk-name=org.gtk.vfs\", \"--talk-name=org.gtk.vfs.*\","], - [62, 59, "@@ -62,7 +59,7 @@"], - [62, 59, " },"], - [63, 60, " {"], - [64, 61, " \"name\": \"gnome-desktop\","], - [65, 62, "- \"config-opts\": [\"--disable-debug-tools\", \"--disable-udev\"],"], - [66, 62, "+ \"config-opts\": [\"--disable-debug-tools\", \"--disable-\"],"], - [66, 63, " \"sources\": ["], - [67, 64, " {"], - [68, 65, " \"type\": \"git\","], - [83, 80, "@@ -83,11 +80,6 @@"], - [83, 80, " \"buildsystem\": \"meson\","], - [84, 81, " \"builddir\": true,"], - [85, 82, " \"name\": \"nautilus\","], - [86, 83, "- \"config-opts\": ["], - [87, 83, "- \"-Denable-desktop=false\","], - [88, 83, "- \"-Denable-selinux=false\","], - [89, 83, "- \"--libdir=/app/lib\""], - [90, 83, "- ],"], - [91, 83, " \"sources\": ["], - [92, 84, " {"], - [93, 85, " \"type\": \"git\","], - # New match line - [104, 96, "@@ -104,7+96,7 @@"], - - # Injected blob lines - [104, 96, " \"sdk\": \"foo\","], - [105, 97, " \"command\": \"foo\","], - [106, 98, " \"tags\": [\"foo\", \"bar\", \"kux\"],"], - [107, 99, " \"desktop-file-name-prefix\": \"(Foo) \","], - [108, 100, " {"], - [109, 101, " \"buildsystem\": \"meson\","], - [110, 102, " \"builddir\": true,"]] - # end - end - - it 'return merge of blob lines with diff lines correctly' do - new_diff_lines = subject.unfolded_diff_lines - - expected_diff_lines.each_with_index do |expected_line, i| - line = new_diff_lines[i] - - expect([line.old_pos, line.new_pos, line.text]) - .to eq([expected_line[0], expected_line[1], expected_line[2]]) - end - end - - it 'merged lines have correct line codes' do - new_diff_lines = subject.unfolded_diff_lines - - new_diff_lines.each_with_index do |line, i| - old_pos, new_pos = expected_diff_lines[i][0], expected_diff_lines[i][1] - - unless line.type == 'match' - expect(line.line_code).to eq(Gitlab::Git.diff_line_code(diff_file.file_path, new_pos, old_pos)) - end - end - end - end - end -end diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index 5acd01828cb..546c9f277c5 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/spec/services/merge_requests/reload_diffs_service_spec.rb @@ -31,11 +31,32 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin end context 'cache clearing' do + before do + allow_any_instance_of(Gitlab::Diff::File).to receive(:text?).and_return(true) + allow_any_instance_of(Gitlab::Diff::File).to receive(:diffable?).and_return(true) + end + + it 'retrieves the diff files to cache the highlighted result' do + new_diff = merge_request.create_merge_request_diff + cache_key = new_diff.diffs_collection.cache_key + + expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) + expect(Rails.cache).to receive(:read).with(cache_key).and_call_original + expect(Rails.cache).to receive(:write).with(cache_key, anything, anything).and_call_original + + subject.execute + end + it 'clears the cache for older diffs on the merge request' do old_diff = merge_request.merge_request_diff old_cache_key = old_diff.diffs_collection.cache_key + new_diff = merge_request.create_merge_request_diff + new_cache_key = new_diff.diffs_collection.cache_key + expect(merge_request).to receive(:create_merge_request_diff).and_return(new_diff) expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original + expect(Rails.cache).to receive(:read).with(new_cache_key).and_call_original + expect(Rails.cache).to receive(:write).with(new_cache_key, anything, anything).and_call_original subject.execute end diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index 80b015d4cd0..b1290fd0d47 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -57,57 +57,6 @@ describe Notes::CreateService do end end - context 'noteable highlight cache clearing' do - let(:project_with_repo) { create(:project, :repository) } - let(:merge_request) do - create(:merge_request, source_project: project_with_repo, - target_project: project_with_repo) - end - - let(:position) do - Gitlab::Diff::Position.new(old_path: "files/ruby/popen.rb", - new_path: "files/ruby/popen.rb", - old_line: nil, - new_line: 14, - diff_refs: merge_request.diff_refs) - end - - let(:new_opts) do - opts.merge(in_reply_to_discussion_id: nil, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: position.to_h) - end - - before do - allow_any_instance_of(Gitlab::Diff::Position) - .to receive(:unfolded_diff?) { true } - end - - it 'clears noteable diff cache when it was unfolded for the note position' do - expect_any_instance_of(Gitlab::Diff::HighlightCache).to receive(:clear) - - described_class.new(project_with_repo, user, new_opts).execute - end - - it 'does not clear cache when note is not the first of the discussion' do - prev_note = - create(:diff_note_on_merge_request, noteable: merge_request, - project: project_with_repo) - reply_opts = - opts.merge(in_reply_to_discussion_id: prev_note.discussion_id, - type: 'DiffNote', - noteable_type: 'MergeRequest', - noteable_id: merge_request.id, - position: position.to_h) - - expect(merge_request).not_to receive(:diffs) - - described_class.new(project_with_repo, user, reply_opts).execute - end - end - context 'note diff file' do let(:project_with_repo) { create(:project, :repository) } let(:merge_request) do diff --git a/spec/services/notes/destroy_service_spec.rb b/spec/services/notes/destroy_service_spec.rb index b1f4e87e8ea..64445be560e 100644 --- a/spec/services/notes/destroy_service_spec.rb +++ b/spec/services/notes/destroy_service_spec.rb @@ -21,38 +21,5 @@ describe Notes::DestroyService do expect { described_class.new(project, user).execute(note) } .to change { user.todos_pending_count }.from(1).to(0) end - - context 'noteable highlight cache clearing' do - let(:repo_project) { create(:project, :repository) } - let(:merge_request) do - create(:merge_request, source_project: repo_project, - target_project: repo_project) - end - - let(:note) do - create(:diff_note_on_merge_request, project: repo_project, - noteable: merge_request) - end - - before do - allow(note.position).to receive(:unfolded_diff?) { true } - end - - it 'clears noteable diff cache when it was unfolded for the note position' do - expect(merge_request).to receive_message_chain(:diffs, :clear_cache) - - described_class.new(repo_project, user).execute(note) - end - - it 'does not clear cache when note is not the first of the discussion' do - reply_note = create(:diff_note_on_merge_request, in_reply_to: note, - project: repo_project, - noteable: merge_request) - - expect(merge_request).not_to receive(:diffs) - - described_class.new(repo_project, user).execute(reply_note) - end - end end end |