PHPORM-75 Defer Model::unset($field) to the save()

CHANGELOG.md

@@ -18,6 +18,7 @@ All notable changes to this project will be documented in this file.
 - Support `%` and `_` in `like` expression [#17](https://github.com/GromNaN/laravel-mongodb/pull/17) by [@GromNaN](https://github.com/GromNaN).
 - Change signature of `Query\Builder::__constructor` to match the parent class [#26](https://github.com/GromNaN/laravel-mongodb-private/pull/26) by [@GromNaN](https://github.com/GromNaN).
 - Fix Query on `whereDate`, `whereDay`, `whereMonth`, `whereYear`, `whereTime` to use MongoDB operators [#2570](https://github.com/jenssegers/laravel-mongodb/pull/2376) by [@Davpyu](https://github.com/Davpyu) and [@GromNaN](https://github.com/GromNaN).
+- `Model::unset()` does not persist the change. Call `Model::save()` to persist the change [#2578](https://github.com/jenssegers/laravel-mongodb/pull/2578) by [@GromNaN](https://github.com/GromNaN).
 
 ## [3.9.2] - 2022-09-01
 

src/Eloquent/Model.php

@@ -52,6 +52,13 @@ abstract class Model extends BaseModel
      */
     protected $parentRelation;
 
+    /**
+     * List of field names to unset from the document on save.
+     *
+     * @var array{string, true}
+     */
+    private array $unset = [];
+
     /**
      * Custom accessor for the model's id.
      *
@@ -151,7 +158,12 @@ public function getTable()
     public function getAttribute($key)
     {
         if (! $key) {
-            return;
+            return null;
+        }
+
+        // An unset attribute is null or throw an exception.
+        if (isset($this->unset[$key])) {
+            return $this->throwMissingAttributeExceptionIfApplicable($key);
         }
 
         // Dot notation support.
@@ -206,6 +218,9 @@ public function setAttribute($key, $value)
             return $this;
         }
 
+        // Setting an attribute cancels the unset operation.
+        unset($this->unset[$key]);
+
         return parent::setAttribute($key, $value);
     }
 
@@ -239,6 +254,21 @@ public function getCasts()
         return $this->casts;
     }
 
+    /**
+     * @inheritdoc
+     */
+    public function getDirty()
+    {
+        $dirty = parent::getDirty();
+
+        // The specified value in the $unset expression does not impact the operation.
+        if (! empty($this->unset)) {
+            $dirty['$unset'] = $this->unset;
+        }
+
+        return $dirty;
+    }
+
     /**
      * @inheritdoc
      */
@@ -248,6 +278,11 @@ public function originalIsEquivalent($key)
             return false;
         }
 
+        // Calling unset on an attribute marks it as "not equivalent".
+        if (isset($this->unset[$key])) {
+            return false;
+        }
+
         $attribute = Arr::get($this->attributes, $key);
         $original = Arr::get($this->original, $key);
 
@@ -275,23 +310,56 @@ public function originalIsEquivalent($key)
             && strcmp((string) $attribute, (string) $original) === 0;
     }
 
+    /**
+     * @inheritdoc
+     */
+    public function offsetUnset($offset): void
+    {
+        parent::offsetUnset($offset);
+
+        // Force unsetting even if the attribute is not set.
+        // End user can optimize DB calls by checking if the attribute is set before unsetting it.
+        $this->unset[$offset] = true;
+    }
+
+    /**
+     * @inheritdoc
+     */
+    public function offsetSet($offset, $value): void
+    {
+        parent::offsetSet($offset, $value);
+
+        // Setting an attribute cancels the unset operation.
+        unset($this->unset[$offset]);
+    }
+
     /**
      * Remove one or more fields.
      *
-     * @param  mixed  $columns
-     * @return int
+     * @param  string|string[]  $columns
+     * @return void
+     *
+     * @deprecated Use unset() instead.
      */
     public function drop($columns)
+    {
+        $this->unset($columns);
+    }
+
+    /**
+     * Remove one or more fields.
+     *
+     * @param  string|string[]  $columns
+     * @return void
+     */
+    public function unset($columns)
     {
         $columns = Arr::wrap($columns);
 
         // Unset attributes
         foreach ($columns as $column) {
             $this->__unset($column);
         }
-
-        // Perform unset only on current document
-        return $this->newQuery()->where($this->getKeyName(), $this->getKey())->unset($columns);
     }
 
     /**
@@ -502,19 +570,6 @@ protected function isGuardableColumn($key)
         return true;
     }
 
-    /**
-     * @inheritdoc
-     */
-    public function __call($method, $parameters)
-    {
-        // Unset method
-        if ($method == 'unset') {
-            return $this->drop(...$parameters);
-        }
-
-        return parent::__call($method, $parameters);
-    }
-
     /**
      * @inheritdoc
      */

src/Query/Builder.php

@@ -607,9 +607,12 @@ public function insertGetId(array $values, $sequence = null)
      */
     public function update(array $values, array $options = [])
     {
-        // Use $set as default operator.
-        if (! str_starts_with(key($values), '$')) {
-            $values = ['$set' => $values];
+        // Use $set as default operator for field names that are not in an operator
+        foreach ($values as $key => $value) {
+            if (! str_starts_with($key, '$')) {
+                $values['$set'][$key] = $value;
+                unset($values[$key]);
+            }
         }
 
         $options = $this->inheritConnectionOptions($options);

tests/ModelTest.php

@@ -473,6 +473,10 @@ public function testUnset(): void
 
         $user1->unset('note1');
 
+        $this->assertFalse(isset($user1->note1));
+
+        $user1->save();
+
         $this->assertFalse(isset($user1->note1));
         $this->assertTrue(isset($user1->note2));
         $this->assertTrue(isset($user2->note1));
@@ -488,9 +492,60 @@ public function testUnset(): void
         $this->assertTrue(isset($user2->note2));
 
         $user2->unset(['note1', 'note2']);
+        $user2->save();
 
         $this->assertFalse(isset($user2->note1));
         $this->assertFalse(isset($user2->note2));
+
+        // Re-re-fetch to be sure
+        $user2 = User::find($user2->_id);
+
+        $this->assertFalse(isset($user2->note1));
+        $this->assertFalse(isset($user2->note2));
+    }
+
+    public function testUnsetAndSet(): void
+    {
+        $user = User::create(['name' => 'John Doe', 'note1' => 'ABC', 'note2' => 'DEF']);
+
+        $this->assertTrue($user->originalIsEquivalent('note1'));
+
+        // Unset the value
+        $user->unset('note1');
+        $this->assertFalse(isset($user->note1));
+        $this->assertNull($user['note1']);
+        $this->assertFalse($user->originalIsEquivalent('note1'));
+        $this->assertTrue($user->isDirty());
+        $this->assertSame(['$unset' => ['note1' => true]], $user->getDirty());
+
+        // Reset the previous value
+        $user->note1 = 'ABC';
+        $this->assertTrue($user->originalIsEquivalent('note1'));
+        $this->assertFalse($user->isDirty());
+        $this->assertSame([], $user->getDirty());
+
+        // Change the value
+        $user->note1 = 'GHI';
+        $this->assertTrue(isset($user->note1));
+        $this->assertSame('GHI', $user['note1']);
+        $this->assertFalse($user->originalIsEquivalent('note1'));
+        $this->assertTrue($user->isDirty());
+        $this->assertSame(['note1' => 'GHI'], $user->getDirty());
+
+        // Fetch to be sure the changes are not persisted yet
+        $userCheck = User::find($user->_id);
+        $this->assertSame('ABC', $userCheck['note1']);
+
+        // Persist the changes
+        $user->save();
+
+        // Re-fetch to be sure
+        $user = User::find($user->_id);
+
+        $this->assertTrue(isset($user->note1));
+        $this->assertSame('GHI', $user->note1);
+        $this->assertTrue($user->originalIsEquivalent('note1'));
+        $this->assertFalse($user->isDirty());
     }
 
     public function testDates(): void

tests/QueryBuilderTest.php

@@ -203,6 +203,38 @@ public function testUpdate()
         $this->assertEquals(20, $jane['age']);
     }
 
+    public function testUpdateOperators()
+    {
+        DB::collection('users')->insert([
+            ['name' => 'Jane Doe', 'age' => 20],
+            ['name' => 'John Doe', 'age' => 19],
+        ]);
+
+        DB::collection('users')->where('name', 'John Doe')->update(
+            [
+                '$unset' => ['age' => 1],
+                'ageless' => true,
+            ]
+        );
+        DB::collection('users')->where('name', 'Jane Doe')->update(
+            [
+                '$inc' => ['age' => 1],
+                '$set' => ['pronoun' => 'she'],
+                'ageless' => false,
+            ]
+        );
+
+        $john = DB::collection('users')->where('name', 'John Doe')->first();
+        $jane = DB::collection('users')->where('name', 'Jane Doe')->first();
+
+        $this->assertArrayNotHasKey('age', $john);
+        $this->assertTrue($john['ageless']);
+
+        $this->assertEquals(21, $jane['age']);
+        $this->assertEquals('she', $jane['pronoun']);
+        $this->assertFalse($jane['ageless']);
+    }
+
     public function testDelete()
     {
         DB::collection('users')->insert([