Bug #28638

Signals can't be defined in abstract classes

Added by Bastian Waidelich almost 2 years ago. Updated over 1 year ago.

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.php
 1 <?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.php
1 <?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

Revision 43f08cbd
Added by Bastian Waidelich over 1 year ago

[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

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)

Also available in: Atom PDF