Skip to content

Support union/intersect types #5470

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 14 commits into from
Jul 6, 2023
Merged
121 changes: 121 additions & 0 deletions features/main/union_intersect_types.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,121 @@
Feature: Union/Intersect types

Scenario Outline: Create a resource with union type
When I add "Content-Type" header equal to "application/ld+json"
And I add "Accept" header equal to "application/ld+json"
And I send a "POST" request to "/issue-5452/books" with body:
"""
{
"number": <number>,
"isbn": "978-3-16-148410-0"
}
"""
Then the response status code should be 201
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON should be valid according to this schema:
"""
{
"type": "object",
"properties": {
"@type": {
"type": "string",
"pattern": "^Book$"
},
"@context": {
"type": "string",
"pattern": "^/contexts/Book$"
},
"@id": {
"type": "string",
"pattern": "^/.well-known/genid/.+$"
},
"number": {
"type": "<type>"
},
"isbn": {
"type": "string",
"pattern": "^978-3-16-148410-0$"
}
},
"required": [
"@type",
"@context",
"@id",
"number",
"isbn"
]
}
"""
Examples:
| number | type |
| "1" | string |
| 1 | integer |

Scenario: Create a resource with valid intersect type
When I add "Content-Type" header equal to "application/ld+json"
And I send a "POST" request to "/issue-5452/books" with body:
"""
{
"number": 1,
"isbn": "978-3-16-148410-0",
"author": "/issue-5452/authors/1"
}
"""
Then the response status code should be 201
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON should be valid according to this schema:
"""
{
"type": "object",
"properties": {
"@type": {
"type": "string",
"pattern": "^Book$"
},
"@context": {
"type": "string",
"pattern": "^/contexts/Book$"
},
"@id": {
"type": "string",
"pattern": "^/.well-known/genid/.+$"
},
"number": {
"type": "integer"
},
"isbn": {
"type": "string",
"pattern": "^978-3-16-148410-0$"
},
"author": {
"type": "string",
"pattern": "^/issue-5452/authors/1$"
}
},
"required": [
"@type",
"@context",
"@id",
"number",
"isbn",
"author"
]
}
"""

Scenario: Create a resource with invalid intersect type
When I add "Content-Type" header equal to "application/ld+json"
And I send a "POST" request to "/issue-5452/books" with body:
"""
{
"number": 1,
"isbn": "978-3-16-148410-0",
"library": "/issue-5452/libraries/1"
}
"""
Then the response status code should be 400
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the JSON node "hydra:description" should be equal to 'Could not denormalize object of type "ApiPlatform\Tests\Fixtures\TestBundle\ApiResource\Issue5452\ActivableInterface", no supporting normalizer found.'
22 changes: 12 additions & 10 deletions features/openapi/docs.feature
Original file line number Diff line number Diff line change
Expand Up @@ -86,19 +86,19 @@ Feature: Documentation support
{
"default": "male",
"example": "male",
"type": "string",
"type": ["string", "null"],
"enum": [
"male",
"female",
null
],
"nullable": true
]
}
"""
And the "playMode" property exists for the OpenAPI class "VideoGame"
And the "playMode" property for the OpenAPI class "VideoGame" should be equal to:
"""
{
"owl:maxCardinality": 1,
"type": "string",
"format": "iri-reference"
}
Expand Down Expand Up @@ -238,8 +238,7 @@ Feature: Documentation support
"type": "string"
},
"property": {
"type": "string",
"nullable": true
"type": ["string", "null"]
},
"required": {
"type": "boolean"
Expand Down Expand Up @@ -310,12 +309,15 @@ Feature: Documentation support
And the "resourceRelated" property for the OpenAPI class "Resource" should be equal to:
"""
{
"readOnly":true,
"anyOf":[
"owl:maxCardinality": 1,
"readOnly": true,
"anyOf": [
{
"$ref": "#/components/schemas/ResourceRelated"
},
{
"$ref":"#/components/schemas/ResourceRelated"
"type": "null"
}
],
"nullable":true
]
}
"""
70 changes: 47 additions & 23 deletions src/Elasticsearch/Filter/AbstractFilter.php
Original file line number Diff line number Diff line change
Expand Up @@ -93,46 +93,70 @@ protected function getMetadata(string $resourceClass, string $property): array
return $noop;
}

$type = $propertyMetadata->getBuiltinTypes()[0] ?? null;
$types = $propertyMetadata->getBuiltinTypes();

if (null === $type) {
if (null === $types) {
return $noop;
}

++$index;
$builtinType = $type->getBuiltinType();

if (Type::BUILTIN_TYPE_OBJECT !== $builtinType && Type::BUILTIN_TYPE_ARRAY !== $builtinType) {
if ($totalProperties === $index) {
break;
// check each type before deciding if it's noop or not
// e.g: maybe the first type is noop, but the second is valid
$isNoop = false;

foreach ($types as $type) {
$builtinType = $type->getBuiltinType();

if (Type::BUILTIN_TYPE_OBJECT !== $builtinType && Type::BUILTIN_TYPE_ARRAY !== $builtinType) {
if ($totalProperties === $index) {
break 2;
}

$isNoop = true;

continue;
}

return $noop;
}
if ($type->isCollection() && null === $type = $type->getCollectionValueTypes()[0] ?? null) {
$isNoop = true;

if ($type->isCollection() && null === $type = $type->getCollectionValueTypes()[0] ?? null) {
return $noop;
}
continue;
}

if (Type::BUILTIN_TYPE_ARRAY === $builtinType && Type::BUILTIN_TYPE_OBJECT !== $type->getBuiltinType()) {
if ($totalProperties === $index) {
break 2;
}

$isNoop = true;

if (Type::BUILTIN_TYPE_ARRAY === $builtinType && Type::BUILTIN_TYPE_OBJECT !== $type->getBuiltinType()) {
if ($totalProperties === $index) {
break;
continue;
}

return $noop;
}
if (null === $className = $type->getClassName()) {
$isNoop = true;

if (null === $className = $type->getClassName()) {
return $noop;
continue;
}

if ($isResourceClass = $this->resourceClassResolver->isResourceClass($className)) {
$currentResourceClass = $className;
} elseif ($totalProperties !== $index) {
$isNoop = true;

continue;
}

$hasAssociation = $totalProperties === $index && $isResourceClass;
$isNoop = false;

break;
}

if ($isResourceClass = $this->resourceClassResolver->isResourceClass($className)) {
$currentResourceClass = $className;
} elseif ($totalProperties !== $index) {
if ($isNoop) {
return $noop;
}

$hasAssociation = $totalProperties === $index && $isResourceClass;
}

return [$type, $hasAssociation, $currentResourceClass, $currentProperty];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have a feeling that having a property that is a scalar and a resource class is such a bad practice that we should just not support it.

Expand Down
45 changes: 21 additions & 24 deletions src/Elasticsearch/Util/FieldDatatypeTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -59,30 +59,27 @@ private function getNestedFieldPath(string $resourceClass, string $property): ?s
return null;
}

// TODO: 3.0 allow multiple types
$type = $propertyMetadata->getBuiltinTypes()[0] ?? null;

if (null === $type) {
return null;
}

if (
Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType()
&& null !== ($nextResourceClass = $type->getClassName())
&& $this->resourceClassResolver->isResourceClass($nextResourceClass)
) {
$nestedPath = $this->getNestedFieldPath($nextResourceClass, implode('.', $properties));

return null === $nestedPath ? $nestedPath : "$currentProperty.$nestedPath";
}

if (
null !== ($type = $type->getCollectionValueTypes()[0] ?? null)
&& Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType()
&& null !== ($className = $type->getClassName())
&& $this->resourceClassResolver->isResourceClass($className)
) {
return $currentProperty;
$types = $propertyMetadata->getBuiltinTypes() ?? [];

foreach ($types as $type) {
if (
Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType()
&& null !== ($nextResourceClass = $type->getClassName())
&& $this->resourceClassResolver->isResourceClass($nextResourceClass)
) {
$nestedPath = $this->getNestedFieldPath($nextResourceClass, implode('.', $properties));

return null === $nestedPath ? $nestedPath : "$currentProperty.$nestedPath";
}

if (
null !== ($type = $type->getCollectionValueTypes()[0] ?? null)
&& Type::BUILTIN_TYPE_OBJECT === $type->getBuiltinType()
&& null !== ($className = $type->getClassName())
&& $this->resourceClassResolver->isResourceClass($className)
) {
return $currentProperty;
}
}

return null;
Expand Down
12 changes: 9 additions & 3 deletions src/GraphQl/Type/FieldsBuilder.php
Original file line number Diff line number Diff line change
Expand Up @@ -213,17 +213,23 @@ public function getResourceObjectTypeFields(?string $resourceClass, Operation $o
'denormalization_groups' => $operation->getDenormalizationContext()['groups'] ?? null,
];
$propertyMetadata = $this->propertyMetadataFactory->create($resourceClass, $property, $context);
$propertyTypes = $propertyMetadata->getBuiltinTypes();

if (
null === ($propertyType = $propertyMetadata->getBuiltinTypes()[0] ?? null)
!$propertyTypes
|| (!$input && false === $propertyMetadata->isReadable())
|| ($input && $operation instanceof Mutation && false === $propertyMetadata->isWritable())
) {
continue;
}

if ($fieldConfiguration = $this->getResourceFieldConfiguration($property, $propertyMetadata->getDescription(), $propertyMetadata->getDeprecationReason(), $propertyType, $resourceClass, $input, $operation, $depth, null !== $propertyMetadata->getSecurity())) {
$fields['id' === $property ? '_id' : $this->normalizePropertyName($property, $resourceClass)] = $fieldConfiguration;
// guess union/intersect types: check each type until finding a valid one
foreach ($propertyTypes as $propertyType) {
if ($fieldConfiguration = $this->getResourceFieldConfiguration($property, $propertyMetadata->getDescription(), $propertyMetadata->getDeprecationReason(), $propertyType, $resourceClass, $input, $operation, $depth, null !== $propertyMetadata->getSecurity())) {
$fields['id' === $property ? '_id' : $this->normalizePropertyName($property, $resourceClass)] = $fieldConfiguration;
// stop at the first valid type
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@alanpoulain doesn't graphql support multiple types ?

Copy link
Contributor Author

@vincentchalamon vincentchalamon Apr 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If it's not an input type, yes. Otherwise no (see graphql/graphql-spec#825).

break;
}
}
}
}
Expand Down
Loading