FEATURE: Include optimized thumbnails for topics (#9215)

This introduces new APIs for obtaining optimized thumbnails for topics. There are a few building blocks required for this:

- Introduces new `image_upload_id` columns on the `posts` and `topics` table. This replaces the old `image_url` column, which means that thumbnails are now restricted to uploads. Hotlinked thumbnails are no longer possible. In normal use (with pull_hotlinked_images enabled), this has no noticeable impact

- A migration attempts to match existing urls to upload records. If a match cannot be found then the posts will be queued for rebake

- Optimized thumbnails are generated during post_process_cooked. If thumbnails are missing when serializing a topic list, then a sidekiq job is queued

- Topic lists and topics now include a `thumbnails` key, which includes all the available images:
   ```
   "thumbnails": [
   {
     "max_width": null,
     "max_height": null,
     "url": "//example.com/original-image.png",
     "width": 1380,
     "height": 1840
   },
   {
     "max_width": 1024,
     "max_height": 1024,
     "url": "//example.com/optimized-image.png",
     "width": 768,
     "height": 1024
   }
   ]
  ```

- Themes can request additional thumbnail sizes by using a modifier in their `about.json` file:
   ```
    "modifiers": {
      "topic_thumbnail_sizes": [
        [200, 200],
        [800, 800]
      ],
      ...
  ```
  Remember that these are generated asynchronously, so your theme should include logic to fallback to other available thumbnails if your requested size has not yet been generated

- Two new raw plugin outlets are introduced, to improve the customisability of the topic list. `topic-list-before-columns` and `topic-list-before-link`
This commit is contained in:
David Taylor 2020-05-05 09:07:50 +01:00 committed by GitHub
parent 5cf6984a1a
commit 03818e642a
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
25 changed files with 563 additions and 57 deletions

View File

@ -1,3 +1,5 @@
{{~raw-plugin-outlet name="topic-list-before-columns"}}
{{#if bulkSelectEnabled}} {{#if bulkSelectEnabled}}
<td class="bulk-select"> <td class="bulk-select">
<input type="checkbox" class="bulk-select"> <input type="checkbox" class="bulk-select">
@ -12,6 +14,7 @@
at the end of the link, preventing it from line wrapping onto its own line. at the end of the link, preventing it from line wrapping onto its own line.
--}} --}}
<td class='main-link clearfix' colspan="1"> <td class='main-link clearfix' colspan="1">
{{~raw-plugin-outlet name="topic-list-before-link"}}
<span class='link-top-line'> <span class='link-top-line'>
{{~raw-plugin-outlet name="topic-list-before-status"}} {{~raw-plugin-outlet name="topic-list-before-status"}}
{{~raw "topic-status" topic=topic}} {{~raw "topic-status" topic=topic}}

View File

@ -1,4 +1,5 @@
<td> <td>
{{~raw-plugin-outlet name="topic-list-before-columns"}}
{{~#unless expandPinned}} {{~#unless expandPinned}}
<div class='pull-left'> <div class='pull-left'>
<a href="{{topic.lastPostUrl}}" data-user-card="{{topic.last_poster_username}}">{{avatar topic.lastPosterUser imageSize="large"}}</a> <a href="{{topic.lastPostUrl}}" data-user-card="{{topic.last_poster_username}}">{{avatar topic.lastPosterUser imageSize="large"}}</a>
@ -14,6 +15,7 @@
This causes the topic-post-badge to be considered the same word as "text" This causes the topic-post-badge to be considered the same word as "text"
at the end of the link, preventing it from line wrapping onto its own line. at the end of the link, preventing it from line wrapping onto its own line.
--}} --}}
{{~raw-plugin-outlet name="topic-list-before-link"}}
<div class='main-link'> <div class='main-link'>
{{~raw-plugin-outlet name="topic-list-before-status"}} {{~raw-plugin-outlet name="topic-list-before-status"}}
{{~raw "topic-status" topic=topic~}} {{~raw "topic-status" topic=topic~}}

View File

@ -0,0 +1,18 @@
# frozen_string_literal: true
module Jobs
class GenerateTopicThumbnails < ::Jobs::Base
sidekiq_options queue: 'ultra_low'
def execute(args)
topic_id = args[:topic_id]
extra_sizes = args[:extra_sizes]
raise Discourse::InvalidParameters.new(:topic_id) if topic_id.blank?
topic = Topic.find(topic_id)
topic.generate_thumbnails!(extra_sizes: extra_sizes)
end
end
end

View File

@ -22,7 +22,8 @@ module Jobs
CategoryTagStat, CategoryTagStat,
User, User,
UserAvatar, UserAvatar,
Category Category,
TopicThumbnail
].each do |klass| ].each do |klass|
klass.ensure_consistency! klass.ensure_consistency!
measure(klass) measure(klass)

View File

@ -51,6 +51,8 @@ class Post < ActiveRecord::Base
has_many :user_actions, foreign_key: :target_post_id has_many :user_actions, foreign_key: :target_post_id
belongs_to :image_upload, class_name: "Upload"
validates_with PostValidator, unless: :skip_validation validates_with PostValidator, unless: :skip_validation
after_save :index_search after_save :index_search
@ -1062,6 +1064,10 @@ class Post < ActiveRecord::Base
Upload.where(access_control_post_id: self.id) Upload.where(access_control_post_id: self.id)
end end
def image_url
image_upload&.url
end
private private
def parse_quote_into_arguments(quote) def parse_quote_into_arguments(quote)
@ -1144,6 +1150,7 @@ end
# action_code :string # action_code :string
# image_url :string # image_url :string
# locked_by_id :integer # locked_by_id :integer
# image_upload_id :bigint
# #
# Indexes # Indexes
# #
@ -1152,6 +1159,7 @@ end
# idx_posts_user_id_deleted_at (user_id) WHERE (deleted_at IS NULL) # idx_posts_user_id_deleted_at (user_id) WHERE (deleted_at IS NULL)
# index_for_rebake_old (id) WHERE (((baked_version IS NULL) OR (baked_version < 2)) AND (deleted_at IS NULL)) # index_for_rebake_old (id) WHERE (((baked_version IS NULL) OR (baked_version < 2)) AND (deleted_at IS NULL))
# index_posts_on_id_and_baked_version (id DESC,baked_version) WHERE (deleted_at IS NULL) # index_posts_on_id_and_baked_version (id DESC,baked_version) WHERE (deleted_at IS NULL)
# index_posts_on_image_upload_id (image_upload_id)
# index_posts_on_reply_to_post_number (reply_to_post_number) # index_posts_on_reply_to_post_number (reply_to_post_number)
# index_posts_on_topic_id_and_percent_rank (topic_id,percent_rank) # index_posts_on_topic_id_and_percent_rank (topic_id,percent_rank)
# index_posts_on_topic_id_and_post_number (topic_id,post_number) UNIQUE # index_posts_on_topic_id_and_post_number (topic_id,post_number) UNIQUE

View File

@ -12,7 +12,7 @@ class ThemeModifierSet < ActiveRecord::Base
def type_validator def type_validator
ThemeModifierSet.modifiers.each do |k, config| ThemeModifierSet.modifiers.each do |k, config|
value = public_send(k) value = read_attribute(k)
next if value.nil? next if value.nil?
case config[:type] case config[:type]
@ -39,7 +39,7 @@ class ThemeModifierSet < ActiveRecord::Base
def self.resolve_modifier_for_themes(theme_ids, modifier_name) def self.resolve_modifier_for_themes(theme_ids, modifier_name)
return nil if !(config = self.modifiers[modifier_name]) return nil if !(config = self.modifiers[modifier_name])
all_values = self.where(theme_id: theme_ids).where.not(modifier_name => nil).pluck(modifier_name) all_values = self.where(theme_id: theme_ids).where.not(modifier_name => nil).map { |s| s.public_send(modifier_name) }
case config[:type] case config[:type]
when :boolean when :boolean
all_values.any? all_values.any?
@ -50,6 +50,26 @@ class ThemeModifierSet < ActiveRecord::Base
end end
end end
def topic_thumbnail_sizes
array = read_attribute(:topic_thumbnail_sizes)
return if array.nil?
array.map do |dimension|
parts = dimension.split("x")
next if parts.length != 2
[parts[0].to_i, parts[1].to_i]
end.filter(&:present?)
end
def topic_thumbnail_sizes=(val)
return write_attribute(:topic_thumbnail_sizes, val) if val.nil?
return write_attribute(:topic_thumbnail_sizes, val) if !val.is_a?(Array)
return write_attribute(:topic_thumbnail_sizes, val) if !val.all? { |v| v.is_a?(Array) && v.length == 2 }
super(val.map { |dim| "#{dim[0]}x#{dim[1]}" })
end
private private
# Build the list of modifiers from the DB schema. # Build the list of modifiers from the DB schema.
@ -78,11 +98,12 @@ end
# #
# Table name: theme_modifier_sets # Table name: theme_modifier_sets
# #
# id :bigint not null, primary key # id :bigint not null, primary key
# theme_id :bigint not null # theme_id :bigint not null
# serialize_topic_excerpts :boolean # serialize_topic_excerpts :boolean
# csp_extensions :string is an Array # csp_extensions :string is an Array
# svg_icons :string is an Array # svg_icons :string is an Array
# topic_thumbnail_sizes :string is an Array
# #
# Indexes # Indexes
# #

View File

@ -28,6 +28,83 @@ class Topic < ActiveRecord::Base
400 400
end end
def self.share_thumbnail_size
[1024, 1024]
end
def self.thumbnail_sizes
[ self.share_thumbnail_size ]
end
def thumbnail_job_redis_key(extra_sizes)
"generate_topic_thumbnail_enqueue_#{id}_#{extra_sizes.inspect}"
end
def filtered_topic_thumbnails(extra_sizes: [])
return nil unless original = image_upload
return nil unless original.width && original.height
thumbnail_sizes = Topic.thumbnail_sizes + extra_sizes
topic_thumbnails.filter { |record| thumbnail_sizes.include?([record.max_width, record.max_height]) }
end
def thumbnail_info(enqueue_if_missing: false, extra_sizes: [])
return nil unless original = image_upload
return nil unless original.width && original.height
infos = []
infos << { # Always add original
max_width: nil,
max_height: nil,
width: original.width,
height: original.height,
url: original.url
}
records = filtered_topic_thumbnails(extra_sizes: extra_sizes)
records.each do |record|
next unless record.optimized_image # Only serialize successful thumbnails
infos << {
max_width: record.max_width,
max_height: record.max_height,
width: record.optimized_image&.width,
height: record.optimized_image&.height,
url: record.optimized_image&.url
}
end
thumbnail_sizes = Topic.thumbnail_sizes + extra_sizes
if SiteSetting.create_thumbnails &&
enqueue_if_missing &&
records.length < thumbnail_sizes.length &&
Discourse.redis.set(thumbnail_job_redis_key(extra_sizes), 1, nx: true, ex: 1.minute)
Jobs.enqueue(:generate_topic_thumbnails, { topic_id: id, extra_sizes: extra_sizes })
end
infos.sort_by! { |i| -i[:width] * i[:height] }
end
def generate_thumbnails!(extra_sizes: [])
return nil unless SiteSetting.create_thumbnails
return nil unless original = image_upload
return nil unless original.width && original.height
(Topic.thumbnail_sizes + extra_sizes).each do |dim|
TopicThumbnail.find_or_create_for!(original, max_width: dim[0], max_height: dim[1])
end
end
def image_url
thumbnail = topic_thumbnails.detect do |record|
record.max_width == Topic.share_thumbnail_size[0] &&
record.max_height == Topic.share_thumbnail_size[1]
end
thumbnail&.optimized_image&.url || image_upload&.url
end
def featured_users def featured_users
@featured_users ||= TopicFeaturedUsers.new(self) @featured_users ||= TopicFeaturedUsers.new(self)
end end
@ -139,6 +216,9 @@ class Topic < ActiveRecord::Base
has_one :topic_search_data has_one :topic_search_data
has_one :topic_embed, dependent: :destroy has_one :topic_embed, dependent: :destroy
belongs_to :image_upload, class_name: 'Upload'
has_many :topic_thumbnails, through: :image_upload
# When we want to temporarily attach some data to a forum topic (usually before serialization) # When we want to temporarily attach some data to a forum topic (usually before serialization)
attr_accessor :user_data attr_accessor :user_data
attr_accessor :category_user_data attr_accessor :category_user_data
@ -1601,6 +1681,7 @@ end
# highest_staff_post_number :integer default(0), not null # highest_staff_post_number :integer default(0), not null
# featured_link :string # featured_link :string
# reviewable_score :float default(0.0), not null # reviewable_score :float default(0.0), not null
# image_upload_id :bigint
# #
# Indexes # Indexes
# #
@ -1611,6 +1692,7 @@ end
# index_topics_on_created_at_and_visible (created_at,visible) WHERE ((deleted_at IS NULL) AND ((archetype)::text <> 'private_message'::text)) # index_topics_on_created_at_and_visible (created_at,visible) WHERE ((deleted_at IS NULL) AND ((archetype)::text <> 'private_message'::text))
# index_topics_on_id_and_deleted_at (id,deleted_at) # index_topics_on_id_and_deleted_at (id,deleted_at)
# index_topics_on_id_filtered_banner (id) UNIQUE WHERE (((archetype)::text = 'banner'::text) AND (deleted_at IS NULL)) # index_topics_on_id_filtered_banner (id) UNIQUE WHERE (((archetype)::text = 'banner'::text) AND (deleted_at IS NULL))
# index_topics_on_image_upload_id (image_upload_id)
# index_topics_on_lower_title (lower((title)::text)) # index_topics_on_lower_title (lower((title)::text))
# index_topics_on_pinned_at (pinned_at) WHERE (pinned_at IS NOT NULL) # index_topics_on_pinned_at (pinned_at) WHERE (pinned_at IS NOT NULL)
# index_topics_on_pinned_globally (pinned_globally) WHERE pinned_globally # index_topics_on_pinned_globally (pinned_globally) WHERE pinned_globally

View File

@ -0,0 +1,60 @@
# frozen_string_literal: true
# This model indicates an 'attempt' to create a topic thumbnail
# for an upload. This means we don't keep trying to create optimized
# images for small/invalid original images.
#
# Foreign keys with ON DELETE CASCADE are used to ensure unneeded data
# is deleted automatically
class TopicThumbnail < ActiveRecord::Base
belongs_to :upload
belongs_to :optimized_image
def self.find_or_create_for!(original, max_width: , max_height:)
existing = TopicThumbnail.find_by(upload: original, max_width: max_width, max_height: max_height)
return existing if existing
return nil if !SiteSetting.create_thumbnails?
target_width, target_height = ImageSizer.resize(original.width, original.height, { max_width: max_width, max_height: max_height })
if target_width < original.width && target_height < original.height
optimized = OptimizedImage.create_for(original, target_width, target_height)
end
create!(upload: original, max_width: max_width, max_height: max_height, optimized_image: optimized)
end
def self.ensure_consistency!
# Clean up records for broken upload links or broken optimized image links
TopicThumbnail
.joins("LEFT JOIN uploads on upload_id = uploads.id")
.joins("LEFT JOIN optimized_images on optimized_image_id = optimized_images.id")
.where(<<~SQL)
(optimized_image_id IS NOT NULL AND optimized_images IS NULL)
OR uploads IS NULL
SQL
.delete_all
# Delete records for sizes which are no longer needed
sizes = Topic.thumbnail_sizes + ThemeModifierHelper.new(theme_ids: Theme.pluck(:id)).topic_thumbnail_sizes
sizes_sql = sizes.map { |s| "(max_width = #{s[0].to_i} AND max_height = #{s[1].to_i})" }.join(" OR ")
TopicThumbnail.where.not(sizes_sql).delete_all
end
end
# == Schema Information
#
# Table name: topic_thumbnails
#
# id :bigint not null, primary key
# upload_id :bigint not null
# optimized_image_id :bigint
# max_width :integer not null
# max_height :integer not null
#
# Indexes
#
# index_topic_thumbnails_on_optimized_image_id (optimized_image_id)
# index_topic_thumbnails_on_upload_id (upload_id)
# unique_topic_thumbnails (upload_id,max_width,max_height) UNIQUE
#

View File

@ -25,6 +25,7 @@ class Upload < ActiveRecord::Base
has_many :optimized_images, dependent: :destroy has_many :optimized_images, dependent: :destroy
has_many :user_uploads, dependent: :destroy has_many :user_uploads, dependent: :destroy
has_many :topic_thumbnails
attr_accessor :for_group_message attr_accessor :for_group_message
attr_accessor :for_theme attr_accessor :for_theme

View File

@ -25,10 +25,20 @@ class ListableTopicSerializer < BasicTopicSerializer
:bookmarked, :bookmarked,
:liked, :liked,
:unicode_title, :unicode_title,
:unread_by_group_member :unread_by_group_member,
:thumbnails
has_one :last_poster, serializer: BasicUserSerializer, embed: :objects has_one :last_poster, serializer: BasicUserSerializer, embed: :objects
def image_url
object.image_url
end
def thumbnails
extra_sizes = ThemeModifierHelper.new(request: scope.request).topic_thumbnail_sizes
object.thumbnail_info(enqueue_if_missing: true, extra_sizes: extra_sizes)
end
def include_unicode_title? def include_unicode_title?
object.title.match?(/:[\w\-+]+:/) object.title.match?(/:[\w\-+]+:/)
end end

View File

@ -73,6 +73,7 @@ class TopicViewSerializer < ApplicationSerializer
:queued_posts_count, :queued_posts_count,
:show_read_indicator, :show_read_indicator,
:requested_group_name, :requested_group_name,
:thumbnails
) )
has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects
@ -284,4 +285,9 @@ class TopicViewSerializer < ApplicationSerializer
def include_published_page? def include_published_page?
SiteSetting.enable_page_publishing? && scope.is_staff? && object.published_page.present? SiteSetting.enable_page_publishing? && scope.is_staff? && object.published_page.present?
end end
def thumbnails
extra_sizes = ThemeModifierHelper.new(request: scope.request).topic_thumbnail_sizes
object.topic.thumbnail_info(enqueue_if_missing: true, extra_sizes: extra_sizes)
end
end end

View File

@ -0,0 +1,19 @@
# frozen_string_literal: true
class AddTopicThumbnailInformation < ActiveRecord::Migration[6.0]
def change
add_reference :posts, :image_upload
add_reference :topics, :image_upload
add_column :theme_modifier_sets, :topic_thumbnail_sizes, :string, array: true
create_table :topic_thumbnails do |t|
t.references :upload, null: false
t.references :optimized_image, null: true
t.integer :max_width, null: false
t.integer :max_height, null: false
end
add_index :topic_thumbnails, [:upload_id, :max_width, :max_height], name: :unique_topic_thumbnails, unique: true
end
end

View File

@ -0,0 +1,95 @@
# frozen_string_literal: true
class MigrateImageUrlToImageUploadId < ActiveRecord::Migration[6.0]
disable_ddl_transaction! # Avoid holding update locks on posts for the whole migration
BATCH_SIZE = 1000
def up
# Defining regex here to avoid needing to double-escape the \ characters
regex = '\/(original|optimized)\/\dX[\/\.\w]*\/([a-zA-Z0-9]+)[\.\w]*'
execute <<~SQL
CREATE TEMPORARY TABLE tmp_post_image_uploads(
post_id int primary key,
upload_id int
)
SQL
# Look for an SHA1 in the existing image_url, and match to the uploads table
execute <<~SQL
INSERT INTO tmp_post_image_uploads(post_id, upload_id)
SELECT
posts.id as post_id,
uploads.id as upload_id
FROM posts
LEFT JOIN LATERAL regexp_matches(posts.image_url, '#{regex}') matched_sha1 ON TRUE
LEFT JOIN uploads on uploads.sha1 = matched_sha1[2]
WHERE posts.image_url IS NOT NULL
AND uploads.id IS NOT NULL
ORDER BY posts.id ASC
SQL
# Update the posts table to match the temp table data
last_update_id = -1
begin
result = DB.query <<~SQL
WITH to_update AS (
SELECT post_id, upload_id
FROM tmp_post_image_uploads
JOIN posts ON posts.id = post_id
WHERE posts.id > #{last_update_id}
ORDER BY post_id ASC
LIMIT #{BATCH_SIZE}
)
UPDATE posts SET image_upload_id = to_update.upload_id
FROM to_update
WHERE to_update.post_id = posts.id
RETURNING posts.id
SQL
last_update_id = result.last&.id
end while last_update_id
# Update the topic image based on the first post image
last_update_id = -1
begin
result = DB.query <<~SQL
WITH to_update AS (
SELECT topic_id, posts.image_upload_id as upload_id
FROM topics
JOIN posts ON post_number = 1 AND posts.topic_id = topics.id
WHERE posts.image_upload_id IS NOT NULL
AND topics.id > #{last_update_id}
ORDER BY topics.id ASC
LIMIT #{BATCH_SIZE}
)
UPDATE topics SET image_upload_id = to_update.upload_id
FROM to_update
WHERE topics.id = to_update.topic_id
RETURNING topics.id
SQL
last_update_id = result.last&.id
end while last_update_id
# For posts we couldn't figure out, mark them for background rebake
last_update_id = -1
begin
updated_count = DB.query <<~SQL
WITH to_update AS (
SELECT id as post_id
FROM posts
WHERE posts.image_url IS NOT NULL
AND posts.image_upload_id IS NULL
AND posts.id > #{last_update_id}
ORDER BY posts.id ASC
LIMIT #{BATCH_SIZE}
)
UPDATE posts SET baked_version = NULL
FROM to_update
WHERE posts.id = to_update.post_id
RETURNING posts.id
SQL
last_update_id = result.last&.id
end while last_update_id
end
end

View File

@ -490,16 +490,26 @@ class CookedPostProcessor
end end
def update_post_image def update_post_image
img = extract_images_for_post.first upload = nil
if img.blank? eligible_image_fragments = extract_images_for_post
@post.update_column(:image_url, nil) if @post.image_url
@post.topic.update_column(:image_url, nil) if @post.topic.image_url && @post.is_first_post? # Loop through those fragments until we find one with an upload record
return @post.each_upload_url(fragments: eligible_image_fragments) do |src, path, sha1|
upload = Upload.find_by(sha1: sha1)
break if upload
end end
if img["src"].present? if upload.present?
@post.update_column(:image_url, img["src"][0...255]) # post @post.update_column(:image_upload_id, upload.id) # post
@post.topic.update_column(:image_url, img["src"][0...255]) if @post.is_first_post? # topic if @post.is_first_post? # topic
@post.topic.update_column(:image_upload_id, upload.id)
extra_sizes = ThemeModifierHelper.new(theme_ids: Theme.user_selectable.pluck(:id)).topic_thumbnail_sizes
@post.topic.generate_thumbnails!(extra_sizes: extra_sizes)
end
else
@post.update_column(:image_upload_id, nil) if @post.image_upload_id
@post.topic.update_column(:image_upload_id, nil) if @post.topic.image_upload_id && @post.is_first_post?
nil
end end
end end

View File

@ -434,7 +434,7 @@ class Search
end end
advanced_filter(/^with:images$/) do |posts| advanced_filter(/^with:images$/) do |posts|
posts.where("posts.image_url IS NOT NULL") posts.where("posts.image_upload_id IS NOT NULL")
end end
advanced_filter(/^category:(.+)$/) do |posts, match| advanced_filter(/^category:(.+)$/) do |posts, match|

View File

@ -828,6 +828,8 @@ class TopicQuery
result = result.where('topics.posts_count <= ?', options[:max_posts]) if options[:max_posts].present? result = result.where('topics.posts_count <= ?', options[:max_posts]) if options[:max_posts].present?
result = result.where('topics.posts_count >= ?', options[:min_posts]) if options[:min_posts].present? result = result.where('topics.posts_count >= ?', options[:min_posts]) if options[:min_posts].present?
result = preload_thumbnails(result)
result = TopicQuery.apply_custom_filters(result, self) result = TopicQuery.apply_custom_filters(result, self)
@guardian.filter_allowed_categories(result) @guardian.filter_allowed_categories(result)
@ -1050,6 +1052,10 @@ class TopicQuery
result.order('topics.bumped_at DESC') result.order('topics.bumped_at DESC')
end end
def preload_thumbnails(result)
result.preload(:image_upload, topic_thumbnails: :optimized_image)
end
private private
def sanitize_sql_array(input) def sanitize_sql_array(input)

View File

@ -751,16 +751,16 @@ describe CookedPostProcessor do
end end
context "topic image" do context "topic image" do
let(:post) { Fabricate(:post_with_uploaded_image) } fab!(:post) { Fabricate(:post_with_uploaded_image) }
let(:cpp) { CookedPostProcessor.new(post) } let(:cpp) { CookedPostProcessor.new(post) }
it "adds a topic image if there's one in the first post" do it "adds a topic image if there's one in the first post" do
FastImage.stubs(:size) FastImage.stubs(:size)
expect(post.topic.image_url).to eq(nil) expect(post.topic.image_upload_id).to eq(nil)
cpp.post_process cpp.post_process
post.topic.reload post.topic.reload
expect(post.topic.image_url).to be_present expect(post.topic.image_upload_id).to be_present
end end
it "removes image if post is edited and no longer has an image" do it "removes image if post is edited and no longer has an image" do
@ -768,14 +768,14 @@ describe CookedPostProcessor do
cpp.post_process cpp.post_process
post.topic.reload post.topic.reload
expect(post.topic.image_url).to be_present expect(post.topic.image_upload_id).to be_present
expect(post.image_url).to be_present expect(post.image_upload_id).to be_present
post.update!(raw: "This post no longer has an image.") post.update!(raw: "This post no longer has an image.")
CookedPostProcessor.new(post).post_process CookedPostProcessor.new(post).post_process
post.topic.reload post.topic.reload
expect(post.topic.image_url).not_to be_present expect(post.topic.image_upload_id).not_to be_present
expect(post.image_url).not_to be_present expect(post.image_upload_id).not_to be_present
end end
it "won't remove the original image if another post doesn't have an image" do it "won't remove the original image if another post doesn't have an image" do
@ -784,15 +784,32 @@ describe CookedPostProcessor do
cpp.post_process cpp.post_process
topic.reload topic.reload
expect(topic.image_url).to be_present expect(topic.image_upload_id).to be_present
expect(post.image_url).to be_present expect(post.image_upload_id).to be_present
post = Fabricate(:post, topic: topic, raw: "this post doesn't have an image") post = Fabricate(:post, topic: topic, raw: "this post doesn't have an image")
CookedPostProcessor.new(post).post_process CookedPostProcessor.new(post).post_process
topic.reload topic.reload
expect(post.topic.image_url).to be_present expect(post.topic.image_upload_id).to be_present
expect(post.image_url).to be_blank expect(post.image_upload_id).to be_blank
end
it "generates thumbnails correctly" do
FastImage.expects(:size).returns([1750, 2000])
topic = post.topic
cpp.post_process
topic.reload
expect(topic.image_upload_id).to be_present
expect(post.image_upload_id).to be_present
post = Fabricate(:post, topic: topic, raw: "this post doesn't have an image")
CookedPostProcessor.new(post).post_process
topic.reload
expect(post.topic.image_upload_id).to be_present
expect(post.image_upload_id).to be_blank
end end
end end
@ -802,10 +819,10 @@ describe CookedPostProcessor do
it "adds a post image if there's one in the post" do it "adds a post image if there's one in the post" do
FastImage.stubs(:size) FastImage.stubs(:size)
expect(reply.image_url).to eq(nil) expect(reply.image_upload_id).to eq(nil)
cpp.post_process cpp.post_process
reply.reload reply.reload
expect(reply.image_url).to be_present expect(reply.image_upload_id).to be_present
end end
end end
end end

View File

@ -1134,13 +1134,11 @@ describe Search do
it 'can find posts with images' do it 'can find posts with images' do
post_uploaded = Fabricate(:post_with_uploaded_image) post_uploaded = Fabricate(:post_with_uploaded_image)
post_with_image_urls = Fabricate(:post_with_image_urls)
Fabricate(:post) Fabricate(:post)
CookedPostProcessor.new(post_uploaded).update_post_image CookedPostProcessor.new(post_uploaded).update_post_image
CookedPostProcessor.new(post_with_image_urls).update_post_image
expect(Search.execute('with:images').posts.map(&:id)).to contain_exactly(post_uploaded.id, post_with_image_urls.id) expect(Search.execute('with:images').posts.map(&:id)).to contain_exactly(post_uploaded.id)
end end
it 'can find by latest' do it 'can find by latest' do

View File

@ -708,9 +708,12 @@ describe TopicView do
end end
describe '#image_url' do describe '#image_url' do
let!(:post1) { Fabricate(:post, topic: topic) } fab!(:op_upload) { Fabricate(:image_upload) }
let!(:post2) { Fabricate(:post, topic: topic) } fab!(:post3_upload) { Fabricate(:image_upload) }
let!(:post3) { Fabricate(:post, topic: topic).tap { |p| p.update_column(:image_url, "post3_image.png") }.reload }
fab!(:post1) { Fabricate(:post, topic: topic) }
fab!(:post2) { Fabricate(:post, topic: topic) }
fab!(:post3) { Fabricate(:post, topic: topic).tap { |p| p.update_column(:image_upload_id, post3_upload.id) }.reload }
def topic_view_for_post(post_number) def topic_view_for_post(post_number)
TopicView.new(topic.id, evil_trout, post_number: post_number) TopicView.new(topic.id, evil_trout, post_number: post_number)
@ -718,14 +721,14 @@ describe TopicView do
context "when op has an image" do context "when op has an image" do
before do before do
topic.update_column(:image_url, "op_image.png") topic.update_column(:image_upload_id, op_upload.id)
post1.update_column(:image_url, "op_image.png") post1.update_column(:image_upload_id, op_upload.id)
end end
it "uses the topic image as a fallback when posts have no image" do it "uses the topic image as a fallback when posts have no image" do
expect(topic_view_for_post(1).image_url).to eq("op_image.png") expect(topic_view_for_post(1).image_url).to eq(op_upload.url)
expect(topic_view_for_post(2).image_url).to eq("op_image.png") expect(topic_view_for_post(2).image_url).to eq(op_upload.url)
expect(topic_view_for_post(3).image_url).to eq("post3_image.png") expect(topic_view_for_post(3).image_url).to eq(post3_upload.url)
end end
end end
@ -733,7 +736,7 @@ describe TopicView do
it "returns nil when posts have no image" do it "returns nil when posts have no image" do
expect(topic_view_for_post(1).image_url).to eq(nil) expect(topic_view_for_post(1).image_url).to eq(nil)
expect(topic_view_for_post(2).image_url).to eq(nil) expect(topic_view_for_post(2).image_url).to eq(nil)
expect(topic_view_for_post(3).image_url).to eq("post3_image.png") expect(topic_view_for_post(3).image_url).to eq(post3_upload.url)
end end
end end
end end

View File

@ -56,7 +56,7 @@ HTML
end end
Fabricator(:post_with_uploaded_image, from: :post) do Fabricator(:post_with_uploaded_image, from: :post) do
raw "<img src=\"/#{Discourse.store.upload_path}/original/2X/3456789012345678.png\" width=\"1500\" height=\"2000\">" raw { "<img src=\"#{Fabricate(:image_upload)}\" width=\"1500\" height=\"2000\">" }
end end
Fabricator(:post_with_an_attachment, from: :post) do Fabricator(:post_with_an_attachment, from: :post) do

View File

@ -0,0 +1,88 @@
# frozen_string_literal: true
require 'rails_helper'
describe "Topic Thumbnails" do
before { SiteSetting.create_thumbnails = true }
fab!(:image) { Fabricate(:image_upload, width: 5000, height: 5000) }
fab!(:topic) { Fabricate(:topic, image_upload_id: image.id) }
fab!(:user) { Fabricate(:user) }
context 'latest' do
def get_topic
Discourse.redis.del(topic.thumbnail_job_redis_key([]))
get '/latest.json'
response.parsed_body["topic_list"]["topics"][0]
end
it "includes thumbnails" do
topic_json = nil
expect do
topic_json = get_topic
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 eq(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
context "with a theme" do
before do
theme = Fabricate(:theme)
theme.theme_modifier_set.topic_thumbnail_sizes = [
[100, 100],
[200, 200],
[300, 300]
]
theme.theme_modifier_set.save!
theme.set_default!
end
it "includes the theme specified resolutions" do
topic_json = nil
expect do
topic_json = get_topic
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(1)
# Run the job
args = Jobs::GenerateTopicThumbnails.jobs.last["args"].first
Jobs::GenerateTopicThumbnails.new.execute(args.with_indifferent_access)
# Request again
expect do
topic_json = get_topic
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(0)
thumbnails = topic_json["thumbnails"]
# Original + Optimized + 3 theme requests
expect(thumbnails.length).to eq(5)
end
end
end
end

View File

@ -7,8 +7,6 @@ RSpec.describe DbHelper do
it 'should remap columns properly' do it 'should remap columns properly' do
post = Fabricate(:post, cooked: "this is a specialcode that I included") post = Fabricate(:post, cooked: "this is a specialcode that I included")
post_attributes = post.reload.attributes post_attributes = post.reload.attributes
post2 = Fabricate(:post, image_url: "/testing/specialcode")
post2_attributes = post2.reload.attributes
badge = Fabricate(:badge, query: "specialcode") badge = Fabricate(:badge, query: "specialcode")
badge_attributes = badge.reload.attributes badge_attributes = badge.reload.attributes
@ -19,13 +17,6 @@ RSpec.describe DbHelper do
expect(post.cooked).to include("codespecial") expect(post.cooked).to include("codespecial")
post2.reload
expect(post2.image_url).to eq("/testing/codespecial")
expect(post2_attributes.except("image_url"))
.to eq(post2.attributes.except("image_url"))
badge.reload badge.reload
expect(badge.query).to eq("codespecial") expect(badge.query).to eq("codespecial")

View File

@ -0,0 +1,46 @@
# frozen_string_literal: true
require 'rails_helper'
describe "TopicThumbnail" do
let(:upload1) { Fabricate(:image_upload, width: 5000, height: 5000) }
let(:topic) { Fabricate(:topic, image_upload: upload1) }
before do
SiteSetting.create_thumbnails = true
topic.generate_thumbnails!
TopicThumbnail.ensure_consistency!
topic.reload
expect(topic.topic_thumbnails.length).to eq(1)
end
it "cleans up deleted uploads" do
upload1.delete
TopicThumbnail.ensure_consistency!
topic.reload
expect(topic.topic_thumbnails.length).to eq(0)
end
it "cleans up deleted optimized images" do
upload1.optimized_images.reload.delete_all
TopicThumbnail.ensure_consistency!
topic.reload
expect(topic.topic_thumbnails.length).to eq(0)
end
it "cleans up unneeded sizes" do
expect(topic.topic_thumbnails.length).to eq(1)
topic.topic_thumbnails[0].update_column(:max_width, 999999)
TopicThumbnail.ensure_consistency!
topic.reload
expect(topic.topic_thumbnails.length).to eq(0)
end
end

View File

@ -46,15 +46,35 @@ describe TopicViewSerializer do
end end
describe '#image_url' do describe '#image_url' do
let(:image_url) { 'http://meta.discourse.org/images/welcome/discourse-edit-post-animated.gif' } let(:image_upload) { Fabricate(:image_upload, width: 5000, height: 5000) }
describe 'when a topic has an image' do describe 'when a topic has an image' do
it 'should return the image url' do before { topic.update!(image_upload_id: image_upload.id) }
topic.update!(image_url: image_url)
it 'should return the image url' do
json = serialize_topic(topic, user) json = serialize_topic(topic, user)
expect(json[:image_url]).to eq(image_url) expect(json[:image_url]).to eq(image_upload.url)
end
it 'should have thumbnails' do
SiteSetting.create_thumbnails = true
Discourse.redis.del(topic.thumbnail_job_redis_key([]))
json = nil
expect do
json = serialize_topic(topic, user)
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(1)
topic.generate_thumbnails!
expect do
json = serialize_topic(topic, user)
end.to change { Jobs::GenerateTopicThumbnails.jobs.size }.by(0)
# Original + Optimized
expect(json[:thumbnails].length).to eq(2)
end end
end end

View File

@ -50,6 +50,7 @@ RSpec.describe WebHookTopicViewSerializer do
created_by created_by
last_poster last_poster
tags tags
thumbnails
} }
keys = serializer.as_json.keys keys = serializer.as_json.keys