doctrine / mongodb-odm

The Official PHP MongoDB ORM/ODM
https://www.doctrine-project.org/projects/doctrine-mongodb-odm/en/latest/
MIT License
1.09k stars 504 forks source link

BC Break if the storeEmptyArray is not defined in the mapping #2588

Closed frenchcomp closed 11 months ago

frenchcomp commented 11 months ago

BC Break Report

The key 'store-empty-array' in a mapping about a collection is not mandatory with 2.5.5 and lower version, but it is mandatory with 2.6

Q A
BC Break yes
Version 2.6.0

Summary

We can not upgrade a code using Doctrine ODM 2.5.5 and lower with the new version 2.6, because the mapping option 'store-empty-array' added is mandatory in the CollectionPersister

Previous behavior

The option does not exist

Current behavior

PHP throw a warning for an non existent key : Warning: Undefined array key "storeEmptyArray" Maybe add a default option in doctrine-mongo-mapping.xsd and in the CollectionPersister, replace line 72 if ($mapping['storeEmptyArray']) { to if (!empty($mapping['storeEmptyArray'])) {

How to reproduce

Use a collection compliant with 2.5 but not 2.6 like

malarzm commented 11 months ago

@frenchcomp we have plenty of tests that are not using the new functionality. Have you regenerated metadata after upgrading ODM?

frenchcomp commented 11 months ago

Yes all caches, included OPCache, was cleaned and all metadata will be regenerated. I had also test with a new clone with a personnal project. I do not use PHP Attributes, only XML mapping. The attribute has the default value, but not the XML mapping

alcaeus commented 11 months ago

The attribute has the default value, but not the XML mapping

~I created a small test in AbstractMappingDriverTestCase, which checks the mapping for both referenced and embedded mappings, and it passed in all mapping drivers.~

~Please provide more information regarding the mapping that causes the error. Is it a ReferenceMany or a EmbedMany relationship? Did you confirm that metadata was freshly loaded and not loaded from cache?~

Nevermind, I already had a potential fix in place. I confirmed the issue and will have a bug fix in a moment.

frenchcomp commented 11 months ago

I had the default value into the xml but the bug is still here. It's en embedded collection.

frenchcomp commented 11 months ago

A breakpoint into public function setOwner(object $document, array $mapping) in lib/Doctrine/ODM/MongoDB/PersistentCollection/PersistentCollectionTrait.php, the $mapping array has not the key storeEmptyArray The key is not present in the classmetaddata of the parent

frenchcomp commented 11 months ago

array ( 'type' => 'many', 'embedded' => true, 'targetDocument' => NULL, 'collectionClass' => NULL, 'name' => 'authData', 'strategy' => 'pushAll', 'nullable' => false, 'discriminatorField' => 'type', 'fieldName' => 'authData', 'isCascadeRemove' => true, 'isCascadePersist' => true, 'isCascadeRefresh' => true, 'isCascadeMerge' => true, 'isCascadeDetach' => true, 'association' => 4, 'isOwningSide' => true, 'isInverseSide' => false, )

dump of $mapping in lib/Doctrine/ODM/MongoDB/Mapping/ClassMetadata.php line 2390 for the embedded collection (during the call to loadMetadataForClass after clean all caches)

malarzm commented 11 months ago

Fixed in #2590, will cut a release soon. Thanks @frenchcomp for living on the edge and finding this one! :tumbler_glass:

frenchcomp commented 11 months ago

Great job ! Thanks :)