Merge remote-tracking branch 'tootsuite/master' into glitchsoc/master

app/helpers/jsonld_helper.rb

@@ -9,6 +9,24 @@ module JsonLdHelper
     value.is_a?(Array) ? value.first : value
   end
 
+  # The url attribute can be a string, an array of strings, or an array of objects.
+  # The objects could include a mimeType. Not-included mimeType means it's text/html.
+  def url_to_href(value, preferred_type = nil)
+    single_value = if value.is_a?(Array) && !value.first.is_a?(String)
+                     value.find { |link| preferred_type.nil? || ((link['mimeType'].presence || 'text/html') == preferred_type) }
+                   elsif value.is_a?(Array)
+                     value.first
+                   else
+                     value
+                   end
+
+    if single_value.nil? || single_value.is_a?(String)
+      single_value
+    else
+      single_value['href']
+    end
+  end
+
   def as_array(value)
     value.is_a?(Array) ? value : [value]
   end

app/lib/activitypub/activity/create.rb

@@ -1,6 +1,9 @@
 # frozen_string_literal: true
 
 class ActivityPub::Activity::Create < ActivityPub::Activity
+  SUPPORTED_TYPES = %w(Article Note).freeze
+  CONVERTED_TYPES = %w(Image Video).freeze
+
   def perform
     return if delete_arrived_first?(object_uri) || unsupported_object_type?
 
@@ -41,7 +44,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
       url: object_url || @object['id'],
       account: @account,
       text: text_from_content || '',
-      language: language_from_content,
+      language: detected_language,
       spoiler_text: @object['summary'] || '',
       created_at: @options[:override_timestamps] ? nil : @object['published'],
       reply: @object['inReplyTo'].present?,
@@ -165,40 +168,62 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
   end
 
   def text_from_content
+    return Formatter.instance.linkify([text_from_name, object_url || @object['id']].join(' ')) if converted_object_type?
+
     if @object['content'].present?
       @object['content']
-    elsif language_map?
+    elsif content_language_map?
       @object['contentMap'].values.first
     end
   end
 
-  def language_from_content
-    return LanguageDetector.instance.detect(text_from_content, @account) unless language_map?
-    @object['contentMap'].keys.first
+  def text_from_name
+    if @object['name'].present?
+      @object['name']
+    elsif name_language_map?
+      @object['nameMap'].values.first
+    end
+  end
+
+  def detected_language
+    if content_language_map?
+      @object['contentMap'].keys.first
+    elsif name_language_map?
+      @object['nameMap'].keys.first
+    elsif supported_object_type?
+      LanguageDetector.instance.detect(text_from_content, @account)
+    end
   end
 
   def object_url
     return if @object['url'].blank?
-
-    value = first_of_value(@object['url'])
-
-    return value if value.is_a?(String)
-
-    value['href']
+    url_to_href(@object['url'], 'text/html')
   end
 
-  def language_map?
+  def content_language_map?
     @object['contentMap'].is_a?(Hash) && !@object['contentMap'].empty?
   end
 
+  def name_language_map?
+    @object['nameMap'].is_a?(Hash) && !@object['nameMap'].empty?
+  end
+
   def unsupported_object_type?
-    @object.is_a?(String) || !%w(Article Note).include?(@object['type'])
+    @object.is_a?(String) || !(supported_object_type? || converted_object_type?)
   end
 
   def unsupported_media_type?(mime_type)
     mime_type.present? && !(MediaAttachment::IMAGE_MIME_TYPES + MediaAttachment::VIDEO_MIME_TYPES).include?(mime_type)
   end
 
+  def supported_object_type?
+    SUPPORTED_TYPES.include?(@object['type'])
+  end
+
+  def converted_object_type?
+    CONVERTED_TYPES.include?(@object['type'])
+  end
+
   def skip_download?
     return @skip_download if defined?(@skip_download)
     @skip_download ||= DomainBlock.find_by(domain: @account.domain)&.reject_media?
@@ -210,7 +235,7 @@ class ActivityPub::Activity::Create < ActivityPub::Activity
 
   def forward_for_reply
     return unless @json['signature'].present? && reply_to_local?
-    ActivityPub::RawDistributionWorker.perform_async(Oj.dump(@json), replied_to_status.account_id)
+    ActivityPub::RawDistributionWorker.perform_async(Oj.dump(@json), replied_to_status.account_id, [@account.preferred_inbox_url])
   end
 
   def lock_options

app/lib/activitypub/activity/delete.rb

@@ -30,8 +30,11 @@ class ActivityPub::Activity::Delete < ActivityPub::Activity
   def forward_for_reblogs(status)
     return if @json['signature'].blank?
 
-    ActivityPub::RawDistributionWorker.push_bulk(status.reblogs.includes(:account).references(:account).merge(Account.local).pluck(:account_id)) do |account_id|
-      [payload, account_id]
+    rebloggers_ids = status.reblogs.includes(:account).references(:account).merge(Account.local).pluck(:account_id)
+    inboxes        = Account.where(id: ::Follow.where(target_account_id: rebloggers_ids).select(:account_id)).inboxes - [@account.preferred_inbox_url]
+
+    ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url|
+      [payload, rebloggers_ids.first, inbox_url]
     end
   end
 

app/lib/formatter.rb

@@ -51,12 +51,7 @@ class Formatter
 
   def simplified_format(account)
     return reformat(account.note).html_safe unless account.local? # rubocop:disable Rails/OutputSafety
-
-    html = encode_and_link_urls(account.note)
-    html = simple_format(html, {}, sanitize: false)
-    html = html.delete("\n")
-
-    html.html_safe # rubocop:disable Rails/OutputSafety
+    linkify(account.note)
   end
 
   def sanitize(html, config)
@@ -69,6 +64,14 @@ class Formatter
     html.html_safe # rubocop:disable Rails/OutputSafety
   end
 
+  def linkify(text)
+    html = encode_and_link_urls(text)
+    html = simple_format(html, {}, sanitize: false)
+    html = html.delete("\n")
+
+    html.html_safe # rubocop:disable Rails/OutputSafety
+  end
+
   private
 
   def encode(html)

app/models/account.rb

@@ -216,6 +216,10 @@ class Account < ApplicationRecord
     Rails.cache.fetch("exclude_domains_for:#{id}") { domain_blocks.pluck(:domain) }
   end
 
+  def preferred_inbox_url
+    shared_inbox_url.presence || inbox_url
+  end
+
   class << self
     def readonly_attributes
       super - %w(statuses_count following_count followers_count)

app/services/activitypub/fetch_remote_status_service.rb

@@ -42,7 +42,7 @@ class ActivityPub::FetchRemoteStatusService < BaseService
   end
 
   def expected_type?
-    %w(Note Article).include? @json['type']
+    (ActivityPub::Activity::Create::SUPPORTED_TYPES + ActivityPub::Activity::Create::CONVERTED_TYPES).include? @json['type']
   end
 
   def needs_update(actor)

app/services/activitypub/process_account_service.rb

@@ -107,12 +107,7 @@ class ActivityPub::ProcessAccountService < BaseService
 
   def url
     return if @json['url'].blank?
-
-    value = first_of_value(@json['url'])
-
-    return value if value.is_a?(String)
-
-    value['href']
+    url_to_href(@json['url'], 'text/html')
   end
 
   def outbox_total_items

app/services/remove_status_service.rb

@@ -3,7 +3,7 @@
 class RemoveStatusService < BaseService
   include StreamEntryRenderer
 
-  def call(status)
+  def call(status, options = {})
     @payload      = Oj.dump(event: :delete, payload: status.id.to_s)
     @status       = status
     @account      = status.account
@@ -11,6 +11,7 @@ class RemoveStatusService < BaseService
     @mentions     = status.mentions.includes(:account).to_a
     @reblogs      = status.reblogs.to_a
     @stream_entry = status.stream_entry
+    @options      = options
 
     remove_from_self if status.account.local?
     remove_from_followers
@@ -23,7 +24,12 @@ class RemoveStatusService < BaseService
 
     @status.destroy!
 
-    return unless @account.local?
+    # There is no reason to send out Undo activities when the
+    # cause is that the original object has been removed, since
+    # original object being removed implicitly removes reblogs
+    # of it. The Delete activity of the original is forwarded
+    # separately.
+    return if !@account.local? || @options[:original_removed]
 
     remove_from_remote_followers
     remove_from_remote_affected
@@ -105,7 +111,7 @@ class RemoveStatusService < BaseService
     # without us being able to do all the fancy stuff
 
     @reblogs.each do |reblog|
-      RemoveStatusService.new.call(reblog)
+      RemoveStatusService.new.call(reblog, original_removed: true)
     end
   end
 

app/workers/activitypub/raw_distribution_worker.rb

@@ -5,10 +5,10 @@ class ActivityPub::RawDistributionWorker
 
   sidekiq_options queue: 'push'
 
-  def perform(json, source_account_id)
+  def perform(json, source_account_id, exclude_inboxes = [])
     @account = Account.find(source_account_id)
 
-    ActivityPub::DeliveryWorker.push_bulk(inboxes) do |inbox_url|
+    ActivityPub::DeliveryWorker.push_bulk(inboxes - exclude_inboxes) do |inbox_url|
       [json, @account.id, inbox_url]
     end
   rescue ActiveRecord::RecordNotFound

db/migrate/20171129172043_add_index_on_stream_entries.rb

@@ -0,0 +1,7 @@
+class AddIndexOnStreamEntries < ActiveRecord::Migration[5.1]
+  def change
+    commit_db_transaction
+    add_index :stream_entries, [:account_id, :activity_type, :id], algorithm: :concurrently
+    remove_index :stream_entries, name: :index_stream_entries_on_account_id
+  end
+end

db/schema.rb

@@ -10,7 +10,7 @@
 #
 # It's strongly recommended that you check this file into your version control system.
 
-ActiveRecord::Schema.define(version: 20171125190735) do
+ActiveRecord::Schema.define(version: 20171129172043) do
 
   # These are extensions that must be enabled in order to support this database
   enable_extension "plpgsql"
@@ -440,7 +440,7 @@ ActiveRecord::Schema.define(version: 20171125190735) do
     t.datetime "updated_at", null: false
     t.boolean "hidden", default: false, null: false
     t.bigint "account_id"
-    t.index ["account_id"], name: "index_stream_entries_on_account_id"
+    t.index ["account_id", "activity_type", "id"], name: "index_stream_entries_on_account_id_and_activity_type_and_id"
     t.index ["activity_id", "activity_type"], name: "index_stream_entries_on_activity_id_and_activity_type"
   end
 

spec/lib/activitypub/activity/delete_spec.rb

@@ -1,8 +1,8 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::Activity::Delete do
-  let(:sender)    { Fabricate(:account, domain: 'example.com') }
-  let(:status)    { Fabricate(:status, account: sender, uri: 'foobar') }
+  let(:sender) { Fabricate(:account, domain: 'example.com') }
+  let(:status) { Fabricate(:status, account: sender, uri: 'foobar') }
 
   let(:json) do
     {
@@ -30,13 +30,13 @@ RSpec.describe ActivityPub::Activity::Delete do
   context 'when the status has been reblogged' do
     describe '#perform' do
       subject { described_class.new(json, sender) }
-      let(:reblogger) { Fabricate(:account) }
-      let(:follower)   { Fabricate(:account, username: 'follower', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') }
+      let!(:reblogger) { Fabricate(:account) }
+      let!(:follower)  { Fabricate(:account, username: 'follower', protocol: :activitypub, domain: 'example.com', inbox_url: 'http://example.com/inbox') }
+      let!(:reblog)    { Fabricate(:status, account: reblogger, reblog: status) }
 
       before do
         stub_request(:post, 'http://example.com/inbox').to_return(status: 200)
         follower.follow!(reblogger)
-        Fabricate(:status, account: reblogger, reblog: status)
         subject.perform
       end
 
@@ -45,8 +45,7 @@ RSpec.describe ActivityPub::Activity::Delete do
       end
 
       it 'sends delete activity to followers of rebloggers' do
-        # one for Delete original post, and one for Undo reblog (normal delivery)
-        expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.twice
+        expect(a_request(:post, 'http://example.com/inbox')).to have_been_made.once
       end
     end
   end

spec/services/activitypub/fetch_remote_status_service_spec.rb

@@ -1,6 +1,8 @@
 require 'rails_helper'
 
 RSpec.describe ActivityPub::FetchRemoteStatusService do
+  include ActionView::Helpers::TextHelper
+
   let(:sender) { Fabricate(:account) }
   let(:recipient) { Fabricate(:account) }
   let(:valid_domain) { Rails.configuration.x.local_domain }
@@ -19,6 +21,7 @@ RSpec.describe ActivityPub::FetchRemoteStatusService do
 
   describe '#call' do
     before do
+      stub_request(:head, 'https://example.com/watch?v=12345').to_return(status: 404, body: '')
       subject.call(object[:id], prefetched_body: Oj.dump(object))
     end
 
@@ -32,5 +35,38 @@ RSpec.describe ActivityPub::FetchRemoteStatusService do
         expect(status.text).to eq 'Lorem ipsum'
       end
     end
+
+    context 'with Video object' do
+      let(:object) do
+        {
+          '@context': 'https://www.w3.org/ns/activitystreams',
+          id: "https://#{valid_domain}/@foo/1234",
+          type: 'Video',
+          name: 'Nyan Cat 10 hours remix',
+          attributedTo: ActivityPub::TagManager.instance.uri_for(sender),
+          url: [
+            {
+              type: 'Link',
+              mimeType: 'application/x-bittorrent',
+              href: 'https://example.com/12345.torrent',
+            },
+
+            {
+              type: 'Link',
+              mimeType: 'text/html',
+              href: 'https://example.com/watch?v=12345',
+            },
+          ],
+        }
+      end
+
+      it 'creates status' do
+        status = sender.statuses.first
+
+        expect(status).to_not be_nil
+        expect(status.url).to eq 'https://example.com/watch?v=12345'
+        expect(strip_tags(status.text)).to eq 'Nyan Cat 10 hours remix https://example.com/watch?v=12345'
+      end
+    end
   end
 end