@@ -14,6 +15,7 @@
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.
--}}
+ {{~raw-plugin-outlet name="topic-list-before-link"}}
{{~raw-plugin-outlet name="topic-list-before-status"}}
{{~raw "topic-status" topic=topic~}}
diff --git a/app/jobs/regular/generate_topic_thumbnails.rb b/app/jobs/regular/generate_topic_thumbnails.rb
new file mode 100644
index 00000000000..a01452670c7
--- /dev/null
+++ b/app/jobs/regular/generate_topic_thumbnails.rb
@@ -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
diff --git a/app/jobs/scheduled/ensure_db_consistency.rb b/app/jobs/scheduled/ensure_db_consistency.rb
index 6d7f3fbbb8a..6c21814fa01 100644
--- a/app/jobs/scheduled/ensure_db_consistency.rb
+++ b/app/jobs/scheduled/ensure_db_consistency.rb
@@ -22,7 +22,8 @@ module Jobs
CategoryTagStat,
User,
UserAvatar,
- Category
+ Category,
+ TopicThumbnail
].each do |klass|
klass.ensure_consistency!
measure(klass)
diff --git a/app/models/post.rb b/app/models/post.rb
index f2ee10543f3..c2f80c8c6fc 100644
--- a/app/models/post.rb
+++ b/app/models/post.rb
@@ -51,6 +51,8 @@ class Post < ActiveRecord::Base
has_many :user_actions, foreign_key: :target_post_id
+ belongs_to :image_upload, class_name: "Upload"
+
validates_with PostValidator, unless: :skip_validation
after_save :index_search
@@ -1062,6 +1064,10 @@ class Post < ActiveRecord::Base
Upload.where(access_control_post_id: self.id)
end
+ def image_url
+ image_upload&.url
+ end
+
private
def parse_quote_into_arguments(quote)
@@ -1144,6 +1150,7 @@ end
# action_code :string
# image_url :string
# locked_by_id :integer
+# image_upload_id :bigint
#
# Indexes
#
@@ -1152,6 +1159,7 @@ end
# 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_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_topic_id_and_percent_rank (topic_id,percent_rank)
# index_posts_on_topic_id_and_post_number (topic_id,post_number) UNIQUE
diff --git a/app/models/theme_modifier_set.rb b/app/models/theme_modifier_set.rb
index 99a8818f58c..9fae7faf54a 100644
--- a/app/models/theme_modifier_set.rb
+++ b/app/models/theme_modifier_set.rb
@@ -12,7 +12,7 @@ class ThemeModifierSet < ActiveRecord::Base
def type_validator
ThemeModifierSet.modifiers.each do |k, config|
- value = public_send(k)
+ value = read_attribute(k)
next if value.nil?
case config[:type]
@@ -39,7 +39,7 @@ class ThemeModifierSet < ActiveRecord::Base
def self.resolve_modifier_for_themes(theme_ids, 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]
when :boolean
all_values.any?
@@ -50,6 +50,26 @@ class ThemeModifierSet < ActiveRecord::Base
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
# Build the list of modifiers from the DB schema.
@@ -78,11 +98,12 @@ end
#
# Table name: theme_modifier_sets
#
-# id :bigint not null, primary key
-# theme_id :bigint not null
-# serialize_topic_excerpts :boolean
-# csp_extensions :string is an Array
-# svg_icons :string is an Array
+# id :bigint not null, primary key
+# theme_id :bigint not null
+# serialize_topic_excerpts :boolean
+# csp_extensions :string is an Array
+# svg_icons :string is an Array
+# topic_thumbnail_sizes :string is an Array
#
# Indexes
#
diff --git a/app/models/topic.rb b/app/models/topic.rb
index aaeca3f1453..371f32d5702 100644
--- a/app/models/topic.rb
+++ b/app/models/topic.rb
@@ -28,6 +28,83 @@ class Topic < ActiveRecord::Base
400
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
@featured_users ||= TopicFeaturedUsers.new(self)
end
@@ -139,6 +216,9 @@ class Topic < ActiveRecord::Base
has_one :topic_search_data
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)
attr_accessor :user_data
attr_accessor :category_user_data
@@ -1601,6 +1681,7 @@ end
# highest_staff_post_number :integer default(0), not null
# featured_link :string
# reviewable_score :float default(0.0), not null
+# image_upload_id :bigint
#
# 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_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_image_upload_id (image_upload_id)
# 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_globally (pinned_globally) WHERE pinned_globally
diff --git a/app/models/topic_thumbnail.rb b/app/models/topic_thumbnail.rb
new file mode 100644
index 00000000000..139ddde2321
--- /dev/null
+++ b/app/models/topic_thumbnail.rb
@@ -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
+#
diff --git a/app/models/upload.rb b/app/models/upload.rb
index cb4cf458afc..bb4e7976f5e 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -25,6 +25,7 @@ class Upload < ActiveRecord::Base
has_many :optimized_images, dependent: :destroy
has_many :user_uploads, dependent: :destroy
+ has_many :topic_thumbnails
attr_accessor :for_group_message
attr_accessor :for_theme
diff --git a/app/serializers/listable_topic_serializer.rb b/app/serializers/listable_topic_serializer.rb
index a00cd2f619d..52d7fb27157 100644
--- a/app/serializers/listable_topic_serializer.rb
+++ b/app/serializers/listable_topic_serializer.rb
@@ -25,10 +25,20 @@ class ListableTopicSerializer < BasicTopicSerializer
:bookmarked,
:liked,
:unicode_title,
- :unread_by_group_member
+ :unread_by_group_member,
+ :thumbnails
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?
object.title.match?(/:[\w\-+]+:/)
end
diff --git a/app/serializers/topic_view_serializer.rb b/app/serializers/topic_view_serializer.rb
index e40dca1d9fb..27cc97523ed 100644
--- a/app/serializers/topic_view_serializer.rb
+++ b/app/serializers/topic_view_serializer.rb
@@ -73,6 +73,7 @@ class TopicViewSerializer < ApplicationSerializer
:queued_posts_count,
:show_read_indicator,
:requested_group_name,
+ :thumbnails
)
has_one :details, serializer: TopicViewDetailsSerializer, root: false, embed: :objects
@@ -284,4 +285,9 @@ class TopicViewSerializer < ApplicationSerializer
def include_published_page?
SiteSetting.enable_page_publishing? && scope.is_staff? && object.published_page.present?
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
diff --git a/db/migrate/20200429095034_add_topic_thumbnail_information.rb b/db/migrate/20200429095034_add_topic_thumbnail_information.rb
new file mode 100644
index 00000000000..be34f93c32e
--- /dev/null
+++ b/db/migrate/20200429095034_add_topic_thumbnail_information.rb
@@ -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
diff --git a/db/migrate/20200429095035_migrate_image_url_to_image_upload_id.rb b/db/migrate/20200429095035_migrate_image_url_to_image_upload_id.rb
new file mode 100644
index 00000000000..5f4747b2c3c
--- /dev/null
+++ b/db/migrate/20200429095035_migrate_image_url_to_image_upload_id.rb
@@ -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
diff --git a/lib/cooked_post_processor.rb b/lib/cooked_post_processor.rb
index 725258cc10d..d2e8f4d2d50 100644
--- a/lib/cooked_post_processor.rb
+++ b/lib/cooked_post_processor.rb
@@ -490,16 +490,26 @@ class CookedPostProcessor
end
def update_post_image
- img = extract_images_for_post.first
- if img.blank?
- @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?
- return
+ upload = nil
+ eligible_image_fragments = extract_images_for_post
+
+ # Loop through those fragments until we find one with an upload record
+ @post.each_upload_url(fragments: eligible_image_fragments) do |src, path, sha1|
+ upload = Upload.find_by(sha1: sha1)
+ break if upload
end
- if img["src"].present?
- @post.update_column(:image_url, img["src"][0...255]) # post
- @post.topic.update_column(:image_url, img["src"][0...255]) if @post.is_first_post? # topic
+ if upload.present?
+ @post.update_column(:image_upload_id, upload.id) # post
+ 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
diff --git a/lib/search.rb b/lib/search.rb
index e1c6509c423..ffe08884eaa 100644
--- a/lib/search.rb
+++ b/lib/search.rb
@@ -434,7 +434,7 @@ class Search
end
advanced_filter(/^with:images$/) do |posts|
- posts.where("posts.image_url IS NOT NULL")
+ posts.where("posts.image_upload_id IS NOT NULL")
end
advanced_filter(/^category:(.+)$/) do |posts, match|
diff --git a/lib/topic_query.rb b/lib/topic_query.rb
index 35bc7347feb..772ea5af28b 100644
--- a/lib/topic_query.rb
+++ b/lib/topic_query.rb
@@ -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[:min_posts]) if options[:min_posts].present?
+ result = preload_thumbnails(result)
+
result = TopicQuery.apply_custom_filters(result, self)
@guardian.filter_allowed_categories(result)
@@ -1050,6 +1052,10 @@ class TopicQuery
result.order('topics.bumped_at DESC')
end
+ def preload_thumbnails(result)
+ result.preload(:image_upload, topic_thumbnails: :optimized_image)
+ end
+
private
def sanitize_sql_array(input)
diff --git a/spec/components/cooked_post_processor_spec.rb b/spec/components/cooked_post_processor_spec.rb
index 51e9eadf69f..a429d64bee7 100644
--- a/spec/components/cooked_post_processor_spec.rb
+++ b/spec/components/cooked_post_processor_spec.rb
@@ -751,16 +751,16 @@ describe CookedPostProcessor do
end
context "topic image" do
- let(:post) { Fabricate(:post_with_uploaded_image) }
+ fab!(:post) { Fabricate(:post_with_uploaded_image) }
let(:cpp) { CookedPostProcessor.new(post) }
it "adds a topic image if there's one in the first post" do
FastImage.stubs(:size)
- expect(post.topic.image_url).to eq(nil)
+ expect(post.topic.image_upload_id).to eq(nil)
cpp.post_process
post.topic.reload
- expect(post.topic.image_url).to be_present
+ expect(post.topic.image_upload_id).to be_present
end
it "removes image if post is edited and no longer has an image" do
@@ -768,14 +768,14 @@ describe CookedPostProcessor do
cpp.post_process
post.topic.reload
- expect(post.topic.image_url).to be_present
- expect(post.image_url).to be_present
+ expect(post.topic.image_upload_id).to be_present
+ expect(post.image_upload_id).to be_present
post.update!(raw: "This post no longer has an image.")
CookedPostProcessor.new(post).post_process
post.topic.reload
- expect(post.topic.image_url).not_to be_present
- expect(post.image_url).not_to be_present
+ expect(post.topic.image_upload_id).not_to be_present
+ expect(post.image_upload_id).not_to be_present
end
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
topic.reload
- expect(topic.image_url).to be_present
- expect(post.image_url).to be_present
+ 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_url).to be_present
- expect(post.image_url).to be_blank
+ expect(post.topic.image_upload_id).to be_present
+ 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
@@ -802,10 +819,10 @@ describe CookedPostProcessor do
it "adds a post image if there's one in the post" do
FastImage.stubs(:size)
- expect(reply.image_url).to eq(nil)
+ expect(reply.image_upload_id).to eq(nil)
cpp.post_process
reply.reload
- expect(reply.image_url).to be_present
+ expect(reply.image_upload_id).to be_present
end
end
end
diff --git a/spec/components/search_spec.rb b/spec/components/search_spec.rb
index 64e0efb726c..a455d81ec13 100644
--- a/spec/components/search_spec.rb
+++ b/spec/components/search_spec.rb
@@ -1134,13 +1134,11 @@ describe Search do
it 'can find posts with images' do
post_uploaded = Fabricate(:post_with_uploaded_image)
- post_with_image_urls = Fabricate(:post_with_image_urls)
Fabricate(:post)
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
it 'can find by latest' do
diff --git a/spec/components/topic_view_spec.rb b/spec/components/topic_view_spec.rb
index 16ee55dab0e..73bb74ac256 100644
--- a/spec/components/topic_view_spec.rb
+++ b/spec/components/topic_view_spec.rb
@@ -708,9 +708,12 @@ describe TopicView do
end
describe '#image_url' do
- let!(:post1) { Fabricate(:post, topic: topic) }
- let!(:post2) { Fabricate(:post, topic: topic) }
- let!(:post3) { Fabricate(:post, topic: topic).tap { |p| p.update_column(:image_url, "post3_image.png") }.reload }
+ fab!(:op_upload) { Fabricate(:image_upload) }
+ fab!(:post3_upload) { Fabricate(:image_upload) }
+
+ 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)
TopicView.new(topic.id, evil_trout, post_number: post_number)
@@ -718,14 +721,14 @@ describe TopicView do
context "when op has an image" do
before do
- topic.update_column(:image_url, "op_image.png")
- post1.update_column(:image_url, "op_image.png")
+ topic.update_column(:image_upload_id, op_upload.id)
+ post1.update_column(:image_upload_id, op_upload.id)
end
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(2).image_url).to eq("op_image.png")
- expect(topic_view_for_post(3).image_url).to eq("post3_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_upload.url)
+ expect(topic_view_for_post(3).image_url).to eq(post3_upload.url)
end
end
@@ -733,7 +736,7 @@ describe TopicView 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(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
diff --git a/spec/fabricators/post_fabricator.rb b/spec/fabricators/post_fabricator.rb
index 412504c7c4e..42235cb94fc 100644
--- a/spec/fabricators/post_fabricator.rb
+++ b/spec/fabricators/post_fabricator.rb
@@ -56,7 +56,7 @@ HTML
end
Fabricator(:post_with_uploaded_image, from: :post) do
- raw "
"
+ raw { "
" }
end
Fabricator(:post_with_an_attachment, from: :post) do
diff --git a/spec/integration/topic_thumbnail_spec.rb b/spec/integration/topic_thumbnail_spec.rb
new file mode 100644
index 00000000000..b288fb6c488
--- /dev/null
+++ b/spec/integration/topic_thumbnail_spec.rb
@@ -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
diff --git a/spec/lib/db_helper_spec.rb b/spec/lib/db_helper_spec.rb
index b178cd1e193..c2074a0f80b 100644
--- a/spec/lib/db_helper_spec.rb
+++ b/spec/lib/db_helper_spec.rb
@@ -7,8 +7,6 @@ RSpec.describe DbHelper do
it 'should remap columns properly' do
post = Fabricate(:post, cooked: "this is a specialcode that I included")
post_attributes = post.reload.attributes
- post2 = Fabricate(:post, image_url: "/testing/specialcode")
- post2_attributes = post2.reload.attributes
badge = Fabricate(:badge, query: "specialcode")
badge_attributes = badge.reload.attributes
@@ -19,13 +17,6 @@ RSpec.describe DbHelper do
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
expect(badge.query).to eq("codespecial")
diff --git a/spec/models/topic_thumbnail_spec.rb b/spec/models/topic_thumbnail_spec.rb
new file mode 100644
index 00000000000..b9e1df75fd1
--- /dev/null
+++ b/spec/models/topic_thumbnail_spec.rb
@@ -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
diff --git a/spec/serializers/topic_view_serializer_spec.rb b/spec/serializers/topic_view_serializer_spec.rb
index a80f884812f..0adfbfe08ae 100644
--- a/spec/serializers/topic_view_serializer_spec.rb
+++ b/spec/serializers/topic_view_serializer_spec.rb
@@ -46,15 +46,35 @@ describe TopicViewSerializer do
end
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
- it 'should return the image url' do
- topic.update!(image_url: image_url)
+ before { topic.update!(image_upload_id: image_upload.id) }
+ it 'should return the image url' do
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
diff --git a/spec/serializers/web_hook_topic_view_serializer_spec.rb b/spec/serializers/web_hook_topic_view_serializer_spec.rb
index f0499f985e0..df878fb7cba 100644
--- a/spec/serializers/web_hook_topic_view_serializer_spec.rb
+++ b/spec/serializers/web_hook_topic_view_serializer_spec.rb
@@ -50,6 +50,7 @@ RSpec.describe WebHookTopicViewSerializer do
created_by
last_poster
tags
+ thumbnails
}
keys = serializer.as_json.keys