From 23930aba0eab2595035a61866b6825e58c0cb7f1 Mon Sep 17 00:00:00 2001 From: Bilge Date: Mon, 9 Apr 2018 00:34:32 +0100 Subject: [PATCH] Changed RecordCollection to force array return type. Added accompanying RecordCollectionTest::testNonArrayYield test. --- src/Collection/RecordCollection.php | 24 ++++++++++++++++++- .../CountableProviderRecordsTest.php | 17 +++++++------ test/Integration/Porter/PorterTest.php | 20 ++++++++++------ .../StaticDataImportSpecificationTest.php | 4 ++-- test/MockFactory.php | 2 +- .../Collection/RecordCollectionTest.php | 18 ++++++++++++++ 6 files changed, 67 insertions(+), 18 deletions(-) diff --git a/src/Collection/RecordCollection.php b/src/Collection/RecordCollection.php index 5a5eb07..abdca2e 100644 --- a/src/Collection/RecordCollection.php +++ b/src/Collection/RecordCollection.php @@ -16,26 +16,48 @@ public function __construct(\Iterator $records, self $previousCollection = null) $this->previousCollection = $previousCollection; } + /** + * @return array + */ public function current() { - return $this->records->current(); + $current = $this->records->current(); + + // TODO: Consider removing when dropping PHP 5 support (replace with type hint). + if (!is_array($current)) { + throw new \RuntimeException('Record collection did not return an array.'); + } + + return $current; } + /** + * @return void + */ public function next() { $this->records->next(); } + /** + * @return mixed + */ public function key() { return $this->records->key(); } + /** + * @return bool + */ public function valid() { return $this->records->valid(); } + /** + * @return void + */ public function rewind() { $this->records->rewind(); diff --git a/test/Integration/Porter/Collection/CountableProviderRecordsTest.php b/test/Integration/Porter/Collection/CountableProviderRecordsTest.php index dfde465..409b0d7 100644 --- a/test/Integration/Porter/Collection/CountableProviderRecordsTest.php +++ b/test/Integration/Porter/Collection/CountableProviderRecordsTest.php @@ -4,19 +4,22 @@ use ScriptFUSION\Porter\Collection\CountableProviderRecords; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; +/** + * @see CountableProviderRecords + */ final class CountableProviderRecordsTest extends \PHPUnit_Framework_TestCase { - public function test() + /** + * Tests that counting the collection matches the passed count value. + */ + public function testCount() { - $data = range(1, 10); - $records = new CountableProviderRecords( - new \ArrayIterator($data), - count($data), + new \EmptyIterator, + $count = 10, \Mockery::mock(ProviderResource::class) ); - self::assertCount(count($data), $records); - self::assertSame($data, iterator_to_array($records)); + self::assertCount($count, $records); } } diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index 21e3c7f..6c8c3d9 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -82,7 +82,7 @@ public function testImport() self::assertInstanceOf(PorterRecords::class, $records); self::assertNotSame($this->specification, $records->getSpecification(), 'Specification was not cloned.'); - self::assertSame('foo', $records->current()); + self::assertSame(['foo'], $records->current()); /** @var ProviderRecords $previous */ self::assertInstanceOf(ProviderRecords::class, $previous = $records->getPreviousCollection()); @@ -167,7 +167,7 @@ public function testImportCustomProviderName() ); $records = $this->porter->import( - (new ImportSpecification(MockFactory::mockResource($provider, new \ArrayIterator([$output = 'bar'])))) + (new ImportSpecification(MockFactory::mockResource($provider, new \ArrayIterator([$output = ['bar']])))) ->setProviderName($providerName) ); @@ -212,7 +212,7 @@ public function testImportOne() { $result = $this->porter->importOne($this->specification); - self::assertSame('foo', $result); + self::assertSame(['foo'], $result); } public function testImportOneOfNone() @@ -226,7 +226,7 @@ public function testImportOneOfNone() public function testImportOneOfMany() { - $this->resource->shouldReceive('fetch')->andReturn(new \ArrayIterator(['foo', 'bar'])); + $this->resource->shouldReceive('fetch')->andReturn(new \ArrayIterator([['foo'], ['bar']])); $this->setExpectedException(ImportException::class); $this->porter->importOne($this->specification); @@ -343,17 +343,23 @@ function (\Exception $exception) use ($connectorException) { public function testFilter() { - $this->resource->shouldReceive('fetch')->andReturn(new \ArrayIterator(range(1, 10))); + $this->resource->shouldReceive('fetch')->andReturnUsing( + static function () { + foreach (range(1, 10) as $i) { + yield [$i]; + } + } + ); $records = $this->porter->import( $this->specification ->addTransformer(new FilterTransformer($filter = function ($record) { - return $record % 2; + return $record[0] % 2; })) ); self::assertInstanceOf(PorterRecords::class, $records); - self::assertSame([1, 3, 5, 7, 9], iterator_to_array($records)); + self::assertSame([[1], [3], [5], [7], [9]], iterator_to_array($records)); /** @var FilteredRecords $previous */ self::assertInstanceOf(FilteredRecords::class, $previous = $records->getPreviousCollection()); diff --git a/test/Integration/Porter/Specification/StaticDataImportSpecificationTest.php b/test/Integration/Porter/Specification/StaticDataImportSpecificationTest.php index 4bebb02..7e7e5d2 100644 --- a/test/Integration/Porter/Specification/StaticDataImportSpecificationTest.php +++ b/test/Integration/Porter/Specification/StaticDataImportSpecificationTest.php @@ -13,8 +13,8 @@ final class StaticDataImportSpecificationTest extends \PHPUnit_Framework_TestCas public function test() { $records = (new Porter(\Mockery::spy(ContainerInterface::class))) - ->import(new StaticDataImportSpecification(new \ArrayIterator(['foo']))); + ->import(new StaticDataImportSpecification(new \ArrayIterator([$output = ['foo']]))); - self::assertSame('foo', $records->current()); + self::assertSame($output, $records->current()); } } diff --git a/test/MockFactory.php b/test/MockFactory.php index 4576a2a..e99e95f 100644 --- a/test/MockFactory.php +++ b/test/MockFactory.php @@ -44,7 +44,7 @@ public static function mockResource(Provider $provider, \Iterator $return = null ->andReturn(get_class($provider)) ->shouldReceive('fetch') ->andReturnUsing(function (ImportConnector $connector) { - return new \ArrayIterator([$connector->fetch('foo')]); + return new \ArrayIterator([[$connector->fetch('foo')]]); }) ->byDefault() ->getMock() diff --git a/test/Unit/Porter/Collection/RecordCollectionTest.php b/test/Unit/Porter/Collection/RecordCollectionTest.php index d9038c4..42b4132 100644 --- a/test/Unit/Porter/Collection/RecordCollectionTest.php +++ b/test/Unit/Porter/Collection/RecordCollectionTest.php @@ -3,6 +3,9 @@ use ScriptFUSION\Porter\Collection\RecordCollection; +/** + * @see RecordCollection + */ final class RecordCollectionTest extends \PHPUnit_Framework_TestCase { public function testFindParent() @@ -30,4 +33,19 @@ public function testFindParent() self::assertSame($collection1, $collection2->findFirstCollection()); self::assertSame($collection1, $collection3->findFirstCollection()); } + + /** + * Tests that when a RecordCollection yields a non-array datum, an exception is thrown. + */ + public function testNonArrayYield() + { + /** @var RecordCollection $collection */ + $collection = \Mockery::mock( + RecordCollection::class, + [new \ArrayIterator(['foo'])] + )->makePartial(); + + $this->setExpectedException(\RuntimeException::class); + $collection->current(); + } }