From 774ac473736cbf348827cf6d861e7fbbb72d7623 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Sun, 7 Oct 2018 23:44:58 +0200 Subject: Add conversations API (#8832) * Add conversations API * Add web UI for conversations * Add test for conversations API * Add tests for ConversationAccount * Improve web UI * Rename ConversationAccount to AccountConversation * Remove conversations on block and mute * Change last_status_id to be a denormalization of status_ids * Add optimistic locking --- app/models/account_conversation.rb | 111 +++++++++++++++++++++++++++++++++++++ app/models/status.rb | 13 +++++ 2 files changed, 124 insertions(+) create mode 100644 app/models/account_conversation.rb (limited to 'app/models') diff --git a/app/models/account_conversation.rb b/app/models/account_conversation.rb new file mode 100644 index 000000000..a7205ec1a --- /dev/null +++ b/app/models/account_conversation.rb @@ -0,0 +1,111 @@ +# frozen_string_literal: true +# == Schema Information +# +# Table name: account_conversations +# +# id :bigint(8) not null, primary key +# account_id :bigint(8) +# conversation_id :bigint(8) +# participant_account_ids :bigint(8) default([]), not null, is an Array +# status_ids :bigint(8) default([]), not null, is an Array +# last_status_id :bigint(8) +# lock_version :integer default(0), not null +# + +class AccountConversation < ApplicationRecord + after_commit :push_to_streaming_api + + belongs_to :account + belongs_to :conversation + belongs_to :last_status, class_name: 'Status' + + before_validation :set_last_status + + def participant_account_ids=(arr) + self[:participant_account_ids] = arr.sort + end + + def participant_accounts + if participant_account_ids.empty? + [account] + else + Account.where(id: participant_account_ids) + end + end + + class << self + def paginate_by_id(limit, options = {}) + if options[:min_id] + paginate_by_min_id(limit, options[:min_id]).reverse + else + paginate_by_max_id(limit, options[:max_id], options[:since_id]) + end + end + + def paginate_by_min_id(limit, min_id = nil) + query = order(arel_table[:last_status_id].asc).limit(limit) + query = query.where(arel_table[:last_status_id].gt(min_id)) if min_id.present? + query + end + + def paginate_by_max_id(limit, max_id = nil, since_id = nil) + query = order(arel_table[:last_status_id].desc).limit(limit) + query = query.where(arel_table[:last_status_id].lt(max_id)) if max_id.present? + query = query.where(arel_table[:last_status_id].gt(since_id)) if since_id.present? + query + end + + def add_status(recipient, status) + conversation = find_or_initialize_by(account: recipient, conversation_id: status.conversation_id, participant_account_ids: participants_from_status(recipient, status)) + conversation.status_ids << status.id + conversation.save + conversation + rescue ActiveRecord::StaleObjectError + retry + end + + def remove_status(recipient, status) + conversation = find_by(account: recipient, conversation_id: status.conversation_id, participant_account_ids: participants_from_status(recipient, status)) + + return if conversation.nil? + + conversation.status_ids.delete(status.id) + + if conversation.status_ids.empty? + conversation.destroy + else + conversation.save + end + + conversation + rescue ActiveRecord::StaleObjectError + retry + end + + private + + def participants_from_status(recipient, status) + ((status.mentions.pluck(:account_id) + [status.account_id]).uniq - [recipient.id]).sort + end + end + + private + + def set_last_status + self.status_ids = status_ids.sort + self.last_status_id = status_ids.last + end + + def push_to_streaming_api + return if destroyed? || !subscribed_to_timeline? + PushConversationWorker.perform_async(id) + end + + def subscribed_to_timeline? + Redis.current.exists("subscribed:#{streaming_channel}") + end + + def streaming_channel + "timeline:direct:#{account_id}" + end +end diff --git a/app/models/status.rb b/app/models/status.rb index f2b5cc6ce..f61bd0fee 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -24,6 +24,8 @@ # class Status < ApplicationRecord + before_destroy :unlink_from_conversations + include Paginable include Streamable include Cacheable @@ -473,4 +475,15 @@ class Status < ApplicationRecord reblog&.decrement_count!(:reblogs_count) if reblog? thread&.decrement_count!(:replies_count) if in_reply_to_id.present? && (public_visibility? || unlisted_visibility?) end + + def unlink_from_conversations + return unless direct_visibility? + + mentioned_accounts = mentions.includes(:account).map(&:account) + inbox_owners = mentioned_accounts.select(&:local?) + (account.local? ? [account] : []) + + inbox_owners.each do |inbox_owner| + AccountConversation.remove_status(inbox_owner, self) + end + end end -- cgit From f194857ac9eeb85f9b27c056c556038aee23cb43 Mon Sep 17 00:00:00 2001 From: ashleyhull-versent Date: Mon, 8 Oct 2018 13:50:11 +1100 Subject: rubocop issues - Cleaning up (#8912) * cleanup pass * undo mistakes * fixed. * revert --- app/models/concerns/omniauthable.rb | 4 ++-- config/initializers/open_uri_redirection.rb | 2 +- lib/mastodon/migration_helpers.rb | 10 ++++----- spec/controllers/api/salmon_controller_spec.rb | 4 ++-- .../api/subscriptions_controller_spec.rb | 2 +- spec/fabricators/user_fabricator.rb | 2 +- spec/features/log_in_spec.rb | 2 +- spec/lib/ostatus/atom_serializer_spec.rb | 24 +++++++++++----------- spec/models/status_spec.rb | 2 +- spec/models/user_spec.rb | 2 +- spec/rails_helper.rb | 4 ++-- .../services/batched_remove_status_service_spec.rb | 2 +- spec/services/fetch_remote_account_service_spec.rb | 2 +- spec/services/process_feed_service_spec.rb | 2 +- .../services/update_remote_profile_service_spec.rb | 2 +- 15 files changed, 33 insertions(+), 33 deletions(-) (limited to 'app/models') diff --git a/app/models/concerns/omniauthable.rb b/app/models/concerns/omniauthable.rb index 50288e700..f263fe7af 100644 --- a/app/models/concerns/omniauthable.rb +++ b/app/models/concerns/omniauthable.rb @@ -26,7 +26,7 @@ module Omniauthable # to prevent the identity being locked with accidentally created accounts. # Note that this may leave zombie accounts (with no associated identity) which # can be cleaned up at a later date. - user = signed_in_resource ? signed_in_resource : identity.user + user = signed_in_resource || identity.user user = create_for_oauth(auth) if user.nil? if identity.user.nil? @@ -61,7 +61,7 @@ module Omniauthable display_name = auth.info.full_name || [auth.info.first_name, auth.info.last_name].join(' ') { - email: email ? email : "#{TEMP_EMAIL_PREFIX}-#{auth.uid}-#{auth.provider}.com", + email: email || "#{TEMP_EMAIL_PREFIX}-#{auth.uid}-#{auth.provider}.com", password: Devise.friendly_token[0, 20], account_attributes: { username: ensure_unique_username(auth.uid), diff --git a/config/initializers/open_uri_redirection.rb b/config/initializers/open_uri_redirection.rb index ea2dcffea..e9de85bdc 100644 --- a/config/initializers/open_uri_redirection.rb +++ b/config/initializers/open_uri_redirection.rb @@ -2,7 +2,7 @@ require 'open-uri' module OpenURI def self.redirectable?(uri1, uri2) # :nodoc: - uri1.scheme.downcase == uri2.scheme.downcase || + uri1.scheme.casecmp(uri2.scheme).zero? || (/\A(?:http|https|ftp)\z/i =~ uri1.scheme && /\A(?:http|https|ftp)\z/i =~ uri2.scheme) end end diff --git a/lib/mastodon/migration_helpers.rb b/lib/mastodon/migration_helpers.rb index 5c135685f..f5dc7e1c6 100644 --- a/lib/mastodon/migration_helpers.rb +++ b/lib/mastodon/migration_helpers.rb @@ -342,8 +342,8 @@ module Mastodon say "Migrating #{table_name}.#{column} (~#{total.to_i} rows)" - started_time = Time.now - last_time = Time.now + started_time = Time.zone.now + last_time = Time.zone.now migrated = 0 loop do stop_row = nil @@ -375,13 +375,13 @@ module Mastodon end migrated += batch_size - if Time.now - last_time > 1 + if Time.zone.now - last_time > 1 status = "Migrated #{migrated} rows" percentage = 100.0 * migrated / total status += " (~#{sprintf('%.2f', percentage)}%, " - remaining_time = (100.0 - percentage) * (Time.now - started_time) / percentage + remaining_time = (100.0 - percentage) * (Time.zone.now - started_time) / percentage status += "#{(remaining_time / 60).to_i}:" status += sprintf('%02d', remaining_time.to_i % 60) @@ -397,7 +397,7 @@ module Mastodon status += ')' say status, true - last_time = Time.now + last_time = Time.zone.now end # There are no more rows left to update. diff --git a/spec/controllers/api/salmon_controller_spec.rb b/spec/controllers/api/salmon_controller_spec.rb index 8ce4913a5..235a29af0 100644 --- a/spec/controllers/api/salmon_controller_spec.rb +++ b/spec/controllers/api/salmon_controller_spec.rb @@ -15,7 +15,7 @@ RSpec.describe Api::SalmonController, type: :controller do describe 'POST #update' do context 'with valid post data' do before do - post :update, params: { id: account.id }, body: File.read(File.join(Rails.root, 'spec', 'fixtures', 'salmon', 'mention.xml')) + post :update, params: { id: account.id }, body: File.read(Rails.root.join('spec', 'fixtures', 'salmon', 'mention.xml')) end it 'contains XML in the request body' do @@ -54,7 +54,7 @@ RSpec.describe Api::SalmonController, type: :controller do service = double(call: false) allow(VerifySalmonService).to receive(:new).and_return(service) - post :update, params: { id: account.id }, body: File.read(File.join(Rails.root, 'spec', 'fixtures', 'salmon', 'mention.xml')) + post :update, params: { id: account.id }, body: File.read(Rails.root.join('spec', 'fixtures', 'salmon', 'mention.xml')) end it 'returns http client error' do diff --git a/spec/controllers/api/subscriptions_controller_spec.rb b/spec/controllers/api/subscriptions_controller_spec.rb index b46971a54..7a4252fe6 100644 --- a/spec/controllers/api/subscriptions_controller_spec.rb +++ b/spec/controllers/api/subscriptions_controller_spec.rb @@ -33,7 +33,7 @@ RSpec.describe Api::SubscriptionsController, type: :controller do end describe 'POST #update' do - let(:feed) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'push', 'feed.atom')) } + let(:feed) { File.read(Rails.root.join('spec', 'fixtures', 'push', 'feed.atom')) } before do stub_request(:post, "https://quitter.no/main/push/hub").to_return(:status => 200, :body => "", :headers => {}) diff --git a/spec/fabricators/user_fabricator.rb b/spec/fabricators/user_fabricator.rb index cf51fe81d..7dfbdb52d 100644 --- a/spec/fabricators/user_fabricator.rb +++ b/spec/fabricators/user_fabricator.rb @@ -2,5 +2,5 @@ Fabricator(:user) do account email { sequence(:email) { |i| "#{i}#{Faker::Internet.email}" } } password "123456789" - confirmed_at { Time.now } + confirmed_at { Time.zone.now } end diff --git a/spec/features/log_in_spec.rb b/spec/features/log_in_spec.rb index ed626d880..53a1f9b12 100644 --- a/spec/features/log_in_spec.rb +++ b/spec/features/log_in_spec.rb @@ -3,7 +3,7 @@ require "rails_helper" feature "Log in" do given(:email) { "test@examle.com" } given(:password) { "password" } - given(:confirmed_at) { Time.now } + given(:confirmed_at) { Time.zone.now } background do Fabricate(:user, email: email, password: password, confirmed_at: confirmed_at) diff --git a/spec/lib/ostatus/atom_serializer_spec.rb b/spec/lib/ostatus/atom_serializer_spec.rb index 3bc4b7efb..891871c1c 100644 --- a/spec/lib/ostatus/atom_serializer_spec.rb +++ b/spec/lib/ostatus/atom_serializer_spec.rb @@ -728,9 +728,9 @@ RSpec.describe OStatus::AtomSerializer do it 'appends id element with unique tag' do block = Fabricate(:block) - time_before = Time.now + time_before = Time.zone.now block_salmon = OStatus::AtomSerializer.new.block_salmon(block) - time_after = Time.now + time_after = Time.zone.now expect(block_salmon.id.text).to( eq(OStatus::TagManager.instance.unique_tag(time_before.utc, block.id, 'Block')) @@ -815,9 +815,9 @@ RSpec.describe OStatus::AtomSerializer do it 'appends id element with unique tag' do block = Fabricate(:block) - time_before = Time.now + time_before = Time.zone.now unblock_salmon = OStatus::AtomSerializer.new.unblock_salmon(block) - time_after = Time.now + time_after = Time.zone.now expect(unblock_salmon.id.text).to( eq(OStatus::TagManager.instance.unique_tag(time_before.utc, block.id, 'Block')) @@ -994,9 +994,9 @@ RSpec.describe OStatus::AtomSerializer do it 'appends id element with unique tag' do favourite = Fabricate(:favourite) - time_before = Time.now + time_before = Time.zone.now unfavourite_salmon = OStatus::AtomSerializer.new.unfavourite_salmon(favourite) - time_after = Time.now + time_after = Time.zone.now expect(unfavourite_salmon.id.text).to( eq(OStatus::TagManager.instance.unique_tag(time_before.utc, favourite.id, 'Favourite')) @@ -1179,9 +1179,9 @@ RSpec.describe OStatus::AtomSerializer do follow = Fabricate(:follow) follow.destroy! - time_before = Time.now + time_before = Time.zone.now unfollow_salmon = OStatus::AtomSerializer.new.unfollow_salmon(follow) - time_after = Time.now + time_after = Time.zone.now expect(unfollow_salmon.id.text).to( eq(OStatus::TagManager.instance.unique_tag(time_before.utc, follow.id, 'Follow')) @@ -1327,9 +1327,9 @@ RSpec.describe OStatus::AtomSerializer do it 'appends id element with unique tag' do follow_request = Fabricate(:follow_request) - time_before = Time.now + time_before = Time.zone.now authorize_follow_request_salmon = OStatus::AtomSerializer.new.authorize_follow_request_salmon(follow_request) - time_after = Time.now + time_after = Time.zone.now expect(authorize_follow_request_salmon.id.text).to( eq(OStatus::TagManager.instance.unique_tag(time_before.utc, follow_request.id, 'FollowRequest')) @@ -1396,9 +1396,9 @@ RSpec.describe OStatus::AtomSerializer do it 'appends id element with unique tag' do follow_request = Fabricate(:follow_request) - time_before = Time.now + time_before = Time.zone.now reject_follow_request_salmon = OStatus::AtomSerializer.new.reject_follow_request_salmon(follow_request) - time_after = Time.now + time_after = Time.zone.now expect(reject_follow_request_salmon.id.text).to( eq(OStatus::TagManager.instance.unique_tag(time_before.utc, follow_request.id, 'FollowRequest')) diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 9d8670129..e8cf18af9 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -154,7 +154,7 @@ RSpec.describe Status, type: :model do describe '#target' do it 'returns nil if the status is self-contained' do - expect(subject.target).to be_nil + expect(subject.target).to be_nil end it 'returns nil if the status is a reply' do diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 42198cb4d..8c6778edc 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -67,7 +67,7 @@ RSpec.describe User, type: :model do describe 'confirmed' do it 'returns an array of users who are confirmed' do user_1 = Fabricate(:user, confirmed_at: nil) - user_2 = Fabricate(:user, confirmed_at: Time.now) + user_2 = Fabricate(:user, confirmed_at: Time.zone.now) expect(User.confirmed).to match_array([user_2]) end end diff --git a/spec/rails_helper.rb b/spec/rails_helper.rb index 79e80220c..1ded751ab 100644 --- a/spec/rails_helper.rb +++ b/spec/rails_helper.rb @@ -72,11 +72,11 @@ RSpec::Sidekiq.configure do |config| end def request_fixture(name) - File.read(File.join(Rails.root, 'spec', 'fixtures', 'requests', name)) + File.read(Rails.root.join('spec', 'fixtures', 'requests', name)) end def attachment_fixture(name) - File.open(File.join(Rails.root, 'spec', 'fixtures', 'files', name)) + File.open(Rails.root.join('spec', 'fixtures', 'files', name)) end def stub_jsonld_contexts! diff --git a/spec/services/batched_remove_status_service_spec.rb b/spec/services/batched_remove_status_service_spec.rb index 23c122e59..c66214555 100644 --- a/spec/services/batched_remove_status_service_spec.rb +++ b/spec/services/batched_remove_status_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe BatchedRemoveStatusService, type: :service do stub_request(:post, 'http://example.com/inbox').to_return(status: 200) Fabricate(:subscription, account: alice, callback_url: 'http://example.com/push', confirmed: true, expires_at: 30.days.from_now) - jeff.user.update(current_sign_in_at: Time.now) + jeff.user.update(current_sign_in_at: Time.zone.now) jeff.follow!(alice) hank.follow!(alice) diff --git a/spec/services/fetch_remote_account_service_spec.rb b/spec/services/fetch_remote_account_service_spec.rb index 20dd505d0..3cd86708b 100644 --- a/spec/services/fetch_remote_account_service_spec.rb +++ b/spec/services/fetch_remote_account_service_spec.rb @@ -19,7 +19,7 @@ RSpec.describe FetchRemoteAccountService, type: :service do end let(:webfinger) { { subject: 'acct:alice@example.com', links: [{ rel: 'self', href: 'https://example.com/alice' }] } } - let(:xml) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'xml', 'mastodon.atom')) } + let(:xml) { File.read(Rails.root.join('spec', 'fixtures', 'xml', 'mastodon.atom')) } shared_examples 'return Account' do it { is_expected.to be_an Account } diff --git a/spec/services/process_feed_service_spec.rb b/spec/services/process_feed_service_spec.rb index 1f26660ed..9d3465f3f 100644 --- a/spec/services/process_feed_service_spec.rb +++ b/spec/services/process_feed_service_spec.rb @@ -4,7 +4,7 @@ RSpec.describe ProcessFeedService, type: :service do subject { ProcessFeedService.new } describe 'processing a feed' do - let(:body) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'xml', 'mastodon.atom')) } + let(:body) { File.read(Rails.root.join('spec', 'fixtures', 'xml', 'mastodon.atom')) } let(:account) { Fabricate(:account, username: 'localhost', domain: 'kickass.zone') } before do diff --git a/spec/services/update_remote_profile_service_spec.rb b/spec/services/update_remote_profile_service_spec.rb index 7ac3a809a..f3ea70b80 100644 --- a/spec/services/update_remote_profile_service_spec.rb +++ b/spec/services/update_remote_profile_service_spec.rb @@ -1,7 +1,7 @@ require 'rails_helper' RSpec.describe UpdateRemoteProfileService, type: :service do - let(:xml) { File.read(File.join(Rails.root, 'spec', 'fixtures', 'push', 'feed.atom')) } + let(:xml) { File.read(Rails.root.join('spec', 'fixtures', 'push', 'feed.atom')) } subject { UpdateRemoteProfileService.new } -- cgit