Skip to content

Commit

Permalink
feat: spec-compliant PUT method (#4996)
Browse files Browse the repository at this point in the history
Co-authored-by: soyuka <soyuka@users.noreply.github.com>
  • Loading branch information
dunglas and soyuka authored Jan 11, 2023
1 parent 9e69e9d commit bde59ba
Show file tree
Hide file tree
Showing 35 changed files with 453 additions and 48 deletions.
2 changes: 1 addition & 1 deletion features/graphql/subscription.feature
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,7 @@ Feature: GraphQL subscription support
And the JSON node "data.updateDummyMercureSubscribe.dummyMercure.id" should be equal to "/dummy_mercures/2"
And the JSON node "data.updateDummyMercureSubscribe.mercureUrl" should match "@^https://demo.mercure.rocks/hub\?topic=http://example.com/subscriptions/[a-f0-9]+$@"

Scenario: Receive Mercure updates with different payloads from subscriptions
Scenario: Receive Mercure updates with different payloads from subscriptions (legacy PUT in non-standard mode)
When I add "Accept" header equal to "application/ld+json"
And I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/dummy_mercures/1" with body:
Expand Down
46 changes: 45 additions & 1 deletion features/hal/hal.feature
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ Feature: HAL support
}
"""

Scenario: Update a resource
Scenario: Update a resource (legacy PUT as standard_put: false)
When I add "Accept" header equal to "application/hal+json"
And I add "Content-Type" header equal to "application/json"
And I send a "PUT" request to "/dummies/1" with body:
Expand Down Expand Up @@ -127,6 +127,50 @@ Feature: HAL support
}
"""

Scenario: Update a resource
When I add "Accept" header equal to "application/hal+json"
When I add "Content-Type" header equal to "application/merge-patch+json"
And I send a "PATCH" request to "/dummies/1" with body:
"""
{"name": "A nice dummy"}
"""
Then the response status code should be 200
And the response should be in JSON
And the JSON should be valid according to the JSON HAL schema
And the header "Content-Type" should be equal to "application/hal+json; charset=utf-8"
And the JSON should be equal to:
"""
{
"_links": {
"self": {
"href": "/dummies/1"
},
"relatedDummy": {
"href": "/related_dummies/1"
},
"relatedDummies": [
{
"href": "/related_dummies/1"
}
]
},
"description": null,
"dummy": null,
"dummyBoolean": null,
"dummyDate": "2015-03-01T10:00:00+00:00",
"dummyFloat": null,
"dummyPrice": null,
"jsonData": [],
"arrayData": [],
"name_converted": null,
"id": 1,
"name": "A nice dummy",
"alias": null,
"foo": null
}
"""


Scenario: Embed a relation in a parent object
When I add "Content-Type" header equal to "application/json"
And I send a "POST" request to "/relation_embedders" with body:
Expand Down
2 changes: 1 addition & 1 deletion features/http_cache/tags.feature
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ Feature: Cache invalidation through HTTP Cache tags
Then the response status code should be 200
And the header "Cache-Tags" should be equal to "/relation3s/1,/relation2s/1,/relation2s/2,/relation3s"

Scenario: Update a collection member only
Scenario: Update a collection member only (legacy non-standard PUT)
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/relation3s/1" with body:
"""
Expand Down
26 changes: 25 additions & 1 deletion features/main/custom_normalized.feature
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ Feature: Using custom normalized entity
}
"""

Scenario: Update a resource
Scenario: Update a resource (legacy non-standard PUT)
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/custom_normalized_dummies/1" with body:
"""
Expand All @@ -171,6 +171,30 @@ Feature: Using custom normalized entity
}
"""

Scenario: Update a resource
When I add "Content-Type" header equal to "application/merge-patch+json"
And I send a "PATCH" request to "/custom_normalized_dummies/1" with body:
"""
{
"name": "My Dummy modified"
}
"""
Then the response status code should be 200
And the response should be in JSON
And the header "Content-Type" should be equal to "application/ld+json; charset=utf-8"
And the header "Content-Location" should be equal to "/custom_normalized_dummies/1"
And the JSON should be equal to:
"""
{
"@context": "/contexts/CustomNormalizedDummy",
"@id": "/custom_normalized_dummies/1",
"@type": "CustomNormalizedDummy",
"id": 1,
"name": "My Dummy modified",
"alias": "My alias"
}
"""

Scenario: API doc is correctly generated
When I send a "GET" request to "/docs.jsonld"
Then the response status code should be 200
Expand Down
2 changes: 1 addition & 1 deletion features/main/custom_writable_identifier.feature
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ Feature: Using custom writable identifier on resource
"""

@!mongodb
Scenario: Update a resource
Scenario: Update a resource (legacy non-standard PUT)
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/custom_writable_identifier_dummies/my_slug" with body:
"""
Expand Down
49 changes: 49 additions & 0 deletions features/main/standard_put.feature
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
Feature: Spec-compliant PUT support
As a client software developer
I need to be able to create or replace resources using the PUT HTTP method

@createSchema
Scenario: Create a new resource
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/standard_puts/5" with body:
"""
{
"foo": "a",
"bar": "b"
}
"""
Then the response status code should be 201
And the response should be in JSON
And the JSON should be equal to:
"""
{
"@context": "/contexts/StandardPut",
"@id": "/standard_puts/5",
"@type": "StandardPut",
"id": 5,
"foo": "a",
"bar": "b"
}
"""

Scenario: Replace an existing resource
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/standard_puts/5" with body:
"""
{
"foo": "c"
}
"""
Then the response status code should be 200
And the response should be in JSON
And the JSON should be equal to:
"""
{
"@context": "/contexts/StandardPut",
"@id": "/standard_puts/5",
"@type": "StandardPut",
"id": 5,
"foo": "c",
"bar": ""
}
"""
25 changes: 24 additions & 1 deletion features/serializer/vo_relations.feature
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ Feature: Value object as ApiResource
}
"""

Scenario: Update Value object with writable and non writable property
Scenario: Update Value object with writable and non writable property (legacy non-standard PUT)
When I add "Content-Type" header equal to "application/ld+json"
And I send a "PUT" request to "/vo_dummy_inspections/1" with body:
"""
Expand All @@ -102,6 +102,29 @@ Feature: Value object as ApiResource
}
"""

Scenario: Update Value object with writable and non writable property
When I add "Content-Type" header equal to "application/merge-patch+json"
And I send a "PATCH" request to "/vo_dummy_inspections/1" with body:
"""
{
"performed": "2018-08-24 00:00:00",
"accepted": false
}
"""
Then the response status code should be 200
And the JSON should be equal to:
"""
{
"@context": "/contexts/VoDummyInspection",
"@id": "/vo_dummy_inspections/1",
"@type": "VoDummyInspection",
"accepted": true,
"car": "/vo_dummy_cars/1",
"performed": "2018-08-24T00:00:00+00:00"
}
"""


@createSchema
Scenario: Create Value object without required params
When I add "Content-Type" header equal to "application/ld+json"
Expand Down
2 changes: 0 additions & 2 deletions phpstan.neon.dist
Original file line number Diff line number Diff line change
Expand Up @@ -85,5 +85,3 @@ parameters:
-
message: '#^Property .+ is unused.$#'
path: tests/Doctrine/Odm/PropertyInfo/Fixtures/DoctrineDummy.php
# Waiting for https://github.com/laminas/laminas-code/pull/150
- '#Call to an undefined method ReflectionEnum::.+#'
7 changes: 7 additions & 0 deletions src/Doctrine/Common/State/LinksHandlerTrait.php
Original file line number Diff line number Diff line change
Expand Up @@ -20,9 +20,12 @@
use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Link;
use ApiPlatform\Metadata\Operation;
use ApiPlatform\Metadata\Resource\Factory\ResourceMetadataCollectionFactoryInterface;

trait LinksHandlerTrait
{
private ?ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory;

/**
* @return Link[]
*/
Expand All @@ -48,6 +51,10 @@ private function getLinks(string $resourceClass, Operation $operation, array $co
return [$newLink];
}

if (!$this->resourceMetadataCollectionFactory) {
return [];
}

// Using GraphQL, it's possible that we won't find a GraphQL Operation of the same type (e.g. it is disabled).
try {
$resourceMetadataCollection = $this->resourceMetadataCollectionFactory->create($linkClass);
Expand Down
84 changes: 75 additions & 9 deletions src/Doctrine/Common/State/PersistProcessor.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@

namespace ApiPlatform\Doctrine\Common\State;

use ApiPlatform\Metadata\HttpOperation;
use ApiPlatform\Metadata\Operation;
use ApiPlatform\State\ProcessorInterface;
use ApiPlatform\Util\ClassInfoTrait;
Expand All @@ -24,6 +25,7 @@
final class PersistProcessor implements ProcessorInterface
{
use ClassInfoTrait;
use LinksHandlerTrait;

public function __construct(private readonly ManagerRegistry $managerRegistry)
{
Expand All @@ -38,10 +40,65 @@ public function __construct(private readonly ManagerRegistry $managerRegistry)
*/
public function process(mixed $data, Operation $operation, array $uriVariables = [], array $context = []): mixed
{
if (!$manager = $this->getManager($data)) {
if (
!\is_object($data) ||
!$manager = $this->managerRegistry->getManagerForClass($class = $this->getObjectClass($data))
) {
return $data;
}

// PUT: reset the existing object managed by Doctrine and merge data sent by the user in it
// This custom logic is needed because EntityManager::merge() has been deprecated and UPSERT isn't supported:
// https://github.com/doctrine/orm/issues/8461#issuecomment-1250233555
if ($operation instanceof HttpOperation && HttpOperation::METHOD_PUT === $operation->getMethod() && ($operation->getExtraProperties()['standard_put'] ?? false)) {
\assert(method_exists($manager, 'getReference'));
// TODO: the call to getReference is most likely to fail with complex identifiers
$newData = $data;
if (isset($context['previous_data'])) {
$newData = 1 === \count($uriVariables) ? $manager->getReference($class, current($uriVariables)) : clone $context['previous_data'];
}

$identifiers = array_reverse($uriVariables);
$links = $this->getLinks($class, $operation, $context);
$reflectionProperties = $this->getReflectionProperties($data);

if (!isset($context['previous_data'])) {
foreach (array_reverse($links) as $link) {
if ($link->getExpandedValue() || !$link->getFromClass()) {
continue;
}

$identifierProperties = $link->getIdentifiers();
$hasCompositeIdentifiers = 1 < \count($identifierProperties);

foreach ($identifierProperties as $identifierProperty) {
$reflectionProperty = $reflectionProperties[$identifierProperty];
$reflectionProperty->setValue($newData, $this->getIdentifierValue($identifiers, $hasCompositeIdentifiers ? $identifierProperty : null));
}
}
} else {
foreach ($reflectionProperties as $propertyName => $reflectionProperty) {
// Don't override the property if it's part of the subresource system
if (isset($uriVariables[$propertyName])) {
continue;
}

foreach ($links as $link) {
$identifierProperties = $link->getIdentifiers();
if (\in_array($propertyName, $identifierProperties, true)) {
continue;
}

if (($newValue = $reflectionProperty->getValue($data)) !== $reflectionProperty->getValue($newData)) {
$reflectionProperty->setValue($newData, $newValue);
}
}
}
}

$data = $newData;
}

if (!$manager->contains($data) || $this->isDeferredExplicit($manager, $data)) {
$manager->persist($data);
}
Expand All @@ -52,14 +109,6 @@ public function process(mixed $data, Operation $operation, array $uriVariables =
return $data;
}

/**
* Gets the Doctrine object manager associated with given data.
*/
private function getManager($data): ?DoctrineObjectManager
{
return \is_object($data) ? $this->managerRegistry->getManagerForClass($this->getObjectClass($data)) : null;
}

/**
* Checks if doctrine does not manage data automatically.
*/
Expand All @@ -72,4 +121,21 @@ private function isDeferredExplicit(DoctrineObjectManager $manager, $data): bool

return false;
}

/**
* Get reflection properties indexed by property name.
*
* @return array<string, \ReflectionProperty>
*/
private function getReflectionProperties(mixed $data): array
{
$ret = [];
$props = (new \ReflectionObject($data))->getProperties(~\ReflectionProperty::IS_STATIC);

foreach ($props as $prop) {
$ret[$prop->getName()] = $prop;
}

return $ret;
}
}
3 changes: 2 additions & 1 deletion src/Doctrine/Odm/State/CollectionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -33,8 +33,9 @@ final class CollectionProvider implements ProviderInterface
/**
* @param AggregationCollectionExtensionInterface[] $collectionExtensions
*/
public function __construct(private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly ManagerRegistry $managerRegistry, private readonly iterable $collectionExtensions = [])
public function __construct(ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly ManagerRegistry $managerRegistry, private readonly iterable $collectionExtensions = [])
{
$this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory;
}

public function provide(Operation $operation, array $uriVariables = [], array $context = []): iterable
Expand Down
3 changes: 2 additions & 1 deletion src/Doctrine/Odm/State/ItemProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ final class ItemProvider implements ProviderInterface
/**
* @param AggregationItemExtensionInterface[] $itemExtensions
*/
public function __construct(private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly ManagerRegistry $managerRegistry, private readonly iterable $itemExtensions = [])
public function __construct(ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly ManagerRegistry $managerRegistry, private readonly iterable $itemExtensions = [])
{
$this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory;
}

public function provide(Operation $operation, array $uriVariables = [], array $context = []): ?object
Expand Down
3 changes: 2 additions & 1 deletion src/Doctrine/Orm/State/CollectionProvider.php
Original file line number Diff line number Diff line change
Expand Up @@ -36,8 +36,9 @@ final class CollectionProvider implements ProviderInterface
/**
* @param QueryCollectionExtensionInterface[] $collectionExtensions
*/
public function __construct(private readonly ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly ManagerRegistry $managerRegistry, private readonly iterable $collectionExtensions = [])
public function __construct(ResourceMetadataCollectionFactoryInterface $resourceMetadataCollectionFactory, private readonly ManagerRegistry $managerRegistry, private readonly iterable $collectionExtensions = [])
{
$this->resourceMetadataCollectionFactory = $resourceMetadataCollectionFactory;
}

public function provide(Operation $operation, array $uriVariables = [], array $context = []): iterable
Expand Down
Loading

0 comments on commit bde59ba

Please sign in to comment.