From 1fbb798d37cbb487c24d3be1ada8f037b199ad07 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?J=C3=A9r=C3=B4me=20Tamarelle?= Date: Wed, 23 Aug 2023 15:56:31 +0200 Subject: [PATCH] PHPORM-75 Defer Model::unset($field) to the save() --- CHANGELOG.md | 2 ++ src/Eloquent/Model.php | 65 ++++++++++++++++++++++++++++++++++---- src/Query/Builder.php | 9 ++++-- tests/ModelTest.php | 51 ++++++++++++++++++++++++++++++ tests/QueryBuilderTest.php | 32 +++++++++++++++++++ 5 files changed, 150 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 60c10feb8..c9bad9a96 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,8 @@ All notable changes to this project will be documented in this file. - Fix validation of unique values when the validated value is found as part of an existing value. [#21](https://github.com/GromNaN/laravel-mongodb/pull/21) by [@GromNaN](https://github.com/GromNaN). - 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). +- `Query\Builder::update()` accepts a mix of field to set and `$`-prefixed operators +- `Model::unset()` does not persist the change anymore. Call `Model::save()` to persist the change. ## [3.9.2] - 2022-09-01 diff --git a/src/Eloquent/Model.php b/src/Eloquent/Model.php index ff7f9a175..5d05d2db0 100644 --- a/src/Eloquent/Model.php +++ b/src/Eloquent/Model.php @@ -20,6 +20,9 @@ use MongoDB\BSON\UTCDateTime; use function uniqid; +/** + * @method void unset(string|string[] $columns) Remove one or more fields. + */ abstract class Model extends BaseModel { use HybridRelations, EmbedsRelations; @@ -52,6 +55,8 @@ abstract class Model extends BaseModel */ protected $parentRelation; + private array $unset = []; + /** * Custom accessor for the model's id. * @@ -151,7 +156,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 +216,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 +252,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 +276,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,11 +308,34 @@ 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 */ public function drop($columns) { @@ -289,9 +345,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); } /** diff --git a/src/Query/Builder.php b/src/Query/Builder.php index a65edd24a..7983a998b 100644 --- a/src/Query/Builder.php +++ b/src/Query/Builder.php @@ -605,9 +605,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..a8184d76f 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,56 @@ 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 + unset($user['note1']); + $this->assertFalse(isset($user->note1)); + $this->assertNull($user['note1']); + $this->assertFalse($user->originalIsEquivalent('note1')); + $this->assertSame(['$unset' => ['note1' => true]], $user->getDirty()); + + // Reset the previous value + $user->note1 = 'ABC'; + $this->assertTrue($user->originalIsEquivalent('note1')); + $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->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')); } 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([