fix: forward $args in Index::forcemerge to Elasticsearch by ruflin · Pull Request #2301 · ruflin/Elastica

ruflin · 2026-04-30T20:22:25Z

Summary

Elastica\Index::forcemerge() had a malformed array_merge() call:

\array_merge(['index' => $this->getName(), $args])

This is a single-argument array_merge, so $args ended up as a numeric
key inside the metadata array and was never sent to Elasticsearch. Options
like max_num_segments, flush, or only_expunge_deletes were silently
ignored.

The fix:

Properly merge with array_merge(['index' => $name], $args).
Tighten the parameter type from untyped to array (matches PHPDoc).
Add a functional test that asserts the options arrive in the last HTTP
request's query string.

Identified during a P0/P1 code-review pass.

Test plan

make docker-run-phpunit PHPUNIT_OPTIONS="--filter=testForcemerge"
CI matrix (PHP 8.1–8.5)
PHPStan + php-cs-fixer

Summary by CodeRabbit

Release Notes

Bug Fixes
- Fixed an issue where the forcemerge functionality was not properly forwarding additional configuration options to Elasticsearch. Extra parameters are now correctly passed through as intended.

A malformed `array_merge(['index' => $name, $args])` call (single argument instead of two) caused user-supplied options like `max_num_segments` and `flush` to be silently appended at a numeric key inside the metadata array, never reaching Elasticsearch. Tighten the parameter type from untyped to `array` and merge the arguments correctly. Add a functional test that asserts the options surface in the last HTTP request's query string.

coderabbitai · 2026-04-30T20:22:38Z

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: e6a3c794-9248-4ada-b5a7-b3a486857b1d

📥 Commits

Reviewing files that changed from the base of the PR and between d0c0d60 and dfa6f16.

📒 Files selected for processing (3)

CHANGELOG.md
src/Index.php
tests/IndexTest.php

📝 Walkthrough

Walkthrough

This PR fixes a bug in Index::forcemerge() where additional arguments passed via the $args parameter were being dropped. The fix updates the method signature with array type-hinting and corrects the parameter merging logic to properly forward all arguments to Elasticsearch, along with a test to verify the behavior.

Changes

Cohort / File(s)	Summary
Documentation `CHANGELOG.md`	Added release note documenting the fix for the `forcemerge()` method's argument handling.
Core Implementation `src/Index.php`	Added array type-hint to `$args` parameter and corrected the `array_merge()` call to properly forward additional arguments when invoking `indices()->forcemerge()`.
Test Coverage `tests/IndexTest.php`	Added functional test `testForcemergeForwardsArgsToRequest()` that verifies extra parameters are correctly forwarded in the HTTP request.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Poem

🐰 A param was lost in the merge, what a plight,
Type hints and logic now shine oh so bright,
Args flow like carrots to Elasticsearch's door,
The test hops along, confirming it more! 🥕

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 25.00% which is insufficient. The required threshold is 80.00%.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly and concisely summarizes the main fix: correcting the forwarding of arguments in the forcemerge method.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

Create stacked PR
Commit on current branch

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch fix/index-forcemerge-args

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

_{Review rate limit: 5/8 reviews remaining, refill in 19 minutes and 1 second.}

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

Copilot

Pull request overview

Fixes Elastica\Index::forcemerge() so caller-supplied options are actually forwarded to Elasticsearch (previously dropped due to a malformed array_merge()), and adds coverage + changelog note.

Changes:

Correctly merges ['index' => $this->getName()] with $args when calling the Elasticsearch indices forcemerge API.
Adds a functional test asserting the provided options appear in the last HTTP request’s query string.
Updates the changelog to document the fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.

File	Description
src/Index.php	Fixes request parameter assembly for `Index::forcemerge()` (and adds an `array` type-hint).
tests/IndexTest.php	Adds a functional regression test ensuring `forcemerge()` forwards query args.
CHANGELOG.md	Documents the fix in the Unreleased “Fixed” section.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-04-30T20:25:58Z

+    public function forcemerge(array $args = []): Response
    {


Index::forcemerge() is a public API; narrowing the parameter from untyped to array is a backward-incompatible signature change for any callers currently passing non-arrays (even if those values were previously ignored due to the bug). If BC is a concern for patch/minor releases, consider keeping the parameter untyped and validating/casting internally (or explicitly documenting this as a BC break).

Suggested change

public function forcemerge(array $args = []): Response

{

public function forcemerge($args = []): Response

{

$args = \is_array($args) ? $args : [];

Copilot · 2026-04-30T20:25:58Z

 ### Deprecated
 ### Removed
 ### Fixed
+* Fixed `Elastica\Index::forcemerge()` silently dropping the `$args` argument due to a malformed `array_merge()` call. Additional options such as `max_num_segments` and `flush` are now forwarded to Elasticsearch as documented.


This entry describes the forwarding fix, but the PR also changes Index::forcemerge()'s public method signature by adding an array type-hint. If you keep the type change, it should be called out under “Backward Compatibility Breaks” (or the type change should be reverted) so consumers aren’t surprised by a new TypeError.

Copilot AI review requested due to automatic review settings April 30, 2026 20:22

Copilot started reviewing on behalf of ruflin April 30, 2026 20:22 View session

Copilot AI reviewed Apr 30, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: forward $args in Index::forcemerge to Elasticsearch#2301

fix: forward $args in Index::forcemerge to Elasticsearch#2301
ruflin wants to merge 1 commit into
9.xfrom
fix/index-forcemerge-args

ruflin commented Apr 30, 2026 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Apr 30, 2026 •

edited

Loading

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Copilot AI Apr 30, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

-    public function forcemerge(array $args = []): Response
-    {
+    public function forcemerge($args = []): Response
+    {
+        $args = \is_array($args) ? $args : [];

Conversation

ruflin commented Apr 30, 2026 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Test plan

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Apr 30, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Estimated code review effort

Poem

❌ Failed checks (1 warning)

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Apr 30, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

ruflin commented Apr 30, 2026 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Apr 30, 2026 •

edited

Loading