From cbcfa70240d981e034f070f85d2077a3baf1c98d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?David=20N=C3=A9grier?= Date: Wed, 3 Apr 2019 11:48:50 +0200 Subject: [PATCH] Making locks not dependent on Semaphore extension Locks now rely on Semaphore extension if available and fall back to Flock otherwise. --- src/GlobControllerQueryProvider.php | 6 ++---- src/Mappers/GlobTypeMapper.php | 5 ++--- src/SchemaFactory.php | 20 ++++++++++++++++++-- tests/AbstractQueryProviderTest.php | 17 +++++++++++++++++ tests/GlobControllerQueryProviderTest.php | 2 +- tests/Integration/EndToEndTest.php | 15 ++++++++++++++- tests/Mappers/GlobTypeMapperTest.php | 22 +++++++++++----------- tests/Mappers/RecursiveTypeMapperTest.php | 2 +- 8 files changed, 66 insertions(+), 23 deletions(-) diff --git a/src/GlobControllerQueryProvider.php b/src/GlobControllerQueryProvider.php index fda6cf4bcf..0e70ed4ea1 100644 --- a/src/GlobControllerQueryProvider.php +++ b/src/GlobControllerQueryProvider.php @@ -69,7 +69,7 @@ final class GlobControllerQueryProvider implements QueryProviderInterface * @param int|null $cacheTtl * @param bool $recursive Whether subnamespaces of $namespace must be analyzed. */ - public function __construct(string $namespace, FieldsBuilderFactory $fieldsBuilderFactory, RecursiveTypeMapperInterface $recursiveTypeMapper, ContainerInterface $container, CacheInterface $cache, ?int $cacheTtl = null, bool $recursive = true) + public function __construct(string $namespace, FieldsBuilderFactory $fieldsBuilderFactory, RecursiveTypeMapperInterface $recursiveTypeMapper, ContainerInterface $container, LockFactory $lockFactory, CacheInterface $cache, ?int $cacheTtl = null, bool $recursive = true) { $this->namespace = $namespace; $this->container = $container; @@ -78,9 +78,7 @@ public function __construct(string $namespace, FieldsBuilderFactory $fieldsBuild $this->fieldsBuilderFactory = $fieldsBuilderFactory; $this->recursiveTypeMapper = $recursiveTypeMapper; $this->recursive = $recursive; - $store = new SemaphoreStore(); - $this->lockFactory = new LockFactory($store); - + $this->lockFactory = $lockFactory; } private function getAggregateControllerQueryProvider(): AggregateControllerQueryProvider diff --git a/src/Mappers/GlobTypeMapper.php b/src/Mappers/GlobTypeMapper.php index 10aa22ad19..f09d5f3e1d 100644 --- a/src/Mappers/GlobTypeMapper.php +++ b/src/Mappers/GlobTypeMapper.php @@ -129,7 +129,7 @@ final class GlobTypeMapper implements TypeMapperInterface /** * @param string $namespace The namespace that contains the GraphQL types (they must have a `@Type` annotation) */ - public function __construct(string $namespace, TypeGenerator $typeGenerator, InputTypeGenerator $inputTypeGenerator, InputTypeUtils $inputTypeUtils, ContainerInterface $container, AnnotationReader $annotationReader, NamingStrategyInterface $namingStrategy, CacheInterface $cache, ?int $globTtl = 2, ?int $mapTtl = null, bool $recursive = true) + public function __construct(string $namespace, TypeGenerator $typeGenerator, InputTypeGenerator $inputTypeGenerator, InputTypeUtils $inputTypeUtils, ContainerInterface $container, AnnotationReader $annotationReader, NamingStrategyInterface $namingStrategy, LockFactory $lockFactory, CacheInterface $cache, ?int $globTtl = 2, ?int $mapTtl = null, bool $recursive = true) { $this->namespace = $namespace; $this->typeGenerator = $typeGenerator; @@ -142,8 +142,7 @@ public function __construct(string $namespace, TypeGenerator $typeGenerator, Inp $this->inputTypeGenerator = $inputTypeGenerator; $this->inputTypeUtils = $inputTypeUtils; $this->recursive = $recursive; - $store = new SemaphoreStore(); - $this->lockFactory = new LockFactory($store); + $this->lockFactory = $lockFactory; } /** diff --git a/src/SchemaFactory.php b/src/SchemaFactory.php index ee49b67aaf..01205aee23 100644 --- a/src/SchemaFactory.php +++ b/src/SchemaFactory.php @@ -8,9 +8,14 @@ use Doctrine\Common\Annotations\Reader; use Doctrine\Common\Annotations\AnnotationReader as DoctrineAnnotationReader; use Doctrine\Common\Cache\ApcuCache; +use function extension_loaded; use GraphQL\Type\SchemaConfig; use Psr\Container\ContainerInterface; use Psr\SimpleCache\CacheInterface; +use Symfony\Component\Lock\Factory as LockFactory; +use Symfony\Component\Lock\Store\FlockStore; +use Symfony\Component\Lock\Store\SemaphoreStore; +use function sys_get_temp_dir; use TheCodingMachine\GraphQLite\Hydrators\FactoryHydrator; use TheCodingMachine\GraphQLite\Hydrators\HydratorInterface; use TheCodingMachine\GraphQLite\Mappers\CompositeTypeMapper; @@ -72,6 +77,10 @@ class SchemaFactory * @var SchemaConfig */ private $schemaConfig; + /** + * @var LockFactory + */ + private $lockFactory; public function __construct(CacheInterface $cache, ContainerInterface $container) { @@ -181,6 +190,13 @@ public function createSchema(): Schema $namingStrategy = $this->namingStrategy ?: new NamingStrategy(); $typeRegistry = new TypeRegistry(); + if (extension_loaded('sysvsem')) { + $lockStore = new SemaphoreStore(); + } else { + $lockStore = new FlockStore(sys_get_temp_dir()); + } + $lockFactory = new LockFactory($lockStore); + $fieldsBuilderFactory = new FieldsBuilderFactory($annotationReader, $hydrator, $authenticationService, $authorizationService, $typeResolver, $cachedDocBlockFactory, $namingStrategy); @@ -192,7 +208,7 @@ public function createSchema(): Schema foreach ($this->typeNamespaces as $typeNamespace) { $typeMappers[] = new GlobTypeMapper($typeNamespace, $typeGenerator, $inputTypeGenerator, $inputTypeUtils, - $this->container, $annotationReader, $namingStrategy, $this->cache); + $this->container, $annotationReader, $namingStrategy, $lockFactory, $this->cache); } foreach ($this->typeMappers as $typeMapper) { @@ -211,7 +227,7 @@ public function createSchema(): Schema $queryProviders = []; foreach ($this->controllerNamespaces as $controllerNamespace) { $queryProviders[] = new GlobControllerQueryProvider($controllerNamespace, $fieldsBuilderFactory, $recursiveTypeMapper, - $this->container, $this->cache); + $this->container, $lockFactory, $this->cache); } foreach ($this->queryProviders as $queryProvider) { diff --git a/tests/AbstractQueryProviderTest.php b/tests/AbstractQueryProviderTest.php index ffb0dcc33d..fd9914e9a3 100644 --- a/tests/AbstractQueryProviderTest.php +++ b/tests/AbstractQueryProviderTest.php @@ -15,6 +15,9 @@ use PHPUnit\Framework\TestCase; use Psr\Container\ContainerInterface; use Symfony\Component\Cache\Simple\ArrayCache; +use Symfony\Component\Lock\Factory as LockFactory; +use Symfony\Component\Lock\Store\FlockStore; +use Symfony\Component\Lock\Store\SemaphoreStore; use TheCodingMachine\GraphQLite\Fixtures\TestObject; use TheCodingMachine\GraphQLite\Fixtures\TestObject2; use TheCodingMachine\GraphQLite\Fixtures\TestObjectWithRecursiveList; @@ -50,6 +53,7 @@ abstract class AbstractQueryProviderTest extends TestCase private $annotationReader; private $typeResolver; private $typeRegistry; + private $lockFactory; protected function getTestObjectType(): MutableObjectType { @@ -318,4 +322,17 @@ protected function getTypeRegistry(): TypeRegistry } return $this->typeRegistry; } + + protected function getLockFactory(): LockFactory + { + if ($this->lockFactory === null) { + if (extension_loaded('sysvsem')) { + $lockStore = new SemaphoreStore(); + } else { + $lockStore = new FlockStore(sys_get_temp_dir()); + } + $this->lockFactory = new LockFactory($lockStore); + } + return $this->lockFactory; + } } diff --git a/tests/GlobControllerQueryProviderTest.php b/tests/GlobControllerQueryProviderTest.php index 05bb3f6c1f..083747b3b9 100644 --- a/tests/GlobControllerQueryProviderTest.php +++ b/tests/GlobControllerQueryProviderTest.php @@ -34,7 +34,7 @@ public function has($id) } }; - $globControllerQueryProvider = new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite', $this->getControllerQueryProviderFactory(), $this->getTypeMapper(), $container, new NullCache()); + $globControllerQueryProvider = new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite', $this->getControllerQueryProviderFactory(), $this->getTypeMapper(), $container, $this->getLockFactory(), new NullCache()); $queries = $globControllerQueryProvider->getQueries(); $this->assertCount(6, $queries); diff --git a/tests/Integration/EndToEndTest.php b/tests/Integration/EndToEndTest.php index 08eede75b0..8a79bb100d 100644 --- a/tests/Integration/EndToEndTest.php +++ b/tests/Integration/EndToEndTest.php @@ -16,6 +16,9 @@ use Psr\Container\ContainerInterface; use Psr\Container\NotFoundExceptionInterface; use Symfony\Component\Cache\Simple\ArrayCache; +use Symfony\Component\Lock\Factory as LockFactory; +use Symfony\Component\Lock\Store\FlockStore; +use Symfony\Component\Lock\Store\SemaphoreStore; use TheCodingMachine\GraphQLite\AnnotationReader; use TheCodingMachine\GraphQLite\FieldsBuilderFactory; use TheCodingMachine\GraphQLite\Fixtures\Integration\Models\Contact; @@ -62,7 +65,7 @@ public function setUp() }, QueryProviderInterface::class => function(ContainerInterface $container) { return new GlobControllerQueryProvider('TheCodingMachine\\GraphQLite\\Fixtures\\Integration\\Controllers', $container->get(FieldsBuilderFactory::class), - $container->get(RecursiveTypeMapperInterface::class), $container->get(BasicAutoWiringContainer::class), new ArrayCache()); + $container->get(RecursiveTypeMapperInterface::class), $container->get(BasicAutoWiringContainer::class), $container->get(LockFactory::class), new ArrayCache()); }, FieldsBuilderFactory::class => function(ContainerInterface $container) { return new FieldsBuilderFactory( @@ -110,6 +113,7 @@ public function setUp() $container->get(BasicAutoWiringContainer::class), $container->get(AnnotationReader::class), $container->get(NamingStrategyInterface::class), + $container->get(LockFactory::class), new ArrayCache() ); }, @@ -121,6 +125,7 @@ public function setUp() $container->get(BasicAutoWiringContainer::class), $container->get(AnnotationReader::class), $container->get(NamingStrategyInterface::class), + $container->get(LockFactory::class), new ArrayCache() ); }, @@ -163,6 +168,14 @@ public function setUp() }, CachedDocBlockFactory::class => function() { return new CachedDocBlockFactory(new ArrayCache()); + }, + LockFactory::class => function() { + if (extension_loaded('sysvsem')) { + $lockStore = new SemaphoreStore(); + } else { + $lockStore = new FlockStore(sys_get_temp_dir()); + } + return new LockFactory($lockStore); } ]); diff --git a/tests/Mappers/GlobTypeMapperTest.php b/tests/Mappers/GlobTypeMapperTest.php index 48effcf4ec..95b27eaed8 100644 --- a/tests/Mappers/GlobTypeMapperTest.php +++ b/tests/Mappers/GlobTypeMapperTest.php @@ -33,7 +33,7 @@ public function testGlobTypeMapper() $cache = new ArrayCache(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $this->assertSame([TestObject::class], $mapper->getSupportedClasses()); $this->assertTrue($mapper->canMapClassToType(TestObject::class)); @@ -43,7 +43,7 @@ public function testGlobTypeMapper() $this->assertFalse($mapper->canMapNameToType('NotExists')); // Again to test cache - $anotherMapperSameCache = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $anotherMapperSameCache = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $this->assertTrue($anotherMapperSameCache->canMapClassToType(TestObject::class)); $this->assertTrue($anotherMapperSameCache->canMapNameToType('Foo')); @@ -61,7 +61,7 @@ public function testGlobTypeMapperDuplicateTypesException() $typeGenerator = $this->getTypeGenerator(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\DuplicateTypes', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), new NullCache()); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\DuplicateTypes', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), new NullCache()); $this->expectException(DuplicateMappingException::class); $mapper->canMapClassToType(TestType::class); @@ -77,7 +77,7 @@ public function testGlobTypeMapperDuplicateInputTypesException() $typeGenerator = $this->getTypeGenerator(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), new NullCache()); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), new NullCache()); $this->expectException(DuplicateMappingException::class); $this->expectExceptionMessage('The class \'TheCodingMachine\GraphQLite\Fixtures\TestObject\' should be mapped to only one GraphQL Input type. Two methods are pointing via the @Factory annotation to this class: \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory::myFactory\' and \'TheCodingMachine\GraphQLite\Fixtures\DuplicateInputTypes\TestFactory2::myFactory\''); @@ -94,7 +94,7 @@ public function testGlobTypeMapperClassNotFoundException() $typeGenerator = $this->getTypeGenerator(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\BadClassType', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), new NullCache()); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\BadClassType', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), new NullCache()); $this->expectException(ClassNotFoundException::class); $this->expectExceptionMessage("Could not autoload class 'Foobar' defined in @Type annotation of class 'TheCodingMachine\\GraphQLite\\Fixtures\\BadClassType\\TestType'"); @@ -111,7 +111,7 @@ public function testGlobTypeMapperNameNotFoundException() $typeGenerator = $this->getTypeGenerator(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), new NullCache()); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), new NullCache()); $this->expectException(CannotMapTypeException::class); $mapper->mapNameToType('NotExists', $this->getTypeMapper()); @@ -132,7 +132,7 @@ public function testGlobTypeMapperInputType() $cache = new ArrayCache(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $this->assertTrue($mapper->canMapClassToInputType(TestObject::class)); @@ -141,7 +141,7 @@ public function testGlobTypeMapperInputType() $this->assertSame('TestObjectInput', $inputType->name); // Again to test cache - $anotherMapperSameCache = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $anotherMapperSameCache = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $this->assertTrue($anotherMapperSameCache->canMapClassToInputType(TestObject::class)); $this->assertSame('TestObjectInput', $anotherMapperSameCache->mapClassToInputType(TestObject::class, $this->getTypeMapper())->name); @@ -167,7 +167,7 @@ public function testGlobTypeMapperExtend() $cache = new ArrayCache(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $type = $mapper->mapClassToType(TestObject::class, null, $this->getTypeMapper()); @@ -178,7 +178,7 @@ public function testGlobTypeMapperExtend() $this->assertFalse($mapper->canExtendTypeForName('NotExists', $type, $this->getTypeMapper())); // Again to test cache - $anotherMapperSameCache = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $anotherMapperSameCache = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $this->assertTrue($anotherMapperSameCache->canExtendTypeForClass(TestObject::class, $type, $this->getTypeMapper())); $this->assertTrue($anotherMapperSameCache->canExtendTypeForName('TestObject', $type, $this->getTypeMapper())); @@ -195,7 +195,7 @@ public function testEmptyGlobTypeMapper() $cache = new ArrayCache(); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Integration\Controllers', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $cache); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Integration\Controllers', $typeGenerator, $inputTypeGenerator, $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), new NamingStrategy(), $this->getLockFactory(), $cache); $this->assertSame([], $mapper->getSupportedClasses()); } diff --git a/tests/Mappers/RecursiveTypeMapperTest.php b/tests/Mappers/RecursiveTypeMapperTest.php index 0ea02117ad..6aa78104ba 100644 --- a/tests/Mappers/RecursiveTypeMapperTest.php +++ b/tests/Mappers/RecursiveTypeMapperTest.php @@ -112,7 +112,7 @@ protected function getTypeMapper() $typeGenerator = new TypeGenerator($this->getAnnotationReader(), $this->getControllerQueryProviderFactory(), $namingStrategy, $this->getTypeRegistry(), $this->getRegistry()); - $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Interfaces\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), $namingStrategy, new NullCache()); + $mapper = new GlobTypeMapper('TheCodingMachine\GraphQLite\Fixtures\Interfaces\Types', $typeGenerator, $this->getInputTypeGenerator(), $this->getInputTypeUtils(), $container, new \TheCodingMachine\GraphQLite\AnnotationReader(new AnnotationReader()), $namingStrategy, $this->getLockFactory(), new NullCache()); return new RecursiveTypeMapper($mapper, new NamingStrategy(), new ArrayCache(), $this->getTypeRegistry()); }