Simplify allowed include paths implementation

[JapSeyz]

Hi,

When loading relations like this:

GET 1?http://test.com/car-manufacturers/1?include=factories.cars%2Cdrivers.children

and having this:

protected $allowedIncludePaths = ['drivers', 'factories'];

The queryRules() fails, as it's not the exact rules provided.

The hack right now is to return include => 'string', but it feels dirty.

Is there a better way to properly validate the relations?

Cheers

[fisharebest]

You need the full relation

protected $allowedIncludePaths = ['drivers.children', 'factories.cars'];

[JapSeyz]

It has worked fine without that so far. It seems wrong to specify which relations a driver may have, on the car-manufacturer?

Adding 'include' => 'string', to the QueryRules keeps the relations working with just the top-level relationship being set. Including a relationship that is not in the top-level defined relations in the allowedIncludePaths, still fails as expected.

[fisharebest]

It seems wrong to specify which relations a driver may have, on the car-manufacturer?

If there's a simpler way, we haven't found it. Some of our $allowedIncludePaths values are very long indeed.

[lindyhopchris]

Yeah it works this way because that's how the underlying neomerx/json-api library worked - you have to specify the full include paths that you allow. So in this example, we'd tend to do this:

protected $allowedIncludePaths = [
    'drivers',
    'drivers.children', 
    'factories',
    'factories.cars',
];

We might be able to simplify it a bit in a future version, mainly because we want to move away from using the neomerx package for anything except encoding. But IMHO being explicit about what paths are supported does kind of make sense, because each one causes eager-loading. So the more you allow, and the longer paths you allow, you're increasingly opening up some pretty demanding database queries.

Stripe for instance only allow a maximum depth of 4 of their expand paths, which are the equivalent of JSON AP include paths.

What might be nice is to simplify it so that if you did this:

protected $allowedIncludePaths = ['foo.bar.baz.bat'];

It would know to allow foo, foo.bar, foo.bar.baz and foo.bar.baz.bat, i.e. without you having to list each individually as you have to do now.

[JapSeyz]

I understand wanting to be explicit about the relationships, but I feel like it's wrong to define which childrens-children is allowed, it would be better to look at the child resource to see which relationships are allowed on that and then propagating it down.

Possibly adding a $maxDepth option, to say that from this resource we will only allow you to eagerload 2 steps down.

Does that make sense?

Cheers

[lindyhopchris]

yeah, that makes sense. think we can look at improving this on the 2.x milestone.

[lindyhopchris]

Include paths have been significantly improved and simplified in the new package laravel-json-api/laravel. See:
https://laraveljsonapi.io/docs/1.0/schemas/eager-loading.html