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

Make *EventArgs generic #273

Merged
merged 1 commit into from
Apr 6, 2022

Conversation

derrabus
Copy link
Member

@derrabus derrabus commented Apr 4, 2022

Implementations of the persistence contracts usually use a more specific contract for their own ObjectManager implementation, like EntityManagerInerface for the Doctrine ORM.

This PR allows implementations to be more specific about the ObjectManager object returned by event objects. That in turn should help us to provide more precise types in the ORM.

@derrabus derrabus added this to the 2.5.0 milestone Apr 4, 2022
@derrabus derrabus requested a review from greg0ire April 4, 2022 21:33
@malarzm
Copy link
Member

malarzm commented Apr 4, 2022

FWIW there's a base event in ODM to force DocumentManager: https://github.com/doctrine/mongodb-odm/blob/4cc90c39f8169078b0c6825384ccfea82559535d/lib/Doctrine/ODM/MongoDB/Event/ManagerEventArgs.php#L17 or sometimes we were just adding that method to our event classes. I'm not sure if we'd be removing those methods with your PR merged, but surely can slap few more pslam annotations to classes :)

@derrabus
Copy link
Member Author

derrabus commented Apr 4, 2022

We have kind-of the same in ORM as well:

https://github.com/doctrine/orm/blob/1ffb9152f76bd3d332d5275bd2272d3984bcf3d6/lib/Doctrine/ORM/Event/LifecycleEventArgs.php#L28-L36

In your case, this PR would allow you to remove the assert($dm instanceof DocumentManager); hack.

@malarzm
Copy link
Member

malarzm commented Apr 4, 2022

Yeah, removing an assert is always welcome :) Will you be considering dropping getEntityManager() methods from ORM's events or are they to stay as well?

@derrabus
Copy link
Member Author

derrabus commented Apr 4, 2022

I think, we should keep them because to an ORM user, the term "entity manager" is probably more familiar than "object manager".

@derrabus derrabus merged commit 5faf904 into doctrine:2.5.x Apr 6, 2022
@derrabus derrabus deleted the improvement/generic-events branch April 6, 2022 12:05
derrabus added a commit to derrabus/persistence that referenced this pull request Apr 6, 2022
* 2.5.x:
  Make *EventArgs generic (doctrine#273)
derrabus added a commit to derrabus/persistence that referenced this pull request Apr 6, 2022
* 2.5.x:
  Flag event templates as covariant (doctrine#276)
  Make *EventArgs generic (doctrine#273)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants