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

[UOW] Only get change set from documentChangeSets if it exists. #740

Closed
wants to merge 1 commit into from

Conversation

rjelierse
Copy link

This prevents notices of Undefined index: 0000000062aacd13000000000d79d12e in documents that use CHANGETRACKING_NOTIFY and for some reason do not have an entry in UnitOfWork::documentChangeSets.

@jwage
Copy link
Member

jwage commented Jan 7, 2014

I am not sure if this is a proper fix. We should probably identify why the index is not set instead of silencing it.

@rjelierse
Copy link
Author

You're probably right. After a day of stepping through my tests using XDebug I was unable to identify the cullprit. I can try to set up a test suite that mimics my situation?

@jwage
Copy link
Member

jwage commented Jan 8, 2014

That would be great.

This prevents notices of 'Undefined index: 0000000062aacd13000000000d79d12e'
in documents that use CHANGETRACKING_NOTIFY.
@malarzm
Copy link
Member

malarzm commented Mar 12, 2015

@rjelierse I know some time has passed but any news on test suite? :)

@malarzm
Copy link
Member

malarzm commented Apr 1, 2015

Closing the issue for now, if anyone would have more information on it please reopen

@malarzm malarzm closed this Apr 1, 2015
@rbrtbn
Copy link
Contributor

rbrtbn commented Sep 22, 2015

I also ran into this problem and it happens when you flush a single (managed) document for the second time without property changes since the first flush and the document uses the NOTIFY change tracking policy. After the first flush the UnitOfWork::$documentChangeSets is cleared (at the end of the UnitOfWork::commit() method) and when you try to flush for the second time (without changing any properties on the document) than this notice gets thrown. If you use a different change tracking policy everything works fine in this scenario, so this should too.

@jwage The required index (the oid of the document) on the UnitOfWork::$documentChangeSets property doesn't get set because no one calls the UnitOfWork::propertyChanged() method on the flushed document beforehand. That's what creates the index and I think it is perfectly acceptable, if you don't call this method between two flushes and you should only get an empty change set during the second flush doing so. So I think this is the proper fix for this problem.

What should be done to reopen and accept this pull request?

@malarzm
Copy link
Member

malarzm commented Sep 23, 2015

@banrobert it seems like you've nailed the problem, best thing to do is create PR with test case (and this fix included if it indeed solves the problem) :)

@rbrtbn
Copy link
Contributor

rbrtbn commented Sep 23, 2015

OK, I'll create a new PR next week with a test case.
I also found an other problem with the UnitOfWork::commit() method when using the NOTIFY change tracking policy (it clears the whole UnitOfWork::$documentChangeSets when flushing only one document and by this all the subsequent flushes become useless). I'll also include the fix for that in the PR as these two problems seem to be connected to each other. I'll write a test case for this too.

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

Successfully merging this pull request may close these issues.

4 participants