From d9ca46b464dc5a5eb0ad308809c15a79dcd17ada Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Thu, 2 Feb 2017 00:03:31 +0100 Subject: [PATCH] Cleaning up format of broadcast real-time messages, removing redis-backed "mentions" timeline as redundant (given notifications) --- .../components/containers/mastodon.jsx | 10 +++--- .../features/hashtag_timeline/index.jsx | 8 +++-- .../features/mentions_timeline/index.jsx | 36 ------------------- .../features/public_timeline/index.jsx | 8 +++-- app/channels/application_cable/channel.rb | 6 ++-- .../api/v1/timelines_controller.rb | 16 --------- app/lib/feed_manager.rb | 2 +- app/models/status.rb | 4 --- app/services/fan_out_on_write_service.rb | 15 ++------ app/services/notify_service.rb | 2 +- app/services/remove_status_service.rb | 6 ++-- config/routes.rb | 1 - docs/Using-the-API/API.md | 1 - docs/Using-the-API/Push-notifications.md | 2 ++ lib/tasks/mastodon.rake | 1 - .../api/v1/timelines_controller_spec.rb | 14 -------- .../services/fan_out_on_write_service_spec.rb | 4 --- 17 files changed, 26 insertions(+), 110 deletions(-) delete mode 100644 app/assets/javascripts/components/features/mentions_timeline/index.jsx diff --git a/app/assets/javascripts/components/containers/mastodon.jsx b/app/assets/javascripts/components/containers/mastodon.jsx index 839f7267e..5fd43fb2b 100644 --- a/app/assets/javascripts/components/containers/mastodon.jsx +++ b/app/assets/javascripts/components/containers/mastodon.jsx @@ -23,7 +23,6 @@ import GettingStarted from '../features/getting_started'; import PublicTimeline from '../features/public_timeline'; import AccountTimeline from '../features/account_timeline'; import HomeTimeline from '../features/home_timeline'; -import MentionsTimeline from '../features/mentions_timeline'; import Compose from '../features/compose'; import Followers from '../features/followers'; import Following from '../features/following'; @@ -68,15 +67,15 @@ const Mastodon = React.createClass({ this.subscription = App.cable.subscriptions.create('TimelineChannel', { received (data) { - switch(data.type) { + switch(data.event) { case 'update': - store.dispatch(updateTimeline(data.timeline, JSON.parse(data.message))); + store.dispatch(updateTimeline('home', JSON.parse(data.payload))); break; case 'delete': - store.dispatch(deleteFromTimelines(data.id)); + store.dispatch(deleteFromTimelines(data.payload)); break; case 'notification': - store.dispatch(updateNotifications(JSON.parse(data.message), getMessagesForLocale(locale), locale)); + store.dispatch(updateNotifications(JSON.parse(data.payload), getMessagesForLocale(locale), locale)); break; } } @@ -108,7 +107,6 @@ const Mastodon = React.createClass({ - diff --git a/app/assets/javascripts/components/features/hashtag_timeline/index.jsx b/app/assets/javascripts/components/features/hashtag_timeline/index.jsx index 011b1e54d..7548e6d56 100644 --- a/app/assets/javascripts/components/features/hashtag_timeline/index.jsx +++ b/app/assets/javascripts/components/features/hashtag_timeline/index.jsx @@ -26,11 +26,13 @@ const HashtagTimeline = React.createClass({ }, { received (data) { - switch(data.type) { + switch(data.event) { case 'update': - return dispatch(updateTimeline('tag', JSON.parse(data.message))); + dispatch(updateTimeline('tag', JSON.parse(data.payload))); + break; case 'delete': - return dispatch(deleteFromTimelines(data.id)); + dispatch(deleteFromTimelines(data.payload)); + break; } } diff --git a/app/assets/javascripts/components/features/mentions_timeline/index.jsx b/app/assets/javascripts/components/features/mentions_timeline/index.jsx deleted file mode 100644 index 8583f59a6..000000000 --- a/app/assets/javascripts/components/features/mentions_timeline/index.jsx +++ /dev/null @@ -1,36 +0,0 @@ -import { connect } from 'react-redux'; -import PureRenderMixin from 'react-addons-pure-render-mixin'; -import StatusListContainer from '../ui/containers/status_list_container'; -import Column from '../ui/components/column'; -import { refreshTimeline } from '../../actions/timelines'; -import { defineMessages, injectIntl } from 'react-intl'; - -const messages = defineMessages({ - title: { id: 'column.mentions', defaultMessage: 'Mentions' } -}); - -const MentionsTimeline = React.createClass({ - - propTypes: { - dispatch: React.PropTypes.func.isRequired - }, - - mixins: [PureRenderMixin], - - componentWillMount () { - this.props.dispatch(refreshTimeline('mentions')); - }, - - render () { - const { intl } = this.props; - - return ( - - - - ); - }, - -}); - -export default connect()(injectIntl(MentionsTimeline)); diff --git a/app/assets/javascripts/components/features/public_timeline/index.jsx b/app/assets/javascripts/components/features/public_timeline/index.jsx index 28cdc639a..42970061c 100644 --- a/app/assets/javascripts/components/features/public_timeline/index.jsx +++ b/app/assets/javascripts/components/features/public_timeline/index.jsx @@ -32,11 +32,13 @@ const PublicTimeline = React.createClass({ this.subscription = App.cable.subscriptions.create('PublicChannel', { received (data) { - switch(data.type) { + switch(data.event) { case 'update': - return dispatch(updateTimeline('public', JSON.parse(data.message))); + dispatch(updateTimeline('public', JSON.parse(data.payload))); + break; case 'delete': - return dispatch(deleteFromTimelines(data.id)); + dispatch(deleteFromTimelines(data.payload)); + break; } } diff --git a/app/channels/application_cable/channel.rb b/app/channels/application_cable/channel.rb index f9154bd7f..ae69032ce 100644 --- a/app/channels/application_cable/channel.rb +++ b/app/channels/application_cable/channel.rb @@ -7,10 +7,10 @@ module ApplicationCable def hydrate_status(encoded_message) message = ActiveSupport::JSON.decode(encoded_message) - return [nil, message] if message['type'] == 'delete' + return [nil, message] if message['event'] == 'delete' - status = Status.find_by(id: message['id']) - message['message'] = FeedManager.instance.inline_render(current_user.account, 'api/v1/statuses/show', status) + status = Status.find_by(id: message['payload']) + message['payload'] = FeedManager.instance.inline_render(current_user.account, 'api/v1/statuses/show', status) [status, message] end diff --git a/app/controllers/api/v1/timelines_controller.rb b/app/controllers/api/v1/timelines_controller.rb index 5042550db..854ca13e6 100644 --- a/app/controllers/api/v1/timelines_controller.rb +++ b/app/controllers/api/v1/timelines_controller.rb @@ -22,22 +22,6 @@ class Api::V1::TimelinesController < ApiController render action: :index end - def mentions - @statuses = Feed.new(:mentions, current_account).get(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id]) - @statuses = cache_collection(@statuses) - - set_maps(@statuses) - set_counters_maps(@statuses) - set_account_counters_maps(@statuses.flat_map { |s| [s.account, s.reblog? ? s.reblog.account : nil] }.compact.uniq) - - next_path = api_v1_mentions_timeline_url(max_id: @statuses.last.id) if @statuses.size == limit_param(DEFAULT_STATUSES_LIMIT) - prev_path = api_v1_mentions_timeline_url(since_id: @statuses.first.id) unless @statuses.empty? - - set_pagination_headers(next_path, prev_path) - - render action: :index - end - def public @statuses = Status.as_public_timeline(current_account).paginate_by_max_id(limit_param(DEFAULT_STATUSES_LIMIT), params[:max_id], params[:since_id]) @statuses = cache_collection(@statuses) diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index cdd26e69c..028fc5218 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -26,7 +26,7 @@ class FeedManager def push(timeline_type, account, status) redis.zadd(key(timeline_type, account.id), status.id, status.reblog? ? status.reblog_of_id : status.id) trim(timeline_type, account.id) - broadcast(account.id, type: 'update', timeline: timeline_type, message: inline_render(account, 'api/v1/statuses/show', status)) + broadcast(account.id, event: 'update', payload: inline_render(account, 'api/v1/statuses/show', status)) end def broadcast(timeline_id, options = {}) diff --git a/app/models/status.rb b/app/models/status.rb index 99e167bc9..63f5d5fa4 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -102,10 +102,6 @@ class Status < ApplicationRecord where(account: [account] + account.following) end - def as_mentions_timeline(account) - where(id: Mention.where(account: account).select(:status_id)) - end - def as_public_timeline(account = nil) query = joins('LEFT OUTER JOIN accounts ON statuses.account_id = accounts.id') .where(visibility: :public) diff --git a/app/services/fan_out_on_write_service.rb b/app/services/fan_out_on_write_service.rb index 4f387c6c2..79c7bf248 100644 --- a/app/services/fan_out_on_write_service.rb +++ b/app/services/fan_out_on_write_service.rb @@ -6,7 +6,6 @@ class FanOutOnWriteService < BaseService def call(status) deliver_to_self(status) if status.account.local? deliver_to_followers(status) - deliver_to_mentioned(status) return if status.account.silenced? || !status.public_visibility? || status.reblog? @@ -33,25 +32,15 @@ class FanOutOnWriteService < BaseService end end - def deliver_to_mentioned(status) - Rails.logger.debug "Delivering status #{status.id} to mentioned accounts" - - status.mentions.includes(:account).each do |mention| - mentioned_account = mention.account - next if !mentioned_account.local? || FeedManager.instance.filter?(:mentions, status, mentioned_account) - FeedManager.instance.push(:mentions, mentioned_account, status) - end - end - def deliver_to_hashtags(status) Rails.logger.debug "Delivering status #{status.id} to hashtags" status.tags.find_each do |tag| - FeedManager.instance.broadcast("hashtag:#{tag.name}", type: 'update', id: status.id) + FeedManager.instance.broadcast("hashtag:#{tag.name}", event: 'update', payload: status.id) end end def deliver_to_public(status) Rails.logger.debug "Delivering status #{status.id} to public timeline" - FeedManager.instance.broadcast(:public, type: 'update', id: status.id) + FeedManager.instance.broadcast(:public, event: 'update', payload: status.id) end end diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index d1504cadf..0cc3cd618 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -51,7 +51,7 @@ class NotifyService < BaseService def create_notification @notification.save! return unless @notification.browserable? - FeedManager.instance.broadcast(@recipient.id, type: 'notification', message: FeedManager.instance.inline_render(@recipient, 'api/v1/notifications/show', @notification)) + FeedManager.instance.broadcast(@recipient.id, event: 'notification', payload: FeedManager.instance.inline_render(@recipient, 'api/v1/notifications/show', @notification)) end def send_email diff --git a/app/services/remove_status_service.rb b/app/services/remove_status_service.rb index 7aca24d12..48e8dd3b8 100644 --- a/app/services/remove_status_service.rb +++ b/app/services/remove_status_service.rb @@ -59,17 +59,17 @@ class RemoveStatusService < BaseService redis.zremrangebyscore(FeedManager.instance.key(type, receiver.id), status.id, status.id) end - FeedManager.instance.broadcast(receiver.id, type: 'delete', id: status.id) + FeedManager.instance.broadcast(receiver.id, event: 'delete', payload: status.id) end def remove_from_hashtags(status) status.tags.each do |tag| - FeedManager.instance.broadcast("hashtag:#{tag.name}", type: 'delete', id: status.id) + FeedManager.instance.broadcast("hashtag:#{tag.name}", event: 'delete', payload: status.id) end end def remove_from_public(status) - FeedManager.instance.broadcast(:public, type: 'delete', id: status.id) + FeedManager.instance.broadcast(:public, event: 'delete', payload: status.id) end def redis diff --git a/config/routes.rb b/config/routes.rb index bce345f2e..699f56833 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -108,7 +108,6 @@ Rails.application.routes.draw do end get '/timelines/home', to: 'timelines#home', as: :home_timeline - get '/timelines/mentions', to: 'timelines#mentions', as: :mentions_timeline get '/timelines/public', to: 'timelines#public', as: :public_timeline get '/timelines/tag/:id', to: 'timelines#tag', as: :hashtag_timeline diff --git a/docs/Using-the-API/API.md b/docs/Using-the-API/API.md index fecc6c59e..51b465927 100644 --- a/docs/Using-the-API/API.md +++ b/docs/Using-the-API/API.md @@ -65,7 +65,6 @@ Returns a media object with an ID that can be attached when creating a status (s ### Retrieving a timeline **GET /api/v1/timelines/home** -**GET /api/v1/timelines/mentions** **GET /api/v1/timelines/public** **GET /api/v1/timelines/tag/:hashtag** diff --git a/docs/Using-the-API/Push-notifications.md b/docs/Using-the-API/Push-notifications.md index 746bf65ef..fd50a75bd 100644 --- a/docs/Using-the-API/Push-notifications.md +++ b/docs/Using-the-API/Push-notifications.md @@ -1,6 +1,8 @@ Push notifications ================== +**Note: This push notification design turned out to not be fully operational on the side of Firebase. A different approach is in consideration** + Mastodon can communicate with the Firebase Cloud Messaging API to send push notifications to apps on users' devices. For this to work, these conditions must be met: * Responsibility of an instance owner: `FCM_API_KEY` set on the instance. This can be obtained on the Firebase dashboard, in project settings, under Cloud Messaging, as "server key" diff --git a/lib/tasks/mastodon.rake b/lib/tasks/mastodon.rake index 13220f68e..1b60bd1c9 100644 --- a/lib/tasks/mastodon.rake +++ b/lib/tasks/mastodon.rake @@ -36,7 +36,6 @@ namespace :mastodon do task clear: :environment do User.where('current_sign_in_at < ?', 14.days.ago).find_each do |user| Redis.current.del(FeedManager.instance.key(:home, user.account_id)) - Redis.current.del(FeedManager.instance.key(:mentions, user.account_id)) end end diff --git a/spec/controllers/api/v1/timelines_controller_spec.rb b/spec/controllers/api/v1/timelines_controller_spec.rb index 5e9954baf..72eed1e5a 100644 --- a/spec/controllers/api/v1/timelines_controller_spec.rb +++ b/spec/controllers/api/v1/timelines_controller_spec.rb @@ -19,13 +19,6 @@ RSpec.describe Api::V1::TimelinesController, type: :controller do end end - describe 'GET #mentions' do - it 'returns http success' do - get :mentions - expect(response).to have_http_status(:success) - end - end - describe 'GET #public' do it 'returns http success' do get :public @@ -55,13 +48,6 @@ RSpec.describe Api::V1::TimelinesController, type: :controller do end end - describe 'GET #mentions' do - it 'returns http unprocessable entity' do - get :mentions - expect(response).to have_http_status(:unprocessable_entity) - end - end - describe 'GET #public' do it 'returns http success' do get :public diff --git a/spec/services/fan_out_on_write_service_spec.rb b/spec/services/fan_out_on_write_service_spec.rb index e46a31c24..07f8c2dc8 100644 --- a/spec/services/fan_out_on_write_service_spec.rb +++ b/spec/services/fan_out_on_write_service_spec.rb @@ -26,10 +26,6 @@ RSpec.describe FanOutOnWriteService do expect(Feed.new(:home, follower).get(10).map(&:id)).to include status.id end - it 'delivers status to mentioned users' do - expect(Feed.new(:mentions, alice).get(10).map(&:id)).to include status.id - end - it 'delivers status to hashtag' do expect(Tag.find_by!(name: 'test').statuses.pluck(:id)).to include status.id end