Merge branch 'faster-cache-keys' into 'master'

Optimize "cache_key" using a concern

## What does this MR do?

This MR adds a concern (used by Issue and Note) that provides an optimized version of Rails' `cache_key` method. See 77c8520e2ecd70520757aed0fbdf434643b60234 for more details.

## Are there points in the code the reviewer needs to double check?

No, though a spell check would be appreciated.

## Why was this MR needed?

When loading a lot of data from Redis (e.g. an issue with lots of notes) quite a large amount of time is spent in generating cache keys. This is due to multiple reasons such as:

* Rails trying to figure out if it should use `updated_at` or `updated_on` using somewhat inefficient code
* Rails relying on pluralization logic to figure out how to generate a cache namespace using a model name
* Rails calling a whole bunch of methods in general in the process of generating cache keys

In short, Rails is trying to cater to every possible use case, at the cost of performance.

## What are the relevant issue numbers?

https://gitlab.com/gitlab-org/gitlab-ce/issues/13651 is not directly related but I ran into this `cache_key` problem when looking into said issue.

See merge request !5715

5 files changed, 36 insertions, 0 deletions

diff --git a/CHANGELOG b/CHANGELOG
index ec2df962675..184704d8594 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -26,6 +26,7 @@ v 8.11.0 (unreleased)
   - Clean up unused routes (Josef Strzibny)
   - Fix issue on empty project to allow developers to only push to protected branches if given permission
   - Add green outline to New Branch button. !5447 (winniehell)
+  - Optimize generating of cache keys for issues and notes
   - Improve performance of syntax highlighting Markdown code blocks
   - Update to gitlab_git 10.4.1 and take advantage of preserved Ref objects
   - Remove delay when hitting "Reply..." button on page with a lot of discussions
diff --git a/app/models/concerns/faster_cache_keys.rb b/app/models/concerns/faster_cache_keys.rb
new file mode 100644
index 00000000000..5b14723fa2d
--- /dev/null
+++ b/app/models/concerns/faster_cache_keys.rb
@@ -0,0 +1,16 @@
+module FasterCacheKeys
+  # A faster version of Rails' "cache_key" method.
+  #
+  # Rails' default "cache_key" method uses all kind of complex logic to figure
+  # out the cache key. In many cases this complexity and overhead may not be
+  # needed.
+  #
+  # This method does not do any timestamp parsing as this process is quite
+  # expensive and not needed when generating cache keys. This method also relies
+  # on the table name instead of the cache namespace name as the latter uses
+  # complex logic to generate the exact same value (as when using the table
+  # name) in 99% of the cases.
+  def cache_key
+    "#{self.class.table_name}/#{id}-#{read_attribute_before_type_cast(:updated_at)}"
+  end
+end
diff --git a/app/models/issue.rb b/app/models/issue.rb
index 11f734cfc6d..d62ffb21467 100644
--- a/app/models/issue.rb
+++ b/app/models/issue.rb
@@ -7,6 +7,7 @@ class Issue < ActiveRecord::Base
   include Sortable
   include Taskable
   include Spammable
+  include FasterCacheKeys
 
   DueDateStruct = Struct.new(:title, :name).freeze
   NoDueDate     = DueDateStruct.new('No Due Date', '0').freeze
diff --git a/app/models/note.rb b/app/models/note.rb
index b6b2ac6aa42..ddcd7f9d034 100644
--- a/app/models/note.rb
+++ b/app/models/note.rb
@@ -5,6 +5,7 @@ class Note < ActiveRecord::Base
   include Mentionable
   include Awardable
   include Importable
+  include FasterCacheKeys
 
   # Attribute containing rendered and redacted Markdown as generated by
   # Banzai::ObjectRenderer.
diff --git a/spec/models/concerns/faster_cache_keys_spec.rb b/spec/models/concerns/faster_cache_keys_spec.rb
new file mode 100644
index 00000000000..8d3f94267fa
--- /dev/null
+++ b/spec/models/concerns/faster_cache_keys_spec.rb
@@ -0,0 +1,17 @@
+require 'spec_helper'
+
+describe FasterCacheKeys do
+  describe '#cache_key' do
+    it 'returns a String' do
+      # We're using a fixed string here so it's easier to set an expectation for
+      # the resulting cache key.
+      time = '2016-08-08 16:39:00+02'
+      issue = build(:issue, updated_at: time)
+      issue.extend(described_class)
+
+      expect(issue).to receive(:id).and_return(1)
+
+      expect(issue.cache_key).to eq("issues/1-#{time}")
+    end
+  end
+end