VichUploaderBundle icon indicating copy to clipboard operation
VichUploaderBundle copied to clipboard

Fix mongodb changeset

Open IonBazan opened this issue 2 years ago • 7 comments

This PR tries to fix the entity/document listeners by:

  • Relying on Doctrine\Persistence events
  • Using getObject() from the event itself instead of using Adapter ~- Adding getChangeSet() to the adapter itself so it can be customized~

The MongoDB ODM part is a bit poorly tested as many classes are final which makes it really difficult to test.

IonBazan avatar Mar 16 '22 12:03 IonBazan

@garak I think this is now ready to go - just the PHPStan errors needs some fixing but most of it doesn't seem to be related to this PR itself.

IonBazan avatar Apr 26 '22 07:04 IonBazan

Unfortunately, this AdapterInterface change is mandatory:

  • getObjectFromArgs() was removed because we can rely on getObject() on the event itself so the method becomes obsolete
  • getChangeSet() was introduced to provide an adapter between MongoDB ODM (getDocumentChangeSet()) and ORM (getEntityChangeSet()) which is the core of the issue solved by this PR

If we really don't want to touch the interface, we could shift this logic to the listener itself but that would defeat the purpose of using the adapter, especially that the adapter is marked as @internal since v1.19.0.

I will take a look at PHPStan errors again and solve them in next commit 👍🏻

IonBazan avatar Apr 27 '22 09:04 IonBazan

@IonBazan I'm not against the interface change, but this PR should include an update to changelog

garak avatar Apr 27 '22 10:04 garak

Any news?

garak avatar Jul 21 '22 07:07 garak

@garak sorry, I think the issue that this PR was initially trying to fix has been solved in v1.19.1. I will rebase it and simplify the interface changes then.

IonBazan avatar Aug 01 '22 09:08 IonBazan

@garak I think this is now ready for a review again. Let me know if the changes still make sense after the changes from v1.19. I've also added some UPGRADING.md notes to highlight the changes in AdapterInterface.

IonBazan avatar Aug 10 '22 03:08 IonBazan

Do you mind proposing the same changes on the master branch?

garak avatar Sep 13 '22 12:09 garak