Fix crashes in ConvertMappingCommand and GenerateEntitiesCommand... #1345
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
… using entities with joined table inheritance ConvertMappingCommand and GenerateEntitiesCommand both use the DisconnectedClassMetadataFactory, which allows metadata manipulation without loading the associated classes. Commit a36bea broke these two commands by adding a bailout condition in ClassMetadataFactory::populateDiscriminatorValue which checks $metadata->reflClass->isAbstract(). If the DisconnectedClassMetadataFactory is being used, $metadata->reflClass will always be null, causing a fatal error, "Fatal error: Call to a member function isAbstract() on null". This commit adds a check to see if $metadata->reflClass is set before checking isAbstract.
|
Hello, thank you for creating this pull request. I have automatically opened an issue http://www.doctrine-project.org/jira/browse/DDC-3632 We use Jira to track the state of pull requests and the versions they got |
|
@zeroedin-bill any way to get a test case for this before merge? I can work on it tomorrow otherwise |
|
@Ocramius I don't have time till late in the week probably :/ travelling and internal bug piles... |
|
Will try dealing with it during lunch break then |
|
I could not reproduce the bug. Neither with a Will merge anyway, as the conditional doesn't hurt there. |
Ocramius
added a commit
that referenced
this pull request
Mar 23, 2015
…ng-command Fix crashes in ConvertMappingCommand and GenerateEntitiesCommand...
|
Thanks! |
manticorp
added a commit
to manticorp/acl
that referenced
this pull request
Sep 7, 2016
Fixes [BUG] Call to member function isAbstract on null in mappedEventSubscriber doctrine:generate:entities This seems to be related to the following issue filed on the doctrine github: doctrine/orm#4459 and the following pull request: doctrine/orm#1345 The generate:entities command uses the ```DisconnectedClassMetadataFactory``` and so ```getReflectionClass``` isn't guaranteed to return anything and can be null, as such the following lines in ```LaravelDoctrine\ACL\Mappings\Subscribers\MappedEventSubscriber``` should be changed: From: ``` /** * A MappedSuperClass or Abstract class cannot be instantiated. * * @param ClassMetadata $metadata * * @return bool */ protected function isInstantiable(ClassMetadata $metadata) { if ($metadata->isMappedSuperclass) { return false; } if ($metadata->getReflectionClass()->isAbstract()) { return false; } return true; } ``` To: ``` /** * A MappedSuperClass or Abstract class cannot be instantiated. * * @param ClassMetadata $metadata * * @return bool */ protected function isInstantiable(ClassMetadata $metadata) { if ($metadata->isMappedSuperclass) { return false; } if (!$metadata->getReflectionClass() || $metadata->getReflectionClass()->isAbstract()) { return false; } return true; } ``` I've tried it and it seems to work fine after that - but I don't know if this will affect anything else internally.
manticorp
added a commit
to manticorp/acl
that referenced
this pull request
Sep 7, 2016
Fixes [BUG] Call to member function isAbstract on null in mappedEventSubscriber doctrine:generate:entities This seems to be related to the following issue filed on the doctrine github: doctrine/orm#4459 and the following pull request: doctrine/orm#1345 The generate:entities command uses the ```DisconnectedClassMetadataFactory``` and so ```getReflectionClass``` isn't guaranteed to return anything and can be null, as such the following lines in ```LaravelDoctrine\ACL\Mappings\Subscribers\MappedEventSubscriber``` should be changed: From: ``` /** * A MappedSuperClass or Abstract class cannot be instantiated. * * @param ClassMetadata $metadata * * @return bool */ protected function isInstantiable(ClassMetadata $metadata) { if ($metadata->isMappedSuperclass) { return false; } if ($metadata->getReflectionClass()->isAbstract()) { return false; } return true; } ``` To: ``` /** * A MappedSuperClass or Abstract class cannot be instantiated. * * @param ClassMetadata $metadata * * @return bool */ protected function isInstantiable(ClassMetadata $metadata) { if ($metadata->isMappedSuperclass) { return false; } if (!$metadata->getReflectionClass() || $metadata->getReflectionClass()->isAbstract()) { return false; } return true; } ``` I've tried it and it seems to work fine after that - but I don't know if this will affect anything else internally.
patrickbrouwers
pushed a commit
to laravel-doctrine/acl
that referenced
this pull request
Sep 27, 2016
Fixes [BUG] Call to member function isAbstract on null in mappedEventSubscriber doctrine:generate:entities This seems to be related to the following issue filed on the doctrine github: doctrine/orm#4459 and the following pull request: doctrine/orm#1345 The generate:entities command uses the ```DisconnectedClassMetadataFactory``` and so ```getReflectionClass``` isn't guaranteed to return anything and can be null, as such the following lines in ```LaravelDoctrine\ACL\Mappings\Subscribers\MappedEventSubscriber``` should be changed: From: ``` /** * A MappedSuperClass or Abstract class cannot be instantiated. * * @param ClassMetadata $metadata * * @return bool */ protected function isInstantiable(ClassMetadata $metadata) { if ($metadata->isMappedSuperclass) { return false; } if ($metadata->getReflectionClass()->isAbstract()) { return false; } return true; } ``` To: ``` /** * A MappedSuperClass or Abstract class cannot be instantiated. * * @param ClassMetadata $metadata * * @return bool */ protected function isInstantiable(ClassMetadata $metadata) { if ($metadata->isMappedSuperclass) { return false; } if (!$metadata->getReflectionClass() || $metadata->getReflectionClass()->isAbstract()) { return false; } return true; } ``` I've tried it and it seems to work fine after that - but I don't know if this will affect anything else internally.
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.
... when using entities with joined table inheritance
ConvertMappingCommand and GenerateEntitiesCommand both use the DisconnectedClassMetadataFactory, which allows metadata manipulation without loading the associated classes. Commit a36bea broke these two commands by adding a bailout condition in ClassMetadataFactory::populateDiscriminatorValue which checks $metadata->reflClass->isAbstract(). If the DisconnectedClassMetadataFactory is being used, $metadata->reflClass will always be null, causing a fatal error, "Fatal error: Call to a member function isAbstract() on null".
This commit adds a check to see if $metadata->reflClass is set before checking isAbstract.