FIX: Do not attempt S3 ACL call if secure status did not change (#24785)

When rebaking and in various other places for posts, we
run through the uploads and call `update_secure_status` on
each of them.

However, if the secure status didn't change, we were still
calling S3 to change the ACL, which would have been a noop
in many cases and takes ~1 second per call, slowing things
down a lot.

Also, we didn't account for the s3_acls_enabled site setting
being false here, and in the specs doing an assertion
that `Discourse.store.update_ACL` is not called doesn't
work; `Discourse.store` isn't a singleton, it re-initializes
`FileStore::S3Store.new` every single time.

app/models/upload.rb

@@ -470,7 +470,7 @@ class Upload < ActiveRecord::Base
     secure_status_did_change = self.secure? != mark_secure
     self.update(secure_params(mark_secure, reason, source))
 
-    if Discourse.store.external?
+    if secure_status_did_change && SiteSetting.s3_use_acls && Discourse.store.external?
       begin
         Discourse.store.update_upload_ACL(self)
       rescue Aws::S3::Errors::NotImplemented => err

spec/models/upload_spec.rb

@@ -444,6 +444,7 @@ RSpec.describe Upload do
       end
 
       it "marks the upload as not secure if its access control post is a public post" do
+        FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload)
         upload.update!(secure: true, access_control_post: Fabricate(:post))
         upload.update_secure_status
         expect(upload.secure).to eq(false)
@@ -455,6 +456,21 @@ RSpec.describe Upload do
         expect(upload.secure).to eq(true)
       end
 
+      it "does not attempt to change the ACL if the secure status has not changed" do
+        FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload).never
+        upload.update!(secure: true, access_control_post: Fabricate(:private_message_post))
+        upload.update_secure_status
+      end
+
+      it "does not attempt to change the ACL if s3_use_acls is disabled" do
+        SiteSetting.secure_uploads = false
+        SiteSetting.s3_use_acls = false
+        FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload).never
+        upload.update!(secure: true, access_control_post: Fabricate(:post))
+        upload.update_secure_status
+        expect(upload.secure).to eq(false)
+      end
+
       it "marks an image upload as secure if login_required is enabled" do
         SiteSetting.login_required = true
         upload.update!(secure: false)

spec/tasks/uploads_spec.rb

@@ -146,7 +146,7 @@ RSpec.describe "tasks/uploads" do
           end
 
           it "does not attempt to update the acl" do
-            Discourse.store.expects(:update_upload_ACL).with(upload2).never
+            FileStore::S3Store.any_instance.expects(:update_upload_ACL).with(upload2).never
             invoke_task
           end
         end