diff options
21 files changed, 408 insertions, 69 deletions
diff --git a/.eslintrc.yml b/.eslintrc.yml index f95c8cf58af..9eba5b2ad3b 100644 --- a/.eslintrc.yml +++ b/.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 diff --git a/.gitlab/ci/reports.gitlab-ci.yml b/.gitlab/ci/reports.gitlab-ci.yml index 003a94bf4ad..7c32aa263c7 100644 --- a/.gitlab/ci/reports.gitlab-ci.yml +++ b/.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 \ diff --git a/changelogs/unreleased/36937-change-title-mr-approvals-settings.yml b/changelogs/unreleased/36937-change-title-mr-approvals-settings.yml new file mode 100644 index 00000000000..f38e7de5459 --- /dev/null +++ b/changelogs/unreleased/36937-change-title-mr-approvals-settings.yml @@ -0,0 +1,5 @@ +--- +title: Changed 'Add approvers' to 'Approval rules' +merge_request: 21079 +author: +type: other diff --git a/doc/development/chatops_on_gitlabcom.md b/doc/development/chatops_on_gitlabcom.md index 456dd1d4b4b..90b9cca54ac 100644 --- a/doc/development/chatops_on_gitlabcom.md +++ b/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 diff --git a/lib/api/internal/pages.rb b/lib/api/internal/pages.rb index 003af7f6dd4..a2fe3e09df8 100644 --- a/lib/api/internal/pages.rb +++ b/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 diff --git a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml index ef2fc561201..f708e95c2cf 100644 --- a/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml +++ b/lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml @@ -1,7 +1,7 @@ # 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 diff --git a/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml b/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml index 3f9d61a79f5..a37714eeed3 100644 --- a/lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml +++ b/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 \ diff --git a/lib/gitlab/diff/deprecated_highlight_cache.rb b/lib/gitlab/diff/deprecated_highlight_cache.rb new file mode 100644 index 00000000000..47347686973 --- /dev/null +++ b/lib/gitlab/diff/deprecated_highlight_cache.rb @@ -0,0 +1,68 @@ +# 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 diff --git a/lib/gitlab/diff/file_collection/merge_request_diff.rb b/lib/gitlab/diff/file_collection/merge_request_diff.rb index 3d661111f13..1d99349304b 100644 --- a/lib/gitlab/diff/file_collection/merge_request_diff.rb +++ b/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 diff --git a/lib/gitlab/diff/highlight_cache.rb b/lib/gitlab/diff/highlight_cache.rb index e4390771db2..12a1db70b36 100644 --- a/lib/gitlab/diff/highlight_cache.rb +++ b/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? + return if uncached_files.empty? - @diff_collection.diff_files.each do |diff_file| + 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) diff --git a/lib/gitlab/diff/line.rb b/lib/gitlab/diff/line.rb index 001748afb41..28ea1921f90 100644 --- a/lib/gitlab/diff/line.rb +++ b/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 diff --git a/locale/gitlab.pot b/locale/gitlab.pot index 52736edf84a..3ba5ffe617d 100644 --- a/locale/gitlab.pot +++ b/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] "" diff --git a/package.json b/package.json index d105fd2c210..92675b526f0 100644 --- a/package.json +++ b/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", diff --git a/spec/features/groups/group_settings_spec.rb b/spec/features/groups/group_settings_spec.rb index 41ecd21a386..d686e0ed9d2 100644 --- a/spec/features/groups/group_settings_spec.rb +++ b/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 diff --git a/spec/features/projects/settings/visibility_settings_spec.rb b/spec/features/projects/settings/visibility_settings_spec.rb index 0e757e647a0..a2b36874aea 100644 --- a/spec/features/projects/settings/visibility_settings_spec.rb +++ b/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 diff --git a/spec/lib/gitlab/diff/deprecated_highlight_cache_spec.rb b/spec/lib/gitlab/diff/deprecated_highlight_cache_spec.rb new file mode 100644 index 00000000000..7e46632ea77 --- /dev/null +++ b/spec/lib/gitlab/diff/deprecated_highlight_cache_spec.rb @@ -0,0 +1,70 @@ +# 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 diff --git a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb b/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb index d89be6fef4e..7f207d5d2ee 100644 --- a/spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb +++ b/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 + 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]) + stub_const('Gitlab::Diff::Line::SERIALIZE_KEYS', [:foo]) - expect(mr_diff.cache_key).not_to eq(key) + expect(mr_diff.cache_key).not_to eq(key) + end end it_behaves_like 'diff statistics' do diff --git a/spec/lib/gitlab/diff/highlight_cache_spec.rb b/spec/lib/gitlab/diff/highlight_cache_spec.rb index bfcfed4231f..97ebe6ae0e4 100644 --- a/spec/lib/gitlab/diff/highlight_cache_spec.rb +++ b/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) } + 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 - subject(:cache) { described_class.new(merge_request.diffs, backend: backend) } + let(:cache_key) { cache.key } - describe '#decorate' do - let(:backend) { double('backend').as_null_object } + 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 + end - it 'submits a single reading from the cache' do - cache.decorate(diff_file) - cache.decorate(diff_file) + 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 - expect(backend).to have_received(:read).with(cache.key).once + 2.times { cache.write_if_empty } end - end - describe '#write_if_empty' do - let(:backend) { double('backend', read: {}).as_null_object } + context 'different diff_collections for the same diffable' do + before do + cache.write_if_empty + end - it 'submits a single writing to the cache' do - cache.write_if_empty - cache.write_if_empty + 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(backend).to have_received(:write).with(cache.key, - hash_including('CHANGELOG-false-false-false'), - expires_in: 1.week).once + 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_any_instance_of(Redis).to receive(:del).with(cache_key) - expect(backend).to have_received(:delete).with(cache.key) + cache.clear end end end diff --git a/spec/requests/api/internal/pages_spec.rb b/spec/requests/api/internal/pages_spec.rb index 23bbd0681d6..2887163fe58 100644 --- a/spec/requests/api/internal/pages_spec.rb +++ b/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 diff --git a/spec/services/merge_requests/reload_diffs_service_spec.rb b/spec/services/merge_requests/reload_diffs_service_spec.rb index cc21348ab11..c450fc0a7dc 100644 --- a/spec/services/merge_requests/reload_diffs_service_spec.rb +++ b/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 + context 'using Gitlab::Diff::DeprecatedHighlightCache' do + before do + stub_feature_flags(hset_redis_diff_caching: false) + end - expect(Rails.cache).to receive(:delete).with(old_cache_key).and_call_original + 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 - subject.execute + 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 diff --git a/spec/services/notes/create_service_spec.rb b/spec/services/notes/create_service_spec.rb index cd4ea9c401d..79ea8f48a58 100644 --- a/spec/services/notes/create_service_spec.rb +++ b/spec/services/notes/create_service_spec.rb @@ -87,10 +87,28 @@ describe Notes::CreateService do .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) + context 'using Gitlab::Diff::DeprecatedHighlightCache' do + before do + stub_feature_flags(hset_redis_diff_caching: false) + end + + it 'clears noteable diff cache when it was unfolded for the note position' do + expect_any_instance_of(Gitlab::Diff::DeprecatedHighlightCache).to receive(:clear) + + described_class.new(project_with_repo, user, new_opts).execute + end + end - described_class.new(project_with_repo, user, new_opts).execute + context 'using Gitlab::Diff::HighlightCache' do + before do + stub_feature_flags(hset_redis_diff_caching: 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 end it 'does not clear cache when note is not the first of the discussion' do |