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

DDC-3344: Flush on a specific entity is not correctly cascaded to associated entities #4142

Closed
doctrinebot opened this issue Oct 10, 2014 · 11 comments
Assignees

Comments

@doctrinebot
Copy link

Jira issue originally created by user pavel.horal:

In a setup with a simple entity associated entity, the cascade is not performed when flushing the parent entity. This violates contract specified by EntityManager#flush($entity) PHPDoc:

bq. If an entity is explicitly passed to this method only this entity and the cascade-persist semantics + scheduled inserts/removals are synchronized.

It seems that the reason behind this is UnitOfWork#computeAssociationChanges, which expects that a #computeChangeSets is called elswehere (which it is not when flushing a specific entity):

bq. MANAGED associated entities are already taken into account during changeset calculation anyway, since they are in the identity map.

This makes flushing and cascading very confusing. Also I believe this might be a cause of other issues, like DDC-3113.

@doctrinebot
Copy link
Author

Comment created by @Ocramius:

Doctrine\ORM\EntityManager#flush() is not supposed to be called with a specific entity when there are cascade operations involved.

The optional argument has to be only used for performance reasons, but can indeed break things.

@doctrinebot
Copy link
Author

Comment created by pavel.horal:

Thank you for the reply. I am probably (and possibly other people) misinterpretting the PHPDoc. Can you explain what is meant by "and the cascade-persist semantics"?

@doctrinebot
Copy link
Author

Comment created by @Ocramius:

[pavel.horal] I don't really understand that bit myself, but git blame states that the comment was introduced in 5d3298e by [beberlei].

Maybe it just needs a docblock rewrite

@doctrinebot
Copy link
Author

Comment created by pavel.horal:

Commit is linked with DDC-720 . Again, that gives me an impression that events should be cascaded:

bq. In a nutshell, this change would mean that flush() can optionally accept an entity as an argument. When that happens, the resulting changeset will be limited to that entity and any entity reachable from it.

@doctrinebot
Copy link
Author

Comment created by @Ocramius:

{quote}Again, that gives me an impression that events should be cascaded{quote}

No, the entire flush($entity) functionality is just to limit the entire Doctrine\ORM\UnitOfWork operations over the single object: it's expected behavior.

In general, you should use flush($entity) only if you have very high priority performance optimizations, as it was never meant to be reliable API when using listeners.

I'd even suggest deprecating it for removal in the next major release, as it has only caused issues so far.

@doctrinebot
Copy link
Author

Comment created by jkleijn:

I also ran into this. I foolishly made the assumption that flush($entity) was intended to flush an entity and all associations (following "persist" cascade rules).

Intended or not, I firmly believe this should be the default behavior (or create a second set of cascade rules, eg "flush"). The reasoning behind it is one of encapsulation. Client code calling flush() cannot know what is flushed unless it owns all entity interaction, which is both undesirable and unrealistic. A single service may very well own all interactions on a specific object and its associations though.

@dave-newson
Copy link

Just tripped over this with an entire team of people, then found this bug report.

What's the intended fix for this bug? Options appear to be:

  • Deprecate the specific entity flush capability in future versions
  • Make EntityManager#flush($entity) perform cascade persist options on $entity when it's provided
  • Remove the misleading PhpDoc block

@lcobucci
Copy link
Member

@dave-newson #6118

@El-Sam
Copy link

El-Sam commented Sep 14, 2017

I just had the same issue as @dave-newson.

Is there any updates on this issue.

@lcobucci
Copy link
Member

lcobucci commented Oct 8, 2017

@El-Sam as mentioned on #4142 (comment) we already removed the feature on v3. So to make it short, don't pass any parameter to EntityManager#flush().

@Majkl578
Copy link
Contributor

Closing, flushing specific entities is going to be removed in Doctrine 3.0.

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

No branches or pull requests

6 participants