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

Cleaned up events.rst #742

Merged
merged 2 commits into from
Aug 25, 2013
Merged

Cleaned up events.rst #742

merged 2 commits into from
Aug 25, 2013

Conversation

caponica
Copy link
Contributor

Was a mix-up between TestEventSubscriber and EventTest (e.g. the definition of TestEventSubscriber referenced TestEvent::preFoo, which did not exist). To clarify this I've renamed EventTest to TestEvent.

Tried to clarify the text in the Naming Convention section.

Added note that onClear is not a lifecycle callback.

Tried to clarify the definition of Lifecycle Callbacks.

Separated key/value descriptions into XML and YAML parts since the details are different

Added note in Implementing Event Listeners section that since 2.4 you do have access to EntityManager and UnitOfWork from lifecycle callbacks.

Added example about how to use the computed changeset to modify a primitive value in preUpdate section

Added naming convention example to Entity listeners class section

The other changes are typos and small fixes.

Was a mix-up between TestEventSubscriber and EventTest (e.g. the definition of TestEventSubscriber referenced TestEvent::preFoo, which did not exist). To clarify this I've renamed EventTest to TestEvent.

Tried to clarify the text in the Naming Convention section.

Added note that onClear is not a lifecycle callback.

Tried to clarify the definition of Lifecycle Callbacks.

Separated key/value descriptions into XML and YAML parts since the details are different

Added note in Implementing Event Listeners section that since 2.4 you do have access to EntityManager and UnitOfWork from lifecycle callbacks.

Added example about how to use the computed changeset to modify a primitive value in preUpdate section

Added naming convention example to Entity listeners class section

The other changes are typos and small fixes.
@doctrinebot
Copy link

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/DDC-2583

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

@caponica
Copy link
Contributor Author

In addition to the commit notes I have the following queries about this document:


The XML example in section 9.3 should be improved to also trigger doOtherStuffOnPrePersistToo (so that it matches the YML and Annotations examples).

<?xml version="1.0" encoding="UTF-8"?>

<doctrine-mapping xmlns="http://doctrine-project.org/schemas/orm/doctrine-mapping"
      xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
      xsi:schemaLocation="http://doctrine-project.org/schemas/orm/doctrine-mapping
                          /Users/robo/dev/php/Doctrine/doctrine-mapping.xsd">

    <entity name="User">

        <lifecycle-callbacks>
            <lifecycle-callback type="prePersist" method="doStuffOnPrePersist"/>
            <lifecycle-callback type="postPersist" method="doStuffOnPostPersist"/>
        </lifecycle-callbacks>

    </entity>

</doctrine-mapping>

I don't use XML mappings myself so not sure if the correct format is

<lifecycle-callback type="prePersist" method="doStuffOnPrePersist, doOtherStuffOnPrePersistToo"/>

or

<lifecycle-callback type="prePersist" method="doStuffOnPrePersist"/>
<lifecycle-callback type="prePersist" method="doOtherStuffOnPrePersistToo"/>

Let me know and I'll update it accordingly.


In section 9.5 there is no explanation of the difference between listeners and subscribers. Is there any difference beyond the documented difference that the subscribers have a getSubscribedEvents() method compared to specifying the events for the listener when you call EventManager#addEventListener() ?

When would/should you use one or the other? If the answer is beyond the scope of Doctrine's documentation should we at least link to some URLs that discuss it?


In section 9.6.2 it says:

There are no restrictions to what methods can be called inside the
``preRemove`` event, except when the remove method itself was
called during a flush operation.

What are the restrictions during a flush operation?


In section 9.6.6 it says:

You could also use this listener to implement validation of all the fields that have changed. This 
is more efficient than using a lifecycle callback when there are expensive validations to call:

I don't think this is the case any more (since v2.4) since (I think!) you can do the same thing with a preUpdate lifecycle callback.

Please confirm that my understanding is correct and, if so, should I remove the second sentence quoted above?


Section 9.7 could be improved with a quick explanation of the difference between an entity lifecycle callback and an entity listener.

At the moment (to me, having only read this documentation) it's not clear how they are really different or when I should think about using one or the other.


After reading section 9.7.2 it is not clear to me what this is, what it does or when I would want to use it! Is this a required step if I want to use an entity listener or do I just need to follow the syntax from section 9.7.1?

In 9.7.1 the YAML and XML examples seem to bind the listener to the entity, but the PHP example does not. When using annotations (but not XML/YAML) are the steps in 9.7.2 required? Or is it enough to just add the /** @Entity @EntityListeners({"UserListener"}) */ annotation to my entity class?

Or do I always need to register the entity listener via the EntityListenerResolver, even when using XML/YAML?

@caponica
Copy link
Contributor Author

Section numbers referred to above are the ones shown here: http://docs.doctrine-project.org/projects/doctrine-orm/en/latest/reference/events.html

@@ -159,7 +158,7 @@ the life-time of their registered entities.
- prePersist - The prePersist event occurs for a given entity
before the respective EntityManager persist operation for that
entity is executed. It should be noted that this event is only triggered on
*initial* persist of an entity
*initial* persist of an entity (i.e. it does not trigger on future updates).
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it does not trigger on subsequent persists (or something similar)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally I prefer to say "update" since I know this has caught some people out. Perhaps I should change it to
(i.e. it does not trigger on future updates or persists)

or
(in traditional database terms, prePersist can be thought of as triggering on INSERT of the entity, but does not fire when the entity is UPDATED)

Addresses all comments made so far, except the one about persists/updates
@Ocramius
Copy link
Member

Goes in :)

Ocramius added a commit that referenced this pull request Aug 25, 2013
@Ocramius Ocramius merged commit e2a67c2 into doctrine:master Aug 25, 2013
@ghost
Copy link

ghost commented Jun 17, 2015

Could we close these pull requests which are merged? It would make it easier to see all currently open and active pull requests. 👍

@ghost
Copy link

ghost commented Jun 17, 2015

Never mind my previous comment. Seems I had deleted the is:open filter. My bad.

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

Successfully merging this pull request may close these issues.

3 participants