From bc7f58191e8f01e3a5127befe09373bc2fda4458 Mon Sep 17 00:00:00 2001
From: Guo Xiang Tan <tgx_world@hotmail.com>
Date: Wed, 19 Sep 2018 11:52:57 +0800
Subject: [PATCH] FIX: `UploadRecovery` should look at links too.

---
 lib/upload_recovery.rb           |  4 ++--
 spec/lib/upload_recovery_spec.rb | 28 ++++++++++++++++++++++++----
 2 files changed, 26 insertions(+), 6 deletions(-)

diff --git a/lib/upload_recovery.rb b/lib/upload_recovery.rb
index 75b6e3e0421..cc3056bf596 100644
--- a/lib/upload_recovery.rb
+++ b/lib/upload_recovery.rb
@@ -4,7 +4,7 @@ class UploadRecovery
   end
 
   def recover(posts = Post)
-    posts.where("raw LIKE '%upload:\/\/%'").find_each do |post|
+    posts.where("raw LIKE '%upload:\/\/%' OR raw LIKE '%href=%'").find_each do |post|
       begin
         analyzer = PostAnalyzer.new(post.raw, post.topic_id)
 
@@ -28,7 +28,7 @@ class UploadRecovery
           elsif media.name == "a"
             href = media["href"]
 
-            if data = Upload.extract_upload_url(href)
+            if href && data = Upload.extract_upload_url(href)
               sha1 = data[2]
 
               unless upload = Upload.get_from_url(href)
diff --git a/spec/lib/upload_recovery_spec.rb b/spec/lib/upload_recovery_spec.rb
index b28e04c192e..c69ed088a62 100644
--- a/spec/lib/upload_recovery_spec.rb
+++ b/spec/lib/upload_recovery_spec.rb
@@ -22,7 +22,6 @@ RSpec.describe UploadRecovery do
     Fabricate(:post,
       raw: <<~SQL,
       ![logo.png](#{upload.short_url})
-      <a class="attachment" href="#{upload2.url}">some.pdf</a>
       SQL
       user: user
     ).tap(&:link_post_uploads)
@@ -67,18 +66,39 @@ RSpec.describe UploadRecovery do
       upload_recovery.recover(Post.where("updated_at >= ?", 1.day.ago))
     end
 
+    describe 'for a missing attachment' do
+      let(:post) do
+        Fabricate(:post,
+          raw: <<~SQL,
+          <a class="attachment" href="#{upload2.url}">some.pdf</a>
+          <a>blank</a>
+          SQL
+          user: user
+        ).tap(&:link_post_uploads)
+      end
+
+      it 'should recover the attachment' do
+        expect do
+          upload2.destroy!
+        end.to change { post.reload.uploads.count }.from(1).to(0)
+
+        expect do
+          upload_recovery.recover
+        end.to change { post.reload.uploads.count }.from(0).to(1)
+      end
+    end
+
     it 'should recover uploads and attachments' do
       stub_request(:get, "http://test.localhost#{upload.url}")
         .to_return(status: 200)
 
       expect do
         upload.destroy!
-        upload2.destroy!
-      end.to change { post.reload.uploads.count }.from(2).to(0)
+      end.to change { post.reload.uploads.count }.from(1).to(0)
 
       expect do
         upload_recovery.recover
-      end.to change { post.reload.uploads.count }.from(0).to(2)
+      end.to change { post.reload.uploads.count }.from(0).to(1)
     end
   end
 end