There are some situations in which people want to have uploads kept in Discourse that are not strictly associated with existing Discourse models and properties that support uploads, i.e. “whitelisted” orphaned uploads. For example some people want to use the Custom Wizard plugin to upload content associated with a topic (not a post per se). The issue is that the clean_up_uploads job removes all orphaned uploads. It’s exceptions are limited to urls in specific site settings. You could of course turn off clean_up_uploads, or increase the clean_orphan_uploads_grace_period_hours, which is what I’m sometimes forced to do. But this is not ideal. Maybe there’s another way of “whitelisting” orphaned uploads from a plugin that I’m not seeing, but if not, it’d be great to add some mechanism of doing that into the upload cleanup process. I’d be happy to prepare a PR along these lines, but I’m wondering if this is something others see as an issue.

This is so clunky app/jobs/scheduled/clean_up_uploads.rb 32b8a2ccf .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 .joins("LEFT JOIN post_uploads pu ON pu.upload_id = uploads.id") .joins("LEFT JOIN users u ON u.uploaded_avatar_id = uploads.id") .joins("LEFT JOIN user_avatars ua ON ua.gravatar_upload_id = uploads.id OR ua.custom_upload_id = uploads.id") .joins("LEFT JOIN user_profiles up2 ON up2.profile_background = uploads.url OR up2.card_background = uploads.url") .joins("LEFT JOIN user_profiles up ON up.profile_background_upload_id = uploads.id OR up.card_background_upload_id = uploads.id") .joins("LEFT JOIN categories c ON c.uploaded_logo_id = uploads.id OR c.uploaded_background_id = uploads.id") .joins("LEFT JOIN custom_emojis ce ON ce.upload_id = uploads.id") .joins("LEFT JOIN theme_fields tf ON tf.upload_id = uploads.id") .joins("LEFT JOIN user_exports ue ON ue.upload_id = uploads.id") For reviewables @eviltrout introduced a general mechanism where arbitrary objects could be linked to a reviewable. Moving to a similar system for uploads would be like moving multiple mountains. It would be a nice mountain to move though cause this current system is pretty fragile. Easiest workaround you have for now is to create a hidden site setting with a data_type for :upload. Then put it in the site settings table.

[image] sam: Easiest workaround you have for now is to create a hidden site setting with a data_type for :upload . Then put it in the site settings table. Yeah, I was thinking this. I didn’t love it as a solution as I’d need to create different a setting for each upload, which feels like an abuse of the site_settings table. As an interim solution, before the mountain is moved, would you mind if made a PR to add an extra exception to the big “exceptions” query that uses plugin_store_row with a plugin_name like whitelisted_orphan_upload_id?

This query is already SOOOO slow, I worry that another join here is going to make it very painful. I am not sure but maybe we start by preparing the new table in core for UploadRefrence(object_id, object_type, upload_id) and joining into that, at least it is cheap and then we can move stuff to it. There are quite a few easy moves, only post uploads would be tricky..er. I think this is work I would prefer the team here to take, it is very fiddly. Curious to know what @zogstrip and @eviltrout think

[image] sam: For reviewables @eviltrout introduced a general mechanism where arbitrary objects could be linked to a reviewable. Are you talking about Rails’ single table inheritance? It’s a really good fit for reviewables because there is core logic to a reviewable that they all share, but then extra fields. In this case though, it seems like the problem is we’re putting upload ids all over the place, and not deleting the uploads when they’re cleared out? Is it unsafe to delete an upload if a user removes it from their profile, because the same upload sha1 could be used elsewhere in the app? If that’s the case I think @sam’s suggestion of an UploadReference table is a good solution. If uploads are never shared this way, I think a better solution would be to delete when those fields are NULLd out.

[image] eviltrout: Is it unsafe to delete an upload if a user removes it from their profile, because the same upload sha1 could be used elsewhere in the app? If that’s the case I think @sam’s suggestion of an UploadReference table is a good solution. That’s exactly why we need the UploadReference table :+1: [image] sam: I think this is work I would prefer the team here to take, it is very fiddly. I actually think that’s a very good task to learn more about Discourse internals. Might be a good task for @cvx, starting early October. We’ll do some pair programming to get things started and properly split into several “small” tasks.

Hello! I’m very interested in an UploadReference table. We are in a situation where we have a lot of uploads in plugins that would be considered to be orphan uploads. Our solution is simply to turn off the clean_up_uploads. I checked the other day and we now have 10GB of orphan uploads that would need to be removed manually with some script that also figures out if we reference any of them in our plugins. A solution like the one discussed here would help out a lot for us. David

Also, on the topic. We reference some uploads purely with html img tags in some cases. This seems to interrupt the baking of these posts and other thing like hyperlinks don’t get cooked. I see that uploads are generally referenced in a upload://.png type manner. My question is, where can this string be found? It’s not stored on the Upload from what I can see.

[image] piratdavid: My question is, where can this string be found? It’s not stored on the Upload from what I can see. It’s called the short url and you can find how it’s computed in the Upload model app/models/upload.rb 7fc99f5e7 def short_url_basename "#{Upload.base62_sha1(sha1)}#{extension.present? ? ".#{extension}" : ""}" end

Oh thanks! I guess I didn’t look close enough :slight_smile:

Adding a stable way of adding whitelisted "orphaned" uploads in a plugin

Development

fzngagan (Faizaan Gagan) June 15, 2022, 7:33am 12

@angus
We recently merged this PR which adds a upload_references table to address the concerns raised in the OP.
https://github.com/discourse/discourse/pull/16146

Topic		Replies	Views
Cleaning up uploads and purging uploads from S3 Site Management reference , file-management	2	1367	December 24, 2024
Uploads not getting orphaned and purged Product Support	9	1884	April 19, 2024
Auto purge uploads from old deleted posts Feature	6	1465	February 23, 2026
Is there a way to delete old/irrelevant uploads? Product Support	23	6685	February 25, 2020
Post_uploads database table Product Support	1	285	November 24, 2023

Adding a stable way of adding whitelisted "orphaned" uploads in a plugin

Related topics