Skip to content

Commit ef75da9

Browse files
MacFJAMacFJA
committed
Fix APPLY option of Aggregate command
1 parent 2960011 commit ef75da9

File tree

7 files changed

+120
-16
lines changed

7 files changed

+120
-16
lines changed

‎CHANGELOG.md‎

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,15 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
1010

1111
- `reset` method for builder (`\MacFJA\RediSearch\IndexBuilder` and `\MacFJA\RediSearch\Query\Builder`)
1212
- Allow PHP 8.0
13+
- (dev) Allow `podman` (in addition to `docker`) as container runner for integration test
14+
15+
### Changed
16+
17+
- `APPLY` option of Aggregate command is now dependent of the last inserted option (if the option have a meaning for `APPLY`)
18+
19+
### Fixed
20+
21+
- Fix the `APPLY` option of the Aggregate command
1322

1423
## [2.0.1]
1524

‎composer.json‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -33,13 +33,13 @@
3333
"php-parallel-lint/php-parallel-lint": "^1.3",
3434
"phpmd/phpmd": "^2.10",
3535
"phpstan/phpstan": "^0.12.92",
36-
"phpunit/phpunit": "^8.5",
36+
"phpunit/phpunit": "^8.5 || ^9.3",
3737
"predis/predis": "^1.1",
3838
"ptrofimov/tinyredisclient": "^1.1",
3939
"redisent/redisent": "dev-master",
4040
"roave/security-advisories": "dev-latest",
4141
"rskuipers/php-assumptions": "^0.8.0",
42-
"sebastian/phpcpd": "^4.1",
42+
"sebastian/phpcpd": "^4.1 || ^6.0",
4343
"ukko/phpredis-phpdoc": "dev-master",
4444
"vimeo/psalm": "^4.7"
4545
},
@@ -54,11 +54,6 @@
5454
"ptrofimov/tinyredisclient": "To use TinyRedisClient implementation",
5555
"redisent/redisent": "To use Redisent implementation"
5656
},
57-
"config": {
58-
"platform": {
59-
"php": "7.2"
60-
}
61-
},
6257
"autoload": {
6358
"psr-4": {
6459
"MacFJA\\RediSearch\\": "src/"

‎src/Redis/Command/AbstractCommand.php‎

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ public function getArguments(): array
8282
$arguments = array_filter($options, function (CommandOption $option) {
8383
return $option->isCompatible($this->rediSearchVersion) && $option->isValid();
8484
});
85+
$arguments = $this->sortArguments($arguments);
8586

8687
return array_reduce($arguments, function ($carry, CommandOption $option) {
8788
return array_merge($carry, $option->render($this->rediSearchVersion));
@@ -100,6 +101,16 @@ public function parseResponse($data)
100101
return $data;
101102
}
102103

104+
/**
105+
* @param array<CommandOption> $arguments
106+
*
107+
* @return array<CommandOption>
108+
*/
109+
protected function sortArguments(array $arguments): array
110+
{
111+
return $arguments;
112+
}
113+
103114
/**
104115
* @return array<string>
105116
*/

‎src/Redis/Command/Aggregate.php‎

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,13 +22,15 @@
2222
namespace MacFJA\RediSearch\Redis\Command;
2323

2424
use function assert;
25+
use function count;
2526
use function is_array;
2627
use MacFJA\RediSearch\Exception\UnexpectedServerResponseException;
2728
use MacFJA\RediSearch\Redis\Command\AggregateCommand\ApplyOption;
2829
use MacFJA\RediSearch\Redis\Command\AggregateCommand\GroupByOption;
2930
use MacFJA\RediSearch\Redis\Command\AggregateCommand\LimitOption;
3031
use MacFJA\RediSearch\Redis\Command\AggregateCommand\SortByOption;
3132
use MacFJA\RediSearch\Redis\Command\AggregateCommand\WithCursor;
33+
use MacFJA\RediSearch\Redis\Command\Option\CommandOption;
3234
use MacFJA\RediSearch\Redis\Command\Option\FlagOption;
3335
use MacFJA\RediSearch\Redis\Command\Option\NamedOption;
3436
use MacFJA\RediSearch\Redis\Command\Option\NamelessOption;
@@ -45,6 +47,9 @@ class Aggregate extends AbstractCommand implements PaginatedCommand
4547
{
4648
use ArrayResponseTrait;
4749

50+
/** @var CommandOption */
51+
private $lastAdded;
52+
4853
public function __construct(string $rediSearchVersion = self::MIN_IMPLEMENTED_VERSION)
4954
{
5055
parent::__construct([
@@ -64,6 +69,7 @@ public function __construct(string $rediSearchVersion = self::MIN_IMPLEMENTED_VE
6469
public function setIndex(string $index): self
6570
{
6671
$this->options['index']->setValue($index);
72+
$this->lastAdded = $this->options['index'];
6773

6874
return $this;
6975
}
@@ -76,13 +82,15 @@ public function getIndex(): string
7682
public function setQuery(string $query): self
7783
{
7884
$this->options['query']->setValue($query);
85+
$this->lastAdded = $this->options['query'];
7986

8087
return $this;
8188
}
8289

8390
public function setVerbatim(bool $active = true): self
8491
{
8592
$this->options['verbatim']->setActive($active);
93+
$this->lastAdded = $this->options['verbatim'];
8694

8795
return $this;
8896
}
@@ -125,6 +133,8 @@ public function addApply(string $expression, string $alias): self
125133
->setDataOfOption('expression', $expression)
126134
->setDataOfOption('alias', $alias)
127135
;
136+
$apply->setParent($this->lastAdded);
137+
$this->lastAdded = $apply;
128138
$this->options['apply'][] = $apply;
129139

130140
return $this;
@@ -133,13 +143,15 @@ public function addApply(string $expression, string $alias): self
133143
public function addGroupBy(GroupByOption $group): self
134144
{
135145
$this->options['groupby'][] = $group;
146+
$this->lastAdded = $group;
136147

137148
return $this;
138149
}
139150

140151
public function setLoad(string ...$field): self
141152
{
142153
$this->options['load']->setArguments($field);
154+
$this->lastAdded = $this->options['load'];
143155

144156
return $this;
145157
}
@@ -215,6 +227,27 @@ public function parseResponse($data)
215227
return new PaginatedResponse($this, $totalCount, $items);
216228
}
217229

230+
protected function sortArguments(array $arguments): array
231+
{
232+
/** @var array<ApplyOption> $applies */
233+
$applies = array_filter($arguments, static function (CommandOption $option) {
234+
return $option instanceof ApplyOption;
235+
});
236+
$withoutApplies = array_values(array_filter($arguments, static function (CommandOption $option) {
237+
return !($option instanceof ApplyOption);
238+
}));
239+
for ($index = count($withoutApplies); $index > 0; --$index) {
240+
$item = $withoutApplies[$index - 1];
241+
foreach ($applies as $apply) {
242+
if ($apply->getParent() === $item || (null === $apply->getParent() && 1 === $index)) {
243+
array_splice($withoutApplies, $index, 0, [$apply]);
244+
}
245+
}
246+
}
247+
248+
return $withoutApplies;
249+
}
250+
218251
protected function getRequiredOptions(): array
219252
{
220253
return ['index', 'query'];

‎src/Redis/Command/AggregateCommand/ApplyOption.php‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,16 +21,30 @@
2121

2222
namespace MacFJA\RediSearch\Redis\Command\AggregateCommand;
2323

24+
use MacFJA\RediSearch\Redis\Command\Option\CommandOption;
2425
use MacFJA\RediSearch\Redis\Command\Option\GroupedOption;
2526
use MacFJA\RediSearch\Redis\Command\Option\NamedOption;
2627

2728
class ApplyOption extends GroupedOption
2829
{
30+
/** @var null|CommandOption */
31+
private $parent;
32+
2933
public function __construct()
3034
{
3135
parent::__construct([
3236
'expression' => new NamedOption('APPLY', null, '>=2.0.0'),
3337
'alias' => new NamedOption('AS', null, '>=2.0.0'),
3438
], ['expression'], [], '>=2.0.0');
3539
}
40+
41+
public function setParent(?CommandOption $parent): void
42+
{
43+
$this->parent = $parent;
44+
}
45+
46+
public function getParent(): ?CommandOption
47+
{
48+
return $this->parent;
49+
}
3650
}

‎tests/Redis/Command/AggregateTest.php‎

Lines changed: 35 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -138,8 +138,42 @@ public function testFullOption(): void
138138
'VERBATIM',
139139
'LOAD', 2, '@geo1', '@tag1',
140140
'GROUPBY', 1, '@text1', 'REDUCE', 'COUNT_DISTINCT', 1, '@user_id', 'REDUCE', 'AVG', 1, '@num1', 'AS', 'average',
141-
'SORTBY', 3, '@text1', '@num1', 'DESC', 'MAX', 10,
142141
'APPLY', '@timestamp - (@timestamp % 86400)', 'AS', 'day',
142+
'SORTBY', 3, '@text1', '@num1', 'DESC', 'MAX', 10,
143+
'LIMIT', 12, 35,
144+
'FILTER', "@name=='foo' && @age < 20",
145+
'WITHCURSOR', 'COUNT', 20, 'MAXIDLE', 30,
146+
], $command->getArguments());
147+
}
148+
149+
public function testApplyOrder(): void
150+
{
151+
$command = new Aggregate();
152+
$command
153+
->setIndex('idx')
154+
->setQuery('@text1:"hello world"')
155+
->setVerbatim()
156+
->setLoad('@geo1', '@tag1')
157+
->addApply('split(@tag1)', 'tag2')
158+
->addGroupBy(new GroupByOption(['@text1'], [ReduceOption::countDistinct('user_id'), ReduceOption::average('num1', 'average')]))
159+
->addFilter("@name=='foo' && @age < 20")
160+
->addApply('@age - 1', 'age')
161+
->addSortBy('@text1')
162+
->addSortBy('@num1', 'DESC')
163+
->setSortByMax(10)
164+
->setLimit(12, 35)
165+
->setWithCursor(20, 30)
166+
;
167+
168+
static::assertSame([
169+
'idx',
170+
'@text1:"hello world"',
171+
'VERBATIM',
172+
'LOAD', 2, '@geo1', '@tag1',
173+
'APPLY', 'split(@tag1)', 'AS', 'tag2',
174+
'GROUPBY', 1, '@text1', 'REDUCE', 'COUNT_DISTINCT', 1, '@user_id', 'REDUCE', 'AVG', 1, '@num1', 'AS', 'average',
175+
'APPLY', '@age - 1', 'AS', 'age',
176+
'SORTBY', 3, '@text1', '@num1', 'DESC', 'MAX', 10,
143177
'LIMIT', 12, 35,
144178
'FILTER', "@name=='foo' && @age < 20",
145179
'WITHCURSOR', 'COUNT', 20, 'MAXIDLE', 30,

‎tests/integration/DockerTest.php‎

Lines changed: 16 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
use Closure;
2727
use Credis_Client;
2828
use function get_class;
29+
use function is_string;
2930
use MacFJA\RediSearch\Index;
3031
use MacFJA\RediSearch\IndexBuilder;
3132
use MacFJA\RediSearch\Query\Builder;
@@ -110,38 +111,45 @@ class DockerTest extends TestCase
110111
{
111112
/** @var bool */
112113
private static $skip = false;
114+
/** @var string */
115+
private static $containerCommand = 'podman';
116+
/** @var null|string */
117+
private static $containerId;
113118

114119
public static function setUpBeforeClass(): void
115120
{
116121
parent::setUpBeforeClass();
117-
exec('which docker', $output, $code);
122+
exec('which podman', $output, $code);
123+
if ($code > 0) {
124+
exec('which docker', $output, $code);
125+
static::$containerCommand = 'docker';
126+
}
118127
if ($code > 0) {
119128
static::$skip = true;
120129

121130
return;
122131
}
123-
exec('docker run --rm -p 16379:6379 -d redislabs/redisearch:latest');
132+
$output = [];
133+
exec(static::$containerCommand.' run --rm -p 16379:6379 -d redislabs/redisearch:latest', $output, $code);
134+
static::$containerId = reset($output) ?: null;
124135
Client\AbstractClient::$disableNotice = true;
125136
}
126137

127138
public static function tearDownAfterClass(): void
128139
{
129140
parent::tearDownAfterClass();
130-
if (static::$skip) {
141+
if (static::$skip || !is_string(static::$containerId)) {
131142
return;
132143
}
133-
exec('docker ps --filter publish=16379 -q', $output);
134-
foreach ($output as $container) {
135-
exec('docker stop '.escapeshellarg($container), $output);
136-
}
144+
exec(static::$containerCommand.' stop '.escapeshellarg(static::$containerId), $output);
137145
Client\AbstractClient::$disableNotice = false;
138146
}
139147

140148
protected function setUp(): void
141149
{
142150
parent::setUp();
143151
if (static::$skip) {
144-
static::markTestSkipped('Docker is missing');
152+
static::markTestSkipped('Podman/Docker is missing');
145153
}
146154
}
147155

0 commit comments

Comments
 (0)