From e1975e293f2625259e925b4a3c93d88d5acfcaa8 Mon Sep 17 00:00:00 2001
From: Sam <sam.saffron@gmail.com>
Date: Fri, 31 Aug 2018 14:46:22 +1000
Subject: [PATCH] FIX: when uploads are destroyed clear up avatar refs in user
 table

This also auto corrects twice daily when we ensure consistency
---
 app/jobs/scheduled/ensure_db_consistency.rb |  2 ++
 app/models/upload.rb                        |  6 ++++
 app/models/user.rb                          | 14 +++++++++
 app/models/user_avatar.rb                   | 25 +++++++++++++++
 spec/models/user_avatar_spec.rb             | 34 +++++++++++++++++++++
 spec/models/user_spec.rb                    | 20 ++++++++++++
 6 files changed, 101 insertions(+)

diff --git a/app/jobs/scheduled/ensure_db_consistency.rb b/app/jobs/scheduled/ensure_db_consistency.rb
index 03060c5d3eb..4f0b6d6ce0b 100644
--- a/app/jobs/scheduled/ensure_db_consistency.rb
+++ b/app/jobs/scheduled/ensure_db_consistency.rb
@@ -17,6 +17,8 @@ module Jobs
       UserOption.ensure_consistency!
       Tag.ensure_consistency!
       CategoryTagStat.ensure_consistency!
+      User.ensure_consistency!
+      UserAvatar.ensure_consistency!
     end
   end
 end
diff --git a/app/models/upload.rb b/app/models/upload.rb
index 25d7ecc4722..4ab07e4ce1b 100644
--- a/app/models/upload.rb
+++ b/app/models/upload.rb
@@ -24,6 +24,12 @@ class Upload < ActiveRecord::Base
 
   validates_with ::Validators::UploadValidator
 
+  after_destroy do
+    User.where(uploaded_avatar_id: self.id).update_all(uploaded_avatar_id: nil)
+    UserAvatar.where(gravatar_upload_id: self.id).update_all(gravatar_upload_id: nil)
+    UserAvatar.where(custom_upload_id: self.id).update_all(custom_upload_id: nil)
+  end
+
   def thumbnail(width = self.thumbnail_width, height = self.thumbnail_height)
     optimized_images.find_by(width: width, height: height)
   end
diff --git a/app/models/user.rb b/app/models/user.rb
index 08ec67d06c0..94e7bbf7b16 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -1283,6 +1283,20 @@ class User < ActiveRecord::Base
     true
   end
 
+  def self.ensure_consistency!
+    DB.exec <<~SQL
+      UPDATE users
+      SET uploaded_avatar_id = NULL
+      WHERE uploaded_avatar_id IN (
+        SELECT u1.uploaded_avatar_id FROM users u1
+        LEFT JOIN uploads up
+          ON u1.uploaded_avatar_id = up.id
+        WHERE u1.uploaded_avatar_id IS NOT NULL AND
+          up.id IS NULL
+      )
+    SQL
+  end
+
 end
 
 # == Schema Information
diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb
index afc0c1dc0ed..69d0ff7764a 100644
--- a/app/models/user_avatar.rb
+++ b/app/models/user_avatar.rb
@@ -123,6 +123,31 @@ class UserAvatar < ActiveRecord::Base
     tempfile.close! if tempfile && tempfile.respond_to?(:close!)
   end
 
+  def self.ensure_consistency!
+    DB.exec <<~SQL
+      UPDATE user_avatars
+      SET gravatar_upload_id = NULL
+      WHERE gravatar_upload_id IN (
+        SELECT u1.gravatar_upload_id FROM user_avatars u1
+        LEFT JOIN uploads up
+          ON u1.gravatar_upload_id = up.id
+        WHERE u1.gravatar_upload_id IS NOT NULL AND
+          up.id IS NULL
+      )
+    SQL
+
+    DB.exec <<~SQL
+      UPDATE user_avatars
+      SET custom_upload_id = NULL
+      WHERE custom_upload_id IN (
+        SELECT u1.custom_upload_id FROM user_avatars u1
+        LEFT JOIN uploads up
+          ON u1.custom_upload_id = up.id
+        WHERE u1.custom_upload_id IS NOT NULL AND
+          up.id IS NULL
+      )
+    SQL
+  end
 end
 
 # == Schema Information
diff --git a/spec/models/user_avatar_spec.rb b/spec/models/user_avatar_spec.rb
index f67cd4896cc..5e9af7a5ea7 100644
--- a/spec/models/user_avatar_spec.rb
+++ b/spec/models/user_avatar_spec.rb
@@ -118,4 +118,38 @@ describe UserAvatar do
       end
     end
   end
+
+  describe "ensure_consistency!" do
+
+    it "will clean up dangling avatars" do
+      upload1 = Fabricate(:upload)
+      upload2 = Fabricate(:upload)
+
+      user_avatar = Fabricate(:user).user_avatar
+
+      user_avatar.update_columns(
+        gravatar_upload_id: upload1.id,
+        custom_upload_id: upload2.id
+      )
+
+      upload1.destroy!
+      upload2.destroy!
+
+      user_avatar.reload
+      expect(user_avatar.gravatar_upload_id).to eq(nil)
+      expect(user_avatar.custom_upload_id).to eq(nil)
+
+      user_avatar.update_columns(
+        gravatar_upload_id: upload1.id,
+        custom_upload_id: upload2.id
+      )
+
+      UserAvatar.ensure_consistency!
+
+      user_avatar.reload
+      expect(user_avatar.gravatar_upload_id).to eq(nil)
+      expect(user_avatar.custom_upload_id).to eq(nil)
+    end
+
+  end
 end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index 0fa4bd5126f..948a315d529 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1794,4 +1794,24 @@ describe User do
     end
   end
 
+  describe "ensure_consistency!" do
+
+    it "will clean up dangling avatars" do
+      upload = Fabricate(:upload)
+      user = Fabricate(:user, uploaded_avatar_id: upload.id)
+
+      upload.destroy!
+      user.reload
+      expect(user.uploaded_avatar_id).to eq(nil)
+
+      user.update_columns(uploaded_avatar_id: upload.id)
+
+      User.ensure_consistency!
+
+      user.reload
+      expect(user.uploaded_avatar_id).to eq(nil)
+    end
+
+  end
+
 end