From 434c70fd9855f55f6238cdd5a551f60c28572970 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 16:41:25 -0600 Subject: [PATCH 01/16] add a local_only column to the statuses table --- app/models/status.rb | 1 + db/migrate/20171210213213_add_local_only_flag_to_statuses.rb | 5 +++++ db/schema.rb | 3 ++- 3 files changed, 8 insertions(+), 1 deletion(-) create mode 100644 db/migrate/20171210213213_add_local_only_flag_to_statuses.rb diff --git a/app/models/status.rb b/app/models/status.rb index 70cfdc1c7..294044139 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -23,6 +23,7 @@ # account_id :integer not null # application_id :integer # in_reply_to_account_id :integer +# local_only :boolean # class Status < ApplicationRecord diff --git a/db/migrate/20171210213213_add_local_only_flag_to_statuses.rb b/db/migrate/20171210213213_add_local_only_flag_to_statuses.rb new file mode 100644 index 000000000..af1e29d6a --- /dev/null +++ b/db/migrate/20171210213213_add_local_only_flag_to_statuses.rb @@ -0,0 +1,5 @@ +class AddLocalOnlyFlagToStatuses < ActiveRecord::Migration[5.1] + def change + add_column :statuses, :local_only, :boolean + end +end diff --git a/db/schema.rb b/db/schema.rb index 4cf886a00..de74a300c 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema.define(version: 20171130000000) do +ActiveRecord::Schema.define(version: 20171210213213) do # These are extensions that must be enabled in order to support this database enable_extension "plpgsql" @@ -420,6 +420,7 @@ ActiveRecord::Schema.define(version: 20171130000000) do t.bigint "account_id", null: false t.bigint "application_id" t.bigint "in_reply_to_account_id" + t.boolean "local_only" t.index ["account_id", "id"], name: "index_statuses_on_account_id_id" t.index ["conversation_id"], name: "index_statuses_on_conversation_id" t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id" From 08519cd4f4bd290e633d5243e8195111471b7bf0 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 17:04:28 -0600 Subject: [PATCH 02/16] status: stub local_only?, add scope, add marked_local_only? --- app/models/status.rb | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/app/models/status.rb b/app/models/status.rb index 294044139..40a8d05c8 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -75,6 +75,8 @@ class Status < ApplicationRecord scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) } + scope :not_local_only, -> { where(local_only: false) } + cache_associated :account, :application, :media_attachments, :tags, :stream_entry, mentions: :account, reblog: [:account, :application, :stream_entry, :tags, :media_attachments, mentions: :account], thread: :account delegate :domain, to: :account, prefix: true @@ -271,6 +273,10 @@ class Status < ApplicationRecord end def local_only? + local_only + end + + def marked_local_only? # match both with and without U+FE0F (the emoji variation selector) /👁\ufe0f?\z/.match?(content) end From cfbb95605b3f04ad18d668861002dec2fab03441 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 17:04:32 -0600 Subject: [PATCH 03/16] set local_only flag on statuses in post_status_service --- app/services/post_status_service.rb | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index 59531a76c..e531384c8 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -40,8 +40,9 @@ class PostStatusService < BaseService LinkCrawlWorker.perform_async(status.id) unless status.spoiler_text? DistributionWorker.perform_async(status.id) - # match both with and without U+FE0F (the emoji variation selector) - unless /👁\ufe0f?\z/.match?(status.content) + status.local_only = status.marked_local_only? + + unless status.local_only Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) ActivityPub::DistributionWorker.perform_async(status.id) ActivityPub::ReplyDistributionWorker.perform_async(status.id) if status.reply? && status.thread.account.local? From 5ef65aab8f500b0675cf632d49f7cb544caefd27 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 17:12:21 -0600 Subject: [PATCH 04/16] replace reblog service check for an :eye: with #local_only --- app/services/reblog_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 52e3ba0e0..6802afe70 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -21,7 +21,7 @@ class ReblogService < BaseService DistributionWorker.perform_async(reblog.id) - unless /👁$/.match?(reblogged_status.content) + unless reblogged_status.local_only Pubsubhubbub::DistributionWorker.perform_async(reblog.stream_entry.id) ActivityPub::DistributionWorker.perform_async(reblog.id) end From 6bd18e43ba6ce74837b813d68b5c96e4f696069f Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 17:23:01 -0600 Subject: [PATCH 05/16] filter local-only statuses from public pages --- app/controllers/accounts_controller.rb | 2 +- app/models/status.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/accounts_controller.rb b/app/controllers/accounts_controller.rb index 309cb65da..31144fe05 100644 --- a/app/controllers/accounts_controller.rb +++ b/app/controllers/accounts_controller.rb @@ -49,7 +49,7 @@ class AccountsController < ApplicationController end def default_statuses - @account.statuses.where(visibility: [:public, :unlisted]) + @account.statuses.not_local_only.where(visibility: [:public, :unlisted]) end def only_media_scope diff --git a/app/models/status.rb b/app/models/status.rb index 40a8d05c8..6f7a8c82d 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -260,7 +260,7 @@ class Status < ApplicationRecord end def filter_timeline_default(query) - query.excluding_silenced_accounts + query.not_local_only.excluding_silenced_accounts end def account_silencing_filter(account) From f080a9fac777e446554c39120c899732eec47b6f Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 19:07:43 -0600 Subject: [PATCH 06/16] filter local-only toots from AP outboxes --- app/controllers/activitypub/outboxes_controller.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index 9f97ff622..e19ca6521 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -4,7 +4,7 @@ class ActivityPub::OutboxesController < Api::BaseController before_action :set_account def show - @statuses = @account.statuses.permitted_for(@account, current_account).paginate_by_max_id(20, params[:max_id], params[:since_id]) + @statuses = @account.statuses.not_local_only.permitted_for(@account, current_account).paginate_by_max_id(20, params[:max_id], params[:since_id]) @statuses = cache_collection(@statuses, Status) render json: outbox_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' From 24f36ca9124353fdc3430c48b4afbb759276739d Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 19:41:33 -0600 Subject: [PATCH 07/16] Status#not_local_only scope should match nils too --- app/models/status.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/models/status.rb b/app/models/status.rb index 6f7a8c82d..920c255d7 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -75,7 +75,7 @@ class Status < ApplicationRecord scope :not_excluded_by_account, ->(account) { where.not(account_id: account.excluded_from_timeline_account_ids) } scope :not_domain_blocked_by_account, ->(account) { account.excluded_from_timeline_domains.blank? ? left_outer_joins(:account) : left_outer_joins(:account).where('accounts.domain IS NULL OR accounts.domain NOT IN (?)', account.excluded_from_timeline_domains) } - scope :not_local_only, -> { where(local_only: false) } + scope :not_local_only, -> { where(local_only: [false, nil]) } cache_associated :account, :application, :media_attachments, :tags, :stream_entry, mentions: :account, reblog: [:account, :application, :stream_entry, :tags, :media_attachments, mentions: :account], thread: :account From c1410af368911f0bbfe7205ba995c7c84dc023f6 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 20:35:57 -0600 Subject: [PATCH 08/16] post_status_service.rb: save the status after setting local_only --- app/services/post_status_service.rb | 1 + 1 file changed, 1 insertion(+) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index e531384c8..c7b5e8ffb 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -41,6 +41,7 @@ class PostStatusService < BaseService DistributionWorker.perform_async(status.id) status.local_only = status.marked_local_only? + status.save! unless status.local_only Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) From 0c46058a43efe6f19fba33146842f7d8ecc8d064 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 21:25:36 -0600 Subject: [PATCH 09/16] remove vestigial Status#local_only? definition --- app/models/status.rb | 4 ---- 1 file changed, 4 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 920c255d7..9ae753447 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -272,10 +272,6 @@ class Status < ApplicationRecord end end - def local_only? - local_only - end - def marked_local_only? # match both with and without U+FE0F (the emoji variation selector) /👁\ufe0f?\z/.match?(content) From 288f1293ef6702fd55262ffa1a143e6a36d47d14 Mon Sep 17 00:00:00 2001 From: Erin Date: Sun, 10 Dec 2017 21:39:27 -0600 Subject: [PATCH 10/16] set local_only in a before_create callback instead of status service --- app/models/status.rb | 8 ++++++++ app/services/post_status_service.rb | 3 --- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 9ae753447..94b2a1aa9 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -141,6 +141,8 @@ class Status < ApplicationRecord around_create Mastodon::Snowflake::Callbacks + before_create :set_locality + before_validation :prepare_contents, if: :local? before_validation :set_reblog before_validation :set_visibility @@ -302,6 +304,12 @@ class Status < ApplicationRecord self.sensitive = sensitive || spoiler_text.present? end + def set_locality + if account.domain.nil? + self.local_only = marked_local_only? + end + end + def set_conversation self.reply = !(in_reply_to_id.nil? && thread.nil?) unless reply diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index c7b5e8ffb..ff6cfbe8f 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -40,9 +40,6 @@ class PostStatusService < BaseService LinkCrawlWorker.perform_async(status.id) unless status.spoiler_text? DistributionWorker.perform_async(status.id) - status.local_only = status.marked_local_only? - status.save! - unless status.local_only Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) ActivityPub::DistributionWorker.perform_async(status.id) From 1138d0c321d3aee9c809e5abc03728d854a72203 Mon Sep 17 00:00:00 2001 From: David Yip Date: Sun, 10 Dec 2017 22:49:59 -0600 Subject: [PATCH 11/16] Add Rake task to backfill local-only flag (#253) --- lib/tasks/glitchsoc.rake | 8 ++++++++ 1 file changed, 8 insertions(+) create mode 100644 lib/tasks/glitchsoc.rake diff --git a/lib/tasks/glitchsoc.rake b/lib/tasks/glitchsoc.rake new file mode 100644 index 000000000..79e864648 --- /dev/null +++ b/lib/tasks/glitchsoc.rake @@ -0,0 +1,8 @@ +namespace :glitchsoc do + desc 'Backfill local-only flag on statuses table' + task backfill_local_only: :environment do + Status.local.where(local_only: nil).find_each do |st| + ActiveRecord::Base.logger.silence { st.update_attribute(:local_only, st.marked_local_only?) } + end + end +end From 3ec47e732b0ae05095c733bd91ab4313f54025dc Mon Sep 17 00:00:00 2001 From: Erin Date: Mon, 11 Dec 2017 15:27:31 -0600 Subject: [PATCH 12/16] post_status_service: stylistic change (local_only -> local_only?) --- app/services/post_status_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/post_status_service.rb b/app/services/post_status_service.rb index ff6cfbe8f..6b6a37676 100644 --- a/app/services/post_status_service.rb +++ b/app/services/post_status_service.rb @@ -40,7 +40,7 @@ class PostStatusService < BaseService LinkCrawlWorker.perform_async(status.id) unless status.spoiler_text? DistributionWorker.perform_async(status.id) - unless status.local_only + unless status.local_only? Pubsubhubbub::DistributionWorker.perform_async(status.stream_entry.id) ActivityPub::DistributionWorker.perform_async(status.id) ActivityPub::ReplyDistributionWorker.perform_async(status.id) if status.reply? && status.thread.account.local? From c5a4eda6946cc2b7afa466e5c4abe0673fa235ad Mon Sep 17 00:00:00 2001 From: Erin Date: Mon, 11 Dec 2017 15:28:04 -0600 Subject: [PATCH 13/16] move outbox filtering to Status#permitted_for (as per @ekiru) --- app/controllers/activitypub/outboxes_controller.rb | 2 +- app/models/status.rb | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/app/controllers/activitypub/outboxes_controller.rb b/app/controllers/activitypub/outboxes_controller.rb index e19ca6521..9f97ff622 100644 --- a/app/controllers/activitypub/outboxes_controller.rb +++ b/app/controllers/activitypub/outboxes_controller.rb @@ -4,7 +4,7 @@ class ActivityPub::OutboxesController < Api::BaseController before_action :set_account def show - @statuses = @account.statuses.not_local_only.permitted_for(@account, current_account).paginate_by_max_id(20, params[:max_id], params[:since_id]) + @statuses = @account.statuses.permitted_for(@account, current_account).paginate_by_max_id(20, params[:max_id], params[:since_id]) @statuses = cache_collection(@statuses, Status) render json: outbox_presenter, serializer: ActivityPub::CollectionSerializer, adapter: ActivityPub::Adapter, content_type: 'application/activity+json' diff --git a/app/models/status.rb b/app/models/status.rb index 94b2a1aa9..1bc4c633d 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -223,7 +223,7 @@ class Status < ApplicationRecord visibility = [:public, :unlisted] if account.nil? - where(visibility: visibility) + where(visibility: visibility).not_local_only elsif target_account.blocking?(account) # get rid of blocked peeps none elsif account.id == target_account.id # author can see own stuff From 2efffe77dc5ff20ba4904d7a72e137d6c0e5c017 Mon Sep 17 00:00:00 2001 From: Erin Date: Mon, 11 Dec 2017 17:39:11 -0600 Subject: [PATCH 14/16] reblog_service.rb: Status#local_only -> local_only? --- app/services/reblog_service.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/app/services/reblog_service.rb b/app/services/reblog_service.rb index 6802afe70..8d8b15a41 100644 --- a/app/services/reblog_service.rb +++ b/app/services/reblog_service.rb @@ -21,7 +21,7 @@ class ReblogService < BaseService DistributionWorker.perform_async(reblog.id) - unless reblogged_status.local_only + unless reblogged_status.local_only? Pubsubhubbub::DistributionWorker.perform_async(reblog.stream_entry.id) ActivityPub::DistributionWorker.perform_async(reblog.id) end From e35a35011915bc0cb77dae933e55fba267d31430 Mon Sep 17 00:00:00 2001 From: David Yip Date: Thu, 14 Dec 2017 02:27:42 -0600 Subject: [PATCH 15/16] Examples for Status#set_locality and .as_tag_timeline. This commit also: - exposes the local-only emoji so that it can be used in examples - allows local_only to be set explicitly, i.e. for timeline filtering specs --- app/models/status.rb | 8 ++++-- spec/models/status_spec.rb | 58 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 64 insertions(+), 2 deletions(-) diff --git a/app/models/status.rb b/app/models/status.rb index 1bc4c633d..db3072571 100644 --- a/app/models/status.rb +++ b/app/models/status.rb @@ -276,7 +276,11 @@ class Status < ApplicationRecord def marked_local_only? # match both with and without U+FE0F (the emoji variation selector) - /👁\ufe0f?\z/.match?(content) + /#{local_only_emoji}\ufe0f?\z/.match?(content) + end + + def local_only_emoji + '👁' end private @@ -305,7 +309,7 @@ class Status < ApplicationRecord end def set_locality - if account.domain.nil? + if account.domain.nil? && !attribute_changed?(:local_only) self.local_only = marked_local_only? end end diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index c6701018e..5b0adb769 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -197,6 +197,43 @@ RSpec.describe Status, type: :model do end end + describe 'on create' do + let(:local_account) { Fabricate(:account, username: 'local', domain: nil) } + let(:remote_account) { Fabricate(:account, username: 'remote', domain: 'example.com') } + + subject { Status.new } + + describe 'on a status that ends with the local-only emoji' do + before do + subject.text = 'A toot ' + subject.local_only_emoji + end + + context 'if the status originates from this instance' do + before do + subject.account = local_account + end + + it 'is marked local-only' do + subject.save! + + expect(subject).to be_local_only + end + end + + context 'if the status is remote' do + before do + subject.account = remote_account + end + + it 'is not marked local-only' do + subject.save! + + expect(subject).to_not be_local_only + end + end + end + end + describe '.mutes_map' do let(:status) { Fabricate(:status) } let(:account) { Fabricate(:account) } @@ -570,6 +607,27 @@ RSpec.describe Status, type: :model do results = Status.as_tag_timeline(tag) expect(results).to include(status) end + + context 'on a local-only status' do + let(:tag) { Fabricate(:tag) } + let(:status) { Fabricate(:status, local_only: true, tags: [tag]) } + + context 'if account is nil' do + let(:account) { nil } + + it 'filters the local-only status out of the result set' do + expect(Status.as_tag_timeline(tag, account)).not_to include(status) + end + end + + context 'if account is not nil and local' do + let(:account) { Fabricate(:account, domain: nil) } + + it 'keeps the local-only status in the result set' do + expect(Status.as_tag_timeline(tag, account)).to include(status) + end + end + end end describe '.permitted_for' do From 6abb0950c6f694607cdc6a0c564751400d52ad5a Mon Sep 17 00:00:00 2001 From: David Yip Date: Thu, 14 Dec 2017 02:57:59 -0600 Subject: [PATCH 16/16] Examples for Status.as_public_timeline. Also adjust the examples for Status.as_tag_timeline to match the nomenclature used in .as_public_timeline (e.g. "account" -> "viewer"). --- spec/models/status_spec.rb | 38 ++++++++++++++++++++++++++++++++------ 1 file changed, 32 insertions(+), 6 deletions(-) diff --git a/spec/models/status_spec.rb b/spec/models/status_spec.rb index 5b0adb769..1f5a03877 100644 --- a/spec/models/status_spec.rb +++ b/spec/models/status_spec.rb @@ -586,6 +586,32 @@ RSpec.describe Status, type: :model do end end end + + context 'with local-only statuses' do + let(:status) { Fabricate(:status, local_only: true) } + + subject { Status.as_public_timeline(viewer) } + + context 'without a viewer' do + let(:viewer) { nil } + + it 'excludes local-only statuses' do + expect(subject).to_not include(status) + end + end + + context 'with a viewer' do + let(:viewer) { Fabricate(:account, username: 'viewer') } + + it 'includes local-only statuses' do + expect(subject).to include(status) + end + end + + # TODO: What happens if the viewer is remote? + # Can the viewer be remote? + # What prevents the viewer from being remote? + end end describe '.as_tag_timeline' do @@ -612,19 +638,19 @@ RSpec.describe Status, type: :model do let(:tag) { Fabricate(:tag) } let(:status) { Fabricate(:status, local_only: true, tags: [tag]) } - context 'if account is nil' do - let(:account) { nil } + context 'without a viewer' do + let(:viewer) { nil } it 'filters the local-only status out of the result set' do - expect(Status.as_tag_timeline(tag, account)).not_to include(status) + expect(Status.as_tag_timeline(tag, viewer)).not_to include(status) end end - context 'if account is not nil and local' do - let(:account) { Fabricate(:account, domain: nil) } + context 'with a viewer' do + let(:viewer) { Fabricate(:account, username: 'viewer', domain: nil) } it 'keeps the local-only status in the result set' do - expect(Status.as_tag_timeline(tag, account)).to include(status) + expect(Status.as_tag_timeline(tag, viewer)).to include(status) end end end