diff --git a/.travis.yml b/.travis.yml index 8c4fd15..2582b2c 100644 --- a/.travis.yml +++ b/.travis.yml @@ -31,11 +31,6 @@ install: script: - composer test -- --coverage-clover=build/logs/clover.xml - # Remove mapper and run all non-Mapper tests again. - - composer --dev --no-update-with-dependencies remove scriptfusion/mapper - - '! composer info | grep ^scriptfusion/mapper\\b' - - composer test -- --exclude-group Mapper - after_success: - composer require satooshi/php-coveralls - vendor/bin/coveralls -v diff --git a/composer.json b/composer.json index c36667d..8723548 100644 --- a/composer.json +++ b/composer.json @@ -18,13 +18,12 @@ "zendframework/zend-uri": "^2" }, "require-dev": { - "scriptfusion/mapper": "^1", "phpunit/phpunit": "^4.8", "mockery/mockery": "^0.9.4", "symfony/process": "^3" }, "suggest" : { - "scriptfusion/mapper": "Transforms imported data." + "transformers/mapping": "Transforms array collections using Mappings." }, "autoload": { "psr-4": { diff --git a/src/Collection/CountableMappedRecords.php b/src/Collection/CountableMappedRecords.php deleted file mode 100644 index 04211fe..0000000 --- a/src/Collection/CountableMappedRecords.php +++ /dev/null @@ -1,21 +0,0 @@ -setCount($count); - } -} diff --git a/src/Collection/MappedRecords.php b/src/Collection/MappedRecords.php deleted file mode 100644 index 1a65691..0000000 --- a/src/Collection/MappedRecords.php +++ /dev/null @@ -1,25 +0,0 @@ -mapping = $mapping; - } - - /** - * @return Mapping - */ - public function getMapping() - { - return $this->mapping; - } -} diff --git a/src/Mapper/PorterMapper.php b/src/Mapper/PorterMapper.php deleted file mode 100644 index 26828f3..0000000 --- a/src/Mapper/PorterMapper.php +++ /dev/null @@ -1,25 +0,0 @@ -porter = $porter; - } - - protected function injectDependencies($object) - { - parent::injectDependencies($object); - - if ($object instanceof PorterAware) { - $object->setPorter($this->porter); - } - } -} diff --git a/src/Mapper/Strategy/InvalidCallbackResultException.php b/src/Mapper/Strategy/InvalidCallbackResultException.php deleted file mode 100644 index 7e8ab19..0000000 --- a/src/Mapper/Strategy/InvalidCallbackResultException.php +++ /dev/null @@ -1,10 +0,0 @@ -specificationOrCallback = $specificationOrCallback; - } - - public function __invoke($data, $context = null) - { - $specification = clone $this->getOrCreateImportSpecification($data, $context); - $specification->setContext($context); - - $generator = $this->getPorter()->import($specification); - - if ($generator->valid()) { - return iterator_to_array($generator); - } - } - - private function getOrCreateImportSpecification($data, $context = null) - { - if ($this->specificationOrCallback instanceof ImportSpecification) { - return $this->specificationOrCallback; - } - - if (($specification = call_user_func($this->specificationOrCallback, $data, $context)) - instanceof ImportSpecification - ) { - return $specification; - } - - throw new InvalidCallbackResultException('Callback failed to create an instance of ImportSpecification.'); - } -} diff --git a/src/Porter.php b/src/Porter.php index 3a998e7..0b2e00f 100644 --- a/src/Porter.php +++ b/src/Porter.php @@ -1,26 +1,21 @@ defaultCacheAdvice = CacheAdvice::SHOULD_NOT_CACHE(); - $this->fetchExceptionHandler = new ExponentialBackoffExceptionHandler; } /** @@ -84,13 +73,7 @@ public function import(ImportSpecification $specification) $records = $this->createProviderRecords($records, $specification->getResource()); } - if ($specification->getFilter()) { - $records = $this->filter($records, $specification->getFilter(), $specification->getContext()); - } - - if ($specification->getMapping()) { - $records = $this->map($records, $specification->getMapping(), $specification->getContext()); - } + $records = $this->transformRecords($records, $specification->getTransformers(), $specification->getContext()); return $this->createPorterRecords($records, $specification); } @@ -121,27 +104,10 @@ public function importOne(ImportSpecification $specification) return $one; } - private function createProviderRecords(\Iterator $records, ProviderResource $resource) - { - if ($records instanceof \Countable) { - return new CountableProviderRecords($records, count($records), $resource); - } - - return new ProviderRecords($records, $resource); - } - - private function createPorterRecords(RecordCollection $records, ImportSpecification $specification) - { - if ($records instanceof \Countable) { - return new CountablePorterRecords($records, count($records), $specification); - } - - return new PorterRecords($records, $specification); - } - private function fetch(ProviderResource $resource, CacheAdvice $cacheAdvice = null) { $provider = $this->getProvider($resource->getProviderClassName(), $resource->getProviderTag()); + $this->applyCacheAdvice($provider, $cacheAdvice ?: $this->defaultCacheAdvice); if (($records = \ScriptFUSION\Retry\retry( @@ -164,35 +130,42 @@ function (\Exception $exception) { throw new ImportException(get_class($provider) . '::fetch() did not return an Iterator.'); } - private function filter(ProviderRecords $records, callable $predicate, $context) + /** + * @param RecordCollection $records + * @param Transformer[] $transformers + * @param mixed $context + * + * @return RecordCollection + */ + private function transformRecords(RecordCollection $records, array $transformers, $context) { - $filter = function () use ($records, $predicate, $context) { - foreach ($records as $record) { - if ($predicate($record, $context)) { - yield $record; - } + foreach ($transformers as $transformer) { + if ($transformer instanceof PorterAware) { + $transformer->setPorter($this); } - }; - return new FilteredRecords($filter(), $records, $filter); + $records = $transformer->transform($records, $context); + } + + return $records; } - private function map(RecordCollection $records, Mapping $mapping, $context) + private function createProviderRecords(\Iterator $records, ProviderResource $resource) { - return $this->createMappedRecords( - $this->getOrCreateMapper()->mapCollection($records, $mapping, $context), - $records, - $mapping - ); + if ($records instanceof \Countable) { + return new CountableProviderRecords($records, count($records), $resource); + } + + return new ProviderRecords($records, $resource); } - private function createMappedRecords(\Iterator $records, RecordCollection $previous, Mapping $mapping) + private function createPorterRecords(RecordCollection $records, ImportSpecification $specification) { - if ($previous instanceof \Countable) { - return new CountableMappedRecords($records, count($previous), $previous, $mapping); + if ($records instanceof \Countable) { + return new CountablePorterRecords($records, count($records), $specification); } - return new MappedRecords($records, $previous, $mapping); + return new PorterRecords($records, $specification); } private function applyCacheAdvice(Provider $provider, CacheAdvice $cacheAdvice) @@ -266,7 +239,7 @@ public function getProvider($name, $tag = null) return $provider; } } catch (ObjectNotCreatedException $exception) { - // Intentionally empty. + // We will throw our own exception. } throw new ProviderNotFoundException( @@ -304,26 +277,6 @@ private function getOrCreateProviderFactory() return $this->providerFactory ?: $this->providerFactory = new ProviderFactory; } - /** - * @return CollectionMapper - */ - private function getOrCreateMapper() - { - return $this->mapper ?: $this->mapper = new PorterMapper($this); - } - - /** - * @param CollectionMapper $mapper - * - * @return $this - */ - public function setMapper(CollectionMapper $mapper) - { - $this->mapper = $mapper; - - return $this; - } - /** * Gets the maximum number of fetch attempts per import. * @@ -353,7 +306,7 @@ public function setMaxFetchAttempts($attempts) */ private function getFetchExceptionHandler() { - return $this->fetchExceptionHandler; + return $this->fetchExceptionHandler ?: $this->fetchExceptionHandler = new ExponentialBackoffExceptionHandler; } /** diff --git a/src/Specification/DuplicateTransformerException.php b/src/Specification/DuplicateTransformerException.php new file mode 100644 index 0000000..d523609 --- /dev/null +++ b/src/Specification/DuplicateTransformerException.php @@ -0,0 +1,10 @@ +resource = $resource; + $this->clearTransformers(); } public function __clone() { $this->resource = clone $this->resource; - $this->mapping !== null && $this->mapping = clone $this->mapping; + + $transformers = $this->transformers; + $this->clearTransformers()->addTransformers(array_map( + function (Transformer $transformer) { + return clone $transformer; + }, + $transformers + )); + is_object($this->context) && $this->context = clone $this->context; } @@ -46,61 +60,78 @@ final public function getResource() } /** - * @return Mapping + * @return Transformer[] */ - final public function getMapping() + final public function getTransformers() { - return $this->mapping; + return $this->transformers; } /** - * @param Mapping $mapping + * Adds the specified transformer. + * + * @param Transformer $transformer Transformer. * * @return $this */ - final public function setMapping(Mapping $mapping = null) + final public function addTransformer(Transformer $transformer) { - $this->mapping = $mapping; + if ($this->hasTransformer($transformer)) { + throw new DuplicateTransformerException('Transformer already added.'); + } + + $this->transformers[spl_object_hash($transformer)] = $transformer; return $this; } /** - * @return mixed + * Adds one or more transformers. + * + * @param Transformer[] $transformers Transformers. + * + * @return $this */ - final public function getContext() + final public function addTransformers(array $transformers) { - return $this->context; + foreach ($transformers as $transformer) { + $this->addTransformer($transformer); + } + + return $this; } /** - * @param mixed $context - * * @return $this */ - final public function setContext($context) + final public function clearTransformers() { - $this->context = $context; + $this->transformers = []; return $this; } + private function hasTransformer(Transformer $transformer) + { + return isset($this->transformers[spl_object_hash($transformer)]); + } + /** - * @return callable + * @return mixed */ - final public function getFilter() + final public function getContext() { - return $this->filter; + return $this->context; } /** - * @param callable $filter + * @param mixed $context * * @return $this */ - final public function setFilter(callable $filter = null) + final public function setContext($context) { - $this->filter = $filter; + $this->context = $context; return $this; } diff --git a/src/Transform/FilterTransformer.php b/src/Transform/FilterTransformer.php new file mode 100644 index 0000000..52d8603 --- /dev/null +++ b/src/Transform/FilterTransformer.php @@ -0,0 +1,34 @@ +filter = $filter; + } + + public function transform(RecordCollection $records, $context) + { + $filter = function ($predicate) use ($records, $context) { + foreach ($records as $record) { + if ($predicate($record, $context)) { + yield $record; + } + } + }; + + return new FilteredRecords($filter($this->filter), $records, $filter); + } +} diff --git a/src/Transform/Transformer.php b/src/Transform/Transformer.php new file mode 100644 index 0000000..9ba857c --- /dev/null +++ b/src/Transform/Transformer.php @@ -0,0 +1,20 @@ +setPorter(new Porter); - - $records = $import(null); - - self::assertSame($record, $records[0]); - } -} diff --git a/test/Integration/Porter/PorterTest.php b/test/Integration/Porter/PorterTest.php index a9caaf2..46789cd 100644 --- a/test/Integration/Porter/PorterTest.php +++ b/test/Integration/Porter/PorterTest.php @@ -3,19 +3,17 @@ use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration; use Mockery\MockInterface; -use ScriptFUSION\Mapper\CollectionMapper; -use ScriptFUSION\Mapper\Mapping; use ScriptFUSION\Porter\Cache\CacheAdvice; use ScriptFUSION\Porter\Cache\CacheToggle; use ScriptFUSION\Porter\Cache\CacheUnavailableException; -use ScriptFUSION\Porter\Collection\CountableMappedRecords; use ScriptFUSION\Porter\Collection\FilteredRecords; -use ScriptFUSION\Porter\Collection\MappedRecords; use ScriptFUSION\Porter\Collection\PorterRecords; use ScriptFUSION\Porter\Collection\ProviderRecords; +use ScriptFUSION\Porter\Collection\RecordCollection; use ScriptFUSION\Porter\Connector\RecoverableConnectorException; use ScriptFUSION\Porter\ImportException; use ScriptFUSION\Porter\Porter; +use ScriptFUSION\Porter\PorterAware; use ScriptFUSION\Porter\Provider\Provider; use ScriptFUSION\Porter\Provider\Resource\ProviderResource; use ScriptFUSION\Porter\Provider\StaticDataProvider; @@ -23,6 +21,8 @@ use ScriptFUSION\Porter\ProviderNotFoundException; use ScriptFUSION\Porter\Specification\ImportSpecification; use ScriptFUSION\Porter\Specification\StaticDataImportSpecification; +use ScriptFUSION\Porter\Transform\FilterTransformer; +use ScriptFUSION\Porter\Transform\Transformer; use ScriptFUSION\Retry\ExceptionHandler\ExponentialBackoffExceptionHandler; use ScriptFUSION\Retry\FailingTooHardException; use ScriptFUSIONTest\MockFactory; @@ -156,25 +156,6 @@ public function testImportCountableRecords() self::assertCount($count, $records); } - /** - * Tests that when the resource is countable the count is propagated to the outermost collection via a mapped - * collection. - * - * @group Mapper - */ - public function testImportAndMapCountableRecords() - { - $records = $this->porter->import( - (new StaticDataImportSpecification( - new \ArrayIterator(range(1, $count = 10)) - ))->setMapping(\Mockery::mock(Mapping::class)) - ); - - self::assertInstanceOf(CountableMappedRecords::class, $records->getPreviousCollection()); - self::assertInstanceOf(\Countable::class, $records); - self::assertCount($count, $records); - } - /** * Tests that when the resource is countable the count is lost when filtering is applied. */ @@ -182,8 +163,8 @@ public function testImportAndFilterCountableRecords() { $records = $this->porter->import( (new StaticDataImportSpecification( - new \ArrayIterator(range(1, $count = 10)) - ))->setFilter([$this, __FUNCTION__]) + new \ArrayIterator(range(1, 10)) + ))->addTransformer(new FilterTransformer([$this, __FUNCTION__])) ); // Innermost collection. @@ -193,6 +174,24 @@ public function testImportAndFilterCountableRecords() self::assertNotInstanceOf(\Countable::class, $records); } + /** + * Tests that when a Transformer is PorterAware it receives the Porter instance that invoked it. + */ + public function testPorterAwareTransformer() + { + $this->porter->import( + $this->specification->addTransformer( + \Mockery::mock(implode(',', [Transformer::class, PorterAware::class])) + ->shouldReceive('setPorter') + ->with($this->porter) + ->once() + ->shouldReceive('transform') + ->andReturn(\Mockery::mock(RecordCollection::class)) + ->getMock() + ) + ); + } + public function testImportTaggedResource() { $this->porter->registerProvider( @@ -243,10 +242,9 @@ public function testImportOneOfNone() public function testImportOneOfMany() { - $this->setExpectedException(ImportException::class); - $this->provider->shouldReceive('fetch')->andReturn(new \ArrayIterator(['foo', 'bar'])); + $this->setExpectedException(ImportException::class); $this->porter->importOne($this->specification); } @@ -310,9 +308,9 @@ public function testFilter() $records = $this->porter->import( $this->specification - ->setFilter(function ($record) { + ->addTransformer(new FilterTransformer($filter = function ($record) { return $record % 2; - }) + })) ); self::assertInstanceOf(PorterRecords::class, $records); @@ -320,26 +318,7 @@ public function testFilter() /** @var FilteredRecords $previous */ self::assertInstanceOf(FilteredRecords::class, $previous = $records->getPreviousCollection()); - self::assertNotSame($previous->getFilter(), $this->specification->getFilter(), 'Filter was not cloned.'); - } - - public function testMap() - { - $records = $this->porter->setMapper( - \Mockery::mock(CollectionMapper::class) - ->shouldReceive('mapCollection') - ->with(\Mockery::type(\Iterator::class), $mapping = \Mockery::type(Mapping::class), \Mockery::any()) - ->once() - ->andReturn(new \ArrayIterator($result = ['foo' => 'bar'])) - ->getMock() - )->import($this->specification->setMapping(\Mockery::mock(Mapping::class))); - - self::assertInstanceOf(PorterRecords::class, $records); - self::assertSame($result, iterator_to_array($records)); - - /** @var MappedRecords $previous */ - self::assertInstanceOf(MappedRecords::class, $previous = $records->getPreviousCollection()); - self::assertNotSame($mapping, $previous->getMapping(), 'Mapping was not cloned.'); + self::assertNotSame($previous->getFilter(), $filter, 'Filter was not cloned.'); } public function testApplyCacheAdvice() diff --git a/test/Unit/Porter/ImportSpecificationTest.php b/test/Unit/Porter/ImportSpecificationTest.php index 47c1788..618c090 100644 --- a/test/Unit/Porter/ImportSpecificationTest.php +++ b/test/Unit/Porter/ImportSpecificationTest.php @@ -1,10 +1,11 @@ specification->setMapping($mapping = \Mockery::mock(Mapping::class))->setContext($context = (object)[]); + $this->specification + ->addTransformer(\Mockery::mock(Transformer::class)) + ->setContext($context = (object)[]); $specification = clone $this->specification; self::assertNotSame($this->resource, $specification->getResource()); - self::assertNotSame($mapping, $specification->getMapping()); + + self::assertNotSame( + array_values($this->specification->getTransformers()), + array_values($specification->getTransformers()) + ); + self::assertNotSame( + array_keys($this->specification->getTransformers()), + array_keys($specification->getTransformers()) + ); + self::assertCount(count($this->specification->getTransformers()), $specification->getTransformers()); + self::assertNotSame($context, $specification->getContext()); } @@ -36,25 +49,45 @@ public function testProviderData() self::assertSame($this->resource, $this->specification->getResource()); } - public function testMapping() + public function testAddTransformer() { - self::assertSame( - $mapping = \Mockery::mock(Mapping::class), - $this->specification->setMapping($mapping)->getMapping() - ); + self::assertEmpty($this->specification->getTransformers()); + + $this->specification->addTransformer($transformer1 = \Mockery::mock(Transformer::class)); + self::assertCount(1, $this->specification->getTransformers()); + self::assertContains($transformer1, $this->specification->getTransformers()); + + $this->specification->addTransformer($transformer2 = \Mockery::mock(Transformer::class)); + self::assertCount(2, $this->specification->getTransformers()); + self::assertContains($transformer1, $this->specification->getTransformers()); + self::assertContains($transformer2, $this->specification->getTransformers()); } - public function testContext() + public function testAddTransformers() { - self::assertSame('foo', $this->specification->setContext('foo')->getContext()); + self::assertEmpty($this->specification->getTransformers()); + + $this->specification->addTransformers([ + $transformer1 = \Mockery::mock(Transformer::class), + $transformer2 = \Mockery::mock(Transformer::class), + ]); + + self::assertCount(2, $this->specification->getTransformers()); + self::assertContains($transformer1, $this->specification->getTransformers()); + self::assertContains($transformer2, $this->specification->getTransformers()); } - public function testFilter() + public function testAddSameTransformer() { - self::assertSame( - $filter = [$this, __FUNCTION__], - $this->specification->setFilter($filter)->getFilter() - ); + $this->specification->addTransformer($transformer1 = \Mockery::mock(Transformer::class)); + + $this->setExpectedException(DuplicateTransformerException::class); + $this->specification->addTransformer($transformer1); + } + + public function testContext() + { + self::assertSame('foo', $this->specification->setContext('foo')->getContext()); } public function testCacheAdvice() diff --git a/test/Unit/Porter/Mapper/PorterMapperTest.php b/test/Unit/Porter/Mapper/PorterMapperTest.php deleted file mode 100644 index 1a1f458..0000000 --- a/test/Unit/Porter/Mapper/PorterMapperTest.php +++ /dev/null @@ -1,41 +0,0 @@ -makePartial(); - - $mappedRecords = $mapper->mapCollection( - $records, - new AnonymousMapping([$strategy = \Mockery::mock(implode(',', [Strategy::class, PorterAware::class]))]) - ); - - $strategy->shouldReceive('__invoke')->andReturnUsing(function ($data) { - return $data[0] * $data[0]; - })->getMock()->shouldReceive('setPorter')->with($porter)->atLeast()->once(); - - self::assertSame([[1], [4], [9]], iterator_to_array($mappedRecords)); - } -} diff --git a/test/Unit/Porter/Mapper/Strategy/SubImportTest.php b/test/Unit/Porter/Mapper/Strategy/SubImportTest.php deleted file mode 100644 index 0360626..0000000 --- a/test/Unit/Porter/Mapper/Strategy/SubImportTest.php +++ /dev/null @@ -1,87 +0,0 @@ -createSubImport(); - } - - public function testInvalidCreate() - { - $this->setExpectedException(\InvalidArgumentException::class); - - $this->createSubImport(true); - } - - public function testImport() - { - self::assertNull($this->import()); - - $this->porter->shouldReceive('import')->andReturn(new \ArrayIterator($array = range(1, 5))); - self::assertSame($array, $this->import()); - } - - public function testSpecificationCallback() - { - $this->createSubImport( - function ($data, $context) { - self::assertSame('foo', $data); - self::assertSame('bar', $context); - - return MockFactory::mockImportSpecification(); - } - ); - - $this->import('foo', 'bar'); - } - - public function testInvalidSpecificationCallback() - { - $this->setExpectedException(InvalidCallbackResultException::class); - - $this->createSubImport(function () { - // Intentionally empty. - }); - - $this->import(); - } - - private function createSubImport($specification = null) - { - $this->subImport = $subImport = new SubImport($specification ?: MockFactory::mockImportSpecification()); - - $subImport->setPorter( - $this->porter = \Mockery::mock(Porter::class) - ->shouldReceive('import') - ->andReturn(new \EmptyIterator) - ->byDefault() - ->getMock() - ); - } - - private function import($data = null, $context = null) - { - return call_user_func($this->subImport, $data, $context); - } -} diff --git a/test/Unit/Porter/PorterAwareTraitTest.php b/test/Unit/Porter/PorterAwareTraitTest.php new file mode 100644 index 0000000..197f9ff --- /dev/null +++ b/test/Unit/Porter/PorterAwareTraitTest.php @@ -0,0 +1,30 @@ +getObjectForTrait(PorterAwareTrait::class); + + $porterAware->setPorter($porter = \Mockery::mock(Porter::class)); + + self::assertSame( + $porter, + call_user_func( + \Closure::bind( + function () { + /** @var PorterAwareTrait $this */ + return $this->getPorter(); + }, + $porterAware, + $porterAware + ) + ) + ); + } +}