diff --git a/CHANGELOG.md b/CHANGELOG.md index bb318f301..978523fe2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ All notable changes to this project will be documented in this file. * Add `mongodb` driver for Batching by @GromNaN in [#2904](https://github.com/mongodb/laravel-mongodb/pull/2904) * Rename queue option `table` to `collection` * Replace queue option `expire` with `retry_after` +* Revert behavior of `createOrFirst` to delegate to `firstOrCreate` when in transaction by @GromNaN in [#2984](https://github.com/mongodb/laravel-mongodb/pull/2984) ## [4.3.1] diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php index 678e7095b..da96b64f1 100644 --- a/src/Eloquent/Builder.php +++ b/src/Eloquent/Builder.php @@ -4,28 +4,24 @@ namespace MongoDB\Laravel\Eloquent; -use Illuminate\Database\ConnectionInterface; use Illuminate\Database\Eloquent\Builder as EloquentBuilder; -use InvalidArgumentException; use MongoDB\Driver\Cursor; -use MongoDB\Laravel\Collection; +use MongoDB\Driver\Exception\WriteException; +use MongoDB\Laravel\Connection; use MongoDB\Laravel\Helpers\QueriesRelationships; -use MongoDB\Laravel\Internal\FindAndModifyCommandSubscriber; use MongoDB\Laravel\Query\AggregationBuilder; use MongoDB\Model\BSONDocument; -use MongoDB\Operation\FindOneAndUpdate; -use function array_intersect_key; use function array_key_exists; use function array_merge; use function collect; use function is_array; use function iterator_to_array; -use function json_encode; /** @method \MongoDB\Laravel\Query\Builder toBase() */ class Builder extends EloquentBuilder { + private const DUPLICATE_KEY_ERROR = 11000; use QueriesRelationships; /** @@ -202,56 +198,37 @@ public function raw($value = null) return $results; } - /** - * Attempt to create the record if it does not exist with the matching attributes. - * If the record exists, it will be returned. - * - * @param array $attributes The attributes to check for duplicate records - * @param array $values The attributes to insert if no matching record is found - */ - public function createOrFirst(array $attributes = [], array $values = []): Model + public function firstOrCreate(array $attributes = [], array $values = []) { - if ($attributes === [] || $attributes === ['_id' => null]) { - throw new InvalidArgumentException('You must provide attributes to check for duplicates. Got ' . json_encode($attributes)); + $instance = (clone $this)->where($attributes)->first(); + if ($instance !== null) { + return $instance; } - // Apply casting and default values to the attributes - // In case of duplicate key between the attributes and the values, the values have priority - $instance = $this->newModelInstance($values + $attributes); + // createOrFirst is not supported in transaction. + if ($this->getConnection()->getSession()?->isInTransaction()) { + return $this->create(array_merge($attributes, $values)); + } - /* @see \Illuminate\Database\Eloquent\Model::performInsert */ - if ($instance->usesTimestamps()) { - $instance->updateTimestamps(); + return $this->createOrFirst($attributes, $values); + } + + public function createOrFirst(array $attributes = [], array $values = []) + { + // The duplicate key error would abort the transaction. Using the regular firstOrCreate in that case. + if ($this->getConnection()->getSession()?->isInTransaction()) { + return $this->firstOrCreate($attributes, $values); } - $values = $instance->getAttributes(); - $attributes = array_intersect_key($attributes, $values); - - return $this->raw(function (Collection $collection) use ($attributes, $values) { - $listener = new FindAndModifyCommandSubscriber(); - $collection->getManager()->addSubscriber($listener); - - try { - $document = $collection->findOneAndUpdate( - $attributes, - // Before MongoDB 5.0, $setOnInsert requires a non-empty document. - // This should not be an issue as $values includes the query filter. - ['$setOnInsert' => (object) $values], - [ - 'upsert' => true, - 'returnDocument' => FindOneAndUpdate::RETURN_DOCUMENT_AFTER, - 'typeMap' => ['root' => 'array', 'document' => 'array'], - ], - ); - } finally { - $collection->getManager()->removeSubscriber($listener); + try { + return $this->create(array_merge($attributes, $values)); + } catch (WriteException $e) { + if ($e->getCode() === self::DUPLICATE_KEY_ERROR) { + return $this->where($attributes)->first() ?? throw $e; } - $model = $this->model->newFromBuilder($document); - $model->wasRecentlyCreated = $listener->created; - - return $model; - }); + throw $e; + } } /** @@ -277,8 +254,7 @@ protected function addUpdatedAtColumn(array $values) return $values; } - /** @return ConnectionInterface */ - public function getConnection() + public function getConnection(): Connection { return $this->query->getConnection(); } diff --git a/src/Internal/FindAndModifyCommandSubscriber.php b/src/Internal/FindAndModifyCommandSubscriber.php deleted file mode 100644 index 335e05562..000000000 --- a/src/Internal/FindAndModifyCommandSubscriber.php +++ /dev/null @@ -1,34 +0,0 @@ -created = ! $event->getReply()->lastErrorObject->updatedExisting; - } -} diff --git a/tests/ModelTest.php b/tests/ModelTest.php index ead5847ca..73374ce57 100644 --- a/tests/ModelTest.php +++ b/tests/ModelTest.php @@ -10,8 +10,8 @@ use Illuminate\Database\Eloquent\Collection as EloquentCollection; use Illuminate\Database\Eloquent\ModelNotFoundException; use Illuminate\Support\Facades\Date; +use Illuminate\Support\Facades\DB; use Illuminate\Support\Str; -use InvalidArgumentException; use MongoDB\BSON\Binary; use MongoDB\BSON\ObjectID; use MongoDB\BSON\UTCDateTime; @@ -48,7 +48,7 @@ class ModelTest extends TestCase public function tearDown(): void { Carbon::setTestNow(); - User::truncate(); + DB::connection('mongodb')->getCollection('users')->drop(); Soft::truncate(); Book::truncate(); Item::truncate(); @@ -1048,10 +1048,23 @@ public function testNumericFieldName(): void $this->assertEquals([3 => 'two.three'], $found[2]); } - public function testCreateOrFirst() + #[TestWith([true])] + #[TestWith([false])] + public function testCreateOrFirst(bool $transaction) { + $connection = DB::connection('mongodb'); + $connection + ->getCollection('users') + ->createIndex(['email' => 1], ['unique' => true]); + + if ($transaction) { + $connection->beginTransaction(); + } + Carbon::setTestNow('2010-06-22'); $createdAt = Carbon::now()->getTimestamp(); + $events = []; + self::registerModelEvents(User::class, $events); $user1 = User::createOrFirst(['email' => 'john.doe@example.com']); $this->assertSame('john.doe@example.com', $user1->email); @@ -1059,8 +1072,10 @@ public function testCreateOrFirst() $this->assertTrue($user1->wasRecentlyCreated); $this->assertEquals($createdAt, $user1->created_at->getTimestamp()); $this->assertEquals($createdAt, $user1->updated_at->getTimestamp()); + $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events); Carbon::setTestNow('2020-12-28'); + $events = []; $user2 = User::createOrFirst( ['email' => 'john.doe@example.com'], ['name' => 'John Doe', 'birthday' => new DateTime('1987-05-28')], @@ -1073,7 +1088,17 @@ public function testCreateOrFirst() $this->assertFalse($user2->wasRecentlyCreated); $this->assertEquals($createdAt, $user1->created_at->getTimestamp()); $this->assertEquals($createdAt, $user1->updated_at->getTimestamp()); + if ($transaction) { + // In a transaction, firstOrCreate is used instead. + // Since a document is found, "save" is not called. + $this->assertEquals([], $events); + } else { + // The "duplicate key error" exception interrupts the save process + // before triggering "created" and "saved". Consistent with Laravel + $this->assertEquals(['saving', 'creating'], $events); + } + $events = []; $user3 = User::createOrFirst( ['email' => 'jane.doe@example.com'], ['name' => 'Jane Doe', 'birthday' => new DateTime('1987-05-28')], @@ -1086,7 +1111,9 @@ public function testCreateOrFirst() $this->assertTrue($user3->wasRecentlyCreated); $this->assertEquals($createdAt, $user1->created_at->getTimestamp()); $this->assertEquals($createdAt, $user1->updated_at->getTimestamp()); + $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events); + $events = []; $user4 = User::createOrFirst( ['name' => 'Robert Doe'], ['name' => 'Maria Doe', 'email' => 'maria.doe@example.com'], @@ -1094,13 +1121,11 @@ public function testCreateOrFirst() $this->assertSame('Maria Doe', $user4->name); $this->assertTrue($user4->wasRecentlyCreated); - } + $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events); - public function testCreateOrFirstRequiresFilter() - { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('You must provide attributes to check for duplicates'); - User::createOrFirst([]); + if ($transaction) { + $connection->commit(); + } } #[TestWith([['_id' => new ObjectID()]])] @@ -1116,6 +1141,8 @@ public function testUpdateOrCreate(array $criteria) Carbon::setTestNow('2010-01-01'); $createdAt = Carbon::now()->getTimestamp(); + $events = []; + self::registerModelEvents(User::class, $events); // Create $user = User::updateOrCreate( @@ -1127,11 +1154,12 @@ public function testUpdateOrCreate(array $criteria) $this->assertEquals(new DateTime('1987-05-28'), $user->birthday); $this->assertEquals($createdAt, $user->created_at->getTimestamp()); $this->assertEquals($createdAt, $user->updated_at->getTimestamp()); - + $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events); Carbon::setTestNow('2010-02-01'); $updatedAt = Carbon::now()->getTimestamp(); // Update + $events = []; $user = User::updateOrCreate( $criteria, ['birthday' => new DateTime('1990-01-12'), 'foo' => 'bar'], @@ -1142,6 +1170,7 @@ public function testUpdateOrCreate(array $criteria) $this->assertEquals(new DateTime('1990-01-12'), $user->birthday); $this->assertEquals($createdAt, $user->created_at->getTimestamp()); $this->assertEquals($updatedAt, $user->updated_at->getTimestamp()); + $this->assertEquals(['saving', 'updating', 'updated', 'saved'], $events); // Stored data $checkUser = User::where($criteria)->first(); @@ -1159,13 +1188,26 @@ public function testCreateWithNullId() $this->assertSame(1, User::count()); } - public function testUpdateOrCreateWithNullId() + /** @param class-string $modelClass */ + private static function registerModelEvents(string $modelClass, array &$events): void { - $this->expectException(InvalidArgumentException::class); - $this->expectExceptionMessage('You must provide attributes to check for duplicates'); - User::updateOrCreate( - ['_id' => null], - ['email' => 'jane.doe@example.com'], - ); + $modelClass::creating(function () use (&$events) { + $events[] = 'creating'; + }); + $modelClass::created(function () use (&$events) { + $events[] = 'created'; + }); + $modelClass::updating(function () use (&$events) { + $events[] = 'updating'; + }); + $modelClass::updated(function () use (&$events) { + $events[] = 'updated'; + }); + $modelClass::saving(function () use (&$events) { + $events[] = 'saving'; + }); + $modelClass::saved(function () use (&$events) { + $events[] = 'saved'; + }); } }