fix(action): forward dropped request params and fix wait_for_active_shards serialization#31
Merged
Merged
Conversation
…hards serialization Correctness fixes to existing HTTP actions, verified against the OpenSearch REST layer. No new dependencies and no API changes. - HttpOpenIndexAction: serialize wait_for_active_shards ActiveShardCount.ALL as "all" (was "ALL", which the server rejects with HTTP 400) and forward indices options. This was the remaining call site still using ActiveShardCount.toString(). - HttpCloseIndexAction: forward wait_for_active_shards and indices options (both were dropped, so close could return before shards were ready). - HttpUpdateAction: remove version/version_type params. The _update endpoint rejects internal versioning (it uses if_seq_no/if_primary_term, already sent), and UpdateRequest's version setters throw UnsupportedOperationException, so the removed block was unreachable. - HttpIndicesAliasesAction: build the request body via the request's own toXContent so is_write_index/is_hidden/must_exist/routing are emitted and a remove_index action no longer emits an empty "aliases". - HttpRefreshAction, HttpFlushAction, HttpForceMergeAction, HttpDeleteIndexAction, HttpGetMappingsAction: forward indices options (ignore_unavailable, allow_no_indices, expand_wildcards). - HttpForceMergeAction: also forward primary_only. Adds unit tests asserting the emitted query params / body for each fix.
… later primary_only is unknown to OpenSearch 1.x and Elasticsearch 7/8, which reject unrecognized query parameters with HTTP 400 (the test_force_merge integration tests failed on those engines). Gate the parameter on the detected engine type, matching the version-conditional pattern already used by HttpGetMappingsAction. ActionTestUtils now stubs getEngineInfo() (with a per-engine-type overload) so version-conditional serialization can be asserted offline; HttpForceMergeActionTest covers both the OpenSearch 2.x/3.x (sent) and legacy (omitted) branches.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Correctness fixes to existing HTTP actions, continuing the request-parameter
serialization work from #30. Each fix was verified against the corresponding
OpenSearch
Rest*Action. No new dependencies; no API changes.Fixes
wait_for_active_shardsforActiveShardCount.ALLnow serializes to
"all"instead of"ALL"(the server rejects the latterwith HTTP 400). This is the same bug class fixed centrally in feat(client): add PIT, reindex/by-query, resolve-index actions and fix request-param serialization #30; this was the
one remaining call site still using
ActiveShardCount.toString(). Also forwardsindices options.
wait_for_active_shardsand indicesoptions, both of which were dropped (close could otherwise return before shards
were ready).
version/version_typeparams. The_updateendpoint rejects internal versioning (it requiresif_seq_no/if_primary_term, already sent), andUpdateRequest's version setters throwUnsupportedOperationException, so the removed block was unreachable.toXContent, sois_write_index,is_hidden,must_existandroutingarenow emitted (a write-index alias was previously not marked as such), and a
remove_indexaction no longer emits a bogus emptyaliases.HttpDeleteIndexAction / HttpGetMappingsAction — forward indices options
(
ignore_unavailable,allow_no_indices,expand_wildcards).primary_only.Testing
the existing
ActionTestUtilsofflineCurlRequestinspection.mvn testexcluding the Docker /Testcontainers
*ClientTestintegration classes). The container-basedintegration tests should be run in CI to validate end-to-end.