Bug #28638

Signals can't be defined in abstract classes

Added by Bastian Waidelich about 4 years ago. Updated almost 4 years ago.

Status:Resolved Start date:2011-08-01
Priority:Should have Due date:
Assigned To:Robert Lemke % Done:

100%
Category:AOP
Target version:TYPO3 Flow Base Distribution - 1.0.0
PHP Version: Complexity:
Has patch: Affected Flow version:

Description

The AOP ProxyClassBuilder produces invalid code when signals are defined in an abstract class.

Steps to reproduce:

AbstractSignalTest.php
 1<?php
 2namespace Foo\Bar;
 3abstract 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
2namespace Foo\Bar;
3class 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 almost 4 years 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

#1 Updated by Mr. Hudson about 4 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

#2 Updated by Bastian Waidelich about 4 years ago

  • Priority changed from Should have to Must have

#3 Updated by Sebastian Kurfuerst almost 5 years ago

  • Priority changed from Must have to Should have

#4 Updated by Robert Lemke almost 4 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.

#5 Updated by Robert Lemke almost 4 years ago

  • Target version changed from 1.0 beta 2 to 1.0.0

#6 Updated by Mr. Hudson almost 4 years ago

Patch set 2 of change I6480321c117dc0eb264fda45a952d27505156f82 has been pushed to the review server.
It is available at http://review.typo3.org/3971

#7 Updated by Bastian Waidelich almost 4 years ago

  • Status changed from Under Review to Resolved
  • % Done changed from 0 to 100

#8 Updated by Christopher Hlubek almost 4 years 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)?

#9 Updated by Christopher Hlubek almost 4 years ago

  • Status changed from Resolved to Needs Feedback

#10 Updated by Bastian Waidelich almost 4 years ago

  • Assigned To changed from Bastian Waidelich to Robert Lemke

#11 Updated by Karsten Dambekalns almost 4 years ago

  • Status changed from Needs Feedback to Resolved

#12 Updated by Karsten Dambekalns almost 4 years ago

The regression has been fixed with Icf6bdf3f789162afbca61d7cf915dbb7ecd583d5 (https://review.typo3.org/#change,5409)

Also available in: Atom PDF