Merge 2.2.x into 2.3.x (#2350)

* Fix locking when ClassMetadata is unserialized Caching / unserializing ClassMetadata broke locking functionality Fixes #2278 * Test serialization of lock/version fields * Update working-with-objects.rst Detach doc text from code block * Update storage-strategies.rst * Fix invalid strict comparison when validating mappings * Correctly handle write concern specified in defaultCommitOptions (#2294) * Fix documentation for uploadFromFile * Fix mapping of the nullable option for XML driver * Fix query preparation when in elemMatch (#2299) * Fix preparation of $elemMatch operators in queries (#2298) * Fix using null values in partial filter expressions (#2300) * Fix errors with nullable typed associations (#2302) * Fix initialising nullable associations * Fix error when merging documents with uninitialised typed properties * Allow mixed value in $not operator (#2307) * [2.2] Fix builds (#2319) * Fix wrong handling for nullable fields in upsert and update (#2318) * Comprehensively test nullable behaviour for embedOne Co-authored-by: wuchen90 <wu.chen@agriconomie.com> * Fix handling of nullable fields for upsert Co-authored-by: wuchen90 <wu.chen@agriconomie.com> * Fix handling of upserts during scheduling for deletion (#2334) * Fix handling of upserts during scheduling for deletion * Added test * Fix wrong assertion (#2335) This was uncovered by Psalm testing when merging 2.2.x up into 2.3.x. * Remove psalm-baseline.xml Co-authored-by: buffcode <buffcode@users.noreply.github.com> Co-authored-by: Laurens Stötzel <l.stoetzel@meeva.de> Co-authored-by: Maciej Malarz <malarzm@gmail.com> Co-authored-by: jeeiex <78592605+jeeiex@users.noreply.github.com> Co-authored-by: Claudio Zizza <859964+SenseException@users.noreply.github.com> Co-authored-by: Gocha Ossinkine <ossinkine@ya.ru> Co-authored-by: Ryan RAJKOMAR <rrajkomar@users.noreply.github.com> Co-authored-by: wuchen90 <wu.chen@agriconomie.com> Co-authored-by: Fran Moreno <franmomu@gmail.com> Co-authored-by: Bernhard Schussek <bschussek@gmail.com>
doctrine · Aug 5, 2021 · 08b3799 · 08b3799
1 parent a74157a
commit 08b3799
Show file tree

Hide file tree

Showing 16 changed files with 372 additions and 68 deletions.
diff --git a/.github/workflows/coding-standards.yml b/.github/workflows/coding-standards.yml
@@ -50,6 +50,11 @@ jobs:
       - name: "Install dependencies with Composer"
         uses: "ramsey/composer-install@v1"
 
+      - name: "Upload composer.lock as build artifact"
+        uses: actions/upload-artifact@v2
+        with:
+          name: composer.lock
+          path: composer.lock
 
       # https://github.com/doctrine/.github/issues/3
       - name: "Run PHP_CodeSniffer"

diff --git a/.github/workflows/continuous-integration.yml b/.github/workflows/continuous-integration.yml
@@ -12,12 +12,10 @@ env:
 jobs:
   phpunit:
     name: "PHPUnit"
-    runs-on: "${{ matrix.os }}"
+    runs-on: "ubuntu-18.04"
 
     strategy:
       matrix:
-        os:
-          - "ubuntu-18.04"
         php-version:
           - "7.2"
           - "7.3"
@@ -35,13 +33,11 @@ jobs:
           - "highest"
         include:
           - deps: "lowest"
-            os: "ubuntu-16.04"
             php-version: "7.2"
             mongodb-version: "3.6"
             driver-version: "1.5.0"
             topology: "server"
           - topology: "sharded_cluster"
-            os: "ubuntu-18.04"
             php-version: "8.0"
             mongodb-version: "4.4"
             driver-version: "stable"
@@ -86,6 +82,12 @@ jobs:
           dependency-versions: "${{ matrix.dependencies }}"
           composer-options: "--prefer-dist"
 
+      - name: "Upload composer.lock as build artifact"
+        uses: actions/upload-artifact@v2
+        with:
+          name: composer.lock
+          path: composer.lock
+
       - id: setup-mongodb
         uses: mongodb-labs/drivers-evergreen-tools@master
         with:

diff --git a/.github/workflows/performance.yml b/.github/workflows/performance.yml
@@ -54,6 +54,12 @@ jobs:
       - name: "Install dependencies with Composer"
         uses: "ramsey/composer-install@v1"
 
+      - name: "Upload composer.lock as build artifact"
+        uses: actions/upload-artifact@v2
+        with:
+          name: composer.lock
+          path: composer.lock
+
       # https://github.com/doctrine/.github/issues/3
       - name: "Run PHP_CodeSniffer"
         run: "vendor/bin/phpbench run --report=default --revs=100 --iterations=5 --report=aggregate"
diff --git a/.github/workflows/static-analysis.yml b/.github/workflows/static-analysis.yml
@@ -49,6 +49,12 @@ jobs:
       - name: "Install dependencies with Composer"
         uses: "ramsey/composer-install@v1"
 
+      - name: "Upload composer.lock as build artifact"
+        uses: actions/upload-artifact@v2
+        with:
+          name: composer.lock
+          path: composer.lock
+
       - name: "Run a static analysis with phpstan/phpstan"
         run: "vendor/bin/phpstan analyse --error-format=github"
 
@@ -75,5 +81,11 @@ jobs:
       - name: "Install dependencies with Composer"
         uses: "ramsey/composer-install@v1"
 
+      - name: "Upload composer.lock as build artifact"
+        uses: actions/upload-artifact@v2
+        with:
+          name: composer.lock
+          path: composer.lock
+
       - name: "Run a static analysis with vimeo/psalm"
         run: "vendor/bin/psalm --show-info=false --stats --output-format=github --threads=$(nproc) --php-version=${{ matrix.php-version }}"
diff --git a/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php b/lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php
@@ -68,14 +68,11 @@ public function prepareInsertData($document)
         foreach ($class->fieldMappings as $mapping) {
             $new = $changeset[$mapping['fieldName']][1] ?? null;
 
-            if ($new === null && $mapping['nullable']) {
-                $insertData[$mapping['name']] = null;
-            }
-
-            /* Nothing more to do for null values, since we're either storing
-             * them (if nullable was true) or not.
-             */
             if ($new === null) {
+                if ($mapping['nullable']) {
+                    $insertData[$mapping['name']] = null;
+                }
+
                 continue;
             }
 
@@ -143,34 +140,36 @@ public function prepareUpdateData($document)
 
             [$old, $new] = $change;
 
+            if ($new === null) {
+                if ($mapping['nullable'] === true) {
+                    $updateData['$set'][$mapping['name']] = null;
+                } else {
+                    $updateData['$unset'][$mapping['name']] = true;
+                }
+
+                continue;
+            }
+
             // Scalar fields
             if (! isset($mapping['association'])) {
-                if ($new === null && $mapping['nullable'] !== true) {
-                    $updateData['$unset'][$mapping['name']] = true;
+                if (isset($mapping['strategy']) && $mapping['strategy'] === ClassMetadata::STORAGE_STRATEGY_INCREMENT) {
+                    $operator = '$inc';
+                    $type     = Type::getType($mapping['type']);
+                    assert($type instanceof Incrementable);
+                    $value = $type->convertToDatabaseValue($type->diff($old, $new));
                 } else {
-                    if ($new !== null && isset($mapping['strategy']) && $mapping['strategy'] === ClassMetadata::STORAGE_STRATEGY_INCREMENT) {
-                        $operator = '$inc';
-                        $type     = Type::getType($mapping['type']);
-                        assert($type instanceof Incrementable);
-                        $value = $type->convertToDatabaseValue($type->diff($old, $new));
-                    } else {
-                        $operator = '$set';
-                        $value    = $new === null ? null : Type::getType($mapping['type'])->convertToDatabaseValue($new);
-                    }
-
-                    $updateData[$operator][$mapping['name']] = $value;
+                    $operator = '$set';
+                    $value    = Type::getType($mapping['type'])->convertToDatabaseValue($new);
                 }
 
+                $updateData[$operator][$mapping['name']] = $value;
+
             // @EmbedOne
             } elseif (isset($mapping['association']) && $mapping['association'] === ClassMetadata::EMBED_ONE) {
                 // If we have a new embedded document then lets set the whole thing
-                if ($new && $this->uow->isScheduledForInsert($new)) {
+                if ($this->uow->isScheduledForInsert($new)) {
                     $updateData['$set'][$mapping['name']] = $this->prepareEmbeddedDocumentValue($mapping, $new);
 
-                // If we don't have a new value then lets unset the embedded document
-                } elseif (! $new) {
-                    $updateData['$unset'][$mapping['name']] = true;
-
                 // Update existing embedded document
                 } else {
                     $update = $this->prepareUpdateData($new);
@@ -182,7 +181,7 @@ public function prepareUpdateData($document)
                 }
 
             // @ReferenceMany, @EmbedMany
-            } elseif (isset($mapping['association']) && $mapping['type'] === ClassMetadata::MANY && $new) {
+            } elseif (isset($mapping['association']) && $mapping['type'] === ClassMetadata::MANY) {
                 if (CollectionHelper::isAtomic($mapping['strategy']) && $this->uow->isCollectionScheduledForUpdate($new)) {
                     $updateData['$set'][$mapping['name']] = $this->prepareAssociatedCollectionValue($new, true);
                 } elseif (CollectionHelper::isAtomic($mapping['strategy']) && $this->uow->isCollectionScheduledForDeletion($new)) {
@@ -208,11 +207,7 @@ public function prepareUpdateData($document)
 
             // @ReferenceOne
             } elseif (isset($mapping['association']) && $mapping['association'] === ClassMetadata::REFERENCE_ONE) {
-                if (isset($new) || $mapping['nullable'] === true) {
-                    $updateData['$set'][$mapping['name']] = $new === null ? null : $this->prepareReferencedDocumentValue($mapping, $new);
-                } else {
-                    $updateData['$unset'][$mapping['name']] = true;
-                }
+                $updateData['$set'][$mapping['name']] = $this->prepareReferencedDocumentValue($mapping, $new);
             }
         }
 
@@ -250,31 +245,36 @@ public function prepareUpsertData($document)
 
             [$old, $new] = $change;
 
+            // Fields with a null value should only be written for inserts
+            if ($new === null) {
+                if ($mapping['nullable'] === true) {
+                    $updateData['$setOnInsert'][$mapping['name']] = null;
+                }
+
+                continue;
+            }
+
             // Scalar fields
             if (! isset($mapping['association'])) {
-                if ($new !== null) {
-                    if (empty($mapping['id']) && isset($mapping['strategy']) && $mapping['strategy'] === ClassMetadata::STORAGE_STRATEGY_INCREMENT) {
-                        $operator = '$inc';
-                        $type     = Type::getType($mapping['type']);
-                        assert($type instanceof Incrementable);
-                        $value = $type->convertToDatabaseValue($type->diff($old, $new));
-                    } else {
-                        $operator = '$set';
-                        $value    = Type::getType($mapping['type'])->convertToDatabaseValue($new);
-                    }
-
-                    $updateData[$operator][$mapping['name']] = $value;
-                } elseif ($mapping['nullable'] === true) {
-                    $updateData['$setOnInsert'][$mapping['name']] = null;
+                if (empty($mapping['id']) && isset($mapping['strategy']) && $mapping['strategy'] === ClassMetadata::STORAGE_STRATEGY_INCREMENT) {
+                    $operator = '$inc';
+                    $type     = Type::getType($mapping['type']);
+                    assert($type instanceof Incrementable);
+                    $value = $type->convertToDatabaseValue($type->diff($old, $new));
+                } else {
+                    $operator = '$set';
+                    $value    = Type::getType($mapping['type'])->convertToDatabaseValue($new);
                 }
 
+                $updateData[$operator][$mapping['name']] = $value;
+
             // @EmbedOne
             } elseif (isset($mapping['association']) && $mapping['association'] === ClassMetadata::EMBED_ONE) {
                 // If we don't have a new value then do nothing on upsert
                 // If we have a new embedded document then lets set the whole thing
-                if ($new && $this->uow->isScheduledForInsert($new)) {
+                if ($this->uow->isScheduledForInsert($new)) {
                     $updateData['$set'][$mapping['name']] = $this->prepareEmbeddedDocumentValue($mapping, $new);
-                } elseif ($new) {
+                } else {
                     // Update existing embedded document
                     $update = $this->prepareUpsertData($new);
                     foreach ($update as $cmd => $values) {
@@ -286,9 +286,7 @@ public function prepareUpsertData($document)
 
             // @ReferenceOne
             } elseif (isset($mapping['association']) && $mapping['association'] === ClassMetadata::REFERENCE_ONE) {
-                if (isset($new) || $mapping['nullable'] === true) {
-                    $updateData['$set'][$mapping['name']] = $new === null ? null : $this->prepareReferencedDocumentValue($mapping, $new);
-                }
+                $updateData['$set'][$mapping['name']] = $this->prepareReferencedDocumentValue($mapping, $new);
 
             // @ReferenceMany, @EmbedMany
             } elseif (

diff --git a/lib/Doctrine/ODM/MongoDB/Query/Builder.php b/lib/Doctrine/ODM/MongoDB/Query/Builder.php
@@ -1028,11 +1028,11 @@ public function nearSphere($x, $y = null): self
      * @see Expr::not()
      * @see https://docs.mongodb.com/manual/reference/operator/not/
      *
-     * @param array|Expr $expression
+     * @param array|Expr|mixed $valueOrExpression
      */
-    public function not($expression): self
+    public function not($valueOrExpression): self
     {
-        $this->expr->not($expression);
+        $this->expr->not($valueOrExpression);
 
         return $this;
     }

diff --git a/lib/Doctrine/ODM/MongoDB/Query/Expr.php b/lib/Doctrine/ODM/MongoDB/Query/Expr.php
@@ -861,7 +861,7 @@ public function nearSphere($x, $y = null): self
      * @see Builder::not()
      * @see https://docs.mongodb.com/manual/reference/operator/not/
      *
-     * @param array|Expr $expression
+     * @param array|Expr|mixed $expression
      */
     public function not($expression): self
     {

diff --git a/lib/Doctrine/ODM/MongoDB/UnitOfWork.php b/lib/Doctrine/ODM/MongoDB/UnitOfWork.php
@@ -1459,6 +1459,10 @@ public function scheduleForDelete(object $document, bool $isView = false): void
             unset($this->documentUpdates[$oid]);
         }
 
+        if (isset($this->documentUpserts[$oid])) {
+            unset($this->documentUpserts[$oid]);
+        }
+
         if (isset($this->documentDeletions[$oid])) {
             return;
         }

diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/EmbeddedTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/EmbeddedTest.php
@@ -431,6 +431,7 @@ public function testRemoveEmbeddedDocument(): void
 
         $check = $this->dm->getDocumentCollection(User::class)->findOne();
         $this->assertEmpty($check['phonenumbers']);
+        $this->assertNull($check['addressNullable']);
         $this->assertArrayNotHasKey('address', $check);
     }
 

diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Functional/QueryTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Functional/QueryTest.php
@@ -16,6 +16,7 @@
 use InvalidArgumentException;
 use IteratorAggregate;
 use MongoDB\BSON\ObjectId;
+use MongoDB\BSON\Regex;
 use MongoDB\BSON\UTCDateTime;
 
 use function array_values;
@@ -102,6 +103,27 @@ public function testAddNot(): void
         $this->assertNotNull($user);
     }
 
+    public function testNotAllowsRegex(): void
+    {
+        $user = new User();
+        $user->setUsername('boo');
+
+        $this->dm->persist($user);
+        $this->dm->flush();
+
+        $qb = $this->dm->createQueryBuilder(User::class);
+        $qb->field('username')->not(new Regex('Boo', 'i'));
+        $query = $qb->getQuery();
+        $user  = $query->getSingleResult();
+        $this->assertNull($user);
+
+        $qb = $this->dm->createQueryBuilder(User::class);
+        $qb->field('username')->not(new Regex('Boo'));
+        $query = $qb->getQuery();
+        $user  = $query->getSingleResult();
+        $this->assertNotNull($user);
+    }
+
     public function testDistinct(): void
     {
         $user = new User();