From f8278f7f8084d64d1ca13a309a5c45370abefd15 Mon Sep 17 00:00:00 2001 From: Sergio Bobillier Date: Mon, 29 Jun 2026 18:19:47 +0200 Subject: [PATCH 1/2] [ESRTEST-30704] Take Script out of QueryBuilder namespace Moves the Script class out of the QueryBuilder namespace and leaves it directly under Elasticsearch. This is done because there are scripted elements in Elasticsearch outside the context of queries, such as scripts that run when documents are updated or reindexed. Therefore it makes sense to have the Script class in a more general namespace. --- CHANGELOG.md | 4 +++ .../elasticsearch/aggregations.rst | 4 +-- lib/jay_api/elasticsearch.rb | 1 + .../aggregations/bucket_selector.rb | 4 +-- .../query_builder/aggregations/terms.rb | 7 ++-- .../elasticsearch/query_builder/script.rb | 29 +++------------- lib/jay_api/elasticsearch/script.rb | 34 +++++++++++++++++++ .../aggregations/bucket_selector_spec.rb | 4 +-- .../query_builder/aggregations/terms_spec.rb | 6 ++-- .../query_builder/aggregations_spec.rb | 6 ++-- .../{query_builder => }/script_spec.rb | 4 +-- 11 files changed, 60 insertions(+), 43 deletions(-) create mode 100644 lib/jay_api/elasticsearch/script.rb rename spec/jay_api/elasticsearch/{query_builder => }/script_spec.rb (96%) diff --git a/CHANGELOG.md b/CHANGELOG.md index 315f5e0..f33dc4c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ Please mark backwards incompatible changes with an exclamation mark at the start ## [Unreleased] +### Deprecated +- The `Elasticsearch::QueryBuilder::Script` class is now deprecated, please + use `Elasticsearch::Script` instead. + ## [29.7.0] - 2026-04-28 ### Added diff --git a/documentation/source/user_guidelines/elasticsearch/aggregations.rst b/documentation/source/user_guidelines/elasticsearch/aggregations.rst index e0f0d61..9865444 100644 --- a/documentation/source/user_guidelines/elasticsearch/aggregations.rst +++ b/documentation/source/user_guidelines/elasticsearch/aggregations.rst @@ -326,7 +326,7 @@ The code above would produce the following query: These scripts **must** be simple strings, they do not follow the pattern of other scripted elements in Elasticsearch's DSL. Do not use - ``QueryBuilder::Script`` objects here. Their use will produce unintended + ``Elasticsearch::Script`` objects here. Their use will produce unintended results. composite @@ -396,7 +396,7 @@ Code example: aggs.sum('total_sales', field: 'price') aggs.bucket_selector( 'sales_bucket_filter', buckets_path: { totalSales: 'total_sales' }, - script: JayAPI::Elasticsearch::QueryBuilder::Script.new(source: 'params.totalSales > 200') + script: JayAPI::Elasticsearch::Script.new(source: 'params.totalSales > 200') ) end diff --git a/lib/jay_api/elasticsearch.rb b/lib/jay_api/elasticsearch.rb index f00a274..b8cac55 100644 --- a/lib/jay_api/elasticsearch.rb +++ b/lib/jay_api/elasticsearch.rb @@ -13,6 +13,7 @@ require_relative 'elasticsearch/query_builder' require_relative 'elasticsearch/query_results' require_relative 'elasticsearch/response' +require_relative 'elasticsearch/script' require_relative 'elasticsearch/search_after_results' require_relative 'elasticsearch/stats' require_relative 'elasticsearch/tasks' diff --git a/lib/jay_api/elasticsearch/query_builder/aggregations/bucket_selector.rb b/lib/jay_api/elasticsearch/query_builder/aggregations/bucket_selector.rb index 079305f..6292905 100644 --- a/lib/jay_api/elasticsearch/query_builder/aggregations/bucket_selector.rb +++ b/lib/jay_api/elasticsearch/query_builder/aggregations/bucket_selector.rb @@ -19,8 +19,8 @@ class BucketSelector < ::JayAPI::Elasticsearch::QueryBuilder::Aggregations::Aggr # The keys are the names of the script variables, the values the # paths to the metrics (relative to the parent aggregation). # The script will receive these variables in its +params+. - # @param [JayAPI::Elasticsearch::QueryBuilder::Script] script - # Script used to decide whether to keep each bucket. + # @param [JayAPI::Elasticsearch::Script] script Script used to decide + # whether to keep each bucket. # @param [String, nil] gap_policy Optional gap policy (e.g. "skip", # "insert_zeros"). def initialize(name, buckets_path:, script:, gap_policy: nil) diff --git a/lib/jay_api/elasticsearch/query_builder/aggregations/terms.rb b/lib/jay_api/elasticsearch/query_builder/aggregations/terms.rb index 290a1d4..18aab7f 100644 --- a/lib/jay_api/elasticsearch/query_builder/aggregations/terms.rb +++ b/lib/jay_api/elasticsearch/query_builder/aggregations/terms.rb @@ -19,10 +19,9 @@ class Terms < ::JayAPI::Elasticsearch::QueryBuilder::Aggregations::Aggregation # @param [String] name The name used by Elasticsearch to identify each # of the aggregations. # @param [String] field The field whose unique values should be counted. - # @param [JayAPI::Elasticsearch::QueryBuilder::Script] script If a - # script is given the aggregation will count the unique values - # returned by the script instead of the unique values in a specific - # field. + # @param [JayAPI::Elasticsearch::Script] script If a script is given + # the aggregation will count the unique values returned by the + # script instead of the unique values in a specific field. # @param [Integer] size By default the aggregation returns the top 10 # unique values (the ones with the higher frequency). By specifying # a size this can be changed. diff --git a/lib/jay_api/elasticsearch/query_builder/script.rb b/lib/jay_api/elasticsearch/query_builder/script.rb index aed13db..b5d2540 100644 --- a/lib/jay_api/elasticsearch/query_builder/script.rb +++ b/lib/jay_api/elasticsearch/query_builder/script.rb @@ -1,36 +1,15 @@ # frozen_string_literal: true +require_relative '../script' + module JayAPI module Elasticsearch class QueryBuilder # Represents a scripted element in a query. This scripted element can be # used in different places. It can be used in a query clause, but can # also be used to create custom aggregations. - class Script - attr_reader :source, :lang, :params - - # @param [String] source The source for the script element. - # @param [String] lang The language the script is written in. - # @param [Hash] params A +Hash+ with key-value pairs for the script's - # parameters. - def initialize(source:, lang: 'painless', params: nil) - @source = source - @lang = lang - - # Keeps the parameters from being modified from the outside after the - # class has been initialized. - @params = params.dup.freeze - end - - # @return [Hash] The hash representation of the scripted element. - def to_h - { - source: source, - lang: lang, - params: params - }.compact - end - end + # @deprecated Use +JayAPI::Elasticsearch::Script+ instead. + Script = JayAPI::Elasticsearch::Script end end end diff --git a/lib/jay_api/elasticsearch/script.rb b/lib/jay_api/elasticsearch/script.rb new file mode 100644 index 0000000..0c8ff9a --- /dev/null +++ b/lib/jay_api/elasticsearch/script.rb @@ -0,0 +1,34 @@ +# frozen_string_literal: true + +module JayAPI + module Elasticsearch + # Represents a scripted element in Elasticsearch. This scripted element + # can be used in different places. It can be used in a query clause, but can + # also be used to create custom aggregations or when updating documents. + class Script + attr_reader :source, :lang, :params + + # @param [String] source The source for the script element. + # @param [String] lang The language the script is written in. + # @param [Hash] params A +Hash+ with key-value pairs for the script's + # parameters. + def initialize(source:, lang: 'painless', params: nil) + @source = source + @lang = lang + + # Keeps the parameters from being modified from the outside after the + # class has been initialized. + @params = params.dup.freeze + end + + # @return [Hash] The hash representation of the scripted element. + def to_h + { + source: source, + lang: lang, + params: params + }.compact + end + end + end +end diff --git a/spec/jay_api/elasticsearch/query_builder/aggregations/bucket_selector_spec.rb b/spec/jay_api/elasticsearch/query_builder/aggregations/bucket_selector_spec.rb index 4d44fc0..ad57a77 100644 --- a/spec/jay_api/elasticsearch/query_builder/aggregations/bucket_selector_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/aggregations/bucket_selector_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'jay_api/elasticsearch/query_builder/aggregations/bucket_selector' -require 'jay_api/elasticsearch/query_builder/script' +require 'jay_api/elasticsearch/script' require_relative 'aggregation_shared' @@ -16,7 +16,7 @@ let(:script) do instance_double( - JayAPI::Elasticsearch::QueryBuilder::Script, + JayAPI::Elasticsearch::Script, to_h: { source: 'params.avgPrice > params.threshold', lang: 'painless', diff --git a/spec/jay_api/elasticsearch/query_builder/aggregations/terms_spec.rb b/spec/jay_api/elasticsearch/query_builder/aggregations/terms_spec.rb index bbb219f..890032f 100644 --- a/spec/jay_api/elasticsearch/query_builder/aggregations/terms_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/aggregations/terms_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'jay_api/elasticsearch/query_builder/aggregations/terms' -require 'jay_api/elasticsearch/query_builder/script' +require 'jay_api/elasticsearch/script' require_relative 'aggregation_shared' @@ -13,7 +13,7 @@ let(:script) do instance_double( - JayAPI::Elasticsearch::QueryBuilder::Script, + JayAPI::Elasticsearch::Script, to_h: { source: <<~PAINLESS, String genre = doc['genre'].value; @@ -107,7 +107,7 @@ end context "when a 'script' has been given" do - let(:script) { instance_double(JayAPI::Elasticsearch::QueryBuilder::Script) } + let(:script) { instance_double(JayAPI::Elasticsearch::Script) } before do constructor_params.delete(:field) diff --git a/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb b/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb index 4e9c992..0f145e6 100644 --- a/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb +++ b/spec/jay_api/elasticsearch/query_builder/aggregations_spec.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true require 'jay_api/elasticsearch/query_builder/aggregations' -require 'jay_api/elasticsearch/query_builder/script' +require 'jay_api/elasticsearch/script' RSpec.describe JayAPI::Elasticsearch::QueryBuilder::Aggregations do subject(:aggregations) { described_class.new } @@ -120,7 +120,7 @@ let(:script) do instance_double( - JayAPI::Elasticsearch::QueryBuilder::Script + JayAPI::Elasticsearch::Script ) end @@ -519,7 +519,7 @@ let(:script) do instance_double( - JayAPI::Elasticsearch::QueryBuilder::Script + JayAPI::Elasticsearch::Script ) end diff --git a/spec/jay_api/elasticsearch/query_builder/script_spec.rb b/spec/jay_api/elasticsearch/script_spec.rb similarity index 96% rename from spec/jay_api/elasticsearch/query_builder/script_spec.rb rename to spec/jay_api/elasticsearch/script_spec.rb index 75a8158..0d59cc0 100644 --- a/spec/jay_api/elasticsearch/query_builder/script_spec.rb +++ b/spec/jay_api/elasticsearch/script_spec.rb @@ -1,8 +1,8 @@ # frozen_string_literal: true -require 'jay_api/elasticsearch/query_builder/script' +require 'jay_api/elasticsearch/script' -RSpec.describe JayAPI::Elasticsearch::QueryBuilder::Script do +RSpec.describe JayAPI::Elasticsearch::Script do subject(:script) { described_class.new(**constructor_params) } let(:source) do From 04fb56eaff9c850604432e11daf5702725357f82 Mon Sep 17 00:00:00 2001 From: Sergio Bobillier Date: Mon, 29 Jun 2026 18:28:54 +0200 Subject: [PATCH 2/2] [ESRTEST-30704] Add #update to Elasticsearch::Index The method allows the caller to update a document in the index by providing the document ID and the updated fields or an update script. --- CHANGELOG.md | 3 + lib/jay_api/elasticsearch/client.rb | 8 + lib/jay_api/elasticsearch/index.rb | 43 +++++ .../jay_api/elasticsearch/client_spec.rb | 17 ++ spec/jay_api/elasticsearch/index_spec.rb | 172 ++++++++++++++++++ 5 files changed, 243 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index f33dc4c..8a649fe 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,9 @@ Please mark backwards incompatible changes with an exclamation mark at the start ## [Unreleased] +### Added +- The `#update` method to `JayAPI::Elasticsearch::Client`. + ### Deprecated - The `Elasticsearch::QueryBuilder::Script` class is now deprecated, please use `Elasticsearch::Script` instead. diff --git a/lib/jay_api/elasticsearch/client.rb b/lib/jay_api/elasticsearch/client.rb index 0e1ce9f..dc9dccb 100644 --- a/lib/jay_api/elasticsearch/client.rb +++ b/lib/jay_api/elasticsearch/client.rb @@ -96,6 +96,14 @@ def tasks def cluster @cluster ||= ::JayAPI::Elasticsearch::Cluster.new(transport_client) end + + # Calls the +Elasticsearch::Client+'s #update method forwarding the given + # parameters. If the request fails, additional retries will be performed. + # @see Elasticsearch::API::Actions#update for more info about the + # arguments and the return value. + def update(**args) + retry_request { transport_client.update(**args) } + end end end end diff --git a/lib/jay_api/elasticsearch/index.rb b/lib/jay_api/elasticsearch/index.rb index df15a97..a96dd5f 100644 --- a/lib/jay_api/elasticsearch/index.rb +++ b/lib/jay_api/elasticsearch/index.rb @@ -1,5 +1,8 @@ # frozen_string_literal: true +require 'active_support' +require 'active_support/core_ext/object/blank' + require_relative 'indexable' require_relative 'indices/settings' require_relative 'errors/writable_index_error' @@ -52,6 +55,46 @@ def index(data, type: DEFAULT_DOC_TYPE) super.first end + # Updates a document by ID. + # @param [String] id The ID of the document to update. + # @param [Hash, nil] doc The partial document to update the existing + # document with. If +nil+ is given, the +script+ parameter must be + # provided. If both +doc+ and +script+ are given, the +doc+ parameter + # will be ignored by Elasticsearch. + # @param [JayAPI::Elasticsearch::Script, Hash, nil] script The script to + # update the existing document with. If +nil+ is given, the +doc+ + # parameter must be provided. It's recommended to use a + # +JayAPI::Elasticsearch::Script+ object, but a Hash can also be used. + # @return [Hash] A Hash containing information about the updated document. + # An example of such Hash is: + # + # { + # "_index" : "xyz01_build_properties", + # "_type" : "_doc", + # "_id" : "ns4AAZ8BXEjZhYMmw-8y", + # "_version" : 7, + # "result" : "updated", + # "_shards" : { + # "total" : 2, + # "successful" : 2, + # "failed" : 0 + # }, + # "_seq_no" : 11, + # "_primary_term" : 1 + # } + # + # For information on how to use the script and about the contents of the + # returned Hash please see: + # https://www.elastic.co/docs/api/doc/elasticsearch/operation/operation-update + # @raise [ArgumentError] If both +doc+ and +script+ are +nil+. + # @raise [Elasticsearch::Transport::Transport::ServerError] If the + # update fails. + def update(id:, doc: nil, script: nil) + raise ArgumentError, "Either 'doc' or 'script' must be provided" if doc.blank? && script.blank? + + client.update(index: index_name, id:, body: { doc: doc, script: script&.to_h }.compact) + end + # @return [JayAPI::Elasticsearch::Indices::Settings] The settings for the # index. def settings diff --git a/spec/integration/jay_api/elasticsearch/client_spec.rb b/spec/integration/jay_api/elasticsearch/client_spec.rb index 37c0a3b..2dd0cfe 100644 --- a/spec/integration/jay_api/elasticsearch/client_spec.rb +++ b/spec/integration/jay_api/elasticsearch/client_spec.rb @@ -364,4 +364,21 @@ expect(method_call).to be(cluster) end end + + describe '#update' do + let(:method_name) { :update } + + let(:client_method_arguments) do + { + index: 'xyz01_integration_test', + id: 'ns4AAZ8BXEjZhYMmw-8y', + body: { doc: { test_case: { owner: 'alice.wolf' } } } + } + end + + let(:used_client) { transport_client } + let(:client_method_name) { :update } + + it_behaves_like 'JayAPI::Elasticsearch::Client#' + end end diff --git a/spec/jay_api/elasticsearch/index_spec.rb b/spec/jay_api/elasticsearch/index_spec.rb index c3ba7f7..25c6d83 100644 --- a/spec/jay_api/elasticsearch/index_spec.rb +++ b/spec/jay_api/elasticsearch/index_spec.rb @@ -205,6 +205,178 @@ it_behaves_like 'Indexable#validate_type' end + describe '#update' do + subject(:method_call) { index.update(id:, **method_params) } + + let(:id) { 'ns4AAZ8BXEjZhYMmw-8y' } + + let(:method_params) { {} } + + let(:successful_response) do + { + '_index' => 'elite_unit_tests', + '_type' => 'nested', + '_id' => 'ns4AAZ8BXEjZhYMmw-8y', + '_version' => 7, + 'result' => 'updated', + '_shards' => { 'total' => 2, 'successful' => 2, 'failed' => 0 }, + '_seq_no' => 11, + '_primary_term' => 1 + } + end + + before do + allow(client).to receive(:update).and_return(successful_response) + end + + shared_examples_for '#update when the update fails' do + let(:error) do + [ + Elasticsearch::Transport::Transport::Errors::BadRequest, + '400 - Bad Request' + ] + end + + before do + allow(client).to receive(:update).and_raise(*error) + end + + it 'raises an Elasticsearch::Transport::Transport::ServerError' do + expect { method_call }.to raise_error(*error) + end + end + + context 'when neither doc nor script are provided' do + let(:method_params) { {} } + + it 'raises an ArgumentError' do + expect { method_call }.to raise_error( + ArgumentError, + "Either 'doc' or 'script' must be provided" + ) + end + end + + context 'when a doc is provided' do + let(:doc) do + { + test_case: { + updated_at: '2026/06/30 09:54:27', + valid: false, + reason: 'The test case was invalidated due to a new firmware update' + } + } + end + + let(:method_params) { { doc: } } + + let(:expected_parameters) do + { + index: 'elite_unit_tests', + id: 'ns4AAZ8BXEjZhYMmw-8y', + body: { doc: doc } + } + end + + it 'calls #update on the client with the expected parameters' do + expect(client).to receive(:update).with(expected_parameters) + method_call + end + + context 'when the update fails' do + it_behaves_like '#update when the update fails' + end + + it 'returns the expected Hash' do + expect(method_call).to eq(successful_response) + end + end + + context 'when a script is provided' do + let(:script_hash) do + { + source: 'ctx._source.test_case.execution_count += params.delta;', + lang: 'painless', + params: { delta: 2 } + } + end + + let(:script) do + instance_double( + JayAPI::Elasticsearch::Script, + to_h: script_hash + ) + end + + let(:method_params) { { script: } } + + let(:expected_parameters) do + { + index: 'elite_unit_tests', + id: 'ns4AAZ8BXEjZhYMmw-8y', + body: { script: script_hash } + } + end + + it 'calls #update on the client with the expected parameters' do + expect(client).to receive(:update).with(expected_parameters) + method_call + end + + context 'when the update fails' do + it_behaves_like '#update when the update fails' + end + + it 'returns the expected Hash' do + expect(method_call).to eq(successful_response) + end + end + + context 'when both doc and script are provided' do + let(:doc) do + { test_case: { owner: 'robert.smith' } } + end + + let(:script_hash) do + { + source: 'ctx._source.test_case.owner = params.owner;', + lang: 'painless', + params: { owner: 'robert.smith' } + } + end + + let(:script) do + instance_double( + JayAPI::Elasticsearch::Script, + to_h: script_hash + ) + end + + let(:method_params) { { doc:, script: } } + + let(:expected_parameters) do + { + index: 'elite_unit_tests', + id: 'ns4AAZ8BXEjZhYMmw-8y', + body: { doc: doc, script: script_hash } + } + end + + it "calls #update on the client with the expected parameters (forwards both 'doc' and 'script')" do + expect(client).to receive(:update).with(expected_parameters) + method_call + end + + context 'when the update fails' do + it_behaves_like '#update when the update fails' + end + + it 'returns the expected Hash' do + expect(method_call).to eq(successful_response) + end + end + end + describe '#settings' do subject(:method_call) { index.settings }