flat3 / lodata

The OData v4.01 Producer for Laravel
https://lodata.io/
MIT License
80 stars 26 forks source link

support for deep inserts #49

Closed Remo closed 3 years ago

Remo commented 3 years ago

I'm trying to create an entity include a child entity for these two models:

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class Product extends Model
{   
    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        'product_name',
        'price',
    ];

    public function descriptions()
    {
        return $this->hasMany(ProductDescription::class);
    }

}

and

<?php

namespace App\Models;

use Illuminate\Database\Eloquent\Model;

class ProductDescription extends Model
{   
    /**
     * The attributes that are mass assignable.
     *
     * @var array
     */
    protected $fillable = [
        'product_id',
        'language',
        'description',
    ];

    public function product()
    {
        return $this->belongsTo(Product::class);
    }
}

My payload looks like this:

{
    "product_name" : "Product A",  
    "price" : 200,
    "descriptions": [
        {
            "language":"de",
            "description": "Produkt A"
        }
    ] 
}

It doesn't seem to care about the descriptions and only creates the top level product record.

According to the specification they call this deep insert. I haven't found anything in the code about that. Is this something which isn't supported? Any plans to support it?

This is the section I'm talking about: https://docs.oasis-open.org/odata/odata/v4.01/os/part1-protocol/odata-v4.01-os-part1-protocol.html#sec_CreateRelatedEntitiesWhenCreatinganE

27pchrisl commented 3 years ago

Hi @Remo

No it doesn't support that. It does support doing an insert on a relation for an existing record using the path syntax (see tests/Unit/Eloquent/EloquentTest.php:140), so there's some support in that area.

The right place in the code is probably in the implementation-specific entity set create methods \Flat3\Lodata\Drivers\SQLEntitySet::create and \Flat3\Lodata\Drivers\EloquentEntitySet::create to call a helper in the base EntitySet class that handles this recursion (There's no reason it shouldn't iterate infinitely deep) after it creates the initial record.

The helper would need to iterate over provided navigation properties for the record after creating the initial record, and call create() again to generate subrecords. Create might have to be split up to support this.

27pchrisl commented 3 years ago

To answer your question though about future support, I'm keen to get feedback on what people actually need in the real world from the library. You're the first to provide feedback : )

Remo commented 3 years ago

Thanks for you quick answer. To give you some more background information: We basically use SPAs and oData as a backend. Mostly VueJS but it doesn't matter much. In a single screen we might have a product record and some child entities like product descriptions. With those SPA frameworks it's nice and easy to build a JSON containing everything need to create the product head as well as the connected child entities.

In this case a deep insert is obviously quite elegant as we can just take the JSON from the client and store it in the database with very little effort and since it all happens in a single call, we're transaction safe too. I think that's the main reason for me to avoid multiple calls to store data. Thanks to HTTP2 the network latency wouldn't be a concern, but ending up with only a head and no children is a no-go.

Our own implementation does quite a lot with relations. We also delete child entities in case they aren't part of the payload. I'm not sure that is compliant with the odata specification to be honest, but it's quite nice to work with for us ;-)

So far this part is the biggest show stopper for us, but I'll play around with lodata for a bit to see if there are other missing functions and then decide if we want to start using lodata and add what we need.

27pchrisl commented 3 years ago

Deep insert support is there now, see https://github.com/flat3/lodata/blob/develop/tests/Unit/Modify/CreateTest.php#L73

Note that currently Lodata does not use database-level transactions to wrap the whole request, but I do plan to add this.

I don't think deleting child entities is part of the spec : ) but you can use a batch request with Lodata if the client can determine what to delete.

Remo commented 3 years ago

Thanks! This is pretty cool.

I'm still wondering about the delete part. Having another batch request would still cause problems with transactions. If there's a problem with the integrity that causes the delete operation to fail, we end up with a wrong result. In our application we might look at a customer with three addresses, modify two addresses and remove one. When we hit save, everything is processed and saved in a single transaction.

I wonder how others who use odata get around that issue. I can only imagine that delete operation are executed immediately, but that feels odd to me if we allow multiple records to updated at the same time. We could also change that and only allow a single record to be modified, but then what happens if I want to create a new customer with three addresses? I would have to hit "save" four times.

I'm just thinking out loud: What if there was a way to hook into certain parts of lodata to achieve such "strange" things that aren't part of odata? I understand that from an odata point of view it would be strange, but being able to process a full json structure in a single transaction has lots of benefits in my opinion. I think it's clear that we wouldn't want this to be part of the default lodata, but maybe we could fire some laravel events to do additional processing?

27pchrisl commented 3 years ago

It looks like odata solves this in https://docs.oasis-open.org/odata/odata/v4.01/os/part1-protocol/odata-v4.01-os-part1-protocol.html#sec_UpdateRelatedEntitiesWhenUpdatinganE where a deep update can cause a delete, and upsert support would mean you can create, update and delete in a single request. All of this is probably not too hard to implement, now that there's a strategy for recursive deep creates.

I will try and think of the best way of including transaction support, so that rollbacks are possible no matter what data backend you're using.

But I also agree that there's not currently really an easy way in Lodata to control its behaviour in detail from your app. The "way" right now would be to subclass EntitySet but that's heavy. Laravel events would probably be good to add, and possibly some sort of easy-to-use filtering.

Remo commented 3 years ago

Somehow I've missed this section of the spec )-: This looks pretty slick. I'm definitely tempted to switch some projects to lodata!

27pchrisl commented 3 years ago

@Remo Deep update support is now available (with the one exception being that you can't remove a related record without deleting it).

There are some examples in https://github.com/flat3/lodata/blob/develop/tests/Unit/Modify/UpdateTest.php#L73

27pchrisl commented 3 years ago

Closing this ticket now that support is implemented