Better support of complex Doctrine association graphs

[janklan]

I tried to use this library to copy a subset of an association graph, where I wanted to ignore some entities completely.

The main obstacles I faced was:

1. Persisting the cloned entities can't be achieved with the current way Filters are working
2. Our app has a number of associations along the line of "Organisation -> User[]", "Organisation -> User Group[]" and "User Group -> User[]". While there is a filter that allows to clearing a specific property, it's impractical: I'd have to explicitly list all User[] collections to be cleared (Organisation::$users and UserGroup::$members)

I figured a good way to address the first problem was to add a callback that would simply be executed at the end of the copying process.

The second problem has a combination of use of ReplaceFilter that would set any x::$user association to null, and a modest extension to the DoctrineCollectionFilter, allowing to give it a list of classes that should NOT be copied over.

This means that using the two filters together will prevent any User from being copied to the new set of objects:

$copier->addTypeFilter(new TypeFilter\ReplaceFilter(fn () => null), new TypeMatcher(User::class));
$copier->addFilter(new Filter\Doctrine\DoctrineCollectionFilter([User::class]), new Matcher\PropertyTypeMatcher(Collection::class));

Doing the above effectively stops the graph traversal when a "blacklisted" entity is found, regardless of whether it's located at the owning side (the TypeFilter kicks in) or the inverse side (the DoctrineCollectionFilter does).

As a side note: If the recursiveCopy() wasn't private, I could just extend DeepCopy and override it so that it would also persist whatever is returned from the parent method. That's sadly not the case, hence the callable.

I'm curious if this was an upgrade others might be interested in. If not, that's OK. I'm happy to just fork the package and keep using that fork.

--

For the record, I have been fighting errors caused by the DeepCopy::recursiveCopy attempting to set an ArrayCollection into a property that was not a collection (it was a Project entity in my case). I worked out it was because the SPL ID of the object copied was used as the index of the DeepCopy::$hashMap field holding said ArrayCollection, and when DeepCopy::copyObject was told to copy the Project entity, it found the SPL ID in the hash map and returned what was stored under that ID - which was the ArrayCollection:

I spent a day and a half trying to figure out what the problem was and still don't know what the internal logic was causing it, but adding my filters and getting the order of filters right solved the symptoms of the problem.

What I observed was likely reported in #180, but that issue's description isn't particularly informative.

[mnapoli]

Thanks for all the details and the work. To be transparent, these are quite heavy changes, I'm not sure I will have the time to dig into that enough to have the confidence to merge this 😬 It's always tricky to deal with pull requests like these, I'd rather be upfront than waiting for months before posting a response.

I think you will be better off using your fork. If there is repeated demand for this maybe we can prioritize this.

[janklan]

Fair enough, thanks for the reply.

[janklan]

If anyone is interested in the changes I have proposed here, they live in my fork: https://github.com/janklan/deepcopy

I make no promises of that package being ever updated again. I recommend you stick with this one.

[JakubKontra]

It would be nice to have it in the library.

[janklan]

@mnapoli I'm zooming back on this in my project and I must ask: if I reduced the PR to just this change, would you still be against merging it?

I'm hoping adding this one (100% optional) extension point is not "heavy".

[mnapoli]

@janklan sounds good to me! (we'd need tests though)