From 5abdc77c8060a62ecf2259a1e9d63e862b9f7be7 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 12 May 2017 19:09:21 +0200 Subject: Add conversation model, (#3016) * Add tag to Atom input/output Only uses ref attribute (not href) because href would be the alternate link that's always included also. Creates new conversation for every non-reply status. Carries over conversation for every reply. Keeps remote URIs verbatim, generates local URIs on the fly like the rest of them. * Fix conversation migration * More spec coverage for status before_create * Prevent n+1 query when generating Atom with the new conversations * Improve code style * Remove redundant local variable --- app/lib/atom_serializer.rb | 7 ++++++ app/models/conversation.rb | 20 ++++++++++++++++++ app/models/status.rb | 41 ++++++++++++++++++++++++++++++------ app/models/stream_entry.rb | 2 +- app/services/process_feed_service.rb | 15 ++++++++++++- 5 files changed, 77 insertions(+), 8 deletions(-) create mode 100644 app/models/conversation.rb (limited to 'app') diff --git a/app/lib/atom_serializer.rb b/app/lib/atom_serializer.rb index 288a9cfad..3113feac9 100644 --- a/app/lib/atom_serializer.rb +++ b/app/lib/atom_serializer.rb @@ -86,6 +86,7 @@ class AtomSerializer 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: :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? entry end @@ -107,6 +108,7 @@ class AtomSerializer append_element(object, 'link', nil, rel: :alternate, type: 'text/html', href: TagManager.instance.url_for(status)) append_element(object, 'thr:in-reply-to', nil, ref: TagManager.instance.uri_for(status.thread), href: TagManager.instance.url_for(status.thread)) if status.reply? && !status.thread.nil? + append_element(object, 'ostatus:conversation', nil, ref: conversation_uri(status.conversation)) unless status.conversation_id.nil? object end @@ -325,6 +327,11 @@ class AtomSerializer raw_str.to_s end + def conversation_uri(conversation) + return conversation.uri if conversation.uri.present? + TagManager.instance.unique_tag(conversation.created_at, conversation.id, 'Conversation') + end + def add_namespaces(parent) parent['xmlns'] = TagManager::XMLNS parent['xmlns:thr'] = TagManager::THR_XMLNS diff --git a/app/models/conversation.rb b/app/models/conversation.rb new file mode 100644 index 000000000..fbec961c7 --- /dev/null +++ b/app/models/conversation.rb @@ -0,0 +1,20 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: conversations +# +# id :integer not null, primary key +# uri :string +# created_at :datetime not null +# updated_at :datetime not null +# + +class Conversation < ApplicationRecord + validates :uri, uniqueness: true + + has_many :statuses + + def local? + uri.nil? + end +end diff --git a/app/models/status.rb b/app/models/status.rb index 4808b1a64..14c6dd9f6 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -21,6 +21,7 @@ # favourites_count :integer default(0), not null # reblogs_count :integer default(0), not null # language :string default("en"), not null +# conversation_id :integer # class Status < ApplicationRecord @@ -34,6 +35,7 @@ class Status < ApplicationRecord belongs_to :account, inverse_of: :statuses, counter_cache: true, required: true belongs_to :in_reply_to_account, foreign_key: 'in_reply_to_account_id', class_name: 'Account' + belongs_to :conversation belongs_to :thread, foreign_key: 'in_reply_to_id', class_name: 'Status', inverse_of: :replies belongs_to :reblog, foreign_key: 'reblog_of_id', class_name: 'Status', inverse_of: :reblogs, counter_cache: :reblogs_count @@ -141,6 +143,11 @@ class Status < ApplicationRecord !sensitive? && media_attachments.any? end + before_validation :prepare_contents + before_create :set_reblog + before_create :set_visibility + before_create :set_conversation + class << self def in_allowed_languages(account) where(language: account.allowed_languages) @@ -242,17 +249,39 @@ class Status < ApplicationRecord end end - before_validation do + private + + def prepare_contents text&.strip! spoiler_text&.strip! + end - self.reply = !(in_reply_to_id.nil? && thread.nil?) unless reply - self.reblog = reblog.reblog if reblog? && reblog.reblog? - self.in_reply_to_account_id = (thread.account_id == account_id && thread.reply? ? thread.in_reply_to_account_id : thread.account_id) if reply? && !thread.nil? - self.visibility = (account.locked? ? :private : :public) if visibility.nil? + def set_reblog + self.reblog = reblog.reblog if reblog? && reblog.reblog? end - private + def set_visibility + self.visibility = (account.locked? ? :private : :public) if visibility.nil? + end + + def set_conversation + self.reply = !(in_reply_to_id.nil? && thread.nil?) unless reply + + if reply? && !thread.nil? + self.in_reply_to_account_id = carried_over_reply_to_account_id + self.conversation_id = thread.conversation_id if conversation_id.nil? + elsif conversation_id.nil? + create_conversation + end + end + + def carried_over_reply_to_account_id + if thread.account_id == account_id && thread.reply? + thread.in_reply_to_account_id + else + thread.account_id + end + end def filter_from_context?(status, account) account&.blocking?(status.account_id) || account&.muting?(status.account_id) || (status.account.silenced? && !account&.following?(status.account_id)) || !status.permitted?(account) diff --git a/app/models/stream_entry.rb b/app/models/stream_entry.rb index d451e0dde..fb349f35c 100644 --- a/app/models/stream_entry.rb +++ b/app/models/stream_entry.rb @@ -22,7 +22,7 @@ class StreamEntry < ApplicationRecord validates :account, :activity, presence: true - STATUS_INCLUDES = [:account, :stream_entry, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :media_attachments, :tags, mentions: :account], thread: [:stream_entry, :account]].freeze + STATUS_INCLUDES = [:account, :stream_entry, :conversation, :media_attachments, :tags, mentions: :account, reblog: [:stream_entry, :account, :conversation, :media_attachments, :tags, mentions: :account], thread: [:stream_entry, :account]].freeze default_scope { where(activity_type: 'Status') } scope :with_includes, -> { includes(:account, status: STATUS_INCLUDES) } diff --git a/app/services/process_feed_service.rb b/app/services/process_feed_service.rb index 1558f8790..afd44aafe 100644 --- a/app/services/process_feed_service.rb +++ b/app/services/process_feed_service.rb @@ -141,7 +141,8 @@ class ProcessFeedService < BaseService created_at: published(entry), reply: thread?(entry), language: content_language(entry), - visibility: visibility_scope(entry) + visibility: visibility_scope(entry), + conversation: find_or_create_conversation(entry) ) if thread?(entry) @@ -164,6 +165,18 @@ class ProcessFeedService < BaseService status end + def find_or_create_conversation(xml) + uri = xml.at_xpath('./ostatus:conversation', ostatus: TagManager::OS_XMLNS)&.attribute('ref')&.content + return if uri.nil? + + if TagManager.instance.local_id?(uri) + local_id = TagManager.instance.unique_tag_to_local_id(uri, 'Conversation') + return Conversation.find_by(id: local_id) + end + + Conversation.find_by(uri: uri) + end + def find_status(uri) if TagManager.instance.local_id?(uri) local_id = TagManager.instance.unique_tag_to_local_id(uri, 'Status') -- cgit