Skip to content

Commit

Permalink
Add support for query string parameters
Browse files Browse the repository at this point in the history
Per ES docs: search_type, request_cache, and the allow_partial_search_results
should not be in the body. They should be sent as query string parameters. My
understanding is that chewy should act like this:

  $ IssuesIndex.request_cache(true).render
  # good (new)
  $ {:index=>["issues"], :type=>["issue"], :body=>{}, :request_cache=>true}
  # bad (old)
  $ {:index=>["issues"], :type=>["issue"], :body=>{:request_cache=>true}

I believe this resolves issue toptal#586 and part of issue toptal#642.

(https://www.elastic.co/guide/en/elasticsearch/reference/6.4/search-request-body.html#_parameters_4)
  • Loading branch information
mattzollinhofer authored and Çağatay Yücelen committed Jan 28, 2023
1 parent 5503f48 commit dbfe243
Show file tree
Hide file tree
Showing 3 changed files with 39 additions and 9 deletions.
23 changes: 18 additions & 5 deletions lib/chewy/search/parameters.rb
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ module Search
# @see Chewy::Search::Request#parameters
# @see Chewy::Search::Parameters::Storage
class Parameters
QUERY_STRING_PARAMS = %i[search_type request_cache].freeze
# Default storage classes warehouse. It is probably possible to
# add your own classes here if necessary, but I'm not sure it will work.
#
Expand Down Expand Up @@ -101,11 +102,7 @@ def merge!(other)
#
# @return [Hash] request body
def render
body = @storages.except(:filter, :query, :none, :function_score, :score_mode).values.inject({}) do |result, storage|
result.merge!(storage.render || {})
end
body.merge!(render_query || {})
body.present? ? {body: body} : {}
{}.merge(render_body).merge(render_query_string_params)
end

protected
Expand All @@ -126,6 +123,22 @@ def assert_storages(names)
names
end

def render_query_string_params
stores = @storages.select { |storage_name, _| QUERY_STRING_PARAMS.include?(storage_name) }
stores.values.inject({}) do |result, storage|
result.merge!(storage.render || {})
end
end

def render_body
exceptions = %i[filter query none function_score score_mode] + QUERY_STRING_PARAMS
body = @storages.except(*exceptions).values.inject({}) do |result, storage|
result.merge!(storage.render || {})
end
body.merge!(render_query || {})
body.present? ? {body: body} : {}
end

def render_query
none = @storages[:none].render

Expand Down
10 changes: 10 additions & 0 deletions spec/chewy/search/parameters_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,16 @@ def storage_object_ids(params)
specify { expect(subject.render).to eq(body: {from: 10, sort: ['foo']}) }
specify { expect(described_class.new.render).to eq({}) }

context do
subject { described_class.new(request_cache: true) }
specify { expect(subject.render).to eq(request_cache: true) }
end

context do
subject { described_class.new(search_type: 'query_then_fetch') }
specify { expect(subject.render).to eq(search_type: 'query_then_fetch') }
end

context do
subject { described_class.new(query: {foo: 'bar'}, filter: {moo: 'baz'}) }
specify { expect(subject.render).to eq(body: {query: {bool: {must: {foo: 'bar'}, filter: {moo: 'baz'}}}}) }
Expand Down
15 changes: 11 additions & 4 deletions spec/chewy/search/request_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -151,13 +151,20 @@
end

describe '#request_cache' do
specify { expect(subject.request_cache(true).render[:body]).to include(request_cache: true) }
specify { expect(subject.request_cache(true).request_cache(false).render[:body]).to include(request_cache: false) }
specify { expect(subject.request_cache(true).request_cache(nil).render[:body]).to be_blank }
specify { expect(subject.request_cache(true).render).to include(request_cache: true) }
specify { expect(subject.request_cache(true).request_cache(false).render).to include(request_cache: false) }
specify { expect(subject.request_cache(true).request_cache(nil).render[:request_cache]).to be_blank }
specify { expect { subject.request_cache(true) }.not_to change { subject.render } }
end

%i[search_type preference timeout].each do |name|
describe '#search_type' do
specify { expect(subject.search_type('foo').render).to include(search_type: 'foo') }
specify { expect(subject.search_type('foo').search_type('bar').render).to include(search_type: 'bar') }
specify { expect(subject.search_type('foo').search_type(nil).render[:search_type]).to be_blank }
specify { expect { subject.search_type('foo') }.not_to change { subject.render } }
end

%i[preference timeout].each do |name|
describe "##{name}" do
specify { expect(subject.send(name, :foo).render[:body]).to include(name => 'foo') }
specify { expect(subject.send(name, :foo).send(name, :bar).render[:body]).to include(name => 'bar') }
Expand Down

0 comments on commit dbfe243

Please sign in to comment.