Array Equality Checking for reordering in UnitOfWork about phpcr-odm HOT 5 CLOSED

thanks a lot for this detailed report! yeah, good old UnitOfWork 🤔

do i understand correctly that the problem is "2015" vs 2015? indeed: https://3v4l.org/TIeQW - just great, thanks PHP :-(

as those keys are always phpcr node names, we know they are strings. imho we should change the comparison to != instead of !== https://3v4l.org/oLHhb . casting all array elements to string before comparing could be relatively expensive, as it would be done a lot on large document trees.

if you want to work on this, is suggest to first try to build a minimal test case that shows the problem, and then change the comparison to non-strict.

from phpcr-odm.

Thanks for the suggestion, I think changing it to != will work just fine. I forked and made the change, and have been using it in my application just fine, and it fixes the issue while still successfully detecting reorders.

But I don't have much experience with UnitTests. I got the UnitOfWork tests to run, but I am not sure how to write a test to cover this.

I would think I would want to:

• make a parent node, with multiple child nodes with integer names.
• persist and commit the node
• then do nothing, call computeChangeSets, and make sure the uow's scheduledUpdates is an empty array.

I think with the current code, that test would fail, but once the strictness was removed it would pass.

I could see an additional test where the same thing happened, but the children are reordered and the test passes if there is a scheduled update.

Any assistance you could provide would be appreciated.

from phpcr-odm.

i guess its technically not a unit test but more of a functional test, because it uses the database and all ;-)

your approach sounds correct to me. the test to add to is called ReorderTest. actually, it should be enough to add a number-name on this test:

from phpcr-odm.

Howdy,

My project finally cleared, and I had time to implement the unit test. You can see it here:
UFTimmy@2c908a3#diff-0fdc105115539a4aae060e42657b1cf2

The test fails with the strict equality checking, but passes once its changed to loose.

Everything look ok, would a pull request with the test and the fix to non strict equality checking be ok?

Thank you!

from phpcr-odm.

yes please, that looks right!

from phpcr-odm.