FIX: Clean up not secure uploads with access_control_post_id (#31189)

Until now, we were excluding uploads with a not null
access_control_post_id from cleanup, because we were still
considering them "secure" even though they might have been
`secure: false`.

It's not necessary to keep these around, these are no more
important than regular uploads if they are not referenced
by anything.

app/jobs/scheduled/clean_up_uploads.rb

@@ -28,16 +28,22 @@ module Jobs
 
       result = Upload.by_users
       Upload.unused_callbacks&.each { |handler| result = handler.call(result) }
+
+      # 1. Exclude uploads that have retain_hours set and are still within that retention period.
+      # 2. Exclude uploads created in the grace period.
+      # 3. Exclude secure uploads that have an access_control_post_id.
+      # 4. Exclude uploads that are link to an upload reference.
+      # 5. Exclude uploads that are linked to anything but a Post via UploadReference.
       result =
         result
           .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) # Don't remove any secure uploads.
-          .where("uploads.access_control_post_id IS NULL")
-          .joins(
-            "LEFT JOIN upload_references ON upload_references.upload_id = uploads.id",
-          ) # Don't remove any uploads linked to an UploadReference.
+          .where("uploads.created_at < ?", grace_period.hour.ago)
+          .where(
+            "((uploads.access_control_post_id IS NULL) OR (uploads.access_control_post_id IS NOT NULL AND NOT uploads.secure))",
+          )
+          .joins("LEFT JOIN upload_references ON upload_references.upload_id = uploads.id")
           .where("upload_references.upload_id IS NULL")
           .with_no_non_post_relations
 

spec/jobs/clean_up_uploads_spec.rb

@@ -321,13 +321,22 @@ RSpec.describe Jobs::CleanUpUploads do
     expect(Upload.exists?(id: upload2.id)).to eq(true)
   end
 
-  it "does not delete uploads with an access control post ID (secure uploads)" do
-    upload = fabricate_upload(access_control_post_id: Fabricate(:post).id, secure: true)
+  it "does not delete uploads with an access control post ID that are marked secure" do
+    secure_upload = fabricate_upload(access_control_post_id: Fabricate(:post).id, secure: true)
 
     Jobs::CleanUpUploads.new.execute(nil)
 
     expect(Upload.exists?(id: expired_upload.id)).to eq(false)
-    expect(Upload.exists?(id: upload.id)).to eq(true)
+    expect(Upload.exists?(id: secure_upload.id)).to eq(true)
+  end
+
+  it "does delete uploads with an access control post ID that are not marked secure" do
+    secure_upload = fabricate_upload(access_control_post_id: Fabricate(:post).id, secure: false)
+
+    Jobs::CleanUpUploads.new.execute(nil)
+
+    expect(Upload.exists?(id: expired_upload.id)).to eq(false)
+    expect(Upload.exists?(id: secure_upload.id)).to eq(false)
   end
 
   it "does not delete custom emojis" do