Skip to content

[10.x] Fix createOrFirst on transactions #48144

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 13 commits into from
Aug 23, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 16 additions & 1 deletion src/Illuminate/Database/Eloquent/Builder.php
Original file line number Diff line number Diff line change
Expand Up @@ -580,7 +580,7 @@ public function firstOrCreate(array $attributes = [], array $values = [])
public function createOrFirst(array $attributes = [], array $values = [])
{
try {
return $this->create(array_merge($attributes, $values));
return $this->withSavepointIfNeeded(fn () => $this->create(array_merge($attributes, $values)));
} catch (UniqueConstraintViolationException $exception) {
return $this->where($attributes)->first();
}
Expand Down Expand Up @@ -1709,6 +1709,21 @@ public function withCasts($casts)
return $this;
}

/**
* Execute the given Closure within a transaction savepoint if needed.
*
* @template TModelValue
*
* @param \Closure(): TModelValue $scope
* @return TModelValue
*/
public function withSavepointIfNeeded(Closure $scope): mixed
{
return $this->getQuery()->getConnection()->transactionLevel() > 0
? $this->getQuery()->getConnection()->transaction($scope)
: $scope();
}

/**
* Get the underlying query builder instance.
*
Expand Down
4 changes: 2 additions & 2 deletions src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php
Original file line number Diff line number Diff line change
Expand Up @@ -643,14 +643,14 @@ public function firstOrCreate(array $attributes = [], array $values = [], array
public function createOrFirst(array $attributes = [], array $values = [], array $joining = [], $touch = true)
{
try {
return $this->create(array_merge($attributes, $values), $joining, $touch);
return $this->getQuery()->withSavePointIfNeeded(fn () => $this->create(array_merge($attributes, $values), $joining, $touch));
} catch (UniqueConstraintViolationException $exception) {
// ...
}

try {
return tap($this->related->where($attributes)->first(), function ($instance) use ($joining, $touch) {
$this->attach($instance, $joining, $touch);
$this->getQuery()->withSavepointIfNeeded(fn () => $this->attach($instance, $joining, $touch));
});
} catch (UniqueConstraintViolationException $exception) {
return (clone $this)->where($attributes)->first();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -252,7 +252,7 @@ public function firstOrCreate(array $attributes = [], array $values = [])
public function createOrFirst(array $attributes = [], array $values = [])
{
try {
return $this->create(array_merge($attributes, $values));
return $this->getQuery()->withSavepointIfNeeded(fn () => $this->create(array_merge($attributes, $values)));
} catch (UniqueConstraintViolationException $exception) {
return $this->where($attributes)->first();
}
Expand Down
9 changes: 9 additions & 0 deletions tests/Database/DatabaseEloquentHasManyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ public function testCreateOrFirstMethodWithValuesFindsFirstModel()
);
}));

$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->once()->with(['foo' => 'bar'])->andReturn($relation->getQuery());
$relation->getQuery()->shouldReceive('first')->once()->with()->andReturn($model = m::mock(stdClass::class));

Expand All @@ -192,6 +195,9 @@ public function testCreateOrFirstMethodCreatesNewModelWithForeignKeySet()
{
$relation = $this->getRelation();

$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->never();
$relation->getQuery()->shouldReceive('first')->never();
$model = $this->expectCreatedModel($relation, ['foo']);
Expand All @@ -202,6 +208,9 @@ public function testCreateOrFirstMethodCreatesNewModelWithForeignKeySet()
public function testCreateOrFirstMethodWithValuesCreatesNewModelWithForeignKeySet()
{
$relation = $this->getRelation();
$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->never();
$relation->getQuery()->shouldReceive('first')->never();
$model = $this->expectCreatedModel($relation, ['foo' => 'bar', 'baz' => 'qux']);
Expand Down
16 changes: 16 additions & 0 deletions tests/Database/DatabaseEloquentIntegrationTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -552,6 +552,22 @@ public function testCreateOrFirst()
$this->assertSame('Nuno Maduro', $user4->name);
}

public function testCreateOrFirstWithinTransaction()
{
$user1 = EloquentTestUniqueUser::create(['email' => '[email protected]']);

DB::transaction(function () use ($user1) {
$user2 = EloquentTestUniqueUser::createOrFirst(
['email' => '[email protected]'],
['name' => 'Taylor Otwell']
);

$this->assertEquals($user1->id, $user2->id);
$this->assertSame('[email protected]', $user2->email);
$this->assertNull($user2->name);
});
}

public function testUpdateOrCreate()
{
$user1 = EloquentTestUser::create(['email' => '[email protected]']);
Expand Down
12 changes: 12 additions & 0 deletions tests/Database/DatabaseEloquentMorphTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -227,6 +227,9 @@ public function testCreateOrFirstMethodFindsFirstModel()
new UniqueConstraintViolationException('mysql', 'example mysql', [], new Exception('SQLSTATE[23000]: Integrity constraint violation: 1062')),
);

$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->once()->with(['foo'])->andReturn($relation->getQuery());
$relation->getQuery()->shouldReceive('first')->once()->with()->andReturn($model = m::mock(Model::class));

Expand All @@ -244,6 +247,9 @@ public function testCreateOrFirstMethodWithValuesFindsFirstModel()
new UniqueConstraintViolationException('mysql', 'example mysql', [], new Exception('SQLSTATE[23000]: Integrity constraint violation: 1062')),
);

$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->once()->with(['foo' => 'bar'])->andReturn($relation->getQuery());
$relation->getQuery()->shouldReceive('first')->once()->with()->andReturn($model = m::mock(Model::class));

Expand All @@ -259,6 +265,9 @@ public function testCreateOrFirstMethodCreatesNewMorphModel()
$model->shouldReceive('setAttribute')->once()->with('morph_type', get_class($relation->getParent()));
$model->shouldReceive('save')->once()->andReturn(true);

$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->never();
$relation->getQuery()->shouldReceive('first')->never();

Expand All @@ -274,6 +283,9 @@ public function testCreateOrFirstMethodWithValuesCreatesNewMorphModel()
$model->shouldReceive('setAttribute')->once()->with('morph_type', get_class($relation->getParent()));
$model->shouldReceive('save')->once()->andReturn(true);

$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
return $scope();
});
$relation->getQuery()->shouldReceive('where')->never();
$relation->getQuery()->shouldReceive('first')->never();

Expand Down
13 changes: 13 additions & 0 deletions tests/Integration/Database/EloquentBelongsToManyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -629,6 +629,19 @@ public function testCreateOrFirstUnrelatedExisting()
$this->assertTrue($tag->is($post->tagsUnique()->first()));
}

public function testCreateOrFirstWithinTransaction()
{
$post = Post::create(['title' => Str::random()]);

$tag = UniqueTag::create(['name' => Str::random()]);

$post->tagsUnique()->attach(UniqueTag::all());

DB::transaction(function () use ($tag, $post) {
$this->assertEquals($tag->id, $post->tagsUnique()->createOrFirst(['name' => $tag->name])->id);
});
}

public function testFirstOrNewMethodWithValues()
{
$post = Post::create(['title' => Str::random()]);
Expand Down
16 changes: 16 additions & 0 deletions tests/Integration/Database/EloquentHasManyTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Eloquent\Relations\HasMany;
use Illuminate\Database\Eloquent\Relations\HasOne;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;
use Illuminate\Support\Str;

Expand Down Expand Up @@ -70,6 +71,21 @@ public function testCreateOrFirst()
$this->assertTrue($post1->is($post2));
$this->assertCount(1, $user->posts()->get());
}

public function testCreateOrFirstWithinTransaction()
{
$user = EloquentHasManyTestUser::create();

$post1 = $user->posts()->create(['title' => Str::random()]);

DB::transaction(function () use ($user, $post1) {
$post2 = $user->posts()->createOrFirst(['title' => $post1->title]);

$this->assertTrue($post1->is($post2));
});

$this->assertCount(1, $user->posts()->get());
}
}

class EloquentHasManyTestUser extends Model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;

class DatabaseEloquentMySqlIntegrationTest extends MySqlTestCase
Expand Down Expand Up @@ -57,6 +58,22 @@ public function testCreateOrFirst()

$this->assertSame('Nuno Maduro', $user4->name);
}

public function testCreateOrFirstWithinTransaction()
{
$user1 = DatabaseEloquentMySqlIntegrationUser::createOrFirst(['email' => '[email protected]']);

DB::transaction(function () use ($user1) {
$user2 = DatabaseEloquentMySqlIntegrationUser::createOrFirst(
['email' => '[email protected]'],
['name' => 'Taylor Otwell']
);

$this->assertEquals($user1->id, $user2->id);
$this->assertSame('[email protected]', $user2->email);
$this->assertNull($user2->name);
});
}
}

class DatabaseEloquentMySqlIntegrationUser extends Model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;

class DatabaseEloquentPostgresIntegrationTest extends PostgresTestCase
Expand Down Expand Up @@ -57,6 +58,22 @@ public function testCreateOrFirst()

$this->assertSame('Nuno Maduro', $user4->name);
}

public function testCreateOrFirstWithinTransaction()
{
$user1 = DatabaseEloquentPostgresIntegrationUser::create(['email' => '[email protected]']);

DB::transaction(function () use ($user1) {
$user2 = DatabaseEloquentPostgresIntegrationUser::createOrFirst(
['email' => '[email protected]'],
['name' => 'Taylor Otwell']
);

$this->assertEquals($user1->id, $user2->id);
$this->assertSame('[email protected]', $user2->email);
$this->assertNull($user2->name);
});
}
}

class DatabaseEloquentPostgresIntegrationUser extends Model
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@

use Illuminate\Database\Eloquent\Model;
use Illuminate\Database\Schema\Blueprint;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\Facades\Schema;

class DatabaseEloquentSqlServerIntegrationTest extends SqlServerTestCase
Expand Down Expand Up @@ -57,6 +58,22 @@ public function testCreateOrFirst()

$this->assertSame('Nuno Maduro', $user4->name);
}

public function testCreateOrFirstWithinTransaction()
{
$user1 = DatabaseEloquentSqlServerIntegrationUser::createOrFirst(['email' => '[email protected]']);

DB::transaction(function () use ($user1) {
$user2 = DatabaseEloquentSqlServerIntegrationUser::createOrFirst(
['email' => '[email protected]'],
['name' => 'Taylor Otwell']
);

$this->assertEquals($user1->id, $user2->id);
$this->assertSame('[email protected]', $user2->email);
$this->assertNull($user2->name);
});
}
}

class DatabaseEloquentSqlServerIntegrationUser extends Model
Expand Down