From f864151aefaf4cce0cdfda898e46ca17d68149d9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Wed, 18 May 2016 13:49:51 +0200 Subject: [PATCH 1/3] Add ScopeFactoryInterface and refactor Manager to use it. --- src/Manager.php | 44 +++++++++++++++++++++------ src/ScopeFactory.php | 56 +++++++++++++++++++++++++++++++++++ src/ScopeFactoryInterface.php | 37 +++++++++++++++++++++++ 3 files changed, 128 insertions(+), 9 deletions(-) create mode 100644 src/ScopeFactory.php create mode 100644 src/ScopeFactoryInterface.php diff --git a/src/Manager.php b/src/Manager.php index 32d03c7c..7203dc80 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -66,6 +66,13 @@ class Manager */ protected $serializer; + /** + * Factory used to create new configured scopes. + * + * @var ScopeFactoryInterface + */ + private $scopeFactory; + /** * Create Data. * @@ -79,18 +86,11 @@ class Manager */ public function createData(ResourceInterface $resource, $scopeIdentifier = null, Scope $parentScopeInstance = null) { - $scopeInstance = new Scope($this, $resource, $scopeIdentifier); - - // Update scope history if ($parentScopeInstance !== null) { - // This will be the new children list of parents (parents parents, plus the parent) - $scopeArray = $parentScopeInstance->getParentScopes(); - $scopeArray[] = $parentScopeInstance->getScopeIdentifier(); - - $scopeInstance->setParentScopes($scopeArray); + return $this->getScopeFactory()->createChildScopeFor($parentScopeInstance, $resource, $scopeIdentifier); } - return $scopeInstance; + return $this->getScopeFactory()->createScopeFor($resource, $scopeIdentifier); } /** @@ -270,6 +270,32 @@ public function setSerializer(SerializerAbstract $serializer) return $this; } + /** + * @return ScopeFactoryInterface + */ + public function getScopeFactory() + { + if (!$this->scopeFactory) { + $this->scopeFactory = new ScopeFactory($this); + } + + return $this->scopeFactory; + } + + /** + * Set ScopeFactory + * + * @param ScopeFactoryInterface $scopeFactory + * + * @return $this + */ + public function setScopeFactory(ScopeFactoryInterface $scopeFactory) + { + $this->scopeFactory = $scopeFactory; + + return $this; + } + /** * Auto-include Parents * diff --git a/src/ScopeFactory.php b/src/ScopeFactory.php new file mode 100644 index 00000000..3e024ef4 --- /dev/null +++ b/src/ScopeFactory.php @@ -0,0 +1,56 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace League\Fractal; + +use League\Fractal\Resource\ResourceInterface; + +class ScopeFactory implements ScopeFactoryInterface +{ + /** + * @var Manager + */ + private $manager; + + public function __construct(Manager $manager) + { + $this->manager = $manager; + } + + /** + * @param ResourceInterface $resource + * @param string|null $scopeIdentifier + * @return Scope + */ + public function createScopeFor(ResourceInterface $resource, $scopeIdentifier = null) + { + return new Scope($this->manager, $resource, $scopeIdentifier); + } + + /** + * @param Scope $parentScopeInstance + * @param ResourceInterface $resource + * @param string|null $scopeIdentifier + * @return Scope + */ + public function createChildScopeFor(Scope $parentScopeInstance, ResourceInterface $resource, $scopeIdentifier = null) + { + $scopeInstance = $this->createScopeFor($resource, $scopeIdentifier); + + // This will be the new children list of parents (parents parents, plus the parent) + $scopeArray = $parentScopeInstance->getParentScopes(); + $scopeArray[] = $parentScopeInstance->getScopeIdentifier(); + + $scopeInstance->setParentScopes($scopeArray); + + return $scopeInstance; + } +} diff --git a/src/ScopeFactoryInterface.php b/src/ScopeFactoryInterface.php new file mode 100644 index 00000000..44cfd62a --- /dev/null +++ b/src/ScopeFactoryInterface.php @@ -0,0 +1,37 @@ + + * + * For the full copyright and license information, please view the LICENSE + * file that was distributed with this source code. + */ + +namespace League\Fractal; + +use League\Fractal\Resource\ResourceInterface; + +/** + * ScopeFactoryInterface + * + * Creates Scope Instances for resources + */ +interface ScopeFactoryInterface +{ + /** + * @param ResourceInterface $resource + * @param string|null $scopeIdentifier + * @return Scope + */ + public function createScopeFor(ResourceInterface $resource, $scopeIdentifier = null); + + /** + * @param Scope $parentScope + * @param ResourceInterface $resource + * @param string|null $scopeIdentifier + * @return Scope + */ + public function createChildScopeFor(Scope $parentScope, ResourceInterface $resource, $scopeIdentifier = null); +} From 064ce609d7428c2444cb2037d4799946f519ad71 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Wed, 18 May 2016 14:15:32 +0200 Subject: [PATCH 2/3] Add ScopeFactory tests --- test/ScopeFactoryTest.php | 85 +++++++++++++++++++++++++++++++++++++++ 1 file changed, 85 insertions(+) create mode 100644 test/ScopeFactoryTest.php diff --git a/test/ScopeFactoryTest.php b/test/ScopeFactoryTest.php new file mode 100644 index 00000000..455da861 --- /dev/null +++ b/test/ScopeFactoryTest.php @@ -0,0 +1,85 @@ +assertInstanceOf('League\\Fractal\\ScopeFactoryInterface', $this->createSut()); + } + + public function testItCreatesScopes() + { + $sut = $this->createSut(); + + $resource = $this->createResource(); + $scopeIdentifier = 'foo_identifier'; + + $scope = $sut->createScopeFor($resource, $scopeIdentifier); + + $this->assertInstanceOf('League\\Fractal\\Scope', $scope); + $this->assertSame($resource, $scope->getResource()); + $this->assertSame($scopeIdentifier, $scope->getScopeIdentifier()); + } + + public function testItCreatesScopesWithParent() + { + $manager = $this->createManager(); + + $scope = new Scope($manager, $this->createResource(), 'parent_identifier'); + $scope->setParentScopes([ + 'parent_scope', + ]); + + $resource = $this->createResource(); + $scopeIdentifier = 'foo_identifier'; + + $expectedParentScopes = [ + 'parent_scope', + 'parent_identifier', + ]; + + $sut = $this->createSut($manager); + $scope = $sut->createChildScopeFor($scope, $resource, $scopeIdentifier); + + $this->assertInstanceOf('League\\Fractal\\Scope', $scope); + $this->assertSame($resource, $scope->getResource()); + $this->assertSame($scopeIdentifier, $scope->getScopeIdentifier()); + $this->assertEquals($expectedParentScopes, $scope->getParentScopes()); + } + + /** + * @param Manager $manager + * @return ScopeFactory + */ + private function createSut(Manager $manager = null) + { + if ($manager === null) { + $manager = $this->createManager(); + } + + return new ScopeFactory($manager); + } + + /** + * @return Manager + */ + private function createManager() + { + return $this->getMock('League\\Fractal\\Manager'); + } + + /** + * @return ResourceInterface + */ + private function createResource() + { + return $this->getMock('League\\Fractal\\Resource\\ResourceInterface'); + } +} From ed148f14023c4475aaac1b2288a92d319c75b52a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Beno=C3=AEt=20Burnichon?= Date: Wed, 18 May 2016 14:43:37 +0200 Subject: [PATCH 3/3] Remove ugly setter injection in Manager. Change ScopeFactoryInterface to require Manager instance for Scope Creation. --- src/Manager.php | 35 +++++++---------------------------- src/ScopeFactory.php | 20 ++++++-------------- src/ScopeFactoryInterface.php | 6 ++++-- test/ScopeFactoryTest.php | 16 ++++++---------- 4 files changed, 23 insertions(+), 54 deletions(-) diff --git a/src/Manager.php b/src/Manager.php index 7203dc80..aa2d42d5 100644 --- a/src/Manager.php +++ b/src/Manager.php @@ -73,6 +73,11 @@ class Manager */ private $scopeFactory; + public function __construct(ScopeFactoryInterface $scopeFactory = null) + { + $this->scopeFactory = $scopeFactory ?: new ScopeFactory(); + } + /** * Create Data. * @@ -87,10 +92,10 @@ class Manager public function createData(ResourceInterface $resource, $scopeIdentifier = null, Scope $parentScopeInstance = null) { if ($parentScopeInstance !== null) { - return $this->getScopeFactory()->createChildScopeFor($parentScopeInstance, $resource, $scopeIdentifier); + return $this->scopeFactory->createChildScopeFor($this, $parentScopeInstance, $resource, $scopeIdentifier); } - return $this->getScopeFactory()->createScopeFor($resource, $scopeIdentifier); + return $this->scopeFactory->createScopeFor($this, $resource, $scopeIdentifier); } /** @@ -270,32 +275,6 @@ public function setSerializer(SerializerAbstract $serializer) return $this; } - /** - * @return ScopeFactoryInterface - */ - public function getScopeFactory() - { - if (!$this->scopeFactory) { - $this->scopeFactory = new ScopeFactory($this); - } - - return $this->scopeFactory; - } - - /** - * Set ScopeFactory - * - * @param ScopeFactoryInterface $scopeFactory - * - * @return $this - */ - public function setScopeFactory(ScopeFactoryInterface $scopeFactory) - { - $this->scopeFactory = $scopeFactory; - - return $this; - } - /** * Auto-include Parents * diff --git a/src/ScopeFactory.php b/src/ScopeFactory.php index 3e024ef4..1d173650 100644 --- a/src/ScopeFactory.php +++ b/src/ScopeFactory.php @@ -16,34 +16,26 @@ class ScopeFactory implements ScopeFactoryInterface { /** - * @var Manager - */ - private $manager; - - public function __construct(Manager $manager) - { - $this->manager = $manager; - } - - /** + * @param Manager $manager * @param ResourceInterface $resource * @param string|null $scopeIdentifier * @return Scope */ - public function createScopeFor(ResourceInterface $resource, $scopeIdentifier = null) + public function createScopeFor(Manager $manager, ResourceInterface $resource, $scopeIdentifier = null) { - return new Scope($this->manager, $resource, $scopeIdentifier); + return new Scope($manager, $resource, $scopeIdentifier); } /** + * @param Manager $manager * @param Scope $parentScopeInstance * @param ResourceInterface $resource * @param string|null $scopeIdentifier * @return Scope */ - public function createChildScopeFor(Scope $parentScopeInstance, ResourceInterface $resource, $scopeIdentifier = null) + public function createChildScopeFor(Manager $manager, Scope $parentScopeInstance, ResourceInterface $resource, $scopeIdentifier = null) { - $scopeInstance = $this->createScopeFor($resource, $scopeIdentifier); + $scopeInstance = $this->createScopeFor($manager, $resource, $scopeIdentifier); // This will be the new children list of parents (parents parents, plus the parent) $scopeArray = $parentScopeInstance->getParentScopes(); diff --git a/src/ScopeFactoryInterface.php b/src/ScopeFactoryInterface.php index 44cfd62a..1175712b 100644 --- a/src/ScopeFactoryInterface.php +++ b/src/ScopeFactoryInterface.php @@ -21,17 +21,19 @@ interface ScopeFactoryInterface { /** + * @param Manager $manager * @param ResourceInterface $resource * @param string|null $scopeIdentifier * @return Scope */ - public function createScopeFor(ResourceInterface $resource, $scopeIdentifier = null); + public function createScopeFor(Manager $manager, ResourceInterface $resource, $scopeIdentifier = null); /** + * @param Manager $manager * @param Scope $parentScope * @param ResourceInterface $resource * @param string|null $scopeIdentifier * @return Scope */ - public function createChildScopeFor(Scope $parentScope, ResourceInterface $resource, $scopeIdentifier = null); + public function createChildScopeFor(Manager $manager, Scope $parentScope, ResourceInterface $resource, $scopeIdentifier = null); } diff --git a/test/ScopeFactoryTest.php b/test/ScopeFactoryTest.php index 455da861..b0be7577 100644 --- a/test/ScopeFactoryTest.php +++ b/test/ScopeFactoryTest.php @@ -18,10 +18,11 @@ public function testItCreatesScopes() { $sut = $this->createSut(); + $manager = $this->createManager(); $resource = $this->createResource(); $scopeIdentifier = 'foo_identifier'; - $scope = $sut->createScopeFor($resource, $scopeIdentifier); + $scope = $sut->createScopeFor($manager, $resource, $scopeIdentifier); $this->assertInstanceOf('League\\Fractal\\Scope', $scope); $this->assertSame($resource, $scope->getResource()); @@ -45,8 +46,8 @@ public function testItCreatesScopesWithParent() 'parent_identifier', ]; - $sut = $this->createSut($manager); - $scope = $sut->createChildScopeFor($scope, $resource, $scopeIdentifier); + $sut = $this->createSut(); + $scope = $sut->createChildScopeFor($manager, $scope, $resource, $scopeIdentifier); $this->assertInstanceOf('League\\Fractal\\Scope', $scope); $this->assertSame($resource, $scope->getResource()); @@ -55,16 +56,11 @@ public function testItCreatesScopesWithParent() } /** - * @param Manager $manager * @return ScopeFactory */ - private function createSut(Manager $manager = null) + private function createSut() { - if ($manager === null) { - $manager = $this->createManager(); - } - - return new ScopeFactory($manager); + return new ScopeFactory(); } /**