Commit a0b14c40 authored by Luke Duncalfe's avatar Luke Duncalfe Committed by Mayra Cabrera

CE-specific changes for designs `user_notes_count`

Notes call `#after_note_created` and `#after_note_destroyed` on their
noteable in callbacks, so the noteable can perform tasks particular to
them, like cache expiry.

This is in preparation of the EE-specific class
`DesignManagement::Design` clearing its `user_notes_count` cache when
its note are created or destroyed.

Refactoring Rspec behaviour testing of a counter caching service into a
shared example.

https://gitlab.com/gitlab-org/gitlab-ee/issues/13353
parent a58f4f00
...@@ -35,7 +35,7 @@ class BaseCountService ...@@ -35,7 +35,7 @@ class BaseCountService
end end
def cache_key def cache_key
raise NotImplementedError, 'cache_key must be implemented and return a String' raise NotImplementedError, 'cache_key must be implemented and return a String, Array, or Hash'
end end
# subclasses can override to add any specific options, such as # subclasses can override to add any specific options, such as
......
...@@ -2,15 +2,17 @@ ...@@ -2,15 +2,17 @@
require 'spec_helper' require 'spec_helper'
describe Projects::ForksCountService do describe Projects::ForksCountService, :use_clean_rails_memory_store_caching do
let(:project) { build(:project) }
subject { described_class.new(project) }
it_behaves_like 'a counter caching service'
describe '#count' do describe '#count' do
it 'returns the number of forks' do it 'returns the number of forks' do
project = build(:project, id: 42) allow(subject).to receive(:uncached_count).and_return(1)
service = described_class.new(project)
allow(service).to receive(:uncached_count).and_return(1)
expect(service.count).to eq(1) expect(subject.count).to eq(1)
end end
end end
end end
...@@ -2,10 +2,13 @@ ...@@ -2,10 +2,13 @@
require 'spec_helper' require 'spec_helper'
describe Projects::OpenIssuesCountService do describe Projects::OpenIssuesCountService, :use_clean_rails_memory_store_caching do
describe '#count' do let(:project) { create(:project) }
let(:project) { create(:project) } subject { described_class.new(project) }
it_behaves_like 'a counter caching service'
describe '#count' do
context 'when user is nil' do context 'when user is nil' do
it 'does not include confidential issues in the issue count' do it 'does not include confidential issues in the issue count' do
create(:issue, :opened, project: project) create(:issue, :opened, project: project)
...@@ -53,9 +56,7 @@ describe Projects::OpenIssuesCountService do ...@@ -53,9 +56,7 @@ describe Projects::OpenIssuesCountService do
end end
end end
context '#refresh_cache', :use_clean_rails_memory_store_caching do context '#refresh_cache' do
let(:subject) { described_class.new(project) }
before do before do
create(:issue, :opened, project: project) create(:issue, :opened, project: project)
create(:issue, :opened, project: project) create(:issue, :opened, project: project)
......
...@@ -2,16 +2,21 @@ ...@@ -2,16 +2,21 @@
require 'spec_helper' require 'spec_helper'
describe Projects::OpenMergeRequestsCountService do describe Projects::OpenMergeRequestsCountService, :use_clean_rails_memory_store_caching do
set(:project) { create(:project) }
subject { described_class.new(project) }
it_behaves_like 'a counter caching service'
describe '#count' do describe '#count' do
it 'returns the number of open merge requests' do it 'returns the number of open merge requests' do
project = create(:project)
create(:merge_request, create(:merge_request,
:opened, :opened,
source_project: project, source_project: project,
target_project: project) target_project: project)
expect(described_class.new(project).count).to eq(1) expect(subject.count).to eq(1)
end end
end end
end end
...@@ -4,7 +4,9 @@ require 'spec_helper' ...@@ -4,7 +4,9 @@ require 'spec_helper'
describe Users::KeysCountService, :use_clean_rails_memory_store_caching do describe Users::KeysCountService, :use_clean_rails_memory_store_caching do
let(:user) { create(:user) } let(:user) { create(:user) }
let(:service) { described_class.new(user) } subject { described_class.new(user) }
it_behaves_like 'a counter caching service'
describe '#count' do describe '#count' do
before do before do
...@@ -12,53 +14,19 @@ describe Users::KeysCountService, :use_clean_rails_memory_store_caching do ...@@ -12,53 +14,19 @@ describe Users::KeysCountService, :use_clean_rails_memory_store_caching do
end end
it 'returns the number of SSH keys as an Integer' do it 'returns the number of SSH keys as an Integer' do
expect(service.count).to eq(1) expect(subject.count).to eq(1)
end
it 'caches the number of keys in Redis', :request_store do
service.delete_cache
control_count = ActiveRecord::QueryRecorder.new { service.count }.count
service.delete_cache
expect { 2.times { service.count } }.not_to exceed_query_limit(control_count)
end
end
describe '#refresh_cache' do
it 'refreshes the Redis cache' do
Rails.cache.write(service.cache_key, 10)
service.refresh_cache
expect(Rails.cache.fetch(service.cache_key, raw: true)).to be_zero
end
end
describe '#delete_cache' do
it 'removes the cache' do
service.count
service.delete_cache
expect(Rails.cache.fetch(service.cache_key, raw: true)).to be_nil
end end
end end
describe '#uncached_count' do describe '#uncached_count' do
it 'returns the number of SSH keys' do it 'returns the number of SSH keys' do
expect(service.uncached_count).to be_zero expect(subject.uncached_count).to be_zero
end
it 'does not cache the number of keys' do
recorder = ActiveRecord::QueryRecorder.new do
2.times { service.uncached_count }
end
expect(recorder.count).to be > 0
end end
end end
describe '#cache_key' do describe '#cache_key' do
it 'returns the cache key' do it 'returns the cache key' do
expect(service.cache_key).to eq("users/key-count-service/#{user.id}") expect(subject.cache_key).to eq("users/key-count-service/#{user.id}")
end end
end end
end end
# frozen_string_literal: true
# The calling spec should use `:use_clean_rails_memory_store_caching`
# when including this shared example. E.g.:
#
# describe MyCountService, :use_clean_rails_memory_store_caching do
# it_behaves_like 'a counter caching service'
# end
shared_examples 'a counter caching service' do
describe '#count' do
it 'caches the count', :request_store do
subject.delete_cache
control_count = ActiveRecord::QueryRecorder.new { subject.count }.count
subject.delete_cache
expect { 2.times { subject.count } }.not_to exceed_query_limit(control_count)
end
end
describe '#refresh_cache' do
it 'refreshes the cache' do
original_count = subject.count
Rails.cache.write(subject.cache_key, original_count + 1, raw: subject.raw?)
subject.refresh_cache
expect(fetch_cache || 0).to eq(original_count)
end
end
describe '#delete_cache' do
it 'removes the cache' do
subject.count
subject.delete_cache
expect(fetch_cache).to be_nil
end
end
describe '#uncached_count' do
it 'does not cache the count' do
subject.delete_cache
subject.uncached_count
expect(fetch_cache).to be_nil
end
end
private
def fetch_cache
Rails.cache.read(subject.cache_key, raw: subject.raw?)
end
end
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment