diff options
6 files changed, 84 insertions, 54 deletions
diff --git a/app/services/base_count_service.rb b/app/services/base_count_service.rb index ad1647842b8..cfad2dd9265 100644 --- a/app/services/base_count_service.rb +++ b/app/services/base_count_service.rb @@ -35,7 +35,7 @@ class BaseCountService end 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 # subclasses can override to add any specific options, such as diff --git a/spec/services/projects/forks_count_service_spec.rb b/spec/services/projects/forks_count_service_spec.rb index 7e35648e9ff..1b44782468a 100644 --- a/spec/services/projects/forks_count_service_spec.rb +++ b/spec/services/projects/forks_count_service_spec.rb @@ -2,15 +2,17 @@ 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 it 'returns the number of forks' do - project = build(:project, id: 42) - service = described_class.new(project) - - allow(service).to receive(:uncached_count).and_return(1) + allow(subject).to receive(:uncached_count).and_return(1) - expect(service.count).to eq(1) + expect(subject.count).to eq(1) end end end diff --git a/spec/services/projects/open_issues_count_service_spec.rb b/spec/services/projects/open_issues_count_service_spec.rb index 8efa34765d0..593a4df1f8f 100644 --- a/spec/services/projects/open_issues_count_service_spec.rb +++ b/spec/services/projects/open_issues_count_service_spec.rb @@ -2,10 +2,13 @@ require 'spec_helper' -describe Projects::OpenIssuesCountService do - describe '#count' do - let(:project) { create(:project) } +describe Projects::OpenIssuesCountService, :use_clean_rails_memory_store_caching do + 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 it 'does not include confidential issues in the issue count' do create(:issue, :opened, project: project) @@ -53,9 +56,7 @@ describe Projects::OpenIssuesCountService do end end - context '#refresh_cache', :use_clean_rails_memory_store_caching do - let(:subject) { described_class.new(project) } - + context '#refresh_cache' do before do create(:issue, :opened, project: project) create(:issue, :opened, project: project) diff --git a/spec/services/projects/open_merge_requests_count_service_spec.rb b/spec/services/projects/open_merge_requests_count_service_spec.rb index 0d8227f7db5..f9fff4cbd4c 100644 --- a/spec/services/projects/open_merge_requests_count_service_spec.rb +++ b/spec/services/projects/open_merge_requests_count_service_spec.rb @@ -2,16 +2,21 @@ 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 it 'returns the number of open merge requests' do - project = create(:project) create(:merge_request, :opened, source_project: project, target_project: project) - expect(described_class.new(project).count).to eq(1) + expect(subject.count).to eq(1) end end end diff --git a/spec/services/users/keys_count_service_spec.rb b/spec/services/users/keys_count_service_spec.rb index bee8380e8b7..6b7493f343f 100644 --- a/spec/services/users/keys_count_service_spec.rb +++ b/spec/services/users/keys_count_service_spec.rb @@ -4,7 +4,9 @@ require 'spec_helper' describe Users::KeysCountService, :use_clean_rails_memory_store_caching do 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 before do @@ -12,53 +14,19 @@ describe Users::KeysCountService, :use_clean_rails_memory_store_caching do end it 'returns the number of SSH keys as an Integer' do - expect(service.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 + expect(subject.count).to eq(1) end end describe '#uncached_count' do it 'returns the number of SSH keys' do - expect(service.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 + expect(subject.uncached_count).to be_zero end end describe '#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 diff --git a/spec/support/shared_examples/services/count_service_shared_examples.rb b/spec/support/shared_examples/services/count_service_shared_examples.rb new file mode 100644 index 00000000000..9bea180a778 --- /dev/null +++ b/spec/support/shared_examples/services/count_service_shared_examples.rb @@ -0,0 +1,54 @@ +# 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 |