From 527ab492bdb95b5b9ee4e11f152236e9018a42f5 Mon Sep 17 00:00:00 2001 From: Filip Halaxa Date: Fri, 29 Nov 2024 16:40:08 +0100 Subject: [PATCH] Fixes #117 --- src/Parser.php | 48 +++++++++++------- src/RecursiveItems.php | 15 +----- src/ResumableIteratorAggregateProxy.php | 56 --------------------- src/ResumableTokensProxy.php | 49 ++++++++++++++++++ src/Tokens.php | 9 +--- test/JsonMachineTest/ItemsTest.php | 8 +++ test/JsonMachineTest/ParserTest.php | 5 +- test/JsonMachineTest/RecursiveItemsTest.php | 17 ++----- 8 files changed, 96 insertions(+), 111 deletions(-) delete mode 100644 src/ResumableIteratorAggregateProxy.php create mode 100644 src/ResumableTokensProxy.php diff --git a/src/Parser.php b/src/Parser.php index d6f62e5..9074376 100644 --- a/src/Parser.php +++ b/src/Parser.php @@ -39,9 +39,6 @@ class Parser implements \IteratorAggregate, PositionAware /** @var Traversable */ private $tokens; - /** @var Iterator */ - private $tokensIterator; - /** @var Generator */ private $generator; @@ -89,13 +86,6 @@ public function __construct(Traversable $tokens, $jsonPointer = '', ?ItemDecoder } $this->tokens = $tokens; - if ($tokens instanceof IteratorAggregate) { - $this->tokensIterator = $tokens->getIterator(); - } elseif ($tokens instanceof Iterator) { - $this->tokensIterator = $tokens; - } else { - throw new InvalidArgumentException('$tokens must be either an instance of Iterator or IteratorAggregate.'); - } if ($jsonDecoder instanceof StringOnlyDecoder) { $this->jsonDecoder = $jsonDecoder; @@ -119,10 +109,12 @@ private function buildPaths(array $jsonPointers): array #[\ReturnTypeWillChange] public function getIterator(): Generator { - if ( ! $this->generator) { - $this->generator = $this->createGenerator(); + if ($this->generator && $this->tokens instanceof ResumableTokensProxy) { + throw new JsonMachineException('Nested RecursiveTokens cannot be iterated more than once.'); } + $this->generator = $this->createGenerator(); + return $this->generator; } @@ -156,10 +148,10 @@ private function createGenerator(): Generator $jsonPointerPath = []; $iteratorLevel = 0; - // local variables for faster name lookups - $tokens = $this->tokensIterator; + /** @var Iterator */ + $tokensIterator = $this->resolveTokensIterator(); - foreach ($tokens as $token) { + foreach ($tokensIterator as $token) { if ($currentPathChanged) { $currentPathChanged = false; $jsonPointerPath = $this->getMatchingJsonPointerPath(); @@ -196,7 +188,7 @@ private function createGenerator(): Generator ) { if ($this->recursive && ($token == '{' || $token == '[')) { $jsonValue = new self( - new ResumableIteratorAggregateProxy($this->tokens), // could single shared instance work? + new ResumableTokensProxy($this->tokens, $tokensIterator), '', $this->jsonDecoder, true @@ -335,10 +327,12 @@ private function createGenerator(): Generator public function ensureIterationComplete(): void { - $generator = $this->getIterator(); + if ( ! $this->generator) { + $this->getIterator(); + } - while ($generator->valid()) { - $generator->next(); + while ($this->generator->valid()) { + $this->generator->next(); } } @@ -495,4 +489,20 @@ private function pathMatchesPointer($pathToken, string $pointerToken): bool return is_int($pathToken) && $pointerToken === '-'; } + + /** + * @throws InvalidArgumentException + */ + private function resolveTokensIterator() + { + if ($this->tokens instanceof IteratorAggregate) { + $tokensIterator = $this->tokens->getIterator(); + } elseif ($this->tokens instanceof Iterator) { + $tokensIterator = $this->tokens; + } else { + throw new InvalidArgumentException('$tokens must be either an instance of Iterator or IteratorAggregate.'); + } + + return $tokensIterator; + } } diff --git a/src/RecursiveItems.php b/src/RecursiveItems.php index 3283e04..5c99944 100644 --- a/src/RecursiveItems.php +++ b/src/RecursiveItems.php @@ -4,7 +4,6 @@ namespace JsonMachine; -use Exception; use Generator; use Iterator; use IteratorAggregate; @@ -136,7 +135,6 @@ public function valid(): bool public function rewind(): void { $this->parserIterator = toIterator($this->parser->getIterator()); - $this->parserIterator->rewind(); } public function hasChildren(): bool @@ -185,21 +183,10 @@ public function advanceToKey($key) /** * Recursively materializes this iterator level to array. * Moves its internal pointer to the end. - * - * @throws JsonMachineException */ public function toArray(): array { - try { - /** @throws Exception */ - $this->rewind(); - } catch (Exception $e) { - if (false !== strpos($e->getMessage(), 'generator')) { - throw new JsonMachineException( - 'Method toArray() can only be called before any items in the collection have been accessed.' - ); - } - } + $this->rewind(); return self::toArrayRecursive($this); } diff --git a/src/ResumableIteratorAggregateProxy.php b/src/ResumableIteratorAggregateProxy.php deleted file mode 100644 index dee5f7b..0000000 --- a/src/ResumableIteratorAggregateProxy.php +++ /dev/null @@ -1,56 +0,0 @@ -iteratorAggregate = $iteratorAggregate; - } - - public function getIterator(): \Traversable - { - $iterator = toIterator($this->iteratorAggregate->getIterator()); - while ($iterator->valid()) { - yield $iterator->key() => $iterator->current(); - $iterator->next(); - } - } - - public function __call($name, $arguments) - { - return $this->iteratorAggregate->$name(...$arguments); - } - - /** - * Returns JSON bytes read so far. - */ - public function getPosition() - { - if ($this->iteratorAggregate instanceof PositionAware) { - return $this->iteratorAggregate->getPosition(); - } - - throw new LogicException('getPosition() may only be called on PositionAware'); - } -} diff --git a/src/ResumableTokensProxy.php b/src/ResumableTokensProxy.php new file mode 100644 index 0000000..ff7871b --- /dev/null +++ b/src/ResumableTokensProxy.php @@ -0,0 +1,49 @@ +generator = $tokensGenerator; + $this->tokens = $tokens; + } + + public function getIterator(): \Traversable + { + $generator = $this->generator; + while ($generator->valid()) { + yield $generator->key() => $generator->current(); + $generator->next(); + } + } + + public function __call($name, $arguments) + { + return $this->generator->$name(...$arguments); + } + + /** + * Returns JSON bytes read so far. + */ + public function getPosition() + { + return $this->tokens->getPosition(); + } +} diff --git a/src/Tokens.php b/src/Tokens.php index 6a4ec30..117f67f 100644 --- a/src/Tokens.php +++ b/src/Tokens.php @@ -11,9 +11,6 @@ class Tokens implements \IteratorAggregate, PositionAware /** @var iterable */ private $jsonChunks; - /** @var Generator */ - private $generator; - /** * @param iterable $jsonChunks */ @@ -28,11 +25,7 @@ public function __construct($jsonChunks) #[\ReturnTypeWillChange] public function getIterator() { - if ( ! $this->generator) { - $this->generator = $this->createGenerator(); - } - - return $this->generator; + return $this->createGenerator(); } private function createGenerator(): Generator diff --git a/test/JsonMachineTest/ItemsTest.php b/test/JsonMachineTest/ItemsTest.php index 9c4ab18..a92167f 100644 --- a/test/JsonMachineTest/ItemsTest.php +++ b/test/JsonMachineTest/ItemsTest.php @@ -5,6 +5,7 @@ namespace JsonMachineTest; use JsonMachine\Items; +use JsonMachine\JsonDecoder\ExtJsonDecoder; use JsonMachine\JsonDecoder\PassThruDecoder; /** @@ -146,4 +147,11 @@ public function testCountViaIteratorCount() $this->assertSame(3, iterator_count($items)); } + + public function testIssue117ItemsCanBeIteratedMoreThanOnce() + { + $rows = Items::fromFile(__DIR__.'/ItemsTest.json', ['decoder' => new ExtJsonDecoder(true)]); + $this->assertSame(1, iterator_count($rows)); + $this->assertSame(1, iterator_count($rows)); + } } diff --git a/test/JsonMachineTest/ParserTest.php b/test/JsonMachineTest/ParserTest.php index 29666ae..6a910e9 100644 --- a/test/JsonMachineTest/ParserTest.php +++ b/test/JsonMachineTest/ParserTest.php @@ -628,14 +628,15 @@ public function testRecursiveParserDoesNotRequireChildParserToBeIteratedToTheEnd $this->assertInstanceOf(Traversable::class, $array[1]); $this->assertSame(4, $array[2]); - $this->expectExceptionMessage('generator'); + $this->expectException(JsonMachineException::class); + $this->expectExceptionMessage('once'); iterator_to_array($array[1]); } public function data_testRecursiveParserDoesNotRequireChildParserToBeIteratedToTheEndByUser() { return [ - ['[1,[{},2,3],4]'], + ['[1,{"x": 1,"y": 2,"z": 3},4]'], ['[1,[[],2,3],4]'], ['[1,[{"key": "value"},2,3],4]'], ['[1,[[null, true, "string"],2,3],4]'], diff --git a/test/JsonMachineTest/RecursiveItemsTest.php b/test/JsonMachineTest/RecursiveItemsTest.php index fcb0b40..afbdd19 100644 --- a/test/JsonMachineTest/RecursiveItemsTest.php +++ b/test/JsonMachineTest/RecursiveItemsTest.php @@ -6,6 +6,7 @@ use Iterator; use IteratorAggregate; +use JsonMachine\JsonDecoder\ExtJsonDecoder; use JsonMachine\RecursiveItems; /** @@ -188,20 +189,12 @@ public function testHasChildrenFollowsIterators() $this->assertSame([false, true, false], $result); } - public function testToArrayThrowsMeaningfulErrorWhenIteratorIsAlreadyOpen() + public function testIssue117ItemsCanBeIteratedMoreThanOnce() { - $generator = function () { - yield 'one' => 1; - yield 'two' => 2; - yield 'three' => 3; - }; - $iterator = new RecursiveItems(toIteratorAggregate($generator())); - - $iterator->rewind(); - $iterator->next(); + $rows = RecursiveItems::fromFile(__DIR__.'/ItemsTest.json', ['decoder' => new ExtJsonDecoder(true)]); - $this->expectExceptionMessage('toArray()'); - $iterator->toArray(); + $this->assertSame(1, iterator_count($rows)); + $this->assertSame(1, iterator_count($rows)); } }