From 40dc0c3bdeb2464401ad895164c2cf359d8ab8f2 Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 18 Jan 2023 16:48:26 +0100 Subject: [PATCH 1/2] refactor(metadata): no default graphql operation name --- .../Resolver/Factory/ItemResolverFactory.php | 2 +- .../Subscription/SubscriptionManager.php | 2 +- src/GraphQl/Type/FieldsBuilder.php | 4 ---- src/GraphQl/Type/TypeConverter.php | 5 +---- src/Metadata/GraphQl/Query.php | 2 +- src/Metadata/GraphQl/QueryCollection.php | 5 +++-- src/Metadata/GraphQl/Subscription.php | 2 +- .../Factory/OperationDefaultsTrait.php | 22 ++++++++++++++----- 8 files changed, 24 insertions(+), 20 deletions(-) diff --git a/src/GraphQl/Resolver/Factory/ItemResolverFactory.php b/src/GraphQl/Resolver/Factory/ItemResolverFactory.php index 3c1b9717e5f..a5a00a6db10 100644 --- a/src/GraphQl/Resolver/Factory/ItemResolverFactory.php +++ b/src/GraphQl/Resolver/Factory/ItemResolverFactory.php @@ -52,7 +52,7 @@ public function __invoke(?string $resourceClass = null, ?string $rootClass = nul $resolverContext = ['source' => $source, 'args' => $args, 'info' => $info, 'is_collection' => false, 'is_mutation' => false, 'is_subscription' => false]; if (!$operation) { - $operation = new Query(); + $operation = new Query(name: 'item_query'); } $item = ($this->readStage)($resourceClass, $rootClass, $operation, $resolverContext); diff --git a/src/GraphQl/Subscription/SubscriptionManager.php b/src/GraphQl/Subscription/SubscriptionManager.php index c9c7c501c79..14016b0a649 100644 --- a/src/GraphQl/Subscription/SubscriptionManager.php +++ b/src/GraphQl/Subscription/SubscriptionManager.php @@ -83,7 +83,7 @@ public function getPushPayloads(object $object): array foreach ($subscriptions as [$subscriptionId, $subscriptionFields, $subscriptionResult]) { $resolverContext = ['fields' => $subscriptionFields, 'is_collection' => false, 'is_mutation' => false, 'is_subscription' => true]; /** @var Operation */ - $operation = (new Subscription())->withName('update_subscription')->withShortName($shortName); + $operation = new Subscription(name: 'update', shortName: $shortName); $data = ($this->serializeStage)($object, $resourceClass, $operation, $resolverContext); unset($data['clientSubscriptionId']); diff --git a/src/GraphQl/Type/FieldsBuilder.php b/src/GraphQl/Type/FieldsBuilder.php index 17d37d2b35a..004e2044a5a 100644 --- a/src/GraphQl/Type/FieldsBuilder.php +++ b/src/GraphQl/Type/FieldsBuilder.php @@ -147,10 +147,6 @@ public function getSubscriptionFields(string $resourceClass, Operation $operatio } $subscriptionName = $operation->getName(); - // TODO: 3.0 change this - if ('update_subscription' === $subscriptionName) { - $subscriptionName = 'update'; - } $subscriptionFields[$subscriptionName.$operation->getShortName().'Subscribe'] = $fieldConfiguration; diff --git a/src/GraphQl/Type/TypeConverter.php b/src/GraphQl/Type/TypeConverter.php index f868962c6ae..d2eb7142eb9 100644 --- a/src/GraphQl/Type/TypeConverter.php +++ b/src/GraphQl/Type/TypeConverter.php @@ -82,10 +82,7 @@ public function convertType(Type $type, bool $input, Operation $rootOperation, s } catch (ResourceClassNotFoundException|OperationNotFoundException) { } /** @var Query $enumOperation */ - $enumOperation = (new Query()) - ->withClass($type->getClassName()) - ->withShortName($operation?->getShortName() ?? (new \ReflectionClass($type->getClassName()))->getShortName()) - ->withDescription($operation?->getDescription()); + $enumOperation = new Query(name: 'item_query', class: $type->getClassName(), shortName: $operation?->getShortName() ?? (new \ReflectionClass($type->getClassName()))->getShortName(), description: $operation?->getDescription()); return $this->typeBuilder->getEnumType($enumOperation); } diff --git a/src/Metadata/GraphQl/Query.php b/src/Metadata/GraphQl/Query.php index 858ce7ba7c6..458014e39ed 100644 --- a/src/Metadata/GraphQl/Query.php +++ b/src/Metadata/GraphQl/Query.php @@ -115,7 +115,7 @@ class: $class, fetchPartial: $fetchPartial, forceEager: $forceEager, priority: $priority, - name: $name ?: 'item_query', + name: $name, provider: $provider, processor: $processor, stateOptions: $stateOptions, diff --git a/src/Metadata/GraphQl/QueryCollection.php b/src/Metadata/GraphQl/QueryCollection.php index 7e16524f113..c36e99b4d5b 100644 --- a/src/Metadata/GraphQl/QueryCollection.php +++ b/src/Metadata/GraphQl/QueryCollection.php @@ -67,7 +67,7 @@ public function __construct( ?string $name = null, $provider = null, $processor = null, - protected ?OptionsInterface $stateOptions = null, + ?OptionsInterface $stateOptions = null, array $extraProperties = [], ?bool $nested = null, @@ -116,10 +116,11 @@ class: $class, fetchPartial: $fetchPartial, forceEager: $forceEager, priority: $priority, - name: $name ?: 'collection_query', + name: $name, provider: $provider, processor: $processor, extraProperties: $extraProperties, + stateOptions: $stateOptions, nested: $nested, ); } diff --git a/src/Metadata/GraphQl/Subscription.php b/src/Metadata/GraphQl/Subscription.php index 6ed4a6e9d8e..938e0567c1a 100644 --- a/src/Metadata/GraphQl/Subscription.php +++ b/src/Metadata/GraphQl/Subscription.php @@ -113,7 +113,7 @@ class: $class, fetchPartial: $fetchPartial, forceEager: $forceEager, priority: $priority, - name: $name ?: 'update_subscription', + name: $name, provider: $provider, processor: $processor, stateOptions: $stateOptions, diff --git a/src/Metadata/Resource/Factory/OperationDefaultsTrait.php b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php index 8d36237d40f..56d7c845633 100644 --- a/src/Metadata/Resource/Factory/OperationDefaultsTrait.php +++ b/src/Metadata/Resource/Factory/OperationDefaultsTrait.php @@ -93,13 +93,13 @@ private function getDefaultHttpOperations($resource): iterable private function addDefaultGraphQlOperations(ApiResource $resource): ApiResource { $graphQlOperations = []; - foreach ([new QueryCollection(), new Query(), (new Mutation())->withName('update'), (new DeleteMutation())->withName('delete'), (new Mutation())->withName('create')] as $operation) { + foreach ([new QueryCollection(name: 'collection_query'), new Query(name: 'item_query'), new Mutation(name: 'update'), new DeleteMutation(name: 'delete'), new Mutation(name: 'create')] as $operation) { [$key, $operation] = $this->getOperationWithDefaults($resource, $operation); $graphQlOperations[$key] = $operation; } if ($resource->getMercure()) { - [$key, $operation] = $this->getOperationWithDefaults($resource, (new Subscription())->withDescription("Subscribes to the update event of a {$operation->getShortName()}.")); + [$key, $operation] = $this->getOperationWithDefaults($resource, new Subscription(name: 'update', description: "Subscribes to the update event of a {$operation->getShortName()}.")); $graphQlOperations[$key] = $operation; } @@ -127,11 +127,11 @@ private function completeGraphQlOperations(ApiResource $resource): ApiResource } if (!$hasQueryOperation) { - $queryOperation = (new Query())->withNested(true); + $queryOperation = new Query(name: 'item_query', nested: true); $graphQlOperations[$queryOperation->getName()] = $queryOperation; } if (!$hasQueryCollectionOperation) { - $queryCollectionOperation = (new QueryCollection())->withNested(true); + $queryCollectionOperation = new QueryCollection(name: 'collection_query', nested: true); $graphQlOperations[$queryCollectionOperation->getName()] = $queryCollectionOperation; } @@ -168,7 +168,7 @@ private function getOperationWithDefaults(ApiResource $resource, Operation $oper if ($operation instanceof GraphQlOperation) { if (!$operation->getName()) { - throw new RuntimeException('No GraphQL operation name.'); + $operation = $operation->withName($this->getDefaultGraphQlOperationName($operation)); } if ($operation instanceof Mutation) { @@ -187,7 +187,6 @@ private function getOperationWithDefaults(ApiResource $resource, Operation $oper $operation = $operation->withName($operation->getRouteName()); } - $path = ($operation->getRoutePrefix() ?? '').($operation->getUriTemplate() ?? ''); $operationName = $operation->getName() ?? $this->getDefaultOperationName($operation, $resource->getClass()); return [ @@ -211,4 +210,15 @@ private function getDefaultOperationName(HttpOperation $operation, string $resou strtolower($operation->getMethod()), $operation instanceof CollectionOperationInterface ? '_collection' : ''); } + + private function getDefaultGraphQlOperationName(GraphQlOperation $operation): string + { + return match ($class = \get_class($operation)) { + Query::class => 'item_query', + QueryCollection::class => 'collection_query', + Mutation::class => 'update', + Subscription::class => 'update', + default => '_api_graphql_'.$this->getDefaultShortname($class), + }; + } } From 01d201b5a304fd25cbcc01a51b9bb69a7b277bec Mon Sep 17 00:00:00 2001 From: soyuka Date: Wed, 18 Jan 2023 17:50:35 +0100 Subject: [PATCH 2/2] remove item_query and collection_query from codebase --- src/GraphQl/Type/FieldsBuilder.php | 3 +- src/GraphQl/Type/TypeBuilder.php | 19 ++++++----- src/GraphQl/Type/TypeConverter.php | 18 +++++----- .../Resource/ResourceMetadataCollection.php | 33 +++++++++++++++++++ 4 files changed, 53 insertions(+), 20 deletions(-) diff --git a/src/GraphQl/Type/FieldsBuilder.php b/src/GraphQl/Type/FieldsBuilder.php index 004e2044a5a..4f015113c8f 100644 --- a/src/GraphQl/Type/FieldsBuilder.php +++ b/src/GraphQl/Type/FieldsBuilder.php @@ -78,6 +78,7 @@ public function getItemQueryFields(string $resourceClass, Operation $operation, return []; } + // TODO: it'd be nice to get rid of this hard coded name $fieldName = lcfirst('item_query' === $operation->getName() ? $operation->getShortName() : $operation->getName().$operation->getShortName()); if ($fieldConfiguration = $this->getResourceFieldConfiguration(null, $operation->getDescription(), $operation->getDeprecationReason(), new Type(Type::BUILTIN_TYPE_OBJECT, true, $resourceClass), $resourceClass, false, $operation)) { @@ -287,7 +288,7 @@ private function getResourceFieldConfiguration(?string $property, ?string $field $resourceOperation = $rootOperation; if ($resourceClass && $depth >= 1 && $this->resourceClassResolver->isResourceClass($resourceClass)) { $resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($resourceClass); - $resourceOperation = $resourceMetadataCollection->getOperation($isCollectionType ? 'collection_query' : 'item_query'); + $resourceOperation = $resourceMetadataCollection->getGraphQlOperation(null, $isCollectionType); } if (!$resourceOperation instanceof Operation) { diff --git a/src/GraphQl/Type/TypeBuilder.php b/src/GraphQl/Type/TypeBuilder.php index 4c9fec5e649..40147abf7ba 100644 --- a/src/GraphQl/Type/TypeBuilder.php +++ b/src/GraphQl/Type/TypeBuilder.php @@ -19,6 +19,7 @@ use ApiPlatform\Metadata\GraphQl\Mutation; use ApiPlatform\Metadata\GraphQl\Operation; use ApiPlatform\Metadata\GraphQl\Query; +use ApiPlatform\Metadata\GraphQl\QueryCollection; use ApiPlatform\Metadata\GraphQl\Subscription; use ApiPlatform\Metadata\Resource\ResourceMetadataCollection; use ApiPlatform\State\Pagination\Pagination; @@ -73,10 +74,12 @@ public function getResourceObjectType(?string $resourceClass, ResourceMetadataCo $shortName .= 'Payload'; } - if ('item_query' === $operationName || 'collection_query' === $operationName) { + if (!$operation->getResolver() && ($operation instanceof Query || $operation instanceof QueryCollection)) { // Test if the collection/item has different groups - if ($resourceMetadataCollection->getOperation($operation instanceof CollectionOperationInterface ? 'item_query' : 'collection_query')->getNormalizationContext() !== $operation->getNormalizationContext()) { - $shortName .= $operation instanceof CollectionOperationInterface ? 'Collection' : 'Item'; + $isCollection = $operation instanceof CollectionOperationInterface; + $otherOperation = $resourceMetadataCollection->getGraphQlOperation(null, !$isCollection); + if ($otherOperation->getNormalizationContext() !== $operation->getNormalizationContext()) { + $shortName .= $isCollection ? 'Collection' : 'Item'; } } @@ -104,12 +107,12 @@ public function getResourceObjectType(?string $resourceClass, ResourceMetadataCo 'name' => $shortName, 'description' => $operation->getDescription(), 'resolveField' => $this->defaultFieldResolver, - 'fields' => function () use ($resourceClass, $operation, $operationName, $resourceMetadataCollection, $input, $wrapData, $depth, $ioMetadata) { + 'fields' => function () use ($resourceClass, $operation, $resourceMetadataCollection, $input, $wrapData, $depth, $ioMetadata) { if ($wrapData) { $queryNormalizationContext = $this->getQueryOperation($resourceMetadataCollection)?->getNormalizationContext() ?? []; try { - $mutationNormalizationContext = $operation instanceof Mutation || $operation instanceof Subscription ? ($resourceMetadataCollection->getOperation($operationName)->getNormalizationContext() ?? []) : []; + $mutationNormalizationContext = $operation instanceof Mutation || $operation instanceof Subscription ? ($operation->getNormalizationContext() ?? []) : []; } catch (OperationNotFoundException) { $mutationNormalizationContext = []; } @@ -117,14 +120,12 @@ public function getResourceObjectType(?string $resourceClass, ResourceMetadataCo // If not, use the query type in order to ensure the client cache could be used. $useWrappedType = $queryNormalizationContext !== $mutationNormalizationContext; - $wrappedOperationName = $operationName; + $wrappedOperation = $operation; if (!$useWrappedType) { - $wrappedOperationName = $operation instanceof Query ? $operationName : 'item_query'; + $wrappedOperation = $resourceMetadataCollection->getGraphQlOperation(null, forceCollection: $operation instanceof Query); } - $wrappedOperation = $resourceMetadataCollection->getOperation($wrappedOperationName); - $fields = [ lcfirst($wrappedOperation->getShortName()) => $this->getResourceObjectType($resourceClass, $resourceMetadataCollection, $wrappedOperation instanceof Operation ? $wrappedOperation : null, $input, true, $depth), ]; diff --git a/src/GraphQl/Type/TypeConverter.php b/src/GraphQl/Type/TypeConverter.php index d2eb7142eb9..62fa6009f81 100644 --- a/src/GraphQl/Type/TypeConverter.php +++ b/src/GraphQl/Type/TypeConverter.php @@ -167,16 +167,14 @@ private function getResourceType(Type $type, bool $input, Operation $rootOperati // We're retrieving the type of a property which is a relation to the root resource. if ($resourceClass !== $rootResource && $rootOperation instanceof Query) { - $operationName = $isCollection ? 'collection_query' : 'item_query'; - } - - try { - $operation = $resourceMetadataCollection->getOperation($operationName); - } catch (OperationNotFoundException) { - $operation = $resourceMetadataCollection->getOperation($isCollection ? 'collection_query' : 'item_query'); - } - if (!$operation instanceof Operation) { - throw new OperationNotFoundException(); + $operation = $resourceMetadataCollection->getGraphQlOperation(null, $isCollection); + } else { + try { + $operation = $resourceMetadataCollection->getGraphQlOperation($operationName); + } catch (OperationNotFoundException) { + // try to fetch the default operation in case we didn't found the named one + $operation = $resourceMetadataCollection->getGraphQlOperation(null, $isCollection); + } } return $this->typeBuilder->getResourceObjectType($resourceClass, $resourceMetadataCollection, $operation, $input, false, $depth); diff --git a/src/Metadata/Resource/ResourceMetadataCollection.php b/src/Metadata/Resource/ResourceMetadataCollection.php index 5260095bced..d48b131b7d7 100644 --- a/src/Metadata/Resource/ResourceMetadataCollection.php +++ b/src/Metadata/Resource/ResourceMetadataCollection.php @@ -16,6 +16,7 @@ use ApiPlatform\Exception\OperationNotFoundException; use ApiPlatform\Metadata\ApiResource; use ApiPlatform\Metadata\CollectionOperationInterface; +use ApiPlatform\Metadata\GraphQl\Operation as GraphQlOperation; use ApiPlatform\Metadata\HttpOperation; use ApiPlatform\Metadata\Operation; @@ -96,6 +97,38 @@ public function getOperation(?string $operationName = null, bool $forceCollectio $this->handleNotFound($operationName, $metadata); } + public function getGraphQlOperation(?string $operationName = null, bool $forceCollection = false): GraphQlOperation + { + $operationName ??= ''; + $cachePrefix = ($forceCollection ? self::FORCE_COLLECTION : ''); + $gqlCacheKey = self::GRAPHQL_PREFIX.$cachePrefix.$operationName; + if (isset($this->operationCache[$gqlCacheKey])) { + return $this->operationCache[$gqlCacheKey]; + } + + $it = $this->getIterator(); + + while ($it->valid()) { + /** @var ApiResource $metadata */ + $metadata = $it->current(); + + foreach ($metadata->getGraphQlOperations() ?? [] as $name => $operation) { + $isCollection = $operation instanceof CollectionOperationInterface; + if ('' === $operationName && ($forceCollection ? $isCollection : !$isCollection)) { + return $this->operationCache[$gqlCacheKey] = $operation; + } + + if ($name === $operationName) { + return $this->operationCache[$gqlCacheKey] = $operation; + } + } + + $it->next(); + } + + $this->handleNotFound($operationName, $metadata); + } + /** * @throws OperationNotFoundException */