Skip to content

Commit

Permalink
Revert "fix up a bit how searchable_by works in the real world (#7)"
Browse files Browse the repository at this point in the history
This reverts commit 307ea70.
  • Loading branch information
jsgoldstein authored Jul 21, 2023
1 parent 307ea70 commit e9eb9f3
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 234 deletions.
34 changes: 6 additions & 28 deletions app/lib/search_query_transformer.rb
Original file line number Diff line number Diff line change
Expand Up @@ -12,34 +12,12 @@ def initialize(clauses)
@filter_clauses = grouped.fetch(:filter, [])
end

def apply(query)
query[:bool] ||= {}

if should_clauses.present?
query[:bool][:should] ||= []

query[:bool][:should] += should_clauses.map { |clause| clause_to_query(clause) }
query[:bool][:minimum_should_match] = 1
end

if must_clauses.present?
query[:bool][:must] ||= []

query[:bool][:must] += must_clauses.map { |clause| clause_to_query(clause) }
end

if must_not_clauses.present?
query[:bool][:must_not] ||= []

query[:bool][:must_not] += must_not_clauses.map { |clause| clause_to_query(clause) }
end

if filter_clauses.present?
query[:bool][:filter] ||= []
query[:bool][:filter] += filter_clauses.map { |clause| clause_to_filter(clause) }
end

query
def apply(search)
should_clauses.each { |clause| search = search.query.should(clause_to_query(clause)) }
must_clauses.each { |clause| search = search.query.must(clause_to_query(clause)) }
must_not_clauses.each { |clause| search = search.query.must_not(clause_to_query(clause)) }
filter_clauses.each { |clause| search = search.filter(**clause_to_filter(clause)) }
search.query.minimum_should_match(1)
end

private
Expand Down
5 changes: 0 additions & 5 deletions app/models/concerns/account_statuses_search.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,6 @@ module AccountStatusesSearch
extend ActiveSupport::Concern

def update_statuses_index!
return unless Chewy.enabled?

# This might not scale if a user has a TON of statuses.
# If that is the case, perhaps for users with many statuses, we should:
# (1) get all their statuses and (2) submit requests to ES in batches.
Chewy.strategy(:sidekiq) do
StatusesIndex.import(query: Status.where(account_id: id))
end
Expand Down
32 changes: 23 additions & 9 deletions app/services/search_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -39,15 +39,25 @@ def perform_accounts_search!
end

def perform_statuses_search!
StatusSearchService.new.call(
@query,
@account,
limit: @limit,
offset: @offset,
account_id: @options[:account_id],
min_id: @options[:min_id],
max_id: @options[:max_id]
)
definition = parsed_query.apply(StatusesIndex.filter(term: { searchable_by: @account.id }))

definition = definition.filter(term: { account_id: @options[:account_id] }) if @options[:account_id].present?

if @options[:min_id].present? || @options[:max_id].present?
range = {}
range[:gt] = @options[:min_id].to_i if @options[:min_id].present?
range[:lt] = @options[:max_id].to_i if @options[:max_id].present?
definition = definition.filter(range: { id: range })
end

results = definition.limit(@limit).offset(@offset).objects.compact
account_ids = results.map(&:account_id)
account_domains = results.map(&:account_domain)
preloaded_relations = @account.relations_map(account_ids, account_domains)

results.reject { |status| StatusFilter.new(status, @account, preloaded_relations).filtered? }
rescue Faraday::ConnectionFailed, Parslet::ParseFailed
[]
end

def perform_hashtags_search!
Expand Down Expand Up @@ -104,4 +114,8 @@ def hashtag_search?
def statuses_search?
@options[:type].blank? || @options[:type] == 'statuses'
end

def parsed_query
SearchQueryTransformer.new.apply(SearchQueryParser.new.parse(@query))
end
end
85 changes: 0 additions & 85 deletions app/services/status_search_service.rb

This file was deleted.

107 changes: 0 additions & 107 deletions spec/lib/search_query_transformer_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -15,111 +15,4 @@
expect(transformer.filter_clauses.first).to be_nil
end
end

describe '#apply' do
subject(:applied_query) do
described_class.new.apply(
SearchQueryParser.new.parse(search_term)
).apply(query)
end

let(:query) { { bool: {} } }
let(:search_term) { '' }

context 'when query is just a bool' do
it 'returns a hash of bool' do
expect(applied_query).to eq(query)
end
end

context 'when should_clauses are present' do
let(:search_term) { 'test' }

it 'adds should clauses to the query' do
expect(applied_query[:bool][:should].length).to eq(1)
expect(applied_query[:bool][:should][0][:multi_match][:query]).to eq(search_term)
end

it 'sets minimum_should_match to 1' do
expect(applied_query[:bool][:minimum_should_match]).to eq(1)
end
end

context 'when must_clauses are present' do
let(:search_term) { '+test' }

it 'adds must clauses to the query' do
expect(applied_query[:bool][:must].length).to eq(1)
expect(applied_query[:bool][:must][0][:multi_match][:query]).to eq(search_term[1..])
end
end

context 'when must_not_clauses are present' do
let(:search_term) { '-test' }

it 'adds must_not clauses to the query' do
expect(applied_query[:bool][:must_not].length).to eq(1)
expect(applied_query[:bool][:must_not][0][:multi_match][:query]).to eq(search_term[1..])
end
end

context 'when filter_clauses are present' do
let(:search_term) { 'from:@[email protected]' }

it 'adds filter clauses to the query' do
account = Fabricate(:account, username: 'test_account', domain: 'example.com')

expect(applied_query[:bool][:filter].length).to eq(1)
expect(applied_query[:bool][:filter][0][:term][:account_id]).to eq(account.id)
puts "Final count of Account records: #{Account.count}"
end
end

context 'when all clauses are present' do
let(:should_term) { 'should' }
let(:must_term) { '+must' }
let(:must_not_term) { '-nope' }
let(:filter_term) { 'from:@[email protected]' }
let(:search_term) { "#{should_term} #{must_term} #{must_not_term} #{filter_term}" }

it 'adds all clauses to the query' do
account = Fabricate(:account, username: 'test_account', domain: 'example.com')

expect(applied_query[:bool][:should][0][:multi_match][:query]).to eq(should_term)
expect(applied_query[:bool][:must][0][:multi_match][:query]).to eq(must_term[1..])
expect(applied_query[:bool][:must_not][0][:multi_match][:query]).to eq(must_not_term[1..])
expect(applied_query[:bool][:filter][0][:term][:account_id]).to eq(account.id)
end

it 'sets minimum_should_match to 1' do
Fabricate(:account, username: 'test_account', domain: 'example.com')

expect(applied_query[:bool][:minimum_should_match]).to eq(1)
end
end

context 'when all clauses are present and query has all clauses too' do
let(:should_term) { 'should' }
let(:must_term) { '+must' }
let(:must_not_term) { '-nope' }
let(:filter_term) { 'from:@[email protected]' }
let(:search_term) { "#{should_term} #{must_term} #{must_not_term} #{filter_term}" }
let(:query) { { bool: { should: [{ term: { exists: true } }], must: [{ term: { exists: true } }], must_not: [{ term: { exists: true } }], filter: [{ term: { exists: true } }] } } }

it 'adds all clauses to the arrays in the query' do
Fabricate(:account, username: 'test_account', domain: 'example.com')

expect(applied_query[:bool][:should].length).to eq(2)
expect(applied_query[:bool][:must].length).to eq(2)
expect(applied_query[:bool][:must_not].length).to eq(2)
expect(applied_query[:bool][:filter].length).to eq(2)
end

it 'sets minimum_should_match to 1' do
Fabricate(:account, username: 'test_account', domain: 'example.com')

expect(applied_query[:bool][:minimum_should_match]).to eq(1)
end
end
end
end

0 comments on commit e9eb9f3

Please sign in to comment.