Forbid mapping class by more than one AbstractDocument

doctrine · Nov 28, 2018 · 274dfb2 · 274dfb2
1 parent f65b87c
commit 274dfb2
Show file tree

Hide file tree

Showing 3 changed files with 74 additions and 20 deletions.
diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/Driver/AnnotationDriver.php b/lib/Doctrine/ODM/MongoDB/Mapping/Driver/AnnotationDriver.php
@@ -12,30 +12,32 @@
 use Doctrine\ODM\MongoDB\Mapping\Annotations\AbstractIndex;
 use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
 use Doctrine\ODM\MongoDB\Mapping\MappingException;
+use ReflectionClass;
 use ReflectionMethod;
 use const E_USER_DEPRECATED;
 use function array_merge;
 use function array_replace;
 use function constant;
 use function get_class;
 use function is_array;
-use function ksort;
-use function reset;
 use function trigger_error;
 
 /**
  * The AnnotationDriver reads the mapping metadata from docblock annotations.
  */
 class AnnotationDriver extends AbstractAnnotationDriver
 {
-    /** @var int[] */
-    protected $entityAnnotationClasses = [
-        ODM\Document::class            => 1,
-        ODM\MappedSuperclass::class    => 2,
-        ODM\EmbeddedDocument::class    => 3,
-        ODM\QueryResultDocument::class => 4,
-        ODM\File::class                => 5,
-    ];
+    public function isTransient($className)
+    {
+        $classAnnotations = $this->reader->getClassAnnotations(new ReflectionClass($className));
+
+        foreach ($classAnnotations as $annot) {
+            if ($annot instanceof ODM\AbstractDocument) {
+                return false;
+            }
+        }
+        return true;
+    }
 
     /**
      * {@inheritdoc}
@@ -47,15 +49,15 @@ public function loadMetadataForClass($className, \Doctrine\Common\Persistence\Ma
 
         $classAnnotations = $this->reader->getClassAnnotations($reflClass);
 
-        $documentAnnots = [];
+        $documentAnnot = null;
         foreach ($classAnnotations as $annot) {
             $classAnnotations[get_class($annot)] = $annot;
 
-            foreach ($this->entityAnnotationClasses as $annotClass => $i) {
-                if ($annot instanceof $annotClass) {
-                    $documentAnnots[$i] = $annot;
-                    continue 2;
+            if ($annot instanceof ODM\AbstractDocument) {
+                if ($documentAnnot !== null) {
+                    throw MappingException::classCanOnlyBeMappedByOneAbstractDocument($className, $documentAnnot, $annot);
                 }
+                $documentAnnot = $annot;
             }
 
             // non-document class annotations
@@ -83,14 +85,10 @@ public function loadMetadataForClass($className, \Doctrine\Common\Persistence\Ma
             }
         }
 
-        if (! $documentAnnots) {
+        if ($documentAnnot === null) {
             throw MappingException::classIsNotAValidDocument($className);
         }
 
-        // find the winning document annotation
-        ksort($documentAnnots);
-        $documentAnnot = reset($documentAnnots);
-
         if ($documentAnnot instanceof ODM\MappedSuperclass) {
             $class->isMappedSuperclass = true;
         } elseif ($documentAnnot instanceof ODM\EmbeddedDocument) {

diff --git a/lib/Doctrine/ODM/MongoDB/Mapping/MappingException.php b/lib/Doctrine/ODM/MongoDB/Mapping/MappingException.php
@@ -6,7 +6,9 @@
 
 use Doctrine\Common\Collections\Collection;
 use Doctrine\Common\Persistence\Mapping\MappingException as BaseMappingException;
+use Doctrine\ODM\MongoDB\Mapping\Annotations\AbstractDocument;
 use ReflectionException;
+use ReflectionObject;
 use function sprintf;
 
 /**
@@ -79,6 +81,16 @@ public static function classIsNotAValidDocument(string $className) : self
         return new self(sprintf('Class %s is not a valid document or mapped super class.', $className));
     }
 
+    public static function classCanOnlyBeMappedByOneAbstractDocument(string $className, AbstractDocument $mappedAs, AbstractDocument $offending) : self
+    {
+        return new self(sprintf(
+            "Can not map class '%s' as %s because it was already mapped as %s.",
+            $className,
+            (new ReflectionObject($offending))->getShortName(),
+            (new ReflectionObject($mappedAs))->getShortName()
+        ));
+    }
+
     public static function reflectionFailure(string $document, ReflectionException $previousException) : self
     {
         return new self('An error occurred in ' . $document, 0, $previousException);

diff --git a/tests/Doctrine/ODM/MongoDB/Tests/Mapping/AnnotationDriverTest.php b/tests/Doctrine/ODM/MongoDB/Tests/Mapping/AnnotationDriverTest.php
@@ -164,6 +164,50 @@ public function testDocumentAnnotationCanSpecifyWriteConcern()
         $this->assertNull($cm->writeConcern);
     }
 
+    /**
+     * @dataProvider provideClassCanBeMappedByOneAbstractDocument
+     */
+    public function testClassCanBeMappedByOneAbstractDocument(object $wrong, string $messageRegExp)
+    {
+        $this->expectException(MappingException::class);
+        $this->expectExceptionMessageRegExp($messageRegExp);
+
+        $cm               = new ClassMetadata(get_class($wrong));
+        $reader           = new AnnotationReader();
+        $annotationDriver = new AnnotationDriver($reader);
+
+        $annotationDriver->loadMetadataForClass(get_class($wrong), $cm);
+    }
+
+    public function provideClassCanBeMappedByOneAbstractDocument()
+    {
+        yield [
+            /** @ODM\Document() @ODM\EmbeddedDocument */
+            new class () {},
+            "/as EmbeddedDocument because it was already mapped as Document\.$/"
+        ];
+        yield [
+            /** @ODM\Document() @ODM\File */
+            new class () {},
+            "/as File because it was already mapped as Document\.$/"
+        ];
+        yield [
+            /** @ODM\Document() @ODM\QueryResultDocument */
+            new class () {},
+            "/as QueryResultDocument because it was already mapped as Document\.$/"
+        ];
+        yield [
+            /** @ODM\Document() @ODM\MappedSuperclass */
+            new class () {},
+            "/as MappedSuperclass because it was already mapped as Document\.$/"
+        ];
+        yield [
+            /** @ODM\MappedSuperclass() @ODM\Document */
+            new class () {},
+            "/as Document because it was already mapped as MappedSuperclass\.$/"
+        ];
+    }
+
     protected function _loadDriverForCMSDocuments()
     {
         $annotationDriver = $this->_loadDriver();