#Recursion on toArray with bidirectional calculated appended attributes

1 messages · Page 1 of 1 (latest)

tight sandal
#

Hi there,

I have an issue with appending attributes that are calculated based on relations.

Specifically, I have a model called User which represents the user to log in, and I have two models attached to the user for additional information. A user can either be a user with a profile or a user attached to a staff.

In the user model, I have a calculated attribute called is_profile_user (and also an attribute called is_staff_user, not included in the example code) for better readable code and also to provide this to the frontend. This is also the reason why I $append that data.

In the profile (and the staff) model, I have a calculated attribute called user_status to display whether that profile in the system already has a user attached and whether that one is active. This is also for the frontend; thus I also added that attribute to the $appends array.

When calling toArray or simply returning User::all() in a controller, this causes an infinite recursion. The same happens when calling jsonSerialize() (called by telescope).

This does not happen when calling attributesToArray() as this excludes the calculated attributes.

Example source code: https://github.com/irobin591/laravel-attribute-loop/commit/7641eee203407586da828d42872cea4ee93c39c3

Failing testcases because of infinite recursion:
https://github.com/irobin591/laravel-attribute-loop/actions/runs/10959054378/job/30430598531

I have no idea how to solve this issue, though. 
Any ideas?

worn wasp
#

I believe the issue here is the $this->user in your Profile Model... since appends will recall the is_staff and if it's not staff, just infi-loops...

tight sandal
#

That is correct, the issue is that the profile model loads the user as the user_status is appended and $this->user is called. This causes the user to load and the is_staff loads the profile again.

This causes a circular dependency.

I still have no idea how to solve or workaround this.

solemn crypt
#

just don't use appends and add it where needed

#

i've never seen $appends used where it doesn't cause chaos

#

and if you need a circular reference to know who is who then maybe just rethink the way it works

#

e.g. maybe morph is not a good way to go about this

#

e.g. user can only have one identifies, so he cannot have staff and profile at the same time? then why not just mark it on the user itself that he's staff?

#

without morph the user could have profile_id and staff_id etc, which would again, solve the problem just by checking if the id is not null

winged mango
#

Try updating to the latest Laravel release. It includes an update to toArray() which handles the infinite recursion issue.

tight sandal
# solemn crypt without morph the user could have profile_id and staff_id etc, which would again...

Thanks for the input. Note that this is only a small portion of the project to display the specific issue. The profile and the staff models have additional data attached to them and are used in different relations, i.e. staff responsible for a profile. They are using the same system to log in though.

Checking if the id is not null might work, but it won't catch soft deleted referenced models.

I am probably going to check the identifies_type for is_staff/is_profile and then do a cached query, independent of the relations itself.

tight sandal
# winged mango Try updating to the latest Laravel release. It includes an update to toArray() w...

Oh, good to know. Thanks!

Haven't updated the original repository yet but I am going to try it out later today. If you are talking about the PreventsCircularRecursion class, this does not seem to work for this minimal example. It gets called within the stack trace multiple times, but does not seem to catch this circular recursion:

#0 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(377): App\Models\User->toArray()
#1 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1664): Illuminate\Database\Eloquent\Model->relationsToArray()
#2 [internal function]: Illuminate\Database\Eloquent\Model->Illuminate\Database\Eloquent\{closure}()
#3 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/PreventsCircularRecursion.php(42): call_user_func()
#4 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1663): Illuminate\Database\Eloquent\Model->withoutRecursion()
#5 /repository/app/Models/Profile.php(47): Illuminate\Database\Eloquent\Model->toArray()
#6 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/HasAttributes.php(377): App\Models\Profile->toArray()
#7 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1664): Illuminate\Database\Eloquent\Model->relationsToArray()
#8 [internal function]: Illuminate\Database\Eloquent\Model->Illuminate\Database\Eloquent\{closure}()
#9 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Concerns/PreventsCircularRecursion.php(42): call_user_func()
#10 /repository/vendor/laravel/framework/src/Illuminate/Database/Eloquent/Model.php(1663): Illuminate\Database\Eloquent\Model->withoutRecursion()
#11 /repository/app/Models/User.php(80): Illuminate\Database\Eloquent\Model->toArray()
[...]

https://github.com/irobin591/laravel-attribute-loop/actions/runs/10959054378/job/30430598531

GitHub

Contribute to irobin591/laravel-attribute-loop development by creating an account on GitHub.

solemn crypt
#

Yeah, I don't know the full structure, but this still to me looks like a code smell in the same way as having role names hardcoded throughout the code. Profile and Staff are separate tables so doesn't matter if it's 2 relations loaded or 1 morph, it's still gonna do 2 queries for both on eager load etc., just one easier to manage than the other. And this also looks like a lot of instanceof littering throughout the code because they're also have different methods etc.. For the soft deletes most of the time is just and easy escape hatch, not a proper solution. Even if it's soft deleted, you should nullify the id in this case or keep it in audit logs.

#

It's also easier if they also have relations, as in load('staff.duties') instead of 'identifies.duties' and then ops, profile does not have this relation.