diff --git a/CHANGELOG.md b/CHANGELOG.md index b8284e2..6edcb4d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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 diff --git a/composer.json b/composer.json index f1846a9..93593d0 100644 --- a/composer.json +++ b/composer.json @@ -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": { diff --git a/src/Processor/Mutator/DeleteMutation.php b/src/Processor/Mutator/DeleteMutation.php index 1a63a8b..0bec889 100644 --- a/src/Processor/Mutator/DeleteMutation.php +++ b/src/Processor/Mutator/DeleteMutation.php @@ -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 @@ -19,9 +24,21 @@ final class DeleteMutation implements MutationInterface */ private array $paths; + /** + * @var list + */ + private array $parentsOfDeletedIndexes = []; + + /** + * @var WeakMap + */ + private WeakMap $countsOfDeletedElements; + public function __construct(PathInterface ...$paths) { $this->paths = array_values($paths); + /** @var WeakMap */ + $this->countsOfDeletedElements = new WeakMap(); } /** @@ -36,6 +53,7 @@ public function __invoke(EventInterface $event, ValueWalkerInterface $valueWalke public function reset(): void { + $this->parentsOfDeletedIndexes = []; } /** @@ -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 + */ + 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 $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; + } } diff --git a/src/Processor/Mutator/Exception/InvalidElementIndexException.php b/src/Processor/Mutator/Exception/InvalidElementIndexException.php new file mode 100644 index 0000000..0b8b30a --- /dev/null +++ b/src/Processor/Mutator/Exception/InvalidElementIndexException.php @@ -0,0 +1,27 @@ +index) ? "'$this->index'" : 'NULL'; + parent::__construct( + message: "Element index is not an integer: $indexText", + previous: $previous, + ); + } + + public function getIndex(): ?string + { + return $this->index; + } +} diff --git a/tests/Processor/Mutator/Exception/InvalidElementIndexExceptionTest.php b/tests/Processor/Mutator/Exception/InvalidElementIndexExceptionTest.php new file mode 100644 index 0000000..e7e8d55 --- /dev/null +++ b/tests/Processor/Mutator/Exception/InvalidElementIndexExceptionTest.php @@ -0,0 +1,51 @@ +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()); + } +} diff --git a/tests/Processor/ProcessorTest.php b/tests/Processor/ProcessorTest.php index 06b8f86..f82a7ec 100644 --- a/tests/Processor/ProcessorTest.php +++ b/tests/Processor/ProcessorTest.php @@ -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, @@ -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()); } @@ -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