Skip to content

Commit

Permalink
Deprecate using "interval" configuration for date-histograms aggregation
Browse files Browse the repository at this point in the history
  • Loading branch information
@romainneutron
romainneutron committed Nov 23, 2020
1 parent a323450 commit 2831bd5
Show file tree
Hide file tree
Showing 8 changed files with 187 additions and 18 deletions.
4 changes: 3 additions & 1 deletion CHANGELOG.md
View file
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down
84 changes: 83 additions & 1 deletion src/Aggregation/DateHistogram.php
View file
Expand Up @@ -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.
*
Expand Down Expand Up @@ -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);
}
}
8 changes: 4 additions & 4 deletions src/QueryBuilder/DSL/Aggregation.php
View file
Expand Up @@ -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
{
Expand All @@ -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);
}
Expand Down
5 changes: 4 additions & 1 deletion tests/Aggregation/BucketSelectorTest.php
View file
Expand Up @@ -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');
Expand Down
92 changes: 84 additions & 8 deletions tests/Aggregation/DateHistogramTest.php
View file
Expand Up @@ -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);
Expand All @@ -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',
],
];

Expand All @@ -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');
Expand All @@ -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',
],
];

Expand Down
5 changes: 4 additions & 1 deletion tests/Aggregation/DerivativeTest.php
View file
Expand Up @@ -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');
Expand Down
5 changes: 4 additions & 1 deletion tests/Aggregation/SerialDiffTest.php
View file
Expand Up @@ -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'))
Expand Down
2 changes: 1 addition & 1 deletion tests/QueryBuilder/DSL/AggregationTest.php
View file
Expand Up @@ -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')]);
Expand Down

0 comments on commit 2831bd5

Please sign in to comment.