From a1b12084d37742fbdfe29c632a836fb69f21c6e6 Mon Sep 17 00:00:00 2001 From: Sergio Bobillier Date: Fri, 28 Aug 2026 18:18:47 +0200 Subject: [PATCH] [ESRTEST-31237] Add 'sort' parameter to TopHits aggregation The parameter allows the user to determine the order the documents are put into to determine what documents are the "top hits". By default, they are ordered by the score of the main query, however, that is not always what one might want. This parameter allows possibilities like sorting the top hits in chronological order or by price in ascending order, etc. These are things that cannot be reflected by the score of the main query even if it includes a "corresponding" sort clause. --- CHANGELOG.md | 1 + .../query_builder/aggregations.rb | 4 +- .../query_builder/aggregations/top_hits.rb | 16 +++-- .../aggregations/top_hits_spec.rb | 63 ++++++++++++++++--- .../query_builder/aggregations_spec.rb | 21 ++++++- 5 files changed, 89 insertions(+), 16 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 690a907..6e6b67c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ Please mark backwards incompatible changes with an exclamation mark at the start ## [Unreleased] ### Added +- The `sort` parameter for Elasticsearch's `TopHits` aggregation. - The `missing` parameter for Elasticsearch's `Terms` aggregation. ## [29.10.0] - 2026-08-21 diff --git a/lib/jay_api/elasticsearch/query_builder/aggregations.rb b/lib/jay_api/elasticsearch/query_builder/aggregations.rb index dd80a85..4ba9185 100644 --- a/lib/jay_api/elasticsearch/query_builder/aggregations.rb +++ b/lib/jay_api/elasticsearch/query_builder/aggregations.rb @@ -70,8 +70,8 @@ def value_count(name, field:) # Adds a +top_hits+ type aggregation. For more information about the parameters # @see JayAPI::Elasticsearch::QueryBuilder::Aggregations::TopHits#initialize - def top_hits(name, size:) - add(::JayAPI::Elasticsearch::QueryBuilder::Aggregations::TopHits.new(name, size: size)) + def top_hits(name, **params) + add(::JayAPI::Elasticsearch::QueryBuilder::Aggregations::TopHits.new(name, **params)) end # Adds an +scripted_metric+ type aggregation. For information about the parameters diff --git a/lib/jay_api/elasticsearch/query_builder/aggregations/top_hits.rb b/lib/jay_api/elasticsearch/query_builder/aggregations/top_hits.rb index e6a98d7..4715e86 100644 --- a/lib/jay_api/elasticsearch/query_builder/aggregations/top_hits.rb +++ b/lib/jay_api/elasticsearch/query_builder/aggregations/top_hits.rb @@ -15,20 +15,25 @@ class Aggregations # Information on this type of aggregation can be found here: # https://www.elastic.co/docs/reference/aggregations/search-aggregations-metrics-top-hits-aggregation class TopHits < ::JayAPI::Elasticsearch::QueryBuilder::Aggregations::Aggregation - attr_reader :size + attr_reader :size, :sort # @param [String] name The name used by Elasticsearch to identify each # of the aggregations. # @param [String] size The number of hits that will be returned. - def initialize(name, size:) + # @param [Hash] sort How to sort the documents to determine the top + # hits. When not specified the documents are sorted by the score of + # the main query. + def initialize(name, size:, sort: nil) super(name) @size = size + @sort = sort end # @return [self] A copy of the receiver. def clone - copy = self.class.new(name, size: size) + optional_params = { sort: sort.deep_dup }.compact + copy = self.class.new(name, size: size, **optional_params) copy.aggregations = aggregations.clone copy end @@ -39,8 +44,9 @@ def to_h super do { top_hits: { - size: size - } + size: size, + sort: sort.deep_dup + }.compact } end end diff --git a/spec/jay_api/elasticsearch/query_builder/aggregations/top_hits_spec.rb b/spec/jay_api/elasticsearch/query_builder/aggregations/top_hits_spec.rb index c59bb44..877d257 100644 --- a/spec/jay_api/elasticsearch/query_builder/aggregations/top_hits_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/aggregations/top_hits_spec.rb @@ -28,6 +28,25 @@ expect(method_call.size).to be(top_hits.size) end + context "when no 'sort' has been given" do + it "has its 'sort' set to nil" do + expect(method_call.sort).to be_nil + end + end + + context "when a 'sort' has been given" do + let(:sort) { { timestamp: :desc } } + let(:constructor_params) { super().merge(sort: sort) } + + it "has its 'sort' set to the same value" do + expect(method_call.sort).to eq(sort) + end + + it 'is not the same hash' do + expect(method_call.sort).not_to be(top_hits.sort) + end + end + context 'when the original object has nested aggregations' do let(:cloned_aggregations) { instance_double(JayAPI::Elasticsearch::QueryBuilder::Aggregations) } @@ -50,18 +69,46 @@ describe '#to_h' do subject(:method_call) { top_hits.to_h } - let(:expected_hash) do - { - 'an_aggregation_sample' => { - top_hits: { size: 1 } + it_behaves_like 'JayAPI::Elasticsearch::QueryBuilder::Aggregations::Aggregation#to_h' + + context "when no 'sort' has been given" do + let(:expected_hash) do + { + 'an_aggregation_sample' => { + top_hits: { size: 1 } + } } - } + end + + it 'returns the expected Hash (does not include the :sort key)' do + expect(method_call).to eq(expected_hash) + end end - it_behaves_like 'JayAPI::Elasticsearch::QueryBuilder::Aggregations::Aggregation#to_h' + context "when a 'sort' has been given" do + let(:sort) { { timestamp: :desc } } + let(:constructor_params) { super().merge(sort: sort) } + + let(:expected_hash) do + { + 'an_aggregation_sample' => { + top_hits: { + size: 1, + sort: { timestamp: :desc } + } + } + } + end - it 'returns the expected Hash' do - expect(method_call).to eq(expected_hash) + it 'returns the expected Hash (includes the expected :sort key)' do + expect(method_call).to eq(expected_hash) + end + + it "does not return a reference to the internal 'sort' Hash" do + sort_hash = method_call.dig('an_aggregation_sample', :top_hits, :sort) + expect(sort_hash).not_to be(top_hits.sort) + expect(sort_hash).to eq(sort) + end end end end diff --git a/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb b/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb index 302846d..ad31619 100644 --- a/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb @@ -450,12 +450,16 @@ describe '#top_hits' do subject(:method_call) do - aggregations.top_hits(name, size: size) + aggregations.top_hits(name, **method_params) end let(:name) { 'recent_logs' } let(:size) { 10 } + let(:method_params) do + { size: } + end + let(:top_hits) do instance_double( JayAPI::Elasticsearch::QueryBuilder::Aggregations::TopHits, @@ -475,6 +479,21 @@ method_call end + context "when a 'sort' parameter is given" do + let(:sort) do + { date: { order: :desc } } + end + + let(:method_params) { super().merge(sort:) } + + it 'creates the TopHits instance with the expected parameters' do + expect(JayAPI::Elasticsearch::QueryBuilder::Aggregations::TopHits) + .to receive(:new).with(name, size: size, sort: sort) + + method_call + end + end + it 'adds the TopHits instance to the array of aggregations' do expect { method_call }.to change(aggregations, :to_h).to(aggs: { top_hits: 'TopHits#to_h' }) end