Skip to content

Commit dc50e1a

Browse files
tonysmtaylorotwell
tonysm
and
taylorotwell
authored
[10.x] Fix createOrFirst on transactions (#48144)
* Test createOrFirst within a transaction on MySQL * Wraps the create part of the createOrFirst in a savepoint if needed * Wraps the create and attach parts of the createOrFirst within a transaction savepoint if needed * Wraps the create part of the createOrFirst in a savepoint if needed * Fix comments * Moves the eloquent contract tests to a trait * Fix the tests using Mocks * Revert "Moves the eloquent contract tests to a trait" This reverts commit a7e57c8. * Only use savepoint for postgres * Revert "Only use savepoint for postgres" This reverts commit c9c2c6a. * use short closures. formatting * StyleCI * Fix StyleCI formatting --------- Co-authored-by: Taylor Otwell <[email protected]>
1 parent f0bf683 commit dc50e1a

11 files changed

+136
-4
lines changed

‎src/Illuminate/Database/Eloquent/Builder.php

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -580,7 +580,7 @@ public function firstOrCreate(array $attributes = [], array $values = [])
580580
public function createOrFirst(array $attributes = [], array $values = [])
581581
{
582582
try {
583-
return $this->create(array_merge($attributes, $values));
583+
return $this->withSavepointIfNeeded(fn () => $this->create(array_merge($attributes, $values)));
584584
} catch (UniqueConstraintViolationException $exception) {
585585
return $this->where($attributes)->first();
586586
}
@@ -1709,6 +1709,21 @@ public function withCasts($casts)
17091709
return $this;
17101710
}
17111711

1712+
/**
1713+
* Execute the given Closure within a transaction savepoint if needed.
1714+
*
1715+
* @template TModelValue
1716+
*
1717+
* @param \Closure(): TModelValue $scope
1718+
* @return TModelValue
1719+
*/
1720+
public function withSavepointIfNeeded(Closure $scope): mixed
1721+
{
1722+
return $this->getQuery()->getConnection()->transactionLevel() > 0
1723+
? $this->getQuery()->getConnection()->transaction($scope)
1724+
: $scope();
1725+
}
1726+
17121727
/**
17131728
* Get the underlying query builder instance.
17141729
*

‎src/Illuminate/Database/Eloquent/Relations/BelongsToMany.php

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -643,14 +643,14 @@ public function firstOrCreate(array $attributes = [], array $values = [], array
643643
public function createOrFirst(array $attributes = [], array $values = [], array $joining = [], $touch = true)
644644
{
645645
try {
646-
return $this->create(array_merge($attributes, $values), $joining, $touch);
646+
return $this->getQuery()->withSavePointIfNeeded(fn () => $this->create(array_merge($attributes, $values), $joining, $touch));
647647
} catch (UniqueConstraintViolationException $exception) {
648648
// ...
649649
}
650650

651651
try {
652652
return tap($this->related->where($attributes)->first(), function ($instance) use ($joining, $touch) {
653-
$this->attach($instance, $joining, $touch);
653+
$this->getQuery()->withSavepointIfNeeded(fn () => $this->attach($instance, $joining, $touch));
654654
});
655655
} catch (UniqueConstraintViolationException $exception) {
656656
return (clone $this)->where($attributes)->first();

‎src/Illuminate/Database/Eloquent/Relations/HasOneOrMany.php

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -252,7 +252,7 @@ public function firstOrCreate(array $attributes = [], array $values = [])
252252
public function createOrFirst(array $attributes = [], array $values = [])
253253
{
254254
try {
255-
return $this->create(array_merge($attributes, $values));
255+
return $this->getQuery()->withSavepointIfNeeded(fn () => $this->create(array_merge($attributes, $values)));
256256
} catch (UniqueConstraintViolationException $exception) {
257257
return $this->where($attributes)->first();
258258
}

‎tests/Database/DatabaseEloquentHasManyTest.php

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -181,6 +181,9 @@ public function testCreateOrFirstMethodWithValuesFindsFirstModel()
181181
);
182182
}));
183183

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

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

198+
$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
199+
return $scope();
200+
});
195201
$relation->getQuery()->shouldReceive('where')->never();
196202
$relation->getQuery()->shouldReceive('first')->never();
197203
$model = $this->expectCreatedModel($relation, ['foo']);
@@ -202,6 +208,9 @@ public function testCreateOrFirstMethodCreatesNewModelWithForeignKeySet()
202208
public function testCreateOrFirstMethodWithValuesCreatesNewModelWithForeignKeySet()
203209
{
204210
$relation = $this->getRelation();
211+
$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
212+
return $scope();
213+
});
205214
$relation->getQuery()->shouldReceive('where')->never();
206215
$relation->getQuery()->shouldReceive('first')->never();
207216
$model = $this->expectCreatedModel($relation, ['foo' => 'bar', 'baz' => 'qux']);

‎tests/Database/DatabaseEloquentIntegrationTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -552,6 +552,22 @@ public function testCreateOrFirst()
552552
$this->assertSame('Nuno Maduro', $user4->name);
553553
}
554554

555+
public function testCreateOrFirstWithinTransaction()
556+
{
557+
$user1 = EloquentTestUniqueUser::create(['email' => '[email protected]']);
558+
559+
DB::transaction(function () use ($user1) {
560+
$user2 = EloquentTestUniqueUser::createOrFirst(
561+
['email' => '[email protected]'],
562+
['name' => 'Taylor Otwell']
563+
);
564+
565+
$this->assertEquals($user1->id, $user2->id);
566+
$this->assertSame('[email protected]', $user2->email);
567+
$this->assertNull($user2->name);
568+
});
569+
}
570+
555571
public function testUpdateOrCreate()
556572
{
557573
$user1 = EloquentTestUser::create(['email' => '[email protected]']);

‎tests/Database/DatabaseEloquentMorphTest.php

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -227,6 +227,9 @@ public function testCreateOrFirstMethodFindsFirstModel()
227227
new UniqueConstraintViolationException('mysql', 'example mysql', [], new Exception('SQLSTATE[23000]: Integrity constraint violation: 1062')),
228228
);
229229

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

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

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

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

268+
$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
269+
return $scope();
270+
});
262271
$relation->getQuery()->shouldReceive('where')->never();
263272
$relation->getQuery()->shouldReceive('first')->never();
264273

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

286+
$relation->getQuery()->shouldReceive('withSavepointIfNeeded')->once()->andReturnUsing(function ($scope) {
287+
return $scope();
288+
});
277289
$relation->getQuery()->shouldReceive('where')->never();
278290
$relation->getQuery()->shouldReceive('first')->never();
279291

‎tests/Integration/Database/EloquentBelongsToManyTest.php

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -629,6 +629,19 @@ public function testCreateOrFirstUnrelatedExisting()
629629
$this->assertTrue($tag->is($post->tagsUnique()->first()));
630630
}
631631

632+
public function testCreateOrFirstWithinTransaction()
633+
{
634+
$post = Post::create(['title' => Str::random()]);
635+
636+
$tag = UniqueTag::create(['name' => Str::random()]);
637+
638+
$post->tagsUnique()->attach(UniqueTag::all());
639+
640+
DB::transaction(function () use ($tag, $post) {
641+
$this->assertEquals($tag->id, $post->tagsUnique()->createOrFirst(['name' => $tag->name])->id);
642+
});
643+
}
644+
632645
public function testFirstOrNewMethodWithValues()
633646
{
634647
$post = Post::create(['title' => Str::random()]);

‎tests/Integration/Database/EloquentHasManyTest.php

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
use Illuminate\Database\Eloquent\Model;
66
use Illuminate\Database\Eloquent\Relations\HasMany;
77
use Illuminate\Database\Eloquent\Relations\HasOne;
8+
use Illuminate\Support\Facades\DB;
89
use Illuminate\Support\Facades\Schema;
910
use Illuminate\Support\Str;
1011

@@ -70,6 +71,21 @@ public function testCreateOrFirst()
7071
$this->assertTrue($post1->is($post2));
7172
$this->assertCount(1, $user->posts()->get());
7273
}
74+
75+
public function testCreateOrFirstWithinTransaction()
76+
{
77+
$user = EloquentHasManyTestUser::create();
78+
79+
$post1 = $user->posts()->create(['title' => Str::random()]);
80+
81+
DB::transaction(function () use ($user, $post1) {
82+
$post2 = $user->posts()->createOrFirst(['title' => $post1->title]);
83+
84+
$this->assertTrue($post1->is($post2));
85+
});
86+
87+
$this->assertCount(1, $user->posts()->get());
88+
}
7389
}
7490

7591
class EloquentHasManyTestUser extends Model

‎tests/Integration/Database/MySql/DatabaseEloquentMySqlIntegrationTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Illuminate\Database\Eloquent\Model;
66
use Illuminate\Database\Schema\Blueprint;
7+
use Illuminate\Support\Facades\DB;
78
use Illuminate\Support\Facades\Schema;
89

910
class DatabaseEloquentMySqlIntegrationTest extends MySqlTestCase
@@ -57,6 +58,22 @@ public function testCreateOrFirst()
5758

5859
$this->assertSame('Nuno Maduro', $user4->name);
5960
}
61+
62+
public function testCreateOrFirstWithinTransaction()
63+
{
64+
$user1 = DatabaseEloquentMySqlIntegrationUser::createOrFirst(['email' => '[email protected]']);
65+
66+
DB::transaction(function () use ($user1) {
67+
$user2 = DatabaseEloquentMySqlIntegrationUser::createOrFirst(
68+
['email' => '[email protected]'],
69+
['name' => 'Taylor Otwell']
70+
);
71+
72+
$this->assertEquals($user1->id, $user2->id);
73+
$this->assertSame('[email protected]', $user2->email);
74+
$this->assertNull($user2->name);
75+
});
76+
}
6077
}
6178

6279
class DatabaseEloquentMySqlIntegrationUser extends Model

‎tests/Integration/Database/Postgres/DatabaseEloquentPostgresIntegrationTest.php

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
use Illuminate\Database\Eloquent\Model;
66
use Illuminate\Database\Schema\Blueprint;
7+
use Illuminate\Support\Facades\DB;
78
use Illuminate\Support\Facades\Schema;
89

910
class DatabaseEloquentPostgresIntegrationTest extends PostgresTestCase
@@ -57,6 +58,22 @@ public function testCreateOrFirst()
5758

5859
$this->assertSame('Nuno Maduro', $user4->name);
5960
}
61+
62+
public function testCreateOrFirstWithinTransaction()
63+
{
64+
$user1 = DatabaseEloquentPostgresIntegrationUser::create(['email' => '[email protected]']);
65+
66+
DB::transaction(function () use ($user1) {
67+
$user2 = DatabaseEloquentPostgresIntegrationUser::createOrFirst(
68+
['email' => '[email protected]'],
69+
['name' => 'Taylor Otwell']
70+
);
71+
72+
$this->assertEquals($user1->id, $user2->id);
73+
$this->assertSame('[email protected]', $user2->email);
74+
$this->assertNull($user2->name);
75+
});
76+
}
6077
}
6178

6279
class DatabaseEloquentPostgresIntegrationUser extends Model

0 commit comments

Comments
 (0)