From 9bfc939692044bca7ecf9b3f7abe07efefe031ec Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 19 Oct 2018 12:51:34 +1100 Subject: [PATCH] cleanup so gravatar download failures are consistent previously we would ignore socket error, but this would mean that there could be conditions where we would keep trying to download gravatars forever (in an hourly job) --- app/models/user_avatar.rb | 11 +++-------- spec/models/user_avatar_spec.rb | 4 +++- 2 files changed, 6 insertions(+), 9 deletions(-) diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb index fd3fb8e4bb2..e271d2eab9d 100644 --- a/app/models/user_avatar.rb +++ b/app/models/user_avatar.rb @@ -13,7 +13,7 @@ class UserAvatar < ActiveRecord::Base def update_gravatar! DistributedMutex.synchronize("update_gravatar_#{user_id}") do begin - self.update_columns(last_gravatar_download_attempt: Time.now) + self.update!(last_gravatar_download_attempt: Time.now) max = Discourse.avatar_sizes.max email_hash = user_id == Discourse::SYSTEM_USER_ID ? User.email_hash("info@discourse.org") : user.email_hash @@ -47,17 +47,12 @@ class UserAvatar < ActiveRecord::Base end gravatar_upload&.destroy! - self.gravatar_upload = upload - save! + self.update!(gravatar_upload: upload) end end end - rescue OpenURI::HTTPError - save! - rescue SocketError - # skip saving, we are not connected to the net ensure - tempfile.try(:close!) + tempfile&.close! end end end diff --git a/spec/models/user_avatar_spec.rb b/spec/models/user_avatar_spec.rb index b52303d7b77..b2aac9adebc 100644 --- a/spec/models/user_avatar_spec.rb +++ b/spec/models/user_avatar_spec.rb @@ -74,7 +74,9 @@ describe UserAvatar do FileHelper.expects(:download).raises(SocketError) - expect { avatar.update_gravatar! }.to_not change { Upload.count } + expect do + expect { avatar.update_gravatar! }.to raise_error(SocketError) + end.to_not change { Upload.count } expect(avatar.last_gravatar_download_attempt).to eq(Time.now) end