Skip to content

Commit

Permalink
Fixed broken indexes on deletion
Browse files Browse the repository at this point in the history
  • Loading branch information
remorhaz committed Feb 18, 2024
1 parent 77e7427 commit 3b932fc
Show file tree
Hide file tree
Showing 6 changed files with 262 additions and 11 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
- Added support for PHP 8.3.
### Removed
- Dropped support for PHP 8.0.
### Fixed
- Broken array indexes on deletion of non-last element.

## [0.8.0] - 2023-06-17
### Removed
Expand Down
2 changes: 1 addition & 1 deletion composer.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"ext-intl": "*",
"ext-json": "*",
"remorhaz/php-unilex": "^0.5.3",
"remorhaz/php-json-data": "^0.6.1",
"remorhaz/php-json-data": "^0.7",
"nikic/php-parser": "^4.12 || ^5"
},
"require-dev": {
Expand Down
131 changes: 131 additions & 0 deletions src/Processor/Mutator/DeleteMutation.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,16 @@
namespace Remorhaz\JSON\Path\Processor\Mutator;

use Iterator;
use Remorhaz\JSON\Data\Event\AfterElementEventInterface;
use Remorhaz\JSON\Data\Event\ElementEventInterface;
use Remorhaz\JSON\Data\Event\EventInterface;
use Remorhaz\JSON\Data\Path\Path;
use Remorhaz\JSON\Data\Path\PathInterface;
use Remorhaz\JSON\Data\Walker\MutationInterface;
use Remorhaz\JSON\Data\Walker\ValueWalkerInterface;
use WeakMap;

use function array_key_last;
use function array_values;

final class DeleteMutation implements MutationInterface
Expand All @@ -19,9 +24,21 @@ final class DeleteMutation implements MutationInterface
*/
private array $paths;

/**
* @var list<PathInterface>
*/
private array $parentsOfDeletedIndexes = [];

/**
* @var WeakMap<PathInterface, int>
*/
private WeakMap $countsOfDeletedElements;

public function __construct(PathInterface ...$paths)
{
$this->paths = array_values($paths);
/** @var WeakMap<PathInterface, int> */
$this->countsOfDeletedElements = new WeakMap();
}

/**
Expand All @@ -36,6 +53,7 @@ public function __invoke(EventInterface $event, ValueWalkerInterface $valueWalke

public function reset(): void
{
$this->parentsOfDeletedIndexes = [];
}

/**
Expand All @@ -45,10 +63,123 @@ public function reset(): void
private function createEventGenerator(EventInterface $event): Iterator
{
foreach ($this->paths as $path) {
if ($path->equals($event->getPath())) {
if ($event instanceof AfterElementEventInterface) {
$elementParentPath = $this->getParentOfDeletedElement($event->getPath());
$this->registerDeletedElement($elementParentPath);
}
return;
}
if ($path->contains($event->getPath())) {
return;
}
}

$indexesToReplace = $this->getIndexesToReplace($event->getPath());
if (!empty($indexesToReplace)) {
$lastIndex = $this->getLastPathIndex($event->getPath());
$newPath = $this->replaceElementIndexes($event->getPath(), $indexesToReplace);
$event = $event instanceof ElementEventInterface
? $event->with(
path: $newPath,
index: $indexesToReplace[$lastIndex] ?? null,
)
: $event->with(
path: $newPath,
);
}

yield $event;
}

private function getLastPathIndex(PathInterface $path): int
{
$pathElements = $path->getElements();
$lastIndex = array_key_last($pathElements);

return $lastIndex ?? throw new Exception\InvalidElementIndexException(null);
}

/**
* @param PathInterface $path
* @return array<int, int>
*/
private function getIndexesToReplace(PathInterface $path): array
{
$indexesToReplace = [];
foreach ($this->parentsOfDeletedIndexes as $elementParentPath) {
if ($elementParentPath->equals($path)) {
continue;
}
if ($elementParentPath->contains($path)) {
[$indexOffset, $indexValue] = $this->getElementIndexWithOffset($path, $elementParentPath);
$indexesToReplace[$indexOffset] = $indexValue - $this->getCountOfDeletedElements($elementParentPath);
}
}

return $indexesToReplace;
}

/**
* @param PathInterface $path
* @param array<int, int> $indexesToReplace
* @return PathInterface
*/
private function replaceElementIndexes(PathInterface $path, array $indexesToReplace): PathInterface
{
foreach ($indexesToReplace as $indexOffset => $newIndex) {
$path = $this->replaceElementIndex($path, $indexOffset, $newIndex);
}

return $path;
}

/**
* @param PathInterface $path
* @param PathInterface $elementParentPath
* @return array{int, int}
*/
private function getElementIndexWithOffset(PathInterface $path, PathInterface $elementParentPath): array
{
$indexOffset = \count($elementParentPath->getElements());
$pathElements = $path->getElements();
$index = $pathElements[$indexOffset] ?? null;

return [
$indexOffset,
\is_int($index)
? $index
: throw new Exception\InvalidElementIndexException($index),
];
}

private function replaceElementIndex(PathInterface $path, int $indexOffset, int $newIndex): PathInterface
{
$pathElements = $path->getElements();
$pathElements[$indexOffset] = $newIndex;

return new Path(...$pathElements);
}

private function registerDeletedElement(PathInterface $parentPath): void
{
$this->countsOfDeletedElements[$parentPath] = $this->getCountOfDeletedElements($parentPath) + 1;
}

private function getCountOfDeletedElements(PathInterface $parentPath): int
{
return $this->countsOfDeletedElements[$parentPath] ?? 0;
}

private function getParentOfDeletedElement(PathInterface $elementPath): PathInterface
{
$parentPath = $elementPath->copyParent();
foreach ($this->parentsOfDeletedIndexes as $addedParentPath) {
if ($parentPath->equals($addedParentPath)) {
return $addedParentPath;
}
}

return $this->parentsOfDeletedIndexes[] = $parentPath;
}
}
27 changes: 27 additions & 0 deletions src/Processor/Mutator/Exception/InvalidElementIndexException.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
<?php

declare(strict_types=1);

namespace Remorhaz\JSON\Path\Processor\Mutator\Exception;

use Throwable;
use UnexpectedValueException;

final class InvalidElementIndexException extends UnexpectedValueException implements ExceptionInterface
{
public function __construct(
private readonly ?string $index,
?Throwable $previous = null,
) {
$indexText = isset($this->index) ? "'$this->index'" : 'NULL';
parent::__construct(
message: "Element index is not an integer: $indexText",
previous: $previous,
);
}

public function getIndex(): ?string
{
return $this->index;
}
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
<?php

declare(strict_types=1);

namespace Remorhaz\JSON\Path\Test\Processor\Mutator\Exception;

use Exception;
use PHPUnit\Framework\Attributes\CoversClass;
use PHPUnit\Framework\TestCase;
use Remorhaz\JSON\Path\Processor\Mutator\Exception\InvalidElementIndexException;

#[CoversClass(InvalidElementIndexException::class)]
class InvalidElementIndexExceptionTest extends TestCase
{
public function testGetMessage_ConstructedWithString_ReturnsMatchingResult(): void
{
$exception = new InvalidElementIndexException('a');
self::assertSame(
'Element index is not an integer: \'a\'',
$exception->getMessage(),
);
}

public function testGetMessage_ConstructedWithNull_ReturnsMatchingResult(): void
{
$exception = new InvalidElementIndexException(null);
self::assertSame(
'Element index is not an integer: NULL',
$exception->getMessage(),
);
}

public function testGetIndex_Constructed_ReturnsMatchingResult(): void
{
$exception = new InvalidElementIndexException('a');
self::assertSame('a', $exception->getIndex());
}

public function testGetPrevious_ConstructedWithoutPrevious_ReturnsNull(): void
{
$exception = new InvalidElementIndexException('a');
self::assertNull($exception->getPrevious());
}

public function testGetPrevious_ConstructedWithPrevious_ReturnsSameInstance(): void
{
$previous = new Exception();
$exception = new InvalidElementIndexException('a', $previous);
self::assertSame($previous, $exception->getPrevious());
}
}
60 changes: 50 additions & 10 deletions tests/Processor/ProcessorTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,19 @@
use Remorhaz\JSON\Data\Value\EncodedJson\NodeValueFactory;
use Remorhaz\JSON\Path\Processor\Exception\IndefiniteQueryException;
use Remorhaz\JSON\Path\Processor\Exception\QueryNotAddressableException;
use Remorhaz\JSON\Path\Processor\Mutator\DeleteMutation;
use Remorhaz\JSON\Path\Processor\Mutator\Exception\ReplaceAtNestedPathsException;
use Remorhaz\JSON\Path\Processor\Mutator\ReplaceMutation;
use Remorhaz\JSON\Path\Processor\Processor;
use Remorhaz\JSON\Path\Query\QueryFactory;

#[CoversClass(Processor::class)]
#[
CoversClass(Processor::class),
CoversClass(DeleteMutation::class),
CoversClass(ReplaceMutation::class),
]
class ProcessorTest extends TestCase
{
/**
* @dataProvider providerSelect
*/
#[DataProvider('providerSelect')]
public function testSelect_GivenQueryAndData_ReturnsMatchingData(
string $json,
Expand All @@ -28,7 +31,7 @@ public function testSelect_GivenQueryAndData_ReturnsMatchingData(
): void {
$actualValue = Processor::create()->select(
QueryFactory::create()->createQuery($path),
NodeValueFactory::create()->createValue($json)
NodeValueFactory::create()->createValue($json),
);
self::assertSame($expectedValue, $actualValue->encode());
}
Expand Down Expand Up @@ -157,14 +160,51 @@ public function testDelete_NonAddressableQuery_ThrowsException(): void
$processor->delete($query, $rootValue);
}

public function testDelete_NonRootAddressableQuery_ResultContainsMatchingData(): void
{
/**
* @param string $path
* @param string $json
* @param string $expectedValue
* @return void
*
*/
#[DataProvider('providerDelete')]
public function testDelete_NonRootAddressableQuery_ResultContainsMatchingData(
string $path,
string $json,
string $expectedValue,
): void {
$processor = Processor::create();
$query = QueryFactory::create()->createQuery('$..a');
$rootValue = NodeValueFactory::create()->createValue('{"a":1,"b":{"a":2,"c":3}}');
$query = QueryFactory::create()->createQuery($path);
$rootValue = NodeValueFactory::create()->createValue($json);

$result = $processor->delete($query, $rootValue);
self::assertSame('{"b":{"c":3}}', $result->encode());
self::assertSame($expectedValue, $result->encode());
}

public static function providerDelete(): iterable
{
return [
'Several properties on different levels' => [
'$..a',
'{"a":1,"b":{"a":2,"c":3}}',
'{"b":{"c":3}}',
],
'First array element' => [
'$[0]',
'["a","b"]',
'["b"]',
],
'Several array elements' => [
'$[0,2]',
'["a","b","c","d"]',
'["b","d"]',
],
'Nested array elements' => [
'$..*[1]',
'{"a":["b","c",{"d":[1,2,3]}]}',
'{"a":["b",{"d":[1,3]}]}',
],
];
}

public function testDelete_RootQuery_ResultNotExists(): void
Expand Down

0 comments on commit 3b932fc

Please sign in to comment.