From 603cf02b703a2df2ae6690077a3e21a5ce64b548 Mon Sep 17 00:00:00 2001 From: David Yip Date: Sat, 14 Oct 2017 20:36:53 -0500 Subject: [PATCH] Rework KeywordMute interface to use a matcher object; spec out matcher. #164. A matcher object that builds a match from KeywordMute data and runs it over text is, in my view, one of the easier ways to write examples for this sort of thing. --- app/lib/feed_manager.rb | 2 +- app/models/keyword_mute.rb | 31 +++++++++++++- spec/models/keyword_mute_spec.rb | 70 +++++++++++++++++++++++++++----- 3 files changed, 91 insertions(+), 12 deletions(-) diff --git a/app/lib/feed_manager.rb b/app/lib/feed_manager.rb index baaa09e86..516bd81af 100644 --- a/app/lib/feed_manager.rb +++ b/app/lib/feed_manager.rb @@ -138,7 +138,7 @@ class FeedManager end def filter_from_home?(status, receiver_id) - return true if KeywordMute.where(account_id: receiver_id).matches?(status.text) + return true if KeywordMute.matcher_for(receiver_id) =~ status.text 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?) diff --git a/app/models/keyword_mute.rb b/app/models/keyword_mute.rb index d397a1f41..d80fcaa60 100644 --- a/app/models/keyword_mute.rb +++ b/app/models/keyword_mute.rb @@ -1,3 +1,4 @@ +# frozen_string_literal: true # == Schema Information # # Table name: keyword_mutes @@ -10,6 +11,34 @@ # class KeywordMute < ApplicationRecord - def self.matches?(text) + belongs_to :account, required: true + + validates_presence_of :keyword + + def self.matcher_for(account) + Rails.cache.fetch("keyword_mutes:matcher:#{account}") { Matcher.new(account) } + end + + class Matcher + attr_reader :regex + + def initialize(account) + re = String.new.tap do |str| + scoped = KeywordMute.where(account: account) + keywords = scoped.select(:id, :keyword) + count = scoped.count + + keywords.find_each.with_index do |kw, index| + str << Regexp.escape(kw.keyword.strip) + str << '|' if index < count - 1 + end + end + + @regex = /\b(?:#{re})\b/i unless re.empty? + end + + def =~(str) + @regex ? @regex =~ str : false + end end end diff --git a/spec/models/keyword_mute_spec.rb b/spec/models/keyword_mute_spec.rb index cb6e554e4..211a9b4c6 100644 --- a/spec/models/keyword_mute_spec.rb +++ b/spec/models/keyword_mute_spec.rb @@ -1,21 +1,71 @@ require 'rails_helper' RSpec.describe KeywordMute, type: :model do - describe '.matches?' do - let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } - let(:status) { Fabricate(:status, account: alice).tap(&:save!) } - let(:keyword_mute) { Fabricate(:keyword_mute, account: alice, keyword: 'take').tap(&:save!) } + let(:alice) { Fabricate(:account, username: 'alice').tap(&:save!) } + let(:bob) { Fabricate(:account, username: 'bob').tap(&:save!) } - it 'returns true if any keyword in the set matches the status text' do - status.update_attribute(:text, 'This is a hot take') + describe '.matcher_for' do + let(:matcher) { KeywordMute.matcher_for(alice) } - expect(KeywordMute.where(account: alice).matches?(status.text)).to be_truthy + describe 'with no KeywordMutes for an account' do + before do + KeywordMute.delete_all + end + + it 'does not match' do + expect(matcher =~ 'This is a hot take').to be_falsy + end end - it 'returns false if no keyword in the set matches the status text' + describe 'with KeywordMutes for an account' do + it 'does not match keywords set by a different account' do + KeywordMute.create!(account: bob, keyword: 'take') - describe 'matching' do - it 'is case-insensitive' + expect(matcher =~ 'This is a hot take').to be_falsy + end + + it 'does not match if no keywords match the status text' do + KeywordMute.create!(account: alice, keyword: 'cold') + + expect(matcher =~ 'This is a hot take').to be_falsy + end + + it 'does not match substrings matching keywords' do + KeywordMute.create!(account: alice, keyword: 'take') + + expect(matcher =~ 'This is a shiitake mushroom').to be_falsy + end + + it 'matches keywords at the beginning of the text' do + KeywordMute.create!(account: alice, keyword: 'take') + + expect(matcher =~ 'Take this').to be_truthy + end + + it 'matches keywords at the beginning of the text' do + 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 + 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 + 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 + 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