add base events class

[jdeniau]

Hi,

All doctrine projects implements events. A lot of them are similars, but there is no base class defining common Events.

I tried to gathen them all in this PR. I looked into CouchDb-orm, doctrine-orm, doctrine-mongodb-odm and phpcr-odm.

I only added events present in all packages, but CouchDB is the only one to not implement some events implemented in every other packages (postPersist, loadClassmetadata, preFlush, etc.).

[doctrinebot]

Hello,

thank you for creating this pull request. I have automatically opened an issue
on our Jira Bug Tracker for you. See the issue link:

http://www.doctrine-project.org/jira/browse/DCOM-250

We use Jira to track the state of pull requests and the versions they got
included in.

[stof]

the events are related to the persistence layer only. So if we provide constants in doctrine common for the event names, they should live somewhere in the Persistence subnamespace, not at the root

[deeky666]

agree with @stof it should live in Doctrine\Common\Persistence.

[deeky666]

Can you please insert the Doctrine file doc comment here? You can copy it from some other class.

[stof]

I would actually include the other events as well, even if the CouchDB ODM does not trigger them currently (the CouchDB ODM is not very actively maintained AFAIK

[deeky666]

@stof do you think it would be good to add a private constructor here just like in ORM events for example so that the class is not instantiable?

[jdeniau]

@deeky666 This class could be an interface instead, as interfaces can contains constants.

[jdeniau]

I updated the code in order to match your recommendations.

I made it an interface as the class can not be instantiated and made the constant names PSR2 complaint.

I added the missing events, as preLoad found in MongoODM. Package that does not implements it can simply not use it, but if fixes the name as a standard.

I did not include phpcr list so, like postBindTranslation, etc. because it does not seems global. (unlike preLoad).

[deeky666]

Isn't this description a bit misleading here? I thought the purpose of this container is to provide common persistence events in general and not ORM specific?

[stof]

yeah, it should saypersistence events. The phpdoc is probably related to a copy-paste which has not been updated

[jdeniau]

Should I leave entity (really close to the ORM) or replace it by something else (object, document) ?

[stof]

We use entity in the common API in other places (entity is not tied to RDBMS btw. The fact that ODMs are using document is to be closer to the underlying API, but they could have kept entity)

[jmikola]

The fact that ODMs are using document is to be closer to the underlying API, but they could have kept entity

In hindsight, they should have kept entity. @jwage has mentioned this as a regret when creating ODM. The subtle naming difference has proved little more than a nuisance over the years.

[jdeniau]

OK, Entity it is :-)

On Friday, August 29, 2014, Jeremy Mikola notifications@github.com wrote:

The fact that ODMs are using document is to be closer to the underlying
API, but they could have kept entity

In hindsight, they should have kept entity. @jwage
https://github.com/jwage has mentioned this as a regret when creating
ODM. The subtle naming difference has proved little more than a nuisance
over the years.

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

[deeky666]

Looks good to me know

[jdeniau]

🆙

[guilhermeblanco]

Strong -1 from me.
It's a dependency breakage that Event system (that exists in package common) understand/know about consumers. It should be the opposite side (each consumer to have its own Events class).

[stof]

@guilhermeblanco this class is not added in the Event system. It is added in the Persistence system, which is a consumer of the Event system. The goal here is to extract the common events to Doctrine Common Persistence too.

[jdeniau]

So, what is the state of this ?

It is just a standardization of event names used in every doctrine packages.

It really helps external packages development as the event names are constants in the common package. If the event names are stored in the consumer, you need to : add every consumer as a dependency (just impossible), or test directly the strings (really not safe).

[Majkl578]

Hello,
we've recently split Event Manager component into a separate doctrine/event-manager repository.
If your PR is still relevant, please open new one to the new repository.
Thanks.