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.

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

Lowered Psalm level to 3 #42

Merged
merged 2 commits into from
Nov 5, 2021
Merged

Conversation

driehle
Copy link
Member

@driehle driehle commented Nov 5, 2021

This required only a few class-string changes to pass.

Once change that might look surprising:

-        $metadata   = $this->objectManager->getClassMetadata(ltrim($target, '\\'));
+        $metadata   = $this->objectManager->getClassMetadata($target);

This shouldn't cause any BC breaks, since $target is filled from $metadata->getAssociationTargetClass($field);, which does not return leading slashs. I think this is still a relict from very early orm versions.

@driehle driehle self-assigned this Nov 5, 2021
@driehle driehle added the Enhancement New feature or request label Nov 5, 2021
@driehle driehle added this to the 2.2.1 milestone Nov 5, 2021
@@ -351,6 +350,7 @@ protected function hydrateByValue(array $data, $object)

if ($metadata->hasAssociation($field)) {
$target = $metadata->getAssociationTargetClass($field);
assert($target !== null);
Copy link
Member

Choose a reason for hiding this comment

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

This is a sign that we should have a method that throws instead of returning null

Copy link
Member Author

Choose a reason for hiding this comment

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

Indeed, yes :-)

Copy link

@eastern-ravindra eastern-ravindra May 13, 2024

Choose a reason for hiding this comment

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

Above assert code is failing for mongoDB, when we are using EmbedMany argument without target document locally.

According to documentation, EmbedMany without target document is allowed to embed different documents in same field.(https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/2.7/reference/embedded-mapping.html#mixing-document-types)

Does this assert check is really needed as null value is also accepted for mongodb?

Created new issue for the same #84

@driehle driehle merged commit f35233f into doctrine:2.2.x Nov 5, 2021
@driehle driehle deleted the feature/psalm-level-3 branch November 5, 2021 11:31
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants