Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix collectionClass not working with the Yaml or Xml drivers. #1458

Closed
wants to merge 8 commits into from
Closed

Fix collectionClass not working with the Yaml or Xml drivers. #1458

Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
c4db286
Fix collectionClass not working with the Yaml or Xml drivers.
trq Jul 14, 2016
12593fa
Add collection-class to xsd
trq Jul 14, 2016
24cbb14
Change collectionClass to collection-class
trq Jul 14, 2016
1b69263
Fix failing tests
trq Jul 14, 2016
4a77f59
Add test for collectionClass Yaml mappings.
trq Jul 14, 2016
2402824
Remove collectionClass from xsd
trq Jul 14, 2016
067608a
More test around collectionClass
trq Jul 15, 2016
d1f0d67
Fixed the last broken test \o/
trq Jul 21, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 2 additions & 2 deletions doctrine-mongo-mapping.xsd
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,10 @@
<xs:element name="default-discriminator-value" type="odm:default-discriminator-value" minOccurs="0"/>
</xs:sequence>
<xs:attribute name="target-document" type="xs:string" />
<xs:attribute name="collection-class" type="xs:string" />
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There seem to be collectionClass in line 115/116 (and 168/170) on which I can't comment, could you remove that?

<xs:attribute name="field" type="xs:NMTOKEN" use="required" />
<xs:attribute name="fieldName" type="xs:NMTOKEN" />
<xs:attribute name="strategy" type="xs:NMTOKEN" default="pushAll" />
<xs:attribute name="collectionClass" type="xs:string" />
</xs:complexType>

<xs:simpleType name="reference-store-as">
Expand Down Expand Up @@ -154,6 +154,7 @@
<xs:element name="criteria" type="odm:criteria-map" minOccurs="0" />
</xs:sequence>
<xs:attribute name="target-document" type="xs:string" />
<xs:attribute name="collection-class" type="xs:string" />
<xs:attribute name="field" type="xs:NMTOKEN" use="required" />
<xs:attribute name="fieldName" type="xs:NMTOKEN" />
<xs:attribute name="simple" type="xs:boolean" /><!-- deprecated -->
Expand All @@ -165,7 +166,6 @@
<xs:attribute name="limit" type="xs:integer" />
<xs:attribute name="skip" type="xs:integer" />
<xs:attribute name="orphan-removal" type="xs:boolean" />
<xs:attribute name="collectionClass" type="xs:string" />
</xs:complexType>

<xs:complexType name="sort-type">
Expand Down
18 changes: 18 additions & 0 deletions lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadataInfo.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2058,6 +2058,24 @@ public function getAssociationTargetClass($assocName)
return $this->associationMappings[$assocName]['targetDocument'];
}

/**
* Retrieve the collectionClass associated with an association
*
* @param string $assocName
*/
public function getAssociationCollectionClass($assocName)
{
if ( ! isset($this->associationMappings[$assocName])) {
throw new InvalidArgumentException("Association name expected, '" . $assocName . "' is not an association.");
}

if ( ! array_key_exists('collectionClass', $this->associationMappings[$assocName])) {
throw new InvalidArgumentException("collectionClass can only be applied to 'embedMany' and 'referenceMany' associations.");
}

return $this->associationMappings[$assocName]['collectionClass'];
}

/**
* {@inheritDoc}
*/
Expand Down
12 changes: 7 additions & 5 deletions lib/Doctrine/ODM/MongoDB/Mapping/Driver/XmlDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -244,11 +244,12 @@ private function addEmbedMapping(ClassMetadataInfo $class, $embed, $type)
$attributes = $embed->attributes();
$defaultStrategy = $type == 'one' ? ClassMetadataInfo::STORAGE_STRATEGY_SET : CollectionHelper::DEFAULT_STRATEGY;
$mapping = array(
'type' => $type,
'embedded' => true,
'targetDocument' => isset($attributes['target-document']) ? (string) $attributes['target-document'] : null,
'name' => (string) $attributes['field'],
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : $defaultStrategy,
'type' => $type,
'embedded' => true,
'targetDocument' => isset($attributes['target-document']) ? (string) $attributes['target-document'] : null,
'collectionClass' => isset($attributes['collection-class']) ? (string) $attributes['collection-class'] : null,
'name' => (string) $attributes['field'],
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : $defaultStrategy,
);
if (isset($attributes['fieldName'])) {
$mapping['fieldName'] = (string) $attributes['fieldName'];
Expand Down Expand Up @@ -291,6 +292,7 @@ private function addReferenceMapping(ClassMetadataInfo $class, $reference, $type
'simple' => isset($attributes['simple']) ? ('true' === (string) $attributes['simple']) : false, // deprecated
'storeAs' => isset($attributes['store-as']) ? (string) $attributes['store-as'] : ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB,
'targetDocument' => isset($attributes['target-document']) ? (string) $attributes['target-document'] : null,
'collectionClass' => isset($attributes['collection-class']) ? (string) $attributes['collection-class'] : null,
'name' => (string) $attributes['field'],
'strategy' => isset($attributes['strategy']) ? (string) $attributes['strategy'] : $defaultStrategy,
'inversedBy' => isset($attributes['inversed-by']) ? (string) $attributes['inversed-by'] : null,
Expand Down
12 changes: 7 additions & 5 deletions lib/Doctrine/ODM/MongoDB/Mapping/Driver/YamlDriver.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -242,11 +242,12 @@ private function addMappingFromEmbed(ClassMetadataInfo $class, $fieldName, $embe
{
$defaultStrategy = $type == 'one' ? ClassMetadataInfo::STORAGE_STRATEGY_SET : CollectionHelper::DEFAULT_STRATEGY;
$mapping = array(
'type' => $type,
'embedded' => true,
'targetDocument' => isset($embed['targetDocument']) ? $embed['targetDocument'] : null,
'fieldName' => $fieldName,
'strategy' => isset($embed['strategy']) ? (string) $embed['strategy'] : $defaultStrategy,
'type' => $type,
'embedded' => true,
'targetDocument' => isset($embed['targetDocument']) ? $embed['targetDocument'] : null,
'collectionClass' => isset($embed['collectionClass']) ? $embed['collectionClass'] : null,
'fieldName' => $fieldName,
'strategy' => isset($embed['strategy']) ? (string) $embed['strategy'] : $defaultStrategy,
);
if (isset($embed['name'])) {
$mapping['name'] = $embed['name'];
Expand Down Expand Up @@ -274,6 +275,7 @@ private function addMappingFromReference(ClassMetadataInfo $class, $fieldName, $
'simple' => isset($reference['simple']) ? (boolean) $reference['simple'] : false, // deprecated
'storeAs' => isset($reference['storeAs']) ? (string) $reference['storeAs'] : ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB,
'targetDocument' => isset($reference['targetDocument']) ? $reference['targetDocument'] : null,
'collectionClass' => isset($reference['collectionClass']) ? $reference['collectionClass'] : null,
'fieldName' => $fieldName,
'strategy' => isset($reference['strategy']) ? (string) $reference['strategy'] : $defaultStrategy,
'inversedBy' => isset($reference['inversedBy']) ? (string) $reference['inversedBy'] : null,
Expand Down
6 changes: 4 additions & 2 deletions tests/Doctrine/ODM/MongoDB/Tests/Mapping/AbstractMappingDriverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -405,7 +405,7 @@ class AbstractMappingDriverUser
public $address;

/**
* @ODM\ReferenceMany(targetDocument="Phonenumber", cascade={"persist"}, discriminatorField="discr", discriminatorMap={"home"="HomePhonenumber", "work"="WorkPhonenumber"}, defaultDiscriminatorValue="home")
* @ODM\ReferenceMany(targetDocument="Phonenumber", collectionClass="PhonenumberCollection", cascade={"persist"}, discriminatorField="discr", discriminatorMap={"home"="HomePhonenumber", "work"="WorkPhonenumber"}, defaultDiscriminatorValue="home")
*/
public $phonenumbers;

Expand All @@ -415,7 +415,7 @@ class AbstractMappingDriverUser
public $groups;

/**
* @ODM\ReferenceMany(targetDocument="Phonenumber", name="more_phone_numbers")
* @ODM\ReferenceMany(targetDocument="Phonenumber", collectionClass="PhonenumberCollection", name="more_phone_numbers")
*/
public $morePhoneNumbers;

Expand Down Expand Up @@ -509,6 +509,7 @@ public static function loadMetadata(ClassMetadata $metadata)
$metadata->mapManyReference(array(
'fieldName' => 'phonenumbers',
'targetDocument' => 'Doctrine\\ODM\\MongoDB\\Tests\\Mapping\\Phonenumber',
'collectionClass' => 'Doctrine\\ODM\\MongoDB\\Tests\\Mapping\\PhonenumberCollection',
'cascade' => array(1 => 'persist'),
'discriminatorField' => 'discr',
'discriminatorMap' => array(
Expand All @@ -521,6 +522,7 @@ public static function loadMetadata(ClassMetadata $metadata)
'fieldName' => 'morePhoneNumbers',
'name' => 'more_phone_numbers',
'targetDocument' => 'Doctrine\\ODM\\MongoDB\\Tests\\Mapping\\Phonenumber',
'collectionClass' => 'Doctrine\\ODM\\MongoDB\\Tests\\Mapping\\PhonenumberCollection',
));
$metadata->mapManyReference(array(
'fieldName' => 'groups',
Expand Down
5 changes: 5 additions & 0 deletions tests/Doctrine/ODM/MongoDB/Tests/Mapping/Driver/AbstractDriverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,7 @@ public function testDriver()
'type' => 'one',
'embedded' => true,
'targetDocument' => 'Documents\Address',
'collectionClass' => null,
'isCascadeDetach' => true,
'isCascadeMerge' => true,
'isCascadePersist' => true,
Expand All @@ -124,6 +125,7 @@ public function testDriver()
'type' => 'many',
'embedded' => true,
'targetDocument' => 'Documents\Phonenumber',
'collectionClass' => null,
'isCascadeDetach' => true,
'isCascadeMerge' => true,
'isCascadePersist' => true,
Expand All @@ -144,6 +146,7 @@ public function testDriver()
'storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_ID,
'simple' => true,
'targetDocument' => 'Documents\Profile',
'collectionClass' => null,
'cascade' => array('remove', 'persist', 'refresh', 'merge', 'detach'),
'isCascadeDetach' => true,
'isCascadeMerge' => true,
Expand Down Expand Up @@ -171,6 +174,7 @@ public function testDriver()
'storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB,
'simple' => false,
'targetDocument' => 'Documents\Account',
'collectionClass' => null,
'cascade' => array('remove', 'persist', 'refresh', 'merge', 'detach'),
'isCascadeDetach' => true,
'isCascadeMerge' => true,
Expand Down Expand Up @@ -198,6 +202,7 @@ public function testDriver()
'storeAs' => ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB,
'simple' => false,
'targetDocument' => 'Documents\Group',
'collectionClass' => null,
'cascade' => array('remove', 'persist', 'refresh', 'merge', 'detach'),
'isCascadeDetach' => true,
'isCascadeMerge' => true,
Expand Down
16 changes: 15 additions & 1 deletion tests/Doctrine/ODM/MongoDB/Tests/Mapping/XmlMappingDriverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,4 +28,18 @@ public function testSetShardKeyOptionsByAttributes()
$this->assertSame(array('unique' => true, 'numInitialChunks' => 4096), $shardKey['options']);
$this->assertSame(array('_id' => 1), $shardKey['keys']);
}
}

public function testGetAssociationCollectionClass()
{
$class = new ClassMetadataInfo('doc');
$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('Doctrine\\ODM\\MongoDB\\Tests\\Mapping\\PhonenumberCollection', $class->getAssociationCollectionClass('phonenumbers'));
}
}
18 changes: 17 additions & 1 deletion tests/Doctrine/ODM/MongoDB/Tests/Mapping/YamlMappingDriverTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,12 +31,23 @@ public function testAlternateRelationshipMappingSyntaxShouldSetDefaults()
}

foreach (array('embeddedPhonenumber', 'otherPhonenumbers') as $embeddedField) {
foreach (array('strategy', 'targetDocument') as $key) {
foreach (array('strategy', 'targetDocument', 'collectionClass') as $key) {
$this->assertArrayHasKey($key, $class->fieldMappings[$embeddedField]);
}
}
}

public function testGetAssociationCollectionClass()
{
$className = __NAMESPACE__.'\AbstractMappingDriverAlternateUser';
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you wanted to use AbstractMappingDriverUser as that's the class you changed mapping for

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P.S. sorry for such long break :)

$mappingDriver = new YamlDriver(__DIR__ . DIRECTORY_SEPARATOR . 'yaml');

$class = new ClassMetadata($className);
$mappingDriver->loadMetadataForClass($className, $class);
$this->assertEquals('Doctrine\ODM\MongoDB\Tests\Mapping\PhonenumberCollection', $class->getAssociationCollectionClass('embeddedPhonenumber'));
$this->assertEquals('Doctrine\ODM\MongoDB\Tests\Mapping\PhonenumberCollection', $class->getAssociationCollectionClass('otherPhoneNumbers'));
}

public function testFieldLevelIndexSyntaxWithBooleanValues()
{
$className = __NAMESPACE__.'\AbstractMappingDriverAlternateUser';
Expand Down Expand Up @@ -70,3 +81,8 @@ class AbstractMappingDriverAlternateUser
public $embeddedPhonenumber;
public $otherPhonenumbers;
}

class PhonenumberCollection extends \Doctrine\Common\Collections\ArrayCollection
{

}
4 changes: 2 additions & 2 deletions ...DB/Tests/Mapping/xml/Doctrine.ODM.MongoDB.Tests.Mapping.AbstractMappingDriverUser.dcm.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@
<remove />
</cascade>
</reference-one>
<reference-many target-document="Phonenumber" field="phonenumbers">
<reference-many target-document="Phonenumber" collection-class="PhonenumberCollection" field="phonenumbers">
<cascade>
<persist />
</cascade>
Expand All @@ -61,7 +61,7 @@
</reference-many>
<embed-one target-document="Phonenumber" fieldName="embeddedPhonenumber" field="embedded_phone_number">
</embed-one>
<embed-many target-document="Phonenumber" field="otherPhonenumbers">
<embed-many target-document="Phonenumber" collection-class="PhonenumberCollection" field="otherPhonenumbers">
<discriminator-field name="discr" />
<discriminator-map>
<discriminator-mapping value="home" class="HomePhonenumber" />
Expand Down
2 changes: 2 additions & 0 deletions ...B/Tests/Mapping/yaml/Doctrine.ODM.MongoDB.Tests.Mapping.AbstractMappingDriverUser.dcm.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -67,6 +67,7 @@ Doctrine\ODM\MongoDB\Tests\Mapping\AbstractMappingDriverUser:
referenceMany:
phonenumbers:
targetDocument: Phonenumber
collectionClass: PhonenumberCollection
cascade: [ persist ]
discriminatorField: discr
discriminatorMap:
Expand All @@ -86,6 +87,7 @@ Doctrine\ODM\MongoDB\Tests\Mapping\AbstractMappingDriverUser:
embedMany:
otherPhonenumbers:
targetDocument: Phonenumber
collectionClass: PhonenumberCollection
discriminatorField: discr
discriminatorMap:
home: HomePhonenumber
Expand Down