From 670e6a33f8eeca628707dc020e02ce32502d74a4 Mon Sep 17 00:00:00 2001 From: David Yip Date: Sat, 21 Oct 2017 14:47:17 -0500 Subject: Move KeywordMute into Glitch namespace. There are two motivations for this: 1. It looks like we're going to add other features that require server-side storage (e.g. user notes). 2. Namespacing glitchsoc modifications is a good idea anyway: even if we do not end up doing (1), if upstream introduces a keyword-mute feature that also uses a "KeywordMute" model, we can avoid some merge conflicts this way and work on the more interesting task of choosing which implementation to use. --- spec/models/glitch/keyword_mute_spec.rb | 83 +++++++++++++++++++++++++++++++++ 1 file changed, 83 insertions(+) create mode 100644 spec/models/glitch/keyword_mute_spec.rb (limited to 'spec/models/glitch') diff --git a/spec/models/glitch/keyword_mute_spec.rb b/spec/models/glitch/keyword_mute_spec.rb new file mode 100644 index 000000000..108cdafec --- /dev/null +++ b/spec/models/glitch/keyword_mute_spec.rb @@ -0,0 +1,83 @@ +require 'rails_helper' + +RSpec.describe Glitch::KeywordMute, type: :model do + let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } + let(:bob) { Fabricate(:account, username: 'bob').tap(&:save!) } + + describe '.matcher_for' do + let(:matcher) { Glitch::KeywordMute.matcher_for(alice) } + + describe 'with no Glitch::KeywordMutes for an account' do + before do + Glitch::KeywordMute.delete_all + end + + it 'does not match' do + expect(matcher =~ 'This is a hot take').to be_falsy + end + end + + describe 'with Glitch::KeywordMutes for an account' do + it 'does not match keywords set by a different account' do + Glitch::KeywordMute.create!(account: bob, keyword: 'take') + + expect(matcher =~ 'This is a hot take').to be_falsy + end + + it 'does not match if no keywords match the status text' do + Glitch::KeywordMute.create!(account: alice, keyword: 'cold') + + expect(matcher =~ 'This is a hot take').to be_falsy + end + + it 'considers word boundaries when matching' do + Glitch::KeywordMute.create!(account: alice, keyword: 'bob', whole_word: true) + + expect(matcher =~ 'bobcats').to be_falsy + end + + it 'matches substrings if whole_word is false' do + Glitch::KeywordMute.create!(account: alice, keyword: 'take', whole_word: false) + + expect(matcher =~ 'This is a shiitake mushroom').to be_truthy + end + + it 'matches keywords at the beginning of the text' do + Glitch::KeywordMute.create!(account: alice, keyword: 'take') + + expect(matcher =~ 'Take this').to be_truthy + end + + it 'matches keywords at the beginning of the text' do + Glitch::KeywordMute.create!(account: alice, keyword: 'take') + + expect(matcher =~ 'This is a hot take').to be_truthy + end + + it 'matches if at least one keyword case-insensitively matches the text' do + Glitch::KeywordMute.create!(account: alice, keyword: 'hot') + + expect(matcher =~ 'This is a HOT take').to be_truthy + end + + it 'matches keywords surrounded by non-alphanumeric ornamentation' do + Glitch::KeywordMute.create!(account: alice, keyword: 'hot') + + expect(matcher =~ 'This is a ~*HOT*~ take').to be_truthy + end + + it 'uses case-folding rules appropriate for more than just English' do + Glitch::KeywordMute.create!(account: alice, keyword: 'großeltern') + + expect(matcher =~ 'besuch der grosseltern').to be_truthy + end + + it 'matches keywords that are composed of multiple words' do + Glitch::KeywordMute.create!(account: alice, keyword: 'a shiitake') + + expect(matcher =~ 'This is a shiitake').to be_truthy + expect(matcher =~ 'This is shiitake').to_not be_truthy + end + end + end +end -- cgit From 4b68e82a19ab2853264515a25af8d39841e43f00 Mon Sep 17 00:00:00 2001 From: David Yip Date: Sun, 22 Oct 2017 00:24:32 -0500 Subject: Don't add \b to whole-word keywords that don't start with word characters. Ditto for ending with \b. Consider muting the phrase "(hot take)". I stipulate it is reasonable to enter this with the default "match whole word" behavior. Under the old behavior, this would be encoded as \b\(hot\ take\)\b However, if \b is before the first character in the string and the first character in the string is not a word character, then the match will fail. Ditto for after. In our example, "(" is not a word character, so this will not match statuses containing "(hot take)", and that's a very surprising behavior. To address this, we only add leading and trailing \b to keywords that start or end with word characters. --- app/models/glitch/keyword_mute.rb | 36 +++++++++++++++++++++++---------- spec/models/glitch/keyword_mute_spec.rb | 12 ++++++++--- 2 files changed, 34 insertions(+), 14 deletions(-) (limited to 'spec/models/glitch') diff --git a/app/models/glitch/keyword_mute.rb b/app/models/glitch/keyword_mute.rb index 823e252d3..20fd89d9b 100644 --- a/app/models/glitch/keyword_mute.rb +++ b/app/models/glitch/keyword_mute.rb @@ -19,35 +19,49 @@ class Glitch::KeywordMute < ApplicationRecord after_commit :invalidate_cached_matcher def self.matcher_for(account_id) - Rails.cache.fetch("keyword_mutes:matcher:#{account_id}") { Matcher.new(account_id) } + Matcher.new(account_id) end private def invalidate_cached_matcher - Rails.cache.delete("keyword_mutes:matcher:#{account_id}") + Rails.cache.delete("keyword_mutes:regex:#{account_id}") end class Matcher + attr_reader :account_id attr_reader :regex def initialize(account_id) - re = [].tap do |arr| - Glitch::KeywordMute.where(account_id: account_id).select(:keyword, :id, :whole_word).find_each do |m| - boundary = m.whole_word ? '\b' : '' - arr << "#{boundary}#{Regexp.escape(m.keyword.strip)}#{boundary}" + @account_id = account_id + @regex = Rails.cache.fetch("keyword_mutes:regex:#{account_id}") { regex_for_account } + end + + def keywords + Glitch::KeywordMute. + where(account_id: account_id). + select(:keyword, :id, :whole_word) + end + + def regex_for_account + re_text = [].tap do |arr| + keywords.find_each do |kw| + arr << (kw.whole_word ? boundary_regex_for_keyword(kw.keyword) : Regexp.escape(kw.keyword)) end end.join('|') - @regex = /#{re}/i unless re.empty? + /#{re_text}/i unless re_text.empty? end - def =~(str) - regex ? regex =~ str : false + def boundary_regex_for_keyword(keyword) + sb = keyword =~ /\A[[:word:]]/ ? '\b' : '' + eb = keyword =~ /[[:word:]]\Z/ ? '\b' : '' + + "#{sb}#{Regexp.escape(keyword)}#{eb}" end - def matches?(str) - !!(regex =~ str) + def =~(str) + regex ? regex =~ str : false end end end diff --git a/spec/models/glitch/keyword_mute_spec.rb b/spec/models/glitch/keyword_mute_spec.rb index 108cdafec..95e59defc 100644 --- a/spec/models/glitch/keyword_mute_spec.rb +++ b/spec/models/glitch/keyword_mute_spec.rb @@ -7,7 +7,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do describe '.matcher_for' do let(:matcher) { Glitch::KeywordMute.matcher_for(alice) } - describe 'with no Glitch::KeywordMutes for an account' do + describe 'with no mutes' do before do Glitch::KeywordMute.delete_all end @@ -17,7 +17,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do end end - describe 'with Glitch::KeywordMutes for an account' do + describe 'with mutes' do it 'does not match keywords set by a different account' do Glitch::KeywordMute.create!(account: bob, keyword: 'take') @@ -63,7 +63,13 @@ RSpec.describe Glitch::KeywordMute, type: :model do it 'matches keywords surrounded by non-alphanumeric ornamentation' do Glitch::KeywordMute.create!(account: alice, keyword: 'hot') - expect(matcher =~ 'This is a ~*HOT*~ take').to be_truthy + expect(matcher =~ '(hot take)').to be_truthy + end + + it 'escapes metacharacters in keywords' do + Glitch::KeywordMute.create!(account: alice, keyword: '(hot take)') + + expect(matcher =~ '(hot take)').to be_truthy end it 'uses case-folding rules appropriate for more than just English' do -- cgit From 641f90e73adcc9718e610ec6c233d863e94fb92c Mon Sep 17 00:00:00 2001 From: David Yip Date: Tue, 24 Oct 2017 18:33:02 -0500 Subject: Fix example description. This example actually checks matches at the end of a string. --- spec/models/glitch/keyword_mute_spec.rb | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) (limited to 'spec/models/glitch') diff --git a/spec/models/glitch/keyword_mute_spec.rb b/spec/models/glitch/keyword_mute_spec.rb index 95e59defc..1423823ba 100644 --- a/spec/models/glitch/keyword_mute_spec.rb +++ b/spec/models/glitch/keyword_mute_spec.rb @@ -48,7 +48,7 @@ RSpec.describe Glitch::KeywordMute, type: :model do expect(matcher =~ 'Take this').to be_truthy end - it 'matches keywords at the beginning of the text' do + it 'matches keywords at the end of the text' do Glitch::KeywordMute.create!(account: alice, keyword: 'take') expect(matcher =~ 'This is a hot take').to be_truthy -- cgit