diff --git a/CHANGELOG.md b/CHANGELOG.md index fc10adb59..3dbdee597 100644 --- a/CHANGELOG.md +++ b/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 diff --git a/src/Eloquent/Model.php b/src/Eloquent/Model.php index ff7f9a175..4e118c46f 100644 --- a/src/Eloquent/Model.php +++ b/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,13 +310,49 @@ 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); @@ -289,9 +360,6 @@ public function drop($columns) 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 */ diff --git a/src/Query/Builder.php b/src/Query/Builder.php index 682c70c19..dce8ee2a4 100644 --- a/src/Query/Builder.php +++ b/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); diff --git a/tests/ModelTest.php b/tests/ModelTest.php index 1fe71f266..75dfaf4bf 100644 --- a/tests/ModelTest.php +++ b/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 diff --git a/tests/QueryBuilderTest.php b/tests/QueryBuilderTest.php index 5dbc67cc2..11817018a 100644 --- a/tests/QueryBuilderTest.php +++ b/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([