Merge pull request #1887 from watari/issue_867

Exception when persisting class unlisted in disciminator map (Issue 867)

docs/en/cookbook/resolve-target-document-listener.rst

@@ -75,7 +75,7 @@ An Invoice document:
     class Invoice
     {
         /**
-         * @ODM\ReferenceOne(targetDocument="Acme\InvoiceModule\Model\InvoiceSubjectInterface")
+         * @ODM\ReferenceOne(targetDocument=Acme\InvoiceModule\Model\InvoiceSubjectInterface::class)
          * @var InvoiceSubjectInterface
          */
         protected $subject;

docs/en/reference/aggregation-builder.rst

@@ -479,7 +479,7 @@ pipeline stages. Take the following relationship for example:
 
     /**
      * @ReferenceMany(
-     *     targetDocument="Documents\Item",
+     *     targetDocument=Documents\Item::class,
      *     cascade="all",
      *     storeAs="id"
      * )
@@ -504,7 +504,7 @@ to be considered when looking up one-to-one relationships:
 
     /**
      * @ReferenceOne(
-     *     targetDocument="Documents\Item",
+     *     targetDocument=Documents\Item::class,
      *     cascade="all",
      *     storeAs="id"
      * )

docs/en/reference/annotations-reference.rst

@@ -73,7 +73,7 @@ correspond to a value from the configured discriminator map.
      * @Document
      * @InheritanceType("SINGLE_COLLECTION")
      * @DiscriminatorField("type")
-     * @DiscriminatorMap({"person" = "Person", "employee" = "Employee"})
+     * @DiscriminatorMap({"person" = Person::class, "employee" = Employee::class})
      * @DefaultDiscriminatorValue("person")
      */
     class Person
@@ -115,9 +115,11 @@ during hydration to select the instantiation class.
 This annotation is required for the top-most class in a
 :ref:`single collection inheritance <single_collection_inheritance>` hierarchy.
 It takes an array as its only argument, which maps keys to class names. The
-class names may be fully qualified or relative to the current namespace. When
+class names must be fully qualified. Using the ``::class constant`` is supported. When
 a document is persisted to the database, its class name key will be stored in
-the discriminator field instead of the |FQCN|.
+the discriminator field instead of the |FQCN|. If the discriminator map is non-empty
+and it does not contain the class name of the persisted document, a
+``\Doctrine\ODM\MongoDB\Mapping\MappingException`` will be thrown.
 
 .. code-block:: php
 
@@ -127,7 +129,7 @@ the discriminator field instead of the |FQCN|.
      * @Document
      * @InheritanceType("SINGLE_COLLECTION")
      * @DiscriminatorField("type")
-     * @DiscriminatorMap({"person" = "Person", "employee" = "Employee"})
+     * @DiscriminatorMap({"person" = Person::class, "employee" = Employee::class})
      */
     class Person
     {
@@ -223,8 +225,8 @@ Optional attributes:
      *     strategy="set",
      *     discriminatorField="type",
      *     discriminatorMap={
-     *         "book"="Documents\BookTag",
-     *         "song"="Documents\SongTag"
+     *         "book"=Documents\BookTag::class,
+     *         "song"=Documents\SongTag::class
      *     },
      *     defaultDiscriminatorValue="book"
      * )
@@ -276,8 +278,8 @@ Optional attributes:
      * @EmbedOne(
      *     discriminatorField="type",
      *     discriminatorMap={
-     *         "user"="Documents\User",
-     *         "author"="Documents\Author"
+     *         "user"=Documents\User::class,
+     *         "author"=Documents\Author::class
      *     },
      *     defaultDiscriminatorValue="user"
      * )
@@ -606,7 +608,7 @@ Examples:
      * @Document
      * @InheritanceType("SINGLE_COLLECTION")
      * @DiscriminatorField("type")
-     * @DiscriminatorMap({"person"="Person", "employee"="Employee"})
+     * @DiscriminatorMap({"person"=Person::class, "employee"=Employee::class})
      */
     class Person
     {
@@ -964,13 +966,13 @@ Optional attributes:
     /**
      * @ReferenceMany(
      *     strategy="set",
-     *     targetDocument="Documents\Item",
+     *     targetDocument=Documents\Item::class,
      *     cascade="all",
      *     sort={"sort_field": "asc"}
      *     discriminatorField="type",
      *     discriminatorMap={
-     *         "book"="Documents\BookItem",
-     *         "song"="Documents\SongItem"
+     *         "book"=Documents\BookItem::class,
+     *         "song"=Documents\SongItem::class
      *     },
      *     defaultDiscriminatorValue="book"
      * )
@@ -1033,12 +1035,12 @@ Optional attributes:
 
     /**
      * @ReferenceOne(
-     *     targetDocument="Documents\Item",
+     *     targetDocument=Documents\Item::class,
      *     cascade="all",
      *     discriminatorField="type",
      *     discriminatorMap={
-     *         "book"="Documents\BookItem",
-     *         "song"="Documents\SongItem"
+     *         "book"=Documents\BookItem::class,
+     *         "song"=Documents\SongItem::class
      *     },
      *     defaultDiscriminatorValue="book"
      * )

docs/en/reference/basic-mapping.rst

@@ -520,7 +520,7 @@ the collection. Here is an example:
     /**
      * @Document(collection="my_documents")
      * @DiscriminatorField("type")
-     * @DiscriminatorMap({"article"="Article", "album"="Album"})
+     * @DiscriminatorMap({"article"=Article::class, "album"=Album::class})
      */
     class Article
     {
@@ -530,7 +530,7 @@ the collection. Here is an example:
     /**
      * @Document(collection="my_documents")
      * @DiscriminatorField("type")
-     * @DiscriminatorMap({"article"="Article", "album"="Album"})
+     * @DiscriminatorMap({"article"=Article::class, "album"=Album::class})
      */
     class Album
     {

docs/en/reference/embedded-mapping.rst

@@ -164,8 +164,8 @@ in each embedded document:
             /**
              * @EmbedMany(
              *   discriminatorMap={
-             *     "download"="DownloadTask",
-             *     "build"="BuildTask"
+             *     "download"=DownloadTask::class,
+             *     "build"=BuildTask::class
              *   }
              * )
              */
@@ -201,8 +201,8 @@ discriminator:
             /**
              * @EmbedMany(
              *   discriminatorMap={
-             *     "download"="DownloadTask",
-             *     "build"="BuildTask"
+             *     "download"=DownloadTask::class,
+             *     "build"=BuildTask::class
              *   },
              *   defaultDiscriminatorValue="download"
              * )

docs/en/reference/inheritance-mapping.rst

@@ -72,7 +72,7 @@ Simple example:
          * @Document
          * @InheritanceType("SINGLE_COLLECTION")
          * @DiscriminatorField("type")
-         * @DiscriminatorMap({"person"="Person", "employee"="Employee"})
+         * @DiscriminatorMap({"person"=Person::class, "employee"=Employee::class})
          */
         class Person
         {
@@ -149,7 +149,7 @@ discriminator field:
          * @Document
          * @InheritanceType("SINGLE_COLLECTION")
          * @DiscriminatorField("type")
-         * @DiscriminatorMap({"person"="Person", "employee"="Employee"})
+         * @DiscriminatorMap({"person"=Person::class, "employee"=Employee::class})
          * @DefaultDiscriminatorValue("person")
          */
         class Person

docs/en/reference/reference-mapping.rst

@@ -208,8 +208,8 @@ in each `DBRef`_ object:
             /**
              * @ReferenceMany(
              *   discriminatorMap={
-             *     "album"="Album",
-             *     "song"="Song"
+             *     "album"=Album::class,
+             *     "song"=Song::class
              *   }
              * )
              */
@@ -244,8 +244,8 @@ a certain class, you can optionally specify a default discriminator value:
             /**
              * @ReferenceMany(
              *   discriminatorMap={
-             *     "album"="Album",
-             *     "song"="Song"
+             *     "album"=Album::class,
+             *     "song"=Song::class
              *   },
              *   defaultDiscriminatorValue="album"
              * )

lib/Doctrine/ODM/MongoDB/DocumentManager.php

@@ -735,37 +735,64 @@ public function createReference(object $document, array $referenceMapping)
                 throw new InvalidArgumentException(sprintf('Reference type %s is invalid.', $storeAs));
         }
 
-        /* If the class has a discriminator (field and value), use it. A child
-         * class that is not defined in the discriminator map may only have a
-         * discriminator field and no value, so default to the full class name.
-         */
-        if (isset($class->discriminatorField)) {
-            $reference[$class->discriminatorField] = $class->discriminatorValue ?? $class->name;
-        }
+        return $reference + $this->getDiscriminatorData($referenceMapping, $class);
+    }
 
-        /* Add a discriminator value if the referenced document is not mapped
-         * explicitly to a targetDocument class.
-         */
-        if (! isset($referenceMapping['targetDocument'])) {
+    /**
+     * Build discriminator portion of reference for specified reference mapping and class metadata.
+     *
+     * @param array         $referenceMapping Mappings of reference for which discriminator data is created.
+     * @param ClassMetadata $class            Metadata of reference document class.
+     *
+     * @return array with next structure [{discriminator field} => {discriminator value}]
+     *
+     * @throws MappingException When discriminator map is present and reference class in not registered in it.
+     */
+    private function getDiscriminatorData(array $referenceMapping, ClassMetadata $class) : array
+    {
+        $discriminatorField = null;
+        $discriminatorValue = null;
+        $discriminatorData  = [];
+        if (isset($referenceMapping['discriminatorField'])) {
             $discriminatorField = $referenceMapping['discriminatorField'];
-            $discriminatorValue = isset($referenceMapping['discriminatorMap'])
-                ? array_search($class->name, $referenceMapping['discriminatorMap'])
-                : $class->name;
-
-            /* If the discriminator value was not found in the map, use the full
-             * class name. In the future, it may be preferable to throw an
-             * exception here (perhaps based on some strictness option).
-             *
-             * @see PersistenceBuilder::prepareEmbeddedDocumentValue()
-             */
-            if ($discriminatorValue === false) {
+            if (isset($referenceMapping['discriminatorMap'])) {
+                $pos = array_search($class->name, $referenceMapping['discriminatorMap']);
+                if ($pos !== false) {
+                    $discriminatorValue = $pos;
+                }
+            } else {
                 $discriminatorValue = $class->name;
             }
+        } else {
+            $discriminatorField = $class->discriminatorField;
+            $discriminatorValue = $class->discriminatorValue;
+        }
 
-            $reference[$discriminatorField] = $discriminatorValue;
+        if ($discriminatorField !== null) {
+            if ($discriminatorValue === null) {
+                throw MappingException::unlistedClassInDiscriminatorMap($class->name);
+            }
+            $discriminatorData = [$discriminatorField => $discriminatorValue];
+        } elseif (! isset($referenceMapping['targetDocument'])) {
+            $discriminatorField = $referenceMapping['discriminatorField'];
+
+            $discriminatorMap = null;
+            if (isset($referenceMapping['discriminatorMap'])) {
+                $discriminatorMap = $referenceMapping['discriminatorMap'];
+            }
+            if ($discriminatorMap === null) {
+                $discriminatorValue = $class->name;
+            } else {
+                $discriminatorValue = array_search($class->name, $discriminatorMap);
+
+                if ($discriminatorValue === false) {
+                    throw MappingException::unlistedClassInDiscriminatorMap($class->name);
+                }
+            }
+            $discriminatorData = [$discriminatorField => $discriminatorValue];
         }
 
-        return $reference;
+        return $discriminatorData;
     }
 
     /**

lib/Doctrine/ODM/MongoDB/Mapping/MappingException.php

@@ -71,6 +71,11 @@ public static function invalidClassInDiscriminatorMap(string $className, string
         return new self(sprintf("Document class '%s' used in the discriminator map of class '%s' does not exist.", $className, $owningClass));
     }
 
+    public static function unlistedClassInDiscriminatorMap(string $className) : self
+    {
+        return new self(sprintf('Document class "%s" is unlisted in the discriminator map.', $className));
+    }
+
     public static function invalidDiscriminatorValue(string $value, string $owningClass) : self
     {
         return new self(sprintf("Discriminator value '%s' used in the declaration of class '%s' does not exist.", $value, $owningClass));

lib/Doctrine/ODM/MongoDB/Persisters/PersistenceBuilder.php

@@ -6,6 +6,7 @@
 
 use Doctrine\ODM\MongoDB\DocumentManager;
 use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
+use Doctrine\ODM\MongoDB\Mapping\MappingException;
 use Doctrine\ODM\MongoDB\PersistentCollection\PersistentCollectionInterface;
 use Doctrine\ODM\MongoDB\Types\Type;
 use Doctrine\ODM\MongoDB\UnitOfWork;
@@ -96,7 +97,13 @@ public function prepareInsertData($document)
 
         // add discriminator if the class has one
         if (isset($class->discriminatorField)) {
-            $insertData[$class->discriminatorField] = $class->discriminatorValue ?? $class->name;
+            if ($class->discriminatorValue === null) {
+                if (! empty($class->discriminatorMap)) {
+                    throw MappingException::unlistedClassInDiscriminatorMap($class->name);
+                }
+                $class->discriminatorValue = $class->name;
+            }
+            $insertData[$class->discriminatorField] = $class->discriminatorValue;
         }
 
         return $insertData;
@@ -277,7 +284,13 @@ public function prepareUpsertData($document)
 
         // add discriminator if the class has one
         if (isset($class->discriminatorField)) {
-            $updateData['$set'][$class->discriminatorField] = $class->discriminatorValue ?? $class->name;
+            if ($class->discriminatorValue === null) {
+                if (! empty($class->discriminatorMap)) {
+                    throw MappingException::unlistedClassInDiscriminatorMap($class->name);
+                }
+                $class->discriminatorValue = $class->name;
+            }
+            $updateData['$set'][$class->discriminatorField] = $class->discriminatorValue;
         }
 
         return $updateData;
@@ -379,17 +392,13 @@ public function prepareEmbeddedDocumentValue(array $embeddedMapping, $embeddedDo
          */
         if (! isset($embeddedMapping['targetDocument'])) {
             $discriminatorField = $embeddedMapping['discriminatorField'];
-            $discriminatorValue = isset($embeddedMapping['discriminatorMap'])
-                ? array_search($class->name, $embeddedMapping['discriminatorMap'])
-                : $class->name;
-
-            /* If the discriminator value was not found in the map, use the full
-             * class name. In the future, it may be preferable to throw an
-             * exception here (perhaps based on some strictness option).
-             *
-             * @see DocumentManager::createDBRef()
-             */
-            if ($discriminatorValue === false) {
+            if (! empty($embeddedMapping['discriminatorMap'])) {
+                $discriminatorValue = array_search($class->name, $embeddedMapping['discriminatorMap']);
+
+                if ($discriminatorValue === false) {
+                    throw MappingException::unlistedClassInDiscriminatorMap($class->name);
+                }
+            } else {
                 $discriminatorValue = $class->name;
             }
 
@@ -401,7 +410,13 @@ public function prepareEmbeddedDocumentValue(array $embeddedMapping, $embeddedDo
          * discriminator field and no value, so default to the full class name.
          */
         if (isset($class->discriminatorField)) {
-            $embeddedDocumentValue[$class->discriminatorField] = $class->discriminatorValue ?? $class->name;
+            if ($class->discriminatorValue === null) {
+                if (! empty($class->discriminatorMap)) {
+                    throw MappingException::unlistedClassInDiscriminatorMap($class->name);
+                }
+                $class->discriminatorValue = $class->name;
+            }
+            $embeddedDocumentValue[$class->discriminatorField] =  $class->discriminatorValue;
         }
 
         // Ensure empty embedded documents are stored as BSON objects

tests/Doctrine/ODM/MongoDB/Tests/DocumentManagerTest.php

@@ -256,7 +256,7 @@ class WrongSimpleRefDocument
     /** @ODM\Id */
     public $id;
 
-    /** @ODM\ReferenceOne(targetDocument="Documents\Tournament\Participant", storeAs="id") */
+    /** @ODM\ReferenceOne(targetDocument=Documents\Tournament\Participant::class, storeAs="id") */
     public $ref;
 }