Skip to content

[Subresource] Fixed the serialization context builder #1617

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

[Subresource] Fixed the serialization context builder #1617

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions src/Bridge/Symfony/Routing/ApiLoader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,6 +111,8 @@ public function load($data, $type = null): RouteCollection
'property' => $operation['property'],
'identifiers' => $operation['identifiers'],
'collection' => $operation['collection'],
'parent_resource_class' => $resourceClass,
Copy link
Member

Choose a reason for hiding this comment

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

We can't say that it's the "parent" because there can be an "infinity" IMO.
Anyway, we need to discuss here is whether we take the "Root" as the metadata entrypoint or the "Queue" (last).
Explanation:

/foo/1/bars/2

This path will get you an entity of type Bar. When this is serialized, where should we look to get the normalization_context?

  • Bar metadata, after all this is a Bar response
  • Foo metadata, because it's a subresource

We need to choose one. At the moment, IIRC the code looks on the Bar metadata to find the normalization_context for the given operation.

If we want to take the "Root" (because this is where we declared the subresource), I don't think that you need the "parent_resource_class".
Take a look at the SubresourceOperationFactory:

core/tests/Operation/Factory/SubresourceOperationFactoryTest.php

Line 84 in a5dc795

],

$rootClass = $identifiers[0][1] is where you want to look at.
the operation_name should be the correct one IMO

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I vote for:

Foo metadata, because it's a subresource

$rootClass = $identifiers[0][1] is where you want to look at.

yes I noticed that. But I thought it was risky to use that ;)

Copy link
Member

Choose a reason for hiding this comment

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

yes I noticed that. But I thought it was risky to use that ;)

It is not and it's meant for that ;)

'parent_operation_name' => $operation['operation_name'],
'operationId' => $operationId,
],
] + $operation['defaults'] ?? [],
Expand Down
39 changes: 23 additions & 16 deletions src/Serializer/SerializerContextBuilder.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,36 +42,43 @@ public function createFromRequest(Request $request, bool $normalization, array $
throw new RuntimeException('Request attributes are not valid.');
}

$resourceMetadata = $this->resourceMetadataFactory->create($attributes['resource_class']);
$key = $normalization ? 'normalization_context' : 'denormalization_context';

$operationKey = null;
$operationType = null;

if (isset($attributes['collection_operation_name'])) {
$operationKey = 'collection_operation_name';
$operationType = OperationType::COLLECTION;
} elseif (isset($attributes['subresource_operation_name'])) {
$operationKey = 'subresource_operation_name';
$operationType = OperationType::SUBRESOURCE;
}
$attribute = $attributes['collection_operation_name'];
$resourceClass = $attributes['resource_class'];

if (null !== $operationKey) {
$attribute = $attributes[$operationKey];
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
$context = $resourceMetadata->getCollectionOperationAttribute($attribute, $key, [], true);
$context[$operationKey] = $attribute;

$context['collection_operation_name'] = $attribute;
$context['resource_class'] = $resourceClass;
$context['operation_type'] = OperationType::COLLECTION;
} elseif (isset($attributes['subresource_operation_name'])) {
$attribute = $attributes['subresource_operation_name'];
$resourceClass = $attributes['resource_class'];
$parentClass = $attributes['subresource_context']['parent_resource_class'];
$parentOperationName = $attributes['subresource_context']['parent_operation_name'];

$parentMetadata = $this->resourceMetadataFactory->create($parentClass);
$context = $parentMetadata->getSubresourceOperationAttribute($parentOperationName, $key, [], true);

$context['subresource_operation_name'] = $attribute;
$context['resource_class'] = $resourceClass;
$context['operation_type'] = OperationType::SUBRESOURCE;
} else {
$resourceClass = $attributes['resource_class'];
$resourceMetadata = $this->resourceMetadataFactory->create($resourceClass);
$context = $resourceMetadata->getItemOperationAttribute($attributes['item_operation_name'], $key, [], true);
$context['item_operation_name'] = $attributes['item_operation_name'];
$context['resource_class'] = $attributes['resource_class'];
$context['operation_type'] = OperationType::ITEM;
}

$context['operation_type'] = $operationType ?: OperationType::ITEM;

if (!$normalization && !isset($context['api_allow_update'])) {
$context['api_allow_update'] = Request::METHOD_PUT === $request->getMethod();
}

$context['resource_class'] = $attributes['resource_class'];
$context['request_uri'] = $request->getRequestUri();

if (isset($attributes['subresource_context'])) {
Expand Down
8 changes: 4 additions & 4 deletions src/Util/RequestAttributesExtractor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,14 +41,14 @@ public static function extractAttributes(Request $request)
{
$result = ['resource_class' => $request->attributes->get('_api_resource_class')];

if ($subresourceContext = $request->attributes->get('_api_subresource_context')) {
$result['subresource_context'] = $subresourceContext;
}

if (null === $result['resource_class']) {
return [];
}

if ($subresourceContext = $request->attributes->get('_api_subresource_context')) {
$result['subresource_context'] = $subresourceContext;
}

$hasRequestAttributeKey = false;
foreach (OperationType::TYPES as $operationType) {
$attribute = "_api_{$operationType}_operation_name";
Expand Down