From 557d694c437dda4f5f050e3a2789dec11c7f8f0f Mon Sep 17 00:00:00 2001 From: Bilge Date: Tue, 4 Jul 2017 22:56:46 +0100 Subject: [PATCH 1/2] Removed abstract provider. Changed Provider interface to require getConnector method instead of fetch. --- src/Porter.php | 29 ++- src/Provider/AbstractProvider.php | 106 ----------- src/Provider/Provider.php | 12 +- src/Provider/ProviderOptions.php | 17 ++ src/Provider/StaticDataProvider.php | 11 +- test/Integration/Porter/PorterTest.php | 52 +++--- test/MockFactory.php | 22 ++- .../Porter/Provider/AbstractProviderTest.php | 172 ------------------ 8 files changed, 96 insertions(+), 325 deletions(-) delete mode 100644 src/Provider/AbstractProvider.php create mode 100644 src/Provider/ProviderOptions.php delete mode 100644 test/Unit/Porter/Provider/AbstractProviderTest.php diff --git a/src/Porter.php b/src/Porter.php index 4164d01..20fca7d 100644 --- a/src/Porter.php +++ b/src/Porter.php @@ -10,10 +10,13 @@ use ScriptFUSION\Porter\Collection\PorterRecords; use ScriptFUSION\Porter\Collection\ProviderRecords; use ScriptFUSION\Porter\Collection\RecordCollection; +use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Connector\RecoverableConnectorException; +use ScriptFUSION\Porter\Provider\ForeignResourceException; use ScriptFUSION\Porter\Provider\ObjectNotCreatedException; use ScriptFUSION\Porter\Provider\Provider; use ScriptFUSION\Porter\Provider\ProviderFactory; +use ScriptFUSION\Porter\Provider\ProviderOptions; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; use ScriptFUSION\Porter\Specification\ImportSpecification; use ScriptFUSION\Porter\Transform\Transformer; @@ -108,12 +111,26 @@ private function fetch( ) { $provider = $this->getProvider($providerName ?: $resource->getProviderClassName()); - $this->applyCacheAdvice($provider, $cacheAdvice); + if ($resource->getProviderClassName() !== get_class($provider)) { + throw new ForeignResourceException(sprintf( + 'Cannot fetch data from foreign source: "%s".', + get_class($resource) + )); + } + + $this->applyCacheAdvice($provider->getConnector(), $cacheAdvice); if (($records = \ScriptFUSION\Retry\retry( $fetchAttempts, function () use ($provider, $resource) { - if (($records = $provider->fetch($resource)) instanceof \Iterator) { + $records = $resource->fetch( + $provider->getConnector(), + $provider instanceof ProviderOptions + ? $provider->getOptions() + : null + ); + + if ($records instanceof \Iterator) { // Force generator to run until first yield to provoke an exception. $records->valid(); } @@ -173,22 +190,22 @@ private function createPorterRecords(RecordCollection $records, ImportSpecificat return new PorterRecords($records, $specification); } - private function applyCacheAdvice(Provider $provider, CacheAdvice $cacheAdvice) + private function applyCacheAdvice(Connector $connector, CacheAdvice $cacheAdvice) { try { - if (!$provider instanceof CacheToggle) { + if (!$connector instanceof CacheToggle) { throw CacheUnavailableException::modify(); } switch ("$cacheAdvice") { case CacheAdvice::MUST_CACHE: case CacheAdvice::SHOULD_CACHE: - $provider->enableCache(); + $connector->enableCache(); break; case CacheAdvice::MUST_NOT_CACHE: case CacheAdvice::SHOULD_NOT_CACHE: - $provider->disableCache(); + $connector->disableCache(); } } catch (CacheUnavailableException $exception) { if ($cacheAdvice === CacheAdvice::MUST_NOT_CACHE() || diff --git a/src/Provider/AbstractProvider.php b/src/Provider/AbstractProvider.php deleted file mode 100644 index 94ec33a..0000000 --- a/src/Provider/AbstractProvider.php +++ /dev/null @@ -1,106 +0,0 @@ -connector = $connector; - } - - /** - * @param ProviderResource $resource - * - * @return \Iterator - * - * @throws ForeignResourceException A foreign resource was received. - */ - public function fetch(ProviderResource $resource) - { - if ($resource->getProviderClassName() !== static::class) { - throw new ForeignResourceException(sprintf( - 'Cannot fetch data from foreign source: "%s".', - get_class($resource) - )); - } - - return $resource->fetch($this->connector, $this->options ? clone $this->options : null); - } - - /** - * @return Connector - */ - public function getConnector() - { - return $this->connector; - } - - public function enableCache() - { - $connector = $this->getConnector(); - - if (!$connector instanceof CacheToggle) { - throw CacheUnavailableException::modify(); - } - - $connector->enableCache(); - } - - public function disableCache() - { - $connector = $this->getConnector(); - - if (!$connector instanceof CacheToggle) { - throw CacheUnavailableException::modify(); - } - - $connector->disableCache(); - } - - public function isCacheEnabled() - { - $connector = $this->getConnector(); - - if (!$connector instanceof CacheToggle) { - return false; - } - - return $connector->isCacheEnabled(); - } - - /** - * Gets the provider options. - * - * @return EncapsulatedOptions|null - */ - protected function getOptions() - { - return $this->options; - } - - /** - * Sets the provider options to the specified options. - * - * @param EncapsulatedOptions $options Options. - */ - protected function setOptions(EncapsulatedOptions $options) - { - $this->options = $options; - } -} diff --git a/src/Provider/Provider.php b/src/Provider/Provider.php index 69bbedf..067d811 100644 --- a/src/Provider/Provider.php +++ b/src/Provider/Provider.php @@ -1,19 +1,17 @@ connector = new NullConnector; + } + + public function getConnector() + { + return $this->connector; } } diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index d663db6..c28006c 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -11,6 +11,7 @@ use ScriptFUSION\Porter\Collection\PorterRecords; use ScriptFUSION\Porter\Collection\ProviderRecords; use ScriptFUSION\Porter\Collection\RecordCollection; +use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Connector\RecoverableConnectorException; use ScriptFUSION\Porter\ImportException; use ScriptFUSION\Porter\Porter; @@ -62,10 +63,8 @@ protected function setUp() $this->registerProvider( $this->provider = \Mockery::mock(Provider::class) - ->shouldReceive('fetch') - ->andReturnUsing(function () { - yield 'foo'; - }) + ->shouldReceive('getConnector') + ->andReturn(\Mockery::mock(Connector::class)) ->byDefault() ->getMock() ); @@ -150,15 +149,13 @@ public function testPorterAwareTransformer() public function testImportCustomProviderName() { $this->registerProvider( - $provider = \Mockery::mock(Provider::class) - ->shouldReceive('fetch') - ->andReturn(new \ArrayIterator([$output = 'bar'])) - ->getMock(), + $provider = clone $this->provider, $providerName = 'foo' ); $records = $this->porter->import( - (new ImportSpecification(MockFactory::mockResource($provider)))->setProviderName($providerName) + (new ImportSpecification(MockFactory::mockResource($provider, new \ArrayIterator([$output = 'bar'])))) + ->setProviderName($providerName) ); self::assertSame($output, $records->current()); @@ -166,7 +163,7 @@ public function testImportCustomProviderName() public function testImportFailure() { - $this->provider->shouldReceive('fetch')->andReturn(null); + $this->resource->shouldReceive('fetch')->andReturn(null); $this->setExpectedException(ImportException::class, get_class($this->provider)); $this->porter->import($this->specification); @@ -192,7 +189,7 @@ public function testImportOne() public function testImportOneOfNone() { - $this->provider->shouldReceive('fetch')->andReturn(new \EmptyIterator); + $this->resource->shouldReceive('fetch')->andReturn(new \EmptyIterator); $result = $this->porter->importOne($this->specification); @@ -201,7 +198,7 @@ public function testImportOneOfNone() public function testImportOneOfMany() { - $this->provider->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); @@ -213,7 +210,7 @@ public function testImportOneOfMany() public function testOneTry() { - $this->provider->shouldReceive('fetch')->once()->andThrow(RecoverableConnectorException::class); + $this->resource->shouldReceive('fetch')->once()->andThrow(RecoverableConnectorException::class); $this->setExpectedException(FailingTooHardException::class, '1'); $this->porter->import($this->specification->setMaxFetchAttempts(1)); @@ -221,7 +218,7 @@ public function testOneTry() public function testDerivedRecoverableException() { - $this->provider->shouldReceive('fetch')->once()->andThrow(\Mockery::mock(RecoverableConnectorException::class)); + $this->resource->shouldReceive('fetch')->once()->andThrow(\Mockery::mock(RecoverableConnectorException::class)); $this->setExpectedException(FailingTooHardException::class); $this->porter->import($this->specification->setMaxFetchAttempts(1)); @@ -229,7 +226,7 @@ public function testDerivedRecoverableException() public function testDefaultTries() { - $this->provider->shouldReceive('fetch')->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS) + $this->resource->shouldReceive('fetch')->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS) ->andThrow(RecoverableConnectorException::class); $this->setExpectedException(FailingTooHardException::class, (string)ImportSpecification::DEFAULT_FETCH_ATTEMPTS); @@ -238,7 +235,7 @@ public function testDefaultTries() public function testUnrecoverableException() { - $this->provider->shouldReceive('fetch')->once()->andThrow(\Exception::class); + $this->resource->shouldReceive('fetch')->once()->andThrow(\Exception::class); $this->setExpectedException(\Exception::class); $this->porter->import($this->specification); @@ -252,7 +249,7 @@ public function testCustomFetchExceptionHandler() ->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS - 1) ->getMock() ); - $this->provider->shouldReceive('fetch')->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS) + $this->resource->shouldReceive('fetch')->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS) ->andThrow(RecoverableConnectorException::class); $this->setExpectedException(FailingTooHardException::class); @@ -266,7 +263,7 @@ public function testCustomFetchExceptionHandler() */ public function testGeneratorException() { - $this->provider->shouldReceive('fetch')->once()->andReturnUsing(function () { + $this->resource->shouldReceive('fetch')->once()->andReturnUsing(function () { throw new RecoverableConnectorException; yield; @@ -280,7 +277,7 @@ public function testGeneratorException() public function testFilter() { - $this->provider->shouldReceive('fetch')->andReturn(new \ArrayIterator(range(1, 10))); + $this->resource->shouldReceive('fetch')->andReturn(new \ArrayIterator(range(1, 10))); $records = $this->porter->import( $this->specification @@ -299,15 +296,16 @@ public function testFilter() public function testApplyCacheAdvice() { - $this->registerProvider( - $provider = \Mockery::mock(implode(',', [Provider::class, CacheToggle::class])) - ->shouldReceive('fetch')->andReturn(new \EmptyIterator) - ->shouldReceive('disableCache')->once() - ->shouldReceive('enableCache')->once() - ->getMock() - ); + $this->provider->shouldReceive('getConnector') + ->andReturn( + \Mockery::mock(implode(',', [Connector::class, CacheToggle::class])) + ->shouldReceive('disableCache')->once() + ->shouldReceive('enableCache')->once() + ->getMock() + ) + ; - $this->porter->import($specification = new ImportSpecification(MockFactory::mockResource($provider))); + $this->porter->import($specification = new ImportSpecification($this->resource)); $this->porter->import($specification->setCacheAdvice(CacheAdvice::SHOULD_CACHE())); } diff --git a/test/MockFactory.php b/test/MockFactory.php index f7f2a35..f720966 100644 --- a/test/MockFactory.php +++ b/test/MockFactory.php @@ -12,15 +12,27 @@ final class MockFactory /** * @param Provider $provider + * @param \Iterator $return * - * @return MockInterface|ProviderResource + * @return ProviderResource|MockInterface */ - public static function mockResource(Provider $provider) + public static function mockResource(Provider $provider, \Iterator $return = null) { - return \Mockery::mock(ProviderResource::class) + $resource = \Mockery::mock(ProviderResource::class) ->shouldReceive('getProviderClassName') - ->andReturn(get_class($provider)) - ->byDefault() + ->andReturn(get_class($provider)) + ->byDefault() + ->shouldReceive('fetch') + ->andReturnUsing(function () { + yield 'foo'; + }) + ->byDefault() ->getMock(); + + if ($return !== null) { + $resource->shouldReceive('fetch')->andReturn($return); + } + + return $resource; } } diff --git a/test/Unit/Porter/Provider/AbstractProviderTest.php b/test/Unit/Porter/Provider/AbstractProviderTest.php deleted file mode 100644 index 2b47ae7..0000000 --- a/test/Unit/Porter/Provider/AbstractProviderTest.php +++ /dev/null @@ -1,172 +0,0 @@ -createProviderMock(); - } - - private function createProviderMock($connector = null) - { - $this->provider = \Mockery::mock( - AbstractProvider::class, - [$this->connector = $connector ?: \Mockery::mock(Connector::class)] - )->makePartial(); - } - - private function setupCachingConnector() - { - $this->createProviderMock($connector = \Mockery::mock(CachingConnector::class)); - - return $connector; - } - - public function testFetchWithoutOptions() - { - self::assertSame( - 'foo', - $this->provider->fetch( - MockFactory::mockResource($this->provider) - ->shouldReceive('fetch') - ->with($this->connector, null) - ->andReturn('foo') - ->getMock() - ) - ); - } - - /** - * Tests that a clone of the provider's options are passed to ProviderResource::fetch(). - */ - public function testFetchWithOptions() - { - $this->setOptions($options = \Mockery::mock(EncapsulatedOptions::class)); - - $this->provider->fetch( - MockFactory::mockResource($this->provider) - ->shouldReceive('fetch') - ->with($this->connector, \Mockery::on( - function (EncapsulatedOptions $argument) use ($options) { - self::assertNotSame($options, $argument); - - return get_class($options) === get_class($argument); - } - )) - ->getMock() - ); - } - - public function testFetchForeignProvider() - { - $this->setExpectedException(ForeignResourceException::class); - - $this->provider->fetch( - \Mockery::mock(ProviderResource::class) - ->shouldReceive('getProviderClassName') - ->andReturn('foo') - ->getMock() - ); - } - - public function testGetConnector() - { - self::assertSame($this->connector, $this->provider->getConnector()); - } - - public function testEnableCacheFails() - { - $this->setExpectedException(CacheUnavailableException::class); - - $this->provider->enableCache(); - } - - public function testEnableCacheSucceeds() - { - $this->setupCachingConnector()->shouldReceive('enableCache'); - - $this->provider->enableCache(); - } - - public function testDisableCacheFails() - { - $this->setExpectedException(CacheUnavailableException::class); - - $this->provider->disableCache(); - } - - public function testDisableCacheSucceeds() - { - $this->setupCachingConnector()->shouldReceive('disableCache'); - - $this->provider->disableCache(); - } - - public function testCacheDisabledWhenConnectorDoesNotSupportCaching() - { - self::assertFalse($this->provider->isCacheEnabled()); - } - - public function testCacheEnabledMirrorsCachingConnector() - { - $this->setupCachingConnector()->shouldReceive('isCacheEnabled')->andReturn(true, false); - - self::assertTrue($this->provider->isCacheEnabled()); - self::assertFalse($this->provider->isCacheEnabled()); - } - - public function testOptions() - { - $this->setOptions($options = \Mockery::mock(EncapsulatedOptions::class)); - - self::assertSame($options, $this->getOptions()); - } - - private function getOptions() - { - return call_user_func( - \Closure::bind( - function () { - return $this->getOptions(); - }, - $this->provider - ) - ); - } - - private function setOptions(EncapsulatedOptions $options) - { - call_user_func( - \Closure::bind( - function () use ($options) { - $this->setOptions($options); - }, - $this->provider - ) - ); - } -} From c342d69518e37c26d5f5cd70f3d762ff34815e5e Mon Sep 17 00:00:00 2001 From: Bilge Date: Wed, 5 Jul 2017 23:07:23 +0100 Subject: [PATCH 2/2] Added test case for importing foreign resource. --- src/Porter.php | 2 +- test/Integration/Porter/PorterTest.php | 33 ++++++++++++++++---------- test/MockFactory.php | 18 ++++++++++++-- 3 files changed, 38 insertions(+), 15 deletions(-) diff --git a/src/Porter.php b/src/Porter.php index 20fca7d..a5acf55 100644 --- a/src/Porter.php +++ b/src/Porter.php @@ -113,7 +113,7 @@ private function fetch( if ($resource->getProviderClassName() !== get_class($provider)) { throw new ForeignResourceException(sprintf( - 'Cannot fetch data from foreign source: "%s".', + 'Cannot fetch data from foreign resource: "%s".', get_class($resource) )); } diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index c28006c..0a4d909 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -16,6 +16,7 @@ use ScriptFUSION\Porter\ImportException; use ScriptFUSION\Porter\Porter; use ScriptFUSION\Porter\PorterAware; +use ScriptFUSION\Porter\Provider\ForeignResourceException; use ScriptFUSION\Porter\Provider\Provider; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; use ScriptFUSION\Porter\ProviderNotFoundException; @@ -42,7 +43,7 @@ final class PorterTest extends \PHPUnit_Framework_TestCase private $provider; /** - * @var ProviderResource + * @var ProviderResource|MockInterface */ private $resource; @@ -60,15 +61,7 @@ protected function setUp() { $this->porter = new Porter($this->container = $container = \Mockery::spy(ContainerInterface::class)); - $this->registerProvider( - $this->provider = - \Mockery::mock(Provider::class) - ->shouldReceive('getConnector') - ->andReturn(\Mockery::mock(Connector::class)) - ->byDefault() - ->getMock() - ); - + $this->registerProvider($this->provider = MockFactory::mockProvider()); $this->resource = MockFactory::mockResource($this->provider); $this->specification = new ImportSpecification($this->resource); } @@ -173,7 +166,19 @@ public function testImportUnregisteredProvider() { $this->setExpectedException(ProviderNotFoundException::class); - $this->porter->import((new ImportSpecification($this->resource))->setProviderName('foo')); + $this->porter->import($this->specification->setProviderName('foo')); + } + + /** + * Tests that when a resource's provider class name does not match the provider an exception is thrown. + */ + public function testImportForeignResource() + { + // Replace existing provider with a different one. + $this->registerProvider(MockFactory::mockProvider(), get_class($this->provider)); + + $this->setExpectedException(ForeignResourceException::class); + $this->porter->import($this->specification); } #endregion @@ -316,13 +321,17 @@ public function testCacheUnavailable() $this->porter->import($this->specification->setCacheAdvice(CacheAdvice::MUST_CACHE())); } + /** + * @param Provider $provider + * @param string|null $name + */ private function registerProvider(Provider $provider, $name = null) { $name = $name ?: get_class($provider); $this->container ->shouldReceive('has')->with($name)->andReturn(true) - ->shouldReceive('get')->with($name)->andReturn($provider) + ->shouldReceive('get')->with($name)->andReturn($provider)->byDefault() ; } } diff --git a/test/MockFactory.php b/test/MockFactory.php index f720966..b2d855e 100644 --- a/test/MockFactory.php +++ b/test/MockFactory.php @@ -2,6 +2,7 @@ namespace ScriptFUSIONTest; use Mockery\MockInterface; +use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Provider\Provider; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; use ScriptFUSION\StaticClass; @@ -10,6 +11,19 @@ final class MockFactory { use StaticClass; + /** + * @return Provider|MockInterface + */ + public static function mockProvider() + { + return \Mockery::namedMock(uniqid(Provider::class, false), Provider::class) + ->shouldReceive('getConnector') + ->andReturn(\Mockery::mock(Connector::class)) + ->byDefault() + ->getMock() + ; + } + /** * @param Provider $provider * @param \Iterator $return @@ -21,13 +35,13 @@ public static function mockResource(Provider $provider, \Iterator $return = null $resource = \Mockery::mock(ProviderResource::class) ->shouldReceive('getProviderClassName') ->andReturn(get_class($provider)) - ->byDefault() ->shouldReceive('fetch') ->andReturnUsing(function () { yield 'foo'; }) ->byDefault() - ->getMock(); + ->getMock() + ; if ($return !== null) { $resource->shouldReceive('fetch')->andReturn($return);