From 521226f4c9dc88d9398438194d3c3f61e673b4f3 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?R=C3=A9gis=20Hanol?= <regis@hanol.fr>
Date: Mon, 15 Dec 2014 22:10:27 +0100
Subject: [PATCH] FIX: registration fails with timeout on gravatar

---
 app/jobs/regular/update_gravatar.rb | 18 ++++++++++++++++++
 app/models/user.rb                  |  8 +-------
 app/models/user_avatar.rb           |  2 --
 spec/jobs/update_gravatar_spec.rb   | 21 +++++++++++++++++++++
 spec/models/user_spec.rb            | 27 +++++----------------------
 spec/spec_helper.rb                 |  3 +--
 6 files changed, 46 insertions(+), 33 deletions(-)
 create mode 100644 app/jobs/regular/update_gravatar.rb
 create mode 100644 spec/jobs/update_gravatar_spec.rb

diff --git a/app/jobs/regular/update_gravatar.rb b/app/jobs/regular/update_gravatar.rb
new file mode 100644
index 000000000..bd8ef1402
--- /dev/null
+++ b/app/jobs/regular/update_gravatar.rb
@@ -0,0 +1,18 @@
+module Jobs
+
+  class UpdateGravatar < Jobs::Base
+
+    def execute(args)
+      user = User.find_by(id: args[:user_id])
+      avatar = UserAvatar.find_by(id: args[:avatar_id])
+
+      if user && avatar
+        avatar.update_gravatar!
+        if !user.uploaded_avatar_id && avatar.gravatar_upload_id
+          user.update_column(:uploaded_avatar_id, avatar.gravatar_upload_id)
+        end
+      end
+    end
+  end
+
+end
diff --git a/app/models/user.rb b/app/models/user.rb
index 7ac2fbc68..18d23b435 100644
--- a/app/models/user.rb
+++ b/app/models/user.rb
@@ -638,15 +638,9 @@ class User < ActiveRecord::Base
     return if @import_mode
 
     avatar = user_avatar || create_user_avatar
-    gravatar_downloaded = false
 
     if SiteSetting.automatically_download_gravatars? && !avatar.last_gravatar_download_attempt
-      avatar.update_gravatar!
-      gravatar_downloaded = avatar.gravatar_upload_id
-    end
-
-    if !self.uploaded_avatar_id && gravatar_downloaded
-      self.update_column(:uploaded_avatar_id, avatar.gravatar_upload_id)
+      Jobs.enqueue(:update_gravatar, user_id: self.id, avatar_id: avatar.id)
     end
   end
 
diff --git a/app/models/user_avatar.rb b/app/models/user_avatar.rb
index aacdf1c14..6516457b1 100644
--- a/app/models/user_avatar.rb
+++ b/app/models/user_avatar.rb
@@ -1,8 +1,6 @@
 require_dependency 'letter_avatar'
 
 class UserAvatar < ActiveRecord::Base
-  MAX_SIZE = 240
-
   belongs_to :user
   belongs_to :gravatar_upload, class_name: 'Upload', dependent: :destroy
   belongs_to :custom_upload, class_name: 'Upload', dependent: :destroy
diff --git a/spec/jobs/update_gravatar_spec.rb b/spec/jobs/update_gravatar_spec.rb
new file mode 100644
index 000000000..883716445
--- /dev/null
+++ b/spec/jobs/update_gravatar_spec.rb
@@ -0,0 +1,21 @@
+require 'spec_helper'
+
+describe Jobs::UpdateGravatar do
+
+  it "picks gravatar if system avatar is picked and gravatar was just downloaded" do
+    user = User.create!(username: "bob", name: "bob", email: "a@a.com")
+    user.uploaded_avatar_id.should == nil
+    user.user_avatar.gravatar_upload_id.should == nil
+
+    png = Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==")
+    FakeWeb.register_uri(:get, "http://www.gravatar.com/avatar/d10ca8d11301c2f4993ac2279ce4b930.png?s=500&d=404", body: png)
+
+    SiteSetting.automatically_download_gravatars = true
+
+    user.refresh_avatar
+    user.reload
+
+    user.uploaded_avatar_id.should == user.user_avatar.gravatar_upload_id
+  end
+
+end
diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb
index ce009f1cb..6d5878f4a 100644
--- a/spec/models/user_spec.rb
+++ b/spec/models/user_spec.rb
@@ -1094,7 +1094,6 @@ describe User do
 
   describe "automatic avatar creation" do
     it "sets a system avatar for new users" do
-      SiteSetting.enable_system_avatars = true
       u = User.create!(username: "bob", email: "bob@bob.com")
       u.reload
       u.uploaded_avatar_id.should == nil
@@ -1128,30 +1127,14 @@ describe User do
   end
 
   describe "refresh_avatar" do
-    it "picks gravatar if system avatar is picked and gravatar was just downloaded" do
-
-      png = Base64.decode64("R0lGODlhAQABALMAAAAAAIAAAACAAICAAAAAgIAAgACAgMDAwICAgP8AAAD/AP//AAAA//8A/wD//wBiZCH5BAEAAA8ALAAAAAABAAEAAAQC8EUAOw==")
-      FakeWeb.register_uri( :get,
-                            "http://www.gravatar.com/avatar/d10ca8d11301c2f4993ac2279ce4b930.png?s=500&d=404",
-                             body: png )
-
-      user = User.create!(username: "bob", name: "bob", email: "a@a.com")
-      user.reload
-
+    it "enqueues the update_gravatar job when automatically downloading gravatars" do
       SiteSetting.automatically_download_gravatars = true
-      SiteSetting.enable_system_avatars = true
+
+      user = Fabricate(:user)
+
+      Jobs.expects(:enqueue).with(:update_gravatar, anything)
 
       user.refresh_avatar
-      user.reload
-
-      user.user_avatar.gravatar_upload_id.should == user.uploaded_avatar_id
-
-      user.uploaded_avatar_id = nil
-      user.save
-      user.refresh_avatar
-
-      user.reload
-      user.uploaded_avatar_id.should == nil
     end
   end
 
diff --git a/spec/spec_helper.rb b/spec/spec_helper.rb
index 19093ec81..3510c0940 100644
--- a/spec/spec_helper.rb
+++ b/spec/spec_helper.rb
@@ -35,8 +35,8 @@ Spork.prefork do
   # let's not run seed_fu every test
   SeedFu.quiet = true if SeedFu.respond_to? :quiet
 
-  SiteSetting.enable_system_avatars = false
   SiteSetting.automatically_download_gravatars = false
+
   SeedFu.seed
 
   RSpec.configure do |config|
@@ -90,7 +90,6 @@ Spork.prefork do
       end
 
       # very expensive IO operations
-      SiteSetting.enable_system_avatars = false
       SiteSetting.automatically_download_gravatars = false
 
       I18n.locale = :en