Throw exception on duplicate database names within a document

UPGRADE-2.0.md

@@ -150,6 +150,9 @@ or annotations. To migrate away from YAML mappings, first update to MongoDB ODM
   `BinMD5`, `BinUUID`, `BinUUIDRFC4122`, `Bool`, `Boolean`, `Collection`,
   `Date`, `Float`, `Hash`, `Increment`, `Int`, `Integer`, `Key`, `ObjectId`,
   `Raw`, `String`, `Timestamp`.
+* The `NotSaved` annotation has been dropped in favor of the `notSaved`
+  attribute on the `Field` annotation. The `notSaved` attribute can also be
+  applied to reference and embed mappings.
 * The `$name` and `$fieldName` properties in the `DiscriminatorField` annotation
   class have been dropped. The field name is now passed via the default `$value`
   property.

docs/en/reference/annotations-reference.rst

@@ -209,6 +209,10 @@ Optional attributes:
     ``collectionClass`` - A |FQCN| of class that implements ``Collection``
     interface and is used to hold documents. Doctrine's ``ArrayCollection`` is
     used by default.
+-
+    ``notSaved`` - ``false`` by default. If set to ``true``, the property is
+    loaded from the database if it exists there. Upon writing data back to the
+    database, the data from this field is not updated at all.
 
 .. code-block:: php
 
@@ -260,6 +264,9 @@ Optional attributes:
 -
     ``defaultDiscriminatorValue`` - A default value for discriminatorField if no
     value has been set in the embedded document.
+-
+    ``notSaved`` - The property is loaded if it exists in the database; however,
+    ODM will not save the property value back to the database.
 
 .. code-block:: php
 
@@ -354,6 +361,9 @@ Optional attributes:
    ``nullable`` - By default, ODM will ``$unset`` fields in MongoDB if the PHP
    value is null. Specify true for this option to force ODM to store a null
    value in the database instead of unsetting the field.
+-
+   ``notSaved`` - The property is loaded if it exists in the database; however,
+   ODM will not save the property value back to the database.
 
 Examples:
 
@@ -633,19 +643,6 @@ and should not be managed directly. See
         // ...
     }
 
-@NotSaved
----------
-
-The annotation is used to specify properties that are loaded if they exist in
-MongoDB; however, ODM will not save the property value back to the database.
-
-.. code-block:: php
-
-    <?php
-
-    /** @NotSaved */
-    public $field;
-
 @PostLoad
 ---------
 
@@ -956,6 +953,9 @@ Optional attributes:
     ``prime`` - A list of references contained in the target document that will
     be initialized when the collection is loaded. Only allowed for inverse
     references.
+-
+    ``notSaved`` - The property is loaded if it exists in the database; however,
+    ODM will not save the property value back to the database.
 
 .. code-block:: php
 
@@ -1023,6 +1023,9 @@ Optional attributes:
     ``limit`` - Limit for the query that loads the reference.
 -
     ``skip`` - Skip for the query that loads the reference.
+-
+    ``notSaved`` - The property is loaded if it exists in the database; however,
+    ODM will not save the property value back to the database.
 
 .. code-block:: php
 

docs/en/reference/migrating-schemas.rst

@@ -109,7 +109,7 @@ Migrating your schema can be a difficult task, but Doctrine provides a few
 different methods for dealing with it:
 
 -  **@AlsoLoad** - load values from old fields or transform data through methods
--  **@NotSaved** - load values into fields without saving them again
+-  **@Field(notSaved=true)** - load values into fields without saving them again
 -  **@PostLoad** - execute code after all fields have been loaded
 -  **@PrePersist** - execute code before your document gets saved
 
@@ -166,10 +166,10 @@ Later on, you may want to migrate this data into an embedded Address document:
         /** @Field(type="string") */
         public $name;
 
-        /** @NotSaved */
+        /** @Field(notSaved=true) */
         public $street;
 
-        /** @NotSaved */
+        /** @Field(notSaved=true) */
         public $city;
 
         /** @EmbedOne(targetDocument=Address::class) */

docs/en/reference/query-builder-api.rst

@@ -642,7 +642,7 @@ document with a text index:
             /** @Field(type="string") */
             public $description;
 
-            /** @Field(type="float") @NotSaved */
+            /** @Field(notSaved=true) */
             public $score;
         }
 

lib/Doctrine/ODM/MongoDB/Mapping/Annotations/AbstractField.php

@@ -24,6 +24,9 @@ abstract class AbstractField extends Annotation
     /** @var string|null */
     public $strategy;
 
+    /** @var bool */
+    public $notSaved = false;
+
     /**
      * Gets deprecation message. The method *WILL* be removed in 2.0.
      *

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

@@ -1939,6 +1939,7 @@ public function mapField(array $mapping) : array
         }
 
         $this->applyStorageStrategy($mapping);
+        $this->checkDuplicateMapping($mapping);
 
         $this->fieldMappings[$mapping['fieldName']] = $mapping;
         if (isset($mapping['association'])) {
@@ -2077,4 +2078,30 @@ private function isAllowedGridFSField(string $name) : bool
     {
         return in_array($name, self::ALLOWED_GRIDFS_FIELDS, true);
     }
+
+    private function checkDuplicateMapping(array $mapping) : void
+    {
+        if ($mapping['notSaved'] ?? false) {
+            return;
+        }
+
+        foreach ($this->fieldMappings as $fieldName => $otherMapping) {
+            // Ignore fields with the same name - we can safely override their mapping
+            if ($mapping['fieldName'] === $fieldName) {
+                continue;
+            }
+
+            // Ignore fields with a different name in the database
+            if ($mapping['name'] !== $otherMapping['name']) {
+                continue;
+            }
+
+            // If the other field is not saved, ignore it as well
+            if ($otherMapping['notSaved'] ?? false) {
+                continue;
+            }
+
+            throw MappingException::duplicateDatabaseFieldName($this->getName(), $mapping['fieldName'], $mapping['name'], $fieldName);
+        }
+    }
 }

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

@@ -54,6 +54,11 @@ public static function duplicateFieldMapping(string $document, string $fieldName
         return new self(sprintf('Property "%s" in "%s" was already declared, but it must be declared only once', $fieldName, $document));
     }
 
+    public static function duplicateDatabaseFieldName(string $document, string $offendingFieldName, string $databaseName, string $originalFieldName) : self
+    {
+        return new self(sprintf('Field "%s" in class "%s" is mapped to field "%s" in the database, but that name is already in use by field "%s".', $offendingFieldName, $document, $databaseName, $originalFieldName));
+    }
+
     public static function discriminatorFieldConflict(string $document, string $fieldName) : self
     {
         return new self(sprintf('Discriminator field "%s" in "%s" conflicts with a mapped field\'s "name" attribute.', $fieldName, $document));

tests/Doctrine/ODM/MongoDB/Tests/Functional/AlsoLoadTest.php

@@ -223,15 +223,15 @@ class AlsoLoadDocument
     public $foo;
 
     /**
-     * @ODM\NotSaved
+     * @ODM\Field(notSaved=true)
      * @ODM\AlsoLoad({"zip", "bar"})
      */
     public $baz;
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $bar;
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $zip;
 
     /**
@@ -240,10 +240,10 @@ class AlsoLoadDocument
      */
     public $zap = 'zap';
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $name;
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $fullName;
 
     /** @ODM\Field(type="string") */
@@ -258,13 +258,13 @@ class AlsoLoadDocument
      */
     public $test = 'test';
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $testNew;
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $testOld;
 
-    /** @ODM\NotSaved */
+    /** @ODM\Field(notSaved=true) */
     public $testOlder;
 
     /** @ODM\AlsoLoad({"name", "fullName"}) */

tests/Doctrine/ODM/MongoDB/Tests/Functional/OwningAndInverseReferencesTest.php

@@ -46,17 +46,17 @@ public function testOneToOne()
         $this->assertEquals($customer->cart->id, $customer->cart->id);
 
         $check = $this->dm->getDocumentCollection(get_class($customer))->findOne();
-        $this->assertArrayHasKey('cart', $check);
-        $this->assertEquals('test', $check['cart']);
+        $this->assertArrayHasKey('cartTest', $check);
+        $this->assertEquals('test', $check['cartTest']);
 
         $customer->cart     = null;
         $customer->cartTest = 'ok';
         $this->dm->flush();
         $this->dm->clear();
 
         $check = $this->dm->getDocumentCollection(get_class($customer))->findOne();
-        $this->assertArrayHasKey('cart', $check);
-        $this->assertEquals('ok', $check['cart']);
+        $this->assertArrayHasKey('cartTest', $check);
+        $this->assertEquals('ok', $check['cartTest']);
 
         $customer = $this->dm->getRepository(Customer::class)->find($customer->id);
         $this->assertInstanceOf(Cart::class, $customer->cart);

tests/Doctrine/ODM/MongoDB/Tests/Mapping/AbstractMappingDriverTest.php

@@ -448,6 +448,24 @@ public function testGridFSMapping()
         ], $class->getFieldMapping('metadata'), true);
     }
 
+    /**
+     * @expectedException \Doctrine\ODM\MongoDB\Mapping\MappingException
+     * @expectedExceptionMessage Field "bar" in class "Doctrine\ODM\MongoDB\Tests\Mapping\AbstractMappingDriverDuplicateDatabaseName" is mapped to field "baz" in the database, but that name is already in use by field "foo".
+     */
+    public function testDuplicateDatabaseNameInMappingCauseErrors()
+    {
+        $this->loadMetadata(AbstractMappingDriverDuplicateDatabaseName::class);
+    }
+
+    public function testDuplicateDatabaseNameWithNotSavedDoesNotThrowExeption()
+    {
+        $metadata = $this->loadMetadata(AbstractMappingDriverDuplicateDatabaseNameNotSaved::class);
+
+        $this->assertTrue($metadata->hasField('foo'));
+        $this->assertTrue($metadata->hasField('bar'));
+        $this->assertTrue($metadata->fieldMappings['bar']['notSaved']);
+    }
+
     protected function loadMetadata($className) : ClassMetadata
     {
         $mappingDriver = $this->_loadDriver();
@@ -704,3 +722,43 @@ class AbstractMappingDriverFileMetadata
     /** @ODM\Field */
     public $contentType;
 }
+
+/** @ODM\MappedSuperclass */
+class AbstractMappingDriverSuperClass
+{
+    /** @ODM\Id */
+    public $id;
+
+    /** @var @ODM\Field(type="string") */
+    protected $override;
+}
+
+/**
+ * @ODM\Document
+ */
+class AbstractMappingDriverDuplicateDatabaseName extends AbstractMappingDriverSuperClass
+{
+    /** @ODM\Field(type="int") */
+    public $override;
+
+    /** @ODM\Field(type="string", name="baz") */
+    public $foo;
+
+    /** @ODM\Field(type="string", name="baz") */
+    public $bar;
+}
+
+/**
+ * @ODM\Document
+ */
+class AbstractMappingDriverDuplicateDatabaseNameNotSaved extends AbstractMappingDriverSuperClass
+{
+    /** @ODM\Field(type="int") */
+    public $override;
+
+    /** @ODM\Field(type="string", name="baz") */
+    public $foo;
+
+    /** @ODM\Field(type="string", name="baz", notSaved=true) */
+    public $bar;
+}

tests/Doctrine/ODM/MongoDB/Tests/Mapping/XmlMappingDriverTest.php

@@ -7,7 +7,6 @@
 use Doctrine\ODM\MongoDB\Mapping\ClassMetadata;
 use Doctrine\ODM\MongoDB\Mapping\Driver\XmlDriver;
 use Doctrine\ODM\MongoDB\Mapping\MappingException;
-use Documents\User;
 use ReflectionMethod;
 use SimpleXmlElement;
 use stdClass;
@@ -38,20 +37,6 @@ public function testSetShardKeyOptionsByAttributes()
         $this->assertSame(['_id' => 1], $shardKey['keys']);
     }
 
-    public function testGetAssociationCollectionClass()
-    {
-        $class   = new ClassMetadata(User::class);
-        $driver  = $this->_loadDriver();
-        $element = new SimpleXmlElement('<reference-many target-document="Phonenumber" collection-class="Doctrine\\ODM\\MongoDB\\Tests\\Mapping\\PhonenumberCollection" field="phonenumbers"></reference-many>');
-
-        /** @uses XmlDriver::setShardKey */
-        $m = new ReflectionMethod(get_class($driver), 'addReferenceMapping');
-        $m->setAccessible(true);
-        $m->invoke($driver, $class, $element, 'many');
-
-        $this->assertEquals(PhonenumberCollection::class, $class->getAssociationCollectionClass('phonenumbers'));
-    }
-
     public function testInvalidMappingFileTriggersException() : void
     {
         $className     = InvalidMappingDocument::class;

tests/Doctrine/ODM/MongoDB/Tests/Mapping/xml/Doctrine.ODM.MongoDB.Tests.Mapping.AbstractMappingDriverDuplicateDatabaseName.dcm.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<doctrine-mongo-mapping xmlns="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping"
+                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  xsi:schemaLocation="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping
+                  http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd">
+
+    <document name="Doctrine\ODM\MongoDB\Tests\Mapping\AbstractMappingDriverDuplicateDatabaseName">
+        <field field-name="override" type="int" />
+
+        <field field-name="foo" type="string" name="baz" />
+        <field field-name="bar" type="string" name="baz" />
+    </document>
+</doctrine-mongo-mapping>

tests/Doctrine/ODM/MongoDB/Tests/Mapping/xml/Doctrine.ODM.MongoDB.Tests.Mapping.AbstractMappingDriverDuplicateDatabaseNameNotSaved.dcm.xml

@@ -0,0 +1,14 @@
+<?xml version="1.0" encoding="UTF-8"?>
+
+<doctrine-mongo-mapping xmlns="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping"
+                  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
+                  xsi:schemaLocation="http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping
+                  http://doctrine-project.org/schemas/odm/doctrine-mongo-mapping.xsd">
+
+    <document name="Doctrine\ODM\MongoDB\Tests\Mapping\AbstractMappingDriverDuplicateDatabaseNameNotSaved">
+        <field field-name="override" type="int" />
+
+        <field field-name="foo" type="string" name="baz" />
+        <field field-name="bar" type="string" name="baz" not-saved="true" />
+    </document>
+</doctrine-mongo-mapping>