diff --git a/CHANGELOG.md b/CHANGELOG.md index 79c13c5..7fe3ae5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,8 @@ Please mark backwards incompatible changes with an exclamation mark at the start ## [Unreleased] ### Added +- The `#intersection` method to `JayAPI::Elasticsearch::QueryBuilder`, which + combines two queries using a boolean `must` clause (equivalent to `AND`). - The `#union` method to `JayAPI::Elasticsearch::QueryBuilder`, which combines two queries using a boolean `should` clause (equivalent to `OR`). diff --git a/documentation/source/user_guidelines/elasticsearch/query_builder.rst b/documentation/source/user_guidelines/elasticsearch/query_builder.rst index b2c26c6..aa43a0c 100644 --- a/documentation/source/user_guidelines/elasticsearch/query_builder.rst +++ b/documentation/source/user_guidelines/elasticsearch/query_builder.rst @@ -476,6 +476,8 @@ field. # The documents matching the given IDs will be matched query_builder.query.ids(ids: %w[eSO6FZ8BUAdcYcWLRszq of2yFZ8BXEjZhYMm9Goe w1KlFZ8BK-fBiJSF1ptV]) +.. _`QueryBuilder#merge`: + #merge ------ @@ -570,6 +572,58 @@ The ``from``, ``size``, ``source`` and ``collapse`` settings, as well as This method returns a new ``QueryBuilder`` object which can be further modified without affecting the source objects. +#intersection +------------- + +The ``intersection`` method combines two ``QueryBuilder`` objects into a +single one. It combines their top-level query clauses with a boolean ``must`` +query (equivalent to ``AND``), so documents must match both queries. + +Example: + +.. code-block:: ruby + + users_query = QueryBuilder.new + .terms(field: 'user.id', terms: %w[kimchy elkbee]) + .sort('user.created_at' => :desc) + .size(100) + + bio_query = QueryBuilder.new + .query_string(query: "user.bio: painter OR poet") + .size(10) + + combined_query = users_query.intersection(bio_query) + +Would be equivalent to: + +.. code-block:: ruby + + combined_query = QueryBuilder.new + .sort('user.created_at' => :desc) # Kept from the users_query + .size(10) # From bio_query, replacing the first size + + combined_query.query.bool.must do |bool_query| + bool_query.terms(field: 'user.id', terms: %w[kimchy elkbee]) + bool_query.query_string(query: "user.bio: painter OR poet") + end + +The ``from``, ``size``, ``source`` and ``collapse`` settings, as well as +``sort`` and ``aggregations``, are combined using the same rules as ``merge``. + +.. note:: + + This method returns a new ``QueryBuilder`` object which can be further + modified without affecting the source objects. + +.. note:: + + This method differs from :ref:`QueryBuilder#merge` because it doesn't merge + the top-level query clauses if both of them are boolean queries, instead it + just wraps them in a boolean ``must`` query. + + In some cases, this is the desired behaviour, especially when ``should`` + clauses are involved. + aggregations ------------ diff --git a/lib/jay_api/elasticsearch/query_builder.rb b/lib/jay_api/elasticsearch/query_builder.rb index cd30971..5458d7e 100644 --- a/lib/jay_api/elasticsearch/query_builder.rb +++ b/lib/jay_api/elasticsearch/query_builder.rb @@ -149,6 +149,26 @@ def union(other) ) end + # Creates a new +QueryBuilder+ object which is the result of taking the + # intersection of the receiver with +other+. + # @param [JayAPI::Elasticsearch::QueryBuilder] other Another instance of + # +QueryBuilder+. + # @return [JayAPI::Elasticsearch::QueryBuilder] A new +QueryBuilder+, the + # result of the intersection of both objects. + # @see JayAPI::Elasticsearch::QueryClauses#intersection + # @raise [TypeError] If the given object is not a +QueryBuilder+. + def intersection(other) + klass = self.class + raise TypeError, "Cannot create an intersection of #{klass} and #{other.class}" unless other.is_a?(klass) + + query_intersection = query.intersection(other.query) + + other.combine( + from: @from, size: @size, source: @source, collapse: @collapse, + sort: @sort, query: query_intersection, aggregations: @aggregations + ) + end + # @return [JayAPI::Elasticsearch::QueryBuilder] A copy of the receiver. def clone copy = super diff --git a/lib/jay_api/elasticsearch/query_builder/query_clauses.rb b/lib/jay_api/elasticsearch/query_builder/query_clauses.rb index c93acef..56b4151 100644 --- a/lib/jay_api/elasticsearch/query_builder/query_clauses.rb +++ b/lib/jay_api/elasticsearch/query_builder/query_clauses.rb @@ -110,6 +110,23 @@ def union(other) end end + # Creates a new +QueryClauses+ object representing the intersection of + # the receiver and the given object. The individual top-query clauses + # are combined using a +must+ boolean clause. + # @param [JayAPI::Elasticsearch::QueryBuilder::QueryClauses] other The + # +QueryClauses+ object the receiver should be combined with. + # @return [JayAPI::Elasticsearch::QueryBuilder::QueryClauses] A new + # +QueryClauses+ object which is a combination of the receiver and + # the given object. + def intersection(other) + combine(other, __method__) do |combined| + combined.bool.must do |bool| + bool << top_level_clause + bool << other.top_level_clause + end + end + end + # Negates the receiver by wrapping its top-level query clause in a # +must_not+ boolean clause or replacing it by its inverse clause. # @return [self] Returns itself. diff --git a/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb b/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb index fd19b62..97039a3 100644 --- a/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb +++ b/spec/integration/jay_api/elasticsearch/query_builder/query_clauses_spec.rb @@ -129,4 +129,41 @@ end end end + + describe '#intersection' do + subject(:method_call) { query_clauses.intersection(other) } + + let(:other) { described_class.new } + + let(:expected_query) do + { + bool: { + must: [ + { + bool: { + should: [ + { term: { 'test_case.result' => { value: 'fail' } } }, + { exists: { field: 'test_case.refs' } } + ] + } + }, + { range: { 'test_case.date' => { gte: 'now-14d/d' } } } + ] + } + } + end + + before do + query_clauses.bool.should do |bool_query| + bool_query.term(field: 'test_case.result', value: 'fail') + bool_query.exists(field: 'test_case.refs') + end + + other.range(field: 'test_case.date', gte: 'now-14d/d') + end + + it 'returns the expected query' do + expect(method_call.to_h).to eq(expected_query) + end + end end diff --git a/spec/integration/jay_api/elasticsearch/query_builder_spec.rb b/spec/integration/jay_api/elasticsearch/query_builder_spec.rb index 47fa019..40c15bb 100644 --- a/spec/integration/jay_api/elasticsearch/query_builder_spec.rb +++ b/spec/integration/jay_api/elasticsearch/query_builder_spec.rb @@ -470,4 +470,79 @@ end end end + + describe '#intersection' do + subject(:method_call) { query_builder.intersection(other) } + + let(:other) { described_class.new } + + it_behaves_like '#combine' + + context 'when two simple queries are merged' do + before do + query_builder.query.term(field: 'user.name', value: 'kimchy') + other.query.range(field: 'age', gte: 10, lte: 20) + end + + let(:expected_query) do + { + query: { + bool: { + must: [ + { term: { 'user.name' => { value: 'kimchy' } } }, + { range: { 'age' => { gte: 10, lte: 20 } } } + ] + } + } + } + end + + it "merges both query using a boolean 'must' (equivalent to AND)" do + expect(method_call.to_query).to eq(expected_query) + end + end + + context 'when two boolean queries are merged' do + before do + query_builder.query.bool.must do |bool_query| + bool_query.term(field: 'user.name', value: 'kimchy') + bool_query.range(field: 'age', gte: 10, lte: 20) + end + + other.query.bool.must_not do |bool_query| + bool_query.exists(field: 'user.pet') + end + end + + let(:expected_query) do + { + query: { + bool: { + must: [ + { + bool: { + must: [ + { term: { 'user.name' => { value: 'kimchy' } } }, + { range: { 'age' => { gte: 10, lte: 20 } } } + ] + } + }, + { + bool: { + must_not: [ + { exists: { field: 'user.pet' } } + ] + } + } + ] + } + } + } + end + + it "merges the two queries with a boolean 'must' (equivalent to AND)" do + expect(method_call.to_query).to eq(expected_query) + end + end + end end diff --git a/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb b/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb index e8c7ada..7d5f84e 100644 --- a/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb @@ -564,6 +564,78 @@ end end + describe '#intersection' do + subject(:method_call) { query_clauses.intersection(other) } + + it_behaves_like '#combine', 'intersection' + + context "when neither the receiver nor 'other' are empty" do + let(:other) { described_class.new } + + let(:bool_clause) do + instance_double( + JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool, + to_h: { bool: { must: [exists_clause_clone.to_h, range_clause_clone.to_h] } } + ) + end + + let(:expected_hash) do + { + bool: { + must: [ + { exists: { field: 'build_job_name' } }, + { range: { 'build_number' => { gt: 150 } } } + ] + } + } + end + + include_context 'with a mocked Range QueryClause' + include_context 'with a mocked Exist QueryClause' + + before do + allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool) + .to receive(:new).and_return(bool_clause) + + allow(bool_clause).to receive(:must).and_yield(bool_clause) + allow(bool_clause).to receive(:<<).and_return(bool_clause) + + query_clauses.exists(field: 'build_job_name') + other.range(field: 'build_number', gt: 150) + end + + it 'returns a new instance of the class' do + expect(method_call).to be_a(described_class) + end + + it 'creates a boolean clause to put together the two enclosed clauses' do + expect(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool).to receive(:new) + method_call + end + + it "uses a 'must' boolean clause to join the two enclosed clauses" do + expect(bool_clause).to receive(:must) + method_call + end + + it "adds the receiver's top-level clause to the created boolean clause" do + expect(bool_clause).to receive(:<<).with(exists_clause) + method_call + end + + it "adds 'other's top-level clause to the created boolean clause" do + expect(bool_clause).to receive(:<<).with(range_clause) + method_call + end + + it 'sets the created boolean clause as top-level clause for the returned object' do + # The #to_h call is used to make sure that the +bool_clause+ double was + # set as top-level clause for the returned object. + expect(method_call.to_h).to eq(expected_hash) + end + end + end + shared_context 'with a mocked MatchNone clause' do let(:match_none) do instance_double( diff --git a/spec/jay_api/elasticsearch/query_builder_spec.rb b/spec/jay_api/elasticsearch/query_builder_spec.rb index 50e06a1..4aae318 100644 --- a/spec/jay_api/elasticsearch/query_builder_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder_spec.rb @@ -738,6 +738,19 @@ end end + describe '#intersection' do + subject(:method_call) { query_builder.intersection(other) } + + let(:expected_error) { "Cannot create an intersection of #{described_class} and Array" } + + it_behaves_like '#combine', operation: :intersection do + it "creates an intersection of the receiver's query and the query from 'other'" do + expect(query_clauses).to receive(:intersection).with(other_query_clauses) + method_call + end + end + end + describe '#clone' do subject(:method_call) { query_builder.clone }