PERF: Do not include thumbnail information in default topic list payload (#10163)

Now it is only included when a theme/plugin has requested it.
This commit is contained in:
David Taylor 2020-07-06 10:59:21 +01:00 committed by GitHub
parent 5284d41a8e
commit cb1b472a0f
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
3 changed files with 38 additions and 33 deletions

View File

@ -101,12 +101,20 @@ class Topic < ActiveRecord::Base
end end
end end
def image_url def image_url(enqueue_if_missing: false)
thumbnail = topic_thumbnails.detect do |record| thumbnail = topic_thumbnails.detect do |record|
record.max_width == Topic.share_thumbnail_size[0] && record.max_width == Topic.share_thumbnail_size[0] &&
record.max_height == Topic.share_thumbnail_size[1] record.max_height == Topic.share_thumbnail_size[1]
end end
if thumbnail.nil? &&
image_upload &&
SiteSetting.create_thumbnails &&
enqueue_if_missing &&
Discourse.redis.set(thumbnail_job_redis_key([]), 1, nx: true, ex: 1.minute)
Jobs.enqueue(:generate_topic_thumbnails, { topic_id: id })
end
raw_url = thumbnail&.optimized_image&.url || image_upload&.url raw_url = thumbnail&.optimized_image&.url || image_upload&.url
UrlHelper.cook_url(raw_url, secure: image_upload&.secure?) UrlHelper.cook_url(raw_url, secure: image_upload&.secure?)
end end

View File

@ -31,7 +31,7 @@ class ListableTopicSerializer < BasicTopicSerializer
has_one :last_poster, serializer: BasicUserSerializer, embed: :objects has_one :last_poster, serializer: BasicUserSerializer, embed: :objects
def image_url def image_url
object.image_url object.image_url(enqueue_if_missing: true)
end end
def thumbnails def thumbnails
@ -39,6 +39,10 @@ class ListableTopicSerializer < BasicTopicSerializer
object.thumbnail_info(enqueue_if_missing: true, extra_sizes: extra_sizes) object.thumbnail_info(enqueue_if_missing: true, extra_sizes: extra_sizes)
end end
def include_thumbnails?
ThemeModifierHelper.new(request: scope.request).topic_thumbnail_sizes.present? || DiscoursePluginRegistry.topic_thumbnail_sizes.present?
end
def include_unicode_title? def include_unicode_title?
object.title.match?(/:[\w\-+]+:/) object.title.match?(/:[\w\-+]+:/)
end end

View File

@ -12,42 +12,15 @@ describe "Topic Thumbnails" do
def get_topic def get_topic
Discourse.redis.del(topic.thumbnail_job_redis_key(Topic.thumbnail_sizes)) Discourse.redis.del(topic.thumbnail_job_redis_key(Topic.thumbnail_sizes))
get '/latest.json' get '/latest.json'
expect(response.status).to eq(200)
response.parsed_body["topic_list"]["topics"][0] response.parsed_body["topic_list"]["topics"][0]
end end
it "includes thumbnails" do it "does not include thumbnails by default" do
topic_json = nil
expect do
topic_json = get_topic topic_json = get_topic
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(1)
thumbnails = topic_json["thumbnails"] expect(topic_json["thumbnails"]).to eq(nil)
# Original only. Optimized not yet generated
expect(thumbnails.length).to eq(1)
# Original
expect(thumbnails[0]["max_width"]).to eq(nil)
expect(thumbnails[0]["max_height"]).to eq(nil)
expect(thumbnails[0]["width"]).to eq(image.width)
expect(thumbnails[0]["height"]).to eq(image.height)
expect(thumbnails[0]["url"]).to end_with(image.url)
# Run the job
args = Jobs::GenerateTopicThumbnails.jobs.last["args"].first
Jobs::GenerateTopicThumbnails.new.execute(args.with_indifferent_access)
# Re-request
expect do
topic_json = get_topic
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(0)
thumbnails = topic_json["thumbnails"]
expect(thumbnails[1]["max_width"]).to eq(Topic.share_thumbnail_size[0])
expect(thumbnails[1]["max_height"]).to eq(Topic.share_thumbnail_size[1])
expect(thumbnails[1]["width"]).to eq(1024)
expect(thumbnails[1]["height"]).to eq(1024)
expect(thumbnails[1]["url"]).to include("/optimized/")
end end
context "with a theme" do context "with a theme" do
@ -69,6 +42,18 @@ describe "Topic Thumbnails" do
topic_json = get_topic topic_json = get_topic
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(1) end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(1)
thumbnails = topic_json["thumbnails"]
# Original only. Optimized not yet generated
expect(thumbnails.length).to eq(1)
# Original
expect(thumbnails[0]["max_width"]).to eq(nil)
expect(thumbnails[0]["max_height"]).to eq(nil)
expect(thumbnails[0]["width"]).to eq(image.width)
expect(thumbnails[0]["height"]).to eq(image.height)
expect(thumbnails[0]["url"]).to end_with(image.url)
# Run the job # Run the job
args = Jobs::GenerateTopicThumbnails.jobs.last["args"].first args = Jobs::GenerateTopicThumbnails.jobs.last["args"].first
Jobs::GenerateTopicThumbnails.new.execute(args.with_indifferent_access) Jobs::GenerateTopicThumbnails.new.execute(args.with_indifferent_access)
@ -82,6 +67,14 @@ describe "Topic Thumbnails" do
# Original + Optimized + 3 theme requests # Original + Optimized + 3 theme requests
expect(thumbnails.length).to eq(5) expect(thumbnails.length).to eq(5)
# Check first optimized
expect(thumbnails[1]["max_width"]).to eq(Topic.share_thumbnail_size[0])
expect(thumbnails[1]["max_height"]).to eq(Topic.share_thumbnail_size[1])
expect(thumbnails[1]["width"]).to eq(1024)
expect(thumbnails[1]["height"]).to eq(1024)
expect(thumbnails[1]["url"]).to include("/optimized/")
end end
end end