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

BUGFIX: Throw MappingException in loadMetaDataForClass #1454

Merged
merged 1 commit into from
Nov 21, 2018

Conversation

kdambekalns
Copy link
Member

When no class schema can be found in loadMetaDataForClass, a Doctrine
MappingException is now thrown. This makes our code work nicely with the
change in doctrine/orm#7471 that otherwise
leads to errors like this as of ORM 2.6.3:

FlowAnnotationDriver.php: No class schema found for "some-non-mapped-class".

89 …\FlowAnnotationDriver_Original::getClassSchema("some-non-mapped-class")
88 …\FlowAnnotationDriver_Original::loadMetadataForClass("some-non-mapped-class", Neos\Flow\Persistence\Doctrine\Mapping\ClassMetadata)

Fixes #1453

When no class schema can be found in `loadMetaDataForClass`, a Doctrine
`MappingException` is now thrown. This makes our code work nicely with the
change in doctrine/orm#7471 that otherwise
leads to errors like this as of ORM 2.6.3:

```
FlowAnnotationDriver.php: No class schema found for "some-non-mapped-class".

89 …\FlowAnnotationDriver_Original::getClassSchema("some-non-mapped-class")
88 …\FlowAnnotationDriver_Original::loadMetadataForClass("some-non-mapped-class", Neos\Flow\Persistence\Doctrine\Mapping\ClassMetadata)
```

Fixes neos#1453
Copy link
Member

@albe albe left a comment

Choose a reason for hiding this comment

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

I was first wondering how rethrowing a different exception would fix anything, but after looking at the doctrine fix at https://github.com/doctrine/doctrine2/pull/7471/files#diff-be509179ad6fbf9ed8770312aa84b301R429 it makes sense! Thanks for digging into this so quickly @kdambekalns !

Copy link
Member

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

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

Looks good by reading, just a remark concerning the try/catch scope

@kitsunet
Copy link
Member

I assume 4.3 doesn't use ORM 2.6.x ?

@albe
Copy link
Member

albe commented Nov 21, 2018

I assume 4.3 doesn't use ORM 2.6.x ?

Correct. 2.5.x due to PHP 7.1 requirement

@kitsunet kitsunet merged commit 931f931 into neos:5.0 Nov 21, 2018
@kitsunet
Copy link
Member

Merged due to the urgency of the problem. We can refator the try catch block any time.

@kdambekalns kdambekalns deleted the bugfix/1453-orm-fix-break branch November 21, 2018 13:49
@kdambekalns
Copy link
Member Author

Ok, I'll roll patch-level releases then… Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants