Skip to content

Commit 55ca36e

Browse files
GromNaNGromNaN
authored
PHPORM-180 Keep createOrFirst in 2 commands to simplify implementation (#2984)
1 parent 3f84373 commit 55ca36e

File tree

4 files changed

+87
-102
lines changed

4 files changed

+87
-102
lines changed

Diff for: ‎CHANGELOG.md

+1
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file.
88
* Add `mongodb` driver for Batching by @GromNaN in [#2904](https://github.com/mongodb/laravel-mongodb/pull/2904)
99
* Rename queue option `table` to `collection`
1010
* Replace queue option `expire` with `retry_after`
11+
* Revert behavior of `createOrFirst` to delegate to `firstOrCreate` when in transaction by @GromNaN in [#2984](https://github.com/mongodb/laravel-mongodb/pull/2984)
1112

1213
## [4.3.1]
1314

Diff for: ‎src/Eloquent/Builder.php

+27-51
Original file line numberDiff line numberDiff line change
@@ -4,28 +4,24 @@
44

55
namespace MongoDB\Laravel\Eloquent;
66

7-
use Illuminate\Database\ConnectionInterface;
87
use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
9-
use InvalidArgumentException;
108
use MongoDB\Driver\Cursor;
11-
use MongoDB\Laravel\Collection;
9+
use MongoDB\Driver\Exception\WriteException;
10+
use MongoDB\Laravel\Connection;
1211
use MongoDB\Laravel\Helpers\QueriesRelationships;
13-
use MongoDB\Laravel\Internal\FindAndModifyCommandSubscriber;
1412
use MongoDB\Laravel\Query\AggregationBuilder;
1513
use MongoDB\Model\BSONDocument;
16-
use MongoDB\Operation\FindOneAndUpdate;
1714

18-
use function array_intersect_key;
1915
use function array_key_exists;
2016
use function array_merge;
2117
use function collect;
2218
use function is_array;
2319
use function iterator_to_array;
24-
use function json_encode;
2520

2621
/** @method \MongoDB\Laravel\Query\Builder toBase() */
2722
class Builder extends EloquentBuilder
2823
{
24+
private const DUPLICATE_KEY_ERROR = 11000;
2925
use QueriesRelationships;
3026

3127
/**
@@ -202,56 +198,37 @@ public function raw($value = null)
202198
return $results;
203199
}
204200

205-
/**
206-
* Attempt to create the record if it does not exist with the matching attributes.
207-
* If the record exists, it will be returned.
208-
*
209-
* @param array $attributes The attributes to check for duplicate records
210-
* @param array $values The attributes to insert if no matching record is found
211-
*/
212-
public function createOrFirst(array $attributes = [], array $values = []): Model
201+
public function firstOrCreate(array $attributes = [], array $values = [])
213202
{
214-
if ($attributes === [] || $attributes === ['_id' => null]) {
215-
throw new InvalidArgumentException('You must provide attributes to check for duplicates. Got ' . json_encode($attributes));
203+
$instance = (clone $this)->where($attributes)->first();
204+
if ($instance !== null) {
205+
return $instance;
216206
}
217207

218-
// Apply casting and default values to the attributes
219-
// In case of duplicate key between the attributes and the values, the values have priority
220-
$instance = $this->newModelInstance($values + $attributes);
208+
// createOrFirst is not supported in transaction.
209+
if ($this->getConnection()->getSession()?->isInTransaction()) {
210+
return $this->create(array_merge($attributes, $values));
211+
}
221212

222-
/* @see \Illuminate\Database\Eloquent\Model::performInsert */
223-
if ($instance->usesTimestamps()) {
224-
$instance->updateTimestamps();
213+
return $this->createOrFirst($attributes, $values);
214+
}
215+
216+
public function createOrFirst(array $attributes = [], array $values = [])
217+
{
218+
// The duplicate key error would abort the transaction. Using the regular firstOrCreate in that case.
219+
if ($this->getConnection()->getSession()?->isInTransaction()) {
220+
return $this->firstOrCreate($attributes, $values);
225221
}
226222

227-
$values = $instance->getAttributes();
228-
$attributes = array_intersect_key($attributes, $values);
229-
230-
return $this->raw(function (Collection $collection) use ($attributes, $values) {
231-
$listener = new FindAndModifyCommandSubscriber();
232-
$collection->getManager()->addSubscriber($listener);
233-
234-
try {
235-
$document = $collection->findOneAndUpdate(
236-
$attributes,
237-
// Before MongoDB 5.0, $setOnInsert requires a non-empty document.
238-
// This should not be an issue as $values includes the query filter.
239-
['$setOnInsert' => (object) $values],
240-
[
241-
'upsert' => true,
242-
'returnDocument' => FindOneAndUpdate::RETURN_DOCUMENT_AFTER,
243-
'typeMap' => ['root' => 'array', 'document' => 'array'],
244-
],
245-
);
246-
} finally {
247-
$collection->getManager()->removeSubscriber($listener);
223+
try {
224+
return $this->create(array_merge($attributes, $values));
225+
} catch (WriteException $e) {
226+
if ($e->getCode() === self::DUPLICATE_KEY_ERROR) {
227+
return $this->where($attributes)->first() ?? throw $e;
248228
}
249229

250-
$model = $this->model->newFromBuilder($document);
251-
$model->wasRecentlyCreated = $listener->created;
252-
253-
return $model;
254-
});
230+
throw $e;
231+
}
255232
}
256233

257234
/**
@@ -277,8 +254,7 @@ protected function addUpdatedAtColumn(array $values)
277254
return $values;
278255
}
279256

280-
/** @return ConnectionInterface */
281-
public function getConnection()
257+
public function getConnection(): Connection
282258
{
283259
return $this->query->getConnection();
284260
}

Diff for: ‎src/Internal/FindAndModifyCommandSubscriber.php

-34
This file was deleted.

Diff for: ‎tests/ModelTest.php

+59-17
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,8 @@
1010
use Illuminate\Database\Eloquent\Collection as EloquentCollection;
1111
use Illuminate\Database\Eloquent\ModelNotFoundException;
1212
use Illuminate\Support\Facades\Date;
13+
use Illuminate\Support\Facades\DB;
1314
use Illuminate\Support\Str;
14-
use InvalidArgumentException;
1515
use MongoDB\BSON\Binary;
1616
use MongoDB\BSON\ObjectID;
1717
use MongoDB\BSON\UTCDateTime;
@@ -48,7 +48,7 @@ class ModelTest extends TestCase
4848
public function tearDown(): void
4949
{
5050
Carbon::setTestNow();
51-
User::truncate();
51+
DB::connection('mongodb')->getCollection('users')->drop();
5252
Soft::truncate();
5353
Book::truncate();
5454
Item::truncate();
@@ -1048,19 +1048,34 @@ public function testNumericFieldName(): void
10481048
$this->assertEquals([3 => 'two.three'], $found[2]);
10491049
}
10501050

1051-
public function testCreateOrFirst()
1051+
#[TestWith([true])]
1052+
#[TestWith([false])]
1053+
public function testCreateOrFirst(bool $transaction)
10521054
{
1055+
$connection = DB::connection('mongodb');
1056+
$connection
1057+
->getCollection('users')
1058+
->createIndex(['email' => 1], ['unique' => true]);
1059+
1060+
if ($transaction) {
1061+
$connection->beginTransaction();
1062+
}
1063+
10531064
Carbon::setTestNow('2010-06-22');
10541065
$createdAt = Carbon::now()->getTimestamp();
1066+
$events = [];
1067+
self::registerModelEvents(User::class, $events);
10551068
$user1 = User::createOrFirst(['email' => '[email protected]']);
10561069

10571070
$this->assertSame('[email protected]', $user1->email);
10581071
$this->assertNull($user1->name);
10591072
$this->assertTrue($user1->wasRecentlyCreated);
10601073
$this->assertEquals($createdAt, $user1->created_at->getTimestamp());
10611074
$this->assertEquals($createdAt, $user1->updated_at->getTimestamp());
1075+
$this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
10621076

10631077
Carbon::setTestNow('2020-12-28');
1078+
$events = [];
10641079
$user2 = User::createOrFirst(
10651080
['email' => '[email protected]'],
10661081
['name' => 'John Doe', 'birthday' => new DateTime('1987-05-28')],
@@ -1073,7 +1088,17 @@ public function testCreateOrFirst()
10731088
$this->assertFalse($user2->wasRecentlyCreated);
10741089
$this->assertEquals($createdAt, $user1->created_at->getTimestamp());
10751090
$this->assertEquals($createdAt, $user1->updated_at->getTimestamp());
1091+
if ($transaction) {
1092+
// In a transaction, firstOrCreate is used instead.
1093+
// Since a document is found, "save" is not called.
1094+
$this->assertEquals([], $events);
1095+
} else {
1096+
// The "duplicate key error" exception interrupts the save process
1097+
// before triggering "created" and "saved". Consistent with Laravel
1098+
$this->assertEquals(['saving', 'creating'], $events);
1099+
}
10761100

1101+
$events = [];
10771102
$user3 = User::createOrFirst(
10781103
['email' => '[email protected]'],
10791104
['name' => 'Jane Doe', 'birthday' => new DateTime('1987-05-28')],
@@ -1086,21 +1111,21 @@ public function testCreateOrFirst()
10861111
$this->assertTrue($user3->wasRecentlyCreated);
10871112
$this->assertEquals($createdAt, $user1->created_at->getTimestamp());
10881113
$this->assertEquals($createdAt, $user1->updated_at->getTimestamp());
1114+
$this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
10891115

1116+
$events = [];
10901117
$user4 = User::createOrFirst(
10911118
['name' => 'Robert Doe'],
10921119
['name' => 'Maria Doe', 'email' => '[email protected]'],
10931120
);
10941121

10951122
$this->assertSame('Maria Doe', $user4->name);
10961123
$this->assertTrue($user4->wasRecentlyCreated);
1097-
}
1124+
$this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
10981125

1099-
public function testCreateOrFirstRequiresFilter()
1100-
{
1101-
$this->expectException(InvalidArgumentException::class);
1102-
$this->expectExceptionMessage('You must provide attributes to check for duplicates');
1103-
User::createOrFirst([]);
1126+
if ($transaction) {
1127+
$connection->commit();
1128+
}
11041129
}
11051130

11061131
#[TestWith([['_id' => new ObjectID()]])]
@@ -1116,6 +1141,8 @@ public function testUpdateOrCreate(array $criteria)
11161141

11171142
Carbon::setTestNow('2010-01-01');
11181143
$createdAt = Carbon::now()->getTimestamp();
1144+
$events = [];
1145+
self::registerModelEvents(User::class, $events);
11191146

11201147
// Create
11211148
$user = User::updateOrCreate(
@@ -1127,11 +1154,12 @@ public function testUpdateOrCreate(array $criteria)
11271154
$this->assertEquals(new DateTime('1987-05-28'), $user->birthday);
11281155
$this->assertEquals($createdAt, $user->created_at->getTimestamp());
11291156
$this->assertEquals($createdAt, $user->updated_at->getTimestamp());
1130-
1157+
$this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
11311158
Carbon::setTestNow('2010-02-01');
11321159
$updatedAt = Carbon::now()->getTimestamp();
11331160

11341161
// Update
1162+
$events = [];
11351163
$user = User::updateOrCreate(
11361164
$criteria,
11371165
['birthday' => new DateTime('1990-01-12'), 'foo' => 'bar'],
@@ -1142,6 +1170,7 @@ public function testUpdateOrCreate(array $criteria)
11421170
$this->assertEquals(new DateTime('1990-01-12'), $user->birthday);
11431171
$this->assertEquals($createdAt, $user->created_at->getTimestamp());
11441172
$this->assertEquals($updatedAt, $user->updated_at->getTimestamp());
1173+
$this->assertEquals(['saving', 'updating', 'updated', 'saved'], $events);
11451174

11461175
// Stored data
11471176
$checkUser = User::where($criteria)->first();
@@ -1159,13 +1188,26 @@ public function testCreateWithNullId()
11591188
$this->assertSame(1, User::count());
11601189
}
11611190

1162-
public function testUpdateOrCreateWithNullId()
1191+
/** @param class-string<Model> $modelClass */
1192+
private static function registerModelEvents(string $modelClass, array &$events): void
11631193
{
1164-
$this->expectException(InvalidArgumentException::class);
1165-
$this->expectExceptionMessage('You must provide attributes to check for duplicates');
1166-
User::updateOrCreate(
1167-
['_id' => null],
1168-
['email' => '[email protected]'],
1169-
);
1194+
$modelClass::creating(function () use (&$events) {
1195+
$events[] = 'creating';
1196+
});
1197+
$modelClass::created(function () use (&$events) {
1198+
$events[] = 'created';
1199+
});
1200+
$modelClass::updating(function () use (&$events) {
1201+
$events[] = 'updating';
1202+
});
1203+
$modelClass::updated(function () use (&$events) {
1204+
$events[] = 'updated';
1205+
});
1206+
$modelClass::saving(function () use (&$events) {
1207+
$events[] = 'saving';
1208+
});
1209+
$modelClass::saved(function () use (&$events) {
1210+
$events[] = 'saved';
1211+
});
11701212
}
11711213
}

0 commit comments

Comments
 (0)