From 9caa90025fd9f1ef46a74f31cefd19335e291e76 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 25 Aug 2017 01:41:18 +0200 Subject: Pinned statuses (#4675) * Pinned statuses * yarn manage:translations --- app/models/status.rb | 4 ++++ 1 file changed, 4 insertions(+) (limited to 'app/models/status.rb') diff --git a/app/models/status.rb b/app/models/status.rb index 24eaf7071..3dc83ad1f 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -164,6 +164,10 @@ class Status < ApplicationRecord ConversationMute.select('conversation_id').where(conversation_id: conversation_ids).where(account_id: account_id).map { |m| [m.conversation_id, true] }.to_h end + def pins_map(status_ids, account_id) + StatusPin.select('status_id').where(status_id: status_ids).where(account_id: account_id).map { |p| [p.status_id, true] }.to_h + end + def reload_stale_associations!(cached_items) account_ids = [] -- 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/models/status.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 7dc5035031a697e7a2726fcd787fc9c294751027 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 1 Sep 2017 16:20:16 +0200 Subject: Make PreviewCard records reuseable between statuses (#4642) * Make PreviewCard records reuseable between statuses **Warning!** Migration truncates preview_cards tablec * Allow a wider thumbnail for link preview, display it in horizontal layout (#4648) * Delete preview cards files before truncating * Rename old table instead of truncating it * Add mastodon:maintenance:remove_deprecated_preview_cards * Ignore deprecated_preview_cards in schema definition * Fix null behaviour --- app/controllers/api/v1/statuses_controller.rb | 2 +- .../mastodon/features/status/components/card.js | 9 +- app/javascript/styles/components.scss | 12 +++ app/models/media_attachment.rb | 3 + app/models/preview_card.rb | 31 +++++-- app/models/status.rb | 3 +- app/services/fetch_link_card_service.rb | 100 ++++++++++++--------- config/environment.rb | 2 + .../20170901141119_truncate_preview_cards.rb | 30 +++++++ ...658_create_join_table_preview_cards_statuses.rb | 7 ++ db/schema.rb | 22 +++-- lib/tasks/mastodon.rake | 23 +++++ spec/services/fetch_link_card_service_spec.rb | 6 +- 13 files changed, 186 insertions(+), 64 deletions(-) create mode 100644 db/migrate/20170901141119_truncate_preview_cards.rb create mode 100644 db/migrate/20170901142658_create_join_table_preview_cards_statuses.rb (limited to 'app/models/status.rb') diff --git a/app/controllers/api/v1/statuses_controller.rb b/app/controllers/api/v1/statuses_controller.rb index 9c7124d0f..544a4ce21 100644 --- a/app/controllers/api/v1/statuses_controller.rb +++ b/app/controllers/api/v1/statuses_controller.rb @@ -29,7 +29,7 @@ class Api::V1::StatusesController < Api::BaseController end def card - @card = PreviewCard.find_by(status: @status) + @card = @status.preview_cards.first if @card.nil? render_empty diff --git a/app/javascript/mastodon/features/status/components/card.js b/app/javascript/mastodon/features/status/components/card.js index bfb40468b..6b13e15cc 100644 --- a/app/javascript/mastodon/features/status/components/card.js +++ b/app/javascript/mastodon/features/status/components/card.js @@ -1,6 +1,7 @@ import React from 'react'; import ImmutablePropTypes from 'react-immutable-proptypes'; import punycode from 'punycode'; +import classnames from 'classnames'; const IDNA_PREFIX = 'xn--'; @@ -32,7 +33,7 @@ export default class Card extends React.PureComponent { if (card.get('image')) { image = (
- {card.get('title')} + {card.get('title')}
); } @@ -41,8 +42,12 @@ export default class Card extends React.PureComponent { provider = decodeIDNA(getHostname(card.get('url'))); } + const className = classnames('status-card', { + 'horizontal': card.get('width') > card.get('height'), + }); + return ( - + {image}
diff --git a/app/javascript/styles/components.scss b/app/javascript/styles/components.scss index 8b932e77c..4c913a931 100644 --- a/app/javascript/styles/components.scss +++ b/app/javascript/styles/components.scss @@ -2057,6 +2057,18 @@ button.icon-button.active i.fa-retweet { background: lighten($ui-base-color, 8%); } +.status-card.horizontal { + display: block; + + .status-card__image { + width: 100%; + } + + .status-card__image-image { + border-radius: 4px 4px 0 0; + } +} + .status-card__image-image { border-radius: 4px 0 0 4px; display: block; diff --git a/app/models/media_attachment.rb b/app/models/media_attachment.rb index 1e8c6d00a..d83ca44f1 100644 --- a/app/models/media_attachment.rb +++ b/app/models/media_attachment.rb @@ -142,9 +142,11 @@ class MediaAttachment < ApplicationRecord def populate_meta meta = {} + file.queued_for_write.each do |style, file| begin geo = Paperclip::Geometry.from_file file + meta[style] = { width: geo.width.to_i, height: geo.height.to_i, @@ -155,6 +157,7 @@ class MediaAttachment < ApplicationRecord meta[style] = {} end end + meta end diff --git a/app/models/preview_card.rb b/app/models/preview_card.rb index c334c48aa..b7efac354 100644 --- a/app/models/preview_card.rb +++ b/app/models/preview_card.rb @@ -4,16 +4,13 @@ # Table name: preview_cards # # id :integer not null, primary key -# status_id :integer # url :string default(""), not null -# title :string -# description :string +# title :string default(""), not null +# description :string default(""), not null # image_file_name :string # image_content_type :string # image_file_size :integer # image_updated_at :datetime -# created_at :datetime not null -# updated_at :datetime not null # type :integer default("link"), not null # html :text default(""), not null # author_name :string default(""), not null @@ -22,6 +19,8 @@ # provider_url :string default(""), not null # width :integer default(0), not null # height :integer default(0), not null +# created_at :datetime not null +# updated_at :datetime not null # class PreviewCard < ApplicationRecord @@ -31,21 +30,37 @@ class PreviewCard < ApplicationRecord enum type: [:link, :photo, :video, :rich] - belongs_to :status + has_and_belongs_to_many :statuses - has_attached_file :image, styles: { original: '120x120#' }, convert_options: { all: '-quality 80 -strip' } + has_attached_file :image, styles: { original: '280x120>' }, convert_options: { all: '-quality 80 -strip' } include Attachmentable include Remotable - validates :url, presence: true + validates :url, presence: true, uniqueness: true validates_attachment_content_type :image, content_type: IMAGE_MIME_TYPES validates_attachment_size :image, less_than: 1.megabytes + before_save :extract_dimensions, if: :link? + def save_with_optional_image! save! rescue ActiveRecord::RecordInvalid self.image = nil save! end + + private + + def extract_dimensions + file = image.queued_for_write[:original] + + return if file.nil? + + geo = Paperclip::Geometry.from_file(file) + self.width = geo.width.to_i + self.height = geo.height.to_i + rescue Paperclip::Errors::NotIdentifiedByImageMagickError + nil + end end diff --git a/app/models/status.rb b/app/models/status.rb index abd902cd7..f44f79aaf 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -47,10 +47,11 @@ class Status < ApplicationRecord has_many :replies, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :thread has_many :mentions, dependent: :destroy has_many :media_attachments, dependent: :destroy + has_and_belongs_to_many :tags + has_and_belongs_to_many :preview_cards 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? diff --git a/app/services/fetch_link_card_service.rb b/app/services/fetch_link_card_service.rb index 20c85e0ea..c38e9e7df 100644 --- a/app/services/fetch_link_card_service.rb +++ b/app/services/fetch_link_card_service.rb @@ -4,29 +4,45 @@ class FetchLinkCardService < BaseService URL_PATTERN = %r{https?://\S+} def call(status) - # Get first http/https URL that isn't local - url = parse_urls(status) + @status = status + @url = parse_urls - return if url.nil? + return if @url.nil? || @status.preview_cards.any? - url = url.to_s - card = PreviewCard.where(status: status).first_or_initialize(status: status, url: url) - res = Request.new(:head, url).perform + @url = @url.to_s - return if res.code != 200 || res.mime_type != 'text/html' + RedisLock.acquire(lock_options) do |lock| + if lock.acquired? + @card = PreviewCard.find_by(url: @url) + process_url if @card.nil? + end + end - attempt_opengraph(card, url) unless attempt_oembed(card, url) + attach_card unless @card.nil? rescue HTTP::ConnectionError, OpenSSL::SSL::SSLError nil end private - def parse_urls(status) - if status.local? - urls = status.text.match(URL_PATTERN).to_a.map { |uri| Addressable::URI.parse(uri).normalize } + def process_url + @card = PreviewCard.new(url: @url) + res = Request.new(:head, @url).perform + + return if res.code != 200 || res.mime_type != 'text/html' + + attempt_oembed || attempt_opengraph + end + + def attach_card + @status.preview_cards << @card + end + + def parse_urls + if @status.local? + urls = @status.text.match(URL_PATTERN).to_a.map { |uri| Addressable::URI.parse(uri).normalize } else - html = Nokogiri::HTML(status.text) + html = Nokogiri::HTML(@status.text) links = html.css('a') urls = links.map { |a| Addressable::URI.parse(a['href']).normalize unless skip_link?(a) }.compact end @@ -44,41 +60,41 @@ class FetchLinkCardService < BaseService a['rel']&.include?('tag') || a['class']&.include?('u-url') end - def attempt_oembed(card, url) - response = OEmbed::Providers.get(url) + def attempt_oembed + response = OEmbed::Providers.get(@url) - card.type = response.type - card.title = response.respond_to?(:title) ? response.title : '' - card.author_name = response.respond_to?(:author_name) ? response.author_name : '' - card.author_url = response.respond_to?(:author_url) ? response.author_url : '' - card.provider_name = response.respond_to?(:provider_name) ? response.provider_name : '' - card.provider_url = response.respond_to?(:provider_url) ? response.provider_url : '' - card.width = 0 - card.height = 0 + @card.type = response.type + @card.title = response.respond_to?(:title) ? response.title : '' + @card.author_name = response.respond_to?(:author_name) ? response.author_name : '' + @card.author_url = response.respond_to?(:author_url) ? response.author_url : '' + @card.provider_name = response.respond_to?(:provider_name) ? response.provider_name : '' + @card.provider_url = response.respond_to?(:provider_url) ? response.provider_url : '' + @card.width = 0 + @card.height = 0 - case card.type + case @card.type when 'link' - card.image = URI.parse(response.thumbnail_url) if response.respond_to?(:thumbnail_url) + @card.image = URI.parse(response.thumbnail_url) if response.respond_to?(:thumbnail_url) when 'photo' - card.url = response.url - card.width = response.width.presence || 0 - card.height = response.height.presence || 0 + @card.url = response.url + @card.width = response.width.presence || 0 + @card.height = response.height.presence || 0 when 'video' - card.width = response.width.presence || 0 - card.height = response.height.presence || 0 - card.html = Formatter.instance.sanitize(response.html, Sanitize::Config::MASTODON_OEMBED) + @card.width = response.width.presence || 0 + @card.height = response.height.presence || 0 + @card.html = Formatter.instance.sanitize(response.html, Sanitize::Config::MASTODON_OEMBED) when 'rich' # Most providers rely on ', 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 From 11bddd31ce33b654ef72b00221715e6026486e7c Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 6 Sep 2017 20:57:52 +0200 Subject: Fix locking migration on statuses table. Nullable column and NO default value (#4825) --- app/models/status.rb | 2 +- db/migrate/20170905165803_add_local_to_statuses.rb | 2 +- db/schema.rb | 2 +- 3 files changed, 3 insertions(+), 3 deletions(-) (limited to 'app/models/status.rb') diff --git a/app/models/status.rb b/app/models/status.rb index 53eff0377..fdc230d8f 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -22,7 +22,7 @@ # reblogs_count :integer default(0), not null # language :string # conversation_id :integer -# local :boolean default(FALSE) +# local :boolean # class Status < ApplicationRecord diff --git a/db/migrate/20170905165803_add_local_to_statuses.rb b/db/migrate/20170905165803_add_local_to_statuses.rb index e89a0469d..fb4e7019d 100644 --- a/db/migrate/20170905165803_add_local_to_statuses.rb +++ b/db/migrate/20170905165803_add_local_to_statuses.rb @@ -1,5 +1,5 @@ class AddLocalToStatuses < ActiveRecord::Migration[5.1] def change - add_column :statuses, :local, :boolean, null: true, default: false + add_column :statuses, :local, :boolean, null: true, default: nil end end diff --git a/db/schema.rb b/db/schema.rb index 21bde2086..d8af0a1f8 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -315,7 +315,7 @@ ActiveRecord::Schema.define(version: 20170905165803) do t.integer "reblogs_count", default: 0, null: false t.string "language" t.bigint "conversation_id" - t.boolean "local", default: false + t.boolean "local" 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" -- cgit