From 57bde0ce65caf2cbbb6a57f21435639cdaa06225 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 24 Mar 2016 15:53:38 +0100
Subject: [PATCH 1/7] Cache Banzai projects/objects using RequestStore

This was originally suggested by @ayufan and modified to be a bit
cleaner and use RequestStore instead of a regular Hash.

By caching the output of the two methods involved the number of queries
is reduced significantly. For example, for an issue with 200 notes (of
which 100 reference a number of merge requests) this cuts down the
amount of queries from around 6300 to around 3300.
---
 Gemfile                                       |  2 +-
 Gemfile.lock                                  |  4 +-
 .../filter/abstract_reference_filter.rb       | 71 +++++++++++++++++--
 3 files changed, 70 insertions(+), 7 deletions(-)

diff --git a/Gemfile b/Gemfile
index 006e53e0c1..6327227282 100644
--- a/Gemfile
+++ b/Gemfile
@@ -214,7 +214,7 @@ gem 'jquery-rails',       '~> 4.0.0'
 gem 'jquery-scrollto-rails', '~> 1.4.3'
 gem 'jquery-ui-rails',    '~> 5.0.0'
 gem 'raphael-rails',      '~> 2.1.2'
-gem 'request_store',      '~> 1.2.0'
+gem 'request_store',      '~> 1.3.0'
 gem 'select2-rails',      '~> 3.5.9'
 gem 'virtus',             '~> 1.0.1'
 gem 'net-ssh',            '~> 3.0.1'
diff --git a/Gemfile.lock b/Gemfile.lock
index bd41cc8419..229089f431 100644
--- a/Gemfile.lock
+++ b/Gemfile.lock
@@ -652,7 +652,7 @@ GEM
       redis-store (~> 1.1.0)
     redis-store (1.1.7)
       redis (>= 2.2)
-    request_store (1.2.1)
+    request_store (1.3.0)
     rerun (0.11.0)
       listen (~> 3.0)
     responders (2.1.1)
@@ -1011,7 +1011,7 @@ DEPENDENCIES
   redcarpet (~> 3.3.3)
   redis-namespace
   redis-rails (~> 4.0.0)
-  request_store (~> 1.2.0)
+  request_store (~> 1.3.0)
   rerun (~> 0.11.0)
   responders (~> 2.0)
   rouge (~> 1.10.1)
diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index 34c3891347..41fd4be76a 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -62,11 +62,53 @@ module Banzai
         # Example: project.merge_requests.find
       end
 
+      def find_object_cached(project, id)
+        if RequestStore.active?
+          cache = find_objects_cache[object_class][project.id]
+
+          if cache.key?(id)
+            cache[id]
+          else
+            cache[id] = find_object(project, id)
+          end
+        else
+          find_object(project, id)
+        end
+      end
+
+      def project_from_ref_cache(ref)
+        if RequestStore.active?
+          cache = project_refs_cache
+
+          if cache.key?(ref)
+            cache[ref]
+          else
+            cache[ref] = project_from_ref(ref)
+          end
+        else
+          project_from_ref(ref)
+        end
+      end
+
       def url_for_object(object, project)
         # Implement in child class
         # Example: project_merge_request_url
       end
 
+      def url_for_object_cached(object, project)
+        if RequestStore.active?
+          cache = url_for_object_cache[object_class][project.id]
+
+          if cache.key?(object)
+            cache[object]
+          else
+            cache[object] = url_for_object(object, project)
+          end
+        else
+          url_for_object(object, project)
+        end
+      end
+
       def call
         if object_class.reference_pattern
           # `#123`
@@ -109,9 +151,9 @@ module Banzai
       # have `gfm` and `gfm-OBJECT_NAME` class names attached for styling.
       def object_link_filter(text, pattern, link_text: nil)
         references_in(text, pattern) do |match, id, project_ref, matches|
-          project = project_from_ref(project_ref)
+          project = project_from_ref_cache(project_ref)
 
-          if project && object = find_object(project, id)
+          if project && object = find_object_cached(project, id)
             title = object_link_title(object)
             klass = reference_class(object_sym)
 
@@ -121,8 +163,11 @@ module Banzai
               object_sym => object.id
             )
 
-            url = matches[:url] if matches.names.include?("url")
-            url ||= url_for_object(object, project)
+            if matches.names.include?("url") && matches[:url]
+              url = matches[:url]
+            else
+              url = url_for_object_cached(object, project)
+            end
 
             text = link_text || object_link_text(object, matches)
 
@@ -157,6 +202,24 @@ module Banzai
 
         text
       end
+
+      private
+
+      def project_refs_cache
+        RequestStore[:banzai_project_refs] ||= {}
+      end
+
+      def find_objects_cache
+        RequestStore[:banzai_find_objects_cache] ||= Hash.new do |hash, key|
+          hash[key] = Hash.new { |h, k| h[k] = {} }
+        end
+      end
+
+      def url_for_object_cache
+        RequestStore[:banzai_url_for_object] ||= Hash.new do |hash, key|
+          hash[key] = Hash.new { |h, k| h[k] = {} }
+        end
+      end
     end
   end
 end

From 141148057703048f5c409c040c80c277f7747273 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 24 Mar 2016 15:57:22 +0100
Subject: [PATCH 2/7] Refactor processing of various Banzai filters

These filters now use a single iteration over all the document nodes
instead of multiple ones. This in turn allows variables to be re-used
(e.g. links only have to be unescaped once). Combined with some other
refactoring this can drastically reduce render timings.
---
 .../filter/abstract_reference_filter.rb       |  57 +++---
 .../filter/external_issue_reference_filter.rb |  24 ++-
 lib/banzai/filter/reference_filter.rb         | 176 ++++++------------
 lib/banzai/filter/user_reference_filter.rb    |  25 ++-
 4 files changed, 134 insertions(+), 148 deletions(-)

diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index 41fd4be76a..16b703a332 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -110,30 +110,45 @@ module Banzai
       end
 
       def call
-        if object_class.reference_pattern
-          # `#123`
-          replace_text_nodes_matching(object_class.reference_pattern) do |content|
-            object_link_filter(content, object_class.reference_pattern)
-          end
+        return doc if project.nil?
 
-          # `[Issue](#123)`, which is turned into
-          # `<a href="#123">Issue</a>`
-          replace_link_nodes_with_href(object_class.reference_pattern) do |link, text|
-            object_link_filter(link, object_class.reference_pattern, link_text: text)
-          end
-        end
+        ref_pattern = object_class.reference_pattern
+        link_pattern = object_class.link_reference_pattern
 
-        if object_class.link_reference_pattern
-          # `http://gitlab.example.com/namespace/project/issues/123`, which is turned into
-          # `<a href="http://gitlab.example.com/namespace/project/issues/123">http://gitlab.example.com/namespace/project/issues/123</a>`
-          replace_link_nodes_with_text(object_class.link_reference_pattern) do |text|
-            object_link_filter(text, object_class.link_reference_pattern)
-          end
+        each_node do |node|
+          if text_node?(node) && ref_pattern
+            replace_text_when_pattern_matches(node, ref_pattern) do |content|
+              object_link_filter(content, ref_pattern)
+            end
 
-          # `[Issue](http://gitlab.example.com/namespace/project/issues/123)`, which is turned into
-          # `<a href="http://gitlab.example.com/namespace/project/issues/123">Issue</a>`
-          replace_link_nodes_with_href(object_class.link_reference_pattern) do |link, text|
-            object_link_filter(link, object_class.link_reference_pattern, link_text: text)
+          elsif element_node?(node)
+            yield_valid_link(node) do |link, text|
+              if ref_pattern && link =~ /\A#{ref_pattern}/
+                replace_link_node_with_href(node, link) do
+                  object_link_filter(link, ref_pattern, link_text: text)
+                end
+
+                next
+              end
+
+              next unless link_pattern
+
+              if link == text && text =~ /\A#{link_pattern}/
+                replace_link_node_with_text(node, link) do
+                  object_link_filter(text, link_pattern)
+                end
+
+                next
+              end
+
+              if link =~ /\A#{link_pattern}\z/
+                replace_link_node_with_href(node, link) do
+                  object_link_filter(link, link_pattern, link_text: text)
+                end
+
+                next
+              end
+            end
           end
         end
 
diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb
index edc2638690..a3b66c4645 100644
--- a/lib/banzai/filter/external_issue_reference_filter.rb
+++ b/lib/banzai/filter/external_issue_reference_filter.rb
@@ -37,13 +37,27 @@ module Banzai
         # Early return if the project isn't using an external tracker
         return doc if project.nil? || project.default_issues_tracker?
 
-        replace_text_nodes_matching(ExternalIssue.reference_pattern) do |content|
-          issue_link_filter(content)
+        ref_pattern = ExternalIssue.reference_pattern
+        ref_start_pattern = /\A#{ref_pattern}\z/
+
+        each_node do |node|
+          if text_node?(node)
+            replace_text_when_pattern_matches(node, ref_pattern) do |content|
+              issue_link_filter(content)
+            end
+
+          elsif element_node?(node)
+            yield_valid_link(node) do |link, text|
+              if link =~ ref_start_pattern
+                replace_link_node_with_href(node, link) do
+                  issue_link_filter(link, link_text: text)
+                end
+              end
+            end
+          end
         end
 
-        replace_link_nodes_with_href(ExternalIssue.reference_pattern) do |link, text|
-          issue_link_filter(link, link_text: text)
-        end
+        doc
       end
 
       # Replace `JIRA-123` issue references in text with links to the referenced
diff --git a/lib/banzai/filter/reference_filter.rb b/lib/banzai/filter/reference_filter.rb
index a3326ae042..31386cf851 100644
--- a/lib/banzai/filter/reference_filter.rb
+++ b/lib/banzai/filter/reference_filter.rb
@@ -52,18 +52,13 @@ module Banzai
         html.html_safe? ? html : ERB::Util.html_escape_once(html)
       end
 
-      def ignore_parents
-        @ignore_parents ||= begin
-          # Don't look for references in text nodes that are children of these
-          # elements.
+      def ignore_ancestor_query
+        @ignore_ancestor_query ||= begin
           parents = %w(pre code a style)
           parents << 'blockquote' if context[:ignore_blockquotes]
-          parents.to_set
-        end
-      end
 
-      def ignored_ancestry?(node)
-        has_ancestor?(node, ignore_parents)
+          parents.map { |n| "ancestor::#{n}" }.join(' or ')
+        end
       end
 
       def project
@@ -74,120 +69,67 @@ module Banzai
         "gfm gfm-#{type}"
       end
 
-      # Iterate through the document's text nodes, yielding the current node's
-      # content if:
-      #
-      # * The `project` context value is present AND
-      # * The node's content matches `pattern` AND
-      # * The node is not an ancestor of an ignored node type
-      #
-      # pattern - Regex pattern against which to match the node's content
-      #
-      # Yields the current node's String contents. The result of the block will
-      # replace the node's existing content and update the current document.
-      #
-      # Returns the updated Nokogiri::HTML::DocumentFragment object.
-      def replace_text_nodes_matching(pattern)
-        return doc if project.nil?
-
-        search_text_nodes(doc).each do |node|
-          next if ignored_ancestry?(node)
-          next unless node.text =~ pattern
-
-          content = node.to_html
-
-          html = yield content
-
-          next if html == content
-
-          node.replace(html)
-        end
-
-        doc
-      end
-
-      # Iterate through the document's link nodes, yielding the current node's
-      # content if:
-      #
-      # * The `project` context value is present AND
-      # * The node's content matches `pattern`
-      #
-      # pattern - Regex pattern against which to match the node's content
-      #
-      # Yields the current node's String contents. The result of the block will
-      # replace the node and update the current document.
-      #
-      # Returns the updated Nokogiri::HTML::DocumentFragment object.
-      def replace_link_nodes_with_text(pattern)
-        return doc if project.nil?
-
-        doc.xpath('descendant-or-self::a').each do |node|
-          klass = node.attr('class')
-          next if klass && klass.include?('gfm')
-
-          link = node.attr('href')
-          text = node.text
-
-          next unless link && text
-
-          link = CGI.unescape(link)
-          next unless link.force_encoding('UTF-8').valid_encoding?
-          # Ignore ending punctionation like periods or commas
-          next unless link == text && text =~ /\A#{pattern}/
-
-          html = yield text
-
-          next if html == text
-
-          node.replace(html)
-        end
-
-        doc
-      end
-
-      # Iterate through the document's link nodes, yielding the current node's
-      # content if:
-      #
-      # * The `project` context value is present AND
-      # * The node's HREF matches `pattern`
-      #
-      # pattern - Regex pattern against which to match the node's HREF
-      #
-      # Yields the current node's String HREF and String content.
-      # The result of the block will replace the node and update the current document.
-      #
-      # Returns the updated Nokogiri::HTML::DocumentFragment object.
-      def replace_link_nodes_with_href(pattern)
-        return doc if project.nil?
-
-        doc.xpath('descendant-or-self::a').each do |node|
-          klass = node.attr('class')
-          next if klass && klass.include?('gfm')
-
-          link = node.attr('href')
-          text = node.text
-
-          next unless link && text
-          link = CGI.unescape(link)
-          next unless link.force_encoding('UTF-8').valid_encoding?
-          next unless link && link =~ /\A#{pattern}\z/
-
-          html = yield link, text
-
-          next if html == link
-
-          node.replace(html)
-        end
-
-        doc
-      end
-
       # Ensure that a :project key exists in context
       #
       # Note that while the key might exist, its value could be nil!
       def validate
         needs :project
       end
+
+      # Iterates over all <a> and text() nodes in a document.
+      #
+      # Nodes are skipped whenever their ancestor is one of the nodes returned
+      # by `ignore_ancestor_query`. Link tags are not processed if they have a
+      # "gfm" class or the "href" attribute is empty.
+      def each_node
+        query = %Q{descendant-or-self::text()[not(#{ignore_ancestor_query})]
+        | descendant-or-self::a[
+          not(contains(concat(" ", @class, " "), " gfm ")) and not(@href = "")
+        ]}
+
+        doc.xpath(query).each do |node|
+          yield node
+        end
+      end
+
+      # Yields the link's URL and text whenever the node is a valid <a> tag.
+      def yield_valid_link(node)
+        link = CGI.unescape(node.attr('href').to_s)
+        text = node.text
+
+        return unless link.force_encoding('UTF-8').valid_encoding?
+
+        yield link, text
+      end
+
+      def replace_text_when_pattern_matches(node, pattern)
+        return unless node.text =~ pattern
+
+        content = node.to_html
+        html = yield content
+
+        node.replace(html) unless content == html
+      end
+
+      def replace_link_node_with_text(node, link)
+        html = yield
+
+        node.replace(html) unless html == node.text
+      end
+
+      def replace_link_node_with_href(node, link)
+        html = yield
+
+        node.replace(html) unless html == link
+      end
+
+      def text_node?(node)
+        node.is_a?(Nokogiri::XML::Text)
+      end
+
+      def element_node?(node)
+        node.is_a?(Nokogiri::XML::Element)
+      end
     end
   end
 end
diff --git a/lib/banzai/filter/user_reference_filter.rb b/lib/banzai/filter/user_reference_filter.rb
index 989fa64e07..eea3af842b 100644
--- a/lib/banzai/filter/user_reference_filter.rb
+++ b/lib/banzai/filter/user_reference_filter.rb
@@ -59,13 +59,28 @@ module Banzai
       end
 
       def call
-        replace_text_nodes_matching(User.reference_pattern) do |content|
-          user_link_filter(content)
+        return doc if project.nil?
+
+        ref_pattern = User.reference_pattern
+        ref_pattern_start = /\A#{ref_pattern}\z/
+
+        each_node do |node|
+          if text_node?(node)
+            replace_text_when_pattern_matches(node, ref_pattern) do |content|
+              user_link_filter(content)
+            end
+          elsif element_node?(node)
+            yield_valid_link(node) do |link, text|
+              if link =~ ref_pattern_start
+                replace_link_node_with_href(node, link) do
+                  user_link_filter(link, link_text: text)
+                end
+              end
+            end
+          end
         end
 
-        replace_link_nodes_with_href(User.reference_pattern) do |link, text|
-          user_link_filter(link, link_text: text)
-        end
+        doc
       end
 
       # Replace `@user` user references in text with links to the referenced

From 8c49eaa937ed3d4332c54e8b0929c328a85d7fe4 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Thu, 24 Mar 2016 16:27:52 +0100
Subject: [PATCH 3/7] Cache Banzai class methods returning static data

These methods always return the same data for every class so there's no
point in computing their values on every call.
---
 lib/banzai/filter/abstract_reference_filter.rb | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index 16b703a332..051b94f9ce 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -11,15 +11,15 @@ module Banzai
       end
 
       def self.object_name
-        object_class.name.underscore
+        @object_name ||= object_class.name.underscore
       end
 
       def self.object_sym
-        object_name.to_sym
+        @object_sym ||= object_name.to_sym
       end
 
       def self.data_reference
-        "data-#{object_name.dasherize}"
+        @data_reference ||= "data-#{object_name.dasherize}"
       end
 
       # Public: Find references in text (like `!123` for merge requests)

From 9fa94326dba11ca3b9197a4f084ba2883c29bdff Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Tue, 29 Mar 2016 11:58:05 +0200
Subject: [PATCH 4/7] Memoize object class titles

For an issue with around 200 notes this cuts down timings by around 150
milliseconds.
---
 lib/banzai/filter/abstract_reference_filter.rb | 10 +++++++++-
 1 file changed, 9 insertions(+), 1 deletion(-)

diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index 051b94f9ce..02ef27cf57 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -22,6 +22,10 @@ module Banzai
         @data_reference ||= "data-#{object_name.dasherize}"
       end
 
+      def self.object_class_title
+        @object_title ||= object_class.name.titleize
+      end
+
       # Public: Find references in text (like `!123` for merge requests)
       #
       #   AnyReferenceFilter.references_in(text) do |match, id, project_ref, matches|
@@ -53,6 +57,10 @@ module Banzai
         self.class.object_sym
       end
 
+      def object_class_title
+        self.class.object_class_title
+      end
+
       def references_in(*args, &block)
         self.class.references_in(*args, &block)
       end
@@ -206,7 +214,7 @@ module Banzai
       end
 
       def object_link_title(object)
-        "#{object_class.name.titleize}: #{object.title}"
+        "#{object_class_title}: #{object.title}"
       end
 
       def object_link_text(object, matches)

From 7f0fd73eeb983782bac26bb983ec8ea52194b80a Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 30 Mar 2016 14:04:28 +0200
Subject: [PATCH 5/7] Cache default_issues_tracker? in Banzai

Every object processed by ExternalIssueReferenceFilter can return a
different Project instance when calling "project". For example, every
note processed will have it's own associated Project. If we were to
cache Project#default_issues_tracker? on Project level this would have
no impact on Markdown rendering timings as the cache would have to be
built for every Project instance without it ever being re-used.

To work around this we cache Project#default_issues_tracker? in
Banzai::Filter::ExternalIssueReferenceFilter using the project's _id_
instead of the whole object. This setup allows re-using of the cached
data even when the Project instances used are different, as long as the
actual project IDs are the same.
---
 .../filter/external_issue_reference_filter.rb   | 17 ++++++++++++++++-
 1 file changed, 16 insertions(+), 1 deletion(-)

diff --git a/lib/banzai/filter/external_issue_reference_filter.rb b/lib/banzai/filter/external_issue_reference_filter.rb
index a3b66c4645..37344b9057 100644
--- a/lib/banzai/filter/external_issue_reference_filter.rb
+++ b/lib/banzai/filter/external_issue_reference_filter.rb
@@ -35,7 +35,7 @@ module Banzai
 
       def call
         # Early return if the project isn't using an external tracker
-        return doc if project.nil? || project.default_issues_tracker?
+        return doc if project.nil? || default_issues_tracker?
 
         ref_pattern = ExternalIssue.reference_pattern
         ref_start_pattern = /\A#{ref_pattern}\z/
@@ -90,6 +90,21 @@ module Banzai
       def url_for_issue(*args)
         IssuesHelper.url_for_issue(*args)
       end
+
+      def default_issues_tracker?
+        if RequestStore.active?
+          default_issues_tracker_cache[project.id] ||=
+            project.default_issues_tracker?
+        else
+          project.default_issues_tracker?
+        end
+      end
+
+      private
+
+      def default_issues_tracker_cache
+        RequestStore[:banzai_default_issues_tracker_cache] ||= {}
+      end
     end
   end
 end

From ede351a99e33d825b7c1421388d2cce2ca1278d5 Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Wed, 30 Mar 2016 14:10:41 +0200
Subject: [PATCH 6/7] Added CHANGELOG entry for Markdown performance

---
 CHANGELOG | 1 +
 1 file changed, 1 insertion(+)

diff --git a/CHANGELOG b/CHANGELOG
index fc5e06ed94..f8f21ed0dc 100644
--- a/CHANGELOG
+++ b/CHANGELOG
@@ -2,6 +2,7 @@ Please view this file on the master branch, on stable branches it's out of date.
 
 v 8.7.0 (unreleased)
   - Don't attempt to fetch any tags from a forked repo (Stan Hu)
+  - Improved Markdown rendering performance !3389 (Yorick Peterse)
   - Don't attempt to look up an avatar in repo if repo directory does not exist (Stan hu)
   - Preserve time notes/comments have been updated at when moving issue
   - Make HTTP(s) label consistent on clone bar (Stan Hu)

From 915fd3f910921bc97abdda5f2ae63093c11533fd Mon Sep 17 00:00:00 2001
From: Yorick Peterse <yorickpeterse@gmail.com>
Date: Mon, 4 Apr 2016 11:39:11 +0200
Subject: [PATCH 7/7] Cleaned up caching in AbstractReferenceFilter

Cleaning this up any further is a bit tricky as the caches in question
should only be evaluated if RequestStore is actually enabled.
---
 .../filter/abstract_reference_filter.rb       | 26 ++++++++-----------
 1 file changed, 11 insertions(+), 15 deletions(-)

diff --git a/lib/banzai/filter/abstract_reference_filter.rb b/lib/banzai/filter/abstract_reference_filter.rb
index 02ef27cf57..f21dbef216 100644
--- a/lib/banzai/filter/abstract_reference_filter.rb
+++ b/lib/banzai/filter/abstract_reference_filter.rb
@@ -74,11 +74,7 @@ module Banzai
         if RequestStore.active?
           cache = find_objects_cache[object_class][project.id]
 
-          if cache.key?(id)
-            cache[id]
-          else
-            cache[id] = find_object(project, id)
-          end
+          get_or_set_cache(cache, id) { find_object(project, id) }
         else
           find_object(project, id)
         end
@@ -88,11 +84,7 @@ module Banzai
         if RequestStore.active?
           cache = project_refs_cache
 
-          if cache.key?(ref)
-            cache[ref]
-          else
-            cache[ref] = project_from_ref(ref)
-          end
+          get_or_set_cache(cache, ref) { project_from_ref(ref) }
         else
           project_from_ref(ref)
         end
@@ -107,11 +99,7 @@ module Banzai
         if RequestStore.active?
           cache = url_for_object_cache[object_class][project.id]
 
-          if cache.key?(object)
-            cache[object]
-          else
-            cache[object] = url_for_object(object, project)
-          end
+          get_or_set_cache(cache, object) { url_for_object(object, project) }
         else
           url_for_object(object, project)
         end
@@ -243,6 +231,14 @@ module Banzai
           hash[key] = Hash.new { |h, k| h[k] = {} }
         end
       end
+
+      def get_or_set_cache(cache, key)
+        if cache.key?(key)
+          cache[key]
+        else
+          cache[key] = yield
+        end
+      end
     end
   end
 end