Fixes #117

halaxa · Dec 1, 2024 · 527ab49 · 527ab49
1 parent c889fdf
commit 527ab49
Show file tree

Hide file tree

Showing 8 changed files with 96 additions and 111 deletions.
diff --git a/src/Parser.php b/src/Parser.php
@@ -39,9 +39,6 @@ class Parser implements \IteratorAggregate, PositionAware
     /** @var Traversable */
     private $tokens;
 
-    /** @var Iterator<int, string> */
-    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
@@ -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
diff --git a/src/ResumableTokensProxy.php b/src/ResumableTokensProxy.php
@@ -0,0 +1,49 @@
+<?php
+
+declare(strict_types=1);
+
+namespace JsonMachine;
+
+use IteratorAggregate;
+
+/**
+ * Allows to resume iteration of the inner IteratorAggregate via foreach, which would be otherwise impossible as
+ * foreach implicitly calls reset(). This Iterator does not pass the reset() call to the inner Iterator thus enabling
+ * to follow up on a previous iteation.
+ */
+class ResumableTokensProxy implements IteratorAggregate, PositionAware
+{
+    /** @var \Iterator */
+    private $generator;
+
+    /** @var \Traversable|PositionAware */
+    private $tokens;
+
+    public function __construct(\Traversable $tokens, \Iterator $tokensGenerator)
+    {
+        $this->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
@@ -11,9 +11,6 @@ class Tokens implements \IteratorAggregate, PositionAware
     /** @var iterable */
     private $jsonChunks;
 
-    /** @var Generator */
-    private $generator;
-
     /**
      * @param iterable<string> $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
@@ -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
@@ -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
@@ -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));
     }
 }