Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Remove aggregation deprecations #2138

Merged
merged 1 commit into from
Jan 3, 2023
Merged

Remove aggregation deprecations #2138

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,17 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
* `Elastica\Result::getType()`
* `Elastica\Suggest\Phrase::addCandidateGenerator()` -> use `Elastica\Suggest\Phrase::addDirectGenerator()` instead
* `Elastica\Util::getParamName()`
* Changed following aggregation constructors [#2138](https://github.com/ruflin/Elastica/pull/2138)
* `Elastica\Aggregation\AvgBucket`: The second argument `$bucketsPath` is now mandatory
* `Elastica\Aggregation\BucketScript`: The second (`array $bucketsPath`) and the third (`string $script`) argument are now mandatory
* `Elastica\Aggregation\BucketSelector`: The second (`array $bucketsPath`) and the third (`string $script`) argument are now mandatory
* `Elastica\Aggregation\Derivative`: The second argument (`string $bucketsPath`) is now mandatory
* `Elastica\Aggregation\NormalizeAggregation`: The second (`string $bucketsPath`) and the third (`string $method`) argument are now mandatory
* `Elastica\Aggregation\PercentilesBucket`: The second argument (`string $bucketsPath`) is now mandatory
* `Elastica\Aggregation\SerialDiff`: The second argument (`string $bucketsPath`) is now mandatory
* `Elastica\Aggregation\StatsBucket`: The second argument (`string $bucketsPath`) is now mandatory
* `Elastica\Aggregation\SumBucket`: The second argument (`string $bucketsPath`) is now mandatory

### Added
* Added support for PHP 8.2 [#2136](https://github.com/ruflin/Elastica/pull/2136)
### Changed
Expand Down
24 changes: 2 additions & 22 deletions src/Aggregation/AvgBucket.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class AvgBucket.
*
Expand All @@ -16,17 +14,11 @@ class AvgBucket extends AbstractAggregation implements GapPolicyInterface

public const DEFAULT_FORMAT_VALUE = null;

public function __construct(string $name, ?string $bucketsPath = null)
public function __construct(string $name, string $bucketsPath)
{
parent::__construct($name);

if (null !== $bucketsPath) {
$this->setBucketsPath($bucketsPath);
} elseif (\func_num_args() >= 2) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}
$this->setBucketsPath($bucketsPath);
}

/**
Expand All @@ -38,16 +30,4 @@ public function setFormat(?string $format = null): self
{
return $this->setParam('format', $format);
}

/**
* @throws InvalidException If buckets path or script is not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}

return parent::toArray();
}
}
28 changes: 3 additions & 25 deletions src/Aggregation/BucketScript.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class BucketScript.
*
Expand All @@ -13,17 +11,12 @@ class BucketScript extends AbstractAggregation implements GapPolicyInterface
{
use Traits\GapPolicyTrait;

public function __construct(string $name, ?array $bucketsPath = null, ?string $script = null)
public function __construct(string $name, array $bucketsPath, string $script)
{
parent::__construct($name);

if (null !== $bucketsPath) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one we did not deprecate.

One thing I've seen in the past is that an object was created when the bucketPath did not exist yet and was set at a later stage (just an example). Is this a use case we want to support or on this scenario we except the object to be created when all info is available?

Copy link
Contributor Author

@franmomu franmomu Dec 29, 2022 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I would deprecate them in 7.x.

When working with ES, these parameters are required:

https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-pipeline-bucket-script-aggregation.html
https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-pipeline-bucket-selector-aggregation.html

Since they are required, I think it makes sense to not be able to create a BucketScript instance with invalid data.

IMHO we should avoid supporting those scenarios where these classes can contain invalid data (in some of them we can even mark them as immutable), it simplifies the code and ease the maintenance since these objects would always be in a valid state.

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lets call this a decision that all required params have to be set when creating an object.

Can you open a PR against 7.x with a deprecation?

$this->setBucketsPath($bucketsPath);
}

if (null !== $script) {
$this->setScript($script);
}
$this->setBucketsPath($bucketsPath);
$this->setScript($script);
}

/**
Expand Down Expand Up @@ -55,19 +48,4 @@ public function setFormat(?string $format = null): self
{
return $this->setParam('format', $format);
}

/**
* @throws InvalidException If buckets path or script is not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}
if (!$this->hasParam('script')) {
throw new InvalidException('Script parameter is required');
}

return parent::toArray();
}
}
11 changes: 3 additions & 8 deletions src/Aggregation/BucketSelector.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,12 @@ class BucketSelector extends AbstractSimpleAggregation implements GapPolicyInter
{
use Traits\GapPolicyTrait;

public function __construct(string $name, ?array $bucketsPath = null, ?string $script = null)
public function __construct(string $name, array $bucketsPath, string $script)
{
parent::__construct($name);

if (null !== $bucketsPath) {
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above. Do we need to deprecate it in 7.x branch if we go down this path?

$this->setBucketsPath($bucketsPath);
}

if (null !== $script) {
$this->setScript($script);
}
$this->setBucketsPath($bucketsPath);
$this->setScript($script);
}

/**
Expand Down
24 changes: 2 additions & 22 deletions src/Aggregation/Derivative.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class Derivative.
*
Expand All @@ -14,17 +12,11 @@ class Derivative extends AbstractAggregation implements GapPolicyInterface
use Traits\BucketsPathTrait;
use Traits\GapPolicyTrait;

public function __construct(string $name, ?string $bucketsPath = null)
public function __construct(string $name, string $bucketsPath)
{
parent::__construct($name);

if (null !== $bucketsPath) {
$this->setBucketsPath($bucketsPath);
} elseif (\func_num_args() >= 2) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}
$this->setBucketsPath($bucketsPath);
}

/**
Expand All @@ -36,16 +28,4 @@ public function setFormat(string $format): self
{
return $this->setParam('format', $format);
}

/**
* @throws InvalidException If buckets path or script is not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}

return parent::toArray();
}
}
37 changes: 3 additions & 34 deletions src/Aggregation/NormalizeAggregation.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class NormalizeAggregation.
*
Expand All @@ -13,25 +11,12 @@ class NormalizeAggregation extends AbstractAggregation
{
use Traits\BucketsPathTrait;

public function __construct(string $name, ?string $bucketsPath = null, ?string $method = null)
public function __construct(string $name, string $bucketsPath, string $method)
{
parent::__construct($name);

if (null !== $bucketsPath) {
$this->setBucketsPath($bucketsPath);
} elseif (\func_num_args() >= 2) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}

if (null !== $method) {
$this->setMethod($method);
} elseif (\func_num_args() >= 3) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 3rd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 3rd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}
$this->setBucketsPath($bucketsPath);
$this->setMethod($method);
}

/**
Expand All @@ -53,20 +38,4 @@ public function setFormat(string $format): self
{
return $this->setParam('format', $format);
}

/**
* @throws InvalidException If buckets path or method are not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}

if (!$this->hasParam('method')) {
throw new InvalidException('Method parameter is required');
}

return parent::toArray();
}
}
24 changes: 2 additions & 22 deletions src/Aggregation/PercentilesBucket.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class PercentilesBucket.
*
Expand All @@ -15,29 +13,11 @@ class PercentilesBucket extends AbstractAggregation implements GapPolicyInterfac
use Traits\GapPolicyTrait;
use Traits\KeyedTrait;

public function __construct(string $name, ?string $bucketsPath = null)
public function __construct(string $name, string $bucketsPath)
{
parent::__construct($name);

if (null !== $bucketsPath) {
$this->setBucketsPath($bucketsPath);
} elseif (\func_num_args() >= 2) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}
}

/**
* @throws InvalidException If buckets path or script is not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}

return parent::toArray();
$this->setBucketsPath($bucketsPath);
}

/**
Expand Down
24 changes: 2 additions & 22 deletions src/Aggregation/SerialDiff.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class SerialDiff.
*
Expand All @@ -15,17 +13,11 @@ class SerialDiff extends AbstractAggregation implements GapPolicyInterface

public const DEFAULT_GAP_POLICY_VALUE = GapPolicyInterface::INSERT_ZEROS;

public function __construct(string $name, ?string $bucketsPath = null)
public function __construct(string $name, string $bucketsPath)
{
parent::__construct($name);

if (null !== $bucketsPath) {
$this->setBucketsPath($bucketsPath);
} elseif (\func_num_args() >= 2) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}
$this->setBucketsPath($bucketsPath);
}

/**
Expand Down Expand Up @@ -57,16 +49,4 @@ public function setFormat(?string $format = null): self
{
return $this->setParam('format', $format);
}

/**
* @throws InvalidException If buckets path is not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}

return parent::toArray();
}
}
24 changes: 2 additions & 22 deletions src/Aggregation/StatsBucket.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,6 @@

namespace Elastica\Aggregation;

use Elastica\Exception\InvalidException;

/**
* Class StatsBucket.
*
Expand All @@ -14,17 +12,11 @@ class StatsBucket extends AbstractAggregation implements GapPolicyInterface
use Traits\BucketsPathTrait;
use Traits\GapPolicyTrait;

public function __construct(string $name, ?string $bucketsPath = null)
public function __construct(string $name, string $bucketsPath)
{
parent::__construct($name);

if (null !== $bucketsPath) {
$this->setBucketsPath($bucketsPath);
} elseif (\func_num_args() >= 2) {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Passing null as 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
} else {
\trigger_deprecation('ruflin/elastica', '7.1.3', 'Not passing a 2nd argument to "%s()" is deprecated, pass a string instead. It will be removed in 8.0.', __METHOD__);
}
$this->setBucketsPath($bucketsPath);
}

/**
Expand All @@ -36,16 +28,4 @@ public function setFormat(string $format): self
{
return $this->setParam('format', $format);
}

/**
* @throws InvalidException If buckets path or script is not set
*/
public function toArray(): array
{
if (!$this->hasParam('buckets_path')) {
throw new InvalidException('Buckets path is required');
}

return parent::toArray();
}
}
Loading