diff --git a/CHANGELOG.md b/CHANGELOG.md index 932dc38..79c13c5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Please mark backwards incompatible changes with an exclamation mark at the start ## [Unreleased] +### Added +- The `#union` method to `JayAPI::Elasticsearch::QueryBuilder`, which combines + two queries using a boolean `should` clause (equivalent to `OR`). + ## [29.11.0] - 2026-09-03 ### Added diff --git a/documentation/source/user_guidelines/elasticsearch/query_builder.rst b/documentation/source/user_guidelines/elasticsearch/query_builder.rst index ec9085c..b2c26c6 100644 --- a/documentation/source/user_guidelines/elasticsearch/query_builder.rst +++ b/documentation/source/user_guidelines/elasticsearch/query_builder.rst @@ -526,6 +526,50 @@ The different clauses of the query are merged like this: These are merged, the result is a compound of the clauses in both objects. Query clauses are merged with a boolean ``must`` query (equivalent to ``AND``) +#union +------ + +The ``union`` method combines two ``QueryBuilder`` objects into a single one. +Unlike the ``merge`` method, which combines query clauses with a boolean +``must`` query (equivalent to ``AND``), ``union`` combines their top-level query +clauses with a boolean ``should`` query (equivalent to ``OR``). + +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.union(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.should 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. + aggregations ------------ diff --git a/lib/jay_api/elasticsearch/query_builder.rb b/lib/jay_api/elasticsearch/query_builder.rb index 4574118..cd30971 100644 --- a/lib/jay_api/elasticsearch/query_builder.rb +++ b/lib/jay_api/elasticsearch/query_builder.rb @@ -121,9 +121,31 @@ def merge(other) klass = self.class raise TypeError, "Cannot merge #{klass} and #{other.class}" unless other.is_a?(klass) + merged_query = query.merge(other.query) + + other.combine( + from: @from, size: @size, source: @source, collapse: @collapse, + sort: @sort, query: merged_query, aggregations: @aggregations + ) + end + + # Creates a new +QueryBuilder+ object which is the result of taking the + # union 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 union of both objects. + # @see JayAPI::Elasticsearch::QueryClauses#union + # @raise [TypeError] If the given object is not a +QueryBuilder+. + def union(other) + klass = self.class + raise TypeError, "Cannot create a union of #{klass} and #{other.class}" unless other.is_a?(klass) + + query_union = query.union(other.query) + other.combine( from: @from, size: @size, source: @source, collapse: @collapse, - sort: @sort, query: @query, aggregations: @aggregations + sort: @sort, query: query_union, aggregations: @aggregations ) end @@ -160,7 +182,7 @@ def combine(from:, size:, source:, collapse:, sort:, query:, aggregations:) combined.source = @source || source combined.collapse = @collapse || collapse combined.sort = sort.merge(@sort) - combined.query = query.merge(self.query) + combined.query = query combined.aggregations = aggregations.merge(self.aggregations) end end diff --git a/lib/jay_api/elasticsearch/query_builder/query_clauses.rb b/lib/jay_api/elasticsearch/query_builder/query_clauses.rb index beeeb12..c93acef 100644 --- a/lib/jay_api/elasticsearch/query_builder/query_clauses.rb +++ b/lib/jay_api/elasticsearch/query_builder/query_clauses.rb @@ -88,16 +88,24 @@ def clone # @return [self] A new +QueryClauses+ object which is a combination of # the receiver and the given object. def merge(other) - klass = self.class - raise TypeError, "Cannot merge #{klass} with #{other.class}" unless other.is_a?(klass) + combine(other, __method__) do |combined| + combined.bool.merge!(top_level_clause).merge!(other.top_level_clause) + end + end - if other.empty? - clone - elsif empty? - other.clone - else - klass.new.tap do |merged| - merged.bool.merge!(top_level_clause).merge!(other.top_level_clause) + # Creates a new +QueryClauses+ object representing the union of the + # receiver and the given object. The individual top-query clauses are + # combined using a +should+ 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 union(other) + combine(other, __method__) do |combined| + combined.bool.should do |bool| + bool << top_level_clause + bool << other.top_level_clause end end end @@ -143,6 +151,32 @@ def top_level_clause=(clause) @top_level_clause = clause end + + # Combines the current +QueryClauses+ object with another one using the + # specified action. The combination is performed by yielding a new + # +QueryClauses+ object to the given block, which should define how the + # combination is done. + # @param [JayAPI::Elasticsearch::QueryBuilder::QueryClauses] other The + # +QueryClauses+ object to combine with. + # @param [Symbol] action The action describing how the combination would + # be performed (used only for the error message when the combination + # is not possible). + # @yield [combined] A new +QueryClauses+ object in which the combination + # should be carried out by the block. + # @return [JayAPI::Elasticsearch::QueryBuilder::QueryClauses] The new + # combined +QueryClauses+ object. + def combine(other, action, &block) + klass = self.class + raise TypeError, "Cannot perform '#{action}' between #{klass} and #{other.class}" unless other.is_a?(klass) + + if other.empty? + clone + elsif empty? + other.clone + else + klass.new.tap { |combined| block.call(combined) } + end + end 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 b34c364..47fa019 100644 --- a/spec/integration/jay_api/elasticsearch/query_builder_spec.rb +++ b/spec/integration/jay_api/elasticsearch/query_builder_spec.rb @@ -186,12 +186,8 @@ it_behaves_like '#to_h' end - describe '#merge' do - subject(:method_call) { query_builder.merge(other) } - - let(:other) { described_class.new } - - context 'when the QueryBuilder has nested aggregations' do + shared_examples_for '#combine' do + context 'when the QueryBuilder objects have nested aggregations' do before do query_builder.aggregations.avg('overall_avg_price', field: 'price') query_builder.aggregations.filter('hats') { |query| query.term(field: 'type', value: 'hat') } @@ -227,7 +223,7 @@ end end - context 'when the receiver has a simple clause and the mergee is empty' do + context 'when the receiver has a simple clause and other is empty' do before do query_builder.query.term(field: 'user.name', value: 'kimchy') end @@ -247,7 +243,7 @@ end end - context 'when the receiver is empty and the mergee has a simple clause' do + context 'when the receiver is empty and other has a simple clause' do before do other.query.range(field: 'age', gte: 10, lte: 20) end @@ -266,6 +262,14 @@ expect(method_call.to_query).to eq(expected_query) end end + end + + describe '#merge' do + subject(:method_call) { query_builder.merge(other) } + + let(:other) { described_class.new } + + it_behaves_like '#combine' context 'when two simple queries are merged' do before do @@ -391,4 +395,79 @@ end end end + + describe '#union' do + subject(:method_call) { query_builder.union(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: { + should: [ + { term: { 'user.name' => { value: 'kimchy' } } }, + { range: { 'age' => { gte: 10, lte: 20 } } } + ] + } + } + } + end + + it "merges both query using a boolean 'should' (equivalent to OR)" 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: { + should: [ + { + 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 'should' (equivalent to OR)" 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 26d1ffd..e8c7ada 100644 --- a/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/query_clauses_spec.rb @@ -336,9 +336,51 @@ end end - describe '#merge' do - subject(:method_call) { query_clauses.merge(other) } + shared_context 'with a mocked Range QueryClause' do + let(:range_clause_clone) do + instance_double( + JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Range, + to_h: { range: { 'build_number' => { gt: 150 } } } + ) + end + + let(:range_clause) do + instance_double( + JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Range, + to_h: { range: { 'build_number' => { gt: 150 } } }, + clone: range_clause_clone + ) + end + before do + allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Range) + .to receive(:new).and_return(range_clause) + end + end + + shared_context 'with a mocked Exist QueryClause' do + let(:exists_clause_clone) do + instance_double( + JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Exists, + to_h: { exists: { field: 'build_job_name' } } + ) + end + + let(:exists_clause) do + instance_double( + JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Exists, + clone: exists_clause_clone, + to_h: { exists: { field: 'build_job_name' } } + ) + end + + before do + allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Exists) + .to receive(:new).and_return(exists_clause) + end + end + + shared_examples_for '#combine' do |action| context "when 'other' is not a #{described_class}" do let(:other) do { query: { query_string: 'age: 27' } } @@ -346,7 +388,7 @@ it 'raises a TypeError' do expect { method_call }.to raise_error( - TypeError, "Cannot merge #{described_class} with Hash" + TypeError, "Cannot perform '#{action}' between #{described_class} and Hash" ) end end @@ -354,6 +396,7 @@ context "when both the receiver and 'other' are empty" do let(:other) { described_class.new } + it_behaves_like '#clone' it_behaves_like '#clone when the receiver is empty' end @@ -363,32 +406,10 @@ it_behaves_like '#clone when the receiver is not empty' end - shared_context 'with a Range QueryClause' do - let(:range_clause_clone) do - instance_double( - JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Range, - to_h: { range: { 'build_number' => { gt: 150 } } } - ) - end - - let(:range_clause) do - instance_double( - JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Range, - to_h: { range: { 'build_number' => { gt: 150 } } }, - clone: range_clause_clone - ) - end - - before do - allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Range) - .to receive(:new).and_return(range_clause) - end - end - context "when the receiver is empty but 'other' is not" do let(:other) { described_class.new } - include_context 'with a Range QueryClause' + include_context 'with a mocked Range QueryClause' before { other.range(field: 'build_number', gt: 150) } @@ -402,24 +423,15 @@ expect(method_call.to_h).to eq(other.to_h) end end + end - context "when neither the receiver nor 'other' are empty" do - let(:other) { described_class.new } + describe '#merge' do + subject(:method_call) { query_clauses.merge(other) } - let(:exists_clause_clone) do - instance_double( - JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Exists, - to_h: { exists: { field: 'build_job_name' } } - ) - end + it_behaves_like '#combine', 'merge' - let(:exists_clause) do - instance_double( - JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Exists, - clone: exists_clause_clone, - to_h: { exists: { field: 'build_job_name' } } - ) - end + context "when neither the receiver nor 'other' are empty" do + let(:other) { described_class.new } let(:bool_clause) do instance_double( @@ -439,12 +451,10 @@ } end - include_context 'with a Range QueryClause' + include_context 'with a mocked Range QueryClause' + include_context 'with a mocked Exist QueryClause' before do - allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Exists) - .to receive(:new).and_return(exists_clause) - allow(JayAPI::Elasticsearch::QueryBuilder::QueryClauses::Bool) .to receive(:new).and_return(bool_clause) @@ -482,6 +492,78 @@ end end + describe '#union' do + subject(:method_call) { query_clauses.union(other) } + + it_behaves_like '#combine', 'union' + + 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: { should: [exists_clause_clone.to_h, range_clause_clone.to_h] } } + ) + end + + let(:expected_hash) do + { + bool: { + should: [ + { 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(:should).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 'should' boolean clause to join the two enclosed clauses" do + expect(bool_clause).to receive(:should) + 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 249ded3..50e06a1 100644 --- a/spec/jay_api/elasticsearch/query_builder_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder_spec.rb @@ -418,9 +418,7 @@ it_behaves_like 'JayAPI::Elasticsearch::QueryBuilder#to_query' end - describe '#merge' do - subject(:method_call) { query_builder.merge(other) } - + shared_examples_for '#combine' do |operation:| let(:other) { described_class.new } let(:other_query_clauses) do @@ -429,10 +427,10 @@ ) end - let(:merged_query_clauses) do + let(:combined_query_clauses) do instance_double( JayAPI::Elasticsearch::QueryBuilder::QueryClauses, - to_h: { merged: 'query' } + to_h: { combined: 'query' } ) end @@ -459,7 +457,7 @@ allow(JayAPI::Elasticsearch::QueryBuilder::Aggregations) .to receive(:new).and_return(aggregations, other_aggregations) - allow(query_clauses).to receive(:merge).and_return(merged_query_clauses) + allow(query_clauses).to receive(operation).and_return(combined_query_clauses) allow(aggregations).to receive(:merge).and_return(merged_aggregations) end @@ -474,14 +472,26 @@ it 'raises a TypeError' do expect { method_call }.to raise_error( TypeError, - "Cannot merge #{described_class} and Array" + expected_error ) end end - it "merges the receiver's query with the query from 'other'" do - expect(query_clauses).to receive(:merge).with(other_query_clauses) - method_call + it 'returns a new QueryBuilder' do + expect(method_call).to be_a(described_class) + expect(method_call).not_to be(query_builder) + end + + it "does not return 'other'" do + expect(method_call).not_to be(other) + end + + it 'uses the combined query clauses' do + expect(method_call.query).to be(combined_query_clauses) + end + + it 'includes the combined query in the generated query' do + expect(method_call.to_h).to include(query: { combined: 'query' }) end it "merges the receiver's aggregations with the aggregations from 'other'" do @@ -499,7 +509,7 @@ before { query_builder.sort('test_case.started_at' => :desc) } let(:expected_clause) do - ['test_case.started_at' => { order: :desc }] + [{ 'test_case.started_at' => { order: :desc } }] end it 'adds the expected sort clause to the merged object (the one from the receiver)' do @@ -514,7 +524,7 @@ end let(:expected_clause) do - ['test_case.id_long' => { order: :asc }] + [{ 'test_case.id_long' => { order: :asc } }] end it "adds the expected sort clause to the merged object (the one from 'other')" do @@ -548,7 +558,7 @@ end let(:expected_clause) do - ['test_case.started_at' => { order: :asc }] + [{ 'test_case.started_at' => { order: :asc } }] end it "adds the expected sort clause to the merged object (the one from 'other')" do @@ -702,6 +712,32 @@ end end + describe '#merge' do + subject(:method_call) { query_builder.merge(other) } + + let(:expected_error) { "Cannot merge #{described_class} and Array" } + + it_behaves_like '#combine', operation: :merge do + it "merges the receiver's query with the query from 'other'" do + expect(query_clauses).to receive(:merge).with(other_query_clauses) + method_call + end + end + end + + describe '#union' do + subject(:method_call) { query_builder.union(other) } + + let(:expected_error) { "Cannot create a union of #{described_class} and Array" } + + it_behaves_like '#combine', operation: :union do + it "creates a union of the receiver's query and the query from 'other'" do + expect(query_clauses).to receive(:union).with(other_query_clauses) + method_call + end + end + end + describe '#clone' do subject(:method_call) { query_builder.clone }