[10.x] Enhancing updateOrCreate() to Use firstOrCreate()

[mpyw]

Summary

This PR proposes a modification to the updateOrCreate() method in line with recent changes brought by the introduction of the createOrFirst() method.

Details

• [10.x] Adds a createOrFirst method to Eloquent #47973
  • [10.x] Fix createOrFirst on transactions #48144

In the previous PR by @tonysm, the firstOrCreate() method was enhanced to leverage createOrFirst(), making it more race-condition resistant, especially in highly concurrent environments. This was achieved by attempting to create a record first and, if encountering a unique constraint violation, falling back to find the matching record. This robust approach, however, was not mirrored in the updateOrCreate() method.

To address this and bring consistency:

public function updateOrCreate(array $attributes, array $values = [])
{
    return tap($this->firstOrCreate($attributes, $values), function ($instance) use ($values) {
        if (!$instance->wasRecentlyCreated) {
            $instance->fill($values)->save();
        }
    });
}

By doing this, the updateOrCreate() method will now utilize the enhanced firstOrCreate(), making it more resilient against potential race conditions and ensuring that both methods share a unified and robust approach in concurrent situations.

[mpyw]

@taylorotwell With reference to PR #47973, the only method that was internally changed to use createOrFirst is Builder::firstOrCreate from the following list:

• Builder::firstOrCreate
• HasOneOrMany::firstOrCreate
• BelongsToMany::firstOrCreate

For this reason, in this PR, I've only made the adjustment to Builder::updateOrCreate.

---

Should we aim to establish the dependency chain createOrFirst ← firstOrCreate ← updateOrCreate across all Builder and Relation classes in another PR? Do you have any concerns or reservations about this approach?

[tonysm]

@mpyw Oh, nice catch. I forgot to update the relation methods. I'll send a separate PR to address that. I think we better attempt to update all updateOrCreate implementations, so it's consistent across the board.

[mpyw]

• [10.x] Make the firstOrCreate methods in relations use createOrFirst behind the scenes #48192

[mpyw]

• Resolves Duplicate entries on updateOrCreate #19372

[ceejayoz]

I'm trying to generate a simple reproduction case, but this change has broken some code for us.

Route::get('test', function() {
    $user = User::findOrFail(11);

    $social = Social::withTrashed()
        ->updateOrCreate(
            [
                'type' => 'facebook_account',
                'external_id' => 'test'
            ],
            [
                'name' => 'test',
                'details' => null,
                'type' => 'facebook_account',
                'external_id' => 'test',
            ],
        );

    $connection = $user->connections()->updateOrCreate(
        [
            'social_id' => $social->id
        ],
        [
            'type' => 'facebook_account',
            'token' => 'test',
            'via_id' => null,
            'details' => null,
            'social_id' => $social->id
        ],
    );
});

This results in an error on the creation of $connection, with social_id being null in the query, despite being in the $fillable array. Reverting the change immediately resolves the error.

Simple repro example is proving tough because both our Social and Connection models are using https://github.com/tighten/parental, but I wanted to note this in case anyone else is encountering the same.

[tonysm]

@ceejayoz from what I can see, I don't see why the Connection::updateOrCreate would fail. When you say "fail" what exactly happens? Does it set the social_id to null? Or do you get an exception?

I don't see why social_id would be set to null, since it's being used to find the connection record in the createOrFirst

Both Connection and Social are part of the same hierarchy? I mean, are these models pointing to the same table? If so, does Connection also use the soft-deletes trait?

Can you create an issue for it referencing this PR?

[ceejayoz]

@tonysm The call results in a violation of the not-null constraint on my social_id column; it's null in the final query.

I've drilled down into internals a bit with dumps of the $attributes and $values arrays in a number of spots; it's getting nulled out somewhere but I haven't quite figured out the tap() stuff's impact there. I need to get xcode running on this particular project so I can track it a little more comprehensively.

$fillable is set on both models appropriately, and undoing the change in vendor/laravel/framework/src/Illuminate/Database/Eloquent/Builder.php definitely puts it back to normal operation.

Connection and Social are different models, each with same-table-inheritance submodels (using Parental), i.e. there's a TwitterSocial and a TwitterConnection. Socials have a hasMany Connections on them. Both have soft deletes enabled.

It may be something peculiar to Parental; I'll try and get a minimum example repo put together.

[tonysm]

@ceejayoz we reverted from using the createOrFirst in the updateOrCreate methods (already merged in the 10.x branch). Can you check if the issue was resolved?