Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Revert "fix up a bit how searchable_by works in the real world (#7)" #8

Merged
merged 1 commit into from
Jul 21, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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