Add latest changes from gitlab-org/gitlab@master

.eslintrc.yml

@@ -31,7 +31,7 @@ rules:
     - error
     - allowElseIf: true
   import/no-useless-path-segments: off
-  import/order: warn
+  import/order: off
   lines-between-class-members: off
   # Disabled for now, to make the plugin-vue 4.5 -> 5.0 update smoother
   vue/no-confusing-v-for-v-if: error

.gitlab/ci/reports.gitlab-ci.yml

@@ -167,7 +167,6 @@ dependency_scanning:
           DS_ANALYZER_IMAGE_TAG \
           DS_DEFAULT_ANALYZERS \
           DS_EXCLUDED_PATHS \
-          DEP_SCAN_DISABLE_REMOTE_CHECKS \
           DS_DOCKER_CLIENT_NEGOTIATION_TIMEOUT \
           DS_PULL_ANALYZER_IMAGE_TIMEOUT \
           DS_RUN_ANALYZER_TIMEOUT \

changelogs/unreleased/36937-change-title-mr-approvals-settings.yml

+---
+title: Changed 'Add approvers' to 'Approval rules'
+merge_request: 21079
+author:
+type: other

doc/development/chatops_on_gitlabcom.md

@@ -14,7 +14,7 @@ tasks such as:
 To request access to Chatops on GitLab.com:
 1. Log into <https://ops.gitlab.net/users/sign_in> **using the same username** as for GitLab.com (you may have to rename it).
-1. Ask [a project member in the `chatops` project](https://ops.gitlab.net/gitlab-com/chatops/-/project_members) to add you by running `/chatops run member add <username> gitlab-com/chatops --ops`.
+1. Ask in the [#production](https://gitlab.slack.com/messages/production) channel to add you by running `/chatops run member add <username> gitlab-com/chatops --ops`.
 ## See also

lib/api/internal/pages.rb

@@ -25,7 +25,7 @@ module API
           end
           get "/" do
             host = Namespace.find_by_pages_host(params[:host]) || PagesDomain.find_by_domain(params[:host])
-            not_found! unless host
+            no_content! unless host
             virtual_domain = host.pages_virtual_domain
             no_content! unless virtual_domain

lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml

 # Read more about this feature here: https://docs.gitlab.com/ee/user/application_security/container_scanning/
 variables:
-  CS_MAJOR_VERSION: 1
+  CS_MAJOR_VERSION: 2
 container_scanning:
   stage: test

lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml

@@ -43,7 +43,6 @@ dependency_scanning:
           DS_ANALYZER_IMAGE_TAG \
           DS_DEFAULT_ANALYZERS \
           DS_EXCLUDED_PATHS \
-          DEP_SCAN_DISABLE_REMOTE_CHECKS \
           DS_DOCKER_CLIENT_NEGOTIATION_TIMEOUT \
           DS_PULL_ANALYZER_IMAGE_TIMEOUT \
           DS_RUN_ANALYZER_TIMEOUT \

lib/gitlab/diff/deprecated_highlight_cache.rb

+# frozen_string_literal: true
+#
+module Gitlab
+  module Diff
+    class DeprecatedHighlightCache
+      delegate :diffable, to: :@diff_collection
+      delegate :diff_options, to: :@diff_collection
+
+      def initialize(diff_collection, backend: Rails.cache)
+        @backend = backend
+        @diff_collection = diff_collection
+      end
+
+      # - Reads from cache
+      # - Assigns DiffFile#highlighted_diff_lines for cached files
+      def decorate(diff_file)
+        if content = read_file(diff_file)
+          diff_file.highlighted_diff_lines = content.map do |line|
+            Gitlab::Diff::Line.init_from_hash(line)
+          end
+        end
+      end
+
+      # It populates a Hash in order to submit a single write to the memory
+      # cache. This avoids excessive IO generated by N+1's (1 writing for
+      # each highlighted line or file).
+      def write_if_empty
+        return if cached_content.present?
+
+        @diff_collection.diff_files.each do |diff_file|
+          next unless cacheable?(diff_file)
+
+          diff_file_id = diff_file.file_identifier
+
+          cached_content[diff_file_id] = diff_file.highlighted_diff_lines.map(&:to_hash)
+        end
+
+        cache.write(key, cached_content, expires_in: 1.week)
+      end
+
+      def clear
+        cache.delete(key)
+      end
+
+      def key
+        [diffable, 'highlighted-diff-files', Gitlab::Diff::Line::SERIALIZE_KEYS, diff_options]
+      end
+
+      private
+
+      def read_file(diff_file)
+        cached_content[diff_file.file_identifier]
+      end
+
+      def cache
+        @backend
+      end
+
+      def cached_content
+        @cached_content ||= cache.read(key) || {}
+      end
+
+      def cacheable?(diff_file)
+        diffable.present? && diff_file.text? && diff_file.diffable?
+      end
+    end
+  end
+end

lib/gitlab/diff/file_collection/merge_request_diff.rb

@@ -37,7 +37,11 @@ module Gitlab
         private
         def cache
-          @cache ||= Gitlab::Diff::HighlightCache.new(self)
+          @cache ||= if Feature.enabled?(:hset_redis_diff_caching, project)
+                       Gitlab::Diff::HighlightCache.new(self)
+                     else
+                       Gitlab::Diff::DeprecatedHighlightCache.new(self)
+                     end
         end
       end
     end

lib/gitlab/diff/highlight_cache.rb

@@ -3,16 +3,19 @@
 module Gitlab
   module Diff
     class HighlightCache
-      delegate :diffable, to: :@diff_collection
+      EXPIRATION = 1.week
+      VERSION = 1
+
+      delegate :diffable,     to: :@diff_collection
       delegate :diff_options, to: :@diff_collection
-      def initialize(diff_collection, backend: Rails.cache)
-        @backend = backend
+      def initialize(diff_collection)
         @diff_collection = diff_collection
       end
       # - Reads from cache
       # - Assigns DiffFile#highlighted_diff_lines for cached files
+      #
       def decorate(diff_file)
         if content = read_file(diff_file)
           diff_file.highlighted_diff_lines = content.map do |line|
@@ -21,43 +24,95 @@ module Gitlab
         end
       end
-      # It populates a Hash in order to submit a single write to the memory
-      # cache. This avoids excessive IO generated by N+1's (1 writing for
-      # each highlighted line or file).
+      # For every file that isn't already contained in the redis hash, store the
+      #   result of #highlighted_diff_lines, then submit the uncached content
+      #   to #write_to_redis_hash to submit a single write. This avoids excessive
+      #   IO generated by N+1's (1 writing for each highlighted line or file).
+      #
       def write_if_empty
-        return if cached_content.present?
-        @diff_collection.diff_files.each do |diff_file|
+        return if uncached_files.empty?
+        new_cache_content = {}
+        uncached_files.each do |diff_file|
           next unless cacheable?(diff_file)
-          diff_file_id = diff_file.file_identifier
-
-          cached_content[diff_file_id] = diff_file.highlighted_diff_lines.map(&:to_hash)
+          new_cache_content[diff_file.file_path] = diff_file.highlighted_diff_lines.map(&:to_hash)
         end
-        cache.write(key, cached_content, expires_in: 1.week)
+        write_to_redis_hash(new_cache_content)
       end
       def clear
-        cache.delete(key)
+        Gitlab::Redis::Cache.with do |redis|
+          redis.del(key)
+        end
       end
       def key
-        [diffable, 'highlighted-diff-files', Gitlab::Diff::Line::SERIALIZE_KEYS, diff_options]
+        @redis_key ||= ['highlighted-diff-files', diffable.cache_key, VERSION, diff_options].join(":")
       end
       private
-      def read_file(diff_file)
-        cached_content[diff_file.file_identifier]
+      def uncached_files
+        diff_files = @diff_collection.diff_files
+
+        diff_files.select { |file| read_cache[file.file_path].nil? }
       end
-      def cache
-        @backend
+      # Given a hash of:
+      #   { "file/to/cache" =>
+      #   [ { line_code: "a5cc2925ca8258af241be7e5b0381edf30266302_19_19",
+      #       rich_text: " <span id=\"LC19\" class=\"line\" lang=\"plaintext\">config/initializers/secret_token.rb</span>\n",
+      #       text: " config/initializers/secret_token.rb",
+      #       type: nil,
+      #       index: 3,
+      #       old_pos: 19,
+      #       new_pos: 19 }
+      #   ] }
+      #
+      #   ...it will write/update a Gitlab::Redis hash (HSET)
+      #
+      def write_to_redis_hash(hash)
+        Gitlab::Redis::Cache.with do |redis|
+          redis.pipelined do
+            hash.each do |diff_file_id, highlighted_diff_lines_hash|
+              redis.hset(key, diff_file_id, highlighted_diff_lines_hash.to_json)
+            end
+
+            # HSETs have to have their expiration date manually updated
+            #
+            redis.expire(key, EXPIRATION)
+          end
+        end
+      end
+
+      def file_paths
+        @file_paths ||= @diff_collection.diffs.collect(&:file_path)
+      end
+
+      def read_file(diff_file)
+        cached_content[diff_file.file_path]
       end
       def cached_content
-        @cached_content ||= cache.read(key) || {}
+        @cached_content ||= read_cache
+      end
+
+      def read_cache
+        return {} unless file_paths.any?
+
+        results = []
+
+        Gitlab::Redis::Cache.with do |redis|
+          results = redis.hmget(key, file_paths)
+        end
+
+        results.map! do |result|
+          JSON.parse(result, symbolize_names: true) unless result.nil?
+        end
+
+        file_paths.zip(results).to_h
       end
       def cacheable?(diff_file)

lib/gitlab/diff/line.rb

@@ -3,6 +3,9 @@
 module Gitlab
   module Diff
     class Line
+      # When SERIALIZE_KEYS is updated, to reset the redis cache entries you'll
+      #   need to bump the VERSION constant on Gitlab::Diff::HighlightCache
+      #
       SERIALIZE_KEYS = %i(line_code rich_text text type index old_pos new_pos).freeze
       attr_reader :line_code, :type, :old_pos, :new_pos

locale/gitlab.pot

@@ -984,9 +984,6 @@ msgstr ""
 msgid "Add approval rule"
 msgstr ""
-msgid "Add approvers"
-msgstr ""
-
 msgid "Add bold text"
 msgstr ""
@@ -1913,6 +1910,9 @@ msgstr ""
 msgid "Applying suggestion"
 msgstr ""
+msgid "Approval rules"
+msgstr ""
+
 msgid "ApprovalRuleRemove|%d member"
 msgid_plural "ApprovalRuleRemove|%d members"
 msgstr[0] ""

package.json

@@ -4,7 +4,7 @@
     "check-dependencies": "scripts/frontend/check_dependencies.sh",
     "clean": "rm -rf public/assets tmp/cache/*-loader",
     "dev-server": "NODE_OPTIONS=\"--max-old-space-size=3584\" nodemon -w 'config/webpack.config.js' --exec 'webpack-dev-server --config config/webpack.config.js'",
-    "eslint": "eslint --max-warnings 900 --report-unused-disable-directives --ext .js,.vue .",
+    "eslint": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue .",
     "eslint-fix": "eslint --max-warnings 0 --report-unused-disable-directives --ext .js,.vue --fix .",
     "eslint-report": "eslint --max-warnings 0 --ext .js,.vue --format html --output-file ./eslint-report.html --no-inline-config .",
     "file-coverage": "scripts/frontend/file_test_coverage.js",

spec/features/groups/group_settings_spec.rb

@@ -19,6 +19,7 @@ describe 'Edit group settings' do
     it 'the group is accessible via the new path' do
       update_path(new_group_path)
       visit new_group_full_path
+
       expect(current_path).to eq(new_group_full_path)
       expect(find('h1.home-panel-title')).to have_content(group.name)
     end
@@ -26,6 +27,7 @@ describe 'Edit group settings' do
     it 'the old group path redirects to the new path' do
       update_path(new_group_path)
       visit old_group_full_path
+
       expect(current_path).to eq(new_group_full_path)
       expect(find('h1.home-panel-title')).to have_content(group.name)
     end
@@ -38,6 +40,7 @@ describe 'Edit group settings' do
       it 'the subgroup is accessible via the new path' do
         update_path(new_group_path)
         visit new_subgroup_full_path
+
         expect(current_path).to eq(new_subgroup_full_path)
         expect(find('h1.home-panel-title')).to have_content(subgroup.name)
       end
@@ -45,6 +48,7 @@ describe 'Edit group settings' do
       it 'the old subgroup path redirects to the new path' do
         update_path(new_group_path)
         visit old_subgroup_full_path
+
         expect(current_path).to eq(new_subgroup_full_path)
         expect(find('h1.home-panel-title')).to have_content(subgroup.name)
       end
@@ -66,6 +70,7 @@ describe 'Edit group settings' do
       it 'the project is accessible via the new path' do
         update_path(new_group_path)
         visit new_project_full_path
+
         expect(current_path).to eq(new_project_full_path)
         expect(find('.breadcrumbs')).to have_content(project.path)
       end
@@ -73,6 +78,7 @@ describe 'Edit group settings' do
       it 'the old project path redirects to the new path' do
         update_path(new_group_path)
         visit old_project_full_path
+
         expect(current_path).to eq(new_project_full_path)
         expect(find('.breadcrumbs')).to have_content(project.path)
       end
@@ -101,7 +107,7 @@ describe 'Edit group settings' do
       attach_file(:group_avatar, Rails.root.join('spec', 'fixtures', 'banana_sample.gif'))
-      expect { save_group }.to change { group.reload.avatar? }.to(true)
+      expect { save_general_group }.to change { group.reload.avatar? }.to(true)
     end
     it 'uploads new group avatar' do
@@ -132,6 +138,21 @@ describe 'Edit group settings' do
     end
   end
+  context 'disable email notifications' do
+    it 'is visible' do
+      visit edit_group_path(group)
+
+      expect(page).to have_selector('#group_emails_disabled', visible: true)
+    end
+
+    it 'accepts the changed state' do
+      visit edit_group_path(group)
+      check 'group_emails_disabled'
+
+      expect { save_permissions_group }.to change { updated_emails_disabled? }.to(true)
+    end
+  end
+
   def update_path(new_group_path)
     visit edit_group_path(group)
@@ -141,9 +162,20 @@ describe 'Edit group settings' do
     end
   end
-  def save_group
+  def save_general_group
     page.within('.gs-general') do
       click_button 'Save changes'
     end
   end
+
+  def save_permissions_group
+    page.within('.gs-permissions') do
+      click_button 'Save changes'
+    end
+  end
+
+  def updated_emails_disabled?
+    group.reload.clear_memoization(:emails_disabled)
+    group.emails_disabled?
+  end
 end

spec/features/projects/settings/visibility_settings_spec.rb

@@ -64,6 +64,12 @@ describe 'Projects > Settings > Visibility settings', :js do
       it 'is visible' do
         expect(page).to have_selector('.js-emails-disabled', visible: true)
       end
+
+      it 'accepts the changed state' do
+        find('.js-emails-disabled input[type="checkbox"]').click
+
+        expect { save_permissions_group }.to change { updated_emails_disabled? }.to(true)
+      end
     end
   end
@@ -89,4 +95,16 @@ describe 'Projects > Settings > Visibility settings', :js do
       end
     end
   end
+
+  def save_permissions_group
+    page.within('.sharing-permissions') do
+      click_button 'Save changes'
+      wait_for_requests
+    end
+  end
+
+  def updated_emails_disabled?
+    project.reload.clear_memoization(:emails_disabled)
+    project.emails_disabled?
+  end
 end

spec/lib/gitlab/diff/deprecated_highlight_cache_spec.rb

+# frozen_string_literal: true
+
+require 'spec_helper'
+
+describe Gitlab::Diff::DeprecatedHighlightCache do
+  let(:merge_request) { create(:merge_request_with_diffs) }
+
+  subject(:cache) { described_class.new(merge_request.diffs, backend: backend) }
+
+  describe '#decorate' do
+    let(:backend) { double('backend').as_null_object }
+
+    # Manually creates a Diff::File object to avoid triggering the cache on
+    # the FileCollection::MergeRequestDiff
+    let(:diff_file) do
+      diffs = merge_request.diffs
+      raw_diff = diffs.diffable.raw_diffs(diffs.diff_options.merge(paths: ['CHANGELOG'])).first
+      Gitlab::Diff::File.new(raw_diff,
+                             repository: diffs.project.repository,
+                             diff_refs: diffs.diff_refs,
+                             fallback_diff_refs: diffs.fallback_diff_refs)
+    end
+
+    it 'does not calculate highlighting when reading from cache' do
+      cache.write_if_empty
+      cache.decorate(diff_file)
+
+      expect_any_instance_of(Gitlab::Diff::Highlight).not_to receive(:highlight)
+
+      diff_file.highlighted_diff_lines
+    end
+
+    it 'assigns highlighted diff lines to the DiffFile' do
+      cache.write_if_empty
+      cache.decorate(diff_file)
+
+      expect(diff_file.highlighted_diff_lines.size).to be > 5
+    end
+
+    it 'submits a single reading from the cache' do
+      cache.decorate(diff_file)
+      cache.decorate(diff_file)
+
+      expect(backend).to have_received(:read).with(cache.key).once
+    end
+  end
+
+  describe '#write_if_empty' do
+    let(:backend) { double('backend', read: {}).as_null_object }
+
+    it 'submits a single writing to the cache' do
+      cache.write_if_empty
+      cache.write_if_empty
+
+      expect(backend).to have_received(:write).with(cache.key,
+                                                    hash_including('CHANGELOG-false-false-false'),
+                                                    expires_in: 1.week).once
+    end
+  end
+
+  describe '#clear' do
+    let(:backend) { double('backend').as_null_object }
+
+    it 'clears cache' do
+      cache.clear
+
+      expect(backend).to have_received(:delete).with(cache.key)
+    end
+  end
+end

spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb

@@ -29,13 +29,19 @@ describe Gitlab::Diff::FileCollection::MergeRequestDiff do
     let(:diffable) { merge_request.merge_request_diff }
   end
-  it 'uses a different cache key if diff line keys change' do
-    mr_diff = described_class.new(merge_request.merge_request_diff, diff_options: nil)
-    key = mr_diff.cache_key
-    stub_const('Gitlab::Diff::Line::SERIALIZE_KEYS', [:foo])
-    expect(mr_diff.cache_key).not_to eq(key)
+  context 'using Gitlab::Diff::DeprecatedHighlightCache' do
+    before do
+      stub_feature_flags(hset_redis_diff_caching: false)
+    end
+
+    it 'uses a different cache key if diff line keys change' do
+      mr_diff = described_class.new(merge_request.merge_request_diff, diff_options: nil)
+      key = mr_diff.cache_key
+      stub_const('Gitlab::Diff::Line::SERIALIZE_KEYS', [:foo])
+      expect(mr_diff.cache_key).not_to eq(key)
+    end
   end
   it_behaves_like 'diff statistics' do

spec/lib/gitlab/diff/highlight_cache_spec.rb

@@ -2,14 +2,46 @@
 require 'spec_helper'
-describe Gitlab::Diff::HighlightCache do
+describe Gitlab::Diff::HighlightCache, :clean_gitlab_redis_cache do
   let(:merge_request) { create(:merge_request_with_diffs) }
-  subject(:cache) { described_class.new(merge_request.diffs, backend: backend) }
-  describe '#decorate' do
-    let(:backend) { double('backend').as_null_object }
+  let(:diff_hash) do
+    { ".gitignore-false-false-false" =>
+      [{ line_code: nil, rich_text: nil, text: "@@ -17,3 +17,4 @@ rerun.txt", type: "match", index: 0, old_pos: 17, new_pos: 17 },
+       { line_code: "a5cc2925ca8258af241be7e5b0381edf30266302_17_17",
+        rich_text: " <span id=\"LC17\" class=\"line\" lang=\"plaintext\">pickle-email-*.html</span>\n",
+        text: " pickle-email-*.html",
+        type: nil,
+        index: 1,
+        old_pos: 17,
+        new_pos: 17 },
+       { line_code: "a5cc2925ca8258af241be7e5b0381edf30266302_18_18",
+        rich_text: " <span id=\"LC18\" class=\"line\" lang=\"plaintext\">.project</span>\n",
+        text: " .project",
+        type: nil,
+        index: 2,
+        old_pos: 18,
+        new_pos: 18 },
+       { line_code: "a5cc2925ca8258af241be7e5b0381edf30266302_19_19",
+        rich_text: " <span id=\"LC19\" class=\"line\" lang=\"plaintext\">config/initializers/secret_token.rb</span>\n",
+        text: " config/initializers/secret_token.rb",
+        type: nil,
+        index: 3,
+        old_pos: 19,
+        new_pos: 19 },
+       { line_code: "a5cc2925ca8258af241be7e5b0381edf30266302_20_20",
+        rich_text: "+<span id=\"LC20\" class=\"line\" lang=\"plaintext\">.DS_Store</span>",
+        text: "+.DS_Store",
+        type: "new",
+        index: 4,
+        old_pos: 20,
+        new_pos: 20 }] }
+  end
+  let(:cache_key) { cache.key }
+  subject(:cache) { described_class.new(merge_request.diffs) }
+  describe '#decorate' do
     # Manually creates a Diff::File object to avoid triggering the cache on
     # the FileCollection::MergeRequestDiff
     let(:diff_file) do
@@ -36,35 +68,43 @@ describe Gitlab::Diff::HighlightCache do
       expect(diff_file.highlighted_diff_lines.size).to be > 5
     end
-    it 'submits a single reading from the cache' do
-      cache.decorate(diff_file)
-      cache.decorate(diff_file)
-      expect(backend).to have_received(:read).with(cache.key).once
+  end
+  describe '#write_if_empty' do
+    it 'filters the key/value list of entries to be caches for each invocation' do
+      expect(cache).to receive(:write_to_redis_hash)
+        .once.with(hash_including(".gitignore")).and_call_original
+      expect(cache).to receive(:write_to_redis_hash).once.with({}).and_call_original
+      2.times { cache.write_if_empty }
     end
-  end
-  describe '#write_if_empty' do
-    let(:backend) { double('backend', read: {}).as_null_object }
-    it 'submits a single writing to the cache' do
-      cache.write_if_empty
-      cache.write_if_empty
-      expect(backend).to have_received(:write).with(cache.key,
-                                                    hash_including('CHANGELOG-false-false-false'),
-                                                    expires_in: 1.week).once
+    context 'different diff_collections for the same diffable' do
+      before do
+        cache.write_if_empty
+      end
+      it 'writes an uncached files in the collection to the same redis hash' do
+        Gitlab::Redis::Cache.with { |r| r.hdel(cache_key, "files/whitespace") }
+        expect { cache.write_if_empty }
+          .to change { Gitlab::Redis::Cache.with { |r| r.hgetall(cache_key) } }
+      end
     end
   end
-  describe '#clear' do
-    let(:backend) { double('backend').as_null_object }
+  describe '#write_to_redis_hash' do
+    it 'creates or updates a Redis hash' do
+      expect { cache.send(:write_to_redis_hash, diff_hash) }
+        .to change { Gitlab::Redis::Cache.with { |r| r.hgetall(cache_key) } }
+    end
+  end
+  describe '#clear' do
     it 'clears cache' do
-      cache.clear
-      expect(backend).to have_received(:delete).with(cache.key)
+      expect_any_instance_of(Redis).to receive(:del).with(cache_key)
+      cache.clear
     end
   end
 end

spec/requests/api/internal/pages_spec.rb

@@ -47,11 +47,12 @@ describe API::Internal::Pages do
           project.mark_pages_as_deployed
         end
-        context 'not existing host' do
-          it 'responds with 404 Not Found' do
+        context 'domain does not exist' do
+          it 'responds with 204 no content' do
             query_host('pages.gitlab.io')
-            expect(response).to have_gitlab_http_status(404)
+            expect(response).to have_gitlab_http_status(204)
+            expect(response.body).to be_empty
           end
         end

spec/services/merge_requests/reload_diffs_service_spec.rb

@@ -33,13 +33,34 @@ describe MergeRequests::ReloadDiffsService, :use_clean_rails_memory_store_cachin
     end
     context 'cache clearing' do
-      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
-        expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original
-        subject.execute
+      context 'using Gitlab::Diff::DeprecatedHighlightCache' do
+        before do
+          stub_feature_flags(hset_redis_diff_caching: false)
+        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
+          expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original
+
+          subject.execute
+        end
+      end
+
+      context 'using Gitlab::Diff::HighlightCache' do
+        before do
+          stub_feature_flags(hset_redis_diff_caching: true)
+        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
+
+          expect_any_instance_of(Redis).to receive(:del).with(old_cache_key).and_call_original
+
+          subject.execute
+        end
       end
       it 'avoids N+1 queries', :request_store do