From 2831bd51a07e7a00e8bdc0a952d858ed6e43c453 Mon Sep 17 00:00:00 2001 From: Romain Neutron Date: Mon, 23 Nov 2020 12:43:19 +0100 Subject: [PATCH] Deprecate using "interval" configuration for date-histograms aggregation --- CHANGELOG.md | 4 +- src/Aggregation/DateHistogram.php | 84 +++++++++++++++++++- src/QueryBuilder/DSL/Aggregation.php | 8 +- tests/Aggregation/BucketSelectorTest.php | 5 +- tests/Aggregation/DateHistogramTest.php | 92 ++++++++++++++++++++-- tests/Aggregation/DerivativeTest.php | 5 +- tests/Aggregation/SerialDiffTest.php | 5 +- tests/QueryBuilder/DSL/AggregationTest.php | 2 +- 8 files changed, 187 insertions(+), 18 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8be0a7d66f..0dddf5bea8 100755 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,7 +6,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased](https://github.com/ruflin/Elastica/compare/7.0.0...master) ### Backward Compatibility Breaks -* Allow the Terms query to accept arrays of strings, ints and floats [#1872](https://github.com/ruflin/Elastica/pull/1872) +* Allow the Terms query to accept arrays of scalars [#1872](https://github.com/ruflin/Elastica/pull/1872) +* `Elastica\Aggregation\DateHistogram` does not extends `Elastica\Aggregation\Histogram` anymore [#1874](https://github.com/ruflin/Elastica/pull/1874) ### 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`) * Added `if_seq_no` / `if_primary_term` to replace `version` for [optimistic concurrency control](https://www.elastic.co/guide/en/elasticsearch/reference/6.8/optimistic-concurrency-control.html) @@ -45,6 +46,7 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 * Deprecated Match query class and introduced MatchQuery instead for PHP 8.0 compatibility reason [#1799](https://github.com/ruflin/Elastica/pull/1799) * Deprecated `version`/`version_type` options [(deprecated in `6.7.0`)](https://www.elastic.co/guide/en/elasticsearch/reference/6.8/docs-update.html) and added `if_seq_no` / `if_primary_term` that replaced it * Deprecated passing `bool` or `null` as 2nd argument to `Elastica\Index::create()` [#1828](https://github.com/ruflin/Elastica/pull/1828) +* Deprecated `Elastica\Aggregation\DateHistogram::__construct` third argument (`$interval`) and `Elastica\Aggregation\DateHistogram::setInterval` in favor of `setFixedInterval()` and `setCalendarInterval()` [#1874](https://github.com/ruflin/Elastica/pull/1874) ### Removed * Removed HHVM proxy detection [#1818](https://github.com/ruflin/Elastica/pull/1818) ### Fixed diff --git a/src/Aggregation/DateHistogram.php b/src/Aggregation/DateHistogram.php index a673c2c47f..507008fdb1 100644 --- a/src/Aggregation/DateHistogram.php +++ b/src/Aggregation/DateHistogram.php @@ -7,10 +7,67 @@ * * @see https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-datehistogram-aggregation.html */ -class DateHistogram extends Histogram +class DateHistogram extends AbstractSimpleAggregation { public const DEFAULT_TIMEZONE_VALUE = 'UTC'; + /** + * @param string $name the name of this aggregation + * @param string $field the name of the field on which to perform the aggregation + * @param int|string $interval the interval by which documents will be bucketed + */ + public function __construct(string $name, string $field, $interval = null) + { + parent::__construct($name, $field); + $this->setField($field); + + if (null !== $interval) { + trigger_deprecation('ruflin/elastica', '7.1.0', 'Argument 3 passed to "%s()" is deprecated, use "setDateInterval()" or "setCalendarInterval()" instead. It will be removed in 8.0.', __METHOD__); + + $this->setParam('interval', $interval); + } + } + + /** + * Set the interval by which documents will be bucketed. + * + * @deprecated Deprecated since 7.1.0 + * + * @param int|string $interval + * + * @return $this + */ + public function setInterval($interval): self + { + trigger_deprecation('ruflin/elastica', '7.1.0', 'The "%s()" method is deprecated, use "setDateInterval()" or "setCalendarInterval()" instead. It will be removed in 8.0.', __METHOD__); + + return $this->setParam('interval', $interval); + } + + /** + * Set the fixed interval by which documents will be bucketed. + * + * @param int|string $interval + * + * @return $this + */ + public function setFixedInterval($interval): self + { + return $this->setParam('fixed_interval', $interval); + } + + /** + * Set the calendar interval by which documents will be bucketed. + * + * @param int|string $interval + * + * @return $this + */ + public function setCalendarInterval($interval): self + { + return $this->setParam('calendar_interval', $interval); + } + /** * Set time_zone option. * @@ -80,4 +137,29 @@ public function setExtendedBounds(string $min = '', string $max = ''): self return $this->setParam('extended_bounds', $bounds); } + + /** + * Set the bucket sort order. + * + * @param string $order "_count", "_term", or the name of a sub-aggregation or sub-aggregation response field + * @param string $direction "asc" or "desc" + * + * @return $this + */ + public function setOrder(string $order, string $direction): self + { + return $this->setParam('order', [$order => $direction]); + } + + /** + * Set the minimum number of documents which must fall into a bucket in order for the bucket to be returned. + * + * @param int $count set to 0 to include empty buckets + * + * @return $this + */ + public function setMinimumDocumentCount(int $count): self + { + return $this->setParam('min_doc_count', $count); + } } diff --git a/src/QueryBuilder/DSL/Aggregation.php b/src/QueryBuilder/DSL/Aggregation.php index 0a34a87535..974c98bc31 100644 --- a/src/QueryBuilder/DSL/Aggregation.php +++ b/src/QueryBuilder/DSL/Aggregation.php @@ -354,9 +354,9 @@ public function ipv4_range(string $name, string $field): IpRange * * @see https://www.elastic.co/guide/en/elasticsearch/reference/current/search-aggregations-bucket-histogram-aggregation.html * - * @param string $name the name of this aggregation - * @param string $field the name of the field on which to perform the aggregation - * @param int $interval the interval by which documents will be bucketed + * @param string $name the name of this aggregation + * @param string $field the name of the field on which to perform the aggregation + * @param int|string $interval the interval by which documents will be bucketed */ public function histogram(string $name, string $field, $interval): Histogram { @@ -372,7 +372,7 @@ public function histogram(string $name, string $field, $interval): Histogram * @param string $field the name of the field on which to perform the aggregation * @param int|string $interval the interval by which documents will be bucketed */ - public function date_histogram(string $name, string $field, $interval): DateHistogram + public function date_histogram(string $name, string $field, $interval = null): DateHistogram { return new DateHistogram($name, $field, $interval); } diff --git a/tests/Aggregation/BucketSelectorTest.php b/tests/Aggregation/BucketSelectorTest.php index 57dc41c5c7..0bca25b690 100644 --- a/tests/Aggregation/BucketSelectorTest.php +++ b/tests/Aggregation/BucketSelectorTest.php @@ -46,9 +46,12 @@ public function testToArray(): void */ public function testMaxAggregation(): void { + $this->_checkVersion('7.2'); + $index = $this->_getIndexForTest(); - $dateHistogramAgg = new DateHistogram('histogram_agg', 'date', 'day'); + $dateHistogramAgg = new DateHistogram('histogram_agg', 'date'); + $dateHistogramAgg->setFixedInterval('1d'); $dateHistogramAgg->setFormat('yyyy-MM-dd'); $maxAgg = new Max('max_agg'); diff --git a/tests/Aggregation/DateHistogramTest.php b/tests/Aggregation/DateHistogramTest.php index b7b0c1fa68..0bec31500f 100644 --- a/tests/Aggregation/DateHistogramTest.php +++ b/tests/Aggregation/DateHistogramTest.php @@ -7,18 +7,86 @@ use Elastica\Index; use Elastica\Mapping; use Elastica\Query; +use Symfony\Bridge\PhpUnit\ExpectDeprecationTrait; /** * @internal */ class DateHistogramTest extends BaseAggregationTest { + use ExpectDeprecationTrait; + /** * @group functional */ public function testDateHistogramAggregation(): void { - $agg = new DateHistogram('hist', 'created', '1h'); + $agg = new DateHistogram('hist', 'created'); + + $version = $this->_getVersion(); + + if (\version_compare($version, '7.2') < 0) { + $agg->setParam('interval', '1h'); + } else { + $agg->setFixedInterval('1h'); + } + + $query = new Query(); + $query->addAggregation($agg); + $results = $this->_getIndexForTest()->search($query)->getAggregation('hist'); + + $docCount = 0; + $nonDocCount = 0; + foreach ($results['buckets'] as $bucket) { + if (1 == $bucket['doc_count']) { + ++$docCount; + } else { + ++$nonDocCount; + } + } + // 3 Documents that were added + $this->assertEquals(3, $docCount); + // 1 document that was generated in between for the missing hour + $this->assertEquals(1, $nonDocCount); + } + + /** + * @group unit + * @group legacy + */ + public function testDateHistogramAggregationWithIntervalTriggersADeprecation(): void + { + $this->expectDeprecation('Since ruflin/elastica 7.1.0: Argument 3 passed to "Elastica\Aggregation\DateHistogram::__construct()" is deprecated, use "setDateInterval()" or "setCalendarInterval()" instead. It will be removed in 8.0.'); + new DateHistogram('hist', 'created', 'day'); + } + + /** + * @group unit + * @group legacy + */ + public function testDateHistogramAggregationSetIntervalTriggersADeprecation(): void + { + $agg = new DateHistogram('hist', 'created'); + + $this->expectDeprecation('Since ruflin/elastica 7.1.0: The "Elastica\Aggregation\DateHistogram::setInterval()" method is deprecated, use "setDateInterval()" or "setCalendarInterval()" instead. It will be removed in 8.0.'); + + $agg->setInterval('day'); + } + + /** + * @group functional + */ + public function testDateHistogramCalendarAggregation(): void + { + $agg = new DateHistogram('hist', 'created'); + + $version = $this->_getVersion(); + + if (\version_compare($version, '7.2') < 0) { + $agg->setParam('interval', '1h'); + } else { + $agg->setCalendarInterval('1h'); + } $query = new Query(); $query->addAggregation($agg); @@ -44,15 +112,16 @@ public function testDateHistogramAggregation(): void */ public function testSetOffset(): void { - $agg = new DateHistogram('hist', 'created', '1h'); + $agg = new DateHistogram('hist', 'created'); + $agg->setFixedInterval('1h'); $agg->setOffset('3m'); $expected = [ 'date_histogram' => [ 'field' => 'created', - 'interval' => '1h', 'offset' => '3m', + 'fixed_interval' => '1h', ], ]; @@ -66,11 +135,17 @@ public function testSetOffset(): void */ public function testSetOffsetWorks(): void { - $this->_checkVersion('1.5'); - - $agg = new DateHistogram('hist', 'created', '1m'); + $agg = new DateHistogram('hist', 'created'); $agg->setOffset('+40s'); + $version = $this->_getVersion(); + + if (\version_compare($version, '7.2') < 0) { + $agg->setParam('interval', '1m'); + } else { + $agg->setFixedInterval('1m'); + } + $query = new Query(); $query->addAggregation($agg); $results = $this->_getIndexForTest()->search($query)->getAggregation('hist'); @@ -83,15 +158,16 @@ public function testSetOffsetWorks(): void */ public function testSetTimezone(): void { - $agg = new DateHistogram('hist', 'created', '1h'); + $agg = new DateHistogram('hist', 'created'); + $agg->setFixedInterval('1h'); $agg->setTimezone('-02:30'); $expected = [ 'date_histogram' => [ 'field' => 'created', - 'interval' => '1h', 'time_zone' => '-02:30', + 'fixed_interval' => '1h', ], ]; diff --git a/tests/Aggregation/DerivativeTest.php b/tests/Aggregation/DerivativeTest.php index 3b451a2eec..464f69feb8 100644 --- a/tests/Aggregation/DerivativeTest.php +++ b/tests/Aggregation/DerivativeTest.php @@ -45,9 +45,12 @@ public function testToArray(): void */ public function testMaxAggregation(): void { + $this->_checkVersion('7.2'); + $index = $this->_getIndexForTest(); - $dateHistogramAgg = new DateHistogram('histogram_agg', 'date', 'day'); + $dateHistogramAgg = new DateHistogram('histogram_agg', 'date'); + $dateHistogramAgg->setFixedInterval('1d'); $dateHistogramAgg->setFormat('yyyy-MM-dd'); $maxAgg = new Max('max_agg'); diff --git a/tests/Aggregation/SerialDiffTest.php b/tests/Aggregation/SerialDiffTest.php index cd29d5cbba..cabae5c01e 100644 --- a/tests/Aggregation/SerialDiffTest.php +++ b/tests/Aggregation/SerialDiffTest.php @@ -20,7 +20,10 @@ class SerialDiffTest extends BaseAggregationTest */ public function testSerialDiffAggregation(): void { - $dateHistogramAggregation = new DateHistogram('measurements', 'measured_at', 'hour'); + $this->_checkVersion('7.2'); + + $dateHistogramAggregation = new DateHistogram('measurements', 'measured_at'); + $dateHistogramAggregation->setFixedInterval('1h'); $dateHistogramAggregation ->addAggregation((new Max('max_value'))->setField('value')) diff --git a/tests/QueryBuilder/DSL/AggregationTest.php b/tests/QueryBuilder/DSL/AggregationTest.php index 356d5a62af..b232aee025 100644 --- a/tests/QueryBuilder/DSL/AggregationTest.php +++ b/tests/QueryBuilder/DSL/AggregationTest.php @@ -34,7 +34,7 @@ public function testInterface(): void $this->_assertImplemented($aggregationDSL, 'avg', Aggregation\Avg::class, ['name']); $this->_assertImplemented($aggregationDSL, 'cardinality', Aggregation\Cardinality::class, ['name']); - $this->_assertImplemented($aggregationDSL, 'date_histogram', Aggregation\DateHistogram::class, ['name', 'field', 1]); + $this->_assertImplemented($aggregationDSL, 'date_histogram', Aggregation\DateHistogram::class, ['name', 'field']); $this->_assertImplemented($aggregationDSL, 'date_range', Aggregation\DateRange::class, ['name']); $this->_assertImplemented($aggregationDSL, 'extended_stats', Aggregation\ExtendedStats::class, ['name']); $this->_assertImplemented($aggregationDSL, 'filter', Aggregation\Filter::class, ['name', new Exists('field')]);