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 #1

Merged
merged 2 commits into from
Aug 28, 2013
Merged

Cleaned up events.rst #1

merged 2 commits into from
Aug 28, 2013

Conversation

caponica
Copy link
Owner

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.
@caponica
Copy link
Owner 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
Owner Author

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

Addresses all comments made so far, except the one about persists/updates
@caponica caponica merged commit c715d91 into master Aug 28, 2013
caponica pushed a commit that referenced this pull request Aug 10, 2014
Update xml mapping driver and schema to work with embeddables
caponica pushed a commit that referenced this pull request Aug 10, 2014
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.

1 participant