diff --git a/app/jobs/regular/sync_acls_for_uploads.rb b/app/jobs/regular/sync_acls_for_uploads.rb new file mode 100644 index 00000000000..d3533ff5ab7 --- /dev/null +++ b/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 diff --git a/lib/file_store/s3_store.rb b/lib/file_store/s3_store.rb index fb066051128..951be4f6184 100644 --- a/lib/file_store/s3_store.rb +++ b/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 diff --git a/spec/jobs/sync_acls_for_uploads_spec.rb b/spec/jobs/sync_acls_for_uploads_spec.rb new file mode 100644 index 00000000000..a9f9017c3ed --- /dev/null +++ b/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