zfcampus / zf-apigility-doctrine

Doctrine-enabled services for Apigility
BSD 3-Clause "New" or "Revised" License
107 stars 50 forks source link

Working with related/embedded entities #215

Closed santiph closed 8 years ago

santiph commented 9 years ago

Hi,

I have three entities with their respective services: Venue, Product and Location. Venue has a ManyToOne relation with Location

    /**
     * @ORM\ManyToOne(targetEntity="Db\Entity\Location", cascade={"persist"})
     * @ORM\JoinColumn(name="location_id", referencedColumnName="id")
     *
     * @var Db\Entity\Location $location
     */
    protected $location;
    public function getLocation() {...}
    public function setLocation($location) {...}

Venue has ManyToMany relation with Product

    /**
     * @ORM\ManyToMany(targetEntity="Db\Entity\Product", inversedBy="venues", cascade={"persist"})
     * @ORM\JoinTable(name="products_venues")
     **/
    protected $products;
    public function getProducts() {...}
    public function setProducts($products) {...}
    public function addProduct(Product $product)
    {
        $product->addVenue($this);
        $this->products[] = $product;

        return $this;
    }

And Product has a ManyToMany relation with Venue

    /**
     * @ORM\ManyToMany(targetEntity="Db\Entity\Venue", mappedBy="products", cascade={"persist"})
     **/
    protected $venues;
    public function getVenues() {...}
    public function setVenues($venues) {...}
    public function addVenues($venue)
    {
        $this->venues[] = $venue;

        return $this;
    }

Now, I have two issues here: 1) I can create a new venue and the Location information like this>

POST /venues
{
  .
  .
  .
  "location":{
      "address":"123 Fake Street",
      "city":"cityTest",
      "country":"CountryTest",
      "postalCode":"12345"
  }
}

And a new Venue is created with a new Location registry. When I get the Venue Id just created, I see the location object with its id among the _embedded ones, but what if I need to edit that address and link the Venue 1 with the location 2 instead of location 1 that was originally created?

Secondly, I couldn't reproduce this process with the products creating and linking the product with the venue in the same POST call where this last one is being created. Nether figured how to add an existing product to the Products collection of a particular Venue. So, which way is the more appropriate? ("1 call creates and link them all" vs "multiple creation calls + Patch calls to link them together") and what do I need to make them happen?

santiph commented 9 years ago

Woo!! This was interesting finding. In order to update the location of a specific venue I had to add a new "location" field in the Venues service. With this, I could post a location object as an extra attribute together with the ones that /Venues requires. Or specify the id by adding location: {"id" : 123} (this also works with PATCH, which is what I've been looking for. Great!). But I realize that there is no actual validation that checks that the location with the specified Id actually exists. If none is found, then the new venue will be linked to a "null" location. :frowning:

So, after that, I saw the Apigility Doctrine validators that are included. :smile: ZF\Apigility\Doctrine\Server\Validator\ObjectExists validates that the object I'm sending actually exist in the DB. Yeeyyy!! All I had to do is add it as a validator for "location" field and specify two options based on https://github.com/doctrine/DoctrineModule/blob/master/docs/validator.md entity_class = Db\Entity\Location so the validator knows what Entity I'm trying to validate, and fields = id, which is like a validation criteria.

Now when I try to post a venue, I need to specify the location with "location": {"id":123}. This is also working with PATCH. I get a great error message when the location is not found with the specified Id, which is great. And a 500 one if no Id is provided (One problem at time, I guess...).

Now, I keep trying to find how to add an object to an entity collection. (like, adding products to a specific venue). I tried to add the "products" field to the venues service, but didn't work. I keep getting the venue without any product in the collection. This is how I'm trying to PATCH:

PATCH /venues/1
{
...
"products":[
  {"id": 5}
]
...
}

Any ideas on this?

TomHAnderson commented 9 years ago

You need a hydrator strategy for the many to many:

https://github.com/zfcampus/zf-apigility-doctrine/blob/master/src/Server/Hydrator/Strategy/CollectionExtract.php The comment in hydrate isn't just exactly perfect.

Here's the hydrate from my talk Roll'n API

    public function hydrate($value)
    {
        $performerAlbums = $this->getObject()->getAlbum();

        // Remove all albums then add
        foreach ($performerAlbums as $album) {
            $this->getObject()->removeAlbum($album);
            $album->removePerformer($this->getObject());
        }

        foreach ($value as $album) {
            $album->addPerformer($this->getObject());
            $this->getObject()->addAlbum($album);
        }
    }

This strategy requires you always post all albums as an array, as you're doing. Maybe you can improve it?

santiph commented 9 years ago

Yes, absolutely. I would love to help here!

But this brings up a couple of questions regarding the nature of collection on APIs (and you, Tom, might have more experience than me on this field) In theory, how collections should be represented in a REST API when they are tied to another entity, like this case? (An artist contains a collection of albums)

How should an item be added to that collection? How should an item be removed from that collection? How should an item be fetched within that collection? (I know this topic has been discussed several times across the issues reported, but please help me clarify the "requirements" here)

So I can imagine, for example, something like GET /artists/:artist_id/albums would return the albums collection for that particular artist. POST /artist/:artist_id/albums would add an album to the collection for that particular artist. DELETE /artist/:artist_id/albums would remove the whole artist's album collection (This only removes the relationship). DELETE /artist/:artist_id/albums/:album_id would remove a particular album from the artist's album collection (This only removes the relationship, not the album itself).

How does it look?

TomHAnderson commented 9 years ago

It doesn't look good because I am a proponent of using a single level resource. If you want to add a new album you have a 1:many with artist and you add the artist to the album. You don't do it backwards; don't try to add an album to an artist.

To solve the problem of sub-resource URLs I created zf-doctrine-querybuilder. I am so opposed to sub-resource URLs because it tries to add complicated structure where there is already simple, elegant structure by handling the related resource directly.

GET /album?[zf-doctrine-querybuilder filter string here] POST /album (include artist embedded resource or just the artist id) DELETE /album/1

(editors note: don't call anything plural unless you want to make up pluralings nameings. If albums is plural why not artists? It's a hairy mess, don't play with that.)

santiph commented 9 years ago

How about many:many? Specially, how to delete relationships between two entities? (Not deleting the entries per se)

TomHAnderson commented 9 years ago

Yes! That's the kind of help I hope we can figure out. In the hydrate above it requires all related data be sent with every update to the entity. Is there a method to flag data as to-delete, to-add? When I wrote that code I didn't see any other way except an all-or-nothing approach.

santiph commented 9 years ago

Given than REST is about Resources or Collections, I think PATCH is the proper way to alter a many:mant relationship, since it's intended to partially update a resource.

If we consider than a music Album could be co-owned by several artists (so we turn it into many:many), then both ends could lead us to the related entities. So the PATCH could be applied to either of them, but the system must assure that both links are being removed. Basically, what has been documented in http://doctrine-orm.readthedocs.org/en/latest/reference/working-with-associations.html#removing-associations

<?php
// Remove by Elements
$user->getComments()->removeElement($comment);
$comment->setAuthor(null);

$user->getFavorites()->removeElement($comment);
$comment->getUserFavorites()->removeElement($user);

// Remove by Key
$user->getComments()->remove($ithComment);
$comment->setAuthor(null);

Now, for the "any vs all" dilemma... Could queries be used to specify addition vs removal operation?

//Adding Artists to a specific Album entity
PATCH /Album/:album_id
{
  "artist": [
    {"id":123},
    {"id":456},
    {"id":789}
  ]
}

//Removing a particular Artist from this Album.
PATCH /Album/:album_id?remove
{
  "artist": [
    {"id":123}
  ]
}

//Removing all artists the Album.
PATCH /Album/:album_id?remove
{
  "artist": []
}
santiph commented 9 years ago

What if more than one many:many relationship should be modified...? Lets also add Tracks to the Album. One track could be present more than one album. As a single, as part of the first album of the artist, and also of the "Best hits".

We could use the queries to specify which relationships will be modified, assuming "addition for all" by default, unless ?remove is appended.

//Adding Artists and Tracks to a specific Album entity
PATCH /Album/:album_id
{
  "artist": [
    {"id":123},
    {"id":456},
    {"id":789}
  ],
  "tracks": [
    {"id":111},
    {"id":222},
    {"id":333}
  ]
}

//Removing a particular Artist and two tracks from this Album.
PATCH /Album/:album_id?remove
{
  "artist": [
    {"id":123}
  ]
  "tracks": [
    {"id":111},
    {"id":222}
  ]
}

//Removing a particular Artist and adding one track back to this Album.
PATCH /Album/:album_id?remove=artist
{
  "artist": [
    {"id":123}
  ]
  "tracks": [
    {"id":222}
  ]
}

//Removing all artists and tracks from the Album.
PATCH /Album/:album_id?remove
{
  "artist": [],
  "tracks: []
}

How does it looks?

akomm commented 9 years ago

Regarding patching, check this out: https://tools.ietf.org/html/rfc6902#section-4.3

Regarding some comments here: I think influence a VERBs action using a query string is a bad idea. A query string is there to describe a query on a resource collection.

Using a single level resource or not is a matter of relation. If a resource of a certain type is ALWAYS owned by a single one resource, it should have a nested endpoint inside the owning resources URI. This type of ownership is what you know as composition. If you only have aggregation, than the related resources are not owned and should not be nested. For owned resources: making them a separate, nested resource allows you to use verbs on them directly, not touching the parent resource. Can make things even more easier if you have an owned to-many relation.

akomm commented 9 years ago

Methods/verbs schould only apply to the resource, not embedded resources. This is the root problem. You want exactly the opposite. This would require to throw away the "ellegant" and easy way of hydrating/merging data and would require more complex descriptive format, which would not represent the actuall model data. This somewhat complexer form of data is neither native for the client model, nor server model, which would require conversion both client- and server-side. The client would have to generate the description for the patch out of comparison, server must process it, instead of simply hydrating into existing set. Simpler is to make the relation a resource itself.