Bug #28638
Signals can't be defined in abstract classes
| Status: | Resolved | Start date: | 2011-08-01 | |
|---|---|---|---|---|
| Priority: | Should have | Due date: | ||
| Assignee: | Robert Lemke | % Done: | 100% |
|
| Category: | AOP | |||
| Target version: | TYPO3 Flow Base Distribution - 1.0.0 | |||
| PHP Version: | Complexity: | |||
| Has patch: | FLOW3 version affected: | |||
| Votes: | 0 |
Description
The AOP ProxyClassBuilder produces invalid code when signals are defined in an abstract class.
Steps to reproduce:
AbstractSignalTest.php1 <?php 2 namespace Foo\Bar; 3 abstract class AbstractSignalTest { 4 5 public function testSignal() { 6 $this->emitTestSignal(); 7 } 8 9 /** 10 * @signal 11 */ 12 public function emitTestSignal() {} 13 } 14 15 ?>
SignalTest.php1 <?php 2 namespace Foo\Bar; 3 class SignalTest extends AbstractSignalTest { 4 5 } 6 ?>
In some controller:1 $signalTest = new \Foo\Bar\SignalTest(); 2 $signalTest->testSignal();
Result:
#1: Notice: Undefined index: emitTestSignal in Development\Cache\Code\FLOW3_Object_Classes\Foo_Bar_AbstractSignalTest.php line 98
The problem is, that the proxy code tries to access $this->FLOW3_AOP_Proxy_targetMethodsAndGroupedAdvices which is declared private.
Associated revisions
[BUGFIX] Proxy code of advised abstract classes don't produce errors
The AOP ProxyClassBuilder produced invalid code when signals were
defined in an abstract class. This was due to invalid proxy class
code and a wrong initialization procedure.
This change fixes the issue by making sure that the advice information
is also built for parent proxy classes when sub classes are used.
Additionally this change set contains functional tests for the
AOP proxy and the signal slot mechanism.
Change-Id: I6480321c117dc0eb264fda45a952d27505156f82
Fixes: #28638
History
Updated by Mr. Hudson almost 2 years ago
- Status changed from Accepted to Under Review
Patch set 1 of change I6480321c117dc0eb264fda45a952d27505156f82 has been pushed to the review server.
It is available at http://review.typo3.org/3971
Updated by Bastian Waidelich almost 2 years ago
- Priority changed from Should have to Must have
Updated by Sebastian Kurfuerst almost 2 years ago
- Priority changed from Must have to Should have
Updated by Robert Lemke almost 2 years ago
- Target version changed from 1.0 beta 1 to 1.0 beta 2
As far as I can see, the problem is not limited to just signals, but any advised method of an abstract class. If so, we should rephrase / rename this issue.
Updated by Robert Lemke almost 2 years ago
- Target version changed from 1.0 beta 2 to 1.0.0
Updated by Mr. Hudson over 1 year ago
Patch set 2 of change I6480321c117dc0eb264fda45a952d27505156f82 has been pushed to the review server.
It is available at http://review.typo3.org/3971
Updated by Bastian Waidelich over 1 year ago
- Status changed from Under Review to Resolved
- % Done changed from 0 to 100
Applied in changeset 43f08cbd553613dea9c927c297ab677440806f04.
Updated by Christopher Hlubek over 1 year ago
I have some kind of a regression with this after further testing (didn't notice this at first):
If a class uses a magic __call method to handle dynamic function calls, a is_callable('parent::FLOW3_AOP...') will always return TRUE, such that an object receives a call with FLOW3_AOP_Proxy_buildMethodsAndAdvicesArray. This could be misleading and caused problems with the SOAP package for me. A quick fix is to filter such method calls (e.g. starting with "FLOW3_AOP") inside a custom "__call" method, but I consider this a quick hack and not a long-term solution.
Can we detect somehow, that a parent really has this method during compilation (e.g. reflection)?
Updated by Christopher Hlubek over 1 year ago
- Status changed from Resolved to Needs Feedback
Updated by Bastian Waidelich over 1 year ago
- Assignee changed from Bastian Waidelich to Robert Lemke
Updated by Karsten Dambekalns over 1 year ago
- Status changed from Needs Feedback to Resolved
Updated by Karsten Dambekalns over 1 year ago
The regression has been fixed with Icf6bdf3f789162afbca61d7cf915dbb7ecd583d5 (https://review.typo3.org/#change,5409)