Allow custom type objects to trigger value updates

[Artusamak]

Custom types are not correctly supported due to the fact that UnitOfWork::computeOrRecomputeChangeSet() uses===operations without checking the objects inner values.
This leads to missed update values.

The added test testValueObjectChangeSets is red if the changeset detection fragment is not altered.

There is also a @TODO added to track the suggestion to use \Doctrine\Common\Comparable interface. The problem is that this interface is part of doctrine/commons which is not required by the package at the moment.

Q	A
Type	improvement
BC Break	no
Fixed issues	#1482

[GromNaN]

Could you have a look at the failing tests?

[Artusamak]

You've been faster light for review! Thanks.

Is it considered to add a .editorfile matching your code style expectations? Fixing manually is not the easiest solution 😅.

[Artusamak]

Thanks to SPL there is now a way to detect the object content update which should do the trick.

[GromNaN]

Thanks for the contribution and the test that clearly reproduces the issue! I have a few comments below.

[GromNaN]

spl_object_hash($a) === spl_object_hash($b) is functionally identical to $a === $b for two simultaneously live objects (PHP docs: the hash can only be reused after the object is destroyed). This import can be dropped.

[GromNaN]

This restructuring introduces a regression for dirty ReferenceMany collections. When a PersistentCollection is dirty but its instance is the same object stored in $originalDocumentData ($orgValue === $actualValue — the normal case), the new code falls through to line 777 and executes continue, silently skipping the dirty collection instead of detecting it as changed.

EmbedMany collections are unaffected by luck — they go through the association changeset loop (~line 870). ReferenceMany collections are explicitly excluded there (if (isset($mapping['reference'])) { continue; }), so this block is their only detection path.

PR #3021 adds a test that demonstrates this: it passes on 2.17.x and fails with this patch applied.

Also, the comment // consider dirty collections as changed as well is now on the continue of the clean collection branch — the opposite of what it says.

Since spl_object_hash is equivalent to === for live objects, it does not help with value comparison either. I'd suggest keeping the original structure and adding a == check as a new elseif. PHP's == compares objects by value: same class and all properties (public and private) equal — https://3v4l.org/sBNOH:

// skip if value has not changed
if ($orgValue === $actualValue) {
    if (! $actualValue instanceof PersistentCollectionInterface) {
        continue;
    }

    if (! $actualValue->isDirty() && ! $this->isCollectionScheduledForDeletion($actualValue)) {
        // consider dirty collections as changed as well
        continue;
    }
} elseif (is_object($orgValue) && is_object($actualValue) && $orgValue == $actualValue) {
    continue;
}

This preserves the dirty-collection behaviour and fixes the value object case without any new dependency. The TODO on Comparable remains valid for edge cases where == is not sufficient, but is not a blocker.

[GromNaN]

CustomValueObjectChild implements no convertToPHPValue/convertToDatabaseValue, so the test doesn't quite cover a realistic custom type scenario where the type actually converts values between PHP and MongoDB.

[GromNaN]

prop1 is declared int, so this should compare against 12 (no quotes). The assertion should use assertSame.