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

[RFC] "Nested" calls to DocumentManager::flush() #1051

Closed
alcaeus opened this issue Mar 19, 2015 · 7 comments
Closed

[RFC] "Nested" calls to DocumentManager::flush() #1051

alcaeus opened this issue Mar 19, 2015 · 7 comments
Assignees
Labels
Milestone

Comments

@alcaeus
Copy link
Member

alcaeus commented Mar 19, 2015

Hello all,

currently it is possible to flush() the DocumentManager multiple times while it is already flushing. One example is a lifecycle event subscriber that calls some method that inserts another document and flushes the DocumentManager.
Currently, this case is not handled, the DocumentManager will happily run the nested flush() operation which can cause a bunch of issues. My suggestion is that the DocumentManager prevents such calls, unless such functionality breaks existing behavior. Comments?

@gruberro
Copy link

I'd like to see this change, as this would prevent me from a lot of trouble 👍

@malarzm
Copy link
Member

malarzm commented Mar 19, 2015

I remember using this to flush single document during bigger flush and nothing bad happened to me in the meantime

@alcaeus
Copy link
Member Author

alcaeus commented Mar 19, 2015

Let's say there is an event subscriber that listens to the onFlush event. That subscriber loops all changesets and does something if a specific field has changed. You've also got a second subscriber that listens to the postPersist event. That one creates a new document to be inserted and calls flush($document). This will cause the onFlush handler to be called a second time, and it will actually re-do whatever it has done for the first document since the document is still in the changesets (it will only be removed from there after the postFlush event has been triggered.
I can try to gist a use case that provokes this if you want.

@malarzm
Copy link
Member

malarzm commented Mar 19, 2015

I'm not saying it can't break, just throwing my 2 cents saying that I'm currently using flush within a flush :)

@alcaeus
Copy link
Member Author

alcaeus commented Mar 19, 2015

Ah, sorry :)
Yeah, I was thinking about how it could be made optional or whether the flushes could be correctly isolated. Technically, if the second flush tracked its own set of changes, it would be no problem.

@alcaeus
Copy link
Member Author

alcaeus commented Sep 18, 2017

Long time no see - just referencing the comments over in the ORM repository:

Calling flush() inside a lifecycle listener is indeed not allowed.

(Source

Given that, I'd try to come up with a solution for 2.0 along with a deprecation notice for 1.2.

@alcaeus alcaeus added this to the 1.2 milestone Sep 18, 2017
@alcaeus alcaeus added Feature and removed Idea labels Sep 18, 2017
@alcaeus alcaeus self-assigned this Sep 18, 2017
@alcaeus alcaeus modified the milestones: 1.2, 2.0.0 Sep 18, 2017
@malarzm
Copy link
Member

malarzm commented Mar 14, 2018

#1722 prevents user from flushing more than once at a time

@malarzm malarzm closed this as completed Mar 14, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants