-
-
Notifications
You must be signed in to change notification settings - Fork 504
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.
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
Add generic reference as replacement for DBRef #1623
Changes from all commits
242211e
23b2c93
5a8b85c
2084552
802d6ab
0ac42f6
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -668,15 +668,15 @@ public function getConfiguration() | |
} | ||
|
||
/** | ||
* Returns a DBRef array for the supplied document. | ||
* Returns a reference to the supplied document. | ||
* | ||
* @param mixed $document A document object | ||
* @param object $document A document object | ||
* @param array $referenceMapping Mapping for the field that references the document | ||
* | ||
* @throws \InvalidArgumentException | ||
* @return array A DBRef array | ||
* @return mixed The reference for the document in question, according to the desired mapping | ||
*/ | ||
public function createDBRef($document, array $referenceMapping = null) | ||
public function createReference($document, array $referenceMapping) | ||
{ | ||
if ( ! is_object($document)) { | ||
throw new \InvalidArgumentException('Cannot create a DBRef, the document is not an object'); | ||
|
@@ -691,36 +691,54 @@ public function createDBRef($document, array $referenceMapping = null) | |
); | ||
} | ||
|
||
if ($referenceMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { | ||
if ($class->inheritanceType === ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_COLLECTION) { | ||
throw MappingException::simpleReferenceMustNotTargetDiscriminatedDocument($referenceMapping['targetDocument']); | ||
} | ||
return $class->getDatabaseIdentifierValue($id); | ||
} | ||
|
||
$dbRef = array( | ||
'$ref' => $class->getCollection(), | ||
'$id' => $class->getDatabaseIdentifierValue($id), | ||
); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm a bit rusty, but is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The name would suggest it creates a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Introduced |
||
|
||
if ($referenceMapping['storeAs'] === ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB) { | ||
$dbRef['$db'] = $this->getDocumentDatabase($class->name)->getName(); | ||
$storeAs = isset($referenceMapping['storeAs']) ? $referenceMapping['storeAs'] : null; | ||
switch ($storeAs) { | ||
case ClassMetadataInfo::REFERENCE_STORE_AS_ID: | ||
if ($class->inheritanceType === ClassMetadataInfo::INHERITANCE_TYPE_SINGLE_COLLECTION) { | ||
throw MappingException::simpleReferenceMustNotTargetDiscriminatedDocument($referenceMapping['targetDocument']); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This appears to be the exception I was expecting for the |
||
} | ||
|
||
return $class->getDatabaseIdentifierValue($id); | ||
break; | ||
|
||
|
||
case ClassMetadataInfo::REFERENCE_STORE_AS_REF: | ||
$reference = ['id' => $class->getDatabaseIdentifierValue($id)]; | ||
break; | ||
|
||
case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF: | ||
$reference = [ | ||
'$ref' => $class->getCollection(), | ||
'$id' => $class->getDatabaseIdentifierValue($id), | ||
]; | ||
break; | ||
|
||
case ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB: | ||
$reference = [ | ||
'$ref' => $class->getCollection(), | ||
'$id' => $class->getDatabaseIdentifierValue($id), | ||
'$db' => $this->getDocumentDatabase($class->name)->getName(), | ||
]; | ||
break; | ||
|
||
default: | ||
throw new \InvalidArgumentException("Reference type {$storeAs} is invalid."); | ||
} | ||
|
||
/* 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)) { | ||
$dbRef[$class->discriminatorField] = isset($class->discriminatorValue) | ||
$reference[$class->discriminatorField] = isset($class->discriminatorValue) | ||
? $class->discriminatorValue | ||
: $class->name; | ||
} | ||
|
||
/* Add a discriminator value if the referenced document is not mapped | ||
* explicitly to a targetDocument class. | ||
*/ | ||
if ($referenceMapping !== null && ! isset($referenceMapping['targetDocument'])) { | ||
if (! isset($referenceMapping['targetDocument'])) { | ||
$discriminatorField = $referenceMapping['discriminatorField']; | ||
$discriminatorValue = isset($referenceMapping['discriminatorMap']) | ||
? array_search($class->name, $referenceMapping['discriminatorMap']) | ||
|
@@ -736,10 +754,31 @@ public function createDBRef($document, array $referenceMapping = null) | |
$discriminatorValue = $class->name; | ||
} | ||
|
||
$dbRef[$discriminatorField] = $discriminatorValue; | ||
$reference[$discriminatorField] = $discriminatorValue; | ||
} | ||
|
||
return $reference; | ||
} | ||
|
||
/** | ||
* Returns a DBRef array for the supplied document. | ||
* | ||
* @param mixed $document A document object | ||
* @param array $referenceMapping Mapping for the field that references the document | ||
* | ||
* @throws \InvalidArgumentException | ||
* @return array A DBRef array | ||
* @deprecated Deprecated in favor of createReference; will be removed in 2.0 | ||
*/ | ||
public function createDBRef($document, array $referenceMapping = null) | ||
{ | ||
@trigger_error('The ' . __METHOD__ . ' method has been deprecated and will be removed in ODM 2.0. Use createReference() instead.', E_USER_DEPRECATED); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What role does There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'm not sure why, but I remember Symfony handling their deprecations like that and we've also done it this way previously. |
||
|
||
if (!isset($referenceMapping['storeAs'])) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Shouldn't this be fixed on class metadata level? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since it's a public method I decided to be defensive regarding what we receive. There's no guarantee someone will pass an actual mapping, they could call it with a partial mapping array (e.g. our tests). One more reason to look forward to metadata objects. |
||
$referenceMapping['storeAs'] = ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF; | ||
} | ||
|
||
return $dbRef; | ||
return $this->createReference($document, $referenceMapping); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -103,6 +103,7 @@ class ClassMetadataInfo implements \Doctrine\Common\Persistence\Mapping\ClassMet | |
const REFERENCE_STORE_AS_ID = 'id'; | ||
const REFERENCE_STORE_AS_DB_REF = 'dbRef'; | ||
const REFERENCE_STORE_AS_DB_REF_WITH_DB = 'dbRefWithDb'; | ||
const REFERENCE_STORE_AS_REF = 'ref'; | ||
|
||
/* The inheritance mapping types */ | ||
/** | ||
|
@@ -474,6 +475,48 @@ public function __construct($documentName) | |
$this->rootDocumentName = $documentName; | ||
} | ||
|
||
/** | ||
* Helper method to get reference id of ref* type references | ||
* @param mixed $reference | ||
* @param string $storeAs | ||
* @return mixed | ||
* @internal | ||
*/ | ||
public static function getReferenceId($reference, $storeAs) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Type hint There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should we still type hint here? Alternatively, should the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'll relax the doc typehint, we can't typehint in the method since it can either be a reference array ( |
||
{ | ||
return $storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID ? $reference : $reference[ClassMetadataInfo::getReferencePrefix($storeAs) . 'id']; | ||
} | ||
|
||
/** | ||
* Returns the reference prefix used for a reference | ||
* @param string $storeAs | ||
* @return string | ||
*/ | ||
private static function getReferencePrefix($storeAs) | ||
{ | ||
if (!in_array($storeAs, [ClassMetadataInfo::REFERENCE_STORE_AS_REF, ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF, ClassMetadataInfo::REFERENCE_STORE_AS_DB_REF_WITH_DB])) { | ||
throw new \LogicException('Can only get a reference prefix for DBRef and reference arrays'); | ||
} | ||
|
||
return $storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_REF ? '' : '$'; | ||
} | ||
|
||
/** | ||
* Returns a fully qualified field name for a given reference | ||
* @param string $storeAs | ||
* @param string $pathPrefix The field path prefix | ||
* @return string | ||
* @internal | ||
*/ | ||
public static function getReferenceFieldName($storeAs, $pathPrefix = '') | ||
{ | ||
if ($storeAs === ClassMetadataInfo::REFERENCE_STORE_AS_ID) { | ||
return $pathPrefix; | ||
} | ||
|
||
return ($pathPrefix ? $pathPrefix . '.' : '') . static::getReferencePrefix($storeAs) . 'id'; | ||
} | ||
|
||
/** | ||
* {@inheritDoc} | ||
*/ | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this specify the logical conflict with
id
mapping, or is that discussed elsewhere? I assume we throw an exception for such mappings at runtime in any event.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It's specified where we explain the different
storeAs
options:I can add another note to each discriminator option for clarity.