Add new aggregation pipeline stages #1654
Conversation
alcaeus
force-pushed
the
add-new-aggregation-stuff
branch
from
158c009 to
65945b3
Compare
| @@ -0,0 +1,49 @@ | |||
| <?php | |||
|
|
|||
| @@ -0,0 +1,49 @@ | |||
| <?php | |||
|
|
|||
| public function connectFromField($connectFromField) | ||
| { | ||
| // No targetClass mapping - simply use field name as is | ||
| if (!$this->targetClass) { |
| } | ||
|
|
||
| // connectFromField doesn't have to be a reference - in this case, just convert the field name | ||
| if (!$this->targetClass->hasReference($connectFromField)) { |
| } | ||
|
|
||
| $mappedByMapping = $this->targetClass->getFieldMapping($referenceMapping['mappedBy']); | ||
| switch ($mappedByMapping['storeAs']) { |
There was a problem hiding this comment.
These two switches are almost the same, what do you think about rewriting whole if-else block to:
$mapping = $referenceMapping['isOwningSide'] ? $referenceMapping : $this->targetClass->getFieldMapping($referenceMapping['mappedBy']);
// if for repository method
// whole switch
| throw MappingException::repositoryMethodLookupNotAllowed($this->class->name, $connectFromField); | ||
| } | ||
|
|
||
| $mappedByMapping = $this->targetClass->getFieldMapping($referenceMapping['mappedBy']); |
There was a problem hiding this comment.
Should there be an exception for when mappedBy is not present? Or is that mappedBy or repositoryMethod?
There was a problem hiding this comment.
There's an exception if repositoryMethod is present - I don't specifically check for mappedBy here. It'd probably be good from a defensive coding point of view. 👍
| parent::from($this->targetClass->getCollection()); | ||
|
|
||
| if ($referenceMapping['isOwningSide']) { | ||
| switch ($referenceMapping['storeAs']) { |
There was a problem hiding this comment.
Same switch again, maybe a private method accepting array $mapping would be better?
There was a problem hiding this comment.
Done - not sure if I want to extract it into a trait for use in Stage\Lookup as well :|
| return array_map([$this, 'convertTargetFieldName'], $fieldName); | ||
| } | ||
|
|
||
| if (!$this->targetClass) { |
|
|
||
| public static function connectFromFieldMustReferenceSameDocument($fieldName) | ||
| { | ||
| return new self("Cannot use field '$fieldName' as connectFromField in \$graphLookup stage. Reference must target the document itself."); |
There was a problem hiding this comment.
if I'm not mistaken, you missed an "a": "in a $graphLookup stage"
alcaeus
force-pushed
the
add-new-aggregation-stuff
branch
from
df4f013 to
2e74384
Compare
alcaeus
force-pushed
the
add-new-aggregation-stuff
branch
from
10f60b3 to
bc70b7a
Compare
alcaeus
changed the title
Fixes #1618.
This pull request adds support for the new aggregation pipeline stages added in MongoDB 3.4 by supporting them with mapping information. It also fixes field name translation in
$lookupstages.Todos:
$lookuplimitations about reference types; document support forstoreAs=ref