Mongo ODM Fields cannot be unset

[kralos]

API Platform version(s) affected: 2.5.7

Description
Treatment of Doctrine\ODM\MongoDB\Mapping\Annotations\Field's nullable property is incorrect.

In MongoDB ODM's mapping info, nullable doesn't mean "is the field allowed to be null" in the database/php etc like it does in ORM. All fields in mongodb are nullable, in ODM it means when mapping from PHP does null map to an absent key in the database document or not.

https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/2.1/reference/annotations-reference.html#field

e.g. nullable=false (Default)

$document->setField(null);
$dm->flush();

{
    "_id": ObjectId("***")
}

e.g. nullable=true

$document->setField(null);
$dm->flush();

{
    "_id": ObjectId("***"),
    "field": null
}

How to reproduce

Given the following field on an ODM document:

use Doctrine\ODM\MongoDB\Mapping\Annotations\Field;
use Doctrine\ODM\MongoDB\Mapping\Annotations\Id;
use Symfony\Component\Validator\Constraints\NotBlank;

class Document
{
    /**
     * @Id()
     */
    private ?string $id = null;

    /**
     * @NotBlank(allowNull=true)
     * @Field(type="string")
     */
    private ?string $field = null;

    public function getId(): ?string
    {
        return $this->id;
    }

    public function setField(
        ?string $field
    ): self {
        $this->field = $field;
    }

    public function getField(): ?string
    {
        return $this->field;
    }
}

API Platform will 400 when setting field to null even though it is nullable (as far as mongodb and PHP are concerned).

The type of the "field" attribute must be "string", "NULL" given.

curl -i -H 'Accept: application/json' -H 'Content-Type: application/merge-patch+json' -X PATCH -d '{"field": null}' http://***/api/items/5f7d021b0cf6b87a7f6d8a12; echo
HTTP/1.1 400 Bad Request
Server: nginx/1.19.2
Content-Type: application/problem+json; charset=utf-8
Transfer-Encoding: chunked
Connection: keep-alive
X-Powered-By: PHP/7.4.10
X-Content-Type-Options: nosniff
X-Frame-Options: deny
Cache-Control: no-cache, private
Date: Wed, 07 Oct 2020 03:40:10 GMT
X-Debug-Token: 2b9265
X-Debug-Token-Link: http://***/_profiler/2b9265
X-Robots-Tag: noindex
Link: <http://***/api/docs.jsonld>; rel="http://www.w3.org/ns/hydra/core#apiDocumentation"
X-Previous-Debug-Token: cf061e

{"type":"https:\/\/tools.ietf.org\/html\/rfc2616#section-10","title":"An error occurred","detail":"The type of the \u0022field\u0022 attribute must be \u0022string\u0022, \u0022NULL\u0022 given."...}

Possible Solution

ApiPlatform\Core\Bridge\Doctrine\MongoDbOdm\PropertyInfo\DoctrineExtractor shouldn't read nullable from Doctrine\ODM\MongoDB\Mapping\ClassMetadata to determine if the Symfony\Component\PropertyInfo\Type is nullable. In mongodb it's always nullable. The closest thing to notnullable is adding a Symfony\Component\Validator\Constraints\NotBlank constraint.

Note: This patch has in no way been tested, just an example of what I mean.

diff --git a/src/Bridge/Doctrine/MongoDbOdm/PropertyInfo/DoctrineExtractor.php b/src/Bridge/Doctrine/MongoDbOdm/PropertyInfo/DoctrineExtractor.php
index 3325392b..22d1cf2b 100644
--- a/src/Bridge/Doctrine/MongoDbOdm/PropertyInfo/DoctrineExtractor.php
+++ b/src/Bridge/Doctrine/MongoDbOdm/PropertyInfo/DoctrineExtractor.php
@@ -66,9 +66,7 @@ final class DoctrineExtractor implements PropertyListExtractorInterface, Propert
             $class = $metadata->getAssociationTargetClass($property);
 
             if ($metadata->isSingleValuedAssociation($property)) {
-                $nullable = $metadata instanceof MongoDbClassMetadata && $metadata->isNullable($property);
-
-                return [new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, $class)];
+                return [new Type(Type::BUILTIN_TYPE_OBJECT, true, $class)];
             }
 
             $collectionKeyType = Type::BUILTIN_TYPE_INT;
@@ -87,19 +85,17 @@ final class DoctrineExtractor implements PropertyListExtractorInterface, Propert
 
         if ($metadata->hasField($property)) {
             $typeOfField = $metadata->getTypeOfField($property);
-            $nullable = $metadata instanceof MongoDbClassMetadata && $metadata->isNullable($property);
-
             switch ($typeOfField) {
                 case MongoDbType::DATE:
-                    return [new Type(Type::BUILTIN_TYPE_OBJECT, $nullable, 'DateTime')];
+                    return [new Type(Type::BUILTIN_TYPE_OBJECT, true, 'DateTime')];
                 case MongoDbType::HASH:
-                    return [new Type(Type::BUILTIN_TYPE_ARRAY, $nullable, null, true)];
+                    return [new Type(Type::BUILTIN_TYPE_ARRAY, true, null, true)];
                 case MongoDbType::COLLECTION:
-                    return [new Type(Type::BUILTIN_TYPE_ARRAY, $nullable, null, true, new Type(Type::BUILTIN_TYPE_INT))];
+                    return [new Type(Type::BUILTIN_TYPE_ARRAY, true, null, true, new Type(Type::BUILTIN_TYPE_INT))];
                 default:
                     $builtinType = $this->getPhpType($typeOfField);
 
-                    return $builtinType ? [new Type($builtinType, $nullable)] : null;
+                    return $builtinType ? [new Type($builtinType, true)] : null;
             }
         }

[kralos]

A workaround for this is to specify nullable=true on the Doctrine\ODM\MongoDB\Mapping\Annotations\Field annotations however this means the fields will be defined in the database.

[alanpoulain]

It seems valid to me. Would you make a PR to fix it?