From 223a25f3284f737d9b4a279d2d6b301aa089547a Mon Sep 17 00:00:00 2001 From: SosthenG Date: Mon, 21 Oct 2024 22:55:22 +0200 Subject: [PATCH] Add support for custom range key and refactor addRange in a trait (#2227) IpRange and GeoDistance aggregations were missing the ability to specify a custom key for a range. As it was already done for the Range aggregation, I moved that to a trait that is now used everywhere we need `addRange`. For the IpRange aggregation, I also added the key option to the `addMaskRange` method. The only drawback with this refactoring is that the arguments of the trait have to allow string|int|float|null to be usable everywhere and we loose some phpdoc details on the arguments. In my opinion, people using this should already know what format is expected and can always check the doc so I personally prefer to have the code deduplicated, but if you disagree I can change it back to add the key everywhere without using a common trait. --- CHANGELOG.md | 1 + src/Aggregation/GeoDistance.php | 32 +------------------ src/Aggregation/IpRange.php | 38 +++++------------------ src/Aggregation/Range.php | 37 +--------------------- src/Aggregation/Traits/RangeTrait.php | 44 +++++++++++++++++++++++++++ tests/Aggregation/GeoDistanceTest.php | 31 +++++++++++++++++++ tests/Aggregation/IpRangeTest.php | 33 ++++++++++++++++++++ 7 files changed, 119 insertions(+), 97 deletions(-) create mode 100644 src/Aggregation/Traits/RangeTrait.php diff --git a/CHANGELOG.md b/CHANGELOG.md index 7bb7e61ab..e82f41c6e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -11,6 +11,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Added * Added support for the multi-match query type `bool_prefix` [#2220](https://github.com/ruflin/Elastica/pull/2220) * Supported PHP 8.4 [#2221](https://github.com/ruflin/Elastica/pull/2221) +* Added support for custom key to IpRange and GeoDistance `addRange` using a common trait [#2227](https://github.com/ruflin/Elastica/pull/2227) ### Changed diff --git a/src/Aggregation/GeoDistance.php b/src/Aggregation/GeoDistance.php index 7f98e112c..57cfe02b1 100644 --- a/src/Aggregation/GeoDistance.php +++ b/src/Aggregation/GeoDistance.php @@ -4,8 +4,6 @@ namespace Elastica\Aggregation; -use Elastica\Exception\InvalidException; - /** * Class GeoDistance. * @@ -14,6 +12,7 @@ class GeoDistance extends AbstractAggregation { use Traits\KeyedTrait; + use Traits\RangeTrait; public const DISTANCE_TYPE_ARC = 'arc'; public const DISTANCE_TYPE_PLANE = 'plane'; @@ -56,35 +55,6 @@ public function setOrigin($origin): self return $this->setParam('origin', $origin); } - /** - * Add a distance range to this aggregation. - * - * @param int|null $fromValue a distance - * @param int|null $toValue a distance - * - * @throws InvalidException - * - * @return $this - */ - public function addRange(?int $fromValue = null, ?int $toValue = null): self - { - if (null === $fromValue && null === $toValue) { - throw new InvalidException('Either fromValue or toValue must be set. Both cannot be null.'); - } - - $range = []; - - if (null !== $fromValue) { - $range['from'] = $fromValue; - } - - if (null !== $toValue) { - $range['to'] = $toValue; - } - - return $this->addParam('ranges', $range); - } - /** * Set the unit of distance measure for this aggregation. * diff --git a/src/Aggregation/IpRange.php b/src/Aggregation/IpRange.php index a44d4805d..2c40c2950 100644 --- a/src/Aggregation/IpRange.php +++ b/src/Aggregation/IpRange.php @@ -4,8 +4,6 @@ namespace Elastica\Aggregation; -use Elastica\Exception\InvalidException; - /** * Class IpRange. * @@ -14,6 +12,7 @@ class IpRange extends AbstractAggregation { use Traits\KeyedTrait; + use Traits\RangeTrait; /** * @param string $name the name of this aggregation @@ -38,42 +37,21 @@ public function setField(string $field): self } /** - * Add an ip range to this aggregation. - * - * @param string|null $fromValue a valid ipv4 address. Low end of this range, exclusive (greater than) - * @param string|null $toValue a valid ipv4 address. High end of this range, exclusive (less than) + * Add an ip range in the form of a CIDR mask. * - * @throws InvalidException + * @param string $mask a valid CIDR mask + * @param string|null $key customized key value * * @return $this */ - public function addRange(?string $fromValue = null, ?string $toValue = null): self + public function addMaskRange(string $mask, ?string $key = null): self { - if (null === $fromValue && null === $toValue) { - throw new InvalidException('Either fromValue or toValue must be set. Both cannot be null.'); - } + $range = ['mask' => $mask]; - $range = []; - if (null !== $fromValue) { - $range['from'] = $fromValue; - } - - if (null !== $toValue) { - $range['to'] = $toValue; + if (null !== $key) { + $range['key'] = $key; } return $this->addParam('ranges', $range); } - - /** - * Add an ip range in the form of a CIDR mask. - * - * @param string $mask a valid CIDR mask - * - * @return $this - */ - public function addMaskRange(string $mask): self - { - return $this->addParam('ranges', ['mask' => $mask]); - } } diff --git a/src/Aggregation/Range.php b/src/Aggregation/Range.php index 785d2ac13..0ada73582 100644 --- a/src/Aggregation/Range.php +++ b/src/Aggregation/Range.php @@ -4,8 +4,6 @@ namespace Elastica\Aggregation; -use Elastica\Exception\InvalidException; - /** * Class Range. * @@ -14,38 +12,5 @@ class Range extends AbstractSimpleAggregation { use Traits\KeyedTrait; - - /** - * Add a range to this aggregation. - * - * @param float|int|string|null $fromValue low end of this range, exclusive (greater than or equal to) - * @param float|int|string|null $toValue high end of this range, exclusive (less than) - * @param string|null $key customized key value - * - * @throws InvalidException - * - * @return $this - */ - public function addRange($fromValue = null, $toValue = null, ?string $key = null): self - { - if (null === $fromValue && null === $toValue) { - throw new InvalidException('Either fromValue or toValue must be set. Both cannot be null.'); - } - - $range = []; - - if (null !== $fromValue) { - $range['from'] = $fromValue; - } - - if (null !== $toValue) { - $range['to'] = $toValue; - } - - if (null !== $key) { - $range['key'] = $key; - } - - return $this->addParam('ranges', $range); - } + use Traits\RangeTrait; } diff --git a/src/Aggregation/Traits/RangeTrait.php b/src/Aggregation/Traits/RangeTrait.php new file mode 100644 index 000000000..93be07df3 --- /dev/null +++ b/src/Aggregation/Traits/RangeTrait.php @@ -0,0 +1,44 @@ +addParam('ranges', $range); + } +} diff --git a/tests/Aggregation/GeoDistanceTest.php b/tests/Aggregation/GeoDistanceTest.php index d4246cc06..b6b0aad8f 100644 --- a/tests/Aggregation/GeoDistanceTest.php +++ b/tests/Aggregation/GeoDistanceTest.php @@ -51,6 +51,37 @@ public function testGeoDistanceKeyedAggregation(): void $this->assertSame($expected, \array_keys($results['buckets'])); } + /** + * @group unit + */ + public function testGeoDistanceAggregationWithKey(): void + { + $agg = new GeoDistance('geo', 'location', ['lat' => 32.804654, 'lon' => -117.242594]); + $agg->addRange(null, 10, 'first'); + $agg->addRange(10, null, 'second'); + $agg->setUnit('mi'); + + $expected = [ + 'geo_distance' => [ + 'field' => 'location', + 'origin' => ['lat' => 32.804654, 'lon' => -117.242594], + 'unit' => 'mi', + 'ranges' => [ + [ + 'to' => 10, + 'key' => 'first', + ], + [ + 'from' => 10, + 'key' => 'second', + ], + ], + ], + ]; + + $this->assertEquals($expected, $agg->toArray()); + } + protected function _getIndexForTest(): Index { $index = $this->_createIndex(); diff --git a/tests/Aggregation/IpRangeTest.php b/tests/Aggregation/IpRangeTest.php index 2d989267b..ddf68c8d8 100644 --- a/tests/Aggregation/IpRangeTest.php +++ b/tests/Aggregation/IpRangeTest.php @@ -67,6 +67,39 @@ public function testIpRangeKeyedAggregation(): void $this->assertSame($expected, \array_keys($results['buckets'])); } + /** + * @group unit + */ + public function testIpRangeAggregationWithKey(): void + { + $agg = new IpRange('ip', 'address'); + $agg->addRange('192.168.1.101', null, 'first'); + $agg->addRange(null, '192.168.1.200', 'second'); + $agg->addMaskRange('192.168.1.0/24', 'mask'); + + $expected = [ + 'ip_range' => [ + 'field' => 'address', + 'ranges' => [ + [ + 'from' => '192.168.1.101', + 'key' => 'first', + ], + [ + 'to' => '192.168.1.200', + 'key' => 'second', + ], + [ + 'mask' => '192.168.1.0/24', + 'key' => 'mask', + ], + ], + ], + ]; + + $this->assertEquals($expected, $agg->toArray()); + } + protected function _getIndexForTest(): Index { $index = $this->_createIndex();