summaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorGitLab Bot <gitlab-bot@gitlab.com>2019-12-04 12:07:12 +0000
committerGitLab Bot <gitlab-bot@gitlab.com>2019-12-04 12:07:12 +0000
commit98420be3dddf5a093c39d96a8ca109aa21d0eaf8 (patch)
tree694cbe805e82d5383dc30c462f5efb60e55ccebe
parentc4038d4bdff93b260cbdcd69f9a6c0b07a849457 (diff)
downloadgitlab-ce-98420be3dddf5a093c39d96a8ca109aa21d0eaf8.tar.gz
Add latest changes from gitlab-org/gitlab@master
-rw-r--r--.eslintrc.yml2
-rw-r--r--.gitlab/ci/reports.gitlab-ci.yml1
-rw-r--r--changelogs/unreleased/36937-change-title-mr-approvals-settings.yml5
-rw-r--r--doc/development/chatops_on_gitlabcom.md2
-rw-r--r--lib/api/internal/pages.rb2
-rw-r--r--lib/gitlab/ci/templates/Security/Container-Scanning.gitlab-ci.yml2
-rw-r--r--lib/gitlab/ci/templates/Security/Dependency-Scanning.gitlab-ci.yml1
-rw-r--r--lib/gitlab/diff/deprecated_highlight_cache.rb68
-rw-r--r--lib/gitlab/diff/file_collection/merge_request_diff.rb6
-rw-r--r--lib/gitlab/diff/highlight_cache.rb93
-rw-r--r--lib/gitlab/diff/line.rb3
-rw-r--r--locale/gitlab.pot6
-rw-r--r--package.json2
-rw-r--r--spec/features/groups/group_settings_spec.rb36
-rw-r--r--spec/features/projects/settings/visibility_settings_spec.rb18
-rw-r--r--spec/lib/gitlab/diff/deprecated_highlight_cache_spec.rb70
-rw-r--r--spec/lib/gitlab/diff/file_collection/merge_request_diff_spec.rb16
-rw-r--r--spec/lib/gitlab/diff/highlight_cache_spec.rb82
-rw-r--r--spec/requests/api/internal/pages_spec.rb7
-rw-r--r--spec/services/merge_requests/reload_diffs_service_spec.rb31
-rw-r--r--spec/services/notes/create_service_spec.rb24
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