From 891987a2843de3ebeed412e01fd6b8064dac3917 Mon Sep 17 00:00:00 2001
From: Jarek Radosz <jradosz@gmail.com>
Date: Thu, 1 Oct 2020 14:54:45 +0200
Subject: [PATCH] DEV: Recover missing files of existing uploads (#10757)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

UploadRecovery only worked on missing Upload records. Now it also works with existing ones that have an invalid_etag status.

The specs (first that test the S3 path) are a bit of stub-a-palooza, but that's how much this class interacts with the the outside world. 🤷‍♂️
---
 lib/upload_recovery.rb           |  9 +++-
 spec/lib/upload_recovery_spec.rb | 75 ++++++++++++++++++++++++++++++--
 2 files changed, 79 insertions(+), 5 deletions(-)

diff --git a/lib/upload_recovery.rb b/lib/upload_recovery.rb
index 221e1d3e957..9092ee03f96 100644
--- a/lib/upload_recovery.rb
+++ b/lib/upload_recovery.rb
@@ -31,12 +31,13 @@ class UploadRecovery
           data = Upload.extract_url(url)
           next unless data
 
-          sha1 = data[2]
+          upload = Upload.get_from_url(url)
 
-          unless upload = Upload.get_from_url(url)
+          if !upload || upload.verification_status == Upload.verification_statuses[:invalid_etag]
             if @dry_run
               puts "#{post.full_url} #{url}"
             else
+              sha1 = data[2]
               recover_post_upload(post, sha1)
             end
           end
@@ -141,6 +142,8 @@ class UploadRecovery
       end
     end
 
+    upload_exists = Upload.exists?(sha1: sha1)
+
     @object_keys.each do |key|
       if key =~ /#{sha1}/
         tombstone_prefix = FileStore::S3Store::TOMBSTONE_PREFIX
@@ -156,6 +159,8 @@ class UploadRecovery
           )
         end
 
+        next if upload_exists
+
         url = "https:#{SiteSetting.Upload.absolute_base_url}/#{key}"
 
         begin
diff --git a/spec/lib/upload_recovery_spec.rb b/spec/lib/upload_recovery_spec.rb
index 9a18a34dbd6..4ab612b33f0 100644
--- a/spec/lib/upload_recovery_spec.rb
+++ b/spec/lib/upload_recovery_spec.rb
@@ -21,9 +21,7 @@ RSpec.describe UploadRecovery do
 
   let(:post) do
     Fabricate(:post,
-      raw: <<~SQL,
-      ![logo.png](#{upload.short_url})
-      SQL
+      raw: "![logo.png](#{upload.short_url})",
       user: user
     ).tap(&:link_post_uploads)
   end
@@ -115,6 +113,77 @@ RSpec.describe UploadRecovery do
         .to eq(File.read(file_from_fixtures("smallest.png")))
     end
 
+    context 'S3 store' do
+      before do
+        setup_s3
+        stub_s3_store
+      end
+
+      it 'recovers the upload' do
+        expect do
+          upload.destroy!
+        end.to change { post.reload.uploads.count }.from(1).to(0)
+
+        original_key = Discourse.store.get_path_for_upload(upload)
+        tombstone_key = original_key.sub("original", "tombstone/original")
+
+        tombstone_copy = stub
+        tombstone_copy.expects(:key).returns(tombstone_key)
+
+        Discourse.store.s3_helper.expects(:list).with("original").returns([])
+        Discourse.store.s3_helper.expects(:list).with("#{FileStore::S3Store::TOMBSTONE_PREFIX}original").returns([tombstone_copy])
+        Discourse.store.s3_helper.expects(:copy).with(tombstone_key, original_key, options: { acl: "public-read" })
+
+        FileHelper.expects(:download).returns(file_from_fixtures("smallest.png"))
+        stub_request(:get, upload.url).to_return(body: file_from_fixtures("smallest.png"))
+
+        expect do
+          upload_recovery.recover
+        end.to change { post.reload.uploads.count }.from(0).to(1)
+      end
+
+      describe 'when the upload exists but its file is missing' do
+        it 'recovers the file' do
+          upload.verification_status = Upload.verification_statuses[:invalid_etag]
+          upload.save!
+
+          original_key = Discourse.store.get_path_for_upload(upload)
+          tombstone_key = original_key.sub("original", "tombstone/original")
+
+          tombstone_copy = stub
+          tombstone_copy.expects(:key).returns(tombstone_key)
+
+          Discourse.store.s3_helper.expects(:list).with("original").returns([])
+          Discourse.store.s3_helper.expects(:list).with("#{FileStore::S3Store::TOMBSTONE_PREFIX}original").returns([tombstone_copy])
+          Discourse.store.s3_helper.expects(:copy).with(tombstone_key, original_key, options: { acl: "public-read" })
+
+          expect do
+            upload_recovery.recover
+          end.to_not change { [post.reload.uploads.count, Upload.count] }
+        end
+
+        it 'does not create a duplicate upload when secure uploads are enabled' do
+          SiteSetting.secure_media = true
+          upload.verification_status = Upload.verification_statuses[:invalid_etag]
+          upload.save!
+
+          original_key = Discourse.store.get_path_for_upload(upload)
+          tombstone_key = original_key.sub("original", "tombstone/original")
+
+          tombstone_copy = stub
+          tombstone_copy.expects(:key).returns(tombstone_key)
+
+          Discourse.store.s3_helper.expects(:list).with("original").returns([])
+          Discourse.store.s3_helper.expects(:list).with("#{FileStore::S3Store::TOMBSTONE_PREFIX}original").returns([tombstone_copy])
+          Discourse.store.s3_helper.expects(:copy).with(tombstone_key, original_key, options: { acl: "public-read" })
+
+          expect do
+            upload_recovery.recover
+          end.to_not change { [post.reload.uploads.count, Upload.count] }
+        end
+      end
+    end
+
     describe 'image tag' do
       let(:post) do
         Fabricate(:post,