Skip to content

Commit

Permalink
Fix #24 - Thread resolving for remote statuses
Browse files Browse the repository at this point in the history
This is a big one, so let me enumerate:

Accounts as well as stream entry pages now contain Link headers that
reference the Atom feed and Webfinger URL for the former and Atom entry
for the latter. So you only need to HEAD those resources to get that
information, no need to download and parse HTML <link>s.

ProcessFeedService will now queue ThreadResolveWorker for each remote
status that it cannot find otherwise. Furthermore, entries are now
processed in reverse order (from bottom to top) in case a newer entry
references a chronologically previous one.

ThreadResolveWorker uses FetchRemoteStatusService to obtain a status
and attach the child status it was queued for to it.

FetchRemoteStatusService looks up the URL, first with a HEAD, tests
if it's an Atom feed, in which case it processes it directly. Next
for Link headers to the Atom feed, in which case that is fetched
and processed. Lastly if it's HTML, it is checked for <link>s to the Atom
feed, and if such is found, that is fetched and processed. The account for
the status is derived from author/name attribute in the XML and the hostname
in the URL (domain). FollowRemoteAccountService and ProcessFeedService
are used.

This means that potentially threads are resolved recursively until a dead-end
is encountered, however it is performed asynchronously over background jobs,
so it should be ok.
  • Loading branch information
Gargron committed Sep 20, 2016
1 parent 6d89edc commit 4bec613
Show file tree
Hide file tree
Showing 9 changed files with 130 additions and 5 deletions.
1 change: 1 addition & 0 deletions Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ gem 'paperclip-av-transcoder'
gem 'http'
gem 'addressable'
gem 'nokogiri'
gem 'link_header'
gem 'ostatus2'
gem 'goldfinger'
gem 'devise'
Expand Down
2 changes: 2 additions & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ GEM
letter_opener (1.4.1)
launchy (~> 2.2)
libv8 (3.16.14.15)
link_header (0.0.8)
lograge (0.4.1)
actionpack (>= 4, < 5.1)
activesupport (>= 4, < 5.1)
Expand Down Expand Up @@ -368,6 +369,7 @@ DEPENDENCIES
jbuilder (~> 2.0)
jquery-rails
letter_opener
link_header
lograge
nokogiri
oj
Expand Down
9 changes: 6 additions & 3 deletions app/controllers/accounts_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ class AccountsController < ApplicationController
layout 'public'

before_action :set_account
before_action :set_webfinger_header
before_action :set_link_headers

def show
respond_to do |format|
Expand Down Expand Up @@ -39,8 +39,11 @@ def set_account
@account = Account.find_local!(params[:username])
end

def set_webfinger_header
response.headers['Link'] = "<#{webfinger_account_url}>; rel=\"lrdd\"; type=\"application/xrd+xml\""
def set_link_headers
response.headers['Link'] = LinkHeader.new([
[webfinger_account_url, [['rel', 'lrdd'], ['type', 'application/xrd+xml']]],
[account_url(@account, format: 'atom'), [['rel', 'alternate'], ['type', 'application/atom+xml']]]
])
end

def webfinger_account_url
Expand Down
7 changes: 7 additions & 0 deletions app/controllers/stream_entries_controller.rb
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ class StreamEntriesController < ApplicationController

before_action :set_account
before_action :set_stream_entry
before_action :set_link_headers

def show
@type = @stream_entry.activity_type.downcase
Expand Down Expand Up @@ -33,6 +34,12 @@ def set_account
@account = Account.find_local!(params[:account_username])
end

def set_link_headers
response.headers['Link'] = LinkHeader.new([
[account_stream_entry_url(@account, @stream_entry, format: 'atom'), [['rel', 'alternate'], ['type', 'application/atom+xml']]]
])
end

def set_stream_entry
@stream_entry = @account.stream_entries.find(params[:id])
end
Expand Down
71 changes: 71 additions & 0 deletions app/services/fetch_remote_status_service.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
class FetchRemoteStatusService < BaseService
def call(url)
response = http_client.head(url)

Rails.logger.debug "Remote status HEAD request returned code #{response.code}"
return nil if response.code != 200

if response.mime_type == 'application/atom+xml'
return process_atom(url, fetch(url))
elsif !response['Link'].blank?
return process_headers(response)
else
return process_html(fetch(url))
end
end

private

def process_atom(url, body)
Rails.logger.debug "Processing Atom for remote status"

xml = Nokogiri::XML(body)
account = extract_author(url, xml)

return nil if account.nil?

statuses = ProcessFeedService.new.(body, account)

return statuses.first
end

def process_html(body)
Rails.logger.debug "Processing HTML for remote status"

page = Nokogiri::HTML(body)
alternate_link = page.xpath('//link[@rel="alternate"]').find { |link| link['type'] == 'application/atom+xml' }

return nil if alternate_link.nil?
return process_atom(alternate_link['href'], fetch(alternate_link['href']))
end

def process_headers(response)
Rails.logger.debug "Processing link header for remote status"

link_header = LinkHeader.parse(response['Link'])
alternate_link = link_header.find_link(['rel', 'alternate'], ['type', 'application/atom+xml'])

return nil if alternate_link.nil?
return process_atom(alternate_link.href, fetch(alternate_link.href))
end

def extract_author(url, xml)
url_parts = Addressable::URI.parse(url)
username = xml.at_xpath('//xmlns:author/xmlns:name').try(:content)
domain = url_parts.host

return nil if username.nil?

Rails.logger.debug "Going to webfinger #{username}@#{domain}"

return FollowRemoteAccountService.new.("#{username}@#{domain}")
end

def fetch(url)
http_client.get(url).to_s
end

def http_client
HTTP.timeout(:per_operation, write: 20, connect: 20, read: 50)
end
end
2 changes: 1 addition & 1 deletion app/services/follow_remote_account_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ def update_remote_profile_service
end

def http_client
HTTP
HTTP.timeout(:per_operation, write: 20, connect: 20, read: 50)
end
end

22 changes: 21 additions & 1 deletion app/services/process_feed_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -2,10 +2,11 @@ class ProcessFeedService < BaseService
# Create local statuses from an Atom feed
# @param [String] body Atom feed
# @param [Account] account Account this feed belongs to
# @return [Enumerable] created statuses
def call(body, account)
xml = Nokogiri::XML(body)
update_remote_profile_service.(xml.at_xpath('/xmlns:feed/xmlns:author'), account) unless xml.at_xpath('/xmlns:feed').nil?
xml.xpath('//xmlns:entry').each { |entry| process_entry(account, entry) }
xml.xpath('//xmlns:entry').reverse_each.map { |entry| process_entry(account, entry) }.compact
end

private
Expand Down Expand Up @@ -45,6 +46,8 @@ def process_entry(account, entry)

DistributionWorker.perform_async(status.id)
end

return status
end

def record_remote_mentions(status, links)
Expand Down Expand Up @@ -103,6 +106,10 @@ def add_reblog!(entry, status)
def add_reply!(entry, status)
status.thread = find_original_status(entry, thread_id(entry))
status.save!

if status.thread.nil? && !thread_href(entry).nil?
ThreadResolveWorker.perform_async(status.id, thread_href(entry))
end
end

def delete_post!(status)
Expand Down Expand Up @@ -131,6 +138,13 @@ def fetch_remote_status(xml)

status = Status.new(account: account, uri: target_id(xml), text: target_content(xml), url: target_url(xml), created_at: published(xml), updated_at: updated(xml))
status.thread = find_original_status(xml, thread_id(xml))
status.save

if status.saved? && status.thread.nil? && !thread_href(xml).nil?
ThreadResolveWorker.perform_async(status.id, thread_href(xml))
end

status
rescue Goldfinger::Error, HTTP::Error
nil
end
Expand All @@ -153,6 +167,12 @@ def thread_id(xml)
nil
end

def thread_href(xml)
xml.at_xpath('./thr:in-reply-to').attribute('href').value
rescue
nil
end

def target_id(xml)
xml.at_xpath('.//activity:object/xmlns:id').content
rescue
Expand Down
13 changes: 13 additions & 0 deletions app/workers/thread_resolve_worker.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
class ThreadResolveWorker
include Sidekiq::Worker

def perform(child_status_id, parent_url)
child_status = Status.find(child_status_id)
parent_status = FetchRemoteStatusService.new.(parent_url)

unless parent_status.nil?
child_status.thread = parent_status
child_status.save!
end
end
end
8 changes: 8 additions & 0 deletions spec/controllers/api/subscriptions_controller_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,14 @@

before do
stub_request(:get, "https://quitter.no/avatar/7477-300-20160211190340.png").to_return(request_fixture('avatar.txt'))
stub_request(:head, "https://quitter.no/notice/1269244").to_return(status: 404)
stub_request(:head, "https://quitter.no/notice/1265331").to_return(status: 404)
stub_request(:head, "https://community.highlandarrow.com/notice/54411").to_return(status: 404)
stub_request(:head, "https://community.highlandarrow.com/notice/53857").to_return(status: 404)
stub_request(:head, "https://community.highlandarrow.com/notice/51852").to_return(status: 404)
stub_request(:head, "https://social.umeahackerspace.se/notice/424348").to_return(status: 404)
stub_request(:head, "https://community.highlandarrow.com/notice/50467").to_return(status: 404)
stub_request(:head, "https://quitter.no/notice/1243309").to_return(status: 404)

request.env['HTTP_X_HUB_SIGNATURE'] = "sha1=#{OpenSSL::HMAC.hexdigest('sha1', 'abc', feed)}"
request.env['RAW_POST_DATA'] = feed
Expand Down

0 comments on commit 4bec613

Please sign in to comment.