-
-
Notifications
You must be signed in to change notification settings - Fork 291
ProxyGenerator: Disallow creating proxies for abstract and final classes #771
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
ProxyGenerator: Disallow creating proxies for abstract and final classes #771
Conversation
@@ -260,6 +260,8 @@ public function setProxyClassTemplate($proxyClassTemplate) | |||
*/ | |||
public function generateProxyClass(ClassMetadata $class, $fileName = false) | |||
{ | |||
$this->verifyClassValidnessForProxy($class); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
verifyClassCanBeProxied
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I admit I was out of fantasy. 😇 Renamed.
@@ -306,6 +308,17 @@ public function generateProxyClass(ClassMetadata $class, $fileName = false) | |||
rename($tmpFileName, $fileName); | |||
} | |||
|
|||
private function verifyClassValidnessForProxy(ClassMetadata $class) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please add relevant @throws
annotations in the private/public API of the class being affected by the change
dc27414
to
c116421
Compare
@Majkl578 awesome, thanks! |
👌 |
Unfortunately, this breaks Sylius (already fixed in dev-master) - looks like we don't really use an entity which was final, so it has started to break after an update to 2.8. If anyone bumped onto this issue as well, after using this workaround https://github.com/Sylius/SyliusElasticSearchPlugin/pull/55/files#diff-b5d0ee8c97c7abd7e3fa29b9a27d1780R41 it works just fine :) |
Yes, this breaks using https://github.com/moneyphp/money/ as an embeddable. |
Related: doctrine/orm#6625 |
Fixes #770
Related to symfony/symfony#21509
This could theoretically be a BC break, but I can't imagine any such scenario.