From 17340365bbf057314d3cb19ec20c5d74b52c6395 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Wed, 2 Sep 2020 02:11:12 +0200 Subject: Add featured hashtags as an ActivityPub collection (#11595) --- .../activitypub/collections_controller.rb | 32 ++++++++++++---------- app/lib/activitypub/adapter.rb | 2 +- app/serializers/activitypub/actor_serializer.rb | 6 +++- .../activitypub/collection_serializer.rb | 2 ++ app/serializers/activitypub/hashtag_serializer.rb | 23 ++++++++++++++++ 5 files changed, 49 insertions(+), 16 deletions(-) create mode 100644 app/serializers/activitypub/hashtag_serializer.rb (limited to 'app') diff --git a/app/controllers/activitypub/collections_controller.rb b/app/controllers/activitypub/collections_controller.rb index 380de54f5..c8b6dcc88 100644 --- a/app/controllers/activitypub/collections_controller.rb +++ b/app/controllers/activitypub/collections_controller.rb @@ -12,7 +12,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController def show expires_in 3.minutes, public: public_fetch_mode? - render_with_cache json: collection_presenter, content_type: 'application/activity+json', serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, skip_activities: true + render_with_cache json: collection_presenter, content_type: 'application/activity+json', serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter end private @@ -20,17 +20,9 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController def set_items case params[:id] when 'featured' - @items = begin - # Because in public fetch mode we cache the response, there would be no - # benefit from performing the check below, since a blocked account or domain - # would likely be served the cache from the reverse proxy anyway - - if authorized_fetch_mode? && !signed_request_account.nil? && (@account.blocking?(signed_request_account) || (!signed_request_account.domain.nil? && @account.domain_blocking?(signed_request_account.domain))) - [] - else - cache_collection(@account.pinned_statuses, Status) - end - end + @items = for_signed_account { cache_collection(@account.pinned_statuses, Status) } + when 'tags' + @items = for_signed_account { @account.featured_tags } when 'devices' @items = @account.devices else @@ -40,7 +32,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController def set_size case params[:id] - when 'featured', 'devices' + when 'featured', 'devices', 'tags' @size = @items.size else not_found @@ -51,7 +43,7 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController case params[:id] when 'featured' @type = :ordered - when 'devices' + when 'devices', 'tags' @type = :unordered else not_found @@ -66,4 +58,16 @@ class ActivityPub::CollectionsController < ActivityPub::BaseController items: @items ) end + + def for_signed_account + # Because in public fetch mode we cache the response, there would be no + # benefit from performing the check below, since a blocked account or domain + # would likely be served the cache from the reverse proxy anyway + + if authorized_fetch_mode? && !signed_request_account.nil? && (@account.blocking?(signed_request_account) || (!signed_request_account.domain.nil? && @account.domain_blocking?(signed_request_account.domain))) + [] + else + yield + end + end end diff --git a/app/lib/activitypub/adapter.rb b/app/lib/activitypub/adapter.rb index 634ed29fa..9a786c9a4 100644 --- a/app/lib/activitypub/adapter.rb +++ b/app/lib/activitypub/adapter.rb @@ -13,7 +13,7 @@ class ActivityPub::Adapter < ActiveModelSerializers::Adapter::Base moved_to: { 'movedTo' => { '@id' => 'as:movedTo', '@type' => '@id' } }, also_known_as: { 'alsoKnownAs' => { '@id' => 'as:alsoKnownAs', '@type' => '@id' } }, emoji: { 'toot' => 'http://joinmastodon.org/ns#', 'Emoji' => 'toot:Emoji' }, - featured: { 'toot' => 'http://joinmastodon.org/ns#', 'featured' => { '@id' => 'toot:featured', '@type' => '@id' } }, + featured: { 'toot' => 'http://joinmastodon.org/ns#', 'featured' => { '@id' => 'toot:featured', '@type' => '@id' }, 'featuredTags' => { '@id' => 'toot:featuredTags', '@type' => '@id' } }, property_value: { 'schema' => 'http://schema.org#', 'PropertyValue' => 'schema:PropertyValue', 'value' => 'schema:value' }, atom_uri: { 'ostatus' => 'http://ostatus.org#', 'atomUri' => 'ostatus:atomUri' }, conversation: { 'ostatus' => 'http://ostatus.org#', 'inReplyToAtomUri' => 'ostatus:inReplyToAtomUri', 'conversation' => 'ostatus:conversation' }, diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index 627d4446b..da4a92728 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -10,7 +10,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer :discoverable, :olm attributes :id, :type, :following, :followers, - :inbox, :outbox, :featured, + :inbox, :outbox, :featured, :featured_tags, :preferred_username, :name, :summary, :url, :manually_approves_followers, :discoverable @@ -81,6 +81,10 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer account_collection_url(object, :featured) end + def featured_tags + account_collection_url(object, :tags) + end + def endpoints object end diff --git a/app/serializers/activitypub/collection_serializer.rb b/app/serializers/activitypub/collection_serializer.rb index ea7af5433..34026a6b5 100644 --- a/app/serializers/activitypub/collection_serializer.rb +++ b/app/serializers/activitypub/collection_serializer.rb @@ -16,6 +16,8 @@ class ActivityPub::CollectionSerializer < ActivityPub::Serializer ActivityPub::NoteSerializer when 'Device' ActivityPub::DeviceSerializer + when 'FeaturedTag' + ActivityPub::HashtagSerializer when 'ActivityPub::CollectionPresenter' ActivityPub::CollectionSerializer when 'String' diff --git a/app/serializers/activitypub/hashtag_serializer.rb b/app/serializers/activitypub/hashtag_serializer.rb new file mode 100644 index 000000000..1a56e4dfe --- /dev/null +++ b/app/serializers/activitypub/hashtag_serializer.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +class ActivityPub::HashtagSerializer < ActivityPub::Serializer + include RoutingHelper + + attributes :type, :href, :name + + def type + 'Hashtag' + end + + def name + "##{object.name}" + end + + def href + if object.class.name == 'FeaturedTag' + short_account_tag_url(object.account, object.tag) + else + tag_url(object) + end + end +end -- cgit From 33ad850c982bbe03214e2e2870751920721c23af Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Wed, 2 Sep 2020 09:13:10 +0900 Subject: Added account featured tags API (#11817) --- .../api/v1/accounts/featured_tags_controller.rb | 22 ++++++++++++++++++++++ .../rest/account_featured_tag_serializer.rb | 19 +++++++++++++++++++ config/routes.rb | 1 + 3 files changed, 42 insertions(+) create mode 100644 app/controllers/api/v1/accounts/featured_tags_controller.rb create mode 100644 app/serializers/rest/account_featured_tag_serializer.rb (limited to 'app') diff --git a/app/controllers/api/v1/accounts/featured_tags_controller.rb b/app/controllers/api/v1/accounts/featured_tags_controller.rb new file mode 100644 index 000000000..d6277261d --- /dev/null +++ b/app/controllers/api/v1/accounts/featured_tags_controller.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +class Api::V1::Accounts::FeaturedTagsController < Api::BaseController + before_action :set_account + before_action :set_featured_tags + + respond_to :json + + def index + render json: @featured_tags, each_serializer: REST::AccountFeaturedTagSerializer + end + + private + + def set_account + @account = Account.find(params[:account_id]) + end + + def set_featured_tags + @featured_tags = @account.featured_tags + end +end diff --git a/app/serializers/rest/account_featured_tag_serializer.rb b/app/serializers/rest/account_featured_tag_serializer.rb new file mode 100644 index 000000000..d8d5fd68c --- /dev/null +++ b/app/serializers/rest/account_featured_tag_serializer.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class REST::AccountFeaturedTagSerializer < ActiveModel::Serializer + include RoutingHelper + + attributes :id, :name, :url + + def id + object.tag.id.to_s + end + + def name + "##{object.name}" + end + + def url + short_account_tag_url(object.account, object.tag) + end +end diff --git a/config/routes.rb b/config/routes.rb index 2c39b36ed..8940101a4 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -430,6 +430,7 @@ Rails.application.routes.draw do resources :following, only: :index, controller: 'accounts/following_accounts' resources :lists, only: :index, controller: 'accounts/lists' resources :identity_proofs, only: :index, controller: 'accounts/identity_proofs' + resources :featured_tags, only: :index, controller: 'accounts/featured_tags' member do post :follow -- cgit From abee40b2322f191ce5da040c60cea1b0f09eee78 Mon Sep 17 00:00:00 2001 From: ThibG Date: Wed, 2 Sep 2020 18:42:50 +0200 Subject: Add outbox attribute to instance actor (#14721) It's not useful for now, but it's required by ActivityPub --- app/controllers/activitypub/outboxes_controller.rb | 20 ++++++++++++++++---- app/controllers/instance_actors_controller.rb | 2 +- app/serializers/activitypub/actor_serializer.rb | 2 +- config/routes.rb | 1 + 4 files changed, 19 insertions(+), 6 deletions(-) (limited to 'app') diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index c33c15255..e066860bf 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -20,9 +20,9 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def outbox_presenter if page_requested? ActivityPub::CollectionPresenter.new( - id: account_outbox_url(@account, page_params), + id: outbox_url(page_params), type: :ordered, - part_of: account_outbox_url(@account), + part_of: outbox_url, prev: prev_page, next: next_page, items: @statuses @@ -32,12 +32,20 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController id: account_outbox_url(@account), type: :ordered, size: @account.statuses_count, - first: account_outbox_url(@account, page: true), - last: account_outbox_url(@account, page: true, min_id: 0) + first: outbox_url(page: true), + last: outbox_url(page: true, min_id: 0) ) end end + def outbox_url(**kwargs) + if params[:account_username].present? + account_outbox_url(@account, **kwargs) + else + instance_actor_outbox_url(**kwargs) + end + end + def next_page account_outbox_url(@account, page: true, max_id: @statuses.last.id) if @statuses.size == LIMIT end @@ -65,4 +73,8 @@ class ActivityPub::OutboxesController < ActivityPub::BaseController def page_params { page: true, max_id: params[:max_id], min_id: params[:min_id] }.compact end + + def set_account + @account = params[:account_username].present? ? Account.find_local!(username_param) : Account.representative + end end diff --git a/app/controllers/instance_actors_controller.rb b/app/controllers/instance_actors_controller.rb index 6f02d6a35..4b074ca19 100644 --- a/app/controllers/instance_actors_controller.rb +++ b/app/controllers/instance_actors_controller.rb @@ -17,6 +17,6 @@ class InstanceActorsController < ApplicationController end def restrict_fields_to - %i(id type preferred_username inbox public_key endpoints url manually_approves_followers) + %i(id type preferred_username inbox outbox public_key endpoints url manually_approves_followers) end end diff --git a/app/serializers/activitypub/actor_serializer.rb b/app/serializers/activitypub/actor_serializer.rb index da4a92728..5d2741b17 100644 --- a/app/serializers/activitypub/actor_serializer.rb +++ b/app/serializers/activitypub/actor_serializer.rb @@ -74,7 +74,7 @@ class ActivityPub::ActorSerializer < ActivityPub::Serializer end def outbox - account_outbox_url(object) + object.instance_actor? ? instance_actor_outbox_url : account_outbox_url(object) end def featured diff --git a/config/routes.rb b/config/routes.rb index 8940101a4..c281a86e3 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -37,6 +37,7 @@ Rails.application.routes.draw do resource :instance_actor, path: 'actor', only: [:show] do resource :inbox, only: [:create], module: :activitypub + resource :outbox, only: [:show], module: :activitypub end devise_scope :user do -- cgit From 272aa4a10940db182c47e7523a34da951fd89f02 Mon Sep 17 00:00:00 2001 From: Takeshi Umeda Date: Fri, 4 Sep 2020 15:49:56 +0900 Subject: Fix direct visibility style for light theme (#14727) --- app/javascript/styles/mastodon-light/diff.scss | 8 -------- 1 file changed, 8 deletions(-) (limited to 'app') diff --git a/app/javascript/styles/mastodon-light/diff.scss b/app/javascript/styles/mastodon-light/diff.scss index 8c8d69fc4..6b81e7623 100644 --- a/app/javascript/styles/mastodon-light/diff.scss +++ b/app/javascript/styles/mastodon-light/diff.scss @@ -256,14 +256,6 @@ html { background: $ui-base-color; } -.status.status-direct { - background: lighten($ui-base-color, 4%); -} - -.focusable:focus .status.status-direct { - background: lighten($ui-base-color, 8%); -} - .detailed-status, .detailed-status__action-bar { background: $white; -- cgit From a6121a159c5305ea9faa95743a50babb23ab41cd Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Fri, 4 Sep 2020 20:22:40 +0200 Subject: Remove obsolete IndexedDB operations from web UI (#14730) Storing objects in IndexedDB was disabled in #7932, but we were still trying to read objects from it before making an API call --- app/javascript/mastodon/actions/accounts.js | 37 +---------------- app/javascript/mastodon/actions/statuses.js | 64 ++--------------------------- 2 files changed, 5 insertions(+), 96 deletions(-) (limited to 'app') diff --git a/app/javascript/mastodon/actions/accounts.js b/app/javascript/mastodon/actions/accounts.js index cb2c682a4..d28f7dad8 100644 --- a/app/javascript/mastodon/actions/accounts.js +++ b/app/javascript/mastodon/actions/accounts.js @@ -1,6 +1,5 @@ import api, { getLinks } from '../api'; -import openDB from '../storage/db'; -import { importAccount, importFetchedAccount, importFetchedAccounts } from './importer'; +import { importFetchedAccount, importFetchedAccounts } from './importer'; export const ACCOUNT_FETCH_REQUEST = 'ACCOUNT_FETCH_REQUEST'; export const ACCOUNT_FETCH_SUCCESS = 'ACCOUNT_FETCH_SUCCESS'; @@ -74,45 +73,13 @@ export const FOLLOW_REQUEST_REJECT_REQUEST = 'FOLLOW_REQUEST_REJECT_REQUEST'; export const FOLLOW_REQUEST_REJECT_SUCCESS = 'FOLLOW_REQUEST_REJECT_SUCCESS'; export const FOLLOW_REQUEST_REJECT_FAIL = 'FOLLOW_REQUEST_REJECT_FAIL'; -function getFromDB(dispatch, getState, index, id) { - return new Promise((resolve, reject) => { - const request = index.get(id); - - request.onerror = reject; - - request.onsuccess = () => { - if (!request.result) { - reject(); - return; - } - - dispatch(importAccount(request.result)); - resolve(request.result.moved && getFromDB(dispatch, getState, index, request.result.moved)); - }; - }); -} - export function fetchAccount(id) { return (dispatch, getState) => { dispatch(fetchRelationships([id])); - - if (getState().getIn(['accounts', id], null) !== null) { - return; - } - dispatch(fetchAccountRequest(id)); - openDB().then(db => getFromDB( - dispatch, - getState, - db.transaction('accounts', 'read').objectStore('accounts').index('id'), - id, - ).then(() => db.close(), error => { - db.close(); - throw error; - })).catch(() => api(getState).get(`/api/v1/accounts/${id}`).then(response => { + api(getState).get(`/api/v1/accounts/${id}`).then(response => { dispatch(importFetchedAccount(response.data)); - })).then(() => { dispatch(fetchAccountSuccess()); }).catch(error => { dispatch(fetchAccountFail(id, error)); diff --git a/app/javascript/mastodon/actions/statuses.js b/app/javascript/mastodon/actions/statuses.js index e565e0b0a..3fc7c0702 100644 --- a/app/javascript/mastodon/actions/statuses.js +++ b/app/javascript/mastodon/actions/statuses.js @@ -1,9 +1,7 @@ import api from '../api'; -import openDB from '../storage/db'; -import { evictStatus } from '../storage/modifier'; import { deleteFromTimelines } from './timelines'; -import { importFetchedStatus, importFetchedStatuses, importAccount, importStatus, importFetchedAccount } from './importer'; +import { importFetchedStatus, importFetchedStatuses, importFetchedAccount } from './importer'; import { ensureComposeIsVisible } from './compose'; export const STATUS_FETCH_REQUEST = 'STATUS_FETCH_REQUEST'; @@ -40,48 +38,6 @@ export function fetchStatusRequest(id, skipLoading) { }; }; -function getFromDB(dispatch, getState, accountIndex, index, id) { - return new Promise((resolve, reject) => { - const request = index.get(id); - - request.onerror = reject; - - request.onsuccess = () => { - const promises = []; - - if (!request.result) { - reject(); - return; - } - - dispatch(importStatus(request.result)); - - if (getState().getIn(['accounts', request.result.account], null) === null) { - promises.push(new Promise((accountResolve, accountReject) => { - const accountRequest = accountIndex.get(request.result.account); - - accountRequest.onerror = accountReject; - accountRequest.onsuccess = () => { - if (!request.result) { - accountReject(); - return; - } - - dispatch(importAccount(accountRequest.result)); - accountResolve(); - }; - })); - } - - if (request.result.reblog && getState().getIn(['statuses', request.result.reblog], null) === null) { - promises.push(getFromDB(dispatch, getState, accountIndex, index, request.result.reblog)); - } - - resolve(Promise.all(promises)); - }; - }); -} - export function fetchStatus(id) { return (dispatch, getState) => { const skipLoading = getState().getIn(['statuses', id], null) !== null; @@ -94,23 +50,10 @@ export function fetchStatus(id) { dispatch(fetchStatusRequest(id, skipLoading)); - openDB().then(db => { - const transaction = db.transaction(['accounts', 'statuses'], 'read'); - const accountIndex = transaction.objectStore('accounts').index('id'); - const index = transaction.objectStore('statuses').index('id'); - - return getFromDB(dispatch, getState, accountIndex, index, id).then(() => { - db.close(); - }, error => { - db.close(); - throw error; - }); - }).then(() => { - dispatch(fetchStatusSuccess(skipLoading)); - }, () => api(getState).get(`/api/v1/statuses/${id}`).then(response => { + api(getState).get(`/api/v1/statuses/${id}`).then(response => { dispatch(importFetchedStatus(response.data)); dispatch(fetchStatusSuccess(skipLoading)); - })).catch(error => { + }).catch(error => { dispatch(fetchStatusFail(id, error, skipLoading)); }); }; @@ -152,7 +95,6 @@ export function deleteStatus(id, routerHistory, withRedraft = false) { dispatch(deleteStatusRequest(id)); api(getState).delete(`/api/v1/statuses/${id}`).then(response => { - evictStatus(id); dispatch(deleteStatusSuccess(id)); dispatch(deleteFromTimelines(id)); dispatch(importFetchedAccount(response.data.account)); -- cgit From e8bc187845b78e4a94894c69ecf930a524ad2056 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Mon, 7 Sep 2020 11:02:04 +0200 Subject: Refactor how public and tag timelines are queried (#14728) --- .../api/v1/timelines/public_controller.rb | 29 ++- app/controllers/api/v1/timelines/tag_controller.rb | 34 +-- app/controllers/tags_controller.rb | 31 +-- app/models/public_feed.rb | 90 ++++++++ app/models/status.rb | 67 +----- app/models/tag_feed.rb | 57 +++++ app/services/hashtag_query_service.rb | 22 -- spec/models/public_feed_spec.rb | 212 +++++++++++++++++++ spec/models/status_spec.rb | 235 --------------------- spec/models/tag_feed_spec.rb | 68 ++++++ spec/services/fan_out_on_write_service_spec.rb | 4 +- spec/services/hashtag_query_service_spec.rb | 60 ------ 12 files changed, 480 insertions(+), 429 deletions(-) create mode 100644 app/models/public_feed.rb create mode 100644 app/models/tag_feed.rb delete mode 100644 app/services/hashtag_query_service.rb create mode 100644 spec/models/public_feed_spec.rb create mode 100644 spec/models/tag_feed_spec.rb delete mode 100644 spec/services/hashtag_query_service_spec.rb (limited to 'app') diff --git a/app/controllers/api/v1/timelines/public_controller.rb b/app/controllers/api/v1/timelines/public_controller.rb index 26d877b00..d253b744f 100644 --- a/app/controllers/api/v1/timelines/public_controller.rb +++ b/app/controllers/api/v1/timelines/public_controller.rb @@ -20,26 +20,25 @@ class Api::V1::Timelines::PublicController < Api::BaseController end def cached_public_statuses_page - cache_collection_paginated_by_id( - public_statuses, - Status, - limit_param(DEFAULT_STATUSES_LIMIT), - params_slice(:max_id, :since_id, :min_id) - ) + cache_collection(public_statuses, Status) end def public_statuses - statuses = public_timeline_statuses - - if truthy_param?(:only_media) - statuses.joins(:media_attachments).group(:id) - else - statuses - end + public_feed.get( + limit_param(DEFAULT_STATUSES_LIMIT), + params[:max_id], + params[:since_id], + params[:min_id] + ) end - def public_timeline_statuses - Status.as_public_timeline(current_account, truthy_param?(:remote) ? :remote : truthy_param?(:local)) + def public_feed + PublicFeed.new( + current_account, + local: truthy_param?(:local), + remote: truthy_param?(:remote), + only_media: truthy_param?(:only_media) + ) end def insert_pagination_headers diff --git a/app/controllers/api/v1/timelines/tag_controller.rb b/app/controllers/api/v1/timelines/tag_controller.rb index 76f7d3590..64a1db58d 100644 --- a/app/controllers/api/v1/timelines/tag_controller.rb +++ b/app/controllers/api/v1/timelines/tag_controller.rb @@ -20,23 +20,29 @@ class Api::V1::Timelines::TagController < Api::BaseController end def cached_tagged_statuses - if @tag.nil? - [] - else - statuses = tag_timeline_statuses - statuses = statuses.joins(:media_attachments) if truthy_param?(:only_media) - - cache_collection_paginated_by_id( - statuses, - Status, - limit_param(DEFAULT_STATUSES_LIMIT), - params_slice(:max_id, :since_id, :min_id) - ) - end + @tag.nil? ? [] : cache_collection(tag_timeline_statuses, Status) end def tag_timeline_statuses - HashtagQueryService.new.call(@tag, params.slice(:any, :all, :none), current_account, truthy_param?(:local)) + tag_feed.get( + limit_param(DEFAULT_STATUSES_LIMIT), + params[:max_id], + params[:since_id], + params[:min_id] + ) + end + + def tag_feed + TagFeed.new( + @tag, + current_account, + any: params[:any], + all: params[:all], + none: params[:none], + local: truthy_param?(:local), + remote: truthy_param?(:remote), + only_media: truthy_param?(:only_media) + ) end def insert_pagination_headers diff --git a/app/controllers/tags_controller.rb b/app/controllers/tags_controller.rb index 6426a7d69..6616ba107 100644 --- a/app/controllers/tags_controller.rb +++ b/app/controllers/tags_controller.rb @@ -10,8 +10,9 @@ class TagsController < ApplicationController before_action :require_signature!, if: -> { request.format == :json && authorized_fetch_mode? } before_action :authenticate_user!, if: :whitelist_mode? - before_action :set_tag before_action :set_local + before_action :set_tag + before_action :set_statuses before_action :set_body_classes before_action :set_instance_presenter @@ -25,20 +26,11 @@ class TagsController < ApplicationController format.rss do expires_in 0, public: true - - limit = params[:limit].present? ? [params[:limit].to_i, PAGE_SIZE_MAX].min : PAGE_SIZE - @statuses = HashtagQueryService.new.call(@tag, filter_params, nil, @local).limit(limit) - @statuses = cache_collection(@statuses, Status) - render xml: RSS::TagSerializer.render(@tag, @statuses) end format.json do expires_in 3.minutes, public: public_fetch_mode? - - @statuses = HashtagQueryService.new.call(@tag, filter_params, current_account, @local).paginate_by_max_id(PAGE_SIZE, params[:max_id]) - @statuses = cache_collection(@statuses, Status) - render json: collection_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' end end @@ -54,6 +46,15 @@ class TagsController < ApplicationController @local = truthy_param?(:local) end + def set_statuses + case request.format&.to_sym + when :json + @statuses = cache_collection(TagFeed.new(@tag, current_account, local: @local).get(PAGE_SIZE, params[:max_id], params[:since_id], params[:min_id]), Status) + when :rss + @statuses = cache_collection(TagFeed.new(@tag, nil, local: @local).get(limit_param), Status) + end + end + def set_body_classes @body_classes = 'with-modals' end @@ -62,16 +63,16 @@ class TagsController < ApplicationController @instance_presenter = InstancePresenter.new end + def limit_param + params[:limit].present? ? [params[:limit].to_i, PAGE_SIZE_MAX].min : PAGE_SIZE + end + def collection_presenter ActivityPub::CollectionPresenter.new( - id: tag_url(@tag, filter_params), + id: tag_url(@tag), type: :ordered, size: @tag.statuses.count, items: @statuses.map { |s| ActivityPub::TagManager.instance.uri_for(s) } ) end - - def filter_params - params.slice(:any, :all, :none).permit(:any, :all, :none) - end end diff --git a/app/models/public_feed.rb b/app/models/public_feed.rb new file mode 100644 index 000000000..c8ce1a140 --- /dev/null +++ b/app/models/public_feed.rb @@ -0,0 +1,90 @@ +# frozen_string_literal: true + +class PublicFeed < Feed + # @param [Account] account + # @param [Hash] options + # @option [Boolean] :with_replies + # @option [Boolean] :with_reblogs + # @option [Boolean] :local + # @option [Boolean] :remote + # @option [Boolean] :only_media + def initialize(account, options = {}) + @account = account + @options = options + end + + # @param [Integer] limit + # @param [Integer] max_id + # @param [Integer] since_id + # @param [Integer] min_id + # @return [Array] + def get(limit, max_id = nil, since_id = nil, min_id = nil) + scope = public_scope + + scope.merge!(without_replies_scope) unless with_replies? + scope.merge!(without_reblogs_scope) unless with_reblogs? + scope.merge!(local_only_scope) if local_only? + scope.merge!(remote_only_scope) if remote_only? + scope.merge!(account_filters_scope) if account? + scope.merge!(media_only_scope) if media_only? + + scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) + end + + private + + def with_reblogs? + @options[:with_reblogs] + end + + def with_replies? + @options[:with_replies] + end + + def local_only? + @options[:local] + end + + def remote_only? + @options[:remote] + end + + def account? + @account.present? + end + + def media_only? + @options[:only_media] + end + + def public_scope + Status.with_public_visibility.joins(:account).merge(Account.without_suspended.without_silenced) + end + + def local_only_scope + Status.local + end + + def remote_only_scope + Status.remote + end + + def without_replies_scope + Status.without_replies + end + + def without_reblogs_scope + Status.without_reblogs + end + + def media_only_scope + Status.joins(:media_attachments).group(:id) + end + + def account_filters_scope + Status.not_excluded_by_account(@account).tap do |scope| + scope.merge!(Status.not_domain_blocked_by_account(@account)) unless local_only? + scope.merge!(Status.in_chosen_languages(@account)) if @account.chosen_languages.present? + end + end +end diff --git a/app/models/status.rb b/app/models/status.rb index 71596ec2f..c6e16ff75 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -85,12 +85,12 @@ class Status < ApplicationRecord scope :recent, -> { reorder(id: :desc) } scope :remote, -> { where(local: false).where.not(uri: nil) } scope :local, -> { where(local: true).or(where(uri: nil)) } - scope :with_accounts, ->(ids) { where(id: ids).includes(:account) } scope :without_replies, -> { where('statuses.reply = FALSE OR statuses.in_reply_to_account_id = statuses.account_id') } scope :without_reblogs, -> { where('statuses.reblog_of_id IS NULL') } scope :with_public_visibility, -> { where(visibility: :public) } scope :tagged_with, ->(tag) { joins(:statuses_tags).where(statuses_tags: { tag_id: tag }) } + scope :in_chosen_languages, ->(account) { where(language: nil).or where(language: account.chosen_languages) } scope :excluding_silenced_accounts, -> { left_outer_joins(:account).where(accounts: { silenced_at: nil }) } scope :including_silenced_accounts, -> { left_outer_joins(:account).where.not(accounts: { silenced_at: nil }) } scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } @@ -277,26 +277,6 @@ class Status < ApplicationRecord visibilities.keys - %w(direct limited) end - def in_chosen_languages(account) - where(language: nil).or where(language: account.chosen_languages) - end - - def as_public_timeline(account = nil, local_only = false) - query = timeline_scope(local_only).without_replies - - apply_timeline_filters(query, account, [:local, true].include?(local_only)) - end - - def as_tag_timeline(tag, account = nil, local_only = false) - query = timeline_scope(local_only).tagged_with(tag) - - apply_timeline_filters(query, account, local_only) - end - - def as_outbox_timeline(account) - where(account: account, visibility: :public) - end - def favourites_map(status_ids, account_id) Favourite.select('status_id').where(status_id: status_ids).where(account_id: account_id).each_with_object({}) { |f, h| h[f.status_id] = true } end @@ -373,51 +353,6 @@ class Status < ApplicationRecord status&.distributable? ? status : nil end.compact end - - private - - def timeline_scope(scope = false) - starting_scope = case scope - when :local, true - Status.local - when :remote - Status.remote - else - Status - end - - starting_scope - .with_public_visibility - .without_reblogs - end - - def apply_timeline_filters(query, account, local_only) - if account.nil? - filter_timeline_default(query) - else - filter_timeline_for_account(query, account, local_only) - end - end - - def filter_timeline_for_account(query, account, local_only) - query = query.not_excluded_by_account(account) - query = query.not_domain_blocked_by_account(account) unless local_only - query = query.in_chosen_languages(account) if account.chosen_languages.present? - query.merge(account_silencing_filter(account)) - end - - def filter_timeline_default(query) - query.excluding_silenced_accounts - end - - def account_silencing_filter(account) - if account.silenced? - including_myself = left_outer_joins(:account).where(account_id: account.id).references(:accounts) - excluding_silenced_accounts.or(including_myself) - else - excluding_silenced_accounts - end - end end def status_stat diff --git a/app/models/tag_feed.rb b/app/models/tag_feed.rb new file mode 100644 index 000000000..50634fe83 --- /dev/null +++ b/app/models/tag_feed.rb @@ -0,0 +1,57 @@ +# frozen_string_literal: true + +class TagFeed < PublicFeed + LIMIT_PER_MODE = 4 + + # @param [Tag] tag + # @param [Account] account + # @param [Hash] options + # @option [Enumerable] :any + # @option [Enumerable] :all + # @option [Enumerable] :none + # @option [Boolean] :local + # @option [Boolean] :remote + # @option [Boolean] :only_media + def initialize(tag, account, options = {}) + @tag = tag + @account = account + @options = options + end + + # @param [Integer] limit + # @param [Integer] max_id + # @param [Integer] since_id + # @param [Integer] min_id + # @return [Array] + def get(limit, max_id = nil, since_id = nil, min_id = nil) + scope = public_scope + + scope.merge!(tagged_with_any_scope) + scope.merge!(tagged_with_all_scope) + scope.merge!(tagged_with_none_scope) + scope.merge!(local_only_scope) if local_only? + scope.merge!(remote_only_scope) if remote_only? + scope.merge!(account_filters_scope) if account? + scope.merge!(media_only_scope) if media_only? + + scope.cache_ids.to_a_paginated_by_id(limit, max_id: max_id, since_id: since_id, min_id: min_id) + end + + private + + def tagged_with_any_scope + Status.group(:id).tagged_with(tags_for(Array(@tag.name) | Array(@options[:any]))) + end + + def tagged_with_all_scope + Status.group(:id).tagged_with_all(tags_for(@options[:all])) + end + + def tagged_with_none_scope + Status.group(:id).tagged_with_none(tags_for(@options[:none])) + end + + def tags_for(names) + Tag.matching_name(Array(names).take(LIMIT_PER_MODE)) if names.present? + end +end diff --git a/app/services/hashtag_query_service.rb b/app/services/hashtag_query_service.rb deleted file mode 100644 index 0bdf60221..000000000 --- a/app/services/hashtag_query_service.rb +++ /dev/null @@ -1,22 +0,0 @@ -# frozen_string_literal: true - -class HashtagQueryService < BaseService - LIMIT_PER_MODE = 4 - - def call(tag, params, account = nil, local = false) - tags = tags_for(Array(tag.name) | Array(params[:any])).pluck(:id) - all = tags_for(params[:all]) - none = tags_for(params[:none]) - - Status.group(:id) - .as_tag_timeline(tags, account, local) - .tagged_with_all(all) - .tagged_with_none(none) - end - - private - - def tags_for(names) - Tag.matching_name(Array(names).take(LIMIT_PER_MODE)) if names.present? - end -end diff --git a/spec/models/public_feed_spec.rb b/spec/models/public_feed_spec.rb new file mode 100644 index 000000000..0392a582c --- /dev/null +++ b/spec/models/public_feed_spec.rb @@ -0,0 +1,212 @@ +require 'rails_helper' + +RSpec.describe PublicFeed, type: :model do + let(:account) { Fabricate(:account) } + + describe '#get' do + subject { described_class.new(nil).get(20).map(&:id) } + + it 'only includes statuses with public visibility' do + public_status = Fabricate(:status, visibility: :public) + private_status = Fabricate(:status, visibility: :private) + + expect(subject).to include(public_status.id) + expect(subject).not_to include(private_status.id) + end + + it 'does not include replies' do + status = Fabricate(:status) + reply = Fabricate(:status, in_reply_to_id: status.id) + + expect(subject).to include(status.id) + expect(subject).not_to include(reply.id) + end + + it 'does not include boosts' do + status = Fabricate(:status) + boost = Fabricate(:status, reblog_of_id: status.id) + + expect(subject).to include(status.id) + expect(subject).not_to include(boost.id) + end + + it 'filters out silenced accounts' do + account = Fabricate(:account) + silenced_account = Fabricate(:account, silenced: true) + status = Fabricate(:status, account: account) + silenced_status = Fabricate(:status, account: silenced_account) + + expect(subject).to include(status.id) + expect(subject).not_to include(silenced_status.id) + end + + context 'without local_only option' do + let(:viewer) { nil } + + let!(:local_account) { Fabricate(:account, domain: nil) } + let!(:remote_account) { Fabricate(:account, domain: 'test.com') } + let!(:local_status) { Fabricate(:status, account: local_account) } + let!(:remote_status) { Fabricate(:status, account: remote_account) } + + subject { described_class.new(viewer).get(20).map(&:id) } + + context 'without a viewer' do + let(:viewer) { nil } + + it 'includes remote instances statuses' do + expect(subject).to include(remote_status.id) + end + + it 'includes local statuses' do + expect(subject).to include(local_status.id) + end + end + + context 'with a viewer' do + let(:viewer) { Fabricate(:account, username: 'viewer') } + + it 'includes remote instances statuses' do + expect(subject).to include(remote_status.id) + end + + it 'includes local statuses' do + expect(subject).to include(local_status.id) + end + end + end + + context 'with a local_only option set' do + let!(:local_account) { Fabricate(:account, domain: nil) } + let!(:remote_account) { Fabricate(:account, domain: 'test.com') } + let!(:local_status) { Fabricate(:status, account: local_account) } + let!(:remote_status) { Fabricate(:status, account: remote_account) } + + subject { described_class.new(viewer, local: true).get(20).map(&:id) } + + context 'without a viewer' do + let(:viewer) { nil } + + it 'does not include remote instances statuses' do + expect(subject).to include(local_status.id) + expect(subject).not_to include(remote_status.id) + end + end + + context 'with a viewer' do + let(:viewer) { Fabricate(:account, username: 'viewer') } + + it 'does not include remote instances statuses' do + expect(subject).to include(local_status.id) + expect(subject).not_to include(remote_status.id) + end + + it 'is not affected by personal domain blocks' do + viewer.block_domain!('test.com') + expect(subject).to include(local_status.id) + expect(subject).not_to include(remote_status.id) + end + end + end + + context 'with a remote_only option set' do + let!(:local_account) { Fabricate(:account, domain: nil) } + let!(:remote_account) { Fabricate(:account, domain: 'test.com') } + let!(:local_status) { Fabricate(:status, account: local_account) } + let!(:remote_status) { Fabricate(:status, account: remote_account) } + + subject { described_class.new(viewer, remote: true).get(20).map(&:id) } + + context 'without a viewer' do + let(:viewer) { nil } + + it 'does not include local instances statuses' do + expect(subject).not_to include(local_status.id) + expect(subject).to include(remote_status.id) + end + end + + context 'with a viewer' do + let(:viewer) { Fabricate(:account, username: 'viewer') } + + it 'does not include local instances statuses' do + expect(subject).not_to include(local_status.id) + expect(subject).to include(remote_status.id) + end + end + end + + describe 'with an account passed in' do + before do + @account = Fabricate(:account) + end + + subject { described_class.new(@account).get(20).map(&:id) } + + it 'excludes statuses from accounts blocked by the account' do + blocked = Fabricate(:account) + @account.block!(blocked) + blocked_status = Fabricate(:status, account: blocked) + + expect(subject).not_to include(blocked_status.id) + end + + it 'excludes statuses from accounts who have blocked the account' do + blocker = Fabricate(:account) + blocker.block!(@account) + blocked_status = Fabricate(:status, account: blocker) + + expect(subject).not_to include(blocked_status.id) + end + + it 'excludes statuses from accounts muted by the account' do + muted = Fabricate(:account) + @account.mute!(muted) + muted_status = Fabricate(:status, account: muted) + + expect(subject).not_to include(muted_status.id) + end + + it 'excludes statuses from accounts from personally blocked domains' do + blocked = Fabricate(:account, domain: 'example.com') + @account.block_domain!(blocked.domain) + blocked_status = Fabricate(:status, account: blocked) + + expect(subject).not_to include(blocked_status.id) + end + + context 'with language preferences' do + it 'excludes statuses in languages not allowed by the account user' do + user = Fabricate(:user, chosen_languages: [:en, :es]) + @account.update(user: user) + en_status = Fabricate(:status, language: 'en') + es_status = Fabricate(:status, language: 'es') + fr_status = Fabricate(:status, language: 'fr') + + expect(subject).to include(en_status.id) + expect(subject).to include(es_status.id) + expect(subject).not_to include(fr_status.id) + end + + it 'includes all languages when user does not have a setting' do + user = Fabricate(:user, chosen_languages: nil) + @account.update(user: user) + + en_status = Fabricate(:status, language: 'en') + es_status = Fabricate(:status, language: 'es') + + expect(subject).to include(en_status.id) + expect(subject).to include(es_status.id) + end + + it 'includes all languages when account does not have a user' do + expect(@account.user).to be_nil + en_status = Fabricate(:status, language: 'en') + es_status = Fabricate(:status, language: 'es') + + expect(subject).to include(en_status.id) + expect(subject).to include(es_status.id) + end + end + end + end +end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 4aee14cbd..20fb894e7 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -267,241 +267,6 @@ RSpec.describe Status, type: :model do end end - describe '.as_public_timeline' do - it 'only includes statuses with public visibility' do - public_status = Fabricate(:status, visibility: :public) - private_status = Fabricate(:status, visibility: :private) - - results = Status.as_public_timeline - expect(results).to include(public_status) - expect(results).not_to include(private_status) - end - - it 'does not include replies' do - status = Fabricate(:status) - reply = Fabricate(:status, in_reply_to_id: status.id) - - results = Status.as_public_timeline - expect(results).to include(status) - expect(results).not_to include(reply) - end - - it 'does not include boosts' do - status = Fabricate(:status) - boost = Fabricate(:status, reblog_of_id: status.id) - - results = Status.as_public_timeline - expect(results).to include(status) - expect(results).not_to include(boost) - end - - it 'filters out silenced accounts' do - account = Fabricate(:account) - silenced_account = Fabricate(:account, silenced: true) - status = Fabricate(:status, account: account) - silenced_status = Fabricate(:status, account: silenced_account) - - results = Status.as_public_timeline - expect(results).to include(status) - expect(results).not_to include(silenced_status) - end - - context 'without local_only option' do - let(:viewer) { nil } - - let!(:local_account) { Fabricate(:account, domain: nil) } - let!(:remote_account) { Fabricate(:account, domain: 'test.com') } - let!(:local_status) { Fabricate(:status, account: local_account) } - let!(:remote_status) { Fabricate(:status, account: remote_account) } - - subject { Status.as_public_timeline(viewer, false) } - - context 'without a viewer' do - let(:viewer) { nil } - - it 'includes remote instances statuses' do - expect(subject).to include(remote_status) - end - - it 'includes local statuses' do - expect(subject).to include(local_status) - end - end - - context 'with a viewer' do - let(:viewer) { Fabricate(:account, username: 'viewer') } - - it 'includes remote instances statuses' do - expect(subject).to include(remote_status) - end - - it 'includes local statuses' do - expect(subject).to include(local_status) - end - end - end - - context 'with a local_only option set' do - let!(:local_account) { Fabricate(:account, domain: nil) } - let!(:remote_account) { Fabricate(:account, domain: 'test.com') } - let!(:local_status) { Fabricate(:status, account: local_account) } - let!(:remote_status) { Fabricate(:status, account: remote_account) } - - subject { Status.as_public_timeline(viewer, true) } - - context 'without a viewer' do - let(:viewer) { nil } - - it 'does not include remote instances statuses' do - expect(subject).to include(local_status) - expect(subject).not_to include(remote_status) - end - end - - context 'with a viewer' do - let(:viewer) { Fabricate(:account, username: 'viewer') } - - it 'does not include remote instances statuses' do - expect(subject).to include(local_status) - expect(subject).not_to include(remote_status) - end - - it 'is not affected by personal domain blocks' do - viewer.block_domain!('test.com') - expect(subject).to include(local_status) - expect(subject).not_to include(remote_status) - end - end - end - - context 'with a remote_only option set' do - let!(:local_account) { Fabricate(:account, domain: nil) } - let!(:remote_account) { Fabricate(:account, domain: 'test.com') } - let!(:local_status) { Fabricate(:status, account: local_account) } - let!(:remote_status) { Fabricate(:status, account: remote_account) } - - subject { Status.as_public_timeline(viewer, :remote) } - - context 'without a viewer' do - let(:viewer) { nil } - - it 'does not include local instances statuses' do - expect(subject).not_to include(local_status) - expect(subject).to include(remote_status) - end - end - - context 'with a viewer' do - let(:viewer) { Fabricate(:account, username: 'viewer') } - - it 'does not include local instances statuses' do - expect(subject).not_to include(local_status) - expect(subject).to include(remote_status) - end - end - end - - describe 'with an account passed in' do - before do - @account = Fabricate(:account) - end - - it 'excludes statuses from accounts blocked by the account' do - blocked = Fabricate(:account) - Fabricate(:block, account: @account, target_account: blocked) - blocked_status = Fabricate(:status, account: blocked) - - results = Status.as_public_timeline(@account) - expect(results).not_to include(blocked_status) - end - - it 'excludes statuses from accounts who have blocked the account' do - blocked = Fabricate(:account) - Fabricate(:block, account: blocked, target_account: @account) - blocked_status = Fabricate(:status, account: blocked) - - results = Status.as_public_timeline(@account) - expect(results).not_to include(blocked_status) - end - - it 'excludes statuses from accounts muted by the account' do - muted = Fabricate(:account) - Fabricate(:mute, account: @account, target_account: muted) - muted_status = Fabricate(:status, account: muted) - - results = Status.as_public_timeline(@account) - expect(results).not_to include(muted_status) - end - - it 'excludes statuses from accounts from personally blocked domains' do - blocked = Fabricate(:account, domain: 'example.com') - @account.block_domain!(blocked.domain) - blocked_status = Fabricate(:status, account: blocked) - - results = Status.as_public_timeline(@account) - expect(results).not_to include(blocked_status) - end - - context 'with language preferences' do - it 'excludes statuses in languages not allowed by the account user' do - user = Fabricate(:user, chosen_languages: [:en, :es]) - @account.update(user: user) - en_status = Fabricate(:status, language: 'en') - es_status = Fabricate(:status, language: 'es') - fr_status = Fabricate(:status, language: 'fr') - - results = Status.as_public_timeline(@account) - expect(results).to include(en_status) - expect(results).to include(es_status) - expect(results).not_to include(fr_status) - end - - it 'includes all languages when user does not have a setting' do - user = Fabricate(:user, chosen_languages: nil) - @account.update(user: user) - - en_status = Fabricate(:status, language: 'en') - es_status = Fabricate(:status, language: 'es') - - results = Status.as_public_timeline(@account) - expect(results).to include(en_status) - expect(results).to include(es_status) - end - - it 'includes all languages when account does not have a user' do - expect(@account.user).to be_nil - en_status = Fabricate(:status, language: 'en') - es_status = Fabricate(:status, language: 'es') - - results = Status.as_public_timeline(@account) - expect(results).to include(en_status) - expect(results).to include(es_status) - end - end - end - end - - describe '.as_tag_timeline' do - it 'includes statuses with a tag' do - tag = Fabricate(:tag) - status = Fabricate(:status, tags: [tag]) - other = Fabricate(:status) - - results = Status.as_tag_timeline(tag) - expect(results).to include(status) - expect(results).not_to include(other) - end - - it 'allows replies to be included' do - original = Fabricate(:status) - tag = Fabricate(:tag) - status = Fabricate(:status, tags: [tag], in_reply_to_id: original.id) - - results = Status.as_tag_timeline(tag) - expect(results).to include(status) - end - end - describe '.permitted_for' do subject { described_class.permitted_for(target_account, account).pluck(:visibility) } diff --git a/spec/models/tag_feed_spec.rb b/spec/models/tag_feed_spec.rb new file mode 100644 index 000000000..17d88eb99 --- /dev/null +++ b/spec/models/tag_feed_spec.rb @@ -0,0 +1,68 @@ +require 'rails_helper' + +describe TagFeed, type: :service do + describe '#get' do + let(:account) { Fabricate(:account) } + let(:tag1) { Fabricate(:tag) } + let(:tag2) { Fabricate(:tag) } + let!(:status1) { Fabricate(:status, tags: [tag1]) } + let!(:status2) { Fabricate(:status, tags: [tag2]) } + let!(:both) { Fabricate(:status, tags: [tag1, tag2]) } + + it 'can add tags in "any" mode' do + results = described_class.new(tag1, nil, any: [tag2.name]).get(20) + expect(results).to include status1 + expect(results).to include status2 + expect(results).to include both + end + + it 'can remove tags in "all" mode' do + results = described_class.new(tag1, nil, all: [tag2.name]).get(20) + expect(results).to_not include status1 + expect(results).to_not include status2 + expect(results).to include both + end + + it 'can remove tags in "none" mode' do + results = described_class.new(tag1, nil, none: [tag2.name]).get(20) + expect(results).to include status1 + expect(results).to_not include status2 + expect(results).to_not include both + end + + it 'ignores an invalid mode' do + results = described_class.new(tag1, nil, wark: [tag2.name]).get(20) + expect(results).to include status1 + expect(results).to_not include status2 + expect(results).to include both + end + + it 'handles being passed non existant tag names' do + results = described_class.new(tag1, nil, any: ['wark']).get(20) + expect(results).to include status1 + expect(results).to_not include status2 + expect(results).to include both + end + + it 'can restrict to an account' do + BlockService.new.call(account, status1.account) + results = described_class.new(tag1, account, none: [tag2.name]).get(20) + expect(results).to_not include status1 + end + + it 'can restrict to local' do + status1.account.update(domain: 'example.com') + status1.update(local: false, uri: 'example.com/toot') + results = described_class.new(tag1, nil, any: [tag2.name], local: true).get(20) + expect(results).to_not include status1 + end + + it 'allows replies to be included' do + original = Fabricate(:status) + status = Fabricate(:status, tags: [tag1], in_reply_to_id: original.id) + + results = described_class.new(tag1, nil).get(20) + expect(results).to include(status) + end + end +end diff --git a/spec/services/fan_out_on_write_service_spec.rb b/spec/services/fan_out_on_write_service_spec.rb index b7fc7f7ed..538dc2592 100644 --- a/spec/services/fan_out_on_write_service_spec.rb +++ b/spec/services/fan_out_on_write_service_spec.rb @@ -28,10 +28,10 @@ RSpec.describe FanOutOnWriteService, type: :service do end it 'delivers status to hashtag' do - expect(Tag.find_by!(name: 'test').statuses.pluck(:id)).to include status.id + expect(TagFeed.new(Tag.find_by(name: 'test'), alice).get(20).map(&:id)).to include status.id end it 'delivers status to public timeline' do - expect(Status.as_public_timeline(alice).map(&:id)).to include status.id + expect(PublicFeed.new(alice).get(20).map(&:id)).to include status.id end end diff --git a/spec/services/hashtag_query_service_spec.rb b/spec/services/hashtag_query_service_spec.rb deleted file mode 100644 index 24282d2f0..000000000 --- a/spec/services/hashtag_query_service_spec.rb +++ /dev/null @@ -1,60 +0,0 @@ -require 'rails_helper' - -describe HashtagQueryService, type: :service do - describe '.call' do - let(:account) { Fabricate(:account) } - let(:tag1) { Fabricate(:tag) } - let(:tag2) { Fabricate(:tag) } - let!(:status1) { Fabricate(:status, tags: [tag1]) } - let!(:status2) { Fabricate(:status, tags: [tag2]) } - let!(:both) { Fabricate(:status, tags: [tag1, tag2]) } - - it 'can add tags in "any" mode' do - results = subject.call(tag1, { any: [tag2.name] }) - expect(results).to include status1 - expect(results).to include status2 - expect(results).to include both - end - - it 'can remove tags in "all" mode' do - results = subject.call(tag1, { all: [tag2.name] }) - expect(results).to_not include status1 - expect(results).to_not include status2 - expect(results).to include both - end - - it 'can remove tags in "none" mode' do - results = subject.call(tag1, { none: [tag2.name] }) - expect(results).to include status1 - expect(results).to_not include status2 - expect(results).to_not include both - end - - it 'ignores an invalid mode' do - results = subject.call(tag1, { wark: [tag2.name] }) - expect(results).to include status1 - expect(results).to_not include status2 - expect(results).to include both - end - - it 'handles being passed non existant tag names' do - results = subject.call(tag1, { any: ['wark'] }) - expect(results).to include status1 - expect(results).to_not include status2 - expect(results).to include both - end - - it 'can restrict to an account' do - BlockService.new.call(account, status1.account) - results = subject.call(tag1, { none: [tag2.name] }, account) - expect(results).to_not include status1 - end - - it 'can restrict to local' do - status1.account.update(domain: 'example.com') - status1.update(local: false, uri: 'example.com/toot') - results = subject.call(tag1, { any: [tag2.name] }, nil, true) - expect(results).to_not include status1 - end - end -end -- cgit From e79d719e92e120ba3dd6ec2d8521f7aaa9482634 Mon Sep 17 00:00:00 2001 From: abcang Date: Tue, 8 Sep 2020 00:47:41 +0900 Subject: Changed tag most_used to recently_used (#14760) --- app/controllers/api/v1/featured_tags/suggestions_controller.rb | 8 ++++---- app/controllers/settings/featured_tags_controller.rb | 8 ++++---- app/models/tag.rb | 2 +- app/views/settings/featured_tags/index.html.haml | 2 +- 4 files changed, 10 insertions(+), 10 deletions(-) (limited to 'app') diff --git a/app/controllers/api/v1/featured_tags/suggestions_controller.rb b/app/controllers/api/v1/featured_tags/suggestions_controller.rb index 8c1b81a0f..75545d3c7 100644 --- a/app/controllers/api/v1/featured_tags/suggestions_controller.rb +++ b/app/controllers/api/v1/featured_tags/suggestions_controller.rb @@ -3,15 +3,15 @@ class Api::V1::FeaturedTags::SuggestionsController < Api::BaseController before_action -> { doorkeeper_authorize! :read, :'read:accounts' }, only: :index before_action :require_user! - before_action :set_most_used_tags, only: :index + before_action :set_recently_used_tags, only: :index def index - render json: @most_used_tags, each_serializer: REST::TagSerializer + render json: @recently_used_tags, each_serializer: REST::TagSerializer end private - def set_most_used_tags - @most_used_tags = Tag.most_used(current_account).where.not(id: current_account.featured_tags).limit(10) + def set_recently_used_tags + @recently_used_tags = Tag.recently_used(current_account).where.not(id: current_account.featured_tags).limit(10) end end diff --git a/app/controllers/settings/featured_tags_controller.rb b/app/controllers/settings/featured_tags_controller.rb index 3a3241425..e9861da56 100644 --- a/app/controllers/settings/featured_tags_controller.rb +++ b/app/controllers/settings/featured_tags_controller.rb @@ -6,7 +6,7 @@ class Settings::FeaturedTagsController < Settings::BaseController before_action :authenticate_user! before_action :set_featured_tags, only: :index before_action :set_featured_tag, except: [:index, :create] - before_action :set_most_used_tags, only: :index + before_action :set_recently_used_tags, only: :index def index @featured_tag = FeaturedTag.new @@ -20,7 +20,7 @@ class Settings::FeaturedTagsController < Settings::BaseController redirect_to settings_featured_tags_path else set_featured_tags - set_most_used_tags + set_recently_used_tags render :index end @@ -41,8 +41,8 @@ class Settings::FeaturedTagsController < Settings::BaseController @featured_tags = current_account.featured_tags.order(statuses_count: :desc).reject(&:new_record?) end - def set_most_used_tags - @most_used_tags = Tag.most_used(current_account).where.not(id: @featured_tags.map(&:id)).limit(10) + def set_recently_used_tags + @recently_used_tags = Tag.recently_used(current_account).where.not(id: @featured_tags.map(&:id)).limit(10) end def featured_tag_params diff --git a/app/models/tag.rb b/app/models/tag.rb index bce76fc16..df2f86d95 100644 --- a/app/models/tag.rb +++ b/app/models/tag.rb @@ -39,7 +39,7 @@ class Tag < ApplicationRecord scope :listable, -> { where(listable: [true, nil]) } scope :trendable, -> { Setting.trendable_by_default ? where(trendable: [true, nil]) : where(trendable: true) } scope :discoverable, -> { listable.joins(:account_tag_stat).where(AccountTagStat.arel_table[:accounts_count].gt(0)).order(Arel.sql('account_tag_stats.accounts_count desc')) } - scope :most_used, ->(account) { joins(:statuses).where(statuses: { account: account }).group(:id).order(Arel.sql('count(*) desc')) } + scope :recently_used, ->(account) { joins(:statuses).where(statuses: { id: account.statuses.select(:id).limit(1000) }).group(:id).order(Arel.sql('count(*) desc')) } scope :matches_name, ->(value) { where(arel_table[:name].matches("#{value}%")) } delegate :accounts_count, diff --git a/app/views/settings/featured_tags/index.html.haml b/app/views/settings/featured_tags/index.html.haml index 6734d027c..297379893 100644 --- a/app/views/settings/featured_tags/index.html.haml +++ b/app/views/settings/featured_tags/index.html.haml @@ -9,7 +9,7 @@ = render 'shared/error_messages', object: @featured_tag .fields-group - = f.input :name, wrapper: :with_block_label, hint: safe_join([t('simple_form.hints.featured_tag.name'), safe_join(@most_used_tags.map { |tag| link_to("##{tag.name}", settings_featured_tags_path(featured_tag: { name: tag.name }), method: :post) }, ', ')], ' ') + = f.input :name, wrapper: :with_block_label, hint: safe_join([t('simple_form.hints.featured_tag.name'), safe_join(@recently_used_tags.map { |tag| link_to("##{tag.name}", settings_featured_tags_path(featured_tag: { name: tag.name }), method: :post) }, ', ')], ' ') .actions = f.button :button, t('featured_tags.add_new'), type: :submit -- cgit From 517af45e32535efe1494c0e1e59304a5a7771dba Mon Sep 17 00:00:00 2001 From: ThibG Date: Mon, 7 Sep 2020 18:00:15 +0200 Subject: Fix multiple boosts of a same toot erroneously appearing in TL (#14759) * Check for and record reblog info atomically Instead of using ZREVRANK to determine whether a reblog is a new reblog or not, use ZADD's NX option to perform the check/addition option atomically. * Replace ZREVRANK call with ZSCORE key which is more efficient * Make tests a bit stricter * Fix off-by-one --- app/lib/feed_manager.rb | 20 +++++++++----------- spec/lib/feed_manager_spec.rb | 4 ++-- 2 files changed, 11 insertions(+), 13 deletions(-) (limited to 'app') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 9ab7b53be..785009b52 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -77,9 +77,11 @@ class FeedManager # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop # tracking anything after it for deduplication purposes. - falloff_rank = FeedManager::REBLOG_FALLOFF - 1 + falloff_rank = FeedManager::REBLOG_FALLOFF falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) - falloff_score = falloff_range&.first&.last&.to_i || 0 + falloff_score = falloff_range&.first&.last&.to_i + + return if falloff_score.nil? # Get any reblogs we might have to clean up after. redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| @@ -279,14 +281,12 @@ class FeedManager return false if !rank.nil? && rank < FeedManager::REBLOG_FALLOFF - reblog_rank = redis.zrevrank(reblog_key, status.reblog_of_id) - - if reblog_rank.nil? + # The ordered set at `reblog_key` holds statuses which have a reblog + # in the top `REBLOG_FALLOFF` statuses of the timeline + if redis.zadd(reblog_key, status.id, status.reblog_of_id, nx: true) # This is not something we've already seen reblogged, so we - # can just add it to the feed (and note that we're - # reblogging it). + # can just add it to the feed (and note that we're reblogging it). redis.zadd(timeline_key, status.id, status.id) - redis.zadd(reblog_key, status.id, status.reblog_of_id) else # Another reblog of the same status was already in the # REBLOG_FALLOFF most recent statuses, so we note that this @@ -300,9 +300,7 @@ class FeedManager # delay of the worker deliverying the original status, the late addition # by merging timelines, and other reasons. # If such a reblog already exists, just do not re-insert it into the feed. - rank = redis.zrevrank(reblog_key, status.id) - - return false unless rank.nil? + return false unless redis.zscore(reblog_key, status.id).nil? redis.zadd(timeline_key, status.id, status.id) end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index 5088d1742..d86dd7993 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -444,8 +444,8 @@ RSpec.describe FeedManager do expect(Redis.current.exists?(reblog_set_key)).to be true expect(Redis.current.zrange(reblogs_key, 0, -1)).to eq [reblogged.id.to_s] - # Push everything off the end of the feed. - FeedManager::MAX_ITEMS.times do + # Push everything past the reblog falloff. + FeedManager::REBLOG_FALLOFF.times do FeedManager.instance.push_to_home(receiver, Fabricate(:status)) end -- cgit From 65760f59df46e388919a9f7ccba1958d967b2695 Mon Sep 17 00:00:00 2001 From: Eugen Rochko Date: Tue, 8 Sep 2020 03:41:16 +0200 Subject: Refactor feed manager (#14761) --- app/lib/feed_manager.rb | 236 +++++++++++++++++++++++++------- app/services/after_block_service.rb | 2 +- app/services/notify_service.rb | 6 +- app/services/precompute_feed_service.rb | 2 +- app/workers/feed_insert_worker.rb | 9 +- app/workers/merge_worker.rb | 4 +- app/workers/mute_worker.rb | 7 +- app/workers/unmerge_worker.rb | 4 +- spec/lib/feed_manager_spec.rb | 88 ++++-------- 9 files changed, 235 insertions(+), 123 deletions(-) (limited to 'app') diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index 785009b52..0876d107b 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -6,31 +6,54 @@ class FeedManager include Singleton include Redisable + # Maximum number of items stored in a single feed MAX_ITEMS = 400 - # Must be <= MAX_ITEMS or the tracking sets will grow forever + # Number of items in the feed since last reblog of status + # before the new reblog will be inserted. Must be <= MAX_ITEMS + # or the tracking sets will grow forever REBLOG_FALLOFF = 40 + # Execute block for every active account + # @yield [Account] + # @return [void] def with_active_accounts(&block) Account.joins(:user).where('users.current_sign_in_at > ?', User::ACTIVE_DURATION.ago).find_each(&block) end + # Redis key of a feed + # @param [Symbol] type + # @param [Integer] id + # @param [Symbol] subtype + # @return [String] def key(type, id, subtype = nil) return "feed:#{type}:#{id}" unless subtype "feed:#{type}:#{id}:#{subtype}" end - def filter?(timeline_type, status, receiver_id) - if timeline_type == :home - filter_from_home?(status, receiver_id, build_crutches(receiver_id, [status])) - elsif timeline_type == :mentions - filter_from_mentions?(status, receiver_id) + # Check if the status should not be added to a feed + # @param [Symbol] timeline_type + # @param [Status] status + # @param [Account|List] receiver + # @return [Boolean] + def filter?(timeline_type, status, receiver) + case timeline_type + when :home + filter_from_home?(status, receiver.id, build_crutches(receiver.id, [status])) + when :list + filter_from_list?(status, receiver) || filter_from_home?(status, receiver.account_id, build_crutches(receiver.account_id, [status])) + when :mentions + filter_from_mentions?(status, receiver.id) else false end end + # Add a status to a home feed and send a streaming API update + # @param [Account] account + # @param [Status] status + # @return [Boolean] def push_to_home(account, status) return false unless add_to_feed(:home, account.id, status, account.user&.aggregates_reblogs?) @@ -39,6 +62,10 @@ class FeedManager true end + # Remove a status from a home feed and send a streaming API update + # @param [Account] account + # @param [Status] status + # @return [Boolean] def unpush_from_home(account, status) return false unless remove_from_feed(:home, account.id, status, account.user&.aggregates_reblogs?) @@ -46,21 +73,22 @@ class FeedManager true end + # Add a status to a list feed and send a streaming API update + # @param [List] list + # @param [Status] status + # @return [Boolean] def push_to_list(list, status) - if status.reply? && status.in_reply_to_account_id != status.account_id - should_filter = status.in_reply_to_account_id != list.account_id - should_filter &&= !list.show_all_replies? - should_filter &&= !(list.show_list_replies? && ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?) - return false if should_filter - end - - return false unless add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) + return false if filter_from_list?(status, list) || !add_to_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) trim(:list, list.id) PushUpdateWorker.perform_async(list.account_id, status.id, "timeline:list:#{list.id}") if push_update_required?("timeline:list:#{list.id}") true end + # Remove a status from a list feed and send a streaming API update + # @param [List] list + # @param [Status] status + # @return [Boolean] def unpush_from_list(list, status) return false unless remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) @@ -68,36 +96,39 @@ class FeedManager true end - def trim(type, account_id) - timeline_key = key(type, account_id) - reblog_key = key(type, account_id, 'reblogs') + # Fill a home feed with an account's statuses + # @param [Account] from_account + # @param [Account] into_account + # @return [void] + def merge_into_home(from_account, into_account) + timeline_key = key(:home, into_account.id) + aggregate = into_account.user&.aggregates_reblogs? + query = from_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(FeedManager::MAX_ITEMS / 4) - # Remove any items past the MAX_ITEMS'th entry in our feed - redis.zremrangebyrank(timeline_key, 0, -(FeedManager::MAX_ITEMS + 1)) + if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4 + oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true).first.last.to_i + query = query.where('id > ?', oldest_home_score) + end - # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop - # tracking anything after it for deduplication purposes. - falloff_rank = FeedManager::REBLOG_FALLOFF - falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) - falloff_score = falloff_range&.first&.last&.to_i + statuses = query.to_a + crutches = build_crutches(into_account.id, statuses) - return if falloff_score.nil? + statuses.each do |status| + next if filter_from_home?(status, into_account.id, crutches) - # Get any reblogs we might have to clean up after. - redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| - # Remove it from the set of reblogs we're tracking *first* to avoid races. - redis.zrem(reblog_key, reblogged_id) - # Just drop any set we might have created to track additional reblogs. - # This means that if this reblog is deleted, we won't automatically insert - # another reblog, but also that any new reblog can be inserted into the - # feed. - redis.del(key(type, account_id, "reblogs:#{reblogged_id}")) + add_to_feed(:home, into_account.id, status, aggregate) end + + trim(:home, into_account.id) end - def merge_into_timeline(from_account, into_account) - timeline_key = key(:home, into_account.id) - aggregate = into_account.user&.aggregates_reblogs? + # Fill a list feed with an account's statuses + # @param [Account] from_account + # @param [List] list + # @return [void] + def merge_into_list(from_account, list) + timeline_key = key(:list, list.id) + aggregate = list.account.user&.aggregates_reblogs? query = from_account.statuses.where(visibility: [:public, :unlisted, :private]).includes(:preloadable_poll, reblog: :account).limit(FeedManager::MAX_ITEMS / 4) if redis.zcard(timeline_key) >= FeedManager::MAX_ITEMS / 4 @@ -106,18 +137,22 @@ class FeedManager end statuses = query.to_a - crutches = build_crutches(into_account.id, statuses) + crutches = build_crutches(list.account_id, statuses) statuses.each do |status| - next if filter_from_home?(status, into_account.id, crutches) + next if filter_from_home?(status, list.account_id, crutches) || filter_from_list?(status, list) - add_to_feed(:home, into_account.id, status, aggregate) + add_to_feed(:list, list.id, status, aggregate) end - trim(:home, into_account.id) + trim(:list, list.id) end - def unmerge_from_timeline(from_account, into_account) + # Remove an account's statuses from a home feed + # @param [Account] from_account + # @param [Account] into_account + # @return [void] + def unmerge_from_home(from_account, into_account) timeline_key = key(:home, into_account.id) oldest_home_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 @@ -126,14 +161,31 @@ class FeedManager end end - def clear_from_timeline(account, target_account) - # Clear from timeline all statuses from or mentionning target_account + # Remove an account's statuses from a list feed + # @param [Account] from_account + # @param [List] list + # @return [void] + def unmerge_from_list(from_account, list) + timeline_key = key(:list, list.id) + oldest_list_score = redis.zrange(timeline_key, 0, 0, with_scores: true)&.first&.last&.to_i || 0 + + from_account.statuses.select('id, reblog_of_id').where('id > ?', oldest_list_score).reorder(nil).find_each do |status| + remove_from_feed(:list, list.id, status, list.account.user&.aggregates_reblogs?) + end + end + + # Clear all statuses from or mentioning target_account from a home feed + # @param [Account] account + # @param [Account] target_account + # @return [void] + def clear_from_home(account, target_account) timeline_key = key(:home, account.id) timeline_status_ids = redis.zrange(timeline_key, 0, -1) statuses = Status.where(id: timeline_status_ids).select(:id, :reblog_of_id, :account_id).to_a reblogged_ids = Status.where(id: statuses.map(&:reblog_of_id).compact, account: target_account).pluck(:id) with_mentions_ids = Mention.active.where(status_id: statuses.flat_map { |s| [s.id, s.reblog_of_id] }.compact, account: target_account).pluck(:status_id) - target_statuses = statuses.filter do |status| + + target_statuses = statuses.select do |status| status.account_id == target_account.id || reblogged_ids.include?(status.reblog_of_id) || with_mentions_ids.include?(status.id) || with_mentions_ids.include?(status.reblog_of_id) end @@ -142,7 +194,10 @@ class FeedManager end end - def populate_feed(account) + # Populate home feed of account from scratch + # @param [Account] account + # @return [void] + def populate_home(account) limit = FeedManager::MAX_ITEMS / 2 aggregate = account.user&.aggregates_reblogs? timeline_key = key(:home, account.id) @@ -177,15 +232,59 @@ class FeedManager private - def push_update_required?(timeline_id) - redis.exists?("subscribed:#{timeline_id}") + # Trim a feed to maximum size by removing older items + # @param [Symbol] type + # @param [Integer] timeline_id + # @return [void] + def trim(type, timeline_id) + timeline_key = key(type, timeline_id) + reblog_key = key(type, timeline_id, 'reblogs') + + # Remove any items past the MAX_ITEMS'th entry in our feed + redis.zremrangebyrank(timeline_key, 0, -(FeedManager::MAX_ITEMS + 1)) + + # Get the score of the REBLOG_FALLOFF'th item in our feed, and stop + # tracking anything after it for deduplication purposes. + falloff_rank = FeedManager::REBLOG_FALLOFF + falloff_range = redis.zrevrange(timeline_key, falloff_rank, falloff_rank, with_scores: true) + falloff_score = falloff_range&.first&.last&.to_i + + return if falloff_score.nil? + + # Get any reblogs we might have to clean up after. + redis.zrangebyscore(reblog_key, 0, falloff_score).each do |reblogged_id| + # Remove it from the set of reblogs we're tracking *first* to avoid races. + redis.zrem(reblog_key, reblogged_id) + # Just drop any set we might have created to track additional reblogs. + # This means that if this reblog is deleted, we won't automatically insert + # another reblog, but also that any new reblog can be inserted into the + # feed. + redis.del(key(type, timeline_id, "reblogs:#{reblogged_id}")) + end end + # Check if there is a streaming API client connected + # for the given feed + # @param [String] timeline_key + # @return [Boolean] + def push_update_required?(timeline_key) + redis.exists?("subscribed:#{timeline_key}") + end + + # Check if the account is blocking or muting any of the given accounts + # @param [Integer] receiver_id + # @param [Array] account_ids + # @param [Symbol] context def blocks_or_mutes?(receiver_id, account_ids, context) Block.where(account_id: receiver_id, target_account_id: account_ids).any? || (context == :home ? Mute.where(account_id: receiver_id, target_account_id: account_ids).any? : Mute.where(account_id: receiver_id, target_account_id: account_ids, hide_notifications: true).any?) end + # Check if status should not be added to the home feed + # @param [Status] status + # @param [Integer] receiver_id + # @param [Hash] crutches + # @return [Boolean] def filter_from_home?(status, receiver_id, crutches) return false if receiver_id == status.account_id return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?) @@ -218,6 +317,11 @@ class FeedManager false end + # Check if status should not be added to the mentions feed + # @see NotifyService + # @param [Status] status + # @param [Integer] receiver_id + # @return [Boolean] def filter_from_mentions?(status, receiver_id) return true if receiver_id == status.account_id return true if phrase_filtered?(status, receiver_id, :notifications) @@ -234,6 +338,27 @@ class FeedManager should_filter end + # Check if status should not be added to the list feed + # @param [Status] status + # @param [List] list + # @return [Boolean] + def filter_from_list?(status, list) + if status.reply? && status.in_reply_to_account_id != status.account_id + should_filter = status.in_reply_to_account_id != list.account_id + should_filter &&= !list.show_all_replies? + should_filter &&= !(list.show_list_replies? && ListAccount.where(list_id: list.id, account_id: status.in_reply_to_account_id).exists?) + + return !!should_filter + end + + false + end + + # Check if the status hits a phrase filter + # @param [Status] status + # @param [Integer] receiver_id + # @param [Symbol] context + # @return [Boolean] def phrase_filtered?(status, receiver_id, context) active_filters = Rails.cache.fetch("filters:#{receiver_id}") { CustomFilter.where(account_id: receiver_id).active_irreversible.to_a }.to_a @@ -269,6 +394,11 @@ class FeedManager # added, and false if it was not added to the feed. Note that this is # an internal helper: callers must call trim or push updates if # either action is appropriate. + # @param [Symbol] timeline_type + # @param [Integer] account_id + # @param [Status] status + # @param [Boolean] aggregate_reblogs + # @return [Boolean] def add_to_feed(timeline_type, account_id, status, aggregate_reblogs = true) timeline_key = key(timeline_type, account_id) reblog_key = key(timeline_type, account_id, 'reblogs') @@ -312,6 +442,11 @@ class FeedManager # with reblogs, and returning true if a status was removed. As with # `add_to_feed`, this does not trigger push updates, so callers must # do so if appropriate. + # @param [Symbol] timeline_type + # @param [Integer] account_id + # @param [Status] status + # @param [Boolean] aggregate_reblogs + # @return [Boolean] def remove_from_feed(timeline_type, account_id, status, aggregate_reblogs = true) timeline_key = key(timeline_type, account_id) reblog_key = key(timeline_type, account_id, 'reblogs') @@ -346,6 +481,11 @@ class FeedManager redis.zrem(timeline_key, status.id) end + # Pre-fetch various objects and relationships for given statuses that + # are going to be checked by the filtering methods + # @param [Integer] receiver_id + # @param [Array] statuses + # @return [Hash] def build_crutches(receiver_id, statuses) crutches = {} diff --git a/app/services/after_block_service.rb b/app/services/after_block_service.rb index 2a0e10a79..314919df8 100644 --- a/app/services/after_block_service.rb +++ b/app/services/after_block_service.rb @@ -13,7 +13,7 @@ class AfterBlockService < BaseService private def clear_home_feed! - FeedManager.instance.clear_from_timeline(@account, @target_account) + FeedManager.instance.clear_from_home(@account, @target_account) end def clear_conversations! diff --git a/app/services/notify_service.rb b/app/services/notify_service.rb index abd676494..e4ca10eb1 100644 --- a/app/services/notify_service.rb +++ b/app/services/notify_service.rb @@ -13,15 +13,13 @@ class NotifyService < BaseService push_to_conversation! if direct_message? send_email! if email_enabled? rescue ActiveRecord::RecordInvalid - # rubocop:disable Style/RedundantReturn - return - # rubocop:enable Style/RedundantReturn + nil end private def blocked_mention? - FeedManager.instance.filter?(:mentions, @notification.mention.status, @recipient.id) + FeedManager.instance.filter?(:mentions, @notification.mention.status, @recipient) end def blocked_favourite? diff --git a/app/services/precompute_feed_service.rb b/app/services/precompute_feed_service.rb index 076dedaca..61f573534 100644 --- a/app/services/precompute_feed_service.rb +++ b/app/services/precompute_feed_service.rb @@ -2,7 +2,7 @@ class PrecomputeFeedService < BaseService def call(account) - FeedManager.instance.populate_feed(account) + FeedManager.instance.populate_home(account) ensure Redis.current.del("account:#{account.id}:regeneration") end diff --git a/app/workers/feed_insert_worker.rb b/app/workers/feed_insert_worker.rb index 1ae3c877b..633ec91bd 100644 --- a/app/workers/feed_insert_worker.rb +++ b/app/workers/feed_insert_worker.rb @@ -27,9 +27,12 @@ class FeedInsertWorker end def feed_filtered? - # Note: Lists are a variation of home, so the filtering rules - # of home apply to both - FeedManager.instance.filter?(:home, @status, @follower.id) + case @type + when :home + FeedManager.instance.filter?(:home, @status, @follower) + when :list + FeedManager.instance.filter?(:list, @status, @list) + end end def perform_push diff --git a/app/workers/merge_worker.rb b/app/workers/merge_worker.rb index d745cb99c..74ef7d4da 100644 --- a/app/workers/merge_worker.rb +++ b/app/workers/merge_worker.rb @@ -6,6 +6,8 @@ class MergeWorker sidekiq_options queue: 'pull' def perform(from_account_id, into_account_id) - FeedManager.instance.merge_into_timeline(Account.find(from_account_id), Account.find(into_account_id)) + FeedManager.instance.merge_into_home(Account.find(from_account_id), Account.find(into_account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/app/workers/mute_worker.rb b/app/workers/mute_worker.rb index 7bf0923a5..c74f657cb 100644 --- a/app/workers/mute_worker.rb +++ b/app/workers/mute_worker.rb @@ -4,9 +4,8 @@ class MuteWorker include Sidekiq::Worker def perform(account_id, target_account_id) - FeedManager.instance.clear_from_timeline( - Account.find(account_id), - Account.find(target_account_id) - ) + FeedManager.instance.clear_from_home(Account.find(account_id), Account.find(target_account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/app/workers/unmerge_worker.rb b/app/workers/unmerge_worker.rb index ea6aacebf..1a23faae5 100644 --- a/app/workers/unmerge_worker.rb +++ b/app/workers/unmerge_worker.rb @@ -6,6 +6,8 @@ class UnmergeWorker sidekiq_options queue: 'pull' def perform(from_account_id, into_account_id) - FeedManager.instance.unmerge_from_timeline(Account.find(from_account_id), Account.find(into_account_id)) + FeedManager.instance.unmerge_from_home(Account.find(from_account_id), Account.find(into_account_id)) + rescue ActiveRecord::RecordNotFound + true end end diff --git a/spec/lib/feed_manager_spec.rb b/spec/lib/feed_manager_spec.rb index d86dd7993..d9c17470f 100644 --- a/spec/lib/feed_manager_spec.rb +++ b/spec/lib/feed_manager_spec.rb @@ -29,14 +29,14 @@ RSpec.describe FeedManager do it 'returns false for followee\'s status' do status = Fabricate(:status, text: 'Hello world', account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, status, bob)).to be false end it 'returns false for reblog by followee' do status = Fabricate(:status, text: 'Hello world', account: jeff) reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be false end it 'returns true for reblog by followee of blocked account' do @@ -44,7 +44,7 @@ RSpec.describe FeedManager do reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) bob.block!(jeff) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns true for reblog by followee of muted account' do @@ -52,7 +52,7 @@ RSpec.describe FeedManager do reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) bob.mute!(jeff) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns true for reblog by followee of someone who is blocking recipient' do @@ -60,14 +60,14 @@ RSpec.describe FeedManager do reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice) jeff.block!(bob) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns true for reblog from account with reblogs disabled' do status = Fabricate(:status, text: 'Hello world', account: jeff) reblog = Fabricate(:status, reblog: status, account: alice) bob.follow!(alice, reblogs: false) - expect(FeedManager.instance.filter?(:home, reblog, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, bob)).to be true end it 'returns false for reply by followee to another followee' do @@ -75,48 +75,48 @@ RSpec.describe FeedManager do reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) bob.follow!(jeff) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reply, bob)).to be false end it 'returns false for reply by followee to recipient' do status = Fabricate(:status, text: 'Hello world', account: bob) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reply, bob)).to be false end it 'returns false for reply by followee to self' do status = Fabricate(:status, text: 'Hello world', account: alice) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, reply, bob)).to be false end it 'returns true for reply by followee to non-followed account' do status = Fabricate(:status, text: 'Hello world', account: jeff) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, reply, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, reply, bob)).to be true end it 'returns true for the second reply by followee to a non-federated status' do reply = Fabricate(:status, text: 'Reply 1', reply: true, account: alice) second_reply = Fabricate(:status, text: 'Reply 2', thread: reply, account: alice) bob.follow!(alice) - expect(FeedManager.instance.filter?(:home, second_reply, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, second_reply, bob)).to be true end it 'returns false for status by followee mentioning another account' do bob.follow!(alice) status = PostStatusService.new.call(alice, text: 'Hey @jeff') - expect(FeedManager.instance.filter?(:home, status, bob.id)).to be false + expect(FeedManager.instance.filter?(:home, status, bob)).to be false end it 'returns true for status by followee mentioning blocked account' do bob.block!(jeff) bob.follow!(alice) status = PostStatusService.new.call(alice, text: 'Hey @jeff') - expect(FeedManager.instance.filter?(:home, status, bob.id)).to be true + expect(FeedManager.instance.filter?(:home, status, bob)).to be true end it 'returns true for reblog of a personally blocked domain' do @@ -124,7 +124,7 @@ RSpec.describe FeedManager do alice.follow!(jeff) status = Fabricate(:status, text: 'Hello world', account: bob) reblog = Fabricate(:status, reblog: status, account: jeff) - expect(FeedManager.instance.filter?(:home, reblog, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, reblog, alice)).to be true end context 'for irreversibly muted phrases' do @@ -132,7 +132,7 @@ RSpec.describe FeedManager do alice.custom_filters.create!(phrase: 'bob', context: %w(home), irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'bobcats', account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be_falsy + expect(FeedManager.instance.filter?(:home, status, alice)).to be_falsy end it 'returns true if phrase is contained' do @@ -140,14 +140,14 @@ RSpec.describe FeedManager do alice.custom_filters.create!(phrase: 'pop tarts', context: %w(home), irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'i sure like POP TARts', account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, status, alice)).to be true end it 'matches substrings if whole_word is false' do alice.custom_filters.create!(phrase: 'take', context: %w(home), whole_word: false, irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'shiitake', account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, status, alice)).to be true end it 'returns true if phrase is contained in a poll option' do @@ -155,7 +155,7 @@ RSpec.describe FeedManager do alice.custom_filters.create!(phrase: 'pop tarts', context: %w(home), irreversible: true) alice.follow!(jeff) status = Fabricate(:status, text: 'what do you prefer', poll: Fabricate(:poll, options: %w(farts POP TARts)), account: jeff) - expect(FeedManager.instance.filter?(:home, status, alice.id)).to be true + expect(FeedManager.instance.filter?(:home, status, alice)).to be true end end end @@ -164,27 +164,27 @@ RSpec.describe FeedManager do it 'returns true for status that mentions blocked account' do bob.block!(jeff) status = PostStatusService.new.call(alice, text: 'Hey @jeff') - expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true + expect(FeedManager.instance.filter?(:mentions, status, bob)).to be true end it 'returns true for status that replies to a blocked account' do status = Fabricate(:status, text: 'Hello world', account: jeff) reply = Fabricate(:status, text: 'Nay', thread: status, account: alice) bob.block!(jeff) - expect(FeedManager.instance.filter?(:mentions, reply, bob.id)).to be true + expect(FeedManager.instance.filter?(:mentions, reply, bob)).to be true end it 'returns true for status by silenced account who recipient is not following' do status = Fabricate(:status, text: 'Hello world', account: alice) alice.silence! - expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be true + expect(FeedManager.instance.filter?(:mentions, status, bob)).to be true end it 'returns false for status by followed silenced account' do status = Fabricate(:status, text: 'Hello world', account: alice) alice.silence! bob.follow!(alice) - expect(FeedManager.instance.filter?(:mentions, status, bob.id)).to be false + expect(FeedManager.instance.filter?(:mentions, status, bob)).to be false end end end @@ -414,52 +414,20 @@ RSpec.describe FeedManager do end end - describe '#merge_into_timeline' do + describe '#merge_into_home' do it "does not push source account's statuses whose reblogs are already inserted" do account = Fabricate(:account, id: 0) reblog = Fabricate(:status) status = Fabricate(:status, reblog: reblog) FeedManager.instance.push_to_home(account, status) - FeedManager.instance.merge_into_timeline(account, reblog.account) + FeedManager.instance.merge_into_home(account, reblog.account) expect(Redis.current.zscore("feed:home:0", reblog.id)).to eq nil end end - describe '#trim' do - let(:receiver) { Fabricate(:account) } - - it 'cleans up reblog tracking keys' do - reblogged = Fabricate(:status) - status = Fabricate(:status, reblog: reblogged) - another_status = Fabricate(:status, reblog: reblogged) - reblogs_key = FeedManager.instance.key('home', receiver.id, 'reblogs') - reblog_set_key = FeedManager.instance.key('home', receiver.id, "reblogs:#{reblogged.id}") - - FeedManager.instance.push_to_home(receiver, status) - FeedManager.instance.push_to_home(receiver, another_status) - - # We should have a tracking set and an entry in reblogs. - expect(Redis.current.exists?(reblog_set_key)).to be true - expect(Redis.current.zrange(reblogs_key, 0, -1)).to eq [reblogged.id.to_s] - - # Push everything past the reblog falloff. - FeedManager::REBLOG_FALLOFF.times do - FeedManager.instance.push_to_home(receiver, Fabricate(:status)) - end - - # `trim` should be called automatically, but do it anyway, as - # we're testing `trim`, not side effects of `push`. - FeedManager.instance.trim('home', receiver.id) - - # We should not have any reblog tracking data. - expect(Redis.current.exists?(reblog_set_key)).to be false - expect(Redis.current.zrange(reblogs_key, 0, -1)).to be_empty - end - end - - describe '#unpush' do + describe '#unpush_from_home' do let(:receiver) { Fabricate(:account) } it 'leaves a reblogged status if original was on feed' do @@ -525,7 +493,7 @@ RSpec.describe FeedManager do end end - describe '#clear_from_timeline' do + describe '#clear_from_home' do let(:account) { Fabricate(:account) } let(:followed_account) { Fabricate(:account) } let(:target_account) { Fabricate(:account) } @@ -543,8 +511,8 @@ RSpec.describe FeedManager do end end - it 'correctly cleans the timeline' do - FeedManager.instance.clear_from_timeline(account, target_account) + it 'correctly cleans the home timeline' do + FeedManager.instance.clear_from_home(account, target_account) expect(Redis.current.zrange("feed:home:#{account.id}", 0, -1)).to eq [status_1.id.to_s, status_7.id.to_s] end -- cgit From afa753a890197b188ddf7ba6a8493d4fe361d956 Mon Sep 17 00:00:00 2001 From: kedama Date: Sun, 28 Oct 2018 14:39:59 +0900 Subject: [Glitch] Set z-index of dropdown to 9999. Port 678f5ed296e71bb80d170027b114d9d30a7ccab7 to glitch-soc Signed-off-by: Thibaut Girka --- app/javascript/flavours/glitch/styles/components/index.scss | 1 + 1 file changed, 1 insertion(+) (limited to 'app') diff --git a/app/javascript/flavours/glitch/styles/components/index.scss b/app/javascript/flavours/glitch/styles/components/index.scss index 306e62342..04266c497 100644 --- a/app/javascript/flavours/glitch/styles/components/index.scss +++ b/app/javascript/flavours/glitch/styles/components/index.scss @@ -464,6 +464,7 @@ padding: 4px 0; border-radius: 4px; box-shadow: 2px 4px 15px rgba($base-shadow-color, 0.4); + z-index: 9999; ul { list-style: none; -- cgit