[10.x] Failing test/bug report in HasManyThrough updateOrCreate() since Laravel 10.21

[ralphjsmit]

Last week, I had a weird bug in an application, where suddenly records from different users become visible to each other. The bug seemed to be related to a certain updateOrCreate() method. Suddenly, old records from one year ago were being updated with recent data from other users.

After much digging, I found that there seems to be a bug introduced in Laravel 10.21.0. Consider the following code:

public function legs(): HasManyThrough
{
    return $this->hasManyThrough(Leg::class, Booking::class, 'user_id', 'booking_id');
}

The error happens in the following code:

$user->legs()->updateOrCreate([
    'legs.external_id' => 1234,
], [
   // 
]);

Say that User A has a booking with an ID of 1234. This updateOrCreate() statement would just randomly find any leg with an ID of 1234 and update that random leg with the data from the second updateOrCreate() parameter. Said again, the error is that this code would retrieve legs by ID instead of the actual conditions specified , and for the leg ID it takes the booking ID. So it could update a leg of User B with data from User A.

This is very dangerous bug in an application. I've been able to reproduce the case in a failing test, which you find attached to this PR. If you run this test on Laravel 10.20.0, the test will pass. If you run this on Laravel 10.21.0 or later, it will fail (like it currently does).

The code where the bug was introduced is #48192 in the HasManyThrough.php file, because if I revert the code in the updateOrCreate() method there, the code works as expected. However, the code in that file doesn't look too strange, so probably it's an underlying bug in hydrating the model. Please see the test for a better explanation.

I checked the queries that are being run and the full flow from a ->where($conditions)->first() until executing the statement, but I do not see anything wrong there:

select
  *
from
  "users"
  inner join "teams" on "teams"."id" = "users"."team_id"
where
  "teams"."owner_id" = 3
  and ("name" = 'John')
limit
  1

update "users"
set
  "slug" = 'john-doe'
where
  "id" = 1

However, even after double-checking multiple times, the test keeps passing on 10.20.0 and keeps failing on 10.21.0, so the bug must be somewhere in these 53 files.

[taylorotwell]

Thanks for the report. We are reverting now.

[mpyw]

@ralphjsmit @taylorotwell I apologize for the inconvenience. Upon further investigation conducted by myself and @fuwasegu, it was found that the implementation of firstOrCreate createOrFirst on HasManyThrough had been missing, which has already been fixed by @tonysm. With the fixed version and revive the changes, it was confirmed to work correctly. However, I agree with the decision to revert as this was overly packed for a non-breaking change in 10.x.

Nonetheless, this is a valuable improvement that would be sad to discard entirely. I am keen on considering its revival in 11.x. Up until now, every time I work on highly concurrent application development, the standard firstOrCreate updateOrCreate methods have been inadequate, leading me to implement retry processes using my custom library https://github.com/mpyw/laravel-retry-on-duplicate-key. It's regrettable that the introduction of a custom library becomes virtually indispensable in future development, so I sincerely wish for this aspect to be integrated into the Laravel core.

Regarding this issue, I am willing to create a discussion if necessary. Just wanted to report this for now.

[mpyw]

Created a discussion:

• [10.x] Revival of the reverted changes in 10.25.0: `firstOrCreate` `updateOrCreate` improvement through `createOrFirst` + additional query tests #48567