Fix #1141, fix #1126 - Avatar/profile info fetching (#1215)

* Fix #1141, fix #1126 - Work through UpdateRemoteProfileService for both <feed> and <entry> top-level tags * Improve code quality, remove line unrelated to fix
mastodon · Apr 8, 2017 · 982fef8 · 982fef8
1 parent 41f8fde
commit 982fef8
Show file tree

Hide file tree

Showing 4 changed files with 23 additions and 16 deletions.
diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb
@@ -5,14 +5,13 @@ def call(body, account)
     xml = Nokogiri::XML(body)
     xml.encoding = 'utf-8'
 
-    update_author(body, xml, account)
+    update_author(body, account)
     process_entries(xml, account)
   end
 
   private
 
-  def update_author(body, xml, account)
-    return if xml.at_xpath('/xmlns:feed', xmlns: TagManager::XMLNS).nil?
+  def update_author(body, account)
     RemoteProfileUpdateWorker.perform_async(account.id, body.force_encoding('UTF-8'), true)
   end
 

diff --git a/app/services/update_remote_profile_service.rb b/app/services/update_remote_profile_service.rb
@@ -1,7 +1,12 @@
 # frozen_string_literal: true
 
 class UpdateRemoteProfileService < BaseService
-  def call(xml, account, resubscribe = false)
+  def call(body, account, resubscribe = false)
+    xml = Nokogiri::XML(body)
+    xml.encoding = 'utf-8'
+
+    xml = xml.at_xpath('/xmlns:feed', xmlns: TagManager::XMLNS) || xml.at_xpath('/xmlns:entry', xmlns: TagManager::XMLNS)
+
     return if xml.nil?
 
     author_xml = xml.at_xpath('./xmlns:author', xmlns: TagManager::XMLNS) || xml.at_xpath('./dfrn:owner', dfrn: TagManager::DFRN_XMLNS)
@@ -12,9 +17,9 @@ def call(xml, account, resubscribe = false)
       account.note         = author_xml.at_xpath('./poco:note', poco: TagManager::POCO_XMLNS).content unless author_xml.at_xpath('./poco:note', poco: TagManager::POCO_XMLNS).nil?
       account.locked       = author_xml.at_xpath('./mastodon:scope', mastodon: TagManager::MTDN_XMLNS)&.content == 'private'
 
-      unless account.suspended? || DomainBlock.find_by(domain: account.domain)&.reject_media?
-        account.avatar_remote_url = author_xml.at_xpath('./xmlns:link[@rel="avatar"]', xmlns: TagManager::XMLNS)['href'] unless author_xml.at_xpath('./xmlns:link[@rel="avatar"]', xmlns: TagManager::XMLNS).nil? || author_xml.at_xpath('./xmlns:link[@rel="avatar"]', xmlns: TagManager::XMLNS)['href'].blank?
-        account.header_remote_url = author_xml.at_xpath('./xmlns:link[@rel="header"]', xmlns: TagManager::XMLNS)['href'] unless author_xml.at_xpath('./xmlns:link[@rel="header"]', xmlns: TagManager::XMLNS).nil? || author_xml.at_xpath('./xmlns:link[@rel="header"]', xmlns: TagManager::XMLNS)['href'].blank?
+      if !account.suspended? && !DomainBlock.find_by(domain: account.domain)&.reject_media?
+        account.avatar_remote_url = link_href_from_xml(author_xml, 'avatar') if link_has_href?(author_xml, 'avatar')
+        account.header_remote_url = link_href_from_xml(author_xml, 'header') if link_has_href?(author_xml, 'header')
       end
     end
 
@@ -25,4 +30,14 @@ def call(xml, account, resubscribe = false)
 
     SubscribeService.new.call(account) if resubscribe && (account.hub_url != old_hub_url)
   end
+
+  private
+
+  def link_href_from_xml(xml, type)
+    xml.at_xpath('./xmlns:link[@rel="' + type + '"]', xmlns: TagManager::XMLNS)['href']
+  end
+
+  def link_has_href?(xml, type)
+    !(xml.at_xpath('./xmlns:link[@rel="' + type + '"]', xmlns: TagManager::XMLNS).nil? || xml.at_xpath('./xmlns:link[@rel="' + type + '"]', xmlns: TagManager::XMLNS)['href'].blank?)
+  end
 end
diff --git a/app/workers/remote_profile_update_worker.rb b/app/workers/remote_profile_update_worker.rb
@@ -6,14 +6,7 @@ class RemoteProfileUpdateWorker
   sidekiq_options queue: 'pull'
 
   def perform(account_id, body, resubscribe)
-    account = Account.find(account_id)
-
-    xml = Nokogiri::XML(body)
-    xml.encoding = 'utf-8'
-
-    author_container = xml.at_xpath('/xmlns:feed', xmlns: TagManager::XMLNS) || xml.at_xpath('/xmlns:entry', xmlns: TagManager::XMLNS)
-
-    UpdateRemoteProfileService.new.call(author_container, account, resubscribe)
+    UpdateRemoteProfileService.new.call(body, Account.find(account_id), resubscribe)
   rescue ActiveRecord::RecordNotFound
     true
   end

diff --git a/spec/services/update_remote_profile_service_spec.rb b/spec/services/update_remote_profile_service_spec.rb
@@ -1,7 +1,7 @@
 require 'rails_helper'
 
 RSpec.describe UpdateRemoteProfileService do
-  let(:xml) { Nokogiri::XML(File.read(File.join(Rails.root, 'spec', 'fixtures', 'push', 'feed.atom'))).at_xpath('//xmlns:feed') }
+  let(:xml) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'push', 'feed.atom')) }
 
   subject { UpdateRemoteProfileService.new }