Skip to content

Commit cc25dea

Browse files
remorhazremorhaz
committed
Fixed duplicating results in some filtered queries
1 parent 54a4fc4 commit cc25dea

File tree

8 files changed

+298
-2
lines changed

8 files changed

+298
-2
lines changed

‎CHANGELOG.md‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
99
- Order of array elements in result corresponds to order of indexes in filter.
1010
- Order of object properties in result corresponds to order of properties in filter.
1111
- Slices with negative steps are returned in reversed order.
12-
- Names now allow hyphen symbol (`U+002D`) as non-starting symbol.
12+
- Names in dot-notation now allow hyphen symbol (`U+002D`) as non-starting symbol.
13+
- Fixed duplicated results in some filtered queries.
1314

1415
## [0.7.1] - 2019-02-17
1516
### Added

‎src/Runtime/Matcher/Exception/AddressNotSortableException.php‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ final class AddressNotSortableException extends DomainException implements Excep
1818
public function __construct($address, Throwable $previous = null)
1919
{
2020
$this->address = $address;
21-
parent::__construct("Index/property is not sortable: {$this->address}", $previous);
21+
parent::__construct("Index/property is not sortable: {$this->address}", 0, $previous);
2222
}
2323

2424
/**

‎src/Value/Exception/ValueInListWithAnotherOuterIndexException.php‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,51 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Remorhaz\JSON\Path\Value\Exception;
6+
7+
use LogicException;
8+
use Remorhaz\JSON\Data\Value\NodeValueInterface;
9+
use Throwable;
10+
11+
final class ValueInListWithAnotherOuterIndexException extends LogicException implements ExceptionInterface
12+
{
13+
14+
private $value;
15+
16+
private $expectedIndex;
17+
18+
private $actualIndex;
19+
20+
public function __construct(
21+
NodeValueInterface $value,
22+
int $expectedIndex,
23+
int $actualIndex,
24+
Throwable $previous = null
25+
) {
26+
$this->value = $value;
27+
$this->expectedIndex = $expectedIndex;
28+
$this->actualIndex = $actualIndex;
29+
parent::__construct($this->buildMessage(), 0, $previous);
30+
}
31+
32+
private function buildMessage(): string
33+
{
34+
return "Value is already in list with outer index {$this->expectedIndex}, not {$this->actualIndex}";
35+
}
36+
37+
public function getValue(): NodeValueInterface
38+
{
39+
return $this->value;
40+
}
41+
42+
public function getExpectedIndex(): int
43+
{
44+
return $this->expectedIndex;
45+
}
46+
47+
public function getActualIndex(): int
48+
{
49+
return $this->actualIndex;
50+
}
51+
}

‎src/Value/NodeValueListBuilder.php‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,17 @@ final class NodeValueListBuilder
1010

1111
private $outerIndexes = [];
1212

13+
/**
14+
* @var NodeValueInterface[]
15+
*/
1316
private $values = [];
1417

1518
public function addValue(NodeValueInterface $value, int $outerIndex): self
1619
{
20+
if ($this->valueExists($value, $outerIndex)) {
21+
return $this;
22+
}
23+
1724
$this->outerIndexes[] = $outerIndex;
1825
$this->values[] = $value;
1926

@@ -24,4 +31,21 @@ public function build(): NodeValueListInterface
2431
{
2532
return new NodeValueList(new IndexMap(...$this->outerIndexes), ...$this->values);
2633
}
34+
35+
private function valueExists(NodeValueInterface $value, int $outerIndex): bool
36+
{
37+
foreach ($this->values as $innerIndex => $addedValue) {
38+
if (!$addedValue->getPath()->equals($value->getPath())) {
39+
continue;
40+
}
41+
$addedOuterIndex = $this->outerIndexes[$innerIndex];
42+
if ($outerIndex == $addedOuterIndex) {
43+
return true;
44+
}
45+
46+
throw new Exception\ValueInListWithAnotherOuterIndexException($value, $addedOuterIndex, $outerIndex);
47+
}
48+
49+
return false;
50+
}
2751
}

‎tests/Parser/TranslationSchemeTest.php‎

Lines changed: 48 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -198,6 +198,22 @@ public function providerParser(): array
198198
['1'],
199199
true,
200200
],
201+
'Property of strict property list filter' => [
202+
(object) [
203+
'a' => (object) [
204+
'a' => 1,
205+
'b' => 2,
206+
'c' => 3,
207+
],
208+
'b' => 4,
209+
'c' => (object) [
210+
'a' => 5,
211+
],
212+
],
213+
'$["a", "c"].a',
214+
['1', '5'],
215+
false,
216+
],
201217
'Mixed names in bracket-notation' => [
202218
(object) ['a' => 1, 'b' => 2, '1' => 3, 'c' => 4],
203219
'$[a, "1", c]',
@@ -510,6 +526,38 @@ public function providerParser(): array
510526
[],
511527
false,
512528
],
529+
'Filter matches array element' => [
530+
(object) ['a' => [(object) ['b' => 1]]],
531+
'$.a[?(@.b==1)]',
532+
['{"b":1}'],
533+
false,
534+
],
535+
'Filter matches property' => [
536+
(object) ['a' => (object) ['b' => 1]],
537+
'$.a[?(@.b==1)]',
538+
['{"b":1}'],
539+
false,
540+
],
541+
'Filter expression after recursive descent' => [
542+
(object) [
543+
"id" => 2,
544+
"more" => [
545+
(object) ["id" => 2],
546+
(object) ["more" => (object) ["id" => 2]],
547+
(object) ["id" => (object) ["id" => 2]],
548+
[(object) ["id" => 2]],
549+
],
550+
],
551+
'$..[?(@.id==2)]',
552+
[
553+
'{"id":2,"more":[{"id":2},{"more":{"id":2}},{"id":{"id":2}},[{"id":2}]]}',
554+
'{"id":2}',
555+
'{"id":2}',
556+
'{"id":2}',
557+
'{"id":2}',
558+
],
559+
false,
560+
],
513561
'Filter with OR' => [
514562
[
515563
(object) ['a' => 1, 'b' => 2],

‎tests/Runtime/Matcher/Exception/AddressNotSortableExceptionTest.php‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,60 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Remorhaz\JSON\Path\Test\Runtime\Matcher\Exception;
6+
7+
use Exception;
8+
use PHPUnit\Framework\TestCase;
9+
use Remorhaz\JSON\Path\Runtime\Matcher\Exception\AddressNotSortableException;
10+
11+
/**
12+
* @covers \Remorhaz\JSON\Path\Runtime\Matcher\Exception\AddressNotSortableException
13+
*/
14+
class AddressNotSortableExceptionTest extends TestCase
15+
{
16+
17+
public function testGetMessage_ConstructedWithAddress_ReturnsMatchingValue(): void
18+
{
19+
$exception = new AddressNotSortableException(1);
20+
self::assertSame('Index/property is not sortable: 1', $exception->getMessage());
21+
}
22+
23+
/**
24+
* @param $address
25+
* @param $expectedValue
26+
* @dataProvider providerGetAddress
27+
*/
28+
public function testGetAddress_ConstructedWithAddress_ReturnsSameValue($address, $expectedValue): void
29+
{
30+
$exception = new AddressNotSortableException($address);
31+
self::assertSame($expectedValue, $exception->getAddress());
32+
}
33+
34+
public function providerGetAddress(): array
35+
{
36+
return [
37+
'String value' => ['a', 'a'],
38+
'Integer value' => [1, 1],
39+
];
40+
}
41+
42+
public function testGetCode_Always_ReturnsZero(): void
43+
{
44+
$exception = new AddressNotSortableException(1);
45+
self::assertSame(0, $exception->getCode());
46+
}
47+
48+
public function testGetPrevious_ConstructedWithoutPrevious_ReturnsNull(): void
49+
{
50+
$exception = new AddressNotSortableException(1);
51+
self::assertNull($exception->getPrevious());
52+
}
53+
54+
public function testGetPrevious_ConstructedWithPrevious_ReturnsSameInstance(): void
55+
{
56+
$previous = new Exception();
57+
$exception = new AddressNotSortableException(1, $previous);
58+
self::assertSame($previous, $exception->getPrevious());
59+
}
60+
}

‎tests/Value/Exception/ValueInListWithAnotherOuterIndexExceptionTest.php‎

Lines changed: 67 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,67 @@
1+
<?php
2+
3+
declare(strict_types=1);
4+
5+
namespace Remorhaz\JSON\Path\Test\Value\Exception;
6+
7+
use Exception;
8+
use PHPUnit\Framework\TestCase;
9+
use Remorhaz\JSON\Data\Value\NodeValueInterface;
10+
use Remorhaz\JSON\Path\Value\Exception\ValueInListWithAnotherOuterIndexException;
11+
12+
/**
13+
* @covers \Remorhaz\JSON\Path\Value\Exception\ValueInListWithAnotherOuterIndexException
14+
*/
15+
class ValueInListWithAnotherOuterIndexExceptionTest extends TestCase
16+
{
17+
18+
public function testGetMessage_ConstructedWithIndexes_ReturnsMatchingValue(): void
19+
{
20+
$value = $this->createMock(NodeValueInterface::class);
21+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2);
22+
self::assertSame('Value is already in list with outer index 1, not 2', $exception->getMessage());
23+
}
24+
25+
public function testGetValue_ConstructedWithValue_ReturnsSameInstance(): void
26+
{
27+
$value = $this->createMock(NodeValueInterface::class);
28+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2);
29+
self::assertSame($value, $exception->getValue());
30+
}
31+
32+
public function testGetExpectedIndex_ConstructedWithExpectedIndex_ReturnsSameValue(): void
33+
{
34+
$value = $this->createMock(NodeValueInterface::class);
35+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2);
36+
self::assertSame(1, $exception->getExpectedIndex());
37+
}
38+
39+
public function testGetActualIndex_ConstructedWithActualIndex_ReturnsSameValue(): void
40+
{
41+
$value = $this->createMock(NodeValueInterface::class);
42+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2);
43+
self::assertSame(2, $exception->getActualIndex());
44+
}
45+
46+
public function testGetCode_Always_ReturnsZero(): void
47+
{
48+
$value = $this->createMock(NodeValueInterface::class);
49+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2);
50+
self::assertSame(0, $exception->getCode());
51+
}
52+
53+
public function testGetPrevious_ConstructedWithoutPrevious_ReturnsNull(): void
54+
{
55+
$value = $this->createMock(NodeValueInterface::class);
56+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2);
57+
self::assertNull($exception->getPrevious());
58+
}
59+
60+
public function testGetPrevious_ConstructedWithPrevious_ReturnsSameInstance(): void
61+
{
62+
$previous = new Exception();
63+
$value = $this->createMock(NodeValueInterface::class);
64+
$exception = new ValueInListWithAnotherOuterIndexException($value, 1, 2, $previous);
65+
self::assertSame($previous, $exception->getPrevious());
66+
}
67+
}

‎tests/Value/NodeValueListBuilderTest.php‎

Lines changed: 45 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,9 @@
44
namespace Remorhaz\JSON\Path\Test\Value;
55

66
use PHPUnit\Framework\TestCase;
7+
use Remorhaz\JSON\Data\Path\PathInterface;
78
use Remorhaz\JSON\Data\Value\NodeValueInterface;
9+
use Remorhaz\JSON\Path\Value\Exception\ValueInListWithAnotherOuterIndexException;
810
use Remorhaz\JSON\Path\Value\NodeValueListBuilder;
911

1012
/**
@@ -47,6 +49,49 @@ public function testBuild_ValuesAdded_ReturnsListWithSameValueInstances(): void
4749
self::assertSame([$firstValue, $secondValue], $values->getValues());
4850
}
4951

52+
public function testBuild_ValueWithSamePathAddedTwiceWithSameOuterIndex_ReturnsListWithFirstInstance(): void
53+
{
54+
$builder = new NodeValueListBuilder;
55+
$path = $this->createMock(PathInterface::class);
56+
$path
57+
->method('equals')
58+
->willReturn(true);
59+
$firstValue = $this->createMock(NodeValueInterface::class);
60+
$firstValue
61+
->method('getPath')
62+
->willReturn($path);
63+
$secondValue = $this->createMock(NodeValueInterface::class);
64+
$secondValue
65+
->method('getPath')
66+
->willReturn($path);
67+
$builder->addValue($firstValue, 1);
68+
$builder->addValue($secondValue, 1);
69+
$values = $builder->build();
70+
self::assertSame([$firstValue], $values->getValues());
71+
}
72+
73+
public function testBuild_ValueWithSamePathAddedTwiceWithAnotherOuterIndex_ThrowsException(): void
74+
{
75+
$builder = new NodeValueListBuilder;
76+
$path = $this->createMock(PathInterface::class);
77+
$path
78+
->method('equals')
79+
->willReturn(true);
80+
$firstValue = $this->createMock(NodeValueInterface::class);
81+
$firstValue
82+
->method('getPath')
83+
->willReturn($path);
84+
$secondValue = $this->createMock(NodeValueInterface::class);
85+
$secondValue
86+
->method('getPath')
87+
->willReturn($path);
88+
$builder->addValue($firstValue, 1);
89+
90+
$this->expectException(ValueInListWithAnotherOuterIndexException::class);
91+
$this->expectExceptionMessage('Value is already in list with outer index 1, not 2');
92+
$builder->addValue($secondValue, 2);
93+
}
94+
5095
public function testBuild_ValueAddedWithGivenOuterIndex_ReturnsListWithSameOuterIndexInMap(): void
5196
{
5297
$builder = new NodeValueListBuilder;

0 commit comments

Comments
 (0)