Skip to content

Commit f2fec55

Browse files
MrkishaMrkisha
authored
Merge pull request palicao#3 from palicao/fix-multifilter-bug
fix bug found by Mrkisha when passing multiple filters to several methods
2 parents 6cad157 + c420a8f commit f2fec55

File tree

5 files changed

+128
-14
lines changed

5 files changed

+128
-14
lines changed

‎README.md

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -109,8 +109,7 @@ to limit the retrieved amount of samples (passing `$count`), and also to pre-agg
109109

110110
See https://oss.redislabs.com/redistimeseries/commands/#tsrange.
111111

112-
### `multiRange`
113-
### `multiRangeRaw`
112+
### `multiRange` and `multiRangeRaw`
114113

115114
`TimeSeries::multiRange(Filter $filter, ?DateTimeInterface $from = null, ?DateTimeInterface $to = null, ?int $count = null, ?AggregationRule $rule = null): Sample[]`
116115
`TimeSeries::multiRangeRaw(Filter $filter, ?DateTimeInterface $from = null, ?DateTimeInterface $to = null, ?int $count = null, ?AggregationRule $rule = null): array[]`
@@ -175,3 +174,6 @@ label1 = value1 and where label2 is one of 'a', 'b' or 'c'.
175174
## Local testing
176175
For local testing you can use the provided `docker-compose.yml` file, which will create a PHP container (with the redis
177176
extension pre-installed), and a redis container (with Redis Time Series included).
177+
178+
## Contributors
179+
Thanks [Mrkisha](https://github.com/Mrkisha) for the precious contributions.

‎src/Filter.php

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -60,7 +60,10 @@ public function add(string $label, int $operation, $value = null): self
6060
return $this;
6161
}
6262

63-
public function toRedisParams(): string
63+
/**
64+
* @return string[]
65+
*/
66+
public function toRedisParams(): array
6467
{
6568
$params = [];
6669
foreach ($this->filters as $filter) {
@@ -85,6 +88,6 @@ public function toRedisParams(): string
8588
break;
8689
}
8790
}
88-
return implode(' ', $params);
91+
return $params;
8992
}
9093
}

‎src/TimeSeries.php

Lines changed: 25 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,8 @@
44
namespace Palicao\PhpRedisTimeSeries;
55

66
use DateTimeInterface;
7-
use RedisException;
87
use Palicao\PhpRedisTimeSeries\Exception\RedisClientException;
8+
use RedisException;
99

1010
class TimeSeries
1111
{
@@ -274,11 +274,22 @@ public function multiRange(
274274
return $samples;
275275
}
276276

277-
public function multiRangeRaw(Filter $filter,
278-
?DateTimeInterface $from = null,
279-
?DateTimeInterface $to = null,
280-
?int $count = null,
281-
?AggregationRule $rule = null
277+
/**
278+
* @param Filter $filter
279+
* @param DateTimeInterface|null $from
280+
* @param DateTimeInterface|null $to
281+
* @param int|null $count
282+
* @param AggregationRule|null $rule
283+
* @return array The row result from Redis (including labels)
284+
* @throws RedisClientException
285+
* @throws RedisException
286+
*/
287+
public function multiRangeRaw(
288+
Filter $filter,
289+
?DateTimeInterface $from = null,
290+
?DateTimeInterface $to = null,
291+
?int $count = null,
292+
?AggregationRule $rule = null
282293
): array
283294
{
284295
$fromTs = $from ? (int)$from->format('Uu') / 1000 : '-';
@@ -293,7 +304,8 @@ public function multiRangeRaw(Filter $filter,
293304
return $this->redis->executeCommand(array_merge(
294305
$params,
295306
$this->getAggregationParams($rule),
296-
['FILTER', $filter->toRedisParams()]
307+
['FILTER'],
308+
$filter->toRedisParams()
297309
));
298310
}
299311

@@ -319,7 +331,9 @@ public function getLastSample(string $key): Sample
319331
*/
320332
public function getLastSamples(Filter $filter): array
321333
{
322-
$results = $this->redis->executeCommand(['TS.MGET', 'FILTER', $filter->toRedisParams()]);
334+
$results = $this->redis->executeCommand(
335+
array_merge(['TS.MGET', 'FILTER'], $filter->toRedisParams())
336+
);
323337
$samples = [];
324338
foreach ($results as $result) {
325339
$samples[] = Sample::createFromTimestamp($result[0], (float)$result[3], (int)$result[2]);
@@ -360,7 +374,9 @@ public function info(string $key): Metadata
360374
*/
361375
public function getKeysByFilter(Filter $filter): array
362376
{
363-
return $this->redis->executeCommand(['TS.QUERYINDEX', $filter->toRedisParams()]);
377+
return $this->redis->executeCommand(
378+
array_merge(['TS.QUERYINDEX'], $filter->toRedisParams())
379+
);
364380
}
365381

366382
private function getRetentionParams(?int $retentionMs = null): array

‎tests/Integration/IntegrationTest.php

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
use DateTimeImmutable;
99
use Palicao\PhpRedisTimeSeries\AggregationRule;
10+
use Palicao\PhpRedisTimeSeries\Filter;
1011
use Palicao\PhpRedisTimeSeries\Label;
1112
use Palicao\PhpRedisTimeSeries\RedisClient;
1213
use Palicao\PhpRedisTimeSeries\RedisConnectionParams;
@@ -57,4 +58,96 @@ public function testAddAndRetrieveAsRange(): void
5758

5859
$this->assertEquals($expectedRange, $range);
5960
}
61+
62+
public function testAddAndRetrieveAsMultiRangeWithMultipleFilters(): void
63+
{
64+
$from = new DateTimeImmutable('2019-11-06 20:34:17.103000');
65+
$to = new DateTimeImmutable('2019-11-06 20:34:17.107000');
66+
67+
$this->sut->create(
68+
'temperature:3:11',
69+
6000,
70+
[new Label('sensor_id', '2'), new Label('area_id', '32')]
71+
);
72+
$this->sut->add(new Sample('temperature:3:11', 30, $from));
73+
$this->sut->add(new Sample('temperature:3:11', 42, $to));
74+
75+
$filter = new Filter('sensor_id', '2');
76+
$filter->add('area_id', Filter::OP_EQUALS, '32');
77+
78+
$range = $this->sut->multiRange($filter);
79+
80+
$expectedRange = [
81+
new Sample('temperature:3:11', 30, new DateTimeImmutable('2019-11-06 20:34:17.103000')),
82+
new Sample('temperature:3:11', 42, new DateTimeImmutable('2019-11-06 20:34:17.107000'))
83+
];
84+
85+
$this->assertEquals($expectedRange, $range);
86+
}
87+
88+
public function testAddAndRetrieveAsLastSamplesWithMultipleFilters(): void
89+
{
90+
$from = new DateTimeImmutable('2019-11-06 20:34:17.103000');
91+
$to = new DateTimeImmutable('2019-11-06 20:34:17.107000');
92+
93+
$this->sut->create(
94+
'temperature:3:11',
95+
6000,
96+
[new Label('sensor_id', '2'), new Label('area_id', '32')]
97+
);
98+
$this->sut->add(new Sample('temperature:3:11', 30, $from));
99+
$this->sut->add(new Sample('temperature:3:11', 42, $to));
100+
101+
$this->sut->create(
102+
'temperature:3:12',
103+
6000,
104+
[new Label('sensor_id', '2'), new Label('area_id', '32')]
105+
);
106+
$this->sut->add(new Sample('temperature:3:12', 30, $from));
107+
$this->sut->add(new Sample('temperature:3:12', 42, $to));
108+
109+
$filter = new Filter('sensor_id', '2');
110+
$filter->add('area_id', Filter::OP_EQUALS, '32');
111+
112+
$range = $this->sut->getLastSamples($filter);
113+
114+
$expectedResult = [
115+
new Sample('temperature:3:11', 42, new DateTimeImmutable('2019-11-06 20:34:17.107000')),
116+
new Sample('temperature:3:12', 42, new DateTimeImmutable('2019-11-06 20:34:17.107000'))
117+
];
118+
119+
$this->assertEquals($expectedResult, $range);
120+
}
121+
122+
public function testAddAndRetrieveKeysWithMultipleFilters(): void
123+
{
124+
$from = new DateTimeImmutable('2019-11-06 20:34:17.103000');
125+
$to = new DateTimeImmutable('2019-11-06 20:34:17.107000');
126+
127+
$this->sut->create(
128+
'temperature:3:11',
129+
6000,
130+
[new Label('sensor_id', '2'), new Label('area_id', '32')]
131+
);
132+
$this->sut->add(new Sample('temperature:3:11', 30, $from));
133+
$this->sut->add(new Sample('temperature:3:11', 42, $to));
134+
135+
$this->sut->create(
136+
'temperature:3:12',
137+
6000,
138+
[new Label('sensor_id', '2'), new Label('area_id', '32')]
139+
);
140+
$this->sut->add(new Sample('temperature:3:12', 30, $from));
141+
$this->sut->add(new Sample('temperature:3:12', 42, $to));
142+
143+
$filter = new Filter('sensor_id', '2');
144+
$filter->add('area_id', Filter::OP_EQUALS, '32');
145+
146+
$range = $this->sut->getKeysByFilter($filter);
147+
148+
$expectedResult = ['temperature:3:11', 'temperature:3:12'];
149+
150+
$this->assertEquals($expectedResult, $range);
151+
}
152+
60153
}

‎tests/Unit/FilterTest.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -68,7 +68,7 @@ public function testToRedisParams(): void
6868
$filter->add('lab6', Filter::OP_NOT_IN, ['d', 'e', 'f']);
6969

7070
$result = $filter->toRedisParams();
71-
$expected = 'lab1=val1 lab2!=val2 lab3= lab4!= lab5=(a,b,c) lab6!=(d,e,f)';
71+
$expected = ['lab1=val1', 'lab2!=val2', 'lab3=', 'lab4!=', 'lab5=(a,b,c)', 'lab6!=(d,e,f)'];
7272

7373
$this->assertEquals($expected, $result);
7474
}

0 commit comments

Comments
 (0)