diff options
author | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 15:26:25 +0300 |
---|---|---|
committer | GitLab Bot <gitlab-bot@gitlab.com> | 2020-07-20 15:26:25 +0300 |
commit | a09983ae35713f5a2bbb100981116d31ce99826e (patch) | |
tree | 2ee2af7bd104d57086db360a7e6d8c9d5d43667a /lib/gitlab/diff | |
parent | 18c5ab32b738c0b6ecb4d0df3994000482f34bd8 (diff) |
Add latest changes from gitlab-org/gitlab@13-2-stable-ee
Diffstat (limited to 'lib/gitlab/diff')
-rw-r--r-- | lib/gitlab/diff/file.rb | 8 | ||||
-rw-r--r-- | lib/gitlab/diff/file_collection/base.rb | 27 | ||||
-rw-r--r-- | lib/gitlab/diff/file_collection/merge_request_diff_base.rb | 33 | ||||
-rw-r--r-- | lib/gitlab/diff/file_collection/wiki_page.rb | 24 | ||||
-rw-r--r-- | lib/gitlab/diff/position_tracer.rb | 4 | ||||
-rw-r--r-- | lib/gitlab/diff/position_tracer/base_strategy.rb | 1 | ||||
-rw-r--r-- | lib/gitlab/diff/position_tracer/image_strategy.rb | 10 | ||||
-rw-r--r-- | lib/gitlab/diff/position_tracer/line_strategy.rb | 30 | ||||
-rw-r--r-- | lib/gitlab/diff/stats_cache.rb | 54 |
9 files changed, 160 insertions, 31 deletions
diff --git a/lib/gitlab/diff/file.rb b/lib/gitlab/diff/file.rb index 72dcc4fde71..dcd4bbdabf5 100644 --- a/lib/gitlab/diff/file.rb +++ b/lib/gitlab/diff/file.rb @@ -230,11 +230,15 @@ module Gitlab end def added_lines - @stats&.additions || diff_lines.count(&:added?) + strong_memoize(:added_lines) do + @stats&.additions || diff_lines.count(&:added?) + end end def removed_lines - @stats&.deletions || diff_lines.count(&:removed?) + strong_memoize(:removed_lines) do + @stats&.deletions || diff_lines.count(&:removed?) + end end def file_identifier diff --git a/lib/gitlab/diff/file_collection/base.rb b/lib/gitlab/diff/file_collection/base.rb index 38b636e4e5a..cf0611e44da 100644 --- a/lib/gitlab/diff/file_collection/base.rb +++ b/lib/gitlab/diff/file_collection/base.rb @@ -60,12 +60,20 @@ module Gitlab end end - def diff_file_with_old_path(old_path) - diff_files.find { |diff_file| diff_file.old_path == old_path } + def diff_file_with_old_path(old_path, a_mode = nil) + if Feature.enabled?(:file_identifier_hash) && a_mode.present? + diff_files.find { |diff_file| diff_file.old_path == old_path && diff_file.a_mode == a_mode } + else + diff_files.find { |diff_file| diff_file.old_path == old_path } + end end - def diff_file_with_new_path(new_path) - diff_files.find { |diff_file| diff_file.new_path == new_path } + def diff_file_with_new_path(new_path, b_mode = nil) + if Feature.enabled?(:file_identifier_hash) && b_mode.present? + diff_files.find { |diff_file| diff_file.new_path == new_path && diff_file.b_mode == b_mode } + else + diff_files.find { |diff_file| diff_file.new_path == new_path } + end end def clear_cache @@ -80,15 +88,18 @@ module Gitlab def diff_stats_collection strong_memoize(:diff_stats) do - # There are scenarios where we don't need to request Diff Stats, - # when caching for instance. - next unless @include_stats - next unless diff_refs + next unless fetch_diff_stats? @repository.diff_stats(diff_refs.base_sha, diff_refs.head_sha) end end + def fetch_diff_stats? + # There are scenarios where we don't need to request Diff Stats, + # when caching for instance. + @include_stats && diff_refs + end + def decorate_diff!(diff) return diff if diff.is_a?(File) diff --git a/lib/gitlab/diff/file_collection/merge_request_diff_base.rb b/lib/gitlab/diff/file_collection/merge_request_diff_base.rb index d126fdb2be2..d54e1aad19a 100644 --- a/lib/gitlab/diff/file_collection/merge_request_diff_base.rb +++ b/lib/gitlab/diff/file_collection/merge_request_diff_base.rb @@ -20,7 +20,7 @@ module Gitlab strong_memoize(:diff_files) do diff_files = super - diff_files.each { |diff_file| cache.decorate(diff_file) } + diff_files.each { |diff_file| highlight_cache.decorate(diff_file) } diff_files end @@ -28,16 +28,14 @@ module Gitlab override :write_cache def write_cache - cache.write_if_empty + highlight_cache.write_if_empty + diff_stats_cache.write_if_empty(diff_stats_collection) end override :clear_cache def clear_cache - cache.clear - end - - def cache_key - cache.key + highlight_cache.clear + diff_stats_cache.clear end def real_size @@ -46,8 +44,25 @@ module Gitlab private - def cache - @cache ||= Gitlab::Diff::HighlightCache.new(self) + def highlight_cache + strong_memoize(:highlight_cache) do + Gitlab::Diff::HighlightCache.new(self) + end + end + + def diff_stats_cache + strong_memoize(:diff_stats_cache) do + Gitlab::Diff::StatsCache.new(cachable_key: @merge_request_diff.cache_key) + end + end + + override :diff_stats_collection + def diff_stats_collection + strong_memoize(:diff_stats) do + next unless fetch_diff_stats? + + diff_stats_cache.read || super + end end end end diff --git a/lib/gitlab/diff/file_collection/wiki_page.rb b/lib/gitlab/diff/file_collection/wiki_page.rb new file mode 100644 index 00000000000..7873e85a0eb --- /dev/null +++ b/lib/gitlab/diff/file_collection/wiki_page.rb @@ -0,0 +1,24 @@ +# frozen_string_literal: true + +module Gitlab + module Diff + module FileCollection + class WikiPage < Base + def initialize(page, diff_options:) + commit = page.wiki.commit(page.version.commit) + diff_options = diff_options.merge( + expanded: true, + paths: [page.path] + ) + + super(commit, + # TODO: Uncouple diffing from projects + # https://gitlab.com/gitlab-org/gitlab/-/issues/217752 + project: page.wiki, + diff_options: diff_options, + diff_refs: commit.diff_refs) + end + end + end + end +end diff --git a/lib/gitlab/diff/position_tracer.rb b/lib/gitlab/diff/position_tracer.rb index a1c82ce9afc..1c21c35fa60 100644 --- a/lib/gitlab/diff/position_tracer.rb +++ b/lib/gitlab/diff/position_tracer.rb @@ -42,6 +42,10 @@ module Gitlab @cd_diffs ||= compare(new_diff_refs.start_sha, new_diff_refs.head_sha) end + def diff_file(position) + position.diff_file(project.repository) + end + private def compare(start_sha, head_sha, straight: false) diff --git a/lib/gitlab/diff/position_tracer/base_strategy.rb b/lib/gitlab/diff/position_tracer/base_strategy.rb index 65049daabf4..61250bd2473 100644 --- a/lib/gitlab/diff/position_tracer/base_strategy.rb +++ b/lib/gitlab/diff/position_tracer/base_strategy.rb @@ -8,6 +8,7 @@ module Gitlab delegate \ :project, + :diff_file, :ac_diffs, :bd_diffs, :cd_diffs, diff --git a/lib/gitlab/diff/position_tracer/image_strategy.rb b/lib/gitlab/diff/position_tracer/image_strategy.rb index 79244a17951..046a6782dda 100644 --- a/lib/gitlab/diff/position_tracer/image_strategy.rb +++ b/lib/gitlab/diff/position_tracer/image_strategy.rb @@ -5,22 +5,26 @@ module Gitlab class PositionTracer class ImageStrategy < BaseStrategy def trace(position) + a_path = position.old_path b_path = position.new_path + diff_file = diff_file(position) + a_mode = diff_file&.a_mode + b_mode = diff_file&.b_mode # If file exists in B->D (e.g. updated, renamed, removed), let the # note become outdated. - bd_diff = bd_diffs.diff_file_with_old_path(b_path) + bd_diff = bd_diffs.diff_file_with_old_path(b_path, b_mode) return { position: new_position(position, bd_diff), outdated: true } if bd_diff # If file still exists in the new diff, update the position. - cd_diff = cd_diffs.diff_file_with_new_path(bd_diff&.new_path || b_path) + cd_diff = cd_diffs.diff_file_with_new_path(b_path, b_mode) return { position: new_position(position, cd_diff), outdated: false } if cd_diff # If file exists in A->C (e.g. rebased and same changes were present # in target branch), let the note become outdated. - ac_diff = ac_diffs.diff_file_with_old_path(position.old_path) + ac_diff = ac_diffs.diff_file_with_old_path(a_path, a_mode) return { position: new_position(position, ac_diff), outdated: true } if ac_diff diff --git a/lib/gitlab/diff/position_tracer/line_strategy.rb b/lib/gitlab/diff/position_tracer/line_strategy.rb index 8db0fc6f963..e3c1e549b96 100644 --- a/lib/gitlab/diff/position_tracer/line_strategy.rb +++ b/lib/gitlab/diff/position_tracer/line_strategy.rb @@ -76,16 +76,20 @@ module Gitlab def trace_added_line(position) b_path = position.new_path b_line = position.new_line + diff_file = diff_file(position) + b_mode = diff_file&.b_mode - bd_diff = bd_diffs.diff_file_with_old_path(b_path) + bd_diff = bd_diffs.diff_file_with_old_path(b_path, b_mode) d_path = bd_diff&.new_path || b_path + d_mode = bd_diff&.b_mode || b_mode d_line = LineMapper.new(bd_diff).old_to_new(b_line) if d_line - cd_diff = cd_diffs.diff_file_with_new_path(d_path) + cd_diff = cd_diffs.diff_file_with_new_path(d_path, d_mode) c_path = cd_diff&.old_path || d_path + c_mode = cd_diff&.a_mode || d_mode c_line = LineMapper.new(cd_diff).new_to_old(d_line) if c_line @@ -98,7 +102,7 @@ module Gitlab else # If the line is no longer in the MR, we unfortunately cannot show # the current state on the CD diff, so we treat it as outdated. - ac_diff = ac_diffs.diff_file_with_new_path(c_path) + ac_diff = ac_diffs.diff_file_with_new_path(c_path, c_mode) { position: new_position(ac_diff, nil, c_line), outdated: true } end @@ -115,22 +119,26 @@ module Gitlab def trace_removed_line(position) a_path = position.old_path a_line = position.old_line + diff_file = diff_file(position) + a_mode = diff_file&.a_mode - ac_diff = ac_diffs.diff_file_with_old_path(a_path) + ac_diff = ac_diffs.diff_file_with_old_path(a_path, a_mode) c_path = ac_diff&.new_path || a_path + c_mode = ac_diff&.b_mode || a_mode c_line = LineMapper.new(ac_diff).old_to_new(a_line) if c_line - cd_diff = cd_diffs.diff_file_with_old_path(c_path) + cd_diff = cd_diffs.diff_file_with_old_path(c_path, c_mode) d_path = cd_diff&.new_path || c_path + d_mode = cd_diff&.b_mode || c_mode d_line = LineMapper.new(cd_diff).old_to_new(c_line) if d_line # If the line is still in C but also in D, it has turned from a # removed line into an unchanged one. - bd_diff = bd_diffs.diff_file_with_new_path(d_path) + bd_diff = bd_diffs.diff_file_with_new_path(d_path, d_mode) { position: new_position(bd_diff, nil, d_line), outdated: true } else @@ -148,17 +156,21 @@ module Gitlab a_line = position.old_line b_path = position.new_path b_line = position.new_line + diff_file = diff_file(position) + a_mode = diff_file&.a_mode + b_mode = diff_file&.b_mode - ac_diff = ac_diffs.diff_file_with_old_path(a_path) + ac_diff = ac_diffs.diff_file_with_old_path(a_path, a_mode) c_path = ac_diff&.new_path || a_path + c_mode = ac_diff&.b_mode || a_mode c_line = LineMapper.new(ac_diff).old_to_new(a_line) - bd_diff = bd_diffs.diff_file_with_old_path(b_path) + bd_diff = bd_diffs.diff_file_with_old_path(b_path, b_mode) d_line = LineMapper.new(bd_diff).old_to_new(b_line) - cd_diff = cd_diffs.diff_file_with_old_path(c_path) + cd_diff = cd_diffs.diff_file_with_old_path(c_path, c_mode) if c_line && d_line # If the line is still in C and D, it is still unchanged. diff --git a/lib/gitlab/diff/stats_cache.rb b/lib/gitlab/diff/stats_cache.rb new file mode 100644 index 00000000000..f38fb21d497 --- /dev/null +++ b/lib/gitlab/diff/stats_cache.rb @@ -0,0 +1,54 @@ +# frozen_string_literal: true +# +module Gitlab + module Diff + class StatsCache + include Gitlab::Metrics::Methods + include Gitlab::Utils::StrongMemoize + + EXPIRATION = 1.week + VERSION = 1 + + def initialize(cachable_key:) + @cachable_key = cachable_key + end + + def read + strong_memoize(:cached_values) do + content = cache.fetch(key) + + next unless content + + stats = content.map { |stat| Gitaly::DiffStats.new(stat) } + + Gitlab::Git::DiffStatsCollection.new(stats) + end + end + + def write_if_empty(stats) + return if cache.exist?(key) + return unless stats + + cache.write(key, stats.as_json, expires_in: EXPIRATION) + end + + def clear + cache.delete(key) + end + + private + + attr_reader :cachable_key + + def cache + Rails.cache + end + + def key + strong_memoize(:redis_key) do + ['diff_stats', cachable_key, VERSION].join(":") + end + end + end + end +end |