Fix changeset handling for new documents #999
Conversation
When changing a newly persisted document within onFlush(), recomputing the changeset for this document will cause a documentUpdate to be created. As a result, events will be triggered for persist and update at the same time, even though only a persist happened.
|
👍 |
|
@alcaeus it seems that your fix has broken some existing tests |
|
Hmm, I guess I missed those. I'll fix them in the next couple of days. |
alcaeus
changed the title
|
I fixed the failing tests - however, considering they were written to test exactly the kind of behavior this PR aims at fixing, I'm not sure what the consequences would be. I'd really appreciate some feedback on this. |
alcaeus
changed the title
|
I've removed the WIP label since there was no feedback on the old tests. |
| @@ -803,7 +803,9 @@ private function computeOrRecomputeChangeSet(ClassMetadata $class, $document, $r | |||
| : $changeSet; | |||
|
|
|||
| $this->originalDocumentData[$oid] = $actualData; | |||
| $this->documentUpdates[$oid] = $document; | |||
| if ( ! isset($this->documentInsertions[$oid])) { | |||
There was a problem hiding this comment.
Should we check $this->documentUpserts[$oid] as well here?
|
This is fixed in #782 |
|
@alcaeus I understand, but I think patching the one off bugs here and there will end up making things much worse and harder or impossible to fix in the future without BC breaks. I finished the last known issues last night and got the tests passing. I need others to test the patch. I am going to be testing it against my application. |
|
I agree. I'll give it a test run next week. Could you add the unit test contained in this PR to yours? It would check against inserts triggering an update event.
|
|
@alcaeus Yes, I copied the test case over to my branch. |
|
👍 |
This fixes an issue that occurs when modifying a newly persisted document in an onFlush() event handler.
The issue causes the insertion to be treated as an update even though only an insert needs to be done. Thus, postUpdate listeners are called which is not correct.