diff --git a/src/Schema/BulkLoader/AbstractBulkLoader.php b/src/Schema/BulkLoader/AbstractBulkLoader.php index df15b55e..ae1f6cad 100644 --- a/src/Schema/BulkLoader/AbstractBulkLoader.php +++ b/src/Schema/BulkLoader/AbstractBulkLoader.php @@ -84,7 +84,10 @@ public function applyConfig(array $config): self * @param Collection $collection * @return Collection */ - abstract public function collect(Collection $collection): Collection; + public function collect(Collection $collection): Collection + { + return Collection::create($collection->getManifest()); + } /** * @return string diff --git a/src/Schema/BulkLoader/BulkLoaderSet.php b/src/Schema/BulkLoader/BulkLoaderSet.php index dacf9e2e..d493bb55 100644 --- a/src/Schema/BulkLoader/BulkLoaderSet.php +++ b/src/Schema/BulkLoader/BulkLoaderSet.php @@ -5,9 +5,20 @@ use InvalidArgumentException; use SilverStripe\Core\ClassInfo; +use ReflectionException; +use SilverStripe\Core\Injector\Injectable; +use SilverStripe\GraphQL\Schema\Exception\SchemaBuilderException; +use SilverStripe\GraphQL\Schema\Interfaces\ConfigurationApplier; +use SilverStripe\GraphQL\Schema\Schema; -class BulkLoaderSet +/** + * Composed with a list of bulk loaders to be executed in serial and return the aggregate result + * of all their collect() calls + */ +class BulkLoaderSet implements ConfigurationApplier { + use Injectable; + /** * @var AbstractBulkLoader[] */ @@ -16,36 +27,71 @@ class BulkLoaderSet /** * @var Collection */ - private $collection; + private $initialCollection; /** * BulkLoaderSet constructor. - * @param AbstractBulkLoader[] ...$loaders - * @param Collection|null $collection + * @param AbstractBulkLoader[] $loaders + * @param Collection|null $initialCollection + * @throws ReflectionException */ - public function __construct(...$loaders, ?Collection $collection = null) + public function __construct(array $loaders = [], ?Collection $initialCollection = null) { $this->setLoaders($loaders); - if ($collection) { - $this->setCollection($collection); + if ($initialCollection) { + $this->initialCollection = $initialCollection; } else { - $this->collection = Collection::create(ClassInfo::allClasses()); + $this->initialCollection = Collection::createFromClassList(ClassInfo::allClasses()); + } + } + + /** + * @param array $config + * @return mixed|void + * @throws SchemaBuilderException + */ + public function applyConfig(array $config): self + { + $registry = Registry::inst(); + foreach ($config as $loaderID => $loaderConfig) { + /* @var AbstractBulkLoader $loader */ + $loader = $registry->getByID($loaderID); + Schema::invariant($loader, 'Loader "%s" does not exist', $loaderID); + $loader->applyConfig($loaderConfig); + $this->addLoader($loader); } + + return $this; } - public function addLoader(AbstractBulkLoader $loader) + /** + * @param AbstractBulkLoader $loader + * @return $this + */ + public function addLoader(AbstractBulkLoader $loader): self { $this->loaders[] = $loader; return $this; } + /** + * @return Collection + */ public function process(): Collection { + $collection = $this->initialCollection; foreach ($this->loaders as $loader) { - $this->collection = $loader->collect($this->collection); + $collection = $loader->collect($collection); } + + return $collection; } + + /** + * @param $loaders + * @return $this + */ public function setLoaders($loaders): self { foreach ($loaders as $loader) { @@ -58,11 +104,6 @@ public function setLoaders($loaders): self } } $this->loaders = $loaders; - } - - public function setCollection(Collection $collection): self - { - $this->collection = $collection; return $this; } diff --git a/src/Schema/BulkLoader/Collection.php b/src/Schema/BulkLoader/Collection.php index c5161e3a..c81e7c01 100644 --- a/src/Schema/BulkLoader/Collection.php +++ b/src/Schema/BulkLoader/Collection.php @@ -3,10 +3,10 @@ namespace SilverStripe\GraphQL\Schema\BulkLoader; -use Composer\Autoload\ClassLoader; use SilverStripe\Core\Injector\Injectable; use Exception; use ReflectionClass; +use ReflectionException; /** * Defines a collection of class names paired with file paths @@ -22,36 +22,21 @@ class Collection /** * Collection constructor. - * @param array $classList + * @param array $manifest An array of classname keys to filepath values ['My\Class' => '/path/to/Class.php'] * @throws Exception */ - public function __construct(array $classList) + public function __construct(array $manifest) { - $this->setClassList($classList); + $this->setManifest($manifest); } /** - * An expensive operation that rebuilds the index of className -> filePath - * @param array $classList + * @param array $manifest * @return $this * @throws Exception */ - public function setClassList(array $classList): self + public function setManifest(array $manifest): self { - $manifest = []; - foreach ($classList as $class) { - if (!class_exists($class)) { - continue; - } - $reflection = new ReflectionClass($class); - $filePath = $reflection->getFileName(); - if (!$filePath) { - continue; - } - - $manifest[$class] = $filePath; - } - $this->manifest = $manifest; return $this; @@ -95,4 +80,37 @@ public function getFiles(): array { return array_values($this->manifest); } + + /** + * @return array + */ + public function getManifest(): array + { + return $this->manifest; + } + + /** + * @param array $classList + * @return Collection + * @throws ReflectionException + * @throws Exception + */ + public static function createFromClassList(array $classList): Collection + { + $manifest = []; + foreach ($classList as $class) { + if (!class_exists($class)) { + continue; + } + $reflection = new ReflectionClass($class); + $filePath = $reflection->getFileName(); + if (!$filePath) { + continue; + } + + $manifest[$class] = $filePath; + } + + return new static($manifest); + } } diff --git a/src/Schema/BulkLoader/ExtensionLoader.php b/src/Schema/BulkLoader/ExtensionLoader.php index b639d89e..1ff6acff 100644 --- a/src/Schema/BulkLoader/ExtensionLoader.php +++ b/src/Schema/BulkLoader/ExtensionLoader.php @@ -28,22 +28,29 @@ public static function getIdentifier(): string */ public function collect(Collection $collection): Collection { + $newCollection = parent::collect($collection); + foreach ($collection->getClasses() as $class) { + $isIncluded = false; foreach ($this->includeList as $pattern) { - if (!Extensible::has_extension($class, $pattern)) { - $collection->removeClass($class); + if (Extensible::has_extension($class, $pattern)) { + $isIncluded = true; break; } } foreach ($this->excludeList as $pattern) { if (Extensible::has_extension($class, $pattern)) { - $collection->removeClass($class); + $isIncluded = false; break; } } + + if (!$isIncluded) { + $newCollection->removeClass($class); + } } - return $collection; + return $newCollection; } /** diff --git a/src/Schema/BulkLoader/FilepathLoader.php b/src/Schema/BulkLoader/FilepathLoader.php index c82bfe7e..5ea5d28d 100644 --- a/src/Schema/BulkLoader/FilepathLoader.php +++ b/src/Schema/BulkLoader/FilepathLoader.php @@ -23,8 +23,10 @@ public static function getIdentifier(): string */ public function collect(Collection $collection): Collection { + $newCollection = parent::collect($collection); $includedFiles = []; $excludedFiles = []; + foreach ($this->includeList as $include) { foreach (glob(Path::join(BASE_PATH, $include)) as $path) { $includedFiles[$path] = true; @@ -34,18 +36,15 @@ public function collect(Collection $collection): Collection foreach (glob(Path::join(BASE_PATH, $exclude)) as $path) { $excludedFiles[$path] = true; } - } foreach ($collection->getFiles() as $file) { - if (!isset($includedFiles[$file])) { - $collection->removeFile($file); - } - if (isset($excludedFiles[$file])) { - $collection->removeFile($file); + $isIncluded = isset($includedFiles[$file]) && !isset($excludedFiles[$file]); + if (!$isIncluded) { + $newCollection->removeFile($file); } } - return $collection; + return $newCollection; } } diff --git a/src/Schema/BulkLoader/InheritanceLoader.php b/src/Schema/BulkLoader/InheritanceLoader.php index 769df03b..73f00cc4 100644 --- a/src/Schema/BulkLoader/InheritanceLoader.php +++ b/src/Schema/BulkLoader/InheritanceLoader.php @@ -26,22 +26,29 @@ public static function getIdentifier(): string */ public function collect(Collection $collection): Collection { + $newCollection = parent::collect($collection); + foreach ($collection->getClasses() as $class) { + $isIncluded = false; foreach ($this->includeList as $pattern) { - if ($class !== $pattern && !is_subclass_of($class, $pattern)) { - $collection->removeClass($class); + if ($class === $pattern || is_subclass_of($class, $pattern)) { + $isIncluded = true; break; } } foreach ($this->excludeList as $pattern) { if ($class === $pattern || is_subclass_of($class, $pattern)) { - $collection->removeClass($class); + $isIncluded = false; break; } } + + if (!$isIncluded) { + $newCollection->removeClass($class); + } } - return $collection; + return $newCollection; } /** diff --git a/src/Schema/BulkLoader/NamespaceLoader.php b/src/Schema/BulkLoader/NamespaceLoader.php index de12929d..a1e1e15d 100644 --- a/src/Schema/BulkLoader/NamespaceLoader.php +++ b/src/Schema/BulkLoader/NamespaceLoader.php @@ -24,21 +24,28 @@ public static function getIdentifier(): string */ public function collect(Collection $collection): Collection { + $newCollection = parent::collect($collection); + foreach ($collection->getClasses() as $class) { + $isIncluded = false; foreach ($this->includeList as $pattern) { - if (!fnmatch($pattern, $class, FNM_NOESCAPE)) { - $collection->removeClass($class); + if (fnmatch($pattern, $class, FNM_NOESCAPE)) { + $isIncluded = true; break; } } foreach ($this->excludeList as $pattern) { if (fnmatch($pattern, $class, FNM_NOESCAPE)) { - $collection->removeClass($class); + $isIncluded = false; break; } } + + if (!$isIncluded) { + $newCollection->removeClass($class); + } } - return $collection; + return $newCollection; } } diff --git a/src/Schema/Schema.php b/src/Schema/Schema.php index 36350423..085b9fcf 100644 --- a/src/Schema/Schema.php +++ b/src/Schema/Schema.php @@ -2,12 +2,12 @@ namespace SilverStripe\GraphQL\Schema; -use SilverStripe\Core\ClassInfo; +use Psr\Log\LoggerInterface; use SilverStripe\Core\Config\Configurable; use SilverStripe\Core\Injector\Injectable; use SilverStripe\GraphQL\Config\Configuration; use SilverStripe\GraphQL\Schema\BulkLoader\AbstractBulkLoader; -use SilverStripe\GraphQL\Schema\BulkLoader\Collection; +use SilverStripe\GraphQL\Schema\BulkLoader\BulkLoaderSet; use SilverStripe\GraphQL\Schema\BulkLoader\Registry; use SilverStripe\GraphQL\Schema\Field\ModelField; use SilverStripe\GraphQL\Schema\Interfaces\ConfigurationApplier; @@ -76,6 +76,11 @@ class Schema implements ConfigurationApplier */ private static $verbose = false; + /** + * @var LoggerInterface + */ + private $logger; + /** * @var string */ @@ -143,6 +148,7 @@ public function __construct(string $schemaKey, SchemaConfig $schemaConfig = null $this->schemaConfig = $schemaConfig ?: SchemaConfig::create(); $this->state = Configuration::create(); + $this->logger = Logger::singleton(); } /** @@ -161,8 +167,6 @@ public function applyConfig(array $schemaConfig): Schema return $this; } - $logger = Logger::singleton(); - $validConfigKeys = [ self::TYPES, self::QUERIES, @@ -230,30 +234,14 @@ public function applyConfig(array $schemaConfig): Schema if (!empty($bulkLoad)) { static::assertValidConfig($bulkLoad); - $registry = Registry::inst(); - $initialCollection = Collection::create(ClassInfo::allClasses()); + foreach ($bulkLoad as $loaderData) { if ($loaderData === false) { continue; } - $collection = clone $initialCollection; static::assertValidConfig($loaderData, ['load', 'apply'], ['load', 'apply']); - $modelConfig = $loaderData['apply']; - foreach ($loaderData['load'] as $loaderID => $loaderConfig) { - /* @var AbstractBulkLoader $loader */ - $loader = $registry->getByID($loaderID); - Schema::invariant($loader, 'Loader "%s" does not exist', $loaderID); - $loader->applyConfig($loaderConfig); - $collection = $loader->collect($collection); - } - foreach ($collection->getClasses() as $includedClass) { - $modelType = $this->createModel($includedClass, $modelConfig); - if (!$modelType) { - continue; - } - $logger->info("Bulk loaded $includedClass"); - $this->addModel($modelType); - } + $loaders = BulkLoaderSet::create()->applyConfig($loaderData['load']); + $this->applyBulkLoaders($loaders, $loaderData['apply']); } } static::assertValidConfig($models); @@ -1161,6 +1149,38 @@ public function getUnions(): array return $this->unions; } + /** + * @param AbstractBulkLoader $loader + * @param array $modelConfig + * @return $this + * @throws SchemaBuilderException + */ + public function applyBulkLoader(AbstractBulkLoader $loader, array $modelConfig): self + { + return $this->applyBulkLoaders(BulkLoaderSet::create([$loader]), $modelConfig); + } + + /** + * @param BulkLoaderSet $loaders + * @param array $modelConfig + * @return $this + * @throws SchemaBuilderException + */ + public function applyBulkLoaders(BulkLoaderSet $loaders, array $modelConfig): self + { + $collection = $loaders->process(); + foreach ($collection->getClasses() as $includedClass) { + $modelType = $this->createModel($includedClass, $modelConfig); + if (!$modelType) { + continue; + } + $this->logger->info("Bulk loaded $includedClass"); + $this->addModel($modelType); + } + + return $this; + } + /** * @return array */ diff --git a/tests/Fake/FakeBulkLoader.php b/tests/Fake/FakeBulkLoader.php new file mode 100644 index 00000000..1f51a6a9 --- /dev/null +++ b/tests/Fake/FakeBulkLoader.php @@ -0,0 +1,37 @@ +shouldReturn = $shouldReturn; + } + + public static function getIdentifier(): string + { + return 'fake'; + } + + /** + * @param Collection $collection + * @return Collection + * @throws \Exception + */ + public function collect(Collection $collection): Collection + { + return new Collection($this->shouldReturn); + } +} diff --git a/tests/Schema/IntegrationTest.php b/tests/Schema/IntegrationTest.php index a27d6bd3..2d47f7a6 100644 --- a/tests/Schema/IntegrationTest.php +++ b/tests/Schema/IntegrationTest.php @@ -5,6 +5,7 @@ use GraphQL\Type\Definition\ObjectType; use SilverStripe\Assets\File; +use SilverStripe\Core\ClassInfo; use SilverStripe\Core\Config\Config; use SilverStripe\Core\Injector\Injector; use SilverStripe\Core\Path; @@ -27,6 +28,7 @@ use SilverStripe\GraphQL\Tests\Fake\FakeRedirectorPage; use SilverStripe\GraphQL\Tests\Fake\FakeReview; use SilverStripe\GraphQL\Tests\Fake\FakeSiteTree; +use SilverStripe\GraphQL\Tests\Fake\Inheritance\A; use SilverStripe\GraphQL\Tests\Fake\IntegrationTestResolver; use SilverStripe\Security\Member; use Symfony\Component\Filesystem\Filesystem; @@ -1056,6 +1058,65 @@ public function testDBFieldArgs() $this->assertEquals('This is a really long text field. It has a few sentences.', $node['myText']); } + public function testBulkLoadInheritance() + { + $schema = $this->createSchema(new TestSchemaBuilder(['_' . __FUNCTION__])); + $this->assertSchemaHasType($schema, 'A1'); + $this->assertSchemaHasType($schema, 'A1a'); + $this->assertSchemaHasType($schema, 'A1b'); + $this->assertSchemaHasType($schema, 'C'); + $this->assertSchemaHasType($schema, 'C1'); + $this->assertSchemaHasType($schema, 'C2'); + $this->assertSchemaNotHasType($schema, 'C2a'); + $this->assertSchemaNotHasType($schema, 'B'); + $this->assertSchemaNotHasType($schema, 'A2'); + + $query = $schema->getQueryType(); + $this->assertNotNull($query->getFieldByName('readA1s')); + $this->assertNotNull($query->getFieldByName('readA1as')); + $this->assertNotNull($query->getFieldByName('readA1bs')); + $this->assertNotNull($query->getFieldByName('readCs')); + $this->assertNotNull($query->getFieldByName('readC1s')); + $this->assertNotNull($query->getFieldByName('readC2s')); + $this->assertNull($query->getFieldByName('readC2as')); + + $a1 = $schema->getType('A1'); + $this->assertNotNull($a1->getFieldByName('A1Field')); + $this->assertNull($a1->getFieldByName('created')); + + $c = $schema->getType('C'); + $this->assertNotNull($c->getFieldByName('cField')); + $this->assertNull($c->getFieldByName('created')); + } + + public function testBulkLoadNamespaceAndFilepath() + { + $schema = $this->createSchema(new TestSchemaBuilder(['_' . __FUNCTION__])); + $this->assertSchemaHasType($schema, 'A1'); + $this->assertSchemaHasType($schema, 'A2'); + $this->assertSchemaHasType($schema, 'A1a'); + $this->assertSchemaHasType($schema, 'A1b'); + $this->assertSchemaHasType($schema, 'A2a'); + + $this->assertSchemaHasType($schema, 'B'); + $this->assertSchemaHasType($schema, 'B1'); + $this->assertSchemaHasType($schema, 'B2'); + $this->assertSchemaHasType($schema, 'B1a'); + $this->assertSchemaHasType($schema, 'B1b'); + + $this->assertSchemaHasType($schema, 'C'); + $this->assertSchemaHasType($schema, 'C1'); + $this->assertSchemaHasType($schema, 'C2'); + + $this->assertSchemaNotHasType($schema, 'C2a'); + + $this->assertSchemaHasType($schema, 'SubFakePage'); + $this->assertSchemaNotHasType($schema, 'FakePage'); + + $this->assertSchemaHasType($schema, 'FakeProductPage'); + $this->assertSchemaHasType($schema, 'FakeRedirectorPage'); + } + /** * @return array */ diff --git a/tests/Schema/_testBulkLoadInheritance/bulkLoad.yml b/tests/Schema/_testBulkLoadInheritance/bulkLoad.yml new file mode 100644 index 00000000..f8473c59 --- /dev/null +++ b/tests/Schema/_testBulkLoadInheritance/bulkLoad.yml @@ -0,0 +1,14 @@ +test: + load: + inheritanceLoader: + include: + - SilverStripe\GraphQL\Tests\Fake\Inheritance\A1 + - SilverStripe\GraphQL\Tests\Fake\Inheritance\C + exclude: + - SilverStripe\GraphQL\Tests\Fake\Inheritance\C2a + apply: + fields: + '*': true + created: false + operations: + read: true diff --git a/tests/Schema/_testBulkLoadNamespaceAndFilepath/bulkLoad.yml b/tests/Schema/_testBulkLoadNamespaceAndFilepath/bulkLoad.yml new file mode 100644 index 00000000..566016d0 --- /dev/null +++ b/tests/Schema/_testBulkLoadNamespaceAndFilepath/bulkLoad.yml @@ -0,0 +1,24 @@ +test1: + load: + namespaceLoader: + include: + - SilverStripe\GraphQL\Tests\Fake\Inheritance\* + - SilverStripe\GraphQL\Tests\Fake\SubFake\* + exclude: + - SilverStripe\GraphQL\Tests\Fake\Inheritance\C2a + apply: + fields: + '*': true + created: false + operations: + read: true +test2: + load: + filepathLoader: + include: + - vendor/silverstripe/graphql/tests/Fake/*Page.php + exclude: + - vendor/silverstripe/graphql/tests/Fake/FakePage.php + apply: + fields: + '*': true diff --git a/tests/Schema/_testBulkLoadNamespaceAndFilepath/config.yml b/tests/Schema/_testBulkLoadNamespaceAndFilepath/config.yml new file mode 100644 index 00000000..05661dd8 --- /dev/null +++ b/tests/Schema/_testBulkLoadNamespaceAndFilepath/config.yml @@ -0,0 +1,2 @@ +typeMapping: + SilverStripe\GraphQL\Tests\Fake\SubFake\FakePage: SubFakePage