3.0: Model.initialize event (originally: Apply Events to TableRegistry)

[robertpustulka]

I was wondering if there could be some way of observing Table creation process.
ie: after every new Table object TR could dispatch an event (using global EventManager) allowing an app or a plugin to hook into this Table and do some stuff with it. This could be helpful if we wanted to apply some changes to every table in registry upon their creation.

I'm not sure it feels right, so I open this for discussion for now.

[lorenzo]

What kind of changers would you apply?

[robertpustulka]

For example I'd like to load some behaviors to every table in my app.

[ADmad]

You can make an AppTable and have all tables extend it. Then do whatever initializations you like in AppTable::initialize().

[robertpustulka]

@ADmad , I can't do that with plugin tables. Or do that from a plugin scope.

[ADmad]

I guess we should just make AppTable a requirement like AppController.

[robertpustulka]

But that's not the point. The point is to have some control over table creation. TableRegistry is static and it's really hard to hook into it.

Besides having an AppTable wouldn't help to attach behaviors from plugin scope.

[lorenzo]

I guess we could have an initialize event like controllers do

[markstory]

This doesn't seem unreasonable to me. What would you call the event?

[lorenzo]

Table.initialize or just Model.initialize to make it similar to the rest

[robertpustulka]

Model.initialize sounds good.

If I'm not mistaken this event could be dispatched by Table itself and could be easely "fetched" by an EventManager singleton?

[markstory]

Firing the event from the table is far more palatable than from the registry. In that case I would go with 'Model.initialize'.

[robertpustulka]

So what about Table::initialize()? Now it feels like that this should be run by EventManager as another callback... But it also would force every developer to update their initialize(array $config) to initialize(Event $event, array $config)

[lorenzo]

No, the method should stay the same. And the event should be called after that method

[ADmad]

@lorenzo That would make Table::initialize() the only callback method which doesn't have Event instance as first param. I would rather cause inconvenience to existing early adopters than have all future users deal with this inconsistency.

[robertpustulka]

@ADmad - this is my point.
I also think this should be dealt with sooner than later. Having all callbacks triggered the same way for clarity. BTW isn't initialize() in its current form a callback which resembles pre 2.1 callbacks :)?

[markstory]

Not the only method. The Controller::initialize() method works the same. I see those initialize() methods as simpler ways too add constructor logic.

[ADmad]

But Controller::initialize() isn't used as an event callback. All callbacks methods that take event instance as an argument should take it as 1st param for consistency imho.

[robertpustulka]

But we have Controller.initialize event... which is mapped to Controller::beforeFilter().
And in Component we have initialize() which is a callback for actual Controller.initialize event...

Isn't that a bit messy?

[ADmad]

@robertpustulka Yeah, Controller::initialize() was added very recently. May be should have named it init() to avoid confusion with the Controller::initialize event name.

[markstory]

If anything is messy it is the component method. I think the initialize() as a constructor hook is a nicer pattern to follow than the event based one. Perhaps we should make the component method follow the controller workflow, and not have a Controller.initialze event listener on Component by default?

[ADmad]

@markstory That sounds reasonable.

[robertpustulka]

But it still leaves Controller.initialize event mapped to beforeFilter(), which does not follow this event-method naming convention.
Maybe we could just leave Controller.startup (and map it to beforeFilter()), as these two events are fired one after another.

[markstory]

The beforeFilter method must be called before the startup event, or many workflows around component setup will no longer work.

[markstory]

Closing, as there is a pull request up now that we can discuss further.