FIX: download avatar from facebook/twitter in a job in order to prevent hangs when avatars are huge

app/jobs/regular/download_avatar_from_url.rb

@@ -0,0 +1,20 @@
+module Jobs
+
+  class DownloadAvatarFromUrl < Jobs::Base
+    sidekiq_options retry: false
+
+    def execute(args)
+      url = args[:url]
+      user_id = args[:user_id]
+
+      raise Discourse::InvalidParameters.new(:url) if url.blank?
+      raise Discourse::InvalidParameters.new(:user_id) if user_id.blank?
+
+      return unless user = User.find_by(id: user_id)
+
+      UserAvatar.import_url_for_user(url, user, override_gravatar: args[:override_gravatar])
+    end
+
+  end
+
+end

lib/auth/facebook_authenticator.rb

@@ -7,7 +7,6 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
   end
 
   def after_authenticate(auth_token)
-
     result = Auth::Result.new
 
     session_info = parse_auth_token(auth_token)
@@ -20,37 +19,16 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
     result.extra_data = facebook_hash
 
     user_info = FacebookUserInfo.find_by(facebook_user_id: facebook_hash[:facebook_user_id])
+
     result.user = user_info.try(:user)
-
     if !result.user && !email.blank? && result.user = User.find_by_email(email)
-      FacebookUserInfo.create({user_id: result.user.id}.merge(facebook_hash))
+      FacebookUserInfo.create({ user_id: result.user.id }.merge(facebook_hash))
     end
 
-    if user_info
-      user_info.update_columns(facebook_hash)
-    end
+    user_info.update_columns(facebook_hash) if user_info
 
-    user = result.user
-    if user && (!user.user_avatar || user.user_avatar.custom_upload_id.nil?)
-      if (avatar_url = facebook_hash[:avatar_url]).present?
-        avatar_url_with_parameters = add_avatar_parameters(avatar_url)
-        UserAvatar.import_url_for_user(avatar_url_with_parameters, user, override_gravatar: false)
-      end
-    end
-
-
-    bio = facebook_hash[:about_me] || facebook_hash[:about]
-    location = facebook_hash[:location]
-    website = facebook_hash[:website]
-
-    if user && (bio || location || website)
-      profile = user.user_profile
-
-      profile.bio_raw = bio unless profile.bio_raw.present?
-      profile.location = location unless profile.location.present?
-      profile.website = website unless profile.website.present?
-      profile.save
-    end
+    retrieve_avatar(result.user, result.extra_data)
+    retrieve_profile(result.user, result.extra_data)
 
     if email.blank?
       UserHistory.create(
@@ -63,32 +41,16 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
   end
 
   def after_create_account(user, auth)
-    data = auth[:extra_data]
-    FacebookUserInfo.create({user_id: user.id}.merge(data))
+    extra_data = auth[:extra_data]
+    FacebookUserInfo.create({ user_id: user.id }.merge(extra_data))
 
-
-    if (avatar_url = data[:avatar_url]).present?
-      avatar_url_with_parameters = add_avatar_parameters(avatar_url)
-      UserAvatar.import_url_for_user(avatar_url_with_parameters, user)
-      user.save
-    end
-
-    bio = data[:about_me]
-    location = data[:location]
-    website = data[:website]
-
-    if bio || location || website
-      user.user_profile.bio_raw = bio
-      user.user_profile.location = location
-      user.user_profile.website = website
-      user.user_profile.save
-    end
+    retrieve_avatar(user, extra_data)
+    retrieve_profile(user, extra_data)
 
     true
   end
 
   def register_middleware(omniauth)
-
     omniauth.provider :facebook,
            :setup => lambda { |env|
               strategy = env["omniauth.strategy"]
@@ -104,38 +66,58 @@ class Auth::FacebookAuthenticator < Auth::Authenticator
 
   protected
 
-  def parse_auth_token(auth_token)
+    def parse_auth_token(auth_token)
+      raw_info = auth_token["extra"]["raw_info"]
+      info = auth_token["info"]
 
-    raw_info = auth_token["extra"]["raw_info"]
-    info = auth_token["info"]
+      email = auth_token["info"][:email]
 
-    email = auth_token["info"][:email]
+      website = (info["urls"] && info["urls"]["Website"]) || nil
 
-    website = (info["urls"] && info["urls"]["Website"]) || nil
-
-    {
-      facebook: {
-        facebook_user_id: auth_token["uid"],
-        link: raw_info["link"],
-        username: raw_info["username"],
-        first_name: raw_info["first_name"],
-        last_name: raw_info["last_name"],
+      {
+        facebook: {
+          facebook_user_id: auth_token["uid"],
+          link: raw_info["link"],
+          username: raw_info["username"],
+          first_name: raw_info["first_name"],
+          last_name: raw_info["last_name"],
+          email: email,
+          gender: raw_info["gender"],
+          name: raw_info["name"],
+          avatar_url: info["image"],
+          location: info["location"],
+          website: website,
+          about_me: info["description"]
+        },
         email: email,
-        gender: raw_info["gender"],
-        name: raw_info["name"],
-        avatar_url: info["image"],
-        location: info["location"],
-        website: website,
-        about_me: info["description"]
-      },
-      email: email,
-      email_valid: true
-    }
+        email_valid: true
+      }
+    end
 
-  end
+    def retrieve_avatar(user, data)
+      return unless user
+      return if user.user_avatar.try(:custom_upload_id).present?
 
-  def add_avatar_parameters(avatar_url)
-    "#{avatar_url}?height=#{AVATAR_SIZE}&width=#{AVATAR_SIZE}"
-  end
+      if (avatar_url = data[:avatar_url]).present?
+        url = "#{avatar_url}?height=#{AVATAR_SIZE}&width=#{AVATAR_SIZE}"
+        Jobs.enqueue(:download_avatar_from_url, url: url, user_id: user.id, override_gravatar: false)
+      end
+    end
+
+    def retrieve_profile(user, data)
+      return unless user
+
+      bio = data[:about_me] || data[:about]
+      location = data[:location]
+      website = data[:website]
+
+      if bio || location || website
+        profile = user.user_profile
+        profile.bio_raw  = bio      unless profile.bio_raw.present?
+        profile.location = location unless profile.location.present?
+        profile.website  = website  unless profile.website.present?
+        profile.save
+      end
+    end
 
 end

lib/auth/twitter_authenticator.rb

@@ -51,6 +51,8 @@ class Auth::TwitterAuthenticator < Auth::Authenticator
 
     retrieve_avatar(user, extra_data)
     retrieve_profile(user, extra_data)
+
+    true
   end
 
   def register_middleware(omniauth)
@@ -62,27 +64,30 @@ class Auth::TwitterAuthenticator < Auth::Authenticator
            }
   end
 
-  def retrieve_avatar(user, data)
-    return unless user
-    return if user.user_avatar.try(:custom_upload_id).present?
+  protected
 
-    if (avatar_url = data[:twitter_image]).present?
-      UserAvatar.import_url_for_user(avatar_url.sub("_normal", ""), user, override_gravatar: false)
+    def retrieve_avatar(user, data)
+      return unless user
+      return if user.user_avatar.try(:custom_upload_id).present?
+
+      if (avatar_url = data[:twitter_image]).present?
+        url = avatar_url.sub("_normal", "")
+        Jobs.enqueue(:download_avatar_from_url, url: url, user_id: user.id, override_gravatar: false)
+      end
     end
-  end
 
-  def retrieve_profile(user, data)
-    return unless user
+    def retrieve_profile(user, data)
+      return unless user
 
-    bio = data[:twitter_description]
-    location = data[:twitter_location]
+      bio = data[:twitter_description]
+      location = data[:twitter_location]
 
-    if user && (bio || location)
-      profile = user.user_profile
-      profile.bio_raw  = bio      unless profile.bio_raw.present?
-      profile.location = location unless profile.location.present?
-      profile.save
+      if bio || location
+        profile = user.user_profile
+        profile.bio_raw  = bio      unless profile.bio_raw.present?
+        profile.location = location unless profile.location.present?
+        profile.save
+      end
     end
-  end
 
 end