Add automatic type detection for Embedded. #8724
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Warxcell
force-pushed
the
add_embeddable
branch
3 times, most recently
from
2f97e71 to
dbab6cf
Compare
troytft
reviewed
May 31, 2021
greg0ire
reviewed
May 31, 2021
| $this->embeddedClasses[$mapping['fieldName']] = [ | ||
| 'class' => $this->fullyQualifiedClassName($mapping['class']), | ||
| 'columnPrefix' => $mapping['columnPrefix'], | ||
| 'columnPrefix' => $mapping['columnPrefix'] ?? null, |
There was a problem hiding this comment.
🤔 what does this have to do with the rest of your PR?
There was a problem hiding this comment.
I assume phpstan failure they mentioned in the PR description.
| @@ -322,9 +322,11 @@ public function loadMetadataForClass($className, ClassMetadata $metadata) | |||
| ? $this->evaluateBoolean($embeddedMapping['use-column-prefix']) | |||
| : true; | |||
|
|
|||
| $class = isset($embeddedMapping['class']) ? (string) $embeddedMapping['class'] : null; | |||
There was a problem hiding this comment.
Let's inline this on line 327 and get rid of this one-time variable?
beberlei
reviewed
May 31, 2021
| $mapping['class'] = $type->getName(); | ||
| } | ||
| } | ||
|
|
There was a problem hiding this comment.
You need to check here again that class isset or otherwise throw an exception. If not, then line 3616 will throw a notice accessing mapping class that does not exist.
There was a problem hiding this comment.
Actually the validation is done "late" in ClassMetadataFactory::doLoadMetadata as per #1133 - this should probably be changed, but not now.
| $this->embeddedClasses[$mapping['fieldName']] = [ | ||
| 'class' => $this->fullyQualifiedClassName($mapping['class']), | ||
| 'columnPrefix' => $mapping['columnPrefix'], | ||
| 'columnPrefix' => $mapping['columnPrefix'] ?? null, |
There was a problem hiding this comment.
I assume phpstan failure they mentioned in the PR description.
beberlei
approved these changes
May 31, 2021
beberlei
added a commit
that referenced
this pull request
May 31, 2021
* Mark 2.8.x as unmaintained, and 2.9.x as current * Fix ClassMetadataInfo template inference * Fix metadata cache compatibility layer * Bump doctrine/cache patch dependency to fix build with lowest deps * Add generics to parameters * Add note about performance and inheritance mapping (#8704) Co-authored-by: Claudio Zizza <[email protected]> * Adapt flush($argument) in documentation as it's deprecated. (#8728) * [GH-8723] Remove use of nullability to automatically detect nullable status (#8732) * [GH-8723] Remove use of nullability to automatically detect nullable status. * [GH-8723] Make Column::$nullable default to false again, fix tests. * Add automatic type detection for Embedded. (#8724) * Add automatic type detection for Embedded. * Inline statement. Co-authored-by: Benjamin Eberlei <[email protected]> Co-authored-by: Grégoire Paris <[email protected]> Co-authored-by: Vincent Langlet <[email protected]> Co-authored-by: Andreas Braun <[email protected]> Co-authored-by: Fran Moreno <[email protected]> Co-authored-by: Juan Iglesias <[email protected]> Co-authored-by: Claudio Zizza <[email protected]> Co-authored-by: Yup <[email protected]> Co-authored-by: Benjamin Eberlei <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Great work on #8439 and #8589.
Following it - This adds same automatic type detection when using typed properties for Embedded.
Needs help on PHPStan failure.
Edit: Figured it out.