Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Events #74

Open
nawarian opened this issue Apr 14, 2015 · 26 comments
Open

Events #74

nawarian opened this issue Apr 14, 2015 · 26 comments

Comments

@nawarian
Copy link

Does anyone have the intention to use events with relational?

Yes, events. Like... dbms's triggers... Anyone?

@henriquemoody
Copy link
Member

That would be nice!

Unfortunately I cannot code this features for these days, I'm pretty busy with other stuff and we have just a few active collaborators.

@nawarian
Copy link
Author

can I start it?

@williamespindola
Copy link
Member

You can start this @nawarian
How you intend to do this?

@nawarian
Copy link
Author

ahmmm...

Thought about writing a simple subscribe/dispatch interface for Collections.

Those would have "onInsert", "onUpdate" and "onDelete" dispatchers available
for subscribing.

What you guys think about it?

@henriquemoody
Copy link
Member

Despite this feature have been never requested before, I was thinking about something more flexible, since we are thinking about it now. I was thinking about these events:

  • Pre insert
  • Post insert
  • Pre update
  • Post update
  • etc...

As you see, there are many events and just write methods doesn't look good idea, but using something like EventDispatcher does. Actually, even with just few events, writing new methods will make Mapper just bigger than it already is.

Any thoughts, @Respect/troubleshooters?

@nawarian
Copy link
Author

Agreed.
Would we use a external EventDispatcher/Listener API or create a Respect's?

Is kind of simple, we could create as a module.

@williamespindola
Copy link
Member

I thought better about this implementation and I'm trying to find for your real benefits.
Since triggers are already events dispatched by dbms, the Relation should just create than and not dispatch. To create we can use Respect\Db like this:

$db = new Db(new Pdo('sqlite:mydb.sq3'));
$db->query("CREATE TRIGGER salary_trigger
    BEFORE UPDATE ON employee_table
    REFERENCING NEW ROW AS n, OLD ROW AS o
    FOR EACH ROW
    IF n.salary <> o.salary THEN

    END IF;
;")->executeStatement();

My thinking is correct?

@nawarian
Copy link
Author

How could I dispatch it to a external application?

Think about pub/sub model, for example.
I would like do publish a data insert to my channel, so others can deal with it to.

This event functionality can handle this and make my code more decoupled.

Can't see how to perform the same with using databases's triggers.

@williamespindola
Copy link
Member

So think this is an event dispatcher only. Out of Respect\Relational. And your can use any one like synfony or zend event dispatcher to work with relational. Not a Trigger.

@nawarian
Copy link
Author

Yes, an event dispatcher. But integrated with inserts, updates and deletes being processed with Respect.

Like @henriquemoody just said, not only events to dispatch after making changes, but before as well.

I've made it myself by extending Mapper class and overwriting the "flushSingle" method. It would be a nice feature to Relational.


Thinking about other frameworks... maybe forcing an event dispatcher interface wouldn't be nice, since some uses "on", others use "addEventListener", some uses "emit" and others "dispatch", some uses events as method-names (and even Interfaces for listeners/dispatchers) and others strings for identifying them...

@nawarian
Copy link
Author

nawarian commented May 4, 2015

Ping...

@henriquemoody
Copy link
Member

I'm sorry by my late.

I was thinking about it, there are many reasons to do inside the library.

As @williamespindola already said, there are many things we can do using an external event dispatcher, but it depends of what you want to do; if you want to make some profiling, for example, or just perform an action after every flush, wrapping everything will be a PITA.

I think it can be really useful but I have no concrete idea about how it should be done inside Relational.

Actually, TBH, I'm not sure what should be done in Relational and what should be make in Data, since it was splitted Relational.

@nawarian, I'm really receptive for any pull request you want to send, I can also make some code review of it but I probably will not do this feature, despite I think it's a great idea.

@williamespindola
Copy link
Member

My concern is. I think that Respect\Relational is not a large lib like Doctrine but have a Power like Doctrine. If we take a look Data, DB, Sql are modules, and following this thought Events must be too.

@nawarian
Copy link
Author

nawarian commented May 7, 2015

@henriquemoody I could start it...

@williamespindola Totally agreed.

Guys... creating it as a module would be great, but I don't think it would be possible since Relational\Mapper handles our main events such as Insert, Update and Delete (take a look at line 71, method flushSingle).

Executing this task would be easy by just inserting something around there. But this module could not be attachable, it should be treated as a Respect\Relational dependency.

Do you guys agree?

@alganet
Copy link
Member

alganet commented May 8, 2015

It could be a module proxying events between Respect\Data and Respect\Relational.

There is a pseudo-interface to Respect\Relational. Things like the magic interface and querying are database independent, mostly by the AbstractMapper.

So, the current structure looks like this:

  • Respect\Data\AbstractMapper handles interface and API, including the concrete (using Respect\Data\Collections*) and magic.
  • Respect\Relational\Mapper extends the interface and implements the database specific methods, binding them with Respect\Relational\Db and Respect\Relational\Sql.

An example of another component in this architecture is Respect\Structural. it is not complete, but implements the basics for a MongoDB database binding for the Respect\Data API.

Events would be interesting right at the interface and binding level, so they should be provided outside driver specific implementations. They should talk directly to the AbstractMapper.

The idea is to add a new component to the formula:

  • Respect\Data\AbstractMapper handles interface and API, including the concrete (using Respect\Data\Collections*) and magic.
  • Respect\Relational\Mapper extends the interface and implements the database specific methods, binding them with Respect\Relational\Db and Respect\Relational\Sql.
  • Respect\NEWCOMPONENT can be used optionally to capture interface (calls to the mapper) and bindings (calls from the abstractmapper to drivers).

Without new component (actual current usage):

$relational = new Respect\Relational\Mapper($pdo);
$structural = new Respect\Structural\Mapper($mongoconnection);

With optional new component:

$relational = new Respect\NEWCOMPONENT\EventProxyMapper(new Respect\Relational\Mapper($pdo));
$structural = new Respect\NEWCOMPONENT\EventProxyMapper(new Respect\Structural\Mapper($mongoconnection));

I suggest this mainly because we spent some effort separating Data from Relational, maybe we should this as a feature now that packages are working. About the interface to actually deal with events, I'm widely open to any implementation.

@williamespindola
Copy link
Member

+1 to module proxyng like that

@nawarian
Copy link
Author

nawarian commented May 8, 2015

Sounds fair.
I'll get onto it.

@felipecwb
Copy link
Member

👍 to proxyng

@nawarian
Copy link
Author

nawarian commented May 8, 2015

So... I was wondering... maybe a event manager package would be a nice start.
Making it decoupled from Relational or Data, it could be used with another projects.

Something like Respect\Event\Manager class:

$em = new Respect\Event\Manager;
$em->on( 'some.event', $aCallback );
$em->dispatch( 'some.event', array( 'argument one' ) );

After that, events about persistence could simply use this event management object.

@nawarian
Copy link
Author

Right here we've been made some implementation to the event business.

It was created under Respect\Data as a Event namespace, but probably it should be moved.
Additional implementations could be done to be more user-friendly.

But I think it solves the problem.

Anyone else?

@williamespindola
Copy link
Member

Great, but I think the new component work out of Respect\Data. Respect\NEWCOMPONENT
Take at look on Gaigalas's code:

$relational = new Respect\NEWCOMPONENT\EventProxyMapper(new Respect\Relational\Mapper($pdo));
$structural = new Respect\NEWCOMPONENT\EventProxyMapper(new Respect\Structural\Mapper($mongoconnection));

@Respect/troubleshooters I think right?

@nawarian
Copy link
Author

I totally agree... In my opinion there would be actually two new components: The event manager and the evented mapper.

But I can't create them in here. You guys would have to create a new repo or something.


About the implementation: a friend of mine told me about a better way to perform thoses events, tell me what you think about it:

// EventedMapper requires a Respect\Data\AbstractMapper instance
$mapper = new EventedMapper(new Respect\Relational\Mapper($db));

// So, there's a collection called person. I wan't to listen every time before a insert:
$mapper->on('insert')->where('person')->when('before')->do($callback);

@williamespindola
Copy link
Member

You can create this repo in your Github account and transfer the ownership to Respect account.

About the implementation. If i can do that without fluent style too, I totally agree.

@nawarian
Copy link
Author

nawarian commented Dec 6, 2015

I've implemented it (better late than never) under this repo: https://github.com/nawarian/EventProxy

But I can't transfer ownership to you since I don't have admin rights under Respect organization.
I see this one as a fresh start but not a complete solution.
Indeed I didn't write any tests too, maybe next year I could create those or someone would ;)

@wesleyvicthor
Copy link
Member

@nawarian cool implementation dude. I think we will freeze this for now till we get tests and a proper review, I noticed few SOLID concepts breaks, plus we will discuss if that layer is a good piece to be part of the relational code base.

@nawarian
Copy link
Author

nawarian commented Dec 7, 2015

Thanks for the review, @wesleyvicthor .
About SOLID concept breaks, I don't see it as something bad right now, refactoring is always possible and it wasn't that bad ^^

My main concern right now is about old and new values, since Respect\Data keeps objects under $tracked, $new, $changed and $removed byref it is almost impossible to track old and new values for comparison. Things like $mapper->on('pre.update', function($old, $new, $collection) {}); aren't possible with our current Respect\Data implementation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants