Thread scopes through #matches?. #454.

Also add an apply_to_mentions attribute on Glitch::KeywordMute, which is
used to calculate scope.  Next up: additions to the test suite to
demonstrate how scoping works.
This commit is contained in:
David Yip 2018-06-03 19:41:54 -05:00
parent 37d495eeeb
commit 26573ad7e6
No known key found for this signature in database
GPG key ID: 7DA0036508FCC0CC
8 changed files with 99 additions and 61 deletions

View file

@ -153,7 +153,7 @@ class FeedManager
def filter_from_home?(status, receiver_id) def filter_from_home?(status, receiver_id)
return false if receiver_id == status.account_id 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?) return true if status.reply? && (status.in_reply_to_id.nil? || status.in_reply_to_account_id.nil?)
return true if keyword_filter?(status, receiver_id) return true if keyword_filter?(status, receiver_id, Glitch::KeywordMute::Scopes::HomeFeed)
check_for_mutes = [status.account_id] check_for_mutes = [status.account_id]
check_for_mutes.concat(status.mentions.pluck(:account_id)) check_for_mutes.concat(status.mentions.pluck(:account_id))
@ -182,8 +182,8 @@ class FeedManager
false false
end end
def keyword_filter?(status, receiver_id) def keyword_filter?(status, receiver_id, scope)
Glitch::KeywordMuteHelper.new(receiver_id).matches?(status) Glitch::KeywordMuteHelper.new(receiver_id).matches?(status, scope)
end end
def filter_from_mentions?(status, receiver_id) def filter_from_mentions?(status, receiver_id)
@ -197,7 +197,7 @@ class FeedManager
should_filter = blocks_or_mutes?(receiver_id, check_for_blocks, :mentions) # Filter if it's from someone I blocked, in reply to someone I blocked, or mentioning someone I blocked (or muted) should_filter = blocks_or_mutes?(receiver_id, check_for_blocks, :mentions) # Filter if it's from someone I blocked, in reply to someone I blocked, or mentioning someone I blocked (or muted)
should_filter ||= (status.account.silenced? && !Follow.where(account_id: receiver_id, target_account_id: status.account_id).exists?) # of if the account is silenced and I'm not following them should_filter ||= (status.account.silenced? && !Follow.where(account_id: receiver_id, target_account_id: status.account_id).exists?) # of if the account is silenced and I'm not following them
should_filter ||= keyword_filter?(status, receiver_id) # or if the mention contains a muted keyword should_filter ||= keyword_filter?(status, receiver_id, Glitch::KeywordMute::Scopes::Mentions) # or if the mention contains a muted keyword
should_filter should_filter
end end

View file

@ -3,11 +3,11 @@
# #
# Table name: bookmarks # Table name: bookmarks
# #
# id :integer not null, primary key # id :bigint(8) not null, primary key
# account_id :bigint(8) not null
# status_id :bigint(8) not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# account_id :integer not null
# status_id :integer not null
# #
class Bookmark < ApplicationRecord class Bookmark < ApplicationRecord

View file

@ -3,12 +3,13 @@
# #
# Table name: glitch_keyword_mutes # Table name: glitch_keyword_mutes
# #
# id :integer not null, primary key # id :bigint(8) not null, primary key
# account_id :integer not null # account_id :bigint(8) not null
# keyword :string not null # keyword :string not null
# whole_word :boolean default(TRUE), not null # whole_word :boolean default(TRUE), not null
# created_at :datetime not null # created_at :datetime not null
# updated_at :datetime not null # updated_at :datetime not null
# apply_to_mentions :boolean default(TRUE), not null
# #
class Glitch::KeywordMute < ApplicationRecord class Glitch::KeywordMute < ApplicationRecord
@ -18,6 +19,12 @@ class Glitch::KeywordMute < ApplicationRecord
after_commit :invalidate_cached_matchers after_commit :invalidate_cached_matchers
module Scopes
Unscoped = 0b00
HomeFeed = 0b01
Mentions = 0b10
end
def self.text_matcher_for(account_id) def self.text_matcher_for(account_id)
TextMatcher.new(account_id) TextMatcher.new(account_id)
end end
@ -26,6 +33,13 @@ class Glitch::KeywordMute < ApplicationRecord
TagMatcher.new(account_id) TagMatcher.new(account_id)
end end
def scope
s = Scopes::Unscoped
s |= Scopes::HomeFeed
s |= Scopes::Mentions if apply_to_mentions?
s
end
private private
def invalidate_cached_matchers def invalidate_cached_matchers
@ -36,10 +50,12 @@ class Glitch::KeywordMute < ApplicationRecord
class CachedKeywordMute class CachedKeywordMute
attr_reader :keyword attr_reader :keyword
attr_reader :whole_word attr_reader :whole_word
attr_reader :scope
def initialize(keyword, whole_word) def initialize(keyword, whole_word, scope)
@keyword = keyword @keyword = keyword
@whole_word = whole_word @whole_word = whole_word
@scope = scope
end end
def boundary_regex_for_keyword def boundary_regex_for_keyword
@ -49,26 +65,27 @@ class Glitch::KeywordMute < ApplicationRecord
/(?mix:#{sb}#{Regexp.escape(keyword)}#{eb})/ /(?mix:#{sb}#{Regexp.escape(keyword)}#{eb})/
end end
def matches?(str) def matches?(str, required_scope)
str =~ (whole_word ? boundary_regex_for_keyword : /#{keyword}/i) ((required_scope & scope) == required_scope) && \
str =~ (whole_word ? boundary_regex_for_keyword : /#{keyword}/i)
end end
end end
class Matcher class Matcher
attr_reader :account_id attr_reader :account_id
attr_reader :words attr_reader :keywords
def initialize(account_id) def initialize(account_id)
@account_id = account_id @account_id = account_id
@words = Rails.cache.fetch(self.class.cache_key(account_id)) { fetch_keywords } @keywords = Rails.cache.fetch(self.class.cache_key(account_id)) { fetch_keywords }
end end
protected protected
def fetch_keywords def fetch_keywords
Glitch::KeywordMute.where(account_id: account_id).pluck(:whole_word, :keyword).map do |whole_word, keyword| Glitch::KeywordMute.select(:whole_word, :keyword, :apply_to_mentions)
CachedKeywordMute.new(transform_keyword(keyword), whole_word) .where(account_id: account_id)
end .map { |kw| CachedKeywordMute.new(transform_keyword(kw.keyword), kw.whole_word, kw.scope) }
end end
def transform_keyword(keyword) def transform_keyword(keyword)
@ -81,8 +98,8 @@ class Glitch::KeywordMute < ApplicationRecord
format('keyword_mutes:regex:text:%s', account_id) format('keyword_mutes:regex:text:%s', account_id)
end end
def matches?(str) def matches?(str, scope)
words.any? { |w| w.matches?(str) } keywords.any? { |kw| kw.matches?(str, scope) }
end end
end end
@ -91,9 +108,9 @@ class Glitch::KeywordMute < ApplicationRecord
format('keyword_mutes:regex:tag:%s', account_id) format('keyword_mutes:regex:tag:%s', account_id)
end end
def matches?(tags) def matches?(tags, scope)
tags.pluck(:name).any? do |n| tags.pluck(:name).any? do |n|
words.any? { |w| w.matches?(n) } keywords.any? { |kw| kw.matches?(n, scope) }
end end
end end

View file

@ -9,16 +9,16 @@ class Glitch::KeywordMuteHelper
@tag_matcher = Glitch::KeywordMute.tag_matcher_for(receiver_id) @tag_matcher = Glitch::KeywordMute.tag_matcher_for(receiver_id)
end end
def matches?(status) def matches?(status, scope)
matchers_match?(status) || (status.reblog? && matchers_match?(status.reblog)) matchers_match?(status, scope) || (status.reblog? && matchers_match?(status.reblog, scope))
end end
private private
def matchers_match?(status) def matchers_match?(status, scope)
text_matcher.matches?(prepare_text(status.text)) || text_matcher.matches?(prepare_text(status.text), scope) ||
text_matcher.matches?(prepare_text(status.spoiler_text)) || text_matcher.matches?(prepare_text(status.spoiler_text), scope) ||
tag_matcher.matches?(status.tags) tag_matcher.matches?(status.tags, scope)
end end
def prepare_text(text) def prepare_text(text)

View file

@ -0,0 +1,17 @@
require 'mastodon/migration_helpers'
class AddApplyToMentionsFlagToKeywordMutes < ActiveRecord::Migration[5.2]
include Mastodon::MigrationHelpers
disable_ddl_transaction!
def up
safety_assured do
add_column_with_default :glitch_keyword_mutes, :apply_to_mentions, :boolean, allow_null: false, default: true
end
end
def down
remove_column :glitch_keyword_mutes, :apply_to_mentions
end
end

View file

@ -10,7 +10,7 @@
# #
# It's strongly recommended that you check this file into your version control system. # It's strongly recommended that you check this file into your version control system.
ActiveRecord::Schema.define(version: 2018_05_28_141303) do ActiveRecord::Schema.define(version: 2018_06_04_000556) do
# These are extensions that must be enabled in order to support this database # These are extensions that must be enabled in order to support this database
enable_extension "plpgsql" enable_extension "plpgsql"
@ -206,6 +206,7 @@ ActiveRecord::Schema.define(version: 2018_05_28_141303) do
t.boolean "whole_word", default: true, null: false t.boolean "whole_word", default: true, null: false
t.datetime "created_at", null: false t.datetime "created_at", null: false
t.datetime "updated_at", null: false t.datetime "updated_at", null: false
t.boolean "apply_to_mentions", default: true, null: false
t.index ["account_id"], name: "index_glitch_keyword_mutes_on_account_id" t.index ["account_id"], name: "index_glitch_keyword_mutes_on_account_id"
end end
@ -466,7 +467,6 @@ ActiveRecord::Schema.define(version: 2018_05_28_141303) do
t.bigint "application_id" t.bigint "application_id"
t.bigint "in_reply_to_account_id" t.bigint "in_reply_to_account_id"
t.boolean "local_only" t.boolean "local_only"
t.text "full_status_text", default: "", null: false
t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20180106", order: { id: :desc } t.index ["account_id", "id", "visibility", "updated_at"], name: "index_statuses_20180106", order: { id: :desc }
t.index ["conversation_id"], name: "index_statuses_on_conversation_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" t.index ["in_reply_to_id"], name: "index_statuses_on_in_reply_to_id"

View file

@ -2,6 +2,8 @@ require 'rails_helper'
RSpec.describe Glitch::KeywordMuteHelper do RSpec.describe Glitch::KeywordMuteHelper do
describe '#matches?' do describe '#matches?' do
Unscoped = Glitch::KeywordMute::Scopes::Unscoped
let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) }
let(:helper) { Glitch::KeywordMuteHelper.new(alice) } let(:helper) { Glitch::KeywordMuteHelper.new(alice) }
@ -9,42 +11,42 @@ RSpec.describe Glitch::KeywordMuteHelper do
status = Fabricate(:status, text: '<addr>uh example</addr>') status = Fabricate(:status, text: '<addr>uh example</addr>')
Glitch::KeywordMute.create!(account: alice, keyword: 'addr') Glitch::KeywordMute.create!(account: alice, keyword: 'addr')
expect(helper.matches?(status)).to be false expect(helper.matches?(status, Unscoped)).to be false
end end
it 'ignores properties of HTML tags in status text' do it 'ignores properties of HTML tags in status text' do
status = Fabricate(:status, text: '<a href="https://www.example.org">uh example</a>') status = Fabricate(:status, text: '<a href="https://www.example.org">uh example</a>')
Glitch::KeywordMute.create!(account: alice, keyword: 'href') Glitch::KeywordMute.create!(account: alice, keyword: 'href')
expect(helper.matches?(status)).to be false expect(helper.matches?(status, Unscoped)).to be false
end end
it 'matches text inside HTML tags' do it 'matches text inside HTML tags' do
status = Fabricate(:status, text: '<p>HEY THIS IS SOMETHING ANNOYING</p>') status = Fabricate(:status, text: '<p>HEY THIS IS SOMETHING ANNOYING</p>')
Glitch::KeywordMute.create!(account: alice, keyword: 'annoying') Glitch::KeywordMute.create!(account: alice, keyword: 'annoying')
expect(helper.matches?(status)).to be true expect(helper.matches?(status, Unscoped)).to be true
end end
it 'matches < in HTML-stripped text' do it 'matches < in HTML-stripped text' do
status = Fabricate(:status, text: '<p>I <3 oats</p>') status = Fabricate(:status, text: '<p>I <3 oats</p>')
Glitch::KeywordMute.create!(account: alice, keyword: '<3') Glitch::KeywordMute.create!(account: alice, keyword: '<3')
expect(helper.matches?(status)).to be true expect(helper.matches?(status, Unscoped)).to be true
end end
it 'matches &lt; in HTML text' do it 'matches &lt; in HTML text' do
status = Fabricate(:status, text: '<p>I &lt;3 oats</p>') status = Fabricate(:status, text: '<p>I &lt;3 oats</p>')
Glitch::KeywordMute.create!(account: alice, keyword: '<3') Glitch::KeywordMute.create!(account: alice, keyword: '<3')
expect(helper.matches?(status)).to be true expect(helper.matches?(status, Unscoped)).to be true
end end
it 'matches link hrefs in HTML text' do it 'matches link hrefs in HTML text' do
status = Fabricate(:status, text: '<p><a href="https://example.com/it-was-milk">yep</a></p>') status = Fabricate(:status, text: '<p><a href="https://example.com/it-was-milk">yep</a></p>')
Glitch::KeywordMute.create!(account: alice, keyword: 'milk') Glitch::KeywordMute.create!(account: alice, keyword: 'milk')
expect(helper.matches?(status)).to be true expect(helper.matches?(status, Unscoped)).to be true
end end
end end
end end

View file

@ -4,6 +4,8 @@ RSpec.describe Glitch::KeywordMute, type: :model do
let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) }
let(:bob) { Fabricate(:account, username: 'bob').tap(&:save!) } let(:bob) { Fabricate(:account, username: 'bob').tap(&:save!) }
Unscoped = Glitch::KeywordMute::Scopes::Unscoped
describe '.text_matcher_for' do describe '.text_matcher_for' do
let(:matcher) { Glitch::KeywordMute.text_matcher_for(alice.id) } let(:matcher) { Glitch::KeywordMute.text_matcher_for(alice.id) }
@ -13,7 +15,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do
end end
it 'does not match' do it 'does not match' do
expect(matcher.matches?('This is a hot take')).to be_falsy expect(matcher.matches?('This is a hot take', Unscoped)).to be_falsy
end end
end end
@ -21,81 +23,81 @@ RSpec.describe Glitch::KeywordMute, type: :model do
it 'does not match keywords set by a different account' do it 'does not match keywords set by a different account' do
Glitch::KeywordMute.create!(account: bob, keyword: 'take') Glitch::KeywordMute.create!(account: bob, keyword: 'take')
expect(matcher.matches?('This is a hot take')).to be_falsy expect(matcher.matches?('This is a hot take', Unscoped)).to be_falsy
end end
it 'does not match if no keywords match the status text' do it 'does not match if no keywords match the status text' do
Glitch::KeywordMute.create!(account: alice, keyword: 'cold') Glitch::KeywordMute.create!(account: alice, keyword: 'cold')
expect(matcher.matches?('This is a hot take')).to be_falsy expect(matcher.matches?('This is a hot take', Unscoped)).to be_falsy
end end
it 'considers word boundaries when matching' do it 'considers word boundaries when matching' do
Glitch::KeywordMute.create!(account: alice, keyword: 'bob', whole_word: true) Glitch::KeywordMute.create!(account: alice, keyword: 'bob', whole_word: true)
expect(matcher.matches?('bobcats')).to be_falsy expect(matcher.matches?('bobcats', Unscoped)).to be_falsy
end end
it 'matches substrings if whole_word is false' do it 'matches substrings if whole_word is false' do
Glitch::KeywordMute.create!(account: alice, keyword: 'take', whole_word: false) Glitch::KeywordMute.create!(account: alice, keyword: 'take', whole_word: false)
expect(matcher.matches?('This is a shiitake mushroom')).to be_truthy expect(matcher.matches?('This is a shiitake mushroom', Unscoped)).to be_truthy
end end
it 'matches keywords at the beginning of the text' do it 'matches keywords at the beginning of the text' do
Glitch::KeywordMute.create!(account: alice, keyword: 'take') Glitch::KeywordMute.create!(account: alice, keyword: 'take')
expect(matcher.matches?('Take this')).to be_truthy expect(matcher.matches?('Take this', Unscoped)).to be_truthy
end end
it 'matches keywords at the end of the text' do it 'matches keywords at the end of the text' do
Glitch::KeywordMute.create!(account: alice, keyword: 'take') Glitch::KeywordMute.create!(account: alice, keyword: 'take')
expect(matcher.matches?('This is a hot take')).to be_truthy expect(matcher.matches?('This is a hot take', Unscoped)).to be_truthy
end end
it 'matches if at least one keyword case-insensitively matches the text' do it 'matches if at least one keyword case-insensitively matches the text' do
Glitch::KeywordMute.create!(account: alice, keyword: 'hot') Glitch::KeywordMute.create!(account: alice, keyword: 'hot')
expect(matcher.matches?('This is a HOT take')).to be_truthy expect(matcher.matches?('This is a HOT take', Unscoped)).to be_truthy
end end
it 'matches if at least one non-whole-word keyword case-insensitively matches the text' do it 'matches if at least one non-whole-word keyword case-insensitively matches the text' do
Glitch::KeywordMute.create!(account: alice, keyword: 'hot', whole_word: false) Glitch::KeywordMute.create!(account: alice, keyword: 'hot', whole_word: false)
expect(matcher.matches?('This is a HOTTY take')).to be_truthy expect(matcher.matches?('This is a HOTTY take', Unscoped)).to be_truthy
end end
it 'maintains case-insensitivity when combining keywords into a single matcher' do it 'maintains case-insensitivity when combining keywords into a single matcher' do
Glitch::KeywordMute.create!(account: alice, keyword: 'hot') Glitch::KeywordMute.create!(account: alice, keyword: 'hot')
Glitch::KeywordMute.create!(account: alice, keyword: 'cold') Glitch::KeywordMute.create!(account: alice, keyword: 'cold')
expect(matcher.matches?('This is a HOT take')).to be_truthy expect(matcher.matches?('This is a HOT take', Unscoped)).to be_truthy
end end
it 'matches keywords surrounded by non-alphanumeric ornamentation' do it 'matches keywords surrounded by non-alphanumeric ornamentation' do
Glitch::KeywordMute.create!(account: alice, keyword: 'hot') Glitch::KeywordMute.create!(account: alice, keyword: 'hot')
expect(matcher.matches?('(hot take)')).to be_truthy expect(matcher.matches?('(hot take)', Unscoped)).to be_truthy
end end
it 'escapes metacharacters in keywords' do it 'escapes metacharacters in keywords' do
Glitch::KeywordMute.create!(account: alice, keyword: '(hot take)') Glitch::KeywordMute.create!(account: alice, keyword: '(hot take)')
expect(matcher.matches?('(hot take)')).to be_truthy expect(matcher.matches?('(hot take)', Unscoped)).to be_truthy
end end
it 'uses case-folding rules appropriate for more than just English' do it 'uses case-folding rules appropriate for more than just English' do
Glitch::KeywordMute.create!(account: alice, keyword: 'großeltern') Glitch::KeywordMute.create!(account: alice, keyword: 'großeltern')
expect(matcher.matches?('besuch der grosseltern')).to be_truthy expect(matcher.matches?('besuch der grosseltern', Unscoped)).to be_truthy
end end
it 'matches keywords that are composed of multiple words' do it 'matches keywords that are composed of multiple words' do
Glitch::KeywordMute.create!(account: alice, keyword: 'a shiitake') Glitch::KeywordMute.create!(account: alice, keyword: 'a shiitake')
expect(matcher.matches?('This is a shiitake')).to be_truthy expect(matcher.matches?('This is a shiitake', Unscoped)).to be_truthy
expect(matcher.matches?('This is shiitake')).to_not be_truthy expect(matcher.matches?('This is shiitake', Unscoped)).to_not be_truthy
end end
end end
end end
@ -112,7 +114,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do
it 'does not match' do it 'does not match' do
status.tags << Fabricate(:tag, name: 'xyzzy') status.tags << Fabricate(:tag, name: 'xyzzy')
expect(matcher.matches?(status.tags)).to be false expect(matcher.matches?(status.tags, Unscoped)).to be false
end end
end end
@ -121,42 +123,42 @@ RSpec.describe Glitch::KeywordMute, type: :model do
status.tags << Fabricate(:tag, name: 'xyzzy') status.tags << Fabricate(:tag, name: 'xyzzy')
Glitch::KeywordMute.create!(account: bob, keyword: 'take') Glitch::KeywordMute.create!(account: bob, keyword: 'take')
expect(matcher.matches?(status.tags)).to be false expect(matcher.matches?(status.tags, Unscoped)).to be false
end end
it 'matches #xyzzy when given the mute "#xyzzy"' do it 'matches #xyzzy when given the mute "#xyzzy"' do
status.tags << Fabricate(:tag, name: 'xyzzy') status.tags << Fabricate(:tag, name: 'xyzzy')
Glitch::KeywordMute.create!(account: alice, keyword: '#xyzzy') Glitch::KeywordMute.create!(account: alice, keyword: '#xyzzy')
expect(matcher.matches?(status.tags)).to be true expect(matcher.matches?(status.tags, Unscoped)).to be true
end end
it 'matches #thingiverse when given the non-whole-word mute "#thing"' do it 'matches #thingiverse when given the non-whole-word mute "#thing"' do
status.tags << Fabricate(:tag, name: 'thingiverse') status.tags << Fabricate(:tag, name: 'thingiverse')
Glitch::KeywordMute.create!(account: alice, keyword: '#thing', whole_word: false) Glitch::KeywordMute.create!(account: alice, keyword: '#thing', whole_word: false)
expect(matcher.matches?(status.tags)).to be true expect(matcher.matches?(status.tags, Unscoped)).to be true
end end
it 'matches #hashtag when given the mute "##hashtag""' do it 'matches #hashtag when given the mute "##hashtag""' do
status.tags << Fabricate(:tag, name: 'hashtag') status.tags << Fabricate(:tag, name: 'hashtag')
Glitch::KeywordMute.create!(account: alice, keyword: '##hashtag') Glitch::KeywordMute.create!(account: alice, keyword: '##hashtag')
expect(matcher.matches?(status.tags)).to be true expect(matcher.matches?(status.tags, Unscoped)).to be true
end end
it 'matches #oatmeal when given the non-whole-word mute "oat"' do it 'matches #oatmeal when given the non-whole-word mute "oat"' do
status.tags << Fabricate(:tag, name: 'oatmeal') status.tags << Fabricate(:tag, name: 'oatmeal')
Glitch::KeywordMute.create!(account: alice, keyword: 'oat', whole_word: false) Glitch::KeywordMute.create!(account: alice, keyword: 'oat', whole_word: false)
expect(matcher.matches?(status.tags)).to be true expect(matcher.matches?(status.tags, Unscoped)).to be true
end end
it 'does not match #oatmeal when given the mute "#oat"' do it 'does not match #oatmeal when given the mute "#oat"' do
status.tags << Fabricate(:tag, name: 'oatmeal') status.tags << Fabricate(:tag, name: 'oatmeal')
Glitch::KeywordMute.create!(account: alice, keyword: 'oat') Glitch::KeywordMute.create!(account: alice, keyword: 'oat')
expect(matcher.matches?(status.tags)).to be false expect(matcher.matches?(status.tags, Unscoped)).to be false
end end
end end
end end