Resolve #107 - Correctly detect event name in OnEventStrategy

[robertlemke]

This change fixes an issue with OnEventStrategy not correctly
determining the event name if a custom event returns a message name
other than its own class name through the messageName() method.

[prolic]

Thank's for the PR.

First, there are some php-cs issues, see travis build.
Second, when you return "Result returned by messageName()" as message name, the invoked handler would be "onResult returned by messageName()", see https://github.com/prooph/service-bus/blob/master/src/Plugin/InvokeStrategy/OnEventStrategy.php#L42.
This means that when you return any message name as string containing illegal characters, like " ", "." and so son, the invoke method of the OnEventStrategy will fail.

Any ideas how to solve this? Perhaps a distinct OnEventStrategy is needed for this case. /cc @codeliner

[robertlemke]

@prolic I wasn't really aiming for a completely custom message name here, I used that string only for testing purposes. Just pushed a follow-up which fixes the CS issue and contains a more realistic message name.

[prolic]

@robertlemke My concern is not about if the message name is realistic or not, but that we need to make assumptions about it's name, that are not validated.

[robertlemke]

@prolic hmm, is this related to this bugfix then? If so, then I probably still don't understand what you mean.

For me, this PR was just about fixing the missing brackets in the ternary operator.

[prolic]

No it's not related to your bugfix. I need to clarify with @codeliner, as i
think we've found one more bug.
Am 05.02.2016 11:56 schrieb "Robert Lemke" notifications@github.com:

@prolic https://github.com/prolic hmm, is this related to this bugfix
then? If so, then I probably still don't understand what you mean.

For me, this PR was just about fixing the missing brackets in the ternary
operator.

—
Reply to this email directly or view it on GitHub
#108 (comment).

[robertlemke]

Allright ;-)

[codeliner]

@robertlemke @prolic I agree. The current implementation could lead to unexpected errors but is a separate issue to solve. I think this PR can be merged, right?