CE-specific changes for designs `user_notes_count`13353-DesignType-notes_count-ce

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

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