Commit 7ed103b4 authored by Douwe Maan's avatar Douwe Maan

Merge branch 'read-multi-rendered-objects' into 'master'

ObjectRenderer uses read_multi to read cache

## What does this MR do?

Get cache rendered content in bulk

## What are the relevant issue numbers?

#19273

## Does this MR meet the acceptance criteria?

- [x] [CHANGELOG](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CHANGELOG) entry added
- ~~[ ] [Documentation created/updated](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/doc/development/doc_styleguide.md)~~
- ~~[ ] API support added~~
- Tests
  - [x] Added for this feature/bug
  - [x] All builds are passing
- [x] Conform by the [style guides](https://gitlab.com/gitlab-org/gitlab-ce/blob/master/CONTRIBUTING.md#style-guides)
- [x] Branch has no merge conflicts with `master` (if you do - rebase it please)
- [x] [Squashed related commits together](https://git-scm.com/book/en/Git-Tools-Rewriting-History#Squashing-Commits)

See merge request !5096
parents ddc1f598 3d2c540d
...@@ -52,6 +52,7 @@ v 8.10.0 (unreleased) ...@@ -52,6 +52,7 @@ v 8.10.0 (unreleased)
- Don't instantiate a git tree on Projects show default view - Don't instantiate a git tree on Projects show default view
- Bump Rinku to 2.0.0 - Bump Rinku to 2.0.0
- Remove unused front-end variable -> default_issues_tracker - Remove unused front-end variable -> default_issues_tracker
- ObjectRenderer retrieve renderer content using Rails.cache.read_multi
- Better caching of git calls on ProjectsController#show. - Better caching of git calls on ProjectsController#show.
- Avoid to retrieve MR closes_issues as much as possible. - Avoid to retrieve MR closes_issues as much as possible.
- Add API endpoint for a group issues !4520 (mahcsig) - Add API endpoint for a group issues !4520 (mahcsig)
......
...@@ -3,6 +3,10 @@ module Banzai ...@@ -3,6 +3,10 @@ module Banzai
Renderer.render(text, context) Renderer.render(text, context)
end end
def self.cache_collection_render(texts_and_contexts)
Renderer.cache_collection_render(texts_and_contexts)
end
def self.render_result(text, context = {}) def self.render_result(text, context = {})
Renderer.render_result(text, context) Renderer.render_result(text, context)
end end
......
...@@ -39,9 +39,7 @@ module Banzai ...@@ -39,9 +39,7 @@ module Banzai
# Renders the attribute of every given object. # Renders the attribute of every given object.
def render_objects(objects, attribute) def render_objects(objects, attribute)
objects.map do |object| render_attributes(objects, attribute)
render_attribute(object, attribute)
end
end end
# Redacts the list of documents. # Redacts the list of documents.
...@@ -64,16 +62,21 @@ module Banzai ...@@ -64,16 +62,21 @@ module Banzai
context context
end end
# Renders the attribute of an object. # Renders the attributes of a set of objects.
# #
# Returns a `Nokogiri::HTML::Document`. # Returns an Array of `Nokogiri::HTML::Document`.
def render_attribute(object, attribute) def render_attributes(objects, attribute)
context = context_for(object, attribute) strings_and_contexts = objects.map do |object|
context = context_for(object, attribute)
string = object.__send__(attribute)
string = object.__send__(attribute) { text: string, context: context }
html = Banzai.render(string, context) end
Banzai::Pipeline[:relative_link].to_document(html, context) Banzai.cache_collection_render(strings_and_contexts).each_with_index.map do |html, index|
Banzai::Pipeline[:relative_link].to_document(html, strings_and_contexts[index][:context])
end
end end
def base_context def base_context
......
...@@ -10,7 +10,7 @@ module Banzai ...@@ -10,7 +10,7 @@ module Banzai
# requiring XHTML, such as Atom feeds, need to call `post_process` on the # requiring XHTML, such as Atom feeds, need to call `post_process` on the
# result, providing the appropriate `pipeline` option. # result, providing the appropriate `pipeline` option.
# #
# markdown - Markdown String # text - Markdown String
# context - Hash of context options passed to our HTML Pipeline # context - Hash of context options passed to our HTML Pipeline
# #
# Returns an HTML-safe String # Returns an HTML-safe String
...@@ -29,6 +29,52 @@ module Banzai ...@@ -29,6 +29,52 @@ module Banzai
end end
end end
# Perform multiple render from an Array of Markdown String into an
# Array of HTML-safe String of HTML.
#
# As the rendered Markdown String can be already cached read all the data
# from the cache using Rails.cache.read_multi operation. If the Markdown String
# is not in the cache or it's not cacheable (no cache_key entry is provided in
# the context) the Markdown String is rendered and stored in the cache so the
# next render call gets the rendered HTML-safe String from the cache.
#
# For further explanation see #render method comments.
#
# texts_and_contexts - An Array of Hashes that contains the Markdown String (:text)
# an options passed to our HTML Pipeline (:context)
#
# If on the :context you specify a :cache_key entry will be used to retrieve it
# and cache the result of rendering the Markdown String.
#
# Returns an Array containing HTML-safe String instances.
#
# Example:
# texts_and_contexts
# => [{ text: '### Hello',
# context: { cache_key: [note, :note] } }]
def self.cache_collection_render(texts_and_contexts)
items_collection = texts_and_contexts.each_with_index do |item, index|
context = item[:context]
cache_key = full_cache_multi_key(context.delete(:cache_key), context[:pipeline])
item[:cache_key] = cache_key if cache_key
end
cacheable_items, non_cacheable_items = items_collection.partition { |item| item.key?(:cache_key) }
items_in_cache = Rails.cache.read_multi(*cacheable_items.map { |item| item[:cache_key] })
items_not_in_cache = cacheable_items.reject do |item|
item[:rendered] = items_in_cache[item[:cache_key]]
items_in_cache.key?(item[:cache_key])
end
(items_not_in_cache + non_cacheable_items).each do |item|
item[:rendered] = render(item[:text], item[:context])
Rails.cache.write(item[:cache_key], item[:rendered]) if item[:cache_key]
end
items_collection.map { |item| item[:rendered] }
end
def self.render_result(text, context = {}) def self.render_result(text, context = {})
text = Pipeline[:pre_process].to_html(text, context) if text text = Pipeline[:pre_process].to_html(text, context) if text
...@@ -78,5 +124,13 @@ module Banzai ...@@ -78,5 +124,13 @@ module Banzai
return unless cache_key return unless cache_key
["banzai", *cache_key, pipeline_name || :full] ["banzai", *cache_key, pipeline_name || :full]
end end
# To map Rails.cache.read_multi results we need to know the Rails.cache.expanded_key.
# Other option will be to generate stringified keys on our side and don't delegate to Rails.cache.expanded_key
# method.
def self.full_cache_multi_key(cache_key, pipeline_name)
return unless cache_key
Rails.cache.send(:expanded_key, full_cache_key(cache_key, pipeline_name))
end
end end
end end
...@@ -29,7 +29,7 @@ describe Banzai::ObjectRenderer do ...@@ -29,7 +29,7 @@ describe Banzai::ObjectRenderer do
renderer = described_class.new(project, user) renderer = described_class.new(project, user)
expect(renderer).to receive(:render_attribute).with(object, :note). expect(renderer).to receive(:render_attributes).with([object], :note).
and_call_original and_call_original
rendered = renderer.render_objects([object], :note) rendered = renderer.render_objects([object], :note)
...@@ -89,14 +89,25 @@ describe Banzai::ObjectRenderer do ...@@ -89,14 +89,25 @@ describe Banzai::ObjectRenderer do
end end
end end
describe '#render_attribute' do describe '#render_attributes' do
it 'renders the attribute of an object' do it 'renders the attribute of a list of objects' do
object = double(:doc, note: 'hello') objects = [double(:doc, note: 'hello'), double(:doc, note: 'bye')]
renderer = described_class.new(project, user, pipeline: :note) renderer = described_class.new(project, user, pipeline: :note)
doc = renderer.render_attribute(object, :note)
expect(doc).to be_an_instance_of(Nokogiri::HTML::DocumentFragment) expect(Banzai).to receive(:cache_collection_render).
expect(doc.to_html).to eq('<p>hello</p>') with([
{ text: 'hello', context: renderer.context_for(objects[0], :note) },
{ text: 'bye', context: renderer.context_for(objects[1], :note) }
]).
and_call_original
docs = renderer.render_attributes(objects, :note)
expect(docs[0]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment)
expect(docs[0].to_html).to eq('<p>hello</p>')
expect(docs[1]).to be_an_instance_of(Nokogiri::HTML::DocumentFragment)
expect(docs[1].to_html).to eq('<p>bye</p>')
end end
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