Deprecate using "interval" configuration for histograms aggregations

ruflin · Nov 23, 2020 · 05805f5 · 05805f5
1 parent a323450
commit 05805f5
Show file tree

Hide file tree

Showing 11 changed files with 87 additions and 16 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -45,6 +45,8 @@ 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\Histogram::__construct` third argument (`$interval`) and `Elastica\Aggregation\Histogram::setInterval` in favor of `setFixedInterval()` and `setCalendarInterval()` [#1874](https://github.com/ruflin/Elastica/pull/1874)
+* 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/Histogram.php b/src/Aggregation/Histogram.php
@@ -14,11 +14,16 @@ class Histogram extends AbstractSimpleAggregation
      * @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)
+    public function __construct(string $name, string $field, $interval = null)
     {
         parent::__construct($name);
         $this->setField($field);
-        $this->setInterval($interval);
+
+        if (null !== $interval) {
+            trigger_deprecation('ruflin/elastica', '7.1.0', 'The "%s::__construct" third argument is deprecated, use "setDateInterval()" or "setCalendarInterval()" instead. It will be removed in 8.0.', __CLASS__);
+
+            $this->setParam('interval', $interval);
+        }
     }
 
     /**
@@ -30,9 +35,35 @@ public function __construct(string $name, string $field, $interval)
      */
     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 the bucket sort order.
      *

diff --git 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
     {

diff --git a/tests/Aggregation/BucketScriptTest.php b/tests/Aggregation/BucketScriptTest.php
@@ -29,7 +29,8 @@ public function testBucketScriptAggregation(): void
             'params.dividend / params.divisor'
         );
 
-        $histogramAggregation = new Histogram('age_groups', 'age', 10);
+        $histogramAggregation = new Histogram('age_groups', 'age');
+        $histogramAggregation->setFixedInterval(10);
 
         $histogramAggregation
             ->addAggregation((new Max('max_weight'))->setField('weight'))

diff --git a/tests/Aggregation/BucketSelectorTest.php b/tests/Aggregation/BucketSelectorTest.php
@@ -48,7 +48,8 @@ public function testMaxAggregation(): void
     {
         $index = $this->_getIndexForTest();
 
-        $dateHistogramAgg = new DateHistogram('histogram_agg', 'date', 'day');
+        $dateHistogramAgg = new DateHistogram('histogram_agg', 'date');
+        $dateHistogramAgg->setFixedInterval('day');
         $dateHistogramAgg->setFormat('yyyy-MM-dd');
 
         $maxAgg = new Max('max_agg');

diff --git a/tests/Aggregation/DateHistogramTest.php b/tests/Aggregation/DateHistogramTest.php
@@ -18,7 +18,35 @@ class DateHistogramTest extends BaseAggregationTest
      */
     public function testDateHistogramAggregation(): void
     {
-        $agg = new DateHistogram('hist', 'created', '1h');
+        $agg = new DateHistogram('hist', 'created');
+        $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 functional
+     */
+    public function testDateHistogramCalendarAggregation(): void
+    {
+        $agg = new DateHistogram('hist', 'created');
+        $agg->setCalendarInterval('1h');
 
         $query = new Query();
         $query->addAggregation($agg);
@@ -44,7 +72,8 @@ 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');
 
@@ -68,7 +97,8 @@ public function testSetOffsetWorks(): void
     {
         $this->_checkVersion('1.5');
 
-        $agg = new DateHistogram('hist', 'created', '1m');
+        $agg = new DateHistogram('hist', 'created');
+        $agg->setFixedInterval('1m');
         $agg->setOffset('+40s');
 
         $query = new Query();
@@ -83,7 +113,8 @@ 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');
 

diff --git a/tests/Aggregation/DerivativeTest.php b/tests/Aggregation/DerivativeTest.php
@@ -47,7 +47,8 @@ public function testMaxAggregation(): void
     {
         $index = $this->_getIndexForTest();
 
-        $dateHistogramAgg = new DateHistogram('histogram_agg', 'date', 'day');
+        $dateHistogramAgg = new DateHistogram('histogram_agg', 'date');
+        $dateHistogramAgg->setFixedInterval('day');
         $dateHistogramAgg->setFormat('yyyy-MM-dd');
 
         $maxAgg = new Max('max_agg');

diff --git a/tests/Aggregation/ExtendedStatsBucketTest.php b/tests/Aggregation/ExtendedStatsBucketTest.php
@@ -21,7 +21,8 @@ public function testExtendedStatBucketAggregation(): void
     {
         $bucketScriptAggregation = new ExtendedStatsBucket('result', 'age_groups>max_weight');
 
-        $histogramAggregation = new Histogram('age_groups', 'age', 10);
+        $histogramAggregation = new Histogram('age_groups', 'age');
+        $histogramAggregation->setFixedInterval(10);
 
         $histogramAggregation->addAggregation((new Max('max_weight'))->setField('weight'));
 

diff --git a/tests/Aggregation/HistogramTest.php b/tests/Aggregation/HistogramTest.php
@@ -17,7 +17,8 @@ class HistogramTest extends BaseAggregationTest
      */
     public function testHistogramAggregation(): void
     {
-        $agg = new Histogram('hist', 'price', 10);
+        $agg = new Histogram('hist', 'price');
+        $agg->setFixedInterval(10);
         $agg->setMinimumDocumentCount(0); // should return empty buckets
 
         $query = new Query();

diff --git a/tests/Aggregation/SerialDiffTest.php b/tests/Aggregation/SerialDiffTest.php
@@ -20,7 +20,8 @@ class SerialDiffTest extends BaseAggregationTest
      */
     public function testSerialDiffAggregation(): void
     {
-        $dateHistogramAggregation = new DateHistogram('measurements', 'measured_at', 'hour');
+        $dateHistogramAggregation = new DateHistogram('measurements', 'measured_at');
+        $dateHistogramAggregation->setFixedInterval('hour');
 
         $dateHistogramAggregation
             ->addAggregation((new Max('max_value'))->setField('value'))

diff --git a/tests/Aggregation/StatsBucketTest.php b/tests/Aggregation/StatsBucketTest.php
@@ -21,7 +21,8 @@ public function testStatBucketAggregation(): void
     {
         $bucketScriptAggregation = new StatsBucket('result', 'age_groups>max_weight');
 
-        $histogramAggregation = new Histogram('age_groups', 'age', 10);
+        $histogramAggregation = new Histogram('age_groups', 'age');
+        $histogramAggregation->setFixedInterval(10);
 
         $histogramAggregation->addAggregation((new Max('max_weight'))->setField('weight'));