From 9db8f00b3dd6f2881adf1b786e29426889225e7a Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Thu, 9 Jun 2022 02:24:30 +0300 Subject: [PATCH] FEATURE: Create upload_references table (#16146) This table holds associations between uploads and other models. This can be used to prevent removing uploads that are still in use. * DEV: Create upload_references * DEV: Use UploadReference instead of PostUpload * DEV: Use UploadReference for SiteSetting * DEV: Use UploadReference for Badge * DEV: Use UploadReference for Category * DEV: Use UploadReference for CustomEmoji * DEV: Use UploadReference for Group * DEV: Use UploadReference for ThemeField * DEV: Use UploadReference for ThemeSetting * DEV: Use UploadReference for User * DEV: Use UploadReference for UserAvatar * DEV: Use UploadReference for UserExport * DEV: Use UploadReference for UserProfile * DEV: Add method to extract uploads from raw text * DEV: Use UploadReference for Draft * DEV: Use UploadReference for ReviewableQueuedPost * DEV: Use UploadReference for UserProfile's bio_raw * DEV: Do not copy user uploads to upload references * DEV: Copy post uploads again after deploy * DEV: Use created_at and updated_at from uploads table * FIX: Check if upload site setting is empty * DEV: Copy user uploads to upload references * DEV: Make upload extraction less strict --- app/jobs/scheduled/clean_up_uploads.rb | 9 +- app/models/badge.rb | 7 + app/models/category.rb | 8 + app/models/custom_emoji.rb | 8 + app/models/draft.rb | 9 +- app/models/group.rb | 8 + app/models/post.rb | 29 ++- app/models/reviewable_queued_post.rb | 7 + app/models/site_setting.rb | 13 + app/models/theme_field.rb | 7 + app/models/theme_setting.rb | 8 + app/models/upload.rb | 62 ++--- app/models/upload_reference.rb | 63 +++++ app/models/user.rb | 7 + app/models/user_avatar.rb | 8 + app/models/user_export.rb | 8 + app/models/user_profile.rb | 8 + ...20220308201942_create_upload_references.rb | 13 + ..._copy_post_uploads_to_upload_references.rb | 17 ++ ...e_settings_uploads_to_upload_references.rb | 26 ++ ...opy_badges_uploads_to_upload_references.rb | 18 ++ ...opy_groups_uploads_to_upload_references.rb | 18 ++ ...er_exports_uploads_to_upload_references.rb | 17 ++ ...eme_fields_uploads_to_upload_references.rb | 18 ++ ...categories_uploads_to_upload_references.rb | 27 ++ ...tom_emojis_uploads_to_upload_references.rb | 18 ++ ...r_profiles_uploads_to_upload_references.rb | 27 ++ ...er_avatars_uploads_to_upload_references.rb | 27 ++ ...e_settings_uploads_to_upload_references.rb | 18 ++ ..._copy_user_uploads_to_upload_references.rb | 18 ++ ...t_uploads_to_upload_references_for_sync.rb | 18 ++ lib/backup_restore/uploads_restorer.rb | 2 +- lib/search.rb | 4 +- lib/shrink_uploaded_image.rb | 7 +- lib/tasks/posts.rake | 5 +- lib/tasks/uploads.rake | 50 ++-- lib/topic_upload_security_manager.rb | 6 +- spec/jobs/clean_up_uploads_spec.rb | 17 +- spec/jobs/pull_hotlinked_images_spec.rb | 10 +- .../update_post_uploads_secure_status_spec.rb | 8 +- spec/lib/cooked_post_processor_spec.rb | 2 +- spec/lib/post_destroyer_spec.rb | 4 +- spec/lib/post_revisor_spec.rb | 4 +- .../lib/topic_upload_security_manager_spec.rb | 10 +- spec/models/post_spec.rb | 20 +- spec/models/post_upload_spec.rb | 8 - spec/models/upload_reference_spec.rb | 239 ++++++++++++++++++ spec/models/upload_spec.rb | 19 ++ spec/tasks/uploads_spec.rb | 20 +- 49 files changed, 842 insertions(+), 142 deletions(-) create mode 100644 app/models/upload_reference.rb create mode 100644 db/migrate/20220308201942_create_upload_references.rb create mode 100644 db/migrate/20220309132719_copy_post_uploads_to_upload_references.rb create mode 100644 db/migrate/20220330160747_copy_site_settings_uploads_to_upload_references.rb create mode 100644 db/migrate/20220330160751_copy_badges_uploads_to_upload_references.rb create mode 100644 db/migrate/20220330160754_copy_groups_uploads_to_upload_references.rb create mode 100644 db/migrate/20220330160757_copy_user_exports_uploads_to_upload_references.rb create mode 100644 db/migrate/20220330164740_copy_theme_fields_uploads_to_upload_references.rb create mode 100644 db/migrate/20220404195635_copy_categories_uploads_to_upload_references.rb create mode 100644 db/migrate/20220404201949_copy_custom_emojis_uploads_to_upload_references.rb create mode 100644 db/migrate/20220404203356_copy_user_profiles_uploads_to_upload_references.rb create mode 100644 db/migrate/20220404204439_copy_user_avatars_uploads_to_upload_references.rb create mode 100644 db/migrate/20220404212716_copy_theme_settings_uploads_to_upload_references.rb create mode 100644 db/migrate/20220526203356_copy_user_uploads_to_upload_references.rb create mode 100644 db/post_migrate/20220309132720_copy_post_uploads_to_upload_references_for_sync.rb delete mode 100644 spec/models/post_upload_spec.rb create mode 100644 spec/models/upload_reference_spec.rb diff --git a/app/jobs/scheduled/clean_up_uploads.rb b/app/jobs/scheduled/clean_up_uploads.rb index df55eeb1f10..9c9ce5df3ca 100644 --- a/app/jobs/scheduled/clean_up_uploads.rb +++ b/app/jobs/scheduled/clean_up_uploads.rb @@ -31,19 +31,20 @@ module Jobs .where("uploads.retain_hours IS NULL OR uploads.created_at < current_timestamp - interval '1 hour' * uploads.retain_hours") .where("uploads.created_at < ?", grace_period.hour.ago) .where("uploads.access_control_post_id IS NULL") - .joins("LEFT JOIN post_uploads pu ON pu.upload_id = uploads.id") - .where("pu.upload_id IS NULL") + .joins("LEFT JOIN upload_references ON upload_references.upload_id = uploads.id") + .where("upload_references.upload_id IS NULL") .with_no_non_post_relations result.find_each do |upload| + next if Upload.in_use_callbacks&.any? { |callback| callback.call(upload) } + if upload.sha1.present? + # TODO: Remove this check after UploadReferences records were created encoded_sha = Base62.encode(upload.sha1.hex) next if ReviewableQueuedPost.pending.where("payload->>'raw' LIKE '%#{upload.sha1}%' OR payload->>'raw' LIKE '%#{encoded_sha}%'").exists? next if Draft.where("data LIKE '%#{upload.sha1}%' OR data LIKE '%#{encoded_sha}%'").exists? next if UserProfile.where("bio_raw LIKE '%#{upload.sha1}%' OR bio_raw LIKE '%#{encoded_sha}%'").exists? - next if Upload.in_use_callbacks&.any? { |callback| callback.call(upload) } - upload.destroy else upload.delete diff --git a/app/models/badge.rb b/app/models/badge.rb index af0c054d66a..77527e2af3e 100644 --- a/app/models/badge.rb +++ b/app/models/badge.rb @@ -108,6 +108,7 @@ class Badge < ActiveRecord::Base belongs_to :image_upload, class_name: 'Upload' has_many :user_badges, dependent: :destroy + has_many :upload_references, as: :target, dependent: :destroy validates :name, presence: true, uniqueness: true validates :badge_type, presence: true @@ -119,6 +120,12 @@ class Badge < ActiveRecord::Base before_create :ensure_not_system before_save :sanitize_description + after_save do + if saved_change_to_image_upload_id? + UploadReference.ensure_exist!(upload_ids: [self.image_upload_id], target: self) + end + end + after_commit do SvgSprite.expire_cache UserStat.update_distinct_badge_count if saved_change_to_enabled? diff --git a/app/models/category.rb b/app/models/category.rb index 3702a03bd6f..9b1201bfc4d 100644 --- a/app/models/category.rb +++ b/app/models/category.rb @@ -45,6 +45,7 @@ class Category < ActiveRecord::Base has_many :category_groups, dependent: :destroy has_many :groups, through: :category_groups has_many :topic_timers, dependent: :destroy + has_many :upload_references, as: :target, dependent: :destroy has_and_belongs_to_many :web_hooks @@ -80,6 +81,13 @@ class Category < ActiveRecord::Base after_save :clear_url_cache after_save :update_reviewables + after_save do + if saved_change_to_uploaded_logo_id? || saved_change_to_uploaded_background_id? + upload_ids = [self.uploaded_logo_id, self.uploaded_background_id] + UploadReference.ensure_exist!(upload_ids: upload_ids, target: self) + end + end + after_destroy :reset_topic_ids_cache after_destroy :publish_category_deletion after_destroy :remove_site_settings diff --git a/app/models/custom_emoji.rb b/app/models/custom_emoji.rb index 809b4ba6c03..61a0334aeb9 100644 --- a/app/models/custom_emoji.rb +++ b/app/models/custom_emoji.rb @@ -3,8 +3,16 @@ class CustomEmoji < ActiveRecord::Base belongs_to :upload + has_many :upload_references, as: :target, dependent: :destroy + validates :name, presence: true, uniqueness: true validates :upload_id, presence: true + + after_save do + if saved_change_to_upload_id? + UploadReference.ensure_exist!(upload_ids: [self.upload_id], target: self) + end + end end # == Schema Information diff --git a/app/models/draft.rb b/app/models/draft.rb index 459ddea41c7..ced0d4df5b4 100644 --- a/app/models/draft.rb +++ b/app/models/draft.rb @@ -82,7 +82,7 @@ class Draft < ActiveRecord::Base owner: owner } - DB.exec(<<~SQL, opts) + draft_id = DB.query_single(<<~SQL, opts).first INSERT INTO drafts (user_id, draft_key, data, sequence, owner, created_at, updated_at) VALUES (:user_id, :draft_key, :data, :sequence, :owner, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP) ON CONFLICT (user_id, draft_key) DO @@ -93,11 +93,18 @@ class Draft < ActiveRecord::Base revisions = drafts.revisions + 1, owner = :owner, updated_at = CURRENT_TIMESTAMP + RETURNING id SQL UserStat.update_draft_count(user.id) end + UploadReference.ensure_exist!( + upload_ids: Upload.extract_upload_ids(data), + target_type: 'Draft', + target_id: draft_id + ) + sequence end diff --git a/app/models/group.rb b/app/models/group.rb index d0a401a8d6e..c6364dbbfe4 100644 --- a/app/models/group.rb +++ b/app/models/group.rb @@ -36,6 +36,8 @@ class Group < ActiveRecord::Base has_many :associated_groups, through: :group_associated_groups, dependent: :destroy belongs_to :flair_upload, class_name: 'Upload' + has_many :upload_references, as: :target, dependent: :destroy + belongs_to :smtp_updated_by, class_name: 'User' belongs_to :imap_updated_by, class_name: 'User' @@ -51,6 +53,12 @@ class Group < ActiveRecord::Base after_save :enqueue_update_mentions_job, if: Proc.new { |g| g.name_before_last_save && g.saved_change_to_name? } + after_save do + if saved_change_to_flair_upload_id? + UploadReference.ensure_exist!(upload_ids: [self.flair_upload_id], target: self) + end + end + after_save :expire_cache after_destroy :expire_cache diff --git a/app/models/post.rb b/app/models/post.rb index 4f81ce7939e..5cfccffe554 100644 --- a/app/models/post.rb +++ b/app/models/post.rb @@ -42,8 +42,8 @@ class Post < ActiveRecord::Base has_many :topic_links has_many :group_mentions, dependent: :destroy - has_many :post_uploads, dependent: :delete_all - has_many :uploads, through: :post_uploads + has_many :upload_references, as: :target, dependent: :destroy + has_many :uploads, through: :upload_references has_one :post_stat @@ -958,16 +958,19 @@ class Post < ActiveRecord::Base upload_ids << upload.id if upload.present? end - post_uploads = upload_ids.map do |upload_id| - { post_id: self.id, upload_id: upload_id } + upload_references = upload_ids.map do |upload_id| + { + target_id: self.id, + target_type: self.class.name, + upload_id: upload_id, + created_at: Time.zone.now, + updated_at: Time.zone.now + } end - PostUpload.transaction do - PostUpload.where(post_id: self.id).delete_all - - if post_uploads.size > 0 - PostUpload.insert_all(post_uploads) - end + UploadReference.transaction do + UploadReference.where(target: self).delete_all + UploadReference.insert_all(upload_references) if upload_references.size > 0 if SiteSetting.secure_media? Upload.where( @@ -1067,7 +1070,11 @@ class Post < ActiveRecord::Base query.find_in_batches do |posts| ids = posts.pluck(:id) - sha1s = Upload.joins(:post_uploads).where("post_uploads.post_id >= ? AND post_uploads.post_id <= ?", ids.min, ids.max).pluck(:sha1) + sha1s = Upload + .joins(:upload_references) + .where(upload_references: { target_type: "Post" }) + .where("upload_references.target_id BETWEEN ? AND ?", ids.min, ids.max) + .pluck(:sha1) posts.each do |post| post.each_upload_url do |src, path, sha1| diff --git a/app/models/reviewable_queued_post.rb b/app/models/reviewable_queued_post.rb index f51d0164eb2..5929242a76f 100644 --- a/app/models/reviewable_queued_post.rb +++ b/app/models/reviewable_queued_post.rb @@ -7,6 +7,13 @@ class ReviewableQueuedPost < Reviewable DiscourseEvent.trigger(:queued_post_created, self) end + after_save do + if saved_change_to_payload? && self.status == Reviewable.statuses[:pending] && self.payload&.[]('raw').present? + upload_ids = Upload.extract_upload_ids(self.payload['raw']) + UploadReference.ensure_exist!(upload_ids: upload_ids, target: self) + end + end + after_commit :compute_user_stats, only: %i[create update] def build_actions(actions, guardian, args) diff --git a/app/models/site_setting.rb b/app/models/site_setting.rb index a6641f967a9..f8e08a7e311 100644 --- a/app/models/site_setting.rb +++ b/app/models/site_setting.rb @@ -4,9 +4,22 @@ class SiteSetting < ActiveRecord::Base extend GlobalPath extend SiteSettingExtension + has_many :upload_references, as: :target, dependent: :destroy + validates_presence_of :name validates_presence_of :data_type + after_save do + if saved_change_to_value? + if self.data_type == SiteSettings::TypeSupervisor.types[:upload] + UploadReference.ensure_exist!(upload_ids: [self.value], target: self) + elsif self.data_type == SiteSettings::TypeSupervisor.types[:uploaded_image_list] + upload_ids = self.value.split('|').compact.uniq + UploadReference.ensure_exist!(upload_ids: upload_ids, target: self) + end + end + end + def self.load_settings(file, plugin: nil) SiteSettings::YamlLoader.new(file).load do |category, name, default, opts| setting(name, default, opts.merge(category: category, plugin: plugin)) diff --git a/app/models/theme_field.rb b/app/models/theme_field.rb index e35d2cedfab..bdc017c6617 100644 --- a/app/models/theme_field.rb +++ b/app/models/theme_field.rb @@ -4,6 +4,13 @@ class ThemeField < ActiveRecord::Base belongs_to :upload has_one :javascript_cache, dependent: :destroy + has_one :upload_reference, as: :target, dependent: :destroy + + after_save do + if self.type_id == ThemeField.types[:theme_upload_var] && saved_change_to_upload_id? + UploadReference.ensure_exist!(upload_ids: [self.upload_id], target: self) + end + end scope :find_by_theme_ids, ->(theme_ids) { return none unless theme_ids.present? diff --git a/app/models/theme_setting.rb b/app/models/theme_setting.rb index 86d8c0247c5..c8d273471bd 100644 --- a/app/models/theme_setting.rb +++ b/app/models/theme_setting.rb @@ -3,6 +3,8 @@ class ThemeSetting < ActiveRecord::Base belongs_to :theme + has_many :upload_references, as: :target, dependent: :destroy + validates_presence_of :name, :theme validates :data_type, numericality: { only_integer: true } validates :name, length: { maximum: 255 } @@ -10,6 +12,12 @@ class ThemeSetting < ActiveRecord::Base after_save :clear_settings_cache after_destroy :clear_settings_cache + after_save do + if self.data_type == ThemeSetting.types[:upload] && saved_change_to_value? + UploadReference.ensure_exist!(upload_ids: [self.value], target: self) + end + end + def clear_settings_cache # All necessary caches will be cleared on next ensure_baked! theme.settings_field&.invalidate_baked! diff --git a/app/models/upload.rb b/app/models/upload.rb index 2ac393401cd..2f84e2bbeaf 100644 --- a/app/models/upload.rb +++ b/app/models/upload.rb @@ -20,13 +20,11 @@ class Upload < ActiveRecord::Base Post.unscoped { super } end - has_many :post_uploads, dependent: :destroy - has_many :posts, through: :post_uploads - has_many :post_hotlinked_media, dependent: :destroy, class_name: "PostHotlinkedMedia" - has_many :optimized_images, dependent: :destroy has_many :user_uploads, dependent: :destroy + has_many :upload_references, dependent: :destroy + has_many :posts, through: :upload_references, source: :target, source_type: 'Post' has_many :topic_thumbnails attr_accessor :for_group_message @@ -87,43 +85,9 @@ class Upload < ActiveRecord::Base end def self.with_no_non_post_relations - scope = self - .joins(<<~SQL) - LEFT JOIN site_settings ss - ON NULLIF(ss.value, '')::integer = uploads.id - AND ss.data_type = #{SiteSettings::TypeSupervisor.types[:upload].to_i} - SQL - .where("ss.value IS NULL") - .joins("LEFT JOIN users u ON u.uploaded_avatar_id = uploads.id") - .where("u.uploaded_avatar_id IS NULL") - .joins("LEFT JOIN user_avatars ua ON ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id") - .where("ua.gravatar_upload_id IS NULL AND ua.custom_upload_id IS NULL") - .joins("LEFT JOIN user_profiles up ON up.profile_background_upload_id = uploads.id OR up.card_background_upload_id = uploads.id") - .where("up.profile_background_upload_id IS NULL AND up.card_background_upload_id IS NULL") - .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id") - .where("c.uploaded_logo_id IS NULL AND c.uploaded_background_id IS NULL") - .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id") - .where("ce.upload_id IS NULL") - .joins("LEFT JOIN theme_fields tf ON tf.upload_id = uploads.id") - .where("tf.upload_id IS NULL") - .joins("LEFT JOIN user_exports ue ON ue.upload_id = uploads.id") - .where("ue.upload_id IS NULL") - .joins("LEFT JOIN groups g ON g.flair_upload_id = uploads.id") - .where("g.flair_upload_id IS NULL") - .joins("LEFT JOIN badges b ON b.image_upload_id = uploads.id") - .where("b.image_upload_id IS NULL") - .joins(<<~SQL) - LEFT JOIN theme_settings ts - ON NULLIF(ts.value, '')::integer = uploads.id - AND ts.data_type = #{ThemeSetting.types[:upload].to_i} - SQL - .where("ts.value IS NULL") - - if SiteSetting.selectable_avatars.present? - scope = scope.where.not(id: SiteSetting.selectable_avatars.map(&:id)) - end - - scope + self + .joins("LEFT JOIN upload_references ur ON ur.upload_id = uploads.id AND ur.target_type != 'Post'") + .where("ur.upload_id IS NULL") end def to_s @@ -542,6 +506,22 @@ class Upload < ActiveRecord::Base problems end + def self.extract_upload_ids(raw) + return [] if raw.blank? + + sha1s = [] + + raw.scan(/\/(\h{40})/).each do |match| + sha1s << match[0] + end + + raw.scan(/\/([a-zA-Z0-9]{27})/).each do |match| + sha1s << Upload.sha1_from_base62_encoded(match[0]) + end + + Upload.where(sha1: sha1s.uniq).pluck(:id) + end + private def short_url_basename diff --git a/app/models/upload_reference.rb b/app/models/upload_reference.rb new file mode 100644 index 00000000000..58cf8f86d19 --- /dev/null +++ b/app/models/upload_reference.rb @@ -0,0 +1,63 @@ +# frozen_string_literal: true + +class UploadReference < ActiveRecord::Base + belongs_to :upload + belongs_to :target, polymorphic: true + + def self.ensure_exist!(upload_ids: [], target: nil, target_type: nil, target_id: nil) + raise "target OR target_type and target_id are required" if !target && !(target_type && target_id) + + if target.present? + target_type = target.class + target_id = target.id + end + + upload_ids = upload_ids.uniq.reject(&:blank?) + target_type = target_type.to_s + + if upload_ids.empty? + UploadReference + .where(target_type: target_type, target_id: target_id) + .delete_all + + return + end + + rows = upload_ids.map do |upload_id| + { + upload_id: upload_id, + target_type: target_type, + target_id: target_id, + created_at: Time.zone.now, + updated_at: Time.zone.now, + } + end + + UploadReference.transaction do |transaction| + UploadReference + .where(target_type: target_type, target_id: target_id) + .where.not(upload_id: upload_ids) + .delete_all + + UploadReference.insert_all(rows) + end + end +end + +# == Schema Information +# +# Table name: upload_references +# +# id :bigint not null, primary key +# upload_id :bigint not null +# target_type :string not null +# target_id :bigint not null +# created_at :datetime not null +# updated_at :datetime not null +# +# Indexes +# +# index_upload_references_on_target (target_type,target_id) +# index_upload_references_on_upload_and_target (upload_id,target_type,target_id) UNIQUE +# index_upload_references_on_upload_id (upload_id) +# diff --git a/app/models/user.rb b/app/models/user.rb index 2bef343009d..2ca32db206e 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -23,6 +23,7 @@ class User < ActiveRecord::Base has_many :email_tokens, dependent: :destroy has_many :topic_links, dependent: :destroy has_many :user_uploads, dependent: :destroy + has_many :upload_references, as: :target, dependent: :destroy has_many :user_emails, dependent: :destroy, autosave: true has_many :user_associated_accounts, dependent: :destroy has_many :oauth2_user_infos, dependent: :destroy @@ -150,6 +151,12 @@ class User < ActiveRecord::Base after_save :index_search after_save :check_site_contact_username + after_save do + if saved_change_to_uploaded_avatar_id? + UploadReference.ensure_exist!(upload_ids: [self.uploaded_avatar_id], target: self) + end + end + after_commit :trigger_user_created_event, on: :create after_commit :trigger_user_destroyed_event, on: :destroy diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index 43e521c260e..349509d0149 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -4,6 +4,14 @@ class UserAvatar < ActiveRecord::Base belongs_to :user belongs_to :gravatar_upload, class_name: 'Upload' belongs_to :custom_upload, class_name: 'Upload' + has_many :upload_references, as: :target, dependent: :destroy + + after_save do + if saved_change_to_custom_upload_id? || saved_change_to_gravatar_upload_id? + upload_ids = [self.custom_upload_id, self.gravatar_upload_id] + UploadReference.ensure_exist!(upload_ids: upload_ids, target: self) + end + end @@custom_user_gravatar_email_hash = { Discourse::SYSTEM_USER_ID => User.email_hash("info@discourse.org") diff --git a/app/models/user_export.rb b/app/models/user_export.rb index a7b5b6784c1..35059147250 100644 --- a/app/models/user_export.rb +++ b/app/models/user_export.rb @@ -5,6 +5,14 @@ class UserExport < ActiveRecord::Base belongs_to :upload, dependent: :destroy belongs_to :topic, dependent: :destroy + has_many :upload_references, as: :target, dependent: :destroy + + after_save do + if saved_change_to_upload_id? + UploadReference.ensure_exist!(upload_ids: [self.upload_id], target: self) + end + end + DESTROY_CREATED_BEFORE = 2.days.ago def self.remove_old_exports diff --git a/app/models/user_profile.rb b/app/models/user_profile.rb index 66186468d71..c5038edd6d5 100644 --- a/app/models/user_profile.rb +++ b/app/models/user_profile.rb @@ -6,6 +6,7 @@ class UserProfile < ActiveRecord::Base belongs_to :profile_background_upload, class_name: "Upload" belongs_to :granted_title_badge, class_name: "Badge" belongs_to :featured_topic, class_name: 'Topic' + has_many :upload_references, as: :target, dependent: :destroy validates :bio_raw, length: { maximum: 3000 }, watched_words: true validates :website, url: true, allow_blank: true, if: Proc.new { |c| c.new_record? || c.website_changed? } @@ -16,6 +17,13 @@ class UserProfile < ActiveRecord::Base after_save :trigger_badges after_save :pull_hotlinked_image + after_save do + if saved_change_to_profile_background_upload_id? || saved_change_to_card_background_upload_id? || saved_change_to_bio_raw? + upload_ids = [self.profile_background_upload_id, self.card_background_upload_id] + Upload.extract_upload_ids(self.bio_raw) + UploadReference.ensure_exist!(upload_ids: upload_ids, target: self) + end + end + validate :website_domain_validator, if: Proc.new { |c| c.new_record? || c.website_changed? } has_many :user_profile_views, dependent: :destroy diff --git a/db/migrate/20220308201942_create_upload_references.rb b/db/migrate/20220308201942_create_upload_references.rb new file mode 100644 index 00000000000..84891f80a52 --- /dev/null +++ b/db/migrate/20220308201942_create_upload_references.rb @@ -0,0 +1,13 @@ +# frozen_string_literal: true + +class CreateUploadReferences < ActiveRecord::Migration[6.1] + def change + create_table :upload_references do |t| + t.references :upload, null: false + t.references :target, polymorphic: true, null: false + t.timestamps + end + + add_index :upload_references, [:upload_id, :target_type, :target_id], unique: true, name: 'index_upload_references_on_upload_and_target' + end +end diff --git a/db/migrate/20220309132719_copy_post_uploads_to_upload_references.rb b/db/migrate/20220309132719_copy_post_uploads_to_upload_references.rb new file mode 100644 index 00000000000..7674be3df14 --- /dev/null +++ b/db/migrate/20220309132719_copy_post_uploads_to_upload_references.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CopyPostUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT post_uploads.upload_id, 'Post', post_uploads.post_id, uploads.created_at, uploads.updated_at + FROM post_uploads + JOIN uploads ON uploads.id = post_uploads.upload_id + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220330160747_copy_site_settings_uploads_to_upload_references.rb b/db/migrate/20220330160747_copy_site_settings_uploads_to_upload_references.rb new file mode 100644 index 00000000000..28fa3cabf8f --- /dev/null +++ b/db/migrate/20220330160747_copy_site_settings_uploads_to_upload_references.rb @@ -0,0 +1,26 @@ +# frozen_string_literal: true + +class CopySiteSettingsUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + WITH site_settings_uploads AS ( + SELECT id, unnest(string_to_array(value, '|'))::integer upload_id + FROM site_settings + WHERE data_type = 17 + UNION + SELECT id, value::integer + FROM site_settings + WHERE data_type = 18 AND value != '' + ) + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT site_settings_uploads.upload_id, 'SiteSetting', site_settings_uploads.id, uploads.created_at, uploads.updated_at + FROM site_settings_uploads + JOIN uploads ON uploads.id = site_settings_uploads.upload_id + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220330160751_copy_badges_uploads_to_upload_references.rb b/db/migrate/20220330160751_copy_badges_uploads_to_upload_references.rb new file mode 100644 index 00000000000..8d2b4162e6e --- /dev/null +++ b/db/migrate/20220330160751_copy_badges_uploads_to_upload_references.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyBadgesUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT badges.image_upload_id, 'Badge', badges.id, uploads.created_at, uploads.updated_at + FROM badges + JOIN uploads ON uploads.id = badges.image_upload_id + WHERE badges.image_upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220330160754_copy_groups_uploads_to_upload_references.rb b/db/migrate/20220330160754_copy_groups_uploads_to_upload_references.rb new file mode 100644 index 00000000000..165177dbc21 --- /dev/null +++ b/db/migrate/20220330160754_copy_groups_uploads_to_upload_references.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyGroupsUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT groups.flair_upload_id, 'Group', groups.id, uploads.created_at, uploads.updated_at + FROM groups + JOIN uploads ON uploads.id = groups.flair_upload_id + WHERE groups.flair_upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220330160757_copy_user_exports_uploads_to_upload_references.rb b/db/migrate/20220330160757_copy_user_exports_uploads_to_upload_references.rb new file mode 100644 index 00000000000..33eef6f2197 --- /dev/null +++ b/db/migrate/20220330160757_copy_user_exports_uploads_to_upload_references.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +class CopyUserExportsUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT user_exports.upload_id, 'UserExport', user_exports.id, uploads.created_at, uploads.updated_at + FROM user_exports + JOIN uploads ON uploads.id = user_exports.upload_id + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220330164740_copy_theme_fields_uploads_to_upload_references.rb b/db/migrate/20220330164740_copy_theme_fields_uploads_to_upload_references.rb new file mode 100644 index 00000000000..ec10d1fe947 --- /dev/null +++ b/db/migrate/20220330164740_copy_theme_fields_uploads_to_upload_references.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyThemeFieldsUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT theme_fields.upload_id, 'ThemeField', theme_fields.id, uploads.created_at, uploads.updated_at + FROM theme_fields + JOIN uploads ON uploads.id = theme_fields.upload_id + WHERE type_id = 2 + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220404195635_copy_categories_uploads_to_upload_references.rb b/db/migrate/20220404195635_copy_categories_uploads_to_upload_references.rb new file mode 100644 index 00000000000..0fa35bc8946 --- /dev/null +++ b/db/migrate/20220404195635_copy_categories_uploads_to_upload_references.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CopyCategoriesUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT categories.uploaded_logo_id, 'Category', categories.id, uploads.created_at, uploads.updated_at + FROM categories + JOIN uploads ON uploads.id = categories.uploaded_logo_id + WHERE categories.uploaded_logo_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT categories.uploaded_background_id, 'Category', categories.id, uploads.created_at, uploads.updated_at + FROM categories + JOIN uploads ON uploads.id = categories.uploaded_background_id + WHERE categories.uploaded_background_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220404201949_copy_custom_emojis_uploads_to_upload_references.rb b/db/migrate/20220404201949_copy_custom_emojis_uploads_to_upload_references.rb new file mode 100644 index 00000000000..953857a74ec --- /dev/null +++ b/db/migrate/20220404201949_copy_custom_emojis_uploads_to_upload_references.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyCustomEmojisUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT custom_emojis.upload_id, 'CustomEmoji', custom_emojis.id, uploads.created_at, uploads.updated_at + FROM custom_emojis + JOIN uploads ON uploads.id = custom_emojis.upload_id + WHERE custom_emojis.upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220404203356_copy_user_profiles_uploads_to_upload_references.rb b/db/migrate/20220404203356_copy_user_profiles_uploads_to_upload_references.rb new file mode 100644 index 00000000000..3c465965a95 --- /dev/null +++ b/db/migrate/20220404203356_copy_user_profiles_uploads_to_upload_references.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CopyUserProfilesUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT user_profiles.profile_background_upload_id, 'UserProfile', user_profiles.user_id, uploads.created_at, uploads.updated_at + FROM user_profiles + JOIN uploads ON uploads.id = user_profiles.profile_background_upload_id + WHERE user_profiles.profile_background_upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT user_profiles.card_background_upload_id, 'UserProfile', user_profiles.user_id, uploads.created_at, uploads.updated_at + FROM user_profiles + JOIN uploads ON uploads.id = user_profiles.card_background_upload_id + WHERE user_profiles.card_background_upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220404204439_copy_user_avatars_uploads_to_upload_references.rb b/db/migrate/20220404204439_copy_user_avatars_uploads_to_upload_references.rb new file mode 100644 index 00000000000..d7e5d1ad02d --- /dev/null +++ b/db/migrate/20220404204439_copy_user_avatars_uploads_to_upload_references.rb @@ -0,0 +1,27 @@ +# frozen_string_literal: true + +class CopyUserAvatarsUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT user_avatars.custom_upload_id, 'UserAvatar', user_avatars.id, uploads.created_at, uploads.updated_at + FROM user_avatars + JOIN uploads ON uploads.id = user_avatars.custom_upload_id + WHERE user_avatars.custom_upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT user_avatars.gravatar_upload_id, 'UserAvatar', user_avatars.id, uploads.created_at, uploads.updated_at + FROM user_avatars + JOIN uploads ON uploads.id = user_avatars.gravatar_upload_id + WHERE user_avatars.gravatar_upload_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220404212716_copy_theme_settings_uploads_to_upload_references.rb b/db/migrate/20220404212716_copy_theme_settings_uploads_to_upload_references.rb new file mode 100644 index 00000000000..8d3cefe6982 --- /dev/null +++ b/db/migrate/20220404212716_copy_theme_settings_uploads_to_upload_references.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyThemeSettingsUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT theme_settings.value::int, 'ThemeSetting', theme_settings.id, uploads.created_at, uploads.updated_at + FROM theme_settings + JOIN uploads ON uploads.id = theme_settings.value::int + WHERE data_type = 6 AND theme_settings.value IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/migrate/20220526203356_copy_user_uploads_to_upload_references.rb b/db/migrate/20220526203356_copy_user_uploads_to_upload_references.rb new file mode 100644 index 00000000000..5ab2de8daf0 --- /dev/null +++ b/db/migrate/20220526203356_copy_user_uploads_to_upload_references.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyUserUploadsToUploadReferences < ActiveRecord::Migration[6.1] + def up + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT users.uploaded_avatar_id, 'User', users.id, uploads.created_at, uploads.updated_at + FROM users + JOIN uploads ON uploads.id = users.uploaded_avatar_id + WHERE users.uploaded_avatar_id IS NOT NULL + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/db/post_migrate/20220309132720_copy_post_uploads_to_upload_references_for_sync.rb b/db/post_migrate/20220309132720_copy_post_uploads_to_upload_references_for_sync.rb new file mode 100644 index 00000000000..722b25fad6a --- /dev/null +++ b/db/post_migrate/20220309132720_copy_post_uploads_to_upload_references_for_sync.rb @@ -0,0 +1,18 @@ +# frozen_string_literal: true + +class CopyPostUploadsToUploadReferencesForSync < ActiveRecord::Migration[6.1] + def up + # Migrates any post uploads that might have been created between the first + # migration and when the deploy process finished. + execute <<~SQL + INSERT INTO upload_references(upload_id, target_type, target_id, created_at, updated_at) + SELECT upload_id, 'Post', post_id, NOW(), NOW() + FROM post_uploads + ON CONFLICT DO NOTHING + SQL + end + + def down + raise ActiveRecord::IrreversibleMigration + end +end diff --git a/lib/backup_restore/uploads_restorer.rb b/lib/backup_restore/uploads_restorer.rb index 0cf18a0558d..7409c38f56c 100644 --- a/lib/backup_restore/uploads_restorer.rb +++ b/lib/backup_restore/uploads_restorer.rb @@ -153,7 +153,7 @@ module BackupRestore DB.exec(<<~SQL) UPDATE posts SET baked_version = NULL - WHERE id IN (SELECT post_id FROM post_uploads) + WHERE id IN (SELECT target_id FROM upload_references WHERE target_type = 'Post') SQL end end diff --git a/lib/search.rb b/lib/search.rb index fb1192425a5..24115742085 100644 --- a/lib/search.rb +++ b/lib/search.rb @@ -695,9 +695,9 @@ class Search UNION - SELECT post_uploads.post_id + SELECT upload_references.target_id FROM uploads - JOIN post_uploads ON post_uploads.upload_id = uploads.id + JOIN upload_references ON upload_references.target_type = 'Post' AND upload_references.upload_id = uploads.id WHERE lower(uploads.extension) IN (:file_extensions) )", file_extensions: file_extensions) end diff --git a/lib/shrink_uploaded_image.rb b/lib/shrink_uploaded_image.rb index 45817669d11..c0de28c52fd 100644 --- a/lib/shrink_uploaded_image.rb +++ b/lib/shrink_uploaded_image.rb @@ -19,7 +19,7 @@ class ShrinkUploadedImage return false end - posts = Post.unscoped.joins(:post_uploads).where(post_uploads: { upload_id: original_upload.id }).uniq.sort_by(&:created_at) + posts = Post.unscoped.joins(:upload_references).where(upload_references: { upload_id: original_upload.id }).uniq.sort_by(&:created_at) if posts.empty? log "Upload not used in any posts" @@ -134,7 +134,10 @@ class ShrinkUploadedImage if existing_upload begin - PostUpload.where(upload_id: original_upload.id).update_all(upload_id: upload.id) + UploadReferences + .where(target_type: 'Post') + .where(upload_id: original_upload.id) + .update_all(upload_id: upload.id) rescue ActiveRecord::RecordNotUnique, PG::UniqueViolation end else diff --git a/lib/tasks/posts.rake b/lib/tasks/posts.rake index 4b7b35e8f72..f0b664fb3e4 100644 --- a/lib/tasks/posts.rake +++ b/lib/tasks/posts.rake @@ -662,7 +662,10 @@ def correct_inline_uploads dry_run = (ENV["DRY_RUN"].nil? ? true : ENV["DRY_RUN"] != "false") verbose = ENV["VERBOSE"] - scope = Post.joins(:post_uploads).distinct("posts.id") + scope = Upload + .joins(:upload_references) + .where(upload_references: { target_type: 'Post' }) + .distinct("posts.id") .where(<<~SQL) raw LIKE '%/uploads/#{RailsMultisite::ConnectionManagement.current_db}/original/%' SQL diff --git a/lib/tasks/uploads.rake b/lib/tasks/uploads.rake index 051b4e95313..898f21cd972 100644 --- a/lib/tasks/uploads.rake +++ b/lib/tasks/uploads.rake @@ -529,7 +529,7 @@ task "uploads:disable_secure_media" => :environment do SiteSetting.secure_media = false - secure_uploads = Upload.joins(:post_uploads).where(secure: true) + secure_uploads = Upload.joins(:upload_references).where(upload_references: { target_type: 'Post' }).where(secure: true) secure_upload_count = secure_uploads.count secure_upload_ids = secure_uploads.pluck(:id) @@ -541,7 +541,7 @@ task "uploads:disable_secure_media" => :environment do ) post_ids_to_rebake = DB.query_single( - "SELECT DISTINCT post_id FROM post_uploads WHERE upload_id IN (?)", secure_upload_ids + "SELECT DISTINCT target_id FROM upload_references WHERE upload_id IN (?) AND target_type = 'Post'", secure_upload_ids ) adjust_acls(secure_upload_ids) post_rebake_errors = rebake_upload_posts(post_ids_to_rebake) @@ -621,8 +621,8 @@ def mark_all_as_secure_login_required post_upload_ids_marked_secure = DB.query_single(<<~SQL) WITH upl AS ( SELECT DISTINCT ON (upload_id) upload_id - FROM post_uploads - INNER JOIN posts ON posts.id = post_uploads.post_id + FROM upload_references + INNER JOIN posts ON posts.id = upload_references.target_id AND upload_references.target_type = 'Post' INNER JOIN topics ON topics.id = posts.topic_id ) UPDATE uploads @@ -646,7 +646,7 @@ def mark_all_as_secure_login_required puts "Finished marking upload(s) as secure." post_ids_to_rebake = DB.query_single( - "SELECT DISTINCT post_id FROM post_uploads WHERE upload_id IN (?)", post_upload_ids_marked_secure + "SELECT DISTINCT target_id FROM upload_references WHERE upload_id IN (?) AND target_type = 'Post'", post_upload_ids_marked_secure ) [post_ids_to_rebake, (post_upload_ids_marked_secure + upload_ids_marked_not_secure).uniq] end @@ -665,8 +665,8 @@ def update_specific_upload_security_no_login_required post_upload_ids_marked_secure = DB.query_single(<<~SQL) WITH upl AS ( SELECT DISTINCT ON (upload_id) upload_id - FROM post_uploads - INNER JOIN posts ON posts.id = post_uploads.post_id + FROM upload_references + INNER JOIN posts ON posts.id = upload_references.target_id AND upload_references.target_type = 'Post' INNER JOIN topics ON topics.id = posts.topic_id LEFT JOIN categories ON categories.id = topics.category_id WHERE (topics.category_id IS NOT NULL AND categories.read_restricted) OR @@ -686,8 +686,8 @@ def update_specific_upload_security_no_login_required post_upload_ids_marked_not_secure = DB.query_single(<<~SQL) WITH upl AS ( SELECT DISTINCT ON (upload_id) upload_id - FROM post_uploads - INNER JOIN posts ON posts.id = post_uploads.post_id + FROM upload_references + INNER JOIN posts ON posts.id = upload_references.target_id AND upload_references.target_type = 'Post' INNER JOIN topics ON topics.id = posts.topic_id LEFT JOIN categories ON categories.id = topics.category_id WHERE (topics.archetype = 'regular' AND topics.category_id IS NOT NULL AND NOT categories.read_restricted) OR @@ -716,14 +716,17 @@ def update_specific_upload_security_no_login_required puts "Finished updating upload security. Marked #{upload_ids_marked_not_secure.length} uploads not linked to posts as not secure." all_upload_ids_changed = (upload_ids_changed + upload_ids_marked_not_secure).uniq - post_ids_to_rebake = DB.query_single("SELECT DISTINCT post_id FROM post_uploads WHERE upload_id IN (?)", upload_ids_changed) + post_ids_to_rebake = DB.query_single("SELECT DISTINCT target_id FROM upload_references WHERE upload_id IN (?) AND target_type = 'Post'", upload_ids_changed) [post_ids_to_rebake, all_upload_ids_changed] end def update_uploads_access_control_post DB.exec(<<~SQL) WITH upl AS ( - SELECT DISTINCT ON (upload_id) upload_id, post_id FROM post_uploads ORDER BY upload_id, post_id + SELECT DISTINCT ON (upload_id) upload_id, target_id AS post_id + FROM upload_references + WHERE target_type = 'Post' + ORDER BY upload_id, target_id ) UPDATE uploads SET access_control_post_id = upl.post_id @@ -855,9 +858,9 @@ def analyze_missing_s3 puts "List of posts with missing images:" sql = <<~SQL SELECT post_id, url, sha1, extension, uploads.id - FROM post_uploads pu - RIGHT JOIN uploads on uploads.id = pu.upload_id - WHERE verification_status = :invalid_etag + FROM upload_references ur + RIGHT JOIN uploads on uploads.id = ur.upload_id + WHERE ur.target_type = 'Post' AND verification_status = :invalid_etag ORDER BY created_at SQL @@ -867,9 +870,9 @@ def analyze_missing_s3 DB.query(sql, invalid_etag: Upload.verification_statuses[:invalid_etag]).each do |r| all << r - if r.post_id - lookup[r.post_id] ||= [] - lookup[r.post_id] << [r.url, r.sha1, r.extension] + if r.target_id + lookup[r.target_id] ||= [] + lookup[r.target_id] << [r.url, r.sha1, r.extension] else other << r end @@ -894,7 +897,7 @@ def analyze_missing_s3 ids = all.map { |r| r.id } lookups = [ - [:post_uploads, :upload_id], + [:upload_references, :upload_id], [:users, :uploaded_avatar_id], [:user_avatars, :gravatar_upload_id], [:user_avatars, :custom_upload_id], @@ -1026,7 +1029,7 @@ def fix_missing_s3 puts "Failed to save upload #{save_error}" else OptimizedImage.where(upload_id: upload.id).destroy_all - rebake_ids = PostUpload.where(upload_id: upload.id).pluck(:post_id) + rebake_ids = UploadReferences.where(upload_id: upload.id).where(target_type: 'Post').pluck(:target_id) if rebake_ids.present? Post.where(id: rebake_ids).each do |post| @@ -1044,11 +1047,12 @@ def fix_missing_s3 puts "Rebaking posts with missing uploads, this can take a while as all rebaking runs inline" sql = <<~SQL - SELECT post_id - FROM post_uploads pu - JOIN uploads on uploads.id = pu.upload_id + SELECT target_id + FROM upload_references ur + JOIN uploads on uploads.id = ur.upload_id + WHERE ur.target_type = 'Post' WHERE verification_status = :invalid_etag - ORDER BY post_id DESC + ORDER BY target_id DESC SQL DB.query_single(sql, invalid_etag: Upload.verification_statuses[:invalid_etag]).each do |post_id| diff --git a/lib/topic_upload_security_manager.rb b/lib/topic_upload_security_manager.rb index 896b813d822..edbb28f7dd0 100644 --- a/lib/topic_upload_security_manager.rb +++ b/lib/topic_upload_security_manager.rb @@ -59,7 +59,7 @@ class TopicUploadSecurityManager post.topic = @topic secure_status_did_change = post.uploads.any? do |upload| - first_post_upload_appeared_in = upload.post_uploads.first.post + first_post_upload_appeared_in = upload.upload_references.where(target_type: 'Post').first.target if first_post_upload_appeared_in == post upload.update(access_control_post: post) upload.update_secure_status(source: "topic upload security") @@ -85,8 +85,8 @@ class TopicUploadSecurityManager def posts_with_unowned_uploads Post .where(topic_id: @topic.id) - .joins('INNER JOIN post_uploads ON post_uploads.post_id = posts.id') - .joins('INNER JOIN uploads ON post_uploads.upload_id = uploads.id') + .joins("INNER JOIN upload_references ON upload_references.target_type = 'Post' AND upload_references.target_id = posts.id") + .joins('INNER JOIN uploads ON upload_references.upload_id = uploads.id') .where('uploads.access_control_post_id IS NULL') .includes(:uploads) end diff --git a/spec/jobs/clean_up_uploads_spec.rb b/spec/jobs/clean_up_uploads_spec.rb index 46a55225b51..202b77ff711 100644 --- a/spec/jobs/clean_up_uploads_spec.rb +++ b/spec/jobs/clean_up_uploads_spec.rb @@ -71,7 +71,7 @@ describe Jobs::CleanUpUploads do it 'deletes other uploads not skipped by an unused callback' do expired_upload2 = fabricate_upload upload = fabricate_upload - PostUpload.create(post: Fabricate(:post), upload: upload) + UploadReference.create(target: Fabricate(:post), upload: upload) expect do Jobs::CleanUpUploads.new.execute(nil) @@ -105,7 +105,7 @@ describe Jobs::CleanUpUploads do it 'deletes other uploads that are not in use by callback' do expired_upload2 = fabricate_upload upload = fabricate_upload - PostUpload.create(post: Fabricate(:post), upload: upload) + UploadReference.create(target: Fabricate(:post), upload: upload) expect do Jobs::CleanUpUploads.new.execute(nil) @@ -193,6 +193,10 @@ describe Jobs::CleanUpUploads do end it "does not clean up selectable avatars" do + original_provider = SiteSetting.provider + SiteSetting.provider = SiteSettings::DbProvider.new(SiteSetting) + SiteSetting.clean_orphan_uploads_grace_period_hours = 1 + avatar1_upload = fabricate_upload avatar2_upload = fabricate_upload @@ -203,6 +207,9 @@ describe Jobs::CleanUpUploads do expect(Upload.exists?(id: expired_upload.id)).to eq(false) expect(Upload.exists?(id: avatar1_upload.id)).to eq(true) expect(Upload.exists?(id: avatar2_upload.id)).to eq(true) + ensure + SiteSetting.delete_all + SiteSetting.provider = original_provider end it "does not delete profile background uploads" do @@ -291,12 +298,12 @@ describe Jobs::CleanUpUploads do upload3 = fabricate_upload Fabricate(:reviewable_queued_post_topic, payload: { - raw: "#{upload.sha1}\n#{upload2.short_url}" + raw: "#{upload.short_url}\n#{upload2.short_url}" }) Fabricate(:reviewable_queued_post_topic, payload: { - raw: "#{upload3.sha1}" + raw: "#{upload3.short_url}" }, status: Reviewable.statuses[:rejected] ) @@ -313,7 +320,7 @@ describe Jobs::CleanUpUploads do upload = fabricate_upload upload2 = fabricate_upload - Draft.set(Fabricate(:user), "test", 0, "#{upload.sha1}\n#{upload2.short_url}") + Draft.set(Fabricate(:user), "test", 0, "upload://#{upload.sha1}\n#{upload2.short_url}") Jobs::CleanUpUploads.new.execute(nil) diff --git a/spec/jobs/pull_hotlinked_images_spec.rb b/spec/jobs/pull_hotlinked_images_spec.rb index c7be60aea6d..3e6cc6088b7 100644 --- a/spec/jobs/pull_hotlinked_images_spec.rb +++ b/spec/jobs/pull_hotlinked_images_spec.rb @@ -87,12 +87,12 @@ describe Jobs::PullHotlinkedImages do stub_image_size post.rebake! post.reload - expect(post.post_uploads.count).to eq(1) + expect(post.upload_references.count).to eq(1) post.update(raw: "Post with no images") post.rebake! post.reload - expect(post.post_uploads.count).to eq(0) + expect(post.upload_references.count).to eq(0) end it 'replaces images again after edit' do @@ -375,7 +375,7 @@ describe Jobs::PullHotlinkedImages do post.reload expect(post.cooked).to match(/