FIX: Validate upload is still valid after calling the "before_upload_creation" event (#13091)

Since we use the event to perform additional validations on the file, we should check if it added any errors to the upload before saving it. This change makes the UploadCreator more consistent since we no longer have to rely on exceptions.

lib/upload_creator.rb

@@ -153,9 +153,10 @@ class UploadCreator
         @upload.assign_attributes(attrs)
       end
 
-      return @upload unless @upload.save(validate: @opts[:validate])
-
-      DiscourseEvent.trigger(:before_upload_creation, @file, is_image, @opts[:for_export])
+      # Callbacks using this event to validate the upload or the file must add errors to the
+      # upload errors object.
+      DiscourseEvent.trigger(:before_upload_creation, @file, is_image, @upload, @opts[:validate])
+      return @upload unless @upload.errors.empty? && @upload.save(validate: @opts[:validate])
 
       # store the file and update its url
       File.open(@file.path) do |f|

spec/lib/upload_creator_spec.rb

@@ -599,4 +599,30 @@ RSpec.describe UploadCreator do
       end
     end
   end
+
+  describe 'before_upload_creation event' do
+    let(:filename) { "logo.jpg" }
+    let(:file) { file_from_fixtures(filename) }
+
+    before do
+      setup_s3
+      stub_s3_store
+    end
+
+    it 'does not save the upload if an event added errors to the upload' do
+      error = 'This upload is invalid'
+
+      event = Proc.new do |file, is_image, upload|
+        upload.errors.add(:base, error)
+      end
+
+      DiscourseEvent.on(:before_upload_creation, &event)
+
+      created_upload = UploadCreator.new(file, filename).create_for(user.id)
+
+      expect(created_upload.persisted?).to eq(false)
+      expect(created_upload.errors).to contain_exactly(error)
+      DiscourseEvent.off(:before_upload_creation, &event)
+    end
+  end
 end