Fixed specification members not being cloned before passed to collect…

…ion members.
ScriptFUSION · Oct 17, 2016 · b29dcb9 · b29dcb9
1 parent 098de60
commit b29dcb9
Show file tree

Hide file tree

Showing 2 changed files with 44 additions and 61 deletions.
diff --git a/src/Porter.php b/src/Porter.php
@@ -60,6 +60,7 @@ public function __construct()
      */
     public function import(ImportSpecification $specification)
     {
+        $specification = clone $specification;
         $records = $this->fetch($specification->getResource(), $specification->getCacheAdvice());
 
         if (!$records instanceof ProviderRecords) {
@@ -74,15 +75,15 @@ public function import(ImportSpecification $specification)
             $records = $this->map($records, $specification->getMapping(), $specification->getContext());
         }
 
-        return $this->createPorterRecords($records, clone $specification);
+        return $this->createPorterRecords($records, $specification);
     }
 
     /**
      * Imports one record according to the design of the specified import specification.
      *
      * @param ImportSpecification $specification Import specification.
      *
-     * @return array Record.
+     * @return array|null Record.
      *
      * @throws ImportException More than one record was imported.
      */

diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php
@@ -9,7 +9,6 @@
 use ScriptFUSION\Porter\Cache\CacheToggle;
 use ScriptFUSION\Porter\Cache\CacheUnavailableException;
 use ScriptFUSION\Porter\Collection\CountableMappedRecords;
-use ScriptFUSION\Porter\Collection\CountableProviderRecords;
 use ScriptFUSION\Porter\Collection\FilteredRecords;
 use ScriptFUSION\Porter\Collection\MappedRecords;
 use ScriptFUSION\Porter\Collection\PorterRecords;
@@ -47,7 +46,9 @@ protected function setUp()
             $this->provider =
                 \Mockery::mock(Provider::class)
                     ->shouldReceive('fetch')
-                    ->andReturn(new \ArrayIterator(['foo']))
+                    ->andReturnUsing(function () {
+                        yield 'foo';
+                    })
                     ->byDefault()
                     ->getMock()
         );
@@ -56,6 +57,8 @@ protected function setUp()
         $this->specification = new ImportSpecification($this->resource);
     }
 
+    #region Providers
+
     public function testGetProvider()
     {
         self::assertSame($this->provider, $this->porter->getProvider(get_class($this->provider)));
@@ -115,29 +118,21 @@ public function testHasProvider()
         self::assertFalse($this->porter->hasProvider('foo'));
     }
 
-    public function testImport()
-    {
-        $records = $this->porter->import($this->specification);
+    #endregion
 
-        self::assertInstanceOf(PorterRecords::class, $records);
-        self::assertNotSame($this->specification, $records->getSpecification());
-        self::assertInstanceOf(CountableProviderRecords::class, $records->getPreviousCollection());
-        self::assertSame('foo', $records->current());
-    }
+    #region Import
 
-    public function testNonCountableIteratorImport()
+    public function testImport()
     {
-        $this->provider->shouldReceive('fetch')->andReturnUsing(function () {
-            yield 'foo';
-        });
-
         $records = $this->porter->import($this->specification);
 
         self::assertInstanceOf(PorterRecords::class, $records);
-        self::assertNotSame($this->specification, $records->getSpecification());
-        self::assertInstanceOf(ProviderRecords::class, $records->getPreviousCollection());
-        self::assertNotInstanceOf(CountableProviderRecords::class, $records->getPreviousCollection());
+        self::assertNotSame($this->specification, $records->getSpecification(), 'Specification was not cloned.');
         self::assertSame('foo', $records->current());
+
+        /** @var ProviderRecords $previous */
+        self::assertInstanceOf(ProviderRecords::class, $previous = $records->getPreviousCollection());
+        self::assertNotSame($this->resource, $previous->getResource(), 'Resource was not cloned.');
     }
 
     /**
@@ -146,9 +141,7 @@ public function testNonCountableIteratorImport()
     public function testImportCountableRecords()
     {
         $records = $this->porter->import(
-            new StaticDataImportSpecification(
-                new CountableProviderRecords(\Mockery::mock(\Iterator::class), $count = rand(1, 9), $this->resource)
-            )
+            new StaticDataImportSpecification(new \ArrayIterator(range(1, $count = 10)))
         );
 
         // Innermost collection.
@@ -160,23 +153,6 @@ public function testImportCountableRecords()
         self::assertCount($count, $records);
     }
 
-    public function testImportAndMapNonCountableRecords()
-    {
-        $iterateOne = function () {
-            yield 'foo';
-        };
-        $records = $this->porter->import(
-            (new StaticDataImportSpecification(
-                new ProviderRecords($iterateOne(), $this->resource)
-            ))->setMapping(\Mockery::mock(Mapping::class))
-        );
-
-        self::assertInstanceOf(MappedRecords::class, $records->getPreviousCollection());
-        self::assertInstanceOf(\Iterator::class, $records);
-        self::assertNotInstanceOf(CountableMappedRecords::class, $records->getPreviousCollection());
-        self::assertNotInstanceOf(\Countable::class, $records);
-    }
-
     /**
      * Tests that when the resource is countable the count is propagated to the outermost collection via a mapped
      * collection.
@@ -185,7 +161,7 @@ public function testImportAndMapCountableRecords()
     {
         $records = $this->porter->import(
             (new StaticDataImportSpecification(
-                new CountableProviderRecords(\Mockery::mock(\Iterator::class), $count = rand(1, 9), $this->resource)
+                new \ArrayIterator(range(1, $count = 10))
             ))->setMapping(\Mockery::mock(Mapping::class))
         );
 
@@ -201,7 +177,7 @@ public function testImportAndFilterCountableRecords()
     {
         $records = $this->porter->import(
             (new StaticDataImportSpecification(
-                new CountableProviderRecords(\Mockery::mock(\Iterator::class), $count = rand(1, 9), $this->resource)
+                new \ArrayIterator(range(1, $count = 10))
             ))->setFilter([$this, __FUNCTION__])
         );
 
@@ -212,6 +188,30 @@ public function testImportAndFilterCountableRecords()
         self::assertNotInstanceOf(\Countable::class, $records);
     }
 
+    public function testImportTaggedResource()
+    {
+        $this->porter->registerProvider(
+            $provider = \Mockery::mock(Provider::class)
+                ->shouldReceive('fetch')
+                ->andReturn(new \ArrayIterator([$output = 'bar']))
+                ->getMock(),
+            $tag = 'foo'
+        );
+
+        $records = $this->porter->import(MockFactory::mockImportSpecification(
+            MockFactory::mockResource($provider)
+                ->shouldReceive('getProviderTag')
+                ->andReturn($tag)
+                ->getMock()
+        ));
+
+        self::assertSame($output, $records->current());
+    }
+
+    #endregion
+
+    #region Import one
+
     public function testImportOne()
     {
         $result = $this->porter->importOne($this->specification);
@@ -237,25 +237,7 @@ public function testImportOneOfMany()
         $this->porter->importOne($this->specification);
     }
 
-    public function testImportTaggedResource()
-    {
-        $this->porter->registerProvider(
-            $provider = \Mockery::mock(Provider::class)
-                ->shouldReceive('fetch')
-                ->andReturn(new \ArrayIterator([$output = 'bar']))
-                ->getMock(),
-            $tag = 'foo'
-        );
-
-        $records = $this->porter->import(MockFactory::mockImportSpecification(
-            MockFactory::mockResource($provider)
-                ->shouldReceive('getProviderTag')
-                ->andReturn($tag)
-                ->getMock()
-        ));
-
-        self::assertSame($output, $records->current());
-    }
+    #endregion
 
     public function testFilter()
     {