Skip to content

Commit

Permalink
DEV: Recover missing files of existing uploads (#10757)
Browse files Browse the repository at this point in the history
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. 🤷‍♂️
  • Loading branch information
CvX committed Oct 1, 2020
1 parent 7d5b18b commit 891987a
Show file tree
Hide file tree
Showing 2 changed files with 79 additions and 5 deletions.
9 changes: 7 additions & 2 deletions lib/upload_recovery.rb
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,13 @@ def recover_post(post)
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
Expand Down Expand Up @@ -141,6 +142,8 @@ def recover_from_s3(sha1:, user_id:)
end
end

upload_exists = Upload.exists?(sha1: sha1)

@object_keys.each do |key|
if key =~ /#{sha1}/
tombstone_prefix = FileStore::S3Store::TOMBSTONE_PREFIX
Expand All @@ -156,6 +159,8 @@ def recover_from_s3(sha1:, user_id:)
)
end

next if upload_exists

url = "https:#{SiteSetting.Upload.absolute_base_url}/#{key}"

begin
Expand Down
75 changes: 72 additions & 3 deletions spec/lib/upload_recovery_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,7 @@

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
Expand Down Expand Up @@ -115,6 +113,77 @@
.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,
Expand Down

0 comments on commit 891987a

Please sign in to comment.