Skip to content
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

Not able to send ID in POST method #343

Closed
akadlec opened this issue Jun 22, 2017 · 24 comments
Closed

Not able to send ID in POST method #343

akadlec opened this issue Jun 22, 2017 · 24 comments
Labels

Comments

@akadlec
Copy link

@akadlec akadlec commented Jun 22, 2017

I would like to send generated ID from UI to API but right now it is not possible. API is trying to do update, response is "Method is not allowed" So right now i'm sending it as uuid and then in the event doing extraction from request setting to entity.

It will be great to allow send ID param to POST and to have "optional" parameter for writing. What do you think?

@Simperfit

This comment has been minimized.

Copy link
Member

@Simperfit Simperfit commented Jun 22, 2017

What do you mean ?

If you do POST /something with

{
"id":"value"
}

And the setters exists (and the good groups if you use any), that should work.

@akadlec

This comment has been minimized.

Copy link
Author

@akadlec akadlec commented Jun 22, 2017

Yes this, but this does not work, the result is: Update is not allowed for this operation.

@Simperfit

This comment has been minimized.

Copy link
Member

@Simperfit Simperfit commented Jun 25, 2017

Could you please paste the entity you are using ?

@Simperfit Simperfit added the question label Jun 25, 2017
@akadlec

This comment has been minimized.

Copy link
Author

@akadlec akadlec commented Jul 7, 2017

/**
 * @ApiResource(
 *     attributes={
 *         "normalization_context"={
 *             "groups"={
 *                 "dashboard-read"
 *             }
 *         },
 *         "denormalization_context"={
 *             "groups"={
 *                 "dashboard-write"
 *             }
 *         }
 *     },
 *     itemOperations={
 *         "get"={
 *             "method"="GET", "path"="/v1/dashboards/{id}"
 *         },
 *         "put"={
 *             "method"="PUT", "path"="/v1/dashboards/{id}"
 *         },
 *         "delete"={
 *             "method"="DELETE", "path"="/v1/dashboards/{id}"
 *         }
 *     },
 *     collectionOperations ={
 *         "get"={
 *             "method"="GET", "path"="/v1/dashboards"
 *         },
 *         "post"={
 *             "method"="POST", "path"="/v1/dashboards"
 *         }
 *     }
 * )
 * @ORM\Entity
 */
class Dashboard implements IDashboard
{
	/**
	 * @var Uuid\Uuid
	 *
	 * @SerializerGroups({"dashboard-read"})
	 * @ORM\Id
	 * @ORM\Column(type="uuid_binary", name="dashboard_id")
	 * @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator")
	 */
	private $id;

	/**
	 * @param string $id
	 *
	 * @return void
	 */
	public function setId(string $id)
	{
		$this->id = Uuid\Uuid::fromString($id);
	}

	/**
	 * @return Uuid\UuidInterface
	 */
	public function getId() : Uuid\UuidInterface
	{
		return $this->id;
	}
}

Problem is, when in POST is ID api-platform is thinking that i want to do an UPDATE action.

@GonZOO82

This comment has been minimized.

Copy link

@GonZOO82 GonZOO82 commented Sep 12, 2017

Any idea?

@akadlec

This comment has been minimized.

Copy link
Author

@akadlec akadlec commented Sep 12, 2017

From me, no, i am leaving this bundle and going back to other solution

@dunglas

This comment has been minimized.

Copy link
Member

@dunglas dunglas commented Sep 12, 2017

It's weird to have a setter for an ID that is generated externally. I'm not sure to get what you try to achieve.

@akadlec

This comment has been minimized.

Copy link
Author

@akadlec akadlec commented Sep 12, 2017

@dunglas it is normal to send ID from app. I case you have app with optimistic ui and all ID's are in UUID format, you could generate ID in you app and send it to api endpoint and you don't have to wait for positive response.

@dunglas

This comment has been minimized.

Copy link
Member

@dunglas dunglas commented Sep 12, 2017

@akadlec I agree on that, but in this case you shouldn't have this line @ORM\CustomIdGenerator(class="Ramsey\Uuid\Doctrine\UuidGenerator").

@akadlec

This comment has been minimized.

Copy link
Author

@akadlec akadlec commented Sep 12, 2017

Right now i'm not 100% surre but i thing i did a test where i remove this row. Problem is not in doctrine mapping but in apiplatform. There is a method which check request and when an ID is present in request and request is in POST method, platform refuse it

@yoshz

This comment has been minimized.

Copy link

@yoshz yoshz commented Feb 8, 2018

This issue still exists for me and I only get the error message "Update is not allowed for this operation" if actually post an id that has the property name "id". For other entities that have an id on the property "code" for example I don't get this error message. So this is pretty weird.

Should this check not rely on that a IdGenerator is actually defined in Doctrine for that entity?

@coudenysj

This comment has been minimized.

Copy link

@coudenysj coudenysj commented Mar 30, 2018

Any news on this? Or ideas how to prepare a merge request to add support for this?

@arnedesmedt

This comment has been minimized.

Copy link

@arnedesmedt arnedesmedt commented Mar 30, 2018

+1

@Toflar

This comment has been minimized.

Copy link

@Toflar Toflar commented May 1, 2018

This issue still exists. I'm trying to fix it but I'm pretty lost and it's hard to fix it without guidance as I'm not sure why things are the way they are at several places in the code.
First of all, this issue should be moved to core (not sure about the policy here though).
Then the issue occurs here: https://github.com/api-platform/core/blob/master/src/Serializer/ItemNormalizer.php#L35

OBJECT_TO_POPULATE is not set at this place yet, it happens only later. And api_allow_update is false because it's a POST request and this variable is only set to true if PATCH or PUT requests are sent (see https://github.com/api-platform/core/blob/master/src/Serializer/SerializerContextBuilder.php#L72).
So I'm not sure how to proceed here but it's perfectly valid to send a POST request with an id if you have no autogenerated strategy.

@Toflar

This comment has been minimized.

Copy link

@Toflar Toflar commented May 1, 2018

Original issue was #132 so cc'ing @meyerbaptiste here too.

@Toflar

This comment has been minimized.

Copy link

@Toflar Toflar commented May 1, 2018

That whole check does not make much sense to me. I mean, what if my identifier was named foobar? It would just happily ignore it 😄

@br750

This comment has been minimized.

Copy link

@br750 br750 commented Aug 28, 2018

hello
This issue still exists. if i send a json with id=0 from my apps with POST request :
message "Update is not allowed for this operation"
expected that the object was created with auto id...
is there a way to fix it ?
thanks

@soyuka soyuka added the wontfix label Sep 12, 2018
@soyuka

This comment has been minimized.

Copy link
Member

@soyuka soyuka commented Sep 12, 2018

See reasoning in api-platform/core#2022 (comment). You can always decorate the ItemNormalizer to implement your own logic.

@nfacciolo

This comment has been minimized.

Copy link

@nfacciolo nfacciolo commented Aug 28, 2019

Seriously there is no way to [POST] an object with a specified id named "id" !?

@maks-rafalko

This comment has been minimized.

Copy link

@maks-rafalko maks-rafalko commented Aug 28, 2019

no, we had to implement it ourselves (see my comment)

@nfacciolo

This comment has been minimized.

Copy link

@nfacciolo nfacciolo commented Aug 28, 2019

I tried your solution but it leads to this error:
Argument 10 passed to ApiPlatform\Core\Serializer\AbstractItemNormalizer::__construct() must be of the type array, null given, called in /usr/src/api/var/cache/dev/ContainerVNe5Qdr/srcApp_KernelDevDebugContainer.php on line 823

btw thanks for the code

@maks-rafalko

This comment has been minimized.

Copy link

@maks-rafalko maks-rafalko commented Aug 28, 2019

This is the full code of our class that allows positing custom id fields and works with 2.4.3 api-platform:

<?php

declare(strict_types=1);

namespace App\Serializer\Normalizer;

use ApiPlatform\Core\Api\IriConverterInterface;
use ApiPlatform\Core\Api\ResourceClassResolverInterface;
use ApiPlatform\Core\DataProvider\ItemDataProviderInterface;
use ApiPlatform\Core\Exception\InvalidArgumentException;
use ApiPlatform\Core\Metadata\Property\Factory\PropertyMetadataFactoryInterface;
use ApiPlatform\Core\Metadata\Property\Factory\PropertyNameCollectionFactoryInterface;
use ApiPlatform\Core\Metadata\Resource\Factory\ResourceMetadataFactoryInterface;
use ApiPlatform\Core\Serializer\AbstractItemNormalizer;
use Psr\Log\LoggerInterface;
use Psr\Log\NullLogger;
use Symfony\Component\PropertyAccess\PropertyAccessorInterface;
use Symfony\Component\Serializer\Mapping\Factory\ClassMetadataFactoryInterface;
use Symfony\Component\Serializer\NameConverter\NameConverterInterface;

/**
 * This class overrides api-platform's built in ItemNormalizer in order to make it possible to POST resources
 * with custom provided ID
 *
 * Related not merged PR and discussion: https://github.com/api-platform/core/pull/2022
 */
class ItemNormalizer extends AbstractItemNormalizer
{
    private const IDENTIFIER = 'id';

    /**
     * @var LoggerInterface
     */
    private $logger;

    public function __construct(PropertyNameCollectionFactoryInterface $propertyNameCollectionFactory, PropertyMetadataFactoryInterface $propertyMetadataFactory, IriConverterInterface $iriConverter, ResourceClassResolverInterface $resourceClassResolver, PropertyAccessorInterface $propertyAccessor = null, NameConverterInterface $nameConverter = null, ClassMetadataFactoryInterface $classMetadataFactory = null, ItemDataProviderInterface $itemDataProvider = null, bool $allowPlainIdentifiers = false, LoggerInterface $logger = null, iterable $dataTransformers = [], ResourceMetadataFactoryInterface $resourceMetadataFactory = null)
    {
        parent::__construct($propertyNameCollectionFactory, $propertyMetadataFactory, $iriConverter, $resourceClassResolver, $propertyAccessor, $nameConverter, $classMetadataFactory, $itemDataProvider, $allowPlainIdentifiers, [], $dataTransformers, $resourceMetadataFactory);

        $this->logger = $logger ?: new NullLogger();
    }

    /**
     * @param mixed $data
     * @param string $class
     * @param string $format
     * @param array $context
     *
     * @return object
     */
    public function denormalize($data, $class, $format = null, array $context = [])
    {
        $context['api_denormalize'] = true;

        if (!isset($context['resource_class'])) {
            $context['resource_class'] = $class;
        }

        $this->setObjectToPopulate($data, $context);

        return parent::denormalize($data, $class, $format, $context);
    }

    /**
     * @param string|object $classOrObject
     * @param array $context
     * @param bool $attributesAsString
     *
     * @return array|bool|string[]|\Symfony\Component\Serializer\Mapping\AttributeMetadataInterface[]
     */
    protected function getAllowedAttributes($classOrObject, array $context, $attributesAsString = false)
    {
        $allowedAttributes = parent::getAllowedAttributes(
            $classOrObject,
            $context,
            $attributesAsString
        );

        if (\array_key_exists('allowed_extra_attributes', $context)) {
            $allowedAttributes = array_merge($allowedAttributes, $context['allowed_extra_attributes']);
        }

        return $allowedAttributes;
    }

    /**
     * @param mixed $data
     * @param array $context
     */
    protected function setObjectToPopulate($data, array &$context): void
    {
        // in PUT request OBJECT_TO_POPULATE is already set by this moment
        if (!\is_array($data) || isset($context[self::OBJECT_TO_POPULATE])) {
            return;
        }

        [$identifierName, $identifierMetadata] = $this->getResourceIdentifierData($context);

        $isUpdateAllowed = (bool) ($context['api_allow_update'] ?? false);
        $hasIdentifierInRequest = \array_key_exists(self::IDENTIFIER, $data);
        $hasWritableIdentifierInRequest = $hasIdentifierInRequest && $identifierMetadata->isWritable();
        // when it is POST, update is not allowed for top level resource, but is allowed for nested resources
        $isTopLevelResourceInPostRequest = !$isUpdateAllowed
            && $context['operation_type'] === 'collection'
            && $context['collection_operation_name'] === 'post';

        // if Resource does not have an ID OR if it is writable custom id - we should not populate Entity from DB
        if (!$hasIdentifierInRequest || ($hasWritableIdentifierInRequest && $isTopLevelResourceInPostRequest)) {
            return;
        }

        if (!$isUpdateAllowed) {
            throw new InvalidArgumentException('Update is not allowed for this operation.');
        }

        try {
            $context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getItemFromIri(
                (string) $data[self::IDENTIFIER],
                $context + ['fetch_data' => true]
            );
        } catch (InvalidArgumentException $e) {
            $context[self::OBJECT_TO_POPULATE] = $this->iriConverter->getItemFromIri(
                sprintf(
                    '%s/%s',
                    $this->iriConverter->getIriFromResourceClass($context['resource_class']),
                    $data[$identifierName]
                ),
                $context + ['fetch_data' => true]
            );
        }
    }

    private function getResourceIdentifierData(array $context): array
    {
        $identifierPropertyName = null;
        $identifierPropertyMetadata = null;
        $className = $context['resource_class'];

        $properties = $this->propertyNameCollectionFactory->create($className, $context);

        foreach ($properties as $propertyName) {
            $property = $this->propertyMetadataFactory->create($className, $propertyName);

            if ($property->isIdentifier()) {
                $identifierPropertyName = $propertyName;
                $identifierPropertyMetadata = $property;
                break;
            }
        }

        if ($identifierPropertyMetadata === null) {
            throw new \LogicException(
                sprintf(
                    'Resource "%s" must have an identifier. Properties: %s.',
                    $className,
                    implode(',', iterator_to_array($properties->getIterator()))
                )
            );
        }

        return [$identifierPropertyName, $identifierPropertyMetadata];
    }
}
@nfacciolo

This comment has been minimized.

Copy link

@nfacciolo nfacciolo commented Aug 29, 2019

With the full code on api-platform version 2.4.6, it works for simple entities. But if there are nested entities, it does not work.

Thanks for the code and the time.

@quentinus95

This comment has been minimized.

Copy link

@quentinus95 quentinus95 commented Dec 18, 2019

I have found out that posting with the content-type header set to application/ld+json fixes the issue on my side.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.