Skip to content

How to add new entry in a ManyToMany? #1628

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

Closed
vincentchalamon opened this issue Jan 5, 2018 · 30 comments
Closed

How to add new entry in a ManyToMany? #1628

vincentchalamon opened this issue Jan 5, 2018 · 30 comments

Comments

@vincentchalamon
Copy link
Contributor

vincentchalamon commented Jan 5, 2018

I have a User entity linked with a bidirectional ManyToMany association to an Event entity: an event have users registered, and it's possible to get a user events through an ApiSubresource.

When a user wants to register to an event, I should update the event to add the user:

PUT /events/{id}
{
    "users": [
        "/users/1",
        "/users/2",
        …
        "/users/8888" <- the user who wants to register
    ]
}

This could also be done by adding the event to the user:

PUT /users/{id}
{
    "events": [
        "/events/1",
        "/events/2",
        …
        "/events/8888" <- the event the user wants to register
    ]
}

In the first example, I'll expose the users id, which could be a security issue. In the second example, I'll have to find all the user events id which is not optimized.
To optimize this process, I've created a custom entry-point PUT /events/{id}/register to register the current user to the event.

It would be awesome to allow to send a write request to my ApiSubresource to add an event without reminding all previous events, for instance:

PUT /users/12345/events
{
    "@id": "/events/67890"
}

And to remove an event from this ApiSubresource: DELETE /users/12345/events/67890

What do you think @api-platform/core-team ?

@soyuka
Copy link
Member

soyuka commented Jan 5, 2018

Writability on subresource is a wanted/needed/interesting feature indeed, it should not be too hard to implement now. Feel free to work on it if you have the time!

@hassanjuniedi
Copy link

@vincentchalamon that's great, am in the middle of similar problem and am trying to create custom api entry but with no success , can you please post an example of how you created custom action for PUT /events/{id}/register
note: the entry am trying to accomplish is like this /products/{id}/add-comment , i have to pass product id and comment object or comment id but inside the _invoke function how to get this data .

@vincentchalamon
Copy link
Contributor Author

Hi @hassanjuniedi, my case changed a little bit as I need a more complex object to register a user to an event. I've created a Registration entity, and am able to send a POST request to /registrations to register a user to an event.

In your case, I think you confuse the front routing and the api routing: you don't need to restrict the comment creation url under /products. IMO, the easiest way would be to send a POST request to /comments.

If you really need to send a POST request to /products/{id}/comments, you'll need to create a custom route:

products_add_comment:
    path: /products/{id}/comments
    controller: api_platform.action.placeholder
    methods: [POST]

Then, add an itemOperation on your Product entity:

/**
 * @ApiResource(attributes={...}, itemOperations={
 *     ...
 *     "add_comment"={"route_name"="products_add_comment"}
 * })
 */
class Product

Finally, you can manage everything in an EventSubcriber listening to kernel.request & kernel.view events:

services:
    App\EventSubscriber\ProductsAddCommentEventSubscriber:
        autoconfigure: true
        autowire: true
public function ProductsAddCommentEventSubscriber implements EventSubscriber
{
    private $serializer;

    public function __construct(SerializerInterface $serializer)
    {
        $this->serializer = $serializer;
    }

    // ...Register events (be careful with priorities!)

    public function deserializeCommentOnKernelRequest($event)
    {
        $request = $event->getRequest();
        if ('products_add_comment' !== $request->attributes->get('_route')) {
            return;
        }
        // Deserialize Comment from $request->getContent();
        // Validate Comment, then throw an exception (status code 400) if invalid
        $request->attributes->set('data', $comment);
    }

    public function saveCommentOnKernelView($event)
    {
        $request = $event->getRequest();
        if ('products_add_comment' !== $request->attributes->get('_route')) {
            return;
        }
        // Persist/flush Comment from $event->getControllerResult();
    }
}

In order to use API Platform EventSubscribers to serialize your Comment object & prepare Response, you may need to add some parameters in your route, like _api_resource_class & _api_item_operation_name.

I let you do the rest of the code 😉Let me know if you have any question about this example.

@hassanjuniedi
Copy link

hassanjuniedi commented Feb 13, 2018

@vincentchalamon thanks for your quick reply and helpful explanation,but i think i need something little different . /comments is working fine but in may case i use many-to-many relationship between product and comment to use join table to link product with comments
Note: I used this approach because i use same comment entity with multiple other entities.

//Product Entity
/**
     * @ORM\ManyToMany(targetEntity="AppBundle\Entity\Comment")
     * @ORM\JoinTable(
     *     name="product_comments",
     *     joinColumns={@ORM\JoinColumn(name="product_id", referencedColumnName="id")},
     *     inverseJoinColumns={@ORM\JoinColumn(name="comment_id", referencedColumnName="id", unique=true)}
     *     )
     */
    private $comments;
/**
 * Comment
 *
 * @ApiResource
 * @ORM\Table(name="comment")
 * @ORM\Entity(repositoryClass="AppBundle\Repository\CommentRepository")
 */
class Comment
{
    /**
     * @var int
     *
     * @ORM\Column(name="id", type="integer")
     * @ORM\Id
     * @ORM\GeneratedValue(strategy="AUTO")
     */
    private $id;

    /**
     * @var string
     *
     * @ORM\Column(name="author_name", type="string", length=255)
     */
    private $authorName;

    /**
     * @var string|null
     *
     * @ORM\Column(name="author_avatar", type="string", length=255)
     */
    private $authorAvatar;

    /**
     * @var string
     *
     * @ORM\Column(name="author_username", type="string", length=255)
     */
    private $authorUsername;

    /**
     * @var string
     *
     * @ORM\Column(name="text", type="text")
     */
    private $text;

    /**
     * @var \DateTime
     *
     * @ORM\Column(name="create_date", type="datetime")
     */
    private $createDate;

    /**
     * @var string
     *
     * @ORM\Column(name="comment_on", type="string", length=255)
     */
    private $commentOn;

    /**
     * @var string
     *
     * @ORM\Column(name="post_slug", type="string", length=255)
     */
    private $postSlug;


    /**
     * @ORM\ManyToOne(targetEntity="AppBundle\Entity\Comment", inversedBy="replies")
     * @ORM\JoinColumn(name="parent_id", referencedColumnName="id" , nullable=true, onDelete="cascade")
     */
    private $parent;

    /**
     * @ORM\OneToMany(targetEntity="AppBundle\Entity\Comment", mappedBy="parent")
     */
    private $replies;

// product_add_comment Action
class productAddComment
{
    /**
     * @Route(
     *     name="product_add_comment",
     *     path="/products/{id}/add-comment",
     *     defaults={
     *          "_api_resource_class"=Comment::Class,
     *          "_api_item_operation_name"="add_comment",
     *          "_api_receive"="false"
     *     }
     * )
     * @Method("PUT")
     * @param Comment $data
     * @return mixed
     */
    public function _invoke(Comment $data) {
        dump($data);
        // here i need to get comment and product objects and link them together .
        return $data;
    }
}

i just need a way to pass comment @id and product @id so i can add comment to product and persist it to database (in join table). i hope that i delivered my issue clearly, thanks in advance .

@vincentchalamon
Copy link
Contributor Author

vincentchalamon commented Feb 13, 2018

OK so maybe you should use a meta-model like:

class Comment
{
    private $objectId;
    private $objectClass;
    private $author;
    private $createdAt;
    private $updatedAt;
    private $comment;

    public function setObject($object)
    {
        $this->setObjectId($object->getId());
        $this->setObjectClass(get_class($object));
    }
}

Then, you could send a POST request to /products/{id}/comments:

{
    "comment": "Lorem ipsum dolor sit amet"
}

In this example, author can be filled by an EventSubscriber listening to kernel.request event with a high priority (check EventPriorities), and get the current user from the tokenStorage. Product can be retrieve from the request attributes.

public function ProductsAddCommentEventSubscriber implements EventSubscriber
{
    private $serializer;

    public function __construct(SerializerInterface $serializer)
    {
        $this->serializer = $serializer;
    }

    // ...Register events (be careful with priorities!)

    public function deserializeCommentOnKernelRequest($event)
    {
        $request = $event->getRequest();
        if ('products_add_comment' !== $request->attributes->get('_route')) {
            return;
        }
        // Deserialize Comment from $request->getContent();
        // If you set correctly the priorities, you should have a `data` request attribute which correspond to the Product (or related object)
        $comment->setObject($request->attributes->get('data'));
        $request->attributes->set('comment', $comment);
    }
}
class ProductAddComment
{
    /**
     * @Route(
     *     name="product_add_comment",
     *     path="/products/{id}/add-comment",
     *     defaults={
     *          "_api_resource_class"=Comment::Class,
     *          "_api_item_operation_name"="add_comment",
     *          "_api_receive"="false"
     *     }
     * )
     * @Method("PUT")
     */
    public function __invoke(Product $data, Comment $comment): Comment
    {
        return $comment;
    }
}

Note: I didn't have the time & opportunity to test this code, so I hope it'll work. Otherwise, it illustrates the approach :-)

@hassanjuniedi
Copy link

Thank you a lot , i will try this approach and let you know .

@hassanjuniedi
Copy link

I tried to get the request object using event listener and i succeed with that approach , but then i figure out a better approach as shown in the code below.
now i can get the request data and manipulate it inside action class and that's what i wanted exactly.
@vincentchalamon thanks for your help .

<?php
//AppBundle/Action/PutComments.php

class PutComments
{
    private $request;
    private $serializer;
    private $validator;
    private $em;

    public function __construct(
        RequestStack $requestStack,
        SerializerInterface $serializer,
        ValidatorInterface $validator,
        EntityManagerInterface $em)
    {
        $this->request = $requestStack->getCurrentRequest();
        $this->serializer = $serializer;
        $this->validator = $validator;
        $this->em = $em;
    }

    /**
     * @Route(
     *     name="product_put_comments",
     *     path="/products/{id}/comments.{_format}",
     *     defaults={
     *          "_format"="jsonld",
     *          "_api_resource_class"=Product::Class,
     *          "_api_item_operation_name"="put_comments",
     *     }
     * )
     * @Method("PUT")
     * @param Product $product
     * @return mixed
     */
    public function _invoke(Product $product) {

        $content = $this->request->getContent();
        $format = $this->request->attributes->get('_format');
        if (!empty($content)) {
            $comment = $this->serializer->deserialize($content,Comment::class, $format);
            $errors = $this->validator->validate($comment);
            if (count($errors) > 0 ) {
                $output = array();
                foreach ($errors as $error) {
                    $output[$error->getPropertyPath()][] = $error->getMessage();
                }
                return new JsonResponse($output, 400);
            }else {
                $product->addComment($comment);
                $this->em->flush();
            }
        }
        return $product;
    }
}

@vincentchalamon
Copy link
Contributor Author

👍 but your code could be optimized ;-)

  • Be careful that __invoke magic method starts with 2 underscores.
  • You shouldn't call ->getCurrentRequest from the constructor, cause the current request might not be initialized yet (for example using cli).
  • You can inject the Request object in the __invoke method as every controller action.
  • For an optimized code aux petits oignons, you should respect the Symfony Coding Standards, the PSRs & the Insight rules.
  • Do not inject the EntityManager as a dependency (cf. Insight rules), please prefer the ManagerRegistry (@doctrine).
  • If you tag your service with controller.service_arguments, you can inject all your dependencies in the __invoke method:
//AppBundle/Action/PutComments.php

final class PutComments
{
    /**
     * @Route(
     *     name="product_put_comments",
     *     path="/products/{id}/comments.{_format}",
     *     defaults={
     *          "_format"="jsonld",
     *          "_api_resource_class"=Product::Class,
     *          "_api_item_operation_name"="put_comments",
     *     }
     * )
     * @Method("PUT")
     */
    public function __invoke(Request $request, $format, SerializerInterface $serializer, ValidatorInterface $validator, ManagerRegistry $registry, Product $product)
    {
        $content = $request->getContent();
        if (empty($content)) {
            // Here you should throw an exception if the content is empty
        }

        $comment = $serializer->deserialize($content, Comment::class, $format);
        $errors = $validator->validate($comment);
        if (0 < count($errors)) {
            $output = array();
            foreach ($errors as $error) {
                $output[$error->getPropertyPath()][] = $error->getMessage();
            }

            return new JsonResponse($output, 400);
        }

        $product->addComment($comment);
        $em = $registry->getManagerForClass(Comment::class);
        $em->persist($comment);
        $em->flush();

        return $product;
    }
}

@vincentchalamon vincentchalamon self-assigned this Mar 20, 2018
@teohhanhui
Copy link
Contributor

I love the idea, but let's not further misuse PUT. For new features at least, we should make sure we respect the semantics of the HTTP verbs.

@vincentchalamon
Copy link
Contributor Author

vincentchalamon commented Jan 2, 2019

I'm currently working on a PR to implement POST & DELETE requests on an ApiSubresource. But, as @dunglas said, it's out of the REST concept. What do you think @soyuka @teohhanhui @api-platform/core-team: should I continue working on it, or would we consider to not implement such borderline feature in API Platform?

@vincentchalamon
Copy link
Contributor Author

Ping @api-platform/core-team @dunglas @soyuka @alanpoulain

@alanpoulain
Copy link
Member

IMHO: if there is a real need, then it should be done, even if it's not REST. But we should document this feature as to be avoided if possible.

@vincentchalamon
Copy link
Contributor Author

I already experienced this need 2 or 3 times in differents projects, and I also talked about it with other API Platform developers who had the same need. Finally, everybody create a custom controller... But I think it could easily be managed by something generic in API Platform.

@soyuka
Copy link
Member

soyuka commented Apr 3, 2019

If it's not too much work to add this in a "generic way" I'm 👍. Also, this feature is a high demanded one.

@vincentchalamon
Copy link
Contributor Author

@soyuka What do you think about the REST concept in this feature?

@soyuka
Copy link
Member

soyuka commented Apr 4, 2019

It's borderline when it comes to REST but I think that we can still provide this feature.

@teohhanhui
Copy link
Contributor

The semantics seem fine to me if it's just POST and DELETE. @soyuka What's your concern on the REST-fulness?

@teohhanhui
Copy link
Contributor

If I understand correctly, the IRI must stay the same (e.g. /children/1), not nested (e.g. /parents/1/children/1).

@vincentchalamon
Copy link
Contributor Author

vincentchalamon commented Apr 5, 2019

@teohhanhui The requests will be:

# Create a new RelatedDummy and add it to a Dummy
POST /dummies/{id}/related_dummies
{
    "name": "Foo"
}
# Add an existing RelatedDummy to a Dummy
POST /dummies/{id}/related_dummies
{
    "@id": "/related_dummies/{id}"
}
# Remove a RelatedDummy from a Dummy
DELETE /dummies/{id}/related_dummies/{id}

I'm not sure about the syntax of the second scenario: "Add an existing RelatedDummy to a Dummy"

@soyuka
Copy link
Member

soyuka commented Apr 5, 2019

My only REST related concern is that we may loose the context of the root resource. Anyway, that's related to subresources in general and as we already have read support adding write support seems to be the correct next step.

@vincentchalamon
Copy link
Contributor Author

vincentchalamon commented Apr 5, 2019

we may loose the context of the root resource

What do you mean? Currently, in my developments, I'm adding the rootData in the request attributes, allowing to get it at any time in a write operation subresource.

@soyuka
Copy link
Member

soyuka commented Apr 5, 2019

when requesting GET /dummies/1/relatedDummies you get relatedDummies but the response doesn't give you information about the original dummy.

@torreytsui
Copy link

@vincentchalamon @alanpoulain Thanks again for bring me in.

To pick up and continue from the PR I raised in #2598 and yours in #2428.

Many-to-many (e.g., User-to-Event) is an interesting use case.

Is there any reason we shouldn't / couldn't do PUT? (I have read through the thread but couldn't find the rationale behind it)

So, here is my thought on the use cases discussed here.

  • For creation, POST
    • If the event already exists, say eventId=1, POST /users/{id}/events/1 with no body. (It is slightly different from your suggestion above where a @id body is used instead. I prefer an id on the route because it is slightly more consistent use of the body, we won't end up having to reserve a special handing on the @id attribute)
    • If the event is created and associated in the same request, POST /users/{id}/events with body of event data.
  • For unlinking, DELETE
    • If delete only the association, DELETE /users/{id}/events/1.
    • If delete an event along with it, it is not supported with subresource. Go for event resource instead DELETE /events/1, as I haven't found an easy way for the subresource delete request to REST-fully indicate this intention.
  • For update, PUT
    • If update the association, it is not supported. Go for subresource delete / post (see above) instead, as update = delete + post, and I haven't found a REST-ful way for it.
    • If update the event data, say eventId=1, PUT /users/{id}/events/1
      I believe we need PUT because I have encountered some use cases where it makes sense to update a subresource through its parent resource, e.g., in a /me one-to-one relationship, /users/me/event where an event belongs to a user, thus a stronger relation, and in addition, without needing to be aware of the event id.
  • The many-to-many (or one-to-may) is a subresource collection operation example. For many-to-one / one-to-one relationship, it is a subresource item operation and will behave slightly differently (a quick reference can be found in the "Subresource operations" section of [RFC] [WIP] Offer post, delete and put subresource #2598 PR's description)

As far as I know, subresource is just another resource in REST (which operation(s) is done on relation with other resource(s) instead of on its own) even though there is no written standard of it. So, I do believe subresource is REST-ful by nature. I could be wrong though.

  • Nested-by-class has real use cases, for example, if there is a Foo class with a child of Foo class, /foo/{id}/child

  • Nested-by-resource doesn't normally though, as it won't end unless intervention, for example, /foo/{id}/child/child/.... Even though this kind of use cases may not be common, I will prefer letting the developers to decide other than limiting them. A maxDepth can be introduced to manually raise the limit. For example, /foo/{id}/child/child is maxDepth=2. By default, maxDepth=null and it will end with the /foo/{id}/child (cause this is where the nested-by-class end)

  • I don't think we need the the root resource to write because it is not generalised and instead, we need the immediate parent resource

    • Root resource works when there is only two level, e.g., foos/{id}/bars/{bar}
    • To generalise, it is actually the immediate parent resource (i.e., bar in this example) needed. And it work for all levels, for example, level 3: foos/{id}/bars/{bar}/bazs

If need helps to implement, I'm more than happy to look at them. (I can't tell how much time I can spend on on a regular basis, but I can promise I'm persisted) A project I'm working on has been working around some subresource use cases (and edge cases) and I don't like building up tech debts day by day.

So, what do you guys think? I would love to know and discuss on them.

@vincentchalamon
Copy link
Contributor Author

vincentchalamon commented Apr 7, 2019

Hi @torreytsui, thanks for your feedback! I'll try to answer your questions as I can.

Is there any reason we shouldn't / couldn't do PUT?

Since subresources are available in API Platform, people use them in a more complex and deep way than expected. Adding POST & DELETE operations on subresource will extend the subresources capabilities for specific reasons (cf. previous comments), but may also increase the complexity of the subresources. We should be careful about this complexity when adding new features on it, and lock them to their only topic. That's also why I would lock the POST & DELETE operation to collection subresources only, to prevent any bad use.

Adding PUT request on a subresource could be an interesting feature in some specific cases, but may be mis-used most of the time. Users must call the object PUT operation instead of updating it through another relation.

If the event already exists, say eventId=1, POST /users/{id}/events/1 with no body.

The difference between an item & a collection operation is the presence of an id attribute in the uri in item operations, that's why POST is a collection operation. Adding a subresource id attribute in this POST operation will break this rule. But it's already possible in API Platform to reference an object through its identifier in the body: embedded references. For instance:

PUT /foo/{id}
{
    "bar": {
        "@id": "/bars/{id}",
        "lorem": "Ipsum"
    }
}

(...) e.g., in a /me one-to-one relationship, /users/me/event

Using /me is not REST-ful, cause it doesn't represent a single resource cause it depends on the authorization header, not only on the uri. A better way is to call /users/12345 for managing your user, and /users/67890 for managing mine. It's also easier for caching ;-)

The many-to-many (or one-to-may) is a subresource collection operation example.

For ManyToMany operations (/foos/{id}/bars), I totally agree, cause the main topic of adding POST & DELETE operations on subresources is to handle data in this ManyToMany association. For OneToMany, the relation can be handled through an ApiResource operation: PUT /bars/{id} => { "foo": "/foos/{id}" }.

For many-to-one / one-to-one relationship, it is a subresource item operation and will behave slightly differently.

Adding PUT|DELETE /foo/{id}/bar on an item subresource would be a bad use. Instead, we should call PUT|DELETE /bars/{id} or embedded references (cf. example above).

A maxDepth can be introduced to manually raise the limit.

There is a maxDepth in the ApiSubresource for the same reason ;-)


For the rest, I totally agree with your points and I think we're going on the same way.

If need helps to implement, I'm more than happy to look at them.

And I would be very happy to work with you on this feature :-) I took a look at your PR, you implemented an interesting approach of this feature. It would be awesome if we could merge both PRs and handle together the differents tasks for it (feel free to complete this list as a common reference for our work):

  • Add Behat scenarios
  • Add POST operation on ManyToMany subresources
  • Add DELETE operation on ManyToMany subresources
  • Add ApiSubresource option to enable/disable POST & DELETE operations
  • Refacto part of SubresourceOperationFactory (and maybe ApiLoader)
  • Update documentations (Swagger, API, technical...)
  • Add/update unit tests

I excluded the refacto of the ApiSubresource cause it must be done in another issue, where I'll include you of course :-)

I'd be very happy to talk with you about it. Don't hesitate to ping me directly on Slack too.

potato

@karser
Copy link
Contributor

karser commented Jul 17, 2019

Allowing POST/PUT/DELETE on ManyToMany subresource is going to be an awesome feature! I just encountered this need in my project. So far implementing it using an array of IRIs.

@teohhanhui
Copy link
Contributor

Is there any reason we shouldn't / couldn't do PUT? (I have read through the thread but couldn't find the rationale behind it)

The correct semantics of PUT means you must provide the complete representation of the resource. There is no such thing as a "partial PUT", but there's PATCH for that.

We've just had Json Merge Patch support in 2.5 / master (see #2895), so we will be able to use that.

But the subresources feature needs a major overhaul. See #2706

@mRoca
Copy link
Contributor

mRoca commented Oct 30, 2019

FYI, the PATCH method implementation can't be used to deal with subresources add/remove.

There are 2 supported PATCH formats, for now: application/merge-patch+json and application/vnd.api+json.

  • The application/merge-patch+json format doesn't allow to add or remove items from/to an array attribute without sending all items (whereas the application/json-patch+json format allows it)
  • The application/vnd.api+json format allows to add or remove items, but only by sending a PATCH request to the relationship sub-route. As the PATCH route is only defined for the root entity, we cannot use this feature neither.

@ziiko10
Copy link

ziiko10 commented Nov 25, 2019

Hello , is this feature implemented or not yet ?
I want to allow POST/DELETE on Many to Many relation.

Thanks

@gonzalovilaseca
Copy link

Any updates on this?? :-) If you guys need help I might have time to do a PR, but would need some guidance.

@vincentchalamon
Copy link
Contributor Author

Hi @gonzalovilaseca,

Thanks for proposing your help and time :-)

This issue has be moved to this RFC: #2706. We're currently trying to organize a hackaton with the core-team to fix this issue as soon as possible. We'll let you know if we need some help, or when the feature will be released.

@vincentchalamon vincentchalamon linked a pull request Aug 27, 2020 that will close this issue
11 tasks
@soyuka soyuka closed this as completed Feb 19, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

10 participants