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
This commit is contained in:
Bianca Nenciu 2022-06-09 02:24:30 +03:00 committed by GitHub
parent 7fc11327b7
commit 9db8f00b3d
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
49 changed files with 842 additions and 142 deletions

View File

@ -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.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.created_at < ?", grace_period.hour.ago)
.where("uploads.access_control_post_id IS NULL") .where("uploads.access_control_post_id IS NULL")
.joins("LEFT JOIN post_uploads pu ON pu.upload_id = uploads.id") .joins("LEFT JOIN upload_references ON upload_references.upload_id = uploads.id")
.where("pu.upload_id IS NULL") .where("upload_references.upload_id IS NULL")
.with_no_non_post_relations .with_no_non_post_relations
result.find_each do |upload| result.find_each do |upload|
next if Upload.in_use_callbacks&.any? { |callback| callback.call(upload) }
if upload.sha1.present? if upload.sha1.present?
# TODO: Remove this check after UploadReferences records were created
encoded_sha = Base62.encode(upload.sha1.hex) 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 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 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 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 upload.destroy
else else
upload.delete upload.delete

View File

@ -108,6 +108,7 @@ class Badge < ActiveRecord::Base
belongs_to :image_upload, class_name: 'Upload' belongs_to :image_upload, class_name: 'Upload'
has_many :user_badges, dependent: :destroy has_many :user_badges, dependent: :destroy
has_many :upload_references, as: :target, dependent: :destroy
validates :name, presence: true, uniqueness: true validates :name, presence: true, uniqueness: true
validates :badge_type, presence: true validates :badge_type, presence: true
@ -119,6 +120,12 @@ class Badge < ActiveRecord::Base
before_create :ensure_not_system before_create :ensure_not_system
before_save :sanitize_description 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 after_commit do
SvgSprite.expire_cache SvgSprite.expire_cache
UserStat.update_distinct_badge_count if saved_change_to_enabled? UserStat.update_distinct_badge_count if saved_change_to_enabled?

View File

@ -45,6 +45,7 @@ class Category < ActiveRecord::Base
has_many :category_groups, dependent: :destroy has_many :category_groups, dependent: :destroy
has_many :groups, through: :category_groups has_many :groups, through: :category_groups
has_many :topic_timers, dependent: :destroy has_many :topic_timers, dependent: :destroy
has_many :upload_references, as: :target, dependent: :destroy
has_and_belongs_to_many :web_hooks has_and_belongs_to_many :web_hooks
@ -80,6 +81,13 @@ class Category < ActiveRecord::Base
after_save :clear_url_cache after_save :clear_url_cache
after_save :update_reviewables 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 :reset_topic_ids_cache
after_destroy :publish_category_deletion after_destroy :publish_category_deletion
after_destroy :remove_site_settings after_destroy :remove_site_settings

View File

@ -3,8 +3,16 @@
class CustomEmoji < ActiveRecord::Base class CustomEmoji < ActiveRecord::Base
belongs_to :upload belongs_to :upload
has_many :upload_references, as: :target, dependent: :destroy
validates :name, presence: true, uniqueness: true validates :name, presence: true, uniqueness: true
validates :upload_id, presence: 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 end
# == Schema Information # == Schema Information

View File

@ -82,7 +82,7 @@ class Draft < ActiveRecord::Base
owner: owner 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) 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) VALUES (:user_id, :draft_key, :data, :sequence, :owner, CURRENT_TIMESTAMP, CURRENT_TIMESTAMP)
ON CONFLICT (user_id, draft_key) DO ON CONFLICT (user_id, draft_key) DO
@ -93,11 +93,18 @@ class Draft < ActiveRecord::Base
revisions = drafts.revisions + 1, revisions = drafts.revisions + 1,
owner = :owner, owner = :owner,
updated_at = CURRENT_TIMESTAMP updated_at = CURRENT_TIMESTAMP
RETURNING id
SQL SQL
UserStat.update_draft_count(user.id) UserStat.update_draft_count(user.id)
end end
UploadReference.ensure_exist!(
upload_ids: Upload.extract_upload_ids(data),
target_type: 'Draft',
target_id: draft_id
)
sequence sequence
end end

View File

@ -36,6 +36,8 @@ class Group < ActiveRecord::Base
has_many :associated_groups, through: :group_associated_groups, dependent: :destroy has_many :associated_groups, through: :group_associated_groups, dependent: :destroy
belongs_to :flair_upload, class_name: 'Upload' 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 :smtp_updated_by, class_name: 'User'
belongs_to :imap_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, after_save :enqueue_update_mentions_job,
if: Proc.new { |g| g.name_before_last_save && g.saved_change_to_name? } 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_save :expire_cache
after_destroy :expire_cache after_destroy :expire_cache

View File

@ -42,8 +42,8 @@ class Post < ActiveRecord::Base
has_many :topic_links has_many :topic_links
has_many :group_mentions, dependent: :destroy has_many :group_mentions, dependent: :destroy
has_many :post_uploads, dependent: :delete_all has_many :upload_references, as: :target, dependent: :destroy
has_many :uploads, through: :post_uploads has_many :uploads, through: :upload_references
has_one :post_stat has_one :post_stat
@ -958,16 +958,19 @@ class Post < ActiveRecord::Base
upload_ids << upload.id if upload.present? upload_ids << upload.id if upload.present?
end end
post_uploads = upload_ids.map do |upload_id| upload_references = upload_ids.map do |upload_id|
{ post_id: self.id, upload_id: 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 end
PostUpload.transaction do UploadReference.transaction do
PostUpload.where(post_id: self.id).delete_all UploadReference.where(target: self).delete_all
UploadReference.insert_all(upload_references) if upload_references.size > 0
if post_uploads.size > 0
PostUpload.insert_all(post_uploads)
end
if SiteSetting.secure_media? if SiteSetting.secure_media?
Upload.where( Upload.where(
@ -1067,7 +1070,11 @@ class Post < ActiveRecord::Base
query.find_in_batches do |posts| query.find_in_batches do |posts|
ids = posts.pluck(:id) 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| posts.each do |post|
post.each_upload_url do |src, path, sha1| post.each_upload_url do |src, path, sha1|

View File

@ -7,6 +7,13 @@ class ReviewableQueuedPost < Reviewable
DiscourseEvent.trigger(:queued_post_created, self) DiscourseEvent.trigger(:queued_post_created, self)
end 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] after_commit :compute_user_stats, only: %i[create update]
def build_actions(actions, guardian, args) def build_actions(actions, guardian, args)

View File

@ -4,9 +4,22 @@ class SiteSetting < ActiveRecord::Base
extend GlobalPath extend GlobalPath
extend SiteSettingExtension extend SiteSettingExtension
has_many :upload_references, as: :target, dependent: :destroy
validates_presence_of :name validates_presence_of :name
validates_presence_of :data_type 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) def self.load_settings(file, plugin: nil)
SiteSettings::YamlLoader.new(file).load do |category, name, default, opts| SiteSettings::YamlLoader.new(file).load do |category, name, default, opts|
setting(name, default, opts.merge(category: category, plugin: plugin)) setting(name, default, opts.merge(category: category, plugin: plugin))

View File

@ -4,6 +4,13 @@ class ThemeField < ActiveRecord::Base
belongs_to :upload belongs_to :upload
has_one :javascript_cache, dependent: :destroy 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) { scope :find_by_theme_ids, ->(theme_ids) {
return none unless theme_ids.present? return none unless theme_ids.present?

View File

@ -3,6 +3,8 @@
class ThemeSetting < ActiveRecord::Base class ThemeSetting < ActiveRecord::Base
belongs_to :theme belongs_to :theme
has_many :upload_references, as: :target, dependent: :destroy
validates_presence_of :name, :theme validates_presence_of :name, :theme
validates :data_type, numericality: { only_integer: true } validates :data_type, numericality: { only_integer: true }
validates :name, length: { maximum: 255 } validates :name, length: { maximum: 255 }
@ -10,6 +12,12 @@ class ThemeSetting < ActiveRecord::Base
after_save :clear_settings_cache after_save :clear_settings_cache
after_destroy :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 def clear_settings_cache
# All necessary caches will be cleared on next ensure_baked! # All necessary caches will be cleared on next ensure_baked!
theme.settings_field&.invalidate_baked! theme.settings_field&.invalidate_baked!

View File

@ -20,13 +20,11 @@ class Upload < ActiveRecord::Base
Post.unscoped { super } Post.unscoped { super }
end 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 :post_hotlinked_media, dependent: :destroy, class_name: "PostHotlinkedMedia"
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 :upload_references, dependent: :destroy
has_many :posts, through: :upload_references, source: :target, source_type: 'Post'
has_many :topic_thumbnails has_many :topic_thumbnails
attr_accessor :for_group_message attr_accessor :for_group_message
@ -87,43 +85,9 @@ class Upload < ActiveRecord::Base
end end
def self.with_no_non_post_relations def self.with_no_non_post_relations
scope = self self
.joins(<<~SQL) .joins("LEFT JOIN upload_references ur ON ur.upload_id = uploads.id AND ur.target_type != 'Post'")
LEFT JOIN site_settings ss .where("ur.upload_id IS NULL")
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
end end
def to_s def to_s
@ -542,6 +506,22 @@ class Upload < ActiveRecord::Base
problems problems
end 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 private
def short_url_basename def short_url_basename

View File

@ -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)
#

View File

@ -23,6 +23,7 @@ class User < ActiveRecord::Base
has_many :email_tokens, dependent: :destroy has_many :email_tokens, dependent: :destroy
has_many :topic_links, dependent: :destroy has_many :topic_links, dependent: :destroy
has_many :user_uploads, 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_emails, dependent: :destroy, autosave: true
has_many :user_associated_accounts, dependent: :destroy has_many :user_associated_accounts, dependent: :destroy
has_many :oauth2_user_infos, dependent: :destroy has_many :oauth2_user_infos, dependent: :destroy
@ -150,6 +151,12 @@ class User < ActiveRecord::Base
after_save :index_search after_save :index_search
after_save :check_site_contact_username 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_created_event, on: :create
after_commit :trigger_user_destroyed_event, on: :destroy after_commit :trigger_user_destroyed_event, on: :destroy

View File

@ -4,6 +4,14 @@ class UserAvatar < ActiveRecord::Base
belongs_to :user belongs_to :user
belongs_to :gravatar_upload, class_name: 'Upload' belongs_to :gravatar_upload, class_name: 'Upload'
belongs_to :custom_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 = { @@custom_user_gravatar_email_hash = {
Discourse::SYSTEM_USER_ID => User.email_hash("info@discourse.org") Discourse::SYSTEM_USER_ID => User.email_hash("info@discourse.org")

View File

@ -5,6 +5,14 @@ class UserExport < ActiveRecord::Base
belongs_to :upload, dependent: :destroy belongs_to :upload, dependent: :destroy
belongs_to :topic, 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 DESTROY_CREATED_BEFORE = 2.days.ago
def self.remove_old_exports def self.remove_old_exports

View File

@ -6,6 +6,7 @@ class UserProfile < ActiveRecord::Base
belongs_to :profile_background_upload, class_name: "Upload" belongs_to :profile_background_upload, class_name: "Upload"
belongs_to :granted_title_badge, class_name: "Badge" belongs_to :granted_title_badge, class_name: "Badge"
belongs_to :featured_topic, class_name: 'Topic' 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 :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? } 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 :trigger_badges
after_save :pull_hotlinked_image 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? } validate :website_domain_validator, if: Proc.new { |c| c.new_record? || c.website_changed? }
has_many :user_profile_views, dependent: :destroy has_many :user_profile_views, dependent: :destroy

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -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

View File

@ -153,7 +153,7 @@ module BackupRestore
DB.exec(<<~SQL) DB.exec(<<~SQL)
UPDATE posts UPDATE posts
SET baked_version = NULL 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 SQL
end end
end end

View File

@ -695,9 +695,9 @@ class Search
UNION UNION
SELECT post_uploads.post_id SELECT upload_references.target_id
FROM uploads 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) WHERE lower(uploads.extension) IN (:file_extensions)
)", file_extensions: file_extensions) )", file_extensions: file_extensions)
end end

View File

@ -19,7 +19,7 @@ class ShrinkUploadedImage
return false return false
end 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? if posts.empty?
log "Upload not used in any posts" log "Upload not used in any posts"
@ -134,7 +134,10 @@ class ShrinkUploadedImage
if existing_upload if existing_upload
begin 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 rescue ActiveRecord::RecordNotUnique, PG::UniqueViolation
end end
else else

View File

@ -662,7 +662,10 @@ def correct_inline_uploads
dry_run = (ENV["DRY_RUN"].nil? ? true : ENV["DRY_RUN"] != "false") dry_run = (ENV["DRY_RUN"].nil? ? true : ENV["DRY_RUN"] != "false")
verbose = ENV["VERBOSE"] 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) .where(<<~SQL)
raw LIKE '%/uploads/#{RailsMultisite::ConnectionManagement.current_db}/original/%' raw LIKE '%/uploads/#{RailsMultisite::ConnectionManagement.current_db}/original/%'
SQL SQL

View File

@ -529,7 +529,7 @@ task "uploads:disable_secure_media" => :environment do
SiteSetting.secure_media = false 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_count = secure_uploads.count
secure_upload_ids = secure_uploads.pluck(:id) 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( 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) adjust_acls(secure_upload_ids)
post_rebake_errors = rebake_upload_posts(post_ids_to_rebake) 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) post_upload_ids_marked_secure = DB.query_single(<<~SQL)
WITH upl AS ( WITH upl AS (
SELECT DISTINCT ON (upload_id) upload_id SELECT DISTINCT ON (upload_id) upload_id
FROM post_uploads FROM upload_references
INNER JOIN posts ON posts.id = post_uploads.post_id 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 INNER JOIN topics ON topics.id = posts.topic_id
) )
UPDATE uploads UPDATE uploads
@ -646,7 +646,7 @@ def mark_all_as_secure_login_required
puts "Finished marking upload(s) as secure." puts "Finished marking upload(s) as secure."
post_ids_to_rebake = DB.query_single( 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] [post_ids_to_rebake, (post_upload_ids_marked_secure + upload_ids_marked_not_secure).uniq]
end end
@ -665,8 +665,8 @@ def update_specific_upload_security_no_login_required
post_upload_ids_marked_secure = DB.query_single(<<~SQL) post_upload_ids_marked_secure = DB.query_single(<<~SQL)
WITH upl AS ( WITH upl AS (
SELECT DISTINCT ON (upload_id) upload_id SELECT DISTINCT ON (upload_id) upload_id
FROM post_uploads FROM upload_references
INNER JOIN posts ON posts.id = post_uploads.post_id 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 INNER JOIN topics ON topics.id = posts.topic_id
LEFT JOIN categories ON categories.id = topics.category_id LEFT JOIN categories ON categories.id = topics.category_id
WHERE (topics.category_id IS NOT NULL AND categories.read_restricted) OR 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) post_upload_ids_marked_not_secure = DB.query_single(<<~SQL)
WITH upl AS ( WITH upl AS (
SELECT DISTINCT ON (upload_id) upload_id SELECT DISTINCT ON (upload_id) upload_id
FROM post_uploads FROM upload_references
INNER JOIN posts ON posts.id = post_uploads.post_id 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 INNER JOIN topics ON topics.id = posts.topic_id
LEFT JOIN categories ON categories.id = topics.category_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 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." 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 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] [post_ids_to_rebake, all_upload_ids_changed]
end end
def update_uploads_access_control_post def update_uploads_access_control_post
DB.exec(<<~SQL) DB.exec(<<~SQL)
WITH upl AS ( 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 UPDATE uploads
SET access_control_post_id = upl.post_id SET access_control_post_id = upl.post_id
@ -855,9 +858,9 @@ def analyze_missing_s3
puts "List of posts with missing images:" puts "List of posts with missing images:"
sql = <<~SQL sql = <<~SQL
SELECT post_id, url, sha1, extension, uploads.id SELECT post_id, url, sha1, extension, uploads.id
FROM post_uploads pu FROM upload_references ur
RIGHT JOIN uploads on uploads.id = pu.upload_id RIGHT JOIN uploads on uploads.id = ur.upload_id
WHERE verification_status = :invalid_etag WHERE ur.target_type = 'Post' AND verification_status = :invalid_etag
ORDER BY created_at ORDER BY created_at
SQL SQL
@ -867,9 +870,9 @@ def analyze_missing_s3
DB.query(sql, invalid_etag: Upload.verification_statuses[:invalid_etag]).each do |r| DB.query(sql, invalid_etag: Upload.verification_statuses[:invalid_etag]).each do |r|
all << r all << r
if r.post_id if r.target_id
lookup[r.post_id] ||= [] lookup[r.target_id] ||= []
lookup[r.post_id] << [r.url, r.sha1, r.extension] lookup[r.target_id] << [r.url, r.sha1, r.extension]
else else
other << r other << r
end end
@ -894,7 +897,7 @@ def analyze_missing_s3
ids = all.map { |r| r.id } ids = all.map { |r| r.id }
lookups = [ lookups = [
[:post_uploads, :upload_id], [:upload_references, :upload_id],
[:users, :uploaded_avatar_id], [:users, :uploaded_avatar_id],
[:user_avatars, :gravatar_upload_id], [:user_avatars, :gravatar_upload_id],
[:user_avatars, :custom_upload_id], [:user_avatars, :custom_upload_id],
@ -1026,7 +1029,7 @@ def fix_missing_s3
puts "Failed to save upload #{save_error}" puts "Failed to save upload #{save_error}"
else else
OptimizedImage.where(upload_id: upload.id).destroy_all 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? if rebake_ids.present?
Post.where(id: rebake_ids).each do |post| 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" puts "Rebaking posts with missing uploads, this can take a while as all rebaking runs inline"
sql = <<~SQL sql = <<~SQL
SELECT post_id SELECT target_id
FROM post_uploads pu FROM upload_references ur
JOIN uploads on uploads.id = pu.upload_id JOIN uploads on uploads.id = ur.upload_id
WHERE ur.target_type = 'Post'
WHERE verification_status = :invalid_etag WHERE verification_status = :invalid_etag
ORDER BY post_id DESC ORDER BY target_id DESC
SQL SQL
DB.query_single(sql, invalid_etag: Upload.verification_statuses[:invalid_etag]).each do |post_id| DB.query_single(sql, invalid_etag: Upload.verification_statuses[:invalid_etag]).each do |post_id|

View File

@ -59,7 +59,7 @@ class TopicUploadSecurityManager
post.topic = @topic post.topic = @topic
secure_status_did_change = post.uploads.any? do |upload| 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 if first_post_upload_appeared_in == post
upload.update(access_control_post: post) upload.update(access_control_post: post)
upload.update_secure_status(source: "topic upload security") upload.update_secure_status(source: "topic upload security")
@ -85,8 +85,8 @@ class TopicUploadSecurityManager
def posts_with_unowned_uploads def posts_with_unowned_uploads
Post Post
.where(topic_id: @topic.id) .where(topic_id: @topic.id)
.joins('INNER JOIN post_uploads ON post_uploads.post_id = posts.id') .joins("INNER JOIN upload_references ON upload_references.target_type = 'Post' AND upload_references.target_id = posts.id")
.joins('INNER JOIN uploads ON post_uploads.upload_id = uploads.id') .joins('INNER JOIN uploads ON upload_references.upload_id = uploads.id')
.where('uploads.access_control_post_id IS NULL') .where('uploads.access_control_post_id IS NULL')
.includes(:uploads) .includes(:uploads)
end end

View File

@ -71,7 +71,7 @@ describe Jobs::CleanUpUploads do
it 'deletes other uploads not skipped by an unused callback' do it 'deletes other uploads not skipped by an unused callback' do
expired_upload2 = fabricate_upload expired_upload2 = fabricate_upload
upload = fabricate_upload upload = fabricate_upload
PostUpload.create(post: Fabricate(:post), upload: upload) UploadReference.create(target: Fabricate(:post), upload: upload)
expect do expect do
Jobs::CleanUpUploads.new.execute(nil) 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 it 'deletes other uploads that are not in use by callback' do
expired_upload2 = fabricate_upload expired_upload2 = fabricate_upload
upload = fabricate_upload upload = fabricate_upload
PostUpload.create(post: Fabricate(:post), upload: upload) UploadReference.create(target: Fabricate(:post), upload: upload)
expect do expect do
Jobs::CleanUpUploads.new.execute(nil) Jobs::CleanUpUploads.new.execute(nil)
@ -193,6 +193,10 @@ describe Jobs::CleanUpUploads do
end end
it "does not clean up selectable avatars" do 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 avatar1_upload = fabricate_upload
avatar2_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: expired_upload.id)).to eq(false)
expect(Upload.exists?(id: avatar1_upload.id)).to eq(true) expect(Upload.exists?(id: avatar1_upload.id)).to eq(true)
expect(Upload.exists?(id: avatar2_upload.id)).to eq(true) expect(Upload.exists?(id: avatar2_upload.id)).to eq(true)
ensure
SiteSetting.delete_all
SiteSetting.provider = original_provider
end end
it "does not delete profile background uploads" do it "does not delete profile background uploads" do
@ -291,12 +298,12 @@ describe Jobs::CleanUpUploads do
upload3 = fabricate_upload upload3 = fabricate_upload
Fabricate(:reviewable_queued_post_topic, payload: { 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, Fabricate(:reviewable_queued_post_topic,
payload: { payload: {
raw: "#{upload3.sha1}" raw: "#{upload3.short_url}"
}, },
status: Reviewable.statuses[:rejected] status: Reviewable.statuses[:rejected]
) )
@ -313,7 +320,7 @@ describe Jobs::CleanUpUploads do
upload = fabricate_upload upload = fabricate_upload
upload2 = 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) Jobs::CleanUpUploads.new.execute(nil)

View File

@ -87,12 +87,12 @@ describe Jobs::PullHotlinkedImages do
stub_image_size stub_image_size
post.rebake! post.rebake!
post.reload 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.update(raw: "Post with no images")
post.rebake! post.rebake!
post.reload post.reload
expect(post.post_uploads.count).to eq(0) expect(post.upload_references.count).to eq(0)
end end
it 'replaces images again after edit' do it 'replaces images again after edit' do
@ -375,7 +375,7 @@ describe Jobs::PullHotlinkedImages do
post.reload post.reload
expect(post.cooked).to match(/<img src=.*\/uploads/) expect(post.cooked).to match(/<img src=.*\/uploads/)
expect(post.post_uploads.count).to eq(1) expect(post.upload_references.count).to eq(1)
end end
it 'associates uploads correctly' do it 'associates uploads correctly' do
@ -384,13 +384,13 @@ describe Jobs::PullHotlinkedImages do
post.rebake! post.rebake!
post.reload post.reload
expect(post.post_uploads.count).to eq(1) expect(post.upload_references.count).to eq(1)
post.update(raw: "no onebox") post.update(raw: "no onebox")
post.rebake! post.rebake!
post.reload post.reload
expect(post.post_uploads.count).to eq(0) expect(post.upload_references.count).to eq(0)
end end
it 'all combinations' do it 'all combinations' do

View File

@ -6,8 +6,8 @@ describe Jobs::UpdatePostUploadsSecureStatus do
fab!(:post) { Fabricate(:post) } fab!(:post) { Fabricate(:post) }
before do before do
PostUpload.create!(post: post, upload: Fabricate(:upload)) UploadReference.create!(target: post, upload: Fabricate(:upload))
PostUpload.create!(post: post, upload: Fabricate(:upload)) UploadReference.create!(target: post, upload: Fabricate(:upload))
end end
context "when secure uploads is enabled" do context "when secure uploads is enabled" do
@ -23,14 +23,14 @@ describe Jobs::UpdatePostUploadsSecureStatus do
it "updates all the uploads to secure" do it "updates all the uploads to secure" do
described_class.new.execute(post_id: post.id) described_class.new.execute(post_id: post.id)
post.reload post.reload
expect(post.post_uploads.map(&:upload).map(&:secure).all?(true)).to eq(true) expect(post.upload_references.map(&:upload).map(&:secure).all?(true)).to eq(true)
end end
it "updates all the uploads to secure even if their extension is not authorized" do it "updates all the uploads to secure even if their extension is not authorized" do
SiteSetting.authorized_extensions = "" SiteSetting.authorized_extensions = ""
described_class.new.execute(post_id: post.id) described_class.new.execute(post_id: post.id)
post.reload post.reload
expect(post.post_uploads.map(&:upload).map(&:secure).all?(true)).to eq(true) expect(post.upload_references.map(&:upload).map(&:secure).all?(true)).to eq(true)
end end
end end
end end

View File

@ -23,7 +23,7 @@ describe CookedPostProcessor do
cpp.expects(:optimize_urls).in_sequence(post_process) cpp.expects(:optimize_urls).in_sequence(post_process)
cpp.post_process cpp.post_process
expect(PostUpload.exists?(post: post, upload: upload)).to eq(true) expect(UploadReference.exists?(target: post, upload: upload)).to eq(true)
end end
describe 'when post contains oneboxes and inline oneboxes' do describe 'when post contains oneboxes and inline oneboxes' do

View File

@ -1064,7 +1064,7 @@ describe PostDestroyer do
fab!(:reply) { Fabricate(:private_message_post, topic: private_message_topic) } fab!(:reply) { Fabricate(:private_message_post, topic: private_message_topic) }
fab!(:post_revision) { Fabricate(:post_revision, post: private_post) } fab!(:post_revision) { Fabricate(:post_revision, post: private_post) }
fab!(:upload1) { Fabricate(:upload_s3, created_at: 5.hours.ago) } fab!(:upload1) { Fabricate(:upload_s3, created_at: 5.hours.ago) }
fab!(:post_upload) { PostUpload.create(post: private_post, upload: upload1) } fab!(:upload_reference) { UploadReference.create(target: private_post, upload: upload1) }
it "destroys the post and topic if deleting first post" do it "destroys the post and topic if deleting first post" do
PostDestroyer.new(reply.user, reply, permanent: true).destroy PostDestroyer.new(reply.user, reply, permanent: true).destroy
@ -1076,7 +1076,7 @@ describe PostDestroyer do
expect { private_message_topic.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { private_message_topic.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect { post_action.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { post_action.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect { post_revision.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { post_revision.reload }.to raise_error(ActiveRecord::RecordNotFound)
expect { post_upload.reload }.to raise_error(ActiveRecord::RecordNotFound) expect { upload_reference.reload }.to raise_error(ActiveRecord::RecordNotFound)
Jobs::CleanUpUploads.new.reset_last_cleanup! Jobs::CleanUpUploads.new.reset_last_cleanup!
SiteSetting.clean_orphan_uploads_grace_period_hours = 1 SiteSetting.clean_orphan_uploads_grace_period_hours = 1

View File

@ -1189,7 +1189,7 @@ describe PostRevisor do
it "updates linked post uploads" do it "updates linked post uploads" do
post.link_post_uploads post.link_post_uploads
expect(post.post_uploads.pluck(:upload_id)).to contain_exactly(image1.id, image2.id) expect(post.upload_references.pluck(:upload_id)).to contain_exactly(image1.id, image2.id)
subject.revise!(user, raw: <<~RAW) subject.revise!(user, raw: <<~RAW)
This is a post with multiple uploads This is a post with multiple uploads
@ -1198,7 +1198,7 @@ describe PostRevisor do
![image4](#{image4.short_url}) ![image4](#{image4.short_url})
RAW RAW
expect(post.reload.post_uploads.pluck(:upload_id)).to contain_exactly(image2.id, image3.id, image4.id) expect(post.reload.upload_references.pluck(:upload_id)).to contain_exactly(image2.id, image3.id, image4.id)
end end
context "secure media uploads" do context "secure media uploads" do

View File

@ -18,8 +18,8 @@ describe TopicUploadSecurityManager do
let!(:upload3) { Fabricate(:secure_upload) } let!(:upload3) { Fabricate(:secure_upload) }
before do before do
PostUpload.create(upload: upload, post: post2) UploadReference.create(upload: upload, target: post2)
PostUpload.create(upload: upload2, post: post3) UploadReference.create(upload: upload2, target: post3)
upload.update(access_control_post: post2) upload.update(access_control_post: post2)
upload2.update(access_control_post: post3) upload2.update(access_control_post: post3)
end end
@ -132,7 +132,7 @@ describe TopicUploadSecurityManager do
context "when this is the first post the upload has appeared in" do context "when this is the first post the upload has appeared in" do
before do before do
PostUpload.create(upload: upload3, post: post4) UploadReference.create(upload: upload3, target: post4)
end end
it "changes the upload secure status to true and changes the ACL and rebakes the post and sets the access control post" do it "changes the upload secure status to true and changes the ACL and rebakes the post and sets the access control post" do
@ -157,8 +157,8 @@ describe TopicUploadSecurityManager do
context "when this is not the first post the upload has appeared in" do context "when this is not the first post the upload has appeared in" do
before do before do
PostUpload.create(upload: upload3, post: Fabricate(:post)) UploadReference.create(upload: upload3, target: Fabricate(:post))
PostUpload.create(upload: upload3, post: post4) UploadReference.create(upload: upload3, target: post4)
end end
it "does not change the upload secure status and does not set the access control post" do it "does not change the upload secure status and does not set the access control post" do

View File

@ -1485,17 +1485,17 @@ describe Post do
post.link_post_uploads post.link_post_uploads
post.trash! post.trash!
expect(PostUpload.count).to eq(6) expect(UploadReference.count).to eq(6)
post.destroy! post.destroy!
expect(PostUpload.count).to eq(0) expect(UploadReference.count).to eq(0)
end end
context "#link_post_uploads" do context "#link_post_uploads" do
it "finds all the uploads in the post" do it "finds all the uploads in the post" do
post.link_post_uploads post.link_post_uploads
expect(PostUpload.where(post: post).pluck(:upload_id)).to contain_exactly( expect(UploadReference.where(target: post).pluck(:upload_id)).to contain_exactly(
video_upload.id, video_upload.id,
image_upload.id, image_upload.id,
audio_upload.id, audio_upload.id,
@ -1508,13 +1508,11 @@ describe Post do
it "cleans the reverse index up for the current post" do it "cleans the reverse index up for the current post" do
post.link_post_uploads post.link_post_uploads
post_uploads_ids = post.post_uploads.pluck(:id) post_uploads_ids = post.upload_references.pluck(:id)
post.link_post_uploads post.link_post_uploads
expect(post.reload.post_uploads.pluck(:id)).to_not contain_exactly( expect(post.reload.upload_references.pluck(:id)).to_not contain_exactly(post_uploads_ids)
post_uploads_ids
)
end end
context "when secure media is enabled" do context "when secure media is enabled" do
@ -1578,7 +1576,7 @@ describe Post do
post.link_post_uploads post.link_post_uploads
post.update_uploads_secure_status(source: "test") post.update_uploads_secure_status(source: "test")
expect(PostUpload.where(post: post).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly( expect(UploadReference.where(target: post).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly(
[attachment_upload.id, true], [attachment_upload.id, true],
[image_upload.id, true] [image_upload.id, true]
) )
@ -1590,7 +1588,7 @@ describe Post do
post.link_post_uploads post.link_post_uploads
post.update_uploads_secure_status(source: "test") post.update_uploads_secure_status(source: "test")
expect(PostUpload.where(post: post).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly( expect(UploadReference.where(target: post).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly(
[attachment_upload.id, false], [attachment_upload.id, false],
[image_upload.id, false] [image_upload.id, false]
) )
@ -1603,7 +1601,7 @@ describe Post do
post.link_post_uploads post.link_post_uploads
post.update_uploads_secure_status(source: "test") post.update_uploads_secure_status(source: "test")
expect(PostUpload.where(post: post).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly( expect(UploadReference.where(target: post).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly(
[attachment_upload.id, true], [attachment_upload.id, true],
[image_upload.id, true] [image_upload.id, true]
) )
@ -1618,7 +1616,7 @@ describe Post do
pm.link_post_uploads pm.link_post_uploads
pm.update_uploads_secure_status(source: "test") pm.update_uploads_secure_status(source: "test")
expect(PostUpload.where(post: pm).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly( expect(UploadReference.where(target: pm).joins(:upload).pluck(:upload_id, :secure)).to contain_exactly(
[attachment_upload.id, false], [attachment_upload.id, false],
[image_upload.id, false] [image_upload.id, false]
) )

View File

@ -1,8 +0,0 @@
# frozen_string_literal: true
describe PostUpload do
it { is_expected.to belong_to :post }
it { is_expected.to belong_to :upload }
end

View File

@ -0,0 +1,239 @@
# frozen_string_literal: true
describe UploadReference do
context 'badge uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload references' do
badge = nil
expect { badge = Fabricate(:badge, image_upload_id: upload.id) }
.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(badge)
expect { badge.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'category uploads' do
fab!(:upload1) { Fabricate(:upload) }
fab!(:upload2) { Fabricate(:upload) }
it 'creates upload references' do
category = nil
expect { category = Fabricate(:category, uploaded_logo_id: upload1.id, uploaded_background_id: upload2.id) }
.to change { UploadReference.count }.by(2)
upload_reference = UploadReference.last
expect(upload_reference.target).to eq(category)
expect { category.destroy! }
.to change { UploadReference.count }.by(-2)
end
end
context 'custom emoji uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload references' do
custom_emoji = nil
expect { custom_emoji = CustomEmoji.create!(name: 'emoji', upload_id: upload.id) }
.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.target).to eq(custom_emoji)
expect { custom_emoji.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'group uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload references' do
group = nil
expect { group = Fabricate(:group, flair_upload_id: upload.id) }
.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(group)
expect { group.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'post uploads' do
fab!(:upload) { Fabricate(:upload) }
fab!(:post) { Fabricate(:post, raw: "[](#{upload.short_url})") }
it 'creates upload references' do
expect { post.link_post_uploads }
.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(post)
expect { post.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'site setting uploads' do
let(:provider) { SiteSettings::DbProvider.new(SiteSetting) }
fab!(:upload) { Fabricate(:upload) }
fab!(:upload2) { Fabricate(:upload) }
it 'creates upload references for uploads' do
expect { provider.save('logo', upload.id, SiteSettings::TypeSupervisor.types[:upload]) }
.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(SiteSetting.find_by(name: 'logo'))
expect { provider.destroy('logo') }
.to change { UploadReference.count }.by(-1)
end
it 'creates upload references for uploaded_image_lists' do
expect { provider.save('selectable_avatars', "#{upload.id}|#{upload2.id}", SiteSettings::TypeSupervisor.types[:uploaded_image_list]) }
.to change { UploadReference.count }.by(2)
upload_references = UploadReference.all.where(target: SiteSetting.find_by(name: 'selectable_avatars'))
expect(upload_references.pluck(:upload_id)).to contain_exactly(upload.id, upload2.id)
expect { provider.destroy('selectable_avatars') }
.to change { UploadReference.count }.by(-2)
end
end
context 'theme field uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload refererences' do
theme_field = nil
expect do
theme_field = ThemeField.create!(
theme_id: Fabricate(:theme).id,
target_id: 0,
name: 'field',
value: '',
upload: upload,
type_id: ThemeField.types[:theme_upload_var],
)
end.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(theme_field)
expect { theme_field.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'theme setting uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload refererences' do
theme_setting = nil
expect do
theme_setting = ThemeSetting.create!(
name: 'field',
data_type: ThemeSetting.types[:upload],
value: upload.id,
theme_id: Fabricate(:theme).id,
)
end.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(theme_setting)
expect { theme_setting.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'user uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload references' do
user = nil
expect { user = Fabricate(:user, uploaded_avatar_id: upload.id) }
.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(user)
expect { user.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'user avatar uploads' do
fab!(:upload1) { Fabricate(:upload) }
fab!(:upload2) { Fabricate(:upload) }
it 'creates upload references' do
user_avatar = nil
expect { user_avatar = Fabricate(:user_avatar, custom_upload_id: upload1.id, gravatar_upload_id: upload2.id) }
.to change { UploadReference.count }.by(2)
upload_reference = UploadReference.last
expect(upload_reference.target).to eq(user_avatar)
expect { user_avatar.destroy! }
.to change { UploadReference.count }.by(-2)
end
end
context 'user export uploads' do
fab!(:upload) { Fabricate(:upload) }
it 'creates upload references' do
user_export = nil
expect do
user_export = UserExport.create!(
file_name: 'export',
user: Fabricate(:user),
upload: upload,
topic: Fabricate(:topic),
)
end.to change { UploadReference.count }.by(1)
upload_reference = UploadReference.last
expect(upload_reference.upload).to eq(upload)
expect(upload_reference.target).to eq(user_export)
expect { user_export.destroy! }
.to change { UploadReference.count }.by(-1)
end
end
context 'user profile uploads' do
fab!(:user) { Fabricate(:user) }
fab!(:upload1) { Fabricate(:upload) }
fab!(:upload2) { Fabricate(:upload) }
it 'creates upload references' do
user_profile = user.user_profile
expect { user_profile.update!(profile_background_upload_id: upload1.id, card_background_upload_id: upload2.id) }
.to change { UploadReference.count }.by(2)
upload_reference = UploadReference.last
expect(upload_reference.target).to eq(user_profile)
expect { user_profile.destroy! }
.to change { UploadReference.count }.by(-2)
end
end
end

View File

@ -517,6 +517,25 @@ describe Upload do
end end
end end
describe '.extract_upload_ids' do
let(:upload) { Fabricate(:upload) }
it 'works with short URLs' do
ids = Upload.extract_upload_ids("This URL #{upload.short_url} is an upload")
expect(ids).to contain_exactly(upload.id)
end
it 'works with SHA1s' do
ids = Upload.extract_upload_ids("This URL /#{upload.sha1} is an upload")
expect(ids).to contain_exactly(upload.id)
end
it 'works with Base62 hashes' do
ids = Upload.extract_upload_ids("This URL /#{Upload.base62_sha1(upload.sha1)} is an upload")
expect(ids).to contain_exactly(upload.id)
end
end
def enable_secure_media def enable_secure_media
setup_s3 setup_s3
SiteSetting.secure_media = true SiteSetting.secure_media = true

View File

@ -26,11 +26,11 @@ RSpec.describe "tasks/uploads" do
let!(:post3) { Fabricate(:post) } let!(:post3) { Fabricate(:post) }
before do before do
PostUpload.create(post: post1, upload: multi_post_upload1) UploadReference.create(target: post1, upload: multi_post_upload1)
PostUpload.create(post: post2, upload: multi_post_upload1) UploadReference.create(target: post2, upload: multi_post_upload1)
PostUpload.create(post: post2, upload: upload1) UploadReference.create(target: post2, upload: upload1)
PostUpload.create(post: post3, upload: upload2) UploadReference.create(target: post3, upload: upload2)
PostUpload.create(post: post3, upload: upload3) UploadReference.create(target: post3, upload: upload3)
end end
def invoke_task def invoke_task
@ -141,7 +141,7 @@ RSpec.describe "tasks/uploads" do
it "changes the upload to not secure and updates the ACL" do it "changes the upload to not secure and updates the ACL" do
upload_to_mark_not_secure = Fabricate(:upload_s3, secure: true) upload_to_mark_not_secure = Fabricate(:upload_s3, secure: true)
post_for_upload = Fabricate(:post) post_for_upload = Fabricate(:post)
PostUpload.create(post: post_for_upload, upload: upload_to_mark_not_secure) UploadReference.create(target: post_for_upload, upload: upload_to_mark_not_secure)
setup_s3 setup_s3
uploads.each { |upload| stub_upload(upload) } uploads.each { |upload| stub_upload(upload) }
@ -167,10 +167,10 @@ RSpec.describe "tasks/uploads" do
uploads.each { |upload| stub_upload(upload) } uploads.each { |upload| stub_upload(upload) }
SiteSetting.secure_media = true SiteSetting.secure_media = true
PostUpload.create(post: post1, upload: upload1) UploadReference.create(target: post1, upload: upload1)
PostUpload.create(post: post1, upload: upload2) UploadReference.create(target: post1, upload: upload2)
PostUpload.create(post: post2, upload: upload3) UploadReference.create(target: post2, upload: upload3)
PostUpload.create(post: post2, upload: upload4) UploadReference.create(target: post2, upload: upload4)
end end
let!(:uploads) do let!(:uploads) do