Skip to content

Commit

Permalink
Allow string argument in AbstractUpdateAction::setRefresh (#1791)
Browse files Browse the repository at this point in the history
Ref #1475 

Currently, `AbstractUpdateAction::setRefresh` only accepts boolean values: 
- `true` - to perform an immediate refresh
- `false` - to not refresh at all

The [documentation](https://www.elastic.co/guide/en/elasticsearch/reference/current/indices-refresh.html), however, strongly suggests the usage of `wait_for` option to ensure a good cluster performance while doing the refresh operation. But the problem is that `wait_for` is of type `string`; thus, we cannot pass it as an argument to the method.

This modifies `AbstractUpdateAction::setRefresh` so that arguments of type `string` such as `wait_for` can also be passed to the method. Additionally, the return type of its getter counterpart, `AbstractUpdateAction::getRefresh`, has also been modified to mirror the accepted types of the setter.
  • Loading branch information
knsarsaba committed Aug 10, 2020
1 parent b43380b commit 9146c80
Show file tree
Hide file tree
Showing 3 changed files with 19 additions and 5 deletions.
3 changes: 2 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
### Added
* Ability to specify the type of authentication manually by the `auth_type` parameter (in the client class config) was added (allowed values are `basic, digest, gssnegotiate, ntlm`)
### Changed

* Allow `string` such as `wait_for` to be passed to `AbstractUpdateAction::setRefresh` [#1791](https://github.com/ruflin/Elastica/pull/1791)
* Changed the return type of `AbstractUpdateAction::getRefresh` to `boolean|string` [#1791](https://github.com/ruflin/Elastica/pull/1791)
### Deprecated
### Removed
### Fixed
Expand Down
16 changes: 12 additions & 4 deletions src/AbstractUpdateAction.php
Original file line number Diff line number Diff line change
Expand Up @@ -252,21 +252,29 @@ public function hasRetryOnConflict()
}

/**
* @param bool $refresh
* @param bool|string $refresh
*
* @return $this
*/
public function setRefresh($refresh = true)
{
return $this->setParam('refresh', (bool) $refresh ? 'true' : 'false');
\is_bool($refresh) && $refresh = $refresh
? Reindex::REFRESH_TRUE
: Reindex::REFRESH_FALSE;

return $this->setParam(Reindex::REFRESH, $refresh);
}

/**
* @return bool
* @return bool|string
*/
public function getRefresh()
{
return 'true' === $this->getParam('refresh');
$refresh = $this->getParam('refresh');

return \in_array($refresh, [Reindex::REFRESH_TRUE, Reindex::REFRESH_FALSE])
? Reindex::REFRESH_TRUE === $refresh
: $refresh;
}

/**
Expand Down
5 changes: 5 additions & 0 deletions tests/DocumentTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Elastica\Document;
use Elastica\Exception\InvalidException;
use Elastica\Index;
use Elastica\Reindex;
use Elastica\Test\Base as BaseTest;

/**
Expand Down Expand Up @@ -118,6 +119,10 @@ public function testGetSetHasRefresh(): void
$document->setRefresh(true);
$this->assertTrue($document->hasRefresh());
$this->assertTrue($document->getRefresh());

$document->setRefresh(Reindex::REFRESH_WAIT_FOR);
$this->assertTrue($document->hasRefresh());
$this->assertEquals(Reindex::REFRESH_WAIT_FOR, $document->getRefresh());
}

/**
Expand Down

0 comments on commit 9146c80

Please sign in to comment.