Avoid loading already loaded relations and allow loading non studly relation names

[Tofandel]

load would load relations even if they were already loaded in the model

But we might not even need to do that explicitely because $this->model->{$studlyName} would also load it

Added a fix for #787 as well

[Tofandel]

$studlyName = Str::studly($name); is also not the correct way to search for a relation, it could be studly or it could not, in fact the convention is either camel or snake, but never studly, but it was not caught because relations are still loaded in initialize

Eg public function available_books(): HasMany

In which case load will fail

[Tofandel]

Also worth noting that with the new normalizer the data class will find the property whether SnakeCaseMapper is used or not

    $dataClass = new class () extends Data {
        #[DataCollectionOf(FakeNestedModelData::class), MapInputName(SnakeCaseMapper::class)]
        public array|Optional $fakeNestedModels;

        #[MapInputName(SnakeCaseMapper::class)]
        public string $oldAccessor;
    };

This is due to $propertyName = $this->model::$snakeAttributes ? Str::snake($name) : $name;

But I believe this is not really undesirable behavior

[sebastiaanluca]

I was checking the new code and noticed the nested eager load (4.5.1...4.6.0#diff-5a676e204bcb067cd1bf62ef73416609c339f8e16479ab7b38021e9799c2d607R82). Is it possible to remove this line completely?

If you have a collection of models it'll create an n+1 relation loading issue when creating a property from a relation. The issue remains even when using loadMissing(). IMO the normalizer class should not be in charge of eager loading anything, that's up to the user, higher up in the controller or elsewhere.

I was going to create a PR for it, but maybe we can include it in this one? I believe I also highlighted the issue a while ago (in this repo or another) where a newly introduced nested eager load triggered a ton of queries in our app and we didn't catch it for a long time because it was included in a minor or patch release. The n+1 exception isn't thrown because the relation isn't lazy loaded.

[Tofandel]

it'll create an n+1 relation loading issue when creating a property from a relation. IMO the normalizer class should not be in charge of eager loading anything, that's up to the user, higher up in the controller or elsewhere.

That doesn't really matter, you're already in charge of this because this only happens with a special attribute and not by default so you already have to be explicit about it (that's why it's in a minor with no breaking change), of course if you have 4 nested relations with 100 of models you'll want to load them yourself, but for 10 models with 1 level or 2 of nesting, it's not worth removing this feature

With loadMissing, if you run load('some.nested.relation') yourself in the controller while still having the attribue the n+1 issue will not happen, while with load it does, so this is completely workable, it would just become a fallback so that the relation is still loaded if you cannot load it (for me there is some instances where some models are included from morphs and it's impossible to eager load the relation in another polymorphic relations (because the relation might not exist in all the models), in this case loadRelation is the only solution)

We could improve it by computing the DataProperty type of each relation and eager load all of them with the attribute, but it might be a big amount of work and it needs to handle/avoid recursive relations like children.children.children.children to infinity, how would you do this, at what point do you stop? Maybe we just add a property maxRecursiveDepth to the loadRelation attribute