Uploaded image for project: 'Doctrine 2 - ORM'
  1. Doctrine 2 - ORM
  2. DDC-73

Lifecycle Events only work with undocumented @HasLifecycleCallbacks in Annonation classes

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Major
    • Resolution: Fixed
    • Affects Version/s: None
    • Fix Version/s: 2.0-ALPHA3
    • Component/s: Mapping Drivers
    • Security Level: All
    • Labels:
      None

      Description

      When using Docblock Annotations, Lifecycle Events don't work as described here:
      http://www.doctrine-project.org/documentation/manual/2_0/en/events

      Functions with e.g. "@PrePersist" annonations are not called unless the class has "@HasLifecycleCallbacks" annotation in its class header docblock.

      This behavior should be either documented or changed (no need for the @HasLifecycleCallbacks annotation)

        Activity

        Hide
        lucasts Lucas Stephanou added a comment -

        Working patch to trunk(r6683)

        WIth this patch the foreach ran every time, but as metadata are cached, I guess that will not be a problem.

        Show
        lucasts Lucas Stephanou added a comment - Working patch to trunk(r6683) WIth this patch the foreach ran every time, but as metadata are cached, I guess that will not be a problem.
        Hide
        romanb Roman S. Borschel added a comment -

        This should rather be fixed in the documentation instead. I think @HasLifecycleCallbacks is useful not only to save some overhead but also as a visual clue to quickly see whether a class has lifecycle callbacks without looking at all methods.

        Show
        romanb Roman S. Borschel added a comment - This should rather be fixed in the documentation instead. I think @HasLifecycleCallbacks is useful not only to save some overhead but also as a visual clue to quickly see whether a class has lifecycle callbacks without looking at all methods.
        Hide
        guilhermeblanco Guilherme Blanco added a comment -

        I think the opposite.

        Overhead will only exist during metadata creation.
        It's then irrelevant since we strongly suggest a cache mechanism at the top of metadata information.

        I'm +1 to commit suggested patch.

        Show
        guilhermeblanco Guilherme Blanco added a comment - I think the opposite. Overhead will only exist during metadata creation. It's then irrelevant since we strongly suggest a cache mechanism at the top of metadata information. I'm +1 to commit suggested patch.
        Hide
        romanb Roman S. Borschel added a comment -

        Its far from irrelevant. Everything should be as fast as possible and during development you usually dont use APC/memcache as metadata cache because you would have to clear it manually after every change. Its simply not worth this absolutely minor convenience that we iterate over all methods of all entity classes just to look for lifecycle callbacks.

        Again, this needs to be fixed in the documentation.

        Show
        romanb Roman S. Borschel added a comment - Its far from irrelevant. Everything should be as fast as possible and during development you usually dont use APC/memcache as metadata cache because you would have to clear it manually after every change. Its simply not worth this absolutely minor convenience that we iterate over all methods of all entity classes just to look for lifecycle callbacks. Again, this needs to be fixed in the documentation.
        Hide
        romanb Roman S. Borschel added a comment -

        And to make it even clearer: Not even a metadata cache is the ultimate thing for performance since the unserialization is not very fast either. So please dont ever think something is irrelevant. This is php and php is pretty slow.

        Show
        romanb Roman S. Borschel added a comment - And to make it even clearer: Not even a metadata cache is the ultimate thing for performance since the unserialization is not very fast either. So please dont ever think something is irrelevant. This is php and php is pretty slow.

          People

          • Assignee:
            romanb Roman S. Borschel
            Reporter:
            nicokaiser Nico Kaiser
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved: