FEATURE: Add a sidekiq job for syncing S3 ACLs (#16449)

Sometimes we need to update a _lot_ of ACLs on S3 (such as when secure media
is enabled), and since it takes ~1s per upload to update the ACL, this is
best spread out over many jobs instead of having to do the whole thing serially.

In future, it will be better to have a job that can be run based on
a column on uploads (e.g. acl_stale) so we can track progress, similar
to how we can set the baked_version to nil to rebake posts.

app/jobs/regular/sync_acls_for_uploads.rb

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+module Jobs
+  # Sometimes we need to update a _lot_ of ACLs on S3 (such as when secure media
+  # is enabled), and since it takes ~1s per upload to update the ACL, this is
+  # best spread out over many jobs instead of having to do the whole thing serially.
+  class SyncAclsForUploads < ::Jobs::Base
+    def execute(args)
+      return if !Discourse.store.external?
+      return if !args.key?(:upload_ids)
+
+      # TODO (martin) Change the logging here to debug after acl_stale implemented.
+      #
+      # Note...these log messages are set to warn to ensure this is working
+      # as intended in initial production trials, this will be set to debug
+      # after an acl_stale column is added to uploads.
+      time = Benchmark.measure do
+        Rails.logger.warn("Syncing ACL for upload ids: #{args[:upload_ids].join(", ")}")
+        Upload.includes(:optimized_images).where(id: args[:upload_ids]).find_in_batches do |uploads|
+          uploads.each do |upload|
+            Discourse.store.update_upload_ACL(upload, optimized_images_preloaded: true)
+          end
+        end
+        Rails.logger.warn("Completed syncing ACL for upload ids in #{time.to_s}. IDs: #{args[:upload_ids].join(", ")}")
+      end
+    end
+  end
+end

lib/file_store/s3_store.rb

@@ -279,13 +279,24 @@ module FileStore
       end
     end
 
-    def update_upload_ACL(upload)
+    def update_upload_ACL(upload, optimized_images_preloaded: false)
       key = get_upload_key(upload)
       update_ACL(key, upload.secure?)
 
-      upload.optimized_images.find_each do |optimized_image|
-        optimized_image_key = get_path_for_optimized_image(optimized_image)
-        update_ACL(optimized_image_key, upload.secure?)
+      # if we do find_each when the images have already been preloaded with
+      # includes(:optimized_images), then the optimized_images are fetched
+      # from the database again, negating the preloading if this operation
+      # is done on a large amount of uploads at once (see Jobs::SyncAclsForUploads)
+      if optimized_images_preloaded
+        upload.optimized_images.each do |optimized_image|
+          optimized_image_key = get_path_for_optimized_image(optimized_image)
+          update_ACL(optimized_image_key, upload.secure?)
+        end
+      else
+        upload.optimized_images.find_each do |optimized_image|
+          optimized_image_key = get_path_for_optimized_image(optimized_image)
+          update_ACL(optimized_image_key, upload.secure?)
+        end
       end
 
       true

spec/jobs/sync_acls_for_uploads_spec.rb

@@ -0,0 +1,31 @@
+# frozen_string_literal: true
+
+require 'rails_helper'
+
+describe Jobs::SyncAclsForUploads do
+  let(:upload1) { Fabricate(:upload) }
+  let(:upload2) { Fabricate(:upload) }
+  let(:upload3) { Fabricate(:secure_upload) }
+  let(:upload_ids) { [upload1.id, upload2.id, upload3.id] }
+
+  def run_job
+    described_class.new.execute(upload_ids: upload_ids)
+  end
+
+  it "does nothing if not using external storage" do
+    Upload.expects(:where).never
+    run_job
+  end
+
+  context "external storage enabled" do
+    before do
+      setup_s3
+      stub_s3_store
+    end
+
+    it "runs update_upload_ACL for each upload" do
+      Discourse.store.expects(:update_upload_ACL).times(3)
+      run_job
+    end
+  end
+end