diff --git a/.travis.yml b/.travis.yml index 4169bb2..bf1a665 100644 --- a/.travis.yml +++ b/.travis.yml @@ -10,6 +10,7 @@ php: - 5.6 - 7.0 - 7.1 + - 7.2 env: matrix: @@ -21,7 +22,7 @@ matrix: cache: directories: - - .composer/cache + - vendor install: - alias composer=composer\ -n && composer selfupdate diff --git a/src/Collection/RecordCollection.php b/src/Collection/RecordCollection.php index 09d6d54..5a5eb07 100644 --- a/src/Collection/RecordCollection.php +++ b/src/Collection/RecordCollection.php @@ -10,7 +10,7 @@ abstract class RecordCollection implements \Iterator private $previousCollection; - public function __construct(\Iterator $records, RecordCollection $previousCollection = null) + public function __construct(\Iterator $records, self $previousCollection = null) { $this->records = $records; $this->previousCollection = $previousCollection; diff --git a/src/Connector/CachingConnector.php b/src/Connector/CachingConnector.php index e163d11..f04b801 100644 --- a/src/Connector/CachingConnector.php +++ b/src/Connector/CachingConnector.php @@ -6,7 +6,6 @@ use ScriptFUSION\Porter\Cache\InvalidCacheKeyException; use ScriptFUSION\Porter\Cache\JsonCacheKeyGenerator; use ScriptFUSION\Porter\Cache\MemoryCache; -use ScriptFUSION\Porter\Options\EncapsulatedOptions; /** * Wraps a connector to cache fetched data using PSR-6-compliant objects. @@ -39,28 +38,27 @@ public function __construct( } /** + * @param ConnectionContext $context * @param string $source - * @param EncapsulatedOptions|null $options * * @return mixed * * @throws InvalidCacheKeyException */ - public function fetch(ConnectionContext $context, $source, EncapsulatedOptions $options = null) + public function fetch(ConnectionContext $context, $source) { if ($context->mustCache()) { - $optionsCopy = $options ? $options->copy() : []; + $options = $this->connector instanceof ConnectorOptions ? $this->connector->getOptions()->copy() : []; + ksort($options); - ksort($optionsCopy); - - $this->validateCacheKey($key = $this->cacheKeyGenerator->generateCacheKey($source, $optionsCopy)); + $this->validateCacheKey($key = $this->cacheKeyGenerator->generateCacheKey($source, $options)); if ($this->cache->hasItem($key)) { return $this->cache->getItem($key)->get(); } } - $data = $this->connector->fetch($context, $source, $options); + $data = $this->connector->fetch($context, $source); isset($key) && $this->cache->save($this->cache->getItem($key)->set($data)); diff --git a/src/Connector/Connector.php b/src/Connector/Connector.php index 52ca67e..e3cd5b0 100644 --- a/src/Connector/Connector.php +++ b/src/Connector/Connector.php @@ -1,8 +1,6 @@ context = $context; } - public function fetch($source, EncapsulatedOptions $options = null) + public function fetch($source) { - return $this->connector->fetch($this->context, $source, $options); + return $this->connector->fetch($this->context, $source); + } + + /** + * Gets the wrapped connector. Useful for resources to reconfigure connector options during this import. + * + * @return Connector + */ + public function getWrappedConnector() + { + return $this->connector; } } diff --git a/src/Connector/NullConnector.php b/src/Connector/NullConnector.php index 314be18..71df7ae 100644 --- a/src/Connector/NullConnector.php +++ b/src/Connector/NullConnector.php @@ -1,11 +1,9 @@ fetch( - new ImportConnector($provider->getConnector(), $context), - $provider instanceof ProviderOptions ? clone $provider->getOptions() : null - ); + $connector = $provider->getConnector(); + + /* __clone method cannot be specified in interface due to Mockery limitation. + See https://github.com/mockery/mockery/issues/669 */ + if ($connector instanceof ConnectorOptions && !method_exists($connector, '__clone')) { + throw new \LogicException( + 'Connector with options must implement __clone() method to deep clone options.' + ); + } + + $records = $resource->fetch(new ImportConnector(clone $connector, $context)); if (!$records instanceof \Iterator) { throw new ImportException(get_class($resource) . '::fetch() did not return an Iterator.'); diff --git a/src/Provider/ProviderOptions.php b/src/Provider/ProviderOptions.php deleted file mode 100644 index f2d2b68..0000000 --- a/src/Provider/ProviderOptions.php +++ /dev/null @@ -1,17 +0,0 @@ -options = new TestOptions; + $this->connector = new CachingConnector( - $this->wrappedConnector = \Mockery::mock(Connector::class) + $this->wrappedConnector = \Mockery::mock(Connector::class, ConnectorOptions::class) ->shouldReceive('fetch') - ->andReturn('foo', 'bar') + ->andReturn('foo', 'bar') + ->shouldReceive('getOptions') + ->andReturn($this->options) + ->byDefault() + ->shouldReceive('clone') + ->andReturn(null) ->getMock() ); $this->context = FixtureFactory::buildConnectionContext(true); - - $this->options = new TestOptions; } /** @@ -57,8 +63,8 @@ protected function setUp() */ public function testCacheEnabled() { - self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); - self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); } /** @@ -69,8 +75,8 @@ public function testCacheDisabled() // The default connection context has caching disabled. $context = FixtureFactory::buildConnectionContext(); - self::assertSame('foo', $this->connector->fetch($context, 'baz', $this->options)); - self::assertSame('bar', $this->connector->fetch($context, 'baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($context, 'baz')); + self::assertSame('bar', $this->connector->fetch($context, 'baz')); } /** @@ -78,10 +84,10 @@ public function testCacheDisabled() */ public function testCacheBypassedForDifferentOptions() { - self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); $this->options->setFoo('bar'); - self::assertSame('bar', $this->connector->fetch($this->context, 'baz', $this->options)); + self::assertSame('bar', $this->connector->fetch($this->context, 'baz')); } /** @@ -89,17 +95,25 @@ public function testCacheBypassedForDifferentOptions() */ public function testCacheUsedForDifferentOptionsInstance() { - self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $this->options)); - self::assertSame('foo', $this->connector->fetch($this->context, 'baz', clone $this->options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); + + $this->wrappedConnector->shouldReceive('getOptions')->andReturn($options = clone $this->options); + self::assertNotSame($this->options, $options); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); + + // Ensure new options have really taken effect by changing option. Cache should no longer be used. + $options->setFoo('bar'); + self::assertSame('bar', $this->connector->fetch($this->context, 'baz')); } public function testNullAndEmptyOptionsAreEquivalent() { /** @var EncapsulatedOptions $options */ $options = \Mockery::mock(EncapsulatedOptions::class)->shouldReceive('copy')->andReturn([])->getMock(); + $this->wrappedConnector->shouldReceive('getOptions')->andReturn($options); + self::assertEmpty($this->wrappedConnector->getOptions()->copy()); - self::assertEmpty($options->copy()); - self::assertSame('foo', $this->connector->fetch($this->context, 'baz', $options)); + self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); self::assertSame('foo', $this->connector->fetch($this->context, 'baz')); } @@ -125,7 +139,7 @@ function ($key) use ($reservedCharacters) { ->shouldReceive('set')->andReturn(\Mockery::mock(CacheItemInterface::class)) ; - $connector->fetch($this->context, $reservedCharacters, (new TestOptions)->setFoo($reservedCharacters)); + $connector->fetch($this->context, $reservedCharacters); } /** @@ -143,9 +157,9 @@ public function testCacheKeyGenerator() ->getMock() ); - self::assertSame('foo', $connector->fetch($this->context, $source, $this->options)); - self::assertSame('foo', $connector->fetch($this->context, $source, $this->options)); - self::assertSame('bar', $connector->fetch($this->context, $source, $this->options)); + self::assertSame('foo', $connector->fetch($this->context, $source)); + self::assertSame('foo', $connector->fetch($this->context, $source)); + self::assertSame('bar', $connector->fetch($this->context, $source)); } /** @@ -162,7 +176,7 @@ public function testFetchThrowsInvalidCacheKeyExceptionOnNonStringCacheKey() ); $this->setExpectedException(InvalidCacheKeyException::class, 'Cache key must be a string.'); - $connector->fetch($this->context, 'baz', $this->options); + $connector->fetch($this->context, 'baz'); } public function testFetchThrowsInvalidCacheKeyExceptionOnNonPSR6CompliantCacheKey() @@ -176,7 +190,7 @@ public function testFetchThrowsInvalidCacheKeyExceptionOnNonPSR6CompliantCacheKe ); $this->setExpectedException(InvalidCacheKeyException::class, 'contains one or more reserved characters'); - $connector->fetch($this->context, 'baz', $this->options); + $connector->fetch($this->context, 'baz'); } private function createConnector(MockInterface $cache = null, MockInterface $cacheKeyGenerator = null) diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index 5d9ad3e..5fdcb19 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -11,10 +11,9 @@ use ScriptFUSION\Porter\Collection\RecordCollection; use ScriptFUSION\Porter\Connector\ConnectionContext; use ScriptFUSION\Porter\Connector\Connector; -use ScriptFUSION\Porter\Connector\ImportConnector; +use ScriptFUSION\Porter\Connector\ConnectorOptions; use ScriptFUSION\Porter\Connector\RecoverableConnectorException; use ScriptFUSION\Porter\ImportException; -use ScriptFUSION\Porter\Options\EncapsulatedOptions; use ScriptFUSION\Porter\Porter; use ScriptFUSION\Porter\PorterAware; use ScriptFUSION\Porter\Provider\ForeignResourceException; @@ -125,33 +124,15 @@ public function testImportAndFilterCountableRecords() } /** - * Tests that a provider implementing ProviderOptions has a clone of its options passed to ProviderResource::fetch - * at import time. + * Tests that when importing using a connector that exports options, but no clone method, an exception is thrown. */ - public function testImportOptions() + public function testImportConnectorWithOptions() { - $this->registerProvider( - $provider = MockFactory::mockProviderOptions() - ->shouldReceive('getOptions') - ->andReturn($options = \Mockery::mock(EncapsulatedOptions::class)) - ->getMock() - ); + $this->provider->shouldReceive('getConnector') + ->andReturn(\Mockery::mock(Connector::class, ConnectorOptions::class)); - $this->porter->import(new ImportSpecification( - MockFactory::mockResource($provider) - ->shouldReceive('fetch') - ->with( - \Mockery::type(ImportConnector::class), - \Mockery::on(function (EncapsulatedOptions $argument) use ($options) { - self::assertNotSame($argument, $options, 'Options not cloned.'); - - return get_class($argument) === get_class($options); - }) - ) - ->andReturn(new \EmptyIterator) - ->once() - ->getMock() - )); + $this->setExpectedException(\LogicException::class); + $this->porter->import($this->specification); } /** @@ -380,7 +361,6 @@ private function arrangeConnectorException(\Exception $exception) throw $exception; }); }), - \Mockery::any(), \Mockery::any() ); } diff --git a/test/MockFactory.php b/test/MockFactory.php index 1400bf6..4576a2a 100644 --- a/test/MockFactory.php +++ b/test/MockFactory.php @@ -5,7 +5,6 @@ use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Connector\ImportConnector; use ScriptFUSION\Porter\Provider\Provider; -use ScriptFUSION\Porter\Provider\ProviderOptions; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; use ScriptFUSION\StaticClass; @@ -27,18 +26,7 @@ public static function mockProvider() ->getMock() ->byDefault() ) - ->getMock() - ; - } - - /** - * @return Provider|MockInterface - */ - public static function mockProviderOptions() - { - return \Mockery::mock(Provider::class, ProviderOptions::class) - ->shouldReceive('getConnector') - ->andReturn(\Mockery::mock(Connector::class)) + ->byDefault() ->getMock() ; } diff --git a/test/Unit/Porter/Connector/ImportConnectorTest.php b/test/Unit/Porter/Connector/ImportConnectorTest.php index e62a234..c9eed19 100644 --- a/test/Unit/Porter/Connector/ImportConnectorTest.php +++ b/test/Unit/Porter/Connector/ImportConnectorTest.php @@ -5,7 +5,6 @@ use ScriptFUSION\Porter\Connector\CachingConnector; use ScriptFUSION\Porter\Connector\Connector; use ScriptFUSION\Porter\Connector\ImportConnector; -use ScriptFUSION\Porter\Options\EncapsulatedOptions; use ScriptFUSIONTest\FixtureFactory; /** @@ -14,8 +13,7 @@ final class ImportConnectorTest extends \PHPUnit_Framework_TestCase { /** - * Tests that when fetching, the specified context, source and options are passed verbatim to the underlying - * connector. + * Tests that when fetching, the specified context and source are passed verbatim to the underlying connector. */ public function testCallGraph() { @@ -24,15 +22,14 @@ public function testCallGraph() ->shouldReceive('fetch') ->with( $context = FixtureFactory::buildConnectionContext(), - $source = 'bar', - $options = \Mockery::mock(EncapsulatedOptions::class) + $source = 'bar' )->once() ->andReturn($output = 'foo') ->getMock(), $context ); - self::assertSame($output, $connector->fetch($source, $options)); + self::assertSame($output, $connector->fetch($source)); } /** @@ -79,4 +76,17 @@ public function testFetchCacheEnabledButNotAvailable() FixtureFactory::buildConnectionContext(true) ); } + + /** + * Tests that getting the wrapped connector returns exactly the same connector as constructed with. + */ + public function testGetWrappedConnector() + { + $connector = new ImportConnector( + $wrappedConnector = \Mockery::mock(Connector::class), + FixtureFactory::buildConnectionContext() + ); + + self::assertSame($wrappedConnector, $connector->getWrappedConnector()); + } }