From 990612e565641ac8f74bc49e75860a1623a9d8a7 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Wed, 29 May 2024 12:22:48 +0200
Subject: [PATCH 1/4] PHPORM-180 Keep createOrFirst in 2 commands to simplify
 implementation

---
 src/Eloquent/Builder.php                      | 63 +++----------------
 .../FindAndModifyCommandSubscriber.php        | 34 ----------
 tests/ModelTest.php                           | 51 ++++++++++-----
 3 files changed, 44 insertions(+), 104 deletions(-)
 delete mode 100644 src/Internal/FindAndModifyCommandSubscriber.php

diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php
index 678e7095b..68dc19b65 100644
--- a/src/Eloquent/Builder.php
+++ b/src/Eloquent/Builder.php
@@ -6,26 +6,22 @@
 
 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\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,17 @@ 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 createOrFirst(array $attributes = [], array $values = []): Builder|Model
     {
-        if ($attributes === [] || $attributes === ['_id' => null]) {
-            throw new InvalidArgumentException('You must provide attributes to check for duplicates. Got ' . json_encode($attributes));
-        }
-
-        // 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);
-
-        /* @see \Illuminate\Database\Eloquent\Model::performInsert */
-        if ($instance->usesTimestamps()) {
-            $instance->updateTimestamps();
-        }
-
-        $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;
+        }
     }
 
     /**
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 @@
-<?php
-
-declare(strict_types=1);
-
-namespace MongoDB\Laravel\Internal;
-
-use MongoDB\Driver\Monitoring\CommandFailedEvent;
-use MongoDB\Driver\Monitoring\CommandStartedEvent;
-use MongoDB\Driver\Monitoring\CommandSubscriber;
-use MongoDB\Driver\Monitoring\CommandSucceededEvent;
-
-/**
- * Track findAndModify command events to detect when a document is inserted or
- * updated.
- *
- * @internal
- */
-final class FindAndModifyCommandSubscriber implements CommandSubscriber
-{
-    public bool $created;
-
-    public function commandFailed(CommandFailedEvent $event): void
-    {
-    }
-
-    public function commandStarted(CommandStartedEvent $event): void
-    {
-    }
-
-    public function commandSucceeded(CommandSucceededEvent $event): void
-    {
-        $this->created = ! $event->getReply()->lastErrorObject->updatedExisting;
-    }
-}
diff --git a/tests/ModelTest.php b/tests/ModelTest.php
index ead5847ca..527576321 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();
@@ -1050,8 +1050,14 @@ public function testNumericFieldName(): void
 
     public function testCreateOrFirst()
     {
+        DB::connection('mongodb')
+            ->getCollection('users')
+            ->createIndex(['email' => 1], ['unique' => true]);
+
         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 +1065,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 +1081,9 @@ public function testCreateOrFirst()
         $this->assertFalse($user2->wasRecentlyCreated);
         $this->assertEquals($createdAt, $user1->created_at->getTimestamp());
         $this->assertEquals($createdAt, $user1->updated_at->getTimestamp());
+        $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 +1096,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 +1106,7 @@ public function testCreateOrFirst()
 
         $this->assertSame('Maria Doe', $user4->name);
         $this->assertTrue($user4->wasRecentlyCreated);
-    }
-
-    public function testCreateOrFirstRequiresFilter()
-    {
-        $this->expectException(InvalidArgumentException::class);
-        $this->expectExceptionMessage('You must provide attributes to check for duplicates');
-        User::createOrFirst([]);
+        $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
     }
 
     #[TestWith([['_id' => new ObjectID()]])]
@@ -1116,6 +1122,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 +1135,13 @@ 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);
+        $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'],
@@ -1159,13 +1169,20 @@ public function testCreateWithNullId()
         $this->assertSame(1, User::count());
     }
 
-    public function testUpdateOrCreateWithNullId()
+    /** @param class-string<Model> $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::saving(function () use (&$events) {
+            $events[] = 'saving';
+        });
+        $modelClass::saved(function () use (&$events) {
+            $events[] = 'saved';
+        });
     }
 }

From b082fd1d275d7067effaaf8bed468257b5494bd1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Wed, 29 May 2024 16:10:40 +0200
Subject: [PATCH 2/4] createOrFirst in transaction fallback to firstOrCreate

---
 src/Eloquent/Builder.php | 31 ++++++++++++++++++++++++++++---
 tests/ModelTest.php      | 29 +++++++++++++++++++++++++----
 2 files changed, 53 insertions(+), 7 deletions(-)

diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php
index 68dc19b65..7a96ee500 100644
--- a/src/Eloquent/Builder.php
+++ b/src/Eloquent/Builder.php
@@ -4,10 +4,10 @@
 
 namespace MongoDB\Laravel\Eloquent;
 
-use Illuminate\Database\ConnectionInterface;
 use Illuminate\Database\Eloquent\Builder as EloquentBuilder;
 use MongoDB\Driver\Cursor;
 use MongoDB\Driver\Exception\WriteException;
+use MongoDB\Laravel\Connection;
 use MongoDB\Laravel\Helpers\QueriesRelationships;
 use MongoDB\Laravel\Query\AggregationBuilder;
 use MongoDB\Model\BSONDocument;
@@ -198,8 +198,34 @@ public function raw($value = null)
         return $results;
     }
 
+    /**
+     * Get the first record matching the attributes. If the record is not found, create it.
+     *
+     * @param  array $attributes
+     * @param  array $values
+     */
+    public function firstOrCreate(array $attributes = [], array $values = []): Model|Builder
+    {
+        $instance = (clone $this)->where($attributes)->first();
+        if ($instance !== null) {
+            return $instance;
+        }
+
+        // createOrFirst is not supported in transaction.
+        if ($this->getConnection()->getSession()?->isInTransaction()) {
+            return $this->create(array_merge($attributes, $values));
+        }
+
+        return $this->createOrFirst($attributes, $values);
+    }
+
     public function createOrFirst(array $attributes = [], array $values = []): Builder|Model
     {
+        // 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);
+        }
+
         try {
             return $this->create(array_merge($attributes, $values));
         } catch (WriteException $e) {
@@ -234,8 +260,7 @@ protected function addUpdatedAtColumn(array $values)
         return $values;
     }
 
-    /** @return ConnectionInterface */
-    public function getConnection()
+    public function getConnection(): Connection
     {
         return $this->query->getConnection();
     }
diff --git a/tests/ModelTest.php b/tests/ModelTest.php
index 527576321..680e0a714 100644
--- a/tests/ModelTest.php
+++ b/tests/ModelTest.php
@@ -1048,12 +1048,19 @@ public function testNumericFieldName(): void
         $this->assertEquals([3 => 'two.three'], $found[2]);
     }
 
-    public function testCreateOrFirst()
+    #[TestWith([true])]
+    #[TestWith([false])]
+    public function testCreateOrFirst(bool $transaction)
     {
-        DB::connection('mongodb')
+        $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 = [];
@@ -1081,7 +1088,11 @@ public function testCreateOrFirst()
         $this->assertFalse($user2->wasRecentlyCreated);
         $this->assertEquals($createdAt, $user1->created_at->getTimestamp());
         $this->assertEquals($createdAt, $user1->updated_at->getTimestamp());
-        $this->assertEquals(['saving', 'creating'], $events);
+        if ($transaction) {
+            $this->assertEquals([], $events);
+        } else {
+            $this->assertEquals(['saving', 'creating'], $events);
+        }
 
         $events = [];
         $user3 = User::createOrFirst(
@@ -1107,6 +1118,10 @@ public function testCreateOrFirst()
         $this->assertSame('Maria Doe', $user4->name);
         $this->assertTrue($user4->wasRecentlyCreated);
         $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
+
+        if ($transaction) {
+            $connection->commit();
+        }
     }
 
     #[TestWith([['_id' => new ObjectID()]])]
@@ -1136,7 +1151,6 @@ public function testUpdateOrCreate(array $criteria)
         $this->assertEquals($createdAt, $user->created_at->getTimestamp());
         $this->assertEquals($createdAt, $user->updated_at->getTimestamp());
         $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
-        $this->assertEquals(['saving', 'creating', 'created', 'saved'], $events);
         Carbon::setTestNow('2010-02-01');
         $updatedAt = Carbon::now()->getTimestamp();
 
@@ -1152,6 +1166,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();
@@ -1178,6 +1193,12 @@ private static function registerModelEvents(string $modelClass, array &$events):
         $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';
         });

From 19bcd1b8266e0dd7c01ba9fb9c9469cdbc1c4db0 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Thu, 30 May 2024 09:22:15 +0200
Subject: [PATCH 3/4] Add comments

---
 CHANGELOG.md        | 1 +
 tests/ModelTest.php | 4 ++++
 2 files changed, 5 insertions(+)

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/tests/ModelTest.php b/tests/ModelTest.php
index 680e0a714..73374ce57 100644
--- a/tests/ModelTest.php
+++ b/tests/ModelTest.php
@@ -1089,8 +1089,12 @@ public function testCreateOrFirst(bool $transaction)
         $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);
         }
 

From ce612e97b74894a2952eaea224abcde8e0468281 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= <jerome@tamarelle.net>
Date: Thu, 30 May 2024 09:25:37 +0200
Subject: [PATCH 4/4] Remove return types

---
 src/Eloquent/Builder.php | 10 ++--------
 1 file changed, 2 insertions(+), 8 deletions(-)

diff --git a/src/Eloquent/Builder.php b/src/Eloquent/Builder.php
index 7a96ee500..da96b64f1 100644
--- a/src/Eloquent/Builder.php
+++ b/src/Eloquent/Builder.php
@@ -198,13 +198,7 @@ public function raw($value = null)
         return $results;
     }
 
-    /**
-     * Get the first record matching the attributes. If the record is not found, create it.
-     *
-     * @param  array $attributes
-     * @param  array $values
-     */
-    public function firstOrCreate(array $attributes = [], array $values = []): Model|Builder
+    public function firstOrCreate(array $attributes = [], array $values = [])
     {
         $instance = (clone $this)->where($attributes)->first();
         if ($instance !== null) {
@@ -219,7 +213,7 @@ public function firstOrCreate(array $attributes = [], array $values = []): Model
         return $this->createOrFirst($attributes, $values);
     }
 
-    public function createOrFirst(array $attributes = [], array $values = []): Builder|Model
+    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()) {