From 870d742ee3ffa1ee621875f263b8f56745766941 Mon Sep 17 00:00:00 2001 From: Bilge Date: Wed, 12 Jul 2017 23:09:13 +0100 Subject: [PATCH 1/4] Added ConnectionContext and accompanying factory to fix caching and durability issues in Porter v3. Added SuperConnector to supervise delivery of ConnectionContext to connectors and validate cache availability. --- src/Cache/Cache.php | 15 ++++ src/Cache/CacheToggle.php | 29 -------- src/Cache/CacheUnavailableException.php | 9 ++- src/Connector/CachingConnector.php | 28 ++------ src/Connector/ConnectionContext.php | 56 +++++++++++++++ src/Connector/ConnectionContextFactory.php | 16 +++++ src/Connector/Connector.php | 3 +- src/Connector/NullConnector.php | 2 +- src/Connector/SuperConnector.php | 39 ++++++++++ src/Porter.php | 83 +++------------------- src/Provider/Resource/ProviderResource.php | 9 ++- src/Provider/Resource/StaticResource.php | 4 +- test/Integration/Porter/PorterTest.php | 4 +- 13 files changed, 162 insertions(+), 135 deletions(-) create mode 100644 src/Cache/Cache.php delete mode 100644 src/Cache/CacheToggle.php create mode 100644 src/Connector/ConnectionContext.php create mode 100644 src/Connector/ConnectionContextFactory.php create mode 100644 src/Connector/SuperConnector.php diff --git a/src/Cache/Cache.php b/src/Cache/Cache.php new file mode 100644 index 0000000..4e7d0f1 --- /dev/null +++ b/src/Cache/Cache.php @@ -0,0 +1,15 @@ +isCacheEnabled()) { + if ($context->shouldCache()) { $optionsCopy = $options ? $options->copy() : []; ksort($optionsCopy); @@ -78,19 +74,9 @@ public function setCache(CacheItemPoolInterface $cache) $this->cache = $cache; } - public function enableCache() - { - $this->cacheEnabled = true; - } - - public function disableCache() - { - $this->cacheEnabled = false; - } - - public function isCacheEnabled() + public function isCacheAvailable() { - return $this->cacheEnabled; + return true; } public function getCacheKeyGenerator() diff --git a/src/Connector/ConnectionContext.php b/src/Connector/ConnectionContext.php new file mode 100644 index 0000000..232ad64 --- /dev/null +++ b/src/Connector/ConnectionContext.php @@ -0,0 +1,56 @@ +cacheAdvice = $cacheAdvice; + $this->fetchExceptionHandler = $fetchExceptionHandler; + $this->maxFetchAttempts = (int)$maxFetchAttempts; + } + + public function retry(callable $callable) + { + return \ScriptFUSION\Retry\retry( + $this->maxFetchAttempts, + $callable, + function (\Exception $exception) { + // Throw exception if unrecoverable. + if (!$exception instanceof RecoverableConnectorException) { + throw $exception; + } + + call_user_func($this->fetchExceptionHandler, $exception); + } + ); + } + + /** + * Gets a value indicating whether a connector should cache data. + * + * @return bool True if the connector should cache data, otherwise false. + */ + public function shouldCache() + { + return $this->cacheAdvice->anyOf(CacheAdvice::SHOULD_CACHE(), CacheAdvice::MUST_CACHE()); + } + + /** + * Gets a value indicating whether a connector must support caching. + * + * @return bool True if connector must support caching, otherwise false. + */ + public function mustSupportCaching() + { + return $this->cacheAdvice->anyOf(CacheAdvice::MUST_CACHE(), CacheAdvice::MUST_NOT_CACHE()); + } +} diff --git a/src/Connector/ConnectionContextFactory.php b/src/Connector/ConnectionContextFactory.php new file mode 100644 index 0000000..e93941a --- /dev/null +++ b/src/Connector/ConnectionContextFactory.php @@ -0,0 +1,16 @@ +getCacheAdvice(), + $specification->getFetchExceptionHandler(), + $specification->getMaxFetchAttempts() + ); + } +} diff --git a/src/Connector/Connector.php b/src/Connector/Connector.php index f7cde3a..85e2ec5 100644 --- a/src/Connector/Connector.php +++ b/src/Connector/Connector.php @@ -12,10 +12,11 @@ interface Connector * Fetches data from the specified source optionally augmented by the * specified options. * + * @param ConnectionContext $context TODO. * @param string $source Source. * @param EncapsulatedOptions $options Optional. Options. * * @return mixed Data. */ - public function fetch($source, EncapsulatedOptions $options = null); + public function fetch(ConnectionContext $context, $source, EncapsulatedOptions $options = null); } diff --git a/src/Connector/NullConnector.php b/src/Connector/NullConnector.php index 0fc32fa..314be18 100644 --- a/src/Connector/NullConnector.php +++ b/src/Connector/NullConnector.php @@ -5,7 +5,7 @@ class NullConnector implements Connector { - public function fetch($source, EncapsulatedOptions $options = null) + public function fetch(ConnectionContext $context, $source, EncapsulatedOptions $options = null) { // Intentionally empty. } diff --git a/src/Connector/SuperConnector.php b/src/Connector/SuperConnector.php new file mode 100644 index 0000000..24c4a1e --- /dev/null +++ b/src/Connector/SuperConnector.php @@ -0,0 +1,39 @@ +connector = $connector; + $this->context = $context; + } + + public function fetch($source, EncapsulatedOptions $options = null) + { + $this->validateCacheState(); + + return $this->connector->fetch($this->context, $source, $options); + } + + private function validateCacheState() + { + if ($this->context->mustSupportCaching()) { + if (!$this->connector instanceof Cache) { + throw CacheUnavailableException::unsupported(); + } + + if (!$this->connector->isCacheAvailable()) { + throw CacheUnavailableException::unavailable(); + } + } + } +} diff --git a/src/Porter.php b/src/Porter.php index b25c8ae..4c37f65 100644 --- a/src/Porter.php +++ b/src/Porter.php @@ -2,21 +2,18 @@ namespace ScriptFUSION\Porter; use Psr\Container\ContainerInterface; -use ScriptFUSION\Porter\Cache\CacheAdvice; -use ScriptFUSION\Porter\Cache\CacheToggle; -use ScriptFUSION\Porter\Cache\CacheUnavailableException; use ScriptFUSION\Porter\Collection\CountablePorterRecords; use ScriptFUSION\Porter\Collection\CountableProviderRecords; 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\Connector\ConnectionContext; +use ScriptFUSION\Porter\Connector\ConnectionContextFactory; +use ScriptFUSION\Porter\Connector\SuperConnector; 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; @@ -62,9 +59,7 @@ public function import(ImportSpecification $specification) $records = $this->fetch( $specification->getResource(), $specification->getProviderName(), - $specification->getCacheAdvice(), - $specification->getMaxFetchAttempts(), - $specification->getFetchExceptionHandler() + ConnectionContextFactory::create($specification) ); if (!$records instanceof ProviderRecords) { @@ -102,13 +97,8 @@ public function importOne(ImportSpecification $specification) return $one; } - private function fetch( - ProviderResource $resource, - $providerName, - CacheAdvice $cacheAdvice, - $fetchAttempts, - $fetchExceptionHandler - ) { + private function fetch(ProviderResource $resource, $providerName, ConnectionContext $context) + { $provider = $this->getProvider($providerName ?: $resource->getProviderClassName()); if ($resource->getProviderClassName() !== get_class($provider)) { @@ -118,38 +108,13 @@ private function fetch( )); } - $this->applyCacheAdvice($provider->getConnector(), $cacheAdvice); - - if (($records = \ScriptFUSION\Retry\retry( - $fetchAttempts, - function () use ($provider, $resource) { - $records = $resource->fetch( - $provider->getConnector(), - $provider instanceof ProviderOptions - ? clone $provider->getOptions() - : null - ); - - if ($records instanceof \Iterator) { - // Force generator to run until first yield to provoke an exception. - $records->valid(); - } - - return $records; - }, - function (\Exception $exception) use ($fetchExceptionHandler) { - // Throw exception if unrecoverable. - if (!$exception instanceof RecoverableConnectorException) { - throw $exception; - } - - $fetchExceptionHandler($exception); - } - )) instanceof \Iterator) { - return $records; + $records = $resource->fetch(new SuperConnector($provider->getConnector(), $context)); + + if (!$records instanceof \Iterator) { + throw new ImportException(get_class($provider) . '::fetch() did not return an Iterator.'); } - throw new ImportException(get_class($provider) . '::fetch() did not return an Iterator.'); + return $records; } /** @@ -190,32 +155,6 @@ private function createPorterRecords(RecordCollection $records, ImportSpecificat return new PorterRecords($records, $specification); } - private function applyCacheAdvice(Connector $connector, CacheAdvice $cacheAdvice) - { - try { - if (!$connector instanceof CacheToggle) { - throw CacheUnavailableException::modify(); - } - - switch ("$cacheAdvice") { - case CacheAdvice::MUST_CACHE: - case CacheAdvice::SHOULD_CACHE: - $connector->enableCache(); - break; - - case CacheAdvice::MUST_NOT_CACHE: - case CacheAdvice::SHOULD_NOT_CACHE: - $connector->disableCache(); - } - } catch (CacheUnavailableException $exception) { - if ($cacheAdvice === CacheAdvice::MUST_NOT_CACHE() || - $cacheAdvice === CacheAdvice::MUST_CACHE() - ) { - throw $exception; - } - } - } - /** * Gets the provider matching the specified name. * diff --git a/src/Provider/Resource/ProviderResource.php b/src/Provider/Resource/ProviderResource.php index 423ba71..8d0eea2 100644 --- a/src/Provider/Resource/ProviderResource.php +++ b/src/Provider/Resource/ProviderResource.php @@ -1,7 +1,7 @@ data; } diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index d99670f..dbe81ad 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -5,7 +5,7 @@ use Mockery\MockInterface; use Psr\Container\ContainerInterface; use ScriptFUSION\Porter\Cache\CacheAdvice; -use ScriptFUSION\Porter\Cache\CacheToggle; +use ScriptFUSION\Porter\Cache\Cache; use ScriptFUSION\Porter\Cache\CacheUnavailableException; use ScriptFUSION\Porter\Collection\FilteredRecords; use ScriptFUSION\Porter\Collection\PorterRecords; @@ -334,7 +334,7 @@ public function testApplyCacheAdvice() { $this->provider->shouldReceive('getConnector') ->andReturn( - \Mockery::mock(implode(',', [Connector::class, CacheToggle::class])) + \Mockery::mock(implode(',', [Connector::class, Cache::class])) ->shouldReceive('disableCache')->once() ->shouldReceive('enableCache')->once() ->getMock() From b73f862454924b097f4c65436b64f748ffffb0ca Mon Sep 17 00:00:00 2001 From: Bilge Date: Fri, 14 Jul 2017 00:17:10 +0100 Subject: [PATCH 2/4] Fixed all tests pertaining to the addition of ConnectionContext and SuperConnector. --- src/Porter.php | 8 +- test/FixtureFactory.php | 35 ++++++ .../Porter/Connector/CachingConnectorTest.php | 91 ++++++++------- test/Integration/Porter/PorterTest.php | 107 ++++++++++-------- test/MockFactory.php | 14 ++- .../Porter/Connector/NullConnectorTest.php | 3 +- .../Provider/Resource/NullResourceTest.php | 8 +- 7 files changed, 170 insertions(+), 96 deletions(-) create mode 100644 test/FixtureFactory.php diff --git a/src/Porter.php b/src/Porter.php index 4c37f65..a87bdb7 100644 --- a/src/Porter.php +++ b/src/Porter.php @@ -14,6 +14,7 @@ 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,10 +109,13 @@ private function fetch(ProviderResource $resource, $providerName, ConnectionCont )); } - $records = $resource->fetch(new SuperConnector($provider->getConnector(), $context)); + $records = $resource->fetch( + new SuperConnector($provider->getConnector(), $context), + $provider instanceof ProviderOptions ? clone $provider->getOptions() : null + ); if (!$records instanceof \Iterator) { - throw new ImportException(get_class($provider) . '::fetch() did not return an Iterator.'); + throw new ImportException(get_class($resource) . '::fetch() did not return an Iterator.'); } return $records; diff --git a/test/FixtureFactory.php b/test/FixtureFactory.php new file mode 100644 index 0000000..18aa5a3 --- /dev/null +++ b/test/FixtureFactory.php @@ -0,0 +1,35 @@ +andReturn('foo', 'bar') ->getMock(); + $this->context = FixtureFactory::buildConnectionContext(CacheAdvice::SHOULD_CACHE()); + $this->options = new TestOptions; } public function testCacheEnabled() { - self::assertSame('foo', $this->connector->fetch('baz', $this->options)); - self::assertSame('foo', $this->connector->fetch('baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); } public function testCacheDisabled() { - $this->connector->disableCache(); + $context = FixtureFactory::buildConnectionContext(CacheAdvice::SHOULD_NOT_CACHE()); - self::assertSame('foo', $this->connector->fetch('baz', $this->options)); - self::assertSame('bar', $this->connector->fetch('baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($context, 'baz', $this->options)); + self::assertSame('bar', $this->connector->fetch($context, 'baz', $this->options)); } public function testGetSetCache() @@ -67,86 +81,77 @@ public function testGetSetCacheKeyGenerator() public function testCacheBypassedForDifferentOptions() { - self::assertSame('foo', $this->connector->fetch('baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); $this->options->setFoo('bar'); - - self::assertSame('bar', $this->connector->fetch('baz', $this->options)); + self::assertSame('bar', $this->connector->fetch($this->context, 'baz', $this->options)); } public function testCacheUsedForDifferentOptionsInstance() { - self::assertSame('foo', $this->connector->fetch('baz', $this->options)); - self::assertSame('foo', $this->connector->fetch('baz', clone $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz', clone $this->options)); } - public function testCacheUsedForCacheKeyGenerator() + /** + * Tests that when the cache key generator returns the same hash the same data is fetched, and when it does not, + * fresh data is fetched. + */ + public function testCacheKeyGenerator() { $this->connector->setCacheKeyGenerator( \Mockery::mock(CacheKeyGenerator::class) ->shouldReceive('generateCacheKey') - ->with('quux', $this->options->copy()) - ->andReturn('quuz', 'quuz', 'corge') + ->with($source = 'baz', $this->options->copy()) + ->andReturn('qux', 'qux', 'quux') ->getMock() ); - self::assertSame('foo', $this->connector->fetch('quux', $this->options)); - self::assertSame('foo', $this->connector->fetch('quux', $this->options)); - self::assertSame('bar', $this->connector->fetch('quux', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, $source, $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, $source, $this->options)); + self::assertSame('bar', $this->connector->fetch($this->context, $source, $this->options)); } - public function testFetchThrowsInvalidCacheKeyExceptionOnNonStringCackeKey() + public function testFetchThrowsInvalidCacheKeyExceptionOnNonStringCacheKey() { $this->connector->setCacheKeyGenerator( \Mockery::mock(CacheKeyGenerator::class) ->shouldReceive('generateCacheKey') - ->with('quux', $this->options->copy()) - ->andReturn([]) + ->andReturn(1) ->getMock() ); $this->setExpectedException(InvalidCacheKeyException::class, 'Cache key must be a string.'); - $this->connector->fetch('quux', $this->options); + $this->connector->fetch($this->context, 'baz', $this->options); } public function testFetchThrowsInvalidCacheKeyExceptionOnNonPSR6CompliantCacheKey() { - $cacheKey = CachingConnector::RESERVED_CHARACTERS; - $this->connector->setCacheKeyGenerator( \Mockery::mock(CacheKeyGenerator::class) ->shouldReceive('generateCacheKey') - ->with('quux', $this->options->copy()) - ->andReturn($cacheKey) + ->andReturn(CachingConnector::RESERVED_CHARACTERS) ->getMock() ); $this->setExpectedException(InvalidCacheKeyException::class, 'contains one or more reserved characters'); - $this->connector->fetch('quux', $this->options); + $this->connector->fetch($this->context, 'baz', $this->options); } - public function testNullAndEmptyAreEquivalent() + public function testNullAndEmptyOptionsAreEquivalent() { /** @var EncapsulatedOptions $options */ $options = \Mockery::mock(EncapsulatedOptions::class)->shouldReceive('copy')->andReturn([])->getMock(); self::assertEmpty($options->copy()); - self::assertSame('foo', $this->connector->fetch('baz', $options)); - - self::assertSame('foo', $this->connector->fetch('baz')); - } - - public function testEnableCache() - { - self::assertTrue($this->connector->isCacheEnabled()); - - $this->connector->disableCache(); - self::assertFalse($this->connector->isCacheEnabled()); - - $this->connector->enableCache(); - self::assertTrue($this->connector->isCacheEnabled()); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); } + /** + * Tests that the default cache key generator does not output reserved characters even when comprised of options + * containing them. + */ public function testCacheKeyExcludesReservedCharacters() { $reservedCharacters = CachingConnector::RESERVED_CHARACTERS; @@ -164,6 +169,6 @@ function ($key) use ($reservedCharacters) { ->shouldReceive('getItem')->andReturnSelf() ->shouldReceive('set')->andReturn(\Mockery::mock(CacheItemInterface::class)); - $this->connector->fetch($reservedCharacters, (new TestOptions)->setFoo($reservedCharacters)); + $this->connector->fetch($this->context, $reservedCharacters, (new TestOptions)->setFoo($reservedCharacters)); } } diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index dbe81ad..7189818 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -5,14 +5,15 @@ use Mockery\MockInterface; use Psr\Container\ContainerInterface; use ScriptFUSION\Porter\Cache\CacheAdvice; -use ScriptFUSION\Porter\Cache\Cache; use ScriptFUSION\Porter\Cache\CacheUnavailableException; use ScriptFUSION\Porter\Collection\FilteredRecords; use ScriptFUSION\Porter\Collection\PorterRecords; use ScriptFUSION\Porter\Collection\ProviderRecords; use ScriptFUSION\Porter\Collection\RecordCollection; +use ScriptFUSION\Porter\Connector\ConnectionContext; use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Connector\RecoverableConnectorException; +use ScriptFUSION\Porter\Connector\SuperConnector; use ScriptFUSION\Porter\ImportException; use ScriptFUSION\Porter\Options\EncapsulatedOptions; use ScriptFUSION\Porter\Porter; @@ -48,6 +49,11 @@ final class PorterTest extends \PHPUnit_Framework_TestCase */ private $resource; + /** + * @var Connector|MockInterface + */ + private $connector; + /** * @var ImportSpecification */ @@ -60,9 +66,10 @@ final class PorterTest extends \PHPUnit_Framework_TestCase protected function setUp() { - $this->porter = new Porter($this->container = $container = \Mockery::spy(ContainerInterface::class)); + $this->porter = new Porter($this->container = \Mockery::spy(ContainerInterface::class)); $this->registerProvider($this->provider = MockFactory::mockProvider()); + $this->connector = $this->provider->getConnector(); $this->resource = MockFactory::mockResource($this->provider); $this->specification = new ImportSpecification($this->resource); } @@ -135,7 +142,7 @@ public function testImportOptions() MockFactory::mockResource($provider) ->shouldReceive('fetch') ->with( - \Mockery::type(Connector::class), + \Mockery::type(SuperConnector::class), \Mockery::on(function (EncapsulatedOptions $argument) use ($options) { self::assertNotSame($argument, $options, 'Options not cloned.'); @@ -185,11 +192,14 @@ public function testImportCustomProviderName() self::assertSame($output, $records->current()); } + /** + * Tests that when a resource does not return an iterator, ImportException is thrown. + */ public function testImportFailure() { $this->resource->shouldReceive('fetch')->andReturn(null); - $this->setExpectedException(ImportException::class, get_class($this->provider)); + $this->setExpectedException(ImportException::class, get_class($this->resource)); $this->porter->import($this->specification); } @@ -244,39 +254,60 @@ public function testImportOneOfMany() #region Durability + /** + * Tests that when a connector throws the recoverable exception type, the connection attempt is retried once. + */ public function testOneTry() { - $this->resource->shouldReceive('fetch')->once()->andThrow(RecoverableConnectorException::class); + $this->arrangeConnectorException(new RecoverableConnectorException); $this->setExpectedException(FailingTooHardException::class, '1'); $this->porter->import($this->specification->setMaxFetchAttempts(1)); } + /** + * Tests that when a connector throws an exception type derived from the recoverable exception type, the connection + * is retried. + */ public function testDerivedRecoverableException() { - $this->resource->shouldReceive('fetch')->once()->andThrow(\Mockery::mock(RecoverableConnectorException::class)); + $this->arrangeConnectorException(new RecoverableConnectorException); $this->setExpectedException(FailingTooHardException::class); $this->porter->import($this->specification->setMaxFetchAttempts(1)); } + /** + * Tests that when a connector throws the recoverable exception type, the connection can be retried the default + * number of times (more than once). + */ public function testDefaultTries() { - $this->resource->shouldReceive('fetch')->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS) - ->andThrow(RecoverableConnectorException::class); + $this->arrangeConnectorException(new RecoverableConnectorException); - $this->setExpectedException(FailingTooHardException::class, (string)ImportSpecification::DEFAULT_FETCH_ATTEMPTS); + $this->setExpectedException( + FailingTooHardException::class, + (string)ImportSpecification::DEFAULT_FETCH_ATTEMPTS + ); $this->porter->import($this->specification); } + /** + * Tests that when a connector throws a non-recoverable exception type, the connection is not retried. + */ public function testUnrecoverableException() { - $this->resource->shouldReceive('fetch')->once()->andThrow(\Exception::class); + // Subclass Exception so it's not an ancestor of any other exception. + $this->arrangeConnectorException($exception = \Mockery::mock(\Exception::class)); - $this->setExpectedException(\Exception::class); + $this->setExpectedException(get_class($exception)); $this->porter->import($this->specification); } + /** + * Tests that a when custom fetch exception handler is specified and the connector throws a recoverable exception + * type, the handler is called on each retry. + */ public function testCustomFetchExceptionHandler() { $this->specification->setFetchExceptionHandler( @@ -285,30 +316,13 @@ public function testCustomFetchExceptionHandler() ->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS - 1) ->getMock() ); - $this->resource->shouldReceive('fetch')->times(ImportSpecification::DEFAULT_FETCH_ATTEMPTS) - ->andThrow(RecoverableConnectorException::class); + + $this->arrangeConnectorException(new RecoverableConnectorException); $this->setExpectedException(FailingTooHardException::class); $this->porter->import($this->specification); } - /** - * Tests that when a generator throws a recoverable exception before the first yield, the fetch is retried. - * - * Note this does not support cases where exceptions may be thrown in subsequent iterations. - */ - public function testGeneratorException() - { - $this->resource->shouldReceive('fetch')->once()->andReturnUsing(function () { - throw new RecoverableConnectorException; - - yield; - }); - - $this->setExpectedException(FailingTooHardException::class, '1'); - $this->porter->import($this->specification->setMaxFetchAttempts(1)); - } - #endregion public function testFilter() @@ -330,21 +344,6 @@ public function testFilter() self::assertNotSame($previous->getFilter(), $filter, 'Filter was not cloned.'); } - public function testApplyCacheAdvice() - { - $this->provider->shouldReceive('getConnector') - ->andReturn( - \Mockery::mock(implode(',', [Connector::class, Cache::class])) - ->shouldReceive('disableCache')->once() - ->shouldReceive('enableCache')->once() - ->getMock() - ) - ; - - $this->porter->import($specification = new ImportSpecification($this->resource)); - $this->porter->import($specification->setCacheAdvice(CacheAdvice::SHOULD_CACHE())); - } - public function testCacheUnavailable() { $this->setExpectedException(CacheUnavailableException::class); @@ -365,4 +364,22 @@ private function registerProvider(Provider $provider, $name = null) ->shouldReceive('get')->with($name)->andReturn($provider)->byDefault() ; } + + /** + * Arranges for the current connector to throw an exception in the retry callback. + * + * @param \Exception $exception + */ + private function arrangeConnectorException(\Exception $exception) + { + $this->connector->shouldReceive('fetch')->with( + \Mockery::on(function (ConnectionContext $context) use ($exception) { + $context->retry(function () use ($exception) { + throw $exception; + }); + }), + \Mockery::any(), + \Mockery::any() + ); + } } diff --git a/test/MockFactory.php b/test/MockFactory.php index e3b9ac7..ea53ce6 100644 --- a/test/MockFactory.php +++ b/test/MockFactory.php @@ -3,6 +3,7 @@ use Mockery\MockInterface; use ScriptFUSION\Porter\Connector\Connector; +use ScriptFUSION\Porter\Connector\SuperConnector; use ScriptFUSION\Porter\Provider\Provider; use ScriptFUSION\Porter\Provider\ProviderOptions; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; @@ -19,8 +20,13 @@ public static function mockProvider() { return \Mockery::namedMock(uniqid(Provider::class, false), Provider::class) ->shouldReceive('getConnector') - ->andReturn(\Mockery::mock(Connector::class)) - ->byDefault() + ->andReturn( + \Mockery::mock(Connector::class) + ->shouldReceive('fetch') + ->andReturn('foo') + ->getMock() + ->byDefault() + ) ->getMock() ; } @@ -49,8 +55,8 @@ public static function mockResource(Provider $provider, \Iterator $return = null ->shouldReceive('getProviderClassName') ->andReturn(get_class($provider)) ->shouldReceive('fetch') - ->andReturnUsing(function () { - yield 'foo'; + ->andReturnUsing(function (SuperConnector $connector) { + return new \ArrayIterator([$connector->fetch('foo')]); }) ->byDefault() ->getMock() diff --git a/test/Unit/Porter/Connector/NullConnectorTest.php b/test/Unit/Porter/Connector/NullConnectorTest.php index 79f7d4f..d045437 100644 --- a/test/Unit/Porter/Connector/NullConnectorTest.php +++ b/test/Unit/Porter/Connector/NullConnectorTest.php @@ -2,11 +2,12 @@ namespace ScriptFUSIONTest\Unit\Porter\Connector; use ScriptFUSION\Porter\Connector\NullConnector; +use ScriptFUSIONTest\FixtureFactory; final class NullConnectorTest extends \PHPUnit_Framework_TestCase { public function test() { - self::assertNull((new NullConnector)->fetch('foo')); + self::assertNull((new NullConnector)->fetch(FixtureFactory::buildConnectionContext(), 'foo')); } } diff --git a/test/Unit/Porter/Provider/Resource/NullResourceTest.php b/test/Unit/Porter/Provider/Resource/NullResourceTest.php index 03a47e0..a4992f5 100644 --- a/test/Unit/Porter/Provider/Resource/NullResourceTest.php +++ b/test/Unit/Porter/Provider/Resource/NullResourceTest.php @@ -2,12 +2,18 @@ namespace ScriptFUSIONTest\Unit\Porter\Provider\Resource; use ScriptFUSION\Porter\Connector\Connector; +use ScriptFUSION\Porter\Connector\SuperConnector; use ScriptFUSION\Porter\Provider\Resource\NullResource; +use ScriptFUSIONTest\FixtureFactory; final class NullResourceTest extends \PHPUnit_Framework_TestCase { public function test() { - self::assertFalse((new NullResource)->fetch(\Mockery::mock(Connector::class))->valid()); + self::assertFalse( + (new NullResource)->fetch( + new SuperConnector(\Mockery::mock(Connector::class), FixtureFactory::buildConnectionContext()) + )->valid() + ); } } From 6b0f01622bd3cf22dca0a52436fd360a997c7500 Mon Sep 17 00:00:00 2001 From: Bilge Date: Sat, 15 Jul 2017 17:09:43 +0100 Subject: [PATCH 3/4] Changed code coverage provider Coveralls -> Codecov. --- .travis.yml | 3 +-- README.md | 4 ++-- 2 files changed, 3 insertions(+), 4 deletions(-) diff --git a/.travis.yml b/.travis.yml index 2582b2c..4169bb2 100644 --- a/.travis.yml +++ b/.travis.yml @@ -32,5 +32,4 @@ script: - composer test -- --coverage-clover=build/logs/clover.xml after_success: - - composer require satooshi/php-coveralls - - vendor/bin/coveralls -v + - bash <(curl -s https://codecov.io/bash) diff --git a/README.md b/README.md index e820af7..808df11 100644 --- a/README.md +++ b/README.md @@ -460,8 +460,8 @@ Porter is published under the open source GNU Lesser General Public License v3.0 [Downloads image]: https://poser.pugx.org/scriptfusion/porter/downloads "Total downloads" [Build]: http://travis-ci.org/ScriptFUSION/Porter [Build image]: https://travis-ci.org/ScriptFUSION/Porter.svg?branch=master "Build status" - [Coverage]: https://coveralls.io/github/ScriptFUSION/Porter - [Coverage image]: https://coveralls.io/repos/ScriptFUSION/Porter/badge.svg "Test coverage" + [Coverage]: https://codecov.io/gh/ScriptFUSION/Porter + [Coverage image]: https://codecov.io/gh/ScriptFUSION/Porter/branch/master/graphs/badge.svg "Test coverage" [Style]: https://styleci.io/repos/49824895 [Style image]: https://styleci.io/repos/49824895/shield?style=flat "Code style" From 5800e700448b22cdf1b2386e2003d619f31abe66 Mon Sep 17 00:00:00 2001 From: Bilge Date: Sun, 16 Jul 2017 17:06:44 +0100 Subject: [PATCH 4/4] Added SuperConnector unit test. --- src/Cache/Cache.php | 2 +- src/Connector/CachingConnector.php | 12 ++-- src/Connector/ConnectionContextFactory.php | 3 + .../Porter/Connector/CachingConnectorTest.php | 5 ++ .../Porter/Connector/SuperConnectorTest.php | 56 +++++++++++++++++++ 5 files changed, 71 insertions(+), 7 deletions(-) create mode 100644 test/Unit/Porter/Connector/SuperConnectorTest.php diff --git a/src/Cache/Cache.php b/src/Cache/Cache.php index 4e7d0f1..da2266d 100644 --- a/src/Cache/Cache.php +++ b/src/Cache/Cache.php @@ -7,7 +7,7 @@ interface Cache { /** - * Gets a value indicating whether the cache is available. + * Gets a value indicating whether the cache is available for read or write access. * * @return bool True if cache is available, otherwise false. */ diff --git a/src/Connector/CachingConnector.php b/src/Connector/CachingConnector.php index f52af5e..1dc6ab9 100644 --- a/src/Connector/CachingConnector.php +++ b/src/Connector/CachingConnector.php @@ -2,8 +2,8 @@ namespace ScriptFUSION\Porter\Connector; use Psr\Cache\CacheItemPoolInterface; -use ScriptFUSION\Porter\Cache\CacheKeyGenerator; use ScriptFUSION\Porter\Cache\Cache; +use ScriptFUSION\Porter\Cache\CacheKeyGenerator; use ScriptFUSION\Porter\Cache\InvalidCacheKeyException; use ScriptFUSION\Porter\Cache\JsonCacheKeyGenerator; use ScriptFUSION\Porter\Cache\MemoryCache; @@ -64,6 +64,11 @@ public function fetch(ConnectionContext $context, $source, EncapsulatedOptions $ abstract public function fetchFreshData($source, EncapsulatedOptions $options = null); + public function isCacheAvailable() + { + return true; + } + public function getCache() { return $this->cache; @@ -74,11 +79,6 @@ public function setCache(CacheItemPoolInterface $cache) $this->cache = $cache; } - public function isCacheAvailable() - { - return true; - } - public function getCacheKeyGenerator() { return $this->cacheKeyGenerator; diff --git a/src/Connector/ConnectionContextFactory.php b/src/Connector/ConnectionContextFactory.php index e93941a..eaaeb4a 100644 --- a/src/Connector/ConnectionContextFactory.php +++ b/src/Connector/ConnectionContextFactory.php @@ -2,9 +2,12 @@ namespace ScriptFUSION\Porter\Connector; use ScriptFUSION\Porter\Specification\ImportSpecification; +use ScriptFUSION\StaticClass; final class ConnectionContextFactory { + use StaticClass; + public static function create(ImportSpecification $specification) { return new ConnectionContext( diff --git a/test/Integration/Porter/Connector/CachingConnectorTest.php b/test/Integration/Porter/Connector/CachingConnectorTest.php index ddd9ce2..2a126dd 100644 --- a/test/Integration/Porter/Connector/CachingConnectorTest.php +++ b/test/Integration/Porter/Connector/CachingConnectorTest.php @@ -61,6 +61,11 @@ public function testCacheDisabled() self::assertSame('bar', $this->connector->fetch($context, 'baz', $this->options)); } + public function testCacheAvailable() + { + self::assertTrue($this->connector->isCacheAvailable()); + } + public function testGetSetCache() { self::assertInstanceOf(CacheItemPoolInterface::class, $this->connector->getCache()); diff --git a/test/Unit/Porter/Connector/SuperConnectorTest.php b/test/Unit/Porter/Connector/SuperConnectorTest.php new file mode 100644 index 0000000..c30c339 --- /dev/null +++ b/test/Unit/Porter/Connector/SuperConnectorTest.php @@ -0,0 +1,56 @@ +superConnector = new SuperConnector( + $this->connector = + \Mockery::spy(Connector::class, Cache::class) + ->shouldReceive('fetch') + ->andReturn('foo') + ->getMock(), + FixtureFactory::buildConnectionContext(CacheAdvice::MUST_CACHE()) + ); + } + + public function testCacheUnavailable() + { + $this->setExpectedException(CacheUnavailableException::class, 'unavailable'); + + $this->superConnector->fetch('foo'); + } + + /** + * Tests that when cache is optional no exception is thrown when connector does not support caching. + */ + public function testCacheOptional() + { + self::assertSame( + 'foo', + (new SuperConnector( + $this->connector, + FixtureFactory::buildConnectionContext(CacheAdvice::SHOULD_CACHE()) + ))->fetch('bar') + ); + } +}