Hotfix db features with eventfeature

[blanchonvincent]

Fix params in constructor

[weierophinney]

Actually, the original version of this line was fine; it was simply using the ternary shortcut. If $tableGatewayEvent evaluated to null, it would create a new instance; otherwise, it wouldn't.

[blanchonvincent]

Sorry I thought that it would be more clear

[weierophinney]

My only comment at this point is that implementing EventManagerAwareInterface as well as having the EM passed in the constructor could lead to issues of double-injection if the feature class is pulled from the service manager. I'm wondering if we should pass it to the constructor at this point.

@ralphschindler Thoughts?

[weierophinney]

@blanchonvincent The travis-ci failures are due to coding standards; run php-cs-fixer on each of the files you've altered/added.

[ralphschindler]

For this object, EventManager is not an optional dependency, it is required. That is why it is passed in via the constructor.

What is the use case for is pull request, @blanchonvincent ?

[weierophinney]

@ralphschindler I realize that -- but it makes it fall outside the normal usage of the Aware interfaces, which are often used for required dependencies as well. We could certainly add a check elsewhere in the code for a registered EventManager, and throw an exception if missing. Thoughts?

Also, I, too, am curious about the reason behind the change -- @blanchonvincent can you elaborate?

[blanchonvincent]

Hi,

My use case :

For use the event, i would like get my event manager, but no getter for that :

FooTable extends TableGateway and work the EventFeature
{
$eventFeature = $this->getFeatureSet()->getFeatureByClassName('Zend\Db\TableGateway\Feature\EventFeature');
$eventManager = $eventFeature->getEventManager();

$eventManager->attach(....); // attach preInsert or preUpdate for exemple
}

Am i clear ?

[ralphschindler]

Hey @blanchonvincent, let me look into your use case tomorrow, and play with some code samples and get back to you.

[blanchonvincent]

ok :)

[weierophinney]

@ralphschindler This now passes. My concern is still the same: elsewhere in the framework, the eventmanager is set via setter, even when required, as we have an initializer that will inject it. While there's logic in that initializer that prevents re-injection, that's not really the issue I have: my issue is that as written now, the only way to use this class is to provide a factory for it; you cannot simply add it as an invokable to the SM configuration. This puts a larger onus on the end-user, when we should be trying to make things easier.

Can you review, so I can tell @blanchonvincent whether or not additional changes are necessary?

[ralphschindler]

Hey @blanchonvincent,

I've worked up a couple of changes to EventFeature, and some examples to go along with it. The one problem I have is that even though EventFeature consumes an EventManager object, it is not intended to be a service, thus, it really should not be instantiated by the service manager.

If you do decide to instantiate this through the service manager (which I don't advise), you would need to mark it as not-sharable.

The reasoning is that injecting features into an instance of an AbstractTableGatway/TableGateway makes that particular instance of that feature part of the consuming table. It has its own state, particularly suited to that table object. They are a way of extending the base class without the use of inheritance, aka Decorators.

Below is an example of usage:

/~https://github.com/ralphschindler/Zend_Db-Examples/blob/master/example-22.php

And here is the branch that it works against:

/~https://github.com/ralphschindler/zf2/compare/master...hotfix;tablegateway-event-feature

Note: the fixes above also add identifiers, which you can see are demonstrated in the example-22.php

Another sidenote: Depending on your applications architecture and whether or not you plan on sharing your tables as services, you might want to make some custom factories for them. What you see in lines 14-15 (/~https://github.com/ralphschindler/Zend_Db-Examples/blob/master/example-22.php#L14-L15) and then again in lines 41 would serve as the body of the factory for your table services (you'd probably want to do $serviceManager->get('EventManager') to inject into the EventFeature.

[blanchonvincent]

The new EventFeature design is really great ! Better practice.
I like really your work ! Thanks. Impatient to use 2.1 version.

[weierophinney]

Closing in favor of #2688