From 40c45f5dd958aa1319b4e8cb664e6b4cac029526 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 18 Aug 2017 01:03:18 +0200 Subject: Put ActivityPub alternate link into Atom, prefer it when processing Atom (#4623) --- app/lib/ostatus/atom_serializer.rb | 2 ++ 1 file changed, 2 insertions(+) (limited to 'app/lib/ostatus/atom_serializer.rb') diff --git a/app/lib/ostatus/atom_serializer.rb b/app/lib/ostatus/atom_serializer.rb index 0d62361be..92a16d228 100644 --- a/app/lib/ostatus/atom_serializer.rb +++ b/app/lib/ostatus/atom_serializer.rb @@ -343,6 +343,8 @@ class OStatus::AtomSerializer end def serialize_status_attributes(entry, status) + append_element(entry, 'link', nil, rel: :alternate, type: 'application/activity+json', href: ActivityPub::TagManager.instance.uri_for(status)) if status.account.local? + append_element(entry, 'summary', status.spoiler_text, 'xml:lang': status.language) if status.spoiler_text? append_element(entry, 'content', Formatter.instance.format(status).to_str, type: 'html', 'xml:lang': status.language) -- cgit From 4c76402ba1d355061e7e208b7a2f8251388a38e1 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 29 Aug 2017 16:11:05 +0200 Subject: Serialize ActivityPub alternate link into OStatus deletes, handle it (#4730) Requires moving Atom rendering from DistributionWorker (where `stream_entry.status` is already nil) to inline (where `stream_entry.status.destroyed?` is true) and distributing that. Unfortunately, such XML renderings can no longer be easily chained together into one payload of n items. --- app/lib/ostatus/activity/deletion.rb | 4 +++- app/lib/ostatus/atom_serializer.rb | 3 +++ app/models/status.rb | 13 ++++++++++-- app/services/batched_remove_status_service.rb | 24 +++++++++++++--------- app/services/remove_status_service.rb | 4 +--- .../pubsubhubbub/raw_distribution_worker.rb | 22 ++++++++++++++++++++ .../services/batched_remove_status_service_spec.rb | 7 +++---- 7 files changed, 57 insertions(+), 20 deletions(-) create mode 100644 app/workers/pubsubhubbub/raw_distribution_worker.rb (limited to 'app/lib/ostatus/atom_serializer.rb') diff --git a/app/lib/ostatus/activity/deletion.rb b/app/lib/ostatus/activity/deletion.rb index 860faf501..c98f5ee0a 100644 --- a/app/lib/ostatus/activity/deletion.rb +++ b/app/lib/ostatus/activity/deletion.rb @@ -3,7 +3,9 @@ class OStatus::Activity::Deletion < OStatus::Activity::Base def perform Rails.logger.debug "Deleting remote status #{id}" - status = Status.find_by(uri: id, account: @account) + + status = Status.find_by(uri: id, account: @account) + status ||= Status.find_by(uri: activitypub_uri, account: @account) if activitypub_uri? if status.nil? redis.setex("delete_upon_arrival:#{@account.id}:#{id}", 6 * 3_600, id) diff --git a/app/lib/ostatus/atom_serializer.rb b/app/lib/ostatus/atom_serializer.rb index 92a16d228..81fae4140 100644 --- a/app/lib/ostatus/atom_serializer.rb +++ b/app/lib/ostatus/atom_serializer.rb @@ -79,6 +79,9 @@ class OStatus::AtomSerializer if stream_entry.status.nil? append_element(entry, 'content', 'Deleted status') + elsif stream_entry.status.destroyed? + append_element(entry, 'content', 'Deleted status') + append_element(entry, 'link', nil, rel: :alternate, type: 'application/activity+json', href: ActivityPub::TagManager.instance.uri_for(stream_entry.status)) if stream_entry.account.local? else serialize_status_attributes(entry, stream_entry.status) end diff --git a/app/models/status.rb b/app/models/status.rb index 3dc83ad1f..abd902cd7 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -51,6 +51,7 @@ class Status < ApplicationRecord has_one :notification, as: :activity, dependent: :destroy has_one :preview_card, dependent: :destroy + has_one :stream_entry, as: :activity, inverse_of: :status validates :uri, uniqueness: true, unless: :local? validates :text, presence: true, unless: :reblog? @@ -90,7 +91,11 @@ class Status < ApplicationRecord end def verb - reblog? ? :share : :post + if destroyed? + :delete + else + reblog? ? :share : :post + end end def object_type @@ -110,7 +115,11 @@ class Status < ApplicationRecord end def title - reblog? ? "#{account.acct} shared a status by #{reblog.account.acct}" : "New status by #{account.acct}" + if destroyed? + "#{account.acct} deleted status" + else + reblog? ? "#{account.acct} shared a status by #{reblog.account.acct}" : "New status by #{account.acct}" + end end def hidden? diff --git a/app/services/batched_remove_status_service.rb b/app/services/batched_remove_status_service.rb index e9e22298d..86eaa5735 100644 --- a/app/services/batched_remove_status_service.rb +++ b/app/services/batched_remove_status_service.rb @@ -20,9 +20,10 @@ class BatchedRemoveStatusService < BaseService @activity_json_batches = [] @json_payloads = statuses.map { |s| [s.id, Oj.dump(event: :delete, payload: s.id)] }.to_h @activity_json = {} + @activity_xml = {} # Ensure that rendered XML reflects destroyed state - Status.where(id: statuses.map(&:id)).in_batches.destroy_all + statuses.each(&:destroy) # Batch by source account statuses.group_by(&:account_id).each do |_, account_statuses| @@ -31,7 +32,7 @@ class BatchedRemoveStatusService < BaseService unpush_from_home_timelines(account_statuses) if account.local? - batch_stream_entries(account_statuses) + batch_stream_entries(account, account_statuses) batch_activity_json(account, account_statuses) end end @@ -42,18 +43,16 @@ class BatchedRemoveStatusService < BaseService batch_salmon_slaps(status) if status.local? end - Pubsubhubbub::DistributionWorker.push_bulk(@stream_entry_batches) { |batch| batch } + Pubsubhubbub::RawDistributionWorker.push_bulk(@stream_entry_batches) { |batch| batch } NotificationWorker.push_bulk(@salmon_batches) { |batch| batch } ActivityPub::DeliveryWorker.push_bulk(@activity_json_batches) { |batch| batch } end private - def batch_stream_entries(statuses) - stream_entry_ids = statuses.map { |s| s.stream_entry.id } - - stream_entry_ids.each_slice(100) do |batch_of_stream_entry_ids| - @stream_entry_batches << [batch_of_stream_entry_ids] + def batch_stream_entries(account, statuses) + statuses.each do |status| + @stream_entry_batches << [build_xml(status.stream_entry), account.id] end end @@ -101,11 +100,10 @@ class BatchedRemoveStatusService < BaseService def batch_salmon_slaps(status) return if @mentions[status.id].empty? - payload = stream_entry_to_xml(status.stream_entry.reload) recipients = @mentions[status.id].map(&:account).reject(&:local?).select(&:ostatus?).uniq(&:domain).map(&:id) recipients.each do |recipient_id| - @salmon_batches << [payload, status.account_id, recipient_id] + @salmon_batches << [build_xml(status.stream_entry), status.account_id, recipient_id] end end @@ -145,6 +143,12 @@ class BatchedRemoveStatusService < BaseService ).as_json) end + def build_xml(stream_entry) + return @activity_xml[stream_entry.id] if @activity_xml.key?(stream_entry.id) + + @activity_xml[stream_entry.id] = stream_entry_to_xml(stream_entry) + end + def sign_json(status, json) Oj.dump(ActivityPub::LinkedDataSignature.new(json).sign!(status.account)) end diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 7ddbd8906..83fc77043 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -22,8 +22,6 @@ class RemoveStatusService < BaseService return unless @account.local? - @stream_entry = @stream_entry.reload - remove_from_remote_followers remove_from_remote_affected end @@ -62,7 +60,7 @@ class RemoveStatusService < BaseService def remove_from_remote_followers # OStatus - Pubsubhubbub::DistributionWorker.perform_async(@stream_entry.id) + Pubsubhubbub::RawDistributionWorker.perform_async(salmon_xml, @account.id) # ActivityPub ActivityPub::DeliveryWorker.push_bulk(@account.followers.inboxes) do |inbox_url| diff --git a/app/workers/pubsubhubbub/raw_distribution_worker.rb b/app/workers/pubsubhubbub/raw_distribution_worker.rb new file mode 100644 index 000000000..16962a623 --- /dev/null +++ b/app/workers/pubsubhubbub/raw_distribution_worker.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class Pubsubhubbub::RawDistributionWorker + include Sidekiq::Worker + + sidekiq_options queue: 'push' + + def perform(xml, source_account_id) + @account = Account.find(source_account_id) + @subscriptions = active_subscriptions.to_a + + Pubsubhubbub::DeliveryWorker.push_bulk(@subscriptions) do |subscription| + [subscription.id, xml] + end + end + + private + + def active_subscriptions + Subscription.where(account: @account).active.select('id, callback_url, domain') + end +end diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index 2484d4b58..b1e9ac567 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -48,11 +48,10 @@ RSpec.describe BatchedRemoveStatusService do expect(Redis.current).to have_received(:publish).with('timeline:public', any_args).at_least(:once) end - it 'sends PuSH update to PuSH subscribers with two payloads united' do + it 'sends PuSH update to PuSH subscribers' do expect(a_request(:post, 'http://example.com/push').with { |req| - matches = req.body.scan(TagManager::VERBS[:delete]) - matches.size == 2 - }).to have_been_made + matches = req.body.match(TagManager::VERBS[:delete]) + }).to have_been_made.at_least_once end it 'sends Salmon slap to previously mentioned users' do -- cgit From e7adbf572a50f77590f889bb4d7cb9efb6fc7036 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 6 Sep 2017 19:01:28 +0200 Subject: Switch to static URIs, new URI format in both protocols for new statuses (#4815) * Decouple Status#local? from uri being nil * Replace on-the-fly URI generation with stored URIs - Generate URI in after_save hook for local statuses - Use static value in TagManager when available, fallback to tag format - Make TagManager use ActivityPub::TagManager to understand new format - Adjust tests * Use other heuristic for locality of old statuses, do not perform long query * Exclude tombstone stream entries from Atom feed * Prevent nil statuses from landing in Pubsubhubbub::DistributionWorker * Fix URI not being saved (#4818) * Add more specs for Status * Save generated uri immediately and also fix method order to minimize diff. * Fix alternate HTML URL in Atom * Fix tests * Remove not-null constraint from statuses migration to speed it up --- app/controllers/accounts_controller.rb | 2 +- app/lib/ostatus/atom_serializer.rb | 4 +-- app/lib/tag_manager.rb | 13 ++++++--- app/models/status.rb | 14 +++++++++- app/workers/pubsubhubbub/distribution_worker.rb | 2 +- db/migrate/20170905165803_add_local_to_statuses.rb | 5 ++++ db/schema.rb | 3 ++- spec/fabricators/status_fabricator.rb | 4 +++ spec/lib/activitypub/activity/delete_spec.rb | 2 +- spec/lib/activitypub/activity/undo_spec.rb | 2 +- spec/lib/formatter_spec.rb | 4 +-- spec/lib/ostatus/atom_serializer_spec.rb | 31 +++++++++++----------- spec/lib/tag_manager_spec.rb | 15 ++--------- spec/models/status_spec.rb | 27 +++++++++++++++++-- spec/services/fetch_link_card_service_spec.rb | 2 +- 15 files changed, 84 insertions(+), 46 deletions(-) create mode 100644 db/migrate/20170905165803_add_local_to_statuses.rb (limited to 'app/lib/ostatus/atom_serializer.rb') diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index afa0417fa..26ab6636b 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -22,7 +22,7 @@ class AccountsController < ApplicationController format.atom do @entries = @account.stream_entries.where(hidden: false).with_includes.paginate_by_max_id(20, params[:max_id], params[:since_id]) - render xml: OStatus::AtomSerializer.render(OStatus::AtomSerializer.new.feed(@account, @entries.to_a)) + render xml: OStatus::AtomSerializer.render(OStatus::AtomSerializer.new.feed(@account, @entries.reject { |entry| entry.status.nil? })) end format.json do diff --git a/app/lib/ostatus/atom_serializer.rb b/app/lib/ostatus/atom_serializer.rb index 81fae4140..b8e22a381 100644 --- a/app/lib/ostatus/atom_serializer.rb +++ b/app/lib/ostatus/atom_serializer.rb @@ -65,7 +65,7 @@ class OStatus::AtomSerializer add_namespaces(entry) if root - append_element(entry, 'id', TagManager.instance.unique_tag(stream_entry.created_at, stream_entry.activity_id, stream_entry.activity_type)) + append_element(entry, 'id', TagManager.instance.uri_for(stream_entry.status)) append_element(entry, 'published', stream_entry.created_at.iso8601) append_element(entry, 'updated', stream_entry.updated_at.iso8601) append_element(entry, 'title', stream_entry&.status&.title || "#{stream_entry.account.acct} deleted status") @@ -86,7 +86,7 @@ class OStatus::AtomSerializer serialize_status_attributes(entry, stream_entry.status) end - append_element(entry, 'link', nil, rel: :alternate, type: 'text/html', href: account_stream_entry_url(stream_entry.account, stream_entry)) + append_element(entry, 'link', nil, rel: :alternate, type: 'text/html', href: TagManager.instance.url_for(stream_entry.status)) append_element(entry, 'link', nil, rel: :self, type: 'application/atom+xml', href: account_stream_entry_url(stream_entry.account, stream_entry, format: 'atom')) append_element(entry, 'thr:in-reply-to', nil, ref: TagManager.instance.uri_for(stream_entry.thread), href: TagManager.instance.url_for(stream_entry.thread)) if stream_entry.threaded? append_element(entry, 'ostatus:conversation', nil, ref: conversation_uri(stream_entry.status.conversation)) unless stream_entry&.status&.conversation_id.nil? diff --git a/app/lib/tag_manager.rb b/app/lib/tag_manager.rb index 5f87a2a48..f33a20c6f 100644 --- a/app/lib/tag_manager.rb +++ b/app/lib/tag_manager.rb @@ -49,12 +49,17 @@ class TagManager def unique_tag_to_local_id(tag, expected_type) return nil unless local_id?(tag) - matches = Regexp.new("objectId=([\\d]+):objectType=#{expected_type}").match(tag) - return matches[1] unless matches.nil? + + if ActivityPub::TagManager.instance.local_uri?(tag) + ActivityPub::TagManager.instance.uri_to_local_id(tag) + else + matches = Regexp.new("objectId=([\\d]+):objectType=#{expected_type}").match(tag) + return matches[1] unless matches.nil? + end end def local_id?(id) - id.start_with?("tag:#{Rails.configuration.x.local_domain}") + id.start_with?("tag:#{Rails.configuration.x.local_domain}") || ActivityPub::TagManager.instance.local_uri?(id) end def web_domain?(domain) @@ -92,7 +97,7 @@ class TagManager when :person account_url(target) when :note, :comment, :activity - unique_tag(target.created_at, target.id, 'Status') + target.uri || unique_tag(target.created_at, target.id, 'Status') end end diff --git a/app/models/status.rb b/app/models/status.rb index f44f79aaf..53eff0377 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -22,6 +22,7 @@ # reblogs_count :integer default(0), not null # language :string # conversation_id :integer +# local :boolean default(FALSE) # class Status < ApplicationRecord @@ -84,7 +85,7 @@ class Status < ApplicationRecord end def local? - uri.nil? + attributes['local'] || uri.nil? end def reblog? @@ -131,11 +132,14 @@ class Status < ApplicationRecord !sensitive? && media_attachments.any? end + after_create :store_uri, if: :local? + before_validation :prepare_contents, if: :local? before_validation :set_reblog before_validation :set_visibility before_validation :set_conversation before_validation :set_sensitivity + before_validation :set_local class << self def not_in_filtered_languages(account) @@ -253,6 +257,10 @@ class Status < ApplicationRecord private + def store_uri + update_attribute(:uri, ActivityPub::TagManager.instance.uri_for(self)) if uri.nil? + end + def prepare_contents text&.strip! spoiler_text&.strip! @@ -292,4 +300,8 @@ class Status < ApplicationRecord thread.account_id end end + + def set_local + self.local = account.local? + end end diff --git a/app/workers/pubsubhubbub/distribution_worker.rb b/app/workers/pubsubhubbub/distribution_worker.rb index 2a5e60fa0..524f6849f 100644 --- a/app/workers/pubsubhubbub/distribution_worker.rb +++ b/app/workers/pubsubhubbub/distribution_worker.rb @@ -6,7 +6,7 @@ class Pubsubhubbub::DistributionWorker sidekiq_options queue: 'push' def perform(stream_entry_ids) - stream_entries = StreamEntry.where(id: stream_entry_ids).includes(:status).reject { |e| e.status&.direct_visibility? } + stream_entries = StreamEntry.where(id: stream_entry_ids).includes(:status).reject { |e| e.status.nil? || e.status.direct_visibility? } return if stream_entries.empty? diff --git a/db/migrate/20170905165803_add_local_to_statuses.rb b/db/migrate/20170905165803_add_local_to_statuses.rb new file mode 100644 index 000000000..e89a0469d --- /dev/null +++ b/db/migrate/20170905165803_add_local_to_statuses.rb @@ -0,0 +1,5 @@ +class AddLocalToStatuses < ActiveRecord::Migration[5.1] + def change + add_column :statuses, :local, :boolean, null: true, default: false + end +end diff --git a/db/schema.rb b/db/schema.rb index ea7edc2f9..21bde2086 100644 --- a/db/schema.rb +++ b/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: 20170905044538) do +ActiveRecord::Schema.define(version: 20170905165803) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -315,6 +315,7 @@ ActiveRecord::Schema.define(version: 20170905044538) do t.integer "reblogs_count", default: 0, null: false t.string "language" t.bigint "conversation_id" + t.boolean "local", default: false t.index ["account_id", "id"], name: "index_statuses_on_account_id_id" t.index ["conversation_id"], name: "index_statuses_on_conversation_id" t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id" diff --git a/spec/fabricators/status_fabricator.rb b/spec/fabricators/status_fabricator.rb index 8ec5f4ba7..04bbbcf4b 100644 --- a/spec/fabricators/status_fabricator.rb +++ b/spec/fabricators/status_fabricator.rb @@ -1,4 +1,8 @@ Fabricator(:status) do account text "Lorem ipsum dolor sit amet" + + after_build do |status| + status.uri = Faker::Internet.device_token if !status.account.local? && status.uri.nil? + end end diff --git a/spec/lib/activitypub/activity/delete_spec.rb b/spec/lib/activitypub/activity/delete_spec.rb index 65e743abb..38254e31c 100644 --- a/spec/lib/activitypub/activity/delete_spec.rb +++ b/spec/lib/activitypub/activity/delete_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Delete do - let(:sender) { Fabricate(:account) } + let(:sender) { Fabricate(:account, domain: 'example.com') } let(:status) { Fabricate(:status, account: sender, uri: 'foobar') } let(:json) do diff --git a/spec/lib/activitypub/activity/undo_spec.rb b/spec/lib/activitypub/activity/undo_spec.rb index 4629a033f..14c68efe5 100644 --- a/spec/lib/activitypub/activity/undo_spec.rb +++ b/spec/lib/activitypub/activity/undo_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe ActivityPub::Activity::Undo do - let(:sender) { Fabricate(:account) } + let(:sender) { Fabricate(:account, domain: 'example.com') } let(:json) do { diff --git a/spec/lib/formatter_spec.rb b/spec/lib/formatter_spec.rb index dfe1d8b8f..ab04ccbab 100644 --- a/spec/lib/formatter_spec.rb +++ b/spec/lib/formatter_spec.rb @@ -178,7 +178,7 @@ RSpec.describe Formatter do end context 'with remote status' do - let(:status) { Fabricate(:status, text: 'Beep boop', uri: 'beepboop') } + let(:status) { Fabricate(:status, account: remote_account, text: 'Beep boop') } it 'reformats' do is_expected.to eq 'Beep boop' @@ -226,7 +226,7 @@ RSpec.describe Formatter do end context 'with remote status' do - let(:status) { Fabricate(:status, text: '', uri: 'beep boop') } + let(:status) { Fabricate(:status, account: remote_account, text: '') } it 'returns tag-stripped text' do is_expected.to eq '' diff --git a/spec/lib/ostatus/atom_serializer_spec.rb b/spec/lib/ostatus/atom_serializer_spec.rb index 301a0ce30..0451eceeb 100644 --- a/spec/lib/ostatus/atom_serializer_spec.rb +++ b/spec/lib/ostatus/atom_serializer_spec.rb @@ -403,8 +403,7 @@ RSpec.describe OStatus::AtomSerializer do it 'returns element whose rendered view triggers creation when processed' do remote_account = Account.create!(username: 'username') - remote_status = Fabricate(:status, account: remote_account) - remote_status.stream_entry.update!(created_at: '2000-01-01T00:00:00Z') + remote_status = Fabricate(:status, account: remote_account, created_at: '2000-01-01T00:00:00Z') entry = OStatus::AtomSerializer.new.entry(remote_status.stream_entry, true) entry.nodes.delete_if { |node| node[:type] == 'application/activity+json' } # Remove ActivityPub link to simplify test @@ -421,7 +420,7 @@ RSpec.describe OStatus::AtomSerializer do ProcessFeedService.new.call(xml, account) - expect(Status.find_by(uri: "tag:remote,2000-01-01:objectId=#{remote_status.id}:objectType=Status")).to be_instance_of Status + expect(Status.find_by(uri: "https://remote/users/#{remote_status.account.to_param}/statuses/#{remote_status.id}")).to be_instance_of Status end end @@ -465,12 +464,11 @@ RSpec.describe OStatus::AtomSerializer do end it 'appends id element with unique tag' do - status = Fabricate(:status, reblog_of_id: nil) - status.stream_entry.update!(created_at: '2000-01-01T00:00:00Z') + status = Fabricate(:status, reblog_of_id: nil, created_at: '2000-01-01T00:00:00Z') entry = OStatus::AtomSerializer.new.entry(status.stream_entry) - expect(entry.id.text).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{status.id}:objectType=Status" + expect(entry.id.text).to eq "https://cb6e6126.ngrok.io/users/#{status.account.to_param}/statuses/#{status.id}" end it 'appends published element with created date' do @@ -515,7 +513,7 @@ RSpec.describe OStatus::AtomSerializer do entry = OStatus::AtomSerializer.new.entry(reblog.stream_entry) object = entry.nodes.find { |node| node.name == 'activity:object' } - expect(object.id.text).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{reblogged.id}:objectType=Status" + expect(object.id.text).to eq "https://cb6e6126.ngrok.io/users/#{reblogged.account.to_param}/statuses/#{reblogged.id}" end it 'does not append activity:object element if target is not present' do @@ -532,7 +530,7 @@ RSpec.describe OStatus::AtomSerializer do link = entry.nodes.find { |node| node.name == 'link' && node[:rel] == 'alternate' && node[:type] == 'text/html' } expect(link[:type]).to eq 'text/html' - expect(link[:href]).to eq "https://cb6e6126.ngrok.io/users/username/updates/#{status.stream_entry.id}" + expect(link[:href]).to eq "https://cb6e6126.ngrok.io/@username/#{status.id}" end it 'appends link element for itself' do @@ -553,7 +551,7 @@ RSpec.describe OStatus::AtomSerializer do entry = OStatus::AtomSerializer.new.entry(reply_status.stream_entry) in_reply_to = entry.nodes.find { |node| node.name == 'thr:in-reply-to' } - expect(in_reply_to[:ref]).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{in_reply_to_status.id}:objectType=Status" + expect(in_reply_to[:ref]).to eq "https://cb6e6126.ngrok.io/users/#{in_reply_to_status.account.to_param}/statuses/#{in_reply_to_status.id}" end it 'does not append thr:in-reply-to element if not threaded' do @@ -934,7 +932,7 @@ RSpec.describe OStatus::AtomSerializer do favourite_salmon = OStatus::AtomSerializer.new.favourite_salmon(favourite) object = favourite_salmon.nodes.find { |node| node.name == 'activity:object' } - expect(object.id.text).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{status.id}:objectType=Status" + expect(object.id.text).to eq "https://cb6e6126.ngrok.io/users/#{status.account.to_param}/statuses/#{status.id}" end it 'appends thr:in-reply-to element for status' do @@ -945,7 +943,7 @@ RSpec.describe OStatus::AtomSerializer do favourite_salmon = OStatus::AtomSerializer.new.favourite_salmon(favourite) in_reply_to = favourite_salmon.nodes.find { |node| node.name == 'thr:in-reply-to' } - expect(in_reply_to.ref).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{status.id}:objectType=Status" + expect(in_reply_to.ref).to eq "https://cb6e6126.ngrok.io/users/#{status.account.to_param}/statuses/#{status.id}" expect(in_reply_to.href).to eq "https://cb6e6126.ngrok.io/@username/#{status.id}" end @@ -1034,7 +1032,7 @@ RSpec.describe OStatus::AtomSerializer do unfavourite_salmon = OStatus::AtomSerializer.new.unfavourite_salmon(favourite) object = unfavourite_salmon.nodes.find { |node| node.name == 'activity:object' } - expect(object.id.text).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{status.id}:objectType=Status" + expect(object.id.text).to eq "https://cb6e6126.ngrok.io/users/#{status.account.to_param}/statuses/#{status.id}" end it 'appends thr:in-reply-to element for status' do @@ -1045,7 +1043,7 @@ RSpec.describe OStatus::AtomSerializer do unfavourite_salmon = OStatus::AtomSerializer.new.unfavourite_salmon(favourite) in_reply_to = unfavourite_salmon.nodes.find { |node| node.name == 'thr:in-reply-to' } - expect(in_reply_to.ref).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{status.id}:objectType=Status" + expect(in_reply_to.ref).to eq "https://cb6e6126.ngrok.io/users/#{status.account.to_param}/statuses/#{status.id}" expect(in_reply_to.href).to eq "https://cb6e6126.ngrok.io/@username/#{status.id}" end @@ -1453,7 +1451,7 @@ RSpec.describe OStatus::AtomSerializer do it 'appends id element with URL for status' do status = Fabricate(:status, created_at: '2000-01-01T00:00:00Z') object = OStatus::AtomSerializer.new.object(status) - expect(object.id.text).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{status.id}:objectType=Status" + expect(object.id.text).to eq "https://cb6e6126.ngrok.io/users/#{status.account.to_param}/statuses/#{status.id}" end it 'appends published element with created date' do @@ -1463,7 +1461,8 @@ RSpec.describe OStatus::AtomSerializer do end it 'appends updated element with updated date' do - status = Fabricate(:status, updated_at: '2000-01-01T00:00:00Z') + status = Fabricate(:status) + status.updated_at = '2000-01-01T00:00:00Z' object = OStatus::AtomSerializer.new.object(status) expect(object.updated.text).to eq '2000-01-01T00:00:00Z' end @@ -1523,7 +1522,7 @@ RSpec.describe OStatus::AtomSerializer do entry = OStatus::AtomSerializer.new.object(reply) in_reply_to = entry.nodes.find { |node| node.name == 'thr:in-reply-to' } - expect(in_reply_to.ref).to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{thread.id}:objectType=Status" + expect(in_reply_to.ref).to eq "https://cb6e6126.ngrok.io/users/#{thread.account.to_param}/statuses/#{thread.id}" expect(in_reply_to.href).to eq "https://cb6e6126.ngrok.io/@username/#{thread.id}" end diff --git a/spec/lib/tag_manager_spec.rb b/spec/lib/tag_manager_spec.rb index 1fae6bec4..1cd6e0a6f 100644 --- a/spec/lib/tag_manager_spec.rb +++ b/spec/lib/tag_manager_spec.rb @@ -157,23 +157,12 @@ RSpec.describe TagManager do describe '#uri_for' do subject { TagManager.instance.uri_for(target) } - context 'activity object' do - let(:target) { Fabricate(:status, reblog: Fabricate(:status)).stream_entry } - - before { target.update!(created_at: '2000-01-01T00:00:00Z') } - - it 'returns the unique tag for status' do - expect(target.object_type).to eq :activity - is_expected.to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{target.id}:objectType=Status" - end - end - context 'comment object' do let(:target) { Fabricate(:status, created_at: '2000-01-01T00:00:00Z', reply: true) } it 'returns the unique tag for status' do expect(target.object_type).to eq :comment - is_expected.to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{target.id}:objectType=Status" + is_expected.to eq target.uri end end @@ -182,7 +171,7 @@ RSpec.describe TagManager do it 'returns the unique tag for status' do expect(target.object_type).to eq :note - is_expected.to eq "tag:cb6e6126.ngrok.io,2000-01-01:objectId=#{target.id}:objectType=Status" + is_expected.to eq target.uri end end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 626fc3f98..484effd5e 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -13,9 +13,15 @@ RSpec.describe Status, type: :model do end it 'returns false if a remote URI is set' do - subject.uri = 'a' + alice.update(domain: 'example.com') + subject.save expect(subject.local?).to be false end + + it 'returns true if a URI is set and `local` is true' do + subject.update(uri: 'example.com', local: true) + expect(subject.local?).to be true + end end describe '#reblog?' do @@ -495,7 +501,7 @@ RSpec.describe Status, type: :model do end end - describe 'before_create' do + describe 'before_validation' do it 'sets account being replied to correctly over intermediary nodes' do first_status = Fabricate(:status, account: bob) intermediary = Fabricate(:status, thread: first_status, account: alice) @@ -512,5 +518,22 @@ RSpec.describe Status, type: :model do parent = Fabricate(:status, text: 'First') expect(Status.create(account: alice, thread: parent, text: 'Response').conversation_id).to eq parent.conversation_id end + + it 'sets `local` to true for status by local account' do + expect(Status.create(account: alice, text: 'foo').local).to be true + end + + it 'sets `local` to false for status by remote account' do + alice.update(domain: 'example.com') + expect(Status.create(account: alice, text: 'foo').local).to be false + end + end + + describe 'after_create' do + it 'saves ActivityPub uri as uri for local status' do + status = Status.create(account: alice, text: 'foo') + status.reload + expect(status.uri).to start_with('https://') + end end end diff --git a/spec/services/fetch_link_card_service_spec.rb b/spec/services/fetch_link_card_service_spec.rb index 3a0786d03..b0aa740ac 100644 --- a/spec/services/fetch_link_card_service_spec.rb +++ b/spec/services/fetch_link_card_service_spec.rb @@ -55,7 +55,7 @@ RSpec.describe FetchLinkCardService do end context 'in a remote status' do - let(:status) { Fabricate(:status, uri: 'abc', text: 'Habt ihr ein paar gute Links zu #Wannacry herumfliegen? Ich will mal unter
https://github.com/qbi/WannaCry was sammeln. !security ') } + let(:status) { Fabricate(:status, account: Fabricate(:account, domain: 'example.com'), text: 'Habt ihr ein paar gute Links zu #Wannacry herumfliegen? Ich will mal unter
https://github.com/qbi/WannaCry was sammeln. !security ') } it 'parses out URLs' do expect(a_request(:head, 'https://github.com/qbi/WannaCry')).to have_been_made.at_least_once -- cgit